diff --git a/app/controllers/admin/statistics_announcements_controller.rb b/app/controllers/admin/statistics_announcements_controller.rb index 9c40e8ba98b..536201eefd9 100644 --- a/app/controllers/admin/statistics_announcements_controller.rb +++ b/app/controllers/admin/statistics_announcements_controller.rb @@ -32,7 +32,10 @@ def edit; end def update @statistics_announcement.assign_attributes(statistics_announcement_params) + require_publication_update = @statistics_announcement.publication && @statistics_announcement.publication_type_id_changed? + if @statistics_announcement.save + Whitehall.edition_services.draft_updater(@statistics_announcement.publication, { current_user: }).perform! if require_publication_update redirect_to [:admin, @statistics_announcement], notice: "Announcement updated successfully" else render :edit diff --git a/app/models/statistics_announcement.rb b/app/models/statistics_announcement.rb index c432943679a..759f4d4e83c 100644 --- a/app/models/statistics_announcement.rb +++ b/app/models/statistics_announcement.rb @@ -22,15 +22,19 @@ def can_publish_to_publishing_api? belongs_to :creator, class_name: "User" belongs_to :cancelled_by, class_name: "User" - belongs_to :publication - - has_one :current_release_date, - lambda { - joins(:statistics_announcement) - .where("statistics_announcements.current_release_date_id = statistics_announcement_dates.id") - }, - class_name: "StatisticsAnnouncementDate", - inverse_of: :statistics_announcement + belongs_to :publication, autosave: true, validate: false + validates_associated :publication, if: :publication, + message: lambda { |_, publication| + "type #{publication[:value].errors[:publication_type_id].first}" + } + + has_one :current_release_date, + lambda { + joins(:statistics_announcement) + .where("statistics_announcements.current_release_date_id = statistics_announcement_dates.id") + }, + class_name: "StatisticsAnnouncementDate", + inverse_of: :statistics_announcement has_many :statistics_announcement_dates, -> { order(created_at: :asc, id: :asc) }, dependent: :destroy @@ -38,11 +42,7 @@ def can_publish_to_publishing_api? has_many :statistics_announcement_organisations, inverse_of: :statistics_announcement, dependent: :destroy has_many :organisations, through: :statistics_announcement_organisations - validates_associated :publication, if: :publication, - message: lambda { |_, publication| - "type #{publication[:value].errors[:publication_type_id].first}" - } - validate :redirect_not_circular, if: :unpublished? + validate :redirect_not_circular, if: :unpublished? validates :publishing_state, inclusion: %w[published unpublished] validates :redirect_url, presence: { message: "must be provided when unpublishing an announcement" }, if: :unpublished? validates :redirect_url, uri: true, allow_blank: true @@ -95,6 +95,7 @@ def can_publish_to_publishing_api? after_touch :publish_redirect_to_publication, if: :publication_has_been_published? set_callback :published, :after, :after_publish + before_validation :update_associated_publication_type, on: :update, if: :publication_type_id_changed? after_commit :notify_unpublished, if: :unpublished? def notify_unpublished @@ -124,7 +125,7 @@ def self.without_published_publication def self.with_topics(topic_ids) joins(:statistics_announcement_topics) - .where(statistics_announcement_topics: { topic_id: topic_ids }) + .where(statistics_announcement_topics: { topic_id: topic_ids }) end def last_change_note @@ -233,6 +234,10 @@ def update_current_release_date private + def update_associated_publication_type + publication.publication_type_id = publication_type_id if publication + end + def publication_has_been_published? publication && publication.published? end diff --git a/app/views/admin/statistics_announcements/_form.html.erb b/app/views/admin/statistics_announcements/_form.html.erb index a0968ebdf06..4eed6aecc0b 100644 --- a/app/views/admin/statistics_announcements/_form.html.erb +++ b/app/views/admin/statistics_announcements/_form.html.erb @@ -5,6 +5,7 @@ heading_size: "l", name: "statistics_announcement[publication_type_id]", id: "statistics_announcement_publication_type_id", + hint: statistics_announcement.new_record? ? nil : "Please note that changing the statistics type will also automatically update the type of the connected document.", items: [ { value: 5, diff --git a/test/functional/admin/statistics_announcements_controller_test.rb b/test/functional/admin/statistics_announcements_controller_test.rb index 64d18fa2a83..5c7954c06ce 100644 --- a/test/functional/admin/statistics_announcements_controller_test.rb +++ b/test/functional/admin/statistics_announcements_controller_test.rb @@ -9,13 +9,7 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase stub_taxonomy_with_world_taxons end - view_test "GET :new renders a new announcement form" do - get :new - - assert_response :success - assert_select "input[name='statistics_announcement[title]']" - end - + # ==== GET :index ==== test "GET :index defaults to future-dated announcements by the current user's organisation" do @future_announcement = create( :statistics_announcement, @@ -41,6 +35,16 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_response :success end + # ==== GET :new ==== + view_test "GET :new renders a new announcement form" do + get :new + + assert_response :success + assert_select "input[name='statistics_announcement[title]']" + refute_select "#statistics_announcement_publication_type_id > fieldset > .govuk-hint" + end + + # ==== POST :create ==== test "POST :create saves the announcement to the database and redirects to the dashboard with provisional date" do post :create, params: { @@ -99,6 +103,7 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_not StatisticsAnnouncement.any? end + # ==== GET :show ==== view_test "GET :show renders the details of the announcement" do announcement = create(:statistics_announcement) stub_publishing_api_expanded_links_with_taxons(announcement.content_id, []) @@ -108,14 +113,69 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_select "h1.govuk-heading-xl", text: announcement.title end + view_test "GET :show renders when announcement is not tagged to the new taxonomy" do + sfa_organisation = create(:organisation, content_id: "3e5a6924-b369-4eb3-8b06-3c0814701de4") + + announcement = create( + :statistics_announcement, + organisations: [sfa_organisation], + ) + + login_as(:user) + + announcement_has_no_expanded_links(announcement.content_id) + get :show, params: { id: announcement } + + assert_select ".govuk-warning-text", /You need to add topic tags before you can publish this document./ + refute_select ".govuk-breadcrumbs__list" + end + + view_test "GET :show renders when announcement is tagged to the new taxonomy" do + sfa_organisation = create(:organisation, content_id: "3e5a6924-b369-4eb3-8b06-3c0814701de4") + + announcement = create( + :statistics_announcement, + organisations: [sfa_organisation], + ) + + login_as(:user) + + announcement_has_expanded_links(announcement.content_id) + + get :show, params: { id: announcement } + + refute_select ".govuk-warning-text" + assert_select ".govuk-breadcrumbs__list-item", "Education, Training and Skills" + assert_select ".govuk-breadcrumbs__list-item", "Primary Education" + end + + view_test "GET :show show a link to tag to the new taxonomy" do + dfe_organisation = create(:organisation, content_id: "ebd15ade-73b2-4eaf-b1c3-43034a42eb37") + + announcement = create( + :statistics_announcement, + organisations: [dfe_organisation], + ) + + login_as(:user) + + announcement_has_no_expanded_links(announcement.content_id) + get :show, params: { id: announcement } + + assert_select "a[href='#{edit_admin_statistics_announcement_tags_path(announcement.id)}']", "Add tags" + end + + # ==== GET :edit ==== view_test "GET :edit renders the edit form for the announcement" do announcement = create(:statistics_announcement) get :edit, params: { id: announcement.id } assert_response :success assert_select "input[name='statistics_announcement[title]'][value='#{announcement.title}']" + assert_select "#statistics_announcement_publication_type_id .govuk-hint", "Please note that changing the statistics type will also automatically update the type of the connected document." end + # ==== PUT :update ==== test "PUT :update saves changes to the announcement" do announcement = create(:statistics_announcement) put :update, params: { id: announcement.id, statistics_announcement: { title: "New announcement title" } } @@ -132,6 +192,89 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_select "ul.govuk-error-summary__list a", text: "Title can't be blank" end + test "PUT :update should update connected draft publication in Publishing API if announcement publication type has been changed" do + national_statistics = create(:draft_national_statistics, lead_organisations: [@organisation]) + announcement = create( + :statistics_announcement, + publication_type_id: PublicationType::NationalStatistics.id, + publication: national_statistics, + organisation_ids: [@organisation.id], + ) + + announcement_presenter = PublishingApiPresenters.presenter_for(announcement) + announcement_content = announcement_presenter.content.merge( + title: "New title", + public_updated_at: Time.zone.now.as_json, + ) + + WebMock.reset! + + expected_requests = [ + stub_publishing_api_patch_links(announcement.content_id, links: announcement_presenter.links), + stub_publishing_api_put_content(announcement.content_id, with_locale(:en) { announcement_content }), + stub_publishing_api_publish(announcement.content_id, locale: "en", update_type: nil), + ] + + put :update, params: { + id: announcement.id, + statistics_announcement: { + title: "New title", + publication_type_id: PublicationType::NationalStatistics.id, + }, + } + + assert_all_requested(expected_requests) + end + + test "PUT :update should not update connected draft publication in Publishing API if announcement publication type has not been changed" do + national_statistics = create(:draft_national_statistics, lead_organisations: [@organisation]) + announcement = create( + :statistics_announcement, + publication_type_id: PublicationType::NationalStatistics.id, + publication: national_statistics, + organisation_ids: [@organisation.id], + ) + + publication_presenter = PublishingApiPresenters.presenter_for(national_statistics) + publication_content = publication_presenter.content.merge( + document_type: "official_statistics", + ) + html_attachment_presenter = PublishingApiPresenters.presenter_for(national_statistics.attachments.first) + + announcement_presenter = PublishingApiPresenters.presenter_for(announcement) + announcement_details = announcement_presenter.content[:details].merge( + format_sub_type: "official", + ) + announcement_content = announcement_presenter.content.merge( + title: "New title", + document_type: "official_statistics_announcement", + details: announcement_details, + public_updated_at: Time.zone.now.as_json, + ) + + WebMock.reset! + + expected_requests = [ + stub_publishing_api_patch_links(publication_presenter.content_id, links: publication_presenter.links), + stub_publishing_api_put_content(publication_presenter.content_id, with_locale(:en) { publication_content }), + stub_publishing_api_put_content(html_attachment_presenter.content_id, html_attachment_presenter.content), + stub_publishing_api_patch_links(announcement.content_id, links: announcement_presenter.links), + stub_publishing_api_put_content(announcement.content_id, with_locale(:en) { announcement_content }), + stub_publishing_api_publish(announcement.content_id, locale: "en", update_type: nil), + ] + + put :update, params: { + id: announcement.id, + statistics_announcement: { + title: "New title", + publication_type_id: PublicationType::OfficialStatistics.id, + }, + } + + assert_all_requested(expected_requests) + end + + # ==== POST :publish_cancellation ==== test "POST :publish_cancellation cancels the announcement" do announcement = create(:statistics_announcement) post :publish_cancellation, @@ -146,6 +289,20 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_equal Time.zone.now, announcement.cancelled_at end + test "POST :publish_cancellation cannot cancel cancelled announcements" do + announcement = create(:cancelled_statistics_announcement) + + get :cancel, params: { id: announcement } + assert_redirected_to [:admin, announcement] + + post :publish_cancellation, + params: { + id: announcement.id, + statistics_announcement: { cancellation_reason: "Reason" }, + } + assert_redirected_to [:admin, announcement] + end + view_test "POST :publish_cancellation re-renders cancellation form if changes are not valid" do announcement = create(:statistics_announcement) post :publish_cancellation, @@ -159,6 +316,7 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_select "ul.govuk-error-summary__list a", text: "Cancellation reason must be provided when cancelling an announcement" end + # ==== PATCH :update_cancel_reason ==== test "PATCH :update_cancel_reason updates the cancellation message" do announcement = create(:cancelled_statistics_announcement) patch :update_cancel_reason, @@ -185,72 +343,6 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_select "ul.govuk-error-summary__list a", text: "Cancellation reason must be provided when cancelling an announcement" end - test "cancelled announcements cannot be cancelled" do - announcement = create(:cancelled_statistics_announcement) - - get :cancel, params: { id: announcement } - assert_redirected_to [:admin, announcement] - - post :publish_cancellation, - params: { - id: announcement.id, - statistics_announcement: { cancellation_reason: "Reason" }, - } - assert_redirected_to [:admin, announcement] - end - - view_test "show a link to tag to the new taxonomy" do - dfe_organisation = create(:organisation, content_id: "ebd15ade-73b2-4eaf-b1c3-43034a42eb37") - - announcement = create( - :statistics_announcement, - organisations: [dfe_organisation], - ) - - login_as(:user) - - announcement_has_no_expanded_links(announcement.content_id) - get :show, params: { id: announcement } - - assert_select "a[href='#{edit_admin_statistics_announcement_tags_path(announcement.id)}']", "Add tags" - end - - view_test "when announcement is not tagged to the new taxonomy" do - sfa_organisation = create(:organisation, content_id: "3e5a6924-b369-4eb3-8b06-3c0814701de4") - - announcement = create( - :statistics_announcement, - organisations: [sfa_organisation], - ) - - login_as(:user) - - announcement_has_no_expanded_links(announcement.content_id) - get :show, params: { id: announcement } - - assert_select ".govuk-warning-text", /You need to add topic tags before you can publish this document./ - refute_select ".govuk-breadcrumbs__list" - end - - view_test "when announcement is tagged to the new taxonomy" do - sfa_organisation = create(:organisation, content_id: "3e5a6924-b369-4eb3-8b06-3c0814701de4") - - announcement = create( - :statistics_announcement, - organisations: [sfa_organisation], - ) - - login_as(:user) - - announcement_has_expanded_links(announcement.content_id) - - get :show, params: { id: announcement } - - refute_select ".govuk-warning-text" - assert_select ".govuk-breadcrumbs__list-item", "Education, Training and Skills" - assert_select ".govuk-breadcrumbs__list-item", "Primary Education" - end - private def announcement_has_no_expanded_links(content_id) diff --git a/test/unit/app/models/statistics_announcement_test.rb b/test/unit/app/models/statistics_announcement_test.rb index f9382abb9a4..69d978639f2 100644 --- a/test/unit/app/models/statistics_announcement_test.rb +++ b/test/unit/app/models/statistics_announcement_test.rb @@ -80,28 +80,6 @@ class StatisticsAnnouncementTest < ActiveSupport::TestCase assert_equal expected_indexed_content, announcement.search_index end - test "only valid when associated publication is of a matching type" do - statistics = create(:draft_statistics) - national_statistics = create(:draft_national_statistics) - policy_paper = create(:draft_policy_paper) - - announcement = build(:statistics_announcement, publication_type_id: PublicationType::OfficialStatistics.id) - - announcement.publication = statistics - assert announcement.valid? - - announcement.publication = national_statistics - assert_not announcement.valid? - assert_equal ["type does not match announcement type: must be 'Official Statistics'"], announcement.errors[:publication] - - announcement.publication_type_id = PublicationType::NationalStatistics.id - assert announcement.valid? - - announcement.publication = policy_paper - assert_not announcement.valid? - assert_equal ["type does not match announcement type: must be 'Accredited Official Statistics'"], announcement.errors[:publication] - end - test ".with_title_containing returns statistics announcements matching provided title" do match = create(:statistics_announcement, title: "MQ5 statistics") create(:statistics_announcement, title: "PQ6 statistics") @@ -292,6 +270,104 @@ class StatisticsAnnouncementTest < ActiveSupport::TestCase assert_equal statistics_announcement_date3.id, statistics_announcement.reload.current_release_date_id end + test "only valid when associated publication is of a matching type" do + statistics = create(:draft_statistics) + national_statistics = create(:draft_national_statistics) + policy_paper = create(:draft_policy_paper) + + announcement = build(:statistics_announcement, publication_type_id: PublicationType::OfficialStatistics.id) + + announcement.publication = statistics + assert announcement.valid? + + announcement.publication = national_statistics + assert_not announcement.valid? + assert_equal ["type does not match announcement type: must be 'Official Statistics'"], announcement.errors[:publication] + + announcement.publication_type_id = PublicationType::NationalStatistics.id + assert announcement.valid? + + announcement.publication = policy_paper + assert_not announcement.valid? + assert_equal ["type does not match announcement type: must be 'Accredited Official Statistics'"], announcement.errors[:publication] + end + + test "deleting statistics announcement does not delete publication" do + national_statistics = create(:draft_national_statistics, title: "Test") + announcement = create(:statistics_announcement, publication_type_id: PublicationType::NationalStatistics.id, publication: national_statistics) + + announcement.delete + assert_equal Publication.find(national_statistics.id).title, "Test" + assert_equal StatisticsAnnouncement.where(id: announcement.id).count, 0 + end + + # === BEGIN: Publication Type update callback === + test "should update publication type when there's no connected publication" do + announcement = create(:statistics_announcement, publication_type_id: PublicationType::OfficialStatistics.id) + + announcement.publication_type_id = PublicationType::NationalStatistics.id + announcement.save! + + assert announcement.valid? + assert announcement.errors.empty? + end + + test "should update with same publication type when there's a connected draft publication" do + national_statistics = create(:draft_national_statistics) + announcement = create(:statistics_announcement, publication_type_id: PublicationType::NationalStatistics.id, publication: national_statistics) + + announcement.assign_attributes( + publication_type_id: PublicationType::NationalStatistics.id, + title: "New title", + ) + announcement.save! + + assert announcement.valid? + assert announcement.errors.empty? + assert_equal PublicationType::NationalStatistics.id, national_statistics.reload.publication_type_id + end + + test "should update with a different publication type and update publication when there's a connected draft publication" do + national_statistics = create(:draft_national_statistics) + announcement = create(:statistics_announcement, publication_type_id: PublicationType::NationalStatistics.id, publication: national_statistics) + + announcement.assign_attributes( + publication_type_id: PublicationType::OfficialStatistics.id, + title: "New title", + ) + announcement.save! + + assert announcement.valid? + assert announcement.errors.empty? + assert_equal PublicationType::OfficialStatistics.id, national_statistics.reload.publication_type_id + end + + test "should not create new announcement with publication of mismatched Publication type" do + national_statistics = create(:draft_national_statistics) + announcement = build(:statistics_announcement, publication_type_id: PublicationType::OfficialStatistics.id, publication: national_statistics) + + assert_not announcement.save + assert_equal 1, announcement.errors.count + assert announcement.errors[:publication].include? "type does not match announcement type: must be 'Official Statistics'" + assert_equal PublicationType::NationalStatistics.id, national_statistics.reload.publication_type_id + end + + test "should not update Publication type nor update publication when connected publication is published" do + national_statistics = create(:published_national_statistics) + announcement = create(:statistics_announcement, publication_type_id: PublicationType::NationalStatistics.id, publication: national_statistics) + + announcement.assign_attributes( + publication_type_id: PublicationType::OfficialStatistics.id, + title: "New title", + ) + + assert_not announcement.save + assert_equal 1, announcement.errors.count + assert_equal announcement.errors.first.full_message, "Publication type cannot be modified when edition is in the published state" + assert_equal PublicationType::NationalStatistics.id, national_statistics.reload.publication_type_id + end + # === END: Publication Type update callback === + private def create_announcement_with_changes