diff --git a/Gemfile.lock b/Gemfile.lock
index 7fdef3710..2f51d6a29 100644
--- a/Gemfile.lock
+++ b/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)
- et-orbi (~> 1, >= 1.2.7)
+ fugit (1.11.1)
+ et-orbi (~> 1, >= 1.2.11)
raabro (~> 1.4)
globalid (1.2.1)
activesupport (>= 6.1)
diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb
index d84f8642a..8a95464a7 100644
--- a/app/controllers/form_controller.rb
+++ b/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,24 +194,49 @@ private
params[@log.model_name.param_key]["interruption_page_referrer_type"].presence
end
- def successful_redirect_path(pages_to_check)
- if FeatureToggle.deduplication_flow_enabled?
- if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner")
- return correcting_duplicate_logs_redirect_path
+ def page_has_duplicate_check_question
+ @page.questions.any? { |q| @log.duplicate_check_question_ids.include?(q.id) }
+ end
+
+ 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)
+ 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
+ end
- dynamic_duplicates = @log.lettings? ? current_user.lettings_logs.duplicate_logs(@log) : current_user.sales_logs.duplicate_logs(@log)
- if dynamic_duplicates.any?
- saved_duplicates = @log.duplicates
- if saved_duplicates.none? || 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
+ 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("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
+ 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")
next_page_id = form.next_page_id(@page, @log, current_user)
next_page = form.get_page(next_page_id)
@@ -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 = "Log #{@log.id}"
changed_labels = {
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 2f7bb2bd6..c5c5241f9 100644
--- a/app/controllers/users_controller.rb
+++ b/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
diff --git a/app/helpers/form_page_error_helper.rb b/app/helpers/form_page_error_helper.rb
index 8a46accca..c2c13e0c6 100644
--- a/app/helpers/form_page_error_helper.rb
+++ b/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
diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb
index 10ab612cd..7bf963212 100644
--- a/app/models/lettings_log.rb
+++ b/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?
diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb
index 6ac2979c8..8312b8bff 100644
--- a/app/models/sales_log.rb
+++ b/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
diff --git a/app/models/user.rb b/app/models/user.rb
index c79ceb0d9..31956e54d 100644
--- a/app/models/user.rb
+++ b/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.
diff --git a/app/models/validations/sales/financial_validations.rb b/app/models/validations/sales/financial_validations.rb
index 3e1d3dfb8..9a119475f 100644
--- a/app/models/validations/sales/financial_validations.rb
+++ b/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
diff --git a/app/models/validations/sales/sale_information_validations.rb b/app/models/validations/sales/sale_information_validations.rb
index 42c6a6172..8bfa46783 100644
--- a/app/models/validations/sales/sale_information_validations.rb
+++ b/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)
diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb
index 810bfb845..91989ff86 100644
--- a/app/services/feature_toggle.rb
+++ b/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
diff --git a/app/views/form/check_errors.html.erb b/app/views/form/check_errors.html.erb
index 27fa3ea0c..bab4858a5 100644
--- a/app/views/form/check_errors.html.erb
+++ b/app/views/form/check_errors.html.erb
@@ -2,6 +2,7 @@
<%= 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 %>
diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb
index 3f7259bc3..6fdfdb5aa 100644
--- a/app/views/users/edit.html.erb
+++ b/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? %>
diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb
index 925f1ada7..f9f9b1e48 100644
--- a/app/views/users/new.html.erb
+++ b/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) } %>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index acb53d51d..df8c0e915 100644
--- a/app/views/users/show.html.erb
+++ b/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
diff --git a/db/migrate/20240819143150_add_phone_extension_to_users.rb b/db/migrate/20240819143150_add_phone_extension_to_users.rb
new file mode 100644
index 000000000..0dcd8d0c5
--- /dev/null
+++ b/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
diff --git a/db/schema.rb b/db/schema.rb
index 774e213e4..00159931b 100644
--- a/db/schema.rb
+++ b/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
diff --git a/docs/Gemfile.lock b/docs/Gemfile.lock
index 849732bc5..2a406a953 100644
--- a/docs/Gemfile.lock
+++ b/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)
diff --git a/lib/tasks/clear_unconfirmed_emails.rake b/lib/tasks/clear_unconfirmed_emails.rake
new file mode 100644
index 000000000..f53a0c893
--- /dev/null
+++ b/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
diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb
index c30abe1e9..119dbfa52 100644
--- a/spec/features/user_spec.rb
+++ b/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
- expect(notify_client).to receive(:send_email).with(email_address: "new_user@example.com", template_id: User::BETA_ONBOARDING_TEMPLATE_ID, personalisation:).once
+ 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::CONFIRMABLE_TEMPLATE_ID, personalisation:).once
click_button("Resend invite link")
end
end
diff --git a/spec/helpers/form_page_error_helper_spec.rb b/spec/helpers/form_page_error_helper_spec.rb
index 5f86e1648..067739e16 100644
--- a/spec/helpers/form_page_error_helper_spec.rb
+++ b/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
diff --git a/spec/lib/tasks/clear_unconfirmed_emails_spec.rb b/spec/lib/tasks/clear_unconfirmed_emails_spec.rb
new file mode 100644
index 000000000..0ff7a3315
--- /dev/null
+++ b/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
diff --git a/spec/mailers/resend_invitation_mailer_spec.rb b/spec/mailers/resend_invitation_mailer_spec.rb
index 2b76484c3..9dec88ace 100644
--- a/spec/mailers/resend_invitation_mailer_spec.rb
+++ b/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
diff --git a/spec/models/validations/sales/financial_validations_spec.rb b/spec/models/validations/sales/financial_validations_spec.rb
index dd71d0b24..d9f47d39a 100644
--- a/spec/models/validations/sales/financial_validations_spec.rb
+++ b/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
diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb
index 020dba3a4..80157e992 100644
--- a/spec/requests/form_controller_spec.rb
+++ b/spec/requests/form_controller_spec.rb
@@ -763,39 +763,162 @@ RSpec.describe FormController, type: :request do
}
end
- before do
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
- end
+ context "when the log will not be a duplicate" do
+ before do
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ end
- it "re-renders the same page with errors if validation fails" do
- expect(response).to have_http_status(:redirect)
- end
+ it "re-renders the same page with errors if validation fails" do
+ expect(response).to have_http_status(:redirect)
+ end
- it "only updates answers that apply to the page being submitted" do
- lettings_log.reload
- expect(lettings_log.age1).to eq(answer)
- expect(lettings_log.age2).to be nil
+ it "only updates answers that apply to the page being submitted" do
+ lettings_log.reload
+ expect(lettings_log.age1).to eq(answer)
+ expect(lettings_log.age2).to be nil
+ end
+
+ it "tracks who updated the record" do
+ lettings_log.reload
+ whodunnit_actor = lettings_log.versions.last.actor
+ expect(whodunnit_actor).to be_a(User)
+ expect(whodunnit_actor.id).to eq(user.id)
+ end
end
- it "tracks who updated the record" do
- lettings_log.reload
- whodunnit_actor = lettings_log.versions.last.actor
- expect(whodunnit_actor).to be_a(User)
- expect(whodunnit_actor.id).to eq(user.id)
+ context "when the answer makes the log a duplicate" do
+ context "with one other log" do
+ 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 "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 "and duplicate logs" do
- let(:duplicate_logs) { create_list(:lettings_log, 2) }
+ 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(duplicate_logs)
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ 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
- 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")
+ 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
@@ -816,19 +939,141 @@ RSpec.describe FormController, type: :request do
},
}
end
+ let(:page_id) { "buyer_1_age" }
- context "and duplicate logs" do
- let!(:duplicate_logs) { create_list(:sales_log, 2) }
+ context "when the answer makes the log a duplicate" do
+ context "with one other log" do
+ let(:new_duplicate) { create(:sales_log) }
- before do
- allow(SalesLog).to receive(:duplicate_logs).and_return(duplicate_logs)
- post "/sales-logs/#{sales_log.id}/buyer-1-age", params:
+ 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 "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
+ 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
- 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")
+ 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
diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb
index d3e8a8155..cb74ca3ab 100644
--- a/spec/requests/organisations_controller_spec.rb
+++ b/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
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index 8e87f7f28..dac2806a5 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/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")