Skip to content

Fixed potential bad method call and premature email sending in bulk asset checkout #16546

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

Merged
merged 6 commits into from
Mar 31, 2025

Conversation

marcusmoore
Copy link
Collaborator

This PR fixes a potential to run into a hard 500 and also fixes the possibility of preemptively sending checkout emails when the bulk checkout ultimately fails.

I originally started to work on this because I noticed we attempted to call getErrors() off of ModelNotFoundException but that method does not exist so we would get a 500 there.

Now, if an id included in the request that is not found the user is redirected back with an error message:
image

Along the way I noticed that there was a scenario where a user might get a checkout email even though the bulk checkout fails. For example if the request has [valid-id, invalid-id] the user would get a checkout email for the first asset, since the id was valid, but the transaction would ultimately revert upon hitting the ModelNotFoundException on the invalid-id. Moving Asset:findOrFail() up solves this issue.

Copy link

what-the-diff bot commented Mar 20, 2025

PR Summary

  • Improvements to the checkout process in our code
    • We made updating the assets you're checking out a bit easier by locating them beforehand, so you aren't having to go through each one by its ID.
    • We're now managing the errors a bit differently too--instead of pulling errors from the error that's being handled, we're using the selected assets from your request to identify errors.
  • New tests to make sure our checkout process is doing what it should be
    • We've created various tests to verify several elements of the checkout process.
    • That includes making sure you have the requisite permissions to check out in bulk, that the checkout is happening correctly, and that non-existent assets are handled correctly.
    • And to make sure nothing is falling through the cracks, we added checks to confirm that email notifications are being sent off after the checkout process is completed.

@snipe snipe merged commit 1394007 into grokability:develop Mar 31, 2025
9 checks passed
@marcusmoore marcusmoore deleted the bug/sc-28024 branch March 31, 2025 17:19
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.

2 participants