From d307f7df50e67fbb612573f7ba67b5ced86f78e3 Mon Sep 17 00:00:00 2001 From: James Simone <16430727+jamessimone@users.noreply.github.com> Date: Thu, 20 Jan 2022 10:38:48 -0700 Subject: [PATCH] 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 | 4 +- extra-tests/classes/RollupFullRecalcTests.cls | 40 ++++++++++++++++--- .../classes/RollupIntegrationTests.cls | 40 +++++++++---------- .../RollupRelationshipFieldFinderTests.cls | 17 ++++++++ extra-tests/triggers/AccountTrigger.trigger | 4 +- package.json | 2 +- plugins/ExtraCodeCoverage/README.md | 4 +- rollup/core/classes/Rollup.cls | 1 + rollup/core/classes/RollupLogger.cls | 2 +- .../classes/RollupRelationshipFieldFinder.cls | 9 ++++- sfdx-project.json | 17 ++++---- 11 files changed, 94 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index f0a6b154..9688b56e 100644 --- a/README.md +++ b/README.md @@ -22,12 +22,12 @@ You have several different options when it comes to making use of `Rollup`: ## Deployment - + Deploy to Salesforce - + Deploy to Salesforce Sandbox diff --git a/extra-tests/classes/RollupFullRecalcTests.cls b/extra-tests/classes/RollupFullRecalcTests.cls index 0215802b..233ed00b 100644 --- a/extra-tests/classes/RollupFullRecalcTests.cls +++ b/extra-tests/classes/RollupFullRecalcTests.cls @@ -101,11 +101,7 @@ private class RollupFullRecalcTests { insert new ContactPointAddress(PreferenceRank = 500, ParentId = acc.Id, Name = 'One'); List cpas = new List{ - 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{ + 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; diff --git a/extra-tests/classes/RollupIntegrationTests.cls b/extra-tests/classes/RollupIntegrationTests.cls index 5d9d3d70..ea99db42 100644 --- a/extra-tests/classes/RollupIntegrationTests.cls +++ b/extra-tests/classes/RollupIntegrationTests.cls @@ -591,8 +591,8 @@ private class RollupIntegrationTests { Account secondGreatGrandparent = new Account(Name = 'Second great-grandparent'); insert new List{ 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 parentApps = new List{ 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{ child, secondChild }; - Rollup.records = parentApps; Rollup.rollupMetadata = new List{ 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{ - 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{ 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 rollupChildren = new List{ + 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{ 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 rollupChildren = new List{ - 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(); diff --git a/extra-tests/classes/RollupRelationshipFieldFinderTests.cls b/extra-tests/classes/RollupRelationshipFieldFinderTests.cls index 00443707..6ac253ea 100644 --- a/extra-tests/classes/RollupRelationshipFieldFinderTests.cls +++ b/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{ 'Name', 'Id' }, + Account.SObjectType, + new Map() + ); + + RollupRelationshipFieldFinder.Traversal traversal = finder.getParents(new List{ hierarchyParent }); + + System.assertEquals(true, traversal.getIsFinished()); + System.assertEquals(true, traversal.getParentLookupToRecords().isEmpty()); + } } diff --git a/extra-tests/triggers/AccountTrigger.trigger b/extra-tests/triggers/AccountTrigger.trigger index 6135fa40..860acfa0 100644 --- a/extra-tests/triggers/AccountTrigger.trigger +++ b/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(); -} \ No newline at end of file +} diff --git a/package.json b/package.json index 3507db7f..6fce3ff4 100644 --- a/package.json +++ b/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", diff --git a/plugins/ExtraCodeCoverage/README.md b/plugins/ExtraCodeCoverage/README.md index 323bfb1b..6c61752e 100644 --- a/plugins/ExtraCodeCoverage/README.md +++ b/plugins/ExtraCodeCoverage/README.md @@ -1,11 +1,11 @@ # Extra Code Coverage - + Deploy to Salesforce - + Deploy to Salesforce Sandbox diff --git a/rollup/core/classes/Rollup.cls b/rollup/core/classes/Rollup.cls index ea13e0c9..1a04fbde 100644 --- a/rollup/core/classes/Rollup.cls +++ b/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) { diff --git a/rollup/core/classes/RollupLogger.cls b/rollup/core/classes/RollupLogger.cls index b72abc6e..e9ba7b49 100644 --- a/rollup/core/classes/RollupLogger.cls +++ b/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(); diff --git a/rollup/core/classes/RollupRelationshipFieldFinder.cls b/rollup/core/classes/RollupRelationshipFieldFinder.cls index d14cfede..43d2253d 100644 --- a/rollup/core/classes/RollupRelationshipFieldFinder.cls +++ b/rollup/core/classes/RollupRelationshipFieldFinder.cls @@ -93,7 +93,7 @@ public without sharing class RollupRelationshipFieldFinder { public Map getParentLookupToRecords() { Map parentToLookupRecords = new Map(); - 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 records) { if (this.isFirstRun) { this.records = records; this.isFirstRun = false; } - return this.getParents(Database.query(query)); } private void populateRelationshipNameToWhereClauses(Schema.SObjectType sObjectType) { diff --git a/sfdx-project.json b/sfdx-project.json index fae4dbe7..8e218c46 100644 --- a/sfdx-project.json +++ b/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": "apex-rollup@1.3.23-0" + "package": "apex-rollup@1.3.25-0" } ] } @@ -97,16 +97,13 @@ "Apex Rollup - Rollup Callback@0.0.2-0": "04t6g000008ShztAAC", "Apex Rollup - Rollup Callback@0.0.3-0": "04t6g000008Sis0AAC", "Apex Rollup - Extra Code Coverage": "0Ho6g000000GnCWCA0", - "Apex Rollup - Extra Code Coverage@0.0.3-0": "04t6g000008Sii9AAC", "Apex Rollup - Extra Code Coverage@0.0.4-0": "04t6g000008SirqAAC", - "apex-rollup@1.3.16-0": "04t6g000008SiSdAAK", - "apex-rollup@1.3.17-0": "04t6g000008SiTgAAK", - "apex-rollup@1.3.18-0": "04t6g000008SiUyAAK", - "apex-rollup@1.3.19-0": "04t6g000008SiVIAA0", + "Apex Rollup - Extra Code Coverage@0.0.5-0": "04t6g000008SizvAAC", "apex-rollup@1.3.20-0": "04t6g000008SiaJAAS", "apex-rollup@1.3.21-0": "04t6g000008SigrAAC", "apex-rollup@1.3.22-0": "04t6g000008SijvAAC", "apex-rollup@1.3.23-0": "04t6g000008SitNAAS", - "apex-rollup@1.3.24-0": "04t6g000008Siv4AAC" + "apex-rollup@1.3.24-0": "04t6g000008Siv4AAC", + "apex-rollup@1.3.25-0": "04t6g000008Sj00AAC" } } \ No newline at end of file