From 87202f2a066b37aff7373d32278b396f31fd16ea Mon Sep 17 00:00:00 2001 From: Edwin Kruglov Date: Mon, 10 Jul 2023 14:12:28 +0100 Subject: [PATCH] feat(cms): always allow mail merge Jira: PWNN-1536 --- .../support/cases/merge_emails_controller.rb | 3 - app/models/support/case.rb | 2 + app/services/support/merge_case_emails.rb | 4 - .../support/cases/show/_case_actions.html.erb | 6 +- .../support/case_merge_emails_spec.rb | 164 ++++++++---------- .../support/merge_case_emails_spec.rb | 8 - 6 files changed, 79 insertions(+), 108 deletions(-) diff --git a/app/controllers/support/cases/merge_emails_controller.rb b/app/controllers/support/cases/merge_emails_controller.rb index 30d870429..1df9f4079 100644 --- a/app/controllers/support/cases/merge_emails_controller.rb +++ b/app/controllers/support/cases/merge_emails_controller.rb @@ -33,9 +33,6 @@ def create end render :new - rescue MergeCaseEmails::CaseNotNewError - clear_session - redirect_to support_case_path(@current_case), notice: I18n.t("support.case_merge_emails.flash.case_not_new") end def show; end diff --git a/app/models/support/case.rb b/app/models/support/case.rb index c4d28af55..a62cbcb3e 100644 --- a/app/models/support/case.rb +++ b/app/models/support/case.rb @@ -34,6 +34,8 @@ class Case < ApplicationRecord event :close do transitions from: :initial, to: :closed + transitions from: :opened, to: :closed + transitions from: :on_hold, to: :closed transitions from: :resolved, to: :closed end end diff --git a/app/services/support/merge_case_emails.rb b/app/services/support/merge_case_emails.rb index d8acc640c..c13717ba8 100644 --- a/app/services/support/merge_case_emails.rb +++ b/app/services/support/merge_case_emails.rb @@ -12,8 +12,6 @@ module Support # original case (e.g. original_case_id). # class MergeCaseEmails - class CaseNotNewError < StandardError; end - extend Dry::Initializer # @!attribute from_case @@ -38,8 +36,6 @@ def call def from! from_case.transaction do - raise CaseNotNewError unless from_case.initial? - # move emails and interactions over to the to_case from_case.interactions&.update_all(case_id: to_case.id) from_case.emails&.update_all(case_id: to_case.id) diff --git a/app/views/support/cases/show/_case_actions.html.erb b/app/views/support/cases/show/_case_actions.html.erb index d65c2316c..8c9063917 100644 --- a/app/views/support/cases/show/_case_actions.html.erb +++ b/app/views/support/cases/show/_case_actions.html.erb @@ -4,6 +4,9 @@
  • <%= link_to I18n.t("support.case.label.add_note"), new_support_case_interaction_path(@current_case, option: :note), class: "govuk-link" %>
    • +
  • + <%= link_to I18n.t("support.case.label.move_email"), new_support_case_merge_emails_path(@current_case), class: "govuk-link" %> +
    • <% if @current_case.opened? %>
  • <%= link_to I18n.t("support.case.label.change_owner"), new_support_case_assignment_path(@current_case), class: "govuk-link" %> @@ -42,9 +45,6 @@
  • <%= link_to I18n.t("support.case.label.add_interaction"), new_support_case_interaction_path(@current_case, option: :contact), class: "govuk-link" %>
    • -
  • - <%= link_to I18n.t("support.case.label.move_email"), new_support_case_merge_emails_path(@current_case), class: "govuk-link" %> -
    • <% end %> <% if @current_case.initial? && @current_case.incoming_email? %> diff --git a/spec/features/support/case_merge_emails_spec.rb b/spec/features/support/case_merge_emails_spec.rb index ed44aeb99..548b48c78 100644 --- a/spec/features/support/case_merge_emails_spec.rb +++ b/spec/features/support/case_merge_emails_spec.rb @@ -1,6 +1,8 @@ -RSpec.feature "Merge a New Cases email(s) into an Existing Case" do +RSpec.feature "Merge a New Cases email(s) into an Existing Case", js: true do before do agent_is_signed_in + visit "/support/cases/#{from_case.id}" + click_link "Move emails to existing case" end let(:agent) { Support::Agent.first } @@ -8,108 +10,90 @@ let!(:from_case) { Support::CasePresenter.new(create(:support_case, ref: "000002", agent:)) } let(:interaction_to) { to_case.interactions.last } - context "when the case to be merged is not new" do - before do - from_case.on_hold! - visit "/support/cases/#{from_case.id}" - end + it "errors if no case is chosen" do + click_continue + expect(find(".govuk-error-summary")).to have_text "You must choose a valid case" + end - it "does not show the link for meging it" do - expect(page).not_to have_link "Move emails to existing case" - end + it "errors if an invalid case ref is given" do + fill_in "case-autocomplete", with: "a nonsense id" + find("#case-autocomplete").send_keys :escape + click_continue + expect(find(".govuk-error-summary")).to have_text "You must choose a valid case" end - context "when the case is new", js: true do - before do - visit "/support/cases/#{from_case.id}" - click_link "Move emails to existing case" - end + it "errors if the same case is given for merging" do + fill_in "case-autocomplete", with: from_case.ref + find("#case-autocomplete").send_keys :escape + click_continue + expect(find(".govuk-error-summary")).to have_text "You cannot merge into the same case" + end - it "errors if no case is chosen" do - click_continue - expect(find(".govuk-error-summary")).to have_text "You must choose a valid case" + it "lets the user merge the new case into another" do + ## SEARCH (step 1) + fill_in "case-autocomplete", with: to_case.ref + find("#case-autocomplete").send_keys :down + find("#case-autocomplete").send_keys :enter + + ## PREVIEW (step 2) + expect(find("h1")).to have_text "You are moving emails" + # FROM table + expect(all("#merge_emails_from td")[0]).to have_text from_case.ref + expect(all("#merge_emails_from td")[1]).to have_text from_case.organisation_name + expect(all("#merge_emails_from td")[2]).to have_text from_case.category&.title + expect(all("#merge_emails_from td")[3]).to have_text from_case.state + expect(all("#merge_emails_from td")[4]).to have_text from_case.agent&.full_name + expect(all("#merge_emails_from td")[5]).to have_text from_case.created_at + # TO table + expect(all("#merge_emails_to td")[0]).to have_text to_case.ref + expect(all("#merge_emails_to td")[1]).to have_text to_case.organisation_name + expect(all("#merge_emails_to td")[2]).to have_text to_case.category&.title + expect(all("#merge_emails_to td")[3]).to have_text to_case.state + expect(all("#merge_emails_to td")[4]).to have_text to_case.agent&.full_name + expect(all("#merge_emails_to td")[5]).to have_text to_case.created_at + + expect(find(".govuk-body-m")).to have_text "Once the emails have been moved, Case #{from_case.ref} will be closed." + + click_continue + + ## SUCCESS (step 3) + within ".govuk-panel--confirmation" do |_p| + expect(page).to have_text "Success" + expect(page).to have_text "The emails have been moved to:" + expect(page).to have_text "Case #{to_case.ref} - #{to_case.organisation_name}" + expect(page).to have_text "Assigned to #{to_case.agent_name}" end - it "errors if an invalid case ref is given" do - fill_in "case-autocomplete", with: "a nonsense id" - find("#case-autocomplete").send_keys :escape - click_continue - expect(find(".govuk-error-summary")).to have_text "You must choose a valid case" - end + expect(find(".govuk-body")).to have_text "Case #{from_case.ref} has been closed." + expect(find(".govuk-heading-m")).to have_text "Actions" + expect(all(".govuk-list.govuk-list--bullet li")[0]).to have_link("Go to Case #{to_case.ref}", href: "/support/cases/#{to_case.id}") + expect(all(".govuk-list.govuk-list--bullet li")[1]).to have_link("Notifications", href: "/support/emails") + expect(all(".govuk-list.govuk-list--bullet li")[2]).to have_link("My Cases", href: "/support/cases") + end - it "errors if the same case is given for merging" do - fill_in "case-autocomplete", with: from_case.ref - find("#case-autocomplete").send_keys :escape - click_continue - expect(find(".govuk-error-summary")).to have_text "You cannot merge into the same case" + context "when successfully merged", bullet: :skip do + before do + ::Support::MergeCaseEmails.new( + from_case: from_case.__getobj__, + to_case: to_case.__getobj__, + agent: ::Support::AgentPresenter.new(agent), + ).call end - it "lets the user merge the new case into another" do - ## SEARCH (step 1) - fill_in "case-autocomplete", with: to_case.ref - find("#case-autocomplete").send_keys :down - find("#case-autocomplete").send_keys :enter - - ## PREVIEW (step 2) - expect(find("h1")).to have_text "You are moving emails" - # FROM table - expect(all("#merge_emails_from td")[0]).to have_text from_case.ref - expect(all("#merge_emails_from td")[1]).to have_text from_case.organisation_name - expect(all("#merge_emails_from td")[2]).to have_text from_case.category&.title - expect(all("#merge_emails_from td")[3]).to have_text from_case.state - expect(all("#merge_emails_from td")[4]).to have_text from_case.agent&.full_name - expect(all("#merge_emails_from td")[5]).to have_text from_case.created_at - # TO table - expect(all("#merge_emails_to td")[0]).to have_text to_case.ref - expect(all("#merge_emails_to td")[1]).to have_text to_case.organisation_name - expect(all("#merge_emails_to td")[2]).to have_text to_case.category&.title - expect(all("#merge_emails_to td")[3]).to have_text to_case.state - expect(all("#merge_emails_to td")[4]).to have_text to_case.agent&.full_name - expect(all("#merge_emails_to td")[5]).to have_text to_case.created_at - - expect(find(".govuk-body-m")).to have_text "Once the emails have been moved, Case #{from_case.ref} will be closed." - - click_continue + it "records the interaction against the from case" do + visit "/support/cases/#{from_case.id}#case-history" + expect(page).to have_text "Status change" + expect(page).to have_text "From new to closed by first_name last_name on #{Time.zone.now.to_formatted_s(:short)}. Email(s) moved to case ##{to_case.ref}" - ## SUCCESS (step 3) - within ".govuk-panel--confirmation" do |_p| - expect(page).to have_text "Success" - expect(page).to have_text "The emails have been moved to:" - expect(page).to have_text "Case #{to_case.ref} - #{to_case.organisation_name}" - expect(page).to have_text "Assigned to #{to_case.agent_name}" - end - - expect(find(".govuk-body")).to have_text "Case #{from_case.ref} has been closed." - expect(find(".govuk-heading-m")).to have_text "Actions" - expect(all(".govuk-list.govuk-list--bullet li")[0]).to have_link("Go to Case #{to_case.ref}", href: "/support/cases/#{to_case.id}") - expect(all(".govuk-list.govuk-list--bullet li")[1]).to have_link("Notifications", href: "/support/emails") - expect(all(".govuk-list.govuk-list--bullet li")[2]).to have_link("My Cases", href: "/support/cases") + expect(page).to have_text "Email merge" + expect(page).to have_text "to ##{to_case.ref}" end - context "when successfully merged", bullet: :skip do - before do - ::Support::MergeCaseEmails.new( - from_case: from_case.__getobj__, - to_case: to_case.__getobj__, - agent: ::Support::AgentPresenter.new(agent), - ).call - end - - it "records the interaction against the from case" do - visit "/support/cases/#{from_case.id}#case-history" - expect(page).to have_text "Status change" - expect(page).to have_text "From new to closed by first_name last_name on #{Time.zone.now.to_formatted_s(:short)}. Email(s) moved to case ##{to_case.ref}" - - expect(page).to have_text "Email merge" - expect(page).to have_text "to ##{to_case.ref}" - end - - it "records the interaction against the to case" do - visit "/support/cases/#{to_case.id}#case-history" + it "records the interaction against the to case" do + visit "/support/cases/#{to_case.id}#case-history" - expect(page).to have_text "Email merge" - expect(page).to have_text "from ##{from_case.ref}" - end + expect(page).to have_text "Email merge" + expect(page).to have_text "from ##{from_case.ref}" end end end diff --git a/spec/services/support/merge_case_emails_spec.rb b/spec/services/support/merge_case_emails_spec.rb index adeb50e4f..ee25c9191 100644 --- a/spec/services/support/merge_case_emails_spec.rb +++ b/spec/services/support/merge_case_emails_spec.rb @@ -49,12 +49,4 @@ expect(to_case.interactions.count).to be 3 end end - - context "when the from_case is not 'new'" do - before { from_case.on_hold! } - - it "raises Support::CaseNotNew error" do - expect { merge.call }.to raise_error(Support::MergeCaseEmails::CaseNotNewError) - end - end end