diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index cc08ddc7e0..934e12057c 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -33,7 +33,6 @@ Layout/LineLength: - app/controllers/admin/product_import_controller.rb - app/controllers/admin/schedules_controller.rb - app/controllers/admin/subscriptions_controller.rb - - app/controllers/admin/variant_overrides_controller.rb - app/controllers/api/enterprise_attachment_controller.rb - app/controllers/api/product_images_controller.rb - app/controllers/spree/paypal_controller_decorator.rb @@ -71,7 +70,6 @@ Layout/LineLength: - app/models/spree/variant.rb - app/models/subscription.rb - app/models/variant_override.rb - - app/models/variant_override_set.rb - app/serializers/api/admin/subscription_line_item_serializer.rb - app/services/cart_service.rb - app/services/checkout/post_checkout_actions.rb @@ -254,7 +252,6 @@ Layout/LineLength: - spec/models/enterprise_relationship_spec.rb - spec/models/enterprise_spec.rb - spec/models/exchange_spec.rb - - spec/models/model_set_spec.rb - spec/models/order_cycle_spec.rb - spec/models/product_importer_spec.rb - spec/models/product_import/reset_absent_spec.rb @@ -385,7 +382,6 @@ Metrics/AbcSize: - app/models/enterprise_group.rb - app/models/enterprise.rb - app/models/enterprise_relationship.rb - - app/models/model_set.rb - app/models/product_import/entry_processor.rb - app/models/product_import/entry_validator.rb - app/models/product_import/product_importer.rb @@ -413,6 +409,7 @@ Metrics/AbcSize: - app/serializers/api/variant_serializer.rb - app/services/cart_service.rb - app/services/create_order_cycle.rb + - app/services/sets/model_set.rb - app/services/order_cycle_form.rb - app/services/order_syncer.rb - app/services/variant_units/option_value_namer.rb @@ -538,8 +535,8 @@ Metrics/CyclomaticComplexity: - app/models/spree/product.rb - app/models/spree/return_authorization.rb - app/models/spree/zone.rb - - app/models/variant_override_set.rb - app/services/cart_service.rb + - app/services/sets/variant_override_set.rb - engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb - engines/order_management/app/services/order_management/stock/estimator.rb - lib/active_merchant/billing/gateways/stripe_payment_intents.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2713d9b86d..95485b26e2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -398,7 +398,6 @@ Style/ClassAndModuleChildren: - 'app/models/calculator/flat_percent_per_item.rb' - 'app/models/spree/gateway/migs.rb' - 'app/models/spree/gateway/pin.rb' - - 'app/models/spree/product_set.rb' - 'app/models/tag_rule/discount_order.rb' - 'app/models/tag_rule/filter_order_cycles.rb' - 'app/models/tag_rule/filter_payment_methods.rb' @@ -575,7 +574,6 @@ Style/FrozenStringLiteralComment: - 'app/models/calculator/flat_percent_per_item.rb' - 'app/models/calculator/weight.rb' - 'app/models/column_preference.rb' - - 'app/models/column_preference_set.rb' - 'app/models/concerns/address_display.rb' - 'app/models/concerns/adjustment_scopes.rb' - 'app/models/concerns/line_item_based_adjustment_handling.rb' @@ -588,17 +586,13 @@ Style/FrozenStringLiteralComment: - 'app/models/customer.rb' - 'app/models/distributor_shipping_method.rb' - 'app/models/enterprise_fee.rb' - - 'app/models/enterprise_fee_set.rb' - 'app/models/enterprise_relationship_permission.rb' - 'app/models/enterprise_role.rb' - - 'app/models/enterprise_set.rb' - 'app/models/exchange.rb' - 'app/models/exchange_fee.rb' - 'app/models/exchange_variant.rb' - 'app/models/inventory_item.rb' - - 'app/models/model_set.rb' - 'app/models/order_cycle.rb' - - 'app/models/order_cycle_set.rb' - 'app/models/preference_sections/footer_and_external_links_section.rb' - 'app/models/preference_sections/group_signup_page_section.rb' - 'app/models/preference_sections/header_section.rb' @@ -623,7 +617,6 @@ Style/FrozenStringLiteralComment: - 'app/models/spree/gateway/pin.rb' - 'app/models/spree/gateway/stripe_connect.rb' - 'app/models/spree/preferences/file_configuration.rb' - - 'app/models/spree/product_set.rb' - 'app/models/spree/property.rb' - 'app/models/spree/user.rb' - 'app/models/stripe_account.rb' @@ -636,7 +629,6 @@ Style/FrozenStringLiteralComment: - 'app/models/tag_rule/filter_products.rb' - 'app/models/tag_rule/filter_shipping_methods.rb' - 'app/models/variant_override.rb' - - 'app/models/variant_override_set.rb' - 'app/serializers/api/address_serializer.rb' - 'app/serializers/api/adjustment_serializer.rb' - 'app/serializers/api/cached_enterprise_serializer.rb' diff --git a/app/controllers/admin/column_preferences_controller.rb b/app/controllers/admin/column_preferences_controller.rb index d8a5ca3e6a..90b4cb850c 100644 --- a/app/controllers/admin/column_preferences_controller.rb +++ b/app/controllers/admin/column_preferences_controller.rb @@ -29,7 +29,8 @@ module Admin collection_hash = Hash[permitted_params[:column_preferences]. each_with_index.map { |cp, i| [i, cp] }] collection_hash.select!{ |_i, cp| cp[:action_name] == permitted_params[:action_name] } - @cp_set = ColumnPreferenceSet.new @column_preferences, collection_attributes: collection_hash + @cp_set = Sets::ColumnPreferenceSet.new(@column_preferences, + collection_attributes: collection_hash) end def collection diff --git a/app/controllers/admin/enterprise_fees_controller.rb b/app/controllers/admin/enterprise_fees_controller.rb index 0aaa2957be..73880badf4 100644 --- a/app/controllers/admin/enterprise_fees_controller.rb +++ b/app/controllers/admin/enterprise_fees_controller.rb @@ -27,7 +27,7 @@ module Admin end def bulk_update - @enterprise_fee_set = EnterpriseFeeSet.new(enterprise_fee_bulk_params) + @enterprise_fee_set = Sets::EnterpriseFeeSet.new(enterprise_fee_bulk_params) if @enterprise_fee_set.save redirect_to redirect_path, notice: I18n.t(:enterprise_fees_update_notice) @@ -40,7 +40,7 @@ module Admin private def load_enterprise_fee_set - @enterprise_fee_set = EnterpriseFeeSet.new collection: collection + @enterprise_fee_set = Sets::EnterpriseFeeSet.new collection: collection end def load_data @@ -80,7 +80,7 @@ module Admin end def enterprise_fee_bulk_params - params.require(:enterprise_fee_set).permit( + params.require(:sets_enterprise_fee_set).permit( collection_attributes: [ :id, :enterprise_id, :fee_type, :name, :tax_category_id, :inherits_tax_category, :calculator_type, diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index c61ab9f30e..99ae12761f 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -81,7 +81,7 @@ module Admin end def bulk_update - @enterprise_set = EnterpriseSet.new(collection, bulk_params) + @enterprise_set = Sets::EnterpriseSet.new(collection, bulk_params) if @enterprise_set.save flash[:success] = I18n.t(:enterprise_bulk_update_success_notice) @@ -129,7 +129,7 @@ module Admin private def load_enterprise_set - @enterprise_set = EnterpriseSet.new(collection) if spree_current_user.admin? + @enterprise_set = Sets::EnterpriseSet.new(collection) if spree_current_user.admin? end def load_countries @@ -235,7 +235,7 @@ module Admin def check_can_change_bulk_sells unless spree_current_user.admin? - params[:enterprise_set][:collection_attributes].each do |_i, enterprise_params| + params[:sets_enterprise_set][:collection_attributes].each do |_i, enterprise_params| unless spree_current_user == Enterprise.find_by(id: enterprise_params[:id]).owner enterprise_params.delete :sells end @@ -269,7 +269,7 @@ module Admin def check_can_change_bulk_owner unless spree_current_user.admin? - params[:enterprise_set][:collection_attributes].each do |_i, enterprise_params| + params[:sets_enterprise_set][:collection_attributes].each do |_i, enterprise_params| enterprise_params.delete :owner_id end end @@ -321,7 +321,7 @@ module Admin end def bulk_params - params.require(:enterprise_set).permit( + params.require(:sets_enterprise_set).permit( collection_attributes: PermittedAttributes::Enterprise.attributes ) end diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index f7de30662a..3482be399f 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -223,7 +223,7 @@ module Admin end def order_cycle_set - @order_cycle_set ||= OrderCycleSet.new(@order_cycles, order_cycle_bulk_params) + @order_cycle_set ||= Sets::OrderCycleSet.new(@order_cycles, order_cycle_bulk_params) end def require_order_cycle_set_params diff --git a/app/controllers/admin/variant_overrides_controller.rb b/app/controllers/admin/variant_overrides_controller.rb index 333455c57b..3c6778640c 100644 --- a/app/controllers/admin/variant_overrides_controller.rb +++ b/app/controllers/admin/variant_overrides_controller.rb @@ -60,14 +60,17 @@ module Admin for_hubs(editable_enterprises.collect(&:id)) options = [{ id: '0', name: 'All' }] - import_dates.collect(&:import_date).map { |i| options.push(id: i.to_date, name: i.to_date.to_formatted_s(:long)) } + import_dates.collect(&:import_date).map { |i| + options.push(id: i.to_date, name: i.to_date.to_formatted_s(:long)) + } options end def load_collection collection_hash = Hash[variant_overrides_params.each_with_index.map { |vo, i| [i, vo] }] - @vo_set = VariantOverrideSet.new @variant_overrides, collection_attributes: collection_hash + @vo_set = Sets::VariantOverrideSet.new(@variant_overrides, + collection_attributes: collection_hash) end def collection @@ -82,8 +85,8 @@ module Admin [:index, :bulk_update, :bulk_reset] end - # This has been pulled from ModelSet as it is useful for compiling a list of errors on any generic collection (not necessarily a ModelSet) - # Could be pulled down into a lower level controller if it is useful in other high level controllers + # This method is also present in ModelSet + # This is useful for compiling a list of errors on any generic collection def collection_errors errors = ActiveModel::Errors.new self full_messages = @collection.map { |element| element.errors.full_messages }.flatten diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 3f9cdd92ac..1cc3f5ae6d 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -163,7 +163,7 @@ module Spree def product_set_from_params(_params) collection_hash = Hash[products_params.each_with_index.map { |p, i| [i, p] }] - Spree::ProductSet.new(collection_attributes: collection_hash) + Sets::ProductSet.new(collection_attributes: collection_hash) end def products_params diff --git a/app/helpers/enterprise_fees_helper.rb b/app/helpers/enterprise_fees_helper.rb index cc22f07326..55647b6413 100644 --- a/app/helpers/enterprise_fees_helper.rb +++ b/app/helpers/enterprise_fees_helper.rb @@ -1,10 +1,10 @@ module EnterpriseFeesHelper def angular_name(method) - "enterprise_fee_set[collection_attributes][{{ $index }}][#{method}]" + "sets_enterprise_fee_set[collection_attributes][{{ $index }}][#{method}]" end def angular_id(method) - "enterprise_fee_set_collection_attributes_{{ $index }}_#{method}" + "sets_enterprise_fee_set_collection_attributes_{{ $index }}_#{method}" end def enterprise_fee_type_options diff --git a/app/models/column_preference_set.rb b/app/models/column_preference_set.rb deleted file mode 100644 index 24508d7f9d..0000000000 --- a/app/models/column_preference_set.rb +++ /dev/null @@ -1,5 +0,0 @@ -class ColumnPreferenceSet < ModelSet - def initialize(collection, attributes = {}) - super(ColumnPreference, collection, attributes, nil, nil ) - end -end diff --git a/app/models/enterprise_fee_set.rb b/app/models/enterprise_fee_set.rb deleted file mode 100644 index ae7ff692ad..0000000000 --- a/app/models/enterprise_fee_set.rb +++ /dev/null @@ -1,7 +0,0 @@ -class EnterpriseFeeSet < ModelSet - def initialize(attributes = {}) - super(EnterpriseFee, EnterpriseFee.all, - attributes, - proc { |attrs| attrs[:name].blank? }) - end -end diff --git a/app/models/enterprise_set.rb b/app/models/enterprise_set.rb deleted file mode 100644 index 20da64b083..0000000000 --- a/app/models/enterprise_set.rb +++ /dev/null @@ -1,5 +0,0 @@ -class EnterpriseSet < ModelSet - def initialize(collection, attributes = {}) - super(Enterprise, collection, attributes) - end -end diff --git a/app/models/model_set.rb b/app/models/model_set.rb deleted file mode 100644 index 8a599b4214..0000000000 --- a/app/models/model_set.rb +++ /dev/null @@ -1,65 +0,0 @@ -# Tableless model to handle updating multiple models at once from a single form -class ModelSet - include ActiveModel::Conversion - extend ActiveModel::Naming - - attr_accessor :collection - - def initialize(klass, collection, attributes = {}, reject_if = nil, delete_if = nil) - @klass, @collection, @reject_if, @delete_if = klass, collection, reject_if, delete_if - - # Set here first, to ensure that we apply collection_attributes to the right collection - @collection = attributes[:collection] if attributes[:collection] - - attributes.each do |name, value| - public_send("#{name}=", value) - end - end - - def collection_attributes=(collection_attributes) - collection_attributes.each do |_k, attributes| - # attributes == {:id => 123, :next_collection_at => '...'} - found_element = @collection.detect do |element| - element.id.to_s == attributes[:id].to_s && !element.id.nil? - end - - if found_element.nil? - @collection << @klass.new(attributes) unless @reject_if.andand.call(attributes) - else - found_element.assign_attributes(attributes.except(:id)) - end - end - end - - def errors - errors = ActiveModel::Errors.new self - full_messages = @collection - .map { |model| model.errors.full_messages } - .flatten - - full_messages.each { |message| errors.add(:base, message) } - errors - end - - def save - collection_to_delete.each(&:destroy) - collection_to_keep.all?(&:save) - end - - def collection_to_delete - # Remove all elements to be deleted from collection and return them - # Allows us to render @model_set.collection without deleted elements - deleted = [] - @collection = collection.to_a - collection.delete_if { |e| deleted << e if @delete_if.andand.call(e.attributes) } - deleted - end - - def collection_to_keep - collection.reject { |e| @delete_if.andand.call(e.attributes) } - end - - def persisted? - false - end -end diff --git a/app/models/order_cycle_set.rb b/app/models/order_cycle_set.rb deleted file mode 100644 index 9f4fb4433e..0000000000 --- a/app/models/order_cycle_set.rb +++ /dev/null @@ -1,5 +0,0 @@ -class OrderCycleSet < ModelSet - def initialize(collection, attributes = {}) - super(OrderCycle, collection, attributes) - end -end diff --git a/app/models/spree/product_set.rb b/app/models/spree/product_set.rb deleted file mode 100644 index 4731c44bf7..0000000000 --- a/app/models/spree/product_set.rb +++ /dev/null @@ -1,138 +0,0 @@ -class Spree::ProductSet < ModelSet - def initialize(attributes = {}) - super(Spree::Product, [], attributes) - end - - def save - @collection_hash.each_value.all? do |product_attributes| - update_product_attributes(product_attributes) - end - end - - def collection_attributes=(attributes) - @collection = Spree::Product - .where(id: attributes.each_value.map { |product| product[:id] }) - @collection_hash = attributes - end - - private - - # A separate method of updating products was required due to an issue with - # the way Rails' assign_attributes and update behave when - # delegated attributes of a nested object are updated via the parent object - # (ie. price of variants). Updating such attributes by themselves did not - # work using: - # - # product.update(variants_attributes: [{ id: y, price: xx.x }]) - # - # and so an explicit call to update on each individual variant was - # required. ie: - # - # variant.update( { price: xx.x } ) - # - def update_product_attributes(attributes) - split_taxon_ids!(attributes) - - product = find_model(@collection, attributes[:id]) - return if product.nil? - - update_product(product, attributes) - end - - def split_taxon_ids!(attributes) - attributes[:taxon_ids] = attributes[:taxon_ids].split(',') if attributes[:taxon_ids].present? - end - - def update_product(product, attributes) - original_supplier = product.supplier_id - return false unless update_product_only_attributes(product, attributes) - - ExchangeVariantDeleter.new.delete(product) if original_supplier != product.supplier_id - - update_product_variants(product, attributes) && - update_product_master(product, attributes) - end - - def update_product_only_attributes(product, attributes) - variant_related_attrs = [:id, :variants_attributes, :master_attributes] - product_related_attrs = attributes.except(*variant_related_attrs) - return true if product_related_attrs.blank? - - product.assign_attributes(product_related_attrs) - - validate_presence_of_unit_value_in_product(product) - - product.errors.empty? && product.save - end - - def validate_presence_of_unit_value_in_product(product) - product.variants.each do |variant| - validate_presence_of_unit_value_in_variant(product, variant) - end - end - - def validate_presence_of_unit_value_in_variant(product, variant) - return unless %w(weight volume).include?(product.variant_unit) - return if variant.unit_value.present? - - product.errors.add(:unit_value, "can't be blank") - end - - def update_product_variants(product, attributes) - return true unless attributes[:variants_attributes] - - update_variants_attributes(product, attributes[:variants_attributes]) - end - - def update_product_master(product, attributes) - return true unless attributes[:master_attributes] - - create_or_update_variant(product, attributes[:master_attributes]) - end - - def update_variants_attributes(product, variants_attributes) - variants_attributes.each do |attributes| - create_or_update_variant(product, attributes) - end - end - - def create_or_update_variant(product, variant_attributes) - variant = find_model(product.variants_including_master, variant_attributes[:id]) - if variant.present? - variant.update(variant_attributes.except(:id)) - else - create_variant(product, variant_attributes) - end - end - - def create_variant(product, variant_attributes) - on_hand = variant_attributes.delete(:on_hand) - on_demand = variant_attributes.delete(:on_demand) - - variant = product.variants.create(variant_attributes) - - begin - variant.on_demand = on_demand if on_demand.present? - variant.on_hand = on_hand.to_i if on_hand.present? - rescue StandardError => e - notify_bugsnag(e, product, variant, variant_attributes) - raise e - end - end - - def notify_bugsnag(error, product, variant, variant_attributes) - Bugsnag.notify(error) do |report| - report.add_tab(:product, product.attributes) - report.add_tab(:product_error, product.errors.first) unless product.valid? - report.add_tab(:variant_attributes, variant_attributes) - report.add_tab(:variant, variant.attributes) - report.add_tab(:variant_error, variant.errors.first) unless variant.valid? - end - end - - def find_model(collection, model_id) - collection.find do |model| - model.id.to_s == model_id.to_s && model.persisted? - end - end -end diff --git a/app/models/variant_override_set.rb b/app/models/variant_override_set.rb deleted file mode 100644 index d380df4144..0000000000 --- a/app/models/variant_override_set.rb +++ /dev/null @@ -1,30 +0,0 @@ -class VariantOverrideSet < ModelSet - def initialize(collection, attributes = {}) - super(VariantOverride, collection, attributes, nil, proc { |attrs, tag_list| deletable?(attrs, tag_list) } ) - end - - private - - def deletable?(attrs, tag_list) - attrs['price'].blank? && - attrs['count_on_hand'].blank? && - attrs['default_stock'].blank? && - attrs['resettable'].blank? && - attrs['sku'].nil? && - attrs['on_demand'].nil? && - tag_list.empty? - end - - # Override of ModelSet method to allow us to check presence of a tag_list (which is not an attribute) - # This method will delete VariantOverrides that have no values (see deletable? above) - # If the user sets all values to nil in the UI the VO will be deleted from the DB - def collection_to_delete - deleted = [] - collection.delete_if { |e| deleted << e if @delete_if.andand.call(e.attributes, e.tag_list) } - deleted - end - - def collection_to_keep - collection.reject { |e| @delete_if.andand.call(e.attributes, e.tag_list) } - end -end diff --git a/app/services/sets/column_preference_set.rb b/app/services/sets/column_preference_set.rb new file mode 100644 index 0000000000..13cba40957 --- /dev/null +++ b/app/services/sets/column_preference_set.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Sets + class ColumnPreferenceSet < ModelSet + def initialize(collection, attributes = {}) + super(ColumnPreference, collection, attributes, nil, nil ) + end + end +end diff --git a/app/services/sets/enterprise_fee_set.rb b/app/services/sets/enterprise_fee_set.rb new file mode 100644 index 0000000000..d651649232 --- /dev/null +++ b/app/services/sets/enterprise_fee_set.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Sets + class EnterpriseFeeSet < ModelSet + def initialize(attributes = {}) + super(EnterpriseFee, EnterpriseFee.all, + attributes, + proc { |attrs| attrs[:name].blank? }) + end + end +end diff --git a/app/services/sets/enterprise_set.rb b/app/services/sets/enterprise_set.rb new file mode 100644 index 0000000000..bb0b2bdc3c --- /dev/null +++ b/app/services/sets/enterprise_set.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Sets + class EnterpriseSet < ModelSet + def initialize(collection, attributes = {}) + super(Enterprise, collection, attributes) + end + end +end diff --git a/app/services/sets/model_set.rb b/app/services/sets/model_set.rb new file mode 100644 index 0000000000..5ecd2cddab --- /dev/null +++ b/app/services/sets/model_set.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +# Tableless model to handle updating multiple models at once from a single form +module Sets + class ModelSet + include ActiveModel::Conversion + extend ActiveModel::Naming + + attr_accessor :collection + + def initialize(klass, collection, attributes = {}, reject_if = nil, delete_if = nil) + @klass, @collection, @reject_if, @delete_if = klass, collection, reject_if, delete_if + + # Set here first, to ensure that we apply collection_attributes to the right collection + @collection = attributes[:collection] if attributes[:collection] + + attributes.each do |name, value| + public_send("#{name}=", value) + end + end + + def collection_attributes=(collection_attributes) + collection_attributes.each do |_k, attributes| + # attributes == {:id => 123, :next_collection_at => '...'} + found_element = @collection.detect do |element| + element.id.to_s == attributes[:id].to_s && !element.id.nil? + end + + if found_element.nil? + @collection << @klass.new(attributes) unless @reject_if.andand.call(attributes) + else + found_element.assign_attributes(attributes.except(:id)) + end + end + end + + def errors + errors = ActiveModel::Errors.new self + full_messages = @collection + .map { |model| model.errors.full_messages } + .flatten + + full_messages.each { |message| errors.add(:base, message) } + errors + end + + def save + collection_to_delete.each(&:destroy) + collection_to_keep.all?(&:save) + end + + def collection_to_delete + # Remove all elements to be deleted from collection and return them + # Allows us to render @model_set.collection without deleted elements + deleted = [] + @collection = collection.to_a + collection.delete_if { |e| deleted << e if @delete_if.andand.call(e.attributes) } + deleted + end + + def collection_to_keep + collection.reject { |e| @delete_if.andand.call(e.attributes) } + end + + def persisted? + false + end + end +end diff --git a/app/services/sets/order_cycle_set.rb b/app/services/sets/order_cycle_set.rb new file mode 100644 index 0000000000..663f684062 --- /dev/null +++ b/app/services/sets/order_cycle_set.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Sets + class OrderCycleSet < ModelSet + def initialize(collection, attributes = {}) + super(OrderCycle, collection, attributes) + end + end +end diff --git a/app/services/sets/product_set.rb b/app/services/sets/product_set.rb new file mode 100644 index 0000000000..4563bce0bb --- /dev/null +++ b/app/services/sets/product_set.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +module Sets + class ProductSet < ModelSet + def initialize(attributes = {}) + super(Spree::Product, [], attributes) + end + + def save + @collection_hash.each_value.all? do |product_attributes| + update_product_attributes(product_attributes) + end + end + + def collection_attributes=(attributes) + @collection = Spree::Product + .where(id: attributes.each_value.map { |product| product[:id] }) + @collection_hash = attributes + end + + private + + # A separate method of updating products was required due to an issue with + # the way Rails' assign_attributes and update behave when + # delegated attributes of a nested object are updated via the parent object + # (ie. price of variants). Updating such attributes by themselves did not + # work using: + # + # product.update(variants_attributes: [{ id: y, price: xx.x }]) + # + # and so an explicit call to update on each individual variant was + # required. ie: + # + # variant.update( { price: xx.x } ) + # + def update_product_attributes(attributes) + split_taxon_ids!(attributes) + + product = find_model(@collection, attributes[:id]) + return if product.nil? + + update_product(product, attributes) + end + + def split_taxon_ids!(attributes) + attributes[:taxon_ids] = attributes[:taxon_ids].split(',') if attributes[:taxon_ids].present? + end + + def update_product(product, attributes) + original_supplier = product.supplier_id + return false unless update_product_only_attributes(product, attributes) + + ExchangeVariantDeleter.new.delete(product) if original_supplier != product.supplier_id + + update_product_variants(product, attributes) && + update_product_master(product, attributes) + end + + def update_product_only_attributes(product, attributes) + variant_related_attrs = [:id, :variants_attributes, :master_attributes] + product_related_attrs = attributes.except(*variant_related_attrs) + return true if product_related_attrs.blank? + + product.assign_attributes(product_related_attrs) + + validate_presence_of_unit_value_in_product(product) + + product.errors.empty? && product.save + end + + def validate_presence_of_unit_value_in_product(product) + product.variants.each do |variant| + validate_presence_of_unit_value_in_variant(product, variant) + end + end + + def validate_presence_of_unit_value_in_variant(product, variant) + return unless %w(weight volume).include?(product.variant_unit) + return if variant.unit_value.present? + + product.errors.add(:unit_value, "can't be blank") + end + + def update_product_variants(product, attributes) + return true unless attributes[:variants_attributes] + + update_variants_attributes(product, attributes[:variants_attributes]) + end + + def update_product_master(product, attributes) + return true unless attributes[:master_attributes] + + create_or_update_variant(product, attributes[:master_attributes]) + end + + def update_variants_attributes(product, variants_attributes) + variants_attributes.each do |attributes| + create_or_update_variant(product, attributes) + end + end + + def create_or_update_variant(product, variant_attributes) + variant = find_model(product.variants_including_master, variant_attributes[:id]) + if variant.present? + variant.update(variant_attributes.except(:id)) + else + create_variant(product, variant_attributes) + end + end + + def create_variant(product, variant_attributes) + on_hand = variant_attributes.delete(:on_hand) + on_demand = variant_attributes.delete(:on_demand) + + variant = product.variants.create(variant_attributes) + + begin + variant.on_demand = on_demand if on_demand.present? + variant.on_hand = on_hand.to_i if on_hand.present? + rescue StandardError => e + notify_bugsnag(e, product, variant, variant_attributes) + raise e + end + end + + def notify_bugsnag(error, product, variant, variant_attributes) + Bugsnag.notify(error) do |report| + report.add_tab(:product, product.attributes) + report.add_tab(:product_error, product.errors.first) unless product.valid? + report.add_tab(:variant_attributes, variant_attributes) + report.add_tab(:variant, variant.attributes) + report.add_tab(:variant_error, variant.errors.first) unless variant.valid? + end + end + + def find_model(collection, model_id) + collection.find do |model| + model.id.to_s == model_id.to_s && model.persisted? + end + end + end +end diff --git a/app/services/sets/variant_override_set.rb b/app/services/sets/variant_override_set.rb new file mode 100644 index 0000000000..3240846e54 --- /dev/null +++ b/app/services/sets/variant_override_set.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Sets + class VariantOverrideSet < ModelSet + def initialize(collection, attributes = {}) + super(VariantOverride, + collection, + attributes, + nil, + proc { |attrs, tag_list| deletable?(attrs, tag_list) } ) + end + + private + + def deletable?(attrs, tag_list) + attrs['price'].blank? && + attrs['count_on_hand'].blank? && + attrs['default_stock'].blank? && + attrs['resettable'].blank? && + attrs['sku'].nil? && + attrs['on_demand'].nil? && + tag_list.empty? + end + + # Overrides ModelSet method to check presence of a tag_list (which is not an attribute) + # This method will delete VariantOverrides that have no values (see deletable? above) + # If the user sets all values to nil in the UI the VO will be deleted from the DB + def collection_to_delete + deleted = [] + collection.delete_if { |e| deleted << e if @delete_if.andand.call(e.attributes, e.tag_list) } + deleted + end + + def collection_to_keep + collection.reject { |e| @delete_if.andand.call(e.attributes, e.tag_list) } + end + end +end diff --git a/app/views/admin/enterprise_fees/_calculator_settings.html.haml b/app/views/admin/enterprise_fees/_calculator_settings.html.haml index f505f0c38a..be05330d6c 100644 --- a/app/views/admin/enterprise_fees/_calculator_settings.html.haml +++ b/app/views/admin/enterprise_fees/_calculator_settings.html.haml @@ -1,7 +1,7 @@ -# Render only the calculator settings and not the surrounding form -- enterprise_fee_set = ModelSet.new(EnterpriseFee, EnterpriseFee.where(:id => enterprise_fee.id)) +- enterprise_fee_set = Sets::ModelSet.new(EnterpriseFee, EnterpriseFee.where(:id => enterprise_fee.id)) - calculator_form_string = nil -- form_for enterprise_fee_set, :as => :enterprise_fee_set, :url => '' do |form| +- form_for enterprise_fee_set, :as => :sets_enterprise_fee_set, :url => '' do |form| - form.fields_for :collection do |f| - calculator_form_string = capture do - if !enterprise_fee.new_record? diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb index 3b4cd1428c..a19cada4d4 100644 --- a/spec/controllers/admin/enterprises_controller_spec.rb +++ b/spec/controllers/admin/enterprises_controller_spec.rb @@ -385,7 +385,7 @@ describe Admin::EnterprisesController, type: :controller do profile_enterprise1.enterprise_roles.build(user: new_owner).save profile_enterprise2.enterprise_roles.build(user: new_owner).save allow(controller).to receive_messages spree_current_user: new_owner - bulk_enterprise_params = { enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, sells: 'any', owner_id: new_owner.id }, '1' => { id: profile_enterprise2.id, sells: 'any', owner_id: new_owner.id } } } } + bulk_enterprise_params = { sets_enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, sells: 'any', owner_id: new_owner.id }, '1' => { id: profile_enterprise2.id, sells: 'any', owner_id: new_owner.id } } } } spree_put :bulk_update, bulk_enterprise_params profile_enterprise1.reload @@ -397,10 +397,10 @@ describe Admin::EnterprisesController, type: :controller do end it "cuts down the list of enterprises displayed when error received on bulk update" do - allow_any_instance_of(EnterpriseSet).to receive(:save) { false } + allow_any_instance_of(Sets::EnterpriseSet).to receive(:save) { false } profile_enterprise1.enterprise_roles.build(user: new_owner).save allow(controller).to receive_messages spree_current_user: new_owner - bulk_enterprise_params = { enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, visible: 'false' } } } } + bulk_enterprise_params = { sets_enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, visible: 'false' } } } } spree_put :bulk_update, bulk_enterprise_params expect(assigns(:enterprise_set).collection).to eq [profile_enterprise1] end @@ -409,7 +409,7 @@ describe Admin::EnterprisesController, type: :controller do context "as the owner of an enterprise" do it "allows 'sells' and 'owner' to be changed" do allow(controller).to receive_messages spree_current_user: original_owner - bulk_enterprise_params = { enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, sells: 'any', owner_id: new_owner.id }, '1' => { id: profile_enterprise2.id, sells: 'any', owner_id: new_owner.id } } } } + bulk_enterprise_params = { sets_enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, sells: 'any', owner_id: new_owner.id }, '1' => { id: profile_enterprise2.id, sells: 'any', owner_id: new_owner.id } } } } spree_put :bulk_update, bulk_enterprise_params profile_enterprise1.reload @@ -426,7 +426,7 @@ describe Admin::EnterprisesController, type: :controller do profile_enterprise1.enterprise_roles.build(user: new_owner).save profile_enterprise2.enterprise_roles.build(user: new_owner).save allow(controller).to receive_messages spree_current_user: admin_user - bulk_enterprise_params = { enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, sells: 'any', owner_id: new_owner.id }, '1' => { id: profile_enterprise2.id, sells: 'any', owner_id: new_owner.id } } } } + bulk_enterprise_params = { sets_enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, sells: 'any', owner_id: new_owner.id }, '1' => { id: profile_enterprise2.id, sells: 'any', owner_id: new_owner.id } } } } spree_put :bulk_update, bulk_enterprise_params profile_enterprise1.reload diff --git a/spec/features/admin/enterprise_fees_spec.rb b/spec/features/admin/enterprise_fees_spec.rb index 862a338f15..057840525b 100644 --- a/spec/features/admin/enterprise_fees_spec.rb +++ b/spec/features/admin/enterprise_fees_spec.rb @@ -18,11 +18,11 @@ feature ' login_as_admin_and_visit spree.edit_admin_general_settings_path click_link 'Enterprise Fees' - expect(page).to have_select "enterprise_fee_set_collection_attributes_0_enterprise_id" - expect(page).to have_select "enterprise_fee_set_collection_attributes_0_fee_type", selected: 'Packing fee' + expect(page).to have_select "sets_enterprise_fee_set_collection_attributes_0_enterprise_id" + expect(page).to have_select "sets_enterprise_fee_set_collection_attributes_0_fee_type", selected: 'Packing fee' expect(page).to have_selector "input[value='$0.50 / kg']" - expect(page).to have_select "enterprise_fee_set_collection_attributes_0_tax_category_id", selected: 'GST' - expect(page).to have_select "enterprise_fee_set_collection_attributes_0_calculator_type", selected: 'Flat Rate (per item)' + expect(page).to have_select "sets_enterprise_fee_set_collection_attributes_0_tax_category_id", selected: 'GST' + expect(page).to have_select "sets_enterprise_fee_set_collection_attributes_0_calculator_type", selected: 'Flat Rate (per item)' expect(page).to have_selector "input[value='#{amount}']" end @@ -34,11 +34,11 @@ feature ' login_as_admin_and_visit admin_enterprise_fees_path # And I fill in the fields for a new enterprise fee and click update - select 'Feedme', from: 'enterprise_fee_set_collection_attributes_0_enterprise_id' - select 'Admin', from: 'enterprise_fee_set_collection_attributes_0_fee_type' - fill_in 'enterprise_fee_set_collection_attributes_0_name', with: 'Hello!' - select 'GST', from: 'enterprise_fee_set_collection_attributes_0_tax_category_id' - select 'Flat Percent', from: 'enterprise_fee_set_collection_attributes_0_calculator_type' + select 'Feedme', from: 'sets_enterprise_fee_set_collection_attributes_0_enterprise_id' + select 'Admin', from: 'sets_enterprise_fee_set_collection_attributes_0_fee_type' + fill_in 'sets_enterprise_fee_set_collection_attributes_0_name', with: 'Hello!' + select 'GST', from: 'sets_enterprise_fee_set_collection_attributes_0_tax_category_id' + select 'Flat Percent', from: 'sets_enterprise_fee_set_collection_attributes_0_calculator_type' click_button 'Update' # Then I should see my fee and fields for the calculator @@ -46,11 +46,11 @@ feature ' expect(page).to have_selector "input[value='Hello!']" # When I fill in the calculator fields and click update - fill_in 'enterprise_fee_set_collection_attributes_0_calculator_attributes_preferred_flat_percent', with: '12.34' + fill_in 'sets_enterprise_fee_set_collection_attributes_0_calculator_attributes_preferred_flat_percent', with: '12.34' click_button 'Update' # Then I should see the correct values in my calculator fields - expect(page).to have_selector "#enterprise_fee_set_collection_attributes_0_calculator_attributes_preferred_flat_percent[value='12.34']" + expect(page).to have_selector "#sets_enterprise_fee_set_collection_attributes_0_calculator_attributes_preferred_flat_percent[value='12.34']" end scenario "editing an enterprise fee" do @@ -62,18 +62,18 @@ feature ' login_as_admin_and_visit admin_enterprise_fees_path # And I update the fields for the enterprise fee and click update - select 'Foo', from: 'enterprise_fee_set_collection_attributes_0_enterprise_id' - select 'Admin', from: 'enterprise_fee_set_collection_attributes_0_fee_type' - fill_in 'enterprise_fee_set_collection_attributes_0_name', with: 'Greetings!' - select 'Inherit From Product', from: 'enterprise_fee_set_collection_attributes_0_tax_category_id' - select 'Flat Percent', from: 'enterprise_fee_set_collection_attributes_0_calculator_type' + select 'Foo', from: 'sets_enterprise_fee_set_collection_attributes_0_enterprise_id' + select 'Admin', from: 'sets_enterprise_fee_set_collection_attributes_0_fee_type' + fill_in 'sets_enterprise_fee_set_collection_attributes_0_name', with: 'Greetings!' + select 'Inherit From Product', from: 'sets_enterprise_fee_set_collection_attributes_0_tax_category_id' + select 'Flat Percent', from: 'sets_enterprise_fee_set_collection_attributes_0_calculator_type' click_button 'Update' # Then I should see the updated fields for my fee - expect(page).to have_select "enterprise_fee_set_collection_attributes_0_enterprise_id", selected: 'Foo' - expect(page).to have_select "enterprise_fee_set_collection_attributes_0_fee_type", selected: 'Admin fee' + expect(page).to have_select "sets_enterprise_fee_set_collection_attributes_0_enterprise_id", selected: 'Foo' + expect(page).to have_select "sets_enterprise_fee_set_collection_attributes_0_fee_type", selected: 'Admin fee' expect(page).to have_selector "input[value='Greetings!']" - expect(page).to have_select 'enterprise_fee_set_collection_attributes_0_tax_category_id', selected: 'Inherit From Product' + expect(page).to have_select 'sets_enterprise_fee_set_collection_attributes_0_tax_category_id', selected: 'Inherit From Product' expect(page).to have_selector "option[selected]", text: 'Flat Percent (per item)' fee.reload @@ -123,17 +123,17 @@ feature ' within(".side_menu") { click_link 'Enterprise Fees' } click_link "Create One Now" - select distributor1.name, from: 'enterprise_fee_set_collection_attributes_0_enterprise_id' - select 'Packing', from: 'enterprise_fee_set_collection_attributes_0_fee_type' - fill_in 'enterprise_fee_set_collection_attributes_0_name', with: 'foo' - select 'GST', from: 'enterprise_fee_set_collection_attributes_0_tax_category_id' - select 'Flat Percent', from: 'enterprise_fee_set_collection_attributes_0_calculator_type' + select distributor1.name, from: 'sets_enterprise_fee_set_collection_attributes_0_enterprise_id' + select 'Packing', from: 'sets_enterprise_fee_set_collection_attributes_0_fee_type' + fill_in 'sets_enterprise_fee_set_collection_attributes_0_name', with: 'foo' + select 'GST', from: 'sets_enterprise_fee_set_collection_attributes_0_tax_category_id' + select 'Flat Percent', from: 'sets_enterprise_fee_set_collection_attributes_0_calculator_type' click_button 'Update' expect(flash_message).to eq('Your enterprise fees have been updated.') # After saving, we should be redirected to the fees for our chosen enterprise - expect(page).not_to have_select 'enterprise_fee_set_collection_attributes_1_enterprise_id', selected: 'Second Distributor' + expect(page).not_to have_select 'sets_enterprise_fee_set_collection_attributes_1_enterprise_id', selected: 'Second Distributor' enterprise_fee = EnterpriseFee.find_by name: 'foo' expect(enterprise_fee.enterprise).to eq(distributor1) @@ -146,14 +146,14 @@ feature ' visit edit_admin_enterprise_path(distributor1) within(".side_menu") { click_link 'Enterprise Fees' } click_link "Manage Enterprise Fees" - expect(page).to have_field 'enterprise_fee_set_collection_attributes_0_name', with: 'One' - expect(page).not_to have_field 'enterprise_fee_set_collection_attributes_1_name', with: 'Two' + expect(page).to have_field 'sets_enterprise_fee_set_collection_attributes_0_name', with: 'One' + expect(page).not_to have_field 'sets_enterprise_fee_set_collection_attributes_1_name', with: 'Two' visit edit_admin_enterprise_path(distributor2) within(".side_menu") { click_link 'Enterprise Fees' } click_link "Manage Enterprise Fees" - expect(page).not_to have_field 'enterprise_fee_set_collection_attributes_0_name', with: 'One' - expect(page).to have_field 'enterprise_fee_set_collection_attributes_0_name', with: 'Two' + expect(page).not_to have_field 'sets_enterprise_fee_set_collection_attributes_0_name', with: 'One' + expect(page).to have_field 'sets_enterprise_fee_set_collection_attributes_0_name', with: 'Two' end it "only allows me to select enterprises I have access to" do @@ -164,7 +164,7 @@ feature ' visit edit_admin_enterprise_path(distributor2) within(".side_menu") { click_link 'Enterprise Fees' } click_link "Manage Enterprise Fees" - expect(page).to have_select('enterprise_fee_set_collection_attributes_0_enterprise_id', + expect(page).to have_select('sets_enterprise_fee_set_collection_attributes_0_enterprise_id', selected: 'Second Distributor', options: ['', 'First Distributor', 'Second Distributor']) end diff --git a/spec/features/admin/enterprises/index_spec.rb b/spec/features/admin/enterprises/index_spec.rb index a79fbaab3c..2709dd2201 100644 --- a/spec/features/admin/enterprises/index_spec.rb +++ b/spec/features/admin/enterprises/index_spec.rb @@ -15,7 +15,7 @@ feature 'Enterprises Index' do within("tr.enterprise-#{s.id}") do expect(page).to have_content s.name - expect(page).to have_select "enterprise_set_collection_attributes_1_sells" + expect(page).to have_select "sets_enterprise_set_collection_attributes_1_sells" expect(page).to have_content "Settings" expect(page).to have_content "Delete" expect(page).to have_no_content "Payment Methods" @@ -25,7 +25,7 @@ feature 'Enterprises Index' do within("tr.enterprise-#{d.id}") do expect(page).to have_content d.name - expect(page).to have_select "enterprise_set_collection_attributes_0_sells" + expect(page).to have_select "sets_enterprise_set_collection_attributes_0_sells" expect(page).to have_content "Settings" expect(page).to have_content "Delete" expect(page).to have_content "Payment Methods" @@ -51,10 +51,10 @@ feature 'Enterprises Index' do it "updates the enterprises" do within("tr.enterprise-#{d.id}") do - expect(page).to have_checked_field "enterprise_set_collection_attributes_0_visible" - uncheck "enterprise_set_collection_attributes_0_visible" - select 'any', from: "enterprise_set_collection_attributes_0_sells" - select d_manager.email, from: 'enterprise_set_collection_attributes_0_owner_id' + expect(page).to have_checked_field "sets_enterprise_set_collection_attributes_0_visible" + uncheck "sets_enterprise_set_collection_attributes_0_visible" + select 'any', from: "sets_enterprise_set_collection_attributes_0_sells" + select d_manager.email, from: 'sets_enterprise_set_collection_attributes_0_owner_id' end click_button "Update" expect(flash_message).to eq('Enterprises updated successfully') @@ -83,12 +83,12 @@ feature 'Enterprises Index' do it "does not update the enterprises and displays errors" do d_row_index = enterprise_row_index(d.name) within("tr.enterprise-#{d.id}") do - select d_manager.email, from: "enterprise_set_collection_attributes_#{d_row_index}_owner_id" + select d_manager.email, from: "sets_enterprise_set_collection_attributes_#{d_row_index}_owner_id" end second_distributor_row_index = enterprise_row_index(second_distributor.name) within("tr.enterprise-#{second_distributor.id}") do - select d_manager.email, from: "enterprise_set_collection_attributes_#{second_distributor_row_index}_owner_id" + select d_manager.email, from: "sets_enterprise_set_collection_attributes_#{second_distributor_row_index}_owner_id" end click_button "Update" expect(flash_message).to eq('Update failed') diff --git a/spec/models/model_set_spec.rb b/spec/services/sets/model_set_spec.rb similarity index 72% rename from spec/models/model_set_spec.rb rename to spec/services/sets/model_set_spec.rb index 2dd5e12c8e..d9154c5bf1 100644 --- a/spec/models/model_set_spec.rb +++ b/spec/services/sets/model_set_spec.rb @@ -2,13 +2,15 @@ require 'spec_helper' -describe ModelSet do +describe Sets::ModelSet do describe "updating" do it "creates new models" do attrs = { collection_attributes: { '1' => { name: 's1' }, '2' => { name: 's2' } } } - ms = ModelSet.new(EnterpriseRelationshipPermission, EnterpriseRelationshipPermission.all, attrs) + ms = Sets::ModelSet.new(EnterpriseRelationshipPermission, + EnterpriseRelationshipPermission.all, + attrs) expect { ms.save }.to change(EnterpriseRelationshipPermission, :count).by(2) @@ -22,7 +24,7 @@ describe ModelSet do attrs = { collection_attributes: { '1' => { id: e1.id, name: 'e1zz', description: 'foo' }, '2' => { id: e2.id, name: 'e2yy', description: 'bar' } } } - ms = ModelSet.new(EnterpriseGroup, EnterpriseGroup.all, attrs) + ms = Sets::ModelSet.new(EnterpriseGroup, EnterpriseGroup.all, attrs) expect { ms.save }.to change(EnterpriseGroup, :count).by(0) @@ -36,8 +38,8 @@ describe ModelSet do attributes = { collection_attributes: { '1' => { id: e1.id, name: 'deleteme' }, '2' => { id: e2.id, name: 'e2' } } } - ms = ModelSet.new(Enterprise, Enterprise.all, attributes, nil, - proc { |attrs| attrs['name'] == 'deleteme' }) + ms = Sets::ModelSet.new(Enterprise, Enterprise.all, attributes, nil, + proc { |attrs| attrs['name'] == 'deleteme' }) expect { ms.save }.to change(Enterprise, :count).by(-1) @@ -48,8 +50,8 @@ describe ModelSet do it "ignores deletable new records" do attributes = { collection_attributes: { '1' => { name: 'deleteme' } } } - ms = ModelSet.new(Enterprise, Enterprise.all, attributes, nil, - proc { |attrs| attrs['name'] == 'deleteme' }) + ms = Sets::ModelSet.new(Enterprise, Enterprise.all, attributes, nil, + proc { |attrs| attrs['name'] == 'deleteme' }) expect { ms.save }.to change(Enterprise, :count).by(0) end diff --git a/spec/models/spree/product_set_spec.rb b/spec/services/sets/product_set_spec.rb similarity index 95% rename from spec/models/spree/product_set_spec.rb rename to spec/services/sets/product_set_spec.rb index c296ed7a73..36aaf9fefb 100644 --- a/spec/models/spree/product_set_spec.rb +++ b/spec/services/sets/product_set_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Spree::ProductSet do +describe Sets::ProductSet do describe '#save' do context 'when passing :collection_attributes' do let(:product_set) do @@ -88,7 +88,11 @@ describe Spree::ProductSet do end let(:distributor) { create(:distributor_enterprise) } - let!(:order_cycle) { create(:simple_order_cycle, variants: [product.variants.first], coordinator: distributor, distributors: [distributor]) } + let!(:order_cycle) { + create(:simple_order_cycle, variants: [product.variants.first], + coordinator: distributor, + distributors: [distributor]) + } it 'updates the product and removes the product from order cycles' do product_set.save