From eb7d05ba98d581b073a8da5e4043f3b66ea3ac4c Mon Sep 17 00:00:00 2001 From: syed-ali-tw Date: Mon, 11 Nov 2024 15:14:52 +0000 Subject: [PATCH] Add delete to admin tab --- app/assets/stylesheets/editions.scss | 6 + app/controllers/editions_controller.rb | 31 ++- .../secondary_nav_tabs/_admin.html.erb | 16 +- .../confirm_destroy.html.erb | 37 ++++ .../confirm_unpublish.html.erb | 1 - config/routes.rb | 2 + test/functional/editions_controller_test.rb | 100 ++++++++-- test/integration/edition_edit_test.rb | 183 +++++++++++++----- 8 files changed, 301 insertions(+), 75 deletions(-) create mode 100644 app/views/editions/secondary_nav_tabs/confirm_destroy.html.erb diff --git a/app/assets/stylesheets/editions.scss b/app/assets/stylesheets/editions.scss index 2d419dc7c..0924abdd0 100644 --- a/app/assets/stylesheets/editions.scss +++ b/app/assets/stylesheets/editions.scss @@ -8,3 +8,9 @@ @include govuk-responsive-padding(4, "top"); border-top: 1px solid $govuk-border-colour; } + +.editions__admin__tab { + .govuk-link { + margin-top: govuk-spacing(3) + } +} diff --git a/app/controllers/editions_controller.rb b/app/controllers/editions_controller.rb index 7c7eb5a49..263251196 100644 --- a/app/controllers/editions_controller.rb +++ b/app/controllers/editions_controller.rb @@ -11,9 +11,12 @@ 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] do + before_action only: %i[progress admin update confirm_destroy] do require_editor_permissions end + before_action only: %i[confirm_destroy destroy] do + destroyable_edition? + end helper_method :locale_to_language @@ -90,6 +93,19 @@ def progress redirect_to edition_path(resource) end + def confirm_destroy + render "secondary_nav_tabs/confirm_destroy" + end + + def destroy + @resource.destroy! + flash[:success] = "Edition deleted" + redirect_to root_url + rescue StandardError + flash[:danger] = downstream_error_message(:deleted) + render "secondary_nav_tabs/confirm_destroy" + end + protected def setup_view_paths @@ -108,12 +124,12 @@ def unpublish_edition(artefact) end def render_confirm_page_with_error - @resource.errors.add(:unpublish, downstream_error_message) + @resource.errors.add(:unpublish, downstream_error_message(:unpublished)) render "secondary_nav_tabs/confirm_unpublish" end - def downstream_error_message - "Due to a service problem, the edition couldn't be unpublished" + def downstream_error_message(action) + "Due to a service problem, the edition couldn't be #{action}" end def locale_to_language(locale) @@ -153,4 +169,11 @@ def progress_action_param def permitted_params params.require(:edition).permit(%i[title overview in_beta body major_change change_note]) end + + def destroyable_edition? + return if @resource.can_destroy? + + flash[:danger] = "Cannot delete a #{description(@resource).downcase} that has ever been published." + redirect_to edition_path(@resource) + end end diff --git a/app/views/editions/secondary_nav_tabs/_admin.html.erb b/app/views/editions/secondary_nav_tabs/_admin.html.erb index 882bfe213..c0755ffc4 100644 --- a/app/views/editions/secondary_nav_tabs/_admin.html.erb +++ b/app/views/editions/secondary_nav_tabs/_admin.html.erb @@ -1,14 +1,12 @@ <% @edition = @resource %>
-
+
<%= header_for("Admin") %> - - <% if @edition.fact_check? %> - <%= form_for @edition, url: skip_fact_check_edition_path(@edition), method: "post" do %> - <%= render "govuk_publishing_components/components/button", { - text: "Skip fact check", - } %> - <% end %> - <% end %> + <%= render "govuk_publishing_components/components/list", { + items: [ + ( primary_button_for(@edition, skip_fact_check_edition_path(@edition), "Skip fact check") if @edition.fact_check? ), + ( link_to("Delete edition #{@edition.version_number}", confirm_destroy_edition_path(@resource), class: "govuk-link gem-link--destructive") if @edition.can_destroy? ), + ], + } %>
diff --git a/app/views/editions/secondary_nav_tabs/confirm_destroy.html.erb b/app/views/editions/secondary_nav_tabs/confirm_destroy.html.erb new file mode 100644 index 000000000..d843f4522 --- /dev/null +++ b/app/views/editions/secondary_nav_tabs/confirm_destroy.html.erb @@ -0,0 +1,37 @@ +<% @edition = @resource %> +<% content_for :title_context, @edition.title %> +<% content_for :page_title, "Delete edition" %> +<% content_for :title, "Delete edition" %> +
+ <% unless @edition.errors.empty? %> + <% content_for :error_summary do %> + <%= render("govuk_publishing_components/components/error_summary", { + id: "error-summary", + title: "There is a problem", + items: @edition.errors.map do |error| + { + text: error.message, + href: "##{error.attribute.to_s}", + } + end, + }) %> + <% end %> + <% end %> + +
+ <%= render "govuk_publishing_components/components/inset_text", { + text: "If you delete this edition it cannot be undone.", + } %> + + <%= form_for @edition, url: admin_delete_edition_path(@edition), method: :delete do %> +

Are you sure you want to delete this edition?

+
+ <%= render "govuk_publishing_components/components/button", { + text: "Delete edition", + destructive: true, + } %> + <%= link_to("Cancel", admin_edition_path, class: "govuk-link govuk-link--no-visited-state") %> +
+ <% end %> +
+
diff --git a/app/views/editions/secondary_nav_tabs/confirm_unpublish.html.erb b/app/views/editions/secondary_nav_tabs/confirm_unpublish.html.erb index 006172cb9..34a5bb9d7 100644 --- a/app/views/editions/secondary_nav_tabs/confirm_unpublish.html.erb +++ b/app/views/editions/secondary_nav_tabs/confirm_unpublish.html.erb @@ -36,4 +36,3 @@ <% end %>
- diff --git a/config/routes.rb b/config/routes.rb index d26998815..5dbc01734 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -27,6 +27,8 @@ get "unpublish" get "unpublish/confirm-unpublish", to: "editions#confirm_unpublish", as: "confirm_unpublish" post "process_unpublish" + get "admin/confirm-destroy", to: "editions#confirm_destroy", as: "confirm_destroy" + delete "admin/delete-edition", to: "editions#destroy", as: "admin_delete" post "progress" post "skip_fact_check", to: "editions#progress", diff --git a/test/functional/editions_controller_test.rb b/test/functional/editions_controller_test.rb index 08ade38b8..e8c19677e 100644 --- a/test/functional/editions_controller_test.rb +++ b/test/functional/editions_controller_test.rb @@ -168,28 +168,98 @@ class EditionsControllerTest < ActionController::TestCase end context "#admin" do - should "show the admin page for the edition" do - get :admin, params: { id: @edition.id } + context "do not have required permissions" do + context "Welsh editors and non Welsh edition" do + setup do + login_as_welsh_editor + end - assert_response :success + %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 } + + assert_redirected_to edition_path(@edition) + assert_equal "You do not have correct editor permissions for this action.", flash[:danger] + end + end + end + + context "nor Welsh nor Govuk editors" 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 } + + assert_redirected_to edition_path(@edition) + assert_equal "You do not have correct editor permissions for this action.", flash[:danger] + end + end + end end - context "Welsh editors" do - setup do - login_as_welsh_editor + context "has required permissions" do + context "Welsh editors 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 } + + assert_response :success + end + end end - should "be able to see the admin page for Welsh editions" do - get :admin, params: { id: @welsh_edition.id } + should "be able to navigate to the admin path" do + get :admin, params: { id: @edition.id } assert_response :success end - should "not be able to see the admin page for non-Welsh editions" do - get :admin, params: { id: @edition.id } + context "#confirm_destroy" do + should "be able to navigate to the confirm destroy path" do + get :confirm_destroy, params: { id: @edition.id } - assert_redirected_to edition_path(@edition) - assert_equal "You do not have correct editor permissions for this action.", flash[:danger] + assert_response :success + end + + should "delete the edition from the database and display success message with redirection to root" do + delete :destroy, params: { id: @edition.id } + + assert_equal 0, Edition.where(id: @edition.id).count + assert_equal "Edition deleted", flash[:success] + assert_redirected_to root_path + end + + %i[published scheduled_for_publishing archived].each do |edition_state| + context "edition with state '#{edition_state}' can not be deleted" 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 + delete :destroy, params: { id: @edition.id } + + assert_redirected_to edition_path + assert_equal "Cannot delete a #{description(@edition)} that has ever been published.", flash[:danger] + end + end + end + + should "render confirm destroy page with error if deleting from database fails" do + Edition.any_instance.stubs(:destroy!).raises(Mongoid::Errors::MongoidError.new) + + delete :destroy, params: { id: @edition.id } + + assert_template "secondary_nav_tabs/confirm_destroy" + assert_equal "Due to a service problem, the edition couldn't be deleted", flash[:danger] + end end end end @@ -330,4 +400,10 @@ class EditionsControllerTest < ActionController::TestCase } end end + +private + + def description(edition) + edition.format.underscore.humanize.downcase + end end diff --git a/test/integration/edition_edit_test.rb b/test/integration/edition_edit_test.rb index beb169331..f51a2ac75 100644 --- a/test/integration/edition_edit_test.rb +++ b/test/integration/edition_edit_test.rb @@ -10,7 +10,7 @@ class EditionEditTest < IntegrationTest context "all tabs" do setup do - visit_edition_in_published + visit_published_edition end should "show all the tabs when user has required permission and edition is published" do @@ -39,7 +39,7 @@ class EditionEditTest < IntegrationTest context "metadata tab" do context "when state is 'draft'" do setup do - visit_edition_in_draft + visit_draft_edition click_link("Metadata") end @@ -69,7 +69,7 @@ class EditionEditTest < IntegrationTest context "when state is not 'draft'" do setup do - visit_edition_in_published + visit_published_edition click_link("Metadata") end @@ -89,7 +89,7 @@ class EditionEditTest < IntegrationTest context "do not have required permissions" do setup do login_as(FactoryBot.create(:user, name: "Stub User")) - visit_edition_in_draft + visit_draft_edition end should "not show unpublish tab when user is not govuk editor" do @@ -100,12 +100,12 @@ class EditionEditTest < IntegrationTest context "has required permissions" do setup do login_as(FactoryBot.create(:user, :govuk_editor, name: "Stub User")) - visit_edition_in_draft + visit_draft_edition end context "when state is 'published'" do setup do - visit_edition_in_published + visit_published_edition click_link("Unpublish") end @@ -149,7 +149,7 @@ class EditionEditTest < IntegrationTest context "do not have required permissions" do setup do login_as(FactoryBot.create(:user, name: "Stub User")) - visit_edition_in_draft + visit_draft_edition end should "not show when user is not govuk editor or welsh editor" do @@ -158,7 +158,7 @@ class EditionEditTest < IntegrationTest should "not show when user is welsh editor and edition is not welsh" do login_as(FactoryBot.create(:user, :welsh_editor, name: "Stub User")) - visit_edition_in_draft + visit_draft_edition assert page.has_no_text?("Admin") end @@ -169,27 +169,53 @@ class EditionEditTest < IntegrationTest login_as(FactoryBot.create(:user, :govuk_editor, name: "Stub User")) end - context "when state is not 'fact_check'" do - setup do - visit_edition_in_draft - click_link("Admin") + %i[published archived scheduled_for_publishing].each do |state| + context "when state is '#{state}'" do + setup do + send "visit_#{state}_edition" + click_link("Admin") + end + + should "show 'Admin' header and not show 'Skip fact check' button" do + within :css, ".gem-c-heading" do + assert page.has_text?("Admin") + end + assert page.has_no_button?("Skip fact check") + end + + should "not show link to delete edition" do + assert page.has_no_link?("Delete edition") + end end + end - should "show 'Admin' header and not show 'Skip fact check' button" do - within :css, ".gem-c-heading" do - assert page.has_text?("Admin") + %i[draft amends_needed in_review fact_check_received ready].each do |state| + context "when state is '#{state}'" do + setup do + send "visit_#{state}_edition" + click_link("Admin") + end + + should "show 'Admin' header and not show 'Skip fact check' button" do + within :css, ".gem-c-heading" do + assert page.has_text?("Admin") + end + assert page.has_no_button?("Skip fact check") + end + + should "show link to delete edition" do + assert page.has_link?("Delete edition") end - assert page.has_no_button?("Skip fact check") end end context "when state is 'fact_check'" do setup do - visit_edition_in_fact_check + visit_fact_check_edition click_link("Admin") end - should "show tab when user is welsh editor and edition is welsh edition" do + should "show 'Admin' tab when user is welsh editor and edition is welsh edition" do login_as(FactoryBot.create(:user, :welsh_editor, name: "Stub User")) welsh_edition = FactoryBot.create(:edition, :fact_check, :welsh) visit edition_path(welsh_edition) @@ -204,6 +230,10 @@ class EditionEditTest < IntegrationTest assert page.has_button?("Skip fact check") end + should "show link to delete edition" do + assert page.has_link?("Delete edition") + end + should "show success message when fact check skipped successfully" do click_button("Skip fact check") @fact_check_edition.reload @@ -222,13 +252,49 @@ class EditionEditTest < IntegrationTest assert page.has_text?("Could not skip fact check for this publication.") end end + + context "confirm delete" do + setup do + visit_draft_edition + click_link("Admin") + click_link("Delete edition #{@draft_edition.version_number}") + end + + should "show the delete edition confirmation page" do + assert page.has_text?(@draft_edition.title) + assert page.has_text?("Delete edition") + assert page.has_text?("If you delete this edition it cannot be undone.") + assert page.has_text?("Are you sure you want to delete this edition?") + assert page.has_link?("Cancel") + assert page.has_button?("Delete edition") + end + + should "navigate to admin tab when 'Cancel' is clicked" do + click_link("Cancel") + + assert_current_path admin_edition_path(@draft_edition.id) + end + + should "navigate to root path when 'Delete edition' is clicked" do + click_button("Delete edition") + + assert_current_path root_path + end + + should "show success message when edition is successfully deleted" do + click_button("Delete edition") + + assert_equal 0, Edition.where(id: @draft_edition.id).count + assert page.has_text?("Edition deleted") + end + end end end context "edit tab" do context "draft edition of a new publication" do setup do - visit_edition_in_draft + visit_draft_edition end should "show 'Metadata' header and an update button" do @@ -300,40 +366,67 @@ class EditionEditTest < IntegrationTest private - def visit_edition_in_draft - @draft_edition = FactoryBot.create( - :answer_edition, - title: "Edit page title", - state: "draft", - overview: "metatags", - in_beta: 1, - body: "The body", - ) + def visit_draft_edition + @draft_edition = FactoryBot.create(:edition, title: "Edit page title", state: "draft", overview: "metatags", in_beta: 1, body: "The body") visit edition_path(@draft_edition) end - def visit_edition_in_published - @published_edition = FactoryBot.create( - :edition, - title: "Edit page title", - panopticon_id: FactoryBot.create( - :artefact, - slug: "can-i-get-a-driving-licence", - ).id, - state: "published", - slug: "can-i-get-a-driving-licence", - ) + def visit_published_edition + create_published_edition visit edition_path(@published_edition) end - def visit_edition_in_fact_check + def visit_fact_check_edition @fact_check_edition = FactoryBot.create(:edition, title: "Edit page title", state: "fact_check") visit edition_path(@fact_check_edition) end + def visit_scheduled_for_publishing_edition + @scheduled_for_publishing_edition = FactoryBot.create(:edition, title: "Edit page title", state: "scheduled_for_publishing", publish_at: Time.zone.now + 1.hour) + visit edition_path(@scheduled_for_publishing_edition) + end + + def visit_archived_edition + @archived_edition = FactoryBot.create(:edition, title: "Edit page title", state: "archived") + visit edition_path(@archived_edition) + end + + def visit_in_review_edition + @in_review_edition = FactoryBot.create(:edition, title: "Edit page title", state: "in_review", review_requested_at: 1.hour.ago) + visit edition_path(@in_review_edition) + end + + def visit_amends_needed_edition + @amends_needed_edition = FactoryBot.create(:edition, title: "Edit page title", state: "amends_needed") + visit edition_path(@amends_needed_edition) + end + + def visit_fact_check_received_edition + @fact_check_received_edition = FactoryBot.create(:edition, title: "Edit page title", state: "fact_check_received") + visit edition_path(@fact_check_received_edition) + end + + def visit_ready_edition + @ready_edition = FactoryBot.create(:edition, title: "Edit page title", state: "ready") + visit edition_path(@ready_edition) + end + def visit_new_edition_of_published_edition - published_edition = FactoryBot.create( + create_published_edition + new_edition = FactoryBot.create( :answer_edition, + panopticon_id: @published_edition.artefact.id, + state: "draft", + version_number: 2, + change_note: "The change note", + ) + visit edition_path(new_edition) + end + + def create_published_edition + @published_edition = FactoryBot.create( + :edition, + title: "Edit page title", panopticon_id: FactoryBot.create( :artefact, slug: "can-i-get-a-driving-licence", @@ -341,13 +434,5 @@ def visit_new_edition_of_published_edition state: "published", slug: "can-i-get-a-driving-licence", ) - new_edition = FactoryBot.create( - :answer_edition, - panopticon_id: published_edition.artefact.id, - state: "draft", - version_number: 2, - change_note: "The change note", - ) - visit edition_path(new_edition) end end