[deploy] Remove draft articles with duplicate feed source URL and cleanup invalid data (#10205)

lib/data_update_scripts/20200904132553_remove_draft_articles_with_duplicate_feed_source_url.rb

@@ -0,0 +1,78 @@
+module DataUpdateScripts
+  class RemoveDraftArticlesWithDuplicateFeedSourceUrl
+    def run
+      # Currently there are duplicate draft articles in the DB with the same feed_source_url.
+
+      # This statement deletes all draft articles in excess found to be duplicate over feed_source_url,
+      # excluding those whose body_markdown is different from the other duplicate occurrences
+      result = ActiveRecord::Base.connection.execute(
+        <<~SQL,
+          WITH duplicates_draft_articles AS
+              (SELECT id
+               FROM
+                   (SELECT id,
+                           published,
+                           body_markdown,
+                           LAG(body_markdown, 1) OVER(PARTITION BY feed_source_url
+                                                      ORDER BY id ASC) AS previous_body_markdown,
+                           ROW_NUMBER() OVER(PARTITION BY feed_source_url
+                                             ORDER BY id ASC) AS row_number
+                    FROM articles
+                    WHERE feed_source_url IS NOT NULL ) duplicates
+               WHERE duplicates.row_number > 1
+                   AND published = 'f' -- drafts
+                   AND body_markdown = previous_body_markdown -- with the same body
+           )
+          DELETE
+          FROM articles
+          WHERE id IN (SELECT id FROM duplicates_draft_articles) RETURNING id;
+        SQL
+      )
+
+      # Sending IDs of deleted articles to Datadog
+      result.map { |row| row["id"] }.in_groups_of(1000) do |ids|
+        DatadogStatsClient.event(
+          "DataUpdateScripts::RemoveDraftArticlesWithDuplicateFeedSourceUrl",
+          "deleted draft articles with the same feed_source_url and same body_markdown",
+          tags: ids,
+        )
+      end
+
+      # Now that all duplicates with the same body are gone, we need to deal with duplicate feed source URLs
+      # with different bodies.
+      # We thus select the oldest for removal preserving the most recent one
+      result = ActiveRecord::Base.connection.execute(
+        <<~SQL,
+          WITH duplicates_draft_articles AS
+              (SELECT id
+               FROM
+                   (SELECT id,
+                           published,
+                           body_markdown,
+                           LAG(body_markdown, 1) OVER(PARTITION BY feed_source_url
+                                                      ORDER BY created_at DESC) AS previous_body_markdown,
+                           ROW_NUMBER() OVER(PARTITION BY feed_source_url
+                                             ORDER BY created_at DESC) AS row_number
+                    FROM articles
+                    WHERE feed_source_url IS NOT NULL ) duplicates
+               WHERE duplicates.row_number > 1
+                   AND published = 'f' -- drafts
+                   AND body_markdown != previous_body_markdown -- with different bodies
+           )
+          DELETE
+          FROM articles
+          WHERE id IN (SELECT id FROM duplicates_draft_articles) RETURNING id;
+        SQL
+      )
+
+      # Sending IDs of deleted articles to Datadog
+      result.map { |row| row["id"] }.in_groups_of(1000) do |ids|
+        DatadogStatsClient.event(
+          "DataUpdateScripts::RemoveDraftArticlesWithDuplicateFeedSourceUrl",
+          "deleted draft articles with the same feed_source_url and different body_markdown",
+          tags: ids,
+        )
+      end
+    end
+  end
+end

lib/data_update_scripts/20200904141057_cleanup_articles_with_invalid_feed_source_url.rb

@@ -0,0 +1,16 @@
+module DataUpdateScripts
+  class CleanupArticlesWithInvalidFeedSourceUrl
+    def run
+      # We need this to intercept those strings that would be valid URLs if the scheme were added,
+      # so I slightly modified the Ruby URL regexp from https://urlregex.com/ to avoid checking for the scheme
+      almost_url_regexp = %r{\A(?:\S+(?::\S*)?@)?(?:(?!10(?:\.\d{1,3}){3})(?!127(?:\.\d{1,3}){3})(?!169\.254(?:\.\d{1,3}){2})(?!192\.168(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))(?::\d{2,5})?(?:/[^\s]*)?\z}i # rubocop:disable Layout/LineLength
+
+      Article.where.not(feed_source_url: nil).where("feed_source_url NOT ILIKE ?", "http%").find_each do |article|
+        fixed_feed_source_url = article.canonical_url.presence ||
+          (almost_url_regexp.match?(article.feed_source_url) ? "https://#{article.feed_source_url}" : nil)
+
+        article.update_columns(feed_source_url: fixed_feed_source_url)
+      end
+    end
+  end
+end

spec/lib/data_update_scripts/cleanup_articles_with_invalid_feed_source_url_spec.rb

@@ -0,0 +1,41 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20200904141057_cleanup_articles_with_invalid_feed_source_url.rb",
+)
+
+describe DataUpdateScripts::CleanupArticlesWithInvalidFeedSourceUrl do
+  let(:article) { create(:article) }
+
+  it "sets feed_source_url to canonical_url if the latter is present" do
+    canonical_url = "https://example.com/article"
+    article.update_columns(feed_source_url: "", canonical_url: canonical_url)
+
+    described_class.new.run
+
+    expect(article.reload.feed_source_url).to eq(canonical_url)
+  end
+
+  it "sets empty string feed source url to nil" do
+    article.update_columns(feed_source_url: "")
+
+    described_class.new.run
+
+    expect(article.reload.feed_source_url).to be(nil)
+  end
+
+  it "sets totally invalid url to nil" do
+    article.update_columns(feed_source_url: "not a url")
+
+    described_class.new.run
+
+    expect(article.reload.feed_source_url).to be(nil)
+  end
+
+  it "sets an 'almost URL' to a https URL" do
+    article.update_columns(feed_source_url: "dev.to")
+
+    described_class.new.run
+
+    expect(article.reload.feed_source_url).to eq("https://dev.to")
+  end
+end

spec/lib/data_update_scripts/remove_draft_articles_with_duplicate_feed_source_url_spec.rb

@@ -0,0 +1,50 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20200904132553_remove_draft_articles_with_duplicate_feed_source_url.rb",
+)
+
+describe DataUpdateScripts::RemoveDraftArticlesWithDuplicateFeedSourceUrl do
+  it "removes draft articles that have the same feed source url and the same body keeping the first" do
+    articles = create_list(:article, 3, published: false)
+
+    # we bypass the callbacks intentionally as feed source url has a unique callback at the Rails level
+    articles.each do |article|
+      article.update_columns(
+        body_markdown: "body",
+        feed_source_url: "https://example.com/article",
+      )
+    end
+
+    number_of_articles_to_remove = articles.size - 1
+
+    expect { described_class.new.run }.to change(Article, :count).by(-number_of_articles_to_remove)
+
+    articles.sort_by!(&:id)
+
+    expect(Article.exists?(articles.first.id)).to be(true)
+    expect(Article.exists?(articles.second.id)).to be(false)
+    expect(Article.exists?(articles.third.id)).to be(false)
+  end
+
+  it "removes draft articles that have the same feed source url and different bodies keeping the most recent" do
+    articles = create_list(:article, 3, published: false)
+
+    # we bypass the callbacks intentionally as feed source url has a unique callback at the Rails level
+    articles.each do |article|
+      article.update_columns(
+        created_at: Faker::Time.between(from: Time.current, to: 2.days.ago),
+        feed_source_url: "https://example.com/article",
+      )
+    end
+
+    number_of_articles_to_remove = articles.size - 1
+
+    expect { described_class.new.run }.to change(Article, :count).by(-number_of_articles_to_remove)
+
+    articles.sort_by! { |a| -a.created_at.to_i }
+
+    expect(Article.exists?(articles.first.id)).to be(true)
+    expect(Article.exists?(articles.second.id)).to be(false)
+    expect(Article.exists?(articles.third.id)).to be(false)
+  end
+end