From e48cf06fc95d1ca2f437bd232498416958cbb5c1 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 23 Apr 2014 13:25:02 -0400 Subject: [PATCH] REFACTOR: Add urls for admin groups, make it more idiomatic ember --- .../controllers/admin_group_controller.js | 77 +++++++++++++ .../controllers/admin_groups_controller.js | 54 +++------ .../admin/routes/admin_group_route.js | 23 ++++ .../admin/routes/admin_groups_route.js | 31 +++++ .../admin/routes/admin_groups_routes.js | 21 ---- .../javascripts/admin/routes/admin_routes.js | 4 +- .../admin/templates/admin.js.handlebars | 2 +- .../admin/templates/group.js.handlebars | 29 +++++ .../admin/templates/groups.js.handlebars | 44 +------- .../templates/groups_index.js.handlebars | 1 + .../javascripts/discourse/lib/autocomplete.js | 4 +- .../javascripts/discourse/models/group.js | 106 +++--------------- app/controllers/admin/groups_controller.rb | 6 +- app/models/group.rb | 1 + 14 files changed, 206 insertions(+), 197 deletions(-) create mode 100644 app/assets/javascripts/admin/controllers/admin_group_controller.js create mode 100644 app/assets/javascripts/admin/routes/admin_group_route.js create mode 100644 app/assets/javascripts/admin/routes/admin_groups_route.js delete mode 100644 app/assets/javascripts/admin/routes/admin_groups_routes.js create mode 100644 app/assets/javascripts/admin/templates/group.js.handlebars create mode 100644 app/assets/javascripts/admin/templates/groups_index.js.handlebars diff --git a/app/assets/javascripts/admin/controllers/admin_group_controller.js b/app/assets/javascripts/admin/controllers/admin_group_controller.js new file mode 100644 index 00000000000..4b5f5bfb004 --- /dev/null +++ b/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); + }); + } + }); + } + } +}); diff --git a/app/assets/javascripts/admin/controllers/admin_groups_controller.js b/app/assets/javascripts/admin/controllers/admin_groups_controller.js index ea5f6509383..563b4f17bc5 100644 --- a/app/assets/javascripts/admin/controllers/admin_groups_controller.js +++ b/app/assets/javascripts/admin/controllers/admin_groups_controller.js @@ -1,49 +1,23 @@ -Discourse.AdminGroupsController = Ember.Controller.extend({ - itemController: 'adminGroup', +Discourse.AdminGroupsController = Ember.ArrayController.extend({ + 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); - Discourse.ajax('/admin/groups/refresh_automatic_groups', {type: 'POST'}).then(function() { - return Discourse.Group.findAll().then(function(groups) { - self.set('model', groups); - 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); - } + 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); }); - } + }); }); } } diff --git a/app/assets/javascripts/admin/routes/admin_group_route.js b/app/assets/javascripts/admin/routes/admin_group_route.js new file mode 100644 index 00000000000..d4a8fd3cef7 --- /dev/null +++ b/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')); + } +}); + diff --git a/app/assets/javascripts/admin/routes/admin_groups_route.js b/app/assets/javascripts/admin/routes/admin_groups_route.js new file mode 100644 index 00000000000..e66299a50fb --- /dev/null +++ b/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); + } + } +}); + diff --git a/app/assets/javascripts/admin/routes/admin_groups_routes.js b/app/assets/javascripts/admin/routes/admin_groups_routes.js deleted file mode 100644 index 6db39d35a68..00000000000 --- a/app/assets/javascripts/admin/routes/admin_groups_routes.js +++ /dev/null @@ -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()); - } - -}); - diff --git a/app/assets/javascripts/admin/routes/admin_routes.js b/app/assets/javascripts/admin/routes/admin_routes.js index 7aceb61d33b..bc2a431a200 100644 --- a/app/assets/javascripts/admin/routes/admin_routes.js +++ b/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() { diff --git a/app/assets/javascripts/admin/templates/admin.js.handlebars b/app/assets/javascripts/admin/templates/admin.js.handlebars index 088de2b2b23..84b4ca72c63 100644 --- a/app/assets/javascripts/admin/templates/admin.js.handlebars +++ b/app/assets/javascripts/admin/templates/admin.js.handlebars @@ -14,7 +14,7 @@
  • {{#link-to 'admin.badges'}}{{i18n admin.badges.title}}{{/link-to}}
    • {{/if}} {{#if currentUser.admin}} -
  • {{#link-to 'admin.groups'}}{{i18n admin.groups.title}}{{/link-to}}
    • +
  • {{#link-to 'adminGroups.index'}}{{i18n admin.groups.title}}{{/link-to}}
    • {{/if}}
  • {{#link-to 'adminEmail'}}{{i18n admin.email.title}}{{/link-to}}
  • {{#link-to 'adminFlags'}}{{i18n admin.flags.title}}{{/link-to}}
    • diff --git a/app/assets/javascripts/admin/templates/group.js.handlebars b/app/assets/javascripts/admin/templates/group.js.handlebars new file mode 100644 index 00000000000..1988e84ce70 --- /dev/null +++ b/app/assets/javascripts/admin/templates/group.js.handlebars @@ -0,0 +1,29 @@ +{{#if automatic}} +

    {{name}}

    +{{else}} + {{textField value=name placeholderKey="admin.groups.name_placeholder"}} +{{/if}} + +
    + +
    + {{userSelector usernames=usernames id="group-users" placeholderKey="admin.groups.selector_placeholder" tabindex="1" disabled=automatic}} +
    +
    +
    +
    + {{input type="checkbox" checked=visible}} {{i18n groups.visible}} +
    +
    +
    + +
    + {{combobox valueAttribute="value" value=alias_level content=aliasLevelOptions}} +
    +
    +
    + + {{#unless automatic}} + {{i18n admin.customize.delete}} + {{/unless}} +
    diff --git a/app/assets/javascripts/admin/templates/groups.js.handlebars b/app/assets/javascripts/admin/templates/groups.js.handlebars index 925e532c06d..fcab6629385 100644 --- a/app/assets/javascripts/admin/templates/groups.js.handlebars +++ b/app/assets/javascripts/admin/templates/groups.js.handlebars @@ -2,9 +2,9 @@

    {{i18n admin.groups.edit}}

    @@ -15,44 +15,6 @@
    - {{#if model.active}} - {{#if model.active.loadedUsers}} - {{#with model.active}} - {{#if automatic}} -

    {{name}}

    - {{else}} - {{textField value=name placeholderKey="admin.groups.name_placeholder"}} - {{/if}} - -
    - -
    - {{userSelector usernames=usernames id="group-users" placeholderKey="admin.groups.selector_placeholder" tabindex="1" disabledBinding="automatic"}} -
    -
    -
    -
    - {{input type="checkbox" checked=visible}} {{i18n groups.visible}} -
    -
    -
    - -
    - {{combobox valueAttribute="value" value=alias_level content=controller.aliasLevelOptions}} -
    -
    -
    - - {{#unless automatic}} - {{i18n admin.customize.delete}} - {{/unless}} -
    - {{/with}} - {{else}} - - {{/if}} - {{else}} - {{i18n admin.groups.about}} - {{/if}} + {{outlet}}
    diff --git a/app/assets/javascripts/admin/templates/groups_index.js.handlebars b/app/assets/javascripts/admin/templates/groups_index.js.handlebars new file mode 100644 index 00000000000..5c29aa24aea --- /dev/null +++ b/app/assets/javascripts/admin/templates/groups_index.js.handlebars @@ -0,0 +1 @@ +{{i18n admin.groups.about}} diff --git a/app/assets/javascripts/discourse/lib/autocomplete.js b/app/assets/javascripts/discourse/lib/autocomplete.js index fdb424c0e86..e3cf8627ead 100644 --- a/app/assets/javascripts/discourse/lib/autocomplete.js +++ b/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; }; diff --git a/app/assets/javascripts/discourse/models/group.js b/app/assets/javascripts/discourse/models/group.js index 3502c92fea0..3ef5edc94df 100644 --- a/app/assets/javascripts/discourse/models/group.js +++ b/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; - - 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; - }); + if(!this.get('id')) return; + return Discourse.ajax("/admin/groups/" + this.get('id'), {type: "DELETE"}); }, asJSON: function() { @@ -91,36 +37,22 @@ Discourse.Group = Discourse.Model.extend({ usernames: this.get('usernames') } }; }, - create: function(){ - var self = this; - self.set('disableSave', true); + createWithUsernames: function(usernames){ + var self = this, + json = this.asJSON(); + json.group.usernames = usernames; - return Discourse.ajax("/admin/groups", {type: "POST", data: this.asJSON()}).then(function(resp) { - self.set('disableSave', false); - 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')); - } + return Discourse.ajax("/admin/groups", {type: "POST", data: json}).then(function(resp) { + self.set('id', resp.basic_group.id); }); }, - save: function(){ - var self = this; - self.set('disableSave', true); - + saveWithUsernames: function(usernames){ + var json = this.asJSON(); + json.group.usernames = usernames; return Discourse.ajax("/admin/groups/" + this.get('id'), { type: "PUT", - data: this.asJSON() - }).then(function(){ - self.set('disableSave', false); - }, function(e){ - var message = $.parseJSON(e.responseText).errors; - bootbox.alert(message); + data: json }); }, @@ -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(); - _.each(groups,function(group){ - list.addObject(Discourse.Group.create(group)); - }); - return list; + return groups.map(function(g) { return Discourse.Group.create(g); }); }); }, @@ -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; } }); diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index fcf92d29496..8803314d001 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/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 diff --git a/app/models/group.rb b/app/models/group.rb index 646e667c383..2741b30dfcb 100644 --- a/app/models/group.rb +++ b/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,