From 8e27396ecfdb6d2ea99cd3abf795402593c8d872 Mon Sep 17 00:00:00 2001 From: Alex Whitehead-Smith Date: Fri, 5 Nov 2021 11:51:12 +0000 Subject: [PATCH 1/2] Include account session flash in Vary header When a user subscribes or unsubscribes to a single page email notification topic, they'll be redirected back to the content page with a message in their account session flash. This message will be detected and used to display the relevant success message on the page. We need to set the vary header to indicate to Fastly that the success messages are different variations and should be cached and served separately. The `GovukPersonalisation::ControllerConcern` handles this for us but sets the header to vary on the full account session. This would result in pages being cached per-user which isn't very helpful in this case. Instead, override the `set_account_vary_header` method to vary on the flash message instead. The message content won't vary per user so this will mean we only cache three versions of the page - one without a flash message, one with the subscribed message and one with the unsubscribed message. --- Gemfile | 1 + Gemfile.lock | 1 + app/controllers/content_items_controller.rb | 8 ++++++++ test/controllers/content_items_controller_test.rb | 7 +++++++ 4 files changed, 17 insertions(+) diff --git a/Gemfile b/Gemfile index eec0e26c8..11f1437f5 100644 --- a/Gemfile +++ b/Gemfile @@ -6,6 +6,7 @@ gem "dalli" gem "gds-api-adapters" gem "govuk_ab_testing" gem "govuk_app_config" +gem "govuk_personalisation" gem "govuk_publishing_components" gem "htmlentities" gem "plek" diff --git a/Gemfile.lock b/Gemfile.lock index 76f21e776..166f4d985 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -419,6 +419,7 @@ DEPENDENCIES gds-api-adapters govuk_ab_testing govuk_app_config + govuk_personalisation govuk_publishing_components govuk_schemas govuk_test diff --git a/app/controllers/content_items_controller.rb b/app/controllers/content_items_controller.rb index 1e3bbc6d1..80a051f8d 100644 --- a/app/controllers/content_items_controller.rb +++ b/app/controllers/content_items_controller.rb @@ -1,4 +1,6 @@ class ContentItemsController < ApplicationController + include GovukPersonalisation::ControllerConcern + rescue_from GdsApi::HTTPForbidden, with: :error_403 rescue_from GdsApi::HTTPNotFound, with: :error_notfound rescue_from GdsApi::HTTPGone, with: :error_410 @@ -199,4 +201,10 @@ def error_redirect(exception) ) redirect_to destination, status: status_code end + + def set_account_vary_header + # Override the default from GovukPersonalisation::ControllerConcern so pages are cached on each flash message + # variation, rather than caching pages per user + response.headers["Vary"] = [response.headers["Vary"], "GOVUK-Account-Session-Flash"].compact.join(", ") + end end diff --git a/test/controllers/content_items_controller_test.rb b/test/controllers/content_items_controller_test.rb index 1261b4448..4abdb55dc 100644 --- a/test/controllers/content_items_controller_test.rb +++ b/test/controllers/content_items_controller_test.rb @@ -366,6 +366,13 @@ class ContentItemsControllerTest < ActionController::TestCase assert_equal response.headers["Access-Control-Allow-Origin"], "*" end + test "sets GOVUK-Account-Session-Flash in the Vary header" do + content_item = content_store_has_schema_example("case_study", "case_study") + get :show, params: { path: path_for(content_item) } + + assert response.headers["Vary"].include?("GOVUK-Account-Session-Flash") + end + def path_for(content_item, locale = nil) base_path = content_item["base_path"].sub(/^\//, "") base_path.gsub!(/\.#{locale}$/, "") if locale From 0c88d6f91a24193fe07d2a29e9bd147e3ee18739 Mon Sep 17 00:00:00 2001 From: Alex Whitehead-Smith Date: Fri, 5 Nov 2021 16:33:55 +0000 Subject: [PATCH 2/2] Add email subscribe/unsubscribe flash messages When a user completes the single page email subscribe or unsubscribe journey they will be redirected to the page they were on with a message in the account session flash. Check for these messages in the flash and render the appropriate success banner. The pages chosen for phase 1 and phase 2 of the rollout are either `publication` or `detailed_guide` content types, so only add the partial to these templates. In the future when we roll out to more pages we can add the partial to the rest of the content types. --- app/controllers/content_items_controller.rb | 2 +- .../content_items/detailed_guide.html.erb | 2 ++ app/views/content_items/publication.html.erb | 2 ++ ...email_subscribe_unsubscribe_flash.html.erb | 20 +++++++++++++++++++ config/locales/en.yml | 4 ++++ .../content_items_controller_test.rb | 16 +++++++++++++++ 6 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 app/views/shared/_email_subscribe_unsubscribe_flash.html.erb diff --git a/app/controllers/content_items_controller.rb b/app/controllers/content_items_controller.rb index 80a051f8d..8f93584eb 100644 --- a/app/controllers/content_items_controller.rb +++ b/app/controllers/content_items_controller.rb @@ -1,6 +1,6 @@ class ContentItemsController < ApplicationController include GovukPersonalisation::ControllerConcern - + rescue_from GdsApi::HTTPForbidden, with: :error_403 rescue_from GdsApi::HTTPNotFound, with: :error_notfound rescue_from GdsApi::HTTPGone, with: :error_410 diff --git a/app/views/content_items/detailed_guide.html.erb b/app/views/content_items/detailed_guide.html.erb index 60e46356a..26ff23ada 100644 --- a/app/views/content_items/detailed_guide.html.erb +++ b/app/views/content_items/detailed_guide.html.erb @@ -4,6 +4,8 @@ ) %> <% end %> +<%= render 'shared/email_subscribe_unsubscribe_flash' %> +
<% brexit_child_taxon = @content_item.brexit_child_taxon %> diff --git a/app/views/content_items/publication.html.erb b/app/views/content_items/publication.html.erb index 30917a961..cc3c181c4 100644 --- a/app/views/content_items/publication.html.erb +++ b/app/views/content_items/publication.html.erb @@ -4,6 +4,8 @@ ) %> <% end %> +<%= render 'shared/email_subscribe_unsubscribe_flash' %> +
<%= render 'context_and_title' %> diff --git a/app/views/shared/_email_subscribe_unsubscribe_flash.html.erb b/app/views/shared/_email_subscribe_unsubscribe_flash.html.erb new file mode 100644 index 000000000..1d9fbbe4d --- /dev/null +++ b/app/views/shared/_email_subscribe_unsubscribe_flash.html.erb @@ -0,0 +1,20 @@ +<% if @account_flash.include?("email-subscription-success") %> +
+
+ <%= render "govuk_publishing_components/components/success_alert", { + message: sanitize(t("email.subscribe_title")), + description: sanitize(t("email.description_html")) + } %> +
+
+ +<% elsif @account_flash.include?("email-unsubscribe-success") %> +
+
+ <%= render "govuk_publishing_components/components/success_alert", { + message: sanitize(t("email.unsubscribe_title")), + description: sanitize(t("email.description_html")) + } %> +
+
+<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 2041ea8f8..f66a83c2e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -319,6 +319,10 @@ en: publication_scheme_html: Read about the types of information we routinely publish in our %{link}. social_media_use_html: Read our policy on %{link}. welsh_language_scheme_html: Find out about our commitment to %{link}. + email: + subscribe_title: You’ve subscribed to emails about this page + unsubscribe_title: You’ve unsubscribed from emails about this page + description_html:

Go to your GOV.UK account to see and manage all your GOV.UK email subscriptions.

fatality_notice: alt_text: Ministry of Defence crest field_of_operation: Field of operation diff --git a/test/controllers/content_items_controller_test.rb b/test/controllers/content_items_controller_test.rb index 4abdb55dc..a68a230cf 100644 --- a/test/controllers/content_items_controller_test.rb +++ b/test/controllers/content_items_controller_test.rb @@ -373,6 +373,22 @@ class ContentItemsControllerTest < ActionController::TestCase assert response.headers["Vary"].include?("GOVUK-Account-Session-Flash") end + test "displays the subscription success banner when the 'email-subscription-success' flash is present" do + content_item = content_store_has_schema_example("publication", "publication") + + request.headers["GOVUK-Account-Session"] = GovukPersonalisation::Flash.encode_session("session-id", %w[email-subscription-success]) + get :show, params: { path: path_for(content_item) } + assert response.body.include?("subscribed to emails about this page") + end + + test "displays the unsubscribe success banner when the 'email-unsubscribe-success' flash is present" do + content_item = content_store_has_schema_example("publication", "publication") + + request.headers["GOVUK-Account-Session"] = GovukPersonalisation::Flash.encode_session("session-id", %w[email-unsubscribe-success]) + get :show, params: { path: path_for(content_item) } + assert response.body.include?("unsubscribed from emails about this page") + end + def path_for(content_item, locale = nil) base_path = content_item["base_path"].sub(/^\//, "") base_path.gsub!(/\.#{locale}$/, "") if locale