Merge pull request #9885 from alphagov/slugs

Make 'identifier' slug-related behaviour more intuitive/flexible

app/models/html_attachment.rb

@@ -83,9 +83,7 @@ def translated_locales
   end
 
   def identifier
-    return slug if slug_eligible?
-
-    content_id
+    slug || content_id
   end
 
   def base_path
@@ -131,10 +129,6 @@ def sluggable_string
     sluggable_locale? ? title : nil
   end
 
-  def slug_eligible?
-    title.ascii_only? && sluggable_locale?
-  end
-
   def clear_slug_if_non_english_locale
     if locale_changed? && !sluggable_locale?
       self.slug = nil

test/unit/app/models/html_attachment_test.rb

@@ -249,13 +249,33 @@ class HtmlAttachmentTest < ActiveSupport::TestCase
     assert_equal expected_slug, attachment.to_param
   end
 
+  test "slug is created even if the English title contains non-ASCII characters (since it's highly unlikely to be ALL non-ASCII characters like other languages)" do
+    attachment = create(:html_attachment, locale: "en", title: "A page about copyright ©")
+
+    expected_slug = "a-page-about-copyright"
+    assert_equal expected_slug, attachment.slug
+    assert_equal expected_slug, attachment.to_param
+  end
+
   test "slug is cleared when changing from english to non-english" do
     attachment = create(:html_attachment, locale: "en")
 
     attachment.update!(locale: "fr")
     assert attachment.slug.blank?
   end
 
+  test "#identifier falls back to content_id if no slug available" do
+    attachment = create(:html_attachment)
+    attachment.slug = nil
+    assert_equal attachment.content_id, attachment.identifier
+  end
+
+  test "#identifier uses the slug if it's been set, irrespective of locale" do
+    attachment = create(:html_attachment, locale: "cy")
+    attachment.slug = "foo"
+    assert_equal "foo", attachment.identifier
+  end
+
   test "#translated_locales lists only the attachment's locale" do
     assert_equal %w[en], HtmlAttachment.new.translated_locales
     assert_equal %w[cy], HtmlAttachment.new(locale: "cy").translated_locales