The revert from including Evaluator for AVERAGE leads to both the num…

…erator/denominator being too enthusiastic
jamessimone · Mar 10, 2021 · fde03b2 · fde03b2
1 parent ad55c59
commit fde03b2
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 2 deletions.
diff --git a/extra-tests/classes/RollupIntegrationTests.cls b/extra-tests/classes/RollupIntegrationTests.cls
@@ -142,6 +142,6 @@ private class RollupIntegrationTests {
     Test.stopTest();
 
     ParentApplication__c updatedParent = [SELECT Engagement_Rollup__c FROM ParentApplication__c];
-    System.assertEquals(45/5, updatedParent.Engagement_Rollup__c, 'Average should be calculated based off of all items for denominator, not just matching items'); 
+    System.assertEquals(45/3, updatedParent.Engagement_Rollup__c, 'Average should be calculated based off of all items for denominator, not just matching items'); 
   }
 }
diff --git a/rollup/main/default/classes/RollupCalculator.cls b/rollup/main/default/classes/RollupCalculator.cls
@@ -837,15 +837,17 @@ public without sharing abstract class RollupCalculator {
 
       List<SObject> applicableCalcItems = this.op == Rollup.Op.DELETE_AVERAGE ? new List<SObject>() : calcItems;
       Decimal newSum = 0;
+      Decimal currentDenominator = 0;
       for (SObject calcItem : applicableCalcItems) {
         if(this.eval == null || this.eval.matches(calcItem) == true) {
           Object potentialDecimal = calcItem.get(this.opFieldOnCalcItem);
           newSum += potentialDecimal == null ? 0 : (Decimal) potentialDecimal;
+          currentDenominator++;
         }
       }
 
       Decimal numerator = oldSum + newSum;
-      Decimal denominator = countOfPreExistingItems + applicableCalcItems.size();
+      Decimal denominator = countOfPreExistingItems + currentDenominator;
 
       // Can't divide by 0, so just return null
       if (denominator == 0) {