Improve enterprise removal #13648
Draft
Improve enterprise removal #13648
+95
−27
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Make enterprise removal use turbo, which provides the following benefits: * More responsive removal since there's no full page reload. * A success flash message (previously nothing was displayed). * No double alert prompt. It also goes in the direction of removing mrujs in favor of turbo.
And not dependent on implementation details.
Make sure failure to delete due to dependent objects is handled through activemodel errors and not by rescuing `ActiveRecord::DeleteRestrictionError` exceptions. Previously we would display two alert prompts, and we would weirdly display the content of our 500 error page on top of the screen. Now, we display a flash error message explaining the reason to fail to remove it.
| @index = params[:index] | ||
|
|
||
| if @object.destroy | ||
| flash.now[:success] = Spree.t(:successfully_removed, resource: "Enterprise") |
There was a problem hiding this comment.
I noticed a small bug here, where this particular translation key is not found as expected.
In general there are a few flash message translation bugs which I'll address in a separate PR.
There was a problem hiding this comment.
I set #13649 as a dependency of this PR, because once that's merged, the system spec that I added will need to be updated to use the properly translated text. Setting this as a draft until that one lands.
sigmundpetersen
moved this from All the things 💤
to In Progress ⚙
in OFN Delivery board
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What? Why?
Removing enterprises has a few UI problems:
This PR fixes all those issues by moving enterprise removal from mrujs to turbo (which is a technical migration we want to do).
Before (success case)
After (success case)
Before (failure case)
After (failure case)
What should we test?
Release notes
Dependencies
#13649