[Admin][Users] Add new admin store credits create flow #6036
Conversation
MadelineCollier
changed the title
|
Spec failures are unrelated as the |
Some naming issue in the paypal extension. Taking a look.... |
github-actions
bot
added
changelog:solidus_legacy_promotions
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6036 +/- ##
==========================================
- Coverage 92.50% 89.59% -2.91%
==========================================
Files 384 791 +407
Lines 8362 18269 +9907
==========================================
+ Hits 7735 16368 +8633
- Misses 627 1901 +1274 ☔ View full report in Codecov by Sentry. |
This was just some leftover WIP code that would have impacted the error
state of the controller. Now the flash messages will display the correct
success and failure states. This wasn't caught by the specs as the old
busted code (now removed) would have just displayed both `flash[:notice]
= t('.success')` (and also) `flash[:error] = t('.failure')`
and this would match on the assertion:
```
expect(response.body).to
include("Something went wrong. Store credit could not be invalidated.")
```
Since they are set using different keys, they didn't overwrite each
other in the flash hash.
Regardless, the old WIP code has been removed and the flow now displays
the expected flash.
Previously these were only used on edit actions which was fine because the `render_edit_with_errors` method is able to accommodate both the `update_amount` and `invalidate` actions. However, I want to re-use the `ensure_amount` method inside of the new/create flow. Since new/create flow would need to render a different template (eg. `render_new_with_errors`) I have altered the ensure method to accept a block instead of being hard-coded to `render_edit_with_errors`.
MadelineCollier
force-pushed
the
admin-user-store-credit-create
branch
from
9e9e36d to
0517f29
Compare
github-actions
bot
removed
changelog:solidus_legacy_promotions
There was a problem hiding this comment.
Thanks for finishing this.
I think next step should be to sit back and think about how we want to continue with the new admin.
Most of the code we introduce in every new admin PR introduces lots of code unrelated to the actual feature. It's mostly a copy and paste of the same code over and over again.
With every new admin index component we currently make it harder to refactor the code in the future.
We repeat ourselves a lot currently. Which was fine in the early stages of the admin, but now it starts getting a burden for us.
Could we please now think about introducing a resource CRUD controller (plus a set of reusable components) like we have in the current backend?
I am happy to help and thanks again for your work here.
|
@MadelineCollier we usually wait for a second approval before we merge a PR. |
Summary
This PR is for issue: #5824
This is the 🎉 last piece 🎉 of the refactored store credits flow! This completes the extraction, migration, and refactoring of the store credits flow from the legacy
solidus_backendto the newsolidus_admin. This also completes the users admin more generally.Screenshots
Screen.Recording.2024-12-17.at.10.19.12.AM.mov
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: