From 5b1a59cef920d8daf43d748143c312f5885680d5 Mon Sep 17 00:00:00 2001 From: Sean Date: Tue, 19 Feb 2019 15:53:40 -0600 Subject: [PATCH 1/4] Adding else statement back in to show weight and dimensions on no-variant products --- backend/app/views/spree/admin/products/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/views/spree/admin/products/_form.html.erb b/backend/app/views/spree/admin/products/_form.html.erb index b145d9066fb..b0c6c1fef8c 100644 --- a/backend/app/views/spree/admin/products/_form.html.erb +++ b/backend/app/views/spree/admin/products/_form.html.erb @@ -109,7 +109,7 @@ <% end %> - + <% else %>
<% [:height, :width, :depth, :weight].each_with_index do |field, index| %>
From f8437cecb98f837d4c1124da35b3e41d099385b7 Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Mon, 18 Feb 2019 16:22:09 +0100 Subject: [PATCH 2/4] 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 c4b0709d177f89cf28b4d46d0adc29389885f795 Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Mon, 18 Feb 2019 16:58:40 +0100 Subject: [PATCH 3/4] 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 From 90311587176d14801a23d0995c5252d5609ef690 Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Tue, 19 Feb 2019 09:22:03 +0100 Subject: [PATCH 4/4] Remove destructive actions from migration 20180710170104 Removing tables and renaming columns are destructive actions as they change the DB schema in a way that is likely to be not compatible with previous code. This is especially dangerous when deploying multi-instance applications as migrations can run before all instances code is synced. When this happens, these instances may experience severe downtime due to high error rates. So destructive changes should be done in subsequent deploys: first add the new table and columns, then later remove the stale ones. Table `spree_store_credit_update_reasons` and its column `update_reason_id` will need to be removed in a future release of Solidus. --- ...create_spree_store_credit_reasons_table.rb | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/core/db/migrate/20180710170104_create_spree_store_credit_reasons_table.rb b/core/db/migrate/20180710170104_create_spree_store_credit_reasons_table.rb index 3ed6048390d..cd50bd414fc 100644 --- a/core/db/migrate/20180710170104_create_spree_store_credit_reasons_table.rb +++ b/core/db/migrate/20180710170104_create_spree_store_credit_reasons_table.rb @@ -21,15 +21,28 @@ def up StoreCreditReason.create!(name: update_reason.name) end - drop_table :spree_store_credit_update_reasons - rename_column :spree_store_credit_events, :update_reason_id, :store_credit_reason_id + add_column :spree_store_credit_events, :store_credit_reason_id, :integer + execute "update spree_store_credit_events set store_credit_reason_id = update_reason_id" + + # TODO: table spree_store_credit_update_reasons and column + # column spree_store_credit_update_reasons.update_reason_id + # must be dropped in a future Solidus release end def down - create_table :spree_store_credit_update_reasons do |t| - t.string :name - - t.timestamps + # This table and column may not exist anymore as another irreversible + # migration may have removed it later. They must be added back or the + # `up` method would fail + unless table_exists? :spree_store_credit_update_reasons + create_table :spree_store_credit_update_reasons do |t| + t.string :name + + t.timestamps + end + + unless column_exists? :spree_store_credit_events, :update_reason_id + add_column :spree_store_credit_events, :update_reason_id, :integer + end end StoreCreditReason.all.each do |store_credit_reason| @@ -37,6 +50,6 @@ def down end drop_table :spree_store_credit_reasons - rename_column :spree_store_credit_events, :store_credit_reason_id, :update_reason_id + remove_column :spree_store_credit_events, :store_credit_reason_id end end