diff --git a/.circleci/config.yml b/.circleci/config.yml index f2218d74418..273f17fc5e8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -218,6 +218,14 @@ jobs: cd /tmp/my_app bundle list | grep 'solidus_paypal_commerce_platform (1.' + - install_solidus: { flags: "--sample=false --frontend=starter --payment-method=braintree" } + - test_page: { expected_text: "The only eCommerce platform you’ll ever need." } + - run: + name: Ensure the correct Braintree is installed for SSF + command: | + cd /tmp/my_app + bundle list | grep 'solidus_braintree (3.' + - install_solidus: { flags: "--sample=false --frontend=none --authentication=none" } - test_page: { expected_text: "Ruby on Rails" } diff --git a/api/app/controllers/spree/api/shipments_controller.rb b/api/app/controllers/spree/api/shipments_controller.rb index 84985e09b23..b21a1a12008 100644 --- a/api/app/controllers/spree/api/shipments_controller.rb +++ b/api/app/controllers/spree/api/shipments_controller.rb @@ -127,7 +127,8 @@ def transfer_to_shipment current_shipment: @original_shipment, desired_shipment: @desired_shipment, variant: @variant, - quantity: @quantity + quantity: @quantity, + track_inventory: Spree::Config.track_inventory_levels ) if fulfilment_changer.run! diff --git a/backend/spec/features/admin/orders/customer_returns_spec.rb b/backend/spec/features/admin/orders/customer_returns_spec.rb index b654d0e94b3..7ea7e6928d7 100644 --- a/backend/spec/features/admin/orders/customer_returns_spec.rb +++ b/backend/spec/features/admin/orders/customer_returns_spec.rb @@ -24,7 +24,7 @@ def order_state_label let(:order) { create :shipped_order, line_items_count: 2 } context 'when creating a return with state "Received"' do - it 'marks the order as returned', :js do + it 'marks the order as returned', :js, :flaky do create_customer_return('receive') expect(page).to have_content 'Customer Return has been successfully created' @@ -41,7 +41,7 @@ def order_state_label expect(page).to have_button("Create", disabled: true) end - context 'when creating a return with state "In Transit" and then marking it as "Received"' do + context 'when creating a return with state "In Transit" and then marking it as "Received"', :flaky do it 'marks the order as returned', :js do create_customer_return('in_transit') expect(page).to have_content 'Customer Return has been successfully created' diff --git a/backend/spec/features/admin/orders/order_details_spec.rb b/backend/spec/features/admin/orders/order_details_spec.rb index f1688a28529..5abc6b66fcb 100644 --- a/backend/spec/features/admin/orders/order_details_spec.rb +++ b/backend/spec/features/admin/orders/order_details_spec.rb @@ -583,6 +583,29 @@ expect(page).to have_content("2 x Backordered") end end + + context 'when inventory levels are not being tracked' do + before do + stub_spree_preferences(track_inventory_levels: false) + # Simulate the default state of stock when track inventory is false. + shipment1.inventory_units.update_all(state: :on_hand) + product.master.stock_items.last.update_column(:backorderable, true) + product.master.stock_items.last.update_column(:count_on_hand, 0) + end + + # Regression for https://github.com/solidusio/solidus/issues/2817 + it "allows to ship an order's items" do + visit spree.edit_admin_order_path(order) + + within('tr', text: line_item.sku) { click_icon 'arrows-h' } + complete_split_to(stock_location2) + + expect(page).to have_css('.shipment', count: 2) + expect(order.shipments.count).to eq(2) + expect(page).not_to have_content('Backordered') + expect(page).to have_content('On hand', count: 2) + end + end end describe 'line item sort order' do diff --git a/core/app/models/spree/fulfilment_changer.rb b/core/app/models/spree/fulfilment_changer.rb index 12c5efe131b..377299f460e 100644 --- a/core/app/models/spree/fulfilment_changer.rb +++ b/core/app/models/spree/fulfilment_changer.rb @@ -17,18 +17,27 @@ module Spree # @attr [Integer] quantity How many units we want to move # class FulfilmentChanger + TRACK_INVENTORY_NOT_PROVIDED = Object.new.freeze include ActiveModel::Validations attr_accessor :current_shipment, :desired_shipment - attr_reader :variant, :quantity, :current_stock_location, :desired_stock_location + attr_reader :variant, :quantity, :current_stock_location, :desired_stock_location, :track_inventory - def initialize(current_shipment:, desired_shipment:, variant:, quantity:) + def initialize(current_shipment:, desired_shipment:, variant:, quantity:, track_inventory: TRACK_INVENTORY_NOT_PROVIDED) @current_shipment = current_shipment @desired_shipment = desired_shipment @current_stock_location = current_shipment.stock_location @desired_stock_location = desired_shipment.stock_location @variant = variant @quantity = quantity + @track_inventory = if track_inventory == TRACK_INVENTORY_NOT_PROVIDED + Spree::Deprecation.warn( + "Not passing `track_inventory` to `Spree::FulfilmentChanger` is deprecated." \ + ) + true + else + track_inventory + end end validates :quantity, numericality: { greater_than: 0 } @@ -45,6 +54,45 @@ def run! return false if invalid? desired_shipment.save! if desired_shipment.new_record? + if track_inventory + run_tracking_inventory + else + run_without_tracking_inventory + end + + # We modified the inventory units at the database level for speed reasons. + # The downside of that is that we need to reload the associations. + current_shipment.inventory_units.reload + desired_shipment.inventory_units.reload + + # If the current shipment now has no inventory units left, we won't need it any longer. + if current_shipment.inventory_units.length.zero? + current_shipment.destroy! + else + # The current shipment has changed, so we need to make sure that shipping rates + # have the correct amount. + current_shipment.refresh_rates + end + + # The desired shipment has also change, so we need to make sure shipping rates + # are up-to-date, too. + desired_shipment.refresh_rates + + # In order to reflect the changes in the order totals + desired_shipment.order.reload + desired_shipment.order.recalculate + + true + end + + private + + # When moving things from one stock location to another, we need to restock items + # from the current location and unstock them at the desired location. + # Also, when we want to track inventory changes, we need to make sure that we have + # enough stock at the desired location to fulfil the order. Based on how many items + # we can take from the desired location, we could end up with some items being backordered. + def run_tracking_inventory # Retrieve how many on hand items we can take from desired stock location available_quantity = [desired_shipment.stock_location.count_on_hand(variant), default_on_hand_quantity].max @@ -79,34 +127,21 @@ def run! limit(quantity - new_on_hand_quantity). update_all(shipment_id: desired_shipment.id, state: :backordered) end + end - # We modified the inventory units at the database level for speed reasons. - # The downside of that is that we need to reload the associations. - current_shipment.inventory_units.reload - desired_shipment.inventory_units.reload - - # If the current shipment now has no inventory units left, we won't need it any longer. - if current_shipment.inventory_units.length.zero? - current_shipment.destroy! - else - # The current shipment has changed, so we need to make sure that shipping rates - # have the correct amount. - current_shipment.refresh_rates + # When we don't track inventory, we can just move the inventory units from one shipment + # to the other. + def run_without_tracking_inventory + ActiveRecord::Base.transaction do + current_shipment. + inventory_units. + where(variant: variant). + order(state: :asc). + limit(quantity). + update_all(shipment_id: desired_shipment.id) end - - # The desired shipment has also change, so we need to make sure shipping rates - # are up-to-date, too. - desired_shipment.refresh_rates - - # In order to reflect the changes in the order totals - desired_shipment.order.reload - desired_shipment.order.recalculate - - true end - private - # We don't need to handle stock counts for incomplete orders. Also, if # the new shipment and the desired shipment will ship from the same stock location, # unstocking and restocking will not be necessary. diff --git a/core/app/models/spree/product/scopes.rb b/core/app/models/spree/product/scopes.rb index 6daa94c35b3..bcaeac5824a 100644 --- a/core/app/models/spree/product/scopes.rb +++ b/core/app/models/spree/product/scopes.rb @@ -158,7 +158,7 @@ def self.property_conditions(property) # order: 'COALESCE(cnt, 0) DESC' add_search_scope :descend_by_popularity do joins(:master). - order(%{ + order(Arel.sql(%{ COALESCE(( SELECT COUNT(#{Spree::LineItem.quoted_table_name}.id) @@ -171,7 +171,7 @@ def self.property_conditions(property) WHERE popular_variants.product_id = #{Spree::Product.quoted_table_name}.id ), 0) DESC - }) + })) end add_search_scope :not_deleted do diff --git a/core/app/models/spree/promotion.rb b/core/app/models/spree/promotion.rb index 6b4794b7a88..999e79b0db6 100644 --- a/core/app/models/spree/promotion.rb +++ b/core/app/models/spree/promotion.rb @@ -2,7 +2,8 @@ module Spree class Promotion < Spree::Base - MATCH_POLICIES = %w(all any) + + autoload(:MATCH_POLICIES, "spree/promotion/match_policies") UNACTIVATABLE_ORDER_STATES = ["complete", "awaiting_return", "returned"] diff --git a/core/app/models/spree/taxon.rb b/core/app/models/spree/taxon.rb index 48ddaf7648e..13a9c7e1f70 100644 --- a/core/app/models/spree/taxon.rb +++ b/core/app/models/spree/taxon.rb @@ -18,9 +18,11 @@ class Taxon < Spree::Base after_update :update_child_permalinks, if: :saved_change_to_permalink? validates :name, presence: true + validates :name, uniqueness: { scope: :parent_id, message: :must_be_unique_under_same_parent }, if: -> { Spree::Config.extra_taxon_validations} validates :meta_keywords, length: { maximum: 255 } validates :meta_description, length: { maximum: 255 } validates :meta_title, length: { maximum: 255 } + validates :taxonomy_id, uniqueness: { message: :can_have_only_one_root }, if: -> { Spree::Config.extra_taxon_validations && root? } after_save :touch_ancestors_and_taxonomy after_touch :touch_ancestors_and_taxonomy diff --git a/core/app/models/spree/taxonomy.rb b/core/app/models/spree/taxonomy.rb index bbaeddcba25..37f6f4acfcc 100644 --- a/core/app/models/spree/taxonomy.rb +++ b/core/app/models/spree/taxonomy.rb @@ -5,6 +5,7 @@ class Taxonomy < Spree::Base acts_as_list validates :name, presence: true + validates :name, uniqueness: true, if: -> { Spree::Config.extra_taxonomy_validations } has_many :taxons, inverse_of: :taxonomy has_one :root, -> { where parent_id: nil }, class_name: "Spree::Taxon", dependent: :destroy diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index 1d54e09b5e4..a9d4d95b6e6 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -498,6 +498,12 @@ en: attributes: base: cannot_destroy_default_store: Cannot destroy the default Store. + spree/taxon: + attributes: + name: + must_be_unique_under_same_parent: must be unique under the same parent Taxon + taxonomy_id: + can_have_only_one_root: can only have one root Taxon spree/user_address: attributes: user_id: diff --git a/core/lib/generators/solidus/install/app_templates/payment_method/braintree.rb b/core/lib/generators/solidus/install/app_templates/payment_method/braintree.rb new file mode 100644 index 00000000000..a3bebb93132 --- /dev/null +++ b/core/lib/generators/solidus/install/app_templates/payment_method/braintree.rb @@ -0,0 +1,5 @@ +unless Bundler.locked_gems.dependencies['solidus_braintree'] + bundle_command 'add solidus_braintree --version "~> 3.0"' +end + +generate 'solidus_braintree:install' diff --git a/core/lib/generators/solidus/install/install_generator.rb b/core/lib/generators/solidus/install/install_generator.rb index 831b53bdca6..fb82b5edb8f 100644 --- a/core/lib/generators/solidus/install/install_generator.rb +++ b/core/lib/generators/solidus/install/install_generator.rb @@ -29,10 +29,31 @@ class InstallGenerator < Rails::Generators::AppBase none ] - PAYMENT_METHODS = %w[ - paypal - bolt - none + PAYMENT_METHODS = [ + { + name: 'paypal', + frontends: %w[none classic starter], + description: 'Install `solidus_paypal_commerce_platform`', + default: true, + }, + { + name: 'bolt', + frontends: %w[classic], + description: 'Install `solidus_bolt`', + default: false, + }, + { + name: 'braintree', + frontends: %w[none starter], + description: 'Install `solidus_braintree`', + default: false, + }, + { + name: 'none', + frontends: %w[none classic starter], + description: 'Skip installing a payment method', + default: false, + }, ] class_option :migrate, type: :boolean, default: true, banner: 'Run Solidus migrations' @@ -48,7 +69,7 @@ class InstallGenerator < Rails::Generators::AppBase class_option :frontend, type: :string, enum: FRONTENDS + LEGACY_FRONTENDS, default: nil, desc: "Indicates which frontend to install." class_option :authentication, type: :string, enum: AUTHENTICATIONS, default: nil, desc: "Indicates which authentication system to install." - class_option :payment_method, type: :string, enum: PAYMENT_METHODS, default: nil, desc: "Indicates which payment method to install." + class_option :payment_method, type: :string, enum: PAYMENT_METHODS.map { |payment_method| payment_method[:name] }, default: nil, desc: "Indicates which payment method to install." # DEPRECATED class_option :with_authentication, type: :boolean, hide: true, default: nil @@ -323,29 +344,26 @@ def detect_payment_method_to_install return 'paypal' if Bundler.locked_gems.dependencies['solidus_paypal_commerce_platform'] return 'bolt' if Bundler.locked_gems.dependencies['solidus_bolt'] - descriptions = { - paypal: "- [#{set_color 'paypal', :bold}] Install `solidus_paypal_commerce_platform` (#{set_color :default, :bold}).", - bolt: "- [#{set_color 'bolt', :bold}] Install `solidus_bolt`.", - none: "- [#{set_color 'none', :bold}] Skip installing a payment method.", - } - - payment_methods = PAYMENT_METHODS - - if @selected_frontend != 'classic' - payment_methods -= ['bolt'] - descriptions.delete(:bolt) + selected_frontend_payment_methods = PAYMENT_METHODS.select do |payment_method| + payment_method[:frontends].include?(@selected_frontend) end selected = options[:payment_method] || (options[:auto_accept] && 'paypal') || ask_with_description( default: 'paypal', - limited_to: payment_methods, + limited_to: selected_frontend_payment_methods.map { |payment_method| payment_method[:name] }, desc: <<~TEXT Which payment method would you like to use? - #{descriptions.values.join("\n")} + #{selected_frontend_payment_methods.map { |payment_method| formatted_payment_method_description(payment_method) }.join("\n")} TEXT ) end + + def formatted_payment_method_description(payment_method) + default_label = " (#{set_color :default, :bold})" if payment_method[:default] + + "- [#{set_color payment_method[:name], :bold}] #{payment_method[:description]}#{default_label}." + end end end diff --git a/core/lib/generators/solidus/update/update_generator.rb b/core/lib/generators/solidus/update/update_generator.rb index b32f1c00b14..8f448fffa7c 100644 --- a/core/lib/generators/solidus/update/update_generator.rb +++ b/core/lib/generators/solidus/update/update_generator.rb @@ -8,7 +8,7 @@ module Solidus class UpdateGenerator < ::Rails::Generators::Base FROM = Spree.previous_solidus_minor_version - desc 'Generates a new initializer to preview the new defaults for current Solidus version' + desc 'Generates a new initializer to preview the new defaults for current Solidus version and copy new migrations' source_root File.expand_path('templates', __dir__) @@ -37,10 +37,15 @@ class UpdateGenerator < ::Rails::Generators::Base default: 'config/initializers/', hide: true + class_option :install_migrations, + type: :boolean, + default: true, + hide: true + def create_new_defaults_initializer previous_version_prompt = options[:previous_version_prompt] return if previous_version_prompt && !yes?(<<~MSG, :red) - The update process is only supported if you are coming from version #{FROM}. If this is not the case, please, skip it and update your application to use Solidus #{FROM} before retrying. + The default preferences update process is only supported if you are coming from version #{FROM}. If this is not the case, please, skip it and update your application to use Solidus #{FROM} before retrying. If you are confident you want to upgrade from a previous version, you must rerun the generator with the "--from={OLD_VERSION}" argument. Are you sure you want to continue? (y/N) MSG @@ -57,6 +62,13 @@ def create_new_defaults_initializer File.join(options[:initializer_directory], "#{options[:initializer_basename]}.rb") end + def install_migrations + return unless options[:install_migrations] + + say_status :copying, "migrations" + rake 'spree:install:migrations' + end + def print_message say <<~MSG diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index 169727b21fd..05649689168 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -65,6 +65,22 @@ class AppConfiguration < Preferences::Configuration # @return [Boolean] When false, admins cannot create promotions with an "any" match policy (default: +false+) # Create individual, separate promotions for each of your rules instead. preference :allow_promotions_any_match_policy, :boolean, default: false + def allow_promotions_any_match_policy=(value) + if value == true + Spree::Deprecation.warn <<~MSG + Solidus 4.0 will remove support for combining promotion rules with the "any" match policy. + + Instead, it's suggested to create individual, separate promotions for each of your current + rules combined with the "any" policy. To automate this task, you can use the provided + task: + + bin/rake solidus:split_promotions_with_any_match_policy + MSG + end + + preferences[:allow_promotions_any_match_policy] = value + end + # @!attribute [rw] guest_token_cookie_options # @return [Hash] Add additional guest_token cookie options here (ie. domain or path) @@ -155,6 +171,38 @@ class AppConfiguration < Preferences::Configuration # @return [Regexp] Regex to be used in email validations, for example in Spree::EmailValidator preference :default_email_regexp, :regexp, default: URI::MailTo::EMAIL_REGEXP + # @!attribute [rw] extra_taxon_validations + # Use extra validations on Taxons in 3.4.0, but default to false so stores can + # upgrade and then fix any now invalid Taxons before enabling. + # @return [Boolean] + versioned_preference :extra_taxon_validations, :boolean, initial_value: false, boundaries: { "3.4.0.dev" => true } + def extra_taxon_validations=(value) + Spree::Deprecation.warn <<~MSG + Solidus will remove `Spree::Config.extra_taxon_validations` preference + in the next major release and will always use the extra Taxon validations. + Before upgrading to the next major, please fix any now invalid Taxons + and then remove the preference definition in `config/initializers/spree.rb`. + MSG + + preferences[:extra_taxon_validations] = value + end + + # @!attribute [rw] extra_taxonomy_validations + # Use extra validations on Taxonomies in 3.4.0, but default to false so stores can + # upgrade and then fix any now invalid Taxonomies before enabling. + # @return [Boolean] + versioned_preference :extra_taxonomy_validations, :boolean, initial_value: false, boundaries: { "3.4.0.dev" => true } + def extra_taxonomy_validations=(value) + Spree::Deprecation.warn <<~MSG + Solidus will remove `Spree::Config.extra_taxonomy_validations` preference + in the next major release and will always use the extra Taxonomy validations. + Before upgrading to the next major, please fix any now invalid Taxons + and then remove the preference definition in `config/initializers/spree.rb`. + MSG + + preferences[:extra_taxonomy_validations] = value + end + # @!attribute [rw] generate_api_key_for_all_roles # @return [Boolean] Allow generating api key automatically for user # at role_user creation for all roles. (default: +false+) diff --git a/core/lib/spree/core/product_duplicator.rb b/core/lib/spree/core/product_duplicator.rb index f2ba0d9383b..882592a3596 100644 --- a/core/lib/spree/core/product_duplicator.rb +++ b/core/lib/spree/core/product_duplicator.rb @@ -45,7 +45,7 @@ def duplicate_master new_master.sku = "COPY OF #{master.sku}" new_master.deleted_at = nil new_master.images = master.images.map { |image| duplicate_image image } if @include_images - new_master.price = master.price + new_master.prices = master.prices.map(&:dup) end end diff --git a/core/lib/spree/promotion/match_policies.rb b/core/lib/spree/promotion/match_policies.rb new file mode 100644 index 00000000000..712dbd029c9 --- /dev/null +++ b/core/lib/spree/promotion/match_policies.rb @@ -0,0 +1,2 @@ +Spree::Promotion::MATCH_POLICIES = %w(all any) +Spree::Deprecation.warn('Spree::Promotion::MATCH_POLICIES is deprecated') diff --git a/core/lib/spree/testing_support/factories/taxon_factory.rb b/core/lib/spree/testing_support/factories/taxon_factory.rb index 8f1527a5b23..1d2a94752af 100644 --- a/core/lib/spree/testing_support/factories/taxon_factory.rb +++ b/core/lib/spree/testing_support/factories/taxon_factory.rb @@ -10,11 +10,13 @@ FactoryBot.define do factory :taxon, class: 'Spree::Taxon' do name { 'Ruby on Rails' } - taxonomy - parent_id { nil } + taxonomy_id { (parent&.taxonomy || create(:taxonomy)).id } + parent_id { parent&.id || taxonomy.root.id } trait :with_icon do - icon { Spree::Core::Engine.root.join('lib', 'spree', 'testing_support', 'fixtures', 'blank.jpg').open } + after(:create) do |taxon| + taxon.update(icon: Spree::Core::Engine.root.join('lib', 'spree', 'testing_support', 'fixtures', 'blank.jpg').open) + end end end end diff --git a/core/lib/spree/testing_support/factories/taxonomy_factory.rb b/core/lib/spree/testing_support/factories/taxonomy_factory.rb index 029f2c86669..da69e768f71 100644 --- a/core/lib/spree/testing_support/factories/taxonomy_factory.rb +++ b/core/lib/spree/testing_support/factories/taxonomy_factory.rb @@ -7,6 +7,8 @@ FactoryBot.define do factory :taxonomy, class: 'Spree::Taxonomy' do - name { 'Brand' } + sequence :name do |seq| + "Brand #{seq}" + end end end diff --git a/core/spec/generators/solidus/install/install_generator_spec.rb b/core/spec/generators/solidus/install/install_generator_spec.rb index 88f4b7ce26f..fe110bdc526 100644 --- a/core/spec/generators/solidus/install/install_generator_spec.rb +++ b/core/spec/generators/solidus/install/install_generator_spec.rb @@ -110,10 +110,11 @@ expect(questions.first[:default]).to eq('paypal') expect(strip_ansi questions.first[:desc]).to include('[paypal]') expect(strip_ansi questions.first[:desc]).to include('[bolt]') + expect(strip_ansi questions.first[:desc]).to_not include('[braintree]') expect(strip_ansi questions.first[:desc]).to include('[none]') end - it 'presents different options for the "classic"' do + it 'presents different options for the "starter"' do questions = [] generator = described_class.new([], ['--frontend=starter', '--authentication=devise']) allow(generator).to receive(:ask_with_description) { |**args| questions << args } @@ -121,10 +122,11 @@ generator.prepare_options expect(questions.size).to eq(1) - expect(questions.first[:limited_to]).to eq(['paypal', 'none']) + expect(questions.first[:limited_to]).to eq(['paypal', 'braintree', 'none']) expect(questions.first[:default]).to eq('paypal') expect(strip_ansi questions.first[:desc]).to include('[paypal]') expect(strip_ansi questions.first[:desc]).not_to include('[bolt]') + expect(strip_ansi questions.first[:desc]).to include('[braintree]') expect(strip_ansi questions.first[:desc]).to include('[none]') end end diff --git a/core/spec/lib/generators/solidus/update/update_generator_spec.rb b/core/spec/lib/generators/solidus/update/update_generator_spec.rb index c7af360bedb..d2d05146e4d 100644 --- a/core/spec/lib/generators/solidus/update/update_generator_spec.rb +++ b/core/spec/lib/generators/solidus/update/update_generator_spec.rb @@ -12,6 +12,7 @@ Rails::Generators.invoke('solidus:update', [ "--initializer_directory=#{Rails.root.join('tmp')}", "--previous_version_prompt=false", + "--install_migrations=false", "--from=#{from}", "--to=#{to}", "--quiet" diff --git a/core/spec/lib/spree/app_configuration_spec.rb b/core/spec/lib/spree/app_configuration_spec.rb index db5fe174f4f..6d2cf5efef7 100644 --- a/core/spec/lib/spree/app_configuration_spec.rb +++ b/core/spec/lib/spree/app_configuration_spec.rb @@ -177,4 +177,36 @@ class DummyClass; end; expect(prefs.mails_from).to eq('solidus@example.com') end end + + context 'extra_taxon_validations=' do + it 'is deprecated' do + expect(Spree::Deprecation).to receive(:warn).with(/Solidus will remove `Spree::Config.extra_taxon_validations`/) + + prefs.extra_taxon_validations=(false) + end + + it "still sets the value so that consumers aren't broken" do + Spree::Deprecation.silence do + prefs.extra_taxon_validations=(false) + end + + expect(prefs.extra_taxon_validations).to eq(false) + end + end + + context 'extra_taxonomy_validations=' do + it 'is deprecated' do + expect(Spree::Deprecation).to receive(:warn).with(/Solidus will remove `Spree::Config.extra_taxonomy_validations`/) + + prefs.extra_taxonomy_validations=(false) + end + + it "still sets the value so that consumers aren't broken" do + Spree::Deprecation.silence do + prefs.extra_taxonomy_validations=(false) + end + + expect(prefs.extra_taxonomy_validations).to eq(false) + end + end end diff --git a/core/spec/lib/spree/core/testing_support/factories/taxon_factory_spec.rb b/core/spec/lib/spree/core/testing_support/factories/taxon_factory_spec.rb index b24fdadf92e..6ee5008495b 100644 --- a/core/spec/lib/spree/core/testing_support/factories/taxon_factory_spec.rb +++ b/core/spec/lib/spree/core/testing_support/factories/taxon_factory_spec.rb @@ -9,5 +9,46 @@ let(:factory) { :taxon } it_behaves_like 'a working factory' + + context "when no taxonomy is given" do + subject { create(:taxon) } + + before do + # ensure that the subject is not accidentally + # getting valid parent and taxonomy ids + create(:taxon) + end + + it "sets its taxonomy to created one and its parent to the taxonomy root" do + expect(subject).to be_valid + expect(subject.taxonomy).to be_present + expect(subject.parent).to eq(subject.taxonomy.root) + expect(subject).to_not eq(subject.taxonomy.root) + end + end + + context "when a taxonomy is given" do + subject { create(:taxon, taxonomy: taxonomy) } + + let!(:taxonomy) { create(:taxonomy) } + + it "sets the taxonomy to the given one and its parent to the taxonomy root" do + expect(subject).to be_valid + expect(subject.taxonomy).to eq(taxonomy) + expect(subject.parent).to eq(taxonomy.root) + end + end + + context "when a parent is given" do + subject { create(:taxon, parent: parent_taxon) } + + let!(:parent_taxon) { create(:taxon) } + + it "sets the parent to the given one and its taxonomy to the parent's taxonomy" do + expect(subject).to be_valid + expect(subject.parent).to eq(parent_taxon) + expect(subject.taxonomy).to eq(parent_taxon.taxonomy) + end + end end end diff --git a/core/spec/models/spree/fulfilment_changer_spec.rb b/core/spec/models/spree/fulfilment_changer_spec.rb index 897d82835cd..1ff25e76453 100644 --- a/core/spec/models/spree/fulfilment_changer_spec.rb +++ b/core/spec/models/spree/fulfilment_changer_spec.rb @@ -4,6 +4,7 @@ RSpec.describe Spree::FulfilmentChanger do let(:variant) { create(:variant) } + let(:track_inventory) { true } let(:order) do create( @@ -20,13 +21,16 @@ let(:current_shipment) { order.shipments.first } let(:desired_shipment) { order.shipments.create!(stock_location: desired_stock_location) } let(:desired_stock_location) { current_shipment.stock_location } + let(:current_shipment_inventory_unit_count) { 1 } + let(:quantity) { current_shipment_inventory_unit_count } let(:shipment_splitter) do described_class.new( current_shipment: current_shipment, desired_shipment: desired_shipment, variant: variant, - quantity: quantity + quantity: quantity, + track_inventory: track_inventory ) end @@ -38,9 +42,6 @@ end context "when the current shipment stock location is the same of the target shipment" do - let(:current_shipment_inventory_unit_count) { 1 } - let(:quantity) { current_shipment_inventory_unit_count } - context "when the stock location is empty" do before do variant.stock_items.first.update_column(:count_on_hand, 0) @@ -70,6 +71,122 @@ end end + context "when track_inventory is not passed" do + let(:shipment_splitter) do + described_class.new( + current_shipment: current_shipment, + desired_shipment: desired_shipment, + variant: variant, + quantity: quantity + ) + end + + it "defaults to true with a deprecation warning" do + expect(Spree::Deprecation).to receive(:warn).with(/Not passing `track_inventory` to `Spree::FulfilmentChanger` is deprecated./) + expect(shipment_splitter.track_inventory).to be true + end + end + + context "when tracking inventory is not set (same as false)" do + let(:current_shipment_inventory_unit_count) { 2 } + let(:quantity) { 1 } + let(:track_inventory) { nil } + + it "adds the desired inventory units to the desired shipment" do + expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity) + end + + it "removes the desired inventory units from the current shipment" do + expect { subject }.to change { current_shipment.inventory_units.length }.by(-quantity) + end + + it "recalculates shipping costs for the current shipment" do + expect(current_shipment).to receive(:refresh_rates) + subject + end + + it 'updates order totals' do + original_total = order.total + original_shipment_total = order.shipment_total + + expect { subject }. + to change { order.total }.from(original_total).to(original_total + original_shipment_total). + and change { order.shipment_total }.by(original_shipment_total) + end + + context "when transferring to another stock location" do + let(:desired_stock_location) { create(:stock_location) } + let!(:stock_item) do + variant.stock_items.find_or_create_by!( + stock_location: desired_stock_location, + variant: variant, + ) + end + + it "is marked as a successful transfer" do + expect(subject).to be true + end + + it "does not stock in the current stock location" do + expect { subject }.not_to change { current_shipment.stock_location.count_on_hand(variant) } + end + + it "does not unstock the desired stock location" do + expect { subject }.not_to change { desired_shipment.stock_location.count_on_hand(variant) } + end + end + end + + context "when not tracking inventory" do + let(:current_shipment_inventory_unit_count) { 2 } + let(:quantity) { 1 } + let(:track_inventory) { false } + + it "adds the desired inventory units to the desired shipment" do + expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity) + end + + it "removes the desired inventory units from the current shipment" do + expect { subject }.to change { current_shipment.inventory_units.length }.by(-quantity) + end + + it "recalculates shipping costs for the current shipment" do + expect(current_shipment).to receive(:refresh_rates) + subject + end + + it 'updates order totals' do + original_total = order.total + original_shipment_total = order.shipment_total + + expect { subject }. + to change { order.total }.from(original_total).to(original_total + original_shipment_total). + and change { order.shipment_total }.by(original_shipment_total) + end + + context "when transferring to another stock location" do + let(:desired_stock_location) { create(:stock_location) } + let!(:stock_item) do + variant.stock_items.find_or_create_by!( + stock_location: desired_stock_location, + variant: variant, + ) + end + + it "is marked as a successful transfer" do + expect(subject).to be true + end + + it "does not stock in the current stock location" do + expect { subject }.not_to change { current_shipment.stock_location.count_on_hand(variant) } + end + + it "does not unstock the desired stock location" do + expect { subject }.not_to change { desired_shipment.stock_location.count_on_hand(variant) } + end + end + end + context "when the current shipment has enough inventory units" do let(:current_shipment_inventory_unit_count) { 2 } let(:quantity) { 1 } diff --git a/core/spec/models/spree/product/scopes_spec.rb b/core/spec/models/spree/product/scopes_spec.rb index e5699e320ec..9be70c23383 100644 --- a/core/spec/models/spree/product/scopes_spec.rb +++ b/core/spec/models/spree/product/scopes_spec.rb @@ -116,6 +116,18 @@ end end + context "descend_by_popularity" do + it "orders products by popularity" do + variant_1 = create(:master_variant) + variant_2 = create(:master_variant) + + create_list(:line_item, 3, variant: variant_1) + create_list(:line_item, 2, variant: variant_2) + + expect(Spree::Product.descend_by_popularity.map(&:id)).to eq([variant_1.product.id, variant_2.product.id, product.id]) + end + end + describe '.available' do context "a product with past available_on" do let!(:product) { create(:product, available_on: 1.day.ago) } diff --git a/core/spec/models/spree/product_duplicator_spec.rb b/core/spec/models/spree/product_duplicator_spec.rb index 7965b556b3c..e5720c0642a 100644 --- a/core/spec/models/spree/product_duplicator_spec.rb +++ b/core/spec/models/spree/product_duplicator_spec.rb @@ -61,6 +61,10 @@ module Spree expect(new_product.name).to eql "COPY OF #{product.name}" end + it "will set the same price" do + expect(new_product.reload.price).to eql product.price + end + it "will set an unique sku" do expect(new_product.sku).to include "COPY OF SKU" end diff --git a/core/spec/models/spree/promotion_spec.rb b/core/spec/models/spree/promotion_spec.rb index 1dcdc483cae..ff848a65299 100644 --- a/core/spec/models/spree/promotion_spec.rb +++ b/core/spec/models/spree/promotion_spec.rb @@ -993,4 +993,11 @@ expect(order.adjustment_total).to eq(-10) end end + + describe "MATCH_POLICIES" do + it "prints a deprecation warning when used" do + expect(Spree::Deprecation).to receive(:warn).once.with(/Spree::Promotion::MATCH_POLICIES is deprecated/) + expect(Spree::Promotion::MATCH_POLICIES).to eq %w(all any) + end + end end diff --git a/core/spec/models/spree/taxon_spec.rb b/core/spec/models/spree/taxon_spec.rb index 8e92960bf04..6040ad53474 100644 --- a/core/spec/models/spree/taxon_spec.rb +++ b/core/spec/models/spree/taxon_spec.rb @@ -53,7 +53,8 @@ end context "set_permalink" do - let(:taxon) { FactoryBot.build(:taxon, name: "Ruby on Rails") } + let(:taxonomy) { create(:taxonomy, name: "Ruby on Rails") } + let(:taxon) { taxonomy.root } it "should set permalink correctly when no parent present" do taxon.set_permalink @@ -175,12 +176,36 @@ end end - # Regression test for https://github.com/spree/spree/issues/2620 - context "creating a child node using first_or_create" do - let(:taxonomy) { create(:taxonomy) } + context "validations" do + context "taxonomy_id validations" do + let(:taxonomy) { create(:taxonomy) } + let(:taxon) { taxonomy.taxons.create(name: 'New node') } + + it "ensures that only one root can be created" do + expect(taxon).to be_invalid + expect(taxon.errors.full_messages).to match_array(["Taxonomy can only have one root Taxon"]) + end + + it "allows for multiple taxons under taxonomy" do + expect(taxon.update(parent_id: taxonomy.root.id)).to eq(true) + expect(taxonomy.taxons.many?).to eq(true) + end + end + + context "name validations" do + let!(:taxonomy) { create(:taxonomy) } + let!(:taxon_level_one) { create(:taxon, name: 'Solidus', parent: taxonomy.root) } + let(:taxon_level_one_duplicate) { build(:taxon, name: 'Solidus', parent: taxonomy.root) } + let(:taxon_level_two) { create(:taxon, name: 'Solidus', parent: taxon_level_one) } - it "does not error out" do - taxonomy.root.children.unscoped.where(name: "Some name").first_or_create + it "ensures that taxons with the same parent must have unique names" do + expect(taxon_level_one_duplicate.save).to eq(false) + expect(taxon_level_one_duplicate.errors.full_messages).to match_array(["Name must be unique under the same parent Taxon"]) + end + + it "allows for multiple taxons with the same name under different parents" do + expect(taxon_level_two).to be_valid + end end end diff --git a/core/spec/models/spree/taxonomy_spec.rb b/core/spec/models/spree/taxonomy_spec.rb index 131ce22361a..4494653d0a4 100644 --- a/core/spec/models/spree/taxonomy_spec.rb +++ b/core/spec/models/spree/taxonomy_spec.rb @@ -3,6 +3,19 @@ require 'rails_helper' RSpec.describe Spree::Taxonomy, type: :model do + context "validations" do + subject { build(:taxonomy, name: 'Brand') } + + let!(:taxonomy) { create(:taxonomy, name: 'Brand') } + + context "name validations" do + it "ensures Taxonomies must have unique names" do + expect(subject.save).to eq(false) + expect(subject.errors.full_messages).to match_array(["Name has already been taken"]) + end + end + end + context "#destroy" do subject(:association_options) do described_class.reflect_on_association(:root).options