diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index f084f9bd0..c14949e5f 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -16,6 +16,7 @@ $govuk-page-width: 1140px; @import 'govuk_publishing_components/components/heading'; @import 'govuk_publishing_components/components/hint'; @import 'govuk_publishing_components/components/input'; +@import 'govuk_publishing_components/components/inset-text'; @import 'govuk_publishing_components/components/label'; @import 'govuk_publishing_components/components/layout-footer'; @import 'govuk_publishing_components/components/layout-for-admin'; diff --git a/app/controllers/editions_controller.rb b/app/controllers/editions_controller.rb index 349b791db..a52958e33 100644 --- a/app/controllers/editions_controller.rb +++ b/app/controllers/editions_controller.rb @@ -3,10 +3,14 @@ class EditionsController < InheritedResources::Base layout "design_system" defaults resource_class: Edition, collection_name: "editions", instance_name: "resource" + before_action :setup_view_paths, except: %i[index] before_action except: %i[index] do require_user_accessibility_to_edition(@resource) end + before_action only: %i[unpublish confirm_unpublish process_unpublish] do + require_govuk_editor(redirect_path: edition_path(resource)) + end helper_method :locale_to_language @@ -21,6 +25,7 @@ def show end alias_method :metadata, :show + alias_method :unpublish, :show def history render action: "show" @@ -34,8 +39,30 @@ def linking render action: "show" end - def unpublish - render action: "show" + def confirm_unpublish + if redirect_url.blank? || validate_redirect(redirect_url) + render "secondary_nav_tabs/confirm_unpublish" + else + error_message = "Redirect path is invalid. #{description(resource)} can not be unpublished." + @resource.errors.add(:redirect_url, error_message) + render "show" + end + end + + def process_unpublish + artefact = @resource.artefact + + success = unpublish_edition(artefact) + if success + notice = "Content unpublished" + notice << " and redirected" if redirect_url.present? + flash[:success] = notice + redirect_to root_path + else + render_confirm_page_with_error + end + rescue StandardError + render_confirm_page_with_error end protected @@ -46,6 +73,19 @@ def setup_view_paths private + def unpublish_edition(artefact) + params["redirect_url"].strip.empty? ? UnpublishService.call(artefact, current_user) : UnpublishService.call(artefact, current_user, redirect_url) + end + + def render_confirm_page_with_error + @resource.errors.add(:unpublish, downstream_error_message) + render "secondary_nav_tabs/confirm_unpublish" + end + + def downstream_error_message + "Due to a service problem, the edition couldn't be unpublished" + end + def setup_view_paths_for(publication) prepend_view_path "app/views/editions" prepend_view_path template_folder_for(publication) @@ -61,4 +101,21 @@ def locale_to_language(locale) "" end end + + def validate_redirect(redirect_url) + regex = /(\/([a-z0-9]+-)*[a-z0-9]+)+/ + redirect_url =~ regex + end + + def make_govuk_url_relative(url = "") + url.sub(%r{^(https?://)?(www\.)?gov\.uk/}, "/") + end + + def redirect_url + make_govuk_url_relative params["redirect_url"] + end + + def description(resource) + resource.format.underscore.humanize + end end diff --git a/app/helpers/common_components_helper.rb b/app/helpers/common_components_helper.rb new file mode 100644 index 000000000..35e63a300 --- /dev/null +++ b/app/helpers/common_components_helper.rb @@ -0,0 +1,36 @@ +module CommonComponentsHelper + def header_for(tab_name) + render "govuk_publishing_components/components/heading", { + text: tab_name, + heading_level: 2, + margin_bottom: 5, + } + end + + def primary_button_for(model, url, text) + form_for model, url:, method: :post do + render "govuk_publishing_components/components/button", { + text:, + margin_bottom: 3, + } + end + end + + def secondary_button_for(model, url, text) + form_for model, url:, method: :post do + render "govuk_publishing_components/components/button", { + text:, + margin_bottom: 3, + secondary_solid: true, + } + end + end + + def primary_link_button_for(url, text) + render "govuk_publishing_components/components/button", { + text:, + href: url, + margin_bottom: 3, + } + end +end diff --git a/app/helpers/popular_links_helper.rb b/app/helpers/popular_links_helper.rb index 0bc89bb71..6c19a09ef 100644 --- a/app/helpers/popular_links_helper.rb +++ b/app/helpers/popular_links_helper.rb @@ -5,31 +5,4 @@ def popular_link_rows(item) rows << { key: "URL", value: item[:url] } rows.compact end - - def primary_button_for(model, url, text) - form_for model, url:, method: :post do - render "govuk_publishing_components/components/button", { - text:, - margin_bottom: 3, - } - end - end - - def secondary_button_for(model, url, text) - form_for model, url:, method: :post do - render "govuk_publishing_components/components/button", { - text:, - margin_bottom: 3, - secondary_solid: true, - } - end - end - - def primary_link_button_for(url, text) - render "govuk_publishing_components/components/button", { - text:, - href: url, - margin_bottom: 3, - } - end end diff --git a/app/helpers/tabbed_nav_helper.rb b/app/helpers/tabbed_nav_helper.rb index 13d95b6e8..a174cf782 100644 --- a/app/helpers/tabbed_nav_helper.rb +++ b/app/helpers/tabbed_nav_helper.rb @@ -3,6 +3,8 @@ def edition_nav_items(edition) nav_items = [] all_tab_names.each do |item| + next if !edition.state.eql?("published") && item == "unpublish" + nav_items << standard_nav_items(item, edition) end @@ -31,7 +33,15 @@ def edit_nav_item(label, href, current) def current_tab_name current_tab = (request.path.split("/") & all_tab_names).first - current_tab == "metadata" ? "metadata" : "temp_nav_text" + + case current_tab + when "metadata" + "metadata" + when "unpublish" + "unpublish" + else + "temp_nav_text" + end end private diff --git a/app/models/edition.rb b/app/models/edition.rb index 5816bf858..ca584a9aa 100644 --- a/app/models/edition.rb +++ b/app/models/edition.rb @@ -343,7 +343,7 @@ def cloning_between_parted_types?(new_edition) def self.find_or_create_from_panopticon_data(panopticon_id, importing_user) existing_publication = Edition.where(panopticon_id:) - .order_by(version_number: :desc).first + .order_by(version_number: :desc).first return existing_publication if existing_publication metadata = Artefact.find(panopticon_id) @@ -422,8 +422,9 @@ def check_for_archived_artefact a = Artefact.find(panopticon_id) if (a.state == "archived") && changes.any? # If we're only changing the state to archived, that's ok - # Any other changes are not allowed - allowed_keys = %w[state updated_at] + # We have to add owning_org_content_ids as Mongo changes the + # value from nil to an empty array on save - reads as a change + allowed_keys = %w[state updated_at owning_org_content_ids] unless (changes.keys - allowed_keys).empty? && (state == "archived") raise "Editing of an edition with an Archived artefact is not allowed" end diff --git a/app/views/editions/secondary_nav_tabs/_metadata.html.erb b/app/views/editions/secondary_nav_tabs/_metadata.html.erb index 65c972f9b..50b56e9bc 100644 --- a/app/views/editions/secondary_nav_tabs/_metadata.html.erb +++ b/app/views/editions/secondary_nav_tabs/_metadata.html.erb @@ -1,10 +1,6 @@
- <%= render "govuk_publishing_components/components/heading", { - text: "Metadata", - heading_level: 2, - margin_bottom: 5, - } %> + <%= header_for("Metadata") %> <% if Edition::PUBLISHING_API_DRAFT_STATES.include? publication.state %> <%= form_for(@artefact, :html => { :class => "artefact", :id => "edit_artefact" }) do |f| %> diff --git a/app/views/editions/secondary_nav_tabs/_unpublish.html.erb b/app/views/editions/secondary_nav_tabs/_unpublish.html.erb new file mode 100644 index 000000000..cd89c2f71 --- /dev/null +++ b/app/views/editions/secondary_nav_tabs/_unpublish.html.erb @@ -0,0 +1,31 @@ +
+
+ <% @edition = @resource %> + <%= header_for("Unpublish") %> + + <%= render "govuk_publishing_components/components/inset_text", { + text: "If you unpublish a page from GOV.UK it cannot be undone.", + } %> + + <%= form_for @edition, url: confirm_unpublish_edition_path, method: "get" do |f| %> + <%= render "govuk_publishing_components/components/input", { + label: { + text: "Redirect to URL", + }, + id: "redirect_url", + name: "redirect_url", + value: params[:redirect_url], + hint: "For example: https://www.gov.uk/redirect-to-replacement-page", + heading_level: 3, + heading_size: "s", + error_items: errors_for(f.object.errors, :redirect_url, use_full_message: false), + } %> +
+ <%= render("govuk_publishing_components/components/button", { + text: "Continue", + type: "submit", + }) %> +
+ <% 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 new file mode 100644 index 000000000..006172cb9 --- /dev/null +++ b/app/views/editions/secondary_nav_tabs/confirm_unpublish.html.erb @@ -0,0 +1,39 @@ +<% @edition = @resource %> +<% content_for :title_context, @edition.title %> +<% content_for :page_title, "Unpublish" %> +<% content_for :title, "Unpublish" %> +
+ <% 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 unpublish a page from GOV.UK it cannot be undone.", + } %> + + <%= form_for @edition, url: process_unpublish_edition_path(@edition), method: :post do %> + <%= hidden_field_tag :redirect_url, params[:redirect_url] %> +

Are you sure you want to unpublish this document?

+
+ <%= render "govuk_publishing_components/components/button", { + text: "Unpublish document", + destructive: true, + } %> + <%= link_to("Cancel", unpublish_edition_path, class: "govuk-link govuk-link--no-visited-state") %> +
+ <% end %> +
+
+
diff --git a/app/views/editions/show.html.erb b/app/views/editions/show.html.erb index 0d1bfc54e..cf004a0ca 100644 --- a/app/views/editions/show.html.erb +++ b/app/views/editions/show.html.erb @@ -3,6 +3,21 @@ <% content_for :page_title, @resource.title %> <% content_for :title, @resource.title %> +<% 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/summary_list", { diff --git a/config/routes.rb b/config/routes.rb index 7e286645a..4b7e8ca9f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -25,6 +25,8 @@ get "related_external_links", to: "editions#linking" get "tagging", to: "editions#linking" get "unpublish" + get "unpublish/confirm-unpublish", to: "editions#confirm_unpublish", as: "confirm_unpublish" + post "process_unpublish" end end end diff --git a/test/functional/editions_controller_test.rb b/test/functional/editions_controller_test.rb index a9791b4cf..e9edb2977 100644 --- a/test/functional/editions_controller_test.rb +++ b/test/functional/editions_controller_test.rb @@ -76,7 +76,7 @@ class EditionsControllerTest < ActionController::TestCase end should "return a 200 when requesting an edition owned by a different organisation" do - login_as(FactoryBot.create(:user, organisation_content_id: "org-one")) + login_as(FactoryBot.create(:user, :govuk_editor, organisation_content_id: "org-one")) get action, params: { id: @edition.id } @@ -113,4 +113,91 @@ class EditionsControllerTest < ActionController::TestCase end end end + + context "#unpublish" do + setup do + artefact = FactoryBot.create( + :artefact, + slug: "test2", + kind: "guide", + name: "test", + owning_app: "publisher", + ) + @guide = GuideEdition.create!(title: "test", slug: "test2", panopticon_id: artefact.id) + end + + should "redirect to edition_path when user does not have govuk-editor permission" do + user = FactoryBot.create(:user, :welsh_editor, name: "Stub User") + login_as(user) + get :unpublish, params: { id: @guide.id } + + assert_redirected_to edition_path(@guide) + end + + context "#confirm_unpublish" do + should "redirect to edition_path when user does not have govuk-editor permission" do + user = FactoryBot.create(:user, :welsh_editor, name: "Stub User") + login_as(user) + get :confirm_unpublish, params: { id: @guide.id } + + assert_redirected_to edition_path(@guide) + end + + should "render 'confirm_unpublish' template if redirect url is blank" do + get :confirm_unpublish, params: { id: @guide.id, redirect_url: "" } + + assert_template "secondary_nav_tabs/confirm_unpublish" + end + + should "render 'confirm_unpublish' template if redirect url is a valid url" do + get :confirm_unpublish, params: { id: @guide.id, redirect_url: "https://www.gov.uk/redirect-to-replacement-page" } + + assert_template "secondary_nav_tabs/confirm_unpublish" + end + + should "render show template with error message when redirect url is not valid" do + get :confirm_unpublish, params: { id: @guide.id, redirect_url: "bob" } + + assert_select ".gem-c-error-summary__list-item", "Redirect path is invalid. Guide can not be unpublished." + assert_template "show" + end + end + + context "#process_unpublish" do + should "redirect to edition_path when user does not have govuk-editor permission" do + user = FactoryBot.create(:user, :welsh_editor, name: "Stub User") + login_as(user) + get :confirm_unpublish, params: { id: @guide.id, redirect_url: nil } + + assert_redirected_to edition_path(@guide) + end + + should "show success message and redirect to root path when unpublished successfully with redirect url" do + get :process_unpublish, params: { id: @guide.id, redirect_url: "https://www.gov.uk/redirect-to-replacement-page" } + + assert_equal "Content unpublished and redirected", flash[:success] + end + + should "show success message and redirect to root path when unpublished successfully without redirect url" do + UnpublishService.stubs(:call).returns(true) + get :process_unpublish, params: { id: @guide.id, redirect_url: nil } + + assert_equal "Content unpublished", flash[:success] + end + + should "show error message when unpublish is unsuccessful" do + UnpublishService.stubs(:call).returns(nil) + get :process_unpublish, params: { id: @guide.id, redirect_url: nil } + + assert_select ".gem-c-error-summary__list-item", "Due to a service problem, the edition couldn't be unpublished" + end + + should "show error message when unpublish service returns an error" do + UnpublishService.stubs(:call).raises(StandardError) + get :process_unpublish, params: { id: @guide.id, redirect_url: nil } + + assert_select ".gem-c-error-summary__list-item", "Due to a service problem, the edition couldn't be unpublished" + end + end + end end diff --git a/test/integration/edition_edit_test.rb b/test/integration/edition_edit_test.rb index 0d06654f2..5b25d7fde 100644 --- a/test/integration/edition_edit_test.rb +++ b/test/integration/edition_edit_test.rb @@ -33,7 +33,10 @@ class EditionEditTest < IntegrationTest assert page.has_text?("History and notes") assert page.has_text?("Admin") assert page.has_text?("Related external links") - assert page.has_text?("Unpublish") + end + + should "not show unpublish tab" do + assert page.has_no_text?("Unpublish") end context "metadata tab" do @@ -67,19 +70,31 @@ class EditionEditTest < IntegrationTest end context "when edition is published" do - context "metadata tab" do - setup do - edition = FactoryBot.create( - :completed_transaction_edition, - panopticon_id: FactoryBot.create( - :artefact, - slug: "can-i-get-a-driving-licence", - ).id, - state: "published", + setup do + @edition = FactoryBot.create( + :completed_transaction_edition, + panopticon_id: FactoryBot.create( + :artefact, slug: "can-i-get-a-driving-licence", - ) + ).id, + state: "published", + slug: "can-i-get-a-driving-licence", + ) + visit edition_path(@edition) + end - visit edition_path(edition) + should "show all the tabs for the published edition" do + assert page.has_text?("Edit") + assert page.has_text?("Tagging") + assert page.has_text?("Metadata") + assert page.has_text?("History and notes") + assert page.has_text?("Admin") + assert page.has_text?("Related external links") + assert page.has_text?("Unpublish") + end + + context "metadata tab" do + setup do click_link("Metadata") end @@ -93,5 +108,54 @@ class EditionEditTest < IntegrationTest assert page.has_text?(/English/) end end + + context "unpublish tab" do + setup do + click_link("Unpublish") + end + + should "show 'Unpublish' header and 'Continue' button" do + within :css, ".gem-c-heading" do + assert page.has_text?("Unpublish") + end + assert page.has_button?("Continue") + end + + should "show 'cannot be undone' banner" do + assert page.has_text?("If you unpublish a page from GOV.UK it cannot be undone.") + end + + should "show 'Redirect to URL' text, input box and example text" do + assert page.has_text?("Redirect to URL") + assert page.has_text?("For example: https://www.gov.uk/redirect-to-replacement-page") + assert page.has_css?(".govuk-input", count: 1) + end + + should "navigate to 'confirm-unpublish' page when clicked on 'Continue' button" do + click_button("Continue") + assert_equal(page.current_path, "/editions/#{@edition.id}/unpublish/confirm-unpublish") + end + end + + context "confirm unpublish" do + setup do + click_link("Unpublish") + click_button("Continue") + end + + should "show 'Unpublish' header and document title" do + assert page.has_text?("Unpublish") + assert page.has_text?(@edition.title.to_s) + end + + should "show 'cannot be undone' banner" do + assert page.has_text?("If you unpublish a page from GOV.UK it cannot be undone.") + end + + should "show 'Unpublish document' button and 'Cancel' link" do + assert page.has_button?("Unpublish document") + assert page.has_link?("Cancel") + end + end end end diff --git a/test/unit/helpers/admin/tabbed_nav_helper_test.rb b/test/unit/helpers/admin/tabbed_nav_helper_test.rb index 36381bd57..d73449e89 100644 --- a/test/unit/helpers/admin/tabbed_nav_helper_test.rb +++ b/test/unit/helpers/admin/tabbed_nav_helper_test.rb @@ -1,9 +1,48 @@ require "test_helper" class TabbedNavHelperTest < ActionView::TestCase - test "#secondary_navigation_tabs_items for edit edition page" do + test "#secondary_navigation_tabs_items for draft edition edit page" do resource = FactoryBot.create(:guide_edition, title: "Edit page title", state: "draft") + expected_output = [ + { + label: "Edit", + href: "/editions/#{resource.id}", + current: false, + }, + { + label: "Tagging", + href: "/editions/#{resource.id}/tagging", + current: false, + }, + { + label: "Metadata", + href: "/editions/#{resource.id}/metadata", + current: false, + }, + { + label: "History and notes", + href: "/editions/#{resource.id}/history", + current: false, + }, + { + label: "Admin", + href: "/editions/#{resource.id}/admin", + current: false, + }, + { + label: "Related external links", + href: "/editions/#{resource.id}/related_external_links", + current: false, + }, + ] + + assert_equal expected_output, edition_nav_items(resource) + end + + test "#secondary_navigation_tabs_items for published edition edit page" do + resource = FactoryBot.create(:guide_edition, title: "Edit page title", state: "published") + expected_output = [ { label: "Edit",