Fix Chrome 134 breaking changes/Add expect checks

aaronskiba · aaronskiba · commit 8dbbddf2273e · 2025-03-19T09:58:13.000-06:00
- These changes address the Chrome 134 breaking changes (#3485). - Added `expect` statements to ensure that page loads are completing successfully. - The `expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})` statements help verify the page load. - It would be better to implement a notification such as "Customised template created successfully." and use that to verify the page load. - These changes should be revisited (and possibly reverted) when a fix is available for these breaking changes via Chrome, Capybara, or Selenium.
diff --git a/spec/features/annotations/annotations_editing_spec.rb b/spec/features/annotations/annotations_editing_spec.rb
@@ -37,6 +37,9 @@
     end
     expect do
       click_link 'Customise'
+      # `org_admin_template_path(Template.last)` would be preferred over %r{#{org_admin_templates_path}/\d+}
+      # However, the test is currently evaluating Template.count prior to the new Template being created
+      expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})
     end.to change { Template.count }.by(1)
 
     # New Template created
@@ -53,7 +56,13 @@
     # NOTE: This is question 2, since Annotation was copied upon clicking "Customise"
     within("#edit_question_#{template.question_ids.last}") do
       # Expect it to destroy the newly cleared Annotation
-      expect { click_button 'Save' }.not_to change { Annotation.count }
+      expect do
+        click_button 'Save'
+        current_path = org_admin_template_phase_path(template,
+                                                     template.phases.first) +
+                       "?section=#{template.phases.first.sections.first.id}"
+        expect(page).to have_current_path(current_path)
+      end.not_to change { Annotation.count }
     end
     expect(annotation.text).to eql('Foo bar')
     expect(Annotation.order('created_at').last.text).to eql('<p>Noo bar</p>')
@@ -67,6 +76,9 @@
     end
     expect do
       click_link 'Customise'
+      # `org_admin_template_path(Template.last)` would be preferred over %r{#{org_admin_templates_path}/\d+}
+      # However, the test is currently evaluating Template.count prior to the new Template being created
+      expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})
     end.to change { Template.count }.by(1)
     template = Template.last
     click_link 'Customise phase'
@@ -79,7 +91,13 @@
     # NOTE: This is question 2, since Annotation was copied upon clicking "Customise"
     within("#edit_question_#{template.question_ids.last}") do
       # Expect it to destroy the newly cleared Annotation
-      expect { click_button 'Save' }.to change { Annotation.count }.by(-1)
+      expect do
+        click_button 'Save'
+        current_path = org_admin_template_phase_path(template,
+                                                     template.phases.first) +
+                       "?section=#{template.phases.first.sections.first.id}"
+        expect(page).to have_current_path(current_path)
+      end.to change { Annotation.count }.by(-1)
     end
     expect(page).not_to have_errors
   end
diff --git a/spec/features/feedback_requests_spec.rb b/spec/features/feedback_requests_spec.rb
@@ -40,6 +40,7 @@
     end
 
     # Expectations
+    expect(page).to have_content('Feedback has been requested.')
     expect(plan.reload).to be_feedback_requested
     expect(ActionMailer::Base.deliveries).to have_exactly(1).item
   end
diff --git a/spec/features/plans_spec.rb b/spec/features/plans_spec.rb
@@ -45,6 +45,7 @@
     click_button 'Create plan'
 
     # Expectations
+    expect(page).to have_content('Successfully created the plan.')
     expect(@user.plans).to be_one
     @plan = Plan.last
     expect(current_path).to eql(plan_path(@plan))
diff --git a/spec/features/registrations_spec.rb b/spec/features/registrations_spec.rb
@@ -26,6 +26,7 @@
     click_button 'Create account'
 
     # Expectations
+    expect(page).to have_text('Welcome! You have signed up successfully.')
     expect(current_path).to eql(plans_path)
     expect(page).to have_text(user_attributes[:firstname])
     expect(page).to have_text(user_attributes[:surname])
diff --git a/spec/features/sessions_spec.rb b/spec/features/sessions_spec.rb
@@ -15,6 +15,7 @@
     click_button 'Sign in'
 
     # Expectation
+    expect(page).to have_content('Signed in successfully.')
     expect(current_path).to eql(plans_path)
     expect(page).to have_text(user.firstname)
     expect(page).to have_text(user.surname)
diff --git a/spec/features/templates/templates_copying_spec.rb b/spec/features/templates/templates_copying_spec.rb
@@ -34,6 +34,7 @@
     end
 
     # Expectations
+    expect(page).to have_content('Template was successfully copied.')
     expect(Template.count).to eql(2)
     new_template = Template.last
     expect(new_template.title).to include(parent_template.title)
diff --git a/spec/features/templates/templates_editings_spec.rb b/spec/features/templates/templates_editings_spec.rb
@@ -26,10 +26,14 @@
 
   scenario "Admin edits a Template's existing question", :js do
     click_link 'Customisable Templates'
+    expect(page).to have_current_path(customisable_org_admin_templates_path)
     within("#template_#{template.id}") do
       click_button 'Actions'
     end
     click_link 'Customise'
+    # `org_admin_template_path(Template.last)` would be preferred over %r{#{org_admin_templates_path}/\d+}
+    # However, the test is currently evaluating Template.count prior to the new Template being created
+    expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})
     # New template created
     template = Template.last
     within("#phase_#{template.phase_ids.first}") do
diff --git a/spec/features/templates/templates_upgrade_customisations_spec.rb b/spec/features/templates/templates_upgrade_customisations_spec.rb
@@ -36,7 +36,12 @@
     click_link('Customisable Templates')
 
     click_button 'Actions'
-    expect { click_link 'Customise' }.to change { Template.count }.by(1)
+    expect do
+      click_link 'Customise'
+      # `org_admin_template_path(Template.last)` would be preferred over %r{#{org_admin_templates_path}/\d+}
+      # However, the test is currently evaluating Template.count prior to the new Template being created
+      expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})
+    end.to change { Template.count }.by(1)
 
     customized_template = Template.last
 
@@ -48,6 +53,7 @@
     # Publish our customisation
     click_button 'Actions'
     click_link 'Publish'
+    expect(page).to have_text('Your customisation has been published')
     expect(customized_template.reload.published?).to eql(true)
 
     # Move to the other funder Org's Templates
@@ -88,6 +94,7 @@
 
     click_button 'Actions'
     click_link 'Publish changes'
+    expect(page).to have_text('Your template has been published')
     expect(new_funder_template.reload.published?).to eql(true)
 
     # Go back to the original Org...
diff --git a/spec/support/helpers/sessions_helper.rb b/spec/support/helpers/sessions_helper.rb
@@ -20,5 +20,6 @@ def sign_in_as_user(user)
       fill_in 'Password', with: user.password.presence || 'password'
       click_button 'Sign in'
     end
+    expect(page).to have_content('Signed in successfully.')
   end
 end
