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.

Sign up for GitHub

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

Jump to bottom

fix data corruption during delete of soft credits #4138

Merged
merged 15 commits into from
Apr 2, 2019
Merged

fix data corruption during delete of soft credits #4138

merged 15 commits into from
Apr 2, 2019

Conversation

patrick-yan-sf
Copy link
Contributor

@patrick-yan-sf patrick-yan-sf commented Mar 21, 2019 •
edited by melissabarber
Loading

Turned on validation before clicking on delete button and added a new attribute to the UTIL_FormField component so that validation doesn't prevent deleting of newly created records. No release note needed for this - no change in the UI or existing behavior.

Critical Changes

Changes

Issues Closed

New Metadata

  • Custom Label: pscManageSoftCreditsContactMissing

Deleted Metadata

@patrick-yan-sf
Copy link
Contributor Author

**lurch: attach W-031957

@LurchTheButler
Copy link

Tracking W-031957

sam-knox
sam-knox requested changes Mar 25, 2019
View reviewed changes
Copy link
Contributor

@sam-knox sam-knox left a comment •
edited
Loading

Choose a reason for hiding this comment

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

In the custom label, please capitalize Contact so the line reads:

"You must enter a Contact for each soft credit."

Also change Please to You must.

Line 5286 in CustomLabels.

Patrick Yan added 2 commits March 25, 2019 09:37
@patrick-yan-sf patrick-yan-sf requested a review from sam-knox March 25, 2019 16:39
@patrick-yan-sf
Copy link
Contributor Author

@sam-knox Thanks - I've fixed the label text

sam-knox
sam-knox requested changes Mar 25, 2019
View reviewed changes
Copy link
Contributor

@sam-knox sam-knox left a comment

Choose a reason for hiding this comment

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

Sorry, one more change. Can you change "Please" to "You must"?

@patrick-yan-sf
Copy link
Contributor Author

@sam-knox how does it look now?

sam-knox
sam-knox approved these changes Mar 25, 2019
View reviewed changes
Copy link
Contributor

@sam-knox sam-knox left a comment

Choose a reason for hiding this comment

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

That's great, thank you!

@sam-knox
Copy link
Contributor

@patrick-yan-sf - Can you look at the release note I wrote for this one? Did I summarize all the changes or is there another validation scenario in addition to the null Contact field one?

seanpat09
seanpat09 reviewed Mar 27, 2019
View reviewed changes
Copy link
Contributor

@seanpat09 seanpat09 left a comment

Choose a reason for hiding this comment

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

Looks pretty good, just one question on the rendering of the required label

src/components/UTIL_FormField.component Outdated
@@ -55,7 +56,7 @@
rendered="{!($ObjectType[sObjType].fields[field].updateable
|| $ObjectType[sObjType].fields[field].createable && sObj['Id']=='') && $ObjectType[sObjType].fields[field].type != 'boolean'}">
<apex:outputLabel for="{!$Component.inputx.util_inputfield.inputx}" styleClass="slds-form-element__label" rendered="{!showLabel == true && ftype!='boolean'}" id="lblInputField">
<apex:outputText rendered="{!IF(required == true, true, NOT($ObjectType[sObjType].fields[field].nillable))}" id="lblInputFieldRequired">
<apex:outputText rendered="{!IF(overrideRequired, false, IF(required == true, true, NOT($ObjectType[sObjType].fields[field].nillable)))}" id="lblInputFieldRequired">
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the field still look required since it's still being required by the backend validation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think you're right - it should still say required as it is required on the backend.
Thanks

seanpat09
seanpat09 approved these changes Mar 27, 2019
View reviewed changes
Copy link
Contributor

@seanpat09 seanpat09 left a comment

Choose a reason for hiding this comment

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

Looks great @patrick-yan-sf, nice work!

@patrick-yan-sf
Copy link
Contributor Author

@rponti-sforg - This is ready for QA. Thanks

rponti-sforg
rponti-sforg approved these changes Apr 1, 2019
View reviewed changes
Copy link
Contributor

@rponti-sforg rponti-sforg left a comment

Choose a reason for hiding this comment

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

QE testing is complete for this PR. Tests have passed.

For further testing details, see: https://foundation.lightning.force.com/lightning/r/agf__ADM_Work__c/a2x1E000008LlgvQAC/view

Merging PR and deleting branch.

@rponti-sforg rponti-sforg merged commit 30eeb7d into master Apr 2, 2019
@rponti-sforg rponti-sforg deleted the feature/pyan/manage-psc-page-bug branch April 2, 2019 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seanpat09 seanpat09 seanpat09 approved these changes

@sam-knox sam-knox sam-knox approved these changes

@rponti-sforg rponti-sforg rponti-sforg approved these changes

Assignees

@rponti-sforg rponti-sforg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@patrick-yan-sf @LurchTheButler @sam-knox @seanpat09 @rponti-sforg @mrbelvedere