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

Move to bulk save. #681

Merged
merged 8 commits into from
Sep 19, 2024
Merged

Move to bulk save. #681

merged 8 commits into from
Sep 19, 2024

Conversation

jjbennett
Copy link
Contributor

@jjbennett jjbennett commented Sep 17, 2024

Critical Changes

Changes

Issues Closed

  • Fixes an issue where Bulk Service Delivery rollups caused a row lock error.

New Metadata

Deleted Metadata

Definition of Done

Refer to Asteroids DoD document to see any additional details for the items below

  • Any net new LWC work has Sa11y tests & 50% or above lines JEST test coverage
  • CRUD/FLS is enforced in Apex
  • Permission sets are updated to account for CRUD|FLS|Tab|Classes
  • Field sets are updated to account for new fields
  • UX approval or UX not necessary
  • Link the pull request and work item by PR comment and Chatter post respectively, e.g. GUS: W-0000000: Work Name (Reorganize code; use custom iterator #303)
  • All acceptance criteria have been met
    • Developer
    • Code Reviewer
    • QA
  • PR contains draft release notes
  • QE story level testing completed

Copy link

Thanks for the contribution! Unfortunately we can't verify the commit author(s): jjbennett <j***@j***.i***.s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Salesforce Inc. Contributor License Agreement and this Pull Request will be revalidated.

@jjbennett
Copy link
Contributor Author

W-14633962

Copy link
Contributor

@mldyang mldyang left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor

@AndersonTarren AndersonTarren left a comment

Choose a reason for hiding this comment

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

Gave it a look this morning, nice job. I think we'll need to get the test coverage to 100%.

@@ -186,6 +186,14 @@ const handleError = (error, fireShowToast = true, showToastMode, returnAsArray)
message = error.body.map(e => e.message).join(", ");
} else if (error.body && typeof error.body.message === "string") {
message = error.body.message;
} else if (Array.isArray(error)) {
// databse save result errors
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: databse -> database.

@@ -33,6 +33,20 @@ public with sharing class ServiceDeliveryController {
return true;
}

@AuraEnabled
Copy link
Contributor

Choose a reason for hiding this comment

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

I was expecting to see an Apex Test update in this PR based on this change. Same with the change to the domain class.

@jjbennett
Copy link
Contributor Author

jjbennett commented Sep 19, 2024

Thanks Tarren for the feedback.
I added the missing tests, should be back to 100%.
I did indeed need data-name, the OR in the querySelector did not work as I hoped.
I cleaned up unused code from the handling of submitting the form.
I fixed an issue where the button was no longer being disabled after Save was clicked.
I also missed displaying the summary toast of records saved/errored.
I checked the setDisabledAttribute and that only pertains to successful saves and has to do with disabling the comboboxes.
We can't reuse the setSaved method because in the error because one flag is different.

@AndersonTarren AndersonTarren self-requested a review September 19, 2024 17:09
Copy link
Contributor

@AndersonTarren AndersonTarren left a comment

Choose a reason for hiding this comment

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

Thanks for the review feedback updates, Jenny!

@mldyang mldyang merged commit 3819b42 into feature/254 Sep 19, 2024
12 of 16 checks passed
@mldyang mldyang deleted the feature/254__fix-bulk-save branch September 19, 2024 17:30
@salesforce-org-metaci salesforce-org-metaci bot mentioned this pull request Sep 19, 2024
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants