<%= activity.subtitle %>
+<%= activity.subtitle(current_user) %>
<% if restricted %> <%= render "restricted", activity: activity %> diff --git a/psd-web/app/views/investigations/activities/investigation/_update_owner.html.erb b/psd-web/app/views/investigations/activities/investigation/_update_owner.html.erb new file mode 100644 index 0000000000..4854ec1e62 --- /dev/null +++ b/psd-web/app/views/investigations/activities/investigation/_update_owner.html.erb @@ -0,0 +1 @@ +<%= markdown simple_format(activity.metadata["rationale"]) %> diff --git a/psd-web/config/locales/en.yml b/psd-web/config/locales/en.yml index cc56107760..d7a1fdab12 100644 --- a/psd-web/config/locales/en.yml +++ b/psd-web/config/locales/en.yml @@ -129,6 +129,11 @@ en: activemodel: errors: models: + change_case_owner_form: + attributes: + owner_id: + blank: Select case owner + not_found: User or team not found invite_user_to_team_form: attributes: email: diff --git a/psd-web/db/migrate/20200610111320_move_update_owner_activity_metadata.rb b/psd-web/db/migrate/20200610111320_move_update_owner_activity_metadata.rb new file mode 100644 index 0000000000..b682f63fc8 --- /dev/null +++ b/psd-web/db/migrate/20200610111320_move_update_owner_activity_metadata.rb @@ -0,0 +1,33 @@ +class MoveUpdateOwnerActivityMetadata < ActiveRecord::Migration[5.2] + def change + klass = AuditActivity::Investigation::UpdateOwner + + klass.all.each do |activity| + next if activity.metadata + + # case owner ID is stored in the title column... + next unless (owner_id = activity[:title]) + + owner = User.find_by(id: owner_id) || Team.find_by(id: owner_id) + + metadata = klass.build_metadata(owner, activity[:body]) + + activity.update!(metadata: metadata, title: nil, body: nil) + end + + klass = AuditActivity::Investigation::AutomaticallyUpdateOwner + + klass.all.each do |activity| + next if activity.metadata + + # case owner ID is stored in the title column... + next unless (owner_id = activity[:title]) + + owner = User.find_by(id: owner_id) || Team.find_by(id: owner_id) + + metadata = klass.build_metadata(owner) + + activity.update!(metadata: metadata, title: nil, body: nil) + end + end +end diff --git a/psd-web/db/schema.rb b/psd-web/db/schema.rb index a0aba4aa57..329477e8bf 100644 --- a/psd-web/db/schema.rb +++ b/psd-web/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_06_05_115455) do +ActiveRecord::Schema.define(version: 2020_06_10_111320) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" diff --git a/psd-web/spec/features/change_ownership_for_investigation_spec.rb b/psd-web/spec/features/change_ownership_for_investigation_spec.rb index ab462a42c6..e618108125 100644 --- a/psd-web/spec/features/change_ownership_for_investigation_spec.rb +++ b/psd-web/spec/features/change_ownership_for_investigation_spec.rb @@ -12,7 +12,7 @@ before do sign_in(user) - visit "/cases/#{investigation.pretty_id}/assign/select-owner" + visit "/cases/#{investigation.pretty_id}/assign/new" end scenario "does not show inactive users" do diff --git a/psd-web/spec/features/record_email_activity_spec.rb b/psd-web/spec/features/record_email_activity_spec.rb index 8c5f809b23..06113b06e7 100644 --- a/psd-web/spec/features/record_email_activity_spec.rb +++ b/psd-web/spec/features/record_email_activity_spec.rb @@ -81,7 +81,7 @@ expect_to_be_on_case_activity_page(case_id: investigation.pretty_id) # Consumer info is not hidden from case owner - expect_case_activity_page_to_show_entered_information(name: name, email: email, date: date, file: file) + expect_case_activity_page_to_show_entered_information(user_name: user.name, name: name, email: email, date: date, file: file) click_link "View email" @@ -109,7 +109,7 @@ visit "/cases/#{investigation.pretty_id}/activity" expect_to_be_on_case_activity_page(case_id: investigation.pretty_id) - expect_case_activity_page_to_show_entered_information(name: name, email: email, date: date, file: file) + expect_case_activity_page_to_show_entered_information(user_name: user.name, name: name, email: email, date: date, file: file) end scenario "with non-consumer contact details and summary and subject and body" do @@ -135,7 +135,7 @@ click_on "Activity" expect_to_be_on_case_activity_page(case_id: investigation.pretty_id) - expect_case_activity_page_to_show_entered_information(name: name, email: email, date: date, summary: summary, subject: email_subject, body: body) + expect_case_activity_page_to_show_entered_information(user_name: user.name, name: name, email: email, date: date, summary: summary, subject: email_subject, body: body) # Test that another user in a different organisation can see all info sign_out @@ -145,7 +145,7 @@ visit "/cases/#{investigation.pretty_id}/activity" expect_to_be_on_case_activity_page(case_id: investigation.pretty_id) - expect_case_activity_page_to_show_entered_information(name: name, email: email, date: date, summary: summary, subject: email_subject, body: body) + expect_case_activity_page_to_show_entered_information(user_name: "#{user.name} (#{user.team.name})", name: name, email: email, date: date, summary: summary, subject: email_subject, body: body) end def fill_in_record_email_form(name:, email:, consumer:, date:) @@ -198,8 +198,8 @@ def expect_record_email_form_to_have_entered_information(name:, email:, consumer expect(find_field("Year").value).to eq date.year.to_s end - def expect_case_activity_page_to_show_entered_information(name:, email:, date:, file: nil, summary: nil, subject: nil, body: nil) - item = page.find("p", text: "Email recorded by #{user.name} (#{user.team.name})").find(:xpath, "..") + def expect_case_activity_page_to_show_entered_information(user_name:, name:, email:, date:, file: nil, summary: nil, subject: nil, body: nil) + item = page.find("p", text: "Email recorded by #{user_name}").find(:xpath, "..") expect(item).to have_text("From: #{name} (#{email})") expect(item).to have_text("Date sent: #{date.strftime('%d/%m/%Y')}") diff --git a/psd-web/spec/features/record_phone_call_activity_spec.rb b/psd-web/spec/features/record_phone_call_activity_spec.rb index d7dfc2fb9f..175436d6fa 100644 --- a/psd-web/spec/features/record_phone_call_activity_spec.rb +++ b/psd-web/spec/features/record_phone_call_activity_spec.rb @@ -78,7 +78,7 @@ expect_to_be_on_case_activity_page(case_id: investigation.pretty_id) # Consumer info is not hidden from case owner - expect_case_activity_page_to_show_entered_information(name: name, phone: phone, date: date, file: file) + expect_case_activity_page_to_show_entered_information(user_name: user.name, name: name, phone: phone, date: date, file: file) click_link "View phone call" @@ -106,7 +106,7 @@ visit "/cases/#{investigation.pretty_id}/activity" expect_to_be_on_case_activity_page(case_id: investigation.pretty_id) - expect_case_activity_page_to_show_entered_information(name: name, phone: phone, date: date, file: file) + expect_case_activity_page_to_show_entered_information(user_name: user.name, name: name, phone: phone, date: date, file: file) end scenario "with non-consumer contact details and summary and notes" do @@ -132,7 +132,7 @@ click_on "Activity" expect_to_be_on_case_activity_page(case_id: investigation.pretty_id) - expect_case_activity_page_to_show_entered_information(name: name, phone: phone, date: date, summary: summary, notes: notes) + expect_case_activity_page_to_show_entered_information(user_name: user.name, name: name, phone: phone, date: date, summary: summary, notes: notes) # Test that another user in a different organisation can see all info sign_out @@ -142,7 +142,7 @@ visit "/cases/#{investigation.pretty_id}/activity" expect_to_be_on_case_activity_page(case_id: investigation.pretty_id) - expect_case_activity_page_to_show_entered_information(name: name, phone: phone, date: date, summary: summary, notes: notes) + expect_case_activity_page_to_show_entered_information(user_name: "#{user.name} (#{user.team.name})", name: name, phone: phone, date: date, summary: summary, notes: notes) end def fill_in_record_phone_call_form(name:, phone:, consumer:, date:) @@ -189,8 +189,8 @@ def expect_record_phone_call_form_to_have_entered_information(name:, phone:, con expect(find_field("Year").value).to eq date.year.to_s end - def expect_case_activity_page_to_show_entered_information(name:, phone:, date:, file: nil, summary: nil, notes: nil) - item = page.find("p", text: "Phone call by #{user.name} (#{user.team.name})").find(:xpath, "..") + def expect_case_activity_page_to_show_entered_information(user_name:, name:, phone:, date:, file: nil, summary: nil, notes: nil) + item = page.find("p", text: "Phone call by #{user_name}").find(:xpath, "..") expect(item).to have_text("Call with: #{name} (#{phone})") expect(item).to have_text("Date: #{date.strftime('%d/%m/%Y')}") diff --git a/psd-web/spec/forms/change_case_owner_form_spec.rb b/psd-web/spec/forms/change_case_owner_form_spec.rb new file mode 100644 index 0000000000..01ab20cf11 --- /dev/null +++ b/psd-web/spec/forms/change_case_owner_form_spec.rb @@ -0,0 +1,60 @@ +require "rails_helper" + +RSpec.describe ChangeCaseOwnerForm do + subject(:form) { described_class.new(owner_id: owner_id, owner_rationale: rationale) } + + let(:owner) { create(:team) } + + let(:owner_id) { owner.id } + let(:rationale) { "Test rationale" } + + describe "#valid?" do + shared_examples_for "valid form" do + it "is is valid" do + expect(form).to be_valid + end + + it "does not contain error messages" do + form.validate + expect(form.errors.full_messages).to be_empty + end + end + + shared_examples_for "invalid form" do |*errors| + it "is not valid" do + expect(form).to be_invalid + end + + it "populates an error message" do + form.validate + errors.each do |property, message| + expect(form.errors.full_messages_for(property)).to eq([message]) + end + end + end + + context "when no owner_id is supplied" do + let(:owner_id) { nil } + + include_examples "invalid form", [:owner_id, "Select case owner"] + end + + context "when owner_id does not match a user or team" do + let(:owner_id) { "invalid" } + + include_examples "invalid form", [:owner_id, "User or team not found"] + end + + context "when owner is an inactive user" do + let(:owner) { create(:user, :inactive) } + + include_examples "invalid form", [:owner_id, "User or team not found"] + end + end + + describe "#owner" do + it "returns the owner" do + expect(form.owner).to eq(owner) + end + end +end diff --git a/psd-web/spec/forms/edit_investigation_collaborator_form_spec.rb b/psd-web/spec/forms/edit_investigation_collaborator_form_spec.rb index 8e5e1bfb67..ca0df213f2 100644 --- a/psd-web/spec/forms/edit_investigation_collaborator_form_spec.rb +++ b/psd-web/spec/forms/edit_investigation_collaborator_form_spec.rb @@ -72,7 +72,7 @@ last_added_activity = investigation.activities.reload.order(created_at: :desc) .find_by!(type: "AuditActivity::Investigation::TeamDeleted") - expect(last_added_activity.title).to eql("test team removed from allegation") + expect(last_added_activity.title(user)).to eql("test team removed from allegation") expect(last_added_activity.source.user_id).to eql(user.id) end end diff --git a/psd-web/spec/models/audit_activity/investigation/automatically_update_owner_spec.rb b/psd-web/spec/models/audit_activity/investigation/automatically_update_owner_spec.rb deleted file mode 100644 index 67614dd4f4..0000000000 --- a/psd-web/spec/models/audit_activity/investigation/automatically_update_owner_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require "rails_helper" - -RSpec.describe AuditActivity::Investigation::AutomaticallyUpdateOwner, :with_stubbed_mailer, :with_stubbed_elasticsearch do - subject(:audit_activity) { described_class.from(investigation) } - - let(:team) { create(:team) } - let(:investigation) { create(:enquiry, owner: team, owner_rationale: "Test owner") } - - describe ".from" do - it "creates an audit activity without information for email notification" do - expect(audit_activity).to have_attributes( - title: "Case owner automatically changed on Enquiry to #{team.display_name}", - body: nil, - email_update_text: nil, - email_subject_text: nil - ) - end - end -end diff --git a/psd-web/spec/models/audit_activity/investigation/update_coronavirus_status_spec.rb b/psd-web/spec/models/audit_activity/investigation/update_coronavirus_status_spec.rb index f8e52d215b..f4a729f816 100644 --- a/psd-web/spec/models/audit_activity/investigation/update_coronavirus_status_spec.rb +++ b/psd-web/spec/models/audit_activity/investigation/update_coronavirus_status_spec.rb @@ -11,10 +11,10 @@ describe ".from" do it "creates an audit activity", :aggregate_failures do expect(audit_activity).to have_attributes( - title: "Status updated: not coronavirus related", body: "The case is not related to the coronavirus outbreak.", email_subject_text: "Coronavirus status updated on #{investigation.case_type.downcase}" ) + expect(audit_activity.title(user)).to eq("Status updated: not coronavirus related") expect(audit_activity.email_update_text(user)).to eq("#{investigation.case_type.capitalize} #{investigation.pretty_id} is not related to the coronavirus outbreak. This status was updated by #{user.name}.") end end diff --git a/psd-web/spec/models/audit_activity/investigation/update_owner_spec.rb b/psd-web/spec/models/audit_activity/investigation/update_owner_spec.rb index 52febdf944..a95ee17c26 100644 --- a/psd-web/spec/models/audit_activity/investigation/update_owner_spec.rb +++ b/psd-web/spec/models/audit_activity/investigation/update_owner_spec.rb @@ -1,21 +1,39 @@ require "rails_helper" -RSpec.describe AuditActivity::Investigation::UpdateOwner, :with_stubbed_mailer, :with_stubbed_elasticsearch do - subject(:audit_activity) { described_class.from(investigation) } +RSpec.describe AuditActivity::Investigation::UpdateOwner, :with_stubbed_elasticsearch, :with_stubbed_mailer do + subject(:activity) { described_class.create(investigation: investigation, metadata: described_class.build_metadata(owner, rationale)) } - let(:user) { create(:user).decorate } - let(:investigation) { create(:enquiry, owner: user, owner_rationale: "Test owner") } + let(:investigation) { create(:allegation, owner: owner) } + let(:owner) { create(:team) } + let(:rationale) { "Test rationale" } + let(:user) { create(:user) } - before { User.current = user } + describe ".build_metadata" do + let(:result) { described_class.build_metadata(owner, rationale) } - describe ".from" do - it "creates an audit activity with information for email notification", :aggregate_failures do - expect(audit_activity).to have_attributes( - title: "Case owner changed to #{user.display_name}", - body: investigation.owner_rationale, - email_subject_text: "Case owner changed for enquiry" - ) - expect(audit_activity.email_update_text(user)).to start_with("Case owner changed on enquiry to #{user.name} by #{user.name}") + it "returns a Hash of the arguments" do + expect(result).to eq({ + owner_id: owner.id, + rationale: rationale + }) + end + end + + describe "#title" do + it "returns a generated String" do + expect(activity.title(user)).to eq("Case owner changed to #{owner.name}") + end + end + + describe "#body" do + it "returns the rationale" do + expect(activity.body).to eq("Test rationale") + end + end + + describe "#owner" do + it "returns the owner" do + expect(activity.owner).to eq(owner) end end end diff --git a/psd-web/spec/services/add_product_to_case_spec.rb b/psd-web/spec/services/add_product_to_case_spec.rb index 2b067b83ea..7297fe7a57 100644 --- a/psd-web/spec/services/add_product_to_case_spec.rb +++ b/psd-web/spec/services/add_product_to_case_spec.rb @@ -59,7 +59,7 @@ expect(activity).to be_a(AuditActivity::Product::Add) expect(activity.source.user).to eq(user) expect(activity.product_id).to eq(product.id) - expect(activity.title).to eq(product.name) + expect(activity.title(nil)).to eq(product.name) end context "when the case owner is a user" do diff --git a/psd-web/spec/services/add_team_to_an_investigation_spec.rb b/psd-web/spec/services/add_team_to_an_investigation_spec.rb index 4213bc7d9e..cbce7adbb8 100644 --- a/psd-web/spec/services/add_team_to_an_investigation_spec.rb +++ b/psd-web/spec/services/add_team_to_an_investigation_spec.rb @@ -51,7 +51,7 @@ aggregate_failures do expect(last_added_activity).to be_a(AuditActivity::Investigation::TeamAdded) - expect(last_added_activity.title).to eql("Testing team added to allegation") + expect(last_added_activity.title(nil)).to eql("Testing team added to allegation") expect(last_added_activity.source.user_id).to eql(user.id) end end diff --git a/psd-web/spec/services/change_case_owner_spec.rb b/psd-web/spec/services/change_case_owner_spec.rb new file mode 100644 index 0000000000..06372d49f8 --- /dev/null +++ b/psd-web/spec/services/change_case_owner_spec.rb @@ -0,0 +1,165 @@ +require "rails_helper" + +RSpec.describe ChangeCaseOwner, :with_stubbed_elasticsearch, :with_test_queue_adapter do + # Create the investigation up front so we can check which activities are generated by the service + let!(:investigation) { create(:enquiry, owner: old_owner, creator: creator) } + + let(:team) { create(:team) } + + let(:creator) { create(:user, :activated, team: team, organisation: team.organisation) } + let(:old_owner) { creator } + let(:new_owner) { create(:user, :activated, team: team, organisation: team.organisation) } + let(:user) { create(:user, :activated, team: team, organisation: team.organisation) } + let(:rationale) { "Test rationale" } + + describe ".call" do + context "with no parameters" do + let(:result) { described_class.call } + + it "returns a failure" do + expect(result).to be_failure + end + end + + context "with no investigation parameter" do + let(:result) { described_class.call(owner: new_owner, user: user) } + + it "returns a failure" do + expect(result).to be_failure + end + end + + context "with no user parameter" do + let(:result) { described_class.call(owner: new_owner, investigation: investigation) } + + it "returns a failure" do + expect(result).to be_failure + end + end + + context "with no owner parameter" do + let(:result) { described_class.call(investigation: investigation, user: user) } + + it "returns a failure" do + expect(result).to be_failure + end + end + + context "with required parameters" do + let(:result) { described_class.call(investigation: investigation, user: user, owner: new_owner, rationale: rationale) } + + it "returns success" do + expect(result).to be_success + end + + context "when the new owner is the same as the old owner" do + let(:new_owner) { old_owner } + + it "does not create an audit activity" do + expect { result }.not_to change(Activity, :count) + end + + it "does not send an email" do + expect { result }.not_to have_enqueued_mail(NotifyMailer, :investigation_updated) + end + end + + it "changes the owner" do + expect { result }.to change(investigation, :owner).from(old_owner).to(new_owner) + end + + it "creates an audit activity for owner changed", :aggregate_failures do + expect { result }.to change(Activity, :count).by(1) + activity = investigation.reload.activities.first + expect(activity).to be_a(AuditActivity::Investigation::UpdateOwner) + expect(activity.source.user).to eq(user) + expect(activity.metadata).to eq(AuditActivity::Investigation::UpdateOwner.build_metadata(new_owner, rationale).deep_stringify_keys) + end + + it "sends a notification email to the new owner" do + expect { result }.to have_enqueued_mail(NotifyMailer, :investigation_updated).with( + investigation.pretty_id, + new_owner.name, + new_owner.email, + expected_email_body, + expected_email_subject + ) + end + + it "sends a notification email to the old owner" do + expect { result }.to have_enqueued_mail(NotifyMailer, :investigation_updated).with( + investigation.pretty_id, + old_owner.name, + old_owner.email, + expected_email_body, + expected_email_subject + ) + end + + context "when no rationale is supplied" do + let(:rationale) { nil } + + it "does not add a message to the notification email" do + expect { result }.to have_enqueued_mail(NotifyMailer, :investigation_updated).with( + investigation.pretty_id, + old_owner.name, + old_owner.email, + "Case owner changed on enquiry to #{new_owner.name} by #{user.name}.", + expected_email_subject + ) + end + end + + context "when the user is the same as the old owner" do + let(:user) { old_owner } + + it "does not send a notification email to the old owner" do + expect { result }.not_to have_enqueued_mail(NotifyMailer, :investigation_updated).with( + investigation.pretty_id, + old_owner.name, + old_owner.email, + expected_email_body, + expected_email_subject + ) + end + end + + context "when the new owner is a Team" do + let(:new_owner) { team } + + context "when the team has a an email address" do + let(:team) { create(:team, team_recipient_email: Faker::Internet.email) } + + it "sends a notification email to the team" do + expect { result }.to have_enqueued_mail(NotifyMailer, :investigation_updated).with( + investigation.pretty_id, + team.name, + team.team_recipient_email, + expected_email_body, + expected_email_subject + ) + end + end + + context "when the team does not have an email address" do + let(:team) { create(:team, team_recipient_email: nil) } + + # Create an inactive user to test email is not delivered to them + before { create(:user, team: team, organisation: team.organisation) } + + it "sends an email to each of the team's active users" do + expect { result }.to have_enqueued_mail(NotifyMailer, :investigation_updated).twice + end + end + end + end + end + + def expected_email_subject + "Case owner changed for enquiry" + end + + def expected_email_body + "Case owner changed on enquiry to #{new_owner.name} by #{user.name}.\n\nMessage from #{user.name}: ^ Test rationale" + end +end diff --git a/psd-web/spec/services/create_case_spec.rb b/psd-web/spec/services/create_case_spec.rb index 8f80ae8d65..ac34727fe9 100644 --- a/psd-web/spec/services/create_case_spec.rb +++ b/psd-web/spec/services/create_case_spec.rb @@ -77,7 +77,7 @@ expect(activity).to be_a(AuditActivity::Product::Add) expect(activity.source.user).to eq(user) expect(activity.product_id).to eq(product.id) - expect(activity.title).to eq(product.name) + expect(activity.title(user)).to eq(product.name) end end diff --git a/psd-web/spec/services/delete_user_spec.rb b/psd-web/spec/services/delete_user_spec.rb index fe63390a2c..60848142d2 100644 --- a/psd-web/spec/services/delete_user_spec.rb +++ b/psd-web/spec/services/delete_user_spec.rb @@ -67,8 +67,8 @@ expect { delete_call }.to change(update_owner_activities, :count).by(3) update_owner_activities.last(3).each do |activity| - expect(activity.title).to start_with "Case owner automatically changed on" - expect(activity.title).to end_with "to #{user.team.display_name}" + expect(activity.title(user)).to start_with "Case owner automatically changed on" + expect(activity.title(user)).to end_with "to #{user.team.name}" end end # rubocop:enable RSpec/ExampleLength diff --git a/psd-web/spec/services/remove_product_from_case_spec.rb b/psd-web/spec/services/remove_product_from_case_spec.rb index eeabbbf297..ed9bed5ad7 100644 --- a/psd-web/spec/services/remove_product_from_case_spec.rb +++ b/psd-web/spec/services/remove_product_from_case_spec.rb @@ -59,7 +59,7 @@ expect(activity).to be_a(AuditActivity::Product::Destroy) expect(activity.source.user).to eq(user) expect(activity.product_id).to eq(product.id) - expect(activity.title).to eq("Removed: #{product.name}") + expect(activity.title(nil)).to eq("Removed: #{product.name}") end context "when the case owner is a user" do diff --git a/psd-web/spec/support/audit_activity_investigation.rb b/psd-web/spec/support/audit_activity_investigation.rb index 35b69791c3..2c4c96b0e0 100644 --- a/psd-web/spec/support/audit_activity_investigation.rb +++ b/psd-web/spec/support/audit_activity_investigation.rb @@ -95,7 +95,7 @@ end describe "#title" do - subject(:title) { activity.title } + subject(:title) { activity.title(nil) } let(:activity) { described_class.create(investigation: investigation, metadata: metadata, title: test_title) } let(:test_title) { nil } @@ -170,14 +170,14 @@ end describe "#restricted_title" do - # This metod will only ever be called for older records with no metadata, + # This method will only ever be called for older records with no metadata, # where the title is pre-generated and stored in the database, so we will # set the title here subject(:activity) { described_class.create(investigation: investigation, title: "Test title") } # titles never contain GDPR data for these activity classes so just return the title it "returns the title" do - expect(activity.restricted_title).to eq("Test title") + expect(activity.restricted_title(nil)).to eq("Test title") end end end diff --git a/psd-web/test/models/complainant_test.rb b/psd-web/test/models/complainant_test.rb index 351d162c10..3ae6264415 100644 --- a/psd-web/test/models/complainant_test.rb +++ b/psd-web/test/models/complainant_test.rb @@ -2,6 +2,7 @@ class ComplainantTest < ActiveSupport::TestCase setup do + stub_notify_mailer @complainant = Complainant.new(complainant_type: "Business") end diff --git a/psd-web/test/models/investigation_test.rb b/psd-web/test/models/investigation_test.rb index 4d0785e15c..08d147457a 100644 --- a/psd-web/test/models/investigation_test.rb +++ b/psd-web/test/models/investigation_test.rb @@ -56,17 +56,11 @@ def pundit_user end test "past owners should be computed" do - user = users(:southampton) - @investigation.update(owner: user) + user = users(:southampton_bob) + ChangeCaseOwner.call!(investigation: @investigation, owner: user, user: user) assert_includes @investigation.past_owners, user end - test "past owner teams should be computed" do - team = Team.first - @investigation.update(owner: team) - assert_includes @investigation.past_teams, team - end - test "people out of current owner's team should not be able to change the case owner" do investigation = create_new_case(users(:southampton)) assert_not policy(investigation).change_owner_or_status?(user: users(:luton)) diff --git a/psd-web/test/models/notification_test.rb b/psd-web/test/models/notification_test.rb index 6ea4e996fb..0daeb02cfe 100644 --- a/psd-web/test/models/notification_test.rb +++ b/psd-web/test/models/notification_test.rb @@ -64,63 +64,6 @@ class NotificationTest < ActiveSupport::TestCase assert_equal 2, @number_of_notifications end - test "should notify previous owner if case is owned by someone else by someone else" do - @investigation.update(owner: teams(:southampton)) - @investigation.update(owner: users(:southampton_bob)) - mock_investigation_updated(who_will_be_notified: [users(:southampton_bob), users(:southampton)].map(&:email)) - @investigation.update(owner: users(:southampton)) - assert_equal @number_of_notifications, 2 - end - - test "should not notify previous owner if case is owned by someone else by them" do - @investigation.update(owner: users(:southampton)) - mock_investigation_updated(who_will_be_notified: [users(:southampton_bob).email]) - @investigation.update(owner: users(:southampton_bob)) - assert_equal @number_of_notifications, 1 - end - - test "should notify previous owner team if case is owned by someone by someone outside" do - @investigation.update!(owner: users(:southampton)) - - team_with_recipient_email = teams(:luton) - - @investigation.update!(owner: team_with_recipient_email) - - expected_recipients = [users(:southampton_bob).email, users(:southampton).email, team_with_recipient_email.team_recipient_email] - - mock_investigation_updated(who_will_be_notified: expected_recipients) - @investigation.update!(owner: users(:southampton)) - assert_equal @number_of_notifications, 3 - end - - test "should not notify previous owner team if case is owned by someone by someone inside" do - @investigation.update(owner: users(:southampton_bob)) - @investigation.update(owner: teams(:southampton)) - mock_investigation_updated(who_will_be_notified: [users(:southampton_bob).email]) - @investigation.update(owner: users(:southampton_bob)) - assert_equal @number_of_notifications, 1 - end - - test "should notify a person who is the case owner" do - mock_investigation_updated(who_will_be_notified: [users(:southampton_bob).email]) - @investigation.update(owner: users(:southampton_bob)) - assert_equal @number_of_notifications, 1 - end - - test "should notify everyone in team that is the case owner" do - mock_investigation_updated(who_will_be_notified: teams(:opss_enforcement).users.map(&:email)) - @investigation.update(owner: teams(:opss_enforcement)) - assert_equal @number_of_notifications, 2 - end - - test "previous owner is computed correctly" do - @investigation.update(owner: users(:southampton)) - @investigation.update(owner: users(:southampton_steve)) - mock_investigation_updated(who_will_be_notified: [users(:southampton_bob), users(:southampton_steve)].map(&:email)) - @investigation.update(owner: users(:southampton_bob)) - assert_equal @number_of_notifications, 2 - end - test "notifies current user when investigation created" do mock_investigation_created(who_will_be_notified: [users(:southampton)]) @investigation_two = investigations :two