Skip to content

Commit

Permalink
Deprecate the creation of option_values without an option_type
Browse files Browse the repository at this point in the history
It makes no sense to have dangling option_values without an associated
option_type.

Ref solidusio#3309 & solidusio#3517
  • Loading branch information
waiting-for-dev authored and cpfergus1 committed Aug 25, 2022
1 parent 36db282 commit 5bcc3e4
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 1 deletion.
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

0 comments on commit 5bcc3e4

Please sign in to comment.