DEV: improve code readability & add tests for user guardian.

a511bea4cc

app/assets/javascripts/admin/controllers/modals/admin-merge-users-confirmation.js

@@ -2,6 +2,7 @@ import Controller, { inject as controller } from "@ember/controller";
 import ModalFunctionality from "discourse/mixins/modal-functionality";
 import discourseComputed from "discourse-common/utils/decorators";
 import { alias } from "@ember/object/computed";
+import { action } from "@ember/object";
 
 export default Controller.extend(ModalFunctionality, {
   adminUserIndex: controller(),
@@ -14,7 +15,10 @@ export default Controller.extend(ModalFunctionality, {
 
   @discourseComputed("username", "targetUsername")
   text(username, targetUsername) {
-    return `transfer @${username} to @${targetUsername}`;
+    return I18n.t(`admin.user.merge.confirmation.text`, {
+      username,
+      targetUsername
+    });
   },
 
   @discourseComputed("value", "text")
@@ -22,14 +26,14 @@ export default Controller.extend(ModalFunctionality, {
     return !value || text !== value;
   },
 
-  actions: {
-    merge() {
-      this.adminUserIndex.send("merge", this.targetUsername);
-      this.send("closeModal");
-    },
+  @action
+  confirm() {
+    this.adminUserIndex.send("merge", this.targetUsername);
+    this.send("closeModal");
+  },
 
-    cancel() {
-      this.send("closeModal");
-    }
+  @action
+  close() {
+    this.send("closeModal");
   }
 });

app/assets/javascripts/admin/controllers/modals/admin-merge-users-prompt.js

@@ -2,6 +2,7 @@ import Controller, { inject as controller } from "@ember/controller";
 import ModalFunctionality from "discourse/mixins/modal-functionality";
 import discourseComputed from "discourse-common/utils/decorators";
 import { alias } from "@ember/object/computed";
+import { action } from "@ember/object";
 
 export default Controller.extend(ModalFunctionality, {
   adminUserIndex: controller(),
@@ -16,14 +17,14 @@ export default Controller.extend(ModalFunctionality, {
     return !targetUsername || username === targetUsername;
   },
 
-  actions: {
-    merge() {
-      this.send("closeModal");
-      this.adminUserIndex.send("showMergeConfirmation", this.targetUsername);
-    },
+  @action
+  showConfirmation() {
+    this.send("closeModal");
+    this.adminUserIndex.send("showMergeConfirmation", this.targetUsername);
+  },
 
-    cancel() {
-      this.send("closeModal");
-    }
+  @action
+  close() {
+    this.send("closeModal");
   }
 });

app/assets/javascripts/admin/models/admin-user.js

@@ -5,7 +5,7 @@ import { ajax } from "discourse/lib/ajax";
 import { propertyNotEqual } from "discourse/lib/computed";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import Group from "discourse/models/group";
-import { userPath } from "discourse/lib/url";
+import DiscourseURL, { userPath } from "discourse/lib/url";
 import { Promise } from "rsvp";
 import User from "discourse/models/user";
 
@@ -514,16 +514,16 @@ const AdminUser = User.extend({
       formData["target_username"] = opts.targetUsername;
     }
 
-    return ajax(`/admin/users/${user.get("id")}/merge.json`, {
+    return ajax(`/admin/users/${user.id}/merge.json`, {
       type: "POST",
       data: formData
     })
-      .then(function(data) {
+      .then(data => {
         if (data.merged) {
           if (/^\/admin\/users\/list\//.test(location)) {
-            document.location = location;
+            DiscourseURL.redirectTo(location);
           } else {
-            document.location = Discourse.getURL(
+            DiscourseURL.redirectTo(
               `/admin/users/${data.user.id}/${data.user.username}`
             );
           }
@@ -534,8 +534,8 @@ const AdminUser = User.extend({
           }
         }
       })
-      .catch(function() {
-        AdminUser.find(user.get("id")).then(u => user.setProperties(u));
+      .catch(() => {
+        AdminUser.find(user.id).then(u => user.setProperties(u));
         bootbox.alert(I18n.t("admin.user.merge_failed"));
       });
   },

app/assets/javascripts/admin/templates/modal/admin-merge-users-confirmation.hbs

@@ -7,14 +7,14 @@
   <div class="modal-footer">
     {{#d-button
       class="btn-danger"
-      action=(action "merge")
+      action=(action "confirm")
       icon="trash-alt"
       disabled=mergeDisabled
     }}
       {{i18n "admin.user.merge.confirmation.transfer_and_delete" username=username}}
     {{/d-button}}
     {{d-button
-      action=(action "cancel")
+      action=(action "close")
       label="admin.user.merge.confirmation.cancel"
     }}
   </div>

app/assets/javascripts/admin/templates/modal/admin-merge-users-prompt.hbs

@@ -10,14 +10,14 @@
   <div class="modal-footer">
     {{#d-button
       class="btn-primary"
-      action=(action "merge")
+      action=(action "showConfirmation")
       icon="trash-alt"
       disabled=mergeDisabled
     }}
       {{i18n "admin.user.merge.prompt.transfer_and_delete" username=username}}
     {{/d-button}}
     {{d-button
-      action=(action "cancel")
+      action=(action "close")
       label="admin.user.merge.prompt.cancel"
     }}
   </div>

app/controllers/admin/users_controller.rb

@@ -475,13 +475,16 @@ class Admin::UsersController < Admin::AdminController
   def merge
     target_username = params.require(:target_username)
     target_user = User.find_by_username(target_username)
+    raise Discourse::NotFound if target_user.blank?
 
     guardian.ensure_can_merge_users!(@user, target_user)
+    serializer_opts = { root: false, scope: guardian }
 
     if user = UserMerger.new(@user, target_user, current_user).merge!
-      render json: success_json.merge(merged: true, user: user)
+      user_json = AdminDetailedUserSerializer.new(user, serializer_opts).as_json
+      render json: success_json.merge(merged: true, user: user_json)
     else
-      render json: failed_json.merge(user: AdminDetailedUserSerializer.new(@user, root: false).as_json)
+      render json: failed_json.merge(user: AdminDetailedUserSerializer.new(@user, serializer_opts).as_json)
     end
   end
 

config/locales/client.en.yml

@@ -4363,6 +4363,7 @@ en:
 
               <p>To continue type: <code>%{text}</code></p>
 
+            text: "transfer @%{username} to @%{targetUsername}"
             transfer_and_delete: "Transfer & Delete @%{username}"
             cancel: "Cancel"
 

spec/components/guardian/user_guardian_spec.rb

@@ -342,6 +342,29 @@ describe UserGuardian do
     end
   end
 
+  describe "#can_merge_user?" do
+    shared_examples "can_merge_user examples" do
+      it "isn't allowed if user is a staff" do
+        staff = Fabricate(:moderator)
+        expect(guardian.can_merge_user?(staff)).to eq(false)
+      end
+    end
+
+    context "for moderators" do
+      let(:guardian) { Guardian.new(moderator) }
+      include_examples "can_merge_user examples"
+
+      it "isn't allowed if current_user is not an admin" do
+        expect(guardian.can_merge_user?(user)).to eq(false)
+      end
+    end
+
+    context "for admins" do
+      let(:guardian) { Guardian.new(admin) }
+      include_examples "can_merge_user examples"
+    end
+  end
+
   describe "#can_see_review_queue?" do
     it 'returns true when the user is a staff member' do
       guardian = Guardian.new(moderator)