Keep stock selection when error on saving

.rubocop_todo.yml

@@ -160,6 +160,7 @@ Metrics/ClassLength:
     - 'app/services/cart_service.rb'
     - 'app/services/order_cycles/form_service.rb'
     - 'app/services/orders/sync_service.rb'
+    - 'app/services/sets/product_set.rb'
     - 'engines/order_management/app/services/order_management/order/updater.rb'
     - 'lib/open_food_network/enterprise_fee_calculator.rb'
     - 'lib/open_food_network/order_cycle_form_applicator.rb'

app/models/concerns/variant_stock.rb

@@ -67,6 +67,18 @@ def on_demand=(new_value)
     end
   end
 
+  def on_demand_desired_or_current
+    return on_demand_desired if new_record?
+
+    on_demand
+  end
+
+  def on_hand_desired_or_current
+    return on_hand_desired if new_record?
+
+    on_hand
+  end
+
   # Moving Spree::Stock::Quantifier.can_supply? to the variant enables us
   #   to override this behaviour for variant overrides
   # We can have this responsibility here in the variant because there is

app/models/spree/variant.rb

@@ -11,6 +11,11 @@ class Variant < ApplicationRecord
 
     self.belongs_to_required_by_default = false
 
+    # 2 not to be persisted attributes to store preferences.
+    # Values to be set via the UI that can be passed by back to UI
+    # in a not yet persisted variant. Setters are below.
+    attr_reader :on_hand_desired, :on_demand_desired
+
     acts_as_paranoid
 
     searchable_attributes :sku, :display_as, :display_name, :primary_taxon_id, :supplier_id
@@ -244,6 +249,15 @@ def variant_unit_with_scale=(variant_unit_with_scale)
       )
     end
 
+    # aiming to deal with UI that deals with 0/"0"/1/"1"
+    def on_demand_desired=(val)
+      @on_demand_desired = ActiveModel::Type::Boolean.new.cast(val)
+    end
+
+    def on_hand_desired=(val)
+      @on_hand_desired = ActiveModel::Type::Integer.new.cast(val)
+    end
+
     private
 
     def check_currency

app/services/permitted_attributes/variant.rb

@@ -4,7 +4,8 @@ module PermittedAttributes
   class Variant
     def self.attributes
       [
-        :id, :sku, :on_hand, :on_demand, :shipping_category_id, :price, :unit_value,
+        :id, :sku, :on_hand, :on_hand_desired, :on_demand, :on_demand_desired,
+        :shipping_category_id, :price, :unit_value,
         :unit_description, :variant_unit, :variant_unit_name, :variant_unit_scale, :display_name,
         :display_as, :tax_category_id, :weight, :height, :width, :depth, :taxon_ids,
         :primary_taxon_id, :supplier_id

app/services/sets/product_set.rb

@@ -127,11 +127,15 @@ def create_variant(product, variant_attributes)
       on_demand = variant_attributes.delete(:on_demand)
 
       variant = product.variants.create(variant_attributes)
+
       return variant if variant.errors.present?
 
       begin
-        variant.on_demand = on_demand if on_demand.present?
-        variant.on_hand = on_hand.to_i if on_hand.present?
+        if on_hand || on_demand
+          create_stock_for_variant(variant, on_demand, on_hand)
+        else
+          create_stock_for_variant_from_desired(variant)
+        end
       rescue StandardError => e
         notify_bugsnag(e, product, variant, variant_attributes)
         raise e
@@ -153,5 +157,15 @@ def notify_bugsnag(error, product, variant, variant_attributes)
         report.add_metadata(:product_set, :variant_error, variant.errors.first) if !variant.valid?
       end
     end
+
+    def create_stock_for_variant(variant, on_demand, on_hand)
+      variant.on_demand = on_demand if on_demand.present?
+      variant.on_hand = on_hand.to_i if on_hand.present?
+    end
+
+    def create_stock_for_variant_from_desired(variant)
+      variant.on_demand = variant.on_demand_desired if variant.on_demand_desired.present?
+      variant.on_hand = variant.on_hand_desired.to_i if variant.on_hand_desired.present?
+    end
   end
 end

app/views/admin/products_v3/_variant_row.html.haml

@@ -1,3 +1,4 @@
+- method_on_demand, method_on_hand = variant.new_record? ? [:on_demand_desired, :on_hand_desired ]: [:on_demand, :on_hand]
 %td.col-image
   -# empty
 %td.col-name.field.naked_inputs
@@ -37,14 +38,14 @@
   = error_message_on variant, :price
 %td.col-on_hand.field.popout{'data-controller': "popout"}
   %button.popout__button{'data-popout-target': "button", 'aria-label': t('admin.products_page.columns.on_hand')}
-    = variant.on_demand ? t(:on_demand) : variant.on_hand
+    = variant.on_demand_desired_or_current ? t(:on_demand) : variant.on_hand_desired_or_current
   %div.popout__container{ style: 'display: none;', 'data-controller': 'toggle-control', 'data-popout-target': "dialog" }
     .field
-      = f.number_field :on_hand, min: 0, 'aria-label': t('admin.products_page.columns.on_hand'), 'data-toggle-control-target': 'control', disabled: f.object.on_demand
-      = error_message_on variant, :on_hand
+      = f.number_field method_on_hand, min: 0, 'aria-label': t('admin.products_page.columns.on_hand'), 'data-toggle-control-target': 'control', disabled: f.object.on_demand_desired_or_current
+      = error_message_on variant, method_on_hand
     .field.checkbox
-      = f.label :on_demand do
-        = f.check_box :on_demand, 'data-action': 'change->toggle-control#disableIfPresent change->popout#closeIfChecked'
+      = f.label method_on_demand do
+        = f.check_box method_on_demand, 'data-action': 'change->toggle-control#disableIfPresent change->popout#closeIfChecked'
         = t(:on_demand)
 %td.col-producer.field.naked_inputs
   = render(SearchableDropdownComponent.new(form: f,

spec/system/admin/products_v3/create_spec.rb

@@ -90,9 +90,9 @@
           select taxon.name, from: 'Category'
 
           if stock == "on_hand"
-            find('input[id$="_on_hand"]').fill_in with: "66"
+            find('input[id$="_on_hand_desired"]').fill_in with: "66"
           elsif stock == "on_demand"
-            find('input[id$="_on_demand"]').check
+            find('input[id$="_on_demand_desired"]').check
           end
         end
 

spec/system/admin/products_v3/update_spec.rb

@@ -654,6 +654,51 @@
       end
     end
 
+    context 'When trying to save an invalid variant with Stock value ' do
+      let(:new_variant_row) { find_field("Name", placeholder: "Apples", with: "").ancestor("tr") }
+
+      before do
+        visit admin_products_url
+        click_on "New variant"
+      end
+
+      it 'displays the correct value afterwards for On Hand' do
+        within new_variant_row do
+          fill_in "Name", with: "Large box"
+          click_on "On Hand"
+          fill_in "On Hand", with: "19"
+          tomselect_select("Weight (kg)", from: "Unit scale")
+          click_on "Unit"
+          fill_in "Unit value", with: "1"
+        end
+
+        click_button "Save changes"
+
+        expect(page).to have_content "Please review the errors and try again"
+        within row_containing_name("Large box") do
+          expect(page).to have_content "19"
+        end
+      end
+
+      it 'displays the correct value afterwards for On demand' do
+        within new_variant_row do
+          fill_in "Name", with: "Large box"
+          click_on "On Hand"
+          check "On demand"
+          tomselect_select("Weight (kg)", from: "Unit scale")
+          click_on "Unit"
+          fill_in "Unit value", with: "1"
+        end
+
+        click_button "Save changes"
+
+        expect(page).to have_content "Please review the errors and try again"
+        within row_containing_name("Large box") do
+          expect(page).to have_content "On demand"
+        end
+      end
+    end
+
     context "pagination" do
       let!(:product_a) { create(:simple_product, name: "zucchini") } # appears on p2