From b8ce7099b2f4ee867ddb8e12e1aff88fb9275cdc Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Mon, 18 Feb 2019 16:22:09 +0100 Subject: [PATCH 1/2] Fix remove code from promotions with type set spree_promotions table used to be named spree_activators in legacy spree versions. This table was used by many models with STI via the type column. After it has been renamed into spree_promotions, having this type field does not make sense anymore. --- .../20190106184413_remove_code_from_spree_promotions.rb | 1 + ...190106184413_remove_code_from_spree_promotions_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/core/db/migrate/20190106184413_remove_code_from_spree_promotions.rb b/core/db/migrate/20190106184413_remove_code_from_spree_promotions.rb index 0d731aec7de..889ba214c7c 100644 --- a/core/db/migrate/20190106184413_remove_code_from_spree_promotions.rb +++ b/core/db/migrate/20190106184413_remove_code_from_spree_promotions.rb @@ -5,6 +5,7 @@ class RemoveCodeFromSpreePromotions < ActiveRecord::Migration[5.1] class Promotion < ActiveRecord::Base self.table_name = "spree_promotions" + self.ignored_columns = %w(type) end def up diff --git a/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb b/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb index 0f1a63d8be0..6db7014dd3d 100644 --- a/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb +++ b/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb @@ -96,6 +96,14 @@ end end + context 'with promotions with type set (legacy feature)' do + let(:promotion_with_code) { create(:promotion, type: 'Spree::Promotion') } + + it 'does not raise a STI error' do + expect { subject }.not_to raise_error + end + end + context 'when there is a Spree::PromotionCode with the same value' do context 'associated with the same promotion' do let!(:existing_promotion_code) { create(:promotion_code, value: 'just an old promo code', promotion: promotion_with_code) } From a1dfc602a58c1d0336ba34ac27385dc7e7167361 Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Mon, 18 Feb 2019 16:58:40 +0100 Subject: [PATCH 2/2] Fix remove code from promotion with multiple empty codes Promotions could have code nil, but also an empty string. This commit takes into account this scenario since we don't want to take any action on promotions with code nil or empty. --- ...84413_remove_code_from_spree_promotions.rb | 2 +- ..._remove_code_from_spree_promotions_spec.rb | 20 +++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/core/db/migrate/20190106184413_remove_code_from_spree_promotions.rb b/core/db/migrate/20190106184413_remove_code_from_spree_promotions.rb index 889ba214c7c..f4a03afa534 100644 --- a/core/db/migrate/20190106184413_remove_code_from_spree_promotions.rb +++ b/core/db/migrate/20190106184413_remove_code_from_spree_promotions.rb @@ -9,7 +9,7 @@ class Promotion < ActiveRecord::Base end def up - promotions_with_code = Promotion.where.not(code: nil) + promotions_with_code = Promotion.where.not(code: [nil, '']) if promotions_with_code.any? # You have some promotions with "code" field present! This is not good diff --git a/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb b/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb index 6db7014dd3d..1ad32aee15f 100644 --- a/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb +++ b/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb @@ -44,7 +44,18 @@ DatabaseCleaner.clean_with(:truncation) end + let(:promotion_with_code) { create(:promotion) } + + before do + # We can't set code via factory since `code=` is currently raising + # an error, see more at: + # https://github.com/solidusio/solidus/blob/cf96b03eb9e80002b69736e082fd485c870ab5d9/core/app/models/spree/promotion.rb#L65 + promotion_with_code.update_column(:code, code) + end + context 'when there are no promotions with code' do + let(:code) { '' } + it 'does not call any promotion with code handler' do expect(described_class).not_to receive(:promotions_with_code_handler) @@ -53,14 +64,7 @@ end context 'when there are promotions with code' do - let(:promotion_with_code) { create(:promotion) } - - before do - # We can't set code via factory since `code=` is currently raising - # an error, see more at: - # https://github.com/solidusio/solidus/blob/cf96b03eb9e80002b69736e082fd485c870ab5d9/core/app/models/spree/promotion.rb#L65 - promotion_with_code.update_column(:code, 'Just An Old Promo Code') - end + let(:code) { 'Just An Old Promo Code' } context 'with the deafult handler (Solidus::Migrations::PromotionWithCodeHandlers::RaiseException)' do it 'raise a StandardError exception' do