Skip to content

Commit

Permalink
Introduce a configuration value for migration path
Browse files Browse the repository at this point in the history
Since Rails 6 multiple database support is a native feature, which
allows users to configure separate read/write databases. This change
allows users to configure the path to the migrations folder from the
default "db/migrate" if they are using the multiple database support and
have specified a different database for the Solidus engine. This will
ensure the check that all migrations are installed can look at a user
configurable folder.

One change required to make the folder path configurable is to run the
migration check after 'load_config_initializers' instead of before. This
will allow the value of `migration_path` configuration to be set in an
initializer.

Co-authored-by: Adam Mueller <adam@super.gd>
Co-authored-by: Alex Blackie <alex@super.gd>
Co-authored-by: Benjamin Willems <benjamin@super.gd>
Co-authored-by: Jared Norman <jared@super.gd>
  • Loading branch information
5 people committed Oct 7, 2021
1 parent a54f713 commit 1cbc7dd
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 4 deletions.
12 changes: 11 additions & 1 deletion core/lib/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ class AppConfiguration < Preferences::Configuration
# @return [] Track on_hand values for variants / products. (default: true)
preference :track_inventory_levels, :boolean, default: true


# Other configurations

# Allows restricting what currencies will be available.
Expand Down Expand Up @@ -525,6 +524,17 @@ def payment_canceller
# Enumerable of taxons adhering to the present_taxon_class interface
class_name_attribute :taxon_attachment_module, default: 'Spree::Taxon::ActiveStorageAttachment'

# Configures the absolute path that contains the Solidus engine
# migrations. This will be checked at app boot to confirm that all Solidus
# migrations are installed.
#
# @!attribute [rw] migration_path
# @return [String] the configured path. (default: "#{Rails.root}/db/migrate")
attr_writer :migration_path
def migration_path
@migration_path ||= ::Rails.root.join('db', 'migrate')
end

# Allows providing your own class instance for generating order numbers.
#
# @!attribute [rw] order_number_generator
Expand Down
2 changes: 1 addition & 1 deletion core/lib/spree/core/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class Engine < ::Rails::Engine
]
end

initializer "spree.core.checking_migrations", before: :load_config_initializers do |_app|
initializer "spree.core.checking_migrations", after: :load_config_initializers do |_app|
Migrations.new(config, engine_name).check
end

Expand Down
2 changes: 1 addition & 1 deletion core/lib/spree/migrations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def app_migrations
end

def app_dir
"#{Rails.root}/db/migrate"
Spree::Config.migration_path
end

def engine_dir
Expand Down
20 changes: 20 additions & 0 deletions core/spec/lib/spree/app_configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,26 @@ class DummyClass; end;
end
end

describe "#migration_path" do
subject { config_instance.migration_path }

let(:config_instance) { described_class.new }

it "has a default value" do
expect(subject.to_s).to end_with "db/migrate"
end

context "with a custom value" do
before do
config_instance.migration_path = "db/secondary_database"
end

it "returns the configured value" do
expect(subject).to eq "db/secondary_database"
end
end
end

it 'has a default admin VAT location with nil values by default' do
expect(prefs.admin_vat_location).to eq(Spree::Tax::TaxLocation.new)
expect(prefs.admin_vat_location.state_id).to eq(nil)
Expand Down
9 changes: 8 additions & 1 deletion core/spec/lib/spree/migrations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,17 @@ module Spree
let(:config) { double("Config", root: "dir") }

let(:engine_dir) { "dir/db/migrate" }
let(:app_dir) { "#{Rails.root}/db/migrate" }
let(:app_dir) { 'app/db/migrate' }

subject { described_class.new(config, "spree") }

around do |example|
original_path = Spree::Config.migration_path
Spree::Config.migration_path = app_dir
example.run
Spree::Config.migration_path = original_path
end

it "detects missing migrations" do
expect(Dir).to receive(:entries).with(app_dir).and_return app_migrations
expect(Dir).to receive(:entries).with(engine_dir).and_return engine_migrations
Expand Down

0 comments on commit 1cbc7dd

Please sign in to comment.