Skip to content

Commit

Permalink
Requested changes
Browse files Browse the repository at this point in the history
- 2 new methods for reading either current/desired on hand/on demand
  depending on variant state. Goal is to get rid of send method in View
- referring in on_hand/on_demand is in fact irrelevant. In the piece of
  code, only desired on_hand/on_demand can be called as we are only in
  new variant (non persisted) mode
- View does not use send method anymore, replaced by current_or_desired
- refactor of the spec -> 2 examples in one to get more speed.
  • Loading branch information
cyrillefr committed Jun 28, 2024
1 parent f7a1936 commit 32b386e
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 9 deletions.
12 changes: 12 additions & 0 deletions app/models/concerns/variant_stock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,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
Expand Down
11 changes: 9 additions & 2 deletions app/services/sets/product_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,11 @@ def create_variant(product, variant_attributes)
return variant if variant.errors.present?

begin
create_stock_for_variant(variant, on_demand, on_hand)
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
Expand All @@ -156,8 +160,11 @@ def notify_bugsnag(error, product, variant, variant_attributes)

def create_stock_for_variant(variant, on_demand, on_hand)
variant.on_demand = on_demand if on_demand.present?
variant.on_demand = variant.on_demand_desired if variant.on_demand_desired.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
Expand Down
4 changes: 2 additions & 2 deletions app/views/admin/products_v3/_variant_row.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
= 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.send(method_on_demand) ? t(:on_demand) : variant.send(method_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 method_on_hand, min: 0, 'aria-label': t('admin.products_page.columns.on_hand'), 'data-toggle-control-target': 'control', disabled: f.object.send(method_on_demand)
= 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 method_on_demand do
Expand Down
7 changes: 2 additions & 5 deletions spec/system/admin/products_v3/products_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ def expect_other_columns_visible
click_on "New variant"
end

it 'displays the correct value afterwards for On Hand' do
it 'displays the correct value afterwards both for On Hand & On demand' do
within new_variant_row do
fill_in "Name", with: "Large box"
click_on "On Hand"
Expand All @@ -947,11 +947,8 @@ def expect_other_columns_visible
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"
within row_containing_name("Large box") do
click_on "On Hand"
check "On demand"
end
Expand Down

0 comments on commit 32b386e

Please sign in to comment.