-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Prototypes move to extension #1517
Prototypes move to extension #1517
Conversation
@@ -94,8 +91,6 @@ def find_or_build_master | |||
validates :shipping_category_id, presence: true | |||
validates :slug, length: { minimum: 3 }, uniqueness: { allow_blank: true } | |||
|
|||
attr_accessor :option_values_hash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be removed, it's a parameter accepted through the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 added back in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in options
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup - sorry - fixed
94af729
to
93486c7
Compare
@@ -74,9 +74,6 @@ def find_or_build_master | |||
|
|||
has_many :variant_images, -> { order(:position) }, source: :images, through: :variants_including_master | |||
|
|||
after_create :add_associations_from_prototype | |||
after_create :build_variants_from_option_values_hash, if: :option_values_hash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to keep this as we ll since we are keeping option_values_hash
# option_values_hash. | ||
# | ||
# @return [Array] the option_values | ||
def ensure_option_types_exist_for_values_hash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep
@@ -310,21 +276,6 @@ def any_variants_not_track_inventory? | |||
end | |||
end | |||
|
|||
# Builds variants from a hash of option types & values | |||
def build_variants_from_option_values_hash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep
Added some comments on the extension: solidusio-contrib/solidus_prototypes@9c36e3e#commitcomment-19425150 |
Previously, @prototype was conditionally (if the corresponding ID as included in the params) in a before create callback on the product. This functionality has be moved out into a Prototypes extension.
93486c7
to
4ffd596
Compare
@jhawthorn I've made those requested changes. @tvdeyen Implemented your suggestions on the extension. Thanks for the feedback, you two! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good by me! 🎆 for smaller core
Thanks @gevann |
This PR removes prototypes from Solidus. The prototype functionality can be added back in to solidus with the
solidus_prototype
extension. Fixes issue #1481