From 642bdf02bac23a1331e0f3628f09b126fc471915 Mon Sep 17 00:00:00 2001
From: Steve Lorek <steve@stevelorek.com>
Date: Tue, 9 Jun 2020 19:38:13 +0100
Subject: [PATCH] Refactor update owner audit activity to store metadata, and
 remove callbacks and dependency on User.current

---
 .../investigations/ownership_controller.rb    |  55 +++---
 .../controllers/investigations_controller.rb  |   2 +-
 psd-web/app/forms/change_case_owner_form.rb   |  28 +++
 psd-web/app/helpers/investigations_helper.rb  |   4 +-
 psd-web/app/models/activity.rb                |  11 +-
 .../correspondence/add_email.rb               |   2 +-
 .../correspondence/add_meeting.rb             |   2 +-
 .../correspondence/add_phone_call.rb          |   2 +-
 .../app/models/audit_activity/document/add.rb |   2 +-
 .../models/audit_activity/document/base.rb    |   2 +-
 .../models/audit_activity/document/destroy.rb |   2 +-
 .../models/audit_activity/document/update.rb  |   2 +-
 .../audit_activity/investigation/add.rb       |   6 +-
 .../automatically_update_owner.rb             |  31 ++--
 .../investigation/team_added.rb               |   4 +-
 .../investigation/team_deleted.rb             |   4 +-
 .../investigation/update_owner.rb             |  63 ++-----
 psd-web/app/models/comment_activity.rb        |   2 +-
 psd-web/app/models/investigation.rb           |  25 +--
 psd-web/app/services/change_case_owner.rb     |  74 ++++++++
 psd-web/app/services/delete_user.rb           |  29 ++-
 .../activities/_activity_card.html.erb        |   4 +-
 .../investigation/_update_owner.html.erb      |   1 +
 psd-web/config/locales/en.yml                 |   5 +
 ...320_move_update_owner_activity_metadata.rb |  33 ++++
 psd-web/db/schema.rb                          |   2 +-
 ...change_ownership_for_investigation_spec.rb |   2 +-
 .../features/record_email_activity_spec.rb    |  12 +-
 .../record_phone_call_activity_spec.rb        |  12 +-
 .../spec/forms/change_case_owner_form_spec.rb |  60 +++++++
 ...it_investigation_collaborator_form_spec.rb |   2 +-
 .../automatically_update_owner_spec.rb        |  19 --
 .../update_coronavirus_status_spec.rb         |   2 +-
 .../investigation/update_owner_spec.rb        |  44 +++--
 .../spec/services/add_product_to_case_spec.rb |   2 +-
 .../add_team_to_an_investigation_spec.rb      |   2 +-
 .../spec/services/change_case_owner_spec.rb   | 165 ++++++++++++++++++
 psd-web/spec/services/create_case_spec.rb     |   2 +-
 psd-web/spec/services/delete_user_spec.rb     |   4 +-
 .../services/remove_product_from_case_spec.rb |   2 +-
 .../support/audit_activity_investigation.rb   |   6 +-
 psd-web/test/models/complainant_test.rb       |   1 +
 psd-web/test/models/investigation_test.rb     |  10 +-
 psd-web/test/models/notification_test.rb      |  57 ------
 44 files changed, 536 insertions(+), 265 deletions(-)
 create mode 100644 psd-web/app/forms/change_case_owner_form.rb
 create mode 100644 psd-web/app/services/change_case_owner.rb
 create mode 100644 psd-web/app/views/investigations/activities/investigation/_update_owner.html.erb
 create mode 100644 psd-web/db/migrate/20200610111320_move_update_owner_activity_metadata.rb
 create mode 100644 psd-web/spec/forms/change_case_owner_form_spec.rb
 delete mode 100644 psd-web/spec/models/audit_activity/investigation/automatically_update_owner_spec.rb
 create mode 100644 psd-web/spec/services/change_case_owner_spec.rb

diff --git a/psd-web/app/controllers/investigations/ownership_controller.rb b/psd-web/app/controllers/investigations/ownership_controller.rb
index dc638ff805..516bbebad2 100644
--- a/psd-web/app/controllers/investigations/ownership_controller.rb
+++ b/psd-web/app/controllers/investigations/ownership_controller.rb
@@ -2,42 +2,37 @@ class Investigations::OwnershipController < ApplicationController
   include Wicked::Wizard
   before_action :set_investigation
   before_action :authorize_user
-  before_action :potential_owner, only: %i[show create]
-  before_action :store_owner, only: %i[update]
 
   steps :"select-owner", :confirm
 
   def show
-    @potential_owner = potential_owner&.decorate
+    @potential_owner = form.owner&.decorate
     render_wizard
   end
 
   def new
-    clear_session
+    session[session_store_key] = nil
     redirect_to wizard_path(steps.first, request.query_parameters)
   end
 
   def update
-    if owner_valid?
-      redirect_to next_wizard_path
-    else
-      render_wizard
-    end
+    return render_wizard unless form.valid?
+
+    session[session_store_key] = form.attributes.compact
+    redirect_to next_wizard_path
   end
 
   def create
-    @investigation.owner = potential_owner
-    @investigation.owner_rationale = params[:investigation][:owner_rationale]
-    @investigation.save
-    redirect_to investigation_path(@investigation), flash: { success: "#{@investigation.case_type.upcase_first} owner changed to #{potential_owner.decorate.display_name}" }
-  end
+    ChangeCaseOwner.call!(investigation: @investigation, owner: form.owner, rationale: form.owner_rationale, user: current_user)
 
-private
+    session[session_store_key] = nil
 
-  def clear_session
-    session[:owner_id] = nil
+    message = "#{@investigation.case_type.upcase_first} owner changed to #{form.owner.decorate.display_name(viewer: current_user)}"
+    redirect_to investigation_path(@investigation), flash: { success: message }
   end
 
+private
+
   def set_investigation
     @investigation = Investigation.find_by!(pretty_id: params[:investigation_pretty_id]).decorate
   end
@@ -46,7 +41,12 @@ def authorize_user
     authorize @investigation, :change_owner_or_status?
   end
 
-  def owner_params
+  def session_store_key
+    "update_case_owner_#{@investigation.pretty_id}_params"
+  end
+
+  def form_params
+    params[:investigation] ||= {}
     params[:investigation][:owner_id] = case params[:investigation][:owner_id]
                                         when "someone_in_your_team"
                                           params[:investigation][:select_team_member]
@@ -59,23 +59,14 @@ def owner_params
                                         else
                                           params[:investigation][:owner_id]
                                         end
-    params.require(:investigation).permit(:owner_id)
-  end
-
-  def store_owner
-    session[:owner_id] = owner_params[:owner_id]
+    params.require(:investigation).permit(:owner_id, :owner_rationale).merge(session_params)
   end
 
-  def owner_valid?
-    if step == :"select-owner"
-      if potential_owner.nil?
-        @investigation.errors.add(:owner_id, :invalid, message: "Select case owner")
-      end
-    end
-    @investigation.errors.empty?
+  def session_params
+    session[session_store_key] || {}
   end
 
-  def potential_owner
-    User.find_by(id: session[:owner_id]) || Team.find_by(id: session[:owner_id])
+  def form
+    @form ||= ChangeCaseOwnerForm.new(form_params)
   end
 end
diff --git a/psd-web/app/controllers/investigations_controller.rb b/psd-web/app/controllers/investigations_controller.rb
index f4736766bb..e0e4504297 100644
--- a/psd-web/app/controllers/investigations_controller.rb
+++ b/psd-web/app/controllers/investigations_controller.rb
@@ -124,6 +124,6 @@ def build_breadcrumbs
   end
 
   def set_suggested_previous_owners
-    @suggested_previous_owners = suggested_previous_owners
+    @suggested_previous_owners = suggested_previous_owners(@investigation)
   end
 end
diff --git a/psd-web/app/forms/change_case_owner_form.rb b/psd-web/app/forms/change_case_owner_form.rb
new file mode 100644
index 0000000000..76e76c1f1b
--- /dev/null
+++ b/psd-web/app/forms/change_case_owner_form.rb
@@ -0,0 +1,28 @@
+class ChangeCaseOwnerForm
+  include ActiveModel::Model
+  include ActiveModel::Attributes
+
+  attribute :owner_id
+  attribute :owner_rationale
+
+  validates_presence_of :owner_id
+  validate :new_owner_must_be_active_user_or_team, if: -> { owner_id.present? }
+
+  def owner
+    user || team
+  end
+
+private
+
+  def new_owner_must_be_active_user_or_team
+    errors.add(:owner_id, :not_found) unless owner
+  end
+
+  def user
+    @user ||= User.active.find_by(id: owner_id)
+  end
+
+  def team
+    @team ||= Team.find_by(id: owner_id)
+  end
+end
diff --git a/psd-web/app/helpers/investigations_helper.rb b/psd-web/app/helpers/investigations_helper.rb
index d01280995c..874b172a17 100644
--- a/psd-web/app/helpers/investigations_helper.rb
+++ b/psd-web/app/helpers/investigations_helper.rb
@@ -241,8 +241,8 @@ def user_ids_from_team(team)
     [team.id] + team.users.map(&:id)
   end
 
-  def suggested_previous_owners
-    all_past_owners = @investigation.past_owners + @investigation.past_teams
+  def suggested_previous_owners(investigation)
+    all_past_owners = investigation.past_owners
     return [] if all_past_owners.empty? || all_past_owners == [current_user]
 
     all_past_owners || []
diff --git a/psd-web/app/models/activity.rb b/psd-web/app/models/activity.rb
index 16aaa5f493..c78365f4a9 100644
--- a/psd-web/app/models/activity.rb
+++ b/psd-web/app/models/activity.rb
@@ -17,8 +17,13 @@ def attachments
     {}
   end
 
-  def subtitle
-    "#{subtitle_slug} by #{source&.show}, #{pretty_date_stamp}"
+  # Can be overridden by child classes but sometimes need to pass in user
+  def title(_user)
+    super()
+  end
+
+  def subtitle(viewer)
+    "#{subtitle_slug} by #{source&.show(viewer)}, #{pretty_date_stamp}"
   end
 
   def search_index;  end
@@ -31,7 +36,7 @@ def can_display_all_data?(_user)
     true
   end
 
-  def restricted_title
+  def restricted_title(_user)
     # where necessary should be implemented by subclasses
   end
 
diff --git a/psd-web/app/models/audit_activity/correspondence/add_email.rb b/psd-web/app/models/audit_activity/correspondence/add_email.rb
index 7955ab660f..a9ac92087e 100644
--- a/psd-web/app/models/audit_activity/correspondence/add_email.rb
+++ b/psd-web/app/models/audit_activity/correspondence/add_email.rb
@@ -48,7 +48,7 @@ def self.build_email_address(correspondence)
     output + "<br>"
   end
 
-  def restricted_title
+  def restricted_title(_user)
     "Email added"
   end
 
diff --git a/psd-web/app/models/audit_activity/correspondence/add_meeting.rb b/psd-web/app/models/audit_activity/correspondence/add_meeting.rb
index 585be78a56..4cc9b660c2 100644
--- a/psd-web/app/models/audit_activity/correspondence/add_meeting.rb
+++ b/psd-web/app/models/audit_activity/correspondence/add_meeting.rb
@@ -23,7 +23,7 @@ def activity_type
     "meeting"
   end
 
-  def restricted_title
+  def restricted_title(_user)
     "Meeting added"
   end
 
diff --git a/psd-web/app/models/audit_activity/correspondence/add_phone_call.rb b/psd-web/app/models/audit_activity/correspondence/add_phone_call.rb
index 88adf4a726..a0d6a828c2 100644
--- a/psd-web/app/models/audit_activity/correspondence/add_phone_call.rb
+++ b/psd-web/app/models/audit_activity/correspondence/add_phone_call.rb
@@ -39,7 +39,7 @@ def self.build_file_body(correspondence)
     file.attached? ? "Attached: #{sanitize_text file.filename}<br>" : ""
   end
 
-  def restricted_title
+  def restricted_title(_user)
     "Phone call added"
   end
 
diff --git a/psd-web/app/models/audit_activity/document/add.rb b/psd-web/app/models/audit_activity/document/add.rb
index fb10d31cfd..27da39f938 100644
--- a/psd-web/app/models/audit_activity/document/add.rb
+++ b/psd-web/app/models/audit_activity/document/add.rb
@@ -8,7 +8,7 @@ def email_update_text(viewer = nil)
     "Document was attached to the #{investigation.case_type.upcase_first} by #{source&.show(viewer)}."
   end
 
-  def restricted_title
+  def restricted_title(_user)
     "Document added"
   end
 
diff --git a/psd-web/app/models/audit_activity/document/base.rb b/psd-web/app/models/audit_activity/document/base.rb
index 2a4bb3168e..ac713fb088 100644
--- a/psd-web/app/models/audit_activity/document/base.rb
+++ b/psd-web/app/models/audit_activity/document/base.rb
@@ -21,7 +21,7 @@ def attached_image?
     attachment.image?
   end
 
-  def restricted_title; end
+  def restricted_title(_user); end
 
   def can_display_all_data?(user)
     can_be_displayed?(attachment, investigation, user)
diff --git a/psd-web/app/models/audit_activity/document/destroy.rb b/psd-web/app/models/audit_activity/document/destroy.rb
index 16b19991d5..9d6fd6cfb2 100644
--- a/psd-web/app/models/audit_activity/document/destroy.rb
+++ b/psd-web/app/models/audit_activity/document/destroy.rb
@@ -4,7 +4,7 @@ def self.from(document, investigation)
     super(document, investigation, title)
   end
 
-  def restricted_title
+  def restricted_title(_user)
     "Document deleted"
   end
 
diff --git a/psd-web/app/models/audit_activity/document/update.rb b/psd-web/app/models/audit_activity/document/update.rb
index dc3dd004cb..25b1ef6192 100644
--- a/psd-web/app/models/audit_activity/document/update.rb
+++ b/psd-web/app/models/audit_activity/document/update.rb
@@ -22,7 +22,7 @@ def self.description_changed?(document, previous_data)
     document.metadata[:description] != previous_data[:description]
   end
 
-  def restricted_title
+  def restricted_title(_user)
     "Document updated"
   end
 
diff --git a/psd-web/app/models/audit_activity/investigation/add.rb b/psd-web/app/models/audit_activity/investigation/add.rb
index ab016d82dd..f3074f05dd 100644
--- a/psd-web/app/models/audit_activity/investigation/add.rb
+++ b/psd-web/app/models/audit_activity/investigation/add.rb
@@ -31,7 +31,7 @@ def complainant
 
   # title may change after investigation was created, so we retrieve from the
   # metadata stored at the time of creation
-  def title
+  def title(_user)
     return self[:title] if self[:title] # older activities stored this in the database
 
     "#{investigation.case_type.titleize} logged: #{metadata['investigation']['title']}"
@@ -49,8 +49,8 @@ def can_display_all_data?(user)
   end
 
   # Only used for old records prior to metadata implementation
-  def restricted_title
-    title
+  def restricted_title(user)
+    title(user)
   end
 
 private
diff --git a/psd-web/app/models/audit_activity/investigation/automatically_update_owner.rb b/psd-web/app/models/audit_activity/investigation/automatically_update_owner.rb
index 073245e6f8..302b8e58bf 100644
--- a/psd-web/app/models/audit_activity/investigation/automatically_update_owner.rb
+++ b/psd-web/app/models/audit_activity/investigation/automatically_update_owner.rb
@@ -1,31 +1,30 @@
 class AuditActivity::Investigation::AutomaticallyUpdateOwner < AuditActivity::Investigation::Base
-  def self.from(investigation)
-    title = investigation.owner.id.to_s
-    super(investigation, title)
+  def self.from(*)
+    raise "Deprecated - use DeleteUser.call instead"
   end
 
-  def owner_id
-    # We store owner_id in title field, this is getting it back
-    # Using alias for accessing parent method causes errors elsewhere :(
-    AuditActivity::Investigation::Base.instance_method(:title).bind(self).call
+  def self.build_metadata(owner)
+    {
+      owner_id: owner.id
+    }
   end
 
-  # We store owner_id in title field, this is computing title based on that
-  def title
+  def title(user)
     type = investigation.case_type.capitalize
-    new_owner = (User.find_by(id: owner_id) || Team.find_by(id: owner_id))&.decorate&.display_name
+    new_owner = owner.decorate.display_name(viewer: user)
     "Case owner automatically changed on #{type} to #{new_owner}"
   end
 
-  def subtitle
+  def subtitle(_viewer)
     "Case owner automatically changed, #{pretty_date_stamp}"
   end
 
-  def entities_to_notify
-    []
-  end
+private
 
-  def email_subject_text; end
+  def owner
+    User.find_by(id: metadata["owner_id"]) || Team.find_by(id: metadata["owner_id"])
+  end
 
-  def email_update_text(viewer = nil); end
+  # Do not send investigation_updated mail. This is handled by the DeleteUser service
+  def notify_relevant_users; end
 end
diff --git a/psd-web/app/models/audit_activity/investigation/team_added.rb b/psd-web/app/models/audit_activity/investigation/team_added.rb
index 2641221a82..a0ca4ade04 100644
--- a/psd-web/app/models/audit_activity/investigation/team_added.rb
+++ b/psd-web/app/models/audit_activity/investigation/team_added.rb
@@ -1,6 +1,6 @@
 class AuditActivity::Investigation::TeamAdded < AuditActivity::Investigation::Base
-  def subtitle
-    "Team added by #{source&.show}, #{pretty_date_stamp}"
+  def subtitle(viewer)
+    "Team added by #{source&.show(viewer)}, #{pretty_date_stamp}"
   end
 
 private
diff --git a/psd-web/app/models/audit_activity/investigation/team_deleted.rb b/psd-web/app/models/audit_activity/investigation/team_deleted.rb
index 715d5497d8..806031e8cd 100644
--- a/psd-web/app/models/audit_activity/investigation/team_deleted.rb
+++ b/psd-web/app/models/audit_activity/investigation/team_deleted.rb
@@ -1,6 +1,6 @@
 class AuditActivity::Investigation::TeamDeleted < AuditActivity::Investigation::Base
-  def subtitle
-    "Team removed by #{source&.show}, #{pretty_date_stamp}"
+  def subtitle(viewer)
+    "Team removed by #{source&.show(viewer)}, #{pretty_date_stamp}"
   end
 
 private
diff --git a/psd-web/app/models/audit_activity/investigation/update_owner.rb b/psd-web/app/models/audit_activity/investigation/update_owner.rb
index 59d9aed90d..a23684a185 100644
--- a/psd-web/app/models/audit_activity/investigation/update_owner.rb
+++ b/psd-web/app/models/audit_activity/investigation/update_owner.rb
@@ -1,37 +1,25 @@
 class AuditActivity::Investigation::UpdateOwner < AuditActivity::Investigation::Base
-  include NotifyHelper
-
-  def self.from(investigation)
-    title = investigation.owner.id.to_s
-    body = investigation.owner_rationale
-    super(investigation, title, sanitize_text(body))
+  def self.from(*)
+    raise "Deprecated - use ChangeCaseOwner.call instead"
   end
 
-  def owner_id
-    # We store owner_id in title field, this is getting it back
-    # Using alias for accessing parent method causes errors elsewhere :(
-    AuditActivity::Investigation::Base.instance_method(:title).bind(self).call
+  def self.build_metadata(owner, rationale)
+    {
+      owner_id: owner.id,
+      rationale: rationale
+    }
   end
 
-  def title
-    # We store owner_id in title field, this is computing title based on that
-    "Case owner changed to #{(User.find_by(id: owner_id) || Team.find_by(id: owner_id))&.decorate&.display_name}"
+  def title(user)
+    "Case owner changed to #{owner.decorate.display_name(viewer: user)}"
   end
 
-  def email_update_text(viewer = nil)
-    body = []
-    body << "Case owner changed on #{investigation.case_type} to #{investigation.owner.decorate.display_name(viewer: viewer)} by #{source&.show(viewer)}."
-
-    if investigation.owner_rationale.present?
-      body << "Message from #{source&.show(viewer)}:"
-      body << inset_text_for_notify(investigation.owner_rationale)
-    end
-
-    body.join("\n\n")
+  def body
+    metadata["rationale"]
   end
 
-  def email_subject_text
-    "Case owner changed for #{investigation.case_type}"
+  def owner
+    User.find_by(id: metadata["owner_id"]) || Team.find_by(id: metadata["owner_id"])
   end
 
 private
@@ -40,27 +28,6 @@ def subtitle_slug
     "Changed"
   end
 
-  def users_to_notify
-    compute_relevant_entities(model: User, compute_users_from_entity: proc { |user| [user] })
-  end
-
-  def teams_to_notify
-    compute_relevant_entities(model: Team, compute_users_from_entity: proc { |team| team.users })
-  end
-
-  def compute_relevant_entities(model:, compute_users_from_entity:)
-    return [] unless investigation.saved_changes.key?("owner_id")
-
-    previous_owner_id = investigation.saved_changes["owner_id"][0]
-    previous_owner = model.find_by(id: previous_owner_id)
-    new_owner = investigation.owner
-    owner_changed_by = source.user
-
-    old_users = previous_owner.present? ? compute_users_from_entity.call(previous_owner) : []
-    old_entities = previous_owner.present? ? [previous_owner] : []
-    new_entities = new_owner.is_a?(model) ? [new_owner] : []
-    return new_entities if old_users.include? owner_changed_by
-
-    (new_entities + old_entities).uniq
-  end
+  # Do not send investigation_updated mail. This is handled by the ChangeCaseOwner service
+  def notify_relevant_users; end
 end
diff --git a/psd-web/app/models/comment_activity.rb b/psd-web/app/models/comment_activity.rb
index cd9b92c896..cc753d64c6 100644
--- a/psd-web/app/models/comment_activity.rb
+++ b/psd-web/app/models/comment_activity.rb
@@ -4,7 +4,7 @@ class CommentActivity < Activity
   validates :body, presence: true
   validates :body, length: { maximum: 10_000 }
 
-  def title
+  def title(_user)
     "Comment added"
   end
 
diff --git a/psd-web/app/models/investigation.rb b/psd-web/app/models/investigation.rb
index bd8bf53d0d..05955fc40a 100644
--- a/psd-web/app/models/investigation.rb
+++ b/psd-web/app/models/investigation.rb
@@ -20,15 +20,17 @@ class Investigation < ApplicationRecord
   validates :type, presence: true # Prevent saving instances of Investigation; must use a subclass instead
 
   validates :description, presence: true, on: :update
-  validates :owner_id, presence: { message: "Select case owner" }, on: :update
+
+  # Can currently only validate owner_id on update since create case wizards
+  # validate partially-built Investigation instances
+  validates :owner_id, presence: true, on: :update
 
   validates :user_title, length: { maximum: 100 }
   validates :description, length: { maximum: 10_000 }
   validates :non_compliant_reason, length: { maximum: 10_000 }
   validates :hazard_description, length: { maximum: 10_000 }
 
-  after_update :create_audit_activity_for_owner,
-               :create_audit_activity_for_status,
+  after_update :create_audit_activity_for_status,
                :create_audit_activity_for_visibility,
                :create_audit_activity_for_summary
 
@@ -96,8 +98,7 @@ def important_owner_people
 
   def past_owners
     activities = AuditActivity::Investigation::UpdateOwner.where(investigation_id: id)
-    user_id_list = activities.map(&:owner_id)
-    User.where(id: user_id_list)
+    activities.map(&:owner)
   end
 
   def important_owner_teams
@@ -110,12 +111,6 @@ def important_owner_teams
     teams
   end
 
-  def past_teams
-    activities = AuditActivity::Investigation::UpdateOwner.where(investigation_id: id)
-    team_id_list = activities.map(&:owner_id)
-    Team.where(id: team_id_list)
-  end
-
   def enquiry?
     is_a?(Investigation::Enquiry)
   end
@@ -171,14 +166,6 @@ def create_audit_activity_for_visibility
     end
   end
 
-  def create_audit_activity_for_owner
-    # TODO: User.current check is here to avoid triggering activity and emails from migrations
-    # Can be safely removed once the migration PopulateAssigneeAndDescription has run
-    if ((saved_changes.key? :owner_id) || (saved_changes.key? :owner_type)) && User.current
-      AuditActivity::Investigation::UpdateOwner.from(self)
-    end
-  end
-
   def create_audit_activity_for_summary
     # TODO: User.current check is here to avoid triggering activity and emails from migrations
     # Can be safely removed once the migration PopulateAssigneeAndDescription has run
diff --git a/psd-web/app/services/change_case_owner.rb b/psd-web/app/services/change_case_owner.rb
new file mode 100644
index 0000000000..3e6dccb6ea
--- /dev/null
+++ b/psd-web/app/services/change_case_owner.rb
@@ -0,0 +1,74 @@
+class ChangeCaseOwner
+  include Interactor
+  include NotifyHelper
+
+  delegate :investigation, :owner, :rationale, :user, :old_owner, to: :context
+
+  def call
+    context.fail!(error: "No investigation supplied") unless investigation.is_a?(Investigation)
+    context.fail!(error: "No owner supplied") unless owner.is_a?(User) || owner.is_a?(Team)
+    context.fail!(error: "No user supplied") unless user.is_a?(User)
+
+    context.old_owner = investigation.owner
+
+    return if old_owner == owner
+
+    ActiveRecord::Base.transaction do
+      investigation.update!(owner: owner)
+      create_audit_activity_for_case_owner_changed
+    end
+
+    send_notification_email
+  end
+
+private
+
+  def create_audit_activity_for_case_owner_changed
+    metadata = activity_class.build_metadata(owner, rationale)
+
+    activity_class.create!(
+      source: UserSource.new(user: user),
+      investigation: investigation,
+      title: nil,
+      body: nil,
+      metadata: metadata
+    )
+  end
+
+  def activity_class
+    AuditActivity::Investigation::UpdateOwner
+  end
+
+  def send_notification_email
+    entities_to_notify.each do |recipient|
+      email = recipient.is_a?(Team) ? recipient.team_recipient_email : recipient.email
+
+      NotifyMailer.investigation_updated(
+        investigation.pretty_id,
+        recipient.name,
+        email,
+        email_body(recipient),
+        "Case owner changed for #{investigation.case_type}"
+      ).deliver_later
+    end
+  end
+
+  # Notify the new owner, and the old one if it's not the user making the change
+  def entities_to_notify
+    entities = [owner]
+    entities << old_owner if old_owner != user
+
+    entities.map { |entity|
+      return entity.users.active if entity.is_a?(Team) && !entity.team_recipient_email
+
+      entity
+    }.flatten.uniq
+  end
+
+  def email_body(viewer = nil)
+    user_name = user.decorate.display_name(viewer: viewer)
+    body = "Case owner changed on #{investigation.case_type} to #{owner.decorate.display_name(viewer: viewer)} by #{user_name}."
+    body << "\n\nMessage from #{user_name}: #{inset_text_for_notify(rationale)}" if rationale
+    body
+  end
+end
diff --git a/psd-web/app/services/delete_user.rb b/psd-web/app/services/delete_user.rb
index e8c28d3e4c..c1328e002f 100644
--- a/psd-web/app/services/delete_user.rb
+++ b/psd-web/app/services/delete_user.rb
@@ -1,12 +1,14 @@
 class DeleteUser
   include Interactor
 
+  delegate :user, to: :context
+
   def call
-    context.fail!(error: "No user supplied") unless context.user
-    context.fail!(error: "User already deleted") if context.user.deleted?
+    context.fail!(error: "No user supplied") unless user.is_a?(User)
+    context.fail!(error: "User already deleted") if user.deleted?
 
     ActiveRecord::Base.transaction do
-      context.user.mark_as_deleted!
+      user.mark_as_deleted!
       change_user_investigations_ownership_to_their_team
     end
   end
@@ -16,10 +18,27 @@ def call
   def change_user_investigations_ownership_to_their_team
     context.team = context.user.team
 
-    context.user.investigations.each do |investigation|
+    user.investigations.each do |investigation|
       investigation.owner = context.team
       investigation.save!
-      AuditActivity::Investigation::AutomaticallyUpdateOwner.from(investigation)
+
+      create_audit_activity_for_case_owner_automatically_changed(investigation)
     end
   end
+
+  def create_audit_activity_for_case_owner_automatically_changed(investigation)
+    metadata = activity_class.build_metadata(investigation.owner)
+
+    activity_class.create!(
+      source: nil, # DeleteUser is called from rake user:delete where no user source is available
+      investigation: investigation,
+      title: nil,
+      body: nil,
+      metadata: metadata
+    )
+  end
+
+  def activity_class
+    AuditActivity::Investigation::AutomaticallyUpdateOwner
+  end
 end
diff --git a/psd-web/app/views/investigations/activities/_activity_card.html.erb b/psd-web/app/views/investigations/activities/_activity_card.html.erb
index f4232c1036..4eba9a9ec2 100644
--- a/psd-web/app/views/investigations/activities/_activity_card.html.erb
+++ b/psd-web/app/views/investigations/activities/_activity_card.html.erb
@@ -1,12 +1,12 @@
 <%
   restricted = !activity.can_display_all_data?(current_user)
-  title = restricted ? activity.restricted_title : activity.title
+  title = restricted ? activity.restricted_title(current_user) : activity.title(current_user)
 %>
 
 <li>
   <h3 class="govuk-heading-s"><%= title %></h3>
 
-  <p class="govuk-body-s timeline-details"><%= activity.subtitle %></p>
+  <p class="govuk-body-s timeline-details"><%= activity.subtitle(current_user) %></p>
 
   <% 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