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

Deprecate dangling option_values and duplicated routes #4385

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Deprecate the creation of option_values without an option_type
It makes no sense to have dangling option_values without an associated
option_type.

Ref #3309 & #3517
  • Loading branch information
waiting-for-dev committed May 26, 2022
commit 3dcc3fae3075fff18f3bb216e2a9a29e3a42e128
7 changes: 7 additions & 0 deletions api/app/controllers/spree/api/option_values_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ def show
end

def create
Spree::Deprecation.warn <<~MSG unless request.path.include?('option_types')
This route is deprecated, as it'll be no longer possible to create an
option_value without an associated option_type. Please, use instead:

POST api/option_types/{option_type_id}/option_values
MSG

authorize! :create, Spree::OptionValue
@option_value = scope.new(option_value_params)
if @option_value.save
Expand Down
1 change: 1 addition & 0 deletions api/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
resources :option_types do
resources :option_values
end
# TODO: Remove :create once option_type is required on Solidus v4.0
resources :option_values

get '/orders/mine', to: 'orders#mine', as: 'my_orders'
Expand Down
2 changes: 2 additions & 0 deletions api/openapi/solidus-api.oas.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2544,6 +2544,8 @@ paths:
$ref: '#/components/responses/unprocessable-entity'
summary: Create option value
description: |-
**DEPRECATED**: Use *POST /option_types/{option_type_id}/option_values* instead

Creates an option value.

Only users with the `create` permission on `Spree::OptionValue` can perform this action.
Expand Down
4 changes: 3 additions & 1 deletion api/spec/requests/spree/api/option_values_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@ module Spree::Api
expect(option_value.name).to eq("Option Value")
end

it "can create an option value" do
it "can create but deprecates creating an option value without option type" do
expect(Spree::Deprecation).to receive(:warn).with(/deprecated/).at_least(:once)

post spree.api_option_values_path, params: { option_value: {
name: "Option Value",
presentation: 'option value'
Expand Down
9 changes: 9 additions & 0 deletions core/app/models/spree/option_value.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

module Spree
class OptionValue < Spree::Base
# TODO: Remove optional on Solidus v4.0. Don't forget adding a migration to
# enforce at the database layer
belongs_to :option_type, class_name: 'Spree::OptionType', inverse_of: :option_values, optional: true
acts_as_list scope: :option_type

Expand All @@ -13,7 +15,14 @@ class OptionValue < Spree::Base

after_save :touch, if: :saved_changes?
after_touch :touch_all_variants
after_save do
Spree::Deprecation.warn <<~MSG if option_type.nil?
Having an option_value with no associated option_type will be deprecated
on Solidus v4.0
MSG
end

# TODO: Remove allow_nil once option_type is required on Solidus v4.0
delegate :name, :presentation, to: :option_type, prefix: :option_type, allow_nil: true

self.whitelisted_ransackable_attributes = %w[name presentation]
Expand Down
6 changes: 6 additions & 0 deletions core/spec/models/spree/option_value_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,10 @@
expect(subject).to eq "Size - S"
end
end

it 'deprecates creating an option_value with no associated option_type' do
expect(Spree::Deprecation).to receive(:warn).with(/deprecated/)

create(:option_value, option_type: nil)
end
end