From 7112b1fa5359b7cac24eb108df6c611b522efb1a Mon Sep 17 00:00:00 2001 From: BeckaL Date: Tue, 11 Jul 2023 14:46:43 +0100 Subject: [PATCH 1/9] Add controller for editing email subscriptions for document collections This adds a basic edit and view method. --- ...llection_email_subscriptions_controller.rb | 16 ++++++++++++ .../edit.html.erb | 11 ++++++++ config/routes.rb | 1 + ...ion_email_subscriptions_controller_test.rb | 25 +++++++++++++++++++ 4 files changed, 53 insertions(+) create mode 100644 app/controllers/admin/document_collection_email_subscriptions_controller.rb create mode 100644 app/views/admin/document_collection_email_subscriptions/edit.html.erb create mode 100644 test/functional/admin/document_collection_email_subscriptions_controller_test.rb diff --git a/app/controllers/admin/document_collection_email_subscriptions_controller.rb b/app/controllers/admin/document_collection_email_subscriptions_controller.rb new file mode 100644 index 00000000000..1a3becc70d2 --- /dev/null +++ b/app/controllers/admin/document_collection_email_subscriptions_controller.rb @@ -0,0 +1,16 @@ +class Admin::DocumentCollectionEmailSubscriptionsController < Admin::BaseController + before_action :load_document_collection + before_action :authorise_user + + def edit; end + +private + + def load_document_collection + @collection = DocumentCollection.find(params[:document_collection_id]) + end + + def authorise_user + redirect_to edit_admin_document_collection_path(@collection) unless current_user.can_edit_email_overrides? + end +end diff --git a/app/views/admin/document_collection_email_subscriptions/edit.html.erb b/app/views/admin/document_collection_email_subscriptions/edit.html.erb new file mode 100644 index 00000000000..9092bff0d9d --- /dev/null +++ b/app/views/admin/document_collection_email_subscriptions/edit.html.erb @@ -0,0 +1,11 @@ +<% content_for :page_title, "Email notification settings" %> +<% content_for :title_margin_bottom, 8 %> +<% content_for :page_class, "document-collection-email-groups edit" %> + +
+
+

Email notifications

+ +
Choose the type of email updates users will get if they sign up for notifications.
+
+
diff --git a/config/routes.rb b/config/routes.rb index 90b4cba27ea..1461f877790 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -47,6 +47,7 @@ def redirect(path, options = { prefix: Whitehall.router_prefix }) put :order, on: :collection end end + get "email-subscriptions" => "document_collection_email_subscriptions#edit", as: :edit_email_subscription post "whitehall-member" => "document_collection_group_memberships#create_whitehall_member", as: :new_whitehall_member end diff --git a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb new file mode 100644 index 00000000000..aeb65a3ea40 --- /dev/null +++ b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb @@ -0,0 +1,25 @@ +require "test_helper" + +class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionController::TestCase + setup do + @collection = create(:document_collection, :with_group) + @user_with_permission = create(:writer, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) + @user_without_permission = create(:writer) + login_as @user_without_permission + end + + should_be_an_admin_controller + + view_test "GET #edit renders successfully when the user has the relevant permission" do + login_as @user_with_permission + get :edit, params: { document_collection_id: @collection.id } + assert_response :ok + assert_select "div", /Choose the type of email updates users will get if they sign up for notifications./ + end + + test "GET #edit redirects to the edit page when the user does not have permission" do + login_as @user_without_permission + get :edit, params: { document_collection_id: @collection.id } + assert_redirected_to admin_document_collection_path(@collection) + end +end From 9c5a0946eca7e947153964fec34468dde6d66e67 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Wed, 2 Aug 2023 15:38:06 +0100 Subject: [PATCH 2/9] Add Email Notifications to document collections tab This will only display when the user has the relevant permission. --- ...llection_email_subscriptions_controller.rb | 1 + app/helpers/admin/editions_helper.rb | 4 +++ app/helpers/admin/tabbed_nav_helper.rb | 12 ++++++- .../edit.html.erb | 5 ++- ...document-collection-email-override.feature | 10 ++++++ ...ocument_collection_email_override_steps.rb | 16 +++++++++ .../app/helpers/admin/editions_helper_test.rb | 18 ++++++++++ .../helpers/admin/tabbed_nav_helper_test.rb | 33 +++++++++++++++++++ 8 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 features/document-collection-email-override.feature create mode 100644 features/step_definitions/document_collection_email_override_steps.rb diff --git a/app/controllers/admin/document_collection_email_subscriptions_controller.rb b/app/controllers/admin/document_collection_email_subscriptions_controller.rb index 1a3becc70d2..40b14a3f50a 100644 --- a/app/controllers/admin/document_collection_email_subscriptions_controller.rb +++ b/app/controllers/admin/document_collection_email_subscriptions_controller.rb @@ -1,6 +1,7 @@ class Admin::DocumentCollectionEmailSubscriptionsController < Admin::BaseController before_action :load_document_collection before_action :authorise_user + layout "design_system" def edit; end diff --git a/app/helpers/admin/editions_helper.rb b/app/helpers/admin/editions_helper.rb index 129229e7330..a3758a16f29 100644 --- a/app/helpers/admin/editions_helper.rb +++ b/app/helpers/admin/editions_helper.rb @@ -200,6 +200,10 @@ def default_edition_tabs(edition) if edition.is_a?(DocumentCollection) && !edition.new_record? tabs["Collection documents"] = admin_document_collection_groups_path(edition) end + + if edition.is_a?(DocumentCollection) && current_user.can_edit_email_overrides? + tabs["Email notifications"] = admin_document_collection_edit_email_subscription_path(edition) + end end end diff --git a/app/helpers/admin/tabbed_nav_helper.rb b/app/helpers/admin/tabbed_nav_helper.rb index 96d72d25561..0b48e893106 100644 --- a/app/helpers/admin/tabbed_nav_helper.rb +++ b/app/helpers/admin/tabbed_nav_helper.rb @@ -78,11 +78,21 @@ def consultation_nav_items(edition, current_path) end def document_collection_nav_items(edition, current_path) - { + collection_documents_element = { label: "Collections", href: admin_document_collection_groups_path(edition), current: current_path == admin_document_collection_groups_path(edition), } + email_notifications_element = { + label: "Email notifications", + href: admin_document_collection_edit_email_subscription_path(edition), + current: current_path == admin_document_collection_edit_email_subscription_path(edition), + } + if current_user.can_edit_email_overrides? + [collection_documents_element, email_notifications_element] + else + [collection_documents_element] + end end def document_collection_group_nav_items(group, current_path) diff --git a/app/views/admin/document_collection_email_subscriptions/edit.html.erb b/app/views/admin/document_collection_email_subscriptions/edit.html.erb index 9092bff0d9d..7aca230a31c 100644 --- a/app/views/admin/document_collection_email_subscriptions/edit.html.erb +++ b/app/views/admin/document_collection_email_subscriptions/edit.html.erb @@ -5,7 +5,10 @@

Email notifications

- + <%= render "components/secondary_navigation", { + aria_label: "Document navigation tabs", + items: secondary_navigation_tabs_items(@collection, request.path), + } %>
Choose the type of email updates users will get if they sign up for notifications.
diff --git a/features/document-collection-email-override.feature b/features/document-collection-email-override.feature new file mode 100644 index 00000000000..ce431d5470a --- /dev/null +++ b/features/document-collection-email-override.feature @@ -0,0 +1,10 @@ +Feature: Setting the taxonomy topic email override for a document collection + + Scenario: Setting the email override. + Given I am a user with email override editor permissions. + And a draft document collection published by my organisation exists. + When I visit the edit document collection page + Then I see the tab "Email notifications" + + + diff --git a/features/step_definitions/document_collection_email_override_steps.rb b/features/step_definitions/document_collection_email_override_steps.rb new file mode 100644 index 00000000000..61abeb65ba2 --- /dev/null +++ b/features/step_definitions/document_collection_email_override_steps.rb @@ -0,0 +1,16 @@ +Given(/^I am a user with email override editor permissions/) do + @user = build(:user, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR], organisation_slug: "gds") + login_as(@user) +end + +And(/^a draft document collection published by my organisation exists/) do + @document_collection = create(:draft_document_collection) +end + +When(/^I visit the edit document collection page/) do + visit edit_admin_document_collection_path(@document_collection) +end + +Then(/^I see the tab "Email notifications/) do + expect(page).to have_content("Email notifications") +end diff --git a/test/unit/app/helpers/admin/editions_helper_test.rb b/test/unit/app/helpers/admin/editions_helper_test.rb index dd32b0347ab..5168901da00 100644 --- a/test/unit/app/helpers/admin/editions_helper_test.rb +++ b/test/unit/app/helpers/admin/editions_helper_test.rb @@ -5,6 +5,14 @@ def govspeak_embedded_contacts(*_args) [] end + def current_user + @user + end + + setup do + @user = create(:user) + end + test "warn_about_lack_of_contacts_in_body? says no if the edition is not a news article" do (Edition.descendants - [NewsArticle] - NewsArticle.descendants).each do |not_a_news_article| assert_not warn_about_lack_of_contacts_in_body?(not_a_news_article.new) @@ -34,6 +42,16 @@ def govspeak_embedded_contacts(*_args) assert_includes default_edition_tabs(document_collection).keys, "Collection documents" end + test "default_edition_tabs includes email notifications tab when the user has the appropriate permission" do + document_collection = create(:document_collection) + + @user = create(:user) + assert_not_includes default_edition_tabs(document_collection).keys, "Email notifications" + + @user = create(:user, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) + assert_includes default_edition_tabs(document_collection).keys, "Email notifications" + end + test "#admin_author_filter_options excludes disabled users" do current_user, _another_user = *create_list(:user, 2) disabled_user = create(:disabled_user) diff --git a/test/unit/app/helpers/admin/tabbed_nav_helper_test.rb b/test/unit/app/helpers/admin/tabbed_nav_helper_test.rb index 95a5fdca541..1d5428beb7c 100644 --- a/test/unit/app/helpers/admin/tabbed_nav_helper_test.rb +++ b/test/unit/app/helpers/admin/tabbed_nav_helper_test.rb @@ -4,6 +4,14 @@ class Admin::TabbedNavHelperTest < ActionView::TestCase include Rails.application.routes.url_helpers include Admin::EditionRoutesHelper + def current_user + @user + end + + setup do + @user = create(:user) + end + test "#secondary_navigation_tabs_items for persisted consultations with no attachments" do consultation = build_stubbed(:consultation) @@ -127,6 +135,31 @@ class Admin::TabbedNavHelperTest < ActionView::TestCase assert_equal expected_output, secondary_navigation_tabs_items(document_collection, admin_document_collection_groups_path(document_collection)) end + test "#secondary_navigation_tabs_items contains email notifications tab when the current user has the relevant permission" do + document_collection = build_stubbed(:document_collection) + @user = create(:user, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) + + expected_output = [ + { + label: "Document", + href: edit_admin_edition_path(document_collection), + current: false, + }, + { + label: "Collections", + href: admin_document_collection_groups_path(document_collection), + current: true, + }, + { + label: "Email notifications", + href: admin_document_collection_edit_email_subscription_path(document_collection), + current: false, + }, + ] + + assert_equal expected_output, secondary_navigation_tabs_items(document_collection, admin_document_collection_groups_path(document_collection)) + end + test "#secondary_navigation_tabs_items for persisted editions which do not allow attachments" do %i[case_study fatality_notice speech].each do |type| edition = build_stubbed(type) From 1ee8b82b764ba9cbf7ff152f000f91179a52c939 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Wed, 2 Aug 2023 16:30:55 +0100 Subject: [PATCH 3/9] Create the document collection email subscription templates - This commit contains a placeholder implementation of the taxonomy topic select, using hardcoded values. It will be replaced in a subsequent commit. --- .../_form.html.erb | 36 +++++++++++++++++++ .../_taxonomy_choice.html.erb | 27 ++++++++++++++ .../edit.html.erb | 7 ++-- ...document-collection-email-override.feature | 2 +- ...ocument_collection_email_override_steps.rb | 5 ++- 5 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 app/views/admin/document_collection_email_subscriptions/_form.html.erb create mode 100644 app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb diff --git a/app/views/admin/document_collection_email_subscriptions/_form.html.erb b/app/views/admin/document_collection_email_subscriptions/_form.html.erb new file mode 100644 index 00000000000..8f2d6786f26 --- /dev/null +++ b/app/views/admin/document_collection_email_subscriptions/_form.html.erb @@ -0,0 +1,36 @@ +<%= form_for [:admin, @collection], url: root_path, method: "put" do |form| %> + <%= render "govuk_publishing_components/components/radio", { + name: "override_email_subscriptions", + id: "email_override_email_subscription_choice", + items: [ + { + value: false, + text: "Emails about this page", + hint_text: "Users will get an email when the document collection or any of the content listed on it is updated.", + checked: true, + bold: true, + }, + { + value: true, + text: "Emails about the topic", + bold: true, + hint_text: "Users will get an email when any content related to the topic is updated. You choose the topic from the topic taxonomy.", + checked: false, + conditional: render("taxonomy_choice"), + }, + ], + } %> + +
+ <%= render "govuk_publishing_components/components/button", { + text: "Save", + data_attributes: { + module: "gem-track-click", + "track-category": "form-button", + "track-action": "update-email-subscriptions-button", + "track-label": "Save", + }, + } %> + <%= link_to("Cancel", root_path, class: "govuk-link govuk-link--no-visited-state") %> +
+<% end %> diff --git a/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb b/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb new file mode 100644 index 00000000000..7aecf1a686d --- /dev/null +++ b/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb @@ -0,0 +1,27 @@ +
+

Choose the topic

+ # TODO: This is a temporary implementation of a topic taxonomy select using hardcoded values. + <%= render "govuk_publishing_components/components/select", { + id: "selected_taxon_content_id", + label: "Choose a topic from the topic taxonomy. You can only choose one.", + options: [ + { text: "", value: "" }, + { text: "Topic One", value: "9b889c60-2191-11ee-be56-0242ac120002" }, + { text: "Topic Two", value: "c60-2191-11ee-be56-0242ac120002" }, + ], + } %> +
+ <%= render "govuk_publishing_components/components/checkboxes", { + id: "email_override_confirmation", + name: "email_override_confirmation", + heading_size: "s", + heading: "You cannot change the email notification settings after the collection is published.", + no_hint_text: true, + items: [ + { + label: "Select this box to confirm you're happy with what you've selected.", + value: true, + }, + ], + } %> +
diff --git a/app/views/admin/document_collection_email_subscriptions/edit.html.erb b/app/views/admin/document_collection_email_subscriptions/edit.html.erb index 7aca230a31c..58dce746740 100644 --- a/app/views/admin/document_collection_email_subscriptions/edit.html.erb +++ b/app/views/admin/document_collection_email_subscriptions/edit.html.erb @@ -4,11 +4,14 @@
-

Email notifications

<%= render "components/secondary_navigation", { aria_label: "Document navigation tabs", items: secondary_navigation_tabs_items(@collection, request.path), } %> -
Choose the type of email updates users will get if they sign up for notifications.
+ +

Email notifications

+

Choose the type of email updates users will get if they sign up for notifications.

+ + <%= render partial: "form" %>
diff --git a/features/document-collection-email-override.feature b/features/document-collection-email-override.feature index ce431d5470a..3941c7dd008 100644 --- a/features/document-collection-email-override.feature +++ b/features/document-collection-email-override.feature @@ -4,7 +4,7 @@ Feature: Setting the taxonomy topic email override for a document collection Given I am a user with email override editor permissions. And a draft document collection published by my organisation exists. When I visit the edit document collection page - Then I see the tab "Email notifications" + Then I click on the tab "Email notifications" diff --git a/features/step_definitions/document_collection_email_override_steps.rb b/features/step_definitions/document_collection_email_override_steps.rb index 61abeb65ba2..bf77aad2d79 100644 --- a/features/step_definitions/document_collection_email_override_steps.rb +++ b/features/step_definitions/document_collection_email_override_steps.rb @@ -11,6 +11,9 @@ visit edit_admin_document_collection_path(@document_collection) end -Then(/^I see the tab "Email notifications/) do +Then(/^I click on the tab "Email notifications/) do expect(page).to have_content("Email notifications") + click_on("Email notifications") + expect(page).to have_content("\nEmails about this page\n") + expect(page).to have_content("\nEmails about the topic\n") end From c0b734d5d697996e92d47eeee3deabfbddb0031d Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Wed, 2 Aug 2023 16:31:54 +0100 Subject: [PATCH 4/9] Add PUT route for updating document collection email override When setting an email override field, a user must select a topic, and check a confirmation box. If they do both of those things, they will be redirected to the document collection edit page. It will only be possible to set and email override on a draft document collection. This restriction will be added in subsequent commit. --- ...llection_email_subscriptions_controller.rb | 22 ++++++ .../_form.html.erb | 4 +- config/routes.rb | 1 + ...document-collection-email-override.feature | 5 ++ ...ocument_collection_email_override_steps.rb | 20 +++++ ...ion_email_subscriptions_controller_test.rb | 61 +++++++++++++++- ...document_collection_email_override_test.rb | 73 +++++++++++++++++++ 7 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 test/integration/document_collection_email_override_test.rb diff --git a/app/controllers/admin/document_collection_email_subscriptions_controller.rb b/app/controllers/admin/document_collection_email_subscriptions_controller.rb index 40b14a3f50a..19eb4ffcf81 100644 --- a/app/controllers/admin/document_collection_email_subscriptions_controller.rb +++ b/app/controllers/admin/document_collection_email_subscriptions_controller.rb @@ -5,6 +5,24 @@ class Admin::DocumentCollectionEmailSubscriptionsController < Admin::BaseControl def edit; end + def update + if user_has_selected_taxonomy_topic_emails? + if params[:selected_taxon_content_id].blank? + flash["selected_taxon_content_id"] = "You must choose a topic" + return redirect_to admin_document_collection_edit_email_subscription_path(@collection) + elsif params[:email_override_confirmation].blank? + flash["email_override_confirmation"] = "You must confirm you’re happy with the email notification settings" + return redirect_to admin_document_collection_edit_email_subscription_path(@collection) + else + @collection.update!(taxonomy_topic_email_override: params[:selected_taxon_content_id]) + end + else + @collection.update!(taxonomy_topic_email_override: nil) + end + flash["notice"] = "You’ve selected the email notification settings. You cannot change these settings after the collection is published", + redirect_to edit_admin_document_collection_path(@collection) + end + private def load_document_collection @@ -14,4 +32,8 @@ def load_document_collection def authorise_user redirect_to edit_admin_document_collection_path(@collection) unless current_user.can_edit_email_overrides? end + + def user_has_selected_taxonomy_topic_emails? + params[:override_email_subscriptions] == "true" + end end diff --git a/app/views/admin/document_collection_email_subscriptions/_form.html.erb b/app/views/admin/document_collection_email_subscriptions/_form.html.erb index 8f2d6786f26..e403e8885a5 100644 --- a/app/views/admin/document_collection_email_subscriptions/_form.html.erb +++ b/app/views/admin/document_collection_email_subscriptions/_form.html.erb @@ -1,4 +1,4 @@ -<%= form_for [:admin, @collection], url: root_path, method: "put" do |form| %> +<%= form_for [:admin, @collection], url: admin_document_collection_update_email_subscription_path(@collection), method: "put" do |form| %> <%= render "govuk_publishing_components/components/radio", { name: "override_email_subscriptions", id: "email_override_email_subscription_choice", @@ -31,6 +31,6 @@ "track-label": "Save", }, } %> - <%= link_to("Cancel", root_path, class: "govuk-link govuk-link--no-visited-state") %> + <%= link_to("Cancel", admin_document_collection_path(@collection), class: "govuk-link govuk-link--no-visited-state") %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 1461f877790..f6e29fa89f2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -48,6 +48,7 @@ def redirect(path, options = { prefix: Whitehall.router_prefix }) end end get "email-subscriptions" => "document_collection_email_subscriptions#edit", as: :edit_email_subscription + put "email-subscriptions" => "document_collection_email_subscriptions#update", as: :update_email_subscription post "whitehall-member" => "document_collection_group_memberships#create_whitehall_member", as: :new_whitehall_member end diff --git a/features/document-collection-email-override.feature b/features/document-collection-email-override.feature index 3941c7dd008..3f2d7ee5e9e 100644 --- a/features/document-collection-email-override.feature +++ b/features/document-collection-email-override.feature @@ -5,6 +5,11 @@ Feature: Setting the taxonomy topic email override for a document collection And a draft document collection published by my organisation exists. When I visit the edit document collection page Then I click on the tab "Email notifications" + And I choose "Emails about this topic" + And I select "Topic One" + And I click the checkbox to confirm my selection. + And I click "Save" + Then I am redirected to the document collection edit page. diff --git a/features/step_definitions/document_collection_email_override_steps.rb b/features/step_definitions/document_collection_email_override_steps.rb index bf77aad2d79..b351db6b5ac 100644 --- a/features/step_definitions/document_collection_email_override_steps.rb +++ b/features/step_definitions/document_collection_email_override_steps.rb @@ -17,3 +17,23 @@ expect(page).to have_content("\nEmails about this page\n") expect(page).to have_content("\nEmails about the topic\n") end + +And(/^I choose "Emails about this topic"/) do + page.choose(name: "override_email_subscriptions", option: "true") +end + +And(/^I select "([^"]*)"$/) do |topic_label| + select topic_label, from: "selected_taxon_content_id" +end + +And(/^I click the checkbox to confirm my selection./) do + check("email_override_confirmation-0") +end + +And(/^I click "Save"/) do + click_on("Save") +end + +Then(/^I am redirected to the document collection edit page/) do + assert_current_path edit_admin_document_collection_path(@document_collection) +end diff --git a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb index aeb65a3ea40..9a152072c95 100644 --- a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb +++ b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb @@ -5,6 +5,13 @@ class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionControll @collection = create(:document_collection, :with_group) @user_with_permission = create(:writer, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) @user_without_permission = create(:writer) + @selected_taxon_content_id = "9b889c60-2191-11ee-be56-0242ac120002" + @put_params = { + document_collection_id: @collection.id, + override_email_subscriptions: "true", + selected_taxon_content_id: @selected_taxon_content_id, + email_override_confirmation: "true", + } login_as @user_without_permission end @@ -20,6 +27,58 @@ class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionControll test "GET #edit redirects to the edit page when the user does not have permission" do login_as @user_without_permission get :edit, params: { document_collection_id: @collection.id } - assert_redirected_to admin_document_collection_path(@collection) + assert_redirected_to edit_admin_document_collection_path(@collection) + end + + test "PUT #edit successfully updates a document collection when the user has permission" do + login_as @user_with_permission + put :update, params: @put_params + @collection.reload + + assert_equal @collection.taxonomy_topic_email_override, @selected_taxon_content_id + assert_redirected_to edit_admin_document_collection_path(@collection) + end + + test "PUT #edit does not update a document collection when the user does not have permission" do + login_as @user_without_permission + put :update, params: @put_params + @collection.reload + + assert_nil @collection.taxonomy_topic_email_override + assert_redirected_to edit_admin_document_collection_path(@collection) + end + + test "PUT #edit does not update a document collection when the confirmation field is not present" do + login_as @user_with_permission + put :update, params: @put_params.reject { |k| k == :email_override_confirmation } + @collection.reload + + assert_nil @collection.taxonomy_topic_email_override + assert_redirected_to admin_document_collection_edit_email_subscription_path(@collection) + end + + test "PUT #edit does not update a document collection when the selected_taxon_content_id field is not present" do + login_as @user_with_permission + put :update, params: @put_params.reject { |k| k == :selected_taxon_content_id } + @collection.reload + + assert_nil @collection.taxonomy_topic_email_override + assert_redirected_to admin_document_collection_edit_email_subscription_path(@collection) + end + + test "PUT #edit successfully updates taxonomy topic override of a draft document collection" do + login_as @user_with_permission + collection = create(:draft_document_collection, taxonomy_topic_email_override: @selected_taxon_content_id) + + params = { + document_collection_id: collection.id, + override_email_subscriptions: "false", + } + + put(:update, params:) + collection.reload + + assert_nil collection.taxonomy_topic_email_override + assert_redirected_to edit_admin_document_collection_path(collection) end end diff --git a/test/integration/document_collection_email_override_test.rb b/test/integration/document_collection_email_override_test.rb new file mode 100644 index 00000000000..ad1f35dde7f --- /dev/null +++ b/test/integration/document_collection_email_override_test.rb @@ -0,0 +1,73 @@ +require "test_helper" +require "capybara/rails" + +class DocumentCollectionEmailOverrideTest < ActionDispatch::IntegrationTest + extend Minitest::Spec::DSL + include Capybara::DSL + include Rails.application.routes.url_helpers + include TaxonomyHelper + + describe "adding an email override" do + let(:document_collection) { create(:document_collection) } + + context "as a user with the email override editor permission" do + let(:user_with_permission_to_override) { create(:writer, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) } + + before do + login_as(user_with_permission_to_override) + end + + it "updates the taxonomy topic email override" do + visit edit_admin_document_collection_path(document_collection) + click_link "Email notifications" + + page.choose("Emails about the topic") + select "Topic One", from: "selected_taxon_content_id" + page.check("Select this box to confirm you're happy with what you've selected.") + click_button("Save") + document_collection.reload + assert_equal document_collection.taxonomy_topic_email_override, "9b889c60-2191-11ee-be56-0242ac120002" + end + + it "does not update taxonomy topic email if confirmation button is unchecked" do + visit edit_admin_document_collection_path(document_collection) + click_link "Email notifications" + + page.choose("Emails about the topic") + select "Topic One", from: "selected_taxon_content_id" + click_button("Save") + document_collection.reload + assert_nil document_collection.taxonomy_topic_email_override + end + + it "does not update taxonomy topic email if topic is not selected" do + visit edit_admin_document_collection_path(document_collection) + click_link "Email notifications" + + page.choose("Emails about the topic") + click_button("Save") + document_collection.reload + assert_nil document_collection.taxonomy_topic_email_override + end + end + + context "as a user without the email override editor permission" do + let(:user_without_permission_to_override) { create(:writer) } + + before do + login_as(user_without_permission_to_override) + end + + it "the tab to edit the taxonomy topic email override is not visible" do + visit edit_admin_document_collection_path(document_collection) + assert page.has_no_link?("Email notifications") + end + + it "visiting the edit email url directly redirects the user with a permission error" do + visit admin_document_collection_edit_email_subscription_path(document_collection) + + assert_equal page.current_path, edit_admin_document_collection_path(document_collection) + end + end + end +end From 29f1aa1d99248e13952b069da2fb82d81c5a42c6 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Thu, 3 Aug 2023 18:51:40 +0100 Subject: [PATCH 5/9] Show error and confirmation messages to the user --- ...llection_email_subscriptions_controller.rb | 36 ++++++++++++++++--- app/helpers/errors_helper.rb | 11 ++++++ .../_taxonomy_choice.html.erb | 3 ++ .../edit.html.erb | 8 +++++ ...document-collection-email-override.feature | 13 ++++++- ...ocument_collection_email_override_steps.rb | 12 +++++-- ...ion_email_subscriptions_controller_test.rb | 1 + ...document_collection_email_override_test.rb | 6 ++-- test/unit/app/helpers/errors_helper_test.rb | 17 +++++++++ 9 files changed, 97 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/document_collection_email_subscriptions_controller.rb b/app/controllers/admin/document_collection_email_subscriptions_controller.rb index 19eb4ffcf81..bb425dcf3dd 100644 --- a/app/controllers/admin/document_collection_email_subscriptions_controller.rb +++ b/app/controllers/admin/document_collection_email_subscriptions_controller.rb @@ -8,10 +8,10 @@ def edit; end def update if user_has_selected_taxonomy_topic_emails? if params[:selected_taxon_content_id].blank? - flash["selected_taxon_content_id"] = "You must choose a topic" + build_flash("selected_taxon_content_id") return redirect_to admin_document_collection_edit_email_subscription_path(@collection) - elsif params[:email_override_confirmation].blank? - flash["email_override_confirmation"] = "You must confirm you’re happy with the email notification settings" + elsif params["email_override_confirmation"].blank? + build_flash("email_override_confirmation") return redirect_to admin_document_collection_edit_email_subscription_path(@collection) else @collection.update!(taxonomy_topic_email_override: params[:selected_taxon_content_id]) @@ -19,7 +19,7 @@ def update else @collection.update!(taxonomy_topic_email_override: nil) end - flash["notice"] = "You’ve selected the email notification settings. You cannot change these settings after the collection is published", + build_flash("notice") redirect_to edit_admin_document_collection_path(@collection) end @@ -33,7 +33,35 @@ def authorise_user redirect_to edit_admin_document_collection_path(@collection) unless current_user.can_edit_email_overrides? end + def build_flash(key) + flash[key] = { + "selected_taxon_content_id" => "You must choose a topic", + "email_override_confirmation" => "You must confirm you’re happy with the email notification settings", + "notice" => "You’ve selected the email notification settings. #{confirmation_message}. You will not be able to change these settings after you publish the collection.", + }[key] + end + + def confirmation_message + if @collection.taxonomy_topic_email_override.present? + "You’ve chosen ‘Emails about the topic’ and the topic #{taxonomy_topic_email_override_title}" + else + "You’ve chosen ‘Emails about the page’" + end + end + def user_has_selected_taxonomy_topic_emails? params[:override_email_subscriptions] == "true" end + + def taxonomy_topic_email_override_title + taxonomy_topic_content_item.fetch("title", "") + end + + def taxonomy_topic_content_item + @taxonomy_topic_content_item ||= Services.publishing_api + .get_content(@collection.taxonomy_topic_email_override) + .to_h + rescue GdsApi::HTTPNotFound + {} + end end diff --git a/app/helpers/errors_helper.rb b/app/helpers/errors_helper.rb index 5483db8da95..f838406f364 100644 --- a/app/helpers/errors_helper.rb +++ b/app/helpers/errors_helper.rb @@ -24,4 +24,15 @@ def errors_for(errors, attribute) } .presence end + + def errors_from_flash(flash) + return nil if flash.blank? + + flash.map do |array| + { + href: "##{array.first}", + text: array.last, + } + end + end end diff --git a/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb b/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb index 7aecf1a686d..b207dda1b42 100644 --- a/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb +++ b/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb @@ -4,6 +4,8 @@ <%= render "govuk_publishing_components/components/select", { id: "selected_taxon_content_id", label: "Choose a topic from the topic taxonomy. You can only choose one.", + error_message: flash["selected_taxon_content_id"], + error_id: flash["selected_taxon_content_id"], options: [ { text: "", value: "" }, { text: "Topic One", value: "9b889c60-2191-11ee-be56-0242ac120002" }, @@ -17,6 +19,7 @@ heading_size: "s", heading: "You cannot change the email notification settings after the collection is published.", no_hint_text: true, + error: flash["email_override_confirmation"], items: [ { label: "Select this box to confirm you're happy with what you've selected.", diff --git a/app/views/admin/document_collection_email_subscriptions/edit.html.erb b/app/views/admin/document_collection_email_subscriptions/edit.html.erb index 58dce746740..c043ff5b0aa 100644 --- a/app/views/admin/document_collection_email_subscriptions/edit.html.erb +++ b/app/views/admin/document_collection_email_subscriptions/edit.html.erb @@ -1,7 +1,15 @@ <% content_for :page_title, "Email notification settings" %> <% content_for :title_margin_bottom, 8 %> <% content_for :page_class, "document-collection-email-groups edit" %> +<% errors = errors_from_flash(flash) %> + <% if errors.present? %> + <%= render "govuk_publishing_components/components/error_summary", { + id: "error-summary", + title: "There are some problems", + items: errors, + } %> +<% end %>
<%= render "components/secondary_navigation", { diff --git a/features/document-collection-email-override.feature b/features/document-collection-email-override.feature index 3f2d7ee5e9e..5de8c89aab9 100644 --- a/features/document-collection-email-override.feature +++ b/features/document-collection-email-override.feature @@ -9,7 +9,18 @@ Feature: Setting the taxonomy topic email override for a document collection And I select "Topic One" And I click the checkbox to confirm my selection. And I click "Save" - Then I am redirected to the document collection edit page. + Then I see the success message "You’ve selected the email notification settings. You’ve chosen ‘Emails about the topic’ and the topic Topic One. You will not be able to change these settings after you publish the collection." + + + Scenario: User cannot set the email override without checking the confirmation box. + Given I am a user with email override editor permissions. + And a draft document collection published by my organisation exists. + When I visit the edit document collection page + Then I click on the tab "Email notifications" + And I choose "Emails about this topic" + And I select "Topic One" + And I click "Save" + Then I see the error "You must confirm you’re happy with the email notification settings" prompting me to confirm my selection. diff --git a/features/step_definitions/document_collection_email_override_steps.rb b/features/step_definitions/document_collection_email_override_steps.rb index b351db6b5ac..cdb1fe3ebc7 100644 --- a/features/step_definitions/document_collection_email_override_steps.rb +++ b/features/step_definitions/document_collection_email_override_steps.rb @@ -14,8 +14,7 @@ Then(/^I click on the tab "Email notifications/) do expect(page).to have_content("Email notifications") click_on("Email notifications") - expect(page).to have_content("\nEmails about this page\n") - expect(page).to have_content("\nEmails about the topic\n") + expect(page).to have_content("Choose the type of email updates users will get if they sign up for notifications.") end And(/^I choose "Emails about this topic"/) do @@ -31,9 +30,16 @@ end And(/^I click "Save"/) do + stub_request(:get, %r{\A#{Plek.find('publishing-api')}/v2/content}) + .to_return(body: { base_path: "/topic-one", content_id: "9b889c60-2191-11ee-be56-0242ac120002", title: "Topic One" }.to_json) click_on("Save") end -Then(/^I am redirected to the document collection edit page/) do +Then(/^I see the success message "([^"]*)"$/) do |message| assert_current_path edit_admin_document_collection_path(@document_collection) + expect(page).to have_content(message) +end + +Then(/^I see the error "([^"]*)" prompting me to confirm my selection./) do |error_message| + expect(page).to have_content(error_message) end diff --git a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb index 9a152072c95..f9f46ef669e 100644 --- a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb +++ b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb @@ -13,6 +13,7 @@ class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionControll email_override_confirmation: "true", } login_as @user_without_permission + stub_publishing_api_has_item(content_id: "9b889c60-2191-11ee-be56-0242ac120002", title: "Topic One") end should_be_an_admin_controller diff --git a/test/integration/document_collection_email_override_test.rb b/test/integration/document_collection_email_override_test.rb index ad1f35dde7f..5074bdffcdb 100644 --- a/test/integration/document_collection_email_override_test.rb +++ b/test/integration/document_collection_email_override_test.rb @@ -12,12 +12,14 @@ class DocumentCollectionEmailOverrideTest < ActionDispatch::IntegrationTest context "as a user with the email override editor permission" do let(:user_with_permission_to_override) { create(:writer, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) } - + let(:taxon_content_id) { "9b889c60-2191-11ee-be56-0242ac120002" } before do login_as(user_with_permission_to_override) end it "updates the taxonomy topic email override" do + stub_publishing_api_has_item(content_id: taxon_content_id, title: "Topic one") + visit edit_admin_document_collection_path(document_collection) click_link "Email notifications" @@ -26,7 +28,7 @@ class DocumentCollectionEmailOverrideTest < ActionDispatch::IntegrationTest page.check("Select this box to confirm you're happy with what you've selected.") click_button("Save") document_collection.reload - assert_equal document_collection.taxonomy_topic_email_override, "9b889c60-2191-11ee-be56-0242ac120002" + assert_equal document_collection.taxonomy_topic_email_override, taxon_content_id end it "does not update taxonomy topic email if confirmation button is unchecked" do diff --git a/test/unit/app/helpers/errors_helper_test.rb b/test/unit/app/helpers/errors_helper_test.rb index b4500dbcc43..693753821d7 100644 --- a/test/unit/app/helpers/errors_helper_test.rb +++ b/test/unit/app/helpers/errors_helper_test.rb @@ -7,6 +7,7 @@ class ErrorsHelperTest < ActionView::TestCase @object_with_unrelated_errors = ErrorTestObject.new("title", nil) @object_with_errors.validate @object_with_unrelated_errors.validate + @flash = ActionDispatch::Flash::FlashHash.new end test "#errors_for_input returns nil when there are no error messages" do @@ -41,6 +42,22 @@ class ErrorsHelperTest < ActionView::TestCase assert_nil errors_for(@object_with_unrelated_errors.errors, :title) end + test "#errors_from_flash returns nil if flash has no errors" do + assert_nil errors_from_flash(@flash) + end + + test "#errors_from_flash returns formatted items" do + @flash[:my_attribute] = "My message" + + expected_output = [ + { + href: "#my_attribute", + text: "My message", + }, + ] + assert_equal expected_output, errors_from_flash(@flash) + end + class ErrorTestObject include ActiveModel::Model attr_accessor :title, :date From ce2e00a49c3a7807ea894da56c997df5e7aa8c2e Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Tue, 14 Nov 2023 20:48:49 +0000 Subject: [PATCH 6/9] Handle validation errors Attempting to set an email override field on a document that has already been published will raise a validation error. Update the controller to handle this exception. This error is not shown to the user as a flash, because in the next commit, we make it impossible to access the document collection email subscription edit forms for published document collections. --- ...ent_collection_email_subscriptions_controller.rb | 2 ++ ...ollection_email_subscriptions_controller_test.rb | 13 ++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/document_collection_email_subscriptions_controller.rb b/app/controllers/admin/document_collection_email_subscriptions_controller.rb index bb425dcf3dd..fc4bde22d82 100644 --- a/app/controllers/admin/document_collection_email_subscriptions_controller.rb +++ b/app/controllers/admin/document_collection_email_subscriptions_controller.rb @@ -21,6 +21,8 @@ def update end build_flash("notice") redirect_to edit_admin_document_collection_path(@collection) + rescue ActiveRecord::RecordInvalid + redirect_to edit_admin_document_collection_path(@collection) end private diff --git a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb index f9f46ef669e..853c122d9de 100644 --- a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb +++ b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb @@ -2,7 +2,7 @@ class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionController::TestCase setup do - @collection = create(:document_collection, :with_group) + @collection = create(:draft_document_collection, :with_group) @user_with_permission = create(:writer, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) @user_without_permission = create(:writer) @selected_taxon_content_id = "9b889c60-2191-11ee-be56-0242ac120002" @@ -67,6 +67,17 @@ class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionControll assert_redirected_to admin_document_collection_edit_email_subscription_path(@collection) end + test "PUT #edit does not update taxonomy topic override of a published document collection" do + login_as @user_with_permission + collection = create(:published_document_collection, taxonomy_topic_email_override: "a-uuid") + + put :update, params: @put_params.merge(document_collection_id: collection.id) + assert_redirected_to edit_admin_document_collection_path(collection) + + collection.reload + assert_equal "a-uuid", collection.taxonomy_topic_email_override + end + test "PUT #edit successfully updates taxonomy topic override of a draft document collection" do login_as @user_with_permission collection = create(:draft_document_collection, taxonomy_topic_email_override: @selected_taxon_content_id) From c9798e09c6f90d6446d86daa5f3aa85d4bc0a885 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Tue, 14 Nov 2023 21:56:43 +0000 Subject: [PATCH 7/9] When a collection is published, show summary page When a document collection is published, we will not allow the email override field to be edited. Users clicking the email notifications tab for a published document collection will see a summary, instead of the form. --- ...llection_email_subscriptions_controller.rb | 15 ++--------- ...cument_collection_email_override_helper.rb | 25 +++++++++++++++++++ .../_email_override_summary_page.html.erb | 23 +++++++++++++++++ .../edit.html.erb | 8 ++++-- ...document_collection_email_override_test.rb | 17 ++++++++++++- 5 files changed, 72 insertions(+), 16 deletions(-) create mode 100644 app/helpers/admin/document_collection_email_override_helper.rb create mode 100644 app/views/admin/document_collection_email_subscriptions/_email_override_summary_page.html.erb diff --git a/app/controllers/admin/document_collection_email_subscriptions_controller.rb b/app/controllers/admin/document_collection_email_subscriptions_controller.rb index fc4bde22d82..a797d8efc0b 100644 --- a/app/controllers/admin/document_collection_email_subscriptions_controller.rb +++ b/app/controllers/admin/document_collection_email_subscriptions_controller.rb @@ -1,4 +1,5 @@ class Admin::DocumentCollectionEmailSubscriptionsController < Admin::BaseController + include Admin::DocumentCollectionEmailOverrideHelper before_action :load_document_collection before_action :authorise_user layout "design_system" @@ -45,7 +46,7 @@ def build_flash(key) def confirmation_message if @collection.taxonomy_topic_email_override.present? - "You’ve chosen ‘Emails about the topic’ and the topic #{taxonomy_topic_email_override_title}" + "You’ve chosen ‘Emails about the topic’ and the topic #{taxonomy_topic_email_override_title(@collection)}" else "You’ve chosen ‘Emails about the page’" end @@ -54,16 +55,4 @@ def confirmation_message def user_has_selected_taxonomy_topic_emails? params[:override_email_subscriptions] == "true" end - - def taxonomy_topic_email_override_title - taxonomy_topic_content_item.fetch("title", "") - end - - def taxonomy_topic_content_item - @taxonomy_topic_content_item ||= Services.publishing_api - .get_content(@collection.taxonomy_topic_email_override) - .to_h - rescue GdsApi::HTTPNotFound - {} - end end diff --git a/app/helpers/admin/document_collection_email_override_helper.rb b/app/helpers/admin/document_collection_email_override_helper.rb new file mode 100644 index 00000000000..20329e11234 --- /dev/null +++ b/app/helpers/admin/document_collection_email_override_helper.rb @@ -0,0 +1,25 @@ +module Admin::DocumentCollectionEmailOverrideHelper + def taxonomy_topic_cannot_be_set?(collection) + collection.document.live_edition_id.present? + end + + def has_page_level_notifications?(collection) + collection.taxonomy_topic_email_override.nil? + end + + def taxonomy_topic_email_override_title(collection) + taxonomy_topic_content_item(collection).fetch("title", "") + end + + def taxonomy_topic_email_override_base_path(collection) + taxonomy_topic_content_item(collection).fetch("base_path", "") + end + + def taxonomy_topic_content_item(collection) + @taxonomy_topic_content_item ||= Services.publishing_api + .get_content(collection.taxonomy_topic_email_override) + .to_h + rescue GdsApi::HTTPNotFound + {} + end +end diff --git a/app/views/admin/document_collection_email_subscriptions/_email_override_summary_page.html.erb b/app/views/admin/document_collection_email_subscriptions/_email_override_summary_page.html.erb new file mode 100644 index 00000000000..211b356fe94 --- /dev/null +++ b/app/views/admin/document_collection_email_subscriptions/_email_override_summary_page.html.erb @@ -0,0 +1,23 @@ +

+ You cannot change the email notifications for this document collection. + If you have any questions about this, contact the managing editor at HMRC. +

+

Users who sign up to notifications on this document collection get an email alert when:

+<% if has_page_level_notifications?(@collection) %> + <%= render "govuk_publishing_components/components/list", { + visible_counters: true, + items: [ + "there’s a major change to any content that’s listed on the document collection (the content items in the ‘Collection documents’ tab)", + "there’s a major change to the document collection itself", + "the page is unpublished", + ], + } %> +<% else %> + <%= render "govuk_publishing_components/components/list", { + visible_counters: true, + items: [ + "there’s a major change to any content tagged to the topic #{link_to taxonomy_topic_email_override_title(@collection), taxonomy_topic_email_override_base_path(@collection)}", + "the page is unpublished", + ], + } %> +<% end %> diff --git a/app/views/admin/document_collection_email_subscriptions/edit.html.erb b/app/views/admin/document_collection_email_subscriptions/edit.html.erb index c043ff5b0aa..9eef637e4d0 100644 --- a/app/views/admin/document_collection_email_subscriptions/edit.html.erb +++ b/app/views/admin/document_collection_email_subscriptions/edit.html.erb @@ -18,8 +18,12 @@ } %>

Email notifications

-

Choose the type of email updates users will get if they sign up for notifications.

- <%= render partial: "form" %> + <% if taxonomy_topic_cannot_be_set?(@collection) %> + <%= render partial: "email_override_summary_page" %> + <% else %> +

Choose the type of email updates users will get if they sign up for notifications.

+ <%= render partial: "form" %> + <% end %>
diff --git a/test/integration/document_collection_email_override_test.rb b/test/integration/document_collection_email_override_test.rb index 5074bdffcdb..6735e15997a 100644 --- a/test/integration/document_collection_email_override_test.rb +++ b/test/integration/document_collection_email_override_test.rb @@ -8,7 +8,7 @@ class DocumentCollectionEmailOverrideTest < ActionDispatch::IntegrationTest include TaxonomyHelper describe "adding an email override" do - let(:document_collection) { create(:document_collection) } + let(:document_collection) { create(:draft_document_collection) } context "as a user with the email override editor permission" do let(:user_with_permission_to_override) { create(:writer, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) } @@ -51,6 +51,21 @@ class DocumentCollectionEmailOverrideTest < ActionDispatch::IntegrationTest document_collection.reload assert_nil document_collection.taxonomy_topic_email_override end + + it "shows the user a summary page if the document collection is in an unmodifiable state" do + published_collection = create(:published_document_collection) + taxons = { "title" => "Foo", "base_path" => "/foo", "content_id" => "123asd", "phase" => "live" } + links = { "meets_user_needs" => %w[123] } + stub_publishing_api_expanded_links_with_taxons(published_collection.content_id, [taxons]) + stub_publishing_api_has_links({ content_id: published_collection.content_id, links: }) + + visit edit_admin_document_collection_path(published_collection) + click_button "Create new edition" + click_link "Email notifications" + + assert page.has_no_field?("override_email_subscriptions") + assert page.has_content?("You cannot change the email notifications for this document collection.") + end end context "as a user without the email override editor permission" do From b64b34604e02142cbb53c4164ad4059cb88600d1 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Wed, 15 Nov 2023 22:46:25 +0000 Subject: [PATCH 8/9] Retain previous selection of radio buttons and checkboxes If editing a draft document collection that already has a taxonomy_topic_email value assigned, select the relevant radio button and topic from the drop down when the page loads. If a user doesn't sumbit all the required values, and presses save they are redirected back, and shown an error. We should still retain their previous selections if any were made. --- ...llection_email_subscriptions_controller.rb | 35 +++++++++++++++---- ...cument_collection_email_override_helper.rb | 4 +++ .../_form.html.erb | 4 +-- ...ion_email_subscriptions_controller_test.rb | 9 +++-- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/app/controllers/admin/document_collection_email_subscriptions_controller.rb b/app/controllers/admin/document_collection_email_subscriptions_controller.rb index a797d8efc0b..286b25fced7 100644 --- a/app/controllers/admin/document_collection_email_subscriptions_controller.rb +++ b/app/controllers/admin/document_collection_email_subscriptions_controller.rb @@ -8,14 +8,11 @@ def edit; end def update if user_has_selected_taxonomy_topic_emails? - if params[:selected_taxon_content_id].blank? - build_flash("selected_taxon_content_id") - return redirect_to admin_document_collection_edit_email_subscription_path(@collection) - elsif params["email_override_confirmation"].blank? - build_flash("email_override_confirmation") - return redirect_to admin_document_collection_edit_email_subscription_path(@collection) + if all_required_params_present? + @collection.update!(taxonomy_topic_email_override: params["selected_taxon_content_id"]) else - @collection.update!(taxonomy_topic_email_override: params[:selected_taxon_content_id]) + build_missing_params_flash + return redirect_to form_with_stored_params end else @collection.update!(taxonomy_topic_email_override: nil) @@ -28,6 +25,30 @@ def update private + def all_required_params_present? + required_params.select { |key| params[key].present? } == required_params + end + + def required_params + %w[selected_taxon_content_id email_override_confirmation] + end + + def form_with_stored_params + admin_document_collection_edit_email_subscription_path(@collection, params_to_store_state_of_failed_form_submission) + end + + def build_missing_params_flash + missing_params = required_params.select { |required_param| params[required_param].blank? } + missing_params.each { |key| build_flash(key) } + end + + def params_to_store_state_of_failed_form_submission + { + "selected_taxon_content_id" => params["selected_taxon_content_id"], + "override_email_subscriptions" => params["override_email_subscriptions"], + } + end + def load_document_collection @collection = DocumentCollection.find(params[:document_collection_id]) end diff --git a/app/helpers/admin/document_collection_email_override_helper.rb b/app/helpers/admin/document_collection_email_override_helper.rb index 20329e11234..e8fca4c100a 100644 --- a/app/helpers/admin/document_collection_email_override_helper.rb +++ b/app/helpers/admin/document_collection_email_override_helper.rb @@ -22,4 +22,8 @@ def taxonomy_topic_content_item(collection) rescue GdsApi::HTTPNotFound {} end + + def emails_about_this_topic_checked?(collection, params) + collection.taxonomy_topic_email_override.present? || (params["override_email_subscriptions"] == "true") + end end diff --git a/app/views/admin/document_collection_email_subscriptions/_form.html.erb b/app/views/admin/document_collection_email_subscriptions/_form.html.erb index e403e8885a5..9313b5f5df9 100644 --- a/app/views/admin/document_collection_email_subscriptions/_form.html.erb +++ b/app/views/admin/document_collection_email_subscriptions/_form.html.erb @@ -7,7 +7,7 @@ value: false, text: "Emails about this page", hint_text: "Users will get an email when the document collection or any of the content listed on it is updated.", - checked: true, + checked: has_page_level_notifications?(@collection), bold: true, }, { @@ -15,7 +15,7 @@ text: "Emails about the topic", bold: true, hint_text: "Users will get an email when any content related to the topic is updated. You choose the topic from the topic taxonomy.", - checked: false, + checked: emails_about_this_topic_checked?(@collection, params), conditional: render("taxonomy_choice"), }, ], diff --git a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb index 853c122d9de..aa629fdda67 100644 --- a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb +++ b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb @@ -54,8 +54,10 @@ class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionControll put :update, params: @put_params.reject { |k| k == :email_override_confirmation } @collection.reload + selected = { override_email_subscriptions: "true", selected_taxon_content_id: @selected_taxon_content_id } + assert_nil @collection.taxonomy_topic_email_override - assert_redirected_to admin_document_collection_edit_email_subscription_path(@collection) + assert_redirected_to admin_document_collection_edit_email_subscription_path(@collection, selected) end test "PUT #edit does not update a document collection when the selected_taxon_content_id field is not present" do @@ -63,14 +65,15 @@ class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionControll put :update, params: @put_params.reject { |k| k == :selected_taxon_content_id } @collection.reload + selected = { override_email_subscriptions: "true" } + assert_nil @collection.taxonomy_topic_email_override - assert_redirected_to admin_document_collection_edit_email_subscription_path(@collection) + assert_redirected_to admin_document_collection_edit_email_subscription_path(@collection, selected) end test "PUT #edit does not update taxonomy topic override of a published document collection" do login_as @user_with_permission collection = create(:published_document_collection, taxonomy_topic_email_override: "a-uuid") - put :update, params: @put_params.merge(document_collection_id: collection.id) assert_redirected_to edit_admin_document_collection_path(collection) From 88a6f9ceaecc155ec81915b819af48be46fb1160 Mon Sep 17 00:00:00 2001 From: Jessica Jones Date: Wed, 15 Nov 2023 22:49:23 +0000 Subject: [PATCH 9/9] Build out the topic list select - the select-with-search component already exists in this application, and supports grouped items i.e. multiple things in a single row of the select. --- ...llection_email_subscriptions_controller.rb | 4 +- app/presenters/topic_list_select_presenter.rb | 84 +++++++++++++++++++ .../_taxonomy_choice.html.erb | 20 ++--- ...document-collection-email-override.feature | 6 +- ...ocument_collection_email_override_steps.rb | 5 +- lib/taxonomy/topic_taxonomy.rb | 8 +- ...ion_email_subscriptions_controller_test.rb | 6 +- ...document_collection_email_override_test.rb | 13 ++- .../topic_list_select_presenter_test.rb | 55 ++++++++++++ 9 files changed, 171 insertions(+), 30 deletions(-) create mode 100644 app/presenters/topic_list_select_presenter.rb create mode 100644 test/unit/app/presenters/topic_list_select_presenter_test.rb diff --git a/app/controllers/admin/document_collection_email_subscriptions_controller.rb b/app/controllers/admin/document_collection_email_subscriptions_controller.rb index 286b25fced7..bec39919f4c 100644 --- a/app/controllers/admin/document_collection_email_subscriptions_controller.rb +++ b/app/controllers/admin/document_collection_email_subscriptions_controller.rb @@ -4,7 +4,9 @@ class Admin::DocumentCollectionEmailSubscriptionsController < Admin::BaseControl before_action :authorise_user layout "design_system" - def edit; end + def edit + @topic_list_select_presenter = TopicListSelectPresenter.new(@collection.taxonomy_topic_email_override) + end def update if user_has_selected_taxonomy_topic_emails? diff --git a/app/presenters/topic_list_select_presenter.rb b/app/presenters/topic_list_select_presenter.rb new file mode 100644 index 00000000000..55c8cccb491 --- /dev/null +++ b/app/presenters/topic_list_select_presenter.rb @@ -0,0 +1,84 @@ +class TopicListSelectPresenter + # This presenter is used to select a taxonomy topic email override for some document collections + #  We are only going to show the branches which the dept has told us they will need to tag to. + TAGGABLE_BRANCHES = [ + "/business-and-industry", + "/business-tax", + "/childcare-parenting", + "/defence", + "/defence-and-armed-forces", + "/education", + "/employment", + "/environment", + "/government", + "/housing-and-local-government", + "/money", + "/regional-and-local-government", + "/society-and-culture", + "/transport", + "/welfare", + "/work", + ].freeze + + def initialize(taxonomy_topic_email_override = nil) + @taxonomy_topic_email_override = taxonomy_topic_email_override + end + + attr_reader :taxonomy_topic_email_override + + def grouped_options(selected_taxon_content_id = nil) + branches_sorted_by_level_one_taxon_name.map do |level_one_taxon| + [level_one_taxon.name, sorted_transformed_descendants(level_one_taxon, selected_taxon_content_id)] + end + end + +private + + def branches_sorted_by_level_one_taxon_name + topic_taxonomy.visible_branches + .select { |branch| TAGGABLE_BRANCHES.include?(branch.base_path) } + .sort_by { |branch| branch.name.downcase } + end + + def sorted_transformed_descendants(level_one_taxon, selected_taxon_content_id) + transform_descendants(level_one_taxon, selected_taxon_content_id) + .sort_by { |s| s[:text].downcase } + end + + def transform_descendants(level_one_taxon, selected_taxon_content_id) + sort_descendants(level_one_taxon).map do |child| + transform_taxon(child, selected_taxon_content_id) + end + end + + def sort_descendants(level_one_taxon) + sorted_descendants = level_one_taxon.descendants.sort_by { |descendant| descendant.name.downcase } + [level_one_taxon, sorted_descendants].flatten + end + + def taxon_with_ancestors(taxon) + ancestors_names = taxon.ancestors.map(&:name) + ancestor_string = ancestors_names.join(" > ") + + "#{ancestor_string} > #{taxon.name} " if ancestors_names.any? + end + + def transform_taxon(taxon, selected_taxon_content_id = nil) + formatted_taxon_name = + taxon.ancestors.present? ? taxon_with_ancestors(taxon) : taxon.name.to_s + { + text: formatted_taxon_name, + value: taxon.content_id, + selected: selected?(taxon.content_id, selected_taxon_content_id), + } + end + + def selected?(content_id, selected_taxon_content_id) + previously_selected = selected_taxon_content_id || taxonomy_topic_email_override + previously_selected == content_id + end + + def topic_taxonomy + @topic_taxonomy ||= Taxonomy::TopicTaxonomy.new + end +end diff --git a/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb b/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb index b207dda1b42..54b616a9631 100644 --- a/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb +++ b/app/views/admin/document_collection_email_subscriptions/_taxonomy_choice.html.erb @@ -1,17 +1,13 @@

Choose the topic

- # TODO: This is a temporary implementation of a topic taxonomy select using hardcoded values. - <%= render "govuk_publishing_components/components/select", { - id: "selected_taxon_content_id", - label: "Choose a topic from the topic taxonomy. You can only choose one.", - error_message: flash["selected_taxon_content_id"], - error_id: flash["selected_taxon_content_id"], - options: [ - { text: "", value: "" }, - { text: "Topic One", value: "9b889c60-2191-11ee-be56-0242ac120002" }, - { text: "Topic Two", value: "c60-2191-11ee-be56-0242ac120002" }, - ], - } %> + <%= render "components/select-with-search", { + label: "Choose a topic from the topic taxonomy. You can only choose one.", + error_message: flash["selected_taxon_content_id"], + error_id: flash["selected_taxon_content_id"], + id: "selected_taxon_content_id", + include_blank: true, + grouped_options: @topic_list_select_presenter.grouped_options(params["selected_taxon_content_id"]), + } %>
<%= render "govuk_publishing_components/components/checkboxes", { id: "email_override_confirmation", diff --git a/features/document-collection-email-override.feature b/features/document-collection-email-override.feature index 5de8c89aab9..832084aae3d 100644 --- a/features/document-collection-email-override.feature +++ b/features/document-collection-email-override.feature @@ -6,10 +6,10 @@ Feature: Setting the taxonomy topic email override for a document collection When I visit the edit document collection page Then I click on the tab "Email notifications" And I choose "Emails about this topic" - And I select "Topic One" + And I select "Education" And I click the checkbox to confirm my selection. And I click "Save" - Then I see the success message "You’ve selected the email notification settings. You’ve chosen ‘Emails about the topic’ and the topic Topic One. You will not be able to change these settings after you publish the collection." + Then I see the success message "You’ve selected the email notification settings. You’ve chosen ‘Emails about the topic’ and the topic Education. You will not be able to change these settings after you publish the collection." Scenario: User cannot set the email override without checking the confirmation box. @@ -18,7 +18,7 @@ Feature: Setting the taxonomy topic email override for a document collection When I visit the edit document collection page Then I click on the tab "Email notifications" And I choose "Emails about this topic" - And I select "Topic One" + And I select "Education" And I click "Save" Then I see the error "You must confirm you’re happy with the email notification settings" prompting me to confirm my selection. diff --git a/features/step_definitions/document_collection_email_override_steps.rb b/features/step_definitions/document_collection_email_override_steps.rb index cdb1fe3ebc7..2f0ef4000ff 100644 --- a/features/step_definitions/document_collection_email_override_steps.rb +++ b/features/step_definitions/document_collection_email_override_steps.rb @@ -12,6 +12,7 @@ end Then(/^I click on the tab "Email notifications/) do + stub_taxonomy_data expect(page).to have_content("Email notifications") click_on("Email notifications") expect(page).to have_content("Choose the type of email updates users will get if they sign up for notifications.") @@ -22,7 +23,7 @@ end And(/^I select "([^"]*)"$/) do |topic_label| - select topic_label, from: "selected_taxon_content_id" + select topic_label, from: :selected_taxon_content_id end And(/^I click the checkbox to confirm my selection./) do @@ -31,7 +32,7 @@ And(/^I click "Save"/) do stub_request(:get, %r{\A#{Plek.find('publishing-api')}/v2/content}) - .to_return(body: { base_path: "/topic-one", content_id: "9b889c60-2191-11ee-be56-0242ac120002", title: "Topic One" }.to_json) + .to_return(body: { base_path: "/education", content_id: "root", title: "Education" }.to_json) click_on("Save") end diff --git a/lib/taxonomy/topic_taxonomy.rb b/lib/taxonomy/topic_taxonomy.rb index da29eba1b02..9ee99244489 100644 --- a/lib/taxonomy/topic_taxonomy.rb +++ b/lib/taxonomy/topic_taxonomy.rb @@ -18,9 +18,11 @@ def all_taxons end def visible_taxons - @visible_taxons = branches.select( - &:visible_to_departmental_editors - ).flat_map(&:taxon_list) + @visible_taxons = visible_branches.flat_map(&:taxon_list) + end + + def visible_branches + @visible_branches ||= branches.select(&:visible_to_departmental_editors) end private diff --git a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb index aa629fdda67..8de8fadc3b8 100644 --- a/test/functional/admin/document_collection_email_subscriptions_controller_test.rb +++ b/test/functional/admin/document_collection_email_subscriptions_controller_test.rb @@ -1,11 +1,12 @@ require "test_helper" class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionController::TestCase + include TaxonomyHelper setup do @collection = create(:draft_document_collection, :with_group) @user_with_permission = create(:writer, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) @user_without_permission = create(:writer) - @selected_taxon_content_id = "9b889c60-2191-11ee-be56-0242ac120002" + @selected_taxon_content_id = root_taxon_content_id @put_params = { document_collection_id: @collection.id, override_email_subscriptions: "true", @@ -13,7 +14,8 @@ class Admin::DocumentCollectionEmailSubscriptionsControllerTest < ActionControll email_override_confirmation: "true", } login_as @user_without_permission - stub_publishing_api_has_item(content_id: "9b889c60-2191-11ee-be56-0242ac120002", title: "Topic One") + stub_publishing_api_has_item(content_id: root_taxon_content_id, title: root_taxon["title"]) + stub_taxonomy_with_all_taxons end should_be_an_admin_controller diff --git a/test/integration/document_collection_email_override_test.rb b/test/integration/document_collection_email_override_test.rb index 6735e15997a..70201a82486 100644 --- a/test/integration/document_collection_email_override_test.rb +++ b/test/integration/document_collection_email_override_test.rb @@ -12,23 +12,22 @@ class DocumentCollectionEmailOverrideTest < ActionDispatch::IntegrationTest context "as a user with the email override editor permission" do let(:user_with_permission_to_override) { create(:writer, permissions: [User::Permissions::EMAIL_OVERRIDE_EDITOR]) } - let(:taxon_content_id) { "9b889c60-2191-11ee-be56-0242ac120002" } before do login_as(user_with_permission_to_override) + stub_taxonomy_with_all_taxons end it "updates the taxonomy topic email override" do - stub_publishing_api_has_item(content_id: taxon_content_id, title: "Topic one") - + stub_publishing_api_has_item(content_id: root_taxon_content_id, title: root_taxon["title"]) visit edit_admin_document_collection_path(document_collection) click_link "Email notifications" page.choose("Emails about the topic") - select "Topic One", from: "selected_taxon_content_id" + select root_taxon["title"], from: "selected_taxon_content_id" page.check("Select this box to confirm you're happy with what you've selected.") click_button("Save") document_collection.reload - assert_equal document_collection.taxonomy_topic_email_override, taxon_content_id + assert_equal document_collection.taxonomy_topic_email_override, root_taxon_content_id end it "does not update taxonomy topic email if confirmation button is unchecked" do @@ -36,7 +35,7 @@ class DocumentCollectionEmailOverrideTest < ActionDispatch::IntegrationTest click_link "Email notifications" page.choose("Emails about the topic") - select "Topic One", from: "selected_taxon_content_id" + select root_taxon["title"], from: "selected_taxon_content_id" click_button("Save") document_collection.reload assert_nil document_collection.taxonomy_topic_email_override @@ -54,7 +53,7 @@ class DocumentCollectionEmailOverrideTest < ActionDispatch::IntegrationTest it "shows the user a summary page if the document collection is in an unmodifiable state" do published_collection = create(:published_document_collection) - taxons = { "title" => "Foo", "base_path" => "/foo", "content_id" => "123asd", "phase" => "live" } + taxons = { "title" => "Foo", "base_path" => "/foo", "content_id" => "123asd" } links = { "meets_user_needs" => %w[123] } stub_publishing_api_expanded_links_with_taxons(published_collection.content_id, [taxons]) stub_publishing_api_has_links({ content_id: published_collection.content_id, links: }) diff --git a/test/unit/app/presenters/topic_list_select_presenter_test.rb b/test/unit/app/presenters/topic_list_select_presenter_test.rb new file mode 100644 index 00000000000..502c14bfca9 --- /dev/null +++ b/test/unit/app/presenters/topic_list_select_presenter_test.rb @@ -0,0 +1,55 @@ +require "test_helper" + +class TopicListSelectPresenterTest < ActiveSupport::TestCase + include TaxonomyHelper + + test ".grouped_options returns subtopics grouped by their parent topic" do + stub_taxonomy_with_all_taxons + #  this stubs a taxonomy with three taxons [Education, About your organisation, Parenting] + # only Education and Parenting branches are taggable, so About your organisation is hidden + #  from the select + + expected = [ + [ + "Education", + [ + { + text: "Education", + value: root_taxon_content_id, + selected: false, + }, + { + text: "Education > School Curriculum ", + value: grandparent_taxon_content_id, + selected: false, + }, + { + text: "Education > School Curriculum > Primary curriculum, key stage 1 ", + value: parent_taxon_content_id, + selected: false, + }, + { + text: "Education > School Curriculum > Primary curriculum, key stage 1 > Tests ", + value: child_taxon_content_id, + selected: false, + }, + ], + ], + [ + "Parenting", + [ + { + text: "Parenting", + value: draft_taxon_1_content_id, + selected: false, + }, + ], + ], + ] + + document_collection = create(:draft_document_collection) + + result = TopicListSelectPresenter.new(document_collection).grouped_options + assert_equal expected, result + end +end