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

Copy all prices from master over children variants #3994

Closed
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
2 changes: 1 addition & 1 deletion api/app/controllers/spree/api/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def product_scope
end

def variants_associations
[{ option_values: :option_type }, :default_price, :images]
[{ option_values: :option_type }, :prices, :images]
end

def product_includes
Expand Down
2 changes: 1 addition & 1 deletion api/app/controllers/spree/api/shipments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def mine_includes
},
variant: {
product: {},
default_price: {},
prices: {},
option_values: {
option_type: {}
}
Expand Down
2 changes: 1 addition & 1 deletion api/app/controllers/spree/api/taxons_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def products
# Products#index does not do the sorting.
taxon = Spree::Taxon.find(params[:id])
@products = paginate(taxon.products.ransack(params[:q]).result)
@products = @products.includes(master: :default_price)
@products = @products.includes(master: :prices)

if params[:simple]
@exclude_data = {
Expand Down
3 changes: 2 additions & 1 deletion api/app/controllers/spree/api/variants_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class VariantsController < Spree::Api::BaseController
def create
authorize! :create, Variant
@variant = scope.new(variant_params)
@variant.inherit_prices unless @variant.prices.any?
if @variant.save
respond_with(@variant, status: 201, default_template: :show)
else
Expand Down Expand Up @@ -83,7 +84,7 @@ def variant_params
end

def include_list
[{ option_values: :option_type }, :product, :default_price, :images, { stock_items: :stock_location }]
[{ option_values: :option_type }, :product, :prices, :images, { stock_items: :stock_location }]
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion api/spec/requests/spree/api/products_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ module Spree

it "gets a single product" do
product.master.images.create!(attachment: image("blank.jpg"))
product.variants.create!
product.variants.create!(price: 50)
product.variants.first.images.create!(attachment: image("blank.jpg"))
product.set_property("spree", "rocks")
product.taxons << create(:taxon)
Expand Down
27 changes: 27 additions & 0 deletions api/spec/requests/spree/api/variants_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,33 @@ module Spree
expect(variant.product.variants.count).to eq(1)
end

it "inherites prices from master" do
product = create(:product, price: 50)

post spree.api_product_variants_path(product), params: { variant: { sku: "12345" } }

variant = Spree::Variant.find(json_response["id"])
expect(variant.price).to eq(50)
end

it "can override prices inherited from master through nested attributes" do
product = create(:product, price: 50)

post spree.api_product_variants_path(product), params: { variant: { sku: "12345", prices_attributes: [amount: 25, currency: 'USD'] } }

variant = Spree::Variant.find(json_response["id"])
expect(variant.price).to eq(25)
end

it "can override prices inherited from master through default price attribute" do
product = create(:product, price: 50)

post spree.api_product_variants_path(product), params: { variant: { sku: "12345", price: 25 } }

variant = Spree::Variant.find(json_response["id"])
expect(variant.price).to eq(25)
end

it "creates new variants with nested option values" do
option_values = create_list(:option_value, 2)
expect do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,9 @@
}
}
}

.field_with_errors {
flex: 1 1 auto;
width: 1%;
}
}
4 changes: 2 additions & 2 deletions backend/app/controllers/spree/admin/prices_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ def index

@search = @product.prices.accessible_by(current_ability, :index).ransack(params[:q])
@master_prices = @search.result
.currently_valid
.prioritized_for_default
.for_master
.order(:variant_id, :country_iso, :currency)
.page(params[:page]).per(Spree::Config.admin_variants_per_page)
@variant_prices = @search.result
.currently_valid
.prioritized_for_default
.for_variant
.order(:variant_id, :country_iso, :currency)
.page(params[:page]).per(Spree::Config.admin_variants_per_page)
Expand Down
2 changes: 1 addition & 1 deletion backend/app/controllers/spree/admin/products_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def update_before
end

def product_includes
[:variant_images, { variants: [:images], master: [:images, :default_price] }]
[:variant_images, { variants: [:images], master: [:images, :prices] }]
end

def clone_object_url(resource)
Expand Down
5 changes: 2 additions & 3 deletions backend/app/controllers/spree/admin/variants_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ class VariantsController < ResourceController
def new_before
@object.attributes = @object.product.master.attributes.except('id', 'created_at', 'deleted_at',
'sku', 'is_master')
# Shallow Clone of the default price to populate the price field.
@object.default_price = @object.product.master.default_price.clone
@object.inherit_prices
end

def collection
Expand All @@ -35,7 +34,7 @@ def load_data
end

def variant_includes
[{ option_values: :option_type }, :default_price]
[{ option_values: :option_type }, :prices]
end

def redirect_on_empty_option_values
Expand Down
2 changes: 1 addition & 1 deletion backend/app/views/spree/admin/products/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
<%= render 'spree/admin/shared/image', image: product.gallery.images.first, size: :mini %>
</td>
<td><%= link_to product.try(:name), edit_admin_product_path(product) %></td>
<td class="align-right"><%= product.display_price.to_html %></td>
<td class="align-right"><%= product.display_price&.to_html %></td>
<td class="actions" data-hook="admin_products_index_row_actions">
<%= link_to_edit product, no_text: true, class: 'edit' if can?(:edit, product) && !product.deleted? %>
&nbsp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
<% currency_attr ||= :currency %>
<% currency ||= nil %>
<% required ||= nil %>
<% extra_attrs ||= {} %>

<div class="input-group number-with-currency <%= "number-with-currency-with-select" unless currency %> js-number-with-currency">
<div class="input-group-prepend">
<span class="input-group-text number-with-currency-symbol"></span>
</div>
<%= f.text_field amount_attr, value: number_to_currency(f.object.public_send(amount_attr), unit: '', delimiter: ''), class: 'form-control number-with-currency-amount', required: required %>
<%= f.text_field amount_attr, extra_attrs.merge(value: number_to_currency(f.object.public_send(amount_attr), unit: '', delimiter: ''), class: 'form-control number-with-currency-amount', required: required) %>
<% if currency %>
<div class="input-group-append">
<span class="input-group-text number-with-currency-addon" data-currency="<%= currency %>">
Expand Down
10 changes: 7 additions & 3 deletions backend/app/views/spree/admin/variants/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,13 @@

<div class="row">
<div class="col-3">
<div class="field" data-hook="price">
<%= f.label :price %>
<%= render "spree/admin/shared/number_with_currency", f: f, amount_attr: :price, currency: @variant.find_or_build_default_price.currency %>
<div class="field">
<fieldset>
<legend><%= t("spree.helpers.prices.legend") %></legend>
<%= f.fields_for :prices do |p_form| %>
<%= render "prices_form", f: p_form, currency: p_form.object.currency %>
<% end %>
</fieldset>
</div>
</div>

Expand Down
4 changes: 4 additions & 0 deletions backend/app/views/spree/admin/variants/_prices_form.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="mb-3">
<%= render "spree/admin/shared/number_with_currency", f: f, amount_attr: :amount, currency: currency, extra_attrs: { "aria-label" => t("spree.helpers.prices.amount_in", currency: currency) } %>
<%= f.hidden_field :currency, value: currency %>
</div>
1 change: 1 addition & 0 deletions backend/spec/features/admin/products/pricing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
let!(:other_price) { product.master.prices.create(amount: 34.56, currency: "EUR") }

it 'has a working edit page' do
product.reload
within "#spree_price_#{product.master.prices.first.id}" do
click_icon :edit
end
Expand Down
4 changes: 3 additions & 1 deletion backend/spec/features/admin/products/variant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
product.options.each do |option|
create(:option_value, option_type: option.option_type)
end
create(:price, amount: 1.75, currency: 'EUR', variant: product.master)

visit spree.admin_path
click_nav "Products"
within_row(1) { click_icon :edit }
click_link "Variants"
click_on "New Variant"
expect(page).to have_field('variant_price', with: "1.99")
expect(page).to have_selector('input[value="1.99"][aria-label="Price in USD"]')
expect(page).to have_selector('input[value="1.75"][aria-label="Price in EUR"]')
expect(page).to have_field('variant_cost_price', with: "1.00")
expect(page).to have_field('variant_weight', with: "2.50")
expect(page).to have_field('variant_height', with: "3.00")
Expand Down
61 changes: 51 additions & 10 deletions core/app/models/concerns/spree/default_price.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,61 @@ module DefaultPrice
extend ActiveSupport::Concern

included do
has_one :default_price,
-> { with_discarded.currently_valid.with_default_attributes },
class_name: 'Spree::Price',
inverse_of: :variant,
dependent: :destroy,
autosave: true
delegate :display_price, :display_amount, :price, to: :default_price, allow_nil: true
delegate :price=, to: :default_price_or_build

# @see Spree::Variant::PricingOptions.default_price_attributes
def self.default_pricing
Spree::Config.default_pricing_options.desired_attributes
end
end

# Returns `#prices` prioritized for being considered as default price
#
# @return [ActiveRecord::Relation] relation of {Spree::Price}
def prioritized_for_default_prices
prices.prioritized_for_default
end

def find_or_build_default_price
default_price || build_default_price(Spree::Config.default_pricing_options.desired_attributes)
# Returns {#default_price} or builds it from {Spree::Price.default_pricing}
#
# @return [Spree::Price, nil]
# @see Spree::Price.default_pricing
def default_price_or_build
default_price ||
prices.build(self.class.default_pricing)
end

delegate :display_price, :display_amount, :price, to: :find_or_build_default_price
delegate :price=, to: :find_or_build_default_price
# Select from {#prices} the one to be considered as the default
#
# This method works with the in-memory association, so non-persisted prices
# are taken into account. Discarded prices are also considered.
#
# A price is a candidate to be considered as the default when it meets
# {Spree::Variant.default_pricing} criteria. When more than one candidate is
# found, non-persisted records take preference. When more than one persisted
# candidate exists, the one most recently created is taken or, in case of
# race condition, the one with higher id.
#
# @return [Spree::Price, nil]
# @see Spree::Price.default_pricing
def default_price
candidates = discarded? ? (prices + prices.with_discarded).to_a.uniq : prices.to_a
candidates.select do |price|
price.
attributes.
values_at(
*self.class.default_pricing.keys.map(&:to_s)
) == self.class.default_pricing.values
end.min do |a, b|
[b, a].map do |i|
[
i.created_at || Time.zone.now,
i.id || Float::INFINITY
]
end.reduce { |x, y| x <=> y }
end
end

def has_default_price?
default_price.present? && !default_price.discarded?
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/price.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Price < Spree::Base
validates :currency, inclusion: { in: ::Money::Currency.all.map(&:iso_code), message: :invalid_code }
validates :country, presence: true, unless: -> { for_any_country? }

scope :currently_valid, -> { order(Arel.sql("country_iso IS NULL")).order(updated_at: :DESC, id: :DESC) }
scope :prioritized_for_default, -> { order(Arel.sql("country_iso IS NULL")).order(created_at: :DESC, id: :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) }
Expand Down
11 changes: 11 additions & 0 deletions core/app/models/spree/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ class Product < Spree::Base
has_many :line_items, through: :variants_including_master
has_many :orders, through: :line_items

scope :sort_by_master_default_price_amount_asc, -> {
with_default_price.order('spree_prices.amount ASC')
}
scope :sort_by_master_default_price_amount_desc, -> {
with_default_price.order('spree_prices.amount DESC')
}
scope :with_default_price, -> {
left_joins(master: :prices)
.where(master: { spree_prices: Spree::Config.default_pricing_options.desired_attributes })
}

def find_or_build_master
master || build_master
end
Expand Down
10 changes: 5 additions & 5 deletions core/app/models/spree/product/scopes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,25 @@ def self.property_conditions(property)
scope :descend_by_name, -> { order(name: :desc) }

add_search_scope :ascend_by_master_price do
joins(master: :default_price).select('spree_products.* , spree_prices.amount')
joins(master: :prices).select('spree_products.* , spree_prices.amount')
.order(Spree::Price.arel_table[:amount].asc)
end

add_search_scope :descend_by_master_price do
joins(master: :default_price).select('spree_products.* , spree_prices.amount')
joins(master: :prices).select('spree_products.* , spree_prices.amount')
.order(Spree::Price.arel_table[:amount].desc)
end

add_search_scope :price_between do |low, high|
joins(master: :default_price).where(Price.table_name => { amount: low..high })
joins(master: :prices).where(Price.table_name => { amount: low..high })
end

add_search_scope :master_price_lte do |price|
joins(master: :default_price).where("#{price_table_name}.amount <= ?", price)
joins(master: :prices).where("#{price_table_name}.amount <= ?", price)
end

add_search_scope :master_price_gte do |price|
joins(master: :default_price).where("#{price_table_name}.amount >= ?", price)
joins(master: :prices).where("#{price_table_name}.amount >= ?", price)
end

# This scope selects products in taxon AND all its descendants
Expand Down
Loading