1

My org uses custom revenue schedule objects created via triggers and classes running off of the Opportunities. It is very complex (beyond my scope for updating in any reasonable time) - it uses a delete and re insert every time the revenue calculation is updated. We are implementing a rebate process (percentage off) for certain Ops and I am implementing the rebate process like this:

  1. Rebate_schedule__c object with dates and the rebate % so rebates can be added for appropriate months for each Op
  2. Trigger on the custom revenue schedule objects to call a class to apply the discount.

Below are my trigger and class. I can see that it does not look efficient - while only Revenue_schedule__c on Ops that have rebates will be called b/c of the trigger, the query is in the FOR loop. I do not know how to do this in a way robust to bulk updates, since every Revenue_schedule__c needs to check for an existing Rebate_schedule__c on that Op for that same month.

How do I make the class bulk capable by moving the query out of the for loop while still finding the correct rebate %, matched by OpId and Month (if it even exists)

Thanks!

Trigger:

trigger CallOpportunityRebateApplicationFrmRevSched on Revenue_Schedule__c (after insert) {

    set <id> RevsToHandle = new SET <id>();
    List <Revenue_Schedule__c> RevScheds = [select id, Opportunity__r.HasRebateSchedule__c
                      from Revenue_Schedule__c
                      where Opportunity__r.HasRebateSchedule__c > 0
                      AND id IN: Trigger.new];


    For (Revenue_schedule__c RR:RevScheds){
        RevsToHandle.add(RR.id);
    }

    if(RevsToHandle!=NULL){
        OpportunityRebateHandlerforRevSched.ApplyRebateMethod(RevsToHandle);
    }

}

Class (Nversion of class - still catching ALL MONTHS are being cut and not just matched by month of Rev_schedule to month of rebate_schedule)

3
  • 1
    What exactly is your question? Please edit your post to make clear your specific, answerable question. May 23, 2017 at 20:38
  • Updated with explicit question
    – jaw999
    May 23, 2017 at 20:55
  • I am for now changing the model to work where the rebate schedule has one month instead of a date range. This way I could figure out how to bulkify using MAP methods b/c I could use a formula field on all objects concatenating OpportunityId and Month to make an easy key. If anyone can figure out how to work this where the rebate object has a date range - a start and end date rather than a date value, I would like to see how you would do it.
    – jaw999
    Jun 8, 2017 at 19:27

3 Answers 3

4
+50

I have tried to refactor your code eliminating extra SOQL queries and using Map.

Trigger

Since you are querying Revenue_Schedule__c records so better to include Amount__c, Month__c, Opportunity__c and put results into a map and pass that into Handler method.

trigger CallOpportunityRebateApplicationFrmRevSched on Revenue_Schedule__c (after insert) 
{
    Map<Id,Revenue_Schedule__c> RevSchedMap = 
                      new Map<Id,Revenue_Schedule__c>(
                      [SELECT id, Amount__c, Month__c, Opportunity__c, Opportunity__r.HasRebateSchedule__c
                      FROM Revenue_Schedule__c
                      WHERE Opportunity__r.HasRebateSchedule__c > 0
                      AND id IN: Trigger.new]
                      ); 

    if(RevSchedMap.keySet().size()>0){
        OpportunityRebateHandlerforRevSched.ApplyRebateMethod(RevSchedMap);
    }
}

Trigger Handler

Given necessary inline comments

public class OpportunityRebateHandlerforRevSched 
{
    static public void ApplyRebateMethod (Map<Id,Revenue_Schedule__c> RevsToHandle)
    {
        Set<Date> monthValues = new Set<Date>(); //this will hold month dates
        Map<Id, Id> revsFedOpptyMap = new Map<Id, Id>(); //holds Revenue_Schedule__c Id and OpportunityId

        //prepare list of month dates and revsFedOpptyMap
        for(Revenue_Schedule__c rsObj: RevsToHandle.values())
        {
            monthValues.add(rsObj.Month__c);
            revsFedOpptyMap.put(rsObj.Id, rsObj.Opportunity__c);
        }

        List <Revenue_Schedule__c> RSStoCUT = new List <Revenue_Schedule__c>();

        //retrieve MaxRebate based on OpportunityIds
        List<AggregateResult> results = [SELECT Opportunity__c, MAX (Rebate_factor__c) MaxRebate    
                                            FROM Rebate_schedule__c 
                                            WHERE Opportunity__c =: revsFedOpptyMap.values() 
                                            AND Rebate_Start_Date__c <=: monthValues
                                            AND Rebate_End_Date__c >=: monthValues
                                            GROUP BY Opportunity__c
                                   ]; 

        //prepare a map which will hold OpportunityId and MaxRebate
        Map<Id, Decimal> opptyRebateMap = new Map<Id, Decimal>();
        for(AggregateResult agg : results)
        {
            opptyRebateMap.put((Id)agg.get('Opportunity__c'),(Decimal) agg.get('MaxRebate'));
        }

        //now compare with Opportunity values and prepare a list of records to be updated   
        for(Revenue_Schedule__c rsObj: RevsToHandle.values())
        {
            if(opptyRebateMap.containsKey(rsObj.Opportunity__c))
            {
                Decimal RebateFactor = opptyRebateMap.get(rsObj.Opportunity__c);
                if(RebateFactor !=null)
                {
                    rsObj.Amount__c = (rsObj.Amount__c-(rsObj.Amount__c*RebateFactor));
                    RSStoCUT.add(rsObj);
                }
            }
        }
        update RSStoCUT;
    }
}
16
  • thanks - there are a couple variables it is not finding - is "revsFed" supposed to be "RevsToHandle" ?
    – jaw999
    May 30, 2017 at 13:53
  • and needed to switch for(AggregateResult agg : aggResult) to for(AggregateResult agg : results) - just checking, thanks
    – jaw999
    May 30, 2017 at 13:57
  • Odd error - i am getting Unexpected Token for the { in the line - if(opptyRebateMap.containsKey(rsObj.Opportunity__c) {
    – jaw999
    May 30, 2017 at 17:25
  • I have written a logic in textpad, since I dont have those objects I am not able to compile. But the logic I have tried to show, it will work May 30, 2017 at 17:31
  • gotcha, thanks. Made one update, RSStoCUT.add(RSSA); became RSStoCUT.add(rsObj);
    – jaw999
    May 30, 2017 at 17:44
0

This is rough, but I hope you can get the jist of what I'm doing.. There may be compile errors because I eyeballed it but you should be figure out the direction that I was going

But essentally my goal here was to try and figure out all the data that is required and then using Maps retrieve the data

public class OpportunityRebateHandlerforRevSched 
{
    static public void ApplyRebateMethod (Set<ID> revsToHandle)
    {
        Map<ID, Revenue_Schedule__c> revOpptys = new Map<ID, Revenue_Schedule__c>();

        for(Revenue_Schedule__c rev : [SELECT id, Amount__c, Month__c, Opportunity__c FROM Revenue_Schedule__c WHERE ID IN: revsToHandle]])
        {
            revOpptys.put(rev.Opportunity__c, rev);
        }

        List<AggregateResult> aggResult = [select ID, Opportunity__c, Rebate_Start_Date__c, Rebate_End_Date__c,  MAX (Rebate_factor__c) MaxRebate FROM 
        Rebate_schedule__c where Opportunity__c =: revOpptyIDS.keySet() GROUP BY Opportunity__c];

        for(ID rssaID : revOpptys.keySet())
        {
            for(AggregateResult agg : aggResult)
            {
                if(aggResult.get('Opportunity__c') == rssaID && agg.Rebate_Start_Date__c <= revOpptys.get('Opportunity__c').Month__c && 
                    agg.Rebate_End_Date__c >=: revOpptys.get('Opportunity__c').Month__c)
                {
                    revOpptys.get(rssaID).Amount__c = (revOpptys.get(rssaID).Amount__c - (revOpptys.get(rssaID).Amount__c * (Decimal) agg.get('MaxRebate');   
                }
            }
        }

        update revOpptys.values();
    }
}
3
  • thanks! I will try to work on this. Maps are my weakest skill in apex.
    – jaw999
    May 24, 2017 at 18:40
  • I think I see where going - some compile issues - but I think the concept - I will post my trial above
    – jaw999
    May 26, 2017 at 19:20
  • Not quite sure how MAP is mapping the Revenue_schedule__c to Rebate Schedule since it must match OPPORTUNITY and the MONTH
    – jaw999
    May 26, 2017 at 19:39
-1

I am for now changing the model to work where the rebate schedule has one month instead of a date range. This way I could figure out how to bulkify using MAP methods b/c I could use a formula field on all objects concatenating OpportunityId and Month to make an easy key.

If anyone can figure out how to work this where the rebate object has a date range - a start and end date rather than a date value, I would like to see how you would do it.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .