Skip to content

Commit

Permalink
Fixes a rare issue reported by @simoneduckworth in #469 which prevent…
Browse files Browse the repository at this point in the history
…ed sum/count rollups from properly handling the case where an item was reparented AND went from a match to a non-match due to the calc item where clause
  • Loading branch information
jamessimone committed Aug 3, 2023
1 parent c1ea41b commit 22ed161
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 3 deletions.
38 changes: 38 additions & 0 deletions extra-tests/classes/RollupTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -2487,6 +2487,44 @@ private class RollupTests {
System.assertEquals(null, oldAcc.AnnualRevenue, 'SUM AFTER_UPDATE should take the diff between the current amount and the pre-existing one');
}

@IsTest
static void shouldCorrectlyReparentWhenOldItemMatchesAndNewOneDoesNot() {
Account acc = [SELECT Id, AnnualRevenue, Name FROM Account];
Account oldAcc = new Account(AnnualRevenue = 50, Name = 'Old Parent');
insert oldAcc;
RollupAsyncProcessor.stubParentRecords = new List<SObject>{ acc, oldAcc };

ContactPointAddress cpa = new ContactPointAddress(
Id = RollupTestUtils.createId(ContactPointAddress.SObjectType),
PreferenceRank = 50,
ParentId = acc.Id,
Name = 'Does Not Match'
);
RollupTestUtils.DMLMock mock = RollupTestUtils.loadMock(new List<ContactPointAddress>{ cpa });
Rollup.apexContext = TriggerOperation.AFTER_UPDATE;

Rollup.oldRecordsMap = new Map<Id, ContactPointAddress>{
cpa.Id => new ContactPointAddress(ParentId = oldAcc.Id, PreferenceRank = oldAcc.AnnualRevenue.intValue(), Id = cpa.Id, Name = 'Match')
};

Test.startTest();
Rollup.sumFromApex(
ContactPointAddress.PreferenceRank,
ContactPointAddress.ParentId,
Account.Id,
Account.AnnualRevenue,
Account.SObjectType,
null,
RollupEvaluator.getWhereEval('Name NOT IN (\'' + cpa.Name + '\')', cpa.getSObjectType())
)
.runCalc();
Test.stopTest();

System.assertEquals(1, mock.Records.size(), 'Only old account should have been updated, ' + mock.Records);
System.assertEquals(null, oldAcc.AnnualRevenue, 'SUM AFTER_UPDATE should remove current amount from old parent');
System.assertEquals(null, acc.AnnualRevenue, 'SUM AFTER_UPDATE should not count excluded item on new parent');
}

@IsTest
static void shouldReparentSameObjectRollups() {
Rollup.onlyUseMockmetadata = true;
Expand Down
6 changes: 3 additions & 3 deletions rollup/core/classes/RollupCalculator.cls
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ public without sharing abstract class RollupCalculator {
if (this.isFullRecalc && matchesCurrent) {
return newVal;
} else if (this.isReparented(calcItem, oldCalcItem)) {
return newVal;
return matchesCurrent ? newVal : 0;
} else if (matchesCurrent && matchesOld == false && this.isChangedFieldCalc == false) {
return newVal;
} else if (matchesCurrent == false && matchesOld && this.isChangedFieldCalc == false) {
Expand Down Expand Up @@ -804,11 +804,11 @@ public without sharing abstract class RollupCalculator {
SObject oldCalcItem = oldCalcItems.get(calcItem.Id);

// for a reparenting, the item counts towards the new record on an update
Boolean matchesCurrent = this.eval?.matches(calcItem) != false;
if (this.isReparented(calcItem, oldCalcItem)) {
return 1.00;
return matchesCurrent ? 1.00 : 0.00;
}

Boolean matchesCurrent = this.eval?.matches(calcItem) != false;
Boolean matchesPrior = oldCalcItem != null && this.eval?.matches(oldCalcItem) != false;

// for updates, we have to decrement the count if the value has been cleared out
Expand Down

0 comments on commit 22ed161

Please sign in to comment.