Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RD2: Consolidate Payment Widget State into App State object #6855

Merged
merged 34 commits into from
Feb 21, 2022

Conversation

bdvorachek
Copy link

@bdvorachek bdvorachek commented Feb 15, 2022

WI: https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00000cEQzOYAW/view

Critical Changes

Changes

Issues Closed

Community Ideas Delivered

Features Intended for Future Release

Features for Elevate Customers

New Metadata

Deleted Metadata

lparrott and others added 19 commits February 9, 2022 14:19
@bdvorachek bdvorachek changed the base branch from feature/238__rd2FormBillingAddress to feature/238 February 15, 2022 00:34
Benjamin Dvorachek added 2 commits February 15, 2022 14:08
…forceFoundation/NPSP into feature/238__rd-payment-state
…nts, update rd2EntryForm tests. Remove unused JSON mocks for getRecurringSettings/getRecurringData. Move crud/fls check into getInitialView.
@bdvorachek bdvorachek marked this pull request as ready for review February 15, 2022 23:37

this.isLoading = disableBtn;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change is preventing error messages from appearing, the Loading spinner stays visible, hiding the error.

Copy link
Author

@bdvorachek bdvorachek Feb 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I think we may need to add a line to handleError to disable the loading spinner to fully replicate the former behavior of setSaveButtonDisabled. Will see if that fix works and get a test in there for it if so.

Copy link
Contributor

@daniel-fuller daniel-fuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work Ben!

@AuraEnabled(Cacheable=true)
public static Boolean hasRequiredFieldPermissions() {
@TestVisible
private static Boolean hasRequiredFieldPermissions() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the collection of fields here, could we use String.valueOf(SObject.FieldName)? That way with other fields are added in the future that are not in the underlying packages, we don't have to worry about including namespace.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, changed this over!


view.hasRequiredPermissions = hasRequiredFieldPermissions();
if(!view.hasRequiredPermissions) {
return view;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What were your thoughts here regarding returning an empty view instead of an AuraHandledException like we do in other AuraEnabled methods in this class?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, it was closer to the original behavior of the component. I think we could do that here too and have that be our standard. Any AuraEnabled function that a user does not have the correct level of permissions for would throw an exception.

@lparrott lparrott self-requested a review February 17, 2022 22:55
@daniel-fuller daniel-fuller merged commit 220fca5 into feature/238 Feb 21, 2022
@daniel-fuller daniel-fuller deleted the feature/238__rd-payment-state branch February 21, 2022 20:38
@salesforce-org-metaci salesforce-org-metaci bot mentioned this pull request Feb 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants