From 87ccfc94ef85be82da164f318df794fc99a28ad1 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 16 Jun 2023 15:23:10 +1000 Subject: [PATCH 01/15] Require Enterprise.belongs_to by default And remove a duplicate spec. --- app/models/enterprise.rb | 7 ++++--- spec/models/enterprise_spec.rb | 12 +++--------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index b65738c189..7ed6016a59 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -10,6 +10,8 @@ class Enterprise < ApplicationRecord WHITE_LABEL_LOGO_SIZES = [:default, :mobile].freeze VALID_INSTAGRAM_REGEX = %r{\A[a-zA-Z0-9._]{1,30}([^/-]*)\z} + self.belongs_to_required_by_default = true + searchable_attributes :sells, :is_primary_producer, :name searchable_associations :properties searchable_scopes :is_primary_producer, :is_distributor, :is_hub, :activated, :visible, @@ -44,7 +46,7 @@ class Enterprise < ApplicationRecord dependent: :destroy has_many :distributed_orders, class_name: 'Spree::Order', foreign_key: 'distributor_id' belongs_to :address, class_name: 'Spree::Address' - belongs_to :business_address, class_name: 'Spree::Address', dependent: :destroy + belongs_to :business_address, optional: true, class_name: 'Spree::Address', dependent: :destroy has_many :enterprise_fees has_many :enterprise_roles, dependent: :destroy has_many :users, through: :enterprise_roles @@ -108,8 +110,7 @@ class Enterprise < ApplicationRecord validates :name, presence: true validate :name_is_unique validates :sells, presence: true, inclusion: { in: SELLS } - validates :address, presence: true, associated: true - validates :owner, presence: true + validates :address, associated: true validates :permalink, uniqueness: true, presence: true validate :shopfront_taxons validate :shopfront_producers diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 83caa3bcf4..558a9db532 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -19,11 +19,11 @@ describe Enterprise do end describe "associations" do - it { is_expected.to belong_to(:owner) } + it { is_expected.to belong_to(:owner).required } it { is_expected.to have_many(:supplied_products) } it { is_expected.to have_many(:distributed_orders) } - it { is_expected.to belong_to(:address) } - it { is_expected.to belong_to(:business_address) } + it { is_expected.to belong_to(:address).required } + it { is_expected.to belong_to(:business_address).optional } it { is_expected.to have_many(:vouchers) } it "destroys enterprise roles upon its own demise" do @@ -125,12 +125,6 @@ describe Enterprise do is_expected.to validate_uniqueness_of(:permalink) end - it "requires an owner" do - enterprise = build_stubbed(:enterprise, owner: nil) - expect(enterprise).not_to be_valid - expect(enterprise.errors[:owner].first).to eq "can't be blank" - end - describe "name uniqueness" do let(:owner) { create(:user, email: 'owner@example.com') } let!(:enterprise) { create(:enterprise, name: 'Enterprise', owner: owner) } From 13b2f37884ff5db5c31eb98cc8afda84411aac7e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 16 Jun 2023 15:54:43 +1000 Subject: [PATCH 02/15] Require EnterpriseFee.belongs_to by default --- app/models/enterprise_fee.rb | 4 +++- .../20230616055032_require_enterprise_on_enterprise_fee.rb | 7 +++++++ db/schema.rb | 2 +- spec/models/enterprise_fee_spec.rb | 2 +- 4 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20230616055032_require_enterprise_on_enterprise_fee.rb diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 598035abed..5abaf2bcb6 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -3,10 +3,12 @@ class EnterpriseFee < ApplicationRecord include CalculatedAdjustments + self.belongs_to_required_by_default = true + acts_as_paranoid belongs_to :enterprise - belongs_to :tax_category, class_name: 'Spree::TaxCategory' + belongs_to :tax_category, optional: true, class_name: 'Spree::TaxCategory' has_many :coordinator_fees, dependent: :destroy has_many :order_cycles, through: :coordinator_fees diff --git a/db/migrate/20230616055032_require_enterprise_on_enterprise_fee.rb b/db/migrate/20230616055032_require_enterprise_on_enterprise_fee.rb new file mode 100644 index 0000000000..4a22cc4c9e --- /dev/null +++ b/db/migrate/20230616055032_require_enterprise_on_enterprise_fee.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class RequireEnterpriseOnEnterpriseFee < ActiveRecord::Migration[7.0] + def change + change_column_null :enterprise_fees, :enterprise_id, false + end +end diff --git a/db/schema.rb b/db/schema.rb index c58419991e..17d9f51edb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -121,7 +121,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_09_201542) do end create_table "enterprise_fees", id: :serial, force: :cascade do |t| - t.integer "enterprise_id" + t.integer "enterprise_id", null: false t.string "fee_type", limit: 255 t.string "name", limit: 255 t.datetime "created_at", precision: nil, null: false diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index ab6f5eda53..cfbd5a9fee 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' describe EnterpriseFee do describe "associations" do - it { is_expected.to belong_to(:enterprise) } + it { is_expected.to belong_to(:enterprise).required } end describe "validations" do From 4b630f187b4f0bbf673fca46eb21f62ee4b842f1 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Jul 2023 14:56:14 +1000 Subject: [PATCH 03/15] Require EnterpriseGroup.belongs_to by default --- app/models/enterprise_group.rb | 10 ++++++++-- spec/models/enterprise_group_spec.rb | 7 +++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index 38b7458aa2..738cb2ad32 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -5,13 +5,15 @@ require 'open_food_network/locking' class EnterpriseGroup < ApplicationRecord include PermalinkGenerator + self.belongs_to_required_by_default = true + acts_as_list has_and_belongs_to_many :enterprises, join_table: 'enterprise_groups_enterprises' - belongs_to :owner, class_name: 'Spree::User', inverse_of: :owned_groups + belongs_to :owner, class_name: 'Spree::User', inverse_of: :owned_groups, optional: true belongs_to :address, class_name: 'Spree::Address' accepts_nested_attributes_for :address - validates :address, presence: true, associated: true + validates :address, associated: true before_validation :set_undefined_address_fields before_validation :set_unused_address_fields before_validation :sanitize_permalink @@ -46,10 +48,14 @@ class EnterpriseGroup < ApplicationRecord } def set_unused_address_fields + return if address.blank? + address.firstname = address.lastname = address.company = I18n.t(:unused) end def set_undefined_address_fields + return if address.blank? + address.phone.present? || address.phone = I18n.t(:undefined) address.address1.present? || address.address1 = I18n.t(:undefined) address.city.present? || address.city = I18n.t(:undefined) diff --git a/spec/models/enterprise_group_spec.rb b/spec/models/enterprise_group_spec.rb index 2d62bce69c..4a74fa65f8 100644 --- a/spec/models/enterprise_group_spec.rb +++ b/spec/models/enterprise_group_spec.rb @@ -3,6 +3,13 @@ require 'spec_helper' describe EnterpriseGroup do + describe "associations" do + subject { build(:enterprise_group) } + + it { is_expected.to belong_to(:owner).optional } + it { is_expected.to belong_to(:address).required } + end + describe "validations" do it "pass with name, description and address" do e = EnterpriseGroup.new From cb494b84f2fd7648884ebdab94fcc72ee06b4411 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Jul 2023 15:31:19 +1000 Subject: [PATCH 04/15] Require EnterpriseRelationship.belongs_to by default --- app/models/enterprise_relationship.rb | 3 ++- ...2_require_parent_and_child_on_enterprise_relationship.rb | 6 ++++++ db/schema.rb | 4 ++-- 3 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20230728052532_require_parent_and_child_on_enterprise_relationship.rb diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 30bddcc31d..e76429c7e5 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true class EnterpriseRelationship < ApplicationRecord + self.belongs_to_required_by_default = true + belongs_to :parent, class_name: 'Enterprise', touch: true belongs_to :child, class_name: 'Enterprise', touch: true has_many :permissions, class_name: 'EnterpriseRelationshipPermission', dependent: :destroy - validates :parent, :child, presence: true validates :child_id, uniqueness: { scope: :parent_id, message: I18n.t('validation_msg_relationship_already_established') diff --git a/db/migrate/20230728052532_require_parent_and_child_on_enterprise_relationship.rb b/db/migrate/20230728052532_require_parent_and_child_on_enterprise_relationship.rb new file mode 100644 index 0000000000..c76411c5ec --- /dev/null +++ b/db/migrate/20230728052532_require_parent_and_child_on_enterprise_relationship.rb @@ -0,0 +1,6 @@ +class RequireParentAndChildOnEnterpriseRelationship < ActiveRecord::Migration[7.0] + def change + change_column_null :enterprise_relationships, :parent_id, false + change_column_null :enterprise_relationships, :child_id, false + end +end diff --git a/db/schema.rb b/db/schema.rb index 17d9f51edb..89ffb1ea23 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -167,8 +167,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_09_201542) do end create_table "enterprise_relationships", id: :serial, force: :cascade do |t| - t.integer "parent_id" - t.integer "child_id" + t.integer "parent_id", null: false + t.integer "child_id", null: false t.index ["child_id"], name: "index_enterprise_relationships_on_child_id" t.index ["parent_id", "child_id"], name: "index_enterprise_relationships_on_parent_id_and_child_id", unique: true t.index ["parent_id"], name: "index_enterprise_relationships_on_parent_id" From 4dd29554009670fa1ad22619cc7b54bdd760c8a2 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Jul 2023 15:37:43 +1000 Subject: [PATCH 05/15] Require EnterpriseRelationshipPermission.belongs_to by default I had to change some specs to keep them simple but I don't think anything is broken. In one case, it would have needed a lot more setup to make the spec work. But in production, the permissions are never used with ModelSet, for example. --- app/models/enterprise_relationship_permission.rb | 2 ++ ...tionship_on_enterprise_relationship_permission.rb | 5 +++++ db/schema.rb | 2 +- spec/models/enterprise_spec.rb | 12 ++++++------ spec/services/sets/model_set_spec.rb | 12 ++++++------ 5 files changed, 20 insertions(+), 13 deletions(-) create mode 100644 db/migrate/20230728060128_require_enterprise_relationship_on_enterprise_relationship_permission.rb diff --git a/app/models/enterprise_relationship_permission.rb b/app/models/enterprise_relationship_permission.rb index f92b10188f..44ac935940 100644 --- a/app/models/enterprise_relationship_permission.rb +++ b/app/models/enterprise_relationship_permission.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class EnterpriseRelationshipPermission < ApplicationRecord + self.belongs_to_required_by_default = true + belongs_to :enterprise_relationship default_scope { order('name') } before_destroy :destroy_related_exchanges diff --git a/db/migrate/20230728060128_require_enterprise_relationship_on_enterprise_relationship_permission.rb b/db/migrate/20230728060128_require_enterprise_relationship_on_enterprise_relationship_permission.rb new file mode 100644 index 0000000000..54e6121817 --- /dev/null +++ b/db/migrate/20230728060128_require_enterprise_relationship_on_enterprise_relationship_permission.rb @@ -0,0 +1,5 @@ +class RequireEnterpriseRelationshipOnEnterpriseRelationshipPermission < ActiveRecord::Migration[7.0] + def change + change_column_null :enterprise_relationship_permissions, :enterprise_relationship_id, false + end +end diff --git a/db/schema.rb b/db/schema.rb index 89ffb1ea23..6813c64165 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -161,7 +161,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_09_201542) do end create_table "enterprise_relationship_permissions", id: :serial, force: :cascade do |t| - t.integer "enterprise_relationship_id" + t.integer "enterprise_relationship_id", null: false t.string "name", limit: 255, null: false t.index ["enterprise_relationship_id"], name: "index_erp_on_erid" end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 558a9db532..6b975c899a 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -817,7 +817,7 @@ describe Enterprise do it "should return only parent producers" do supplier = create(:supplier_enterprise) distributor = create(:distributor_enterprise, is_primary_producer: false) - permission = EnterpriseRelationshipPermission.create(name: "add_to_order_cycle") + permission = EnterpriseRelationshipPermission.new(name: "add_to_order_cycle") create(:enterprise_relationship, parent: distributor, child: supplier, permissions: [permission]) expect(Enterprise.parents_of_one_union_others(supplier, nil)).to include(distributor) @@ -827,7 +827,7 @@ describe Enterprise do another_enterprise = create(:enterprise) supplier = create(:supplier_enterprise) distributor = create(:distributor_enterprise, is_primary_producer: false) - permission = EnterpriseRelationshipPermission.create(name: "add_to_order_cycle") + permission = EnterpriseRelationshipPermission.new(name: "add_to_order_cycle") create(:enterprise_relationship, parent: distributor, child: supplier, permissions: [permission]) expect( @@ -838,7 +838,7 @@ describe Enterprise do it "does not find child in the relationship" do supplier = create(:supplier_enterprise) distributor = create(:distributor_enterprise, is_primary_producer: false) - permission = EnterpriseRelationshipPermission.create(name: "add_to_order_cycle") + permission = EnterpriseRelationshipPermission.new(name: "add_to_order_cycle") create(:enterprise_relationship, parent: distributor, child: supplier, permissions: [permission]) expect(Enterprise.parents_of_one_union_others(distributor, nil)).not_to include(supplier) @@ -862,7 +862,7 @@ describe Enterprise do it "finds parent in the relationship" do supplier = create(:supplier_enterprise) distributor = create(:distributor_enterprise, is_primary_producer: false) - permission = EnterpriseRelationshipPermission.create(name: "add_to_order_cycle") + permission = EnterpriseRelationshipPermission.new(name: "add_to_order_cycle") product = create(:product) order_cycle = create( :simple_order_cycle, @@ -878,7 +878,7 @@ describe Enterprise do it "does not find child in the relationship" do supplier = create(:supplier_enterprise) distributor = create(:distributor_enterprise, is_primary_producer: false) - permission = EnterpriseRelationshipPermission.create(name: "add_to_order_cycle") + permission = EnterpriseRelationshipPermission.new(name: "add_to_order_cycle") create(:enterprise_relationship, parent: distributor, child: supplier, permissions: [permission]) product = create(:product) @@ -896,7 +896,7 @@ describe Enterprise do supplier = create(:supplier_enterprise) sender = create(:supplier_enterprise) distributor = create(:distributor_enterprise, is_primary_producer: false) - permission = EnterpriseRelationshipPermission.create(name: "add_to_order_cycle") + permission = EnterpriseRelationshipPermission.new(name: "add_to_order_cycle") create(:enterprise_relationship, parent: distributor, child: supplier, permissions: [permission]) product = create(:product) diff --git a/spec/services/sets/model_set_spec.rb b/spec/services/sets/model_set_spec.rb index 97528f5df4..416aa63fb4 100644 --- a/spec/services/sets/model_set_spec.rb +++ b/spec/services/sets/model_set_spec.rb @@ -5,16 +5,16 @@ require 'spec_helper' describe Sets::ModelSet do describe "updating" do it "creates new models" do - attrs = { collection_attributes: { '1' => { name: 's1' }, - '2' => { name: 's2' } } } + attrs = { collection_attributes: { '1' => { name: "Fantasia", iso_name: "FAN" }, + '2' => { name: "Utopia", iso_name: "UTO" } } } - ms = Sets::ModelSet.new(EnterpriseRelationshipPermission, - EnterpriseRelationshipPermission.all, + ms = Sets::ModelSet.new(Spree::Country, + Spree::Country.all, attrs) - expect { ms.save }.to change(EnterpriseRelationshipPermission, :count).by(2) + expect { ms.save }.to change(Spree::Country, :count).by(2) - expect(EnterpriseRelationshipPermission.where(name: ['s1', 's2']).count).to eq(2) + expect(Spree::Country.where(name: ["Fantasia", "Utopia"]).count).to eq(2) end it "updates existing models" do From 070d1e722ec7cd93c57be8d6b3ed40d1f35334af Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Jul 2023 16:06:48 +1000 Subject: [PATCH 06/15] Require EnterpriseRole.belongs_to by default --- app/models/enterprise_role.rb | 3 ++- ...060433_require_user_and_enterprise_on_enterprise_role.rb | 6 ++++++ db/schema.rb | 4 ++-- 3 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20230728060433_require_user_and_enterprise_on_enterprise_role.rb diff --git a/app/models/enterprise_role.rb b/app/models/enterprise_role.rb index e19762d4a0..9fe57584f6 100644 --- a/app/models/enterprise_role.rb +++ b/app/models/enterprise_role.rb @@ -1,10 +1,11 @@ # frozen_string_literal: true class EnterpriseRole < ApplicationRecord + self.belongs_to_required_by_default = true + belongs_to :user, class_name: "Spree::User" belongs_to :enterprise - validates :user, :enterprise, presence: true validates :enterprise_id, uniqueness: { scope: :user_id, message: I18n.t(:enterprise_role_uniqueness_error) } diff --git a/db/migrate/20230728060433_require_user_and_enterprise_on_enterprise_role.rb b/db/migrate/20230728060433_require_user_and_enterprise_on_enterprise_role.rb new file mode 100644 index 0000000000..23b6711d68 --- /dev/null +++ b/db/migrate/20230728060433_require_user_and_enterprise_on_enterprise_role.rb @@ -0,0 +1,6 @@ +class RequireUserAndEnterpriseOnEnterpriseRole < ActiveRecord::Migration[7.0] + def change + change_column_null :enterprise_roles, :user_id, false + change_column_null :enterprise_roles, :enterprise_id, false + end +end diff --git a/db/schema.rb b/db/schema.rb index 6813c64165..38d709e9e8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -175,8 +175,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_09_201542) do end create_table "enterprise_roles", id: :serial, force: :cascade do |t| - t.integer "user_id" - t.integer "enterprise_id" + t.integer "user_id", null: false + t.integer "enterprise_id", null: false t.boolean "receives_notifications", default: false t.index ["enterprise_id", "user_id"], name: "index_enterprise_roles_on_enterprise_id_and_user_id", unique: true t.index ["enterprise_id"], name: "index_enterprise_roles_on_enterprise_id" From 252f8094638c13564f145a230fe86f4f93f74113 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Jul 2023 16:19:42 +1000 Subject: [PATCH 07/15] Require CustomTab.belongs_to by default --- app/models/custom_tab.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/custom_tab.rb b/app/models/custom_tab.rb index 22058efcc1..321ae0a5a2 100644 --- a/app/models/custom_tab.rb +++ b/app/models/custom_tab.rb @@ -1,7 +1,9 @@ # frozen_string_literal: false class CustomTab < ApplicationRecord - belongs_to :enterprise, optional: false + self.belongs_to_required_by_default = true + + belongs_to :enterprise validates :title, presence: true, length: { maximum: 20 } end From 8ef69668910a582bffbe3619f027b46883ff682d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Jul 2023 16:22:20 +1000 Subject: [PATCH 08/15] Declare old belongs_to default on remaining models It would take ages to go through all files now and assess all belongs_to associations. So I just declare the old default and then we can move on and apply the new default for the application while these classes still use the old one. All new models will then use the new default which is the goal of this excercise and we can refactor old classes when we touch them anyway. --- app/models/exchange.rb | 2 ++ app/models/exchange_fee.rb | 2 ++ app/models/exchange_variant.rb | 2 ++ app/models/inventory_item.rb | 2 ++ app/models/invoice.rb | 2 ++ app/models/order_cycle.rb | 2 ++ app/models/order_cycle_schedule.rb | 2 ++ app/models/producer_property.rb | 2 ++ app/models/proxy_order.rb | 2 ++ app/models/report_rendering_options.rb | 2 ++ app/models/spree/address.rb | 2 ++ app/models/spree/adjustment.rb | 2 ++ app/models/spree/asset.rb | 2 ++ app/models/spree/calculator.rb | 2 ++ app/models/spree/credit_card.rb | 2 ++ app/models/spree/inventory_unit.rb | 2 ++ app/models/spree/line_item.rb | 2 ++ app/models/spree/log_entry.rb | 2 ++ app/models/spree/order.rb | 2 ++ app/models/spree/payment.rb | 2 ++ app/models/spree/payment_method.rb | 2 ++ app/models/spree/price.rb | 2 ++ app/models/spree/product.rb | 2 ++ app/models/spree/product_property.rb | 2 ++ app/models/spree/return_authorization.rb | 2 ++ app/models/spree/shipment.rb | 2 ++ app/models/spree/shipping_method_category.rb | 2 ++ app/models/spree/shipping_rate.rb | 2 ++ app/models/spree/state.rb | 2 ++ app/models/spree/state_change.rb | 2 ++ app/models/spree/stock_item.rb | 2 ++ app/models/spree/stock_location.rb | 2 ++ app/models/spree/stock_movement.rb | 2 ++ app/models/spree/tax_rate.rb | 2 ++ app/models/spree/taxon.rb | 2 ++ app/models/spree/tokenized_permission.rb | 2 ++ app/models/spree/user.rb | 2 ++ app/models/spree/variant.rb | 2 ++ app/models/spree/zone_member.rb | 2 ++ app/models/stripe_account.rb | 2 ++ app/models/subscription.rb | 2 ++ app/models/subscription_line_item.rb | 2 ++ app/models/tag_rule.rb | 2 ++ app/models/variant_override.rb | 2 ++ app/models/voucher.rb | 2 ++ 45 files changed, 90 insertions(+) diff --git a/app/models/exchange.rb b/app/models/exchange.rb index 732d7a2f7d..8dbf3faf27 100644 --- a/app/models/exchange.rb +++ b/app/models/exchange.rb @@ -10,6 +10,8 @@ # shopfront (outgoing products). But the set of shown products can be smaller # than all incoming products. class Exchange < ApplicationRecord + self.belongs_to_required_by_default = false + acts_as_taggable belongs_to :order_cycle diff --git a/app/models/exchange_fee.rb b/app/models/exchange_fee.rb index e2eda0d611..7e73068be8 100644 --- a/app/models/exchange_fee.rb +++ b/app/models/exchange_fee.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ExchangeFee < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :exchange belongs_to :enterprise_fee end diff --git a/app/models/exchange_variant.rb b/app/models/exchange_variant.rb index a3dfc01a40..094f1a8b50 100644 --- a/app/models/exchange_variant.rb +++ b/app/models/exchange_variant.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ExchangeVariant < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :exchange belongs_to :variant, class_name: 'Spree::Variant' diff --git a/app/models/inventory_item.rb b/app/models/inventory_item.rb index 601172999d..d013ea52f2 100644 --- a/app/models/inventory_item.rb +++ b/app/models/inventory_item.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class InventoryItem < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :enterprise belongs_to :variant, class_name: "Spree::Variant" diff --git a/app/models/invoice.rb b/app/models/invoice.rb index b66b0c5ba5..b019463867 100644 --- a/app/models/invoice.rb +++ b/app/models/invoice.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Invoice < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :order, class_name: 'Spree::Order' serialize :data, Hash before_validation :serialize_order diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 4eb3bc9060..d9f8205ef3 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -3,6 +3,8 @@ require 'open_food_network/scope_variant_to_hub' class OrderCycle < ApplicationRecord + self.belongs_to_required_by_default = false + searchable_attributes :orders_open_at, :orders_close_at, :coordinator_id searchable_scopes :active, :inactive, :active_or_complete, :upcoming, :closed, :not_closed, :dated, :undated, :soonest_opening, :soonest_closing, :most_recently_closed diff --git a/app/models/order_cycle_schedule.rb b/app/models/order_cycle_schedule.rb index b3087cf11b..0f356cf980 100644 --- a/app/models/order_cycle_schedule.rb +++ b/app/models/order_cycle_schedule.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class OrderCycleSchedule < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :schedule belongs_to :order_cycle end diff --git a/app/models/producer_property.rb b/app/models/producer_property.rb index 171601cfa3..3cae30a59b 100644 --- a/app/models/producer_property.rb +++ b/app/models/producer_property.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ProducerProperty < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :producer, class_name: 'Enterprise', touch: true belongs_to :property, class_name: 'Spree::Property' diff --git a/app/models/proxy_order.rb b/app/models/proxy_order.rb index 43638aee08..5908db5ade 100644 --- a/app/models/proxy_order.rb +++ b/app/models/proxy_order.rb @@ -5,6 +5,8 @@ # This reduces the need to keep Orders in sync with their parent Subscriptions class ProxyOrder < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :order, class_name: 'Spree::Order' belongs_to :subscription belongs_to :order_cycle diff --git a/app/models/report_rendering_options.rb b/app/models/report_rendering_options.rb index e94d77046b..feadbaf7ff 100644 --- a/app/models/report_rendering_options.rb +++ b/app/models/report_rendering_options.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ReportRenderingOptions < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :user, class_name: "Spree::User" serialize :options, Hash end diff --git a/app/models/spree/address.rb b/app/models/spree/address.rb index 2b917f18cb..9aa847c861 100644 --- a/app/models/spree/address.rb +++ b/app/models/spree/address.rb @@ -4,6 +4,8 @@ module Spree class Address < ApplicationRecord include AddressDisplay + self.belongs_to_required_by_default = false + searchable_attributes :firstname, :lastname, :phone, :full_name searchable_associations :country, :state diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb index 7ebacacafd..8c8bfb848e 100644 --- a/app/models/spree/adjustment.rb +++ b/app/models/spree/adjustment.rb @@ -31,6 +31,8 @@ module Spree class Adjustment < ApplicationRecord extend Spree::LocalizedNumber + self.belongs_to_required_by_default = false + # Deletion of metadata is handled in the database. # So we don't need the option `dependent: :destroy` as long as # AdjustmentMetadata has no destroy logic itself. diff --git a/app/models/spree/asset.rb b/app/models/spree/asset.rb index 168435c12f..062c661a20 100644 --- a/app/models/spree/asset.rb +++ b/app/models/spree/asset.rb @@ -2,6 +2,8 @@ module Spree class Asset < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :viewable, polymorphic: true, touch: true acts_as_list scope: :viewable end diff --git a/app/models/spree/calculator.rb b/app/models/spree/calculator.rb index fa7880f3ec..1b561d2549 100644 --- a/app/models/spree/calculator.rb +++ b/app/models/spree/calculator.rb @@ -2,6 +2,8 @@ module Spree class Calculator < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :calculable, polymorphic: true # This method must be overriden in concrete calculator. diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index d721f57fde..1d386bdac8 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -2,6 +2,8 @@ module Spree class CreditCard < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :payment_method belongs_to :user diff --git a/app/models/spree/inventory_unit.rb b/app/models/spree/inventory_unit.rb index aea1935d07..44a5c4acf9 100644 --- a/app/models/spree/inventory_unit.rb +++ b/app/models/spree/inventory_unit.rb @@ -2,6 +2,8 @@ module Spree class InventoryUnit < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :variant, -> { with_deleted }, class_name: "Spree::Variant" belongs_to :order, class_name: "Spree::Order" belongs_to :shipment, class_name: "Spree::Shipment" diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index 2ecd84f0ba..846b88ba3c 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -8,6 +8,8 @@ module Spree include VariantUnits::VariantAndLineItemNaming include LineItemStockChanges + self.belongs_to_required_by_default = false + searchable_attributes :price, :quantity, :order_id, :variant_id, :tax_category_id searchable_associations :order, :order_cycle, :variant, :product, :supplier, :tax_category searchable_scopes :with_tax, :without_tax diff --git a/app/models/spree/log_entry.rb b/app/models/spree/log_entry.rb index 449df8c4ec..060ba50ecd 100644 --- a/app/models/spree/log_entry.rb +++ b/app/models/spree/log_entry.rb @@ -2,6 +2,8 @@ module Spree class LogEntry < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :source, polymorphic: true # Fix for Spree #1767 diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index f7bb58e9ea..6fc2ec1ac0 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -13,6 +13,8 @@ module Spree include Balance include SetUnusedAddressFields + self.belongs_to_required_by_default = false + searchable_attributes :number, :state, :shipment_state, :payment_state, :distributor_id, :order_cycle_id, :email, :total, :customer_id searchable_associations :shipping_method, :bill_address, :distributor diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 21d5a55166..c7f7ec54d0 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -7,6 +7,8 @@ module Spree include Spree::Payment::Processing extend Spree::LocalizedNumber + self.belongs_to_required_by_default = false + localize_number :amount IDENTIFIER_CHARS = (('A'..'Z').to_a + ('0'..'9').to_a - %w(0 1 I O)).freeze diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index 29802f9047..6113291103 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -7,6 +7,8 @@ module Spree include CalculatedAdjustments include PaymentMethodDistributors + self.belongs_to_required_by_default = false + acts_as_taggable acts_as_paranoid diff --git a/app/models/spree/price.rb b/app/models/spree/price.rb index 871b7886e3..9818ffd154 100644 --- a/app/models/spree/price.rb +++ b/app/models/spree/price.rb @@ -2,6 +2,8 @@ module Spree class Price < ApplicationRecord + self.belongs_to_required_by_default = false + acts_as_paranoid without_default_scope: true belongs_to :variant, -> { with_deleted }, class_name: 'Spree::Variant' diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 606de8ceda..c9ca6ca063 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -25,6 +25,8 @@ module Spree class Product < ApplicationRecord include ProductStock + self.belongs_to_required_by_default = false + acts_as_paranoid searchable_attributes :supplier_id, :primary_taxon_id, :meta_keywords, :sku diff --git a/app/models/spree/product_property.rb b/app/models/spree/product_property.rb index 2f67dbcd9b..6e7f46bdd8 100644 --- a/app/models/spree/product_property.rb +++ b/app/models/spree/product_property.rb @@ -2,6 +2,8 @@ module Spree class ProductProperty < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :product, class_name: "Spree::Product", touch: true belongs_to :property, class_name: 'Spree::Property' diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb index b173e7c679..42b9349e6d 100644 --- a/app/models/spree/return_authorization.rb +++ b/app/models/spree/return_authorization.rb @@ -2,6 +2,8 @@ module Spree class ReturnAuthorization < ApplicationRecord + self.belongs_to_required_by_default = false + acts_as_paranoid belongs_to :order, class_name: 'Spree::Order', inverse_of: :return_authorizations diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb index c3c50e6898..f7428f2341 100644 --- a/app/models/spree/shipment.rb +++ b/app/models/spree/shipment.rb @@ -4,6 +4,8 @@ require 'ostruct' module Spree class Shipment < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :order, class_name: 'Spree::Order' belongs_to :address, class_name: 'Spree::Address' belongs_to :stock_location, class_name: 'Spree::StockLocation' diff --git a/app/models/spree/shipping_method_category.rb b/app/models/spree/shipping_method_category.rb index 0a32c66125..f04607fb34 100644 --- a/app/models/spree/shipping_method_category.rb +++ b/app/models/spree/shipping_method_category.rb @@ -2,6 +2,8 @@ module Spree class ShippingMethodCategory < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :shipping_method, class_name: 'Spree::ShippingMethod' belongs_to :shipping_category, class_name: 'Spree::ShippingCategory', inverse_of: :shipping_method_categories diff --git a/app/models/spree/shipping_rate.rb b/app/models/spree/shipping_rate.rb index 5116a2de6b..aef9a91b20 100644 --- a/app/models/spree/shipping_rate.rb +++ b/app/models/spree/shipping_rate.rb @@ -2,6 +2,8 @@ module Spree class ShippingRate < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :shipment, class_name: 'Spree::Shipment' belongs_to :shipping_method, class_name: 'Spree::ShippingMethod', inverse_of: :shipping_rates diff --git a/app/models/spree/state.rb b/app/models/spree/state.rb index 55bd4b5ae6..cbbd159a55 100644 --- a/app/models/spree/state.rb +++ b/app/models/spree/state.rb @@ -2,6 +2,8 @@ module Spree class State < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :country, class_name: 'Spree::Country' validates :country, :name, presence: true diff --git a/app/models/spree/state_change.rb b/app/models/spree/state_change.rb index 0e770096b5..e5401d5ea9 100644 --- a/app/models/spree/state_change.rb +++ b/app/models/spree/state_change.rb @@ -2,6 +2,8 @@ module Spree class StateChange < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :user belongs_to :stateful, polymorphic: true before_create :assign_user diff --git a/app/models/spree/stock_item.rb b/app/models/spree/stock_item.rb index 96950899b2..192a154b3d 100644 --- a/app/models/spree/stock_item.rb +++ b/app/models/spree/stock_item.rb @@ -2,6 +2,8 @@ module Spree class StockItem < ApplicationRecord + self.belongs_to_required_by_default = false + acts_as_paranoid belongs_to :stock_location, class_name: 'Spree::StockLocation', inverse_of: :stock_items diff --git a/app/models/spree/stock_location.rb b/app/models/spree/stock_location.rb index 7cf9e26d51..a3e7d588bf 100644 --- a/app/models/spree/stock_location.rb +++ b/app/models/spree/stock_location.rb @@ -2,6 +2,8 @@ module Spree class StockLocation < ApplicationRecord + self.belongs_to_required_by_default = false + has_many :stock_items, dependent: :delete_all, inverse_of: :stock_location has_many :stock_movements, through: :stock_items diff --git a/app/models/spree/stock_movement.rb b/app/models/spree/stock_movement.rb index 1409c8a606..1cf432df5c 100644 --- a/app/models/spree/stock_movement.rb +++ b/app/models/spree/stock_movement.rb @@ -2,6 +2,8 @@ module Spree class StockMovement < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :stock_item, class_name: 'Spree::StockItem' belongs_to :originator, polymorphic: true diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb index 2936d37d37..f603db2c02 100644 --- a/app/models/spree/tax_rate.rb +++ b/app/models/spree/tax_rate.rb @@ -14,6 +14,8 @@ end module Spree class TaxRate < ApplicationRecord + self.belongs_to_required_by_default = false + acts_as_paranoid include CalculatedAdjustments diff --git a/app/models/spree/taxon.rb b/app/models/spree/taxon.rb index 49f89f0980..2ed96f4d57 100644 --- a/app/models/spree/taxon.rb +++ b/app/models/spree/taxon.rb @@ -2,6 +2,8 @@ module Spree class Taxon < ApplicationRecord + self.belongs_to_required_by_default = false + acts_as_nested_set dependent: :destroy belongs_to :taxonomy, class_name: 'Spree::Taxonomy', touch: true diff --git a/app/models/spree/tokenized_permission.rb b/app/models/spree/tokenized_permission.rb index e2d5329dec..7aea858d82 100644 --- a/app/models/spree/tokenized_permission.rb +++ b/app/models/spree/tokenized_permission.rb @@ -2,6 +2,8 @@ module Spree class TokenizedPermission < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :permissable, polymorphic: true end end diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 1932542edc..f1d929332e 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -4,6 +4,8 @@ module Spree class User < ApplicationRecord include SetUnusedAddressFields + self.belongs_to_required_by_default = false + searchable_attributes :email devise :database_authenticatable, :token_authenticatable, :registerable, :recoverable, diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index d3215fddb0..74a0db5e60 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -11,6 +11,8 @@ module Spree include VariantUnits::VariantAndLineItemNaming include VariantStock + self.belongs_to_required_by_default = false + acts_as_paranoid searchable_attributes :sku, :display_as, :display_name diff --git a/app/models/spree/zone_member.rb b/app/models/spree/zone_member.rb index 45a28efebd..6065e7c4f6 100644 --- a/app/models/spree/zone_member.rb +++ b/app/models/spree/zone_member.rb @@ -2,6 +2,8 @@ module Spree class ZoneMember < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :zone, class_name: 'Spree::Zone', counter_cache: true, inverse_of: :zone_members belongs_to :zoneable, polymorphic: true diff --git a/app/models/stripe_account.rb b/app/models/stripe_account.rb index 5eaeb4f3f4..76a4487dfd 100644 --- a/app/models/stripe_account.rb +++ b/app/models/stripe_account.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class StripeAccount < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :enterprise validates :stripe_user_id, :stripe_publishable_key, presence: true validates :enterprise_id, uniqueness: true diff --git a/app/models/subscription.rb b/app/models/subscription.rb index 926bbf71d1..04e57d1a54 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -6,6 +6,8 @@ class Subscription < ApplicationRecord ALLOWED_PAYMENT_METHOD_TYPES = ["Spree::PaymentMethod::Check", "Spree::Gateway::StripeSCA"].freeze + self.belongs_to_required_by_default = false + searchable_attributes :shop_id, :canceled_at, :paused_at searchable_associations :shop searchable_scopes :active, :not_ended, :not_paused, :not_canceled diff --git a/app/models/subscription_line_item.rb b/app/models/subscription_line_item.rb index 1e30f567ad..a4a0b1ede5 100644 --- a/app/models/subscription_line_item.rb +++ b/app/models/subscription_line_item.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class SubscriptionLineItem < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :subscription, inverse_of: :subscription_line_items belongs_to :variant, -> { with_deleted }, class_name: 'Spree::Variant' diff --git a/app/models/tag_rule.rb b/app/models/tag_rule.rb index b1d5b83ef9..cdff7315b0 100644 --- a/app/models/tag_rule.rb +++ b/app/models/tag_rule.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class TagRule < ApplicationRecord + self.belongs_to_required_by_default = false + belongs_to :enterprise preference :customer_tags, :string, default: "" diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb index 3c9f0bd85d..40f9a16a1a 100644 --- a/app/models/variant_override.rb +++ b/app/models/variant_override.rb @@ -6,6 +6,8 @@ class VariantOverride < ApplicationRecord extend Spree::LocalizedNumber include StockSettingsOverrideValidation + self.belongs_to_required_by_default = false + acts_as_taggable belongs_to :hub, class_name: 'Enterprise' diff --git a/app/models/voucher.rb b/app/models/voucher.rb index 109ffd2965..58df7538c8 100644 --- a/app/models/voucher.rb +++ b/app/models/voucher.rb @@ -1,6 +1,8 @@ # frozen_string_literal: false class Voucher < ApplicationRecord + self.belongs_to_required_by_default = false + acts_as_paranoid belongs_to :enterprise, optional: false From e6c679cb34ed7b82052cad58be9799733ac670db Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Jul 2023 16:25:22 +1000 Subject: [PATCH 09/15] Apply belongs_to_required_by_default to all models Unless they state otherwise. The new standard also changes the default behaviour of the Shoulda matcher in Rspec. It now defaults to asserting that an association is required. That needed some spec updates. In one case, Spree::Product, I also had to update the model because the presence validation was somehow not recognised by the Shoulda matcher. The error message may change slightly but the outcome should be the same. --- app/models/spree/product.rb | 7 ++----- config/application.rb | 1 - spec/models/spree/adjustment_spec.rb | 8 ++++---- spec/models/spree/product_spec.rb | 4 ++-- spec/models/subscription_spec.rb | 14 +++++++------- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index c9ca6ca063..68437005e8 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -34,8 +34,8 @@ module Spree searchable_scopes :active, :with_properties belongs_to :shipping_category, class_name: 'Spree::ShippingCategory' - belongs_to :supplier, class_name: 'Enterprise', touch: true - belongs_to :primary_taxon, class_name: 'Spree::Taxon', touch: true + belongs_to :supplier, class_name: 'Enterprise', optional: false, touch: true + belongs_to :primary_taxon, class_name: 'Spree::Taxon', optional: false, touch: true has_one :image, class_name: "Spree::Image", as: :viewable, dependent: :destroy @@ -54,9 +54,6 @@ module Spree validates :name, presence: true validates :shipping_category, presence: true - validates :supplier, presence: true - validates :primary_taxon, presence: true - validates :variant_unit, presence: true validates :unit_value, presence: { if: ->(p) { %w(weight volume).include?(p.variant_unit) && new_record? } } diff --git a/config/application.rb b/config/application.rb index 3f01f13856..0d2ae298fc 100644 --- a/config/application.rb +++ b/config/application.rb @@ -227,7 +227,6 @@ module Openfoodnetwork # https://guides.rubyonrails.org/configuring.html#results-of-config-load-defaults config.load_defaults 6.1 config.action_view.form_with_generates_remote_forms = false - config.active_record.belongs_to_required_by_default = false config.active_record.cache_versioning = false config.active_record.has_many_inversing = false config.active_record.yaml_column_permitted_classes = [BigDecimal, Symbol] diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 5482d03024..f8e0388c67 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -11,10 +11,10 @@ module Spree it { is_expected.to have_one(:metadata) } it { is_expected.to have_many(:adjustments) } - it { is_expected.to belong_to(:adjustable) } - it { is_expected.to belong_to(:originator) } - it { is_expected.to belong_to(:order) } - it { is_expected.to belong_to(:tax_category) } + it { is_expected.to belong_to(:adjustable).optional } + it { is_expected.to belong_to(:originator).optional } + it { is_expected.to belong_to(:order).optional } + it { is_expected.to belong_to(:tax_category).optional } end describe "scopes" do diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index dc2be5b509..d99fe291f9 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -154,8 +154,8 @@ module Spree end describe "associations" do - it { is_expected.to belong_to(:supplier) } - it { is_expected.to belong_to(:primary_taxon) } + it { is_expected.to belong_to(:supplier).required } + it { is_expected.to belong_to(:primary_taxon).required } end describe "validations and defaults" do diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index 06fee1dfcb..742e1952f1 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -4,13 +4,13 @@ require 'spec_helper' describe Subscription, type: :model do describe "associations" do - it { expect(subject).to belong_to(:shop) } - it { expect(subject).to belong_to(:customer) } - it { expect(subject).to belong_to(:schedule) } - it { expect(subject).to belong_to(:shipping_method) } - it { expect(subject).to belong_to(:payment_method) } - it { expect(subject).to belong_to(:ship_address) } - it { expect(subject).to belong_to(:bill_address) } + it { expect(subject).to belong_to(:shop).optional } + it { expect(subject).to belong_to(:customer).optional } + it { expect(subject).to belong_to(:schedule).optional } + it { expect(subject).to belong_to(:shipping_method).optional } + it { expect(subject).to belong_to(:payment_method).optional } + it { expect(subject).to belong_to(:ship_address).optional } + it { expect(subject).to belong_to(:bill_address).optional } it { expect(subject).to have_many(:subscription_line_items) } it { expect(subject).to have_many(:order_cycles) } it { expect(subject).to have_many(:proxy_orders) } From 4a13576f781c0f0c7f3ed7c4719f14f5518d80a4 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Jul 2023 16:33:52 +1000 Subject: [PATCH 10/15] Remove now superfluous belongs_to default declaration --- app/models/adjustment_metadata.rb | 2 -- app/models/column_preference.rb | 2 -- app/models/coordinator_fee.rb | 2 -- app/models/custom_tab.rb | 2 -- app/models/customer.rb | 2 -- app/models/distributor_payment_method.rb | 1 - app/models/distributor_shipping_method.rb | 1 - app/models/enterprise.rb | 2 -- app/models/enterprise_fee.rb | 2 -- app/models/enterprise_group.rb | 2 -- app/models/enterprise_relationship.rb | 2 -- app/models/enterprise_relationship_permission.rb | 2 -- app/models/enterprise_role.rb | 2 -- app/models/spree/shipping_method.rb | 2 -- 14 files changed, 26 deletions(-) diff --git a/app/models/adjustment_metadata.rb b/app/models/adjustment_metadata.rb index 4753592a46..eb99ad1de7 100644 --- a/app/models/adjustment_metadata.rb +++ b/app/models/adjustment_metadata.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class AdjustmentMetadata < ApplicationRecord - self.belongs_to_required_by_default = true - belongs_to :adjustment, class_name: 'Spree::Adjustment' belongs_to :enterprise end diff --git a/app/models/column_preference.rb b/app/models/column_preference.rb index adcc6999bd..3a08aa3a85 100644 --- a/app/models/column_preference.rb +++ b/app/models/column_preference.rb @@ -5,8 +5,6 @@ require 'open_food_network/column_preference_defaults' class ColumnPreference < ApplicationRecord extend OpenFoodNetwork::ColumnPreferenceDefaults - self.belongs_to_required_by_default = true - # Non-persisted attributes that only have one # setting (ie. the default) for a given column attr_accessor :name diff --git a/app/models/coordinator_fee.rb b/app/models/coordinator_fee.rb index 4a69085e03..b308c611b1 100644 --- a/app/models/coordinator_fee.rb +++ b/app/models/coordinator_fee.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class CoordinatorFee < ApplicationRecord - self.belongs_to_required_by_default = true - belongs_to :order_cycle belongs_to :enterprise_fee end diff --git a/app/models/custom_tab.rb b/app/models/custom_tab.rb index 321ae0a5a2..fca7ffa4ac 100644 --- a/app/models/custom_tab.rb +++ b/app/models/custom_tab.rb @@ -1,8 +1,6 @@ # frozen_string_literal: false class CustomTab < ApplicationRecord - self.belongs_to_required_by_default = true - belongs_to :enterprise validates :title, presence: true, length: { maximum: 20 } diff --git a/app/models/customer.rb b/app/models/customer.rb index ab357dc8ea..94fe138491 100644 --- a/app/models/customer.rb +++ b/app/models/customer.rb @@ -10,8 +10,6 @@ class Customer < ApplicationRecord include SetUnusedAddressFields - self.belongs_to_required_by_default = true - acts_as_taggable searchable_attributes :first_name, :last_name, :email, :code diff --git a/app/models/distributor_payment_method.rb b/app/models/distributor_payment_method.rb index 57287f4422..7712b9bcea 100644 --- a/app/models/distributor_payment_method.rb +++ b/app/models/distributor_payment_method.rb @@ -2,7 +2,6 @@ class DistributorPaymentMethod < ApplicationRecord self.table_name = "distributors_payment_methods" - self.belongs_to_required_by_default = true belongs_to :payment_method, class_name: "Spree::PaymentMethod", touch: true belongs_to :distributor, class_name: "Enterprise", touch: true diff --git a/app/models/distributor_shipping_method.rb b/app/models/distributor_shipping_method.rb index bb09eaec5f..5d0b943ef1 100644 --- a/app/models/distributor_shipping_method.rb +++ b/app/models/distributor_shipping_method.rb @@ -2,7 +2,6 @@ class DistributorShippingMethod < ApplicationRecord self.table_name = "distributors_shipping_methods" - self.belongs_to_required_by_default = true belongs_to :shipping_method, class_name: "Spree::ShippingMethod", touch: true belongs_to :distributor, class_name: "Enterprise", touch: true diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 7ed6016a59..450ff29666 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -10,8 +10,6 @@ class Enterprise < ApplicationRecord WHITE_LABEL_LOGO_SIZES = [:default, :mobile].freeze VALID_INSTAGRAM_REGEX = %r{\A[a-zA-Z0-9._]{1,30}([^/-]*)\z} - self.belongs_to_required_by_default = true - searchable_attributes :sells, :is_primary_producer, :name searchable_associations :properties searchable_scopes :is_primary_producer, :is_distributor, :is_hub, :activated, :visible, diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 5abaf2bcb6..fdbdb82a23 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -3,8 +3,6 @@ class EnterpriseFee < ApplicationRecord include CalculatedAdjustments - self.belongs_to_required_by_default = true - acts_as_paranoid belongs_to :enterprise diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index 738cb2ad32..deadc531e3 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -5,8 +5,6 @@ require 'open_food_network/locking' class EnterpriseGroup < ApplicationRecord include PermalinkGenerator - self.belongs_to_required_by_default = true - acts_as_list has_and_belongs_to_many :enterprises, join_table: 'enterprise_groups_enterprises' diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index e76429c7e5..9093f00509 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class EnterpriseRelationship < ApplicationRecord - self.belongs_to_required_by_default = true - belongs_to :parent, class_name: 'Enterprise', touch: true belongs_to :child, class_name: 'Enterprise', touch: true has_many :permissions, class_name: 'EnterpriseRelationshipPermission', dependent: :destroy diff --git a/app/models/enterprise_relationship_permission.rb b/app/models/enterprise_relationship_permission.rb index 44ac935940..f92b10188f 100644 --- a/app/models/enterprise_relationship_permission.rb +++ b/app/models/enterprise_relationship_permission.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class EnterpriseRelationshipPermission < ApplicationRecord - self.belongs_to_required_by_default = true - belongs_to :enterprise_relationship default_scope { order('name') } before_destroy :destroy_related_exchanges diff --git a/app/models/enterprise_role.rb b/app/models/enterprise_role.rb index 9fe57584f6..402abc339c 100644 --- a/app/models/enterprise_role.rb +++ b/app/models/enterprise_role.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class EnterpriseRole < ApplicationRecord - self.belongs_to_required_by_default = true - belongs_to :user, class_name: "Spree::User" belongs_to :enterprise diff --git a/app/models/spree/shipping_method.rb b/app/models/spree/shipping_method.rb index 0e023413ab..d84cac35a8 100644 --- a/app/models/spree/shipping_method.rb +++ b/app/models/spree/shipping_method.rb @@ -8,8 +8,6 @@ module Spree back_end: "back_end" }.freeze - self.belongs_to_required_by_default = true - acts_as_paranoid acts_as_taggable From faeb8ae8f8750a6fcd16e813ac8ca9da123cf86a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Jul 2023 17:20:54 +1000 Subject: [PATCH 11/15] Fix Exchange clone tagging The validation of ActsAsTaggableOn::Taggable failed when trying to copy tag ids during cloning. But I found that it's totally unnecessary because `Exchange.dup` copies the `tag_list` attribute already and it gets saved correctly. There's an existing spec for this. --- app/models/exchange.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/exchange.rb b/app/models/exchange.rb index 8dbf3faf27..f244201398 100644 --- a/app/models/exchange.rb +++ b/app/models/exchange.rb @@ -90,7 +90,6 @@ class Exchange < ApplicationRecord exchange = dup exchange.order_cycle = new_order_cycle exchange.enterprise_fee_ids = enterprise_fee_ids - exchange.tag_ids = tag_ids exchange.save! clone_all_exchange_variants(exchange.id) exchange From 1b6eeb0928d53c28bdc4dc032cb83bcdc5837780 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 31 Jul 2023 17:10:54 +1000 Subject: [PATCH 12/15] Reduce line length of PaymentMethod class Rubocop was complaining. So I found code that could be simplified and specced it before refactoring. --- app/models/spree/payment_method.rb | 14 ++++++-------- spec/models/spree/payment_method_spec.rb | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index 6113291103..891f418eed 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -28,14 +28,12 @@ module Spree scope :production, -> { where(environment: 'production') } scope :managed_by, lambda { |user| - if user.has_spree_role?('admin') - where(nil) - else - joins(:distributors). - where('distributors_payment_methods.distributor_id IN (?)', - user.enterprises.select(&:id)). - select('DISTINCT spree_payment_methods.*') - end + return where(nil) if user.admin? + + joins(:distributors). + where('distributors_payment_methods.distributor_id IN (?)', + user.enterprises.select(&:id)). + select('DISTINCT spree_payment_methods.*') } scope :for_distributors, ->(distributors) { diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb index 8951832e56..d69895d934 100644 --- a/spec/models/spree/payment_method_spec.rb +++ b/spec/models/spree/payment_method_spec.rb @@ -6,6 +6,25 @@ class Spree::Gateway::Test < Spree::Gateway end describe Spree::PaymentMethod do + describe ".managed_by scope" do + subject! { create(:payment_method) } + let(:owner) { subject.distributors.first.owner } + let(:other_user) { create(:user) } + let(:admin) { create(:admin_user) } + + it "returns everything for admins" do + expect(Spree::PaymentMethod.managed_by(admin)).to eq [subject] + end + + it "returns payment methods of managed enterprises" do + expect(Spree::PaymentMethod.managed_by(owner)).to eq [subject] + end + + it "returns nothing for other users" do + expect(Spree::PaymentMethod.managed_by(other_user)).to eq [] + end + end + describe "#available" do let(:enterprise) { create(:enterprise) } From efbf1d6b8fec77212100db5a6393f4a8c3577f85 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 2 Aug 2023 13:14:28 +1000 Subject: [PATCH 13/15] Stabilise flaky assertion on PDF text The PDF reader can't always place text elements in the right place. So something that should be in one line can be in multiple lines and those lines can be in a different order. I'm asserting the moving parts separately as done in other places of this spec. --- spec/system/admin/invoice_print_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/system/admin/invoice_print_spec.rb b/spec/system/admin/invoice_print_spec.rb index 29945bf6a0..a33ab225e1 100644 --- a/spec/system/admin/invoice_print_spec.rb +++ b/spec/system/admin/invoice_print_spec.rb @@ -212,7 +212,9 @@ describe ' end it "displays $0.00 when a line item has no tax" do - expect(page).to have_content "#{Spree::Product.first.name} (1g) 1 $0.00 $12.54" + expect(page).to have_content Spree::Product.first.name + expect(page).to have_content "(1g)" + expect(page).to have_content "1 $0.00 $12.54" end it "displays the taxes correctly" do @@ -343,7 +345,9 @@ describe ' convert_pdf_to_page end it "displays $0.0 when a line item has no tax" do - expect(page).to have_content "#{Spree::Product.first.name} (1g) 1 $0.00 $12.54" + expect(page).to have_content Spree::Product.first.name + expect(page).to have_content "(1g)" + expect(page).to have_content "1 $0.00 $12.54" end it "displays the added tax on the GST colum" do From 146b09dc570f90158cc175c981bccc413e0bb62c Mon Sep 17 00:00:00 2001 From: Maikel Date: Thu, 3 Aug 2023 16:22:56 +1000 Subject: [PATCH 14/15] Spec EnterpriseFee tax_category association Co-authored-by: David Cook --- spec/models/enterprise_fee_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index cfbd5a9fee..892669e2ec 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' describe EnterpriseFee do describe "associations" do it { is_expected.to belong_to(:enterprise).required } + it { is_expected.to belong_to(:tax_category).optional } end describe "validations" do From a34a351ceeaa64d50f2a3d40d1da74e35a2000dc Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 7 Aug 2023 16:36:20 +1000 Subject: [PATCH 15/15] Stabilise flaky spec with deterministic row order --- .../orders_cycle_supplier_totals_report_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb b/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb index 43f767faf8..8ff0cd6087 100644 --- a/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb +++ b/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb @@ -95,6 +95,10 @@ describe Reporting::Reports::OrdersAndFulfillment::OrderCycleSupplierTotals do # This second line item will have a default a bigint value. order.line_items << create(:line_item) + # Create deterministic / aphabetical order of items: + order.line_items[0].variant.product.update!(name: "Apple") + order.line_items[1].variant.product.update!(name: "Banana") + # Generating the report used to raise: # > TypeError: no implicit conversion of BigDecimal into String expect(table_headers[4]).to eq "Total Units"