FEATURE: Publish a message when reviewable claimer changes (#9019)

This commit ensures that all users are kept in sync and no user can claim
a topic that has been claimed already.

app/assets/javascripts/discourse/routes/review-index.js.es6

@@ -30,6 +30,26 @@ export default DiscourseRoute.extend({
     });
   },
 
+  activate() {
+    this.messageBus.subscribe("/reviewable_claimed", data => {
+      const reviewables = this.controller.reviewables;
+      if (reviewables) {
+        const user = data.user
+          ? this.store.createRecord("user", data.user)
+          : null;
+        reviewables.forEach(reviewable => {
+          if (data.topic_id === reviewable.topic.id) {
+            reviewable.set("claimed_by", user);
+          }
+        });
+      }
+    });
+  },
+
+  deactivate() {
+    this.messageBus.unsubscribe("/reviewable_claimed");
+  },
+
   actions: {
     refreshRoute() {
       this.refresh();

app/controllers/reviewable_claimed_topics_controller.rb

@@ -6,13 +6,18 @@ class ReviewableClaimedTopicsController < ApplicationController
   def create
     topic = Topic.find_by(id: params[:reviewable_claimed_topic][:topic_id])
     guardian.ensure_can_claim_reviewable_topic!(topic)
-    ReviewableClaimedTopic.create(user_id: current_user.id, topic_id: topic.id)
+
+    begin
+      ReviewableClaimedTopic.create!(user_id: current_user.id, topic_id: topic.id)
+    rescue ActiveRecord::RecordInvalid
+      return render_json_error(I18n.t('reviewables.conflict'), status: 409)
+    end
+
     topic.reviewables.find_each do |reviewable|
       reviewable.log_history(:claimed, current_user)
     end
-    render json: success_json
-  rescue ActiveRecord::RecordNotUnique
-    # This is just in case the validation fails under concurrency
+
+    notify_users(topic, current_user)
     render json: success_json
   end
 
@@ -26,6 +31,26 @@ class ReviewableClaimedTopicsController < ApplicationController
       reviewable.log_history(:unclaimed, current_user)
     end
 
+    notify_users(topic, nil)
     render json: success_json
   end
+
+  private
+
+  def notify_users(topic, claimed_by)
+    user_ids = User.staff.pluck(:id)
+
+    if SiteSetting.enable_category_group_review? && group_id = topic.category&.reviewable_by_group_id.presence
+      user_ids.concat(GroupUser.where(group_id: group_id).pluck(:user_id))
+      user_ids.uniq!
+    end
+
+    if claimed_by.present?
+      data = { topic_id: topic.id, user: BasicUserSerializer.new(claimed_by, root: false).as_json }
+    else
+      data = { topic_id: topic.id }
+    end
+
+    MessageBus.publish("/reviewable_claimed", data, user_ids: user_ids)
+  end
 end

spec/requests/reviewable_claimed_topics_controller_spec.rb

@@ -5,72 +5,86 @@ require 'rails_helper'
 describe ReviewableClaimedTopicsController do
   fab!(:moderator) { Fabricate(:moderator) }
 
-  describe '#create' do
-    fab!(:topic) { Fabricate(:topic) }
-    fab!(:reviewable) { Fabricate(:reviewable_flagged_post, topic: topic) }
-    let(:params) do
-      { reviewable_claimed_topic: { topic_id: topic.id } }
-    end
+  fab!(:topic) { Fabricate(:topic) }
+  fab!(:reviewable) { Fabricate(:reviewable_flagged_post, topic: topic) }
 
-    it "requires you to be logged in" do
+  describe '#create' do
+    let(:params) { { reviewable_claimed_topic: { topic_id: topic.id } } }
+
+    it "requires user to be logged in" do
       post "/reviewable_claimed_topics.json", params: params
-      expect(response.code).to eq("403")
+
+      expect(response.status).to eq(403)
     end
 
     context "when logged in" do
-
       before do
         sign_in(moderator)
       end
 
-      it "will raise an error if you can't claim the topic" do
-        post "/reviewable_claimed_topics.json", params: params
-        expect(response.code).to eq("403")
-      end
-
-      it "will return 200 if the user can claim the topic" do
+      it "works" do
         SiteSetting.reviewable_claiming = 'optional'
-        post "/reviewable_claimed_topics.json", params: params
-        expect(response.code).to eq("200")
+
+        messages = MessageBus.track_publish { post "/reviewable_claimed_topics.json", params: params }
+
+        expect(response.status).to eq(200)
         expect(ReviewableClaimedTopic.where(user_id: moderator.id, topic_id: topic.id).exists?).to eq(true)
         expect(topic.reviewables.first.history.where(reviewable_history_type: ReviewableHistory.types[:claimed]).size).to eq(1)
+        expect(messages.size).to eq(1)
+        expect(messages[0].channel).to eq("/reviewable_claimed")
+        expect(messages[0].data[:topic_id]).to eq(topic.id)
+        expect(messages[0].data[:user][:id]).to eq(moderator.id)
       end
 
-      it "won't an error if you claim twice" do
+      it "raises an error if user cannot claim the topic" do
+        post "/reviewable_claimed_topics.json", params: params
+
+        expect(response.status).to eq(403)
+      end
+
+      it "raises an error if topic is already claimed" do
         SiteSetting.reviewable_claiming = 'optional'
+
         post "/reviewable_claimed_topics.json", params: params
         expect(ReviewableClaimedTopic.where(user_id: moderator.id, topic_id: topic.id).exists?).to eq(true)
+
         post "/reviewable_claimed_topics.json", params: params
-        expect(response.code).to eq("200")
+        expect(response.status).to eq(409)
       end
     end
   end
 
   describe '#destroy' do
-    fab!(:topic) { Fabricate(:topic) }
-    fab!(:reviewable) { Fabricate(:reviewable_flagged_post, topic: topic) }
     fab!(:claimed) { Fabricate(:reviewable_claimed_topic, topic: topic) }
 
     before do
       sign_in(moderator)
     end
 
-    it "404s for a missing topic" do
-      delete "/reviewable_claimed_topics/111111111.json"
-      expect(response.code).to eq("404")
-    end
-
-    it "403s when you can't claim the topic" do
-      delete "/reviewable_claimed_topics/#{claimed.topic_id}.json"
-      expect(response.code).to eq("403")
-    end
-
-    it "works when the feature is enabled" do
+    it "works" do
       SiteSetting.reviewable_claiming = 'optional'
-      delete "/reviewable_claimed_topics/#{claimed.topic_id}.json"
-      expect(response.code).to eq("200")
+
+      messages = MessageBus.track_publish { delete "/reviewable_claimed_topics/#{claimed.topic_id}.json" }
+
+      expect(response.status).to eq(200)
       expect(ReviewableClaimedTopic.where(topic_id: claimed.topic_id).exists?).to eq(false)
       expect(topic.reviewables.first.history.where(reviewable_history_type: ReviewableHistory.types[:unclaimed]).size).to eq(1)
+      expect(messages.size).to eq(1)
+      expect(messages[0].channel).to eq("/reviewable_claimed")
+      expect(messages[0].data[:topic_id]).to eq(topic.id)
+      expect(messages[0].data[:user]).to eq(nil)
+    end
+
+    it "raises an error if topic is missing" do
+      delete "/reviewable_claimed_topics/111111111.json"
+
+      expect(response.status).to eq(404)
+    end
+
+    it "raises an error if user cannot claim the topic" do
+      delete "/reviewable_claimed_topics/#{claimed.topic_id}.json"
+
+      expect(response.status).to eq(403)
     end
   end
 end