From 0e83b33f710b7112a237ac23560b667f0e0fbf6a Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 15 Nov 2017 14:50:52 -0800 Subject: [PATCH] Remove stock transfers from core --- core/app/models/spree/stock_transfer.rb | 110 ------- core/app/models/spree/transfer_item.rb | 54 --- core/lib/spree/permission_sets.rb | 4 - .../restricted_stock_transfer_display.rb | 17 - .../restricted_stock_transfer_management.rb | 52 --- .../permission_sets/stock_transfer_display.rb | 10 - .../stock_transfer_management.rb | 11 - core/lib/spree/permitted_attributes.rb | 3 - .../factories/stock_transfer_factory.rb | 31 -- .../factories/stock_transfer_factory_spec.rb | 12 - .../restricted_stock_transfer_display_spec.rb | 49 --- ...stricted_stock_transfer_management_spec.rb | 218 ------------- .../stock_transfer_display_spec.rb | 23 -- .../stock_transfer_management_spec.rb | 23 -- core/spec/models/spree/stock_transfer_spec.rb | 308 ------------------ core/spec/models/spree/transfer_item_spec.rb | 282 ---------------- 16 files changed, 1207 deletions(-) delete mode 100644 core/app/models/spree/stock_transfer.rb delete mode 100644 core/app/models/spree/transfer_item.rb delete mode 100644 core/lib/spree/permission_sets/restricted_stock_transfer_display.rb delete mode 100644 core/lib/spree/permission_sets/restricted_stock_transfer_management.rb delete mode 100644 core/lib/spree/permission_sets/stock_transfer_display.rb delete mode 100644 core/lib/spree/permission_sets/stock_transfer_management.rb delete mode 100644 core/lib/spree/testing_support/factories/stock_transfer_factory.rb delete mode 100644 core/spec/lib/spree/core/testing_support/factories/stock_transfer_factory_spec.rb delete mode 100644 core/spec/models/spree/permission_sets/restricted_stock_transfer_display_spec.rb delete mode 100644 core/spec/models/spree/permission_sets/restricted_stock_transfer_management_spec.rb delete mode 100644 core/spec/models/spree/permission_sets/stock_transfer_display_spec.rb delete mode 100644 core/spec/models/spree/permission_sets/stock_transfer_management_spec.rb delete mode 100644 core/spec/models/spree/stock_transfer_spec.rb delete mode 100644 core/spec/models/spree/transfer_item_spec.rb diff --git a/core/app/models/spree/stock_transfer.rb b/core/app/models/spree/stock_transfer.rb deleted file mode 100644 index 636d1e54860..00000000000 --- a/core/app/models/spree/stock_transfer.rb +++ /dev/null @@ -1,110 +0,0 @@ -module Spree - class StockTransfer < Spree::Base - class InvalidTransferMovement < StandardError; end - - acts_as_paranoid - - has_many :stock_movements, as: :originator - has_many :transfer_items, inverse_of: :stock_transfer - - belongs_to :created_by, class_name: Spree::UserClassHandle.new - belongs_to :finalized_by, class_name: Spree::UserClassHandle.new - belongs_to :closed_by, class_name: Spree::UserClassHandle.new - belongs_to :source_location, class_name: 'Spree::StockLocation' - belongs_to :destination_location, class_name: 'Spree::StockLocation' - - validates_presence_of :source_location - validates_presence_of :destination_location, if: :finalized? - - make_permalink field: :number, prefix: 'T' - - before_destroy :ensure_not_finalized - - self.whitelisted_ransackable_attributes = %w[source_location_id destination_location_id closed_at created_at number] - - def to_param - number - end - - def finalized? - finalized_at.present? - end - - def closed? - closed_at.present? - end - - def shipped? - shipped_at.present? - end - - def finalizable? - !finalized? && !shipped? && !closed? - end - - def receivable? - finalized? && shipped? && !closed? - end - - def ship(tracking_number: self.tracking_number, shipped_at: nil) - update_attributes!(tracking_number: tracking_number, shipped_at: shipped_at) - end - - def received_item_count - transfer_items.sum(:received_quantity) - end - - def expected_item_count - transfer_items.sum(:expected_quantity) - end - - def source_movements - stock_movements.joins(:stock_item) - .where('spree_stock_items.stock_location_id' => source_location_id) - end - - def destination_movements - stock_movements.joins(:stock_item) - .where('spree_stock_items.stock_location_id' => destination_location_id) - end - - def finalize(finalized_by) - if finalizable? - update_attributes({ finalized_at: Time.current, finalized_by: finalized_by }) - else - errors.add(:base, I18n.t('spree.stock_transfer_cannot_be_finalized')) - false - end - end - - def transfer - transaction do - transfer_items.each do |item| - raise InvalidTransferMovement unless item.valid? - source_location.unstock(item.variant, item.expected_quantity, self) - end - end - rescue InvalidTransferMovement - errors.add(:base, I18n.t('spree.not_enough_stock')) - false - end - - def close(closed_by) - if receivable? - update_attributes({ closed_at: Time.current, closed_by: closed_by }) - else - errors.add(:base, I18n.t('spree.stock_transfer_must_be_receivable')) - false - end - end - - private - - def ensure_not_finalized - if finalized? - errors.add(:base, I18n.t('spree.errors.messages.cannot_delete_finalized_stock_transfer')) - throw :abort - end - end - end -end diff --git a/core/app/models/spree/transfer_item.rb b/core/app/models/spree/transfer_item.rb deleted file mode 100644 index dcc497a0d8d..00000000000 --- a/core/app/models/spree/transfer_item.rb +++ /dev/null @@ -1,54 +0,0 @@ -module Spree - class TransferItem < Spree::Base - acts_as_paranoid - belongs_to :stock_transfer, inverse_of: :transfer_items - belongs_to :variant, -> { with_deleted } - - validate :stock_availability, if: :check_stock? - validates :stock_transfer, :variant, presence: true - validates :variant_id, uniqueness: { scope: :stock_transfer_id }, allow_blank: true - validates :expected_quantity, numericality: { greater_than: 0 } - validates :received_quantity, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: :expected_quantity } - - scope :received, -> { where('received_quantity > 0') } - scope :fully_received, -> { where('expected_quantity = received_quantity') } - scope :partially_received, -> { received.where('expected_quantity > received_quantity') } - - before_destroy :ensure_stock_transfer_not_finalized - before_validation :ensure_stock_transfer_not_closed - before_update :prevent_expected_quantity_update_stock_transfer_finalized - - private - - def ensure_stock_transfer_not_closed - if stock_transfer.closed? - errors.add(:base, I18n.t('spree.errors.messages.cannot_modify_transfer_item_closed_stock_transfer')) - end - end - - def ensure_stock_transfer_not_finalized - unless stock_transfer.finalizable? - errors.add(:base, I18n.t('spree.errors.messages.cannot_delete_transfer_item_with_finalized_stock_transfer')) - throw :abort - end - end - - def prevent_expected_quantity_update_stock_transfer_finalized - if expected_quantity_changed? && stock_transfer.finalized? - errors.add(:base, I18n.t('spree.errors.messages.cannot_update_expected_transfer_item_with_finalized_stock_transfer')) - throw :abort - end - end - - def stock_availability - stock_item = variant.stock_items.find_by(stock_location: stock_transfer.source_location) - if stock_item.nil? || stock_item.count_on_hand < expected_quantity - errors.add(:base, I18n.t('spree.errors.messages.transfer_item_insufficient_stock')) - end - end - - def check_stock? - !stock_transfer.shipped? && stock_transfer.source_location.check_stock_on_transfer? - end - end -end diff --git a/core/lib/spree/permission_sets.rb b/core/lib/spree/permission_sets.rb index 47e2051bc2d..6f5b85be409 100644 --- a/core/lib/spree/permission_sets.rb +++ b/core/lib/spree/permission_sets.rb @@ -12,12 +12,8 @@ require 'spree/permission_sets/report_display' require 'spree/permission_sets/restricted_stock_display' require 'spree/permission_sets/restricted_stock_management' -require 'spree/permission_sets/restricted_stock_transfer_display' -require 'spree/permission_sets/restricted_stock_transfer_management' require 'spree/permission_sets/stock_display' require 'spree/permission_sets/stock_management' -require 'spree/permission_sets/stock_transfer_display' -require 'spree/permission_sets/stock_transfer_management' require 'spree/permission_sets/super_user' require 'spree/permission_sets/user_display' require 'spree/permission_sets/user_management' diff --git a/core/lib/spree/permission_sets/restricted_stock_transfer_display.rb b/core/lib/spree/permission_sets/restricted_stock_transfer_display.rb deleted file mode 100644 index 219b162f0ce..00000000000 --- a/core/lib/spree/permission_sets/restricted_stock_transfer_display.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Spree - module PermissionSets - class RestrictedStockTransferDisplay < PermissionSets::Base - def activate! - can [:display, :admin], Spree::StockTransfer, source_location_id: location_ids - can [:display, :admin], Spree::StockTransfer, destination_location_id: location_ids - can :display, Spree::StockLocation, id: location_ids - end - - private - - def location_ids - @ids ||= user.stock_locations.pluck(:id) - end - end - end -end diff --git a/core/lib/spree/permission_sets/restricted_stock_transfer_management.rb b/core/lib/spree/permission_sets/restricted_stock_transfer_management.rb deleted file mode 100644 index 125978bfbd9..00000000000 --- a/core/lib/spree/permission_sets/restricted_stock_transfer_management.rb +++ /dev/null @@ -1,52 +0,0 @@ -module Spree - module PermissionSets - # This is a permission set that offers an alternative to {StockManagement}. - # - # Instead of allowing management access for all stock transfers and items, only allow - # the management of stock transfers for locations the user is associated with. - # - # Users can be associated with stock locations via the admin user interface. - # - # The logic here is unfortunately rather complex and boils down to: - # - A user has read only access to all stock locations (including inactive ones) - # - A user can see all stock transfers for their associated stock locations regardless of the - # fact that they may not be associated with both the destination and the source, as long as - # they are associated with at least one of the two. - # - A user can manage stock transfers only if they are associated with both the destination and the source, - # or if the user is associated with the source, and the transfer has not yet been assigned a destination. - # - # @see Spree::PermissionSets::Base - class RestrictedStockTransferManagement < PermissionSets::Base - def activate! - if user.stock_locations.any? - can :display, Spree::StockLocation, id: user_location_ids - - can :transfer_from, Spree::StockLocation, id: user_location_ids - can :transfer_to, Spree::StockLocation, id: user_location_ids - - can :display, Spree::StockTransfer, source_location_id: user_location_ids - can :manage, Spree::StockTransfer, source_location_id: user_location_ids + [nil], shipped_at: nil - can :manage, Spree::StockTransfer, destination_location_id: user_location_ids - # Do not allow managing transfers to a permitted destination_location_id from an - # unauthorized stock location until it's been shipped to the permitted location. - cannot :manage, Spree::StockTransfer, source_location_id: not_permitted_location_ids, shipped_at: nil - - can :display, Spree::TransferItem, stock_transfer: { source_location_id: user_location_ids } - can :manage, Spree::TransferItem, stock_transfer: { source_location_id: user_location_ids + [nil], shipped_at: nil } - can :manage, Spree::TransferItem, stock_transfer: { destination_location_id: user_location_ids } - cannot :manage, Spree::TransferItem, stock_transfer: { source_location_id: not_permitted_location_ids, shipped_at: nil } - end - end - - private - - def user_location_ids - @user_location_ids ||= user.stock_locations.pluck(:id) - end - - def not_permitted_location_ids - @not_permitted_location_ids ||= Spree::StockLocation.where.not(id: user_location_ids).pluck(:id) - end - end - end -end diff --git a/core/lib/spree/permission_sets/stock_transfer_display.rb b/core/lib/spree/permission_sets/stock_transfer_display.rb deleted file mode 100644 index 94e103d53e5..00000000000 --- a/core/lib/spree/permission_sets/stock_transfer_display.rb +++ /dev/null @@ -1,10 +0,0 @@ -module Spree - module PermissionSets - class StockTransferDisplay < PermissionSets::Base - def activate! - can [:display, :admin], Spree::StockTransfer - can :display, Spree::StockLocation - end - end - end -end diff --git a/core/lib/spree/permission_sets/stock_transfer_management.rb b/core/lib/spree/permission_sets/stock_transfer_management.rb deleted file mode 100644 index 482c300757c..00000000000 --- a/core/lib/spree/permission_sets/stock_transfer_management.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Spree - module PermissionSets - class StockTransferManagement < PermissionSets::Base - def activate! - can :manage, Spree::StockTransfer - can :manage, Spree::TransferItem - can :display, Spree::StockLocation - end - end - end -end diff --git a/core/lib/spree/permitted_attributes.rb b/core/lib/spree/permitted_attributes.rb index 69a8383d983..d5f2ca9cdb4 100644 --- a/core/lib/spree/permitted_attributes.rb +++ b/core/lib/spree/permitted_attributes.rb @@ -27,7 +27,6 @@ module PermittedAttributes :store_attributes, :taxon_attributes, :taxonomy_attributes, - :transfer_item_attributes, :user_attributes, :variant_attributes ] @@ -116,8 +115,6 @@ module PermittedAttributes :meta_description, :meta_keywords, :meta_title, :child_index ] - @@transfer_item_attributes = [:variant_id, :expected_quantity, :received_quantity] - # intentionally leaving off email here to prevent privilege escalation # by changing a user with higher priveleges' email to one a lower-priveleged # admin owns. creating a user with an email is handled separate at the diff --git a/core/lib/spree/testing_support/factories/stock_transfer_factory.rb b/core/lib/spree/testing_support/factories/stock_transfer_factory.rb deleted file mode 100644 index 6c3723a07b3..00000000000 --- a/core/lib/spree/testing_support/factories/stock_transfer_factory.rb +++ /dev/null @@ -1,31 +0,0 @@ -FactoryBot.define do - sequence(:source_code) { |n| "SRC#{n}" } - sequence(:destination_code) { |n| "DEST#{n}" } - - factory :stock_transfer, class: 'Spree::StockTransfer' do - source_location { Spree::StockLocation.create!(name: "Source Location", code: generate(:source_code), admin_name: "Source") } - - factory :stock_transfer_with_items do - destination_location { Spree::StockLocation.create!(name: "Destination Location", code: generate(:destination_code), admin_name: "Destination") } - - after(:create) do |stock_transfer, _evaluator| - variant_1 = create(:variant) - variant_2 = create(:variant) - - variant_1.stock_items.find_by(stock_location: stock_transfer.source_location).set_count_on_hand(10) - variant_2.stock_items.find_by(stock_location: stock_transfer.source_location).set_count_on_hand(10) - - stock_transfer.transfer_items.create(variant: variant_1, expected_quantity: 5) - stock_transfer.transfer_items.create(variant: variant_2, expected_quantity: 5) - - stock_transfer.created_by = create(:admin_user) - stock_transfer.save! - end - - factory :receivable_stock_transfer_with_items do - finalized_at { Time.current } - shipped_at { Time.current } - end - end - end -end diff --git a/core/spec/lib/spree/core/testing_support/factories/stock_transfer_factory_spec.rb b/core/spec/lib/spree/core/testing_support/factories/stock_transfer_factory_spec.rb deleted file mode 100644 index 77dd377929c..00000000000 --- a/core/spec/lib/spree/core/testing_support/factories/stock_transfer_factory_spec.rb +++ /dev/null @@ -1,12 +0,0 @@ -require 'rails_helper' -require 'spree/testing_support/factories/stock_transfer_factory' - -RSpec.describe 'stock transfer factory' do - let(:factory_class) { Spree::StockTransfer } - - describe 'plain stock transfer' do - let(:factory) { :stock_transfer } - - it_behaves_like 'a working factory' - end -end diff --git a/core/spec/models/spree/permission_sets/restricted_stock_transfer_display_spec.rb b/core/spec/models/spree/permission_sets/restricted_stock_transfer_display_spec.rb deleted file mode 100644 index 510a2b9f3f3..00000000000 --- a/core/spec/models/spree/permission_sets/restricted_stock_transfer_display_spec.rb +++ /dev/null @@ -1,49 +0,0 @@ -require 'rails_helper' - -RSpec.describe Spree::PermissionSets::RestrictedStockTransferDisplay do - let(:ability) { Spree::Ability.new(user) } - let(:user) { create :user } - - subject { ability } - - let!(:sl1) { create :stock_location, active: false } - let!(:sl2) { create :stock_location, active: false } - - let!(:source_transfer) { create :stock_transfer, source_location: sl1 } - let!(:other_source_transfer) { create :stock_transfer, source_location: sl2 } - let!(:dest_transfer) { create :stock_transfer, source_location: sl2, destination_location: sl1 } - - before do - user.stock_locations << sl1 - end - - context "when activated" do - before do - described_class.new(ability).activate! - end - - it { is_expected.to be_able_to(:display, sl1) } - it { is_expected.to_not be_able_to(:display, sl2) } - - it { is_expected.to be_able_to(:display, source_transfer) } - it { is_expected.to_not be_able_to(:display, other_source_transfer) } - it { is_expected.to be_able_to(:display, dest_transfer) } - - it { is_expected.to be_able_to(:admin, source_transfer) } - it { is_expected.to_not be_able_to(:admin, other_source_transfer) } - it { is_expected.to be_able_to(:admin, dest_transfer) } - end - - context "when not activated" do - it { is_expected.to_not be_able_to(:display, sl1) } - it { is_expected.to_not be_able_to(:display, sl2) } - - it { is_expected.to_not be_able_to(:display, source_transfer) } - it { is_expected.to_not be_able_to(:display, other_source_transfer) } - it { is_expected.to_not be_able_to(:display, dest_transfer) } - - it { is_expected.to_not be_able_to(:admin, source_transfer) } - it { is_expected.to_not be_able_to(:admin, other_source_transfer) } - it { is_expected.to_not be_able_to(:admin, dest_transfer) } - end -end diff --git a/core/spec/models/spree/permission_sets/restricted_stock_transfer_management_spec.rb b/core/spec/models/spree/permission_sets/restricted_stock_transfer_management_spec.rb deleted file mode 100644 index 987cac45a72..00000000000 --- a/core/spec/models/spree/permission_sets/restricted_stock_transfer_management_spec.rb +++ /dev/null @@ -1,218 +0,0 @@ -require 'rails_helper' - -RSpec.describe Spree::PermissionSets::RestrictedStockTransferManagement do - let(:ability) { Spree::Ability.new(user) } - - subject { ability } - - # Inactive stock locations will default to not being visible - # for users without explicit permissions. - let!(:source_location) { create :stock_location, active: false } - let!(:destination_location) { create :stock_location, active: false } - - # This has the side effect of creating a stock item for each stock location above, - # which is what we actually want. - let!(:variant) { create :variant } - - let(:transfer_with_source) { create :stock_transfer, source_location: source_location } - let(:transfer_with_destination) { create :stock_transfer, source_location: destination_location } - let(:transfer_with_source_and_destination) do - create :stock_transfer, source_location: source_location, destination_location: destination_location - end - - let(:transfer_amount) { 1 } - let(:source_transfer_item) do - transfer_with_source.transfer_items.create(variant: variant, expected_quantity: transfer_amount) - end - let(:destination_transfer_item) do - transfer_with_destination.transfer_items.create(variant: variant, expected_quantity: transfer_amount) - end - let(:source_and_destination_transfer_item) do - transfer_with_source_and_destination.transfer_items.create(variant: variant, expected_quantity: transfer_amount) - end - - context "when activated" do - let(:user) { create :user, stock_locations: stock_locations } - let(:stock_locations) { [] } - - before do - user.stock_locations = stock_locations - # When creating transfer_items for a stock transfer, stock items must have a count on hand - # with an amount that would allow a transfer item to pass validations (meaning the count on hand has to be equal - # to the expected_quantity for the transfer) - variant.stock_items.update_all count_on_hand: transfer_amount - - described_class.new(ability).activate! - end - - context "when the user is only associated with the source location" do - let(:stock_locations) { [source_location] } - - it { is_expected.to be_able_to(:display, source_location) } - it { is_expected.to_not be_able_to(:display, destination_location) } - - it { is_expected.to be_able_to(:display, Spree::StockTransfer) } - it { is_expected.to be_able_to(:admin, Spree::StockTransfer) } - it { is_expected.to be_able_to(:create, Spree::StockTransfer) } - - it { is_expected.to be_able_to(:transfer_from, source_location) } - it { is_expected.to be_able_to(:transfer_to, source_location) } - - it { is_expected.to_not be_able_to(:transfer_from, destination_location) } - it { is_expected.to_not be_able_to(:transfer_to, destination_location) } - - it { is_expected.to be_able_to(:display, transfer_with_source) } - it { is_expected.to be_able_to(:display, transfer_with_source_and_destination) } - it { is_expected.to_not be_able_to(:display, transfer_with_destination) } - - it { is_expected.to be_able_to(:manage, transfer_with_source) } - it { is_expected.to be_able_to(:manage, transfer_with_source_and_destination) } - it { is_expected.to_not be_able_to(:manage, transfer_with_destination) } - - it { is_expected.to be_able_to(:manage, source_transfer_item) } - it { is_expected.to be_able_to(:manage, source_and_destination_transfer_item) } - it { is_expected.to_not be_able_to(:manage, destination_transfer_item) } - - context "stock transfer has been shipped" do - before do - transfer_with_source_and_destination.update_attributes!(shipped_at: Time.current) - described_class.new(ability).activate! - end - - it { is_expected.to_not be_able_to(:manage, transfer_with_source_and_destination) } - it { is_expected.to_not be_able_to(:manage, source_and_destination_transfer_item) } - end - end - - context "when the user is only associated with the destination location" do - let(:stock_locations) { [destination_location] } - - it { is_expected.to be_able_to(:display, destination_location) } - it { is_expected.to_not be_able_to(:display, source_location) } - - it { is_expected.to be_able_to(:display, Spree::StockTransfer) } - it { is_expected.to be_able_to(:admin, Spree::StockTransfer) } - it { is_expected.to be_able_to(:create, Spree::StockTransfer) } - - it { is_expected.to_not be_able_to(:transfer_from, source_location) } - it { is_expected.to_not be_able_to(:transfer_to, source_location) } - - it { is_expected.to be_able_to(:transfer_from, destination_location) } - it { is_expected.to be_able_to(:transfer_to, destination_location) } - - it { is_expected.to be_able_to(:display, transfer_with_destination) } - it { is_expected.to_not be_able_to(:display, transfer_with_source) } - it { is_expected.to_not be_able_to(:display, transfer_with_source_and_destination) } - - it { is_expected.to be_able_to(:manage, transfer_with_destination) } - it { is_expected.to_not be_able_to(:manage, transfer_with_source) } - it { is_expected.to_not be_able_to(:manage, transfer_with_source_and_destination) } - - it { is_expected.to be_able_to(:manage, destination_transfer_item) } - it { is_expected.to_not be_able_to(:manage, source_transfer_item) } - it { is_expected.to_not be_able_to(:manage, source_and_destination_transfer_item) } - - context "stock transfer has been shipped" do - before do - transfer_with_source_and_destination.update_attributes!(shipped_at: Time.current) - described_class.new(ability).activate! - end - - it { is_expected.to be_able_to(:manage, transfer_with_source_and_destination) } - it { is_expected.to be_able_to(:manage, source_and_destination_transfer_item) } - end - end - - context "when the user is associated with both locations" do - let(:stock_locations) { [source_location, destination_location] } - - it { is_expected.to be_able_to(:display, source_location) } - it { is_expected.to be_able_to(:display, destination_location) } - - it { is_expected.to be_able_to(:display, Spree::StockTransfer) } - it { is_expected.to be_able_to(:admin, Spree::StockTransfer) } - it { is_expected.to be_able_to(:create, Spree::StockTransfer) } - - it { is_expected.to be_able_to(:transfer_from, source_location) } - it { is_expected.to be_able_to(:transfer_to, source_location) } - - it { is_expected.to be_able_to(:transfer_from, destination_location) } - it { is_expected.to be_able_to(:transfer_to, destination_location) } - - it { is_expected.to be_able_to(:display, transfer_with_source) } - it { is_expected.to be_able_to(:display, transfer_with_source_and_destination) } - it { is_expected.to be_able_to(:display, transfer_with_destination) } - - it { is_expected.to be_able_to(:manage, transfer_with_source) } - it { is_expected.to be_able_to(:manage, transfer_with_destination) } - it { is_expected.to be_able_to(:manage, transfer_with_source_and_destination) } - - it { is_expected.to be_able_to(:manage, source_transfer_item) } - it { is_expected.to be_able_to(:manage, destination_transfer_item) } - it { is_expected.to be_able_to(:manage, source_and_destination_transfer_item) } - - context "stock transfer has been shipped" do - before do - transfer_with_source_and_destination.update_attributes!(shipped_at: Time.current) - described_class.new(ability).activate! - end - - it { is_expected.to be_able_to(:manage, transfer_with_source_and_destination) } - it { is_expected.to be_able_to(:manage, source_and_destination_transfer_item) } - end - end - - context "when the user is associated with neither location" do - let(:stock_locations) { [] } - - it { is_expected.to_not be_able_to(:display, source_location) } - it { is_expected.to_not be_able_to(:display, destination_location) } - - it { is_expected.to_not be_able_to(:display, Spree::StockTransfer) } - it { is_expected.to_not be_able_to(:admin, Spree::StockTransfer) } - it { is_expected.to_not be_able_to(:create, Spree::StockTransfer) } - - it { is_expected.to_not be_able_to(:transfer_from, source_location) } - it { is_expected.to_not be_able_to(:transfer_to, source_location) } - - it { is_expected.to_not be_able_to(:transfer_from, destination_location) } - it { is_expected.to_not be_able_to(:transfer_to, destination_location) } - - it { is_expected.to_not be_able_to(:manage, transfer_with_source) } - it { is_expected.to_not be_able_to(:manage, transfer_with_destination) } - it { is_expected.to_not be_able_to(:manage, transfer_with_source_and_destination) } - - it { is_expected.to_not be_able_to(:manage, source_transfer_item) } - it { is_expected.to_not be_able_to(:manage, destination_transfer_item) } - it { is_expected.to_not be_able_to(:manage, source_and_destination_transfer_item) } - end - end - - context "when not activated" do - let(:user) { create :user } - - it { is_expected.to_not be_able_to(:display, Spree::StockTransfer) } - it { is_expected.to_not be_able_to(:admin, Spree::StockTransfer) } - it { is_expected.to_not be_able_to(:create, Spree::StockTransfer) } - - it { is_expected.to_not be_able_to(:display, source_location) } - it { is_expected.to_not be_able_to(:display, destination_location) } - - it { is_expected.to_not be_able_to(:transfer_from, source_location) } - it { is_expected.to_not be_able_to(:transfer_to, source_location) } - - it { is_expected.to_not be_able_to(:transfer_from, destination_location) } - it { is_expected.to_not be_able_to(:transfer_to, destination_location) } - - it { is_expected.to_not be_able_to(:display, source_location) } - it { is_expected.to_not be_able_to(:display, destination_location) } - - it { is_expected.to_not be_able_to(:manage, transfer_with_source) } - it { is_expected.to_not be_able_to(:manage, transfer_with_destination) } - it { is_expected.to_not be_able_to(:manage, transfer_with_source_and_destination) } - - it { is_expected.to_not be_able_to(:manage, source_transfer_item) } - it { is_expected.to_not be_able_to(:manage, destination_transfer_item) } - it { is_expected.to_not be_able_to(:manage, source_and_destination_transfer_item) } - end -end diff --git a/core/spec/models/spree/permission_sets/stock_transfer_display_spec.rb b/core/spec/models/spree/permission_sets/stock_transfer_display_spec.rb deleted file mode 100644 index 1c8f50d6670..00000000000 --- a/core/spec/models/spree/permission_sets/stock_transfer_display_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'rails_helper' - -RSpec.describe Spree::PermissionSets::StockTransferDisplay do - let(:ability) { DummyAbility.new } - - subject { ability } - - context "when activated" do - before do - described_class.new(ability).activate! - end - - it { is_expected.to be_able_to(:display, Spree::StockTransfer) } - it { is_expected.to be_able_to(:admin, Spree::StockTransfer) } - it { is_expected.to be_able_to(:display, Spree::StockLocation) } - end - - context "when not activated" do - it { is_expected.not_to be_able_to(:display, Spree::StockTransfer) } - it { is_expected.not_to be_able_to(:admin, Spree::StockTransfer) } - it { is_expected.not_to be_able_to(:display, Spree::StockLocation) } - end -end diff --git a/core/spec/models/spree/permission_sets/stock_transfer_management_spec.rb b/core/spec/models/spree/permission_sets/stock_transfer_management_spec.rb deleted file mode 100644 index e33af15d8e4..00000000000 --- a/core/spec/models/spree/permission_sets/stock_transfer_management_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'rails_helper' - -RSpec.describe Spree::PermissionSets::StockTransferManagement do - let(:ability) { DummyAbility.new } - - subject { ability } - - context "when activated" do - before do - described_class.new(ability).activate! - end - - it { is_expected.to be_able_to(:manage, Spree::StockTransfer) } - it { is_expected.to be_able_to(:manage, Spree::TransferItem) } - it { is_expected.to be_able_to(:display, Spree::StockLocation) } - end - - context "when not activated" do - it { is_expected.to_not be_able_to(:manage, Spree::StockTransfer) } - it { is_expected.to_not be_able_to(:manage, Spree::TransferItem) } - it { is_expected.not_to be_able_to(:display, Spree::StockLocation) } - end -end diff --git a/core/spec/models/spree/stock_transfer_spec.rb b/core/spec/models/spree/stock_transfer_spec.rb deleted file mode 100644 index 69f9366ef75..00000000000 --- a/core/spec/models/spree/stock_transfer_spec.rb +++ /dev/null @@ -1,308 +0,0 @@ -require 'rails_helper' - -module Spree - RSpec.describe StockTransfer, type: :model do - let(:destination_location) { create(:stock_location_with_items) } - let(:source_location) { create(:stock_location_with_items) } - let(:stock_item) { source_location.stock_items.order(:id).first } - let(:variant) { stock_item.variant } - let(:stock_transfer) do - StockTransfer.create(description: 'PO123', source_location: source_location, destination_location: destination_location) - end - - subject { stock_transfer } - - describe '#description' do - subject { super().description } - it { is_expected.to eq 'PO123' } - end - - describe '#to_param' do - subject { super().to_param } - it { is_expected.to match /T\d+/ } - end - - describe "transfer item building" do - let(:stock_transfer) do - variant = source_location.stock_items.first.variant - stock_transfer = Spree::StockTransfer.new( - number: "T123", - source_location: source_location, - destination_location: destination_location - ) - stock_transfer.transfer_items.build(variant: variant, expected_quantity: 5) - stock_transfer - end - - subject { stock_transfer.save } - - it { is_expected.to eq true } - - it "creates the associated transfer item" do - expect { subject }.to change { Spree::TransferItem.count }.by(1) - end - end - - describe "#receivable?" do - subject { stock_transfer.receivable? } - - context "finalized" do - before do - stock_transfer.update_attributes(finalized_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "shipped" do - before do - stock_transfer.update_attributes(shipped_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "closed" do - before do - stock_transfer.update_attributes(closed_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "finalized and closed" do - before do - stock_transfer.update_attributes(finalized_at: Time.current, closed_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "shipped and closed" do - before do - stock_transfer.update_attributes(shipped_at: Time.current, closed_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "finalized and shipped" do - before do - stock_transfer.update_attributes(finalized_at: Time.current, shipped_at: Time.current) - end - - it { is_expected.to eq true } - end - end - - describe "#finalizable?" do - subject { stock_transfer.finalizable? } - - context "finalized" do - before do - stock_transfer.update_attributes(finalized_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "shipped" do - before do - stock_transfer.update_attributes(shipped_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "closed" do - before do - stock_transfer.update_attributes(closed_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "finalized and closed" do - before do - stock_transfer.update_attributes(finalized_at: Time.current, closed_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "shipped and closed" do - before do - stock_transfer.update_attributes(shipped_at: Time.current, closed_at: Time.current) - end - - it { is_expected.to eq false } - end - - context "no action taken on stock transfer" do - before do - stock_transfer.update_attributes(finalized_at: nil, shipped_at: nil, closed_at: nil) - end - - it { is_expected.to eq true } - end - end - - describe "#finalize" do - let(:user) { create(:user) } - - subject { stock_transfer.finalize(user) } - - context "can be finalized" do - it "sets a finalized_at date" do - expect { subject }.to change { stock_transfer.finalized_at } - end - - it "sets the finalized_by to the supplied user" do - subject - expect(stock_transfer.finalized_by).to eq user - end - end - - context "can't be finalized" do - before do - stock_transfer.update_attributes(finalized_at: Time.current) - end - - it "doesn't set a finalized_at date" do - expect { subject }.to_not change { stock_transfer.finalized_at } - end - - it "doesn't set a finalized_by user" do - expect { subject }.to_not change { stock_transfer.finalized_by } - end - - it "adds an error message" do - subject - expect(stock_transfer.errors.full_messages).to include I18n.t('spree.stock_transfer_cannot_be_finalized') - end - end - end - - describe "#close" do - let(:user) { create(:user) } - let(:stock_transfer) { create(:receivable_stock_transfer_with_items) } - - subject { stock_transfer.close(user) } - - context "can be closed" do - it "sets a closed_at date" do - expect { subject }.to change { stock_transfer.closed_at } - end - - it "sets the closed_by to the supplied user" do - subject - expect(stock_transfer.closed_by).to eq user - end - end - - context "can't be closed" do - before do - stock_transfer.update_attributes(finalized_at: nil) - end - - it "doesn't set a closed_at date" do - expect { subject }.to_not change { stock_transfer.closed_at } - end - - it "doesn't set a closed_by user" do - expect { subject }.to_not change { stock_transfer.closed_by } - end - - it "adds an error message" do - subject - expect(stock_transfer.errors.full_messages).to include I18n.t('spree.stock_transfer_must_be_receivable') - end - end - end - - describe "destroying" do - subject { stock_transfer.paranoia_destroy } - - context "stock transfer is finalized" do - before do - stock_transfer.update_attributes!(finalized_at: Time.current) - end - - it "doesn't destroy the stock transfer" do - expect { subject }.to_not change { Spree::StockTransfer.count } - end - - it "adds an error message to the model" do - subject - expect(stock_transfer.errors.full_messages).to include I18n.t('spree.errors.messages.cannot_delete_finalized_stock_transfer') - end - end - - context "stock transfer is not finalized" do - before do - stock_transfer.update_attributes!(finalized_at: nil) - end - - it "destroys the stock transfer" do - expect { subject }.to change { Spree::StockTransfer.count }.by(-1) - end - end - end - - describe '#ship' do - let(:stock_transfer) { create(:stock_transfer, tracking_number: "ABC123") } - - context "tracking number is provided" do - subject { stock_transfer.ship(tracking_number: "XYZ123") } - - it "updates the tracking number" do - expect { subject }.to change { stock_transfer.tracking_number }.from("ABC123").to("XYZ123") - end - end - - context "tracking number is not provided" do - subject { stock_transfer.ship } - - it "preserves the existing tracking number" do - expect { subject }.to_not change { stock_transfer.tracking_number }.from("ABC123") - end - end - end - - describe '#transfer' do - let(:stock_transfer) { create(:stock_transfer_with_items) } - - before do - stock_transfer.transfer_items.each { |item| item.update_attributes(expected_quantity: 1) } - end - - subject { stock_transfer.transfer } - - context 'with enough stock' do - it 'creates stock movements for transfer items' do - expect{ subject }.to change{ Spree::StockMovement.count }.by(stock_transfer.transfer_items.count) - end - end - - context 'without enough stock' do - before do - stockless_variant = stock_transfer.transfer_items.last.variant - stock_transfer.source_location.stock_item(stockless_variant).set_count_on_hand(0) - end - - it 'rollsback the transaction' do - expect{ subject }.to_not change{ Spree::StockMovement.count } - end - - it 'adds errors' do - subject - expect(stock_transfer.errors.full_messages.join(', ')).to match /not enough inventory/ - end - - it 'returns false' do - expect(subject).to eq false - end - end - end - end -end diff --git a/core/spec/models/spree/transfer_item_spec.rb b/core/spec/models/spree/transfer_item_spec.rb deleted file mode 100644 index 9cf1525e83b..00000000000 --- a/core/spec/models/spree/transfer_item_spec.rb +++ /dev/null @@ -1,282 +0,0 @@ -require 'rails_helper' - -RSpec.describe Spree::TransferItem do - let(:stock_location) { create(:stock_location, name: "Warehouse") } - let(:stock_transfer) { create(:stock_transfer_with_items, source_location: stock_location) } - let(:transfer_item) { stock_transfer.transfer_items.first } - - subject { transfer_item } - - describe "validation" do - before do - transfer_item.assign_attributes(expected_quantity: expected_quantity, received_quantity: received_quantity) - end - - describe "expected vs received quantity" do - context "expected quantity is the same as the received quantity" do - let(:expected_quantity) { 1 } - let(:received_quantity) { 1 } - it { is_expected.to be_valid } - end - - context "expected quantity is larger than the received quantity" do - let(:expected_quantity) { 3 } - let(:received_quantity) { 1 } - it { is_expected.to be_valid } - end - - context "expected quantity is lower than the received quantity" do - let(:expected_quantity) { 1 } - let(:received_quantity) { 3 } - it { is_expected.to_not be_valid } - end - end - - describe "numericality" do - context "expected_quantity is less than 0" do - let(:expected_quantity) { -1 } - let(:received_quantity) { 3 } - it { is_expected.to_not be_valid } - end - - context "received_quantity is less than 0" do - let(:expected_quantity) { 1 } - let(:received_quantity) { -3 } - it { is_expected.to_not be_valid } - end - end - - describe "availability" do - let(:stock_item) do - transfer_item.variant.stock_items.find_by(stock_location: stock_transfer.source_location) - end - let(:expected_quantity) { 1 } - let(:received_quantity) { 1 } - - subject { transfer_item.valid? } - - shared_examples_for 'availability check fails' do - it "validates the availability" do - subject - expect(transfer_item.errors.full_messages).to include I18n.t('spree.errors.messages.transfer_item_insufficient_stock') - end - end - - shared_examples_for 'availability check passes' do - it "doesn't validate the availability" do - subject - expect(transfer_item.errors.full_messages).to_not include I18n.t('spree.errors.messages.transfer_item_insufficient_stock') - end - end - - context "transfer order is shipped" do - before do - stock_transfer.update_attributes!(shipped_at: Time.current) - end - - context "variant is not available" do - before do - stock_item.set_count_on_hand(0) - end - include_examples 'availability check passes' - - context "stock location doesn't check stock" do - before do - stock_location.update_attributes!(check_stock_on_transfer: false) - end - include_examples 'availability check passes' - end - end - - context "variant available" do - before do - stock_item.set_count_on_hand(transfer_item.expected_quantity) - end - include_examples 'availability check passes' - end - - context "variant does not exist in stock location" do - before do - stock_item.really_destroy! - end - include_examples 'availability check passes' - end - end - - context "transfer order isn't closed" do - before do - stock_transfer.update_attributes!(closed_at: nil) - end - - context "variant is not available" do - before do - stock_item.set_count_on_hand(0) - end - include_examples 'availability check fails' - - context "stock location doesn't check stock" do - before do - stock_location.update_attributes!(check_stock_on_transfer: false) - end - include_examples 'availability check passes' - end - end - - context "variant available" do - before do - stock_item.set_count_on_hand(transfer_item.expected_quantity) - end - include_examples 'availability check passes' - end - - context "variant does not exist in stock location" do - before do - stock_item.really_destroy! - end - include_examples 'availability check fails' - end - - context "variant stock_item soft-deleted in stock location" do - before do - stock_item.paranoia_destroy! - end - include_examples 'availability check fails' - end - end - end - end - - describe "received stock transfer guard" do - subject { transfer_item.reload.update_attributes(received_quantity: 2) } - - describe "closed stock transfer" do - context "stock_transfer is not closed" do - before do - stock_transfer.update_attributes!(closed_at: nil) - end - - it { is_expected.to eq true } - end - - context "stock_transfer is closed" do - before do - stock_transfer.update_attributes!(closed_at: Time.current) - end - - it { is_expected.to eq false } - - it "adds an error message" do - subject - expect(transfer_item.errors.full_messages).to include I18n.t('spree.errors.messages.cannot_modify_transfer_item_closed_stock_transfer') - end - end - end - end - - describe "expected quantity update guard" do - let(:attrs) { { expected_quantity: 1 } } - - subject { transfer_item.update_attributes(attrs) } - - context "stock transfer is finalized" do - before do - stock_transfer.update_attributes(finalized_at: Time.current) - end - - it "adds an error message" do - subject - expect(transfer_item.errors.full_messages).to include I18n.t('spree.errors.messages.cannot_update_expected_transfer_item_with_finalized_stock_transfer') - end - - context "updating received_quantity" do - let(:attrs) { { received_quantity: 1 } } - - it "updates the received quantity successfully" do - expect { subject }.to change { transfer_item.received_quantity }.to(1) - end - end - end - - context "stock transfer is not finalized" do - before do - stock_transfer.update_attributes(finalized_at: nil, shipped_at: nil) - end - - it "updates the expected quantity successfully" do - expect { subject }.to change { transfer_item.expected_quantity }.to(1) - end - end - end - - describe "destroy finalized stock transfer guard" do - subject { transfer_item.paranoia_destroy } - - context "stock transfer is finalized" do - before do - stock_transfer.update_attributes(finalized_at: Time.current) - end - - it "does not destroy the transfer item" do - expect { subject }.to_not change { Spree::TransferItem.count } - end - - it "adds an error message" do - subject - expect(transfer_item.errors.full_messages).to include I18n.t('spree.errors.messages.cannot_delete_transfer_item_with_finalized_stock_transfer') - end - end - - context "stock transfer is not finalized" do - before do - stock_transfer.update_attributes(finalized_at: nil, shipped_at: nil) - end - - it "destroys the transfer item" do - expect { subject }.to change { Spree::TransferItem.count }.by(-1) - end - end - - context "scopes" do - let(:partially_received) { stock_transfer.transfer_items.first } - let(:fully_received) { stock_transfer.transfer_items.last } - let(:variant) { create(:variant) } - - before do - fully_received.update_attributes(expected_quantity: 1, received_quantity: 1) - partially_received.update_attributes(expected_quantity: 2, received_quantity: 1) - - stock_transfer.source_location.stock_item(variant).set_count_on_hand(5) - stock_transfer.transfer_items.create!(variant: variant, expected_quantity: 1, received_quantity: 0) - end - - context '.received' do - it 'only returns items that have received quantity greater than 0' do - expect(Spree::TransferItem.received).to match_array [fully_received, partially_received] - end - end - - context '.fully_received' do - it 'returns only items that have not been fully received' do - expect(Spree::TransferItem.fully_received).to eq [fully_received] - end - end - - context '.partially_received' do - it 'returns only items where received quantity is less that expected' do - expect(Spree::TransferItem.partially_received).to eq [partially_received] - end - end - end - end - - describe "variant association" do - context "variant has been soft-deleted" do - before do - subject.variant.paranoia_destroy - end - it "still returns the variant" do - expect(subject.variant).not_to be_nil - end - end - end -end