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 MainTriggerOpportunity on Opportunity (after update) {
    ClassRenewalOppClone updater13 = new ClassRenewalOppClone();

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;
                insert newItemList;

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

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

Also need to ensure you have trigger.oldMap


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

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



General trigger bulkification - best practices


  • 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. Feb 19 '15 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 – Chris Duncombe Feb 19 '15 at 20:46
  • Ah, that did it. Now I'll work on bulkifying it. Thanks so much! – John N. Feb 19 '15 at 21:04
  • No worries. Glad to help – Chris Duncombe Feb 19 '15 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 Cook Feb 19 '15 at 21:35

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)
  • He also has DML operations inside for loop and queries the database. – Mihai Neagoe Feb 19 '15 at 19:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.