Merge pull request #6022 from nhsuk/fix-redirect-after-resolve-duplicate-to-import

Fix: return to relevant import after resolving an issue

Author: jhenderson

app/components/app_import_review_issues_summary_component.rb

@@ -132,7 +132,9 @@ def review_link(record, type)
     full_name =
       record.is_a?(Patient) ? record.full_name : record.patient&.full_name
 
-    helpers.link_to(imports_issue_path(record, type:)) do
+    helpers.link_to(
+      imports_issue_path(record, type:, return_to: url_for(import))
+    ) do
       helpers.safe_join(
         ["Review ", tag.span(full_name, class: "nhsuk-u-visually-hidden")]
       )

app/controllers/imports/issues_controller.rb

@@ -19,9 +19,11 @@ def show
 
   def update
     if @form.save
-      redirect_to imports_issues_path, flash: { success: "Record updated" }
+      redirect_target = params[:return_to].presence || imports_issues_path
+
+      redirect_to redirect_target, flash: { success: "Record updated" }
     else
-      render :show, status: :unprocessable_content and return
+      render :show, status: :unprocessable_content
     end
   end
 

app/views/imports/issues/show.html.erb

@@ -42,7 +42,11 @@
 
 <%= form_with(
       model: @form,
-      url: imports_issue_path(@record, type: params[:type]),
+      url: imports_issue_path(
+        @record,
+        type: params[:type],
+        return_to: params[:return_to],
+      ),
       method: :patch,
       class: "nhsuk-u-full-width",
     ) do |f| %>

spec/components/app_import_review_issues_summary_component_spec.rb

@@ -3,6 +3,8 @@
 describe AppImportReviewIssuesSummaryComponent do
   subject(:rendered) { render_inline(component) }
 
+  let(:url_helpers) { Rails.application.routes.url_helpers }
+
   let(:component) { described_class.new(import:, records:, review_screen:) }
 
   let(:team) { create(:team) }
@@ -165,9 +167,10 @@
       expect(rendered).to have_link(
         "Review",
         href:
-          Rails.application.routes.url_helpers.imports_issue_path(
+          url_helpers.imports_issue_path(
             patient,
-            type: "patient"
+            type: "patient",
+            return_to: url_helpers.cohort_import_path(import)
           )
       )
     end
@@ -204,9 +207,10 @@
       expect(rendered).to have_link(
         "Review",
         href:
-          Rails.application.routes.url_helpers.imports_issue_path(
+          url_helpers.imports_issue_path(
             vaccination_record,
-            type: "vaccination-record"
+            type: "vaccination-record",
+            return_to: url_helpers.cohort_import_path(import)
           )
       )
     end

spec/features/import_child_records_with_duplicates_spec.rb

@@ -27,17 +27,20 @@
     when_i_choose_to_keep_the_duplicate_record
     and_i_confirm_my_selection
     then_i_should_see_a_success_message
+    and_i_should_return_to_the_import_page
     and_the_first_duplicate_record_should_be_persisted
 
-    when_i_review_the_second_duplicate_record
+    when_i_visit_the_import_issues_page
+    and_i_review_the_second_duplicate_record
     then_i_should_see_the_second_duplicate_record
 
     when_i_choose_to_keep_the_previously_uploaded_record
     and_i_confirm_my_selection
     then_i_should_see_a_success_message
     and_the_second_record_should_not_be_updated
 
-    when_i_review_the_third_duplicate_record
+    when_i_visit_the_import_issues_page
+    and_i_review_the_third_duplicate_record
     then_i_should_see_the_third_duplicate_record
 
     when_i_choose_to_keep_both_records
@@ -75,17 +78,20 @@
       when_i_choose_to_keep_the_duplicate_record
       and_i_confirm_my_selection
       then_i_should_see_a_success_message
+      and_i_should_return_to_the_import_page
       and_the_first_duplicate_record_should_be_persisted
 
-      when_i_review_the_second_duplicate_record
+      when_i_visit_the_import_issues_page
+      and_i_review_the_second_duplicate_record
       then_i_should_see_the_second_duplicate_record
 
       when_i_choose_to_keep_the_previously_uploaded_record
       and_i_confirm_my_selection
       then_i_should_see_a_success_message
       and_the_second_record_should_not_be_updated
 
-      when_i_review_the_third_duplicate_record
+      when_i_visit_the_import_issues_page
+      and_i_review_the_third_duplicate_record
       then_i_should_see_the_third_duplicate_record
 
       when_i_choose_to_keep_both_records
@@ -132,8 +138,10 @@
     and_i_choose_to_keep_the_duplicate_record
     and_i_confirm_my_selection
     then_search_vaccination_records_in_nhs_job_should_be_enqueued
+    and_i_should_return_to_the_import_page
 
-    when_i_review_the_second_duplicate_record_jimmy
+    when_i_visit_the_import_issues_page
+    and_i_review_the_second_duplicate_record_jimmy
     and_i_choose_to_keep_the_previously_uploaded_record
     and_i_confirm_my_selection
     then_search_vaccination_records_in_nhs_job_should_be_enqueued_for_second_patient
@@ -272,6 +280,10 @@ def when_i_visit_the_import_page
     click_link "Import", match: :first
   end
 
+  def when_i_visit_the_import_issues_page
+    visit imports_issues_path
+  end
+
   def and_i_start_adding_children_to_the_cohort
     click_button "Upload records"
     choose "Child records"
@@ -325,6 +337,7 @@ def when_i_choose_to_keep_both_records
   def when_i_choose_to_keep_the_previously_uploaded_record
     choose "Keep existing child"
   end
+
   alias_method :and_i_choose_to_keep_the_previously_uploaded_record,
                :when_i_choose_to_keep_the_previously_uploaded_record
 
@@ -341,6 +354,11 @@ def then_i_should_see_a_success_message
     expect(page).to have_content("Record updated")
   end
 
+  def and_i_should_return_to_the_import_page
+    cohort_import = CohortImport.order(:created_at).last
+    expect(page).to have_current_path(cohort_import_path(cohort_import))
+  end
+
   def when_i_review_the_first_duplicate_record
     find(".nhsuk-details__summary", text: "3 upload issues").click
     click_on "Review CLARKE, Jennifer"
@@ -363,11 +381,11 @@ def then_i_should_see_a_validation_error
     expect(page).to have_content("There is a problem")
   end
 
-  def when_i_review_the_second_duplicate_record
+  def and_i_review_the_second_duplicate_record
     click_on "Review SMITH, James"
   end
 
-  def when_i_review_the_second_duplicate_record_jimmy
+  def and_i_review_the_second_duplicate_record_jimmy
     click_on "Review SMITH, Jimmy"
   end
 
@@ -410,7 +428,7 @@ def and_a_new_patient_record_should_be_created
     expect(session).to eq(@session)
   end
 
-  def when_i_review_the_third_duplicate_record
+  def and_i_review_the_third_duplicate_record
     click_on "Review DOE, Mark"
   end
 

spec/features/import_class_lists_with_duplicates_spec.rb

@@ -26,15 +26,17 @@
     then_i_should_see_a_success_message
     and_the_first_record_should_be_updated
 
-    when_i_review_the_second_duplicate_record
+    when_i_visit_the_import_issues_page
+    and_i_review_the_second_duplicate_record
     then_i_should_see_the_second_duplicate_record
 
     when_i_choose_to_keep_the_existing_record
     and_i_confirm_my_selection
     then_i_should_see_a_success_message
     and_the_second_record_should_not_be_updated
 
-    when_i_review_the_third_duplicate_record
+    when_i_visit_the_import_issues_page
+    and_i_review_the_third_duplicate_record
     then_i_should_see_the_third_duplicate_record
 
     when_i_choose_to_keep_both_records
@@ -69,15 +71,17 @@
       then_i_should_see_a_success_message
       and_the_first_record_should_be_updated
 
-      when_i_review_the_second_duplicate_record
+      when_i_visit_the_import_issues_page
+      and_i_review_the_second_duplicate_record
       then_i_should_see_the_second_duplicate_record
 
       when_i_choose_to_keep_the_existing_record
       and_i_confirm_my_selection
       then_i_should_see_a_success_message
       and_the_second_record_should_not_be_updated
 
-      when_i_review_the_third_duplicate_record
+      when_i_visit_the_import_issues_page
+      and_i_review_the_third_duplicate_record
       then_i_should_see_the_third_duplicate_record
 
       when_i_choose_to_keep_both_records
@@ -191,6 +195,10 @@ def when_i_visit_a_school_page_for_the_hpv_programme
     click_on "Waterloo Road"
   end
 
+  def when_i_visit_the_import_issues_page
+    visit imports_issues_path
+  end
+
   def and_i_start_adding_children_to_the_school
     click_on "Import class lists"
   end
@@ -262,7 +270,7 @@ def and_the_first_record_should_be_updated
     expect(@existing_patient.pending_changes).to eq({})
   end
 
-  def when_i_review_the_second_duplicate_record
+  def and_i_review_the_second_duplicate_record
     click_on "Review JONES, Sarah"
   end
 
@@ -277,7 +285,7 @@ def and_the_second_record_should_not_be_updated
     expect(@second_patient.pending_changes).to eq({})
   end
 
-  def when_i_review_the_third_duplicate_record
+  def and_i_review_the_third_duplicate_record
     click_on "Review SALLES, Jenny"
   end
 

spec/features/import_vaccination_records_with_duplicates_spec.rb

@@ -23,7 +23,8 @@
     then_i_should_see_a_success_message
     and_the_duplicate_record_should_be_persisted
 
-    when_i_review_the_second_duplicate_record
+    when_i_visit_the_import_issues_page
+    and_i_review_the_second_duplicate_record
     then_i_should_see_the_second_duplicate_record
 
     when_i_choose_to_keep_the_previously_uploaded_record
@@ -137,6 +138,10 @@ def and_i_am_signed_in
     visit dashboard_path
   end
 
+  def when_i_visit_the_import_issues_page
+    visit imports_issues_path
+  end
+
   def and_i_click_on_the_upload_link
     click_on "Upload records"
     choose "Vaccination records"
@@ -212,7 +217,7 @@ def then_i_should_see_a_validation_error
     expect(page).to have_content("There is a problem")
   end
 
-  def when_i_review_the_second_duplicate_record
+  def and_i_review_the_second_duplicate_record
     click_on "Review HAMILTON, Joanna"
   end