Merge branch 'main' into CLDC-3241-Part-1-Explanation-of-variables-on-CSVs

Gemfile.lock

@@ -141,7 +141,7 @@ GEM
     coderay (1.1.3)
     coercible (1.0.0)
       descendants_tracker (~> 0.0.1)
-    concurrent-ruby (1.3.3)
+    concurrent-ruby (1.3.4)
     connection_pool (2.4.1)
     crack (1.0.0)
       bigdecimal
@@ -180,7 +180,7 @@ GEM
       rubocop
       smart_properties
     erubi (1.13.0)
-    et-orbi (1.2.7)
+    et-orbi (1.2.11)
       tzinfo
     event_stream_parser (1.0.0)
     excon (0.109.0)
@@ -198,8 +198,8 @@ GEM
     faraday-net_http (3.1.0)
       net-http
     ffi (1.16.3)
-    fugit (1.10.0)
+    fugit (1.11.1)
-      et-orbi (~> 1, >= 1.2.7)
+      et-orbi (~> 1, >= 1.2.11)
       raabro (~> 1.4)
     globalid (1.2.1)
       activesupport (>= 6.1)

app/controllers/form_controller.rb

@@ -26,6 +26,8 @@ class FormController < ApplicationController
           flash[:notice] = "You have successfully updated #{updated_question_string}"
         end
 
+        update_duplication_tracking
+
         pages_requiring_update = pages_requiring_update(shown_page_ids_with_unanswered_questions_before_update)
         redirect_to(successful_redirect_path(pages_requiring_update))
       else
@@ -192,13 +194,16 @@ private
     params[@log.model_name.param_key]["interruption_page_referrer_type"].presence
   end
 
-  def successful_redirect_path(pages_to_check)
+  def page_has_duplicate_check_question
-    if FeatureToggle.deduplication_flow_enabled?
+    @page.questions.any? { |q| @log.duplicate_check_question_ids.include?(q.id) }
-      if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner")
-        return correcting_duplicate_logs_redirect_path
   end
 
-      dynamic_duplicates = @log.lettings? ? current_user.lettings_logs.duplicate_logs(@log) : current_user.sales_logs.duplicate_logs(@log)
+  def update_duplication_tracking
+    return unless page_has_duplicate_check_question
+
+    class_name = @log.class.name.underscore
+    dynamic_duplicates = current_user.send(class_name.pluralize).duplicate_logs(@log)
+
     if dynamic_duplicates.any?
       saved_duplicates = @log.duplicates
       if saved_duplicates.none? || duplicates_changed?(dynamic_duplicates, saved_duplicates)
@@ -206,8 +211,30 @@ private
         update_logs_with_duplicate_set_id(@log, dynamic_duplicates, duplicate_set_id)
         saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1
       end
-        return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
+    else
+      remove_fixed_duplicate_set_ids(@log)
+    end
+  end
+
+  def successful_redirect_path(pages_to_check)
+    class_name = @log.class.name.underscore
+
+    if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner")
+      original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id"))
+
+      if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).any?
+        if @log.duplicate_set_id.nil?
+          flash[:notice] = deduplication_success_banner
+        end
+        return send("#{class_name}_duplicate_logs_path", original_log, original_log_id: original_log.id, referrer: params[:referrer], organisation_id: params[:organisation_id])
+      else
+        flash[:notice] = deduplication_success_banner
+        return send("#{class_name}_duplicate_logs_path", "#{class_name}_id".to_sym => from_referrer_query("first_remaining_duplicate_id"), original_log_id: from_referrer_query("original_log_id"), referrer: params[:referrer], organisation_id: params[:organisation_id])
+      end
     end
+
+    unless @log.duplicate_set_id.nil?
+      return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
     end
 
     if is_referrer_type?("check_answers")
@@ -315,35 +342,6 @@ private
 
   CONFIRMATION_PAGE_IDS = %w[uprn_confirmation uprn_selection].freeze
 
-  def correcting_duplicate_logs_redirect_path
-    class_name = @log.class.name.underscore
-
-    original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id"))
-    dynamic_duplicates = current_user.send(class_name.pluralize).duplicate_logs(@log)
-
-    if dynamic_duplicates.any?
-      saved_duplicates = @log.duplicates
-      if duplicates_changed?(dynamic_duplicates, saved_duplicates)
-        duplicate_set_id = dynamic_duplicates.first.duplicate_set_id || new_duplicate_set_id(@log)
-        update_logs_with_duplicate_set_id(@log, dynamic_duplicates, duplicate_set_id)
-        saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1
-      end
-    else
-      remove_fixed_duplicate_set_ids(@log)
-    end
-
-    if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).any?
-      if dynamic_duplicates.none?
-        flash[:notice] = deduplication_success_banner
-      end
-      send("#{class_name}_duplicate_logs_path", original_log, original_log_id: original_log.id, referrer: params[:referrer], organisation_id: params[:organisation_id])
-    else
-      remove_fixed_duplicate_set_ids(original_log)
-      flash[:notice] = deduplication_success_banner
-      send("#{class_name}_duplicate_logs_path", "#{class_name}_id".to_sym => from_referrer_query("first_remaining_duplicate_id"), original_log_id: from_referrer_query("original_log_id"), referrer: params[:referrer], organisation_id: params[:organisation_id])
-    end
-  end
-
   def deduplication_success_banner
     deduplicated_log_link = "<a class=\"govuk-notification-banner__link govuk-!-font-weight-bold\" href=\"#{send("#{@log.class.name.underscore}_path", @log)}\">Log #{@log.id}</a>"
     changed_labels = {

app/controllers/users_controller.rb

@@ -192,14 +192,14 @@ private
   def user_params
     if @user == current_user
       if current_user.data_coordinator? || current_user.support?
-        params.require(:user).permit(:email, :phone, :name, :password, :password_confirmation, :role, :is_dpo, :is_key_contact, :initial_confirmation_sent)
+        params.require(:user).permit(:email, :phone, :phone_extension, :name, :password, :password_confirmation, :role, :is_dpo, :is_key_contact, :initial_confirmation_sent)
       else
-        params.require(:user).permit(:email, :phone, :name, :password, :password_confirmation, :initial_confirmation_sent)
+        params.require(:user).permit(:email, :phone, :phone_extension, :name, :password, :password_confirmation, :initial_confirmation_sent)
       end
     elsif current_user.data_coordinator?
-      params.require(:user).permit(:email, :phone, :name, :role, :is_dpo, :is_key_contact, :active, :initial_confirmation_sent)
+      params.require(:user).permit(:email, :phone, :phone_extension, :name, :role, :is_dpo, :is_key_contact, :active, :initial_confirmation_sent)
     elsif current_user.support?
-      params.require(:user).permit(:email, :phone, :name, :role, :is_dpo, :is_key_contact, :organisation_id, :active, :initial_confirmation_sent)
+      params.require(:user).permit(:email, :phone, :phone_extension, :name, :role, :is_dpo, :is_key_contact, :organisation_id, :active, :initial_confirmation_sent)
     end
   end
 

app/helpers/form_page_error_helper.rb

@@ -4,6 +4,15 @@ module FormPageErrorHelper
     other_page_error_ids.each { |id| lettings_log.errors.delete(id) }
   end
 
+  def remove_duplicate_page_errors(lettings_log)
+    lettings_log.errors.group_by(&:message).each do |_, errors|
+      next if errors.size == 1
+
+      errors.shift
+      errors.each { |error| lettings_log.errors.delete(error.attribute) }
+    end
+  end
+
   def all_questions_affected_by_errors(log)
     log.errors.map(&:attribute) - [:base]
   end

app/models/lettings_log.rb

@@ -701,7 +701,9 @@ class LettingsLog < Log
   end
 
   def duplicates
-    LettingsLog.where.not(duplicate_set_id: nil).where(duplicate_set_id:).where.not(id:)
+    return LettingsLog.none if duplicate_set_id.nil?
+
+    LettingsLog.where(duplicate_set_id:).where.not(id:)
   end
 
   def address_search_given?

app/models/sales_log.rb

@@ -514,7 +514,9 @@ class SalesLog < Log
   end
 
   def duplicates
-    SalesLog.where.not(duplicate_set_id: nil).where(duplicate_set_id:).where.not(id:)
+    return SalesLog.none if duplicate_set_id.nil?
+
+    SalesLog.where(duplicate_set_id:).where.not(id:)
   end
 
   def nationality2_uk_or_prefers_not_to_say?
@@ -542,4 +544,8 @@ class SalesLog < Log
   def address_search_given?
     address_line1_input.present? && postcode_full_input.present?
   end
+
+  def is_resale?
+    resale == 1
+  end
 end

app/models/user.rb

@@ -19,6 +19,7 @@ class User < ApplicationRecord
   validates :password, presence: { if: :password_required? }
   validates :password, length: { within: Devise.password_length, allow_blank: true }
   validates :password, confirmation: { if: :password_required? }
+  validates :phone_extension, format: { with: /\A\d+\z/, allow_blank: true, message: I18n.t("validations.numeric.format", field: "") }
 
   after_validation :send_data_protection_confirmation_reminder, if: :is_dpo_changed?
 
@@ -142,6 +143,7 @@ class User < ApplicationRecord
       sign_in_count: 0,
       initial_confirmation_sent: false,
       reactivate_with_organisation:,
+      unconfirmed_email: nil,
     )
   end
 
@@ -156,7 +158,6 @@ class User < ApplicationRecord
   RESET_PASSWORD_TEMPLATE_ID = "2c410c19-80a7-481c-a531-2bcb3264f8e6".freeze
   CONFIRMABLE_TEMPLATE_ID = "3fc2e3a7-0835-4b84-ab7a-ce51629eb614".freeze
   RECONFIRMABLE_TEMPLATE_ID = "bcdec787-f0a7-46e9-8d63-b3e0a06ee455".freeze
-  BETA_ONBOARDING_TEMPLATE_ID = "b48bc2cd-5887-4611-8296-d0ab3ed0e7fd".freeze
   USER_REACTIVATED_TEMPLATE_ID = "ac45a899-490e-4f59-ae8d-1256fc0001f9".freeze
   FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID = "3eb80517-1051-4dfc-b4cc-cb18228a3829".freeze
   FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID = "0cdd0be1-7fa5-4808-8225-ae4c5a002352".freeze
@@ -168,8 +169,6 @@ class User < ApplicationRecord
   def confirmable_template
     if last_sign_in_at.present? && (unconfirmed_email.blank? || unconfirmed_email == email)
       USER_REACTIVATED_TEMPLATE_ID
-    elsif was_migrated_from_softwire? && last_sign_in_at.blank?
-      BETA_ONBOARDING_TEMPLATE_ID
     elsif initial_confirmation_sent && !confirmed?
       RECONFIRMABLE_TEMPLATE_ID
     else
@@ -177,10 +176,6 @@ class User < ApplicationRecord
     end
   end
 
-  def was_migrated_from_softwire?
-    legacy_users.any? || old_user_id.present?
-  end
-
   def send_confirmation_instructions
     return unless active?
 
@@ -269,6 +264,12 @@ class User < ApplicationRecord
     save!(validate: false)
   end
 
+  def phone_with_extension
+    return phone if phone_extension.blank?
+
+    "#{phone}, Ext. #{phone_extension}"
+  end
+
 protected
 
   # Checks whether a password is needed or not. For validations only.

app/models/validations/sales/financial_validations.rb

@@ -96,9 +96,10 @@ module Validations::Sales::FinancialValidations
     if record.equity < range.min
       record.errors.add :type, I18n.t("validations.financial.equity.under_min", min_equity: range.min)
       record.errors.add :equity, :under_min, message: I18n.t("validations.financial.equity.under_min", min_equity: range.min)
-    elsif record.equity > range.max
+    elsif !record.is_resale? && record.equity > range.max
       record.errors.add :type, I18n.t("validations.financial.equity.over_max", max_equity: range.max)
       record.errors.add :equity, :over_max, message: I18n.t("validations.financial.equity.over_max", max_equity: range.max)
+      record.errors.add :resale, I18n.t("validations.financial.equity.over_max", max_equity: range.max)
     end
   end
 

app/models/validations/sales/sale_information_validations.rb

@@ -222,7 +222,7 @@ module Validations::Sales::SaleInformationValidations
                                           value: record.field_formatted_as_currency("value"),
                                           equity: "#{record.equity}%",
                                           mortgage_and_deposit_total: record.field_formatted_as_currency("mortgage_and_deposit_total"),
-                                          expected_shared_ownership_deposit_value: record.field_formatted_as_currency("expected_shared_ownership_deposit_value"))
+                                          expected_shared_ownership_deposit_value: record.field_formatted_as_currency("expected_shared_ownership_deposit_value")).html_safe
         end
         record.errors.add :type, :skip_bu_error, message: I18n.t("validations.sale_information.non_staircasing_mortgage.mortgage_used",
                                                                  mortgage: record.field_formatted_as_currency("mortgage"),
@@ -230,7 +230,7 @@ module Validations::Sales::SaleInformationValidations
                                                                  value: record.field_formatted_as_currency("value"),
                                                                  equity: "#{record.equity}%",
                                                                  mortgage_and_deposit_total: record.field_formatted_as_currency("mortgage_and_deposit_total"),
-                                                                 expected_shared_ownership_deposit_value: record.field_formatted_as_currency("expected_shared_ownership_deposit_value"))
+                                                                 expected_shared_ownership_deposit_value: record.field_formatted_as_currency("expected_shared_ownership_deposit_value")).html_safe
       end
     elsif record.mortgage_not_used?
       if over_tolerance?(record.deposit, record.expected_shared_ownership_deposit_value, 1)

app/services/feature_toggle.rb

@@ -11,10 +11,6 @@ class FeatureToggle
     !Rails.env.development?
   end
 
-  def self.deduplication_flow_enabled?
-    true
-  end
-
   def self.duplicate_summary_enabled?
     true
   end

app/views/form/check_errors.html.erb

@@ -2,6 +2,7 @@
   <div class="govuk-grid-column-three-quarters-from-desktop">
 
     <%= form_with model: @log, url: send("#{@log.model_name.param_key}_confirm_clear_answer_path", @log), method: "post", local: true do |f| %>
+      <% remove_duplicate_page_errors(@log) %>
       <%= f.govuk_error_summary %>
       <%= f.hidden_field :page_id, value: @page.id %>
 

app/views/users/edit.html.erb

@@ -24,7 +24,12 @@
 
       <%= f.govuk_phone_field :phone,
         label: { text: "Telephone number", size: "m" },
-        autocomplete: "phone",
+        autocomplete: "tel-national",
+        spellcheck: "false" %>
+
+      <%= f.govuk_phone_field :phone_extension,
+        label: { text: "Extension number (optional)", size: "m" },
+        autocomplete: "tel-extension",
         spellcheck: "false" %>
 
       <% if current_user.data_coordinator? || current_user.support? %>

app/views/users/new.html.erb

@@ -25,10 +25,16 @@
 
       <%= f.govuk_phone_field :phone,
         label: { text: "Telephone number", size: "m" },
-        autocomplete: "phone",
+        autocomplete: "tel-national",
         spellcheck: "false",
         value: @user.phone %>
 
+      <%= f.govuk_phone_field :phone_extension,
+        label: { text: "Extension number (optional)", size: "m" },
+        autocomplete: "tel-extension",
+        spellcheck: "false",
+        value: @user.phone_extension %>
+
       <% if current_user.support? %>
         <% null_option = [OpenStruct.new(id: "", name: "Select an option")] %>
         <% organisations = Organisation.filter_by_active.map { |org| OpenStruct.new(id: org.id, name: org.name) } %>

app/views/users/show.html.erb

@@ -43,7 +43,7 @@
 
       <%= summary_list.with_row do |row|
             row.with_key { "Telephone number" }
-            row.with_value { @user.phone }
+            row.with_value { @user.phone_with_extension }
             if UserPolicy.new(current_user, @user).edit_telephone_numbers?
               row.with_action(visually_hidden_text: "telephone number", href: aliased_user_edit(@user, current_user), html_attributes: { "data-qa": "change-telephone-number" })
             else

db/migrate/20240819143150_add_phone_extension_to_users.rb

@@ -0,0 +1,5 @@
+class AddPhoneExtensionToUsers < ActiveRecord::Migration[7.0]
+  def change
+    add_column :users, :phone_extension, :string
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_08_13_142947) do
+ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -800,6 +800,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_13_142947) do
     t.boolean "initial_confirmation_sent"
     t.boolean "reactivate_with_organisation"
     t.datetime "discarded_at"
+    t.string "phone_extension"
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["encrypted_otp_secret_key"], name: "index_users_on_encrypted_otp_secret_key", unique: true

docs/Gemfile.lock

@@ -226,7 +226,7 @@ GEM
     rb-fsevent (0.11.2)
     rb-inotify (0.10.1)
       ffi (~> 1.0)
-    rexml (3.3.3)
+    rexml (3.3.6)
       strscan
     rouge (3.26.0)
     ruby2_keywords (0.0.5)

lib/tasks/clear_unconfirmed_emails.rake

@@ -0,0 +1,4 @@
+desc "Clear unconfimed emails for deactivated users"
+task clear_unconfirmed_emails: :environment do
+  User.deactivated.where.not(unconfirmed_email: nil).update(unconfirmed_email: nil)
+end

spec/features/user_spec.rb

@@ -600,8 +600,8 @@ RSpec.describe "User Features" do
         visit(user_path(other_user))
       end
 
-      it "sends beta onboarding email to be sent when user is legacy" do
+      it "sends initial confirmable template email when user is legacy" do
-        expect(notify_client).to receive(:send_email).with(email_address: "new_user@example.com", template_id: User::BETA_ONBOARDING_TEMPLATE_ID, personalisation:).once
+        expect(notify_client).to receive(:send_email).with(email_address: "new_user@example.com", template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
         click_button("Resend invite link")
       end
     end

spec/helpers/form_page_error_helper_spec.rb

@@ -21,4 +21,22 @@ RSpec.describe FormPageErrorHelper do
       end
     end
   end
+
+  describe "#remove_duplicate_page_errors" do
+    context "when non base other questions are removed" do
+      let!(:lettings_log) { FactoryBot.create(:lettings_log, :in_progress) }
+
+      before do
+        lettings_log.errors.add :layear, "error"
+        lettings_log.errors.add :period, "error_one"
+        lettings_log.errors.add :base, "error_one"
+      end
+
+      it "returns details and user tabs" do
+        remove_duplicate_page_errors(lettings_log)
+        expect(lettings_log.errors.count).to eq(2)
+        expect(lettings_log.errors.map(&:message)).to match_array(%w[error_one error])
+      end
+    end
+  end
 end

spec/lib/tasks/clear_unconfirmed_emails_spec.rb

@@ -0,0 +1,36 @@
+require "rails_helper"
+require "rake"
+
+RSpec.describe "clear_unconfirmed_emails" do
+  describe ":clear_unconfirmed_emails", type: :task do
+    subject(:task) { Rake::Task["clear_unconfirmed_emails"] }
+
+    before do
+      Rake.application.rake_require("tasks/clear_unconfirmed_emails")
+      Rake::Task.define_task(:environment)
+      task.reenable
+    end
+
+    context "when the rake task is run" do
+      context "and there are deactivated users with unconfirmed emails" do
+        let!(:user) { create(:user, active: false, unconfirmed_email: "some_email@example.com") }
+
+        it "clears unconfirmed_email" do
+          task.invoke
+
+          expect(user.reload.unconfirmed_email).to eq(nil)
+        end
+      end
+
+      context "and there are active users with unconfirmed emails" do
+        let!(:user) { create(:user, active: true, unconfirmed_email: "some_email@example.com") }
+
+        it "does not clear unconfirmed_email" do
+          task.invoke
+
+          expect(user.reload.unconfirmed_email).not_to eq(nil)
+        end
+      end
+    end
+  end
+end

spec/mailers/resend_invitation_mailer_spec.rb

@@ -42,7 +42,7 @@ RSpec.describe ResendInvitationMailer do
 
       it "sends an initial invitation" do
         FactoryBot.create(:legacy_user, old_user_id: new_active_migrated_user.old_user_id, user: new_active_migrated_user)
-        expect(notify_client).to receive(:send_email).with(email_address: "new_active_migrated_user@example.com", template_id: User::BETA_ONBOARDING_TEMPLATE_ID, personalisation:).once
+        expect(notify_client).to receive(:send_email).with(email_address: "new_active_migrated_user@example.com", template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
         described_class.new.resend_invitation_email(new_active_migrated_user)
       end
     end

spec/models/validations/sales/financial_validations_spec.rb

@@ -338,7 +338,7 @@ RSpec.describe Validations::Sales::FinancialValidations do
   end
 
   describe "#validate_equity_in_range_for_year_and_type" do
-    let(:record) { FactoryBot.build(:sales_log, saledate:) }
+    let(:record) { FactoryBot.build(:sales_log, saledate:, resale: nil) }
 
     context "with a log in the 22/23 collection year" do
       let(:saledate) { Time.zone.local(2023, 1, 1) }
@@ -373,6 +373,14 @@ RSpec.describe Validations::Sales::FinancialValidations do
         expect(record.errors["equity"]).to include(match I18n.t("validations.financial.equity.over_max", max_equity: 75))
         expect(record.errors["type"]).to include(match I18n.t("validations.financial.equity.over_max", max_equity: 75))
       end
+
+      it "does not add an error if it's a resale" do
+        record.type = 2
+        record.equity = 90
+        record.resale = 1
+        financial_validator.validate_equity_in_range_for_year_and_type(record)
+        expect(record.errors).to be_empty
+      end
     end
 
     context "with a log in 23/24 collection year" do

spec/requests/form_controller_spec.rb

@@ -763,6 +763,7 @@ RSpec.describe FormController, type: :request do
             }
           end
 
+          context "when the log will not be a duplicate" do
             before do
               post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
             end
@@ -783,21 +784,143 @@ RSpec.describe FormController, type: :request do
               expect(whodunnit_actor).to be_a(User)
               expect(whodunnit_actor.id).to eq(user.id)
             end
+          end
 
-          context "and duplicate logs" do
+          context "when the answer makes the log a duplicate" do
-            let(:duplicate_logs) { create_list(:lettings_log, 2) }
+            context "with one other log" do
+              let(:new_duplicate) { create(:lettings_log) }
 
               before do
-              allow(LettingsLog).to receive(:duplicate_logs).and_return(duplicate_logs)
+                allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
                 post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
               end
 
+              it "sets a new duplicate set id on both logs" do
+                lettings_log.reload
+                new_duplicate.reload
+                expect(lettings_log.duplicate_set_id).not_to be_nil
+                expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+              end
+
               it "redirects to the duplicate logs page" do
                 expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
                 follow_redirect!
                 expect(page).to have_content("These logs are duplicates")
               end
             end
+
+            context "with a set of other logs" do
+              let(:duplicate_set_id) { 100 }
+              let(:new_duplicates) { create_list(:lettings_log, 2, duplicate_set_id:) }
+
+              before do
+                allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicates.pluck(:id)))
+                post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "sets the logs duplicate set id to that of the set it is now part of" do
+                lettings_log.reload
+                expect(lettings_log.duplicate_set_id).to eql(duplicate_set_id)
+                new_duplicates.each do |log|
+                  log.reload
+                  expect(log.duplicate_set_id).to eql(duplicate_set_id)
+                end
+              end
+
+              it "redirects to the duplicate logs page" do
+                expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
+                follow_redirect!
+                expect(page).to have_content("These logs are duplicates")
+              end
+            end
+
+            context "when the log was previously in a different duplicate set" do
+              context "with a single other log" do
+                let(:old_duplicate_set_id) { 110 }
+                let!(:old_duplicate) { create(:lettings_log, duplicate_set_id: old_duplicate_set_id) }
+                let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+                let(:new_duplicate) { create(:lettings_log) }
+
+                before do
+                  allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
+                  post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+                end
+
+                it "updates the relevant duplicate set ids" do
+                  lettings_log.reload
+                  old_duplicate.reload
+                  new_duplicate.reload
+                  expect(old_duplicate.duplicate_set_id).to be_nil
+                  expect(lettings_log.duplicate_set_id).not_to be_nil
+                  expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+                end
+              end
+
+              context "with multiple other logs" do
+                let(:old_duplicate_set_id) { 120 }
+                let!(:old_duplicates) { create_list(:lettings_log, 2, duplicate_set_id: old_duplicate_set_id) }
+                let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+                let(:new_duplicate) { create(:lettings_log) }
+
+                before do
+                  allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
+                  post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+                end
+
+                it "updates the relevant duplicate set ids" do
+                  lettings_log.reload
+                  new_duplicate.reload
+                  old_duplicates.each do |log|
+                    log.reload
+                    expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+                  end
+                  expect(lettings_log.duplicate_set_id).not_to be_nil
+                  expect(lettings_log.duplicate_set_id).not_to eql(old_duplicate_set_id)
+                  expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+                end
+              end
+            end
+          end
+
+          context "when the answer makes the log stop being a duplicate" do
+            context "when the log had one duplicate" do
+              let(:old_duplicate_set_id) { 130 }
+              let!(:old_duplicate) { create(:lettings_log, duplicate_set_id: old_duplicate_set_id) }
+              let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+
+              before do
+                allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none)
+                post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "updates the relevant duplicate set ids" do
+                lettings_log.reload
+                old_duplicate.reload
+                expect(old_duplicate.duplicate_set_id).to be_nil
+                expect(lettings_log.duplicate_set_id).to be_nil
+              end
+            end
+
+            context "when the log had multiple duplicates" do
+              let(:old_duplicate_set_id) { 140 }
+              let!(:old_duplicates) { create_list(:lettings_log, 2, duplicate_set_id: old_duplicate_set_id) }
+              let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+
+              before do
+                allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none)
+                post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "updates the relevant duplicate set ids" do
+                lettings_log.reload
+                old_duplicates.each do |log|
+                  log.reload
+                  expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+                end
+                expect(lettings_log.duplicate_set_id).to be_nil
+              end
+            end
+          end
         end
 
         context "with valid sales answers" do
@@ -816,13 +939,22 @@ RSpec.describe FormController, type: :request do
               },
             }
           end
+          let(:page_id) { "buyer_1_age" }
 
-          context "and duplicate logs" do
+          context "when the answer makes the log a duplicate" do
-            let!(:duplicate_logs) { create_list(:sales_log, 2) }
+            context "with one other log" do
+              let(:new_duplicate) { create(:sales_log) }
 
               before do
-              allow(SalesLog).to receive(:duplicate_logs).and_return(duplicate_logs)
+                allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id))
-              post "/sales-logs/#{sales_log.id}/buyer-1-age", params:
+                post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "sets a new duplicate set id on both logs" do
+                sales_log.reload
+                new_duplicate.reload
+                expect(sales_log.duplicate_set_id).not_to be_nil
+                expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
               end
 
               it "redirects to the duplicate logs page" do
@@ -831,6 +963,119 @@ RSpec.describe FormController, type: :request do
                 expect(page).to have_content("These logs are duplicates")
               end
             end
+
+            context "with a set of other logs" do
+              let(:duplicate_set_id) { 100 }
+              let(:new_duplicates) { create_list(:sales_log, 2, duplicate_set_id:) }
+
+              before do
+                allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicates.pluck(:id)))
+                post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "sets the logs duplicate set id to that of the set it is now part of" do
+                sales_log.reload
+                expect(sales_log.duplicate_set_id).to eql(duplicate_set_id)
+                new_duplicates.each do |log|
+                  log.reload
+                  expect(log.duplicate_set_id).to eql(duplicate_set_id)
+                end
+              end
+
+              it "redirects to the duplicate logs page" do
+                expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
+                follow_redirect!
+                expect(page).to have_content("These logs are duplicates")
+              end
+            end
+
+            context "when the log was previously in a different duplicate set" do
+              context "with a single other log" do
+                let(:old_duplicate_set_id) { 110 }
+                let!(:old_duplicate) { create(:sales_log, duplicate_set_id: old_duplicate_set_id) }
+                let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+                let(:new_duplicate) { create(:sales_log) }
+
+                before do
+                  allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id))
+                  post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+                end
+
+                it "updates the relevant duplicate set ids" do
+                  sales_log.reload
+                  old_duplicate.reload
+                  new_duplicate.reload
+                  expect(old_duplicate.duplicate_set_id).to be_nil
+                  expect(sales_log.duplicate_set_id).not_to be_nil
+                  expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+                end
+              end
+
+              context "with multiple other logs" do
+                let(:old_duplicate_set_id) { 120 }
+                let!(:old_duplicates) { create_list(:sales_log, 2, duplicate_set_id: old_duplicate_set_id) }
+                let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+                let(:new_duplicate) { create(:sales_log) }
+
+                before do
+                  allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id))
+                  post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+                end
+
+                it "updates the relevant duplicate set ids" do
+                  sales_log.reload
+                  new_duplicate.reload
+                  old_duplicates.each do |log|
+                    log.reload
+                    expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+                  end
+                  expect(sales_log.duplicate_set_id).not_to be_nil
+                  expect(sales_log.duplicate_set_id).not_to eql(old_duplicate_set_id)
+                  expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+                end
+              end
+            end
+          end
+
+          context "when the answer makes the log stop being a duplicate" do
+            context "when the log had one duplicate" do
+              let(:old_duplicate_set_id) { 130 }
+              let!(:old_duplicate) { create(:sales_log, duplicate_set_id: old_duplicate_set_id) }
+              let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+
+              before do
+                allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.none)
+                post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "updates the relevant duplicate set ids" do
+                sales_log.reload
+                old_duplicate.reload
+                expect(old_duplicate.duplicate_set_id).to be_nil
+                expect(sales_log.duplicate_set_id).to be_nil
+              end
+            end
+
+            context "when the log had multiple duplicates" do
+              let(:old_duplicate_set_id) { 140 }
+              let!(:old_duplicates) { create_list(:sales_log, 2, duplicate_set_id: old_duplicate_set_id) }
+              let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+
+              before do
+                allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.none)
+                post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "updates the relevant duplicate set ids" do
+                sales_log.reload
+                old_duplicates.each do |log|
+                  log.reload
+                  expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+                end
+                expect(sales_log.duplicate_set_id).to be_nil
+              end
+            end
+          end
         end
 
         context "when the question has a conditional question" do

spec/requests/organisations_controller_spec.rb

@@ -1688,7 +1688,7 @@ RSpec.describe OrganisationsController, type: :request do
           end
 
           it "sends invitation emails" do
-            expect(notify_client).to have_received(:send_email).with(email_address: user_to_reactivate.email, template_id: User::BETA_ONBOARDING_TEMPLATE_ID, personalisation: expected_personalisation).once
+            expect(notify_client).to have_received(:send_email).with(email_address: user_to_reactivate.email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation: expected_personalisation).once
           end
         end
       end

spec/requests/users_controller_spec.rb

@@ -434,7 +434,7 @@ RSpec.describe UsersController, type: :request do
 
   context "when user is signed in as a data coordinator" do
     let(:user) { create(:user, :data_coordinator, email: "coordinator@example.com", organisation: create(:organisation, :without_dpc)) }
-    let!(:other_user) { create(:user, organisation: user.organisation, name: "filter name", email: "filter@example.com") }
+    let!(:other_user) { create(:user, organisation: user.organisation, name: "filter name", email: "filter@example.com", unconfirmed_email: "email@something.com") }
 
     before do
       sign_in user
@@ -885,6 +885,11 @@ RSpec.describe UsersController, type: :request do
                 expect { patch "/users/#{other_user.id}", headers:, params: }
                   .to change { other_user.reload.active }.from(true).to(false)
               end
+
+              it "discards unconfirmed email" do
+                expect { patch "/users/#{other_user.id}", headers:, params: }
+                  .to change { other_user.reload.unconfirmed_email }.from("email@something.com").to(nil)
+              end
             end
 
             context "and tries to activate deactivated user" do
@@ -997,6 +1002,7 @@ RSpec.describe UsersController, type: :request do
             email: "new_user@example.com",
             role: "data_coordinator",
             phone: "12345678910",
+            phone_extension: "1234",
           },
         }
       end
@@ -1020,12 +1026,14 @@ RSpec.describe UsersController, type: :request do
         request
       end
 
-      it "creates a new scheme for user organisation with valid params" do
+      it "creates a new user for user organisation with valid params" do
         request
 
         expect(User.last.name).to eq("new user")
         expect(User.last.email).to eq("new_user@example.com")
         expect(User.last.role).to eq("data_coordinator")
+        expect(User.last.phone).to eq("12345678910")
+        expect(User.last.phone_extension).to eq("1234")
       end
 
       it "redirects back to organisation users page" do
@@ -1612,11 +1620,12 @@ RSpec.describe UsersController, type: :request do
           expect(page).to have_content("Change your personal details")
         end
 
-        it "has fields for name, email, role, dpo and key contact" do
+        it "has fields for name, email, role, phone number and phone extension" do
           expect(page).to have_field("user[name]")
           expect(page).to have_field("user[email]")
           expect(page).to have_field("user[role]")
           expect(page).to have_field("user[phone]")
+          expect(page).to have_field("user[phone_extension]")
         end
 
         it "allows setting the role to `support`" do
@@ -1638,10 +1647,12 @@ RSpec.describe UsersController, type: :request do
             expect(page).to have_content("Change #{other_user.name}’s personal details")
           end
 
-          it "has fields for name, email, role, dpo and key contact" do
+          it "has fields for name, email, role, phone number and phone extension" do
             expect(page).to have_field("user[name]")
             expect(page).to have_field("user[email]")
             expect(page).to have_field("user[role]")
+            expect(page).to have_field("user[phone]")
+            expect(page).to have_field("user[phone_extension]")
           end
         end
 
@@ -1656,10 +1667,12 @@ RSpec.describe UsersController, type: :request do
             expect(page).to have_content("Change #{other_user.name}’s personal details")
           end
 
-          it "has fields for name, email, role, dpo and key contact" do
+          it "has fields for name, email, role, phone number and phone extension" do
             expect(page).to have_field("user[name]")
             expect(page).to have_field("user[email]")
             expect(page).to have_field("user[role]")
+            expect(page).to have_field("user[phone]")
+            expect(page).to have_field("user[phone_extension]")
           end
         end
 
@@ -1989,6 +2002,7 @@ RSpec.describe UsersController, type: :request do
             email:,
             role: "data_coordinator",
             phone: "12345612456",
+            phone_extension: "1234",
             organisation_id: organisation.id,
           },
         }
@@ -2004,6 +2018,14 @@ RSpec.describe UsersController, type: :request do
         expect(User.find_by(email:).organisation).to eq(organisation)
       end
 
+      it "sets expected values on the user" do
+        request
+        user = User.find_by(email:)
+        expect(user.name).to eq("new user")
+        expect(user.phone).to eq("12345612456")
+        expect(user.phone_extension).to eq("1234")
+      end
+
       it "redirects back to users page" do
         request
         expect(response).to redirect_to("/users")