REFACTOR: Add urls for admin groups, make it more idiomatic ember

app/assets/javascripts/admin/controllers/admin_group_controller.js

@@ -0,0 +1,77 @@
+Discourse.AdminGroupController = Em.ObjectController.extend({
+  needs: ['adminGroups'],
+  members: null,
+  disableSave: false,
+
+  aliasLevelOptions: function() {
+    return [
+      { name: I18n.t("groups.alias_levels.nobody"), value: 0},
+      { name: I18n.t("groups.alias_levels.mods_and_admins"), value: 2},
+      { name: I18n.t("groups.alias_levels.members_mods_and_admins"), value: 3},
+      { name: I18n.t("groups.alias_levels.everyone"), value: 99}
+    ];
+  }.property(),
+
+  usernames: function(key, value) {
+    var members = this.get('members');
+    if (arguments.length > 1) {
+      this.set('_usernames', value);
+    } else {
+      var usernames;
+      if(members) {
+        usernames = members.map(function(user) {
+          return user.get('username');
+        }).join(',');
+      }
+      this.set('_usernames', usernames);
+    }
+    return this.get('_usernames');
+  }.property('members.@each.username'),
+
+  actions: {
+    save: function() {
+      var self = this,
+          group = this.get('model');
+
+      self.set('disableSave', true);
+
+      var promise;
+      if (group.get('id')) {
+        promise = group.saveWithUsernames(this.get('usernames'));
+      } else {
+        promise = group.createWithUsernames(this.get('usernames')).then(function() {
+          var groupsController = self.get('controllers.adminGroups');
+          groupsController.addObject(group);
+        });
+      }
+      promise.then(function() {
+        self.send('showGroup', group);
+      }, function(e) {
+        var message = $.parseJSON(e.responseText).errors;
+        bootbox.alert(message);
+      }).finally(function() {
+        self.set('disableSave', false);
+      });
+    },
+
+    destroy: function() {
+      var group = this.get('model'),
+          groupsController = this.get('controllers.adminGroups'),
+          self = this;
+
+      bootbox.confirm(I18n.t("admin.groups.delete_confirm"), I18n.t("no_value"), I18n.t("yes_value"), function(result) {
+        if (result) {
+          self.set('disableSave', true);
+          group.destroy().then(function() {
+            groupsController.get('model').removeObject(group);
+            self.transitionToRoute('adminGroups.index');
+          }, function() {
+            bootbox.alert(I18n.t("admin.groups.delete_failed"));
+          }).finally(function() {
+            self.set('disableSave', false);
+          });
+        }
+      });
+    }
+  }
+});

app/assets/javascripts/admin/controllers/admin_groups_controller.js

@@ -1,49 +1,23 @@
-Discourse.AdminGroupsController = Ember.Controller.extend({
+Discourse.AdminGroupsController = Ember.ArrayController.extend({
-  itemController: 'adminGroup',
+  sortProperties: ['name'],
+
+  refreshingAutoGroups: false,
 
   actions: {
-    edit: function(group){
-      this.get('model').select(group);
-      group.loadUsers();
-    },
-
     refreshAutoGroups: function(){
-      var self = this;
+      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(groups) {
+          return Discourse.Group.findAll().then(function(newGroups) {
-          self.set('model', groups);
+            groups.clear();
+            groups.addObjects(newGroups);
+          }).finally(function() {
             self.set('refreshingAutoGroups', false);
           });
         });
-    },
-
-    newGroup: function(){
-      var group = Discourse.Group.create({ loadedUsers: true, visible: true }),
-          model = this.get("model");
-      model.addObject(group);
-      model.select(group);
-    },
-
-    save: function(group){
-      if(!group.get("id")){
-        group.create();
-      } else {
-        group.save();
-      }
-    },
-
-    destroy: function(group){
-      var self = this;
-      return bootbox.confirm(I18n.t("admin.groups.delete_confirm"), I18n.t("no_value"), I18n.t("yes_value"), function(result) {
-        if (result) {
-          group.destroy().then(function(deleted) {
-            if (deleted) {
-              self.get("model").removeObject(group);
-            }
-          });
-        }
       });
     }
   }

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

@@ -0,0 +1,23 @@
+Discourse.AdminGroupRoute = Em.Route.extend({
+
+  model: function(params) {
+    var groups = this.modelFor('adminGroups'),
+        group = groups.findProperty('name', params.name);
+
+    if (!group) { return this.transitionTo('adminGroups.index'); }
+    return group;
+  },
+
+  afterModel: function(model) {
+    var self = this;
+    return model.findMembers().then(function(members) {
+      self.set('_members', members);
+    });
+  },
+
+  setupController: function(controller, model) {
+    controller.set('model', model);
+    controller.set('members', this.get('_members'));
+  }
+});
+

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

@@ -0,0 +1,31 @@
+/**
+  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/routes/admin_groups_routes.js

@@ -1,21 +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();
-  },
-
-  setupController: function(controller, model) {
-    controller.set('model', model);
-    controller.set('aliasLevelOptions', Discourse.Group.aliasLevelOptions());
-  }
-
-});
-

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

@@ -43,7 +43,9 @@ Discourse.Route.buildRoutes(function() {
       this.route('screenedUrls', { path: '/screened_urls' });
     });
 
-    this.route('groups');
+    this.resource('adminGroups', { path: '/groups'}, function() {
+      this.resource('adminGroup', { path: '/:name' });
+    });
 
     this.resource('adminUsers', { path: '/users' }, function() {
       this.resource('adminUser', { path: '/:username' }, function() {

app/assets/javascripts/admin/templates/admin.js.handlebars

@@ -14,7 +14,7 @@
           <li>{{#link-to 'admin.badges'}}{{i18n admin.badges.title}}{{/link-to}}</li>
         {{/if}}
         {{#if currentUser.admin}}
-          <li>{{#link-to 'admin.groups'}}{{i18n admin.groups.title}}{{/link-to}}</li>
+          <li>{{#link-to 'adminGroups.index'}}{{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/group.js.handlebars

@@ -0,0 +1,29 @@
+{{#if automatic}}
+  <h3>{{name}}</h3>
+{{else}}
+  {{textField value=name placeholderKey="admin.groups.name_placeholder"}}
+{{/if}}
+
+<div class="control-group">
+  <label class="control-label">{{i18n admin.groups.group_members}}</label>
+  <div class="controls">
+    {{userSelector usernames=usernames id="group-users" placeholderKey="admin.groups.selector_placeholder" tabindex="1" disabled=automatic}}
+  </div>
+</div>
+<div class="control-group">
+  <div class="controls">
+    {{input type="checkbox" checked=visible}} {{i18n groups.visible}}
+  </div>
+</div>
+<div class="control-group">
+  <label class="control-label">{{i18n groups.alias_levels.title}}</label>
+  <div class="controls">
+    {{combobox valueAttribute="value" value=alias_level content=aliasLevelOptions}}
+  </div>
+</div>
+<div class='controls'>
+  <button {{action save}} {{bind-attr disabled="disableSave"}} class='btn'>{{i18n admin.customize.save}}</button>
+  {{#unless automatic}}
+    <a {{action destroy}} class='delete-link'>{{i18n admin.customize.delete}}</a>
+  {{/unless}}
+</div>

app/assets/javascripts/admin/templates/groups.js.handlebars

@@ -2,9 +2,9 @@
   <div class='content-list span6'>
     <h3>{{i18n admin.groups.edit}}</h3>
     <ul>
-      {{#each group in model}}
+      {{#each group in arrangedContent}}
         <li>
-          <a href="#" {{action "edit" group}} {{bind-attr class="group.active"}}>{{group.name}} <span class="count">{{group.userCountDisplay}}</span></a>
+        <a href='#' {{action showGroup group}}>{{group.name}} <span class="count">{{group.userCountDisplay}}</span></a>
         </li>
       {{/each}}
     </ul>
@@ -15,44 +15,6 @@
   </div>
 
   <div class='content-editor'>
-    {{#if model.active}}
+    {{outlet}}
-      {{#if model.active.loadedUsers}}
-        {{#with model.active}}
-          {{#if automatic}}
-            <h3>{{name}}</h3>
-          {{else}}
-            {{textField value=name placeholderKey="admin.groups.name_placeholder"}}
-          {{/if}}
-
-          <div class="control-group">
-            <label class="control-label">{{i18n admin.groups.group_members}}</label>
-            <div class="controls">
-              {{userSelector usernames=usernames id="group-users" placeholderKey="admin.groups.selector_placeholder" tabindex="1" disabledBinding="automatic"}}
-            </div>
-          </div>
-          <div class="control-group">
-            <div class="controls">
-              {{input type="checkbox" checked=visible}} {{i18n groups.visible}}
-            </div>
-          </div>
-          <div class="control-group">
-            <label class="control-label">{{i18n groups.alias_levels.title}}</label>
-            <div class="controls">
-              {{combobox valueAttribute="value" value=alias_level content=controller.aliasLevelOptions}}
-            </div>
-          </div>
-          <div class='controls'>
-            <button {{action save this}} {{bind-attr disabled="disableSave"}} class='btn'>{{i18n admin.customize.save}}</button>
-            {{#unless automatic}}
-              <a {{action destroy this}} class='delete-link'>{{i18n admin.customize.delete}}</a>
-            {{/unless}}
-          </div>
-        {{/with}}
-      {{else}}
-        <div class='spinner'>{{i18n loading}}</div>
-      {{/if}}
-    {{else}}
-      {{i18n admin.groups.about}}
-    {{/if}}
   </div>
 </div>

app/assets/javascripts/admin/templates/groups_index.js.handlebars

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

app/assets/javascripts/discourse/lib/autocomplete.js

@@ -270,7 +270,7 @@ $.fn.autocomplete = function(options) {
     }
   });
 
-  return $(this).keydown(function(e) {
+  $(this).keydown(function(e) {
     var c, caretPosition, i, initial, next, prev, prevIsGood, stopFound, term, total, userToComplete;
 
     if(options.allowAny){
@@ -405,4 +405,6 @@ $.fn.autocomplete = function(options) {
       }
     }
   });
+
+  return this;
 };

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

@@ -6,22 +6,7 @@
   @namespace Discourse
   @module Discourse
 **/
-var ALIAS_LEVELS = {
-    nobody: 0,
-    only_admins: 1,
-    mods_and_admins: 2,
-    members_mods_and_admins: 3,
-    everyone: 99
-  },
-  aliasLevelOptions = [
-    { name: I18n.t("groups.alias_levels.nobody"), value: ALIAS_LEVELS.nobody},
-    { name: I18n.t("groups.alias_levels.mods_and_admins"), value: ALIAS_LEVELS.mods_and_admins},
-    { name: I18n.t("groups.alias_levels.members_mods_and_admins"), value: ALIAS_LEVELS.members_mods_and_admins},
-    { name: I18n.t("groups.alias_levels.everyone"), value: ALIAS_LEVELS.everyone}
-  ];
-
 Discourse.Group = Discourse.Model.extend({
-  loadedUsers: false,
 
   userCountDisplay: function(){
     var c = this.get('user_count');
@@ -31,56 +16,17 @@ Discourse.Group = Discourse.Model.extend({
     }
   }.property('user_count'),
 
-  // TODO: Refactor so adminGroups doesn't store the groups inside itself either.
   findMembers: function() {
+    if (Em.isEmpty(this.get('name'))) { return Ember.RSVP.resolve([]); }
+
     return Discourse.ajax('/groups/' + this.get('name') + '/members').then(function(result) {
       return result.map(function(u) { return Discourse.User.create(u) });
     });
   },
 
-  loadUsers: function() {
-    var id = this.get('id');
-    if(id && !this.get('loadedUsers')) {
-      var self = this;
-      return this.findMembers().then(function(users) {
-        self.set('users', users);
-        self.set('loadedUsers', true);
-        return self;
-      });
-    }
-    return Ember.RSVP.resolve(this);
-  },
-
-  usernames: function(key, value) {
-    var users = this.get('users');
-    if (arguments.length > 1) {
-      this.set('_usernames', value);
-    } else {
-      var usernames = "";
-      if(users) {
-        usernames = users.map(function(user) {
-          return user.get('username');
-        }).join(',');
-      }
-      this.set('_usernames', usernames);
-    }
-    return this.get('_usernames');
-  }.property('users.@each.username'),
-
   destroy: function(){
-    if(!this.id) return;
+    if(!this.get('id')) return;
-
+    return Discourse.ajax("/admin/groups/" + this.get('id'), {type: "DELETE"});
-    var self = this;
-    this.set('disableSave', true);
-
-    return Discourse.ajax("/admin/groups/" + this.get('id'), {type: "DELETE"})
-      .then(function(){
-        return true;
-      }, function() {
-        self.set('disableSave', false);
-        bootbox.alert(I18n.t("admin.groups.delete_failed"));
-        return false;
-      });
   },
 
   asJSON: function() {
@@ -91,36 +37,22 @@ Discourse.Group = Discourse.Model.extend({
              usernames: this.get('usernames') } };
   },
 
-  create: function(){
+  createWithUsernames: function(usernames){
-    var self = this;
+    var self = this,
-    self.set('disableSave', true);
+        json = this.asJSON();
+    json.group.usernames = usernames;
 
-    return Discourse.ajax("/admin/groups", {type: "POST", data: this.asJSON()}).then(function(resp) {
+    return Discourse.ajax("/admin/groups", {type: "POST", data: json}).then(function(resp) {
-      self.set('disableSave', false);
+      self.set('id', resp.basic_group.id);
-      self.set('id', resp.id);
-    }, function (error) {
-      self.set('disableSave', false);
-      if (error && error.responseText) {
-        bootbox.alert($.parseJSON(error.responseText).errors);
-      }
-      else {
-        bootbox.alert(I18n.t('generic_error'));
-      }
     });
   },
 
-  save: function(){
+  saveWithUsernames: function(usernames){
-    var self = this;
+    var json = this.asJSON();
-    self.set('disableSave', true);
+    json.group.usernames = usernames;
-
     return Discourse.ajax("/admin/groups/" + this.get('id'), {
       type: "PUT",
-      data: this.asJSON()
+      data: json
-    }).then(function(){
-      self.set('disableSave', false);
-    }, function(e){
-      var message = $.parseJSON(e.responseText).errors;
-      bootbox.alert(message);
     });
   },
 
@@ -142,11 +74,7 @@ Discourse.Group = Discourse.Model.extend({
 Discourse.Group.reopenClass({
   findAll: function(){
     return Discourse.ajax("/admin/groups.json").then(function(groups){
-      var list = Discourse.SelectableArray.create();
+      return groups.map(function(g) { return Discourse.Group.create(g); });
-      _.each(groups,function(group){
-        list.addObject(Discourse.Group.create(group));
-      });
-      return list;
     });
   },
 
@@ -160,9 +88,5 @@ Discourse.Group.reopenClass({
     return Discourse.ajax("/groups/" + name + ".json").then(function(g) {
       return Discourse.Group.create(g.basic_group);
     });
-  },
-
-  aliasLevelOptions: function() {
-    return aliasLevelOptions;
   }
 });

app/controllers/admin/groups_controller.rb

@@ -4,6 +4,10 @@ class Admin::GroupsController < Admin::AdminController
     render_serialized(groups, BasicGroupSerializer)
   end
 
+  def show
+    render nothing: true
+  end
+
   def refresh_automatic_groups
     Group.refresh_automatic_groups!
     render json: success_json
@@ -31,7 +35,7 @@ class Admin::GroupsController < Admin::AdminController
 
   def create
     group = Group.new
-    group.name = params[:group][:name].strip
+    group.name = (params[:group][:name] || '').strip
     group.usernames = params[:group][:usernames] if params[:group][:usernames]
     group.visible = params[:group][:visible] == "true"
     if group.save

app/models/group.rb

@@ -8,6 +8,7 @@ class Group < ActiveRecord::Base
   after_save :destroy_deletions
 
   validate :name_format_validator
+  validates_uniqueness_of :name
 
   AUTO_GROUPS = {
     :everyone => 0,