From ce531913a8ccdc1c17b7fe432065b81072268b8c Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Thu, 2 Feb 2023 10:19:51 +0800
Subject: [PATCH] FIX: Sync user's reviewables count when loading reviewables
 list (#20128)

1. What is the problem here?

When a user's reviewables count changes, the changes are published via
MessageBus in a background Sidekiq job which means there is a delay before the
client receives the MessageBus message with the updated count. During
the time the reviewables count for a user has been updated and the time
when the client receives the MessageBus message with the updated count,
a user may view the reviewables list in the user menu. When that happens, the number of
reviewables in the list may be out of sync with the count shown.

2. What is the fix?

Going forward, the response for the `ReviewablesController#user_menu_list` action will include the user's reviewables count as
the `reviewables_count` attribute. This is then used by the client side
to update the user's reviewables count to ensure that the reviewables
list and count are kept in sync.
---
 .../app/components/user-menu/reviewables-list.js       |  2 ++
 .../discourse/tests/acceptance/user-menu-test.js       | 10 ++++++++++
 .../discourse/tests/helpers/review-pretender.js        |  1 +
 app/controllers/reviewables_controller.rb              |  1 +
 spec/requests/reviewables_controller_spec.rb           | 10 ++++++++++
 5 files changed, 24 insertions(+)

diff --git a/app/assets/javascripts/discourse/app/components/user-menu/reviewables-list.js b/app/assets/javascripts/discourse/app/components/user-menu/reviewables-list.js
index d8155ef6c88..e75efd52b69 100644
--- a/app/assets/javascripts/discourse/app/components/user-menu/reviewables-list.js
+++ b/app/assets/javascripts/discourse/app/components/user-menu/reviewables-list.js
@@ -25,6 +25,8 @@ export default class UserMenuReviewablesList extends UserMenuItemsList {
 
   fetchItems() {
     return ajax("/review/user-menu-list").then((data) => {
+      this.currentUser.updateReviewableCount(data.reviewable_count);
+
       return data.reviewables.map((item) => {
         return new UserMenuReviewableItem({
           reviewable: UserMenuReviewable.create(item),
diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js
index 34177d934d6..6b7bd1df9ca 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js
@@ -120,9 +120,19 @@ acceptance("User menu", function (needs) {
 
   test("clicking on user menu items", async function (assert) {
     updateCurrentUser({ reviewable_count: 1 });
+
     await visit("/");
     await click(".d-header-icons .current-user");
     await click("#user-menu-button-review-queue");
+
+    assert.strictEqual(
+      query(
+        "#user-menu-button-review-queue .badge-notification"
+      ).textContent.trim(),
+      "8",
+      "updates user's reviewable count based on request's response"
+    );
+
     await click("#quick-access-review-queue li.reviewable.pending a");
 
     assert.strictEqual(
diff --git a/app/assets/javascripts/discourse/tests/helpers/review-pretender.js b/app/assets/javascripts/discourse/tests/helpers/review-pretender.js
index b864494a75c..e1952574eab 100644
--- a/app/assets/javascripts/discourse/tests/helpers/review-pretender.js
+++ b/app/assets/javascripts/discourse/tests/helpers/review-pretender.js
@@ -214,6 +214,7 @@ export default function (helpers) {
           is_new_topic: false,
         },
       ],
+      reviewable_count: 8,
       __rest_serializer: "1",
     });
   });
diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb
index 61af8f0e3ec..68a218100b0 100644
--- a/app/controllers/reviewables_controller.rb
+++ b/app/controllers/reviewables_controller.rb
@@ -81,6 +81,7 @@ class ReviewablesController < ApplicationController
           Reviewable.user_menu_list_for(current_user),
           current_user,
         ).as_json,
+      reviewable_count: current_user.reviewable_count,
     }
     render_json_dump(json, rest_serializer: true)
   end
diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb
index 286b86ec398..f0cbcc6400d 100644
--- a/spec/requests/reviewables_controller_spec.rb
+++ b/spec/requests/reviewables_controller_spec.rb
@@ -300,6 +300,16 @@ RSpec.describe ReviewablesController do
         expect(reviewables[0]["pending"]).to eq(true)
       end
 
+      it "responds with current user's reviewables count" do
+        reviewable = Fabricate(:reviewable)
+
+        get "/review/user-menu-list.json"
+
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["reviewables"].length).to eq(1)
+        expect(response.parsed_body["reviewable_count"]).to eq(1)
+      end
+
       it "responds with pending reviewables only" do
         Fabricate(:reviewable, status: Reviewable.statuses[:approved])
         pending1 = Fabricate(:reviewable, status: Reviewable.statuses[:pending])