Merge pull request #3653 from filippoliverani/filippoliverani/variant…

…-property-rules-all-fix

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

backend/app/views/spree/admin/product_properties/index.html.erb

@@ -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 %>">

backend/spec/controllers/spree/admin/products_controller_spec.rb

@@ -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" => {
@@ -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

core/app/models/spree/variant_property_rule.rb

@@ -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

core/config/locales/en.yml

@@ -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

core/db/migrate/20180416083007_add_apply_to_all_to_variant_property_rule.rb

@@ -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

core/spec/models/spree/variant_property_rule_spec.rb

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

core/spec/models/spree/variant_spec.rb

@@ -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