From ba590c1750baa80e0ec58a38bf82be2380e7ccc7 Mon Sep 17 00:00:00 2001
From: Daniel Uber <djuber@gmail.com>
Date: Wed, 29 Dec 2021 09:48:34 -0600
Subject: [PATCH] Load article before creating a page view for an invalid one
 (#15898)

* Load article before creating a page view for an invalid one

This should resolve a validation error in PageView.create! when the
article does not exist (perhaps it was deleted, or the user suspended,
or it was invalid data sent from the client).

This had been happening dozens of times per day for the last 10
months.

* Use an intention revealing symbol instead of calculated id

Since we only require that find_by not find anything, pass a symbol
that's not going to be the id for any article under any conditions.

As an added benefit, this provides a clear indication of the purpose
of the symbol, without needing to mentally confirm that

```sql
  SELECT * FROM articles
  WHERE id IN (
    SELECT (1 + MAX(id)) FROM articles
  ) LIMIT 1
```

actually never gives any articles back.
---
 .../articles/update_page_views_worker.rb      |  4 ++-
 .../articles/update_page_views_worker_spec.rb | 35 +++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 spec/workers/articles/update_page_views_worker_spec.rb

diff --git a/app/workers/articles/update_page_views_worker.rb b/app/workers/articles/update_page_views_worker.rb
index 622c8270c..574983d30 100644
--- a/app/workers/articles/update_page_views_worker.rb
+++ b/app/workers/articles/update_page_views_worker.rb
@@ -8,9 +8,11 @@ module Articles
                     retry: false
 
     def perform(create_params)
+      article = Article.find_by(id: create_params["article_id"])
+      return unless article
+
       PageView.create!(create_params)
 
-      article = Article.find(create_params["article_id"])
       updated_count = article.page_views.sum(:counts_for_number_of_views)
       if updated_count > article.page_views_count
         article.update_column(:page_views_count, updated_count)
diff --git a/spec/workers/articles/update_page_views_worker_spec.rb b/spec/workers/articles/update_page_views_worker_spec.rb
new file mode 100644
index 000000000..6cde17c4a
--- /dev/null
+++ b/spec/workers/articles/update_page_views_worker_spec.rb
@@ -0,0 +1,35 @@
+require "rails_helper"
+
+RSpec.describe Articles::UpdatePageViewsWorker, type: :worker do
+  let(:worker) { described_class.new }
+
+  context "when the article id is invalid" do
+    let(:article_id) { :no_article_with_this_id }
+
+    it "exits gracefully" do
+      expect { worker.perform(article_id: article_id) }.not_to raise_error
+    end
+
+    it "does not attempt to create a page view for an invalid article" do
+      allow(PageView).to receive(:create!)
+
+      worker.perform(article_id: article_id)
+
+      expect(PageView).not_to have_received(:create!)
+    end
+  end
+
+  context "when the article exists" do
+    let(:user) { create(:user) }
+    let(:referrer) { nil }
+    let(:article) { create(:article) }
+
+    it "creates a page view" do
+      expect do
+        worker.perform("article_id" => article.id,
+                       "user_id" => user.id,
+                       "referrer" => referrer)
+      end.to change(PageView, :count).by(1)
+    end
+  end
+end