Merge pull request #3378 from techAPJ/patch-1

FIX: show error message if user already exist in group

app/assets/javascripts/discourse/models/group.js.es6

@@ -52,6 +52,12 @@ const Group = Discourse.Model.extend({
     }).then(function() {
       // reload member list
       self.findMembers();
+    }).catch(function(error) {
+      if (error && error.responseText) {
+        bootbox.alert($.parseJSON(error.responseText).errors[0]);
+      } else {
+        bootbox.alert(I18n.t('generic_error'));
+      }
     });
   },
 

app/controllers/admin/groups_controller.rb

@@ -83,7 +83,11 @@ class Admin::GroupsController < Admin::AdminController
     end
 
     users.each do |user|
+      if !group.users.include?(user)
         group.add(user)
+      else
+        return render_json_error I18n.t('groups.errors.member_already_exist', username: user.username)
+      end
     end
 
     if group.save

config/locales/server.en.yml

@@ -180,6 +180,7 @@ en:
   groups:
     errors:
       can_not_modify_automatic: "You can not modify an automatic group"
+      member_already_exist: "'%{username}' is already a member of this group."
     default_names:
       everyone: "everyone"
       admins: "admins"

spec/controllers/admin/groups_controller_spec.rb

@@ -136,6 +136,16 @@ describe Admin::GroupsController do
       end
     end
 
+    it "returns 422 if member already exists" do
+      group = Fabricate(:group)
+      existing_member = Fabricate(:user)
+      group.add(existing_member)
+      group.save
+
+      xhr :put, :add_members, id: group.id, usernames: existing_member.username
+      expect(response.status).to eq(422)
+    end
+
   end
 
   context ".remove_member" do