Fix for Manage HH Page when Undeliverable Address permissions are missing #6749

lparrott · 2021-11-22T21:18:36Z

Critical Changes

Changes

We fixed an issue with the Manage Household page where the page wouldn't load without permissions to Undeliverable Address fields.

Issues Closed

Fixes #6733

Community Ideas Delivered

Features Intended for Future Release

Features for Elevate Customers

New Metadata

Deleted Metadata

…are missing

lparrott · 2021-11-22T21:20:04Z

force-app/main/default/classes/HH_Container_LCTRL.cls

- String.valueOf(Contact.SObjectType), String.valueOf(Contact.Undeliverable_Address__c)),
- String.valueOf(Address__c.Undeliverable__c) => UTIL_Describe.getFieldLabel(
- String.valueOf(Address__c.SObjectType), String.valueOf(Address__c.Undeliverable__c))
+ 'Undeliverable_Address__c' => undeliverableAddressLabel,


I changed this to avoid additional permissions checking in the front end, the Label will just be blank in cases where permissions are missing.

force-app/main/default/aura/HH_Container/HH_ContainerHelper.js

force2b · 2021-11-22T21:36:56Z

force-app/main/default/classes/HH_Container_LCTRL.cls

@@ -615,8 +640,10 @@ public with sharing class HH_Container_LCTRL {
 address.MailingState__c = (String)household.get('BillingState');
 address.MailingPostalCode__c = (String)household.get('BillingPostalCode');
 address.MailingCountry__c = (String)household.get('BillingCountry');
- address.Undeliverable__c = (Boolean)household.get(
- UTIL_Namespace.StrTokenNSPrefix('Undeliverable_Address__c'));
+ if (canReadAddressUndeliverable && canReadAccountUndeliverable) {


Does this check (and the ones below) also need for canUpdate?

During my testing, I didn't see a case where the User can explicitly set the Undeliverable checkbox through this page. If changing an Address causes a record to be set as Undeliverable, that code should run without sharing via the TDTM trigger, not in the Manage Household logic.

Interesting. If the page doesn't let the user modify the field value, why does the controller allow updating the field value?

Ha, I walked away and thought the same thing. Since the Apex is updating values, the User will need that Perm as well. I can't speak to the "why", maybe some of this can be rewritten now that we're allowing use of the form without Undeliverable Address permissions, this first pass was just to get it working.

…erable field

force2b

What do you think about adding a Unit test for the new condition?

…thout Undeliverable Address permissions

…rceFoundation/NPSP into feature/fixManageHHPerms

force-app/main/default/classes/HH_Container_LCTRL.cls

bdvorachek · 2021-11-29T17:12:59Z

force-app/main/default/aura/HH_Container/HH_ContainerHelper.js

@@ -474,7 +476,9 @@
 con.MailingState = addr.MailingState__c;
 con.MailingPostalCode = addr.MailingPostalCode__c;
 con.MailingCountry = addr.MailingCountry__c;
- con.Undeliverable_Address__c = addr.Undeliverable__c === undefined ? false : addr.Undeliverable__c;
+ if(con.hasOwnProperty('Undeliverable_Address__c') && addr.hasOwnProperty('Undeliverable__c')){
+ con.Undeliverable_Address__c = addr.Undeliverable__c === undefined ? false : addr.Undeliverable__c;


In the event that a user doesn't have access/permissions to the Undeliverable field, its value wouldn't get copied here. Wanted to double check to make sure that we're ok with that here?

Yeah, I'm questioning this now, but the update call on Contact will fail is this field gets changed without permissions, so we'd need to include Update access to the Undeliverable field on Contact in our permissions check.

force-app/main/default/aura/HH_Container/HH_ContainerHelper.js

force-app/main/default/classes/HH_Container_LCTRL.cls

…ssfully when the permissions error is thrown.

daniel-fuller · 2021-11-29T19:19:04Z

Talked to @lparrott and I just pushed e6a06db to address the spinner not clearing when the permissions error is thrown. FYI @bdvorachek I believed you flagged this when we first looked at this. This should now at least keep the page from being inaccessible whenever the user doesn't have correct FLS. The issue here is that we were referencing a self variable in the setCallback() and that variable was not declared so when the page loaded an unhandled aura exception was thrown with a message that read, "self.reportError is not a function", and this is the error which kept the spinner overlay from clearing.

…ndeliverable value to a new Address if it was changed

…rceFoundation/NPSP into feature/fixManageHHPerms

force2b · 2021-11-30T15:53:26Z

force-app/main/default/classes/HH_Container_TEST.cls

+ OR Field = : accountUndeliverableField
+ )
+ ];
+ for(FieldPermissions fieldPermission : fieldPermissions){


Is it worth testing some other combinations, such as:

Edit Access to the Address.Undeliverable field, but only Read to the Account/Contact fields

Read Access to the Address.Undeliverable field, but no to the Account/Contact fields

No Access to the Address.Undeliverable field, but Edit to the Account/Contact fields

There's obviously a lot of permutations. The question is how many of them we want to test to be sure this isn't going to still generate unexpected errors for customers.

I added one additional test that retains Account.Undeliverable access, since I needed to query that field as part of the test, I'm not sure testing every combination is worth it here, unless we find issues.

We're more concerned with data integrity than unexpected errors in this situation. The problem we were seeing is that the Undeliverable field would get set to an unexpected value if you didn't have permissions to any of the fields, because we relied on field visibility to drive the automated updates.

…nc without permissions

…gressions

daniel-fuller

looks good. thanks Luke!

…ssions are set

Fix for Manage Household Page when Undeliverable Address permissions …

865191c

…are missing

lparrott requested review from force2b and npsp-reedestockton November 22, 2021 21:18

lparrott commented Nov 22, 2021

View reviewed changes

force2b reviewed Nov 22, 2021

View reviewed changes

Manage Household fix: Adding permission check to Edit Address Undeliv…

83f0893

…erable field

force2b reviewed Nov 23, 2021

View reviewed changes

salesforce-org-metaci bot and others added 3 commits November 23, 2021 16:53

Merge d87ce0e into feature/fixManageHHPerms

2413908

Manage Household fix: Adding Unit test to check controller methods wi…

578d8e6

…thout Undeliverable Address permissions

Merge branch 'feature/fixManageHHPerms' of https:/Salesfo…

99ed196

…rceFoundation/NPSP into feature/fixManageHHPerms

npsp-reedestockton requested changes Nov 23, 2021

View reviewed changes

force-app/main/default/classes/HH_Container_LCTRL.cls Show resolved Hide resolved

force-app/main/default/classes/HH_Container_LCTRL.cls Outdated Show resolved Hide resolved

force-app/main/default/classes/HH_Container_LCTRL.cls Outdated Show resolved Hide resolved

lparrott requested a review from force2b November 23, 2021 22:27

npsp-reedestockton approved these changes Nov 23, 2021

View reviewed changes

salesforce-org-metaci bot added 2 commits November 24, 2021 02:21

Merge 48038e5 into feature/fixManageHHPerms

95e09db

Merge 914ec5f into feature/fixManageHHPerms

bdec77b

bdvorachek reviewed Nov 29, 2021

View reviewed changes

force2b reviewed Nov 29, 2021

View reviewed changes

force-app/main/default/aura/HH_Container/HH_ContainerHelper.js Outdated Show resolved Hide resolved

force-app/main/default/classes/HH_Container_LCTRL.cls Show resolved Hide resolved

add self variable reference so that the page spinner will clear succe…

e6a06db

…ssfully when the permissions error is thrown.

lparrott added 2 commits November 29, 2021 16:53

Manage Household Undeliverable Address fix: only pass the Account's U…

39009c1

…ndeliverable value to a new Address if it was changed

Merge branch 'feature/fixManageHHPerms' of https:/Salesfo…

030155f

…rceFoundation/NPSP into feature/fixManageHHPerms

force2b reviewed Nov 30, 2021

View reviewed changes

lparrott added 2 commits November 30, 2021 21:45

Manage Household Undeliverable Fix, making sure checkboxes stay in sy…

4c4d9ff

…nc without permissions

Manage Household fix: Adding Unit test and updating logic to avoid re…

717492a

…gressions

bdvorachek approved these changes Dec 1, 2021

View reviewed changes

daniel-fuller approved these changes Dec 2, 2021

View reviewed changes

lparrott and others added 4 commits December 2, 2021 10:12

Manage HH permissions fix: removing unneeded hasOwnProperty check in JS

d2645ec

Attempt to fix Manage HH QA issues

3c3146a

Manage Household fixes: Null pointer when merging, Unit test fix

0d412fd

Merge 8ff19b8 into feature/fixManageHHPerms

ae0d2bf

salesforce-org-metaci bot and others added 3 commits December 7, 2021 16:49

Merge 18d9486 into feature/fixManageHHPerms

698f6dc

Manage HH updates to only show Undeliverable Status when proper permi…

bedf351

…ssions are set

Merge branch 'main' into feature/fixManageHHPerms

a727202

npsp-reedestockton merged commit d521858 into main Dec 16, 2021

npsp-reedestockton deleted the feature/fixManageHHPerms branch December 16, 2021 01:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for Manage HH Page when Undeliverable Address permissions are missing #6749

Fix for Manage HH Page when Undeliverable Address permissions are missing #6749

lparrott commented Nov 22, 2021

lparrott Nov 22, 2021

force2b Nov 22, 2021

lparrott Nov 22, 2021

force2b Nov 22, 2021

lparrott Nov 22, 2021

force2b left a comment

bdvorachek Nov 29, 2021

lparrott Nov 29, 2021

daniel-fuller commented Nov 29, 2021 •

edited

Loading

force2b Nov 30, 2021

lparrott Dec 1, 2021

bdvorachek Dec 1, 2021

daniel-fuller left a comment

Fix for Manage HH Page when Undeliverable Address permissions are missing #6749

Fix for Manage HH Page when Undeliverable Address permissions are missing #6749

Conversation

lparrott commented Nov 22, 2021

Critical Changes

Changes

Issues Closed

Community Ideas Delivered

Features Intended for Future Release

Features for Elevate Customers

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

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

daniel-fuller commented Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniel-fuller left a comment

Choose a reason for hiding this comment

daniel-fuller commented Nov 29, 2021 •

edited

Loading