I am not sure what I am doing wrong with my savepoint.  It seems no matter where I put it, the cloned opportunity still saves despite the user clicking cancel.  Is there something else that I am missing?  Should I have a custom cancel method in my VF page? 

<apex:page standardController="Opportunity" extensions="OppCloneController" 
    action="{!cloneWithItems}">  
    <apex:pageMessages />  

</apex:page>

Thanks!

Pat

public class OppCloneController {  


    private ApexPages.StandardController controller {get; set;}  

    public Opportunity opp {get;set;} 
    public List<OpportunityLineItem> items = new List<OpportunityLineItem>();
    Schema.DescribeSObjectResult R = Opportunity.SObjectType.getDescribe();
    List<Schema.RecordTypeInfo> RT = R.getRecordTypeInfos();
    

    // set the id of the record that is created -- ONLY USED BY THE TEST CLASS  

    public ID newRecordId {get;set;}  

     // initialize the controller  

    public OppCloneController(ApexPages.StandardController controller) {  

     //initialize the standard controller  

      this.controller = controller;  

    // load the current record  

         opp = (Opportunity)controller.getRecord(); 
          


     }  

    // setup the save point for rollback  
    
        Savepoint sp = Database.setSavepoint();
        
    // method called from the VF's action attribute to clone the opp 

    public PageReference cloneWithItems() {  



        Opportunity newOP;  


         try {  


          //copy the opportunity - ONLY INCLUDE THE FIELDS YOU WANT TO CLONE  

             opp = [select Id, Name, Account.Name, AccountID, StageName, CloseDate, OwnerID, Type_of_Job__c, 
             Invoice_Requirements__c, Customer_Account__c, Contact_Name__c, Customer_E_mail__c, RecordTypeID,
             Phone__c, fax__c, Pricebook2Id
             from Opportunity where id = :opp.id]; 
             
 
             newOP = opp.clone(false);  
           //check to see if the opportunity name contains DTV or DirecTV, if so apply the appropriate record type,
           //otherwise use standard record type
                          
                    
   
                      // VALUE MAPPING 

            newOp.Name = opp.Name;
            newOp.Account.Name = opp.Account.Name;     
            newOp.AccountId = opp.AccountId;
            newOp.CloseDate = opp.CloseDate;
            newOp.StageName = 'New Work Order';
            newOp.OwnerID = opp.OwnerID;
            newOp.Type_of_Job__c = opp.Type_of_Job__c;
            newOp.Invoice_Requirements__c = opp.Invoice_Requirements__c;
            newOp.Customer_Account__c = opp.Customer_Account__c;
             newOp.RecordTypeId = opp.RecordTypeId;      
            newOp.Contact_Name__c = opp.Contact_Name__c;
            newOp.Customer_E_mail__c = opp.Customer_E_mail__c;
            newOp.Phone__c = opp.Phone__c;
            newOp.fax__c = opp.fax__c;
            newOp.Pricebook2Id = opp.Pricebook2Id;
            
             If (newOp.Name.contains('DirecTV'))
            newOp.RecordTypeId = [Select Id From RecordType RT Where RT.Name='DTV New Standard' limit 1].Id;
  
            else if (newOp.Name.contains('DTV'))
            newOp.RecordTypeId = [Select Id From RecordType RT Where RT.Name='DTV New Standard' limit 1].Id;
             else 
                newOp.RecordTypeID = [Select ID From RecordType r Where r.Name='New Standard' limit 1].ID;
                
                   insert newOP;  

             // set the id of the new opp created for testing  

              newRecordId = newOP.id;  
 



  // copy over the line items - ONLY INCLUDE THE FIELDS YOU WANT TO CLONE 
               
               for(OpportunityLineItem oli : [Select ID, OpportunityID,  
               UnitPrice, Quantity, PricebookEntryID
               
               from OpportunityLineItem oliList where OpportunityID = :opp.id]
               )
           

               {
               OpportunityLineItem newOli = new OpportunityLineItem();
                   newOli.PricebookEntryID = oli.PricebookEntryID;
                   newOli.Quantity = oli.Quantity;
                   newOli.UnitPrice = oli.UnitPrice;
                   newOlI.OpportunityID=newOP.id;
              
                   items.add(newOli);
                   } //end loop
                   
                   insert items;
           
         } catch (Exception e){  

              // roll everything back in case of error  

             Database.rollback(sp);  
             ApexPages.addMessages(e);
             opp = opp.clone(false);
  
             return null;  

         }  
         
    

         return new PageReference('/'+newOp.id+'/e?retURL=%2F'+newOp.id);  

     }  

    

 }