How do I stop this trigger from running more than once?

Question

I'm still fairly new to Apex.

Currently in our org we have a class called CommitmentProcess and a trigger called CommitmentTrigger. We use this to clone Opportunities that are up for renewal. So for example, imagine we have a client that signs up for a 3 year product; when that opportunity closes, this code will clone that opportunity twice and set the close dates to a year in advance (i.e. if that opp was closed today, then it would create two opps, one with the close date as 26/09/2019 and one with the close date as 26/09/2020).

The issue we're having is that sometimes the code will clone more opportunities than necessary. This happens if someone edits an opportunity that was previously closed won. My question is: How can I amend the code so that it does not fire if an opportunity has been previously closed won?

I tried to create a global class with a static variable to prevent the code from being called twice but it's not going so well. Any help would be super appreciated. My amendments to the trigger are marked with /*****START OF MY SOLUTION********/.

CommitmentProcess

    public without sharing class CommitmentProcess {



    public static void applyDiscount(List<Opportunity> LOpp)
    {
      addCommitmentDiscount(LOpp,true);
    }

    public static void cloneOpportunity(List<Opportunity> LOpp)
    {

      system.debug(LOpp);
      List<Opportunity> LOppCloneFrom = new List<Opportunity>();
      List<Opportunity> LOppCloneTo = new List<Opportunity>();
      List<OpportunityLineItem> LOppProd = new List<OpportunityLineItem>();

      set<Id> oppAccountIds = new set<Id>();


      //Add Commitment Discount to existing Opprotunity Product
      LOppProd = addCommitmentDiscount(LOpp,false);

      //Get the original opportunity
      String OppSQL = getCreatableFieldsSOQL('Opportunity','id IN :LOpp','');
      LOppCloneFrom = Database.query(OppSQL);

      // Cloning remaining renewal opportunities (for multi year) 
      for(Opportunity OppCloneFrom : LOppCloneFrom)
      {


        OppCloneFrom.Commitment_Year__c = '1';
        decimal cloneNo = decimal.valueof(OppCloneFrom.Commitment_Type_picklist__c.substring(0,1))-1;
        //Clone Opportunity according to number of extra opportunity


        Opportunity tempOpp = OppCloneFrom.clone(false,true);

        // keep for query
        oppAccountIds.add(OppCloneFrom.AccountId);
        Map<Id,Account> accountMap = new Map<Id,Account>([select Id, name from Account where Id IN: oppAccountIds]);

        // cloning the values of the renewal opportunities   
        for(integer i=0;i<cloneNo;i++)
        {
          tempOpp = tempOpp.clone(false,true);

          Datetime renewalDate = tempOpp.Commencement_Date__c.addyears(1);
          Datetime ExpirationDate = tempOpp.Commencement_Date__c.addyears(2).adddays(-1);
          String renewaldateString = renewalDate.format('dd MMM yyyy');
          String expirationDateString = ExpirationDate.format('dd MMM yyyy');

          tempOpp.name = tempOpp.name.replace(String.valueof(tempOpp.closeDate.year()),String.valueof(tempOpp.closeDate.year()+1));
          tempOpp.closeDate = tempOpp.Commencement_Date__c.addmonths(11);
          //tempOpp.name = 'Renewal';
          tempOpp.Commitment_Type_picklist__c = 
          tempOpp.Commitment_Type_picklist__c;
          tempOpp.order_type__c = 'Renewal';
          tempOpp.Probability = 90;
          tempOpp.StageName = '90% - Contracts/ Order Forms';
          tempOpp.Commitment_Year__c = String.valueof(i+2);
          tempOpp.Lead_Converted__c = false;
          tempOpp.Lead_Created_Date__c = null;
          tempOpp.New_Business__c = false;
          tempOpp.Commencement_Date__c = 
          tempOpp.Commencement_Date__c.addYears(1);
          tempOpp.Within_Existing_Initial_Renewal_Term__c = true;
          tempOpp.Initial_Terms__c = 12;
          tempOpp.Next_Payment_Number__c = 1;
          tempOpp.ForecastCategoryName = 'Commit';
          tempOpp.LeadSource = null;
          tempOpp.Lead_Source_Description__c = null;
          tempOpp.Lead_Source_Description_Text__c = null;
          tempOpp.Ready_for_Approval__c = False;
          tempOpp.Request_for_RFP__c = False;
          tempOpp.Stage_of_Completion__c = null;
          tempOpp.RFP_Opportuntity__c = False;
          tempOpp.Contract_Sale__c = False;
          tempOpp.Ready_for_Quote_Generation__c = False;
          LOppCloneTo.add(tempOpp);
        }
      }
      insert LOppCloneTo;
      update LOppCloneFrom;


      List<OpportunityLineItem> LOppPordClone = new 
      List<OpportunityLineItem>();
      for(Opportunity OppCloneTo : LOppCloneTo)
      {
        for(OpportunityLineItem OppProd: LOppProd)
        {
          OpportunityLineItem tempProd = OppProd.clone(false,true);
          tempProd.description = null;
          tempProd.opportunityid = OppCloneTo.id;
          tempProd.totalPrice = null;
          LOppPordClone.add(tempProd);
        }
      }
      insert LOppPordClone;
    }

    // cloning the products and discounts

    public static List<OpportunityLineItem> addCommitmentDiscount(List<Opportunity> LOpp, boolean applyDiscount){
      Set<id> OppSet = new set<id>();
      String SQL;
      List<OpportunityLineItem> LOppProd = new List<OpportunityLineItem>();
      List<OpportunityLineItem> LUpdatedOppProd = new List<OpportunityLineItem>();

      Map<id, decimal> OppDiscountMap = new Map<id, decimal>();


      for(Opportunity Opp: LOpp){
        OppDiscountMap.put(Opp.id,Opp.Commitment_Discount__c);
      }

      SQL = getCreatableFieldsSOQL('OpportunityLineItem','Opportunityid IN :LOpp ORDER By Opportunityid','PricebookEntry.Product2.Is_Perpetual__c,PricebookEntry.Product2.Is_S_M__c,PricebookEntry.Product2.Apply_Commitment_Discount__c');

      LOppProd = Database.query(SQL);

      if(LOppProd.size() > 0){
        id OppidCheck = LOppProd[0].OpportunityId;
        //Update existing product discount

          for (OpportunityLineItem OppProd : LOppProd){
              if(applyDiscount){
                if(OppProd.PricebookEntry.Product2.Is_Perpetual__c){
                  if(OppProd.PricebookEntry.Product2.Is_S_M__c)
                    if(OppProd.PricebookEntry.Product2.Apply_Commitment_Discount__c)
                      OppProd.discount = OppDiscountMap.get(OppProd.Opportunityid);
                }
                else{
                  if(OppProd.PricebookEntry.Product2.Apply_Commitment_Discount__c)
                    OppProd.discount = OppDiscountMap.get(OppProd.Opportunityid);
                }
              }
              if(OppidCheck == OppProd.Opportunityid){
                LUpdatedOppProd.add(OppProd);
              }
              else{
                LUpdatedOppProd.add(OppProd);
              }

          }
          update LUpdatedOppProd;

      }
      return LUpdatedOppProd;
    }


    private static string getCreatableFieldsSOQL(String objectName, String 
whereClause, String extrafield){

          String selects = '';

          if (whereClause == null || whereClause == ''){ return null; }

          // Get a map of field name and field token
          Map<String, Schema.SObjectField> fMap = Schema.getGlobalDescribe().get(objectName.toLowerCase()).getDescribe().Fields.getMap();
          list<string> selectFields = new list<string>();

          if (fMap != null){
              for (Schema.SObjectField ft : fMap.values()){ // loop through all field tokens (ft)
                  Schema.DescribeFieldResult fd = ft.getDescribe(); // describe each field (fd)
                  if (fd.isCreateable()){ // field is creatable
                      selectFields.add(fd.getName());
                  }
              }
          }

          if (!selectFields.isEmpty()){
              for (string s:selectFields){
                  selects += s + ',';
              }
              if (selects.endsWith(',')){selects = selects.substring(0,selects.lastIndexOf(','));}

          }

          if(extrafield.length() > 0)
            selects = selects + ',' + extrafield;

          return 'SELECT ' + selects + ' FROM ' + objectName + ' WHERE ' + 
whereClause;

      }
}

CommitmentTrigger

    trigger CommitmentTrigger on Opportunity (after update) {

    /*****START OF MY SOLUTION********/
    if(SingleExecution.hasAlreadyDone()) return;
    // else
        SingleExecution.setAlreadyDone();
    // Clone
    /*****END OF MY SOLUTION********/


        List<Opportunity> LOppOld = new List<Opportunity>();
        List<Opportunity> LOppNew = new List<Opportunity>();
        Opportunity OppOld = new opportunity(); 
        List<id> LOppid = new List<id>();
        List<id> LOppidDiscount = new List<id>();

        for (Opportunity OppNew: trigger.new){
            if(OppNew.Commitment_Type_picklist__c !=null && OppNew.StageName == 'Closed Won'){
                OppOld = trigger.oldMap.get(OppNew.id);
                if(OppOld.StageName != 'Closed Won' && OppOld.recordtypeid == OppNew.recordtypeid && (OppNew.Commitment_Year__c == '1' || OppOld.Commitment_Year__c == '1') ){
                    LOppid.add(OppNew.id);
                }
            }
            if(OppNew.Commitment_Type_picklist__c !=null){
                OppOld = trigger.oldMap.get(OppNew.id);
                System.debug('SECOND IF');
                if(OppOld.Commitment_Type_picklist__c == '' || OppOld.Commitment_Type_picklist__c == null ){
                    System.debug('SECOND NESTED IF');
                    LOppidDiscount.add(OppNew.id);
                    System.debug(LOppidDiscount);
                }
            }
        }

        // add commitment discount
        LOppNew = [Select id, Commitment_Year__c, Commitment_Type_picklist__c,Commitment_Discount__c from Opportunity where id IN :LOppidDiscount];
        if(LOppNew.size() > 0 )
        CommitmentProcess.applyDiscount(LOppNew);

        // clone oppty
        LOppNew = [Select id, 
        AccountID,
        CloseDate, 
        name, 
        order_type__c, 
        Probability, 
        StageName, 
        Lead_Converted__c, 
        Lead_Created_Date__c, 
        New_Business__c, 
        Commencement_Date__c, 
        Commitment_Year__c, 
        Within_Existing_Initial_Renewal_Term__c,         
        Initial_Terms__c, 
        Next_Payment_Number__c, 
        ForecastCategoryName, 
        LeadSource, 
        Lead_Source_Description__c, 
        Lead_Source_Description_Text__c, 
        Ready_for_Approval__c, 
        Commitment_Type_picklist__c,
        Commitment_Discount__c from Opportunity where id IN :LOppid];

        if(LOppNew.size() > 0 )
        CommitmentProcess.cloneOpportunity(LOppNew);


    }

My stab at a solution

    global class SingleExecution {

    private static boolean blnAlreadyDone = false;

    public static boolean hasAlreadyDone(){ 
        return blnAlreadyDone;
    }

    public static void setAlreadyDone() {
        blnAlreadyDone = true;  
    }

    public static void forceResetAlreadyDone() {
        blnAlreadyDone = false; 
    }


    }

Answer 1

I recommend you create a custom checkbox field on the opportunity to indicate that an opportunity has been cloned and have your trigger only clone opportunities where the box is unchecked / false and then have the trigger check the box when it clones the opportunities.

It's good practice to use a trigger pattern to control the flow of execution, but that won't help you in this scenario where the later update to the opportunity comes in a new execution context.

It's also a good idea to check the trigger.oldMap as Robert Ellis suggested above, but again this could fail to prevent creating duplicate opportunities if someone were to re-open the opportunity and then close it again.

If you didn't want to add a custom field to the Opportunity object, you could add logic to the trigger to query for cloned opportunities and verify their existence programatically, but that would make the trigger slower, which makes the user experience worse since end users have to wait for triggers to finish executing after they hit the "save" button in order to see their results.

Also, as a final note, I would recommend moving all of your logic out of the trigger file itself and putting it into a handler class, and even better learning and implementing a trigger pattern that follows best practices such as having only one trigger per object and preventing unwanted recursive execution (but allowing wanted recursive execution) for all triggers.

Answer 2

I see a couple of ways to approach this depending on what should happen to the two future opportunities when things change on the original.

If an opportunity is changed from closed-won, then to some other status, and then back to closed-won, do you need to update the expected close dates (or anything else) on the two future opportunities?

If so, then add a field that stores the original opportunity's Id. Then in the trigger, query for opps where that field has the Id of one of the opportunities in the trigger context. For each original opportunity, if there are future opportunity records, update them as needed. Where there aren't future opportunity records, create them.

If you don't need to update the future opps, you could just add a checkbox on Opportunity and, if it's false, then set it to True at the end of the trigger. At the beginning of the trigger, add logic that only runs the clone function if the checkbox is False.

Does that cover it?

The problem with the "hasAlreadyRun" concept (generally) is that sometimes we do want a trigger to run again. I can't imagine that being a problem in this situation, but in a hypothetical scenario, there could be code that runs on update, that creates a new record that the trigger should affect on insert (for example). By turning the trigger off after the first run, the new record wouldn't be processed.

Answer 3

It sounds like the only time you want to clone, is when the first If block in the for loop of your trigger evaluates to true. However, as it is written right now, if a different field on that 'Closed Won' Opportunity is edited, it will clone it again.

If you also check trigger.oldMap.get(OppNew.id).StageName != 'Closed Won' it will only clone the Opportunity when the Stage goes from something other than 'Closed Won' to 'Closed Won'.