From 2846a5f0f65c249804809da5930e3044ab311581 Mon Sep 17 00:00:00 2001 From: elasticspoon Date: Wed, 1 May 2024 21:47:13 -0400 Subject: [PATCH 1/2] refactor: remove database seeding reliance medium models --- db/schema.rb | 2 +- spec/models/adjustment_spec.rb | 31 +++++----- spec/models/audit_spec.rb | 43 +++++++------- spec/models/barcode_item_spec.rb | 42 +++++++------- spec/models/base_item_spec.rb | 33 ++++------- spec/models/broadcast_announcement_spec.rb | 39 +++++++------ spec/models/distribution_spec.rb | 66 ++++++++++------------ spec/models/donation_site_spec.rb | 41 ++------------ spec/models/donation_spec.rb | 11 ++-- spec/models/inventory_item_spec.rb | 37 +++--------- spec/models/line_item_spec.rb | 22 ++------ spec/models/manufacturer_spec.rb | 15 ++--- spec/models/partner_spec.rb | 25 +++----- spec/models/purchase_spec.rb | 17 ++---- spec/models/request_spec.rb | 6 +- spec/models/transfer_spec.rb | 46 +++++++-------- spec/models/users_role_spec.rb | 2 +- spec/support/itemizable_shared_example.rb | 32 ++++++----- 18 files changed, 214 insertions(+), 296 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 34cf476eed..32461cf2ca 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_04_05_154448) do +ActiveRecord::Schema[7.0].define(version: 2024_04_19_225137) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/models/adjustment_spec.rb b/spec/models/adjustment_spec.rb index da460af1da..2c3a0a0bbf 100644 --- a/spec/models/adjustment_spec.rb +++ b/spec/models/adjustment_spec.rb @@ -11,10 +11,12 @@ # user_id :bigint # -RSpec.describe Adjustment, type: :model do +RSpec.describe Adjustment, type: :model, skip_seed: true do it_behaves_like "itemizable" # This mixes feature specs with model specs... idealy we do not want to do this # it_behaves_like "pagination" + # + let(:organization) { create(:organization, skip_items: true) } context "Validations >" do it "must belong to an organization" do @@ -26,9 +28,9 @@ end it "allows you to remove all the inventory that exists in the storage location" do - storage_location1 = create(:storage_location, organization: @organization) + storage_location1 = create(:storage_location) item1 = create(:item) - TestInventory.create_inventory(@organization, { + TestInventory.create_inventory(storage_location1.organization, { storage_location1.id => { item1.id => 10 } @@ -38,6 +40,9 @@ end context "Scopes >" do + let(:user) { create(:user, organization: organization) } + let(:organization_admin) { create(:organization_admin, organization: organization) } + it "`at_location` can filter out adjustments to a specific location" do adj1 = create(:adjustment) create(:adjustment) @@ -45,29 +50,29 @@ end it "`by_user` can filter out adjustments to a specific user" do - adj1 = create(:adjustment, user_id: @user.id) - create(:adjustment, user_id: @organization_admin.id) + adj1 = create(:adjustment, user_id: user.id) + create(:adjustment, user_id: organization_admin.id) expect(Adjustment.by_user(adj1.user_id).size).to eq(1) end end context "Methods >" do it "`self.storage_locations_adjusted_for` returns only storage_locations that are used in adjustments for one org" do - storage_location1 = create(:storage_location, organization: @organization) - storage_location2 = create(:storage_location, organization: @organization) - create(:storage_location, organization: @organization) - storage_location4 = create(:storage_location, organization: create(:organization)) - create(:adjustment, storage_location: storage_location1, organization: @organization) - create(:adjustment, storage_location: storage_location2, organization: @organization) + storage_location1 = create(:storage_location, organization: organization) + storage_location2 = create(:storage_location, organization: organization) + create(:storage_location, organization: organization) + storage_location4 = create(:storage_location, organization: create(:organization, skip_items: true)) + create(:adjustment, storage_location: storage_location1, organization: organization) + create(:adjustment, storage_location: storage_location2, organization: organization) create(:adjustment, storage_location: storage_location4, organization: storage_location4.organization) - expect(Adjustment.storage_locations_adjusted_for(@organization).to_a).to match_array([storage_location1, storage_location2]) + expect(Adjustment.storage_locations_adjusted_for(organization).to_a).to match_array([storage_location1, storage_location2]) end describe "split_difference" do it "returns two adjustment objects" do item = create(:item) storage_location = create(:storage_location, :with_items, item: item, item_quantity: 10) - TestInventory.create_inventory(@organization, { + TestInventory.create_inventory(organization, { storage_location.id => { create(:item).id => 10 } diff --git a/spec/models/audit_spec.rb b/spec/models/audit_spec.rb index 2a36e4bbce..761d18df60 100644 --- a/spec/models/audit_spec.rb +++ b/spec/models/audit_spec.rb @@ -14,18 +14,23 @@ require 'rails_helper' -RSpec.describe Audit, type: :model do +RSpec.describe Audit, type: :model, skip_seed: true do + let(:organization) { create(:organization, skip_items: true) } + it_behaves_like "itemizable" context "Validations >" do + let(:user) { create(:user, organization: organization) } + let(:organization_admin) { create(:organization_admin, organization: organization) } + it "must belong to an organization" do expect(build(:audit, storage_location: create(:storage_location), organization_id: nil)).not_to be_valid - expect(build(:audit, organization: @organization)).to be_valid + expect(build(:audit, organization: organization)).to be_valid end it "must belong to an organization admin of its organization" do - expect(build(:audit, storage_location: create(:storage_location, organization: @organization), user: @user, organization: @organization)).not_to be_valid - expect(build(:audit, storage_location: create(:storage_location, organization: @organization), user: @organization_admin, organization: @organization)).to be_valid + expect(build(:audit, storage_location: create(:storage_location, organization: organization), user: user, organization: organization)).not_to be_valid + expect(build(:audit, storage_location: create(:storage_location, organization: organization), user: organization_admin, organization: organization)).to be_valid end it "can not have line items that has quantity as negative integer" do @@ -101,28 +106,28 @@ context "Methods >" do it "`self.storage_locations_audited_for` returns only storage_locations that are used for one org" do - storage_location1 = create(:storage_location, organization: @organization) - storage_location2 = create(:storage_location, organization: @organization) - create(:storage_location, organization: @organization) - storage_location4 = create(:storage_location, organization: create(:organization)) - create(:audit, storage_location: storage_location1, organization: @organization) - create(:audit, storage_location: storage_location2, organization: @organization) + storage_location1 = create(:storage_location, organization: organization) + storage_location2 = create(:storage_location, organization: organization) + create(:storage_location, organization: organization) + storage_location4 = create(:storage_location, organization: create(:organization, skip_items: true)) + create(:audit, storage_location: storage_location1, organization: organization) + create(:audit, storage_location: storage_location2, organization: organization) create(:audit, storage_location: storage_location4, organization: storage_location4.organization) - expect(Audit.storage_locations_audited_for(@organization).to_a).to match_array([storage_location1, storage_location2]) + expect(Audit.storage_locations_audited_for(organization).to_a).to match_array([storage_location1, storage_location2]) end it "`self.finalized_since?` returns true iff some finalized audit occurred after itemizable created_at that shares item for location(s)" do - storage_location1 = create(:storage_location, :with_items, item_quantity: 10, organization: @organization) - storage_location2 = create(:storage_location, :with_items, item_quantity: 10, organization: @organization) - storage_location3 = create(:storage_location, :with_items, item_quantity: 10, organization: @organization) - storage_location4 = create(:storage_location, organization: @organization) - storage_location5 = create(:storage_location, :with_items, item_quantity: 10, organization: @organization) + storage_location1 = create(:storage_location, :with_items, item_quantity: 10, organization: organization) + storage_location2 = create(:storage_location, :with_items, item_quantity: 10, organization: organization) + storage_location3 = create(:storage_location, :with_items, item_quantity: 10, organization: organization) + storage_location4 = create(:storage_location, organization: organization) + storage_location5 = create(:storage_location, :with_items, item_quantity: 10, organization: organization) create(:audit, storage_location: storage_location2, status: "finalized", line_items_attributes: [{item_id: storage_location2.items.first.id, quantity: 10}]) - xfer1 = create(:transfer, :with_items, item_quantity: 5, item: storage_location1.items.first, from: storage_location1, to: storage_location2, organization: @organization) - xfer2 = create(:transfer, :with_items, item_quantity: 5, item: storage_location1.items.first, from: storage_location1, to: storage_location3, organization: @organization) - xfer3 = create(:transfer, :with_items, item_quantity: 10, item: storage_location2.items.first, from: storage_location2, to: storage_location3, organization: @organization) + xfer1 = create(:transfer, :with_items, item_quantity: 5, item: storage_location1.items.first, from: storage_location1, to: storage_location2, organization: organization) + xfer2 = create(:transfer, :with_items, item_quantity: 5, item: storage_location1.items.first, from: storage_location1, to: storage_location3, organization: organization) + xfer3 = create(:transfer, :with_items, item_quantity: 10, item: storage_location2.items.first, from: storage_location2, to: storage_location3, organization: organization) create(:audit, storage_location: storage_location1, status: "finalized", line_items_attributes: [{item_id: storage_location1.items.first.id, quantity: 5}]) create(:audit, storage_location: storage_location3, status: "finalized", line_items_attributes: [{item_id: storage_location3.items.first.id, quantity: 10}]) diff --git a/spec/models/barcode_item_spec.rb b/spec/models/barcode_item_spec.rb index 15df046aab..e54446d3ec 100644 --- a/spec/models/barcode_item_spec.rb +++ b/spec/models/barcode_item_spec.rb @@ -40,7 +40,9 @@ end end -RSpec.describe BarcodeItem, type: :model do +RSpec.describe BarcodeItem, type: :model, skip_seed: true do + let(:organization) { create(:organization, skip_items: true) } + context "Barcodes of BaseItems ('Global')" do let(:base_item) { create(:base_item) } let(:global_barcode_item) { create(:global_barcode_item, barcodeable: base_item) } @@ -94,8 +96,7 @@ context "validations >" do it "is valid with or without an organization" do expect(build(:global_barcode_item, organization: nil)).to be_valid - org = Organization.try(:first) || create(:organization) - expect(build(:global_barcode_item, organization: org)).to be_valid + expect(build(:global_barcode_item, organization: organization)).to be_valid end it "enforces uniqueness in the global scope" do @@ -104,7 +105,7 @@ end it "allows multiple barcodes to point at the same base item" do - base_item = BaseItem.first + base_item = create(:base_item) create(:global_barcode_item, barcodeable: base_item) expect(build(:global_barcode_item, barcodeable: base_item)).to be_valid end @@ -114,7 +115,7 @@ end context "Organization barcodes" do - let(:item) { create(:item) } + let(:item) { create(:item, name: "First Item") } let(:barcode_item) { create(:barcode_item, barcodeable: item) } it "updates a counter in Item whenever it tracks a new barcode" do @@ -134,14 +135,15 @@ context "scopes >" do it "->for_csv_export will accept an organization and provide all barcodes for that org" do barcode_item - create(:barcode_item, organization: create(:organization)) + create(:barcode_item, organization: create(:organization, skip_items: true)) results = BarcodeItem.for_csv_export(barcode_item.organization) expect(results).to eq([barcode_item]) end it "#by_item_partner_key returns barcodes that match the partner key" do - i1 = create(:item, base_item: BaseItem.first) - i2 = create(:item, base_item: BaseItem.last) + bases = create_list(:base_item, 2) + i1 = create(:item, name: "Item 1", base_item: bases.first) + i2 = create(:item, name: "Item 2", base_item: bases.last) b1 = create(:barcode_item, barcodeable: i1) create(:barcode_item, barcodeable: i2) expect(BarcodeItem.by_item_partner_key(i1.partner_key).first).to eq(b1) @@ -156,11 +158,11 @@ context "when searching for a barcode where there is a global and local with the same value" do let!(:base_item) { create(:base_item, partner_key: "foo", name: "base item") } - let!(:item) { create(:item, partner_key: "foo", name: "custom item", organization: @organization) } + let!(:item) { create(:item, partner_key: "foo", name: "custom item", organization: organization) } let!(:other_item) { create(:item, partner_key: "foo", name: "other item", organization: create(:organization, skip_items: true)) } let!(:global) { create(:global_barcode_item, value: "DEADBEEF", barcodeable: base_item) } - let!(:local) { create(:barcode_item, value: "DEADBEEF", barcodeable: item, organization: @organization) } + let!(:local) { create(:barcode_item, value: "DEADBEEF", barcodeable: item, organization: organization) } let!(:other_local) { create(:barcode_item, value: "DEADBEEF", barcodeable: other_item, organization: other_item.organization) } it "favors the local barcode" do @@ -170,31 +172,27 @@ end context "validations >" do - it "is valid only with an organization" do - expect(build(:barcode_item, organization: nil)).not_to be_valid - org = Organization.try(:first) || create(:organization) - expect(build(:barcode_item, organization: org)).to be_valid - end + it { should validate_presence_of(:organization) } it "does not enforces value uniqueness across organizations" do - barcode = create(:barcode_item, value: "DEADBEEF", organization: @organization) + barcode = create(:barcode_item, value: "DEADBEEF", organization: organization) expect(build(:barcode_item, value: barcode.value, organization: create(:organization, skip_items: true))).to be_valid end it "enforces value uniqueness within the organization" do - barcode = create(:barcode_item, value: "DEADBEEF", organization: @organization) - expect(build(:barcode_item, value: barcode.value, organization: @organization)).not_to be_valid + barcode = create(:barcode_item, value: "DEADBEEF", organization: organization) + expect(build(:barcode_item, value: barcode.value, organization: organization)).not_to be_valid end it "does not enforce value uniqueness compared with the global scope" do barcode = create(:global_barcode_item, value: "DEADBEEF") - expect(build(:barcode_item, value: barcode.value, organization: @organization)).to be_valid + expect(build(:barcode_item, value: barcode.value, organization: organization)).to be_valid end it "allows multiple barcodes to point at the same item" do - item = create(:item, organization: @organization) - create(:barcode_item, organization: @organization, barcodeable: item) - expect(build(:barcode_item, organization: @organization, barcodeable: item)).to be_valid + item = create(:item, organization: organization) + create(:barcode_item, organization: organization, barcodeable: item) + expect(build(:barcode_item, organization: organization, barcodeable: item)).to be_valid end include_examples "common barcode tests", :barcode_item diff --git a/spec/models/base_item_spec.rb b/spec/models/base_item_spec.rb index 9d9a17185c..c4dcb7f505 100644 --- a/spec/models/base_item_spec.rb +++ b/spec/models/base_item_spec.rb @@ -15,37 +15,23 @@ require "rails_helper" -RSpec.describe BaseItem, type: :model do - describe "Validations >" do - it "is invalid without a name" do - expect(build(:base_item, name: nil)).not_to be_valid - end - - it "is invalid without a unique name" do - f = create(:base_item) - expect(build(:base_item, name: f.name)).not_to be_valid - end - - it "is invalid without a partner key" do - expect(build(:base_item, partner_key: nil)).not_to be_valid - end +RSpec.describe BaseItem, type: :model, skip_seed: true do + let(:organization) { create(:organization, skip_items: true) } - it "is invalid without a uniqueness key" do - f = create(:base_item) - expect(build(:base_item, partner_key: f.partner_key)).not_to be_valid - end + describe "Validations >" do + it { should validate_presence_of(:name) } + it { should validate_uniqueness_of(:name) } + it { should validate_presence_of(:partner_key) } + it { should validate_uniqueness_of(:partner_key) } end describe "Associations >" do it "keeps count of its associated items" do - c = BaseItem.first + c = create(:base_item, name: "Base", item_count: 0) expect { create_list(:item, 2, base_item: c) }.to change { c.item_count }.by(2) end end - describe "Methods >" do - end - describe "Filtering >" do describe '->without_kit' do subject { BaseItem.without_kit } @@ -58,7 +44,8 @@ describe "->by_partner_key" do it "shows the Base Items by partner_key" do - expect(BaseItem.by_partner_key(BaseItem.first.partner_key).size).to eq(1) + base_item = create(:base_item) + expect(BaseItem.by_partner_key(base_item.partner_key).size).to eq(1) expect(BaseItem.by_partner_key("random_string").size).to eq(0) end end diff --git a/spec/models/broadcast_announcement_spec.rb b/spec/models/broadcast_announcement_spec.rb index 50e3ab2125..ca7a212832 100644 --- a/spec/models/broadcast_announcement_spec.rb +++ b/spec/models/broadcast_announcement_spec.rb @@ -13,7 +13,7 @@ # require "rails_helper" -RSpec.describe BroadcastAnnouncement, type: :model do +RSpec.describe BroadcastAnnouncement, type: :model, skip_seed: true do it { should belong_to(:organization).optional } it { should belong_to(:user) } it { should validate_presence_of(:message) } @@ -46,30 +46,35 @@ end context "filter_announcements" do + let(:organization) { create(:organization, skip_items: true) } + let(:user) { create(:user, organization: organization) } + let(:user_id) { user.id } + let(:organization_id) { organization.id } + it "should include only announcements from the passed organization" do - BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id) - BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id) - BroadcastAnnouncement.create!(message: "test", user_id: @user.id) - BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: 0) - expect(BroadcastAnnouncement.filter_announcements(@organization.id).count).to eq(2) + create(:broadcast_announcement, message: "test", user_id: user_id, organization_id: organization_id) + create(:broadcast_announcement, message: "test", user_id: user_id, organization_id: organization_id) + create(:broadcast_announcement, message: "test", user_id: user_id) + create(:broadcast_announcement, message: "test", user_id: user_id, organization_id: create(:organization, skip_items: true).id) + expect(BroadcastAnnouncement.filter_announcements(organization_id).count).to eq(2) end it "shouldn't include expired announcements" do - BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id) - BroadcastAnnouncement.create!(message: "test", user_id: @user.id, expiry: 2.days.ago, organization_id: @organization.id) - BroadcastAnnouncement.create!(message: "test", user_id: @user.id, expiry: 5.days.ago, organization_id: @organization.id) - BroadcastAnnouncement.create!(message: "test", user_id: @user.id, expiry: Time.zone.today, organization_id: @organization.id) - expect(BroadcastAnnouncement.filter_announcements(@organization.id).count).to eq(2) + create(:broadcast_announcement, message: "test", user_id: user_id, organization_id: organization_id) + create(:broadcast_announcement, message: "test", user_id: user_id, expiry: 2.days.ago, organization_id: organization_id) + create(:broadcast_announcement, message: "test", user_id: user_id, expiry: 5.days.ago, organization_id: organization_id) + create(:broadcast_announcement, message: "test", user_id: user_id, expiry: Time.zone.today, organization_id: organization_id) + expect(BroadcastAnnouncement.filter_announcements(organization_id).count).to eq(2) end it "sorts announcements from most recently created to last" do - announcement_1 = BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id, created_at: Time.zone.today) - announcement_2 = BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id, created_at: 2.days.ago) - announcement_3 = BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id, expiry: 1.day.ago, created_at: 3.days.ago) - announcement_4 = BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id, created_at: 4.days.ago) + announcement_1 = create(:broadcast_announcement, message: "test", user_id: user_id, organization_id: organization_id, created_at: Time.zone.today) + announcement_2 = create(:broadcast_announcement, message: "test", user_id: user_id, organization_id: organization_id, created_at: 2.days.ago) + announcement_3 = create(:broadcast_announcement, message: "test", user_id: user_id, organization_id: organization_id, expiry: 1.day.ago, created_at: 3.days.ago) + announcement_4 = create(:broadcast_announcement, message: "test", user_id: user_id, organization_id: organization_id, created_at: 4.days.ago) - expect(BroadcastAnnouncement.filter_announcements(@organization.id)).to eq([announcement_1, announcement_2, announcement_4]) - expect(BroadcastAnnouncement.filter_announcements(@organization.id).include?(announcement_3)).to be(false) + expect(BroadcastAnnouncement.filter_announcements(organization_id)).to eq([announcement_1, announcement_2, announcement_4]) + expect(BroadcastAnnouncement.filter_announcements(organization_id).include?(announcement_3)).to be(false) end end diff --git a/spec/models/distribution_spec.rb b/spec/models/distribution_spec.rb index 635e93e851..259a8798aa 100644 --- a/spec/models/distribution_spec.rb +++ b/spec/models/distribution_spec.rb @@ -17,26 +17,22 @@ # storage_location_id :integer # -RSpec.describe Distribution, type: :model do +RSpec.describe Distribution, type: :model, skip_seed: true do + let(:organization) { create(:organization, skip_items: true) } + it_behaves_like "itemizable" context "Validations >" do - it "must belong to an organization" do - expect(build(:distribution, organization: nil)).not_to be_valid - end - it "requires a storage location" do - expect(build(:distribution, storage_location: nil)).not_to be_valid - end - - it "requires a partner" do - expect(build(:distribution, partner: nil)).not_to be_valid - end + it { should validate_presence_of(:organization) } + it { should validate_presence_of(:partner) } + it { should validate_presence_of(:storage_location) } it "ensures the associated line_items are valid" do - storage_location = create(:storage_location) + organization = create(:organization, skip_items: true) + storage_location = create(:storage_location, organization: organization) d = build(:distribution, storage_location: storage_location) - line_item = build(:line_item, quantity: 1) - TestInventory.create_inventory(@organization, { + line_item = build(:line_item, quantity: 1, item: create(:item, organization: organization)) + TestInventory.create_inventory(organization, { storage_location.id => { line_item.item_id => 10 } }) d.line_items << line_item @@ -56,7 +52,7 @@ end it "ensures that any included items are found in the associated storage location" do - unless Event.read_events?(@organization) # not relevant in event world + unless Event.read_events?(organization) # not relevant in event world d = build(:distribution) item_missing = create(:item, name: "missing") d.line_items << build(:line_item, item: item_missing) @@ -118,8 +114,8 @@ end it "doesn't include distributions past Sunday" do - sunday_distribution = create(:distribution, organization: @organization, issued_at: Time.zone.local(2019, 6, 30)) - create(:distribution, organization: @organization, issued_at: Time.zone.local(2019, 7, 1)) + sunday_distribution = create(:distribution, organization: organization, issued_at: Time.zone.local(2019, 6, 30)) + create(:distribution, organization: organization, issued_at: Time.zone.local(2019, 7, 1)) distributions = Distribution.this_week expect(distributions.count).to eq(1) expect(distributions.first).to eq(sunday_distribution) @@ -136,9 +132,9 @@ end it "includes distributions as early as Monday and as late as upcoming Sunday" do - create(:distribution, organization: @organization, issued_at: Time.zone.local(2019, 6, 30)) - tuesday_distribution = create(:distribution, organization: @organization, issued_at: Time.zone.local(2019, 7, 2)) - sunday_distribution = create(:distribution, organization: @organization, issued_at: Time.zone.local(2019, 7, 7)) + create(:distribution, organization: organization, issued_at: Time.zone.local(2019, 6, 30)) + tuesday_distribution = create(:distribution, organization: organization, issued_at: Time.zone.local(2019, 7, 2)) + sunday_distribution = create(:distribution, organization: organization, issued_at: Time.zone.local(2019, 7, 7)) distributions = Distribution.this_week expect(distributions.count).to eq(2) expect(distributions.first).to eq(tuesday_distribution) @@ -250,11 +246,11 @@ describe "#copy_from_request" do it "copy over relevant request information into the distrubution" do - item1 = create(:item, name: "Item1") - item2 = create(:item, name: "Item2") + item1 = create(:item, name: "Item1", organization: organization) + item2 = create(:item, name: "Item2", organization: organization) request = create(:request, - organization: @organization, - partner_user: ::User.partner_users.first, + organization: organization, + partner_user: create(:partner_user), request_items: [ { item_id: item1.id, quantity: 15 }, { item_id: item2.id, quantity: 18 } @@ -264,7 +260,7 @@ expect(distribution.line_items.size).to eq 2 expect(distribution.line_items.first.quantity).to eq 15 expect(distribution.line_items.second.quantity).to eq 18 - expect(distribution.organization_id).to eq @organization.id + expect(distribution.organization_id).to eq organization.id expect(distribution.partner_id).to eq request.partner_id expect(distribution.agency_rep).to eq "#{request.partner_user.name} <#{request.partner_user.email}>" expect(distribution.comment).to eq request.comments @@ -291,30 +287,30 @@ end context "CSV export >" do - let(:organization_2) { create(:organization) } - let(:item1) { create(:item) } - let(:item2) { create(:item) } - let!(:distribution_1) { create(:distribution, :with_items, item: item1, organization: @organization, issued_at: 3.days.ago) } - let!(:distribution_2) { create(:distribution, :with_items, item: item2, organization: @organization, issued_at: 1.day.ago) } + let(:organization_2) { create(:organization, skip_items: true) } + let(:item1) { create(:item, organization: organization) } + let(:item2) { create(:item, organization: organization) } + let!(:distribution_1) { create(:distribution, :with_items, item: item1, organization: organization, issued_at: 3.days.ago) } + let!(:distribution_2) { create(:distribution, :with_items, item: item2, organization: organization, issued_at: 1.day.ago) } let!(:distribution_3) { create(:distribution, organization: organization_2, issued_at: Time.zone.today) } describe "for_csv_export >" do it "filters only to the given organization" do - expect(Distribution.for_csv_export(@organization)).to match_array [distribution_1, distribution_2] + expect(Distribution.for_csv_export(organization)).to match_array [distribution_1, distribution_2] end it "filters only to the given filter" do - expect(Distribution.for_csv_export(@organization, { by_item_id: item1.id })).to match_array [distribution_1] + expect(Distribution.for_csv_export(organization, { by_item_id: item1.id })).to match_array [distribution_1] end it "filters only to the given issue time range" do - expect(Distribution.for_csv_export(@organization, {}, 4.days.ago..2.days.ago)).to match_array [distribution_1] + expect(Distribution.for_csv_export(organization, {}, 4.days.ago..2.days.ago)).to match_array [distribution_1] end end describe "csv_export_attributes" do - let(:item) { create(:item) } - let!(:distribution) { create(:distribution, :with_items, item: item, organization: @organization, issued_at: 3.days.ago) } + let(:item) { create(:item, organization: organization) } + let!(:distribution) { create(:distribution, :with_items, item: item, organization: organization, issued_at: 3.days.ago) } it "returns the set of attributes which define a row in case of distribution export" do distribution_details = [distribution].map(&:csv_export_attributes).first diff --git a/spec/models/donation_site_spec.rb b/spec/models/donation_site_spec.rb index becda45323..52b180b09c 100644 --- a/spec/models/donation_site_spec.rb +++ b/spec/models/donation_site_spec.rb @@ -16,46 +16,15 @@ # organization_id :integer # -RSpec.describe DonationSite, type: :model do +RSpec.describe DonationSite, type: :model, skip_seed: true do context "Validations >" do - it "must belong to an organization" do - expect(build(:donation_site, organization_id: nil)).not_to be_valid - end - it "is invalid without a name" do - expect(build(:donation_site, name: nil)).not_to be_valid - end - - it "is invalid without an address" do - expect(build(:donation_site, address: nil)).not_to be_valid - end - - it "is valid without an contact name" do - expect(build(:donation_site, contact_name: nil)).to be_valid - end - - it "is valid without an phone" do - expect(build(:donation_site, phone: nil)).to be_valid - end - - it "is valid without an email" do - expect(build(:donation_site, email: nil)).to be_valid - end - - it "is valid with Contact Name" do - expect(build(:donation_site, contact_name: "Mr. Smith")).to be_valid - end - - it "is valid with an email" do - expect(build(:donation_site, email: "smith@mail.com")).to be_valid - end - - it "is valid with a phone number" do - expect(build(:donation_site, phone: "555-555-1234")).to be_valid - end + it { should belong_to(:organization) } + it { should validate_presence_of(:name) } + it { should validate_presence_of(:address) } end describe "import_csv" do it "imports storage locations from a csv file" do - organization = create(:organization) + organization = create(:organization, skip_items: true) import_file_path = Rails.root.join("spec", "fixtures", "files", "donation_sites.csv") data = File.read(import_file_path, encoding: "BOM|UTF-8") csv = CSV.parse(data, headers: true) diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb index 162b7d5004..c6d10905a0 100644 --- a/spec/models/donation_spec.rb +++ b/spec/models/donation_spec.rb @@ -17,15 +17,15 @@ # storage_location_id :integer # -RSpec.describe Donation, type: :model do +RSpec.describe Donation, type: :model, skip_seed: true do it_behaves_like "itemizable" # This mixes feature specs with model specs... idealy we do not want to do this # it_behaves_like "pagination" context "Validations >" do - it "must belong to an organization" do - expect(build(:donation, organization_id: nil)).not_to be_valid - end + it { should belong_to(:organization) } + it { should belong_to(:storage_location) } + it "requires a donation_site if the source is 'Donation Site'" do expect(build_stubbed(:donation_site_donation, source: "Donation Site", donation_site: nil)).not_to be_valid expect(build(:donation, source: "Misc. Donation", donation_site: nil)).to be_valid @@ -45,9 +45,6 @@ expect(build(:donation, source: nil)).not_to be_valid expect(build(:donation, source: "Something new")).not_to be_valid end - it "requires an inventory (storage location)" do - expect(build(:donation, storage_location_id: nil)).not_to be_valid - end it "is invalid when the line items are invalid" do d = build(:donation) d.line_items << build(:line_item, quantity: nil) diff --git a/spec/models/inventory_item_spec.rb b/spec/models/inventory_item_spec.rb index 8cbfee0cda..43724887a8 100644 --- a/spec/models/inventory_item_spec.rb +++ b/spec/models/inventory_item_spec.rb @@ -10,31 +10,15 @@ # storage_location_id :integer # -RSpec.describe InventoryItem, type: :model do +RSpec.describe InventoryItem, type: :model, skip_seed: true do context "Validations >" do describe "quantity >" do - it "is required" do - expect(build(:inventory_item, quantity: nil)).not_to be_valid - end - - it "is numerical" do - expect(build(:inventory_item, quantity: "a")).not_to be_valid - end - - it "is gte 0" do - expect(build(:inventory_item, quantity: -1)).not_to be_valid - expect(create(:inventory_item, quantity: 0)).to be_valid - end - - it "is less than the max integer" do - expect(build(:inventory_item, quantity: 2**31)).not_to be_valid - end - end - it "requires an inventory association" do - expect(build(:inventory_item, storage_location_id: nil)).not_to be_valid - end - it "requires an item" do - expect(build(:inventory_item, item_id: nil)).not_to be_valid + it { should validate_presence_of(:quantity) } + it { should validate_numericality_of(:quantity) } + it { should validate_numericality_of(:quantity).is_greater_than_or_equal_to(0) } + it { should validate_numericality_of(:quantity).is_less_than(2**31) } + it { should validate_presence_of(:storage_location_id) } + it { should validate_presence_of(:item_id) } end end @@ -44,12 +28,9 @@ context "Filtering >" do describe "->by_partner_key" do - before(:each) do - InventoryItem.delete_all - @item1 = create(:inventory_item) - end it "shows the Base Items by partner_key" do - expect(InventoryItem.by_partner_key(@item1.item.partner_key).size).to eq(1) + item = create(:inventory_item) + expect(InventoryItem.by_partner_key(item.item.partner_key).size).to eq(1) end end end diff --git a/spec/models/line_item_spec.rb b/spec/models/line_item_spec.rb index 1fe9ac6a03..40787f5ba1 100644 --- a/spec/models/line_item_spec.rb +++ b/spec/models/line_item_spec.rb @@ -11,27 +11,15 @@ # itemizable_id :integer # -RSpec.describe LineItem, type: :model do +RSpec.describe LineItem, type: :model, skip_seed: true do let(:line_item) { build :line_item } context "Validations >" do - it "requires an item" do - expect(build(:line_item, item: nil)).not_to be_valid - end - - it "requires a quantity" do - expect(build(:line_item, quantity: nil)).not_to be_valid - end + subject { line_item } - it "the quantity must be a valid integer and cannot be 0" do - expect(build(:line_item, :purchase, quantity: "a")).not_to be_valid - expect(build(:line_item, :purchase, quantity: "1.0")).not_to be_valid - expect(build(:line_item, :purchase, quantity: 0)).to be_valid - expect(build(:line_item, :purchase, quantity: 2**31)).not_to be_valid - expect(build(:line_item, :purchase, quantity: -2**32)).not_to be_valid - expect(build_stubbed(:line_item, :purchase, quantity: -1)).to be_valid - expect(build_stubbed(:line_item, :purchase, quantity: 1)).to be_valid - end + it { should validate_numericality_of(:quantity).is_greater_than(-2**31) } + it { should validate_numericality_of(:quantity).is_less_than(2**31) } + it { should validate_numericality_of(:quantity).only_integer } end describe "package_count" do diff --git a/spec/models/manufacturer_spec.rb b/spec/models/manufacturer_spec.rb index 623723a8d2..f31904a319 100644 --- a/spec/models/manufacturer_spec.rb +++ b/spec/models/manufacturer_spec.rb @@ -11,12 +11,13 @@ require "rails_helper" -RSpec.describe Manufacturer, type: :model do +RSpec.describe Manufacturer, type: :model, skip_seed: true do context "Validations" do - it "must belong to an organization" do - expect(build(:manufacturer, organization: nil)).not_to be_valid - expect(build(:manufacturer, organization: create(:organization))).to be_valid - end + subject { build(:manufacturer) } + + it { should belong_to(:organization) } + it { should validate_presence_of(:name) } + it "must have a unique name within organization" do manufacturer = create(:manufacturer) expect(build(:manufacturer, name: nil)).not_to be_valid @@ -51,10 +52,10 @@ context "Private Methods" do describe "#exists_in_org?" do - let(:organization) { create(:organization) } + let(:organization) { create(:organization, skip_items: true) } it "returns true if manufacturer exists in an organization" do - manufacturer = FactoryBot.create(:manufacturer, organization_id: organization.id) + manufacturer = create(:manufacturer, organization_id: organization.id) expect(manufacturer.send(:exists_in_org?)).to eq(true) end end diff --git a/spec/models/partner_spec.rb b/spec/models/partner_spec.rb index ab3e5bfc93..7f71e33e9e 100644 --- a/spec/models/partner_spec.rb +++ b/spec/models/partner_spec.rb @@ -16,7 +16,7 @@ # partner_group_id :bigint # -RSpec.describe Partner, type: :model do +RSpec.describe Partner, type: :model, skip_seed: true do describe 'associations' do it { should belong_to(:organization) } it { should belong_to(:partner_group).optional } @@ -45,10 +45,6 @@ end context "Validations >" do - it "must belong to an organization" do - expect(build(:partner, organization_id: nil)).not_to be_valid - end - it "requires a unique name within an organization" do expect(build(:partner, name: nil)).not_to be_valid create(:partner, name: "Foo") @@ -73,10 +69,7 @@ expect(build(:partner, email: "boooooooooo")).not_to be_valid end - it "validates the quota is a number but it is not required" do - is_expected.to validate_numericality_of(:quota) - expect(build(:partner, email: "foo@bar.com", quota: "")).to be_valid - end + it { should validate_numericality_of(:quota).allow_nil } end context "callbacks" do @@ -259,7 +252,7 @@ end describe "import_csv" do - let(:organization) { create(:organization) } + let(:organization) { create(:organization, skip_items: true) } it "imports partners from a csv file and prevents multiple imports" do before_import = Partner.count @@ -342,15 +335,15 @@ describe "#impact_metrics" do subject { partner.impact_metrics } - let(:partner) { FactoryBot.create(:partner) } + let(:partner) { create(:partner) } context "when partner has related information" do - let!(:family1) { FactoryBot.create(:partners_family, guardian_zip_code: "45612-123", partner: partner) } - let!(:family2) { FactoryBot.create(:partners_family, guardian_zip_code: "45612-126", partner: partner) } - let!(:family3) { FactoryBot.create(:partners_family, guardian_zip_code: "45612-123", partner: partner) } + let!(:family1) { create(:partners_family, guardian_zip_code: "45612-123", partner: partner) } + let!(:family2) { create(:partners_family, guardian_zip_code: "45612-126", partner: partner) } + let!(:family3) { create(:partners_family, guardian_zip_code: "45612-123", partner: partner) } - let!(:child1) { FactoryBot.create_list(:partners_child, 2, family: family1) } - let!(:child2) { FactoryBot.create_list(:partners_child, 2, family: family3) } + let!(:child1) { create_list(:partners_child, 2, family: family1) } + let!(:child2) { create_list(:partners_child, 2, family: family3) } it { is_expected.to eq({families_served: 3, children_served: 4, family_zipcodes: 2, family_zipcodes_list: %w[45612-123 45612-126]}) } end diff --git a/spec/models/purchase_spec.rb b/spec/models/purchase_spec.rb index 1e17b2ed69..583240453a 100644 --- a/spec/models/purchase_spec.rb +++ b/spec/models/purchase_spec.rb @@ -18,21 +18,14 @@ # vendor_id :integer # -RSpec.describe Purchase, type: :model do +RSpec.describe Purchase, type: :model, skip_seed: true do it_behaves_like "itemizable" context "Validations >" do - it "must belong to an organization" do - expect(build(:purchase, organization_id: nil)).not_to be_valid - end - it "requires an inventory (storage location)" do - expect(build(:purchase, storage_location_id: nil)).not_to be_valid - end - it "is invalid when the line items are invalid" do - d = build(:purchase) - d.line_items << build(:line_item, quantity: nil) - expect(d).not_to be_valid - end + it { should belong_to(:organization) } + it { should belong_to(:storage_location) } + it { should belong_to(:vendor) } + it "is valid if categories have no values" do d = build(:purchase, amount_spent_in_cents: 450) expect(d).to be_valid diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb index a396f847ee..1e57c1edcf 100644 --- a/spec/models/request_spec.rb +++ b/spec/models/request_spec.rb @@ -16,7 +16,7 @@ # partner_user_id :integer # -RSpec.describe Request, type: :model do +RSpec.describe Request, type: :model, skip_seed: true do describe "Enums >" do describe "#status" do let!(:request_pending) { create(:request) } @@ -47,7 +47,9 @@ end describe "total_items" do - let(:request) { create(:request, request_items: [{ item_id: Item.active.first.id, quantity: 15 }, { item_id: Item.active.last.id, quantity: 18 }]) } + let(:id_one) { create(:item).id } + let(:id_two) { create(:item).id } + let(:request) { create(:request, request_items: [{ item_id: id_one, quantity: 15 }, { item_id: id_two, quantity: 18 }]) } it "adds the quantity of all items in the request" do expect(request.total_items).to eq(33) diff --git a/spec/models/transfer_spec.rb b/spec/models/transfer_spec.rb index 8f013430c8..b89f00e349 100644 --- a/spec/models/transfer_spec.rb +++ b/spec/models/transfer_spec.rb @@ -11,23 +11,15 @@ # to_id :integer # -RSpec.describe Transfer, type: :model do +RSpec.describe Transfer, type: :model, skip_seed: true do + let(:organization) { create(:organization, skip_items: true) } + it_behaves_like "itemizable" context "Validations >" do - it "must belong to an organization" do - expect(build(:transfer, organization_id: nil)).not_to be_valid - end - - it "must have storage locations set" do - transfer = build(:transfer) - old_from = transfer.from - transfer.from = nil # Doing this separately so the after hook doesn't fix it - expect(transfer).not_to be_valid - transfer.from = old_from - transfer.to = nil - expect(transfer).not_to be_valid - end + it { should belong_to(:organization).inverse_of(:transfers) } + it { should belong_to(:from).class_name("StorageLocation").inverse_of(:transfers_from) } + it { should belong_to(:to).class_name("StorageLocation").inverse_of(:transfers_to) } it "must have different storage locations" do transfer = build(:transfer) @@ -37,7 +29,7 @@ it "must only use storage locations that belong to the organization (no hacks!)" do transfer = build(:transfer) - other_org = create(:organization) + other_org = create(:organization, skip_items: true) other_storage = create(:storage_location, organization: other_org) transfer.from = other_storage expect(transfer).not_to be_valid @@ -66,29 +58,29 @@ context "Scopes >" do it "`from_location` can filter out transfers from a specific location" do - xfer1 = create(:transfer, organization: @organization) - create(:transfer, organization: @organization) + xfer1 = create(:transfer, organization: organization) + create(:transfer, organization: organization) expect(Transfer.from_location(xfer1.from_id).size).to eq(1) end it "`to_location` can filter out transfers to a specific location" do - xfer1 = create(:transfer, organization: @organization) - create(:transfer, organization: @organization) + xfer1 = create(:transfer, organization: organization) + create(:transfer, organization: organization) expect(Transfer.to_location(xfer1.to_id).size).to eq(1) end end context "Methods >" do it "`self.storage_locations_transferred_to` and `..._from` constrains appropriately" do - storage_location1 = create(:storage_location, name: "loc1", organization: @organization) - storage_location2 = create(:storage_location, name: "loc2", organization: @organization) - storage_location3 = create(:storage_location, name: "loc3", organization: @organization) - storage_location4 = create(:storage_location, name: "loc4", organization: create(:organization)) + storage_location1 = create(:storage_location, name: "loc1", organization: organization) + storage_location2 = create(:storage_location, name: "loc2", organization: organization) + storage_location3 = create(:storage_location, name: "loc3", organization: organization) + storage_location4 = create(:storage_location, name: "loc4", organization: create(:organization, skip_items: true)) storage_location5 = create(:storage_location, name: "loc5", organization: storage_location4.organization) - create(:transfer, from: storage_location3, to: storage_location1, organization: @organization) - create(:transfer, from: storage_location3, to: storage_location2, organization: @organization) + create(:transfer, from: storage_location3, to: storage_location1, organization: organization) + create(:transfer, from: storage_location3, to: storage_location2, organization: organization) create(:transfer, from: storage_location5, to: storage_location4, organization: storage_location4.organization) - expect(Transfer.storage_locations_transferred_to_in(@organization).to_a).to match_array([storage_location1, storage_location2]) - expect(Transfer.storage_locations_transferred_from_in(@organization).to_a).to match_array([storage_location3]) + expect(Transfer.storage_locations_transferred_to_in(organization).to_a).to match_array([storage_location1, storage_location2]) + expect(Transfer.storage_locations_transferred_from_in(organization).to_a).to match_array([storage_location3]) end end diff --git a/spec/models/users_role_spec.rb b/spec/models/users_role_spec.rb index af0a6add7c..369e9e7f86 100644 --- a/spec/models/users_role_spec.rb +++ b/spec/models/users_role_spec.rb @@ -6,7 +6,7 @@ # role_id :bigint # user_id :bigint # -RSpec.describe UsersRole, type: :model do +RSpec.describe UsersRole, type: :model, skip_seed: true do describe "#current_role_for" do context "when last_role is nil" do context "for org user" do diff --git a/spec/support/itemizable_shared_example.rb b/spec/support/itemizable_shared_example.rb index 2c5df05254..1e6dd9b44e 100644 --- a/spec/support/itemizable_shared_example.rb +++ b/spec/support/itemizable_shared_example.rb @@ -1,13 +1,11 @@ -shared_examples_for "itemizable" do +shared_examples_for "itemizable", skip_seed: true do let(:model_f) { described_class.to_s.underscore.to_sym } + let(:organization) { create(:organization, skip_items: true) } let(:item) { create(:item) } - context ".items" do - end - describe ".to_a" do - let(:storage_location) { create(:storage_location, :with_items, item: item, organization: @organization) } - let(:obj) { build(model_f, storage_location: storage_location, organization: @organization) } + let(:storage_location) { create(:storage_location, :with_items, item: item, organization: organization) } + let(:obj) { build(model_f, storage_location: storage_location, organization: organization) } context "with Flipper flag :deprecate_to_a disabled" do it "calls line_item_values" do allow(Flipper).to receive(:enabled?).with(:deprecate_to_a).and_return(false) @@ -30,8 +28,10 @@ context ".line_items" do describe "combine!" do it "combines multiple line_items with the same item_id into a single record" do - storage_location = create(:storage_location, :with_items, item: item, organization: @organization) - obj = build(model_f, storage_location: storage_location, organization: @organization) + organization = create(:organization, :with_items) + item = create(:item, organization: organization) + storage_location = create(:storage_location, :with_items, item: item, organization: organization) + obj = build(model_f, storage_location: storage_location, organization: organization) 2.times { obj.line_items.build(item_id: item.id, quantity: 5) } @@ -48,7 +48,7 @@ it "incrementally combines line_items on donations that have already been created" do # Start with some items of one kind - obj = build(model_f, :with_items, item: item, item_quantity: 10, organization: @organization) + obj = build(model_f, :with_items, item: item, item_quantity: 10, organization: organization) obj.save # Add some additional of that item obj.line_items.build(item_id: item.id, quantity: 5) @@ -63,8 +63,8 @@ end describe "quantities_by_name" do - let(:item1) { create(:item, name: "item1", organization: @organization) } - let(:item2) { create(:item, name: "item2", organization: @organization) } + let(:item1) { create(:item, name: "item1", organization: organization) } + let(:item2) { create(:item, name: "item2", organization: organization) } subject do s = create(model_f) @@ -95,7 +95,10 @@ end describe "sorted" do - subject { create(model_f, organization: @organization) } + subject { + storage_location = create(:storage_location, organization: organization) + create(model_f, organization: organization, storage_location: storage_location) + } # the class that includes the concern it "displays the items, sorted by name" do names = %w(abc def ghi) @@ -109,7 +112,10 @@ end end describe "total" do - subject { create(model_f, organization: @organization) } # the class that includes the concern + subject { + storage_location = create(:storage_location, organization: organization) + create(model_f, organization: organization, storage_location: storage_location) + } # the class that includes the concern it "has an item total" do expect do From c1b104ead9990f0f41412bcad493154df166b1d7 Mon Sep 17 00:00:00 2001 From: elasticspoon Date: Fri, 3 May 2024 19:42:11 -0400 Subject: [PATCH 2/2] fix: using hard-coded partner key values --- spec/models/base_item_spec.rb | 4 ++-- spec/models/inventory_item_spec.rb | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/spec/models/base_item_spec.rb b/spec/models/base_item_spec.rb index c4dcb7f505..9c5d2b822b 100644 --- a/spec/models/base_item_spec.rb +++ b/spec/models/base_item_spec.rb @@ -44,8 +44,8 @@ describe "->by_partner_key" do it "shows the Base Items by partner_key" do - base_item = create(:base_item) - expect(BaseItem.by_partner_key(base_item.partner_key).size).to eq(1) + create(:base_item, partner_key: "UniqueString") + expect(BaseItem.by_partner_key("UniqueString").size).to eq(1) expect(BaseItem.by_partner_key("random_string").size).to eq(0) end end diff --git a/spec/models/inventory_item_spec.rb b/spec/models/inventory_item_spec.rb index 43724887a8..e072787076 100644 --- a/spec/models/inventory_item_spec.rb +++ b/spec/models/inventory_item_spec.rb @@ -28,9 +28,10 @@ context "Filtering >" do describe "->by_partner_key" do - it "shows the Base Items by partner_key" do - item = create(:inventory_item) - expect(InventoryItem.by_partner_key(item.item.partner_key).size).to eq(1) + it "shows the Inventory Items by partner_key" do + create(:base_item, partner_key: "UniqueString") + create(:inventory_item, item: create(:item, partner_key: "UniqueString")) + expect(InventoryItem.by_partner_key("UniqueString").size).to eq(1) end end end