Add required to product master price field #2262
Merged
Conversation
This file contains 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
gmacdougall
approved these changes
Oct 6, 2017
jhawthorn
suggested changes
Oct 11, 2017
| @@ -32,7 +32,7 @@ | |||
| <div data-hook="admin_product_form_price"> | |||
| <%= f.field_container :price do %> | |||
| <%= f.label :price, class: 'required' %> | |||
| <%= render "spree/admin/shared/number_with_currency", f: f, amount_attr: :price, | |||
| <%= render "spree/admin/shared/number_with_currency", f: f, amount_attr: :price, required: true, | |||
There was a problem hiding this comment.
We have a Spree::Config.require_master_price (which I don't know how widely is used but exists).
Can we change this line to required: Spree::Config.require_master_price?
gregdaynes
force-pushed
the
require-master-variant-price
branch
from
d6fb428
to
7d99f72
Compare
Depending on Spree::Config.require_master_price, this adds `required: true` to the master price field on the products form, as well as the indicator to the field label. When config is set to false, the field can be cleared, and the form submitted - resulting in the price being defaulted to `0.00` This is part of issue solidusio#1861. Though in the issue report it talks of the product price showing a deleted value when removed (which has been resolved). This further makes it a conscious decision to set the price to `0`.
gregdaynes
force-pushed
the
require-master-variant-price
branch
from
7d99f72
to
0ecd93e
Compare
jhawthorn
approved these changes
Oct 14, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This adds
required: trueto the master price field on the productsform. The label for the field indicates that it is required, but the
field can be cleared, and the form submitted - resulting in the price
being defaulted to
0.00This is part of issue #1861. Though in the issue report it talks of the
product price showing a deleted value when removed (which has been
resolved). This further makes it a conscious decision to set the price
to
0.