FEATURE: split group admin in 2 tabs (custom & automatic)

FIX: clear the user-selector when adding new members

app/assets/javascripts/admin/controllers/admin-group.js.es6

@@ -1,7 +1,6 @@
 export default Em.ObjectController.extend({
-  needs: ['adminGroups'],
+  needs: ['adminGroupsType'],
   disableSave: false,
-  usernames: null,
 
   currentPage: function() {
     if (this.get("user_count") == 0) { return 0; }
@@ -59,28 +58,29 @@ export default Em.ObjectController.extend({
     },
 
     addMembers: function() {
-      // TODO: should clear the input
       if (Em.isEmpty(this.get("usernames"))) { return; }
       this.get("model").addMembers(this.get("usernames"));
+      // clear the user selector
+      this.set("usernames", null);
     },
 
     save: function() {
       var self = this,
-          group = this.get('model');
+          group = this.get('model'),
+          groupsController = this.get("controllers.adminGroupsType");
 
-      self.set('disableSave', true);
+      this.set('disableSave', true);
 
       var promise;
-      if (group.get('id')) {
+      if (group.get("id")) {
         promise = group.save();
       } else {
         promise = group.create().then(function() {
-          var groupsController = self.get('controllers.adminGroups');
           groupsController.addObject(group);
         });
       }
       promise.then(function() {
-        self.send('showGroup', group);
+        self.transitionToRoute("adminGroup", group);
       }, function(e) {
         var message = $.parseJSON(e.responseText).errors;
         bootbox.alert(message);
@@ -91,12 +91,13 @@ export default Em.ObjectController.extend({
 
     destroy: function() {
       var group = this.get('model'),
-          groupsController = this.get('controllers.adminGroups'),
+          groupsController = this.get('controllers.adminGroupsType'),
           self = this;
 
-      bootbox.confirm(I18n.t("admin.groups.delete_confirm"), I18n.t("no_value"), I18n.t("yes_value"), function(result) {
+      this.set('disableSave', true);
-        if (result) {
+
-          self.set('disableSave', true);
+      bootbox.confirm(I18n.t("admin.groups.delete_confirm"), I18n.t("no_value"), I18n.t("yes_value"), function(confirmed) {
+        if (confirmed) {
           group.destroy().then(function() {
             groupsController.get('model').removeObject(group);
             self.transitionToRoute('adminGroups.index');

app/assets/javascripts/admin/controllers/admin-groups-type.js.es6

@@ -0,0 +1,16 @@
+export default Ember.ArrayController.extend({
+  sortProperties: ['name'],
+  refreshingAutoGroups: false,
+
+  actions: {
+    refreshAutoGroups: function(){
+      var self = this;
+      this.set('refreshingAutoGroups', true);
+      Discourse.ajax('/admin/groups/refresh_automatic_groups', {type: 'POST'}).then(function() {
+        self.transitionToRoute("adminGroupsType", "automatic").then(function() {
+          self.set('refreshingAutoGroups', false);
+        });
+      });
+    }
+  }
+});

app/assets/javascripts/admin/controllers/admin-groups.js.es6

@@ -1,24 +0,0 @@
-export default Ember.ArrayController.extend({
-  sortProperties: ['name'],
-
-  refreshingAutoGroups: false,
-
-  actions: {
-    refreshAutoGroups: function(){
-      var self = this,
-          groups = this.get('model');
-
-      self.set('refreshingAutoGroups', true);
-      this.transitionToRoute('adminGroups.index').then(function() {
-        Discourse.ajax('/admin/groups/refresh_automatic_groups', {type: 'POST'}).then(function() {
-          return Discourse.Group.findAll().then(function(newGroups) {
-            groups.clear();
-            groups.addObjects(newGroups);
-          }).finally(function() {
-            self.set('refreshingAutoGroups', false);
-          });
-        });
-      });
-    }
-  }
-});

app/assets/javascripts/admin/routes/admin-group-new.js.es6

@@ -0,0 +1,6 @@
+export default Discourse.Route.extend({
+  renderTemplate: function() {
+    debugger;
+    this.render("admin/templates/group");
+  }
+});

app/assets/javascripts/admin/routes/admin-group.js.es6

@@ -1,17 +1,20 @@
-Discourse.AdminGroupRoute = Discourse.Route.extend({
+export default Discourse.Route.extend({
 
   model: function(params) {
-    var groups = this.modelFor('adminGroups'),
+    var groups = this.modelFor('adminGroupsType'),
         group = groups.findProperty('name', params.name);
 
     if (!group) { return this.transitionTo('adminGroups.index'); }
+
     return group;
   },
 
   setupController: function(controller, model) {
     controller.set("model", model);
+    // clear the user selector
+    controller.set("usernames", null);
+    // load the members of the group
     model.findMembers();
   }
 
 });
-

app/assets/javascripts/admin/routes/admin-groups-index.js.es6

@@ -0,0 +1,5 @@
+export default Discourse.Route.extend({
+  redirect: function() {
+    this.transitionTo("adminGroupsType", "custom");
+  }
+})

app/assets/javascripts/admin/routes/admin-groups-type.js.es6

@@ -0,0 +1,17 @@
+export default Discourse.Route.extend({
+  model: function(params) {
+    return Discourse.Group.findAll().then(function(groups) {
+      return groups.filterBy("type", params.type);
+    });
+  },
+
+  actions: {
+    newGroup: function() {
+      var self = this;
+      this.transitionTo("adminGroupsType", "custom").then(function() {
+        var group = Discourse.Group.create({ automatic: false, visible: true });
+        self.transitionTo("adminGroup", group);
+      })
+    }
+  }
+});

app/assets/javascripts/admin/routes/admin-route-map.js.es6

@@ -40,8 +40,10 @@ export default function() {
       this.route('screenedUrls', { path: '/screened_urls' });
     });
 
-    this.resource('adminGroups', { path: '/groups'}, function() {
+    this.resource('adminGroups', { path: '/groups' }, function() {
-      this.resource('adminGroup', { path: '/:name' });
+      this.resource('adminGroupsType', { path: '/:type' }, function() {
+        this.resource('adminGroup', { path: '/:name' });
+      });
     });
 
     this.resource('adminUsers', { path: '/users' }, function() {
@@ -51,7 +53,7 @@ export default function() {
       });
 
       this.resource('adminUsersList', { path: '/list' }, function() {
-        this.route('show', {path: '/:filter'});
+        this.route('show', { path: '/:filter' });
       });
     });
 

app/assets/javascripts/admin/routes/admin_groups_route.js

@@ -1,31 +0,0 @@
-/**
-  Handles routes for admin groups
-
-  @class AdminGroupsRoute
-  @extends Discourse.Route
-  @namespace Discourse
-  @module Discourse
-**/
-Discourse.AdminGroupsRoute = Discourse.Route.extend({
-  model: function() {
-    return Discourse.Group.findAll();
-  },
-
-  actions: {
-    showGroup: function(g) {
-      // This hack is needed because the autocomplete plugin does not
-      // refresh properly when the underlying data changes. TODO should
-      // be to update the plugin so it works properly and remove this hack.
-      var self = this;
-      this.transitionTo('adminGroups.index').then(function() {
-        self.transitionTo('adminGroup', g);
-      });
-    },
-
-    newGroup: function(){
-      var group = Discourse.Group.create({ visible: true });
-      this.send('showGroup', group);
-    }
-  }
-});
-

app/assets/javascripts/admin/templates/admin.hbs

@@ -13,7 +13,7 @@
           <li>{{#link-to 'adminBadges.index'}}{{i18n 'admin.badges.title'}}{{/link-to}}</li>
         {{/if}}
         {{#if currentUser.admin}}
-          <li>{{#link-to 'adminGroups.index'}}{{i18n 'admin.groups.title'}}{{/link-to}}</li>
+          <li>{{#link-to 'adminGroups'}}{{i18n 'admin.groups.title'}}{{/link-to}}</li>
         {{/if}}
         <li>{{#link-to 'adminEmail'}}{{i18n 'admin.email.title'}}{{/link-to}}</li>
         <li>{{#link-to 'adminFlags'}}{{i18n 'admin.flags.title'}}{{/link-to}}</li>

app/assets/javascripts/admin/templates/groups.hbs

@@ -1,20 +1,11 @@
-<div class='row groups'>
+<div class="admin-controls">
-  <div class='content-list span6'>
+  <div class="span15">
-    <h3>{{i18n 'admin.groups.edit'}}</h3>
+    <ul class="nav nav-pills">
-    <ul>
+      <li>{{#link-to "adminGroupsType" "custom"}}{{i18n 'admin.groups.custom'}}{{/link-to}}</li>
-      {{#each group in arrangedContent}}
+      <li>{{#link-to "adminGroupsType" "automatic"}}{{i18n 'admin.groups.automatic'}}{{/link-to}}</li>
-        <li>
-        <a href='#' {{action "showGroup" group}}>{{group.name}} <span class="count">{{group.userCountDisplay}}</span></a>
-        </li>
-      {{/each}}
     </ul>
-    <div class='controls'>
-      <button class='btn' {{bind-attr disabled="refreshingAutoGroups"}} {{action "refreshAutoGroups"}}><i class="fa fa-refresh"></i>{{i18n 'admin.groups.refresh'}}</button>
-      <button class='btn' {{action "newGroup"}}><i class="fa fa-plus"></i>{{i18n 'admin.groups.new'}}</button>
-    </div>
-  </div>
-
-  <div class='content-editor'>
-    {{outlet}}
   </div>
 </div>
+<div class="admin-container">
+  {{outlet}}
+</div>

app/assets/javascripts/admin/templates/groups_index.hbs

@@ -1 +0,0 @@
-{{i18n 'admin.groups.about'}}

app/assets/javascripts/admin/templates/groups_type.hbs

@@ -0,0 +1,20 @@
+<div class='row groups'>
+  <div class='content-list span6'>
+    <h3>{{i18n 'admin.groups.edit'}}</h3>
+    <ul>
+      {{#each group in controller}}
+        <li>
+          {{#link-to "adminGroup" group.type group.name}}{{group.name}} <span class="count">{{group.userCountDisplay}}</span>{{/link-to}}
+        </li>
+      {{/each}}
+    </ul>
+    <div class='controls'>
+      {{d-button action="newGroup" icon="plus" label="admin.groups.new"}}
+      {{d-button action="refreshAutoGroups" icon="refresh" label="admin.groups.refresh" disabled=refreshingAutoGroups}}
+    </div>
+  </div>
+
+  <div class='content-editor'>
+    {{outlet}}
+  </div>
+</div>

app/assets/javascripts/discourse/components/user-selector.js.es6

@@ -16,18 +16,15 @@ export default TextField.extend({
       return selected;
     }
 
-    var template = this.container.lookup('template:user-selector-autocomplete.raw');
+    this.$().val(this.get('usernames')).autocomplete({
-    $(this.get('element')).val(this.get('usernames')).autocomplete({
+      template: this.container.lookup('template:user-selector-autocomplete.raw'),
-      template: template,
-
       disabled: this.get('disabled'),
       single: this.get('single'),
       allowAny: this.get('allowAny'),
 
       dataSource: function(term) {
-        term = term.replace(/[^a-zA-Z0-9_]/, '');
         return userSearch({
-          term: term,
+          term: term.replace(/[^a-zA-Z0-9_]/, ''),
           topicId: self.get('topicId'),
           exclude: excludedUsernames(),
           includeGroups: includeGroups
@@ -58,6 +55,15 @@ export default TextField.extend({
       }
 
     });
-  }.on('didInsertElement')
+  }.on('didInsertElement'),
+
+  // THIS IS A HUGE HACK TO SUPPORT CLEARING THE INPUT
+  _clearInput: function() {
+    if (arguments.length > 1) {
+      if (Em.isEmpty(this.get("usernames"))) {
+        this.$().parent().find("a").click();
+      }
+    }
+  }.observes("usernames")
 
 });

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

@@ -11,6 +11,10 @@ Discourse.Group = Discourse.Model.extend({
   offset: 0,
   user_count: 0,
 
+  type: function() {
+    return this.get("automatic") ? "automatic" : "custom";
+  }.property("automatic"),
+
   userCountDisplay: function(){
     var c = this.get('user_count');
     // don't display zero its ugly
@@ -20,7 +24,8 @@ Discourse.Group = Discourse.Model.extend({
   findMembers: function() {
     if (Em.isEmpty(this.get('name'))) { return ; }
 
-    var self = this, offset = Math.min(this.get("user_count"), Math.max(this.get("offset"), 0));
+    var self = this,
+        offset = Math.min(this.get("user_count"), Math.max(this.get("offset"), 0));
 
     return Discourse.ajax('/groups/' + this.get('name') + '/members.json', {
       data: {
@@ -100,7 +105,7 @@ Discourse.Group = Discourse.Model.extend({
 
 Discourse.Group.reopenClass({
   findAll: function(opts){
-    return Discourse.ajax("/admin/groups.json", { data: opts }).then(function(groups){
+    return Discourse.ajax("/admin/groups.json", { data: opts }).then(function (groups){
       return groups.map(function(g) { return Discourse.Group.create(g); });
     });
   },
@@ -112,8 +117,8 @@ Discourse.Group.reopenClass({
   },
 
   find: function(name) {
-    return Discourse.ajax("/groups/" + name + ".json").then(function(g) {
+    return Discourse.ajax("/groups/" + name + ".json").then(function (result) {
-      return Discourse.Group.create(g.basic_group);
+      return Discourse.Group.create(result.basic_group);
     });
   }
 });

app/assets/stylesheets/common/admin/admin_base.scss

@@ -477,6 +477,9 @@ section.details {
   .btn.add {
     margin-top: 7px;
   }
+  .controls {
+    margin-top: 10px;
+  }
 }
 
 // Customise area

app/controllers/admin/groups_controller.rb

@@ -32,7 +32,7 @@ class Admin::GroupsController < Admin::AdminController
   end
 
   def update
-    group = Group.find(params[:id].to_i)
+    group = Group.find(params[:id])
 
     group.alias_level = params[:alias_level].to_i if params[:alias_level].present?
     group.visible = params[:visible] == "true"
@@ -47,7 +47,7 @@ class Admin::GroupsController < Admin::AdminController
   end
 
   def destroy
-    group = Group.find(params[:id].to_i)
+    group = Group.find(params[:id])
 
     if group.automatic
       can_not_modify_automatic
@@ -63,7 +63,7 @@ class Admin::GroupsController < Admin::AdminController
   end
 
   def add_members
-    group = Group.find(params.require(:group_id).to_i)
+    group = Group.find(params.require(:id))
     usernames = params.require(:usernames)
 
     return can_not_modify_automatic if group.automatic
@@ -82,7 +82,7 @@ class Admin::GroupsController < Admin::AdminController
   end
 
   def remove_member
-    group = Group.find(params.require(:group_id).to_i)
+    group = Group.find(params.require(:id))
     user_id = params.require(:user_id).to_i
 
     return can_not_modify_automatic if group.automatic

config/locales/client.en.yml

@@ -1631,6 +1631,8 @@ en:
         name: "Name"
         add: "Add"
         add_members: "Add members"
+        custom: "Custom"
+        automatic: "Automatic"
 
       api:
         generate_master: "Generate Master API Key"

config/routes.rb

@@ -46,10 +46,15 @@ Discourse::Application.routes.draw do
       collection do
         post "refresh_automatic_groups" => "groups#refresh_automatic_groups"
       end
-      delete "members" => "groups#remove_member"
+      member do
-      put "members" => "groups#add_members"
+        put "members" => "groups#add_members"
+        delete "members" => "groups#remove_member"
+      end
     end
 
+    get "groups/:type" => "groups#show", constraints: AdminConstraint.new
+    get "groups/:type/:id" => "groups#show", constraints: AdminConstraint.new
+
     resources :users, id: USERNAME_ROUTE_FORMAT do
       collection do
         get "list/:query" => "users#index"

spec/controllers/admin/groups_controller_spec.rb

@@ -91,7 +91,7 @@ describe Admin::GroupsController do
   context ".add_members" do
 
     it "cannot add members to automatic groups" do
-      xhr :put, :add_members, group_id: 1, usernames: "l77t"
+      xhr :put, :add_members, id: 1, usernames: "l77t"
       expect(response.status).to eq(422)
     end
 
@@ -100,7 +100,7 @@ describe Admin::GroupsController do
       user2 = Fabricate(:user)
       group = Fabricate(:group)
 
-      xhr :put, :add_members, group_id: group.id, usernames: [user1.username, user2.username].join(",")
+      xhr :put, :add_members, id: group.id, usernames: [user1.username, user2.username].join(",")
 
       expect(response).to be_success
       group.reload
@@ -112,7 +112,7 @@ describe Admin::GroupsController do
   context ".remove_member" do
 
     it "cannot remove members from automatic groups" do
-      xhr :put, :remove_member, group_id: 1, user_id: 42
+      xhr :put, :remove_member, id: 1, user_id: 42
       expect(response.status).to eq(422)
     end
 
@@ -122,7 +122,7 @@ describe Admin::GroupsController do
       group.add(user)
       group.save
 
-      xhr :delete, :remove_member, group_id: group.id, user_id: user.id
+      xhr :delete, :remove_member, id: group.id, user_id: user.id
 
       expect(response).to be_success
       group.reload