From 25371ee9d0f98bd852dd12ee7c65dc5103111d86 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 23 Apr 2024 13:55:16 +1000 Subject: [PATCH] Fix admin pages - move supplier to variant row on Bulk Edit product page - add supplier dropdow on add/update variant page --- .../spree/admin/products_controller.rb | 10 ++-- .../spree/admin/variants_controller.rb | 8 +++ app/reflexes/products_reflex.rb | 2 +- app/services/exchange_variant_deleter.rb | 6 +-- app/services/permitted_attributes/product.rb | 2 +- .../admin/products_v3/_product_row.html.haml | 9 +--- .../admin/products_v3/_variant_row.html.haml | 9 +++- .../spree/admin/products/_form.html.haml | 6 --- .../spree/admin/variants/_form.html.haml | 4 ++ .../spree/admin/products_controller_spec.rb | 19 +------ .../spree/admin/variants_controller_spec.rb | 53 +++++++++++++++++++ spec/models/spree/product_spec.rb | 4 ++ 12 files changed, 87 insertions(+), 45 deletions(-) diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 32099627d8..b8aa975f2f 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -27,6 +27,9 @@ module Spree end def new + @producers = OpenFoodNetwork::Permissions.new(spree_current_user). + managed_product_enterprises.is_primary_producer.by_name + @object.shipping_category_id = DefaultShippingCategory.find_or_create.id end @@ -52,14 +55,9 @@ module Spree def update @url_filters = ::ProductFilters.new.extract(request.query_parameters) - original_supplier_id = @product.supplier_id delete_stock_params_and_set_after do params[:product] ||= {} if params[:clear_product_properties] if @object.update(permitted_resource_params) - if original_supplier_id != @product.supplier_id - ExchangeVariantDeleter.new.delete(@product) - end - flash[:success] = flash_message_for(@object, :successfully_updated) end redirect_to spree.edit_admin_product_url(@object, @url_filters) @@ -157,8 +155,6 @@ module Spree end def load_form_data - @producers = OpenFoodNetwork::Permissions.new(spree_current_user). - managed_product_enterprises.is_primary_producer.by_name @taxons = Spree::Taxon.order(:name) @import_dates = product_import_dates.uniq.to_json end diff --git a/app/controllers/spree/admin/variants_controller.rb b/app/controllers/spree/admin/variants_controller.rb index 47505f7b1d..33f29e1b45 100644 --- a/app/controllers/spree/admin/variants_controller.rb +++ b/app/controllers/spree/admin/variants_controller.rb @@ -46,7 +46,13 @@ module Spree def update @url_filters = ::ProductFilters.new.extract(request.query_parameters) + original_supplier_id = @object.supplier_id + if @object.update(permitted_resource_params) + if original_supplier_id != @object.supplier_id + ExchangeVariantDeleter.new.delete(@object) + end + flash[:success] = flash_message_for(@object, :successfully_updated) redirect_to spree.admin_product_variants_url(params[:product_id], @url_filters) else @@ -113,6 +119,8 @@ module Spree private def load_data + @producers = OpenFoodNetwork::Permissions.new(spree_current_user). + managed_product_enterprises.is_primary_producer.by_name @tax_categories = TaxCategory.order(:name) @shipping_categories = ShippingCategory.order(:name) end diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index a2feef4d28..3816ce88fc 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -108,7 +108,7 @@ class ProductsReflex < ApplicationReflex def ransack_query query = {} - query.merge!(supplier_id_in: @producer_id) if @producer_id.present? + query.merge!(variant_supplier_id_in: @producer_id) if @producer_id.present? if @search_term.present? query.merge!(Spree::Variant::SEARCH_KEY => @search_term) end diff --git a/app/services/exchange_variant_deleter.rb b/app/services/exchange_variant_deleter.rb index 0db1abd884..beb005795b 100644 --- a/app/services/exchange_variant_deleter.rb +++ b/app/services/exchange_variant_deleter.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true class ExchangeVariantDeleter - def delete(product) - ExchangeVariant. - where(variant_id: product.variants.select(:id)). - delete_all + def delete(variant) + ExchangeVariant.where(variant_id: variant.id).delete_all end end diff --git a/app/services/permitted_attributes/product.rb b/app/services/permitted_attributes/product.rb index 1fd91fcd1a..f080f2b730 100644 --- a/app/services/permitted_attributes/product.rb +++ b/app/services/permitted_attributes/product.rb @@ -8,7 +8,7 @@ module PermittedAttributes :variant_unit, :variant_unit_scale, :variant_unit_with_scale, :unit_value, :unit_description, :variant_unit_name, :display_as, :sku, :group_buy, :group_buy_unit_size, - :taxon_ids, :primary_taxon_id, :tax_category_id, + :taxon_ids, :primary_taxon_id, :tax_category_id, :supplier_id, :meta_keywords, :notes, :inherits_properties, { product_properties_attributes: [:id, :property_name, :value], variants_attributes: [PermittedAttributes::Variant.attributes], diff --git a/app/views/admin/products_v3/_product_row.html.haml b/app/views/admin/products_v3/_product_row.html.haml index 375e8ad47a..f5c4fd8b5d 100644 --- a/app/views/admin/products_v3/_product_row.html.haml +++ b/app/views/admin/products_v3/_product_row.html.haml @@ -25,13 +25,8 @@ -# empty %td.col-on_hand.align-right -# empty -%td.col-producer.naked_inputs - = render(SearchableDropdownComponent.new(form: f, - name: :supplier_id, - aria_label: t('.producer_field_name'), - options: producer_options, - selected_option: product.supplier_id, - placeholder_value: t('admin.products_v3.filters.search_for_producers'))) +%td.col-on_hand.align-right + -# empty %td.col-category.align-left -# empty %td.col-tax_category.align-left diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index 311b833e04..b137c25de0 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -39,8 +39,13 @@ = f.label :on_demand do = f.check_box :on_demand, 'data-action': 'change->toggle-control#disableIfPresent change->popout#closeIfChecked' = t(:on_demand) -%td.col-producer.align-left - -# empty producer name +%td.col-producer.naked_inputs + = render(SearchableDropdownComponent.new(form: f, + name: :supplier_id, + aria_label: t('.producer_field_name'), + options: producer_options, + selected_option: variant.supplier_id, + placeholder_value: t('admin.products_v3.filters.search_for_producers'))) %td.col-category.field.naked_inputs = render(SearchableDropdownComponent.new(form: f, name: :primary_taxon_id, diff --git a/app/views/spree/admin/products/_form.html.haml b/app/views/spree/admin/products/_form.html.haml index 1f3492c123..546efeace4 100644 --- a/app/views/spree/admin/products/_form.html.haml +++ b/app/views/spree/admin/products/_form.html.haml @@ -27,12 +27,6 @@ = f.text_field :variant_unit_name, {placeholder: t('admin.products.unit_name_placeholder')} = f.error_message_on :variant_unit_name - = f.field_container :supplier do - = f.label :supplier, t(:spree_admin_supplier) - %br - = f.collection_select(:supplier_id, @producers, :id, :name, {:include_blank => true}, {:class => "select2"}) - = f.error_message_on :supplier - .clear .clear diff --git a/app/views/spree/admin/variants/_form.html.haml b/app/views/spree/admin/variants/_form.html.haml index f90f15c1f7..dcb4d68b6e 100644 --- a/app/views/spree/admin/variants/_form.html.haml +++ b/app/views/spree/admin/variants/_form.html.haml @@ -76,4 +76,8 @@ = f.label :primary_taxon, t('spree.admin.products.primary_taxon_form.product_category') = f.collection_select(:primary_taxon_id, Spree::Taxon.order(:name), :id, :name, { include_blank: true }, { class: "select2 fullwidth" }) + .field + = f.label :supplier, t(:spree_admin_supplier) + = f.collection_select(:supplier_id, @producers, :id, :name, {:include_blank => true}, {:class => "select2 fullwidth"}) + .clear diff --git a/spec/controllers/spree/admin/products_controller_spec.rb b/spec/controllers/spree/admin/products_controller_spec.rb index 010e1468e4..f41c4adfcd 100644 --- a/spec/controllers/spree/admin/products_controller_spec.rb +++ b/spec/controllers/spree/admin/products_controller_spec.rb @@ -113,7 +113,8 @@ RSpec.describe Spree::Admin::ProductsController, type: :controller do "unit_value" => 4, "unit_description" => "", "display_name" => "name", - "primary_taxon_id" => taxon.id + "primary_taxon_id" => taxon.id, + "supplier_id" => producer.id } ] } @@ -180,22 +181,6 @@ RSpec.describe Spree::Admin::ProductsController, type: :controller do controller_login_as_enterprise_user [producer] end - describe "change product supplier" do - let(:distributor) { create(:distributor_enterprise) } - let!(:order_cycle) { - create(:simple_order_cycle, variants: [product.variants.first], coordinator: distributor, - distributors: [distributor]) - } - - it "should remove product from existing Order Cycles" do - new_producer = create(:enterprise) - spree_put :update, id: product, product: { supplier_id: new_producer.id } - - expect(product.reload.supplier.id).to eq new_producer.id - expect(order_cycle.reload.distributed_variants).not_to include product.variants.first - end - end - describe "product stock setting with errors" do it "notifies bugsnag and still raise error" do # forces an error in the variant diff --git a/spec/controllers/spree/admin/variants_controller_spec.rb b/spec/controllers/spree/admin/variants_controller_spec.rb index 4ef69d61f7..3ee35450ff 100644 --- a/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/spec/controllers/spree/admin/variants_controller_spec.rb @@ -31,6 +31,59 @@ module Spree end end + describe "#update" do + let!(:variant) { create(:variant, display_name: "Tomatoes", sku: 123, supplier: producer) } + let(:producer) { create(:enterprise) } + + it "updates the variant" do + expect { + spree_put( + :update, + id: variant.id, + product_id: variant.product.id, + variant: { display_name: "Better tomatoes", sku: 456 } + ) + variant.reload + }.to change { variant.display_name }.to("Better tomatoes") + .and change { variant.sku }.to(456.to_s) + end + + context "when updating supplier" do + let(:new_producer) { create(:enterprise) } + + it "updates the supplier" do + expect { + spree_put( + :update, + id: variant.id, + product_id: variant.product.id, + variant: { supplier_id: new_producer.id } + ) + variant.reload + }.to change { variant.supplier_id }.to(new_producer.id) + end + + it "removes associated product from existing Order Cycles" do + distributor = create(:distributor_enterprise) + order_cycle = create( + :simple_order_cycle, + variants: [variant], + coordinator: distributor, + distributors: [distributor] + ) + + spree_put( + :update, + id: variant.id, + product_id: variant.product.id, + variant: { supplier_id: new_producer.id } + ) + + expect(order_cycle.reload.distributed_variants).to_not include variant + end + end + end + describe "#search" do let(:supplier) { create(:supplier_enterprise) } let!(:p1) { create(:simple_product, name: 'Product 1', supplier_id: supplier.id) } diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 41aa38f8e1..b9dce1b3b1 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -184,6 +184,7 @@ module Spree let!(:product){ Spree::Product.new } let!(:shipping_category){ create(:shipping_category) } let!(:taxon){ create(:taxon) } + let(:supplier){ create(:enterprise) } before do create(:stock_location) @@ -194,15 +195,18 @@ module Spree product.unit_value = 1 product.price = 4.27 product.shipping_category_id = shipping_category.id + product.supplier_id = supplier.id product.save! end it "copies properties to the first standard variant" do expect(product.variants.reload.length).to eq 1 standard_variant = product.variants.reload.first + expect(standard_variant).to be_valid expect(standard_variant.price).to eq 4.27 expect(standard_variant.shipping_category).to eq shipping_category expect(standard_variant.primary_taxon).to eq taxon + expect(standard_variant.supplier).to eq supplier end end