mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-01-24 20:36:49 +00:00
Merge pull request #11221 from Matt-Yorkley/product-shipping-category
[Product Refactor] Shipping Category
This commit is contained in:
@@ -27,7 +27,7 @@ module Spree
|
||||
end
|
||||
|
||||
def new
|
||||
@object.shipping_category = DefaultShippingCategory.find_or_create
|
||||
@object.shipping_category_id = DefaultShippingCategory.find_or_create.id
|
||||
end
|
||||
|
||||
def edit
|
||||
|
||||
@@ -15,6 +15,7 @@ module Spree
|
||||
|
||||
def new
|
||||
@url_filters = ::ProductFilters.new.extract(request.query_parameters)
|
||||
@object.shipping_category ||= DefaultShippingCategory.find_or_create
|
||||
end
|
||||
|
||||
def edit
|
||||
@@ -114,6 +115,7 @@ module Spree
|
||||
|
||||
def load_data
|
||||
@tax_categories = TaxCategory.order(:name)
|
||||
@shipping_categories = ShippingCategory.order(:name)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -29,7 +29,6 @@ module ProductImport
|
||||
unit_type: :variant_unit_scale,
|
||||
variant_unit_name: :variant_unit_name,
|
||||
tax_category: :tax_category_id,
|
||||
shipping_category: :shipping_category_id
|
||||
}
|
||||
end
|
||||
|
||||
|
||||
@@ -33,7 +33,6 @@ module Spree
|
||||
searchable_associations :supplier, :properties, :primary_taxon, :variants
|
||||
searchable_scopes :active, :with_properties
|
||||
|
||||
belongs_to :shipping_category, class_name: 'Spree::ShippingCategory'
|
||||
belongs_to :supplier, class_name: 'Enterprise', optional: false, touch: true
|
||||
belongs_to :primary_taxon, class_name: 'Spree::Taxon', optional: false, touch: true
|
||||
|
||||
@@ -52,7 +51,6 @@ module Spree
|
||||
through: :variants
|
||||
|
||||
validates :name, presence: true
|
||||
validates :shipping_category, presence: true
|
||||
|
||||
validates :variant_unit, presence: true
|
||||
validates :unit_value, presence:
|
||||
@@ -71,7 +69,8 @@ module Spree
|
||||
|
||||
# Transient attributes used temporarily when creating a new product,
|
||||
# these values are persisted on the product's variant
|
||||
attr_accessor :price, :display_as, :unit_value, :unit_description, :tax_category_id
|
||||
attr_accessor :price, :display_as, :unit_value, :unit_description, :tax_category_id,
|
||||
:shipping_category_id
|
||||
|
||||
after_create :ensure_standard_variant
|
||||
after_save :update_units
|
||||
@@ -292,6 +291,7 @@ module Spree
|
||||
variant.unit_value = unit_value
|
||||
variant.unit_description = unit_description
|
||||
variant.tax_category_id = tax_category_id
|
||||
variant.shipping_category_id = shipping_category_id
|
||||
variants << variant
|
||||
end
|
||||
|
||||
|
||||
@@ -29,9 +29,9 @@ module Spree
|
||||
|
||||
belongs_to :product, -> { with_deleted }, touch: true, class_name: 'Spree::Product'
|
||||
belongs_to :tax_category, class_name: 'Spree::TaxCategory'
|
||||
belongs_to :shipping_category, class_name: 'Spree::ShippingCategory', optional: false
|
||||
|
||||
delegate_belongs_to :product, :name, :description, :shipping_category_id,
|
||||
:meta_keywords, :shipping_category
|
||||
delegate_belongs_to :product, :name, :description, :meta_keywords
|
||||
|
||||
has_many :inventory_units, inverse_of: :variant
|
||||
has_many :line_items, inverse_of: :variant
|
||||
@@ -77,6 +77,7 @@ module Spree
|
||||
}
|
||||
|
||||
before_validation :set_cost_currency
|
||||
before_validation :ensure_shipping_category
|
||||
before_validation :ensure_unit_value
|
||||
before_validation :update_weight_from_unit_value, if: ->(v) { v.product.present? }
|
||||
|
||||
@@ -248,6 +249,10 @@ module Spree
|
||||
self.unit_value = 1.0
|
||||
end
|
||||
|
||||
def ensure_shipping_category
|
||||
self.shipping_category ||= DefaultShippingCategory.find_or_create
|
||||
end
|
||||
|
||||
def convert_variant_weight_to_decimal
|
||||
self.weight = weight.to_d
|
||||
end
|
||||
|
||||
@@ -3,9 +3,7 @@
|
||||
class OrderAvailableShippingMethods
|
||||
attr_reader :order, :customer
|
||||
|
||||
delegate :distributor,
|
||||
:order_cycle,
|
||||
to: :order
|
||||
delegate :distributor, :order_cycle, to: :order
|
||||
|
||||
def initialize(order, customer = nil)
|
||||
@order, @customer = order, customer
|
||||
@@ -23,7 +21,7 @@ class OrderAvailableShippingMethods
|
||||
return methods unless OpenFoodNetwork::FeatureToggle.enabled?(:match_shipping_categories,
|
||||
distributor&.owner)
|
||||
|
||||
required_category_ids = order.products.pluck(:shipping_category_id).to_set
|
||||
required_category_ids = order.variants.pluck(:shipping_category_id).to_set
|
||||
return methods if required_category_ids.empty?
|
||||
|
||||
methods.select do |method|
|
||||
|
||||
@@ -7,7 +7,7 @@ module PermittedAttributes
|
||||
:id, :name, :description, :supplier_id, :price,
|
||||
:variant_unit, :variant_unit_scale, :unit_value, :unit_description, :variant_unit_name,
|
||||
:display_as, :sku, :group_buy, :group_buy_unit_size,
|
||||
:taxon_ids, :primary_taxon_id, :tax_category_id, :shipping_category_id,
|
||||
:taxon_ids, :primary_taxon_id, :tax_category_id,
|
||||
:meta_keywords, :notes, :inherits_properties,
|
||||
{ product_properties_attributes: [:id, :property_name, :value],
|
||||
variants_attributes: [PermittedAttributes::Variant.attributes],
|
||||
|
||||
@@ -4,7 +4,7 @@ module PermittedAttributes
|
||||
class Variant
|
||||
def self.attributes
|
||||
[
|
||||
:id, :sku, :on_hand, :on_demand,
|
||||
:id, :sku, :on_hand, :on_demand, :shipping_category_id,
|
||||
:price, :unit_value, :unit_description,
|
||||
:display_name, :display_as, :tax_category_id,
|
||||
:weight, :height, :width, :depth
|
||||
|
||||
@@ -38,11 +38,6 @@
|
||||
|
||||
.clear
|
||||
|
||||
= f.field_container :shipping_categories do
|
||||
= f.label :shipping_category_id, t(:shipping_categories)
|
||||
= f.collection_select(:shipping_category_id, @shipping_categories, :id, :name, {}, { :class => 'select2' })
|
||||
= f.error_message_on :shipping_category
|
||||
|
||||
.clear
|
||||
|
||||
%div
|
||||
|
||||
@@ -68,4 +68,8 @@
|
||||
= f.label :tax_category_id, t(:tax_category)
|
||||
= f.collection_select(:tax_category_id, @tax_categories, :id, :name, { include_blank: t(:none) }, { class: 'select2 fullwidth' })
|
||||
|
||||
.field
|
||||
= f.label :shipping_category_id, t(:shipping_categories)
|
||||
= f.collection_select(:shipping_category_id, @shipping_categories, :id, :name, {}, { class: 'select2 fullwidth' })
|
||||
|
||||
.clear
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
class AddShippingCategoryToVariants < ActiveRecord::Migration[7.0]
|
||||
def change
|
||||
add_reference :spree_variants, :shipping_category, foreign_key: { to_table: :spree_shipping_categories }
|
||||
end
|
||||
end
|
||||
15
db/migrate/20230715145329_migrate_shipping_category.rb
Normal file
15
db/migrate/20230715145329_migrate_shipping_category.rb
Normal file
@@ -0,0 +1,15 @@
|
||||
class MigrateShippingCategory < ActiveRecord::Migration[7.0]
|
||||
def up
|
||||
migrate_shipping_category
|
||||
end
|
||||
|
||||
def migrate_shipping_category
|
||||
ActiveRecord::Base.connection.execute(<<-SQL
|
||||
UPDATE spree_variants
|
||||
SET shipping_category_id = spree_products.shipping_category_id
|
||||
FROM spree_products
|
||||
WHERE spree_variants.product_id = spree_products.id
|
||||
SQL
|
||||
)
|
||||
end
|
||||
end
|
||||
@@ -948,7 +948,9 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_09_201542) do
|
||||
t.datetime "created_at", default: -> { "now()" }, null: false
|
||||
t.datetime "updated_at", default: -> { "now()" }, null: false
|
||||
t.bigint "tax_category_id"
|
||||
t.bigint "shipping_category_id"
|
||||
t.index ["product_id"], name: "index_variants_on_product_id"
|
||||
t.index ["shipping_category_id"], name: "index_spree_variants_on_shipping_category_id"
|
||||
t.index ["sku"], name: "index_spree_variants_on_sku"
|
||||
t.index ["tax_category_id"], name: "index_spree_variants_on_tax_category_id"
|
||||
t.check_constraint "unit_value > 0::double precision", name: "positive_unit_value"
|
||||
@@ -1176,6 +1178,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_09_201542) do
|
||||
add_foreign_key "spree_users", "spree_addresses", column: "bill_address_id", name: "spree_users_bill_address_id_fk"
|
||||
add_foreign_key "spree_users", "spree_addresses", column: "ship_address_id", name: "spree_users_ship_address_id_fk"
|
||||
add_foreign_key "spree_variants", "spree_products", column: "product_id", name: "spree_variants_product_id_fk"
|
||||
add_foreign_key "spree_variants", "spree_shipping_categories", column: "shipping_category_id"
|
||||
add_foreign_key "spree_variants", "spree_tax_categories", column: "tax_category_id"
|
||||
add_foreign_key "spree_zone_members", "spree_zones", column: "zone_id", name: "spree_zone_members_zone_id_fk"
|
||||
add_foreign_key "subscription_line_items", "spree_variants", column: "variant_id", name: "subscription_line_items_variant_id_fk"
|
||||
|
||||
@@ -22,7 +22,6 @@ class SuppliedProductBuilder < DfcBuilder
|
||||
description: supplied_product.description,
|
||||
price: 0, # will be in DFC Offer
|
||||
primary_taxon: Spree::Taxon.first, # dummy value until we have a mapping
|
||||
shipping_category: DefaultShippingCategory.find_or_create,
|
||||
).tap do |product|
|
||||
QuantitativeValueBuilder.apply(supplied_product.quantity, product)
|
||||
end
|
||||
|
||||
@@ -23,8 +23,8 @@ module Reporting
|
||||
reflect query.join(association(Spree::Product, :supplier, supplier_alias))
|
||||
end
|
||||
|
||||
def joins_product_shipping_category
|
||||
reflect query.join(association(Spree::Product, :shipping_category))
|
||||
def joins_variant_shipping_category
|
||||
reflect query.join(association(Spree::Variant, :shipping_category))
|
||||
end
|
||||
|
||||
def joins_order_and_distributor
|
||||
|
||||
@@ -26,7 +26,7 @@ module Reporting
|
||||
|
||||
def has_temperature_controlled_items?(order)
|
||||
order.line_items.any? { |line_item|
|
||||
line_item.product.shipping_category&.temperature_controlled
|
||||
line_item.variant.shipping_category&.temperature_controlled
|
||||
}
|
||||
end
|
||||
end
|
||||
|
||||
@@ -19,7 +19,7 @@ module Reporting
|
||||
joins_variant.
|
||||
joins_variant_product.
|
||||
joins_product_supplier.
|
||||
joins_product_shipping_category.
|
||||
joins_variant_shipping_category.
|
||||
selecting(select_fields).
|
||||
ordered_by(ordering_fields)
|
||||
end
|
||||
|
||||
@@ -68,7 +68,6 @@ module SampleData
|
||||
variant_unit: "weight",
|
||||
variant_unit_scale: 1,
|
||||
unit_value: 1,
|
||||
shipping_category: DefaultShippingCategory.find_or_create,
|
||||
tax_category_id: find_or_create_tax_category.id
|
||||
)
|
||||
product = Spree::Product.create_with(params).find_or_create_by!(name: params[:name])
|
||||
|
||||
@@ -102,8 +102,7 @@ describe Api::V0::ProductsController, type: :controller do
|
||||
expect(response.status).to eq(422)
|
||||
expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.")
|
||||
errors = json_response["errors"]
|
||||
expect(errors.keys).to match_array(["name", "primary_taxon", "shipping_category",
|
||||
"supplier", "variant_unit"])
|
||||
expect(errors.keys).to match_array(["name", "primary_taxon", "supplier", "variant_unit"])
|
||||
end
|
||||
|
||||
it "can update a product" do
|
||||
|
||||
@@ -71,7 +71,7 @@ describe Api::V0::ReportsController, type: :controller do
|
||||
"variant" => line_item.full_name,
|
||||
"quantity" => line_item.quantity,
|
||||
"price" => (line_item.quantity * line_item.price).to_s,
|
||||
"temp_controlled" => line_item.product.shipping_category&.temperature_controlled
|
||||
"temp_controlled" => line_item.variant.shipping_category&.temperature_controlled
|
||||
}.
|
||||
merge(dimensions(line_item)).
|
||||
merge(contacts(line_item.order.bill_address))
|
||||
@@ -89,7 +89,7 @@ describe Api::V0::ReportsController, type: :controller do
|
||||
"variant" => line_item.full_name,
|
||||
"quantity" => line_item.quantity,
|
||||
"price" => (line_item.quantity * line_item.price).to_s,
|
||||
"temp_controlled" => line_item.product.shipping_category&.temperature_controlled
|
||||
"temp_controlled" => line_item.variant.shipping_category&.temperature_controlled
|
||||
}.merge(dimensions(line_item))
|
||||
end
|
||||
|
||||
|
||||
@@ -18,8 +18,6 @@ FactoryBot.define do
|
||||
variant_unit_scale { 1 }
|
||||
variant_unit_name { '' }
|
||||
|
||||
shipping_category { DefaultShippingCategory.find_or_create }
|
||||
|
||||
# ensure stock item will be created for this products master
|
||||
before(:create) { create(:stock_location) if Spree::StockLocation.count.zero? }
|
||||
|
||||
|
||||
@@ -285,7 +285,7 @@ describe ProductImport::ProductImporter do
|
||||
expect(carrots.on_hand).to eq 5
|
||||
expect(carrots.variants.first.price).to eq 3.20
|
||||
expect(carrots.primary_taxon.name).to eq "Vegetables"
|
||||
expect(carrots.shipping_category).to eq shipping_category
|
||||
expect(carrots.variants.first.shipping_category).to eq shipping_category
|
||||
expect(carrots.supplier).to eq enterprise
|
||||
expect(carrots.variants.first.unit_presentation).to eq "500g"
|
||||
end
|
||||
|
||||
@@ -123,8 +123,7 @@ describe Spree::Order::Checkout do
|
||||
let(:order) { create(:order_with_totals_and_distribution, ship_address: create(:address) ) }
|
||||
let(:shipping_method) { create(:shipping_method, distributors: [order.distributor]) }
|
||||
let(:other_shipping_category) { create(:shipping_category) }
|
||||
let(:other_product) { create(:product, shipping_category: other_shipping_category ) }
|
||||
let(:other_variant) { other_product.variants.first }
|
||||
let(:other_variant) { create(:variant, shipping_category: other_shipping_category) }
|
||||
|
||||
before do
|
||||
order.order_cycle = create(:simple_order_cycle,
|
||||
|
||||
@@ -207,6 +207,7 @@ module Spree
|
||||
|
||||
context "saving a new product" do
|
||||
let!(:product){ Spree::Product.new }
|
||||
let!(:shipping_category){ create(:shipping_category) }
|
||||
|
||||
before do
|
||||
create(:stock_location)
|
||||
@@ -217,7 +218,7 @@ module Spree
|
||||
product.variant_unit_scale = 1000
|
||||
product.unit_value = 1
|
||||
product.price = 4.27
|
||||
product.shipping_category = create(:shipping_category)
|
||||
product.shipping_category_id = shipping_category.id
|
||||
product.save!
|
||||
end
|
||||
|
||||
@@ -225,6 +226,7 @@ module Spree
|
||||
expect(product.variants.reload.length).to eq 1
|
||||
standard_variant = product.variants.reload.first
|
||||
expect(standard_variant.price).to eq 4.27
|
||||
expect(standard_variant.shipping_category).to eq shipping_category
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -217,9 +217,7 @@ describe OrderAvailableShippingMethods do
|
||||
|
||||
context "when certain shipping categories are required" do
|
||||
subject { OrderAvailableShippingMethods.new(order) }
|
||||
let(:order) {
|
||||
build(:order, distributor: distributor, order_cycle: oc)
|
||||
}
|
||||
let(:order) { build(:order, distributor: distributor, order_cycle: oc) }
|
||||
let(:oc) { create(:order_cycle) }
|
||||
let(:distributor) { oc.distributors.first }
|
||||
let(:standard_shipping) {
|
||||
@@ -249,7 +247,7 @@ describe OrderAvailableShippingMethods do
|
||||
|
||||
it "filters shipping methods for products needing refrigeration" do
|
||||
product = oc.products.first
|
||||
product.update!(shipping_category: refrigerated)
|
||||
product.variants.first.update!(shipping_category: refrigerated)
|
||||
order.line_items << build(:line_item, variant: product.variants.first)
|
||||
expect(subject.to_a).to eq [cooled_shipping]
|
||||
end
|
||||
|
||||
@@ -182,7 +182,7 @@ describe "Product Import" do
|
||||
|
||||
carrots = Spree::Product.find_by(name: 'Carrots')
|
||||
expect(carrots.variants.first.tax_category).to eq tax_category
|
||||
expect(carrots.shipping_category).to eq shipping_category
|
||||
expect(carrots.variants.first.shipping_category).to eq shipping_category
|
||||
end
|
||||
|
||||
it "records a timestamp on import that can be viewed and filtered under Bulk Edit Products" do
|
||||
|
||||
@@ -110,7 +110,7 @@ describe '
|
||||
expect(product.variants.first.price.to_s).to eq('19.99')
|
||||
expect(product.on_hand).to eq(5)
|
||||
expect(product.variants.first.tax_category_id).to eq(tax_category.id)
|
||||
expect(product.shipping_category).to eq(shipping_category)
|
||||
expect(product.variants.first.shipping_category).to eq(shipping_category)
|
||||
expect(product.description).to eq("<p>A description...</p>")
|
||||
expect(product.group_buy).to be_falsey
|
||||
expect(product.variants.first.unit_presentation).to eq("5kg")
|
||||
|
||||
Reference in New Issue
Block a user