1

I'm very new to triggers.

The following trigger updates a custom field in a custom object (Events__c) with the sum of all "TotalPrice" from a related object (OpportunityLineItem).

I'm getting the following error:

Error:Apex trigger OppTrigger caused an unexpected exception, contact your administrator: OppTrigger: execution of AfterUpdate caused by: System.DmlException: Update failed. First exception on row 0; first error: MISSING_ARGUMENT, Id not specified in an update call: []: Class.OpportunityHandler.aggregateResults: line 73, column 1

Here is the Trigger:

trigger OppTrigger on OpportunityLineItem (after delete,after update,
after insert, after undelete) {
    if(trigger.isInsert)
    {
        OpportunityHandler oppHandler =new OpportunityHandler();
        oppHandler.oppInsert(Trigger.New); 
    }
    else if(trigger.isDelete)
    {
        OpportunityHandler oppHandler =new OpportunityHandler();
        oppHandler.oppDelete(Trigger.Old);
    }
    else if(trigger.isUnDelete)
    {
        OpportunityHandler oppHandler =new OpportunityHandler();
        oppHandler.oppundelete(Trigger.New);
    }
    else if(trigger.isUpdate)
    {
        OpportunityHandler oppHandler =new OpportunityHandler();
        oppHandler.oppUpdate(Trigger.New,Trigger.Oldmap);
    }

}

Here is the Class:

public class OpportunityHandler
{
    list<Events__c> oppList = new list<Events__c>();
    set<id> oppIDs = new set<id>();


    public void oppInsert(List<OpportunityLineItem> oppLists)
    {
        for(OpportunityLineItem opp : oppLists)
        {
            if(opp.Events__c!=null)
            {
                oppIDs.add(opp.Events__c);
            }
        }
        aggregateResults();
    }

    public void oppUpdate(List<OpportunityLineItem> oppLists,Map<Id,OpportunityLineItem> oppOldmap)
    {
        for(OpportunityLineItem opp : oppLists)
        {
            if(opp.Events__c!=null)
            {
                oppIDs.add(opp.Events__c);
                if(oppOldmap.get(opp.id).Events__c!=opp.Events__c)
                {
                    oppIDs.add(oppOldmap.get(opp.id).Events__c);
                }
            }

        }
        aggregateResults();

    }

    public void oppDelete(List<OpportunityLineItem> oppLists)
    {
        for(OpportunityLineItem opp : oppLists)
        {
            if(opp.Events__c!=null)
            {
                oppIDs.add(opp.Events__c);
            }
        }
        aggregateResults();
    }
    public void oppundelete(List<OpportunityLineItem> oppLists)
    {
        for(OpportunityLineItem opp : oppLists)
        {
            if(opp.Events__c!=null)
            {
                oppIDs.add(opp.Events__c);
            }
        }
        aggregateResults();
    }

    public void aggregateResults()
    {
        AggregateResult[] groupedResults = [SELECT SUM(TotalPrice), Events__c FROM OpportunityLineItem where Events__c IN :oppIDs GROUP BY Events__c];
        for(AggregateResult ar:groupedResults) 
        {
            Id oppId = (ID)ar.get('Events__c');
            Double sum = (DOUBLE)ar.get('expr0');
            Events__c opp1 = new Events__c(Id=oppId);
            opp1.Confirmed__c= sum;
            oppList.add(opp1);
        }
            if(oppList.size()>0 && oppList!=null)
            {
                update oppList;
            }
    }
}
  • Pls try upserting opplist, instead of plain update. – Ysr Shk Oct 21 '19 at 11:31
  • I don't think I understand. What do you mean, @YsrShk? Thank you – Sharky Oct 21 '19 at 12:25
1

Your code looks mostly fine to me. I think the only way you could get this error is if you somehow had a null in your oppIds set.

I do see that you're doing null checks before adding an id into your set, but there's one line where you're not doing a null check.

In the following code:

public void oppUpdate(List<OpportunityLineItem> oppLists,Map<Id,OpportunityLineItem> oppOldmap)
{
    for(OpportunityLineItem opp : oppLists)
    {
        if(opp.Events__c!=null)
        {
            // opp.Events__c is null checked
            oppIDs.add(opp.Events__c);
            if(oppOldmap.get(opp.id).Events__c!=opp.Events__c)
            {
                // but oppOldMap's Events__c is _not_ null checked
                oppIDs.add(oppOldmap.get(opp.id).Events__c);
            }
        }

    }
    aggregateResults();

}

The easy way around this is to take advantage of the remove() method of the Set class. Instead of checking for nulls in a loop, just add everything, and then remove the null (if one is in the set).

public void oppUpdate(List<OpportunityLineItem> oppLists,Map<Id,OpportunityLineItem> oppOldmap)
{
    for(OpportunityLineItem opp : oppLists)
    {
        oppIDs.add(opp.Events__c);
        oppIDs.add(oppOldmap.get(opp.id).Events__c);
    }

    // We can just remove nulls (well, max of 1 null since it's a set) after the loop
    // This is marginally faster than evaluating IFs in a loop, but this change isn't work it
    //   for that speedup.
    // The real gain here is the fewer lines of code and increased readability/reliability
    oppIDs.remove(null);

    aggregateResults();
}

You can apply the same pattern to your other methods too.

  • Thank you, @DerekF – Sharky Oct 21 '19 at 13:24

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.