From 983c6145d42d8b17f24d948b29c460780f982d71 Mon Sep 17 00:00:00 2001
From: Adam Dunkley <adamdunkley@users.noreply.github.com>
Date: Sun, 15 Apr 2018 20:59:45 +0100
Subject: [PATCH] Variant property rules to optionally match all conditions

A new `apply_to_all` flag on `VariantPropertyRule` which, if true, only matches
properties to variants if all of the conditions are met. Previously it would match
any conditions which meant you could never apply a specific property to a variant
that had more than one option type.
---
 .../views/spree/admin/product_properties/index.html.erb   | 8 ++++++++
 .../controllers/spree/admin/products_controller_spec.rb   | 6 ++++++
 core/app/models/spree/variant_property_rule.rb            | 6 +++++-
 core/config/locales/en.yml                                | 1 +
 .../migrate/20180416083007_property_rules_apply_to_all.rb | 7 +++++++
 core/spec/models/spree/variant_property_rule_spec.rb      | 7 +++++++
 6 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 core/db/migrate/20180416083007_property_rules_apply_to_all.rb

diff --git a/backend/app/views/spree/admin/product_properties/index.html.erb b/backend/app/views/spree/admin/product_properties/index.html.erb
index 77b807fa6e5..2c8003af11b 100644
--- a/backend/app/views/spree/admin/product_properties/index.html.erb
+++ b/backend/app/views/spree/admin/product_properties/index.html.erb
@@ -66,6 +66,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 %>">
diff --git a/backend/spec/controllers/spree/admin/products_controller_spec.rb b/backend/spec/controllers/spree/admin/products_controller_spec.rb
index d6713527616..9311b0e5d80 100644
--- a/backend/spec/controllers/spree/admin/products_controller_spec.rb
+++ b/backend/spec/controllers/spree/admin/products_controller_spec.rb
@@ -62,6 +62,7 @@
           id: product.id,
           variant_property_rules_attributes: {
             "0" => {
+              apply_to_all: true,
               option_value_ids: option_value.id,
               values_attributes: {
                 "0" => {
@@ -85,6 +86,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
diff --git a/core/app/models/spree/variant_property_rule.rb b/core/app/models/spree/variant_property_rule.rb
index e41f06a97da..6494a10f501 100644
--- a/core/app/models/spree/variant_property_rule.rb
+++ b/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
diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml
index f2660484f68..c7609e6844b 100644
--- a/core/config/locales/en.yml
+++ b/core/config/locales/en.yml
@@ -797,6 +797,7 @@ en:
     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
diff --git a/core/db/migrate/20180416083007_property_rules_apply_to_all.rb b/core/db/migrate/20180416083007_property_rules_apply_to_all.rb
new file mode 100644
index 00000000000..0a064c5fd41
--- /dev/null
+++ b/core/db/migrate/20180416083007_property_rules_apply_to_all.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class PropertyRulesApplyToAll < ActiveRecord::Migration[5.1]
+  def change
+    add_column :spree_variant_property_rules, :apply_to_all, :boolean, default: false
+  end
+end
diff --git a/core/spec/models/spree/variant_property_rule_spec.rb b/core/spec/models/spree/variant_property_rule_spec.rb
index 921b56c07df..7d9df24d400 100644
--- a/core/spec/models/spree/variant_property_rule_spec.rb
+++ b/core/spec/models/spree/variant_property_rule_spec.rb
@@ -70,6 +70,13 @@
       it { is_expected.to eq true }
     end
 
+    context "variant cannot match only some of the rule's conditions when applies to all" do
+      let(:rule) { create(:variant_property_rule, option_value: rule_option_value, apply_to_all: true) }
+      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)] }