function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
George AdamsGeorge Adams 

Trigger not updating fields on Opp

Hi all,

I'm using a trigger to count the number of opps on an account, then label the current opp (as it's being inserted) either "Old Business" or "New Business". I know that the count oppCount_var does hold the correct number of opps for the current Account. The problem is when I go to update the field on the opp, nothing happens.
  
trigger newVsOldBusiness on Opportunity (before insert) {

    //create set to hold the current opp's associated Account ID
    Set<Id> CurrentOppAccountID = new Set<Id>();
    
    for (Opportunity o: Trigger.new) {
    	CurrentOppAccountID.add(o.AccountId);
    }
    
    integer oppCount;
    integer oppCount_var;
    
    //count all opps on Account object associated with current opp's Account ID
    AggregateResult[] AccountsByOppCount = [SELECT count(Id)oppCount
                                       				FROM Opportunity
                                       				WHERE AccountId in :CurrentOppAccountID group by AccountId
                                            		LIMIT 1];
    
    //loop over the aggregate result set
	for (AggregateResult ar : AccountsByOppCount) { 
		oppCount_var = integer.valueOf(ar.get('oppCount'));
	}

    System.debug('oppCount_var at line 18: ' + oppCount_var);
    
    //search for Type_of_Business__c on Opp    
    List<Opportunity> OppToUpdate = [SELECT Type_of_Business__c
                                                       FROM Opportunity
                                      				   WHERE AccountId in :CurrentOppAccountID
                                       				   LIMIT 1];
    
    if (oppCount_var>1) {
        for (Opportunity oppot: OppToUpdate) {
            oppot.Type_of_Business__c = 'Old';
            oppot.Accessory_Insert_Sent__c = true;
            System.debug('IN LOOP 1 BEFORE UPDATE');
            update OppToUpdate;
            System.debug('IN LOOP 1 AFTER UPDATE');
        } 
    } else {
        for (Opportunity oppot: OppToUpdate) {
            oppot.Type_of_Business__c = 'New';
            update OppToUpdate;
            System.debug('IN LOOP 2');
        }
    }

        
}

Here's the relevant debug for it:
  
14:13:13.18 (194613807)|USER_DEBUG|[45]|DEBUG|IN LOOP 1 BEFORE UPDATE
14:13:13.18 (194621079)|STATEMENT_EXECUTE|[46]
14:13:13.18 (194688155)|DML_BEGIN|[46]|Op:Update|Type:Opportunity|Rows:1
14:13:13.18 (194721316)|HEAP_ALLOCATE|[EXTERNAL]|Bytes:8
14:13:13.221 (221153364)|HEAP_ALLOCATE|[EXTERNAL]|Bytes:8
14:13:13.221 (221201260)|HEAP_ALLOCATE|[EXTERNAL]|Bytes:8
14:13:13.221 (221233789)|ENTERING_MANAGED_PKG|npe01
14:13:13.221 (221267697)|HEAP_ALLOCATE|[EXTERNAL]|Bytes:8
14:13:13.221 (221281371)|VARIABLE_SCOPE_BEGIN|[30]|this|npe01.OpportunityPayments|true|false
14:13:13.221 (221348248)|VARIABLE_ASSIGNMENT|[30]|this|{}|0x51733c3c
14:13:13.221 (224853834)|SOQL_EXECUTE_BEGIN|[107]|Aggregations:1|select id, CloseDate, Amount, isClosed, isWon, payments_made__c, (SELECT id, Paid__c, Payment_Amount__c, Payment_Date__c, scheduled_date__c from Opportunity.OppPayment__r) from Opportunity WHERE id in :newOpps
14:13:13.221 (233545826)|SOQL_EXECUTE_END|[107]|Rows:1
14:13:13.234 (234287513)|CUMULATIVE_LIMIT_USAGE
14:13:13.234 (234287513)|CUMULATIVE_LIMIT_USAGE_END

14:13:13.18 (260858150)|DML_END|[46]
14:13:13.18 (260931890)|STATEMENT_EXECUTE|[47]
14:13:13.18 (260946548)|HEAP_ALLOCATE|[47]|Bytes:22
14:13:13.18 (261028354)|USER_DEBUG|[47]|DEBUG|IN LOOP 1 AFTER UPDATE

Anyone know why this loop isn't updating the current opp as it's being inserted?

Thanks!
Best Answer chosen by George Adams
pconpcon
That is my fault.  I forgot you still have to use the get method
  
trigger newVsOldBusiness on Opportunity (before insert) {
    Set<Id> accountIds = new Set<Id>();

    for (Opportunity o : Trigger.new) {
        accoutnIds.add(o.AccountId);
    }

    List<AggregateResult> oppCount = [
        select count(Id) oppCount,
            AccountId
        from Opportunity
        where AccountId in :accountIds
        group by AccountId
    ];

    Map<Id, Integer> accountMap = new Map<Id, Integer>();

    for (AggregateResult ar : oppCount) {
        accountMap.put((Id) ar.get('AccountId'), Integer.valueOf(ar.get('oppCount'));
    }

    for (Opportunity o : Trigger.new) {
        if (
            accountMap.containsKey(o.AccountId) &&
            accountMap.get(o.AccountId) > 0
        ) {
            o.Type_of_Business__c = 'Old';
            o.Accessory_Insert_Sent__c = true;
        } else {
            o.Type_of_Business__c = 'New';
        }
    }
}

 

All Answers

pconpcon
You've got a couple of problems with your code.
  1. Not bulk ready
  2. Doing updates in a before trigger
  3. Querying when you don't need to
I've taken the liberty of updating your code
  
trigger newVsOldBusiness on Opportunity (before insert) {
    Set<Id> accountIds = new Set<Id>();

    for (Opportunity o : Trigger.new) {
        accoutnIds.add(o.AccountId);
    }

    List<AggregateResult> oppCount = [
        select count(Id) oppCount,
            AccountId
        from Opportunity
        where AccountId in :accountIds
        group by AccountId
    ];

    Map<Id, Integer> accountMap = new Map<Id, Integer>();

    for (AggregateResult ar : oppCount) {
        accountMap.put(ar.AccountId, Integer.valueOf(ar.get('oppCount'));
    }

    for (Opportunity o : Trigger.new) {
        if (
            accountMap.containsKey(o.AccountId) &&
            accountMap.get(o.AccountId) > 0
        ) {
            o.Type_of_Business__c = 'Old';
            o.Accessory_Insert_Sent__c = true;
        } else {
            o.Type_of_Business__c = 'New';
        }
    }
}
NOTE: This code has not been tested and may contain typographical or logical errors

Based on what you are trying to do, this should cover all three points mentioned above.  What we are doing is
  • Line 2-6: Building up a set of all account ids
  • Line 8-14: Getting a count of existing opportunities for those accounts. This does not include the current Opportunity since we are in a before trigger
  • Line 16-20: Build a map of AccountId to the number of opportunities
  • Line 22: Iterate over each of the new triggers
  • Line 23-28: If we have a count for that AccountId and it is greater than 0 (ie we have an existing opportunity) then set Type_Of_Business__c to 'Old' and set Accessory_Insert_Sent__c to true
  • Line 29-31: Otherwise set the Type_of_Business__c to false.
George AdamsGeorge Adams
Thanks for the reply and explaination. Very much appreciated.

With your code, I'm getting the error "Invalid field AccountId for SObject AggregateResult" on line 18. This is confusing because it looks like your code does select the key AccountId in line 10, so that field should exist in oppCount.

Do you know what the issue is?
pconpcon
That is my fault.  I forgot you still have to use the get method
  
trigger newVsOldBusiness on Opportunity (before insert) {
    Set<Id> accountIds = new Set<Id>();

    for (Opportunity o : Trigger.new) {
        accoutnIds.add(o.AccountId);
    }

    List<AggregateResult> oppCount = [
        select count(Id) oppCount,
            AccountId
        from Opportunity
        where AccountId in :accountIds
        group by AccountId
    ];

    Map<Id, Integer> accountMap = new Map<Id, Integer>();

    for (AggregateResult ar : oppCount) {
        accountMap.put((Id) ar.get('AccountId'), Integer.valueOf(ar.get('oppCount'));
    }

    for (Opportunity o : Trigger.new) {
        if (
            accountMap.containsKey(o.AccountId) &&
            accountMap.get(o.AccountId) > 0
        ) {
            o.Type_of_Business__c = 'Old';
            o.Accessory_Insert_Sent__c = true;
        } else {
            o.Type_of_Business__c = 'New';
        }
    }
}

 
This was selected as the best answer
George AdamsGeorge Adams
Works great. Thanks again.