0

I have the trigger class below and I would like it to fire only when an Opportunity Stage is changed to Closed Won. Can anyone help me figure out how to do this? I have an if statement, but it fires any time an Opportunity that is Closed Won is edited, and when I tried to add another condition it gives me the error message:

Error: Compile Error: Field expression not allowed for generic SObject at line 12 column 85.

I only want it to fire when the Stage is initially changed. Thanks.

Trigger:

trigger MainTriggerOpportunity on Opportunity (after update) {
    ClassRenewalOppClone updater13 = new ClassRenewalOppClone();
    updater13.cloneOpp(Trigger.new);
}

Trigger Class:

public class ClassRenewalOppClone {

    public void cloneOpp(List<Opportunity> cloneOpp) {

        String recordTypeName = 'Renewals';
        Map<String, Schema.RecordTypeInfo> rtMapByName = Schema.SObjectType.Opportunity.getRecordTypeInfosByName();
        Schema.RecordTypeInfo rtInfo =  rtMapByName.get(recordTypeName);
        id recType = rtInfo.getRecordTypeId();

        FOR(Opportunity opp1 : cloneOpp) {
            IF(opp1.StageName.contains('Closed Won') && trigger.OldMap.get(opp1.Id).isclosed == false && opp1.RecordTypeId == recType) {

                String OppId = opp1.Id;

                //Clone the Opportunity that is associated with the handoff and all createable fields
                /* query Opportunity and then clone it */
                String soql = RecClone.getCreatableFieldsSOQL('Opportunity', 'Id =: OppId');
                Opportunity opp = (Opportunity)Database.query(soql);
                Opportunity opp2 = opp.clone(false, true);
                insert opp2;

                List<OpportunityLineItem> itemList = (List<OpportunityLineItem>)Database.query(RecClone.getCreatableFieldsSOQL('OpportunityLineItem', 'OpportunityId =: OppId'));

                List<OpportunityLineItem> newItemList = new List<OpportunityLineItem>();

                for (OpportunityLineItem item : itemList) {
                    OpportunityLineItem ol = item.clone(false, true);
                    ol.totalprice = null;
                    ol.opportunityid = opp2.id;
                    newItemList.add(ol);
                }
                insert newItemList;
            }
        }
    }
}

2 Answers 2

1

You need to adjust your IF() statement to something like this

If(opp1.isWon && !oldMap.get(opp1.Id).isWon){
    //code
}

Also need to ensure you have trigger.oldMap

trigger

trigger MainTriggerOpportunity on Opportunity (after update) {
    ClassRenewalOppClone updater13 = new ClassRenewalOppClone();
    updater13.cloneOpp(Trigger.new, trigger.oldMap);
}

Utility Class

public void cloneOpp(List<Opportunity> cloneOpp, map<Id,Opportunity> oldMap) {
    //code
}

You have a few issues with the trigger that you may also that are not best practice. You have SOQL within a For loop as well as DML statements within a For loop, so you are likely to run into some governor issues very soon.

I would try to utilize collections (Lists, Sets, and Maps) to move your SOQL and DML outside of the for loop.

See below for some best practices

https://developer.salesforce.com/page/Apex_Code_Best_Practices

https://developer.salesforce.com/page/Trigger_Frameworks_and_Apex_Trigger_Best_Practices

General trigger bulkification - best practices

http://blog.jeffdouglas.com/2011/01/06/fun-with-salesforce-collections/

5
  • Thanks, but even with that change to my class I still receive the same error message. Would removing the SOQL and DML statements from the FOR loop require massive changes to the class?
    – John N.
    Commented Feb 19, 2015 at 20:10
  • Ohhhhh. I just realized we dont have trigger.oldMap in this utility class. Adjust your trigger to pass in trigger.oldMap. See above Commented Feb 19, 2015 at 20:46
  • Ah, that did it. Now I'll work on bulkifying it. Thanks so much!
    – John N.
    Commented Feb 19, 2015 at 21:04
  • No worries. Glad to help Commented Feb 19, 2015 at 21:05
  • Be careful with the !oldMap.get(opp1.Id).isWon part of the trigger. This trigger only fires on after update so it's currently safe, but if the trigger is later changed to after insert as well, that statement will cause an exception (attempt to de-reference a null object) when a record is inserted in a 'won' state. When dealing with inserted records as well, I usually add a check on the oldMap to cover this: oldMap != null && oldMap.containsKey(opp1.Id) && !oldMap.get(opp1.Id).isWon
    – Nick C
    Commented Feb 19, 2015 at 21:35
2

You will just need to update your if statement to check the value of the 'old stage' and then the value of the 'new stage', should be something like this:

if (opp.StageName == 'Closed Won' &&
    opp.StageName != Trigger.oldMap.get(opp.Id).StageName)
1
  • He also has DML operations inside for loop and queries the database. Commented Feb 19, 2015 at 19:48

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .