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

[Admin][Users] Add new admin store credits edit_amount flow #6031

Conversation

MadelineCollier
Copy link
Contributor

@MadelineCollier MadelineCollier commented Dec 11, 2024

Summary

This PR is for issue: #5824

The store credits flow is in the process of being significantly refactored so this is only part of the edit flow. It was a bit of a nightmare to get working but it's finally there, with error messages and all the different turbo refreshes working as desired. The path stays the same throughout. Again, no designs, so I did my best.

Screenshots

Screen.Recording.2024-12-11.at.5.21.21.PM.mov

Sometimes, as part of a form, you might want to include a field for
something which is not strictly part of the object you are updating.

In this instance it was a form for updating a StoreCredit, which needed
to include a select for a StoreCreditReason id. Since StoreCreditReason
is only linked to StoreCredit via a StoreCreditEvent, this select was
blowing up when we tried to use it (due to trying to call
store_credit.public_send(store_credit_reason_id) which is not defined).

This extra check allows this form field to be used more freely and
flexibly without immediately blowing up.
@MadelineCollier MadelineCollier changed the title admin user store credit refactor [Admin][Users] Add new admin store_credits edit_amount flow Dec 11, 2024
@MadelineCollier MadelineCollier changed the title [Admin][Users] Add new admin store_credits edit_amount flow [Admin][Users] Add new admin store credits edit_amount flow Dec 11, 2024
Copy link

codecov bot commented Dec 11, 2024

Codecov Report

Attention: Patch coverage is 98.21429% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.54%. Comparing base (2d5bda7) to head (2eefd30).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...trollers/solidus_admin/store_credits_controller.rb 97.61% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6031      +/-   ##
==========================================
+ Coverage   89.51%   89.54%   +0.02%     
==========================================
  Files         787      788       +1     
  Lines       18120    18174      +54     
==========================================
+ Hits        16220    16273      +53     
- Misses       1900     1901       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MadelineCollier MadelineCollier marked this pull request as ready for review December 11, 2024 16:44
@MadelineCollier MadelineCollier requested a review from a team as a code owner December 11, 2024 16:44
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks Madeline!

@MadelineCollier MadelineCollier merged commit dd5bbc4 into solidusio:main Dec 12, 2024
16 checks passed
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