<%= render "form_components/govuk_textarea",
diff --git a/psd-web/app/views/investigations/ownership/select-owner.html.slim b/psd-web/app/views/investigations/ownership/select-owner.html.slim
index 9f15681931..5698ac038e 100644
--- a/psd-web/app/views/investigations/ownership/select-owner.html.slim
+++ b/psd-web/app/views/investigations/ownership/select-owner.html.slim
@@ -14,7 +14,7 @@
- if @investigation.owner.present?
p
' Current case owner:
- strong = @investigation.owner.display_name
+ strong = @investigation.owner.decorate.display_name(viewer: current_user)
- else
p This case has no owner
diff --git a/psd-web/app/views/investigations/show.pdf.slim b/psd-web/app/views/investigations/show.pdf.slim
index dc19f0951a..dd92f6d9b2 100644
--- a/psd-web/app/views/investigations/show.pdf.slim
+++ b/psd-web/app/views/investigations/show.pdf.slim
@@ -20,12 +20,12 @@ table[style="width:100%; text-align:left"]
th
| Case owner
td
- = @investigation.owner.nil? ? "No case owner" : @investigation.owner.display_name
+ = @investigation.owner.nil? ? "No case owner" : @investigation.owner.display_name(viewer: current_user)
tr
th
| Source
td
- = @investigation.source.show
+ = @investigation.source.show(current_user)
- if @investigation.products.any?
h3
| Products
@@ -52,6 +52,6 @@ table[style="width:100%; text-align:left"]
td
= activity.type.upcase_first
td
- = activity.source.show
+ = activity.source.show(current_user)
td
= activity.body
diff --git a/psd-web/app/views/invitations/new.html.erb b/psd-web/app/views/invitations/new.html.erb
index 0e0b63a193..edeaba396d 100644
--- a/psd-web/app/views/invitations/new.html.erb
+++ b/psd-web/app/views/invitations/new.html.erb
@@ -1,4 +1,4 @@
-<%= page_title "Invite a team member to #{@team.decorate.display_name}", errors: @invite_user_to_team_form.errors.any? %>
+<%= page_title "Invite a team member to #{@team.decorate.display_name(viewer: current_user)}", errors: @invite_user_to_team_form.errors.any? %>
<% content_for :after_header do %>
<%= govukBackLink(text: "Back", href: @team) %>
diff --git a/psd-web/app/views/teams/show.html.erb b/psd-web/app/views/teams/show.html.erb
index 370935ace9..3b33755fc8 100644
--- a/psd-web/app/views/teams/show.html.erb
+++ b/psd-web/app/views/teams/show.html.erb
@@ -1,8 +1,8 @@
-<%= page_title(@team.display_name) %>
+<%= page_title(@team.display_name(viewer: current_user)) %>
-
<%= @team.display_name %>
+ <%= @team.display_name(viewer: current_user) %>
<%= link_to "Invite a team member", new_team_invitation_path(@team), class: "govuk-button", role: "button" if current_user.is_team_admin? %>
diff --git a/psd-web/config/locales/en.yml b/psd-web/config/locales/en.yml
index 4caf07e424..fd1a99f0c7 100644
--- a/psd-web/config/locales/en.yml
+++ b/psd-web/config/locales/en.yml
@@ -133,6 +133,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/db/seeds.rb b/psd-web/db/seeds.rb
index 887c06ac41..f7900a5c13 100644
--- a/psd-web/db/seeds.rb
+++ b/psd-web/db/seeds.rb
@@ -435,7 +435,7 @@ def create_blob(filename, title: nil, description: nil)
AddProductToCase.call(product: product, investigation: investigation, user: user)
- if Rails.env.production? && (organisations = CF::App::Credentials.find_by_service_tag("psd-seeds").try(:[], "organisations")) # rubocop:disable Rails/DynamicFindBy
+ if Rails.env.production? && (organisations = CF::App::Credentials.find_by_service_tag("psd-seeds").try(:[], "organisations"))
# The structure is as follows:
# If you want to inspect the current structure on you review app you can inspect the review app env:
# $ cf7 env REVIEW_APP_NAME
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/create_allegation_as_opss_user_spec.rb b/psd-web/spec/features/create_allegation_as_opss_user_spec.rb
index eb7b5dc178..dd773f0452 100644
--- a/psd-web/spec/features/create_allegation_as_opss_user_spec.rb
+++ b/psd-web/spec/features/create_allegation_as_opss_user_spec.rb
@@ -191,8 +191,7 @@ def expect_case_activity_page_to_show_restricted_information(allegation)
expect(page).to have_text("Hazard type: #{allegation.fetch(:hazard_type)}")
expect(page).to have_text(allegation.fetch(:description))
- expect(page).to have_text("Restricted access")
- expect(page).to have_text("Consumer contact details hidden to comply with GDPR legislation. Contact test organisation, who created this activity, to obtain these details if required.")
+ expect(page).to have_css("p", text: "Only teams added to the case can view allegation contact details")
expect(page).not_to have_text("Name")
expect(page).not_to have_text("Email address")
diff --git a/psd-web/spec/features/create_enquiry_as_opss_user_spec.rb b/psd-web/spec/features/create_enquiry_as_opss_user_spec.rb
index ddd7df95c7..30285998b2 100644
--- a/psd-web/spec/features/create_enquiry_as_opss_user_spec.rb
+++ b/psd-web/spec/features/create_enquiry_as_opss_user_spec.rb
@@ -171,8 +171,7 @@ def expect_case_activity_page_to_show_restricted_information(enquiry)
expect(page).to have_css("p", text: "Case is related to the coronavirus outbreak.")
expect(page).to have_css("p", text: enquiry.fetch(:enquiry_description))
- expect(page).to have_text("Restricted access")
- expect(page).to have_text("Consumer contact details hidden to comply with GDPR legislation. Contact test organisation, who created this activity, to obtain these details if required.")
+ expect(page).to have_css("p", text: "Only teams added to the case can view enquiry contact details")
expect(page).not_to have_text("Name")
expect(page).not_to have_text("Email address")
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/models/user_spec.rb b/psd-web/spec/models/user_spec.rb
index a16688b76b..f7f27483c3 100644
--- a/psd-web/spec/models/user_spec.rb
+++ b/psd-web/spec/models/user_spec.rb
@@ -163,7 +163,7 @@
it "includes associations needed for display_name" do
owners = described_class.get_owners.to_a # to_a forces the query execution and load immediately
expect(lambda {
- owners.map { |owner| owner.decorate.display_name }
+ owners.map { |owner| owner.decorate.display_name(viewer: active_user) }
}).to not_talk_to_db
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..8fd8130cf9 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 }
@@ -155,29 +155,35 @@
context "when there is a complainant" do
let(:factory_trait) { :with_complainant }
- let(:complainant) { investigation.complainant }
- before do
- allow(complainant).to receive(:can_be_displayed?).with(user).and_return(true)
+ context "when the user is on a team collaborating on the case" do
+ before do
+ create(:collaboration_edit_access, investigation: investigation, collaborator: user.team)
+ end
+
+ it "returns true" do
+ expect(can_display).to be true
+ end
end
- it "returns the value of complainant#can_be_displayed?", :aggregate_failures do
- expect(can_display).to be true
- expect(complainant).to have_received(:can_be_displayed?).with(user).once
+ context "when the user is not on a team collaborating on the case" do
+ it "returns false" do
+ expect(can_display).to be false
+ end
end
end
end
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