Skip to content

Commit

Permalink
v1.6.21 - Rollup to Multicurrency Parents Bugfix (#580)
Browse files Browse the repository at this point in the history 
* Adds safety step in pipeline for cleaning up scratch org

* Fixes compilation issue with null coalesce/ternary

* Fixes an edge case in RollupMultiCurrencytTests scratch orgs without multiple currencies loaded

* Fixes an issue reported by Joseph Mason where rollups to different parents accidentally cached the converted IsoCode value of the first child -> parent multicurrency conversion
  • Loading branch information
@jamessimone
jamessimone authored Apr 9, 2024
1 parent 2a9e4de commit 936c135
Show file tree
Hide file tree
Showing 14 changed files with 16,646 additions and 5,395 deletions.
7 changes: 7 additions & 0 deletions .github/workflows/deploy.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,16 @@ jobs:
DEVHUB_SERVER_KEY: ${{ secrets.DEVHUB_SERVER_KEY }}

- name: 'Deploy & Test'
id: 'deploy'
shell: pwsh
run: '. ./scripts/test.ps1'

- name: 'Possible Scratch Org Cleanup'
if: ${{ failure() && steps.deploy.conclusion == 'failure' }}
run: |
npm run delete:org
exit 1
# Delete temporary test files that Codecov is unable to parse
- name: 'Delete unparseable test coverage'
run: rm ./tests/apex/test-result-707*-codecoverage.json -f
Expand Down
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ As well, don't miss [the Wiki](../../wiki), which includes even more info for co

## Deployment & Setup

<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OanPAAS">
<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OatsAAC">
<img alt="Deploy to Salesforce"
src="./media/deploy-package-to-prod.png">
</a>

<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OanPAAS">
<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OatsAAC">
<img alt="Deploy to Salesforce Sandbox"
src="./media/deploy-package-to-sandbox.png">
</a>
Expand Down
16 changes: 8 additions & 8 deletions extra-tests/classes/RollupCurrencyInfoTests.cls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ private class RollupCurrencyInfoTests {
RollupCurrencyInfo.setCurrencyIsoCode(opp, mockUsdInfo.IsoCode);
RollupCurrencyInfo.transform(new List<SObject>{ opp }, Opportunity.Amount, mockEurInfo.IsoCode, new List<RollupOrderBy__mdt>());

opp = (Opportunity) RollupCurrencyInfo.getCalcItem(opp);
opp = (Opportunity) RollupCurrencyInfo.getCalcItem(opp, mockEurInfo.IsoCode);
System.assertEquals(0, opp.Amount, 'Should make it here without divide by zero error!');
}

Expand All @@ -41,7 +41,7 @@ private class RollupCurrencyInfoTests {
RollupCurrencyInfo.setCurrencyIsoCode(opp, 'USD');
RollupCurrencyInfo.transform(new List<SObject>{ opp }, Opportunity.Amount, 'EUR', new List<RollupOrderBy__mdt>());

opp = (Opportunity) RollupCurrencyInfo.getCalcItem(opp);
opp = (Opportunity) RollupCurrencyInfo.getCalcItem(opp, 'EUR');
System.assertEquals(null, opp.Amount, 'Should make it here without NPE!');
}

Expand Down Expand Up @@ -76,12 +76,12 @@ private class RollupCurrencyInfoTests {
RollupCurrencyInfo.setCurrencyIsoCode(camp, mockUsdInfo.IsoCode);
RollupCurrencyInfo.transform(new List<SObject>{ camp }, Campaign.BudgetedCost, mockEurInfo.IsoCode, new List<RollupOrderBy__mdt>());

Campaign updatedCamp = (Campaign) RollupCurrencyInfo.getCalcItem(camp);
Campaign updatedCamp = (Campaign) RollupCurrencyInfo.getCalcItem(camp, mockEurInfo.IsoCode);
System.assertEquals(mockEurInfo.ConversionRate / (mockUsdInfo.ConversionRate / camp.BudgetedCost), updatedCamp.BudgetedCost);

// now for the second field update
RollupCurrencyInfo.transform(new List<SObject>{ camp }, Campaign.ActualCost, mockEurInfo.IsoCode, new List<RollupOrderBy__mdt>());
updatedCamp = (Campaign) RollupCurrencyInfo.getCalcItem(camp);
updatedCamp = (Campaign) RollupCurrencyInfo.getCalcItem(camp, mockEurInfo.IsoCode);
System.assertEquals((mockEurInfo.ConversionRate / (mockUsdInfo.ConversionRate / camp.ActualCost)).doubleValue(), updatedCamp.ActualCost);
}

Expand All @@ -107,7 +107,7 @@ private class RollupCurrencyInfoTests {

RollupCurrencyInfo.transform(new List<SObject>{ opp }, currencyFormulaToken, mockEurInfo.IsoCode, new List<RollupOrderBy__mdt>());

Opportunity updatedOpp = (Opportunity) RollupCurrencyInfo.getCalcItem(opp);
Opportunity updatedOpp = (Opportunity) RollupCurrencyInfo.getCalcItem(opp, mockEurInfo.IsoCode);
System.assertEquals((mockEurInfo.ConversionRate / (mockUsdInfo.ConversionRate / opp.AmountFormula__c)).doubleValue(), updatedOpp.AmountFormula__c);
}

Expand Down Expand Up @@ -154,8 +154,8 @@ private class RollupCurrencyInfoTests {
// quite a bit of ceremony to get here - but finally the method under test
RollupCurrencyInfo.transform(opps, Opportunity.Amount, eurPeriodOne.IsoCode, new List<RollupOrderBy__mdt>());

firstOpp = (Opportunity) RollupCurrencyInfo.getCalcItem(opps.get(0));
secondOpp = (Opportunity) RollupCurrencyInfo.getCalcItem(opps.get(1));
firstOpp = (Opportunity) RollupCurrencyInfo.getCalcItem(opps.get(0), eurPeriodOne.IsoCode);
secondOpp = (Opportunity) RollupCurrencyInfo.getCalcItem(opps.get(1), eurPeriodOne.IsoCode);
System.assertEquals(eurPeriodOne.ConversionRate / (usdInfo.ConversionRate / opps[0].Amount), firstOpp.Amount);
System.assertEquals(eurPeriodTwo.ConversionRate / (usdInfo.ConversionRate / opps[1].Amount), secondOpp.Amount);
}
Expand Down Expand Up @@ -201,7 +201,7 @@ private class RollupCurrencyInfoTests {
RollupCurrencyInfo.overrideDatedMultiCurrency(olis.getSObjectType().getDescribe().getName(), new List<String>{ 'Opportunity', 'CloseDate' });
RollupCurrencyInfo.transform(olis, OpportunityLineItem.TotalPrice, eurPeriodOne.IsoCode, new List<RollupOrderBy__mdt>());

OpportunityLineItem oli = (OpportunityLineItem) RollupCurrencyInfo.getCalcItem(oliToUpdate);
OpportunityLineItem oli = (OpportunityLineItem) RollupCurrencyInfo.getCalcItem(oliToUpdate, eurPeriodOne.IsoCode);
System.assertEquals(eurPeriodOne.ConversionRate / (usdInfo.ConversionRate / oliToUpdate.TotalPrice), oli.TotalPrice);
}
}
74 changes: 74 additions & 0 deletions extra-tests/classes/RollupMultiCurrencyTests.cls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ private class RollupMultiCurrencyTests {

@IsTest
static void shouldCorrectlyRollupMaxForMultiCurrency() {
if (hasCurrencyInfo('EUR') == false) {
return;
}
Account acc = (Account) RollupTestUtils.queryRecord(Account.SObjectType, new List<Schema.SObjectField>{ Account.AnnualRevenue });
System.assertEquals(null, acc.AnnualRevenue, 'Test has started under the wrong conditions!');
System.assertEquals('USD', getCurrencyIsoCode(acc), 'Test has started under the wrong conditions!');
Expand Down Expand Up @@ -81,6 +84,9 @@ private class RollupMultiCurrencyTests {

@IsTest
static void shouldCorrectlyRollupMinForMultiCurrency() {
if (hasCurrencyInfo('EUR') == false) {
return;
}
Account acc = (Account) RollupTestUtils.queryRecord(Account.SObjectType, new List<Schema.SObjectField>{ Account.AnnualRevenue });
System.assertEquals(null, acc.AnnualRevenue, 'Test has started under the wrong conditions!');
System.assertEquals('USD', getCurrencyIsoCode(acc), 'Test has started under the wrong conditions!');
Expand Down Expand Up @@ -131,6 +137,9 @@ private class RollupMultiCurrencyTests {

@IsTest
static void shouldCorrectlyRollupSumForMultiCurrency() {
if (hasCurrencyInfo('EUR') == false) {
return;
}
Account acc = (Account) RollupTestUtils.queryRecord(Account.SObjectType, new List<Schema.SObjectField>{ Account.AnnualRevenue });
System.assertEquals(null, acc.AnnualRevenue, 'Test has started under the wrong conditions!');
System.assertEquals('USD', getCurrencyIsoCode(acc), 'Test has started under the wrong conditions!');
Expand Down Expand Up @@ -182,6 +191,9 @@ private class RollupMultiCurrencyTests {

@IsTest
static void shouldCorrectlyRollupAverageForMultiCurrency() {
if (hasCurrencyInfo('EUR') == false) {
return;
}
Account acc = (Account) RollupTestUtils.queryRecord(Account.SObjectType, new List<Schema.SObjectField>{ Account.AnnualRevenue });
System.assertEquals(null, acc.AnnualRevenue, 'Test has started under the wrong conditions!');
System.assertEquals('USD', getCurrencyIsoCode(acc), 'Test has started under the wrong conditions!');
Expand Down Expand Up @@ -240,6 +252,9 @@ private class RollupMultiCurrencyTests {

@IsTest
static void shouldCorrectlyRollupFirstForMultiCurrency() {
if (hasCurrencyInfo('EUR') == false) {
return;
}
Account acc = (Account) RollupTestUtils.queryRecord(Account.SObjectType, new List<Schema.SObjectField>{ Account.AnnualRevenue });
System.assertEquals(null, acc.AnnualRevenue, 'Test has started under the wrong conditions!');
System.assertEquals('USD', getCurrencyIsoCode(acc), 'Test has started under the wrong conditions!');
Expand Down Expand Up @@ -300,6 +315,9 @@ private class RollupMultiCurrencyTests {

@IsTest
static void shouldCorrectlyRollupLastForMultiCurrency() {
if (hasCurrencyInfo('EUR') == false) {
return;
}
Account acc = (Account) RollupTestUtils.queryRecord(Account.SObjectType, new List<Schema.SObjectField>{ Account.AnnualRevenue });
System.assertEquals(null, acc.AnnualRevenue, 'Test has started under the wrong conditions!');
System.assertEquals('USD', getCurrencyIsoCode(acc), 'Test has started under the wrong conditions!');
Expand Down Expand Up @@ -358,6 +376,62 @@ private class RollupMultiCurrencyTests {
System.assertEquals(lastOpportunityId, acc.Name, 'Should have taken last based on multi-currency Amount! Records: ' + opportunities);
}

@IsTest
static void properlyTracksCurrencyItemsForDifferentParentCurrencies() {
if (hasCurrencyInfo('EUR') == false || hasCurrencyInfo('JPY') == false) {
return;
}
delete [SELECT Id FROM Opportunity];

Account acc = (Account) RollupTestUtils.queryRecord(Account.SObjectType, new List<Schema.SObjectField>{ Account.AnnualRevenue });
acc.AnnualRevenue = 100;
update acc;
System.assertEquals('USD', getCurrencyIsoCode(acc), 'Test has started under the wrong conditions!');

Opportunity eurOpp = new Opportunity(Name = 'First Parent', StageName = 'a', CloseDate = System.today(), AccountId = acc.Id);
RollupCurrencyInfo.setCurrencyIsoCode(eurOpp, 'EUR');
Contract contract = new Contract(AccountId = acc.Id, Name = 'Second Parent');
RollupCurrencyInfo.setCurrencyIsoCode(contract, 'JPY');
insert new List<SObject>{ eurOpp, contract };

Rollup.onlyUseMockMetadata = true;
Rollup__mdt firstParent = new Rollup__mdt(
RollupFieldOnCalcItem__c = 'AnnualRevenue',
LookupObject__c = 'Opportunity',
LookupFieldOnCalcItem__c = 'Id',
LookupFieldOnLookupObject__c = 'AccountId',
RollupFieldOnLookupObject__c = 'Amount',
RollupOperation__c = 'SUM',
CalcItem__c = 'Account'
);
Rollup__mdt secondParent = new Rollup__mdt(
RollupFieldOnCalcItem__c = 'AnnualRevenue',
LookupObject__c = 'Contract',
LookupFieldOnCalcItem__c = 'Id',
LookupFieldOnLookupObject__c = 'AccountId',
RollupFieldOnLookupObject__c = 'ContractTerm',
RollupOperation__c = 'SUM',
CalcItem__c = 'Account'
);
Rollup.rollupMetadata = new List<Rollup__mdt>{ firstParent, secondParent };
Rollup.apexContext = TriggerOperation.AFTER_INSERT;
Rollup.shouldRun = true;
Rollup.records = new List<Account>{ acc };

Test.startTest();
Rollup.runFromTrigger();
Test.stopTest();

contract = [SELECT ContractTerm FROM Contract WHERE Id = :contract.Id];
eurOpp = [SELECT Amount FROM Opportunity WHERE Id = :eurOpp.Id];
System.assertNotEquals(null, contract.ContractTerm, 'Rollup should have occurred');
System.assertNotEquals(contract.ContractTerm, eurOpp.Amount);
}

private static Boolean hasCurrencyInfo(String currencyCode) {
return RollupCurrencyInfo.getCurrencyInfo(currencyCode).IsoCode != null;
}

private static String getCurrencyIsoCode(SObject record) {
return UserInfo.isMultiCurrencyOrganization() ? (String) record.get(RollupCurrencyInfo.CURRENCY_ISO_CODE_FIELD_NAME) : 'USD';
}
Expand Down
Loading

0 comments on commit 936c135

Please sign in to comment.