mobile buttons for commit / followup don't allow self to approve / mark for followup add brand new first spec

mobile buttons for commit / followup
don’t allow self to approve / mark for followup
add brand new first spec

From 64d0ec487f97e828b2ccf8b1f7826ee4c02b93d2 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 21 Nov 2018 15:51:34 +1100
Subject: [PATCH] mobile buttons for commit / followup don't allow self to
 approve / mark for followup add brand new first spec


diff --git a/app/controllers/discourse_code_review/code_review_controller.rb b/app/controllers/discourse_code_review/code_review_controller.rb
new file mode 100644
index 0000000..708e7b9
--- /dev/null
+++ b/app/controllers/discourse_code_review/code_review_controller.rb
@@ -0,0 +1,63 @@
+class ::DiscourseCodeReview::CodeReviewController < ::ApplicationController
+  before_action :ensure_logged_in
+  before_action :ensure_staff
+
+  def followup
+    topic = Topic.find_by(id: params[:topic_id])
+
+    PostRevisor.new(topic.ordered_posts.first, topic)
+      .revise!(current_user,
+        category_id: SiteSetting.code_review_followup_category_id)
+
+    topic.add_moderator_post(
+      current_user,
+      nil,
+      bump: false,
+      post_type: Post.types[:small_action],
+      action_code: "followup"
+    )
+
+    render_next_topic
+
+  end
+
+  def approve
+    topic = Topic.find_by(id: params[:topic_id])
+
+    if topic.user_id == current_user.id
+      raise Discourse::InvalidAccess
+    end
+
+    PostRevisor.new(topic.ordered_posts.first, topic)
+      .revise!(current_user,
+        category_id: SiteSetting.code_review_approved_category_id)
+
+    topic.add_moderator_post(
+      current_user,
+      nil,
+      bump: false,
+      post_type: Post.types[:small_action],
+      action_code: "approved"
+    )
+
+    render_next_topic
+
+  end
+
+  protected
+
+  def render_next_topic
+    next_topic = Topic
+      .where(category_id: SiteSetting.code_review_pending_category_id)
+      .where('topics.id not in (select categories.topic_id from categories where categories.id = category_id)')
+      .order('bumped_at asc')
+      .first
+
+    url = next_topic&.relative_url
+
+    render json: {
+      next_topic_url: url
+    }
+  end
+
+end
diff --git a/assets/javascripts/discourse/initializers/init-code-review.js.es6 b/assets/javascripts/discourse/initializers/init-code-review.js.es6
index 659c6dd..89104e9 100644
--- a/assets/javascripts/discourse/initializers/init-code-review.js.es6
+++ b/assets/javascripts/discourse/initializers/init-code-review.js.es6
@@ -3,10 +3,60 @@ import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import DiscourseURL from "discourse/lib/url";
 
+function actOnCommit(topic, action) {
+  let topicId = topic.get("id");
+  return ajax(`/code-review/${action}.json`, {
+    type: "POST",
+    data: { topic_id: topicId }
+  })
+    .then(result => {
+      if (result.next_topic_url) {
+        DiscourseURL.routeTo(result.next_topic_url);
+      }
+    })
+    .catch(popupAjaxError);
+}
+
 function initialize(api) {
   api.addPostSmallActionIcon("followup", "clock-o");
   api.addPostSmallActionIcon("approved", "thumbs-up");
 
+  function allowUser(topic) {
+    const currentUser = api.getCurrentUser();
+    if (!currentUser) {
+      return false;
+    }
+    return (
+      currentUser.get("staff") && currentUser.get("id") !== topic.get("user_id")
+    );
+  }
+
+  api
+    .modifySelectKit("topic-footer-mobile-dropdown")
+    .modifyContent((context, existingContent) => {
+      if (allowUser(context.get("currentUser"))) {
+        existingContent.push({
+          id: "approve",
+          icon: "thumbs-up",
+          name: I18n.t("code_review.approve.label")
+        });
+
+        existingContent.push({
+          id: "followup",
+          icon: "clock-o",
+          name: I18n.t("code_review.followup.label")
+        });
+      }
+      return existingContent;
+    })
+    .onSelect((context, value) => {
+      if (value === "approve" || value === "followup") {
+        const topic = context.get("topic");
+        actOnCommit(topic, value);
+        return true;
+      }
+    });
+
   api.registerConnectorClass(
     "topic-footer-main-buttons-before-create",
     "approve",
@@ -15,41 +65,17 @@ function initialize(api) {
         this.set("topic", args.topic);
       },
       shouldRender: function(args, component) {
-        const currentUser = api.getCurrentUser();
-        if (!currentUser) {
-          return false;
-        }
         if (component.get("site.mobileView")) {
           return false;
         }
-        return currentUser.get("staff");
+        return allowUser(args.topic);
       },
       actions: {
         followupCommit() {
-          let topicId = this.get("topic.id");
-          return ajax("/code-review/followup.json", {
-            type: "POST",
-            data: { topic_id: topicId }
-          })
-            .then(result => {
-              if (result.next_topic_url) {
-                DiscourseURL.routeTo(result.next_topic_url);
-              }
-            })
-            .catch(popupAjaxError);
+          actOnCommit(this.get("topic"), "followup");
         },
         approveCommit() {
-          let topicId = this.get("topic.id");
-          return ajax("/code-review/approve.json", {
-            type: "POST",
-            data: { topic_id: topicId }
-          })
-            .then(result => {
-              if (result.next_topic_url) {
-                DiscourseURL.routeTo(result.next_topic_url);
-              }
-            })
-            .catch(popupAjaxError);
+          actOnCommit(this.get("topic"), "approve");
         }
       }
     }
diff --git a/plugin.rb b/plugin.rb
index 9b19db5..342e589 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -221,66 +221,7 @@ after_initialize do
   end
 
   require File.expand_path("../jobs/import_commits.rb", __FILE__)
-
-  class ::DiscourseCodeReview::CodeReviewController < ::ApplicationController
-    before_action :ensure_logged_in
-    before_action :ensure_staff
-
-    def followup
-      topic = Topic.find_by(id: params[:topic_id])
-
-      PostRevisor.new(topic.ordered_posts.first, topic)
-        .revise!(current_user,
-          category_id: SiteSetting.code_review_followup_category_id)
-
-      topic.add_moderator_post(
-        current_user,
-        nil,
-        bump: false,
-        post_type: Post.types[:small_action],
-        action_code: "followup"
-      )
-
-      render_next_topic
-
-    end
-
-    def approve
-      topic = Topic.find_by(id: params[:topic_id])
-
-      PostRevisor.new(topic.ordered_posts.first, topic)
-        .revise!(current_user,
-          category_id: SiteSetting.code_review_approved_category_id)
-
-      topic.add_moderator_post(
-        current_user,
-        nil,
-        bump: false,
-        post_type: Post.types[:small_action],
-        action_code: "approved"
-      )
-
-      render_next_topic
-
-    end
-
-    protected
-
-    def render_next_topic
-      next_topic = Topic
-        .where(category_id: SiteSetting.code_review_pending_category_id)
-        .where('topics.id not in (select categories.topic_id from categories where categories.id = category_id)')
-        .order('bumped_at asc')
-        .first
-
-      url = next_topic&.relative_url
-
-      render json: {
-        next_topic_url: url
-      }
-    end
-
-  end
+  require File.expand_path("../app/controllers/discourse_code_review/code_review_controller.rb", __FILE__)
 
   DiscourseCodeReview::Engine.routes.draw do
     post '/approve' => 'code_review#approve'
diff --git a/spec/requests/discourse_code_review/code_review_controller_spec.rb b/spec/requests/discourse_code_review/code_review_controller_spec.rb
new file mode 100644
index 0000000..34335d9
--- /dev/null
+++ b/spec/requests/discourse_code_review/code_review_controller_spec.rb
@@ -0,0 +1,18 @@
+require 'rails_helper'
+
+describe DiscourseCodeReview::CodeReviewController do
+  before do
+    SiteSetting.code_review_enabled = true
+  end
+  context '.approve' do
+    it 'does not allow you to approve your own commit' do
+      user = Fabricate(:admin

GitHub

1 Like