Ensure db:seed runs before spree_sample:load

Author: lsizani

README.md

@@ -148,13 +148,17 @@ bin/rails g solidus:install --migrate=false --sample=false --seed=false
 
 You can always perform any of these steps later by using these commands.
 
+**Note: Loading sample data will automatically load seed data as well because the sample data depends on the data provided by the seeds.**
+
 ```bash
 bin/rails railties:install:migrations
 bin/rails db:migrate
-bin/rails db:seed
 bin/rails spree_sample:load
 ```
 
+If you don't want to load sample data you can use `bin/rails db:seed` instead. This command
+will populate the database with only the basic data needed for Solidus to work.
+
 There are also options and rake tasks provided by
 [solidus\_auth\_devise](https://github.com/solidusio/solidus_auth_devise).
 

core/lib/generators/solidus/install/install_generator.rb

@@ -36,8 +36,8 @@ class InstallGenerator < Rails::Generators::AppBase
     ]
 
     class_option :migrate, type: :boolean, default: true, banner: 'Run Solidus migrations'
-    class_option :seed, type: :boolean, default: true, banner: 'Load seed data (migrations must be run)'
-    class_option :sample, type: :boolean, default: true, banner: 'Load sample data (migrations and seeds must be run)'
+    class_option :seed, type: :boolean, default: true, banner: 'Run seed data task(migrations must be run)'
+    class_option :sample, type: :boolean, default: true, banner: 'Run sample data task(migrations must be run)'
     class_option :active_storage, type: :boolean, default: (
       Rails.gem_version >= Gem::Version.new("6.1.0")
     ), banner: 'Install ActiveStorage as image attachments handler for products and taxons'
@@ -63,8 +63,8 @@ def self.exit_on_failure?
 
     def prepare_options
       @run_migrations = options[:migrate]
-      @load_seed_data = options[:seed] && @run_migrations
-      @load_sample_data = options[:sample] && @run_migrations && @load_seed_data
+      @run_sample = options[:sample] && @run_migrations
+      @run_seeds = options[:seed] && @run_migrations && !@run_sample
       @selected_frontend = detect_frontend_to_install
       @selected_authentication = detect_authentication_to_install
       @selected_payment_method = detect_payment_method_to_install
@@ -172,27 +172,12 @@ def install_payment_method
       apply_template_for :payment_method, @selected_payment_method
     end
 
-    def populate_seed_data
-      if @load_seed_data
-        say_status :loading, "seed data"
-        rake_options = []
-        rake_options << "AUTO_ACCEPT=1" if options[:auto_accept]
-        rake_options << "ADMIN_EMAIL=#{options[:admin_email]}" if options[:admin_email]
-        rake_options << "ADMIN_PASSWORD=#{options[:admin_password]}" if options[:admin_password]
+    def run_data_loaders
+      say_status_and_run_task("seed and sample data", "spree_sample:load") if @run_sample
+      say_status_and_run_task("seed data", "db:seed #{seed_data_overrides.join(' ')}") if @run_seeds
 
-        rake("db:seed #{rake_options.join(' ')}")
-      else
-        say_status :skipping, "seed data (you can always run rake db:seed)"
-      end
-    end
-
-    def load_sample_data
-      if @load_sample_data
-        say_status :loading, "sample data"
-        rake 'spree_sample:load'
-      else
-        say_status :skipping, "sample data (you can always run rake spree_sample:load)"
-      end
+      say_status :skipping, "seed data (you can always run rake db:seed)" unless @run_seeds || @run_sample
+      say_status :skipping, "sample data (you can always run rake spree_sample:load)" unless @run_sample
     end
 
     def complete
@@ -201,6 +186,19 @@ def complete
 
     private
 
+    def say_status_and_run_task(status_text, task)
+      say_status :loading, status_text
+      rake task
+    end
+
+    def seed_data_overrides
+      rake_options = []
+      rake_options << "AUTO_ACCEPT=1" if options[:auto_accept]
+      rake_options << "ADMIN_EMAIL=#{options[:admin_email]}" if options[:admin_email]
+      rake_options << "ADMIN_PASSWORD=#{options[:admin_password]}" if options[:admin_password]
+      rake_options
+    end
+
     def generate(what, *args, abort_on_failure: true)
       args << '--auto-accept' if options[:auto_accept]
       args << '--auto-run-migrations' if options[:migrate]

core/spec/generators/solidus/install/install_generator_spec.rb

@@ -14,8 +14,8 @@
         expect(generator.instance_variable_get(:@selected_authentication)).to eq("devise")
         expect(generator.instance_variable_get(:@selected_payment_method)).to eq("paypal")
         expect(generator.instance_variable_get(:@run_migrations)).to eq(true)
-        expect(generator.instance_variable_get(:@load_seed_data)).to eq(true)
-        expect(generator.instance_variable_get(:@load_sample_data)).to eq(true)
+        expect(generator.instance_variable_get(:@run_seeds)).to eq(false)
+        expect(generator.instance_variable_get(:@run_sample)).to eq(true)
       end
     end
 
@@ -36,8 +36,19 @@
 
       aggregate_failures do
         expect(generator.instance_variable_get(:@run_migrations)).to eq(false)
-        expect(generator.instance_variable_get(:@load_seed_data)).to eq(false)
-        expect(generator.instance_variable_get(:@load_sample_data)).to eq(false)
+        expect(generator.instance_variable_get(:@run_seeds)).to eq(false)
+        expect(generator.instance_variable_get(:@run_sample)).to eq(false)
+      end
+    end
+
+    it 'disables "seeds" when "sample" is enabled' do
+      generator = described_class.new([], ['--auto-accept', '--sample=true'])
+      generator.prepare_options
+
+      aggregate_failures do
+        expect(generator.instance_variable_get(:@run_migrations)).to eq(true)
+        expect(generator.instance_variable_get(:@run_seeds)).to eq(false)
+        expect(generator.instance_variable_get(:@run_sample)).to eq(true)
       end
     end
 
@@ -68,17 +79,8 @@
       generator.prepare_options
 
       expect(generator.instance_variable_get(:@run_migrations)).to eq(false)
-      expect(generator.instance_variable_get(:@load_seed_data)).to eq(false)
-      expect(generator.instance_variable_get(:@load_sample_data)).to eq(false)
-    end
-
-    it 'skips sample data if seeds are disabled' do
-      generator = described_class.new([], ['--auto-accept', '--seed=false'])
-      generator.prepare_options
-
-      expect(generator.instance_variable_get(:@run_migrations)).to eq(true)
-      expect(generator.instance_variable_get(:@load_seed_data)).to eq(false)
-      expect(generator.instance_variable_get(:@load_sample_data)).to eq(false)
+      expect(generator.instance_variable_get(:@run_seeds)).to eq(false)
+      expect(generator.instance_variable_get(:@run_sample)).to eq(false)
     end
 
     context 'supports legacy frontend option names' do
@@ -130,6 +132,60 @@
     end
   end
 
+  describe "#run_data_loaders" do
+    it "executes spree_sample:load task only when sample and seed are enabled" do
+      generator = described_class.new([], ['--auto-accept', '--sample=true', '--seed=true'])
+      allow(generator).to receive(:say_status_and_run_task)
+
+      generator.prepare_options
+      generator.run_data_loaders
+
+      aggregate_failures do
+        expect(generator).to have_received(:say_status_and_run_task).with("seed and sample data", "spree_sample:load")
+        expect(generator).not_to have_received(:say_status_and_run_task).with("seed data", "db:seed")
+      end
+    end
+
+    it "does not execute spree_sample:load task when sample is disabled" do
+      generator = described_class.new([], ['--auto-accept', '--sample=false', '--seed=true'])
+      allow(generator).to receive(:say_status_and_run_task)
+      allow(generator).to receive(:say_status)
+
+      generator.prepare_options
+      generator.run_data_loaders
+
+      aggregate_failures do
+        expect(generator).to have_received(:say_status_and_run_task).with("seed data", "db:seed AUTO_ACCEPT=1")
+        expect(generator).to have_received(:say_status).with(:skipping, "sample data (you can always run rake spree_sample:load)")
+      end
+    end
+
+    it "skips both spree_sample:load and db:seed tasks when task and sample are disabled" do
+      generator = described_class.new([], ['--auto-accept', '--sample=false', '--seed=false'])
+      allow(generator).to receive(:say_status_and_run_task)
+      allow(generator).to receive(:say_status)
+
+      generator.prepare_options
+      generator.run_data_loaders
+
+      aggregate_failures do
+        expect(generator).to have_received(:say_status).with(:skipping, "seed data (you can always run rake db:seed)")
+        expect(generator).to have_received(:say_status).with(:skipping, "sample data (you can always run rake spree_sample:load)")
+      end
+    end
+
+    it "includes expected rake options for db:seed" do
+      generator = described_class.new([], ['--auto-accept', '--sample=false', '--seed=true', '--admin_email=test@solidus.io', '--admin_password=P@55word!'])
+      allow(generator).to receive(:say_status_and_run_task)
+      allow(generator).to receive(:say_status)
+
+      generator.prepare_options
+      generator.run_data_loaders
+
+      expect(generator).to have_received(:say_status_and_run_task).with("seed data", "db:seed AUTO_ACCEPT=1 ADMIN_EMAIL=test@solidus.io ADMIN_PASSWORD=P@55word!")
+    end
+  end
+
   private
 
   def strip_ansi(string)

sample/db/samples/shipping_methods.rb

@@ -3,8 +3,11 @@
 begin
 north_america = Spree::Zone.find_by!(name: "North America")
 rescue ActiveRecord::RecordNotFound
-  puts "Couldn't find 'North America' zone. Did you run `rake db:seed` first?"
-  puts "That task will set up the countries, states and zones required for Spree."
+  puts <<~TEXT
+    Couldn't find 'North America' zone. Did you run `rails db:seed` first?
+
+    That task will set up the countries, states and zones required for your store.
+  TEXT
   exit
 end
 

sample/db/samples/tax_rates.rb

@@ -1,6 +1,16 @@
 # frozen_string_literal: true
 
-north_america = Spree::Zone.find_by!(name: "North America")
+begin
+  north_america = Spree::Zone.find_by!(name: "North America")
+rescue ActiveRecord::RecordNotFound
+  puts <<~TEXT
+    Couldn't find 'North America' zone. Did you run `rails db:seed` first?
+
+    That task will set up the countries, states and zones required for your store.
+  TEXT
+  exit
+end
+
 clothing = Spree::TaxCategory.find_by!(name: "Default")
 tax_rate = Spree::TaxRate.create(
   name: "North America",

sample/lib/tasks/sample.rake

@@ -5,7 +5,7 @@ require 'spree/sample'
 
 namespace :spree_sample do
   desc 'Loads sample data'
-  task load: :environment do
+  task load: ['db:seed', :environment] do
     if ARGV.include?("db:migrate")
       puts <<~TEXT
         Please run db:migrate separately from spree_sample:load.

sample/spec/lib/load_sample_spec.rb

@@ -1,12 +1,38 @@
 # frozen_string_literal: true
 
-require 'spec_helper'
+require "spec_helper"
+require "rake"
 
 describe "Load samples" do
   it "doesn't raise any error" do
-    expect {
-      Spree::Core::Engine.load_seed
+    expect do
+      pid = fork { Spree::Core::Engine.load_seed }
+      Process.wait(pid)
       SpreeSample::Engine.load_samples
-    }.to output.to_stdout
+    ensure
+      Process.kill(:KILL, pid) unless $?.exitstatus.zero?
+    end.not_to raise_error
+  end
+
+  it "has db:seed as a prerequisite" do
+    Rails.application.load_tasks
+
+    task = Rake::Task["spree_sample:load"]
+    seed_task = Rake::Task["db:seed"]
+    expect(task.prerequisite_tasks).to include(seed_task)
+  end
+end
+
+describe "Load seeds multiple times" do
+  it "doesn't duplicate records" do
+    4.times do
+      pid = fork { Spree::Core::Engine.load_seed }
+      Process.wait(pid)
+    ensure
+      Process.kill(:KILL, pid) unless $?.exitstatus.zero?
+    end
+
+    expect(Spree::Store.count).to eq(1)
+    expect(Spree::Zone.count).to eq(2)
   end
 end