From 288c9ed31f2cbf8e7752208cc7b73bff353cea22 Mon Sep 17 00:00:00 2001
From: Anna Buianova <lightallloy@gmail.com>
Date: Fri, 1 Mar 2024 15:54:15 +0300
Subject: [PATCH] Fix articles counts based on display rules (#20669)

* Comment counts for signed in users

* Adjust comments count for articles

* Fixed counts for signed out users, added specs for Comments::Count

* Fixed counts for signed out and added specs for Comments::Count

* Removed debug info from the view

* Removed separate count for signed out users

* Remove irrelevant specs

* Remove unused scope
---
 app/controllers/stories_controller.rb         |  3 ++
 app/queries/comments/count.rb                 | 25 +++++++++
 .../articles/_full_comment_area.html.erb      |  2 +-
 .../articles/_multiple_reactions.html.erb     |  2 +-
 spec/queries/comments/count_spec.rb           | 54 +++++++++++++++++++
 spec/requests/articles/articles_show_spec.rb  | 11 ++++
 6 files changed, 95 insertions(+), 2 deletions(-)
 create mode 100644 app/queries/comments/count.rb
 create mode 100644 spec/queries/comments/count_spec.rb

diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb
index 48af8bc79..960e0562b 100644
--- a/app/controllers/stories_controller.rb
+++ b/app/controllers/stories_controller.rb
@@ -274,6 +274,9 @@ class StoriesController < ApplicationController
     @user = @article.user
     @organization = @article.organization
     @comments_order = fetch_sort_order
+
+    @comments_count = Comments::Count.new(@article).call
+
     if @article.collection
       @collection = @article.collection
 
diff --git a/app/queries/comments/count.rb b/app/queries/comments/count.rb
new file mode 100644
index 000000000..69f01b232
--- /dev/null
+++ b/app/queries/comments/count.rb
@@ -0,0 +1,25 @@
+# find comments count for an article based on our display rules for signed in users
+# the count includes both comments displayed as usual (text), comments displayed as "deleted" or "hidden by post author"
+# the count doesn't include comments not displayed at all (childless comments with score below HIDE_THRESHOLD)
+
+module Comments
+  class Count
+    attr_reader :article
+
+    def initialize(article)
+      @article = article
+    end
+
+    def call
+      # comments that are not displayed at all (not even a "comment deleted" message):
+      # with the score below hiding threshold and w/o children
+      count_sql = "SELECT COUNT(id) FROM comments c1 WHERE score < ? AND commentable_id = ? " \
+                  "AND commentable_type = ? AND NOT EXISTS " \
+                  "(SELECT 1 FROM comments c2 WHERE c2.ancestry LIKE CONCAT('%/', c1.id::varchar(255)) " \
+                  "OR c2.ancestry = c1.id::varchar(255))"
+      san_count_sql = Comment.sanitize_sql([count_sql, Comment::HIDE_THRESHOLD, @article.id, "Article"])
+      hidden_comments_cnt = Comment.count_by_sql(san_count_sql)
+      article.comments_count - hidden_comments_cnt
+    end
+  end
+end
diff --git a/app/views/articles/_full_comment_area.html.erb b/app/views/articles/_full_comment_area.html.erb
index 13027faf9..ad69d25d5 100644
--- a/app/views/articles/_full_comment_area.html.erb
+++ b/app/views/articles/_full_comment_area.html.erb
@@ -5,7 +5,7 @@
         <div class="flex items-center">
           <h2 class="crayons-subtitle-1">
             <%= t("views.articles.comments.subtitle.#{@comments_order}_html",
-                  num: tag.span(t("views.articles.comments.num", num: @article.comments_count), class: "js-comments-count", data: { comments_count: @article.comments_count })) %>
+                  num: tag.span(t("views.articles.comments.num", num: @comments_count), class: "js-comments-count", data: { comments_count: @comments_count })) %>
           </h2>
           <% if user_signed_in? %>
             <button class="c-btn c-btn--ghost crayons-btn--icon-left" id="toggle-comments-sort-dropdown" aria-controls="comments-sort-dropdown-container" aria-expanded="false" aria-label="<%= t("views.articles.comments.sort_button.aria_label") %>" aria-haspopup="true">
diff --git a/app/views/articles/_multiple_reactions.html.erb b/app/views/articles/_multiple_reactions.html.erb
index a9be0be2e..37e047962 100644
--- a/app/views/articles/_multiple_reactions.html.erb
+++ b/app/views/articles/_multiple_reactions.html.erb
@@ -44,7 +44,7 @@
     <span class="crayons-reaction__icon crayons-reaction__icon--borderless crayons-reaction__icon--inactive">
       <%= crayons_icon_tag("comment.svg", aria_hidden: true) %>
     </span>
-    <span class="crayons-reaction__count" id="reaction-number-comment" data-count="<%= @article.comments_count %>">
+    <span class="crayons-reaction__count" id="reaction-number-comment" data-count="<%= @comments_count %>">
       <span class="bg-base-40 opacity-25 p-2 inline-block radius-default"></span>
     </span>
 
diff --git a/spec/queries/comments/count_spec.rb b/spec/queries/comments/count_spec.rb
new file mode 100644
index 000000000..6c5080aad
--- /dev/null
+++ b/spec/queries/comments/count_spec.rb
@@ -0,0 +1,54 @@
+require "rails_helper"
+
+RSpec.describe Comments::Count do
+  let(:article) { create(:article) }
+  let!(:comment) { create(:comment, commentable: article) }
+  let!(:comment2) { create(:comment, commentable: article) }
+
+  it "returns correct number with regular comments" do
+    article.reload
+    count = described_class.new(article).call
+    expect(count).to eq(2)
+  end
+
+  it "returns correct number with children" do
+    create(:comment, commentable: article, parent: comment2, score: 20)
+    article.reload
+    count = described_class.new(article).call
+    expect(count).to eq(3)
+  end
+
+  it "doesn't include childless children" do
+    create(:comment, commentable: article, parent: comment, score: -450)
+    article.reload
+    count = described_class.new(article).call
+    expect(count).to eq(2)
+  end
+
+  it "includes ok children of a low-score comment (but not low-score children)" do
+    comment.update_column(:score, -500)
+    create(:comment, commentable: article, parent: comment, score: 10)
+    create(:comment, commentable: article, parent: comment, score: -490)
+    create(:comment, commentable: article, parent: comment2, score: 0)
+    article.reload
+    count = described_class.new(article).call
+    expect(count).to eq(4)
+  end
+
+  it "includes children of a low-score comment" do
+    child = create(:comment, commentable: article, parent: comment, score: -401)
+    create(:comment, commentable: article, parent: child, score: 10)
+    article.reload
+    count = described_class.new(article).call
+    expect(count).to eq(4)
+  end
+
+  it "includes a comment with low-score ancestors" do
+    comment.update_column(:score, -500)
+    child = create(:comment, commentable: article, parent: comment, score: -401)
+    create(:comment, commentable: article, parent: child, score: 10)
+    article.reload
+    count = described_class.new(article).call
+    expect(count).to eq(4)
+  end
+end
diff --git a/spec/requests/articles/articles_show_spec.rb b/spec/requests/articles/articles_show_spec.rb
index aad64b35f..53e4f6421 100644
--- a/spec/requests/articles/articles_show_spec.rb
+++ b/spec/requests/articles/articles_show_spec.rb
@@ -217,6 +217,17 @@ RSpec.describe "ArticlesShow" do
         expect(response.body).to include("Child comment")
         expect(response.body).to include("Comment deleted") # instead of the low quality one
       end
+
+      it "displays comments count w/o including super low-quality ones" do
+        get article.path
+        expect(response.body).to include("<span class=\"js-comments-count\" data-comments-count=\"3\">(3)</span>")
+      end
+
+      it "displays includes spam comments in comments count if they have children" do
+        create(:comment, score: 0, commentable: article, parent: spam_comment, body_markdown: "Child comment")
+        get article.path
+        expect(response.body).to include("<span class=\"js-comments-count\" data-comments-count=\"5\">(5)</span>")
+      end
     end
 
     context "when user not signed in" do