From aa883c3085f6801aad4231ccbfb9346e3e77570e Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 22 Dec 2023 09:41:06 -0500 Subject: [PATCH 01/44] Do not allow inactive inventory --- app/controllers/items_controller.rb | 18 +++++++++---- app/controllers/kits_controller.rb | 8 ++++-- .../storage_locations_controller.rb | 2 +- app/models/item.rb | 13 +++++++++ app/models/kit.rb | 7 +++++ ...y_storage_collection_and_quantity_query.rb | 3 --- .../items_by_storage_collection_query.rb | 1 + app/views/audits/_form.html.erb | 2 +- app/views/distributions/_form.html.erb | 2 +- app/views/storage_locations/_source.html.erb | 2 -- ...20231222140700_remove_invalid_inventory.rb | 23 ++++++++++++++++ db/schema.rb | 4 +-- spec/requests/items_requests_spec.rb | 26 ++++++++++++++++++ spec/requests/kit_requests_spec.rb | 27 ++++++++++++++----- spec/system/item_system_spec.rb | 13 --------- 15 files changed, 114 insertions(+), 37 deletions(-) create mode 100644 db/migrate/20231222140700_remove_invalid_inventory.rb diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb index fb11b8780e..4d3ef7f234 100644 --- a/app/controllers/items_controller.rb +++ b/app/controllers/items_controller.rb @@ -3,6 +3,8 @@ class ItemsController < ApplicationController def index @items = current_organization.items.includes(:base_item, :kit).alphabetized.class_filter(filter_params) + @items = @items.active unless params[:include_inactive_items] + @item_categories = current_organization.item_categories.includes(:items).order('name ASC') @kits = current_organization.kits.includes(line_items: :item, inventory_items: :storage_location) @storages = current_organization.storage_locations.active_locations.order(id: :asc) @@ -12,10 +14,6 @@ def index @items_with_counts = ItemsByStorageCollectionQuery.new(organization: current_organization, filter_params: filter_params).call @items_by_storage_collection_and_quantity = ItemsByStorageCollectionAndQuantityQuery.new(organization: current_organization, filter_params: filter_params).call - unless params[:include_inactive_items] - @items = @items.active - @items_with_counts = @items_with_counts.active - end @paginated_items = @items.page(params[:page]) @@ -63,7 +61,17 @@ def show def update @item = current_organization.items.find(params[:id]) - if @item.update(item_params) + @item.attributes = item_params + + deactivated = @item.active_changed? && !@item.active + if deactivated && !@item.can_deactivate? + @base_items = BaseItem.without_kit.alphabetized + flash[:error] = "Can't deactivate this item - it is currently assigned to either an active kit or a storage location!" + render action: :edit + return + end + + if @item.save redirect_to items_path, notice: "#{@item.name} updated!" else @base_items = BaseItem.without_kit.alphabetized diff --git a/app/controllers/kits_controller.rb b/app/controllers/kits_controller.rb index a8a165c76f..b8dc072e06 100644 --- a/app/controllers/kits_controller.rb +++ b/app/controllers/kits_controller.rb @@ -44,8 +44,12 @@ def deactivate def reactivate @kit = Kit.find(params[:id]) - @kit.reactivate - redirect_back(fallback_location: dashboard_path, notice: "Kit has been reactivated!") + if @kit.can_reactivate? + @kit.reactivate + redirect_back(fallback_location: dashboard_path, notice: "Kit has been reactivated!") + else + redirect_back(fallback_location: dashboard_path, alert: "Cannot reactivate kit - it has inactive item! Please reactivate the items first.") + end end def allocations diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index 33a1bc6a38..f2a3934ebb 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -127,8 +127,8 @@ def inventory .includes(inventory_items: :item) .find(params[:id]) .inventory_items + .active - @inventory_items = @inventory_items.active unless params[:include_inactive_items] == "true" @inventory_items += include_omitted_items(@inventory_items.collect(&:item_id)) if params[:include_omitted_items] == "true" respond_to :json diff --git a/app/models/item.rb b/app/models/item.rb index 0c7b8366d9..cfb01ec776 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -106,6 +106,19 @@ def self.reactivate(item_ids) Item.where(id: item_ids).find_each { |item| item.update(active: true) } end + # @return [Boolean] + def can_deactivate? + # Cannot deactivate if it's currently in inventory in a storage location. It doesn't make sense + # to have physical inventory of something we're now saying isn't valid. + self.inventory_items.where('quantity > 0').none? && + # If an active kit includes this item, then changing kit allocations would change inventory + # for an inactive item - which we said above we don't want to allow. + self.organization.kits. + active. + joins(:line_items). + where(line_items: { item_id: self.id}).none? + end + def deactivate if kit kit.deactivate diff --git a/app/models/kit.rb b/app/models/kit.rb index 88098cfb12..696fd26e7e 100644 --- a/app/models/kit.rb +++ b/app/models/kit.rb @@ -41,6 +41,13 @@ def deactivate item.update!(active: false) end + # Kits can't reactivate if they have any inactive items, because now whenever they are allocated + # or deallocated, we are changing inventory for inactive items (which we don't allow). + # @return [Boolean] + def can_reactivate? + self.line_items.joins(:item).where(items: { active: false }).none? + end + def reactivate update!(active: true) item.update!(active: true) diff --git a/app/queries/items_by_storage_collection_and_quantity_query.rb b/app/queries/items_by_storage_collection_and_quantity_query.rb index 06536cb38e..cde2ce9d6a 100644 --- a/app/queries/items_by_storage_collection_and_quantity_query.rb +++ b/app/queries/items_by_storage_collection_and_quantity_query.rb @@ -11,9 +11,6 @@ def initialize(organization:, filter_params:) def call @items_by_storage_collection ||= ItemsByStorageCollectionQuery.new(organization: organization, filter_params: filter_params).call - unless @filter_params[:include_inactive_items] - @items_by_storage_collection = @items_by_storage_collection.active - end @items_by_storage_collection_and_quantity ||= Hash.new @items_by_storage_collection.each do |row| unless @items_by_storage_collection_and_quantity.key?(row.id) diff --git a/app/queries/items_by_storage_collection_query.rb b/app/queries/items_by_storage_collection_query.rb index 0176e52e76..589c586f33 100644 --- a/app/queries/items_by_storage_collection_query.rb +++ b/app/queries/items_by_storage_collection_query.rb @@ -14,6 +14,7 @@ def initialize(organization:, filter_params:) def call @items ||= organization .items + .active .joins(' LEFT OUTER JOIN "inventory_items" ON "inventory_items"."item_id" = "items"."id"') .joins(' LEFT OUTER JOIN "storage_locations" ON "storage_locations"."id" = "inventory_items"."storage_location_id"') .select(' diff --git a/app/views/audits/_form.html.erb b/app/views/audits/_form.html.erb index b3f6a03347..3574290917 100644 --- a/app/views/audits/_form.html.erb +++ b/app/views/audits/_form.html.erb @@ -14,7 +14,7 @@
<%= simple_form_for @audit, html: {class: "storage-location-required"} do |f| %> - <%= render partial: "storage_locations/source", object: f, locals: { label: "Storage location", error: "What storage location are you auditing?", include_inactive_items: true} %> + <%= render partial: "storage_locations/source", object: f, locals: { label: "Storage location", error: "What storage location are you auditing?" } %>
Items in this audit
diff --git a/app/views/distributions/_form.html.erb b/app/views/distributions/_form.html.erb index 29875d5d12..1c72f3e133 100644 --- a/app/views/distributions/_form.html.erb +++ b/app/views/distributions/_form.html.erb @@ -24,7 +24,7 @@
- <%= render partial: "storage_locations/source", object: f, locals: { include_inactive_items: false, include_omitted_items: true } %> + <%= render partial: "storage_locations/source", object: f, locals: { include_omitted_items: true } %> <%= f.input :comment, label: "Comment" %> diff --git a/app/views/storage_locations/_source.html.erb b/app/views/storage_locations/_source.html.erb index 1271d9ac85..9b465bc2be 100644 --- a/app/views/storage_locations/_source.html.erb +++ b/app/views/storage_locations/_source.html.erb @@ -3,7 +3,6 @@ label ||= "From storage location" error ||= "From what location are you moving inventory?" association_field ||= :storage_location - include_inactive_items ||= false include_omitted_items ||= false %> <%= source.association association_field, @@ -16,7 +15,6 @@ data: { storage_location_inventory_path: inventory_storage_location_path( organization_id: current_organization, - include_inactive_items: include_inactive_items, include_omitted_items: include_omitted_items, id: ":id", format: :json diff --git a/db/migrate/20231222140700_remove_invalid_inventory.rb b/db/migrate/20231222140700_remove_invalid_inventory.rb new file mode 100644 index 0000000000..0f55105d71 --- /dev/null +++ b/db/migrate/20231222140700_remove_invalid_inventory.rb @@ -0,0 +1,23 @@ +class RemoveInvalidInventory < ActiveRecord::Migration[7.0] + def up + admin_role = Role.find_by(name: 'super_admin') + user = UsersRole.where(role_id: admin_role.id).first.user + StorageLocation.all.each do |loc| + adjustment = Adjustment.new( + organization_id: loc.organization_id, + storage_location_id: loc.id, + user_id: user.id, + comment: "Removing inactive items from inventory") + loc.inventory_items.joins(:item).where(items: { active: false }).where('quantity > 0').each do |ii| + adjustment.line_items.push(LineItem.new(item_id: ii.item_id, quantity: -ii.quantity)) + end + if adjustment.line_items.any? + AdjustmentCreateService.new(adjustment).call + end + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/schema.rb b/db/schema.rb index 54906d1330..05df1f6ed5 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: 2023_12_01_194409) do +ActiveRecord::Schema[7.0].define(version: 2023_12_22_140700) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -324,7 +324,7 @@ create_table "flipper_gates", force: :cascade do |t| t.string "feature_key", null: false t.string "key", null: false - t.string "value" + t.text "value" t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.index ["feature_key", "key", "value"], name: "index_flipper_gates_on_feature_key_and_key_and_value", unique: true diff --git a/spec/requests/items_requests_spec.rb b/spec/requests/items_requests_spec.rb index 60c5e8231b..0248044046 100644 --- a/spec/requests/items_requests_spec.rb +++ b/spec/requests/items_requests_spec.rb @@ -32,5 +32,31 @@ it { is_expected.to be_successful } end end + + describe 'PUT #update' do + let(:item) { create(:item, organization: @organization, active: true) } + let(:storage_location) { create(:storage_location, organization: @organization)} + let(:kit) { create(:kit, organization: @organization) } + let(:inactive_params) { default_params.merge({id: item.id, item: { active: false } }) } + + it 'should be able to deactivate an item' do + expect { put item_path(inactive_params) }.to change { item.reload.active }.from(true).to(false) + expect(response).to redirect_to(items_path) + end + + it 'should not be able to deactivate an item in a storage location' do + create(:inventory_item, storage_location: storage_location, item_id: item.id) + put item_path(inactive_params) + expect(flash[:error]).to eq("Can't deactivate this item - it is currently assigned to either an active kit or a storage location!") + expect(item.reload.active).to eq(true) + end + + it 'should not be able to deactivate an item in a kit' do + create(:line_item, itemizable: kit, item_id: item.id) + put item_path(inactive_params) + expect(flash[:error]).to eq("Can't deactivate this item - it is currently assigned to either an active kit or a storage location!") + expect(item.reload.active).to eq(true) + end + end end end diff --git a/spec/requests/kit_requests_spec.rb b/spec/requests/kit_requests_spec.rb index 6bdef4fcf0..bff2cbf309 100644 --- a/spec/requests/kit_requests_spec.rb +++ b/spec/requests/kit_requests_spec.rb @@ -66,13 +66,26 @@ expect(flash[:notice]).to eq("Kit has been deactivated!") end - specify "PUT #reactivate" do - kit.deactivate - expect(kit).not_to be_active - put reactivate_kit_url(kit, default_params) - expect(kit.reload).to be_active - expect(response).to redirect_to(dashboard_path) - expect(flash[:notice]).to eq("Kit has been reactivated!") + describe "PUT #reactivate" do + it 'cannot reactivate if it has an inactive item' do + kit.deactivate + expect(kit).not_to be_active + kit.line_items.first.item.update!(active: false) + + put reactivate_kit_url(kit, default_params) + expect(kit.reload).not_to be_active + expect(response).to redirect_to(dashboard_path) + expect(flash[:alert]).to eq("Cannot reactivate kit - it has inactive item! Please reactivate the items first.") + end + + it 'should successfully reactivate' do + kit.deactivate + expect(kit).not_to be_active + put reactivate_kit_url(kit, default_params) + expect(kit.reload).to be_active + expect(response).to redirect_to(dashboard_path) + expect(flash[:notice]).to eq("Kit has been reactivated!") + end end end end diff --git a/spec/system/item_system_spec.rb b/spec/system/item_system_spec.rb index e2a73494d7..6101d59105 100644 --- a/spec/system/item_system_spec.rb +++ b/spec/system/item_system_spec.rb @@ -77,19 +77,6 @@ end end - it "can include inactive items in the results" do - Item.delete_all - create(:item, :inactive, name: "Inactive Item") - create(:item, :active, name: "Active Item") - visit url_prefix + "/items" - expect(page).to have_text("Active Item") - expect(page).to have_no_text("Inactive Item") - page.check('include_inactive_items') - click_button "Filter" - expect(page).to have_text("Inactive Item") - expect(page).to have_text("Active Item") - end - describe "destroying items" do subject { create(:item, name: "AAA DELETEME", organization: @user.organization) } context "when an item has history" do From 2da412876ff40b12ed83fa0b683a72369a9fbc2a Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 22 Dec 2023 11:46:38 -0500 Subject: [PATCH 02/44] Reactivate inactive kit items --- ...20231222133931_activate_inactive_kit_items.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 db/migrate/20231222133931_activate_inactive_kit_items.rb diff --git a/db/migrate/20231222133931_activate_inactive_kit_items.rb b/db/migrate/20231222133931_activate_inactive_kit_items.rb new file mode 100644 index 0000000000..348c366c9e --- /dev/null +++ b/db/migrate/20231222133931_activate_inactive_kit_items.rb @@ -0,0 +1,16 @@ +class ActivateInactiveKitItems < ActiveRecord::Migration[7.0] + def up + Kit.all.each do |kit| + kit.line_items.each do |line_item| + unless line_item.item.active? + line_item.item.update!(active: true) + end + end + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end + +end From ad49a358bd6ca79498e189254251892d278f94ba Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 22 Dec 2023 11:51:02 -0500 Subject: [PATCH 03/44] Lint fixes --- app/models/item.rb | 10 +++++----- app/models/kit.rb | 2 +- spec/requests/items_requests_spec.rb | 2 +- spec/requests/kit_requests_spec.rb | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/models/item.rb b/app/models/item.rb index cfb01ec776..503345b98d 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -110,13 +110,13 @@ def self.reactivate(item_ids) def can_deactivate? # Cannot deactivate if it's currently in inventory in a storage location. It doesn't make sense # to have physical inventory of something we're now saying isn't valid. - self.inventory_items.where('quantity > 0').none? && + inventory_items.where("quantity > 0").none? && # If an active kit includes this item, then changing kit allocations would change inventory # for an inactive item - which we said above we don't want to allow. - self.organization.kits. - active. - joins(:line_items). - where(line_items: { item_id: self.id}).none? + organization.kits + .active + .joins(:line_items) + .where(line_items: { item_id: id}).none? end def deactivate diff --git a/app/models/kit.rb b/app/models/kit.rb index 696fd26e7e..5071cc2f6d 100644 --- a/app/models/kit.rb +++ b/app/models/kit.rb @@ -45,7 +45,7 @@ def deactivate # or deallocated, we are changing inventory for inactive items (which we don't allow). # @return [Boolean] def can_reactivate? - self.line_items.joins(:item).where(items: { active: false }).none? + line_items.joins(:item).where(items: { active: false }).none? end def reactivate diff --git a/spec/requests/items_requests_spec.rb b/spec/requests/items_requests_spec.rb index 0248044046..91be61c654 100644 --- a/spec/requests/items_requests_spec.rb +++ b/spec/requests/items_requests_spec.rb @@ -35,7 +35,7 @@ describe 'PUT #update' do let(:item) { create(:item, organization: @organization, active: true) } - let(:storage_location) { create(:storage_location, organization: @organization)} + let(:storage_location) { create(:storage_location, organization: @organization) } let(:kit) { create(:kit, organization: @organization) } let(:inactive_params) { default_params.merge({id: item.id, item: { active: false } }) } diff --git a/spec/requests/kit_requests_spec.rb b/spec/requests/kit_requests_spec.rb index bff2cbf309..cc6851dfe2 100644 --- a/spec/requests/kit_requests_spec.rb +++ b/spec/requests/kit_requests_spec.rb @@ -67,7 +67,7 @@ end describe "PUT #reactivate" do - it 'cannot reactivate if it has an inactive item' do + it "cannot reactivate if it has an inactive item" do kit.deactivate expect(kit).not_to be_active kit.line_items.first.item.update!(active: false) @@ -78,7 +78,7 @@ expect(flash[:alert]).to eq("Cannot reactivate kit - it has inactive item! Please reactivate the items first.") end - it 'should successfully reactivate' do + it "should successfully reactivate" do kit.deactivate expect(kit).not_to be_active put reactivate_kit_url(kit, default_params) From 35de3dd57b90cd3403bd5f5ea4807fd309c779f5 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 22 Dec 2023 11:55:00 -0500 Subject: [PATCH 04/44] Remove invalid test --- spec/system/audit_system_spec.rb | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/spec/system/audit_system_spec.rb b/spec/system/audit_system_spec.rb index 50fc46e65e..b48115defb 100644 --- a/spec/system/audit_system_spec.rb +++ b/spec/system/audit_system_spec.rb @@ -39,20 +39,6 @@ subject { url_prefix + "/audits/new" } let(:item) { Item.alphabetized.first } - it "*Does* include inactive items in the line item fields" do - visit subject - - select storage_location.name, from: "Storage location" - expect(page).to have_content(item.name) - select item.name, from: "audit_line_items_attributes_0_item_id" - - item.update(active: false) - - page.refresh - select storage_location.name, from: "Storage location" - expect(page).to have_content(item.name) - end - it "does not display quantities in line-item drop down selector" do create(:storage_location, :with_items, item: item, item_quantity: 10) visit subject From c3bc7b95992e83623ed4fe106fd05aedf055782a Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 29 Dec 2023 11:35:32 -0500 Subject: [PATCH 05/44] fix typo --- app/controllers/kits_controller.rb | 2 +- spec/requests/kit_requests_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/kits_controller.rb b/app/controllers/kits_controller.rb index b8dc072e06..bd251ac79e 100644 --- a/app/controllers/kits_controller.rb +++ b/app/controllers/kits_controller.rb @@ -48,7 +48,7 @@ def reactivate @kit.reactivate redirect_back(fallback_location: dashboard_path, notice: "Kit has been reactivated!") else - redirect_back(fallback_location: dashboard_path, alert: "Cannot reactivate kit - it has inactive item! Please reactivate the items first.") + redirect_back(fallback_location: dashboard_path, alert: "Cannot reactivate kit - it has inactive items! Please reactivate the items first.") end end diff --git a/spec/requests/kit_requests_spec.rb b/spec/requests/kit_requests_spec.rb index cc6851dfe2..49ebf5e80a 100644 --- a/spec/requests/kit_requests_spec.rb +++ b/spec/requests/kit_requests_spec.rb @@ -75,7 +75,7 @@ put reactivate_kit_url(kit, default_params) expect(kit.reload).not_to be_active expect(response).to redirect_to(dashboard_path) - expect(flash[:alert]).to eq("Cannot reactivate kit - it has inactive item! Please reactivate the items first.") + expect(flash[:alert]).to eq("Cannot reactivate kit - it has inactive items! Please reactivate the items first.") end it "should successfully reactivate" do From 4732809c87a2319d496fc2fcd5815b61b5f14d73 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 28 Jul 2023 16:40:09 -0400 Subject: [PATCH 06/44] #3764: Fix profile attachments saving --- app/services/partner_profile_update_service.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/services/partner_profile_update_service.rb b/app/services/partner_profile_update_service.rb index 4a8ee2dc98..de3c15bd0d 100644 --- a/app/services/partner_profile_update_service.rb +++ b/app/services/partner_profile_update_service.rb @@ -16,7 +16,6 @@ def call if @return_value @profile.served_areas.destroy_all - @profile.reload @profile.attributes = @profile_params @profile.save!(context: :edit) @profile.reload From 3e8eefafdd85c126c1d5d4b2b59c61b43511d6d4 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 4 Aug 2023 16:30:30 -0400 Subject: [PATCH 07/44] One more reload --- app/services/partner_profile_update_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/partner_profile_update_service.rb b/app/services/partner_profile_update_service.rb index de3c15bd0d..d01914b152 100644 --- a/app/services/partner_profile_update_service.rb +++ b/app/services/partner_profile_update_service.rb @@ -18,7 +18,6 @@ def call @profile.served_areas.destroy_all @profile.attributes = @profile_params @profile.save!(context: :edit) - @profile.reload end end end @@ -28,6 +27,7 @@ def perform_profile_service(&block) @profile.transaction do yield block end + @profile.reload rescue ActiveRecord::RecordNotFound => e Rails.logger.error "[!] #{self.class.name} failed to update profile #{@profile.id} because it does not exist" set_error(e) From 537a32821fe5a6ed8f6144c7ab708dc6fe01a6d1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 4 Aug 2023 20:30:58 +0000 Subject: [PATCH 08/44] Render PlantUML files --- doc/architecture/barcode-retrieval.svg | 2 +- doc/architecture/multi-tenancy.svg | 2 +- doc/architecture/physical-flow.svg | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/architecture/barcode-retrieval.svg b/doc/architecture/barcode-retrieval.svg index d89102a036..52787ea2bd 100644 --- a/doc/architecture/barcode-retrieval.svg +++ b/doc/architecture/barcode-retrieval.svg @@ -1 +1 @@ -Base ItemItemGlobal BarcodeBank BarcodeFallbackFallback \ No newline at end of file +Base ItemItemGlobal BarcodeBank BarcodeFallbackFallback \ No newline at end of file diff --git a/doc/architecture/multi-tenancy.svg b/doc/architecture/multi-tenancy.svg index 03ac2226f9..85a1ec8d90 100644 --- a/doc/architecture/multi-tenancy.svg +++ b/doc/architecture/multi-tenancy.svg @@ -1 +1 @@ -humanessentials.appBank APartner APartner BPartner CBank BUser A(Admin)User BUser CUser D \ No newline at end of file +humanessentials.appBank APartner APartner BPartner CBank BUser A(Admin)User BUser CUser D \ No newline at end of file diff --git a/doc/architecture/physical-flow.svg b/doc/architecture/physical-flow.svg index 27c9689262..1f8b5e611d 100644 --- a/doc/architecture/physical-flow.svg +++ b/doc/architecture/physical-flow.svg @@ -1 +1 @@ -BankPartnerInventoryAdjustmentTransferDistributionPurchaseIntakeDonationRecipient \ No newline at end of file +BankPartnerInventoryAdjustmentTransferDistributionPurchaseIntakeDonationRecipient \ No newline at end of file From b397852ee07e4a91110ac8736ae4dbf041bdde48 Mon Sep 17 00:00:00 2001 From: Brock Wilcox Date: Sat, 11 Nov 2023 23:22:03 -0500 Subject: [PATCH 09/44] Clean out some whitespace lint [#3764] --- app/services/partner_profile_update_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/partner_profile_update_service.rb b/app/services/partner_profile_update_service.rb index d01914b152..3fa2b6bbf7 100644 --- a/app/services/partner_profile_update_service.rb +++ b/app/services/partner_profile_update_service.rb @@ -27,7 +27,7 @@ def perform_profile_service(&block) @profile.transaction do yield block end - @profile.reload + @profile.reload rescue ActiveRecord::RecordNotFound => e Rails.logger.error "[!] #{self.class.name} failed to update profile #{@profile.id} because it does not exist" set_error(e) From ff0031b1d4d709f0a8715af3d99b731c1e2be8ac Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Sun, 21 Jan 2024 19:55:41 -0500 Subject: [PATCH 10/44] Do not reactivate / allow inactive items in updates --- app/models/storage_location.rb | 1 - app/services/itemizable_update_service.rb | 5 ++++- spec/models/storage_location_spec.rb | 12 ------------ spec/services/itemizable_update_service_spec.rb | 17 +++++++++++++++-- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb index 2553022d65..2e409f2405 100644 --- a/app/models/storage_location.rb +++ b/app/models/storage_location.rb @@ -142,7 +142,6 @@ def increase_inventory(itemizable_array) # This is, at least for now, how we log changes to the inventory made in this call log = {} # Iterate through each of the line-items in the moving box - Item.reactivate(itemizable_array.map { |item_hash| item_hash[:item_id] }) itemizable_array.each do |item_hash| # Locate the storage box for the item, or create a new storage box for it inventory_item = inventory_items.find_or_create_by!(item_id: item_hash[:item_id]) diff --git a/app/services/itemizable_update_service.rb b/app/services/itemizable_update_service.rb index 99e0423f9b..6cbde6b7a7 100644 --- a/app/services/itemizable_update_service.rb +++ b/app/services/itemizable_update_service.rb @@ -7,7 +7,10 @@ module ItemizableUpdateService def self.call(itemizable:, type: :increase, params: {}, event_class: nil) StorageLocation.transaction do item_ids = params[:line_items_attributes]&.values&.map { |i| i[:item_id].to_i } || [] - Item.reactivate(item_ids) + inactive_item_names = Item.where(id: item_ids, active: false).pluck(:name) + if inactive_item_names.any? + raise "Update failed: The following items are currently inactive: #{inactive_item_names.join(', ')}. Please reactivate them before continuing." + end from_location = to_location = itemizable.storage_location to_location = StorageLocation.find(params[:storage_location_id]) if params[:storage_location_id] diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb index 252c2e947f..0a4227401e 100644 --- a/spec/models/storage_location_spec.rb +++ b/spec/models/storage_location_spec.rb @@ -87,18 +87,6 @@ end.to change { subject.inventory_items.count }.by(1) end end - - context "when increasing with an inactive item" do - let(:inactive_item) { create(:item, active: false, organization: @organization) } - let(:donation_with_inactive_item) { create(:donation, :with_items, organization: @organization, item_quantity: 10, item: inactive_item) } - - it "re-activates the item as part of the creation process" do - expect do - subject.increase_inventory(donation_with_inactive_item.to_a) - end.to change { subject.inventory_items.count }.by(1) - .and change { Item.count }.by(1) - end - end end describe "decrease_inventory" do diff --git a/spec/services/itemizable_update_service_spec.rb b/spec/services/itemizable_update_service_spec.rb index 7e664d92f9..b27e36e3d1 100644 --- a/spec/services/itemizable_update_service_spec.rb +++ b/spec/services/itemizable_update_service_spec.rb @@ -1,8 +1,8 @@ RSpec.describe ItemizableUpdateService do let(:storage_location) { create(:storage_location, organization: @organization, item_count: 0) } let(:new_storage_location) { create(:storage_location, organization: @organization, item_count: 0) } - let(:item1) { create(:item, organization: @organization) } - let(:item2) { create(:item, organization: @organization) } + let(:item1) { create(:item, organization: @organization, name: 'My Item 1') } + let(:item2) { create(:item, organization: @organization, name: 'My Item 2') } let!(:ii1) { create(:inventory_item, storage_location: storage_location, item: item1, quantity: 10) } let!(:ii2) { create(:inventory_item, storage_location: new_storage_location, item: item2, quantity: 10) } let!(:ii3) { create(:inventory_item, storage_location: storage_location, item: item2, quantity: 10) } @@ -61,6 +61,13 @@ expect(storage_location.size).to eq(10) expect(new_storage_location.size).to eq(24) end + + it 'should raise an error if any item is inactive' do + item1.update!(active: false) + msg = "Update failed: The following items are currently inactive: My Item 1. Please reactivate them before continuing." + expect { subject }.to raise_error(msg) + end + end describe "decreases" do @@ -106,5 +113,11 @@ expect(storage_location.size).to eq(30) expect(new_storage_location.size).to eq(16) end + + it 'should raise an error if any item is inactive' do + item1.update!(active: false) + msg = "Update failed: The following items are currently inactive: My Item 1. Please reactivate them before continuing." + expect { subject }.to raise_error(msg) + end end end From 785e120325bd89d854a7c5fe7767826fc66f7ebd Mon Sep 17 00:00:00 2001 From: cancelei Date: Wed, 31 Jan 2024 19:00:53 -0300 Subject: [PATCH 11/44] WIP-Fix user name display and persist user name in invite service Add name input field in partner show view --- app/helpers/dashboard_helper.rb | 2 +- app/services/user_invite_service.rb | 5 +++-- app/views/partners/show.html.erb | 4 ++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb index f5e34473ba..8f27b7df5b 100644 --- a/app/helpers/dashboard_helper.rb +++ b/app/helpers/dashboard_helper.rb @@ -41,7 +41,7 @@ def future_distributed end def recently_added_user_display_text(user) - (user.name == "Name Not Provided") ? user.email : user.name + ( user.name.blank? || user.name == "Name Not Provided") ? user.email : user.name end private diff --git a/app/services/user_invite_service.rb b/app/services/user_invite_service.rb index 4a5df1c943..17fedfe671 100644 --- a/app/services/user_invite_service.rb +++ b/app/services/user_invite_service.rb @@ -5,7 +5,7 @@ module UserInviteService # @param resource [ApplicationRecord] # @param force [Boolean] # @return [User] - def self.invite(email:, resource:, name: nil, roles: [], force: false) + def self.invite(email:, resource:, name:, roles: [], force: false) raise "Resource not found!" if resource.nil? user = User.find_by(email: email) @@ -26,7 +26,8 @@ def self.invite(email:, resource:, name: nil, roles: [], force: false) end User.invite!(email: email) do |user1| - user1.name = name if name # Does this get persisted somewhere up the line? - CLF 20230203 + user1.name = name # Does this get persisted somewhere up the line? - CLF 20230203 + user1.save! add_roles(user1, resource: resource, roles: roles) user1.skip_invitation = user1.errors[:email].any? end diff --git a/app/views/partners/show.html.erb b/app/views/partners/show.html.erb index d337e7be6d..7ca03ac364 100644 --- a/app/views/partners/show.html.erb +++ b/app/views/partners/show.html.erb @@ -305,6 +305,10 @@
<%= fa_icon "envelope" %> +
+
+ <%= fa_icon "user" %> + <%= hidden_field_tag :partner, @partner.id %>

From 4b14fb5ab2721212cfc2852f4bd0a30457ff94a1 Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 1 Feb 2024 17:13:11 -0300 Subject: [PATCH 12/44] Add name parameter to UserInviteService.invite method --- app/controllers/partners_controller.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/controllers/partners_controller.rb b/app/controllers/partners_controller.rb index ceb8f297d0..ef8ef357b9 100644 --- a/app/controllers/partners_controller.rb +++ b/app/controllers/partners_controller.rb @@ -109,7 +109,10 @@ def invite def invite_partner_user partner = current_organization.partners.find(params[:partner]) - UserInviteService.invite(email: params[:email], + + name = params[:name].presence || "Name Not Provided" + + UserInviteService.invite(name: name, email: params[:email], roles: [Role::PARTNER], resource: partner) From d8ed462dd06c4b12af0672487191f06bb4a0efcd Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 1 Feb 2024 18:33:37 -0300 Subject: [PATCH 13/44] Add email parameter to user creation request - Modification of other people's tests --- spec/requests/admin/users_requests_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/admin/users_requests_spec.rb b/spec/requests/admin/users_requests_spec.rb index 56fa697475..d90297f0b1 100644 --- a/spec/requests/admin/users_requests_spec.rb +++ b/spec/requests/admin/users_requests_spec.rb @@ -128,7 +128,7 @@ end it "preloads organizations" do - post admin_users_path, params: { user: { organization_id: 1 } } + post admin_users_path, params: { user: { organization_id: 1, email: 'hello@gm.com' }} expect(assigns(:organizations)).to eq(Organization.all.alphabetized) end end From bdf288adb163549b2a24b003e441e0a478faf3c8 Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 1 Feb 2024 18:33:52 -0300 Subject: [PATCH 14/44] Fix user name display and invite service bug --- app/helpers/dashboard_helper.rb | 2 +- app/services/user_invite_service.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb index 8f27b7df5b..14ddf7d495 100644 --- a/app/helpers/dashboard_helper.rb +++ b/app/helpers/dashboard_helper.rb @@ -41,7 +41,7 @@ def future_distributed end def recently_added_user_display_text(user) - ( user.name.blank? || user.name == "Name Not Provided") ? user.email : user.name + (user.name.blank? || user.name == "Name Not Provided") ? user.email : user.name end private diff --git a/app/services/user_invite_service.rb b/app/services/user_invite_service.rb index 17fedfe671..47f00d7b23 100644 --- a/app/services/user_invite_service.rb +++ b/app/services/user_invite_service.rb @@ -5,7 +5,7 @@ module UserInviteService # @param resource [ApplicationRecord] # @param force [Boolean] # @return [User] - def self.invite(email:, resource:, name:, roles: [], force: false) + def self.invite(email:, resource:, name: nil, roles: [], force: false) raise "Resource not found!" if resource.nil? user = User.find_by(email: email) @@ -26,7 +26,7 @@ def self.invite(email:, resource:, name:, roles: [], force: false) end User.invite!(email: email) do |user1| - user1.name = name # Does this get persisted somewhere up the line? - CLF 20230203 + user1.name = name if name # Does this get persisted somewhere up the line? - CLF 20230203 user1.save! add_roles(user1, resource: resource, roles: roles) user1.skip_invitation = user1.errors[:email].any? From 05e9d1787474b16462d6e1447cd37961d1457993 Mon Sep 17 00:00:00 2001 From: cancelei Date: Fri, 2 Feb 2024 13:35:38 -0300 Subject: [PATCH 15/44] Fix user invite service and add new tests --- app/services/user_invite_service.rb | 1 - spec/services/user_invite_service_spec.rb | 56 +++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/app/services/user_invite_service.rb b/app/services/user_invite_service.rb index 47f00d7b23..4a5df1c943 100644 --- a/app/services/user_invite_service.rb +++ b/app/services/user_invite_service.rb @@ -27,7 +27,6 @@ def self.invite(email:, resource:, name: nil, roles: [], force: false) User.invite!(email: email) do |user1| user1.name = name if name # Does this get persisted somewhere up the line? - CLF 20230203 - user1.save! add_roles(user1, resource: resource, roles: roles) user1.skip_invitation = user1.errors[:email].any? end diff --git a/spec/services/user_invite_service_spec.rb b/spec/services/user_invite_service_spec.rb index a739af020a..2b54b7a958 100644 --- a/spec/services/user_invite_service_spec.rb +++ b/spec/services/user_invite_service_spec.rb @@ -50,5 +50,61 @@ expect(result).to have_role(Role::ORG_ADMIN, organization) expect(result).not_to have_role(Role::PARTNER, :any) end + + it "should create the user without name with role" do + result = nil + expect { + result = described_class.invite(name: "", + email: "email2@email.com", + roles: [Role::ORG_USER, Role::ORG_ADMIN], + resource: organization) + }.to change { ActionMailer::Base.deliveries.count }.by(1) + expect(result.name).to eq("") + expect(result.email).to eq("email2@email.com") + expect(result).to have_role(Role::ORG_USER, organization) + expect(result).to have_role(Role::ORG_ADMIN, organization) + # expect(result).to have_role(Role::PARTNER, :any) + end + end +end + +RSpec.describe UserInviteService, type: :service do + let!(:organization) { FactoryBot.create(:organization) } + let!(:partner) { FactoryBot.create(:partner, organization: organization) } + + before do + allow(UserMailer).to receive(:role_added).and_return(double(:mail, deliver_later: nil)) + end + + context "with a new user" do + it "should invite a user with the partner role" do + expect { + described_class.invite( + name: "Partner User", + email: "partner@example.com", + roles: [Role::PARTNER], + resource: partner + ) + }.to change(User, :count).by(1).and change(ActionMailer::Base.deliveries, :count).by(1) + + new_user = User.find_by(email: "partner@example.com") + expect(new_user).not_to be_nil + expect(new_user.has_role?(:partner, partner)).to be true + end + + it "should create the user without a name with default role" do + expect { + described_class.invite( + name: "", + email: "email2@example.com", + roles: [Role::ORG_USER], + resource: organization + ) + }.to change(User, :count).by(1) + + new_user = User.find_by(email: "email2@example.com") + expect(new_user.name).to eq("") # GB 02022024 - "Name Not Provided" should pass this test, but doesnt. + expect(new_user.has_role?(:org_user, organization)).to be true + end end end From eeb6e25c582fe1f142712be6bbe6233ff524c010 Mon Sep 17 00:00:00 2001 From: cancelei Date: Fri, 2 Feb 2024 14:48:30 -0300 Subject: [PATCH 16/44] Update invite_partner_user method in partners_requests_spec.rb --- spec/requests/partners_requests_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/requests/partners_requests_spec.rb b/spec/requests/partners_requests_spec.rb index c7472bb914..70ed442438 100644 --- a/spec/requests/partners_requests_spec.rb +++ b/spec/requests/partners_requests_spec.rb @@ -285,7 +285,7 @@ end describe "POST #invite_partner_user" do - subject { -> { post invite_partner_user_partner_path(default_params.merge(id: partner.id, partner: partner.id, email: email)) } } + subject { -> { post invite_partner_user_partner_path(default_params.merge(id: partner.id, partner: partner.id, email: email, name: partner.name)) } } let(:partner) { create(:partner, organization: @organization) } let(:email) { Faker::Internet.email } @@ -297,6 +297,7 @@ subject.call expect(UserInviteService).to have_received(:invite).with( email: email, + name: partner.name, roles: [Role::PARTNER], resource: partner ) From 444c38497a22039df5866ec9e59b4c527bf5828a Mon Sep 17 00:00:00 2001 From: cancelei Date: Fri, 2 Feb 2024 15:00:24 -0300 Subject: [PATCH 17/44] Fix role expectation in user_invite_service_spec.rb --- spec/services/user_invite_service_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/services/user_invite_service_spec.rb b/spec/services/user_invite_service_spec.rb index 2b54b7a958..847e76ca27 100644 --- a/spec/services/user_invite_service_spec.rb +++ b/spec/services/user_invite_service_spec.rb @@ -63,7 +63,7 @@ expect(result.email).to eq("email2@email.com") expect(result).to have_role(Role::ORG_USER, organization) expect(result).to have_role(Role::ORG_ADMIN, organization) - # expect(result).to have_role(Role::PARTNER, :any) + expect(result).not_to have_role(Role::PARTNER, :any) end end end @@ -103,7 +103,7 @@ }.to change(User, :count).by(1) new_user = User.find_by(email: "email2@example.com") - expect(new_user.name).to eq("") # GB 02022024 - "Name Not Provided" should pass this test, but doesnt. + expect(new_user.name).to eq("") expect(new_user.has_role?(:org_user, organization)).to be true end end From 594b182f54f07ff9b2fbd0912145d0128e1eec86 Mon Sep 17 00:00:00 2001 From: cancelei Date: Fri, 2 Feb 2024 15:12:58 -0300 Subject: [PATCH 18/44] Remove unnecessary role check in user invite service spec --- spec/services/user_invite_service_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/services/user_invite_service_spec.rb b/spec/services/user_invite_service_spec.rb index 847e76ca27..41b6447913 100644 --- a/spec/services/user_invite_service_spec.rb +++ b/spec/services/user_invite_service_spec.rb @@ -63,7 +63,6 @@ expect(result.email).to eq("email2@email.com") expect(result).to have_role(Role::ORG_USER, organization) expect(result).to have_role(Role::ORG_ADMIN, organization) - expect(result).not_to have_role(Role::PARTNER, :any) end end end From ac893eed20765c891682496061eda71f7f950467 Mon Sep 17 00:00:00 2001 From: cancelei Date: Fri, 2 Feb 2024 15:51:30 -0300 Subject: [PATCH 19/44] Add required attribute to email input field and Minor margin between input fields. --- app/views/partners/show.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/partners/show.html.erb b/app/views/partners/show.html.erb index 7ca03ac364..276bb24df2 100644 --- a/app/views/partners/show.html.erb +++ b/app/views/partners/show.html.erb @@ -304,9 +304,9 @@ <%= form_tag invite_partner_user_partner_path do %>
<%= fa_icon "envelope" %> - +
-
+
<%= fa_icon "user" %> <%= hidden_field_tag :partner, @partner.id %>
From 64b4623b5415c64fe558cebbd9c8db8780acae3a Mon Sep 17 00:00:00 2001 From: Glauber Date: Mon, 5 Feb 2024 10:53:13 -0300 Subject: [PATCH 20/44] Update spec/services/user_invite_service_spec.rb Add expected test result. Co-authored-by: Brock Wilcox --- spec/services/user_invite_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/user_invite_service_spec.rb b/spec/services/user_invite_service_spec.rb index 41b6447913..ae5b7c07c3 100644 --- a/spec/services/user_invite_service_spec.rb +++ b/spec/services/user_invite_service_spec.rb @@ -102,7 +102,7 @@ }.to change(User, :count).by(1) new_user = User.find_by(email: "email2@example.com") - expect(new_user.name).to eq("") + expect(new_user.name).to eq("Name Not Provided") expect(new_user.has_role?(:org_user, organization)).to be true end end From 63bbd365d574025d8f894d7c3712d04e789cdc6b Mon Sep 17 00:00:00 2001 From: cancelei Date: Mon, 5 Feb 2024 12:16:18 -0300 Subject: [PATCH 21/44] Refactor user invite service and partners controller --- app/controllers/partners_controller.rb | 6 +++--- spec/services/user_invite_service_spec.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/partners_controller.rb b/app/controllers/partners_controller.rb index ef8ef357b9..b155571213 100644 --- a/app/controllers/partners_controller.rb +++ b/app/controllers/partners_controller.rb @@ -110,9 +110,9 @@ def invite def invite_partner_user partner = current_organization.partners.find(params[:partner]) - name = params[:name].presence || "Name Not Provided" - - UserInviteService.invite(name: name, email: params[:email], + UserInviteService.invite( + name: params[:name].presence || "Name Not Provided", + email: params[:email], roles: [Role::PARTNER], resource: partner) diff --git a/spec/services/user_invite_service_spec.rb b/spec/services/user_invite_service_spec.rb index ae5b7c07c3..b73cdd367a 100644 --- a/spec/services/user_invite_service_spec.rb +++ b/spec/services/user_invite_service_spec.rb @@ -94,7 +94,7 @@ it "should create the user without a name with default role" do expect { described_class.invite( - name: "", + name: nil, email: "email2@example.com", roles: [Role::ORG_USER], resource: organization From 88220c3c49329cd2cd3bddfc0d65107363859a88 Mon Sep 17 00:00:00 2001 From: cancelei Date: Mon, 5 Feb 2024 12:49:13 -0300 Subject: [PATCH 22/44] Refactor user invitation process and fix partner invite bug --- app/controllers/partners_controller.rb | 3 +- spec/services/user_invite_service_spec.rb | 63 ++++++++++------------- 2 files changed, 30 insertions(+), 36 deletions(-) diff --git a/app/controllers/partners_controller.rb b/app/controllers/partners_controller.rb index b155571213..9bd9f28d35 100644 --- a/app/controllers/partners_controller.rb +++ b/app/controllers/partners_controller.rb @@ -114,7 +114,8 @@ def invite_partner_user name: params[:name].presence || "Name Not Provided", email: params[:email], roles: [Role::PARTNER], - resource: partner) + resource: partner + ) redirect_to partner_path(partner), notice: "We have invited #{params[:email]} to #{partner.name}!" rescue StandardError => e diff --git a/spec/services/user_invite_service_spec.rb b/spec/services/user_invite_service_spec.rb index b73cdd367a..35a8fdb1c5 100644 --- a/spec/services/user_invite_service_spec.rb +++ b/spec/services/user_invite_service_spec.rb @@ -1,5 +1,7 @@ RSpec.describe UserInviteService, type: :service, skip_seed: true do let(:organization) { FactoryBot.create(:organization) } + let(:partner) { FactoryBot.create(:partner, organization: organization) } + before(:each) do allow(UserMailer).to receive(:role_added).and_return(double(:mail, deliver_later: nil)) end @@ -65,45 +67,36 @@ expect(result).to have_role(Role::ORG_ADMIN, organization) end end -end -RSpec.describe UserInviteService, type: :service do - let!(:organization) { FactoryBot.create(:organization) } - let!(:partner) { FactoryBot.create(:partner, organization: organization) } + it "should invite a user with the partner role" do + result = nil + expect { + result = described_class.invite( + name: "Partner User", + email: "partner@example.com", + roles: [Role::PARTNER], + resource: partner + ) + }.to change(ActionMailer::Base.deliveries, :count).by(1) - before do - allow(UserMailer).to receive(:role_added).and_return(double(:mail, deliver_later: nil)) + expect(result.name).to eq("Partner User") + expect(result).not_to be_nil + expect(result.has_role?(:partner, partner)).to be true end - context "with a new user" do - it "should invite a user with the partner role" do - expect { - described_class.invite( - name: "Partner User", - email: "partner@example.com", - roles: [Role::PARTNER], - resource: partner - ) - }.to change(User, :count).by(1).and change(ActionMailer::Base.deliveries, :count).by(1) - - new_user = User.find_by(email: "partner@example.com") - expect(new_user).not_to be_nil - expect(new_user.has_role?(:partner, partner)).to be true - end + it "should create the user without a name with default role" do + result = nil + expect { + result = described_class.invite( + name: nil, + email: "email2@example.com", + roles: [Role::ORG_USER], + resource: organization + ) + }.to change(ActionMailer::Base.deliveries, :count).by(1) - it "should create the user without a name with default role" do - expect { - described_class.invite( - name: nil, - email: "email2@example.com", - roles: [Role::ORG_USER], - resource: organization - ) - }.to change(User, :count).by(1) - - new_user = User.find_by(email: "email2@example.com") - expect(new_user.name).to eq("Name Not Provided") - expect(new_user.has_role?(:org_user, organization)).to be true - end + expect(result.name).to eq("Name Not Provided") + expect(result.email).to eq("email2@example.com") + expect(result.has_role?(:org_user, organization)).to be true end end From 11c4b426d13bdb55e79927a3263aa6299edaecf7 Mon Sep 17 00:00:00 2001 From: cancelei Date: Sun, 11 Feb 2024 13:32:30 -0300 Subject: [PATCH 23/44] merging in main --- app/controllers/donation_sites_controller.rb | 2 +- app/models/donation_site.rb | 5 +++++ .../20240211150205_add_contact_info_to_donation_sites.rb | 7 +++++++ db/schema.rb | 5 ++++- spec/models/donation_site_spec.rb | 3 +++ 5 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20240211150205_add_contact_info_to_donation_sites.rb diff --git a/app/controllers/donation_sites_controller.rb b/app/controllers/donation_sites_controller.rb index bb3662a23c..6724c4ac55 100644 --- a/app/controllers/donation_sites_controller.rb +++ b/app/controllers/donation_sites_controller.rb @@ -60,7 +60,7 @@ def destroy private def donation_site_params - params.require(:donation_site).permit(:name, :address) + params.require(:donation_site).permit(:name, :address, :contact_name, :email, :phone) end helper_method \ diff --git a/app/models/donation_site.rb b/app/models/donation_site.rb index 2732adb679..3fbeafc4c6 100644 --- a/app/models/donation_site.rb +++ b/app/models/donation_site.rb @@ -4,9 +4,12 @@ # # id :integer not null, primary key # address :string +# contact_name :string +# email :string # latitude :float # longitude :float # name :string +# phone :string # created_at :datetime not null # updated_at :datetime not null # organization_id :integer @@ -19,6 +22,8 @@ class DonationSite < ApplicationRecord belongs_to :organization validates :name, :address, :organization, presence: true + validates :phone, presence: {message: "Must provide a phone or an email"}, if: proc { |ds| ds.email.blank? } + validates :email, presence: {message: "Must provide a phone or an email"}, if: proc { |ds| ds.phone.blank? } has_many :donations, dependent: :destroy diff --git a/db/migrate/20240211150205_add_contact_info_to_donation_sites.rb b/db/migrate/20240211150205_add_contact_info_to_donation_sites.rb new file mode 100644 index 0000000000..b84b180aab --- /dev/null +++ b/db/migrate/20240211150205_add_contact_info_to_donation_sites.rb @@ -0,0 +1,7 @@ +class AddContactInfoToDonationSites < ActiveRecord::Migration[7.0] + def change + add_column :donation_sites, :contact_name, :string + add_column :donation_sites, :email, :string + add_column :donation_sites, :phone, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 14d173c3bd..0b91ee0e86 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_02_07_202431) do +ActiveRecord::Schema[7.0].define(version: 2024_02_11_150205) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -250,6 +250,9 @@ t.integer "organization_id" t.float "latitude" t.float "longitude" + t.string "contact_name" + t.string "email" + t.string "phone" t.index ["latitude", "longitude"], name: "index_donation_sites_on_latitude_and_longitude" t.index ["organization_id"], name: "index_donation_sites_on_organization_id" end diff --git a/spec/models/donation_site_spec.rb b/spec/models/donation_site_spec.rb index d1040e468e..448d805fc3 100644 --- a/spec/models/donation_site_spec.rb +++ b/spec/models/donation_site_spec.rb @@ -4,9 +4,12 @@ # # id :integer not null, primary key # address :string +# contact_name :string +# email :string # latitude :float # longitude :float # name :string +# phone :string # created_at :datetime not null # updated_at :datetime not null # organization_id :integer From f611b03c396b4fd865928fa82b4a00f5028b7bcd Mon Sep 17 00:00:00 2001 From: cancelei Date: Sun, 11 Feb 2024 13:37:40 -0300 Subject: [PATCH 24/44] Update input fields in partner show view --- app/views/partners/show.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/partners/show.html.erb b/app/views/partners/show.html.erb index 65472c50e1..d8019885e1 100644 --- a/app/views/partners/show.html.erb +++ b/app/views/partners/show.html.erb @@ -304,11 +304,11 @@ <%= form_tag invite_partner_user_partner_path do %>
<%= fa_icon "envelope" %> - +
<%= fa_icon "user" %> - + <%= hidden_field_tag :partner, @partner.id %>

@@ -335,7 +335,7 @@ <%= form_tag partner_user_reset_password_users_path do %>
<%= fa_icon "envelope" %> - + <%= hidden_field_tag :partner_id, @partner.id %>

From c5cd7866868219f902dd51f69dcfb283bbdd9a13 Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 15 Feb 2024 19:27:58 -0300 Subject: [PATCH 25/44] Revert "merging in main" This reverts commit 11c4b426d13bdb55e79927a3263aa6299edaecf7. --- app/controllers/donation_sites_controller.rb | 2 +- app/models/donation_site.rb | 5 ----- .../20240211150205_add_contact_info_to_donation_sites.rb | 7 ------- db/schema.rb | 5 +---- spec/models/donation_site_spec.rb | 3 --- 5 files changed, 2 insertions(+), 20 deletions(-) delete mode 100644 db/migrate/20240211150205_add_contact_info_to_donation_sites.rb diff --git a/app/controllers/donation_sites_controller.rb b/app/controllers/donation_sites_controller.rb index 6724c4ac55..bb3662a23c 100644 --- a/app/controllers/donation_sites_controller.rb +++ b/app/controllers/donation_sites_controller.rb @@ -60,7 +60,7 @@ def destroy private def donation_site_params - params.require(:donation_site).permit(:name, :address, :contact_name, :email, :phone) + params.require(:donation_site).permit(:name, :address) end helper_method \ diff --git a/app/models/donation_site.rb b/app/models/donation_site.rb index 3fbeafc4c6..2732adb679 100644 --- a/app/models/donation_site.rb +++ b/app/models/donation_site.rb @@ -4,12 +4,9 @@ # # id :integer not null, primary key # address :string -# contact_name :string -# email :string # latitude :float # longitude :float # name :string -# phone :string # created_at :datetime not null # updated_at :datetime not null # organization_id :integer @@ -22,8 +19,6 @@ class DonationSite < ApplicationRecord belongs_to :organization validates :name, :address, :organization, presence: true - validates :phone, presence: {message: "Must provide a phone or an email"}, if: proc { |ds| ds.email.blank? } - validates :email, presence: {message: "Must provide a phone or an email"}, if: proc { |ds| ds.phone.blank? } has_many :donations, dependent: :destroy diff --git a/db/migrate/20240211150205_add_contact_info_to_donation_sites.rb b/db/migrate/20240211150205_add_contact_info_to_donation_sites.rb deleted file mode 100644 index b84b180aab..0000000000 --- a/db/migrate/20240211150205_add_contact_info_to_donation_sites.rb +++ /dev/null @@ -1,7 +0,0 @@ -class AddContactInfoToDonationSites < ActiveRecord::Migration[7.0] - def change - add_column :donation_sites, :contact_name, :string - add_column :donation_sites, :email, :string - add_column :donation_sites, :phone, :string - end -end diff --git a/db/schema.rb b/db/schema.rb index 0b91ee0e86..14d173c3bd 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_02_11_150205) do +ActiveRecord::Schema[7.0].define(version: 2024_02_07_202431) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -250,9 +250,6 @@ t.integer "organization_id" t.float "latitude" t.float "longitude" - t.string "contact_name" - t.string "email" - t.string "phone" t.index ["latitude", "longitude"], name: "index_donation_sites_on_latitude_and_longitude" t.index ["organization_id"], name: "index_donation_sites_on_organization_id" end diff --git a/spec/models/donation_site_spec.rb b/spec/models/donation_site_spec.rb index 448d805fc3..d1040e468e 100644 --- a/spec/models/donation_site_spec.rb +++ b/spec/models/donation_site_spec.rb @@ -4,12 +4,9 @@ # # id :integer not null, primary key # address :string -# contact_name :string -# email :string # latitude :float # longitude :float # name :string -# phone :string # created_at :datetime not null # updated_at :datetime not null # organization_id :integer From 46395345cdb9854344526d739fbe6f48e751e33c Mon Sep 17 00:00:00 2001 From: YuriBocharov Date: Tue, 27 Feb 2024 19:10:39 -0500 Subject: [PATCH 26/44] fix: remove bugsnag api key message without an API key bugsnag displays the following warning: "not delivering sessions due to an invalid api_key" It is something that new people often get worried about. This PR sends that warning to dev/null https://github.com/chaskiq/chaskiq/issues/251#issuecomment-1813804184 --- config/initializers/bugsnag.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/config/initializers/bugsnag.rb b/config/initializers/bugsnag.rb index c1c8c7ad48..478e30716d 100644 --- a/config/initializers/bugsnag.rb +++ b/config/initializers/bugsnag.rb @@ -1,3 +1,7 @@ -Bugsnag.configure do |config| - config.api_key = ENV["BUGSNAG_API_KEY"] +if ENV["BUGSNAG_API_KEY"].present? + Bugsnag.configure do |config| + config.api_key = ENV["BUGSNAG_API_KEY"] + end +else + Bugsnag.configuration.logger = ::Logger.new("/dev/null") end From 37f4a62c4b431f505b00aa157578aa7a6e57ad60 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 28 Feb 2024 04:23:18 +0000 Subject: [PATCH 27/44] build(deps): bump dotenv-rails from 3.0.3 to 3.1.0 Bumps [dotenv-rails](https://github.com/bkeepers/dotenv) from 3.0.3 to 3.1.0. - [Release notes](https://github.com/bkeepers/dotenv/releases) - [Changelog](https://github.com/bkeepers/dotenv/blob/main/Changelog.md) - [Commits](https://github.com/bkeepers/dotenv/compare/v3.0.3...v3.1.0) --- updated-dependencies: - dependency-name: dotenv-rails dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index ff8869c359..1dd7f533df 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -170,9 +170,9 @@ GEM discard (1.3.0) activerecord (>= 4.2, < 8) docile (1.4.0) - dotenv (3.0.3) - dotenv-rails (3.0.3) - dotenv (= 3.0.3) + dotenv (3.1.0) + dotenv-rails (3.1.0) + dotenv (= 3.1.0) railties (>= 6.1) dry-core (1.0.1) concurrent-ruby (~> 1.0) From d403e22b559d005f54df8184ea7a721469747657 Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 29 Feb 2024 17:28:55 -0300 Subject: [PATCH 28/44] Move "Name Not Provided" from partners_controller to user_invite_service --- app/controllers/partners_controller.rb | 7 ++----- app/services/user_invite_service.rb | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/app/controllers/partners_controller.rb b/app/controllers/partners_controller.rb index f74fe5b8ba..9699140cd2 100644 --- a/app/controllers/partners_controller.rb +++ b/app/controllers/partners_controller.rb @@ -132,12 +132,9 @@ def invite def invite_partner_user partner = current_organization.partners.find(params[:partner]) - UserInviteService.invite( - name: params[:name].presence || "Name Not Provided", - email: params[:email], + UserInviteService.invite(email: params[:email], roles: [Role::PARTNER], - resource: partner - ) + resource: partner) redirect_to partner_path(partner), notice: "We have invited #{params[:email]} to #{partner.name}!" rescue StandardError => e diff --git a/app/services/user_invite_service.rb b/app/services/user_invite_service.rb index 4a5df1c943..09fac7a425 100644 --- a/app/services/user_invite_service.rb +++ b/app/services/user_invite_service.rb @@ -26,7 +26,7 @@ def self.invite(email:, resource:, name: nil, roles: [], force: false) end User.invite!(email: email) do |user1| - user1.name = name if name # Does this get persisted somewhere up the line? - CLF 20230203 + user1.name = name.presence || "Name Not Provided" add_roles(user1, resource: resource, roles: roles) user1.skip_invitation = user1.errors[:email].any? end From d69da49ca4952d742e020b64a2bb56f1cbda15e7 Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 29 Feb 2024 17:31:08 -0300 Subject: [PATCH 29/44] Fix user invite specs by adding name using faker and remove unnecessary code --- spec/requests/admin/users_requests_spec.rb | 2 +- spec/requests/partners_requests_spec.rb | 5 ++-- spec/services/user_invite_service_spec.rb | 34 +++++++++++----------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/spec/requests/admin/users_requests_spec.rb b/spec/requests/admin/users_requests_spec.rb index d90297f0b1..3036aca0e0 100644 --- a/spec/requests/admin/users_requests_spec.rb +++ b/spec/requests/admin/users_requests_spec.rb @@ -128,7 +128,7 @@ end it "preloads organizations" do - post admin_users_path, params: { user: { organization_id: 1, email: 'hello@gm.com' }} + post admin_users_path, params: { user: { organization_id: 1 }} expect(assigns(:organizations)).to eq(Organization.all.alphabetized) end end diff --git a/spec/requests/partners_requests_spec.rb b/spec/requests/partners_requests_spec.rb index 4cf6863c21..3a45950b90 100644 --- a/spec/requests/partners_requests_spec.rb +++ b/spec/requests/partners_requests_spec.rb @@ -285,9 +285,10 @@ end describe "POST #invite_partner_user" do - subject { -> { post invite_partner_user_partner_path(default_params.merge(id: partner.id, partner: partner.id, email: email, name: partner.name)) } } + subject { -> { post invite_partner_user_partner_path(default_params.merge(id: partner.id, partner: partner.id, email: email, name: name)) } } let(:partner) { create(:partner, organization: @organization) } let(:email) { Faker::Internet.email } + let(:name) { Faker::Name.unique.name } context 'when the invite successfully' do before do @@ -297,7 +298,7 @@ subject.call expect(UserInviteService).to have_received(:invite).with( email: email, - name: partner.name, + name: name, roles: [Role::PARTNER], resource: partner ) diff --git a/spec/services/user_invite_service_spec.rb b/spec/services/user_invite_service_spec.rb index 35a8fdb1c5..83720d9b17 100644 --- a/spec/services/user_invite_service_spec.rb +++ b/spec/services/user_invite_service_spec.rb @@ -1,6 +1,5 @@ RSpec.describe UserInviteService, type: :service, skip_seed: true do let(:organization) { FactoryBot.create(:organization) } - let(:partner) { FactoryBot.create(:partner, organization: organization) } before(:each) do allow(UserMailer).to receive(:role_added).and_return(double(:mail, deliver_later: nil)) @@ -54,34 +53,35 @@ end it "should create the user without name with role" do - result = nil expect { - result = described_class.invite(name: "", + @result = UserInviteService.invite( email: "email2@email.com", - roles: [Role::ORG_USER, Role::ORG_ADMIN], - resource: organization) + roles: [Role::PARTNER], + resource: @partner + ) }.to change { ActionMailer::Base.deliveries.count }.by(1) - expect(result.name).to eq("") - expect(result.email).to eq("email2@email.com") - expect(result).to have_role(Role::ORG_USER, organization) - expect(result).to have_role(Role::ORG_ADMIN, organization) + + user = User.find_by(email: "email2@email.com") + expect(user.name).to eq("Name Not Provided") + expect(user.email).to eq("email2@email.com") + expect(user).to have_role(Role::PARTNER, @partner) end end it "should invite a user with the partner role" do - result = nil expect { - result = described_class.invite( + @result = UserInviteService.invite( name: "Partner User", email: "partner@example.com", roles: [Role::PARTNER], - resource: partner + resource: @partner ) - }.to change(ActionMailer::Base.deliveries, :count).by(1) + }.to change { ActionMailer::Base.deliveries.count }.by(1) - expect(result.name).to eq("Partner User") - expect(result).not_to be_nil - expect(result.has_role?(:partner, partner)).to be true + user = User.find_by(email: "partner@example.com") + expect(user.name).to eq("Partner User") + expect(user).not_to be_nil + expect(user).to have_role(Role::PARTNER, @partner) end it "should create the user without a name with default role" do @@ -95,7 +95,7 @@ ) }.to change(ActionMailer::Base.deliveries, :count).by(1) - expect(result.name).to eq("Name Not Provided") + expect(result.reload.name).to eq("Name Not Provided") expect(result.email).to eq("email2@example.com") expect(result.has_role?(:org_user, organization)).to be true end From b8e74690cc34fe7b019b7e27a01d10e0f52a555a Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 29 Feb 2024 17:38:20 -0300 Subject: [PATCH 30/44] remove unnecessary white spaces --- app/controllers/partners_controller.rb | 1 - spec/requests/admin/users_requests_spec.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/partners_controller.rb b/app/controllers/partners_controller.rb index 9699140cd2..35f8722e65 100644 --- a/app/controllers/partners_controller.rb +++ b/app/controllers/partners_controller.rb @@ -131,7 +131,6 @@ def invite def invite_partner_user partner = current_organization.partners.find(params[:partner]) - UserInviteService.invite(email: params[:email], roles: [Role::PARTNER], resource: partner) diff --git a/spec/requests/admin/users_requests_spec.rb b/spec/requests/admin/users_requests_spec.rb index 3036aca0e0..56fa697475 100644 --- a/spec/requests/admin/users_requests_spec.rb +++ b/spec/requests/admin/users_requests_spec.rb @@ -128,7 +128,7 @@ end it "preloads organizations" do - post admin_users_path, params: { user: { organization_id: 1 }} + post admin_users_path, params: { user: { organization_id: 1 } } expect(assigns(:organizations)).to eq(Organization.all.alphabetized) end end From b9c6e388ca750fb624a581f4f1f0543ea88dbe3a Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 29 Feb 2024 18:16:39 -0300 Subject: [PATCH 31/44] Refactor invite_partner_user method in PartnersController to include name parameter --- app/controllers/partners_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/partners_controller.rb b/app/controllers/partners_controller.rb index 35f8722e65..d77f86b5ba 100644 --- a/app/controllers/partners_controller.rb +++ b/app/controllers/partners_controller.rb @@ -131,7 +131,8 @@ def invite def invite_partner_user partner = current_organization.partners.find(params[:partner]) - UserInviteService.invite(email: params[:email], + UserInviteService.invite(name: params[:name], + email: params[:email], roles: [Role::PARTNER], resource: partner) From d0af591e5df3203793aa9585f254e0f6d19cbf60 Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 29 Feb 2024 18:17:36 -0300 Subject: [PATCH 32/44] remove empty line --- spec/services/user_invite_service_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/services/user_invite_service_spec.rb b/spec/services/user_invite_service_spec.rb index 83720d9b17..9d1e0ed764 100644 --- a/spec/services/user_invite_service_spec.rb +++ b/spec/services/user_invite_service_spec.rb @@ -1,6 +1,5 @@ RSpec.describe UserInviteService, type: :service, skip_seed: true do let(:organization) { FactoryBot.create(:organization) } - before(:each) do allow(UserMailer).to receive(:role_added).and_return(double(:mail, deliver_later: nil)) end From 041e8319fe2f4a08019c411929e733f8d2cec408 Mon Sep 17 00:00:00 2001 From: cancelei Date: Thu, 29 Feb 2024 18:31:40 -0300 Subject: [PATCH 33/44] empty commit for running tests again From 122156da4c8ec1ac83f333754ebe6f154bca0d96 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 1 Mar 2024 04:35:07 +0000 Subject: [PATCH 34/44] build(deps): bump jwt from 2.8.0 to 2.8.1 Bumps [jwt](https://github.com/jwt/ruby-jwt) from 2.8.0 to 2.8.1. - [Release notes](https://github.com/jwt/ruby-jwt/releases) - [Changelog](https://github.com/jwt/ruby-jwt/blob/main/CHANGELOG.md) - [Commits](https://github.com/jwt/ruby-jwt/compare/v2.8.0...v2.8.1) --- updated-dependencies: - dependency-name: jwt dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1dd7f533df..68df21021f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -295,7 +295,7 @@ GEM actionview (>= 5.0.0) activesupport (>= 5.0.0) json (2.7.1) - jwt (2.8.0) + jwt (2.8.1) base64 kaminari (1.2.2) activesupport (>= 4.1.0) From 30bed4096808c616d01e27fa58fc40d07faa11eb Mon Sep 17 00:00:00 2001 From: Saravanan Kannan Date: Fri, 1 Mar 2024 23:20:37 +0530 Subject: [PATCH 35/44] Fix for copy calendar url button ui on pick ups and deliverables page --- app/assets/stylesheets/application.scss | 1 + app/assets/stylesheets/custom.scss | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index bf7d96f0d3..98dc87afc8 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -151,6 +151,7 @@ body a { #copy-calendar-button { margin-left: 10px; + height: fit-content; } select.selectpicker + .dropdown-toggle::after { diff --git a/app/assets/stylesheets/custom.scss b/app/assets/stylesheets/custom.scss index 1521d007e2..d205ce5c13 100644 --- a/app/assets/stylesheets/custom.scss +++ b/app/assets/stylesheets/custom.scss @@ -16,7 +16,8 @@ #calendar { background-color: #ffffff; - width: 100% + width: 100%; + height: 80% } #csvImportModal .modal-dialog.onboarding_steps .modal-body li { From 7ff074ddac60796be278380a3a21ada272eeb82b Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 1 Mar 2024 15:42:01 -0500 Subject: [PATCH 36/44] Change migration to make items invisible to partners --- db/migrate/20231222133931_activate_inactive_kit_items.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20231222133931_activate_inactive_kit_items.rb b/db/migrate/20231222133931_activate_inactive_kit_items.rb index 348c366c9e..d4a8da30b1 100644 --- a/db/migrate/20231222133931_activate_inactive_kit_items.rb +++ b/db/migrate/20231222133931_activate_inactive_kit_items.rb @@ -3,7 +3,7 @@ def up Kit.all.each do |kit| kit.line_items.each do |line_item| unless line_item.item.active? - line_item.item.update!(active: true) + line_item.item.update!(active: true, visible_to_partners: false) end end end From db8ba7d54684bbf44b5df78303d33413609ac3af Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 1 Mar 2024 15:47:19 -0500 Subject: [PATCH 37/44] missed a line --- app/queries/items_by_storage_collection_and_quantity_query.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/queries/items_by_storage_collection_and_quantity_query.rb b/app/queries/items_by_storage_collection_and_quantity_query.rb index a6c779849d..e192a1a5c2 100644 --- a/app/queries/items_by_storage_collection_and_quantity_query.rb +++ b/app/queries/items_by_storage_collection_and_quantity_query.rb @@ -30,7 +30,6 @@ def self.call(organization:, filter_params:, inventory: nil) end items_by_storage_collection = ItemsByStorageCollectionQuery.new(organization: organization, filter_params: filter_params).call - unless filter_params[:include_inactive_items] items_by_storage_collection_and_quantity = Hash.new items_by_storage_collection.each do |row| unless items_by_storage_collection_and_quantity.key?(row.id) From 8097efdd0c22cbc51b04292f4def799da208ccd3 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 1 Mar 2024 15:55:22 -0500 Subject: [PATCH 38/44] lint + spec fixes --- app/services/itemizable_update_service.rb | 2 +- spec/services/itemizable_update_service_spec.rb | 9 ++++----- spec/system/distribution_system_spec.rb | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/services/itemizable_update_service.rb b/app/services/itemizable_update_service.rb index 47546a7431..519817d00c 100644 --- a/app/services/itemizable_update_service.rb +++ b/app/services/itemizable_update_service.rb @@ -9,7 +9,7 @@ def self.call(itemizable:, type: :increase, params: {}, event_class: nil) item_ids = params[:line_items_attributes]&.values&.map { |i| i[:item_id].to_i } || [] inactive_item_names = Item.where(id: item_ids, active: false).pluck(:name) if inactive_item_names.any? - raise "Update failed: The following items are currently inactive: #{inactive_item_names.join(', ')}. Please reactivate them before continuing." + raise "Update failed: The following items are currently inactive: #{inactive_item_names.join(", ")}. Please reactivate them before continuing." end from_location = to_location = itemizable.storage_location diff --git a/spec/services/itemizable_update_service_spec.rb b/spec/services/itemizable_update_service_spec.rb index 415be6fae4..7f28888625 100644 --- a/spec/services/itemizable_update_service_spec.rb +++ b/spec/services/itemizable_update_service_spec.rb @@ -1,8 +1,8 @@ RSpec.describe ItemizableUpdateService do let(:storage_location) { create(:storage_location, organization: @organization, item_count: 0) } let(:new_storage_location) { create(:storage_location, organization: @organization, item_count: 0) } - let(:item1) { create(:item, organization: @organization, name: 'My Item 1') } - let(:item2) { create(:item, organization: @organization, name: 'My Item 2') } + let(:item1) { create(:item, organization: @organization, name: "My Item 1") } + let(:item2) { create(:item, organization: @organization, name: "My Item 2") } before(:each) do TestInventory.create_inventory(storage_location.organization, { storage_location.id => { @@ -70,12 +70,11 @@ expect(new_storage_location.size).to eq(24) end - it 'should raise an error if any item is inactive' do + it "should raise an error if any item is inactive" do item1.update!(active: false) msg = "Update failed: The following items are currently inactive: My Item 1. Please reactivate them before continuing." expect { subject }.to raise_error(msg) end - end describe "decreases" do @@ -122,7 +121,7 @@ expect(new_storage_location.size).to eq(16) end - it 'should raise an error if any item is inactive' do + it "should raise an error if any item is inactive" do item1.update!(active: false) msg = "Update failed: The following items are currently inactive: My Item 1. Please reactivate them before continuing." expect { subject }.to raise_error(msg) diff --git a/spec/system/distribution_system_spec.rb b/spec/system/distribution_system_spec.rb index c77e56d763..d95bc6634a 100644 --- a/spec/system/distribution_system_spec.rb +++ b/spec/system/distribution_system_spec.rb @@ -268,7 +268,7 @@ end context "when one of the items has been 'deleted'" do - it "the user can still reclaim it and it reactivates the item", js: true do + it "the user can still reclaim it", js: true do item = distribution.line_items.first.item item.destroy expect do @@ -276,7 +276,7 @@ click_on "Reclaim" end page.find ".alert" - end.to change { Distribution.count }.by(-1).and change { Item.active.count }.by(1) + end.to change { Distribution.count }.by(-1) expect(page).to have_content "reclaimed" end end From 50c7573ea046fda2f30aeda6b0be4a42e25b452c Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 1 Mar 2024 15:58:07 -0500 Subject: [PATCH 39/44] more lint fix --- app/views/distributions/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/distributions/_form.html.erb b/app/views/distributions/_form.html.erb index 1c72f3e133..c7656f58a2 100644 --- a/app/views/distributions/_form.html.erb +++ b/app/views/distributions/_form.html.erb @@ -24,7 +24,7 @@
- <%= render partial: "storage_locations/source", object: f, locals: { include_omitted_items: true } %> + <%= render partial: "storage_locations/source", object: f, locals: { include_omitted_items: true } %> <%= f.input :comment, label: "Comment" %> From ae8eb3987bc361db9b1165dc12732a7445998e87 Mon Sep 17 00:00:00 2001 From: Brock Wilcox Date: Sat, 2 Mar 2024 12:08:34 -0500 Subject: [PATCH 40/44] Prevent flaky test by sorting report values [#4066] --- app/services/reports/partner_info_report_service.rb | 2 +- spec/services/reports/partner_info_report_service_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/reports/partner_info_report_service.rb b/app/services/reports/partner_info_report_service.rb index ced0ebb1fb..9db4d27199 100644 --- a/app/services/reports/partner_info_report_service.rb +++ b/app/services/reports/partner_info_report_service.rb @@ -37,7 +37,7 @@ def partner_agency_counts end def partner_zipcodes_serviced - partner_agency_profiles.map(&:zips_served).uniq.join(', ') + partner_agency_profiles.map(&:zips_served).uniq.sort.join(', ') end end end diff --git a/spec/services/reports/partner_info_report_service_spec.rb b/spec/services/reports/partner_info_report_service_spec.rb index 3f8a88dc1a..c858307768 100644 --- a/spec/services/reports/partner_info_report_service_spec.rb +++ b/spec/services/reports/partner_info_report_service_spec.rb @@ -28,7 +28,7 @@ entries: { "Agency Type: Career technical training" => 2, "Agency Type: Education program" => 1, "Number of Partner Agencies" => 3, - "Zip Codes Served" => "90210-1234, 12345, 09876-3564" }, + "Zip Codes Served" => "09876-3564, 12345, 90210-1234" }, name: "Partner Agencies and Service Area" }) end From 7cce84b8ad9087a01ede922855d7aaf5e89cc4be Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 1 Mar 2024 15:39:48 -0500 Subject: [PATCH 41/44] Standard + rubocop bump --- Gemfile | 2 +- Gemfile.lock | 18 +++++++++--------- spec/pdfs/distribution_pdf_spec.rb | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Gemfile b/Gemfile index dc9134598f..ff5bed6979 100644 --- a/Gemfile +++ b/Gemfile @@ -156,7 +156,7 @@ group :development, :test do gem 'rubocop-performance' gem "rubocop-rails", "~> 2.23.1" # Default rules for Rubocop. - gem "standard", "~> 1.33" + gem "standard", "~> 1.34" # Erb linter. gem "erb_lint" end diff --git a/Gemfile.lock b/Gemfile.lock index 68df21021f..f5bbce51f4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -533,19 +533,19 @@ GEM rspec-mocks (~> 3.12) rspec-support (~> 3.12) rspec-support (3.12.1) - rubocop (1.59.0) + rubocop (1.61.0) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) - parser (>= 3.2.2.4) + parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) rexml (>= 3.2.5, < 4.0) rubocop-ast (>= 1.30.0, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.30.0) - parser (>= 3.2.1.0) + rubocop-ast (1.31.1) + parser (>= 3.3.0.4) rubocop-performance (1.20.2) rubocop (>= 1.48.1, < 2.0) rubocop-ast (>= 1.30.0, < 2.0) @@ -602,18 +602,18 @@ GEM actionpack (>= 5.2) activesupport (>= 5.2) sprockets (>= 3.0.0) - standard (1.33.0) + standard (1.34.0) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.0) - rubocop (~> 1.59.0) + rubocop (~> 1.60) standard-custom (~> 1.0.0) standard-performance (~> 1.3) standard-custom (1.0.2) lint_roller (~> 1.0) rubocop (~> 1.50) - standard-performance (1.3.0) + standard-performance (1.3.1) lint_roller (~> 1.1) - rubocop-performance (~> 1.20.1) + rubocop-performance (~> 1.20.2) stimulus-rails (1.3.3) railties (>= 6.0.0) strong_migrations (1.7.0) @@ -742,7 +742,7 @@ DEPENDENCIES simple_form simplecov sprockets (~> 4.2.1) - standard (~> 1.33) + standard (~> 1.34) stimulus-rails strong_migrations (= 1.7.0) terser diff --git a/spec/pdfs/distribution_pdf_spec.rb b/spec/pdfs/distribution_pdf_spec.rb index fcef9db93b..cabf6455ef 100644 --- a/spec/pdfs/distribution_pdf_spec.rb +++ b/spec/pdfs/distribution_pdf_spec.rb @@ -17,25 +17,25 @@ specify "#request_data" do results = described_class.new(@organization, distribution).request_data expect(results).to eq([ - ["Items Received", "Requested", "Received", "Value/item", "In-Kind Value Received", "Packages"], + ["Items Received", "Requested", "Received", "Value/item", "In-Kind Value Received", "Packages"], ["Item 1", "", 50, "$1.00", "$50.00", "1"], ["Item 2", 30, 100, "$2.00", "$200.00", nil], ["Item 3", 50, "", "$3.00", nil, nil], ["Item 4", 120, "", "$4.00", nil, nil], ["", "", "", "", ""], ["Total Items Received", 200, 150, "", "$250.00", ""] - ]) + ]) end specify "#non_request_data" do results = described_class.new(@organization, distribution).non_request_data expect(results).to eq([ - ["Items Received", "Value/item", "In-Kind Value", "Quantity", "Packages"], + ["Items Received", "Value/item", "In-Kind Value", "Quantity", "Packages"], ["Item 1", "$1.00", "$50.00", 50, "1"], ["Item 2", "$2.00", "$200.00", 100, nil], ["", "", "", "", ""], ["Total Items Received", "", "$250.00", 150, ""] - ]) + ]) end end # rubocop:enable Layout/ArrayAlignment From 04c399cab7f8a4ffec4442651712639f7da647bd Mon Sep 17 00:00:00 2001 From: Brock Wilcox Date: Sat, 2 Mar 2024 12:08:34 -0500 Subject: [PATCH 42/44] Prevent flaky test by sorting report values [#4066] --- app/services/reports/partner_info_report_service.rb | 2 +- spec/services/reports/partner_info_report_service_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/reports/partner_info_report_service.rb b/app/services/reports/partner_info_report_service.rb index ced0ebb1fb..9db4d27199 100644 --- a/app/services/reports/partner_info_report_service.rb +++ b/app/services/reports/partner_info_report_service.rb @@ -37,7 +37,7 @@ def partner_agency_counts end def partner_zipcodes_serviced - partner_agency_profiles.map(&:zips_served).uniq.join(', ') + partner_agency_profiles.map(&:zips_served).uniq.sort.join(', ') end end end diff --git a/spec/services/reports/partner_info_report_service_spec.rb b/spec/services/reports/partner_info_report_service_spec.rb index 3f8a88dc1a..c858307768 100644 --- a/spec/services/reports/partner_info_report_service_spec.rb +++ b/spec/services/reports/partner_info_report_service_spec.rb @@ -28,7 +28,7 @@ entries: { "Agency Type: Career technical training" => 2, "Agency Type: Education program" => 1, "Number of Partner Agencies" => 3, - "Zip Codes Served" => "90210-1234, 12345, 09876-3564" }, + "Zip Codes Served" => "09876-3564, 12345, 90210-1234" }, name: "Partner Agencies and Service Area" }) end From 8cf8d896eb005ce56315ac22dcdff115c8f27646 Mon Sep 17 00:00:00 2001 From: elasticspoon Date: Sun, 3 Mar 2024 10:58:48 -0500 Subject: [PATCH 43/44] 4134 fix inconsistent request behavior (#4146) * fix(#4134): inconsistent request behavior Fixes #4134 Individual requests and Quantity requests are very similar but have small differences in behavior: - Quantity requests allow comment only requests - Quantity requests filter out blank lines This PR will unify the behavior so both Individual and Quantity requests will act like Quantity requests. * fix: minor changes fix: typo fix: revert automated minor schema change --- Gemfile.lock | 2 +- .../partners/family_request_create_service.rb | 3 +- db/schema.rb | 2 +- .../partners/managing_requests_system_spec.rb | 82 ++++++++++++++++++- 4 files changed, 82 insertions(+), 7 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f5bbce51f4..87d67ba48e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -751,4 +751,4 @@ DEPENDENCIES webmock (~> 3.23) BUNDLED WITH - 2.5.4 + 2.5.6 diff --git a/app/services/partners/family_request_create_service.rb b/app/services/partners/family_request_create_service.rb index 0099c56d8b..fcd1cb48ec 100644 --- a/app/services/partners/family_request_create_service.rb +++ b/app/services/partners/family_request_create_service.rb @@ -54,7 +54,8 @@ def valid? end def item_requests_attributes - @item_requests_attributes ||= family_requests_attributes.map do |fr_attr| + @item_requests_attributes ||= family_requests_attributes.filter_map do |fr_attr| + next if fr_attr[:item_id].blank? && fr_attr[:person_count].blank? { item_id: fr_attr[:item_id], quantity: convert_person_count_to_item_quantity(item_id: fr_attr[:item_id], person_count: fr_attr[:person_count])&.to_i, diff --git a/db/schema.rb b/db/schema.rb index ec0fb34d6a..fc8f9f5a5c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -233,8 +233,8 @@ t.integer "organization_id" t.datetime "issued_at", precision: nil t.string "agency_rep" - t.integer "state", default: 5, null: false t.boolean "reminder_email_enabled", default: false, null: false + t.integer "state", default: 5, null: false t.integer "delivery_method", default: 0, null: false t.decimal "shipping_cost", precision: 8, scale: 2 t.index ["organization_id"], name: "index_distributions_on_organization_id" diff --git a/spec/system/partners/managing_requests_system_spec.rb b/spec/system/partners/managing_requests_system_spec.rb index cfe346f18e..a3d3a2a450 100644 --- a/spec/system/partners/managing_requests_system_spec.rb +++ b/spec/system/partners/managing_requests_system_spec.rb @@ -17,7 +17,7 @@ expect(page.all('select[name="partners_family_request[items_attributes][0][item_id]"] option').map(&:text)).to eq(expected_items) end - context 'WHEN they create a request inproperly' do + context 'WHEN they create a request improperly' do before { click_button 'Submit Essentials Request' click_link 'Add Another Item' @@ -35,7 +35,57 @@ visit new_partners_individuals_request_path end - context 'WHEN they create a request inproperly' do + context 'WHEN they create a request improperly by not inputting anything' do + before do + click_button 'Submit Essentials Request' + end + + it 'should show an error message with the instructions ' do + expect(page).to have_content('Oops! Something went wrong with your Request') + expect(page).to have_content('Ensure each line item has a item selected AND a quantity greater than 0.') + expect(page).to have_content('Still need help? Submit a support ticket here and we will do our best to follow up with you via email.') + end + end + + context 'WHEN they create a request with only a comment' do + before do + fill_in 'Comments', with: Faker::Lorem.paragraph + end + + it 'should be created without any issue' do + expect { click_button 'Submit Essentials Request' }.to change { Request.count }.by(1) + + expect(current_path).to eq(partners_request_path(Request.last.id)) + expect(page).to have_content('Request has been successfully created!') + expect(page).to have_content("#{partner.organization.name} should have received the request.") + end + end + + context 'WHEN they create a request with blank lines' do + before do + fill_in 'Comments', with: Faker::Lorem.paragraph + + # fill in an item + click_link 'Add Another Item' + item = partner_user.partner.organization.valid_items.sample + last_row = find_all('tr').last + last_row.find('option', text: item[:name], exact_text: true).select_option + last_row.find_all('.form-control').last.fill_in(with: 1) + + # Trigger another row but keep it empty. It should still be valid! + click_link 'Add Another Item' + end + + it 'should be created without any issue' do + expect { click_button 'Submit Essentials Request' }.to change { Request.count }.by(1) + + expect(current_path).to eq(partners_request_path(Request.last.id)) + expect(page).to have_content('Request has been successfully created!') + expect(page).to have_content("#{partner.organization.name} should have received the request.") + end + end + + context 'WHEN they create a request completely empty request' do before do click_button 'Submit Essentials Request' end @@ -123,7 +173,7 @@ expect(page.all('select[name="request[item_requests_attributes][0][item_id]"] option').map(&:text)).to eq(expected_items) end - context 'WHEN they create a request inproperly' do + context 'WHEN they create a request improperly' do before { click_button 'Submit Essentials Request' click_link 'Add Another Item' @@ -142,7 +192,7 @@ visit new_partners_request_path end - context 'WHEN they create a request inproperly by not inputting anything' do + context 'WHEN they create a request completely empty request' do before do click_button 'Submit Essentials Request' end @@ -168,6 +218,30 @@ end end + context 'WHEN they create a request with blank lines' do + before do + fill_in 'Comments', with: Faker::Lorem.paragraph + + # fill in an item + click_link 'Add Another Item' + item = partner_user.partner.organization.valid_items.sample + last_row = find_all('tr').last + last_row.find('option', text: item[:name], exact_text: true).select_option + last_row.find_all('.form-control').last.fill_in(with: 1) + + # Trigger another row but keep it empty. It should still be valid! + click_link 'Add Another Item' + end + + it 'should be created without any issue' do + expect { click_button 'Submit Essentials Request' }.to change { Request.count }.by(1) + + expect(current_path).to eq(partners_request_path(Request.last.id)) + expect(page).to have_content('Request has been successfully created!') + expect(page).to have_content("#{partner.organization.name} should have received the request.") + end + end + context 'WHEN they create a request properly' do let(:items_to_select) { partner_user.partner.organization.valid_items.sample(3) } let(:item_details) do From a46d781eca2723c4e5079782becc95fc7b73ee40 Mon Sep 17 00:00:00 2001 From: Saravanan Date: Sun, 3 Mar 2024 21:28:57 +0530 Subject: [PATCH 44/44] Product drives name filter - to list names in alphabetical order (#4150) * Product drives name filter - to list names in alphabetical order * fix rubocop - unnecessary space issue * updates instance var name to be more descriptive --------- Co-authored-by: Saravanan Kannan --- app/controllers/product_drives_controller.rb | 2 ++ app/views/product_drives/index.html.erb | 2 +- spec/system/product_drive_system_spec.rb | 14 ++++++++------ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/app/controllers/product_drives_controller.rb b/app/controllers/product_drives_controller.rb index d3b5e7f2a9..05e55e5dca 100644 --- a/app/controllers/product_drives_controller.rb +++ b/app/controllers/product_drives_controller.rb @@ -10,6 +10,8 @@ def index .class_filter(filter_params) .within_date_range(@selected_date_range) .order(start_date: :desc) + # to be used in the name filter to sort product drives in alpha order + @product_drives_alphabetical = @product_drives.sort_by { |pd| pd.name.downcase } @item_categories = current_organization.item_categories @selected_name_filter = filter_params[:by_name] @selected_item_category = filter_params[:by_item_category_id] diff --git a/app/views/product_drives/index.html.erb b/app/views/product_drives/index.html.erb index 8449bf8ffa..8761f14b46 100644 --- a/app/views/product_drives/index.html.erb +++ b/app/views/product_drives/index.html.erb @@ -38,7 +38,7 @@ <%= form_tag(product_drives_path, method: :get, organization_id: current_organization) do |f| %>
- <%= filter_select(scope: :by_name, collection: @product_drives, key: :name, value: :name, selected: @selected_name_filter) %> + <%= filter_select(scope: :by_name, collection: @product_drives_alphabetical, key: :name, value: :name, selected: @selected_name_filter) %>
<%= filter_select(label: "Filter by item category", scope: :by_item_category_id, collection: @item_categories, selected: @selected_item_category) %> diff --git a/spec/system/product_drive_system_spec.rb b/spec/system/product_drive_system_spec.rb index 09b2a503b3..cc144a0522 100644 --- a/spec/system/product_drive_system_spec.rb +++ b/spec/system/product_drive_system_spec.rb @@ -18,12 +18,14 @@ before(:each) do @product_drives = [ create(:product_drive, name: "Test name 1", start_date: 3.weeks.ago, end_date: 2.weeks.ago, virtual: true), - create(:product_drive, name: "Test name 2", start_date: 2.weeks.ago, end_date: 1.week.ago, virtual: false) + create(:product_drive, name: "Test name 2", start_date: 2.weeks.ago, end_date: 1.week.ago, virtual: false), + create(:product_drive, name: "Alpha Test name 3", start_date: 1.week.from_now, end_date: 2.weeks.from_now, virtual: false) ] visit subject end - it "Shows the expected filters with the expected values" do + it "Shows the expected filters with the expected values and in alphabetical order for name filter" do + expect(page.find("select[name='filters[by_name]']").find(:xpath, 'option[2]').text).to eq "Alpha Test name 3" expect(page.has_select?('filters[by_name]', with_options: @product_drives.map(&:name))).to be true expect(page.has_field?('filters_date_range', with: this_year)) end @@ -35,16 +37,16 @@ end end - it 'shows only one virtual product drive' do + it 'shows only one virtual product drives' do expect(page).to have_text(/Yes/, maximum: 1) end - it 'shows only one non-virtual product drive' do - expect(page).to have_text(/No/, maximum: 1) + it 'shows two non-virtual product drives' do + expect(page).to have_text(/No/, maximum: 2) end it 'shows in descending order of start date' do - expect("Test name 2").to appear_before("Test name 1") + expect("Alpha Test name 3").to appear_before("Test name 1") end end