From 073cb41fac15acb6b38053f49ac93985e173b400 Mon Sep 17 00:00:00 2001 From: davidgisbey Date: Thu, 28 Dec 2023 12:16:42 +0000 Subject: [PATCH] Don't attempt to discard draft for published sections When a manual is published all sections are published as well. Post publication sections retain a published state expect in 2 circumstances. 1. they are edited 2. they are withdrawn If they are edited a draft section is pushed downstream to Publishing API and the Draft Content Store. When the section is withdrawn, we need to discard the draft from Publishing API. There's some logic to handle this in the remove_service which checks to see if the section is a draft (i.e. has a draft edition in Publishing API) and if so it attempts to discard the draft. However, there's an issue with this. Within the remove service assign_attributes is called on the section. If you go and have a look at the method defined in the Section model, you can see that it actually creates a new draft section when called if the latest edition is published. It then passes change note params to the new draft edition of the section. This means that when #draft? is called on the section it will always return true. This causes an issue when the following is called: ``` if section.draft? Adapters.publishing.discard_section(section) end ``` There blows up when a section was in a published state prior to the remove_service being called as there is no draft section in the Publishing API database. There are feature specs which explicity cover that the section is removed when Published sections are withdrawn. The proposed fix of capturing the state prior so assign_attributes being called breaks these tests. I'm pretty sure that the tests are actually testing the incorrect behaviour which is causing the bug though. At first glance I thought it might be possible to not bother creating a new draft edition prior to removing the section from the manual, but it's actually required for capturing the change note the user inputs so that users can see why the section was removed. When an edition is published downstream, any change notes are pulled from removed sections added as PublicationLogs These are then rendered on the frontend. --- app/services/section/remove_service.rb | 10 +++++++++- features/removing-a-manual-section.feature | 12 ++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/services/section/remove_service.rb b/app/services/section/remove_service.rb index f1cc3c047..1fae5bace 100644 --- a/app/services/section/remove_service.rb +++ b/app/services/section/remove_service.rb @@ -20,6 +20,14 @@ def call minor_update: attributes.fetch(:minor_update, "0"), change_note: attributes.fetch(:change_note, ""), } + + # We need to capture the state of the section before assigning attributes. + # The Section#assign_attributes method always creates a new draft section if + # the latest edition is published. + # This causes Adapters.publishing.discard_section(section) to be called which + # blows up as there is no draft section in the Publishing API database. + draft_section = section.draft? + section.assign_attributes(change_note_params) if section.valid? @@ -30,7 +38,7 @@ def call manual.save!(user) Adapters.publishing.save_draft(manual, include_sections: false) - if section.draft? + if draft_section Adapters.publishing.discard_section(section) end end diff --git a/features/removing-a-manual-section.feature b/features/removing-a-manual-section.feature index 000f8203b..f4dcd1367 100644 --- a/features/removing-a-manual-section.feature +++ b/features/removing-a-manual-section.feature @@ -60,8 +60,7 @@ Feature: Removing a section from a manual Given I am logged in as a GDS editor And a published manual exists When I remove one of the sections from the manual - Then the section is removed from the manual - When I publish the manual + And I publish the manual Then the removed section is not published But the removed section is withdrawn with a redirect to the manual And the removed section is archived @@ -70,8 +69,7 @@ Feature: Removing a section from a manual Given I am logged in as an editor And a published manual exists When I remove one of the sections from the manual - Then the section is removed from the manual - When I publish the manual + And I publish the manual Then the removed section is not published But the removed section is withdrawn with a redirect to the manual And the removed section is archived @@ -87,14 +85,12 @@ Feature: Removing a section from a manual When I remove one of the sections from the manual with a major update omitting the note Then I see an error requesting that I provide a change note When I remove one of the sections from the manual with a major update - Then the section is removed from the manual - When I add another section and publish the manual later + And I add another section and publish the manual later Then the removed section change note is included Scenario: Removing a section with a minor update change notes Given I am logged in as a GDS editor And a published manual exists When I remove one of the sections from the manual with a minor update - Then the section is removed from the manual - When I add another section and publish the manual later + And I add another section and publish the manual later Then the removed section change note is not included