FIX: sync reviewable count when opening the hamburger menu (#10368)

FIX: sync reviewable count when opening the hamburger menu (#10368)

When a tab is open but left unattended for a while, the red, green, and blue pills tend to go out of sync.

So whevener we open the notifications menu, we sync up the notification count (eg. blue and green pills) with the server.

However, the reviewable count (eg. the red pill) is not a notification and is located in the hamburger menu. This commit adds a new route on the server side to retrieve the reviewable count for the current user and a ping (refreshReviewableCount) from the client side to sync the reviewable count whenever they open the hamburger menu.

REFACTOR: I also refactored the hamburger-menu widget code to prevent repetitive uses of “this.”.

PERF: I improved the performance of the ‘notify_reviewable’ job by doing only 1 query to the database to retrieve all the pending reviewables and then tallying based on the various rights.

diff --git a/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js b/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js
index 8f24468..8caaa0f 100644
--- a/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js
+++ b/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js
@@ -27,16 +27,22 @@ createWidget("priority-faq-link", {
   },
 
   click(e) {
-    if (this.siteSettings.faq_url === this.attrs.href) {
+    const {
+      attrs: { href },
+      currentUser,
+      siteSettings
+    } = this;
+
+    if (siteSettings.faq_url === href) {
       ajax(userPath("read-faq"), { type: "POST" }).then(() => {
-        this.currentUser.set("read_faq", true);
+        currentUser.set("read_faq", true);
 
         if (wantsNewWindow(e)) {
           return;
         }
 
         e.preventDefault();
-        DiscourseURL.routeTo(this.attrs.href);
+        DiscourseURL.routeTo(href);
       });
     } else {
       if (wantsNewWindow(e)) {
@@ -44,12 +50,14 @@ createWidget("priority-faq-link", {
       }
 
       e.preventDefault();
-      DiscourseURL.routeTo(this.attrs.href);
+      DiscourseURL.routeTo(href);
     }
   }
 });
 
 export default createWidget("hamburger-menu", {
+  buildKey: () => "hamburger-menu",
+
   tagName: "div.hamburger-panel",
 
   settings: {
@@ -59,6 +67,10 @@ export default createWidget("hamburger-menu", {
     showAbout: true
   },
 
+  defaultState() {
+    return { loaded: false, loading: false };
+  },
+
   adminLinks() {
     const { currentUser } = this;
 
@@ -88,15 +100,8 @@ export default createWidget("hamburger-menu", {
     return tts ? tts.lookupCount(type) : 0;
   },
 
-  showUserDirectory() {
-    if (!this.siteSettings.enable_user_directory) return false;
-    if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser)
-      return false;
-    return true;
-  },
-
   generalLinks() {
-    const { siteSettings } = this;
+    const { attrs, currentUser, siteSettings, state } = this;
     const links = [];
 
     links.push({
@@ -106,7 +111,7 @@ export default createWidget("hamburger-menu", {
       title: "filters.latest.help"
     });
 
-    if (this.currentUser) {
+    if (currentUser) {
       links.push({
         route: "discovery.new",
         className: "new-topics-link",
@@ -124,22 +129,20 @@ export default createWidget("hamburger-menu", {
         title: "filters.unread.help",
         count: this.lookupCount("unread")
       });
-    }
 
-    // Staff always see the review link. Non-staff will see it if there are items to review
-    if (
-      this.currentUser &&
-      (this.currentUser.staff || this.currentUser.reviewable_count)
-    ) {
-      links.push({
-        route: siteSettings.reviewable_default_topics
-          ? "review.topics"
-          : "review",
-        className: "review",
-        label: "review.title",
-        badgeCount: "reviewable_count",
-        badgeClass: "reviewables"
-      });
+      // Staff always see the review link.
+      // Non-staff will see it if there are items to review
+      if (currentUser.staff || currentUser.reviewable_count) {
+        links.push({
+          route: siteSettings.reviewable_default_topics
+            ? "review.topics"
+            : "review",
+          className: "review",
+          label: "review.title",
+          badgeCount: "reviewable_count",
+          badgeClass: "reviewables"
+        });
+      }
     }
 
     links.push({
@@ -157,7 +160,9 @@ export default createWidget("hamburger-menu", {
       });
     }
 
-    if (this.showUserDirectory()) {
+    const canSeeUserProfiles =
+      currentUser || !siteSettings.hide_user_profiles_from_public;
+    if (siteSettings.enable_user_directory && canSeeUserProfiles) {
       links.push({
         route: "users",
         className: "user-directory-link",
@@ -165,7 +170,7 @@ export default createWidget("hamburger-menu", {
       });
     }
 
-    if (this.siteSettings.enable_group_directory) {
+    if (siteSettings.enable_group_directory) {
       links.push({
         route: "groups",
         className: "groups-link",
@@ -173,23 +178,25 @@ export default createWidget("hamburger-menu", {
       });
     }
 
-    if (this.siteSettings.tagging_enabled) {
+    if (siteSettings.tagging_enabled) {
       links.push({ route: "tags", label: "tagging.tags" });
     }
 
     const extraLinks = flatten(
-      applyDecorators(this, "generalLinks", this.attrs, this.state)
+      applyDecorators(this, "generalLinks", attrs, state)
     );
+
     return links.concat(extraLinks).map(l => this.attach("link", l));
   },
 
   listCategories() {
-    const maxCategoriesToDisplay = this.siteSettings
-      .header_dropdown_category_count;
+    const { currentUser, site, siteSettings } = this;
+    const maxCategoriesToDisplay = siteSettings.header_dropdown_category_count;
+
     let categories = [];
 
-    if (this.currentUser) {
-      const allCategories = this.site
+    if (currentUser) {
+      const allCategories = site
         .get("categories")
         .filter(c => c.notification_level !== NotificationLevels.MUTED);
 
@@ -203,7 +210,8 @@ export default createWidget("hamburger-menu", {
           );
         });
 
-      const topCategoryIds = this.currentUser.get("top_category_ids") || [];
+      const topCategoryIds = currentUser.get("top_category_ids") || [];
+
       topCategoryIds.forEach(id => {
         const category = allCategories.find(c => c.id === id);
         if (category && !categories.includes(category)) {
@@ -217,14 +225,14 @@ export default createWidget("hamburger-menu", {
           .sort((a, b) => b.topic_count - a.topic_count)
       );
     } else {
-      categories = this.site
+      categories = site
         .get("categoriesByCount")
         .filter(c => c.notification_level !== NotificationLevels.MUTED);
     }
 
-    if (!this.siteSettings.allow_uncategorized_topics) {
+    if (!siteSettings.allow_uncategorized_topics) {
       categories = categories.filter(
-        c => c.id !== this.site.uncategorized_category_id
+        c => c.id !== site.uncategorized_category_id
       );
     }
 
@@ -235,8 +243,10 @@ export default createWidget("hamburger-menu", {
   },
 
   footerLinks(prioritizeFaq, faqUrl) {
+    const { attrs, capabilities, settings, site, siteSettings, state } = this;
     const links = [];
-    if (this.settings.showAbout) {
+
+    if (settings.showAbout) {
       links.push({
         route: "about",
         className: "about-link",
@@ -244,12 +254,11 @@ export default createWidget("hamburger-menu", {
       });
     }
 
-    if (this.settings.showFAQ && !prioritizeFaq) {
+    if (settings.showFAQ && !prioritizeFaq) {
       links.push({ href: faqUrl, className: "faq-link", label: "faq" });
     }
 
-    const { site } = this;
-    if (!site.mobileView && !this.capabilities.touch) {
+    if (!site.mobileView && !capabilities.touch) {
       links.push({
         href: "",
         action: "showKeyboard",
@@ -258,29 +267,28 @@ export default createWidget("hamburger-menu", {
       });
     }
 
-    if (
-      this.site.mobileView ||
-      (this.siteSettings.enable_mobile_theme && this.capabilities.touch)
-    ) {
+    const mobileTouch = siteSettings.enable_mobile_theme && capabilities.touch;
+    if (site.mobileView || mobileTouch) {
       links.push({
         action: "toggleMobileView",
         className: "mobile-toggle-link",
-        label: this.site.mobileView ? "desktop_view" : "mobile_view"
+        label: site.mobileView ? "desktop_view" : "mobile_view"
       });
     }
 
     const extraLinks = flatten(
-      applyDecorators(this, "footerLinks", this.attrs, this.state)
+      applyDecorators(this, "footerLinks", attrs, state)
     );
+
     return links.concat(extraLinks).map(l => this.attach("link", l));
   },
 
   panelContents() {
-    const { currentUser } = this;
+    const { attrs, currentUser, settings, siteSettings, state } = this;
     const results = [];

[... diff too long, it was truncated ...]

GitHub sha: bc63232d

This commit appears in #10368 which was approved by eviltrout. It was merged by ZogStriP.