Account Soft Credits: Object and Rollups integration #3220

bethbrains · 2018-05-29T21:08:41Z

Critical Changes

Account Soft Credits

IMPORTANT: To get the most value out of Account Soft Credits, you'll want to enable Customizable Rollups so you can roll up the soft credit values to the Account records. At this time, you can't roll up Account soft credits with legacy NPSP rollups or User Defined Rollups.

Before you can start using Account soft credits, you need to give users access to the Account Soft Credit object and fields, and add the Account Soft Credit related list to your page layout. You can also optionally add more Account soft credit roles. For instructions, see the Account Soft Credit Setup section of Soft Credit and Matching Gift Setup.

Changes

Account Soft Credits

Account soft credits are credits for donations that Accounts don't actually make, but may have somehow influenced. They're very useful for situations where an Account, such as a Donor Advised Fund, uses another organization, such as a bank, to manage their money. When a donation comes from the Donor Advised Fund, the check is actually generated by the bank, so the bank gets the hard credit for the donation. Using Account soft credits, you can ensure that the Donor Advised Fund gets a soft credit for their donation.

For details on Account Soft Credits, see the Soft Credits Overview. For details on creating Account Soft Credits, see Create Soft Credits.

Issues Closed

New Metadata

Account_Soft_Credit__c object
CRLP_AccountSkew_AccSoftCredit_BATCH
CRLP_Account_AccSoftCredit_BATCH
CRLP_RollupAccSoftCredit_SVC
TDTM_AccountSoftCredit
CRLP_RollupAccSoftCredit_TEST

Deleted Metadata

force2b · 2018-07-31T13:38:56Z

src/classes/CRLP_RollupAccount_TEST.cls

@@ -48,6 +48,8 @@ private class CRLP_RollupAccount_TEST {
    private static void setupBaseTestData() {
        Contact c = UTIL_UnitTestData_TEST.getContact();
        insert c;
+        Account acc = UTIL_UnitTestData_TEST.createMultipleTestAccounts(1, CAO_Constants.HH_ACCOUNT_TYPE)[0];


Why not create a new unit test class that specifically validates Account Soft Credits, and can test both batch jobs? I see that this does test one Account Soft Credit rollup, but coverage of the two new batch classes is is only around 25%. Does it make sense to create a new unit test class that more completely tests various rollup types and conditions for just Account Soft Credit by itself?

force2b · 2018-07-31T13:39:27Z

src/classes/CRLP_RollupAccount_TEST.cls

+            Account_Soft_Credit__c accSC = new Account_Soft_Credit__c (
+                    Amount__c = scAmount,
+                    Account__c = softCreditAcc.Id,
+                    Opportunity__c = opps[i].Id);


Is it worth setting a Role value and validating the filter rule against the Role? Right now, it's just including all ASC records. I think more detailed test should validate that its only including the correct ASC records using Filter Rules.

force2b · 2018-07-31T13:41:00Z

src/classes/CRLP_VRollupHandler.cls

@@ -53,6 +53,8 @@ public virtual class CRLP_VRollupHandler implements CRLP_Rollup_SVC.IRollupHandl
    protected final String allocObjectName = UTIL_Describe.getObjectDescribe(UTIL_Namespace.StrAllNSPrefix('Allocation__c')).getName();
    /* @description 'Name' of the object referenced in the rollup. Visible to classes that extend this virtual class. */
    protected final String rdObjectName = UTIL_Describe.getObjectDescribe('npe03__Recurring_Donation__c').getName();
+    /* @description 'Name' of the object referenced in the rollup. Visible to classes that extend this virtual class. */
+    protected final String ascObjectName = UTIL_Describe.getObjectDescribe(UTIL_Namespace.StrAllNSPrefix('Account_Soft_Credit__c')).getName();


Not sure this is used anywhere

Most of them in this section aren't -- all but opp and payment, from the looks of it. Should we remove all of them?

@bethbrains Good point. I added them early on thinking they'd be used. Feel free to remove any of them ones that are not referenced in this class or the classes that implement this class.

force2b · 2018-07-31T13:42:45Z

src/classes/OPP_AccountSoftCredit_TDTM.cls

+        // BEFORE INSERT
+        if (triggerAction == TDTM_Runnable.Action.BeforeInsert) {
+            synchronizeCurrencyIsoCode(listNew);
+            return null;


Is the return null; needed here and in the next if block?

force2b · 2018-07-31T13:43:17Z

src/classes/OPP_AccountSoftCredit_TDTM.cls

+    private void synchronizeCurrencyIsoCode(list<Account_Soft_Credit__c> listAccSC) {
+
+        // collect all our Opps and Contacts referenced by the PSC's
+        set<Id> setOppId = new set<Id>();


Collection types and var declarations in this class need to use capital letters:
Set, List, String, Map, Integer

force2b · 2018-07-31T13:47:17Z

src/classes/OPP_AccountSoftCredit_TDTM.cls

+            Account_Soft_Credit__c accSCOld = listAccSCOld[i];
+
+            // don't allow manually changing the AccSC currency: it must be changed through the parent opportunity
+            if (userInfo.isMultiCurrencyOrganization() && accSCNew.get('CurrencyIsoCode') != accSCOld.get('CurrencyIsoCode') && blockCurrencyChange) {


Since this static var has a similar name to this method, I suggest specifying the class name for it: OPP_AccountSoftCredit_TDTM .blockCurrencyChange

I'd like to understand the rationale here a bit better at some point. Thanks.

force2b · 2018-07-31T13:53:36Z

src/classes/PSC_Opportunity_TDTM.cls

+        if (objType == Partial_Soft_Credit__c.SObjectType) {
+            //query all PSCs associated with these opps, update their ISO code
+            for (Partial_Soft_Credit__c psc : database.query('SELECT Id, Opportunity__c, CurrencyIsoCode FROM Partial_Soft_Credit__c WHERE Opportunity__c IN :oppIds')) {
+                string oppCurrency = mapOppIdToISO.get(psc.Opportunity__c);


Capitalization of var types and collections should be fixed in your new code/method.

force2b · 2018-07-31T14:26:56Z

src/classes/PSC_Opportunity_TDTM.cls


-            psc.put('CurrencyIsoCode', oppCurrency);
-            results.add(psc);
+                accountSC.put('CurrencyIsoCode', oppCurrency);


Seems that the code that updates the AccountSoftCredit and PartialSoftCredit Currency code is not actually working correctly.

In the example below, I started with a $151 USD Opportunity that had one AccountSoftCredit for $151, one PartialSoftCredit for $1 and one Payment for $151. I then changed the Opportunity.Currency to CAD (where the CAD exchange rate is 1.33). When saved, the Opportunity changes to CAD$151 (USD $113.53). The payment update does the same thing (i.e., the two amounts match). That is not the case for the AccountSoftCredit and PartialSoftCredit objects though. In these cases, the amounts changed to CAD$200.83 and CAD$1.33 respectively. See the PMT_Payment_TDTM.setPaymentCurrencyFromOpportunity method for an example of how this is handled to ensure the amount is correct. Basically, you have to update both the CurrencyIsoCode and the Amount fields.

force2b · 2018-07-31T20:22:44Z

src/classes/CRLP_AccountSkew_AccSoftCredit_BATCH.cls

+     * @description Scheduler execute method to support scheduling this job directly from the UI
+     */
+    public void execute(SchedulableContext context) {
+        String className = CRLP_AccountSkew_AccSoftCredit_BATCH.class.getName();


@bethbrains It seems that using .class.getName() returns back a namespaced class in the format of npsp.CRLP_AccountSkew_AccSoftCredit_BATCH as opposed to npsp__ CRLP_AccountSkew_AccSoftCredit_BATCH. This means that the hasRunningJob() method that attempts to strip out the namespace does not actually work properly in a namespaced org. That code should be changed to use something like className.substringAfter('.'); to get the part of the string after the ".". Even though it's a bug not specific to Account Soft Credits, I'm fine if you include that fix in here.

@bethbrains Just making sure you saw this above comment.

…er misc fixes

bethbrains · 2018-08-01T18:22:31Z

src/labels/CustomLabels.labels

@@ -4634,7 +4634,7 @@ To check your GAU Allocation settings, go to Donations | GAU Allocations.</value
        <language>en_US</language>
        <protected>false</protected>
        <shortDescription>pscManageSoftCreditsCantChangeCurrency</shortDescription>
-        <value>You can't modify Partial Soft Credit currencies directly. Update the parent Opportunity's currency, and NPSP will automatically update the curency of all related Partial Soft Credit records.</value>
+        <value>You can't modify soft credit currencies directly. Update the parent Opportunity's currency, and NPSP will automatically update the currency of all related soft credits.</value>


@smarovich FYI this small change because we're using the same error message on PSC and AccSC. OK?

force2b

Comments/Suggestions on updated code

force2b · 2018-08-01T19:07:01Z

src/classes/CRLP_RollupAccSoftCredit_TEST.cls

+ * Created by bbreisnes on 2018-07-31.
+ */
+
+public with sharing class CRLP_RollupAccSoftCredit_TEST {


This should be `private' without a sharing declaration to match the format of all the other CRLP unit tests. Generally there isn't a reason for unit tests to be public, and "with sharing" doesn't necessarily specify anything here because the methods cannot be called by other classes that might use "without sharing".

yep sorry, this was just a placeholder!! it'll go in correctly on tuesday.

force2b · 2018-08-01T19:53:31Z

src/classes/PSC_Opportunity_TDTM.cls


                psc.put('CurrencyIsoCode', oppCurrency);
+                psc.put('Amount__c', psc.Amount__c);


I'm always a bit wary when it comes to using non-namespaced field names in a string. It may work in a namespaced org, but it may fail. Given that we know the Amount__c field exists here (and below), it might be safer to just use psc.Amount__c = psc.Amount__c; (though we'll want to test that it still works as expected). The alternative is to call the UTIL_Namespace class to apply the namespace to the field.

force2b · 2018-08-01T19:53:57Z

src/labels/CustomLabels.labels

@@ -4634,7 +4634,7 @@ To check your GAU Allocation settings, go to Donations | GAU Allocations.</value
        <language>en_US</language>
        <protected>false</protected>
        <shortDescription>pscManageSoftCreditsCantChangeCurrency</shortDescription>
-        <value>You can't modify Partial Soft Credit currencies directly. Update the parent Opportunity's currency, and NPSP will automatically update the curency of all related Partial Soft Credit records.</value>
+        <value>You can't modify soft credit currencies directly. Update the parent Opportunity's currency, and NPSP will automatically update the currency of all related soft credits.</value>


The single apostrophes should be escaped.

…removed, trigger mode fix to exclude accSC rollups

bethbrains · 2018-08-08T21:58:42Z

@force2b all addressed and ready for final (!) review.

force2b

Looking absolutely fantastic. Two comments for you to review and possibly take action on, but otherwise approved!

force2b · 2018-08-08T22:25:52Z

src/classes/CRLP_AccountSkew_AccSoftCredit_BATCH.cls

+* @author Salesforce.org
+* @date 2018
+* @group Rollups
+* @group-content ../../ApexDocContent/Rollups2.htm


Can you update the Rollups2.htm file with the new classes?

force2b · 2018-08-08T22:30:45Z

src/classes/CRLP_Batch_Base_Skew.cls

@@ -217,6 +221,8 @@ public abstract class CRLP_Batch_Base_Skew extends CRLP_Batch_Base {
                parentKeyField = 'ContactId';
            } else if (this.jobType == CRLP_RollupProcessingOptions.RollupType.AccountContactSoftCredit) {
                parentKeyField = 'Contact.AccountId';
+            } else if (this.jobType == CRLP_RollupProcessingOptions.RollupType.AccountSoftCredit) {
+                parentKeyField = 'Account__c';


Not sure this will work in a namespaced context. The rest of the parentKeyField values are standard fields. This is the only one referencing a custom field. I feel like it should have the namespace applied, though I'm always a bit unsure of when the namespace is absolutely required for these kind of references. Probably worth testing in a namespaced scratch org to be sure it's working as expected.

…/rollups" This reverts commit d6d77ca, reversing changes made to df4db3e.

bethbrains and others added 28 commits May 18, 2018 12:40

Account Soft Credits object, ASCs in CRLP UI

58e9002

ASC object and page layout

addb2a1

small formatting tweaks, remove debugs

a6a49f4

Merge d6ce2e9 into feature/rollups__acct-soft-credits

da53341

Merge 4f63379 into feature/rollups__acct-soft-credits

34b18e7

Merge 842b7c5 into feature/rollups__acct-soft-credits

4c41218

Merge b0cdf11 into feature/rollups__acct-soft-credits

85c78b3

Merge d17d620 into feature/rollups__acct-soft-credits

e9f4779

Merge cf3dc77 into feature/rollups__acct-soft-credits

4faba1e

Merge 55f92f6 into feature/rollups__acct-soft-credits

0c5038c

Merge e391867 into feature/rollups__acct-soft-credits

5fa38ca

Merge a0e7897 into feature/rollups__acct-soft-credits

26fcaff

Merge 933fc5c into feature/rollups__acct-soft-credits

97045e7

Merge 6421a6d into feature/rollups__acct-soft-credits

62802d0

Merge 0035dcd into feature/rollups__acct-soft-credits

295569c

force merge

8c9a4ed

Merge b6dc7aa into feature/rollups__acct-soft-credits

e6e4575

Merge 2fdccf1 into feature/rollups__acct-soft-credits

e195463

Merge 82011f4 into feature/rollups__acct-soft-credits

441190b

Merge 740c96b into feature/rollups__acct-soft-credits

1d5758e

first pass ASC working

51e43f5

first pass ASC working v2

0ad5dab

account soft credit working POC fully tested

8d7c549

ASCs into default batch job scheduler

b53caf0

uncomment batches from panel controller

9359926

fixed queueable refactor for RDs

ed66e26

applied namespace to ASC obj

4dd4af4

more namespacing

6245c4d

bethbrains assigned force2b and njjc May 29, 2018

Merge ac88b42 into feature/rollups__acct-soft-credits

071494e

force2b requested changes Jul 31, 2018

View reviewed changes

force2b reviewed Jul 31, 2018

View reviewed changes

fixed currency sync, added role filter rule in validation script, oth…

9691916

…er misc fixes

bethbrains commented Aug 1, 2018

View reviewed changes

force2b reviewed Aug 1, 2018

View reviewed changes

mrbelvedere and others added 9 commits August 1, 2018 13:00

Merge 637f173 into feature/rollups

62356d6

Merge 62356d6 into feature/rollups__acct-soft-credits

825a622

Merge 91c3dd9 into feature/rollups

3b8585c

Merge 3b8585c into feature/rollups__acct-soft-credits

575d98e

Merge 3eb12ae into feature/rollups

df4db3e

Merge df4db3e into feature/rollups__acct-soft-credits

d15f647

additional apex tests, string split fix, labels escaped, unused vars …

f927d6f

…removed, trigger mode fix to exclude accSC rollups

force merge

aa00bdb

fixed CRLP scheduler test, cleaned up accSC test

e4c09b9

force2b changed the base branch from feature/rollups to master August 8, 2018 22:18

force2b approved these changes Aug 8, 2018

View reviewed changes

Kofi Johnson and others added 10 commits August 8, 2018 16:30

Merge branch 'feature/rollups__IncrementalBatch' into feature/rollups

d6d77ca

multicurrency updates to test script

6b51351

apexdoc updates, small ns fix in test

1244fb4

Revert "Merge branch 'feature/rollups__IncrementalBatch' into feature…

fc75f43

…/rollups" This reverts commit d6d77ca, reversing changes made to df4db3e.

Merge a7e4f46 into feature/rollups

30c010f

Merge 30c010f into feature/rollups__acct-soft-credits

49bdeac

Merge 497f774 into feature/rollups

821fec6

Merge 821fec6 into feature/rollups__acct-soft-credits

0ac8ca6

Merge a94cc0f into feature/rollups

cc2bf45

Merge cc2bf45 into feature/rollups__acct-soft-credits

65eb98c

dgamboni merged commit 2dafd17 into master Aug 14, 2018

dgamboni deleted the feature/rollups__acct-soft-credits branch August 14, 2018 00:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Account Soft Credits: Object and Rollups integration #3220

Account Soft Credits: Object and Rollups integration #3220

bethbrains commented May 29, 2018 •

edited by smarovich

force2b Jul 31, 2018

force2b Jul 31, 2018

force2b Jul 31, 2018

bethbrains Jul 31, 2018

force2b Aug 1, 2018

force2b Jul 31, 2018

force2b Jul 31, 2018

force2b Jul 31, 2018

bethbrains Jul 31, 2018

force2b Jul 31, 2018

force2b Jul 31, 2018

force2b Jul 31, 2018

force2b Aug 1, 2018

bethbrains Aug 8, 2018

bethbrains Aug 1, 2018

force2b left a comment

force2b Aug 1, 2018

bethbrains Aug 1, 2018

force2b Aug 1, 2018

force2b Aug 1, 2018

bethbrains commented Aug 8, 2018

force2b left a comment

force2b Aug 8, 2018

force2b Aug 8, 2018


		psc.put('CurrencyIsoCode', oppCurrency);
		psc.put('Amount__c', psc.Amount__c);

Account Soft Credits: Object and Rollups integration #3220

Account Soft Credits: Object and Rollups integration #3220

Conversation

bethbrains commented May 29, 2018 • edited by smarovich

Critical Changes

Account Soft Credits

Changes

Account Soft Credits

Issues Closed

New Metadata

Deleted Metadata

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

force2b left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bethbrains commented Aug 8, 2018

force2b left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bethbrains commented May 29, 2018 •

edited by smarovich