From 03306bd2641e584c997dd06abb36e719e8709cca Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Fri, 7 Oct 2016 14:04:16 -0700 Subject: [PATCH 01/11] Add for_master and for_variant scopes to price model --- core/app/models/spree/price.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/app/models/spree/price.rb b/core/app/models/spree/price.rb index 4bf597f9da9..cd9b9b56dd2 100644 --- a/core/app/models/spree/price.rb +++ b/core/app/models/spree/price.rb @@ -19,6 +19,8 @@ class Price < Spree::Base validates :country, presence: true, unless: -> { for_any_country? } scope :currently_valid, -> { order("country_iso IS NULL, updated_at DESC") } + scope :for_master, -> { joins(:variant).where(spree_variants: { is_master: true }) } + scope :for_variant, -> { joins(:variant).where(spree_variants: { is_master: false }) } scope :for_any_country, -> { where(country: nil) } scope :with_default_attributes, -> { where(Spree::Config.default_pricing_options.desired_attributes) } From 9eec41f6e29bfc52d62385360ef6be5ef8777bf2 Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Fri, 7 Oct 2016 14:05:23 -0700 Subject: [PATCH 02/11] Partition prices for master and variants in prices controller --- backend/app/controllers/spree/admin/prices_controller.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/backend/app/controllers/spree/admin/prices_controller.rb b/backend/app/controllers/spree/admin/prices_controller.rb index 25d4902f77d..a4011d1d270 100644 --- a/backend/app/controllers/spree/admin/prices_controller.rb +++ b/backend/app/controllers/spree/admin/prices_controller.rb @@ -7,8 +7,14 @@ def index params[:q] ||= {} @search = @product.prices.accessible_by(current_ability, :index).ransack(params[:q]) - @prices = @search.result + @master_prices = @search.result .currently_valid + .for_master + .order(:variant_id, :country_iso, :currency) + .page(params[:page]).per(Spree::Config.admin_variants_per_page) + @variant_prices = @search.result + .currently_valid + .for_variant .order(:variant_id, :country_iso, :currency) .page(params[:page]).per(Spree::Config.admin_variants_per_page) end From 01b6f85c5fb639d5c3b7fb1d348734ba5592cb66 Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Fri, 7 Oct 2016 14:06:15 -0700 Subject: [PATCH 03/11] Expand price table view to show prices for master seperately --- .../views/spree/admin/prices/_table.html.erb | 46 +++++++++++++++++-- .../views/spree/admin/prices/index.html.erb | 2 +- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/backend/app/views/spree/admin/prices/_table.html.erb b/backend/app/views/spree/admin/prices/_table.html.erb index f1e4a36e6aa..c768ca664be 100644 --- a/backend/app/views/spree/admin/prices/_table.html.erb +++ b/backend/app/views/spree/admin/prices/_table.html.erb @@ -1,4 +1,41 @@ -<%= paginate prices, theme: "solidus_admin" %> +<%= paginate variant_prices, theme: "solidus_admin" %> + +
+
+ + + + + + + + + + + + + <% master_prices.each do |price| %> + <%= cycle('odd', 'even')%>"> + + + + + + <% end %> +
Master Variant
<%= Spree::Price.human_attribute_name(:country) %><%= Spree::Price.human_attribute_name(:currency) %><%= Spree::Price.human_attribute_name(:amount) %>
<%= price.display_country %><%= price.currency %><%= price.money.to_html %> + <% if can?(:update, price) %> + <%= link_to_edit(price, :no_text => true) unless price.deleted? %> + <% end %> + <% if can?(:destroy, price) %> +   + <%= link_to_delete(price, :no_text => true) unless price.deleted? %> + <% end %> +
+
+
+ +
+
@@ -10,12 +47,11 @@ - - <% prices.each do |price| %> + <% variant_prices.each do |price| %> <%= cycle('odd', 'even')%>"> -
<%= price.variant.descriptive_name %><%= price.display_country %> + <%= price.display_country %> <%= price.currency %> <%= price.money.to_html %> @@ -32,4 +68,4 @@
-<%= paginate prices, theme: "solidus_admin" %> +<%= paginate variant_prices, theme: "solidus_admin" %> diff --git a/backend/app/views/spree/admin/prices/index.html.erb b/backend/app/views/spree/admin/prices/index.html.erb index cbe616df177..5d476af6985 100644 --- a/backend/app/views/spree/admin/prices/index.html.erb +++ b/backend/app/views/spree/admin/prices/index.html.erb @@ -71,4 +71,4 @@ <% end %> -<%= render 'table', prices: @prices %> +<%= render 'table', variant_prices: @variant_prices, master_prices: @master_prices %> From 31f43b09bd9fff5719633fded6f24e32caa7d6cd Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Fri, 7 Oct 2016 15:51:37 -0700 Subject: [PATCH 04/11] Add I18n variables for static text in price table --- backend/app/views/spree/admin/prices/_table.html.erb | 2 +- core/config/locales/en.yml | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/backend/app/views/spree/admin/prices/_table.html.erb b/backend/app/views/spree/admin/prices/_table.html.erb index c768ca664be..91cdedb3702 100644 --- a/backend/app/views/spree/admin/prices/_table.html.erb +++ b/backend/app/views/spree/admin/prices/_table.html.erb @@ -5,7 +5,7 @@ - + diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index c5a12e492b5..0364fa008b5 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -1177,6 +1177,8 @@ en: hints: spree/price: country: "This determines in what country the price is valid.
Default: Any Country" + master_variant: "Changing master variant prices will not change variant prices below, but will be used to populate all new variants" + options: "These options are used to create variants in the variants table. They can be changed in the variants tab" spree/product: promotionable: "This determines whether or not promotions can apply to this product.
Default: Checked" shipping_category: "This determines what kind of shipping this product requires.
Default: Default" @@ -1337,6 +1339,7 @@ en: manual_intervention_required: Manual intervention required manage_variants: Manage Variants master_price: Master Price + master_variant: Master Variant match_choices: all: All none: None @@ -1992,6 +1995,7 @@ en: value: Value variant: Variant variant_placeholder: Choose a variant + variant_pricing: Variant Pricing variant_properties: Variant Properties variant_search: Variant Search variant_search_placeholder: "SKU or Option Value" From 106f1e6425e5a2c1fc24fedc534f0ce51efdb869 Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Fri, 7 Oct 2016 15:52:05 -0700 Subject: [PATCH 05/11] Add table view for option types and values to admin prices view --- .../views/spree/admin/prices/_table.html.erb | 20 ++++++++++++++++++- .../views/spree/admin/prices/index.html.erb | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/backend/app/views/spree/admin/prices/_table.html.erb b/backend/app/views/spree/admin/prices/_table.html.erb index 91cdedb3702..6c93ca06368 100644 --- a/backend/app/views/spree/admin/prices/_table.html.erb +++ b/backend/app/views/spree/admin/prices/_table.html.erb @@ -33,7 +33,25 @@
Master Variant<%= I18n.t(:master_variant, scope: :spree) %> <%= admin_hint I18n.t(:master_variant, scope: :spree), I18n.t(:master_variant, scope: [:spree, :hints, "spree/price"]) %>
<%= Spree::Price.human_attribute_name(:country) %>
- + + + + + + + + + + + + <% option_types.each do |option_type| %> + + + + + <% end %> + +
<%= I18n.t(:options, scope: :spree) %> <%= admin_hint I18n.t(:options, scope: :spree), I18n.t(:options, scope: [:spree, :hints, "spree/price"]) %>
<%= Spree::Price.human_attribute_name(:option_type) %><%= Spree::Price.human_attribute_name(:option_values) %>
<%= option_type.presentation %><%= option_type.option_values.map {|option_value| option_value.presentation}.join(", ") %>
diff --git a/backend/app/views/spree/admin/prices/index.html.erb b/backend/app/views/spree/admin/prices/index.html.erb index 5d476af6985..85e252f1711 100644 --- a/backend/app/views/spree/admin/prices/index.html.erb +++ b/backend/app/views/spree/admin/prices/index.html.erb @@ -71,4 +71,4 @@ <% end %> -<%= render 'table', variant_prices: @variant_prices, master_prices: @master_prices %> +<%= render 'table', variant_prices: @variant_prices, master_prices: @master_prices, option_types: @product.option_types %> From cd5f6f923cc0817c37f2f7db30e560a95473b12a Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Fri, 7 Oct 2016 15:24:09 -0700 Subject: [PATCH 06/11] Update prices controller spec to reflect new assigns --- .../controllers/spree/admin/prices_controller_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/backend/spec/controllers/spree/admin/prices_controller_spec.rb b/backend/spec/controllers/spree/admin/prices_controller_spec.rb index 71fabeb5a61..2f1f5f396af 100644 --- a/backend/spec/controllers/spree/admin/prices_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/prices_controller_spec.rb @@ -16,7 +16,8 @@ it 'assigns usable instance variables' do subject expect(assigns(:search)).to be_a(Ransack::Search) - expect(assigns(:prices)).to eq(product.prices) + expect(assigns(:variant_prices)).to eq(product.prices.for_variant) + expect(assigns(:master_prices)).to eq(product.prices.for_master) expect(assigns(:product)).to eq(product) end end @@ -32,8 +33,9 @@ it 'assigns usable instance variables' do subject expect(assigns(:search)).to be_a(Ransack::Search) - expect(assigns(:prices)).to eq(product.prices) - expect(assigns(:prices)).to include(variant.default_price) + expect(assigns(:variant_prices)).to eq(product.prices.for_variant) + expect(assigns(:master_prices)).to eq(product.prices.for_master) + expect(assigns(:variant_prices)).to include(variant.default_price) expect(assigns(:product)).to eq(product) end end From edcd253d18c658e1b659438a3e7ffd9001ee94c2 Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Fri, 7 Oct 2016 15:24:46 -0700 Subject: [PATCH 07/11] Update pricing feature spec to reflect new assigns --- backend/spec/features/admin/products/pricing_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/spec/features/admin/products/pricing_spec.rb b/backend/spec/features/admin/products/pricing_spec.rb index c72e9a8366e..d760c599fd9 100644 --- a/backend/spec/features/admin/products/pricing_spec.rb +++ b/backend/spec/features/admin/products/pricing_spec.rb @@ -30,12 +30,11 @@ expect(page).to have_content("Prices") end - within('table.prices') do + within('table.master_prices') do expect(page).to have_content("$19.99") expect(page).to have_content("USD") expect(page).to have_content("34.56 ₽") expect(page).to have_content("RUB") - expect(page).to have_content("Master") expect(page).to have_content("Any Country") expect(page).to have_content("Germany") end From 431971c55bee06b140c99629af27567b492c7f8d Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Tue, 11 Oct 2016 09:50:25 -0700 Subject: [PATCH 08/11] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7291a983853..20291d714d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,9 @@ * The `lastname` field on `Address` is now optional. [#1369](https://github.com/solidusio/solidus/pull/1369) +* The admin prices listings page now shows master and variant prices + seperately. This changes `@prices` to `@master_prices` and `@variant_prices` in prices_controller + * Removals * Removed deprecated method `Spree::TaxRate.adjust` (not to be confused with From 471979fc4b80df75f375377378dffe8986ff3478 Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Tue, 11 Oct 2016 14:06:07 -0700 Subject: [PATCH 09/11] Add fieldset legends to sections of admin price page --- .../views/spree/admin/prices/_table.html.erb | 139 +++++++++--------- 1 file changed, 71 insertions(+), 68 deletions(-) diff --git a/backend/app/views/spree/admin/prices/_table.html.erb b/backend/app/views/spree/admin/prices/_table.html.erb index 6c93ca06368..70b1b8dd32f 100644 --- a/backend/app/views/spree/admin/prices/_table.html.erb +++ b/backend/app/views/spree/admin/prices/_table.html.erb @@ -2,20 +2,75 @@
- - - - - - - - - - - - - <% master_prices.each do |price| %> +
+ <%= I18n.t(:master_variant, scope: :spree) %> <%= admin_hint I18n.t(:master_variant, scope: :spree), I18n.t(:master_variant, scope: [:spree, :hints, "spree/price"]) %> +
<%= I18n.t(:master_variant, scope: :spree) %> <%= admin_hint I18n.t(:master_variant, scope: :spree), I18n.t(:master_variant, scope: [:spree, :hints, "spree/price"]) %>
<%= Spree::Price.human_attribute_name(:country) %><%= Spree::Price.human_attribute_name(:currency) %><%= Spree::Price.human_attribute_name(:amount) %>
+ + + + + + + + + <% master_prices.each do |price| %> + <%= cycle('odd', 'even')%>"> + + + + + + <% end %> +
<%= Spree::Price.human_attribute_name(:country) %><%= Spree::Price.human_attribute_name(:currency) %><%= Spree::Price.human_attribute_name(:amount) %>
<%= price.display_country %><%= price.currency %><%= price.money.to_html %> + <% if can?(:update, price) %> + <%= link_to_edit(price, :no_text => true) unless price.deleted? %> + <% end %> + <% if can?(:destroy, price) %> +   + <%= link_to_delete(price, :no_text => true) unless price.deleted? %> + <% end %> +
+ +
+
+
+ <%= I18n.t(:options, scope: :spree) %> <%= admin_hint I18n.t(:options, scope: :spree), I18n.t(:options, scope: [:spree, :hints, "spree/price"]) %> + + + + + + + + + <% option_types.each do |option_type| %> + + + + + <% end %> + +
<%= Spree::Price.human_attribute_name(:option_type) %><%= Spree::Price.human_attribute_name(:option_values) %>
<%= option_type.presentation %><%= option_type.option_values.map {|option_value| option_value.presentation}.join(", ") %>
+
+
+
+ +
+ <%= I18n.t(:variant_pricing, scope: :spree) %> + + + + + + + + + + + + <% variant_prices.each do |price| %> <%= cycle('odd', 'even')%>"> + @@ -30,60 +85,8 @@ <% end %> -
<%= Spree::Variant.model_name.human %> <%= Spree::Price.human_attribute_name(:country) %><%= Spree::Price.human_attribute_name(:currency) %><%= Spree::Price.human_attribute_name(:amount) %>
<%= price.variant.descriptive_name %> <%= price.display_country %> <%= price.currency %> <%= price.money.to_html %>
- -
- - - - - - - - - - - - <% option_types.each do |option_type| %> - - - - - <% end %> - -
<%= I18n.t(:options, scope: :spree) %> <%= admin_hint I18n.t(:options, scope: :spree), I18n.t(:options, scope: [:spree, :hints, "spree/price"]) %>
<%= Spree::Price.human_attribute_name(:option_type) %><%= Spree::Price.human_attribute_name(:option_values) %>
<%= option_type.presentation %><%= option_type.option_values.map {|option_value| option_value.presentation}.join(", ") %>
-
- - - - - - - - - - - - - - <% variant_prices.each do |price| %> - <%= cycle('odd', 'even')%>"> - - - - - - - <% end %> - -
<%= Spree::Variant.model_name.human %> <%= Spree::Price.human_attribute_name(:country) %><%= Spree::Price.human_attribute_name(:currency) %><%= Spree::Price.human_attribute_name(:amount) %>
<%= price.variant.descriptive_name %><%= price.display_country %><%= price.currency %><%= price.money.to_html %> - <% if can?(:update, price) %> - <%= link_to_edit(price, :no_text => true) unless price.deleted? %> - <% end %> - <% if can?(:destroy, price) %> -   - <%= link_to_delete(price, :no_text => true) unless price.deleted? %> - <% end %> -
+ + +
<%= paginate variant_prices, theme: "solidus_admin" %> From a793e5efc3b12855d6ff99872fc2366c7be6da40 Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Mon, 7 Nov 2016 14:37:31 -0800 Subject: [PATCH 10/11] Remove options table view from admin prices view --- .../views/spree/admin/prices/_table.html.erb | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/backend/app/views/spree/admin/prices/_table.html.erb b/backend/app/views/spree/admin/prices/_table.html.erb index 70b1b8dd32f..941d6fd6504 100644 --- a/backend/app/views/spree/admin/prices/_table.html.erb +++ b/backend/app/views/spree/admin/prices/_table.html.erb @@ -32,27 +32,6 @@ -
-
- <%= I18n.t(:options, scope: :spree) %> <%= admin_hint I18n.t(:options, scope: :spree), I18n.t(:options, scope: [:spree, :hints, "spree/price"]) %> - - - - - - - - - <% option_types.each do |option_type| %> - - - - - <% end %> - -
<%= Spree::Price.human_attribute_name(:option_type) %><%= Spree::Price.human_attribute_name(:option_values) %>
<%= option_type.presentation %><%= option_type.option_values.map {|option_value| option_value.presentation}.join(", ") %>
-
-
From bcff2f004f0e362b1bbabe29502d5abf0a8d99a1 Mon Sep 17 00:00:00 2001 From: Graham Bouvier Date: Mon, 7 Nov 2016 14:38:17 -0800 Subject: [PATCH 11/11] Render variants only if some exist prices on admin prices page --- .../prices/_master_variant_table.html.erb | 41 +++++++++++++++++++ .../views/spree/admin/prices/_table.html.erb | 34 --------------- .../views/spree/admin/prices/index.html.erb | 3 +- 3 files changed, 43 insertions(+), 35 deletions(-) create mode 100644 backend/app/views/spree/admin/prices/_master_variant_table.html.erb diff --git a/backend/app/views/spree/admin/prices/_master_variant_table.html.erb b/backend/app/views/spree/admin/prices/_master_variant_table.html.erb new file mode 100644 index 00000000000..bd1a2ba8e42 --- /dev/null +++ b/backend/app/views/spree/admin/prices/_master_variant_table.html.erb @@ -0,0 +1,41 @@ +
+
+
"> + <% if variants %> + <%= I18n.t(:master_variant, scope: :spree) %> <%= admin_hint I18n.t(:master_variant, scope: :spree), I18n.t(:master_variant, scope: [:spree, :hints, "spree/price"]) %> + <% end %> + + + + + + + + + + + + + + + + <% master_prices.each do |price| %> + <%= cycle('odd', 'even')%>"> + + + + + + <% end %> +
<%= Spree::Price.human_attribute_name(:country) %><%= Spree::Price.human_attribute_name(:currency) %><%= Spree::Price.human_attribute_name(:amount) %>
<%= price.display_country %><%= price.currency %><%= price.money.to_html %> + <% if can?(:update, price) %> + <%= link_to_edit(price, :no_text => true) unless price.deleted? %> + <% end %> + <% if can?(:destroy, price) %> +   + <%= link_to_delete(price, :no_text => true) unless price.deleted? %> + <% end %> +
+
+
+
diff --git a/backend/app/views/spree/admin/prices/_table.html.erb b/backend/app/views/spree/admin/prices/_table.html.erb index 941d6fd6504..7cf7a7f7e83 100644 --- a/backend/app/views/spree/admin/prices/_table.html.erb +++ b/backend/app/views/spree/admin/prices/_table.html.erb @@ -1,39 +1,5 @@ <%= paginate variant_prices, theme: "solidus_admin" %> -
-
-
- <%= I18n.t(:master_variant, scope: :spree) %> <%= admin_hint I18n.t(:master_variant, scope: :spree), I18n.t(:master_variant, scope: [:spree, :hints, "spree/price"]) %> - - - - - - - - - - <% master_prices.each do |price| %> - <%= cycle('odd', 'even')%>"> - - - - - - <% end %> -
<%= Spree::Price.human_attribute_name(:country) %><%= Spree::Price.human_attribute_name(:currency) %><%= Spree::Price.human_attribute_name(:amount) %>
<%= price.display_country %><%= price.currency %><%= price.money.to_html %> - <% if can?(:update, price) %> - <%= link_to_edit(price, :no_text => true) unless price.deleted? %> - <% end %> - <% if can?(:destroy, price) %> -   - <%= link_to_delete(price, :no_text => true) unless price.deleted? %> - <% end %> -
-
-
-
-
<%= I18n.t(:variant_pricing, scope: :spree) %> diff --git a/backend/app/views/spree/admin/prices/index.html.erb b/backend/app/views/spree/admin/prices/index.html.erb index 85e252f1711..95a70f2839f 100644 --- a/backend/app/views/spree/admin/prices/index.html.erb +++ b/backend/app/views/spree/admin/prices/index.html.erb @@ -71,4 +71,5 @@ <% end %> -<%= render 'table', variant_prices: @variant_prices, master_prices: @master_prices, option_types: @product.option_types %> +<%= render 'master_variant_table', master_prices: @master_prices, variants: @product.variants.any? %> +<%= render 'table', variant_prices: @variant_prices if @product.variants.any? %>