Fire Trigger Only When Stages Moves to Closed Won

Question

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;
            }
        }
    }
}

Answer 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/

Answer 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)