Skip to content

Commit

Permalink
Add activity when changing case owner team but not ultimate case owner
Browse files Browse the repository at this point in the history
  • Loading branch information
slorek committed Sep 21, 2020
1 parent 9a69d56 commit 023d1c9
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 11 deletions.
28 changes: 23 additions & 5 deletions app/services/delete_team.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ def call

def remove_team_as_owner_from_cases
team.owner_collaborations.each do |collaboration|
next change_case_owner(collaboration) if collaboration.investigation.owner == team # Team is the ultimate owner
next change_case_owner_to_new_team(collaboration) if collaboration.investigation.owner == team # Team is the ultimate owner

collaboration.update!(collaborator_id: new_team.id) # User on the team is the ultimate owner
change_case_owner_team(collaboration) # User on the team is the ultimate owner
end
end

def change_case_owner(collaboration)
def change_case_owner_to_new_team(collaboration)
ChangeCaseOwner.call!(
investigation: collaboration.investigation,
owner: new_team,
Expand All @@ -38,6 +38,24 @@ def change_case_owner(collaboration)
)
end

def change_case_owner_team(collaboration)
collaboration.update!(collaborator_id: new_team.id)

metadata = update_owner_activity_class.build_metadata(new_team, change_case_owner_rationale)

update_owner_activity_class.create!(
source: UserSource.new(user: user),
investigation: collaboration.investigation,
title: nil,
body: nil,
metadata: metadata
)
end

def update_owner_activity_class
AuditActivity::Investigation::UpdateOwner
end

def remove_team_as_collaborator_from_cases
team.collaboration_accesses.changeable.each do |collaboration|
add_new_team_to_case(collaboration) unless new_team_already_collaborating?(collaboration.investigation)
Expand Down Expand Up @@ -70,11 +88,11 @@ def remove_team_from_case(collaboration)
end

def change_case_owner_rationale
"#{team_display_name} was merged into #{new_team_display_name} by #{user_display_name}. #{team_display_name} previously owned this case."
I18n.t("delete_team.change_case_owner_rationale", team: team_display_name, new_team: new_team_display_name, user: user_display_name)
end

def add_remove_team_message
"#{team_display_name} was merged into #{new_team_display_name} by #{user_display_name}. #{team_display_name} previously had access to this case."
I18n.t("delete_team.add_remove_team_message", team: team_display_name, new_team: new_team_display_name, user: user_display_name)
end

def team_display_name
Expand Down
3 changes: 3 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ en:
safe_and_compliant: "Product reported because it is safe and compliant."
invite_user_to_team:
invite_sent: "Invite sent to %{email}"
delete_team:
change_case_owner_rationale: "%{team} was merged into %{new_team} by %{user}. %{team} previously owned this case."
add_remove_team_message: "%{team} was merged into %{new_team} by %{user}. %{team} previously had access to this case."
case:
badges:
coronavirus_related: "COVID-19 related case"
Expand Down
27 changes: 21 additions & 6 deletions spec/services/delete_team_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
let(:new_team_user) { create(:user, :activated, :team_admin, team: new_team, organisation: new_team.organisation) }
let(:deleting_user) { create(:user) }

let(:team_case) { create(:allegation, creator: team_user) }

describe ".call" do
subject(:result) { delete_team }

Expand Down Expand Up @@ -52,6 +50,7 @@ def delete_team
end

context "with required parameters" do
let!(:team_case) { create(:allegation, creator: team_user) }
let(:deleting_user) { team_user }

context "when the team is already deleted" do
Expand Down Expand Up @@ -114,10 +113,11 @@ def delete_team
expect { delete_team }.to change { team_case.reload.owner }.from(team).to(new_team)
end

it "adds activity showing the case ownership changed to the new team" do
it "adds activity showing the case ownership changed to the new team", :aggregate_failures do
delete_team
activity = team_case.activities.find_by!(type: AuditActivity::Investigation::UpdateOwner.to_s)
expect(activity.owner).to eq(new_team)
expect(activity.body).to eq("#{team.name} was merged into #{new_team.name} by #{deleting_user.name} (#{deleting_user.team.name}). #{team.name} previously owned this case.")
end

it "does not send notification e-mails", :with_test_queue_adapter, :aggregate_failures do
Expand Down Expand Up @@ -149,6 +149,18 @@ def delete_team
it "updates the owner team to the user's new team" do
expect { delete_team }.to change { team_case.reload.owner_team }.from(team).to(new_team)
end

it "adds activity showing the case ownership changed to the new team", :aggregate_failures do
delete_team
activity = team_case.activities.find_by!(type: AuditActivity::Investigation::UpdateOwner.to_s)
expect(activity.owner).to eq(new_team)
expect(activity.body).to eq("#{team.name} was merged into #{new_team.name} by #{deleting_user.name} (#{deleting_user.team.name}). #{team.name} previously owned this case.")
end

it "does not send notification e-mails", :with_test_queue_adapter, :aggregate_failures do
expect { delete_team }.not_to have_enqueued_mail(NotifyMailer, :investigation_updated)
expect { delete_team }.not_to have_enqueued_mail(NotifyMailer, :team_deleted_from_case_email)
end
end

context "when the team is a collaborator on a case owned by the new team" do
Expand All @@ -158,10 +170,11 @@ def delete_team
expect { delete_team }.to change { new_team_case.reload.teams_with_read_only_access }.from([team]).to([])
end

it "adds activity showing the old team removed from the case" do
it "adds activity showing the old team removed from the case", :aggregate_failures do
delete_team
activity = new_team_case.activities.find_by!(type: AuditActivity::Investigation::TeamDeleted.to_s)
expect(activity.team).to eq(team)
expect(activity.metadata["message"]).to eq("#{team.name} was merged into #{new_team.name} by #{deleting_user.name} (#{deleting_user.team.name}). #{team.name} previously had access to this case.")
end

it "does not change the new team's access level on the case" do
Expand All @@ -178,10 +191,11 @@ def delete_team
let(:read_only_teams) { [team] }
let(:edit_access_teams) { nil }

it "adds activity showing the old team removed from the case" do
it "adds activity showing the old team removed from the case", :aggregate_failures do
delete_team
activity = other_team_case.activities.find_by!(type: AuditActivity::Investigation::TeamDeleted.to_s)
expect(activity.team).to eq(team)
expect(activity.metadata["message"]).to eq("#{team.name} was merged into #{new_team.name} by #{deleting_user.name} (#{deleting_user.team.name}). #{team.name} previously had access to this case.")
end

it "does not send notification e-mails", :with_test_queue_adapter, :aggregate_failures do
Expand All @@ -195,10 +209,11 @@ def delete_team
expect { delete_team }.to change { other_team_case.reload.teams_with_read_only_access }.from([team]).to([new_team])
end

it "adds activity showing the new team added to the case" do
it "adds activity showing the new team added to the case", :aggregate_failures do
delete_team
activity = other_team_case.activities.find_by!(type: AuditActivity::Investigation::TeamAdded.to_s)
expect(activity.team).to eq(new_team)
expect(activity.metadata["message"]).to eq("#{team.name} was merged into #{new_team.name} by #{deleting_user.name} (#{deleting_user.team.name}). #{team.name} previously had access to this case.")
end
end

Expand Down

0 comments on commit 023d1c9

Please sign in to comment.