From 658b3103052b4e023f2d2096061cc506d17f4548 Mon Sep 17 00:00:00 2001
From: Krzysztof Kotlarek <kotlarek.krzysztof@gmail.com>
Date: Mon, 5 Jun 2023 13:25:29 +1000
Subject: [PATCH] FIX: simplify review community section link (#21907)

Before, the review button was shown in `primary section` when there were items to review. Otherwise, it was hidden in `more section`.

Because we are allowing admins to customize community section and reorder link, it makes sense to simplify that logic and review link should follow admin's decision.
---
 .../app/lib/sidebar/community-section.js      | 12 +-----
 .../community-section/review-section-link.js  | 42 +------------------
 spec/system/custom_sidebar_sections_spec.rb   | 14 ++++---
 3 files changed, 13 insertions(+), 55 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/community-section.js b/app/assets/javascripts/discourse/app/lib/sidebar/community-section.js
index 0f12705106d..38840c5d357 100644
--- a/app/assets/javascripts/discourse/app/lib/sidebar/community-section.js
+++ b/app/assets/javascripts/discourse/app/lib/sidebar/community-section.js
@@ -22,8 +22,6 @@ import {
 } from "discourse/lib/sidebar/custom-community-section-links";
 import showModal from "discourse/lib/show-modal";
 
-const LINKS_IN_BOTH_SEGMENTS = ["/review"];
-
 const SPECIAL_LINKS_MAP = {
   "/latest": EverythingSectionLink,
   "/new": EverythingSectionLink,
@@ -66,10 +64,7 @@ export default class CommunitySection {
       .map((link) => this.#initializeSectionLink(link, { inMoreDrawer: true }));
 
     this.links = this.section.links.reduce((filtered, link) => {
-      if (
-        link.segment === "primary" ||
-        LINKS_IN_BOTH_SEGMENTS.includes(link.value)
-      ) {
+      if (link.segment === "primary") {
         const generatedLink = this.#generateLink(link);
 
         if (generatedLink) {
@@ -82,10 +77,7 @@ export default class CommunitySection {
 
     this.moreLinks = this.section.links
       .reduce((filtered, link) => {
-        if (
-          link.segment === "secondary" ||
-          LINKS_IN_BOTH_SEGMENTS.includes(link.value)
-        ) {
+        if (link.segment === "secondary") {
           const generatedLink = this.#generateLink(link, true);
 
           if (generatedLink) {
diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/user/community-section/review-section-link.js b/app/assets/javascripts/discourse/app/lib/sidebar/user/community-section/review-section-link.js
index f9a66aa4885..a3db1f591d1 100644
--- a/app/assets/javascripts/discourse/app/lib/sidebar/user/community-section/review-section-link.js
+++ b/app/assets/javascripts/discourse/app/lib/sidebar/user/community-section/review-section-link.js
@@ -1,43 +1,9 @@
 import I18n from "I18n";
-
-import { tracked } from "@glimmer/tracking";
-
-import { bind } from "discourse-common/utils/decorators";
 import BaseSectionLink from "discourse/lib/sidebar/base-community-section-link";
 
 export default class ReviewSectionLink extends BaseSectionLink {
-  @tracked canDisplay;
-
-  constructor() {
-    super(...arguments);
-
-    this._refreshCanDisplay();
-
-    this.appEvents?.on(
-      "user-reviewable-count:changed",
-      this._refreshCanDisplay
-    );
-  }
-
-  teardown() {
-    this.appEvents?.off(
-      "user-reviewable-count:changed",
-      this._refreshCanDisplay
-    );
-  }
-
-  @bind
-  _refreshCanDisplay() {
-    if (!this.currentUser?.can_review) {
-      this.canDisplay = false;
-      return;
-    }
-
-    if (this.inMoreDrawer) {
-      this.canDisplay = this.currentUser.reviewable_count < 1;
-    } else {
-      this.canDisplay = this.currentUser.reviewable_count > 0;
-    }
+  get shouldDisplay() {
+    return !!this.currentUser?.can_review;
   }
 
   get name() {
@@ -59,10 +25,6 @@ export default class ReviewSectionLink extends BaseSectionLink {
     );
   }
 
-  get shouldDisplay() {
-    return this.canDisplay;
-  }
-
   get badgeText() {
     // force a tracker for reviewable_count by using .get to ensure badgeText
     // rerenders when reviewable_count changes
diff --git a/spec/system/custom_sidebar_sections_spec.rb b/spec/system/custom_sidebar_sections_spec.rb
index 3875f320a4a..52422a015ea 100644
--- a/spec/system/custom_sidebar_sections_spec.rb
+++ b/spec/system/custom_sidebar_sections_spec.rb
@@ -197,7 +197,9 @@ describe "Custom sidebar sections", type: :system, js: true do
     sign_in admin
     visit("/latest")
 
-    expect(sidebar.primary_section_icons("community")).to eq(%w[layer-group user wrench ellipsis-v])
+    expect(sidebar.primary_section_icons("community")).to eq(
+      %w[layer-group user flag wrench ellipsis-v],
+    )
 
     sidebar.edit_custom_section("Community")
     section_modal.fill_link("Everything", "/latest", "paper-plane")
@@ -207,10 +209,10 @@ describe "Custom sidebar sections", type: :system, js: true do
 
     expect(sidebar).to have_section("Edited community section")
     expect(sidebar.primary_section_links("edited-community-section")).to eq(
-      ["My Posts", "Everything", "Admin", "More"],
+      ["My Posts", "Everything", "Review", "Admin", "More"],
     )
     expect(sidebar.primary_section_icons("edited-community-section")).to eq(
-      %w[user paper-plane wrench ellipsis-v],
+      %w[user paper-plane flag wrench ellipsis-v],
     )
 
     sidebar.edit_custom_section("Edited community section")
@@ -218,9 +220,11 @@ describe "Custom sidebar sections", type: :system, js: true do
 
     expect(sidebar).to have_section("Community")
     expect(sidebar.primary_section_links("community")).to eq(
-      ["Everything", "My Posts", "Admin", "More"],
+      ["Everything", "My Posts", "Review", "Admin", "More"],
+    )
+    expect(sidebar.primary_section_icons("community")).to eq(
+      %w[layer-group user flag wrench ellipsis-v],
     )
-    expect(sidebar.primary_section_icons("community")).to eq(%w[layer-group user wrench ellipsis-v])
   end
 
   it "shows anonymous public sections" do