raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-03-05 02:41:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #11346 from Matt-Yorkley/remove-multiple-taxons

Browse Source 
[Product Refactor] Remove multiple taxons
This commit is contained in:
Konrad
2023-08-09 11:19:11 +02:00
committed by GitHub
parent 655fdf3fff 6fee1d2dde
commit 89d8fa0306
27 changed files with 44 additions and 197 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
app/models/spree/ability.rb
View File

@@ -234,7 +234,6 @@ module Spree
can [:admin, :index, :read, :create, :edit, :update, :destroy], Spree::Image
can [:admin, :index, :read, :search], Spree::Taxon
can [:admin, :index, :read, :create, :edit], Spree::Classification
can [:admin, :index, :guide, :import, :save, :save_data,
:validate_data, :reset_absent_products], ProductImport::ProductImporter

21
app/models/spree/classification.rb
View File

@@ -1,21 +0,0 @@
# frozen_string_literal: true
module Spree
class Classification < ApplicationRecord
self.table_name = 'spree_products_taxons'
belongs_to :product, class_name: "Spree::Product", touch: true
belongs_to :taxon, class_name: "Spree::Taxon", touch: true
before_destroy :dont_destroy_if_primary_taxon
private
def dont_destroy_if_primary_taxon
return unless product.primary_taxon == taxon
errors.add :base, I18n.t(:spree_classification_primary_taxon_error, taxon: taxon.name,
product: product.name)
throw :abort
end
end
end

15
app/models/spree/product.rb
View File

@@ -39,8 +39,6 @@ module Spree
has_many :product_properties, dependent: :destroy
has_many :properties, through: :product_properties
has_many :classifications, dependent: :delete_all
has_many :taxons, through: :classifications
has_many :variants, -> { order("spree_variants.position ASC") }, class_name: 'Spree::Variant',
dependent: :destroy
@@ -78,10 +76,7 @@ module Spree
# these values are persisted on the product's variant
attr_accessor :price, :display_as, :unit_value, :unit_description, :tax_category_id
before_save :add_primary_taxon_to_taxons
after_create :ensure_standard_variant
after_save :remove_previous_primary_taxon_from_taxons
after_save :update_units
scope :with_properties, ->(*property_ids) {
@@ -290,16 +285,6 @@ module Spree
Enterprise.distributing_products(id).each(&:touch)
end
def add_primary_taxon_to_taxons
taxons << primary_taxon unless taxons.include? primary_taxon
end
def remove_previous_primary_taxon_from_taxons
return unless saved_change_to_primary_taxon_id? && primary_taxon_id_before_last_save
taxons.destroy(primary_taxon_id_before_last_save)
end
def ensure_standard_variant
return unless variants.empty?

8
app/models/spree/taxon.rb
View File

@@ -5,8 +5,8 @@ module Spree
acts_as_nested_set dependent: :destroy
belongs_to :taxonomy, class_name: 'Spree::Taxonomy', touch: true
has_many :classifications, dependent: :destroy
has_many :products, through: :classifications
has_many :products, class_name: "Spree::Product", foreign_key: "primary_taxon_id",
inverse_of: :primary_taxon, dependent: :restrict_with_error
before_create :set_permalink
@@ -39,10 +39,6 @@ module Spree
permalink
end
def active_products
products.active
end
def pretty_name
ancestor_chain = ancestors.inject("") do |name, ancestor|
name + "#{ancestor.name} -> "

1
app/serializers/api/product_serializer.rb
View File

@@ -10,7 +10,6 @@ class Api::ProductSerializer < ActiveModel::Serializer
has_many :variants, serializer: Api::VariantSerializer
has_one :primary_taxon, serializer: Api::TaxonSerializer
has_many :taxons, serializer: Api::IdSerializer
has_one :image, serializer: Api::ImageSerializer
has_one :supplier, serializer: Api::IdSerializer

2
app/views/admin/products_v3/_table.html.haml
View File

@@ -39,7 +39,7 @@
%td.align-left
.line-clamp-1= product.supplier.name
%td.align-left
.line-clamp-1= product.taxons.map(&:name).join(', ')
.line-clamp-1= product.primary_taxon.name
%td.align-left
%td.align-left
.line-clamp-1= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below)

13
db/migrate/20230807145022_remove_product_taxons_table.rb Normal file
View File

@@ -0,0 +1,13 @@
class RemoveProductTaxonsTable < ActiveRecord::Migration[7.0]
def change
remove_foreign_key "spree_products_taxons", "spree_products", column: "product_id", name: "spree_products_taxons_product_id_fk", on_delete: :cascade
remove_foreign_key "spree_products_taxons", "spree_taxons", column: "taxon_id", name: "spree_products_taxons_taxon_id_fk", on_delete: :cascade
drop_table :spree_products_taxons do |t|
t.integer "product_id"
t.integer "taxon_id"
t.index ["product_id"], name: "index_products_taxons_on_product_id"
t.index ["taxon_id"], name: "index_products_taxons_on_taxon_id"
end
end
end

11
db/schema.rb
View File

@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2023_07_20_080504) do
ActiveRecord::Schema[7.0].define(version: 2023_08_07_145022) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_stat_statements"
enable_extension "plpgsql"
@@ -763,13 +763,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_20_080504) do
t.index ["supplier_id"], name: "index_spree_products_on_supplier_id"
end
create_table "spree_products_taxons", id: :serial, force: :cascade do |t|
t.integer "product_id"
t.integer "taxon_id"
t.index ["product_id"], name: "index_products_taxons_on_product_id"
t.index ["taxon_id"], name: "index_products_taxons_on_taxon_id"
end
create_table "spree_properties", id: :serial, force: :cascade do |t|
t.string "name", limit: 255
t.string "presentation", limit: 255, null: false
@@ -1272,8 +1265,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_20_080504) do
add_foreign_key "spree_products", "spree_shipping_categories", column: "shipping_category_id", name: "spree_products_shipping_category_id_fk"
add_foreign_key "spree_products", "spree_tax_categories", column: "tax_category_id", name: "spree_products_tax_category_id_fk"
add_foreign_key "spree_products", "spree_taxons", column: "primary_taxon_id", name: "spree_products_primary_taxon_id_fk"
add_foreign_key "spree_products_taxons", "spree_products", column: "product_id", name: "spree_products_taxons_product_id_fk", on_delete: :cascade
add_foreign_key "spree_products_taxons", "spree_taxons", column: "taxon_id", name: "spree_products_taxons_taxon_id_fk", on_delete: :cascade
add_foreign_key "spree_return_authorizations", "spree_orders", column: "order_id", name: "spree_return_authorizations_order_id_fk"
add_foreign_key "spree_roles_users", "spree_roles", column: "role_id", name: "spree_roles_users_role_id_fk"
add_foreign_key "spree_roles_users", "spree_users", column: "user_id", name: "spree_roles_users_user_id_fk"

2
lib/reporting/reports/products_and_inventory/base.rb
View File

@@ -17,7 +17,7 @@ module Reporting
producer_suburb: proc { |variant| variant.product.supplier.address.city },
product: proc { |variant| variant.product.name },
product_properties: proc { |v| v.product.properties.map(&:name).join(", ") },
taxons: proc { |variant| variant.product.taxons.map(&:name).join(", ") },
taxons: proc { |variant| variant.product.primary_taxon.name },
variant_value: proc { |variant| variant.full_name },
price: proc { |variant| variant.price },
group_buy_unit_quantity: proc { |variant| variant.product.group_buy_unit_size },

1
lib/spree/core/product_duplicator.rb
View File

@@ -20,7 +20,6 @@ module Spree
def duplicate_product
product.dup.tap do |new_product|
new_product.name = "COPY OF #{product.name}"
new_product.taxons = product.taxons
new_product.sku = ""
new_product.created_at = nil
new_product.deleted_at = nil

24
spec/components/product_component_spec.rb
View File

@@ -19,30 +19,6 @@ describe ProductComponent, type: :component do
end
end
describe 'category' do
let(:product) do
product = create(:simple_product)
product.taxons = taxons
product
end
let(:taxons) { [create(:taxon, name: 'random 1'), create(:taxon, name: 'random 2')] }
before do
render_inline(
ProductComponent.new(
product: product, columns: [{ label: "Category", value: "category", sortable: false }]
)
)
end
it "joins the categories' name" do
expect(page.find('.category')).to have_content(
/random 1, random 2/, exact: true, normalize_ws: true
)
end
end
describe 'on_hand' do
let(:product) { create(:simple_product, on_hand: on_hand) }
let(:on_hand) { 5 }

3
spec/factories.rb
View File

@@ -7,9 +7,6 @@ FactoryBot.define do
sequence(:random_description) { FFaker::Lorem.paragraphs(Kernel.rand(1..5)).join("\n") }
sequence(:random_email) { FFaker::Internet.email }
factory :classification, class: Spree::Classification do
end
factory :exchange, class: Exchange do
incoming { false }
order_cycle { OrderCycle.first || FactoryBot.create(:simple_order_cycle) }

7
spec/lib/reports/products_and_inventory_report_spec.rb
View File

@@ -43,9 +43,8 @@ module Reporting
allow(variant).to receive_message_chain(:product, :name).and_return("Product Name")
allow(variant).to receive_message_chain(:product, :properties)
.and_return [double(name: "property1"), double(name: "property2")]
allow(variant).to receive_message_chain(:product,
:taxons).and_return [double(name: "taxon1"),
double(name: "taxon2")]
allow(variant).to receive_message_chain(:product, :primary_taxon).
and_return double(name: "taxon1")
allow(variant).to receive_message_chain(:product, :group_buy_unit_size).and_return(21)
allow(subject).to receive(:query_result).and_return [variant]
@@ -54,7 +53,7 @@ module Reporting
"A city",
"Product Name",
"property1, property2",
"taxon1, taxon2",
"taxon1",
"Variant Name",
100,
21,

2
spec/lib/spree/core/product_duplicator_spec.rb
View File

@@ -6,7 +6,6 @@ describe Spree::Core::ProductDuplicator do
let(:product) do
double 'Product',
name: "foo",
taxons: [],
product_properties: [property],
variants: [variant],
image: image
@@ -68,7 +67,6 @@ describe Spree::Core::ProductDuplicator do
it "can duplicate a product" do
duplicator = Spree::Core::ProductDuplicator.new(product)
expect(new_product).to receive(:name=).with("COPY OF foo")
expect(new_product).to receive(:taxons=).with([])
expect(new_product).to receive(:sku=).with("")
expect(new_product).to receive(:product_properties=).with([new_property])
expect(new_product).to receive(:created_at=).with(nil)

15
spec/models/enterprise_caching_spec.rb
View File

@@ -7,11 +7,11 @@ describe Enterprise do
describe "is touched when a(n)" do
let(:enterprise) { create(:distributor_enterprise) }
let(:taxon) { create(:taxon) }
let(:taxon2) { create(:taxon) }
let(:supplier2) { create(:supplier_enterprise) }
describe "with a supplied product" do
let(:product) { create(:simple_product, supplier: enterprise) }
let!(:classification) { create(:classification, taxon: taxon, product: product) }
let(:product) { create(:simple_product, supplier: enterprise, primary_taxon_id: taxon.id) }
let(:property) { product.product_properties.last }
let(:producer_property) { enterprise.producer_properties.last }
@@ -20,9 +20,9 @@ describe Enterprise do
enterprise.set_producer_property 'Biodynamic', 'ASDF 4321'
end
it "touches enterprise when a classification on that product changes" do
it "touches enterprise when a taxon on that product changes" do
expect {
later { classification.touch }
later { product.update(primary_taxon_id: taxon2.id) }
}.to change { enterprise.reload.updated_at }
end
@@ -46,13 +46,12 @@ describe Enterprise do
end
describe "with a distributed product" do
let(:product) { create(:simple_product) }
let(:product) { create(:simple_product, primary_taxon_id: taxon.id) }
let(:oc) {
create(:simple_order_cycle, distributors: [enterprise],
variants: [product.variants.first])
}
let(:supplier) { product.supplier }
let!(:classification) { create(:classification, taxon: taxon, product: product) }
let(:property) { product.product_properties.last }
let(:producer_property) { supplier.producer_properties.last }
@@ -64,9 +63,9 @@ describe Enterprise do
context "with an order cycle" do
before { oc }
it "touches enterprise when a classification on that product changes" do
it "touches enterprise when a taxon on that product changes" do
expect {
later { classification.touch }
later { product.update(primary_taxon_id: taxon2.id) }
}.to change { enterprise.reload.updated_at }
end

4
spec/models/enterprise_spec.rb
View File

@@ -677,8 +677,8 @@ describe Enterprise do
let(:taxon1) { create(:taxon) }
let(:taxon2) { create(:taxon) }
let(:taxon3) { create(:taxon) }
let(:product1) { create(:simple_product, primary_taxon: taxon1, taxons: [taxon1]) }
let(:product2) { create(:simple_product, primary_taxon: taxon1, taxons: [taxon1, taxon2]) }
let(:product1) { create(:simple_product, primary_taxon: taxon1) }
let(:product2) { create(:simple_product, primary_taxon: taxon2) }
let(:product3) { create(:simple_product, primary_taxon: taxon3) }
let(:oc) { create(:order_cycle) }
let(:ex) {

5
spec/models/spree/ability_spec.rb
View File

@@ -399,11 +399,6 @@ describe Spree::Ability do
is_expected.to have_ability([:admin, :index, :read, :search], for: Spree::Taxon)
end
it "should be able to read/write Classifications on a product" do
is_expected.to have_ability([:admin, :index, :read, :create, :edit],
for: Spree::Classification)
end
it "should be able to read/write their enterprises' producer properties" do
is_expected.to have_ability(
[:admin, :index, :read, :create, :edit, :update_positions,

22
spec/models/spree/classification_spec.rb
View File

@@ -1,22 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
module Spree
describe Classification do
let(:product) { create(:simple_product) }
let(:taxon) { create(:taxon) }
it "won't destroy if classification is the primary taxon" do
classification = Classification.create(taxon: taxon, product: product)
product.update(primary_taxon: taxon)
expect(classification.destroy).to be false
expect(classification.errors.messages[:base])
.to eq(
["Taxon #{taxon.name} is the primary taxon of #{product.name} and cannot be deleted"]
)
expect(classification.reload).to be
end
end
end

45
spec/models/spree/product_spec.rb
View File

@@ -9,10 +9,6 @@ module Spree
let(:product) { create(:product) }
context '#duplicate' do
before do
allow(product).to receive_messages taxons: [create(:taxon)]
end
it 'duplicates product' do
clone = product.duplicate
expect(clone.name).to eq 'COPY OF ' + product.name
@@ -140,19 +136,6 @@ module Spree
end
end
# Regression tests for Spree #2352
context "classifications and taxons" do
it "is joined through classifications" do
reflection = Spree::Product.reflect_on_association(:taxons)
reflection.options[:through] = :classifications
end
it "will delete all classifications" do
reflection = Spree::Product.reflect_on_association(:classifications)
reflection.options[:dependent] = :delete_all
end
end
describe '#total_on_hand' do
it 'returns sum of stock items count_on_hand' do
product = build(:product)
@@ -183,7 +166,7 @@ module Spree
end
it "requires a primary taxon" do
expect(build(:simple_product, taxons: [], primary_taxon: nil)).not_to be_valid
expect(build(:simple_product, primary_taxon: nil)).not_to be_valid
end
it "requires a unit value" do
@@ -329,22 +312,6 @@ module Spree
end
end
it "adds the primary taxon to the product's taxon list" do
taxon = create(:taxon)
product = create(:product, primary_taxon: taxon)
expect(product.taxons).to include(taxon)
end
it "removes the previous primary taxon from the taxon list" do
original_taxon = create(:taxon)
product = create(:product, primary_taxon: original_taxon)
product.primary_taxon = create(:taxon)
product.save!
expect(product.taxons).not_to include(original_taxon)
end
it "updates units when saved change to variant unit" do
product.variant_unit = 'items'
product.variant_unit_scale = nil
@@ -736,16 +703,6 @@ module Spree
end
end
describe "taxons" do
let(:taxon1) { create(:taxon) }
let(:taxon2) { create(:taxon) }
let(:product) { create(:simple_product) }
it "returns the first taxon as the primary taxon" do
expect(product.taxons).to eq([product.primary_taxon])
end
end
describe "deletion" do
let(:p) { create(:simple_product) }
let(:v) { create(:variant, product: p) }

11
spec/models/spree/taxon_spec.rb
View File

@@ -11,10 +11,10 @@ module Spree
let(:t2) { create(:taxon) }
describe "finding all supplied taxons" do
let!(:p1) { create(:simple_product, supplier: e, taxons: [t1, t2]) }
let!(:p1) { create(:simple_product, supplier: e, primary_taxon_id: t1.id) }
it "finds taxons" do
expect(Taxon.supplied_taxons).to eq(e.id => Set.new(p1.taxons.map(&:id)))
expect(Taxon.supplied_taxons).to eq(e.id => Set.new([t1.id]))
end
end
@@ -40,12 +40,7 @@ module Spree
describe "touches" do
let!(:taxon1) { create(:taxon) }
let!(:taxon2) { create(:taxon) }
let!(:taxon3) { create(:taxon) }
let!(:product) { create(:simple_product, primary_taxon: taxon1, taxons: [taxon1, taxon2]) }
it "is touched when a taxon is applied to a product" do
expect{ product.taxons << taxon3 }.to change { taxon3.reload.updated_at }
end
let!(:product) { create(:simple_product, primary_taxon: taxon1) }
it "is touched when assignment of primary_taxon on a product changes" do
expect do

5
spec/serializers/api/enterprise_shopfront_serializer_spec.rb
View File

@@ -12,10 +12,10 @@ describe Api::EnterpriseShopfrontSerializer do
let!(:taxon1) { create(:taxon, name: 'Meat') }
let!(:taxon2) { create(:taxon, name: 'Veg') }
let!(:product) {
create(:product, supplier: producer, primary_taxon: taxon1, taxons: [taxon1, taxon2] )
create(:product, supplier: producer, primary_taxon: taxon1 )
}
let!(:product2) {
create(:product, supplier: producer_hidden, primary_taxon: taxon1, taxons: [taxon1, taxon2] )
create(:product, supplier: producer_hidden, primary_taxon: taxon2 )
}
let(:close_time) { 2.days.from_now }
@@ -67,6 +67,5 @@ describe Api::EnterpriseShopfrontSerializer do
it "serializes taxons" do
expect(serializer.serializable_hash[:taxons]).to be_a ActiveModel::ArraySerializer
expect(serializer.serializable_hash[:taxons].to_json).to match 'Meat'
expect(serializer.serializable_hash[:taxons].to_json).to match 'Veg'
end
end

6
spec/serializers/api/product_serializer_spec.rb
View File

@@ -28,7 +28,7 @@ describe Api::ProductSerializer do
it "serializes various attributes" do
expect(serializer.serializable_hash.keys).to eq [
:id, :name, :meta_keywords, :group_buy, :notes, :description, :description_html,
:properties_with_values, :variants, :primary_taxon, :taxons, :image, :supplier
:properties_with_values, :variants, :primary_taxon, :image, :supplier
]
end
@@ -37,8 +37,4 @@ describe Api::ProductSerializer do
expect(serializer.serializable_hash[:properties_with_values]).to include product_property
end
it "serializes taxons" do
expect(serializer.serializable_hash[:taxons]).to eq [id: taxon.id]
end
end

2
spec/system/admin/reports_spec.rb
View File

@@ -402,8 +402,6 @@ describe '
before do
product1.set_property 'Organic', 'NASAA 12345'
product2.set_property 'Organic', 'NASAA 12345'
product1.taxons = [taxon]
product2.taxons = [taxon]
variant1.on_hand = 10
variant1.update!(sku: "sku1")
variant2.on_hand = 20

2
spec/system/consumer/caching/darkswarm_caching_spec.rb
View File

@@ -11,7 +11,7 @@ describe "Darkswarm data caching", caching: true do
create(:distributor_enterprise, with_payment_and_shipping: true, is_primary_producer: true)
}
let!(:product) {
create(:simple_product, supplier: producer, primary_taxon: taxon, taxons: [taxon],
create(:simple_product, supplier: producer, primary_taxon: taxon,
properties: [property])
}
let!(:order_cycle) {

3
spec/system/consumer/caching/shops_caching_spec.rb
View File

@@ -53,7 +53,7 @@ describe "Shops caching", caching: true do
let!(:property) { create(:property, presentation: "Cached Property") }
let!(:property2) { create(:property, presentation: "New Property") }
let!(:product) {
create(:product, taxons: [taxon], primary_taxon: taxon, properties: [property])
create(:product, primary_taxon: taxon, properties: [property])
}
let(:exchange) { order_cycle.exchanges.to_enterprises(distributor).outgoing.first }
@@ -92,7 +92,6 @@ describe "Shops caching", caching: true do
expect(page).to have_content taxon.name
expect(page).to have_content property.presentation
product.taxons << taxon2
product.update_attribute(:primary_taxon, taxon2)
product.update_attribute(:properties, [property2])

4
spec/system/consumer/producers_spec.rb
View File

@@ -19,10 +19,10 @@ describe '
let(:taxon_veg) { create(:taxon, name: 'Vegetables') }
let!(:product1) {
create(:simple_product, supplier: producer1, primary_taxon: taxon_fruit, taxons: [taxon_fruit])
create(:simple_product, supplier: producer1, primary_taxon: taxon_fruit)
}
let!(:product2) {
create(:simple_product, supplier: producer2, primary_taxon: taxon_veg, taxons: [taxon_veg])
create(:simple_product, supplier: producer2, primary_taxon: taxon_veg)
}
let(:shop) { create(:distributor_enterprise) }

6
spec/system/consumer/shops_spec.rb
View File

@@ -146,7 +146,7 @@ describe 'Shops' do
let!(:closed_oc) {
create(:closed_order_cycle, distributors: [shop], variants: [p_closed.variants.first])
}
let!(:p_closed) { create(:simple_product, primary_taxon: taxon_closed, taxons: [taxon_closed]) }
let!(:p_closed) { create(:simple_product, primary_taxon: taxon_closed) }
let(:shop) { create(:distributor_enterprise, with_payment_and_shipping: true) }
let(:taxon_closed) { create(:taxon, name: 'Closed') }
@@ -154,7 +154,7 @@ describe 'Shops' do
let!(:open_oc) {
create(:open_order_cycle, distributors: [shop], variants: [p_open.variants.first])
}
let!(:p_open) { create(:simple_product, primary_taxon: taxon_open, taxons: [taxon_open]) }
let!(:p_open) { create(:simple_product, primary_taxon: taxon_open) }
let(:taxon_open) { create(:taxon, name: 'Open') }
it "shows taxons for open order cycles only" do
@@ -207,7 +207,7 @@ describe 'Shops' do
end
describe "hub producer modal" do
let!(:product) { create(:simple_product, supplier: producer, taxons: [taxon]) }
let!(:product) { create(:simple_product, supplier: producer, primary_taxon: taxon) }
let!(:taxon) { create(:taxon, name: 'Fruit') }
let!(:order_cycle) {
create(