From 8e9c6fc04acc2f083cec0c66c8c81a5837e5b3df Mon Sep 17 00:00:00 2001 From: Chris Lowis Date: Tue, 26 Sep 2023 15:45:14 +0100 Subject: [PATCH 1/6] Include production environment in email subjects We've done some user-testing of our email subject lines and users generally preferred to see the environment name in all cases, even in the production environment. I've had to make a small change to the NoisyBatchInvitationTest as it was also partially testing the subject as well as the email from address. Changing the email from address is out of scope for this bit of work. --- app/mailers/mailer_helper.rb | 6 +----- test/models/noisy_batch_invitation_test.rb | 3 ++- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/app/mailers/mailer_helper.rb b/app/mailers/mailer_helper.rb index d925853fd..cecaf2986 100644 --- a/app/mailers/mailer_helper.rb +++ b/app/mailers/mailer_helper.rb @@ -4,11 +4,7 @@ def email_from end def app_name - if GovukEnvironment.production? - I18n.t("mailer.app_name.no_instance") - else - I18n.t("mailer.app_name.instance", instance_name: GovukEnvironment.name) - end + I18n.t("mailer.app_name.instance", instance_name: GovukEnvironment.name) end def email_from_address diff --git a/test/models/noisy_batch_invitation_test.rb b/test/models/noisy_batch_invitation_test.rb index 6223ef4df..4d5d97d4f 100644 --- a/test/models/noisy_batch_invitation_test.rb +++ b/test/models/noisy_batch_invitation_test.rb @@ -53,6 +53,7 @@ class NoisyBatchInvitationTest < ActionMailer::TestCase context "work correctly in production environment" do setup do GovukEnvironment.stubs(:production?).returns(true) + GovukEnvironment.stubs(:name).returns("production") user = create(:user, name: "Bob Loblaw") @batch_invitation = create(:batch_invitation, user:) @@ -61,7 +62,7 @@ class NoisyBatchInvitationTest < ActionMailer::TestCase end should "from address should not include the environment name" do - assert_match(/".* Signon" /, @email[:from].to_s) + assert_match(/".* Signon production" /, @email[:from].to_s) end end end From 0c1db342b2e93ad14f5a25d2ac6eec7db9729d3b Mon Sep 17 00:00:00 2001 From: Chris Lowis Date: Thu, 28 Sep 2023 16:06:21 +0100 Subject: [PATCH 2/6] Include "production" in UserMailer#account_name Our user testing indicated that users find it less confusing (particularly when they have production and integration signon accounts) to have the environment name in this text. --- app/mailers/user_mailer.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 9e376cc0c..92455b716 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -119,11 +119,7 @@ def unlock_time end def account_name - if GovukEnvironment.production? - "account" - else - "#{GovukEnvironment.name} account" - end + "#{GovukEnvironment.name} account" end def subject_for(key) From 38f4a973faccc01beb2ebca8bbf86b79941849da Mon Sep 17 00:00:00 2001 From: Chris Lowis Date: Wed, 27 Sep 2023 11:45:49 +0100 Subject: [PATCH 3/6] Update suspension reminder email content Trello: https://trello.com/c/0oTag1bi We've done some testing of this email and come up with this updated content. Notify, which we use to send emails, has limited markdown support[1]. I've used the heading markup for "How to stop suspension", and the `^` to call out the "please do not reply to this email". [1] https://www.notifications.service.gov.uk/using-notify/formatting --- .../user_mailer/suspension_reminder.text.erb | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/app/views/user_mailer/suspension_reminder.text.erb b/app/views/user_mailer/suspension_reminder.text.erb index 2046ae90a..fb9372e2b 100644 --- a/app/views/user_mailer/suspension_reminder.text.erb +++ b/app/views/user_mailer/suspension_reminder.text.erb @@ -1,19 +1,24 @@ -<%= t('department.name') %> Signon accounts are suspended after <%= User::SUSPENSION_THRESHOLD_PERIOD.inspect %> of inactivity. +Your <%= t('department.name') %> Signon <%= account_name %>, for <%= @user.email %> will be suspended <%= suspension_time %>. -Your <%= t('department.name') %> Signon <%= account_name %>, for <%= @user.email %>, will be suspended <%= suspension_time %>. After suspension you won't be able to access any <%= t('department.name') %> <%= account_name %> publishing applications. - -<% unless GovukEnvironment.production? %> -<%= account_name.humanize %> suspensions do not suspend production accounts or remove access to <%= t('department.name') %> production applications. +<% if GovukEnvironment.production? %> +If this happens you won’t be able to access any of your publishing apps from this account, including Whitehall and Publisher. +<% else %> +If this happens you won’t be able to do any training from this account. <% end %> -If you don't need the account you can ignore these emails. +## How to stop suspension -To stop your account suspension you will need to sign in: +^ Please do not reply to this email. +To stop your account being suspended, all you have to do is sign in: <%= new_user_session_url(protocol: 'https') %> -If you’ve forgotten your password, you can request a new one from the sign in screen. +<%= t('department.name') %> Signon accounts are automatically suspended after <%= User::SUSPENSION_THRESHOLD_PERIOD.inspect %> of inactivity. -Read our blog post about suspending unused <%= t('department.name') %> accounts: +<% unless GovukEnvironment.production? %> +Please note that Integration account suspensions do not suspend Production accounts. + +Find about more about the two kinds of accounts on the [How to Publish on GOV.UK page](https://www.gov.uk/guidance/how-to-publish-on-gov-uk/introduction-and-access-to-whitehall-publisher). +<% end %> -https://insidegovuk.blog.gov.uk/2014/08/21/suspending-unused-gov-uk-accounts +If you no longer need this account, please ignore these instructions. From 4e80e56c19074ef9f040cf8fd7bf60f09278f44e Mon Sep 17 00:00:00 2001 From: Chris Lowis Date: Wed, 27 Sep 2023 12:07:39 +0100 Subject: [PATCH 4/6] Update suspension notification email content Trello: https://trello.com/c/0oTag1bi We've done some testing of this email and come up with this updated content. Notify, which we use to send emails, has limited markdown support[1], notably it only supports two levels of heading. I've used the second-level heading markup for "How to reactive your account" and "If you're a managing editor or user manager". I've also used the the `^` to call out the "please do not reply to this email". [1] https://www.notifications.service.gov.uk/using-notify/formatting --- .../user_mailer/suspension_notification.text.erb | 16 +++++++--------- test/models/user_mailer_test.rb | 4 ---- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/app/views/user_mailer/suspension_notification.text.erb b/app/views/user_mailer/suspension_notification.text.erb index 823ab8da9..437509ab0 100644 --- a/app/views/user_mailer/suspension_notification.text.erb +++ b/app/views/user_mailer/suspension_notification.text.erb @@ -1,15 +1,13 @@ -<%= t('department.name') %> Signon accounts are suspended after <%= User::SUSPENSION_THRESHOLD_PERIOD.inspect %> of inactivity. +Your <%= t('department.name') %> Signon <%= account_name %> for <%= @user.email %> is now suspended. -Your <%= t('department.name') %> Signon <%= account_name %>, for <%= @user.email %>, has now been suspended and you won't be able to access any <%= t('department.name') %> <%= account_name %> publishing applications. +^ Please do not reply to this email. -<% unless GovukEnvironment.production? %> -<%= account_name.humanize %> suspensions do not suspend production accounts or remove access to <%= t('department.name') %> production applications. -<% end %> +## How to reactive your account -If you don't need the account you can ignore this email. +Contact your managing editor or user manager ‒ they can unsuspend your account. -If you do still need the account you will have to contact a managing <%= t('department.name') %> editor in your organisation (or your parent organisation). They can use the support form (<%= t('support.url') %>) to get help from the <%= t('department.name') %> team. +## If you’re a managing editor or user manager -Read our blog post about suspending unused <%= t('department.name') %> accounts: +Contact another managing editor or user manager and ask them to unsuspend you. -https://insidegovuk.blog.gov.uk/2014/08/21/suspending-unused-gov-uk-accounts +Please note that managing editors in an arm's length body may not have this authority. diff --git a/test/models/user_mailer_test.rb b/test/models/user_mailer_test.rb index c10fbcd4d..89cbc251c 100644 --- a/test/models/user_mailer_test.rb +++ b/test/models/user_mailer_test.rb @@ -133,10 +133,6 @@ def assert_support_present_in_text(link_text, email = @email) should "say 'suspended' in the body" do assert_body_includes "suspended" end - - should "include support links" do - assert_support_present_in_text "support form" - end end context "on a non-production Signon instance" do From 9f98e3ea85902091f2fa8cea961d822fadf0574f Mon Sep 17 00:00:00 2001 From: Chris Lowis Date: Wed, 27 Sep 2023 12:20:12 +0100 Subject: [PATCH 5/6] Update unlock instructions email content Trello: https://trello.com/c/0oTag1bi We've done some testing of this email and come up with this updated content. Notify, which we use to send emails, has limited markdown support[1], notably it only supports two levels of heading. I've used the second-level heading markup for "What happens now" and "If you're not a managing editor or user manager". [1] https://www.notifications.service.gov.uk/using-notify/formatting --- .../user_mailer/unlock_instructions.text.erb | 16 +++++++++++----- test/models/user_mailer_test.rb | 8 -------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/app/views/user_mailer/unlock_instructions.text.erb b/app/views/user_mailer/unlock_instructions.text.erb index ff23006d3..76e22f93c 100644 --- a/app/views/user_mailer/unlock_instructions.text.erb +++ b/app/views/user_mailer/unlock_instructions.text.erb @@ -1,7 +1,13 @@ -Your <%= t('department.name') %> Signon <%= account_name %>, for <%= @user.email %>, was locked at <%= locked_time %> because the wrong sign on details were entered too many times. +Your <%= t('department.name') %> Signon <%= account_name %>, for <%= @user.email %> has been locked. -<% unless GovukEnvironment.production? %> - <%= account_name.humanize %> locks do not lock production accounts. -<% end %> +This has happened because the wrong sign in details were entered too many times. -Your account will be unlocked at <%= unlock_time %>. If you need your account unlocked sooner, please contact a managing <%= t('department.name') %> editor in your organisation (or your parent organisation). They can use the support form (<%= t('support.url') %>) to get help from the <%= t('department.name') %> team. +## What happens now + +Your account will be unlocked at <%= unlock_time %> UK time. + +## If you need your account to be unlocked immediately + +Please contact a GOV.UK managing editor or user manager in your organisation (or parent organisation) for further assistance. + +Please note that managing editors in an arm’s length body may not have the authority to unlock. diff --git a/test/models/user_mailer_test.rb b/test/models/user_mailer_test.rb index 89cbc251c..5148f7ecf 100644 --- a/test/models/user_mailer_test.rb +++ b/test/models/user_mailer_test.rb @@ -166,17 +166,9 @@ def assert_support_present_in_text(link_text, email = @email) assert_body_includes "for user@example.com" end - should "state when the account was locked" do - assert_body_includes "was locked at #{@the_time.to_fs(:govuk_date)}" - end - should "state when the account will be unlocked" do assert_body_includes "Your account will be unlocked at #{(@the_time + 1.hour).to_fs(:govuk_date)}" end - - should "include correct support links" do - assert_support_present_in_text "support form" - end end context "emailing a user to notify that reset password is dis-allowed" do From f8021af34ed078810a29096a6105a935e3596357 Mon Sep 17 00:00:00 2001 From: Chris Lowis Date: Thu, 28 Sep 2023 16:27:00 +0100 Subject: [PATCH 6/6] Improve readability of UserMailer#unlock_time This matches the new content design. We've decided to replace the magic number `1.hour` in the test with the relevant variable `User.unlock_in` defined in `config/initializers/devise.rb` since this is what determines the unlock time and we don't want the two to get out of sync. --- app/mailers/user_mailer.rb | 6 +++++- test/models/user_mailer_test.rb | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 92455b716..5ff7c6250 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -115,7 +115,11 @@ def locked_time end def unlock_time - (@user.locked_at + 1.hour).to_fs(:govuk_date) + t = (@user.locked_at + User.unlock_in) + time_part = t.to_fs(:govuk_time) + date_part = t.to_date.to_fs(:govuk_date) + + "#{time_part} UK time on #{date_part}" end def account_name diff --git a/test/models/user_mailer_test.rb b/test/models/user_mailer_test.rb index 5148f7ecf..e59b6661d 100644 --- a/test/models/user_mailer_test.rb +++ b/test/models/user_mailer_test.rb @@ -156,7 +156,7 @@ def assert_support_present_in_text(link_text, email = @email) setup do GovukEnvironment.stubs(:production?).returns(false) GovukEnvironment.stubs(:name).returns("test") - @the_time = Time.zone.now + @the_time = Time.zone.parse("2023-10-31 02:00:00") user = User.new(name: "User", email: "user@example.com", locked_at: @the_time) @email = UserMailer.unlock_instructions(user, "afaketoken") end @@ -167,7 +167,7 @@ def assert_support_present_in_text(link_text, email = @email) end should "state when the account will be unlocked" do - assert_body_includes "Your account will be unlocked at #{(@the_time + 1.hour).to_fs(:govuk_date)}" + assert_body_includes "Your account will be unlocked at 3:00am UK time on 31 October 2023" end end