From 5ff64e4478aeed2188a0be1121b57e2338db44cc Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 11 Oct 2024 14:51:26 +0100 Subject: [PATCH] Change user update success banner (#2663) * Change user update success banner * Display banner when unconfirmed email changes --- app/controllers/users_controller.rb | 10 +++--- config/locales/en.yml | 5 +-- spec/requests/users_controller_spec.rb | 48 +++++++++++++++++++------- 3 files changed, 45 insertions(+), 18 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 1197f1533..b7c323ca1 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -60,12 +60,14 @@ class UsersController < ApplicationController def update validate_attributes + unconfirmed_email_changed = @user.unconfirmed_email.present? && @user.unconfirmed_email != user_params[:email] && @user.email != user_params[:email] if @user.errors.empty? && @user.update(user_params_without_org) if @user == current_user bypass_sign_in @user flash[:notice] = I18n.t("devise.passwords.updated") if user_params.key?("password") - if user_params.key?("email") && user_params[:email] != @user.email - flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email) + + if @user.saved_changes? + flash[:notice] = I18n.t("notification.user_updated.self") end if updating_organisation? @@ -82,8 +84,8 @@ class UsersController < ApplicationController @user.reactivate! @user.send_confirmation_instructions flash[:notice] = I18n.t("devise.activation.reactivated", user_name:) - elsif user_params.key?("email") && user_params[:email] != @user.email - flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email) + elsif @user.saved_changes? || unconfirmed_email_changed + flash[:notice] = I18n.t("notification.user_updated.other", name: @user.name) end if updating_organisation? diff --git a/config/locales/en.yml b/config/locales/en.yml index 764b5d687..81d059269 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -216,6 +216,9 @@ en: scheme_deleted: "%{service_name} has been deleted." user_deleted: "%{name} has been deleted." organisation_deleted: "%{name} has been deleted." + user_updated: + self: "Your account details have been updated." + other: "%{name}’s details have been updated." validations: organisation: @@ -818,8 +821,6 @@ Make sure these answers are correct." title: "You told us there are more than 1 persons with 'Partner' relationship to buyer 1." devise: - email: - updated: "An email has been sent to %{email} to confirm this change." two_factor_authentication: success: "Two-factor authentication successful." attempt_failed: "Attempt failed." diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 05dc06d5b..1b62196bb 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1786,18 +1786,18 @@ RSpec.describe UsersController, type: :request do end it "shows flash notice" do - patch("/users/#{other_user.id}", headers:, params:) + request - expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.") + expect(flash[:notice]).to eq("Your account details have been updated.") end it "sends new flow emails" do expect(notify_client).to receive(:send_email).with( - email_address: other_user.email, + email_address: user.email, template_id: User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID, personalisation: { new_email:, - old_email: other_user.email, + old_email: user.email, }, ).once @@ -1806,14 +1806,14 @@ RSpec.describe UsersController, type: :request do template_id: User::FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID, personalisation: { new_email:, - old_email: other_user.email, + old_email: user.email, link: include("/account/confirmation?confirmation_token="), }, ).once expect(notify_client).not_to receive(:send_email) - patch "/users/#{other_user.id}", headers:, params: + request end context "when user has never confirmed email address" do @@ -1826,9 +1826,9 @@ RSpec.describe UsersController, type: :request do end it "shows flash notice" do - patch("/users/#{other_user.id}", headers:, params:) + request - expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.") + expect(flash[:notice]).to eq("Your account details have been updated.") end it "sends new flow emails" do @@ -1858,14 +1858,38 @@ RSpec.describe UsersController, type: :request do end end - context "and email address hasn't changed" do - let(:params) { { id: user.id, user: { name: new_name, email: other_user.email, is_dpo: "true", is_key_contact: "true" } } } + context "and no fields have changed" do + let(:params) { { id: user.id, user: { name: user.name, email: user.email, is_dpo: user.is_dpo, is_key_contact: user.is_key_contact } } } before do user.legacy_users.destroy_all end - it "shows flash notice" do + it "does not show flash notice" do + request + + expect(flash[:notice]).to be_nil + end + end + end + + context "when user changes phone number", :aggregate_failures do + let(:params) { { id: user.id, user: { phone: "123123123123" } } } + + it "shows flash notice" do + request + + expect(flash[:notice]).to eq("Your account details have been updated.") + end + + context "and phone numbr hasn't changed" do + let(:params) { { id: user.id, user: { phone: other_user.phone } } } + + before do + user.legacy_users.destroy_all + end + + it "does not show flash notice" do patch("/users/#{other_user.id}", headers:, params:) expect(flash[:notice]).to be_nil @@ -2030,7 +2054,7 @@ RSpec.describe UsersController, type: :request do it "shows flash notice" do patch("/users/#{other_user.id}", headers:, params:) - expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.") + expect(flash[:notice]).to eq("new name’s details have been updated.") end it "sends new flow emails" do