From f8e1c2ba9ad1bcbdf6155c237244d173fb60b1e5 Mon Sep 17 00:00:00 2001 From: Mark Taylor <138604938+mtaylorgds@users.noreply.github.com> Date: Thu, 28 Nov 2024 13:58:58 +0000 Subject: [PATCH 1/3] Indicate when nobody is assigned to an edition Update the document summary on the edition show page to display the text 'None' when no-one is assigned to an edition, rather than just leaving a blank space. This makes it clearer to the user. Extracts the code that generates the document summary items to a helper method to make the code in the ERB file simpler. --- app/helpers/editions_helper.rb | 18 ++++++++++++++++++ app/views/editions/show.html.erb | 16 +--------------- test/integration/edition_edit_test.rb | 16 ++++++++++++++++ 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/app/helpers/editions_helper.rb b/app/helpers/editions_helper.rb index 5bfd15844..ca8bd3688 100644 --- a/app/helpers/editions_helper.rb +++ b/app/helpers/editions_helper.rb @@ -53,4 +53,22 @@ def legacy_format_filter_selection_options [displayed_format_name, format_name] end end + + def document_summary_items(edition) + [ + { + field: "Assigned to", + value: edition.assigned_to || "None", + edit: assignee_edit_link(edition), + }, + { + field: "Content type", + value: edition.format.underscore.humanize, + }, + { + field: "Edition", + value: sanitize("#{edition.version_number} #{edition.status_text}"), + }, + ] + end end diff --git a/app/views/editions/show.html.erb b/app/views/editions/show.html.erb index 8f13a3eab..08c79a9ed 100644 --- a/app/views/editions/show.html.erb +++ b/app/views/editions/show.html.erb @@ -21,21 +21,7 @@
<%= render "govuk_publishing_components/components/summary_list", { - items: [ - { - field: "Assigned to", - value: @resource.assigned_to, - edit: assignee_edit_link(@resource), - }, - { - field: "Content type", - value: @resource.format.underscore.humanize, - }, - { - field: "Edition", - value: sanitize("#{@resource.version_number} #{@resource.status_text}"), - }, - ], + items: document_summary_items(@resource), } %>
diff --git a/test/integration/edition_edit_test.rb b/test/integration/edition_edit_test.rb index df6bf8602..edac4bd37 100644 --- a/test/integration/edition_edit_test.rb +++ b/test/integration/edition_edit_test.rb @@ -36,6 +36,22 @@ class EditionEditTest < IntegrationTest assert row[2].has_text?("1") assert row[2].has_text?("Published") end + + should "indicate when an edition does not have an assignee" do + within all(".govuk-summary-list__row")[0] do + assert_selector(".govuk-summary-list__key", text: "Assigned to") + assert_selector(".govuk-summary-list__value", text: "None") + end + end + + should "show the person assigned to an edition" do + visit_draft_edition + + within all(".govuk-summary-list__row")[0] do + assert_selector(".govuk-summary-list__key", text: "Assigned to") + assert_selector(".govuk-summary-list__value", text: @draft_edition.assignee) + end + end end context "metadata tab" do From 566f92c252858e53f0973f46618e4ba7db44392c Mon Sep 17 00:00:00 2001 From: Mark Taylor <138604938+mtaylorgds@users.noreply.github.com> Date: Thu, 28 Nov 2024 15:42:48 +0000 Subject: [PATCH 2/3] Rename method parameter The "resource" should always be an edition, so use that more descriptive name. --- app/helpers/tabbed_nav_helper.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/helpers/tabbed_nav_helper.rb b/app/helpers/tabbed_nav_helper.rb index 7c8e2c2f2..6a5f5a34b 100644 --- a/app/helpers/tabbed_nav_helper.rb +++ b/app/helpers/tabbed_nav_helper.rb @@ -73,15 +73,15 @@ def edit_nav_item(label, href, current) ] end - def available_assignee_items(resource) + def available_assignee_items(edition) items = [] - unless resource.assignee.nil? - items << { value: resource.assigned_to_id, text: resource.assignee, checked: true } + unless edition.assignee.nil? + items << { value: edition.assigned_to_id, text: edition.assignee, checked: true } items << { value: "none", text: "None" } items << :or end User.enabled.order_by([%i[name asc]]).each do |user| - items << { value: user.id, text: user.name } unless user.name == resource.assignee + items << { value: user.id, text: user.name } unless user.name == edition.assignee end items end From 829823046e4a8058196be86507c4f7d1d589b98d Mon Sep 17 00:00:00 2001 From: Mark Taylor <138604938+mtaylorgds@users.noreply.github.com> Date: Tue, 3 Dec 2024 16:40:09 +0000 Subject: [PATCH 3/3] Hide non-editor users when assigning to an edition It is not possible to assign a user that doesn't have editor permissions on an edition to that edition (attempting to do so will result in an error being raised). Therefore, don't display non-editor users for an edition in the list of available users to choose from. This reduces the size of the list (which is quite sizeable), and also helps prevent users from triggering an error condition. --- app/helpers/tabbed_nav_helper.rb | 2 +- test/integration/edition_edit_test.rb | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/helpers/tabbed_nav_helper.rb b/app/helpers/tabbed_nav_helper.rb index 6a5f5a34b..a02a3d135 100644 --- a/app/helpers/tabbed_nav_helper.rb +++ b/app/helpers/tabbed_nav_helper.rb @@ -81,7 +81,7 @@ def available_assignee_items(edition) items << :or end User.enabled.order_by([%i[name asc]]).each do |user| - items << { value: user.id, text: user.name } unless user.name == edition.assignee + items << { value: user.id, text: user.name } unless user.name == edition.assignee || !user.has_editor_permissions?(edition) end items end diff --git a/test/integration/edition_edit_test.rb b/test/integration/edition_edit_test.rb index edac4bd37..b405f8e6b 100644 --- a/test/integration/edition_edit_test.rb +++ b/test/integration/edition_edit_test.rb @@ -10,7 +10,7 @@ class EditionEditTest < IntegrationTest stub_linkables end - context "all tabs" do + context "edit page" do setup do visit_published_edition end @@ -54,6 +54,18 @@ class EditionEditTest < IntegrationTest end end + context "edit assignee page" do + should "only show editors as available for assignment" do + edition = FactoryBot.create(:answer_edition, state: "draft") + non_editor_user = FactoryBot.create(:user, name: "Non Editor User") + + visit edit_assignee_edition_path(edition) + + assert_selector "label", text: @govuk_editor.name + assert_no_selector "label", text: non_editor_user.name + end + end + context "metadata tab" do context "when state is 'draft'" do setup do