From 391f33d5dc013a1d14b1e15ab540da2f8d23120f Mon Sep 17 00:00:00 2001 From: davidgisbey Date: Tue, 19 Dec 2023 13:39:07 +0000 Subject: [PATCH 1/2] Ensure document retains slug when edition title has special chars We've been doing some investigative work into a bunch of sentry errors. I've gone down a bit of a rabbit hole and came across some fairly strange behaviour. There are a subset of documents in our database that don't have a slug. This is despite their being an after create for document which sets the slug to the documents id if one is not set based on the title of the first edition for whatever reason. This is being overriden by the #update_slug_if_possible method which is triggered by the #update_document_slug before_validation method. When the #update_slug_if_possible is passed a special character or characters from other languages that different alphabets are used friendly_id is unable to handle it and updates the slug to nil. We want to retain the default behaviour of the slug being defaulted to the id of the document. This commit ensures that's the case. --- app/models/document.rb | 5 +++++ test/unit/app/models/document_test.rb | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/app/models/document.rb b/app/models/document.rb index f30d30e8c18..43c0b8908d7 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -86,6 +86,11 @@ def update_slug_if_possible(new_title) candidate_slug = normalize_friendly_id(new_title) unless candidate_slug == slug update!(sluggable_string: new_title) + # when special characters or scipts are used from the non-latin alphabets + # friendly_id sets the documents slug to nil. This ensures that it + # retains the default behaviour id of the document as the slug rather than being nil + # as implemented in the #ensure_document_has_a_slug after_create callback + ensure_document_has_a_slug end end diff --git a/test/unit/app/models/document_test.rb b/test/unit/app/models/document_test.rb index 54ce948d94f..e5d32f7e070 100644 --- a/test/unit/app/models/document_test.rb +++ b/test/unit/app/models/document_test.rb @@ -350,4 +350,28 @@ class DocumentTest < ActiveSupport::TestCase assert_nil unpublished.document.live_edition assert_nil draft.document.live_edition end + + test "#update_slug_if_possible updates the slug to the title passed maps to a diffrent slug" do + document = build(:document) + new_slug = "new_slug" + document.expects(:update!).with(sluggable_string: new_slug).once + + document.update_slug_if_possible(new_slug) + end + + test "#update_slug_if_possible does nothing to the slug if the title passed in maps to the current slug" do + document = build_stubbed(:document) + document.expects(:update!).never + + document.update_slug_if_possible(document.slug) + end + + test "#update_slug_if_possible ensures that the slug is set to the documents id if the new title contains special characters" do + document = create(:document, slug: nil) + slug_with_special_characters = "首次中英高级别安全" + + document.update_slug_if_possible(slug_with_special_characters) + + assert_equal document.id.to_s, document.reload.slug + end end From d1202588139cf555daf38b3607264bc58e1bf6ff Mon Sep 17 00:00:00 2001 From: davidgisbey Date: Tue, 19 Dec 2023 19:46:32 +0000 Subject: [PATCH 2/2] Add data migration to backfill missing document slugs --- .../20231219193828_backfill_documents_with_blank_slugs.rb | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 db/data_migration/20231219193828_backfill_documents_with_blank_slugs.rb diff --git a/db/data_migration/20231219193828_backfill_documents_with_blank_slugs.rb b/db/data_migration/20231219193828_backfill_documents_with_blank_slugs.rb new file mode 100644 index 00000000000..96d5d578cba --- /dev/null +++ b/db/data_migration/20231219193828_backfill_documents_with_blank_slugs.rb @@ -0,0 +1,3 @@ +Document.where(slug: nil).each do |document| + document.update_column(:slug, document.id.to_s) +end