Take the 2-minute tour ×
Salesforce Stack Exchange is a question and answer site for Salesforce administrators, implementation experts, developers and anybody in-between. It's 100% free, no registration required.

Hi this is my trigger for Rollup summary. No problem it is working how i wanted it to be but the problem is when i change the parent name it is not updating the field value in new parent record Kindly tell me where exactly i need to do changes.

Here Region_Target__c is a parent object and Salesorder is a child object. Q1_Achived__c is the field i'm trying to update using roll up trigger, When i change parent name in the new parent record Q1_Achived__c field is not updating automatically.

trigger target_calculation12 on Sales_Order__c (before insert, after insert, before update, after update, after delete, after undelete) {
set<id> accid=new set<id>();
list<Region_Target__c> Acnt = new list<Region_Target__c>();

Sales_Order__c[] enq;



if(Trigger.isdelete || Trigger.isupdate )
     { 
         enq=Trigger.old; 
     }
 else
     {
       enq=Trigger.new;
     }

for(Sales_Order__c opp:enq)
{
accid.add(opp.Region_Target__c);
}


for(Region_Target__c acc:[SELECT Id,Q1_Achived__c FROM Region_Target__c where id=:accid])
{
acc.Q1_Achived__c=0;

for(Sales_Order__c opp:[SELECT Region_Target__c,Amount__c,Stage__c,Id,quarter__c FROM Sales_Order__c where Region_Target__c = :accid and stage__c = 'Won' and quarter__c = 2 ])
{

acc.Q1_Achived__c=acc.Q1_Achived__c+opp.Amount__c;

}
update acc;

}
}

Thank you very much in advance.

share|improve this question
add comment

2 Answers 2

The main issue that you're facing is the SOQL statement inside of your for loop, which is a sure fire way to run into limits in triggers.

Beyond that though, I can't quite understand what you're trying to accomplish. YTD_Booking__c is always getting reset to 0 and then reset to the latest opp amount. It's not actually summing the opps at all, but each time you go through the loop, you are setting the field to the latest opp amount. What you really want here can be accomplished without apex, as far as I can tell from your requirements.

You could create a rollup summary field that sums the amount of 'won' opportunities related to an account.

If you still feel that you need to do this in apex, you should consider an aggregate query.

AggregateResult[] groupedResults = [SELECT AccountId, SUM(Amount) bookingTotal
    FROM Opportunity 
    WHERE Stage__c = 'won'
    GROUP BY AccountId];
for (AggregateResult ar : groupedResults)  {
    System.debug('Account Id' + ar.get('AccountId'));
    System.debug('Total Bookings' + ar.get('bookingTotal'));
}

There are also some other curiosities worth calling out. Why are you using a custom stage field on opportunity, where there is a native stage field (StageName). Also, You don't need to create a set of Ids from the trigger set, your Query WHERE clause can use trigger.new or trigger.old but you need to use the IN clause instead of equals:

for(Account acc:[SELECT Id,YTD_Booking__c FROM Account where id IN : enq])
share|improve this answer
    
No it's not resetting to 0 everytime i'm using it only when there are no opportunity which are closed won..!!! I can not use standard roll up since it is in look up relationship, YTD_Booking__c should be the addition of all opportunity which are closed won. –  Sharat Mar 28 at 17:56
1  
I see how you are building the amount sum now, my mistake. However, standard rollup summary fields are available from Opportunity to Account. Although the AccountId field is not a typical master-detail, it is a special field, and rollup summaries are allowed through that relationship without code. –  greenstork Mar 28 at 18:33
add comment

Was writing this when greenstork posted his original answer. His last comment about rollup summary fields being available looks like by far the best way to go. (Though the original question used Opportunity__c not Opportunity?) But in case this helps anyone here it is...

If the rollup field was not available (because of the relationship type or the 10 available have already been used) then code like this could be used. It uses one SOQL statement only: it leaves the aggregation to the database; because it doesn't care about the starting value of the Account.YTD_Booking__c field it doesn't need to query Account. And it does one update only.

// Make sure all Accounts will be updated
Map<Id, Account> accounts = new Map<Id, Account>();
for (Id id : accIds) {
    accounts.put(id, new Account(Id = id, YTD_Booking__c = 0));
}
// Sum the won amounts
for (AggregateResult ar : [
        select Account__c aid, sum(Amount__c) total
        from Opportunity__c
        where Stage__c = 'won'
        and Account__c in :accIds
        group by Account__c
        ]) {
    accounts.get((Id) ar.get('aid')).YTD_Booking__c = (Decimal) ar.get('total');
}
update accounts.values();
share|improve this answer
    
Does it work fine if i do re-parenting for opportunity ? I mean does it update the new parents record field ? –  Sharat Apr 11 at 6:42
    
@Sharat If you change the parent Account? That would depend on the pattern of the code before this i.e. which trigger events are responded to and which collections are used. If that is right it should work: note the important first loop in my code to get back to zero values after a delete. –  Keith C Apr 11 at 7:39
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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