Skip to content

Commit ae69cf1

Browse files
ChrisBAshtonpatrickpatrickpatrick
authored andcommitted
Capture same base path error and display to user
Instead of presenting a server error to the user if they try and create an edition that has the base path of another published edition, the error is now presented to the user.
1 parent df82f5b commit ae69cf1

8 files changed

Lines changed: 44 additions & 5 deletions

File tree

app/controllers/admin/editions_controller.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ def create
9797
@edition.destroy!
9898
build_edition
9999
build_edition_dependencies
100+
@edition.errors.add(:title, "has been used before on GOV.UK, although the page may no longer exist. Please use another title")
100101
render :new
101102
end
102103

app/models/document.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ def first_edition_id
8585

8686
def similar_slug_exists?
8787
scope = Document.where(document_type:)
88-
sequence_separator = friendly_id_config.sequence_separator
8988

9089
# slug is a nullable column, so we can't assume that it exists
9190
return false if slug.nil?
@@ -99,6 +98,8 @@ def similar_slug_exists?
9998
).count > 1
10099
end
101100

101+
delegate :sequence_separator, to: :friendly_id_config
102+
102103
def should_generate_new_friendly_id?
103104
sluggable_string.present?
104105
end

app/models/edition.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,10 @@ def base_path
400400
"/government/generic-editions/#{url_slug}"
401401
end
402402

403+
def base_path_without_sequence
404+
base_path.split(document.sequence_separator).first
405+
end
406+
403407
def public_path(options = {})
404408
return if base_path.nil?
405409

app/services/draft_edition_updater.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ def perform!
88
end
99

1010
def failure_reason
11+
return @failure_reason if @failure_reason.present?
12+
1113
if !edition.pre_publication?
1214
"A #{edition.state} edition may not be updated."
1315
elsif should_check_current_user_will_retain_access? && access_limit_excludes_current_user?

features/news-article.feature

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Feature: News article
2+
3+
Background:
4+
Given I am an editor
5+
6+
Scenario: Creating a new draft publication with a reserved path
7+
Given a published news article "Stubble to be Outlawed" exists
8+
When I start drafting a new news article "Stubble to be Outlawed"
9+
Then I should see an error of "Title has been used before on GOV.UK, although the page may no longer exist. Please use another title" on the new news article page
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
When(/^I start drafting a new news article "([^"]*)"$/) do |title|
2+
begin_drafting_news_article({ title: })
3+
Whitehall::PublishingApi.unstub(:ensure_base_path_is_associated_with_this_content_id!)
4+
Services.publishing_api.stubs(:lookup_content_id).returns("not a real content id")
5+
click_button "Save and go to document summary"
6+
Whitehall::PublishingApi.stubs(:ensure_base_path_is_associated_with_this_content_id!).returns(nil)
7+
end
8+
9+
Then(/^I should see an error of "(.*)" on the new news article page$/) do |error|
10+
expect(page).to have_selector(".gem-c-error-summary__list-item")
11+
expect(page).to have_content(error.to_s)
12+
end

features/support/document_helper.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ def begin_drafting_document(options)
2121
page.choose(options[:type].humanize)
2222
click_button("Next")
2323

24+
if options[:subtype]
25+
page.choose(options[:subtype].humanize)
26+
click_button("Next")
27+
end
28+
2429
if options[:locale]
2530
check "Create a foreign language only"
2631
select options[:locale], from: "Document language"
@@ -56,7 +61,7 @@ def begin_editing_document(title)
5661
end
5762

5863
def begin_drafting_news_article(options)
59-
begin_drafting_document(options.merge(type: "news_article", previously_published: false))
64+
begin_drafting_document(options.merge(type: "news_article", subtype: "news_story", previously_published: false))
6065
fill_in_news_article_fields(**options.slice(:first_published, :announcement_type))
6166
end
6267

@@ -117,8 +122,7 @@ def fill_in_worldwide_organisation_fields(world_location: "United Kingdom")
117122
fill_in "Logo formatted name", with: "Logo\r\nformatted\r\nname\r\n"
118123
end
119124

120-
def fill_in_news_article_fields(first_published: "2010-01-01", announcement_type: "News story")
121-
select announcement_type, from: "News article type"
125+
def fill_in_news_article_fields(first_published: "2010-01-01")
122126
checkbox_label = "This document has previously been published on another website"
123127
check checkbox_label
124128
within_conditional_reveal checkbox_label do

lib/whitehall/publishing_api.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,13 @@ def self.save_draft_translation(
6969

7070
content.merge!(bulk_publishing: true) if bulk_publishing
7171

72-
ensure_base_path_is_associated_with_this_content_id!(content[:base_path], model_instance.content_id)
72+
base_path = if model_instance.respond_to?(:base_path_without_sequence)
73+
model_instance.base_path_without_sequence
74+
else
75+
content[:base_path]
76+
end
77+
78+
ensure_base_path_is_associated_with_this_content_id!(base_path, model_instance.content_id)
7379

7480
Services.publishing_api.put_content(presenter.content_id, content)
7581
end

0 commit comments

Comments
 (0)