Merge pull request #368 from richardnuno/variant-property-rules

Introduce variant level properties

api/app/helpers/spree/api/api_helpers.rb

@@ -33,7 +33,8 @@ module ApiHelpers
  :store_credit_history_attributes,
  :stock_transfer_attributes,
  :transfer_item_attributes,
- :transfer_item_variant_attributes
+ :transfer_item_variant_attributes,
+ :variant_property_attributes
  ]
 
  mattr_reader *ATTRIBUTES
@@ -65,6 +66,10 @@ def required_fields_for(model)
  :slug, :description, :track_inventory
  ]
 
+ @@variant_property_attributes = [
+ :id, :property_id, :value, :property_name
+ ]
+
  @@image_attributes = [
  :id, :position, :attachment_content_type, :attachment_file_name, :type,
  :attachment_updated_at, :attachment_width, :attachment_height, :alt

api/app/views/spree/api/variants/big.v1.rabl

@@ -9,6 +9,9 @@ node :total_on_hand do
  root_object.total_on_hand
 end
 
+child :variant_properties => :variant_properties do
+ attributes *variant_property_attributes
+end
 
 child(root_object.stock_items.accessible_by(current_ability) => :stock_items) do
  attributes :id, :count_on_hand, :stock_location_id, :backorderable

api/spec/controllers/spree/api/variants_controller_spec.rb

@@ -12,7 +12,7 @@ module Spree
  end
 
  let!(:base_attributes) { Api::ApiHelpers.variant_attributes }
- let!(:show_attributes) { base_attributes.dup.push(:in_stock, :display_price) }
+ let!(:show_attributes) { base_attributes.dup.push(:in_stock, :display_price, :variant_properties) }
  let!(:new_attributes) { base_attributes }
 
  before do
@@ -143,8 +143,10 @@ module Spree
 
  describe "#show" do
 
+ subject { api_get :show, id: variant.to_param }
+
  it "can see a single variant" do
- api_get :show, :id => variant.to_param
+ subject
  expect(json_response).to have_attributes(show_attributes)
  expect(json_response["stock_items"]).to be_present
  option_values = json_response["option_values"]
@@ -157,7 +159,7 @@ module Spree
  it "can see a single variant with images" do
  variant.images.create!(:attachment => image("thinking-cat.jpg"))
 
- api_get :show, :id => variant.to_param
+ subject
 
  expect(json_response).to have_attributes(show_attributes + [:images])
  option_values = json_response["option_values"]
@@ -166,6 +168,33 @@ module Spree
  :option_type_name,
  :option_type_id])
  end
+
+ context "variant doesn't have variant properties" do
+ before { subject }
+
+ it "contains the expected attributes" do
+ expect(json_response).to have_attributes(show_attributes)
+ end
+
+ it "variant properties is an empty list" do
+ expect(json_response["variant_properties"]).to eq []
+ end
+ end
+
+ context "variant has variant properties" do
+ let!(:rule) { create(:variant_property_rule, product: variant.product, option_value: variant.option_values.first) }
+
+ before { subject }
+
+ it "contains the expected attributes" do
+ expect(json_response).to have_attributes(show_attributes)
+ end
+
+ it "variant properties is an array of variant property values" do
+ expected_attrs = [:id, :property_id, :value, :property_name]
+ expect(json_response["variant_properties"].first).to have_attributes(expected_attrs)
+ end
+ end
  end
 
  it "can learn how to create a new variant" do

backend/app/assets/javascripts/spree/backend/admin.js.erb

@@ -112,8 +112,9 @@ $(document).ready(function(){
  new_table_row.find("input, select").each(function () {
  var el = $(this);
  el.val("");
- el.prop("id", el.prop("id").replace(/\d+/, new_id))
- el.prop("name", el.prop("name").replace(/\d+/, new_id))
+ // Replace last occurrence of a number
+ el.prop("id", el.prop("id").replace(/\d+(?=[^\d]*$)/, new_id))
+ el.prop("name", el.prop("name").replace(/\d+(?=[^\d]*$)/, new_id))
  })
  // When cloning a new row, set the href of all icons to be an empty "#"
  // This is so that clicking on them does not perform the actions for the
@@ -204,18 +205,21 @@ $(document).ready(function(){
  placeholder: 'ui-sortable-placeholder',
  update: function(event, ui) {
  $("#progress").show();
+ tableEl = $(ui.item).closest("table.sortable")
  positions = {};
- $.each($('table.sortable tbody tr'), function(position, obj){
- reg = /spree_(\w+_?)+_(\d+)/;
- parts = reg.exec($(obj).prop('id'));
- if (parts) {
- positions['positions['+parts[2]+']'] = position;
+ $.each(tableEl.find('tbody tr'), function(position, obj){
+ idAttr = $(obj).prop('id');
+ if (idAttr) {
+ objId = idAttr.split('_').slice(-1);
+ if (!isNaN(objId)) {
+ positions['positions['+objId+']'] = position;
+ }
  }
  });
  Spree.ajax({
  type: 'POST',
  dataType: 'script',
- url: $(ui.item).closest("table.sortable").data("sortable-link"),
+ url: tableEl.data("sortable-link"),
  data: positions,
  success: function(data){ $("#progress").hide(); }
  });
@@ -227,9 +231,10 @@ $(document).ready(function(){
  ui.placeholder.html("<td colspan='"+(td_count-1)+"'></td><td class='actions'></td>")
  },
  stop: function (event, ui) {
+ tableEl = $(ui.item).closest("table.sortable")
  // Fix odd/even classes after reorder
- $("table.sortable tr:even").removeClass("odd even").addClass("even");
- $("table.sortable tr:odd").removeClass("odd even").addClass("odd");
+ tableEl.find("tr:even").removeClass("odd even").addClass("even");
+ tableEl.find("tr:odd").removeClass("odd even").addClass("odd");
  }
 
  });

backend/app/controllers/spree/admin/product_properties_controller.rb

@@ -4,15 +4,24 @@ class ProductPropertiesController < ResourceController
  belongs_to 'spree/product', :find_by => :slug
  before_action :find_properties
  before_action :setup_property, only: :index, if: -> { can?(:create, model_class) }
+ before_action :setup_variant_property_rules, only: :index
 
  private
+
  def find_properties
  @properties = Spree::Property.pluck(:name)
  end
 
  def setup_property
  @product.product_properties.build
  end
+
+ def setup_variant_property_rules
+ @option_types = @product.variant_option_values_by_option_type
+ @option_value_ids = (params[:ovi] || []).reject(&:blank?).map(&:to_i)
+ @variant_property_rule = @product.find_variant_property_rule(@option_value_ids) || @product.variant_property_rules.build
+ @variant_property_rule.values.build if can?(:create, Spree::VariantPropertyRuleValue)
+ end
  end
  end
 end

backend/app/controllers/spree/admin/products_controller.rb

@@ -25,6 +25,11 @@ def update
  if params[:product][:option_type_ids].present?
  params[:product][:option_type_ids] = params[:product][:option_type_ids].split(',')
  end
+ if updating_variant_property_rules?
+ params[:product][:variant_property_rules_attributes].each do |index, param_attrs|
+ param_attrs[:option_value_ids] = param_attrs[:option_value_ids].split(',')
+ end
+ end
  invoke_callbacks(:update, :before)
  if @object.update_attributes(permitted_resource_params)
  invoke_callbacks(:update, :after)
@@ -73,7 +78,16 @@ def find_resource
  end
 
  def location_after_save
- spree.edit_admin_product_url(@product)
+ if updating_variant_property_rules?
+ url_params = {}
+ url_params[:ovi] = []
+ params[:product][:variant_property_rules_attributes].each do |index, param_attrs|
+ url_params[:ovi] += param_attrs[:option_value_ids]
+ end
+ spree.admin_product_product_properties_url(@product, url_params)
+ else
+ spree.edit_admin_product_url(@product)
+ end
  end
 
  def load_data
@@ -126,10 +140,13 @@ def variant_stock_includes
  [:images, stock_items: :stock_location, option_values: :option_type]
  end
 
-
  def variant_scope
  @product.variants
  end
+
+ def updating_variant_property_rules?
+ params[:product][:variant_property_rules_attributes].present?
+ end
  end
  end
 end

backend/app/controllers/spree/admin/variant_property_rule_values_controller.rb

@@ -0,0 +1,7 @@
+module Spree
+ module Admin
+ class VariantPropertyRuleValuesController < ResourceController
+ belongs_to 'spree/product', find_by: :slug
+ end
+ end
+end

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

@@ -18,7 +18,8 @@
 <% end %>
 
 <%= form_for @product, :url => admin_product_url(@product), :method => :put do |f| %>
- <fieldset class="no-border-top">
+ <fieldset>
+ <legend align="center"><%= Spree.t(:product_properties) %></legend>
  <div class="add_product_properties" data-hook="add_product_properties"></div>
 
  <div id="prototypes" data-hook></div>
@@ -47,9 +48,58 @@
  </fieldset>
 <% end %>
 
+<%= form_tag admin_product_product_properties_path, method: :get, id: 'variant_option_value_selections' do %>
+ <fieldset class='no-border-bottom'>
+ <legend align="center"><%= Spree.t(:variant_properties) %></legend>
+ <fieldset class='no-border-top'>
+ <% @option_types.each do |option_type, option_values| %>
+ <div class="field">
+ <%= label :option_type_presentation, option_type.presentation %>
+ <%= select_tag "ovi[]", options_from_collection_for_select(option_values, :id, :presentation, params[:ovi]), class: 'select2 fullwidth', include_blank: true, id: "#{option_type.name}_option_type_select" %>
+ </div>
+ <% end %>
+ <div class="form-buttons filter-actions actions">
+ <%= button Spree.t(:filter_results) %>
+ <% if @option_value_ids.present? %>
+ <span class="or"><%= Spree.t(:or) %></span>
+ <%= link_to_add_fields Spree.t(:add_variant_properties), 'tbody#variant_property_values', class: 'plus button' %>
+ <% end %>
+ </div>
+ </fieldset>
+ </fieldset>
+<% end %>
+
+<%= form_for @product, url: admin_product_url(@product), method: :put do |f| %>
+ <%= 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(',') %>
+ <% 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 %>">
+ <thead>
+ <tr data-hook="variant_property_values_header">
+ <th colspan="2"><%= Spree.t(:property) %></th>
+ <th><%= Spree.t(:value) %></th>
+ <th class="actions"></th>
+ </tr>
+ </thead>
+ <tbody id="variant_property_values" data-hook>
+ <%= rule_form.fields_for :values do |values_form| %>
+ <%= render 'product_property_fields', f: values_form %>
+ <% end %>
+ </tbody>
+ </table>
+ <% if can?([:create, :update], Spree::VariantPropertyRule) %>
+ <%= render 'spree/admin/shared/edit_resource_links' %>
+ <% end %>
+ </fieldset>
+ <% end %>
+ <% end %>
+<% end %>
+
 <%= javascript_tag do -%>
  var properties = <%= raw(@properties.to_json) %>;
- $('#product_properties').on('keydown', 'input.autocomplete', function() {
+ $('#product_properties, #variant_property_values').on('keydown', 'input.autocomplete', function() {
  already_auto_completed = $(this).is('ac_input');
  if (!already_auto_completed) {
  $(this).autocomplete({source: properties});

backend/config/routes.rb

@@ -31,6 +31,11 @@
  post :update_positions
  end
  end
+ resources :variant_property_rule_values, only: [:destroy] do
+ collection do
+ post :update_positions
+ end
+ end
  resources :images do
  collection do
  post :update_positions

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

@@ -0,0 +1,69 @@
+require 'spec_helper'
+
+module Spree
+ module Admin
+ describe ProductPropertiesController, type: :controller do
+ stub_authorization!
+
+ describe "#index" do
+ subject { spree_get :index, parameters }
+
+ context "no option values are provided" do
+ let(:product) { create(:product) }
+ let(:parameters) do
+ { product_id: product.to_param }
+ end
+
+ before { subject }
+
+ it "instantiates a new variant property rule" do
+ expect(assigns(:variant_property_rule)).to_not be_persisted
+ end
+
+ it "instantiates a new variant property rule value" do
+ expect(assigns(:variant_property_rule).values.size).to eq 1
+ expect(assigns(:variant_property_rule).values.first).to_not be_persisted
+ end
+ end
+
+ context "option values are provided" do
+ let(:size) { create(:option_type, name: 'size') }
+ let(:product) { create(:product, option_types: [size]) }
+ let(:size_small) { create(:option_value, name: 'small', option_type: size) }
+ let(:size_large) { create(:option_value, name: 'large', option_type: size) }
+ let!(:first_rule) { create(:variant_property_rule, product: product, option_value: size_small) }
+
+ context "no rules match the option values" do
+ let(:parameters) do
+ {
+ product_id: product.to_param,
+ ovi: [size_large.id]
+ }
+ end
+
+ before { subject }
+
+ it "instantiates a new variant property rule" do
+ expect(assigns(:variant_property_rule)).to_not be_persisted
+ end
+ end
+
+ context "a rule matches the option values" do
+ let(:parameters) do
+ {
+ product_id: product.to_param,
+ ovi: [size_small.id]
+ }
+ end
+
+ before { subject }
+
+ it "assigns the property rule to the only property rule that matches the option values" do
+ expect(assigns(:variant_property_rule)).to eq first_rule
+ end
+ end
+ end
+ end
+ end
+ end
+end