Skip to content

Commit

Permalink
[API][Extract to PR?] Enforce creating option values with an option type
Browse files Browse the repository at this point in the history
Add a NOT NULL validation at the database level on top
of removing the existing deprecation warning.

Ref solidusio#4385.
  • Loading branch information
kennyadsl committed Apr 14, 2023
1 parent d7b0d0b commit 71f1c7d
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 15 deletions.
13 changes: 2 additions & 11 deletions core/app/models/spree/option_value.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

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
belongs_to :option_type, class_name: 'Spree::OptionType', inverse_of: :option_values
acts_as_list scope: :option_type

has_many :option_values_variants, dependent: :destroy
Expand All @@ -15,15 +13,8 @@ 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
delegate :name, :presentation, to: :option_type, prefix: :option_type

self.allowed_ransackable_attributes = %w[name presentation]

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class ChangeColumnNullOptionValuesOptionTypeId < ActiveRecord::Migration[5.2]
def change
change_column_null(:spree_option_values, :option_type_id, false)
end
end
8 changes: 4 additions & 4 deletions core/spec/models/spree/option_value_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@
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)
it 'raises when creating an option_value with no associated option_type' do
expect {
create(:option_value, option_type: nil)
}.to raise_error(ActiveRecord::RecordInvalid)
end
end

0 comments on commit 71f1c7d

Please sign in to comment.