mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-03-14 04:04:23 +00:00
Merge pull request #6899 from Matt-Yorkley/cost-price-dead-code
Remove cost_price and other dead code
This commit is contained in:
@@ -22,7 +22,7 @@ module Spree
|
||||
|
||||
def variant_attributes
|
||||
[:id, :name, :sku, :price, :weight, :height, :width, :depth,
|
||||
:is_master, :cost_price, :permalink]
|
||||
:is_master, :permalink]
|
||||
end
|
||||
|
||||
def image_attributes
|
||||
|
||||
@@ -105,7 +105,6 @@ module Spree
|
||||
return unless variant
|
||||
|
||||
self.price = variant.price if price.nil?
|
||||
self.cost_price = variant.cost_price if cost_price.nil?
|
||||
self.currency = variant.currency if currency.nil?
|
||||
end
|
||||
|
||||
|
||||
@@ -67,10 +67,8 @@ module Spree
|
||||
has_many :stock_items, through: :variants
|
||||
|
||||
delegate_belongs_to :master, :sku, :price, :currency, :display_amount, :display_price, :weight,
|
||||
:height, :width, :depth, :is_master, :default_price?, :cost_currency,
|
||||
:height, :width, :depth, :is_master, :cost_currency,
|
||||
:price_in, :amount_in, :unit_value, :unit_description
|
||||
delegate_belongs_to :master, :cost_price if Variant.table_exists? &&
|
||||
Variant.column_names.include?('cost_price')
|
||||
delegate :images_attributes=, :display_as=, to: :master
|
||||
|
||||
after_create :set_master_variant_defaults
|
||||
|
||||
@@ -47,13 +47,12 @@ module Spree
|
||||
has_many :variant_overrides
|
||||
has_many :inventory_items
|
||||
|
||||
localize_number :price, :cost_price, :weight
|
||||
localize_number :price, :weight
|
||||
|
||||
validate :check_price
|
||||
validates :price, numericality: { greater_than_or_equal_to: 0 },
|
||||
presence: true,
|
||||
if: proc { Spree::Config[:require_master_price] }
|
||||
validates :cost_price, numericality: { greater_than_or_equal_to: 0, allow_nil: true }
|
||||
|
||||
validates :unit_value, presence: true, if: ->(variant) {
|
||||
%w(weight volume).include?(variant.product.andand.variant_unit)
|
||||
@@ -168,15 +167,6 @@ module Spree
|
||||
OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle).fees_by_type_for self
|
||||
end
|
||||
|
||||
# returns number of units currently on backorder for this variant.
|
||||
def on_backorder
|
||||
inventory_units.with_state('backordered').size
|
||||
end
|
||||
|
||||
def gross_profit
|
||||
cost_price.nil? ? 0 : (price - cost_price)
|
||||
end
|
||||
|
||||
# use deleted? rather than checking the attribute directly. this
|
||||
# allows extensions to override deleted? if they want to provide
|
||||
# their own definition.
|
||||
@@ -184,47 +174,10 @@ module Spree
|
||||
deleted_at
|
||||
end
|
||||
|
||||
def set_option_value(opt_name, opt_value)
|
||||
# no option values on master
|
||||
return if is_master
|
||||
|
||||
option_type = Spree::OptionType.where(name: opt_name).first_or_initialize do |o|
|
||||
o.presentation = opt_name
|
||||
o.save!
|
||||
end
|
||||
|
||||
current_value = option_values.detect { |o| o.option_type.name == opt_name }
|
||||
|
||||
if current_value.nil?
|
||||
# then we have to check to make sure that the product has the option type
|
||||
unless product.option_types.include? option_type
|
||||
product.option_types << option_type
|
||||
product.save
|
||||
end
|
||||
else
|
||||
return if current_value.name == opt_value
|
||||
|
||||
option_values.delete(current_value)
|
||||
end
|
||||
|
||||
option_value = Spree::OptionValue.where(option_type_id: option_type.id,
|
||||
name: opt_value).first_or_initialize do |o|
|
||||
o.presentation = opt_value
|
||||
o.save!
|
||||
end
|
||||
|
||||
option_values << option_value
|
||||
save
|
||||
end
|
||||
|
||||
def option_value(opt_name)
|
||||
option_values.detect { |o| o.option_type.name == opt_name }.try(:presentation)
|
||||
end
|
||||
|
||||
def default_price?
|
||||
!default_price.nil?
|
||||
end
|
||||
|
||||
def price_in(currency)
|
||||
prices.select{ |price| price.currency == currency }.first ||
|
||||
Spree::Price.new(variant_id: id, currency: currency)
|
||||
@@ -234,10 +187,6 @@ module Spree
|
||||
price_in(currency).try(:amount)
|
||||
end
|
||||
|
||||
def name_and_sku
|
||||
"#{name} - #{sku}"
|
||||
end
|
||||
|
||||
# Product may be created with deleted_at already set,
|
||||
# which would make AR's default finder return nil.
|
||||
# This is a stopgap for that little problem.
|
||||
|
||||
@@ -4,7 +4,7 @@ module PermittedAttributes
|
||||
class Product
|
||||
def self.attributes
|
||||
[
|
||||
:id, :name, :description, :supplier_id, :price, :cost_price, :permalink,
|
||||
:id, :name, :description, :supplier_id, :price, :permalink,
|
||||
:variant_unit, :variant_unit_scale, :unit_value, :unit_description, :variant_unit_name,
|
||||
:display_as, :sku, :available_on, :group_buy, :group_buy_unit_size,
|
||||
:taxon_ids, :primary_taxon_id, :tax_category_id, :shipping_category_id,
|
||||
|
||||
@@ -3547,7 +3547,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using
|
||||
new:
|
||||
new_variant: "New Variant"
|
||||
form:
|
||||
cost_price: "Cost Price"
|
||||
sku: "SKU"
|
||||
price: "Price"
|
||||
display_as: "Display As"
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
class RemoveCostPriceFromVariantAndLineItem < ActiveRecord::Migration
|
||||
def up
|
||||
remove_column :spree_variants, :cost_price
|
||||
remove_column :spree_line_items, :cost_price
|
||||
end
|
||||
|
||||
def down
|
||||
add_column :spree_variants, :cost_price, :decimal,
|
||||
precision: 8, scale: 2
|
||||
add_column :spree_line_items, :cost_price, :integer,
|
||||
precision: 8, scale: 2
|
||||
end
|
||||
end
|
||||
@@ -11,7 +11,7 @@
|
||||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema.define(version: 20210203215049) do
|
||||
ActiveRecord::Schema.define(version: 20210216203057) do
|
||||
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "plpgsql"
|
||||
@@ -504,7 +504,6 @@ ActiveRecord::Schema.define(version: 20210203215049) do
|
||||
t.string "currency", limit: 255
|
||||
t.decimal "distribution_fee", precision: 10, scale: 2
|
||||
t.decimal "final_weight_volume", precision: 10, scale: 2
|
||||
t.decimal "cost_price", precision: 10, scale: 2
|
||||
t.integer "tax_category_id"
|
||||
end
|
||||
|
||||
@@ -1069,7 +1068,6 @@ ActiveRecord::Schema.define(version: 20210203215049) do
|
||||
t.datetime "deleted_at"
|
||||
t.boolean "is_master", default: false
|
||||
t.integer "product_id"
|
||||
t.decimal "cost_price", precision: 10, scale: 2
|
||||
t.integer "position"
|
||||
t.string "cost_currency", limit: 255
|
||||
t.float "unit_value", default: 1.0, null: false
|
||||
|
||||
@@ -5,7 +5,6 @@ FactoryBot.define do
|
||||
sequence(:name) { |n| "Product ##{n} - #{Kernel.rand(9999)}" }
|
||||
description { generate(:random_description) }
|
||||
price { 19.99 }
|
||||
cost_price { 17.00 }
|
||||
sku { 'ABC' }
|
||||
available_on { 1.year.ago }
|
||||
deleted_at { nil }
|
||||
|
||||
@@ -5,7 +5,6 @@ FactoryBot.define do
|
||||
|
||||
factory :base_variant, class: Spree::Variant do
|
||||
price { 19.99 }
|
||||
cost_price { 17.00 }
|
||||
sku { SecureRandom.hex }
|
||||
weight { generate(:random_float) }
|
||||
height { generate(:random_float) }
|
||||
|
||||
@@ -39,12 +39,10 @@ module Spree
|
||||
context '#copy_price' do
|
||||
it "copies over a variant's prices" do
|
||||
line_item.price = nil
|
||||
line_item.cost_price = nil
|
||||
line_item.currency = nil
|
||||
line_item.copy_price
|
||||
variant = line_item.variant
|
||||
expect(line_item.price).to eq variant.price
|
||||
expect(line_item.cost_price).to eq variant.cost_price
|
||||
expect(line_item.currency).to eq variant.currency
|
||||
end
|
||||
end
|
||||
|
||||
@@ -20,72 +20,6 @@ module Spree
|
||||
end
|
||||
end
|
||||
|
||||
context "product has other variants" do
|
||||
describe "option value accessors" do
|
||||
before {
|
||||
@multi_variant = create(:variant, product: variant.product)
|
||||
variant.product.reload
|
||||
}
|
||||
|
||||
let(:multi_variant) { @multi_variant }
|
||||
|
||||
it "should set option value" do
|
||||
expect(multi_variant.option_value('media_type')).to be_nil
|
||||
|
||||
multi_variant.set_option_value('media_type', 'DVD')
|
||||
expect(multi_variant.option_value('media_type')).to eq 'DVD'
|
||||
|
||||
multi_variant.set_option_value('media_type', 'CD')
|
||||
expect(multi_variant.option_value('media_type')).to eq 'CD'
|
||||
end
|
||||
|
||||
it "should not duplicate associated option values when set multiple times" do
|
||||
multi_variant.set_option_value('media_type', 'CD')
|
||||
|
||||
expect {
|
||||
multi_variant.set_option_value('media_type', 'DVD')
|
||||
}.to_not change(multi_variant.option_values, :count)
|
||||
|
||||
expect {
|
||||
multi_variant.set_option_value('coolness_type', 'awesome')
|
||||
}.to change(multi_variant.option_values, :count).by(1)
|
||||
end
|
||||
end
|
||||
|
||||
context "product has other variants" do
|
||||
describe "option value accessors" do
|
||||
before {
|
||||
@multi_variant = create(:variant, product: variant.product)
|
||||
variant.product.reload
|
||||
}
|
||||
|
||||
let(:multi_variant) { @multi_variant }
|
||||
|
||||
it "should set option value" do
|
||||
expect(multi_variant.option_value('media_type')).to be_nil
|
||||
|
||||
multi_variant.set_option_value('media_type', 'DVD')
|
||||
expect(multi_variant.option_value('media_type')).to eq 'DVD'
|
||||
|
||||
multi_variant.set_option_value('media_type', 'CD')
|
||||
expect(multi_variant.option_value('media_type')).to eq 'CD'
|
||||
end
|
||||
|
||||
it "should not duplicate associated option values when set multiple times" do
|
||||
multi_variant.set_option_value('media_type', 'CD')
|
||||
|
||||
expect {
|
||||
multi_variant.set_option_value('media_type', 'DVD')
|
||||
}.to_not change(multi_variant.option_values, :count)
|
||||
|
||||
expect {
|
||||
multi_variant.set_option_value('coolness_type', 'awesome')
|
||||
}.to change(multi_variant.option_values, :count).by(1)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "price parsing" do
|
||||
before(:each) do
|
||||
I18n.locale = I18n.default_locale
|
||||
@@ -752,7 +686,7 @@ module Spree
|
||||
context "extends LocalizedNumber" do
|
||||
subject! { build_stubbed(:variant) }
|
||||
|
||||
it_behaves_like "a model using the LocalizedNumber module", [:price, :cost_price, :weight]
|
||||
it_behaves_like "a model using the LocalizedNumber module", [:price, :weight]
|
||||
end
|
||||
|
||||
context "in a circular order cycle setup" do
|
||||
@@ -773,7 +707,7 @@ module Spree
|
||||
end
|
||||
|
||||
it "saves without infinite loop" do
|
||||
expect(variant1.update(cost_price: 1)).to be_truthy
|
||||
expect(variant1.update(price: 1)).to be_truthy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user