Fix promotions admin UI inconsistency#5936
Closed
mamhoff wants to merge 3 commits intosolidusio:mainfrom
Closed
Conversation
github-actions
Bot
added
the
changelog:repository
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5936 +/- ##
=======================================
Coverage 89.53% 89.54%
=======================================
Files 782 782
Lines 17981 17987 +6
=======================================
+ Hits 16100 16106 +6
Misses 1881 1881 ☔ View full report in Codecov by Sentry. |
jarednorman
approved these changes
Nov 17, 2024
github-actions
Bot
added
changelog:solidus_promotions
mamhoff
force-pushed
the
fix-promotions-admin-ui-inconsistency
branch
from
9d41eea to
43a9dcd
Compare
There are three ways of activating a promotion, and they are mutually exclusive. This adds a few helper methods so we know when a promotion can have a code added, or promotion codes added, or whether it can change the `apply_automatically` Boolean.
Because promotion codes have the `-> {with_discarded}` scope on the
`promotion` association, a promo code that's instantiated with
`promotion.codes.new` will not have the promotion associated at the
beginning, leading to errors. This passes in the promotion so that the
promo code knows where it belongs.
This makes use of the previously defined methods to make the promotion activation form more user-friendly and add some features: - Activation is now a bootstrap menu - Only one option can ever be edited - Reintroduces feature where a promotion code batch is created with the promotion
mamhoff
force-pushed
the
fix-promotions-admin-ui-inconsistency
branch
from
43a9dcd to
7c2b843
Compare
tvdeyen
requested changes
Nov 18, 2024
Member
tvdeyen
left a comment
tvdeyen
left a comment
There was a problem hiding this comment.
I do not think a tabbed interface is the best approach here, because it might hide activation restrictions.
| <%= text_field_tag :single_code, @promotion.codes.first.try!(:value), class: "fullwidth", disabled: f.object.apply_automatically || f.object.path.present? %> | ||
| <div class="col-9"> | ||
| <div class="tab-content" id="v-pills-tabContent"> | ||
| <div class="tab-pane fade show active" id="v-pills-automatic" role="tabpanel" aria-labelledby="v-pills-automatic-tab"> |
Member
There was a problem hiding this comment.
Can we use a more useful name for the Tab id?
<div class="tab-pane fade show active" id="activation-tab-automatic" role="tabpanel" aria-labelledby="v-pills-automatic-tab">
| <%= label_tag :single_code, SolidusPromotions::PromotionCode.model_name.human %> | ||
| <%= text_field_tag :single_code, @promotion.codes.first.try!(:value), class: "fullwidth", disabled: f.object.apply_automatically || f.object.path.present? %> | ||
| <div class="col-9"> | ||
| <div class="tab-content" id="v-pills-tabContent"> |
Member
There was a problem hiding this comment.
This id does not seems to be necessary
Suggested change
| <div class="tab-content" id="v-pills-tabContent"> | |
| <div class="tab-content"> |
adammathys
reviewed
Nov 27, 2024
| @promotion = model_class.new(permitted_resource_params) | ||
| @promotion.codes.new(value: params[:single_code]) if params[:single_code].present? | ||
|
|
||
| @promotion.codes.new(promotion: @promotion, value: params[:single_code]) if params[:single_code].present? |
Member
There was a problem hiding this comment.
I think we can solve this by setting the inverse_of instead on the association. (I opened a PR that does just that when I encountered this error locally: #5953)
Contributor
Author
|
Let's close this for now. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
This improves the UI of the promotion edit page. The page now has an "activation" menu that allows for configuring either automatic, path-based, single-code or multiple-code activation, similar to how the legacy promotion system does it.
This PR also solves a bug with single-code promotions which previously could not be created without a persisted promotion.
After:
Before:
The following are mandatory for all PRs:
The following are not always needed: