V1.3.25 - Ultimate Parent Full Recalc Bugfix (#238)

* Fixes #237 by ensuring ultimate parent rollups don't run endlessly and properly abort at the top-level parents after calculating rollups - also ensures full recalc feature works for hierarchical parents

README.md

@@ -22,12 +22,12 @@ You have several different options when it comes to making use of `Rollup`:
 
 ## Deployment
 
-<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008Siv4AAC">
+<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008Sj00AAC">
   <img alt="Deploy to Salesforce"
        src="./media/deploy-package-to-prod.png">
 </a>
 
-<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008Siv4AAC">
+<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008Sj00AAC">
   <img alt="Deploy to Salesforce Sandbox"
        src="./media/deploy-package-to-sandbox.png">
 </a>

extra-tests/classes/RollupFullRecalcTests.cls

@@ -101,11 +101,7 @@ private class RollupFullRecalcTests {
     insert new ContactPointAddress(PreferenceRank = 500, ParentId = acc.Id, Name = 'One');
 
     List<ContactPointAddress> cpas = new List<ContactPointAddress>{
-      new ContactPointAddress(
-        PreferenceRank = second.AnnualRevenue.intValue(),
-        ParentId = second.Id,
-        Name = 'Two'
-      )
+      new ContactPointAddress(PreferenceRank = second.AnnualRevenue.intValue(), ParentId = second.Id, Name = 'Two')
     };
     insert cpas;
 
@@ -156,6 +152,40 @@ private class RollupFullRecalcTests {
     System.assertEquals(1500, updatedAcc.AnnualRevenue, 'SUM REFRESH from flow should fully recalc');
   }
 
+  @IsTest
+  static void shouldPerformBulkFullRecalcWithHierarchy() {
+    // also validates that RollupRelationshipFieldFinder aborts early if it's called
+    // for a top-level parent
+    Rollup.defaultControl = new RollupControl__mdt(ShouldAbortRun__c = true);
+
+    Account acc = [SELECT Id FROM Account];
+    Account childAccount = new Account(Name = 'Hierarchy child', ParentId = acc.Id, AnnualRevenue = 5);
+    insert childAccount;
+
+    Rollup.defaultControl = null;
+    Rollup.onlyUseMockMetadata = true;
+    Rollup.rollupMetadata = new List<Rollup__mdt>{
+      new Rollup__mdt(
+        CalcItem__c = 'Account',
+        LookupObject__c = 'Account',
+        RollupToUltimateParent__c = true,
+        LookupFieldOnCalcItem__c = 'ParentId',
+        UltimateParentLookup__c = 'ParentId',
+        RollupOperation__c = 'SUM',
+        RollupFieldOnLookupObject__c = 'AnnualRevenue',
+        RollupFieldOnCalcItem__c = 'AnnualRevenue',
+        LookupFieldOnLookupObject__c = 'Id'
+      )
+    };
+
+    Test.startTest();
+    Rollup.performBulkFullRecalc(Rollup.rollupMetadata, Rollup.InvocationPoint.FROM_LWC.name());
+    Test.stopTest();
+
+    acc = [SELECT Id, AnnualRevenue FROM Account WHERE Id = :acc.Id];
+    System.assertEquals(childAccount.AnnualRevenue, acc.AnnualRevenue);
+  }
+
   @IsTest
   static void shouldPerformFullRecalcWithGrandparentHierarchy() {
     Rollup.onlyUseMockMetadata = true;

extra-tests/classes/RollupIntegrationTests.cls

@@ -591,8 +591,8 @@ private class RollupIntegrationTests {
     Account secondGreatGrandparent = new Account(Name = 'Second great-grandparent');
     insert new List<Account>{ greatGrandparent, secondGreatGrandparent };
 
-    ParentApplication__c grandParent = new ParentApplication__c(Name = 'Grandparent', Account__c = greatGrandparent.Id);
-    ParentApplication__c secondGrandparent = new ParentApplication__c(Name = 'Second grandparent', Account__c = secondGreatGrandparent.Id);
+    ParentApplication__c grandParent = new ParentApplication__c(Name = 'Grandparent', Account__c = secondGreatGrandparent.Id);
+    ParentApplication__c secondGrandparent = new ParentApplication__c(Name = 'Second grandparent', Account__c = greatGrandparent.Id);
     List<ParentApplication__c> parentApps = new List<ParentApplication__c>{ grandParent, secondGrandparent };
     insert parentApps;
 
@@ -604,7 +604,6 @@ private class RollupIntegrationTests {
     ApplicationLog__c secondChild = new ApplicationLog__c(Name = 'Reparenting deux', Application__c = parent.Id);
     insert new List<ApplicationLog__c>{ child, secondChild };
 
-    Rollup.records = parentApps;
     Rollup.rollupMetadata = new List<Rollup__mdt>{
       new Rollup__mdt(
         CalcItem__c = 'ApplicationLog__c',
@@ -619,13 +618,11 @@ private class RollupIntegrationTests {
     };
     Rollup.shouldRun = true;
     Rollup.apexContext = TriggerOperation.AFTER_UPDATE;
-    Rollup.oldRecordsMap = new Map<Id, SObject>{
-      grandParent.Id => new ParentApplication__c(Id = grandParent.Id, Account__c = secondGreatGrandparent.Id),
-      secondGrandparent.Id => new ParentApplication__c(Id = secondGrandparent.Id, Account__c = greatGrandparent.Id)
-    };
 
     Test.startTest();
-    Rollup.runFromTrigger();
+    parentApps[0].Account__c = greatGrandparent.Id;
+    parentApps[1].Account__c = secondGreatGrandparent.Id;
+    update parentApps;
     Test.stopTest();
 
     Account updatedGreatGrandparent = [SELECT Name FROM Account WHERE Id = :greatGrandparent.Id];
@@ -637,6 +634,7 @@ private class RollupIntegrationTests {
 
   @IsTest
   static void shouldRunGrandparentRollupsWhenIntermediateObjectsAreUpdatedFromFlow() {
+    Rollup.onlyUseMockMetadata = true;
     Account greatGrandparent = new Account(Name = 'Great-grandparent Flow');
     Account secondGreatGrandparent = new Account(Name = 'Second great-grandparent Flow');
     insert new List<Account>{ greatGrandparent, secondGreatGrandparent };
@@ -998,6 +996,19 @@ private class RollupIntegrationTests {
   static void shouldCorrectlyRollupFromTriggerOnMerge() {
     Account parent = [SELECT Id, Name FROM Account];
 
+    // you can only merge contacts / accounts / leads / cases
+    Account parentToMerge = new Account(Name = 'Second parent');
+    insert parentToMerge;
+
+    List<ContactPointAddress> rollupChildren = new List<ContactPointAddress>{
+      new ContactPointAddress(Name = 'Child one', ParentId = parent.Id, PreferenceRank = 1),
+      new ContactPointAddress(Name = 'Child two', ParentId = parentToMerge.Id, PreferenceRank = 2)
+    };
+
+    // we don't even need the parent account's annual revenue to be set; rather
+    // we need to validate that post merge, the rollup is recalculated correctly
+    insert rollupChildren;
+
     // this test relies on AccountTrigger.trigger having AFTER DELETE set up
     Rollup.rollupMetadata = new List<Rollup__mdt>{
       new Rollup__mdt(
@@ -1032,19 +1043,6 @@ private class RollupIntegrationTests {
       )
     };
 
-    // you can only merge contacts / accounts / leads / cases
-    Account parentToMerge = new Account(Name = 'Second parent');
-    insert parentToMerge;
-
-    List<ContactPointAddress> rollupChildren = new List<ContactPointAddress>{
-      new ContactPointAddress(Name = 'Child one', ParentId = parent.Id, PreferenceRank = 1),
-      new ContactPointAddress(Name = 'Child two', ParentId = parentToMerge.Id, PreferenceRank = 2)
-    };
-
-    // we don't even need the parent account's annual revenue to be set; rather
-    // we need to validate that post merge, the rollup is recalculated correctly
-    insert rollupChildren;
-
     Test.startTest();
     Database.merge(parent, parentToMerge.Id, true);
     Test.stopTest();

extra-tests/classes/RollupRelationshipFieldFinderTests.cls

@@ -326,4 +326,21 @@ private class RollupRelationshipFieldFinderTests {
     Account retrievedAcc = (Account) traversal.retrieveParent(con.Id);
     System.assertEquals(expectedAcc.Id, retrievedAcc.Id, 'Should correctly retrieve hierarchy');
   }
+
+  @IsTest
+  static void regressionTopLevelHierarchyDoesNotThrowException() {
+    Account hierarchyParent = [SELECT Id, ParentId FROM Account];
+    RollupRelationshipFieldFinder finder = new RollupRelationshipFieldFinder(
+      control,
+      new Rollup__mdt(RollupToUltimateParent__c = true, UltimateParentLookup__c = 'ParentId', LookupFieldOnCalcItem__c = 'ParentId'),
+      new Set<String>{ 'Name', 'Id' },
+      Account.SObjectType,
+      new Map<Id, SObject>()
+    );
+
+    RollupRelationshipFieldFinder.Traversal traversal = finder.getParents(new List<Account>{ hierarchyParent });
+
+    System.assertEquals(true, traversal.getIsFinished());
+    System.assertEquals(true, traversal.getParentLookupToRecords().isEmpty());
+  }
 }

extra-tests/triggers/AccountTrigger.trigger

@@ -1,4 +1,4 @@
-trigger AccountTrigger on Account (after delete) {
+trigger AccountTrigger on Account(after insert, after update, after delete) {
   // included specifically to test merges
   Rollup.runFromTrigger();
-}
+}

package.json

@@ -1,6 +1,6 @@
 {
   "name": "apex-rollup",
-  "version": "1.3.24",
+  "version": "1.3.25",
   "description": "Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.",
   "repository": {
     "type": "git",

plugins/ExtraCodeCoverage/README.md

@@ -1,11 +1,11 @@
 # Extra Code Coverage
 
-<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008SirqAAC">
+<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008SizvAAC">
   <img alt="Deploy to Salesforce"
        src="../../media/deploy-package-to-prod.png">
 </a>
 
-<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008SirqAAC">
+<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008SizvAAC">
   <img alt="Deploy to Salesforce Sandbox"
        src="../../media/deploy-package-to-sandbox.png">
 </a>

rollup/core/classes/Rollup.cls

@@ -1478,6 +1478,7 @@ global without sharing virtual class Rollup {
       }
       if (rollupMetadata != null) {
         cachedMetadata.addAll(rollupMetadata);
+        rollupMetadata = null;
       }
       matchingMetadata = cachedMetadata.clone();
     } else if (metadataType == RollupControl__mdt.SObjectType) {

rollup/core/classes/RollupLogger.cls

@@ -1,7 +1,7 @@
 public without sharing virtual class RollupLogger extends Rollup implements ILogger {
   @TestVisible
   // this gets updated via the pipeline as the version number gets incremented
-  private static final String CURRENT_VERSION_NUMBER = 'v1.3.24';
+  private static final String CURRENT_VERSION_NUMBER = 'v1.3.25';
   private static final LoggingLevel FALLBACK_LOGGING_LEVEL = LoggingLevel.DEBUG;
   private static final RollupPlugin PLUGIN = new RollupPlugin();
 

rollup/core/classes/RollupRelationshipFieldFinder.cls

@@ -93,7 +93,7 @@ public without sharing class RollupRelationshipFieldFinder {
 
     public Map<String, Rollup.CalcItemBag> getParentLookupToRecords() {
       Map<String, Rollup.CalcItemBag> parentToLookupRecords = new Map<String, Rollup.CalcItemBag>();
-      if (this.isAbortedEarly) {
+      if (this.isAbortedEarly || this.finder.records == null) {
         return parentToLookupRecords;
       }
       for (SObject record : this.finder.records) {
@@ -300,6 +300,7 @@ public without sharing class RollupRelationshipFieldFinder {
     }
     // no matter how far up the chain we are, if we arrive at a point where there are no records, we're done
     if (firstId == null) {
+      this.setFirstRunFlags(records);
       this.prepFinishedObject(records);
       this.traversal.isAbortedEarly = true;
       return this.traversal;
@@ -329,13 +330,17 @@ public without sharing class RollupRelationshipFieldFinder {
       currentRelationshipName,
       nextFieldMap
     );
+    this.setFirstRunFlags(records);
 
     // recurse through till we get to the top/bottom of the chain
+    return this.getParents(Database.query(query));
+  }
+
+  private void setFirstRunFlags(List<SObject> records) {
     if (this.isFirstRun) {
       this.records = records;
       this.isFirstRun = false;
     }
-    return this.getParents(Database.query(query));
   }
 
   private void populateRelationshipNameToWhereClauses(Schema.SObjectType sObjectType) {

sfdx-project.json

@@ -4,8 +4,8 @@
             "default": true,
             "package": "apex-rollup",
             "path": "rollup",
-            "versionName": "Flow bugfixes, logging readability improvements, some tooling work related to plugins",
-            "versionNumber": "1.3.24.0",
+            "versionName": "Fixing issue with ultimate parent rollups and full recalculation",
+            "versionNumber": "1.3.25.0",
             "versionDescription": "Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.",
             "releaseNotesUrl": "https://github.com/jamessimone/apex-rollup/releases/latest",
             "unpackagedMetadata": {
@@ -71,11 +71,11 @@
             "path": "plugins\\ExtraCodeCoverage",
             "package": "Apex Rollup - Extra Code Coverage",
             "versionName": "Updating code coverage",
-            "versionNumber": "0.0.4.0",
+            "versionNumber": "0.0.5.0",
             "default": false,
             "dependencies": [
                 {
-                    "package": "[email protected].23-0"
+                    "package": "[email protected].25-0"
                 }
             ]
         }
@@ -97,16 +97,13 @@
         "Apex Rollup - Rollup [email protected]": "04t6g000008ShztAAC",
         "Apex Rollup - Rollup [email protected]": "04t6g000008Sis0AAC",
         "Apex Rollup - Extra Code Coverage": "0Ho6g000000GnCWCA0",
-        "Apex Rollup - Extra Code [email protected]": "04t6g000008Sii9AAC",
         "Apex Rollup - Extra Code [email protected]": "04t6g000008SirqAAC",
-        "[email protected]": "04t6g000008SiSdAAK",
-        "[email protected]": "04t6g000008SiTgAAK",
-        "[email protected]": "04t6g000008SiUyAAK",
-        "[email protected]": "04t6g000008SiVIAA0",
+        "Apex Rollup - Extra Code [email protected]": "04t6g000008SizvAAC",
         "[email protected]": "04t6g000008SiaJAAS",
         "[email protected]": "04t6g000008SigrAAC",
         "[email protected]": "04t6g000008SijvAAC",
         "[email protected]": "04t6g000008SitNAAS",
-        "[email protected]": "04t6g000008Siv4AAC"
+        "[email protected]": "04t6g000008Siv4AAC",
+        "[email protected]": "04t6g000008Sj00AAC"
     }
 }