ShowAll Questionssorted byDate Posted
Austin_Steve

# MRR Calculation question

I'm trying to calculate the Monthly Recurring revenue, and place it on a field on the Opportunity screen.  I am running into a number of errors trying to do it my way.  I was wondering if anyone can help out with this calculation. Here is what I have so far.

trigger CalcMRR on Opportunity (after update) {
list<OpportunityLineItem> opps = new List<OpportunityLineItem>();
list<Opportunity> Oppts = new List<Opportunity>();
Set<String> OppSet= new Set<String>();

for(Opportunity Opp : Trigger.new) {
}

Oppts = [Select id, Month__c, Contract_Months__c  from Opportunity where ID in :OppSet limit 1];
Opps =  [Select Id, OpportunityId, PricebookEntryId, Quantity,
TotalPrice, UnitPrice, ListPrice, ServiceDate, Description,
PricebookEntry.Name, PricebookEntry.ProductCode, PricebookEntry.Product2id
From OpportunityLineItem
Where PricebookEntry.ProductCode = 'Software'
And opportunitylineitem.opportunityid IN :Oppset];

for(Opportunity Opp: Trigger.new){
for (OpportunityLineItem olist: Opps){
if (Opp.Month__c!=null)
Opp.Month__c = Opp.Month__c + olist.TotalPrice/Opp.Contract_Months__c;
else
Opp.Month__c = olist.TotalPrice/Opp.Contract_Months__c;
}
}
if (Oppts.size() >0){
update Oppts;
}
}

Austin Steve

SteveBower

Hi Steve, a few points in no particular order.

1. Just in general, wrap code in Try/Except blocks to catch errors.

2. The Oppts = query has a limit of 1 record returned.  I'm not sure what you're trying to do there.  Since the trigger has been called, there is at least one record there.  So, Oppts.size() used down below will always be > 0.   Given that, it's not clear the whole OppSet construct is useful.

3. To follow that thought:  For the Opps = query, instead of using "in :Oppset", you could use "in :Trigger.newMap.keyset()"since it's an "in" statement, having duplicates in the search range won't matter.  Sometimes it's a good idea to remove duplicates yourself, but since you're not iterating over the set anywhere, it doesn't matter in this case.

4. Lastly, it looks like your nested loops are taking every OpportunityLineItem that's relevent to all the Opportunities in the Trigger, and adding them all to every Opportunity.

5. Your query is pulling in all sorts of fields you don't need for your trigger.

6. Why do this as an "after" update trigger instead of "before"?  You're going to want to update the Opportunities with the new totals, might as well just modify the data in the Trigger and let it proceed with it's normal update.

7. Comments.  Just good practice, and you may find logic errors when you "explain" the code to yourself while writing them.

So, I think you want something simpler as in (leaving out the error handling):

trigger CalcMRR on Opportunity (before update) { // Go through all the Line Items for all the Opportunities in the Trigger invocation for (OpportunityLineItem oli: [Select OpportunityId, TotalPrice From OpportunityLineItem Where PricebookEntry.ProductCode = 'Software' And opportunityid IN :Trigger.newmap.keyset()]) { // For each OLI, compute the monthly revenue and add it to the total for that // Opportunity. Trigger.newMap.get(oli.opportunityId).Month__c += (oli.TotalPrice / Trigger.newMap.get(oid.opportunityId).Contract_Months__c); }

I don't know what processes you're using to do updates to your Opportunity Line Items.   However, why are you trying to capture updates to the *Opportunity* object instead of having a trigger on the OpportunityLineItem object?   Perhaps you have something which automatically updates the Opportunity when an OLI is created, removed, etc.?  Just asking.

Lastly, is there a reason you can't have a rollup summary field on the Opportunity add up all the TotalPrices for all the OLI's for a given Opportunity, and then have a different Formula field just take that number and divide it by the Contract_Months value?   (there may be a reason, again, just asking.)

Hope this helps and that I haven't misunderstood what you're trying to do.  Best, Steve.

Austin_Steve

You are correct there was a bunch of information in that first code snippet that was not needed and was probably causing a lot of issues.

The object being updated has to reside on the Opportunity Object.

What I'm trying to do is update a field called MRR on the opportunity object. MRR is calculated from any product associated with that Opportunity that has a productcode of "Software" divided by the number of contract months.

Originally I was trying to use  Formula or a Roll-up Summary field.  Unfortunately I was unable to see the OpportunityLineItem object, so I was unable to choose only those products that had a Product code of "software".

This code below works in the sandbox but errors out when I try  to bring it to the live environment.

`trigger CalcMRR on Opportunity (before update) {list<OpportunityLineItem> opps = new List<OpportunityLineItem>(); List<Opportunity> Oppts = new List<Opportunity>(); Set<String> OppSet= new Set<String>();    for(Opportunity Opp : Trigger.new) {           OppSet.add(Opp.ID);    }  Opps =  [Select Id, Quantity, UnitPrice, PricebookEntry.ProductCode          From OpportunityLineItem          Where PricebookEntry.ProductCode = 'Software'          And opportunitylineitem.opportunityid IN :Oppset];          for(Opportunity Opp: Trigger.new){          // zero out the field if it already had an amount in there          opp.Month__c = 0;         for (OpportunityLineItem olist: Opps){            if (Opp.Month__c!=null){                Opp.Month__c = (Opp.Month__c + olist.UnitPrice);            }else{                Opp.Month__c = olist.UnitPrice;            }         }                  if (opp.Month__c !=Null){             Opp.Month__c = Opp.Month__c/Opp.Contract_Months__c;       }                              }     if (OppSet.size() >0){        update Opps;     }}`

The error I'm getting is:

Description    Resource    Path    Location    Type
System.DmlException: Insert failed. First exception on row 0; first error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, CalcMRR: execution of BeforeUpdate caused by: System.NullPointerException: Attempt to de-reference a null object
Trigger.CalcMRR: line 25, column 42

Any suggestions on how to fix this would be greatly appreciated.

Austin_Steve

I some how Forrest Gumped my way through and it is working now.  Thank you for your suggestions.