Skip to content

Commit 52e5953

Browse files
committed
Rule 2: Prevent orphaned children
A parent draft may not be discarded while it has associated child documents whose only edition is linked to that parent draft. We could offer to drop the draft documents automatically when dropping the parent. But the simpler solution for now is to raise a validation error and force the publisher to drop the child documents manually first.
1 parent 485045e commit 52e5953

6 files changed

Lines changed: 61 additions & 0 deletions

File tree

app/controllers/admin/editions_controller.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ def destroy
197197
else
198198
redirect_to admin_edition_path(@edition), alert: edition_deleter.failure_reason
199199
end
200+
rescue WhitehallError => e
201+
redirect_to admin_edition_path(@edition), alert: e.message
200202
end
201203

202204
def update_bypass_id

app/models/concerns/standard_edition/parent_document.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
module StandardEdition::ParentDocument
22
extend ActiveSupport::Concern
33

4+
class UnableToDelete < ::WhitehallError; end
5+
46
class Trait < Edition::Traits::Trait
57
def process_associations_after_save(new_edition)
68
ParentChildRelationship
@@ -30,6 +32,8 @@ def process_associations_after_save(new_edition)
3032
through: :child_relationships,
3133
source: :child_document
3234

35+
before_update :ensure_no_new_child_documents!, if: :deleting?
36+
3337
add_trait Trait
3438
end
3539

@@ -48,4 +52,16 @@ def child_editions
4852
def new_child_documents
4953
child_documents.where(live_edition_id: nil)
5054
end
55+
56+
private
57+
58+
def deleting?
59+
will_save_change_to_state? && state == "deleted"
60+
end
61+
62+
def ensure_no_new_child_documents!
63+
if allows_child_documents? && new_child_documents.any?
64+
raise UnableToDelete, "This document cannot be deleted while it has child documents that have never been published. Delete the draft child documents first."
65+
end
66+
end
5167
end

app/models/whitehall_error.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# Base class for Whitehall-specific domain errors.
2+
#
3+
# We rescue WhitehallError in places where we want to handle
4+
# expected business/application failures gracefully, without also
5+
# swallowing unexpected framework or programming errors.
6+
#
7+
# Avoid rescuing StandardError directly unless we genuinely intend
8+
# to catch any application exception, including Rails, ActiveRecord,
9+
# or Ruby runtime errors.
10+
#
11+
# Any error inheriting from WhitehallError is considered part of
12+
# Whitehall's intentional public error surface.
13+
class WhitehallError < StandardError
14+
end

features/standard-edition-child-pages.feature

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,12 @@ Feature: Standard Editions - Child Pages
2424
And I have a published parent configurable document
2525
Then I should see a "Child documents" section on the document summary page
2626
And there should be no "Add child document" link
27+
28+
Scenario: Not able to delete a draft parent if it contains any non-live children
29+
Given I am a writer
30+
And the configurable document types feature flag is enabled
31+
And the test configurable document type group is defined
32+
And I have drafted a parent and a child configurable document
33+
Then when I click the link "Delete draft"
34+
And when I click "Delete"
35+
Then I should get the error message "This document cannot be deleted while it has child documents that have never been published. Delete the draft child documents first."

features/step_definitions/standard_edition_child_pages_steps.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,11 @@ def create_parent_standard_edition(state)
7474
Then(/^there should be no "(.+)" link$/) do |string|
7575
expect(page).not_to have_link(string)
7676
end
77+
78+
Given("I have drafted a parent and a child configurable document") do
79+
step "I draft a new parent configurable document"
80+
step "when I click the link \"Add child document\""
81+
step "when I choose a child document type"
82+
step "when I fill in and create the child document"
83+
visit admin_standard_edition_path(@standard_edition)
84+
end

test/unit/app/models/concerns/standard_edition/parent_document_test.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,4 +227,16 @@ class StandardEdition::ParentDocumentTest < ActiveSupport::TestCase
227227

228228
assert_not_includes parent_edition.child_documents, child_edition.document
229229
end
230+
231+
test "unable to delete parent edition if it contains any new child documents" do
232+
parent_edition = create(:standard_edition)
233+
234+
parent_edition.stubs(:new_child_documents).returns([create(:standard_edition).document])
235+
236+
error = assert_raises(StandardEdition::ParentDocument::UnableToDelete) do
237+
parent_edition.update!(state: "deleted")
238+
end
239+
240+
assert_equal "This document cannot be deleted while it has child documents that have never been published. Delete the draft child documents first.", error.message
241+
end
230242
end

0 commit comments

Comments
 (0)