From 53f80c2e926283804740b41a6543635cdcb07d42 Mon Sep 17 00:00:00 2001 From: Minno Dang Date: Mon, 7 Oct 2024 09:56:28 +0100 Subject: [PATCH 1/5] Re-arranging tests for Statistics Announcements Controller Unfortunately adding the MiniTest DSL and describe blocks or context blocks did not work - there was a lot of failures with setups and undefined variables. I've opted for adding comments instead to section out the tests to their respective test methods. --- ...tatistics_announcements_controller_test.rb | 154 +++++++++--------- 1 file changed, 81 insertions(+), 73 deletions(-) diff --git a/test/functional/admin/statistics_announcements_controller_test.rb b/test/functional/admin/statistics_announcements_controller_test.rb index 64d18fa2a83..88e290fbc0d 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,15 @@ 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]']" + 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 +102,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,6 +112,59 @@ 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 } @@ -116,6 +173,7 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_select "input[name='statistics_announcement[title]'][value='#{announcement.title}']" 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 +190,7 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_select "ul.govuk-error-summary__list a", text: "Title can't be blank" end + # ==== POST :publish_cancellation ==== test "POST :publish_cancellation cancels the announcement" do announcement = create(:statistics_announcement) post :publish_cancellation, @@ -146,6 +205,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 +232,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 +259,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) From eed440698462544e1c7963479c8f7c348815c114 Mon Sep 17 00:00:00 2001 From: Minno Dang Date: Wed, 9 Oct 2024 14:49:02 +0100 Subject: [PATCH 2/5] Change Statistics Announcement to automatically update linked publication type Currently users are unable to update an announcement's statistics type, due to the validation on the type of publication blocking this change. We can make it so that any update to the announcement will automatically update the publication type. This should work better for the user's workflow. --- app/models/statistics_announcement.rb | 11 +- .../models/statistics_announcement_test.rb | 118 ++++++++++++++---- 2 files changed, 104 insertions(+), 25 deletions(-) diff --git a/app/models/statistics_announcement.rb b/app/models/statistics_announcement.rb index c432943679a..a83512afc67 100644 --- a/app/models/statistics_announcement.rb +++ b/app/models/statistics_announcement.rb @@ -22,7 +22,7 @@ def can_publish_to_publishing_api? belongs_to :creator, class_name: "User" belongs_to :cancelled_by, class_name: "User" - belongs_to :publication + belongs_to :publication, autosave: true has_one :current_release_date, lambda { @@ -40,8 +40,8 @@ def can_publish_to_publishing_api? validates_associated :publication, if: :publication, message: lambda { |_, publication| - "type #{publication[:value].errors[:publication_type_id].first}" - } + "type #{publication[:value].errors[:publication_type_id].first}" + } 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? @@ -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 @@ -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/test/unit/app/models/statistics_announcement_test.rb b/test/unit/app/models/statistics_announcement_test.rb index f9382abb9a4..2ff084d9be3 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,102 @@ 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 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 announcement.errors[:publication].include? "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 From d1a3253d8bddb3ed64e18701d0963df8ce08a665 Mon Sep 17 00:00:00 2001 From: Minno Dang Date: Wed, 9 Oct 2024 15:11:11 +0100 Subject: [PATCH 3/5] Update draft in publishing API for statistics announcement publication If the publication type has been changed, push draft update. This change is made on the controller, instead of within the before_validation callback like the publication save call because the Publishing API call breaks the transaction within the callback, meaning the publication gets saved before the announcement, resulting in a publication type mismatch error. The publishing API call can only happen once the announcement save has completed sucessfully. We also could have put it in an after_save callback, but since it's already separate from the publication save call, it's probably more explicit to make that call at the controller level rather than putting it within activerecord callbacks. --- .../statistics_announcements_controller.rb | 3 + ...tatistics_announcements_controller_test.rb | 82 +++++++++++++++++++ 2 files changed, 85 insertions(+) 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/test/functional/admin/statistics_announcements_controller_test.rb b/test/functional/admin/statistics_announcements_controller_test.rb index 88e290fbc0d..b6c8b295a67 100644 --- a/test/functional/admin/statistics_announcements_controller_test.rb +++ b/test/functional/admin/statistics_announcements_controller_test.rb @@ -190,6 +190,88 @@ 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) From 75626c0df09e5ad4263a1d5e33d38fc6a8fa6530 Mon Sep 17 00:00:00 2001 From: Minno Dang Date: Wed, 9 Oct 2024 15:53:54 +0100 Subject: [PATCH 4/5] Showing users hint text when updating statistics announcement publication type This will warn them to avoid confusion about why the type has been automatically changed for publications linked to the statistics announcement when they change the announcement type. --- app/views/admin/statistics_announcements/_form.html.erb | 1 + .../admin/statistics_announcements_controller_test.rb | 2 ++ 2 files changed, 3 insertions(+) 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 b6c8b295a67..5c7954c06ce 100644 --- a/test/functional/admin/statistics_announcements_controller_test.rb +++ b/test/functional/admin/statistics_announcements_controller_test.rb @@ -41,6 +41,7 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase assert_response :success assert_select "input[name='statistics_announcement[title]']" + refute_select "#statistics_announcement_publication_type_id > fieldset > .govuk-hint" end # ==== POST :create ==== @@ -171,6 +172,7 @@ class Admin::StatisticsAnnouncementsControllerTest < ActionController::TestCase 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 ==== From 4845f61eba7fb910b8268db405b46c9252b34f79 Mon Sep 17 00:00:00 2001 From: Minno Dang Date: Fri, 11 Oct 2024 16:07:54 +0100 Subject: [PATCH 5/5] Fix error messaging when changing statistics announcement type with a published publication Autosaving as well as validating publication now surfaces the error message for published publication twice. To handle this, I've tried putting on: :create back onto validates_associated for publication but this started failing to catch the scenario where we are trying to connect a document of a wrong type. In the end, the only way to satisfy all test cases, was to force a validate: false on the autosave, and rely entirely on validates_associated --- app/models/statistics_announcement.rb | 30 +++++++++---------- .../models/statistics_announcement_test.rb | 4 ++- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/models/statistics_announcement.rb b/app/models/statistics_announcement.rb index a83512afc67..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, autosave: true - - 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 @@ -125,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 diff --git a/test/unit/app/models/statistics_announcement_test.rb b/test/unit/app/models/statistics_announcement_test.rb index 2ff084d9be3..69d978639f2 100644 --- a/test/unit/app/models/statistics_announcement_test.rb +++ b/test/unit/app/models/statistics_announcement_test.rb @@ -347,6 +347,7 @@ class StatisticsAnnouncementTest < ActiveSupport::TestCase 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 @@ -361,7 +362,8 @@ class StatisticsAnnouncementTest < ActiveSupport::TestCase ) assert_not announcement.save - assert announcement.errors[:publication].include? "type cannot be modified when edition is in the published state" + 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 ===