From f3f58034b7b93be9f565b07bae357c1ea92c2d1a Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Wed, 10 Mar 2021 16:56:39 +0100 Subject: [PATCH 1/2] Remove 2.11 migration tasks They should be ran with the code present in Solidus 2.11 so it's useless to keep them here as well. --- .../migrations/migrate_address_names.rake | 158 ------------------ ...ult_billing_addresses_to_address_book.rake | 38 ----- .../migrations/migrate_address_names_spec.rb | 93 ----------- ..._billing_addresses_to_address_book_spec.rb | 103 ------------ 4 files changed, 392 deletions(-) delete mode 100644 core/lib/tasks/migrations/migrate_address_names.rake delete mode 100644 core/lib/tasks/migrations/migrate_default_billing_addresses_to_address_book.rake delete mode 100644 core/spec/lib/tasks/migrations/migrate_address_names_spec.rb delete mode 100644 core/spec/lib/tasks/migrations/migrate_default_billing_addresses_to_address_book_spec.rb diff --git a/core/lib/tasks/migrations/migrate_address_names.rake b/core/lib/tasks/migrations/migrate_address_names.rake deleted file mode 100644 index 7ee4113a560..00000000000 --- a/core/lib/tasks/migrations/migrate_address_names.rake +++ /dev/null @@ -1,158 +0,0 @@ -# frozen_string_literal: true - -require 'thor' - -namespace :solidus do - namespace :migrations do - namespace :migrate_address_names do - desc 'Backfills Spree::Address name attribute using firstname and lastname - concatenation in order to retain historical data when upgrading to new - address name format' - task up: :environment do - puts "Combining addresses' firstname and lastname into name ... " - class Spree::AddressForMigration < ApplicationRecord - self.table_name = 'spree_addresses' - end - - records = Spree::AddressForMigration.unscoped.where(name: [nil, '']) - count = records.count - connection = ActiveRecord::Base.connection - adapter_name = connection.adapter_name.downcase - shell = Thor::Shell::Basic.new - puts " Your DB contains #{count} addresses that may be affected by this task." - # `trim` is not needed for pg or mysql when using `concat_ws`: - # select concat_ws('joinstring', 'foo', null); - # concat_ws - # ----------- - # foo - # (1 row) - # select concat_ws('joinstring', 'foo', null) = trim(concat_ws('joinstring', 'foo', null)); - # ?column? - # ---------- - # t - # (1 row) - unless count.zero? - concat_statement = begin - case adapter_name - when /sqlite/ - "name = TRIM(COALESCE(firstname, '') || ' ' || COALESCE(lastname, ''))" - when /postgres/, /mysql2/ - "name = CONCAT_WS(' ', firstname, lastname)" - else - abort " No migration path available for adapter #{adapter_name}. Please write your own." - end - end - - # The batch size should be limited to avoid locking the table records for too long. These are - # the numbers I got with 1_000_000 records in `spree_addresses`, all with different name and - # surname, with postgresql: - # - # Updating 1000000 records in one shot - # batch took 178 seconds - # - # Updating 1000000 addresses in batches of 200000 - # batch took 36 seconds - # batch took 31 seconds - # batch took 31 seconds - # batch took 31 seconds - # batch took 30 seconds - # - # Updating 1000000 addresses in batches of 150000 - # batch took 29 seconds - # batch took 27 seconds - # batch took 27 seconds - # batch took 27 seconds - # batch took 26 seconds - # batch took 26 seconds - # batch took 19 seconds - # - # Updating 1000000 addresses in batches of 100000 - # batch took 17 seconds - # batch took 15 seconds - # batch took 17 seconds - # batch took 17 seconds - # batch took 17 seconds - # batch took 17 seconds - # batch took 17 seconds - # batch took 17 seconds - # batch took 17 seconds - # batch took 17 seconds - # - # This is with mysql: - # Updating 1000000 records in one shot - # batch updated in 153 seconds - # - # Updating 1000000 records in batches of 200000, this may take a while... - # batch took 41 seconds - # batch took 37 seconds - # batch took 35 seconds - # batch took 28 seconds - # batch took 27 seconds - # - # Updating 1000000 records in batches of 150000, this may take a while... - # batch took 30 seconds - # batch took 29 seconds - # batch took 18 seconds - # batch took 18 seconds - # batch took 17 seconds - # batch took 29 seconds - # batch took 12 seconds - # - # Updating 1000000 records in batches of 100000, this may take a while... - # batch took 10 seconds - # batch took 11 seconds - # batch took 12 seconds - # batch took 13 seconds - # batch took 12 seconds - # batch took 12 seconds - # batch took 14 seconds - # batch took 19 seconds - # batch took 20 seconds - # batch took 21 seconds - # - # Please note that the migration will be much faster when there's no index - # on the `name` column. For example, with mysql each batch takes exactly - # the same time: - # - # Updating 1000000 records in batches of 200000, this may take a while... - # batch took 17 seconds - # batch took 17 seconds - # batch took 17 seconds - # batch took 16 seconds - # batch took 17 seconds - # - # So, if special need arise, one can drop the index added with migration - # 20210122110141_add_name_to_spree_addresses.rb and add the index later, - # in non blocking ways. For postgresql: - # add_index(:spree_addresses, :name, algorithm: :concurrently) - # - # For mysql 5.6: - # sql = "ALTER TABLE spree_addresses ADD INDEX index_spree_addresses_on_name (name), ALGORITHM=INPLACE, LOCK=NONE;" - # ActiveRecord::Base.connection.execute sql - # - puts ' Data migration will happen in batches. The default value is 100_000, which should take less than 20 seconds on mysql or postgresql.' - size = shell.ask(' Please enter a different batch size, or press return to confirm the default: ') - size = (size.presence || 100_000).to_i - - abort " Invalid batch size number #{size}, please run the task again." unless size.positive? - - batches_total = (count / size).ceil - puts " We're going to migrate #{count} records in #{batches_total} batches of #{size}." - - answer = shell.ask(' Do you want to proceed?', limited_to: ['Y', 'N'], case_insensitive: true) - if answer == 'Y' - puts " Updating #{count} records in batches of #{size}, this may take a while..." - - records.in_batches(of: size).each.with_index(1) do |batch, index| - now = Time.zone.now - batch.update_all(concat_statement) - puts " Batch #{index}/#{batches_total} done in #{(Time.zone.now - now).to_i} seconds." - end - else - puts " Database not migrated. Please, make sure to fill address's name field on your own." - end - end - end - end - end -end diff --git a/core/lib/tasks/migrations/migrate_default_billing_addresses_to_address_book.rake b/core/lib/tasks/migrations/migrate_default_billing_addresses_to_address_book.rake deleted file mode 100644 index 0369769d178..00000000000 --- a/core/lib/tasks/migrations/migrate_default_billing_addresses_to_address_book.rake +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -namespace :solidus do - namespace :migrations do - namespace :migrate_default_billing_addresses_to_address_book do - task up: :environment do - print "Migrating default billing addresses to address book ... " - if Spree::UserAddress.where(default_billing: true).any? - Spree.user_class.joins(:bill_address).update_all(bill_address_id: nil) # rubocop:disable Rails/SkipsModelValidations - end - adapter_type = Spree::Base.connection.adapter_name.downcase.to_sym - if adapter_type == :mysql2 - sql = <<~SQL - UPDATE spree_user_addresses - JOIN spree_users ON spree_user_addresses.user_id = spree_users.id - AND spree_user_addresses.address_id = spree_users.bill_address_id - SET spree_user_addresses.default_billing = true - SQL - else - sql = <<~SQL - UPDATE spree_user_addresses - SET default_billing = true - FROM spree_users - WHERE spree_user_addresses.address_id = spree_users.bill_address_id - AND spree_user_addresses.user_id = spree_users.id; - SQL - end - Spree::Base.connection.execute sql - puts "Success" - end - - task down: :environment do - Spree::UserAddress.update_all(default_billing: false) # rubocop:disable Rails/SkipsModelValidations - puts "Rolled back default billing address migration to address book" - end - end - end -end diff --git a/core/spec/lib/tasks/migrations/migrate_address_names_spec.rb b/core/spec/lib/tasks/migrations/migrate_address_names_spec.rb deleted file mode 100644 index b345298f489..00000000000 --- a/core/spec/lib/tasks/migrations/migrate_address_names_spec.rb +++ /dev/null @@ -1,93 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -path = Spree::Core::Engine.root.join('lib/tasks/migrations/migrate_address_names.rake') - -RSpec.describe 'solidus:migrations:migrate_address_names' do - around do |example| - ignored_columns = Spree::Address.ignored_columns - Spree::Address.ignored_columns = [] - Spree::Address.reset_column_information - - original_stderr = $stderr - original_stdout = $stdout - $stderr = File.open(File::NULL, "w") - $stdout = File.open(File::NULL, "w") - - example.run - ensure - $stderr = original_stderr - $stdout = original_stdout - - Spree::Address.ignored_columns = ignored_columns - Spree::Address.reset_column_information - end - - describe 'up' do - include_context( - 'rake', - task_path: path, - task_name: 'solidus:migrations:migrate_address_names:up' - ) - - context "when there are no records to migrate" do - it "simply exits" do - expect { task.invoke }.to output( - "Combining addresses' firstname and lastname into name ... \n Your DB contains 0 addresses that may be affected by this task.\n" - ).to_stdout - end - end - - context "when there are records to migrate" do - let!(:complete_address) { create(:address, firstname: 'Jane', lastname: 'Smith') } - let!(:partial_address) { create(:address, firstname: nil, lastname: 'Doe') } - - before do - Spree::Address.update_all(name: nil) - end - - context "when the DB adapter is not supported" do - before do - allow(ActiveRecord::Base.connection).to receive(:adapter_name) { 'ms_sql' } - end - - it "exists with error" do - expect { task.invoke }.to raise_error(SystemExit) - end - end - - context "when the DB adapter is supported" do - before do - allow_any_instance_of(Thor::Shell::Basic).to receive(:ask).with( - ' Do you want to proceed?', - limited_to: ['Y', 'N'], - case_insensitive: true - ).and_return('Y') - - allow_any_instance_of(Thor::Shell::Basic).to receive(:ask).with( - ' Please enter a different batch size, or press return to confirm the default: ' - ).and_return(size) - end - - context 'when providing valid batch size number' do - let(:size) { 10 } - - it 'migrates name data by setting the actual field on the DB' do - expect { task.invoke }.to change { complete_address.reload[:name] }.to('Jane Smith') - .and change { partial_address.reload[:name] }.to('Doe') - end - end - - context 'when providing invalid batch size number' do - let(:size) { 'foobar' } - - it 'exits without migrating name data' do - expect { task.invoke }.to raise_error(SystemExit) - expect(Spree::Address.pluck(:name)).to be_all(&:nil?) - end - end - end - end - end -end diff --git a/core/spec/lib/tasks/migrations/migrate_default_billing_addresses_to_address_book_spec.rb b/core/spec/lib/tasks/migrations/migrate_default_billing_addresses_to_address_book_spec.rb deleted file mode 100644 index 17982f06682..00000000000 --- a/core/spec/lib/tasks/migrations/migrate_default_billing_addresses_to_address_book_spec.rb +++ /dev/null @@ -1,103 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe 'solidus:migrations:migrate_default_billing_addresses_to_address_book' do - describe 'up' do - include_context( - 'rake', - task_path: Spree::Core::Engine.root.join('lib/tasks/migrations/migrate_default_billing_addresses_to_address_book.rake'), - task_name: 'solidus:migrations:migrate_default_billing_addresses_to_address_book:up', - ) - - context "migrate from Solidus 2.11" do - let(:user1) { create(:user) } - let(:user2) { create(:user) } - let(:bill_address1) { FactoryBot.create(:address) } - let(:bill_address2) { FactoryBot.create(:address) } - let(:bill_address3) { FactoryBot.create(:address) } - - before do - # Set two billing addresses for User1, the second address is the default. - user1.save_in_address_book(bill_address1.attributes, false, :billing) - user1.save_in_address_book(bill_address2.attributes, true, :billing) - - # Update the "bill_address_id" for user1 to be different from the address_book's default address. - user1.update!(bill_address_id: bill_address1.id) - - # Set user2's bill address using old `bill_address_id` method. - user2.save_in_address_book(bill_address3.attributes, false, :billing) - user2.update!(bill_address_id: bill_address3.id) - Spree::UserAddress.where(user_id: user2.id).first.update!(default_billing: false) - end - - it 'runs' do - expect { task.invoke }.to output( - "Migrating default billing addresses to address book ... Success\n" - ).to_stdout - end - - it "does not migrate a user's `bill_address_id` when a user already has a default `bill_address` in the address book" do - task.invoke - expect(user1.bill_address_id).not_to eq bill_address2.id - expect(user1.bill_address).to eq bill_address2 - end - - it "migrates a user's `bill_address_id` when a user does not have a default `bill_address` in the address book" do - task.invoke - expect(user2.bill_address_id).to eq bill_address3.id - expect(user2.bill_address).to eq bill_address3 - end - end - - context "migrate from Solidus 2.10" do - let(:user) { create(:user) } - let(:bill_address) { FactoryBot.create(:address) } - - before do - # Set the user's bill address using old `bill_address_id` method. - user.save_in_address_book(bill_address.attributes, false, :billing) - user.update!(bill_address_id: bill_address.id) - Spree::UserAddress.where(default_billing: true).update_all(default_billing: false) - end - - it 'runs' do - expect { task.invoke }.to output( - "Migrating default billing addresses to address book ... Success\n" - ).to_stdout - end - - it "migrates a user's `bill_address_id` when a user does not have a default `bill_address` in the address book" do - task.invoke - expect(user.bill_address_id).to eq bill_address.id - expect(user.bill_address).to eq bill_address - end - end - end - - describe 'down' do - include_context( - 'rake', - task_path: Spree::Core::Engine.root.join('lib/tasks/migrations/migrate_default_billing_addresses_to_address_book.rake'), - task_name: 'solidus:migrations:migrate_default_billing_addresses_to_address_book:down', - ) - - let(:user) { create(:user) } - let(:bill_address) { FactoryBot.create(:address) } - - before do - user.save_in_address_book(bill_address.attributes, true, :billing) - end - - it 'runs' do - expect { task.invoke }.to output( - "Rolled back default billing address migration to address book\n" - ).to_stdout - end - - it "Rolls back default billing address migration to address book" do - task.invoke - expect(user.bill_address).to eq nil - end - end -end From f9b317cdfab49f16e1475c5012f123a84c3e9b01 Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Thu, 18 Feb 2021 17:32:21 +0100 Subject: [PATCH 2/2] Add a post install/update message We already introduced this post-install message in 2.11.5 when we started requesting users to run a rake task after bumping Solidus. Even if we don't have any specific rake task here in 3.0 we can keep this convention so we always have a place where to put this kind of tasks. Also, we can be consistent between versions in terms of how we request people to upgrade Solidus: 1. Change Gemfile 2. Run `bin/rails solidus:upgrade` Even if it does nothing special, I think it's a positive thing to have. --- core/lib/tasks/upgrade.rake | 12 +++++++----- core/solidus_core.gemspec | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/core/lib/tasks/upgrade.rake b/core/lib/tasks/upgrade.rake index d57aba14e43..cc784e2f48f 100644 --- a/core/lib/tasks/upgrade.rake +++ b/core/lib/tasks/upgrade.rake @@ -2,12 +2,14 @@ namespace :solidus do namespace :upgrade do - desc "Upgrade Solidus to version 2.11" - task two_point_eleven: [ - 'solidus:migrations:migrate_default_billing_addresses_to_address_book:up', - 'solidus:migrations:migrate_address_names:up' + task three_point_zero: [ + 'railties:install:migrations', + 'db:migrate' ] do - puts "Your Solidus install is ready for Solidus 2.11" + puts "Your Solidus install is ready for Solidus 3.0" end end + + desc "Upgrade to the current Solidus version" + task upgrade: 'upgrade:three_point_zero' end diff --git a/core/solidus_core.gemspec b/core/solidus_core.gemspec index 1cb01114fb6..76fba589d06 100644 --- a/core/solidus_core.gemspec +++ b/core/solidus_core.gemspec @@ -42,4 +42,24 @@ Gem::Specification.new do |s| s.add_dependency 'kt-paperclip', '~> 6.3' s.add_dependency 'ransack', '~> 2.0' s.add_dependency 'state_machines-activerecord', '~> 0.6' + + s.post_install_message = <<-MSG +------------------------------------------------------------- + Thank you for using Solidus +------------------------------------------------------------- +If this is a fresh install, don't forget to run the Solidus +installer with the following command: + +$ bin/rails g solidus:install + +If you are updating Solidus from an older version, please run +the following commands to complete the update: + +$ bin/rails solidus:upgrade + +Please report any issues at: +- https://github.com/solidusio/solidus/issues +- http://slack.solidus.io/ +------------------------------------------------------------- +MSG end