Merge pull request #9508 from alphagov/allow-statistics-announcement-…

…update

Allow statistics announcement update

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

app/models/statistics_announcement.rb

@@ -22,27 +22,27 @@ 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
 
   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

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,

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)