diff --git a/app/controllers/editions_controller.rb b/app/controllers/editions_controller.rb index 263251196..9c79a8258 100644 --- a/app/controllers/editions_controller.rb +++ b/app/controllers/editions_controller.rb @@ -11,13 +11,17 @@ class EditionsController < InheritedResources::Base before_action only: %i[unpublish confirm_unpublish process_unpublish] do require_govuk_editor(redirect_path: edition_path(resource)) end - before_action only: %i[progress admin update confirm_destroy] do + before_action only: %i[progress admin update confirm_destroy edit_assignee update_assignee] do require_editor_permissions end before_action only: %i[confirm_destroy destroy] do destroyable_edition? end + before_action only: %i[edit_assignee update_assignee] do + require_assignee_editable + end + helper_method :locale_to_language def index @@ -35,7 +39,7 @@ def show alias_method :admin, :show def update - @resource.assign_attributes(permitted_params) + @resource.assign_attributes(permitted_update_params) if @resource.save UpdateWorker.perform_async(resource.id.to_s) @@ -106,6 +110,28 @@ def destroy render "secondary_nav_tabs/confirm_destroy" end + def edit_assignee + render "secondary_nav_tabs/_edit_assignee" + end + + def update_assignee + assignee_id = params.require(:assignee_id) + + if update_assignment(@resource, assignee_id) + flash[:success] = "Assigned person updated." + redirect_to edition_path + else + render "secondary_nav_tabs/_edit_assignee" + end + rescue ActionController::ParameterMissing + flash.now[:danger] = "Please select a person to assign, or 'None' to unassign the currently assigned person." + render "secondary_nav_tabs/_edit_assignee" + rescue StandardError => e + Rails.logger.error "Error #{e.class} #{e.message}" + flash.now[:danger] = "Due to a service problem, the assigned person couldn't be saved" + render "secondary_nav_tabs/_edit_assignee" + end + protected def setup_view_paths @@ -166,7 +192,7 @@ def progress_action_param nil end - def permitted_params + def permitted_update_params params.require(:edition).permit(%i[title overview in_beta body major_change change_note]) end @@ -176,4 +202,31 @@ def destroyable_edition? flash[:danger] = "Cannot delete a #{description(@resource).downcase} that has ever been published." redirect_to edition_path(@resource) end + + def require_assignee_editable + return if can_update_assignee?(@resource) + + flash[:danger] = "Cannot edit the assignee of an edition that has been published." + redirect_to edition_path(@resource) + end + + def update_assignment(edition, assignee_id) + return true if assignee_id == edition.assigned_to_id + + if assignee_id == "none" + current_user.unassign(edition) + return true + end + + assignee = User.where(id: assignee_id).first + raise StandardError, "An attempt was made to assign non-existent user '#{assignee_id}' to edition '#{edition.id}'." unless assignee + + if assignee.has_editor_permissions?(@resource) + current_user.assign(edition, assignee) + return true + end + + flash.now[:danger] = "Chosen assignee does not have correct editor permissions." + false + end end diff --git a/app/helpers/tabbed_nav_helper.rb b/app/helpers/tabbed_nav_helper.rb index 22033d6ef..7c8e2c2f2 100644 --- a/app/helpers/tabbed_nav_helper.rb +++ b/app/helpers/tabbed_nav_helper.rb @@ -28,6 +28,17 @@ def current_tab_name end end + def assignee_edit_link(edition) + if current_user.has_editor_permissions?(edition) && can_update_assignee?(edition) + { + href: edit_assignee_edition_path, + link_text: "Edit", + } + else + {} + end + end + private def all_tab_names @@ -61,4 +72,21 @@ def edit_nav_item(label, href, current) }, ] end + + def available_assignee_items(resource) + items = [] + unless resource.assignee.nil? + items << { value: resource.assigned_to_id, text: resource.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 + end + items + end + + def can_update_assignee?(resource) + %w[published archived scheduled_for_publishing].exclude?(resource.state) + end end diff --git a/app/views/editions/secondary_nav_tabs/_edit_assignee.html.erb b/app/views/editions/secondary_nav_tabs/_edit_assignee.html.erb new file mode 100644 index 000000000..5c64d3a50 --- /dev/null +++ b/app/views/editions/secondary_nav_tabs/_edit_assignee.html.erb @@ -0,0 +1,37 @@ +<% content_for :title_context, @resource.title %> +<% content_for :page_title, "Assign person" %> +<% content_for :title, "Assign person" %> + +<%= form_for @resource, url: update_assignee_edition_path(@resource) do %> +
+
+ <%= render "govuk_publishing_components/components/radio", { + name: "assignee_id", + heading: "Choose a person to assign", + visually_hidden_heading: true, + items: available_assignee_items(@resource), + } %> +
+ +
+ +
+
+<% end %> diff --git a/app/views/editions/show.html.erb b/app/views/editions/show.html.erb index cf004a0ca..8f13a3eab 100644 --- a/app/views/editions/show.html.erb +++ b/app/views/editions/show.html.erb @@ -25,6 +25,7 @@ { field: "Assigned to", value: @resource.assigned_to, + edit: assignee_edit_link(@resource), }, { field: "Content type", diff --git a/config/routes.rb b/config/routes.rb index 5dbc01734..ca97f41f3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,6 +30,8 @@ get "admin/confirm-destroy", to: "editions#confirm_destroy", as: "confirm_destroy" delete "admin/delete-edition", to: "editions#destroy", as: "admin_delete" post "progress" + get "edit_assignee" + patch "update_assignee" post "skip_fact_check", to: "editions#progress", edition: { diff --git a/test/functional/editions_controller_test.rb b/test/functional/editions_controller_test.rb index e8c19677e..77461f92a 100644 --- a/test/functional/editions_controller_test.rb +++ b/test/functional/editions_controller_test.rb @@ -168,15 +168,15 @@ class EditionsControllerTest < ActionController::TestCase end context "#admin" do - context "do not have required permissions" do - context "Welsh editors and non Welsh edition" do + context "user without required permissions" do + context "Welsh editor and non-Welsh edition" do setup do login_as_welsh_editor end - %i[admin confirm_destroy].each do |url_path| - should "show permission error and redirects to edition path for #{url_path} path" do - get url_path, params: { id: @edition.id } + %i[admin confirm_destroy].each do |path| + should "show permission error and redirect to edition path for #{path} path" do + get path, params: { id: @edition.id } assert_redirected_to edition_path(@edition) assert_equal "You do not have correct editor permissions for this action.", flash[:danger] @@ -184,15 +184,15 @@ class EditionsControllerTest < ActionController::TestCase end end - context "nor Welsh nor Govuk editors" do + context "non-Welsh, non-govuk editor" do setup do user = FactoryBot.create(:user, name: "Stub User") login_as(user) end - %i[admin confirm_destroy].each do |url_path| - should "show permission error and redirects to edition path for #{url_path} path" do - get url_path, params: { id: @edition.id } + %i[admin confirm_destroy].each do |path| + should "show permission error and redirect to edition path for #{path} path" do + get path, params: { id: @edition.id } assert_redirected_to edition_path(@edition) assert_equal "You do not have correct editor permissions for this action.", flash[:danger] @@ -201,15 +201,15 @@ class EditionsControllerTest < ActionController::TestCase end end - context "has required permissions" do - context "Welsh editors and welsh edition" do + context "user with required permissions" do + context "Welsh editor and Welsh edition" do setup do login_as_welsh_editor end - %i[admin confirm_destroy].each do |url_path| - should "be able to navigate successfully to #{url_path} path" do - get url_path, params: { id: @welsh_edition.id } + %i[admin confirm_destroy].each do |path| + should "be able to navigate successfully to #{path} path" do + get path, params: { id: @welsh_edition.id } assert_response :success end @@ -238,7 +238,7 @@ class EditionsControllerTest < ActionController::TestCase end %i[published scheduled_for_publishing archived].each do |edition_state| - context "edition with state '#{edition_state}' can not be deleted" do + context "edition in '#{edition_state}' state can not be deleted" do setup do @edition = FactoryBot.create(:edition, state: edition_state, publish_at: Time.zone.now + 1.hour) end @@ -265,7 +265,7 @@ class EditionsControllerTest < ActionController::TestCase end context "#progress" do - context "Welsh editors" do + context "Welsh editor" do setup do login_as_welsh_editor end @@ -307,7 +307,7 @@ class EditionsControllerTest < ActionController::TestCase end end - context "govuk editors" do + context "govuk editor" do setup do login_as_govuk_editor end @@ -401,6 +401,186 @@ class EditionsControllerTest < ActionController::TestCase end end + context "#edit_assignee" do + context "user without required permissions" do + context "Welsh editor and non-Welsh edition" do + setup do + login_as_welsh_editor + end + + should "show permission error and redirect to edition path" do + get :edit_assignee, params: { id: @edition.id } + + assert_redirected_to edition_path(@edition) + assert_equal "You do not have correct editor permissions for this action.", flash[:danger] + end + end + + context "non-Welsh, non-govuk editor" do + setup do + user = FactoryBot.create(:user, name: "Stub User") + login_as(user) + end + + should "show permission error and redirect to edition path" do + get :edit_assignee, params: { id: @edition.id } + + assert_redirected_to edition_path(@edition) + assert_equal "You do not have correct editor permissions for this action.", flash[:danger] + end + end + end + + context "user with required permissions" do + context "Welsh editor and Welsh edition" do + setup do + login_as_welsh_editor + end + + should "be able to navigate successfully to edit assignee path" do + get :edit_assignee, params: { id: @welsh_edition.id } + + assert_response :success + end + end + + should "be able to navigate to the edit assignee path" do + get :edit_assignee, params: { id: @edition.id } + + assert_response :success + end + + %i[published scheduled_for_publishing archived].each do |edition_state| + context "edition in '#{edition_state}' state" do + setup do + @edition = FactoryBot.create(:edition, state: edition_state, publish_at: Time.zone.now + 1.hour) + end + + should "redirect to edition path with error message" do + get :edit_assignee, params: { id: @edition.id } + + assert_redirected_to edition_path + assert_equal "Cannot edit the assignee of an edition that has been published.", flash[:danger] + end + end + end + end + end + + context "#update_assignee" do + context "user without required permissions" do + context "Welsh editor and non-Welsh edition" do + setup do + login_as_welsh_editor + end + + should "show permission error and redirect to edition path" do + patch :update_assignee, params: { id: @edition.id, assignee_id: @user.id } + + assert_redirected_to edition_path(@edition) + assert_equal "You do not have correct editor permissions for this action.", flash[:danger] + end + end + + context "non-Welsh, non-govuk editor" do + setup do + user = FactoryBot.create(:user, name: "Stub User") + login_as(user) + end + + should "show permission error and redirect to edition path" do + patch :update_assignee, params: { id: @edition.id, assignee_id: @user.id } + + assert_redirected_to edition_path(@edition) + assert_equal "You do not have correct editor permissions for this action.", flash[:danger] + end + end + end + + context "user with required permissions" do + context "Welsh editor and Welsh edition" do + setup do + login_as_welsh_editor + end + + should "be able to successfully update assignee" do + patch :update_assignee, params: { id: @welsh_edition.id, assignee_id: @user.id } + + assert_redirected_to edition_path(@welsh_edition.id) + end + end + + should "be able to successfully update assignee" do + patch :update_assignee, params: { id: @edition.id, assignee_id: @user.id } + + assert_redirected_to edition_path(@edition.id) + end + + should "update the assignee" do + new_assignee = FactoryBot.create(:user, :govuk_editor, name: "Updated Assignee") + patch :update_assignee, params: { id: @edition.id, assignee_id: new_assignee.id } + + @edition.reload + assert_equal "Updated Assignee", @edition.assignee + end + + should "be able to unassign the current assignee" do + patch :update_assignee, params: { id: @edition.id, assignee_id: "none" } + + @edition.reload + assert_nil @edition.assignee + assert_nil @edition.assigned_to_id + end + + %i[published scheduled_for_publishing archived].each do |edition_state| + context "edition in '#{edition_state}' state" do + setup do + @edition = FactoryBot.create(:edition, state: edition_state, publish_at: Time.zone.now + 1.hour) + end + + should "redirect to edition path with error message" do + get :edit_assignee, params: { id: @edition.id, assignee_id: @user.id } + + assert_redirected_to edition_path + assert_equal "Cannot edit the assignee of an edition that has been published.", flash[:danger] + end + end + end + + should "show error when database save fails" do + new_assignee = FactoryBot.create(:user, :govuk_editor, name: "Updated Assignee") + User.any_instance.stubs(:assign).raises(StandardError) + + patch :update_assignee, params: { id: @edition.id, assignee_id: new_assignee.id } + + assert_template "secondary_nav_tabs/_edit_assignee" + assert_equal "Due to a service problem, the assigned person couldn't be saved", flash[:danger] + end + + should "show error when new assignee does not have editor permission" do + new_assignee = FactoryBot.create(:user, name: "Stub User") + patch :update_assignee, params: { id: @edition.id, assignee_id: new_assignee.id } + + assert_template "secondary_nav_tabs/_edit_assignee" + assert_equal "Chosen assignee does not have correct editor permissions.", flash[:danger] + end + + should "show error when no assignee option is selected" do + patch :update_assignee, params: { id: @edition.id } + + assert_template "secondary_nav_tabs/_edit_assignee" + assert_equal "Please select a person to assign, or 'None' to unassign the currently assigned person.", flash[:danger] + end + + should "show error when a non-existent assignee ID is provided" do + patch :update_assignee, params: { id: @edition.id, assignee_id: "non-existent ID" } + + assert_template "secondary_nav_tabs/_edit_assignee" + assert_equal "Due to a service problem, the assigned person couldn't be saved", flash[:danger] + end + end + end + private def description(edition) diff --git a/test/integration/edition_edit_test.rb b/test/integration/edition_edit_test.rb index f51a2ac75..b0f0b433d 100644 --- a/test/integration/edition_edit_test.rb +++ b/test/integration/edition_edit_test.rb @@ -2,7 +2,8 @@ class EditionEditTest < IntegrationTest setup do - login_as(FactoryBot.create(:user, :govuk_editor, name: "Stub User")) + @govuk_editor = FactoryBot.create(:user, :govuk_editor, name: "Stub User") + login_as(@govuk_editor) test_strategy = Flipflop::FeatureSet.current.test! test_strategy.switch!(:design_system_edit, true) stub_linkables @@ -86,7 +87,7 @@ class EditionEditTest < IntegrationTest end context "unpublish tab" do - context "do not have required permissions" do + context "user does not have required permissions" do setup do login_as(FactoryBot.create(:user, name: "Stub User")) visit_draft_edition @@ -97,9 +98,8 @@ class EditionEditTest < IntegrationTest end end - context "has required permissions" do + context "user has required permissions" do setup do - login_as(FactoryBot.create(:user, :govuk_editor, name: "Stub User")) visit_draft_edition end @@ -146,7 +146,7 @@ class EditionEditTest < IntegrationTest end context "admin tab" do - context "do not have required permissions" do + context "user does not have required permissions" do setup do login_as(FactoryBot.create(:user, name: "Stub User")) visit_draft_edition @@ -164,11 +164,7 @@ class EditionEditTest < IntegrationTest end end - context "has required permissions" do - setup do - login_as(FactoryBot.create(:user, :govuk_editor, name: "Stub User")) - end - + context "user has required permissions" do %i[published archived scheduled_for_publishing].each do |state| context "when state is '#{state}'" do setup do @@ -362,6 +358,113 @@ class EditionEditTest < IntegrationTest assert page.has_field?("edition[change_note]") end end + + context "edit assignee link" do + context "user does not have required permissions" do + setup do + login_as(FactoryBot.create(:user, name: "Stub User")) + visit_draft_edition + end + + should "not show 'Edit' link when user is not govuk editor or welsh editor" do + within :css, ".editions__edit__summary" do + assert page.has_no_link?("Edit") + end + end + + should "not show 'Edit' link when user is welsh editor and edition is not welsh" do + login_as(FactoryBot.create(:user, :welsh_editor, name: "Stub User")) + visit_draft_edition + + within :css, ".editions__edit__summary" do + assert page.has_no_link?("Edit") + end + end + end + + context "user has required permissions" do + %i[published archived scheduled_for_publishing].each do |state| + context "when state is '#{state}'" do + setup do + send "visit_#{state}_edition" + end + + should "not show 'Edit' link" do + within :css, ".editions__edit__summary" do + assert page.has_no_link?("Edit") + end + end + end + end + + %i[draft amends_needed in_review fact_check_received fact_check ready].each do |state| + context "when state is '#{state}'" do + setup do + send "visit_#{state}_edition" + click_link("Admin") + end + + should "show 'Edit' link" do + within :css, ".editions__edit__summary" do + assert page.has_link?("Edit") + end + end + + should "navigate to edit assignee page when 'Edit' assignee is clicked" do + within :css, ".editions__edit__summary" do + click_link("Edit") + end + + assert(page.current_path.include?("/edit_assignee")) + end + end + end + + context "edit assignee page" do + setup do + visit_draft_edition + within :css, ".editions__edit__summary" do + click_link("Edit") + end + end + + should "show title and page title" do + assert page.has_title?("Assign person") + assert page.has_text?(@draft_edition.title) + end + + should "show only enabled users as radio button options" do + FactoryBot.create(:user, name: "Disabled User", disabled: true) + all_enabled_users_names = [] + User.enabled.each { |user| all_enabled_users_names << user.name } + all_user_radio_buttons = find_all(".govuk-radios__item").map(&:text) + + assert all_user_radio_buttons.exclude?("Disabled User") + + all_enabled_users_names.each do |users| + assert all_user_radio_buttons.include?(users) + end + end + + should "allow currently assigned user to be unassigned" do + user = FactoryBot.create(:user, :govuk_editor) + @govuk_editor.assign(@draft_edition, user) + visit current_path + + choose "None" + click_on "Save" + + assert_equal(page.current_path, "/editions/#{@draft_edition.id}") + end + + should "navigate to editions edit page when 'Cancel' link is clicked" do + click_link("Cancel") + + assert_equal(page.current_path, "/editions/#{@draft_edition.id}") + end + end + end + end end private