From 63fdaffb69079ae5085abf4c1f2b03522dca34e7 Mon Sep 17 00:00:00 2001
From: Rebecca Pearce <17481621+beccapearce@users.noreply.github.com>
Date: Mon, 30 Dec 2024 11:45:28 +0000
Subject: [PATCH] Review changes

---
 lib/govspeak/remove_advisory_service.rb       |  6 +-
 lib/tasks/remove_advisory.rake                | 67 ++-----------------
 .../govspeak/remove_advisory_service_test.rb  | 41 ++++++++++++
 3 files changed, 46 insertions(+), 68 deletions(-)

diff --git a/lib/govspeak/remove_advisory_service.rb b/lib/govspeak/remove_advisory_service.rb
index 7572de876da..739c4d0d787 100644
--- a/lib/govspeak/remove_advisory_service.rb
+++ b/lib/govspeak/remove_advisory_service.rb
@@ -91,11 +91,7 @@ def advisory_match_group(body_content)
     end
 
     def regexp_for_advisory_markup
-      opening_at = "(^@)"
-      content_after_at = '([\s\S]*?)'
-      closing_at = "(@?)"
-      other_possible_line_ends = '(?:^\$CTA|\r?\n\r?\n|^@|$)'
-      Regexp.new("#{opening_at}#{content_after_at}#{closing_at}(?=#{other_possible_line_ends})", Regexp::MULTILINE)
+      Govspeak::EmbeddedContentPatterns::ADVISORY.to_s
     end
 
     def replace_advisory_with_information_callout(match, body_content)
diff --git a/lib/tasks/remove_advisory.rake b/lib/tasks/remove_advisory.rake
index c4f2e9d4546..df45f480626 100644
--- a/lib/tasks/remove_advisory.rake
+++ b/lib/tasks/remove_advisory.rake
@@ -1,6 +1,6 @@
 namespace :remove_advisory do
   desc "Process advisory govspeak in published editions"
-  task published_editions: :environment do
+  task :published_editions, %i[dry_run] => :environment do |_, args|
     regex = Govspeak::EmbeddedContentPatterns::ADVISORY.to_s
     successes = []
     failures = []
@@ -18,38 +18,7 @@ namespace :remove_advisory do
 
     published_content_containing_advisory_govspeak.each do |document_id|
       edition = Document.find(document_id).latest_edition
-      Govspeak::RemoveAdvisoryService.new(edition, dry_run: false).process!
-      successes << edition.content_id
-      print "S"
-    rescue StandardError => e
-      failures << { content_id: edition.content_id, error: e.message }
-      print "F"
-    end
-
-    summarize_results(successes, failures)
-  end
-
-  desc "Dry run to show which editions would have advisory govspeak processed"
-  task dry_run_published_editions: :environment do
-    regex = Govspeak::EmbeddedContentPatterns::ADVISORY.to_s
-
-    successes = []
-    failures = []
-    published_content_containing_advisory_govspeak = []
-
-    puts "\nStarting dry run of published editions...\n"
-
-    Edition
-      .where(state: "published")
-      .joins("RIGHT JOIN edition_translations ON edition_translations.edition_id = editions.id")
-      .where("body REGEXP ?", regex)
-      .find_each do |object|
-        published_content_containing_advisory_govspeak << object.document_id
-      end
-
-    published_content_containing_advisory_govspeak.each do |document_id|
-      edition = Document.find(document_id).latest_edition
-      Govspeak::RemoveAdvisoryService.new(edition, dry_run: true).process!
+      Govspeak::RemoveAdvisoryService.new(edition, dry_run: args[:dry_run]).process!
       successes << edition.content_id
       print "S"
     rescue StandardError => e
@@ -61,7 +30,7 @@ namespace :remove_advisory do
   end
 
   desc "Process advisory govspeak in published HTML attachments"
-  task published_html_attachments: :environment do
+  task :published_html_attachments, %i[dry_run] => :environment do |_, args|
     regex = Govspeak::EmbeddedContentPatterns::ADVISORY.to_s
 
     successes = []
@@ -77,7 +46,7 @@ namespace :remove_advisory do
       .find_each do |attachment|
         next if attachment.attachable.respond_to?(:state) && attachment.attachable.state != "published"
 
-        Govspeak::RemoveAdvisoryService.new(attachment, dry_run: false).process!
+        Govspeak::RemoveAdvisoryService.new(attachment, dry_run: args[:dry_run]).process!
         successes << attachment.content_id
         print "S"
     rescue StandardError => e
@@ -89,34 +58,6 @@ namespace :remove_advisory do
   end
 end
 
-desc "Dry run to show which HTML publications would have advisory govspeak processed"
-task dry_run_published_html_attachments: :environment do
-  regex = Govspeak::EmbeddedContentPatterns::ADVISORY.to_s
-
-  successes = []
-  failures = []
-
-  puts "\nStarting dry run of published HTML attachments...\n"
-
-  HtmlAttachment
-    .joins(:govspeak_content)
-    .where(deleted: false)
-    .where.not(attachable: nil)
-    .where("govspeak_contents.body REGEXP ?", regex)
-    .find_each do |attachment|
-      next if attachment.attachable.respond_to?(:state) && attachment.attachable.state != "published"
-
-      Govspeak::RemoveAdvisoryService.new(attachment, dry_run: true).process!
-      successes << attachment.content_id
-      print "S"
-  rescue StandardError => e
-    failures << { content_id: attachment.content_id, error: e.message }
-    print "F"
-    end
-
-  summarize_results(successes, failures)
-end
-
 def summarize_results(successes, failures)
   puts "\n\nSummary:\n"
   puts "Successes: #{successes.count}"
diff --git a/test/unit/lib/govspeak/remove_advisory_service_test.rb b/test/unit/lib/govspeak/remove_advisory_service_test.rb
index e665a101100..b4d0f2df150 100644
--- a/test/unit/lib/govspeak/remove_advisory_service_test.rb
+++ b/test/unit/lib/govspeak/remove_advisory_service_test.rb
@@ -48,4 +48,45 @@ class Govspeak::RemoveAdvisoryServiceTest < ActiveSupport::TestCase
 
     assert_equal expected, service.replace_all_advisories(edition.body)
   end
+
+  test "replace_all_advisories will replace advisories with no space after the @" do
+    body = "@This is a very important message or warning@"
+    edition = create(:published_edition, body:)
+    service = Govspeak::RemoveAdvisoryService.new(edition)
+
+    expected = "^This is a very important message or warning^"
+
+    assert_equal expected, service.replace_all_advisories(edition.body)
+  end
+
+  test "replace_all_advisories will replace advisories with no closing @" do
+    body = "\r\n@ New online safety legislation is coming which will aim to reduce online harms.\r\n\r\n"
+    edition = create(:published_edition, body:)
+    service = Govspeak::RemoveAdvisoryService.new(edition)
+
+    expected = "\r\n^ New online safety legislation is coming which will aim to reduce online harms.^\r\n\r\n"
+
+    assert_equal expected, service.replace_all_advisories(edition.body)
+  end
+
+  test "replace_all_advisories will not replace anything resembling an email address" do
+    body = "\r\nFor further information please get in touch at contact@foobar.com.\r\n\r\n"
+    edition = create(:published_edition, body:)
+    service = Govspeak::RemoveAdvisoryService.new(edition)
+
+    expected = "\r\nFor further information please get in touch at contact@foobar.com.\r\n\r\n"
+
+    assert_equal expected, service.replace_all_advisories(edition.body)
+  end
+
+  test "replace_all_advisories will not replace twitter handles" do
+    # NB any instances of twitter handles at the start of a line have been handled separately
+    body = "\r\nTo hear more you can follow us at on @foobar\r\n\r\n"
+    edition = create(:published_edition, body:)
+    service = Govspeak::RemoveAdvisoryService.new(edition)
+
+    expected = "\r\nTo hear more you can follow us at on @foobar\r\n\r\n"
+
+    assert_equal expected, service.replace_all_advisories(edition.body)
+  end
 end