Merge branch 'main' into 4248-fix-distribution-flaky-test

Gemfile

@@ -210,6 +210,6 @@ if %w(mingw mswin x64_mingw jruby).include?(RUBY_PLATFORM)
 end
 
 # Use Redis for Action Cable
-gem "redis", "~> 5.1"
+gem "redis", "~> 5.2"
 
 gem "importmap-rails", "~> 2.0"

Gemfile.lock

@@ -252,7 +252,7 @@ GEM
       rack (>= 1.4, < 4)
       rack-protection (>= 1.5.3, <= 4.0.0)
       sanitize (< 7)
-    foreman (0.87.2)
+    foreman (0.88.1)
     formatador (1.1.0)
     geocoder (1.8.2)
     globalid (1.2.1)
@@ -500,9 +500,9 @@ GEM
     rb-inotify (0.10.1)
       ffi (~> 1.0)
     recaptcha (5.16.0)
-    redis (5.1.0)
-      redis-client (>= 0.17.0)
-    redis-client (0.20.0)
+    redis (5.2.0)
+      redis-client (>= 0.22.0)
+    redis-client (0.22.0)
       connection_pool
     regexp_parser (2.9.0)
     request_store (1.5.1)
@@ -732,7 +732,7 @@ DEPENDENCIES
   rails-controller-testing
   rails-erd
   recaptcha
-  redis (~> 5.1)
+  redis (~> 5.2)
   rolify (~> 6.0)
   rspec-rails (~> 6.1.2)
   rubocop

spec/models/broadcast_announcement_spec.rb

@@ -47,29 +47,29 @@
 
   context "filter_announcements" do
     it "should include only announcements from the passed organization" do
-      BroadcastAnnouncement.create!(message: "test", user_id: 1, organization_id: 1)
-      BroadcastAnnouncement.create!(message: "test", user_id: 1, organization_id: 1)
-      BroadcastAnnouncement.create!(message: "test", user_id: 1)
-      BroadcastAnnouncement.create!(message: "test", user_id: 1, organization_id: 2)
-      expect(BroadcastAnnouncement.filter_announcements(1).count).to eq(2)
+      BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id)
+      BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id)
+      BroadcastAnnouncement.create!(message: "test", user_id: @user.id)
+      BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: 0)
+      expect(BroadcastAnnouncement.filter_announcements(@organization.id).count).to eq(2)
     end
 
     it "shouldn't include expired announcements" do
-      BroadcastAnnouncement.create!(message: "test", user_id: 1, organization_id: 1)
-      BroadcastAnnouncement.create!(message: "test", user_id: 1, expiry: 2.days.ago, organization_id: 1)
-      BroadcastAnnouncement.create!(message: "test", user_id: 1, expiry: 5.days.ago, organization_id: 1)
-      BroadcastAnnouncement.create!(message: "test", user_id: 1, expiry: Time.zone.today, organization_id: 1)
-      expect(BroadcastAnnouncement.filter_announcements(1).count).to eq(2)
+      BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id)
+      BroadcastAnnouncement.create!(message: "test", user_id: @user.id, expiry: 2.days.ago, organization_id: @organization.id)
+      BroadcastAnnouncement.create!(message: "test", user_id: @user.id, expiry: 5.days.ago, organization_id: @organization.id)
+      BroadcastAnnouncement.create!(message: "test", user_id: @user.id, expiry: Time.zone.today, organization_id: @organization.id)
+      expect(BroadcastAnnouncement.filter_announcements(@organization.id).count).to eq(2)
     end
 
     it "sorts announcements from most recently created to last" do
-      announcement_1 = BroadcastAnnouncement.create!(message: "test", user_id: 1, organization_id: 1, created_at: Time.zone.today)
-      announcement_2 = BroadcastAnnouncement.create!(message: "test", user_id: 1, organization_id: 1, created_at: 2.days.ago)
-      announcement_3 = BroadcastAnnouncement.create!(message: "test", user_id: 1, organization_id: 1, expiry: 1.day.ago, created_at: 3.days.ago)
-      announcement_4 = BroadcastAnnouncement.create!(message: "test", user_id: 1, organization_id: 1, created_at: 4.days.ago)
+      announcement_1 = BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id, created_at: Time.zone.today)
+      announcement_2 = BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id, created_at: 2.days.ago)
+      announcement_3 = BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id, expiry: 1.day.ago, created_at: 3.days.ago)
+      announcement_4 = BroadcastAnnouncement.create!(message: "test", user_id: @user.id, organization_id: @organization.id, created_at: 4.days.ago)
 
-      expect(BroadcastAnnouncement.filter_announcements(1)).to eq([announcement_1, announcement_2, announcement_4])
-      expect(BroadcastAnnouncement.filter_announcements(1).include?(announcement_3)).to be(false)
+      expect(BroadcastAnnouncement.filter_announcements(@organization.id)).to eq([announcement_1, announcement_2, announcement_4])
+      expect(BroadcastAnnouncement.filter_announcements(@organization.id).include?(announcement_3)).to be(false)
     end
   end
 

spec/requests/admin/broadcast_announcements_spec.rb

@@ -9,7 +9,7 @@
       expiry: Time.zone.today,
       link: "http://google.com",
       message: "test",
-      user_id: 1,
+      user_id: @user.id,
       organization_id: nil
     }
   }
@@ -74,8 +74,8 @@
           expiry: Time.zone.yesterday,
           link: "http://google.com",
           message: "new_test",
-          user_id: 1,
-          organization_id: 1
+          user_id: @user.id,
+          organization_id: @organization.id
         }
       }
 
@@ -121,7 +121,7 @@
 
     describe "POST /create" do
       it "redirects" do
-        post admin_broadcast_announcements_url, params: {user: 1, message: "test"}
+        post admin_broadcast_announcements_url, params: {user: @user.id, message: "test"}
         expect(response).to redirect_to(dashboard_path(organization_name: @organization_admin.organization))
       end
     end

spec/requests/admin/users_requests_spec.rb

@@ -124,12 +124,12 @@
 
     describe "POST #create" do
       it "returns http success" do
-        post admin_users_path, params: { user: { email: '[email protected]', organization_id: 1 } }
+        post admin_users_path, params: { user: { email: @organization.email, organization_id: @organization.id } }
         expect(response).to redirect_to(admin_users_path)
       end
 
       it "preloads organizations" do
-        post admin_users_path, params: { user: { organization_id: 1 } }
+        post admin_users_path, params: { user: { organization_id: @organization.id } }
         expect(assigns(:organizations)).to eq(Organization.all.alphabetized)
       end
     end
@@ -150,7 +150,7 @@
 
     describe "POST #create" do
       it "redirects" do
-        post admin_users_path, params: { user: { organization_id: 1 } }
+        post admin_users_path, params: { user: { organization_id: @organization.id } }
         expect(response).to redirect_to(dashboard_path(organization_name: @organization_admin.organization))
       end
     end
@@ -171,7 +171,7 @@
 
     describe "POST #create" do
       it "redirects" do
-        post admin_users_path, params: { user: { organization_id: 1 } }
+        post admin_users_path, params: { user: { organization_id: @organization.id } }
         expect(response).to redirect_to(dashboard_path(organization_name: @user.organization))
       end
     end

spec/requests/broadcast_announcements_spec.rb

@@ -9,8 +9,8 @@
       expiry: Time.zone.today,
       link: "http://google.com",
       message: "test",
-      user_id: 1,
-      organization_id: 1
+      user_id: @user.id,
+      organization_id: @organization.id
     }
   }
 
@@ -74,8 +74,8 @@
           expiry: Time.zone.yesterday,
           link: "http://google.com",
           message: "new_test",
-          user_id: 1,
-          organization_id: 1
+          user_id: @user.id,
+          organization_id: @organization.id
         }
       }
 

spec/requests/dashboard_requests_spec.rb

@@ -37,13 +37,13 @@
 
     context "BroadcastAnnouncement card" do
       it "displays announcements if there are valid ones" do
-        BroadcastAnnouncement.create(message: "test announcement", user_id: 1, organization_id: nil)
+        BroadcastAnnouncement.create(message: "test announcement", user_id: @user.id, organization_id: nil)
         get dashboard_path(default_params)
         expect(response.body).to include("test announcement")
       end
 
       it "doesn't display announcements if they are not from super admins" do
-        BroadcastAnnouncement.create(message: "test announcement", user_id: 1, organization_id: 1)
+        BroadcastAnnouncement.create(message: "test announcement", user_id: @user.id, organization_id: @organization.id)
         get dashboard_path(default_params)
         expect(response.body).not_to include("test announcement")
       end

spec/requests/donations_requests_spec.rb

@@ -30,46 +30,48 @@
         end
 
         context "when given a product drive" do
-          let(:product_drive) { create(:product_drive) }
+          let(:product_drive) { create(:product_drive, name: "Drive Name") }
           let(:donation) { create(:donation, source: "Product Drive", product_drive: product_drive) }
 
           it "should display Product Drive and the name of the drive" do
             donation
-            expect(subject.body).to include("<td>#{donation.source}</td>")
-            expect(subject.body).to include("<td>#{product_drive.name}</td>")
+            expect(subject.body).to include("<td>Product Drive</td>")
+            expect(subject.body).to include("<td>Drive Name</td>")
           end
         end
 
         context "when given a donation site" do
-          let(:donation_site) { create(:donation_site) }
+          let(:donation_site) { create(:donation_site, name: "Site Name") }
           let(:donation) { create(:donation, source: "Donation Site", donation_site: donation_site) }
 
           it "should display Donation Site and the name of the site" do
             donation
-            expect(subject.body).to include("<td>#{donation.source}</td>")
-            expect(subject.body).to include("<td>#{donation_site.name}</td>")
+            expect(subject.body).to include("<td>Donation Site</td>")
+            expect(subject.body).to include("<td>Site Name</td>")
           end
         end
 
         context "when given a manufacturer" do
-          let(:manufacturer) { create(:manufacturer) }
+          let(:manufacturer) { create(:manufacturer, name: "Manufacturer Name") }
           let(:donation) { create(:donation, source: "Manufacturer", manufacturer: manufacturer) }
 
           it "should display Manufacturer and the manufacturer name" do
             donation
-            expect(subject.body).to include("<td>#{donation.source}</td>")
-            expect(subject.body).to include("<td>#{manufacturer.name}</td>")
+            expect(subject.body).to include("<td>Manufacturer</td>")
+            expect(subject.body).to include("<td>Manufacturer Name</td>")
           end
         end
 
         context "when given a misc donation" do
-          let(:donation) { create(:donation, source: "Misc. Donation", comment: Faker::Lorem.paragraph) }
+          let(:full_comment) { Faker::Lorem.paragraph }
+          let(:donation) { create(:donation, source: "Misc. Donation", comment: full_comment) }
 
           it "should display Misc Donation and a truncated comment" do
             donation
-            short_comment = donation.comment.truncate(25, separator: /\s/)
-            expect(subject.body).to include("<td>#{donation.source}</td>")
+            short_comment = full_comment.truncate(25, separator: /\s/)
+            expect(subject.body).to include("<td>Misc. Donation</td>")
             expect(subject.body).to include("<td>#{short_comment}</td>")
+            expect(subject.body).to_not include("<td>#{full_comment}</td>")
           end
         end
       end

spec/requests/partners/dashboard_requests_spec.rb

@@ -59,19 +59,19 @@
 
   context "BroadcastAnnouncement card" do
     it "displays announcements if there are valid ones" do
-      BroadcastAnnouncement.create(message: "test announcement", user_id: 1, organization_id: 1)
+      BroadcastAnnouncement.create(message: "test announcement", user_id: @user.id, organization_id: @organization.id)
       get partners_dashboard_path
       expect(response.body).to include("test announcement")
     end
 
     it "doesn't display announcements if there are not valid ones" do
-      BroadcastAnnouncement.create(expiry: 5.days.ago, message: "test announcement", user_id: 1, organization_id: 1)
+      BroadcastAnnouncement.create(expiry: 5.days.ago, message: "test announcement", user_id: @user.id, organization_id: @organization.id)
       get partners_dashboard_path
       expect(response.body).not_to include("test announcement")
     end
 
     it "doesn't display announcements from super admins" do
-      BroadcastAnnouncement.create(message: "test announcement", user_id: 1, organization_id: nil)
+      BroadcastAnnouncement.create(message: "test announcement", user_id: @user.id, organization_id: nil)
       get partners_dashboard_path
       expect(response.body).not_to include("test announcement")
     end

spec/requests/storage_locations_requests_spec.rb

@@ -9,7 +9,7 @@
     end
 
     describe "GET #index" do
-      before { create(:storage_location, name: "Test Storage Location", warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) }
+      before { create(:storage_location, name: "Test Storage Location", address: "123 Donation Site Way", warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) }
 
       context "html" do
         let(:response_format) { 'html' }
@@ -67,9 +67,11 @@
         end
 
         context "when read_events feature toggle is enabled" do
-          let(:storage_location_with_items) { create(:storage_location, name: "Storage Location with Items", warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) }
-          let(:storage_location_with_duplicate_item) { create(:storage_location, name: "Storage Location with Duplicate Items", warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) }
-          let(:storage_location_with_unique_item) { create(:storage_location, name: "Storage Location with Unique Items", warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) }
+          # Addresses used for storage locations must have associated geocoder stubs.
+          # See calls to Geocoder::Lookup::Test.add_stub in spec/rails_helper.rb
+          let(:storage_location_with_duplicate_item) { create(:storage_location, name: "Storage Location with Duplicate Items", address: "1500 Remount Road, Front Royal, VA 22630", warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) }
+          let(:storage_location_with_items) { create(:storage_location, name: "Storage Location with Items", address: "123 Donation Site Way", warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) }
+          let(:storage_location_with_unique_item) { create(:storage_location, name: "Storage Location with Unique Items", address: "Smithsonian Conservation Center new", warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) }
           let(:item1) { create(:item, name: 'A') }
           let(:item2) { create(:item, name: 'B') }
           let(:item3) { create(:item, name: 'C') }
@@ -96,12 +98,13 @@
 
           it "Generates csv with Storage Location fields, alphabetized item names, item quantities lined up in their columns, and zeroes for no inventory" do
             get storage_locations_path(default_params.merge(format: response_format))
+            # The first address below is quoted since it contains commas
             csv = <<~CSV
               Name,Address,Square Footage,Warehouse Type,Total Inventory,A,B,C,D
               Storage Location with Duplicate Items,"1500 Remount Road, Front Royal, VA 22630",100,Residential space used,1,0,0,1,0
-              Storage Location with Items,"1500 Remount Road, Front Royal, VA 22630",100,Residential space used,3,1,1,1,0
-              Storage Location with Unique Items,"1500 Remount Road, Front Royal, VA 22630",100,Residential space used,5,0,0,0,5
-              Test Storage Location,"1500 Remount Road, Front Royal, VA 22630",100,Residential space used,0,0,0,0,0
+              Storage Location with Items,123 Donation Site Way,100,Residential space used,3,1,1,1,0
+              Storage Location with Unique Items,Smithsonian Conservation Center new,100,Residential space used,5,0,0,0,5
+              Test Storage Location,123 Donation Site Way,100,Residential space used,0,0,0,0,0
             CSV
             expect(response.body).to eq(csv)
           end

spec/services/partners/request_create_service_spec.rb

@@ -56,14 +56,6 @@
 
     context 'when the arguments are correct' do
       let(:items_to_request) { BaseItem.all.sample(3) }
-      let(:item_requests_attributes) do
-        items_to_request.map do |item|
-          ActionController::Parameters.new(
-            item_id: item.id,
-            quantity: Faker::Number.within(range: 1..10)
-          )
-        end
-      end
       let(:fake_organization_valid_items) do
         items_to_request.map do |item|
           {

spec/system/admin/users_system_spec.rb

@@ -72,11 +72,11 @@
     end
 
     it "filters users by email" do
-      user_email = "[email protected]"
+      user_email = @organization_admin.email
 
       visit admin_users_path
       fill_in "filterrific_search_email", with: user_email
-      expect(page.find("table")).to have_content(user_email)
+      expect(page).to have_element("table", text: user_email)
     end
   end
 end

spec/system/item_system_spec.rb

@@ -180,7 +180,7 @@
         let(:item) { Item.first }
 
         before do
-          find('tr', text: item.name).find('a', text: 'Edit').click
+          find("tr[data-item-id=\"#{item.id}\"]").find('a', text: 'Edit').click
           select new_item_category, from: 'Category'
           click_on 'Save'
         end