FEATURE: Show stale reviewable to other clients (#13114)

FEATURE: Show stale reviewable to other clients (#13114)

The previous commits removed reviewables leading to a bad user experience. This commit updates the status, replaces actions with a message and greys out the reviewable.

diff --git a/app/assets/javascripts/discourse/app/components/reviewable-item.js b/app/assets/javascripts/discourse/app/components/reviewable-item.js
index bf436f4..23071ca 100644
--- a/app/assets/javascripts/discourse/app/components/reviewable-item.js
+++ b/app/assets/javascripts/discourse/app/components/reviewable-item.js
@@ -29,12 +29,17 @@ export default Component.extend({
 
   @discourseComputed(
     "reviewable.type",
+    "reviewable.stale",
     "siteSettings.blur_tl0_flagged_posts_media",
     "reviewable.target_created_by_trust_level"
   )
-  customClasses(type, blurEnabled, trustLevel) {
+  customClasses(type, stale, blurEnabled, trustLevel) {
     let classes = type.dasherize();
 
+    if (stale) {
+      classes = `${classes} reviewable-stale`;
+    }
+
     if (blurEnabled && trustLevel === 0) {
       classes = `${classes} blur-images`;
     }
diff --git a/app/assets/javascripts/discourse/app/routes/review-index.js b/app/assets/javascripts/discourse/app/routes/review-index.js
index b4d15e6..187c91c 100644
--- a/app/assets/javascripts/discourse/app/routes/review-index.js
+++ b/app/assets/javascripts/discourse/app/routes/review-index.js
@@ -55,6 +55,18 @@ export default DiscourseRoute.extend({
         });
       }
     });
+
+    this.messageBus.subscribe("/reviewable_counts", (data) => {
+      if (data.updates) {
+        this.controller.reviewables.forEach((reviewable) => {
+          const updates = data.updates[reviewable.id];
+          if (updates) {
+            reviewable.setProperties(updates);
+            reviewable.set("stale", true);
+          }
+        });
+      }
+    });
   },
 
   deactivate() {
diff --git a/app/assets/javascripts/discourse/app/templates/components/reviewable-item.hbs b/app/assets/javascripts/discourse/app/templates/components/reviewable-item.hbs
index 79f25ca..9288db1 100644
--- a/app/assets/javascripts/discourse/app/templates/components/reviewable-item.hbs
+++ b/app/assets/javascripts/discourse/app/templates/components/reviewable-item.hbs
@@ -44,42 +44,46 @@
     {{/if}}
   </div>
   <div class="reviewable-actions">
-    {{#if claimEnabled}}
-      <div class="claimed-actions">
-        <span class="help">{{html-safe claimHelp}}</span>
-        {{reviewable-claimed-topic topicId=topicId claimedBy=reviewable.claimed_by}}
-      </div>
-    {{/if}}
-
-    {{#if canPerform}}
-      {{#if editing}}
-        {{d-button
-          class="btn-primary reviewable-action save-edit"
-          disabled=updating
-          icon="check"
-          action=(action "saveEdit")
-          label="review.save"}}
-        {{d-button
-          class="btn-danger reviewable-action cancel-edit"
-          disabled=updating
-          icon="times"
-          action=(action "cancelEdit")
-          label="review.cancel"}}
-      {{else}}
-        {{#each reviewable.bundled_actions as |bundle|}}
-          {{reviewable-bundled-action
-            bundle=bundle
-            performAction=(action "perform")
-            reviewableUpdating=updating}}
-        {{/each}}
+    {{#if reviewable.stale}}
+      <div class="stale-help">{{i18n "review.stale_help"}}</div>
+    {{else}}
+      {{#if claimEnabled}}
+        <div class="claimed-actions">
+          <span class="help">{{html-safe claimHelp}}</span>
+          {{reviewable-claimed-topic topicId=topicId claimedBy=reviewable.claimed_by}}
+        </div>
+      {{/if}}
 
-        {{#if reviewable.can_edit}}
+      {{#if canPerform}}
+        {{#if editing}}
           {{d-button
-            class="reviewable-action edit"
-            disabled=updating
-            icon="pencil-alt"
-            action=(action "edit")
-            label="review.edit"}}
+            class="btn-primary reviewable-action save-edit"
+            disabled=disabled
+            icon="check"
+            action=(action "saveEdit")
+            label="review.save"}}
+          {{d-button
+            class="btn-danger reviewable-action cancel-edit"
+            disabled=disabled
+            icon="times"
+            action=(action "cancelEdit")
+            label="review.cancel"}}
+        {{else}}
+          {{#each reviewable.bundled_actions as |bundle|}}
+            {{reviewable-bundled-action
+              bundle=bundle
+              performAction=(action "perform")
+              reviewableUpdating=disabled}}
+          {{/each}}
+
+          {{#if reviewable.can_edit}}
+            {{d-button
+              class="reviewable-action edit"
+              disabled=disabled
+              icon="pencil-alt"
+              action=(action "edit")
+              label="review.edit"}}
+          {{/if}}
         {{/if}}
       {{/if}}
     {{/if}}
diff --git a/app/assets/javascripts/discourse/tests/acceptance/review-test.js b/app/assets/javascripts/discourse/tests/acceptance/review-test.js
index ad7041a..dbdf2f0 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/review-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/review-test.js
@@ -1,4 +1,8 @@
-import { acceptance, queryAll } from "discourse/tests/helpers/qunit-helpers";
+import {
+  acceptance,
+  publishToMessageBus,
+  queryAll,
+} from "discourse/tests/helpers/qunit-helpers";
 import { click, fillIn, visit } from "@ember/test-helpers";
 import I18n from "I18n";
 import selectKit from "discourse/tests/helpers/select-kit-helper";
@@ -193,4 +197,26 @@ acceptance("Review", function (needs) {
     );
     assert.equal(queryAll(`${topic} .category-name`).text().trim(), "support");
   });
+
+  test("Reviewables can become stale", async function (assert) {
+    await visit("/review");
+
+    const reviewable = find("[data-reviewable-id=1234]")[0];
+    assert.notOk(reviewable.className.includes("reviewable-stale"));
+    assert.equal(find("[data-reviewable-id=1234] .status .pending").length, 1);
+    assert.equal(find(".stale-help").length, 0);
+
+    publishToMessageBus("/reviewable_counts", {
+      review_count: 1,
+      updates: {
+        1234: { status: 1 },
+      },
+    });
+
+    await visit("/review"); // wait for re-render
+
+    assert.ok(reviewable.className.includes("reviewable-stale"));
+    assert.equal(find("[data-reviewable-id=1234] .status .approved").length, 1);
+    assert.equal(find(".stale-help").length, 1);
+  });
 });
diff --git a/app/assets/stylesheets/common/base/reviewables.scss b/app/assets/stylesheets/common/base/reviewables.scss
index 7fbbb1a..5a22222 100644
--- a/app/assets/stylesheets/common/base/reviewables.scss
+++ b/app/assets/stylesheets/common/base/reviewables.scss
@@ -276,6 +276,10 @@
   padding-bottom: 1em;
 }
 
+.reviewable-stale {
+  opacity: 0.7;
+}
+
 .blur-images {
   img:not(.avatar):not(.emoji) {
     filter: blur(10px);
diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb
index f924999..5880bb7 100644
--- a/app/controllers/reviewables_controller.rb
+++ b/app/controllers/reviewables_controller.rb
@@ -12,11 +12,11 @@ class ReviewablesController < ApplicationController
     offset = params[:offset].to_i
 
     if params[:type].present?
-      raise Discourse::InvalidParameter.new(:type) unless Reviewable.valid_type?(params[:type])
+      raise Discourse::InvalidParameters.new(:type) unless Reviewable.valid_type?(params[:type])
     end
 
     status = (params[:status] || 'pending').to_sym
-    raise Discourse::InvalidParameter.new(:status) unless allowed_statuses.include?(status)
+    raise Discourse::InvalidParameters.new(:status) unless allowed_statuses.include?(status)
 
     topic_id = params[:topic_id] ? params[:topic_id].to_i : nil
     category_id = params[:category_id] ? params[:category_id].to_i : nil
diff --git a/app/jobs/regular/notify_reviewable.rb b/app/jobs/regular/notify_reviewable.rb
index 88632a6..3205129 100644
--- a/app/jobs/regular/notify_reviewable.rb
+++ b/app/jobs/regular/notify_reviewable.rb
@@ -15,28 +15,58 @@ class Jobs::NotifyReviewable < ::Jobs::Base

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

GitHub sha: 197e3f24

This commit appears in #13114 which was approved by ZogStriP. It was merged by SamSaffron.

@udan11 there is a minor bug here, when I handle a flag it tells me “someone else handled this flag” instead of telling me that I did it. It gets a bit confusing.

Maybe instead of saying “someone else” we just say the username of the person who handled it?

1 Like