Fix admin pages

- move supplier to variant row on Bulk Edit product page
- add supplier dropdow on add/update variant page

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

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

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

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

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],

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

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,

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

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

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

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) }

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