Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variant property rules to optionally match all conditions (cont.) #3653

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@
<%= f.fields_for :variant_property_rules, @variant_property_rule do |rule_form| %>
<%= rule_form.hidden_field 'id', value: @variant_property_rule.id %>
<%= rule_form.hidden_field 'option_value_ids', value: @option_value_ids.join(',') %>
<div class="col-12">
<div class="field checkbox">
<label>
<%= rule_form.check_box 'apply_to_all', value: @variant_property_rule.apply_to_all %>
<%= t('spree.applies_to_all_variant_properties') %>
</label>
</div>
</div>
<% if @option_value_ids.present? %>
<fieldset class='no-border-top'>
<table class="index sortable" data-hook data-sortable-link="<%= update_positions_admin_product_variant_property_rule_values_url %>">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@
id: product.id,
variant_property_rules_attributes: {
"0" => {
apply_to_all: true,
option_value_ids: option_value.id,
values_attributes: {
"0" => {
Expand All @@ -157,6 +158,11 @@
expect { subject }.to change { product.variant_property_rules.count }.by(1)
end

it "creates a variant property rule that applies to all" do
subject
expect(product.variant_property_rules.first.apply_to_all).to be_truthy
end

it "creates a variant property rule condition" do
expect { subject }.to change { product.variant_property_rule_conditions.count }.by(1)
end
Expand Down
6 changes: 5 additions & 1 deletion core/app/models/spree/variant_property_rule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ def matches_option_value_ids?(option_value_ids)
# @param variant [Spree::Variant] variant to check
# @return [Boolean]
def applies_to_variant?(variant)
(option_value_ids & variant.option_value_ids).present?
if apply_to_all
matches_option_value_ids?(variant.option_value_ids)
else
(option_value_ids & variant.option_value_ids).present?
end
end
end
end
2 changes: 2 additions & 0 deletions core/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,8 @@ en:
add_option_value: Add Option Value
add_product: Add Product
add_product_properties: Add Product Properties
add_variant_properties: Add Variant Properties
applies_to_all_variant_properties: Applies to all Variant Properties above
add_rule_of_type: Add rule of type
add_state: Add State
add_stock: Add Stock
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# frozen_string_literal: true

class AddApplyToAllToVariantPropertyRule < ActiveRecord::Migration[5.1]
def change
add_column :spree_variant_property_rules, :apply_to_all, :boolean, default: false, null: false
change_column :spree_variant_property_rules, :apply_to_all, :boolean, default: true
end

def down
remove_column :spree_variant_property_rules, :apply_to_all
end
end
9 changes: 8 additions & 1 deletion core/spec/models/spree/variant_property_rule_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,19 @@

subject { rule.applies_to_variant?(variant) }

context "variant matches some of the rule's conditions" do
context "variant matches some of the rule's conditions when apply_to_all is false" do
let(:rule) { create(:variant_property_rule, option_value: rule_option_value, apply_to_all: false) }
let(:option_values) { [variant_option_value_1, variant_option_value_2] }

it { is_expected.to eq true }
end

context "variant cannot match only some of the rule's conditions when apply_to_all is true" do
let(:option_values) { [variant_option_value_1, variant_option_value_2] }

it { is_expected.to eq false }
end

context "variant matches none of the rule's conditions" do
let(:option_values) { [create(:option_value)] }

Expand Down
4 changes: 2 additions & 2 deletions core/spec/models/spree/variant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -807,8 +807,8 @@
subject { variant.variant_properties }

context "variant has properties" do
let!(:rule_1) { create(:variant_property_rule, product: variant.product, option_value: option_value_1) }
let!(:rule_2) { create(:variant_property_rule, product: variant.product, option_value: option_value_2) }
let!(:rule_1) { create(:variant_property_rule, product: variant.product, option_value: option_value_1, apply_to_all: false) }
let!(:rule_2) { create(:variant_property_rule, product: variant.product, option_value: option_value_2, apply_to_all: false) }

it "returns the variant property rule's values" do
expect(subject).to match_array rule_1.values + rule_2.values
Expand Down