From e300945879211816a86860d89d440a1220c33e75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 21 Jan 2015 20:52:48 +0100 Subject: [PATCH] FEATURE: split group admin in 2 tabs (custom & automatic) FIX: clear the user-selector when adding new members --- .../admin/controllers/admin-group.js.es6 | 25 ++++++++------- .../controllers/admin-groups-type.js.es6 | 16 ++++++++++ .../admin/controllers/admin-groups.js.es6 | 24 -------------- .../admin/routes/admin-group-new.js.es6 | 6 ++++ ...dmin_group_route.js => admin-group.js.es6} | 9 ++++-- .../admin/routes/admin-groups-index.js.es6 | 5 +++ .../admin/routes/admin-groups-type.js.es6 | 17 ++++++++++ .../admin/routes/admin-route-map.js.es6 | 8 +++-- .../admin/routes/admin_groups_route.js | 31 ------------------- .../javascripts/admin/templates/admin.hbs | 2 +- .../javascripts/admin/templates/groups.hbs | 25 +++++---------- .../admin/templates/groups_index.hbs | 1 - .../admin/templates/groups_type.hbs | 20 ++++++++++++ .../discourse/components/user-selector.js.es6 | 20 +++++++----- .../javascripts/discourse/models/group.js | 13 +++++--- .../stylesheets/common/admin/admin_base.scss | 3 ++ app/controllers/admin/groups_controller.rb | 8 ++--- config/locales/client.en.yml | 2 ++ config/routes.rb | 9 ++++-- .../admin/groups_controller_spec.rb | 8 ++--- 20 files changed, 139 insertions(+), 113 deletions(-) create mode 100644 app/assets/javascripts/admin/controllers/admin-groups-type.js.es6 delete mode 100644 app/assets/javascripts/admin/controllers/admin-groups.js.es6 create mode 100644 app/assets/javascripts/admin/routes/admin-group-new.js.es6 rename app/assets/javascripts/admin/routes/{admin_group_route.js => admin-group.js.es6} (60%) create mode 100644 app/assets/javascripts/admin/routes/admin-groups-index.js.es6 create mode 100644 app/assets/javascripts/admin/routes/admin-groups-type.js.es6 delete mode 100644 app/assets/javascripts/admin/routes/admin_groups_route.js delete mode 100644 app/assets/javascripts/admin/templates/groups_index.hbs create mode 100644 app/assets/javascripts/admin/templates/groups_type.hbs diff --git a/app/assets/javascripts/admin/controllers/admin-group.js.es6 b/app/assets/javascripts/admin/controllers/admin-group.js.es6 index 6bd8eda6b..89b448b52 100644 --- a/app/assets/javascripts/admin/controllers/admin-group.js.es6 +++ b/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) { - if (result) { - self.set('disableSave', true); + this.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'); diff --git a/app/assets/javascripts/admin/controllers/admin-groups-type.js.es6 b/app/assets/javascripts/admin/controllers/admin-groups-type.js.es6 new file mode 100644 index 000000000..2d75d1911 --- /dev/null +++ b/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); + }); + }); + } + } +}); diff --git a/app/assets/javascripts/admin/controllers/admin-groups.js.es6 b/app/assets/javascripts/admin/controllers/admin-groups.js.es6 deleted file mode 100644 index 03de7cfe9..000000000 --- a/app/assets/javascripts/admin/controllers/admin-groups.js.es6 +++ /dev/null @@ -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); - }); - }); - }); - } - } -}); diff --git a/app/assets/javascripts/admin/routes/admin-group-new.js.es6 b/app/assets/javascripts/admin/routes/admin-group-new.js.es6 new file mode 100644 index 000000000..5e1cff95a --- /dev/null +++ b/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"); + } +}); diff --git a/app/assets/javascripts/admin/routes/admin_group_route.js b/app/assets/javascripts/admin/routes/admin-group.js.es6 similarity index 60% rename from app/assets/javascripts/admin/routes/admin_group_route.js rename to app/assets/javascripts/admin/routes/admin-group.js.es6 index 89388e1c3..f39729776 100644 --- a/app/assets/javascripts/admin/routes/admin_group_route.js +++ b/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(); } }); - diff --git a/app/assets/javascripts/admin/routes/admin-groups-index.js.es6 b/app/assets/javascripts/admin/routes/admin-groups-index.js.es6 new file mode 100644 index 000000000..33c42cbb0 --- /dev/null +++ b/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"); + } +}) diff --git a/app/assets/javascripts/admin/routes/admin-groups-type.js.es6 b/app/assets/javascripts/admin/routes/admin-groups-type.js.es6 new file mode 100644 index 000000000..5c04e2506 --- /dev/null +++ b/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); + }) + } + } +}); diff --git a/app/assets/javascripts/admin/routes/admin-route-map.js.es6 b/app/assets/javascripts/admin/routes/admin-route-map.js.es6 index 8d868d711..c5ce804ef 100644 --- a/app/assets/javascripts/admin/routes/admin-route-map.js.es6 +++ b/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('adminGroup', { path: '/:name' }); + this.resource('adminGroups', { path: '/groups' }, function() { + 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' }); }); }); diff --git a/app/assets/javascripts/admin/routes/admin_groups_route.js b/app/assets/javascripts/admin/routes/admin_groups_route.js deleted file mode 100644 index e66299a50..000000000 --- a/app/assets/javascripts/admin/routes/admin_groups_route.js +++ /dev/null @@ -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); - } - } -}); - diff --git a/app/assets/javascripts/admin/templates/admin.hbs b/app/assets/javascripts/admin/templates/admin.hbs index 209a6dc29..0958fe0d4 100644 --- a/app/assets/javascripts/admin/templates/admin.hbs +++ b/app/assets/javascripts/admin/templates/admin.hbs @@ -13,7 +13,7 @@
  • {{#link-to 'adminBadges.index'}}{{i18n 'admin.badges.title'}}{{/link-to}}
    • {{/if}} {{#if currentUser.admin}} -
  • {{#link-to 'adminGroups.index'}}{{i18n 'admin.groups.title'}}{{/link-to}}
    • +
  • {{#link-to 'adminGroups'}}{{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/groups.hbs b/app/assets/javascripts/admin/templates/groups.hbs index 061bf715d..751e46323 100644 --- a/app/assets/javascripts/admin/templates/groups.hbs +++ b/app/assets/javascripts/admin/templates/groups.hbs @@ -1,20 +1,11 @@ -
    -
    -

    {{i18n 'admin.groups.edit'}}

    -
      - {{#each group in arrangedContent}} -
    • - {{group.name}} {{group.userCountDisplay}} -
      • - {{/each}} +
      +
      +
        +
      • {{#link-to "adminGroupsType" "custom"}}{{i18n 'admin.groups.custom'}}{{/link-to}}
        • +
      • {{#link-to "adminGroupsType" "automatic"}}{{i18n 'admin.groups.automatic'}}{{/link-to}}
      -
      - - -
      -
      - -
      - {{outlet}}
      +
      + {{outlet}} +
      diff --git a/app/assets/javascripts/admin/templates/groups_index.hbs b/app/assets/javascripts/admin/templates/groups_index.hbs deleted file mode 100644 index f8a3f440e..000000000 --- a/app/assets/javascripts/admin/templates/groups_index.hbs +++ /dev/null @@ -1 +0,0 @@ -{{i18n 'admin.groups.about'}} diff --git a/app/assets/javascripts/admin/templates/groups_type.hbs b/app/assets/javascripts/admin/templates/groups_type.hbs new file mode 100644 index 000000000..0aec861a6 --- /dev/null +++ b/app/assets/javascripts/admin/templates/groups_type.hbs @@ -0,0 +1,20 @@ +
      +
      +

      {{i18n 'admin.groups.edit'}}

      +
        + {{#each group in controller}} +
      • + {{#link-to "adminGroup" group.type group.name}}{{group.name}} {{group.userCountDisplay}}{{/link-to}} +
        • + {{/each}} +
      +
      + {{d-button action="newGroup" icon="plus" label="admin.groups.new"}} + {{d-button action="refreshAutoGroups" icon="refresh" label="admin.groups.refresh" disabled=refreshingAutoGroups}} +
      +
      + +
      + {{outlet}} +
      +
      diff --git a/app/assets/javascripts/discourse/components/user-selector.js.es6 b/app/assets/javascripts/discourse/components/user-selector.js.es6 index 15d50faed..f008c599b 100644 --- a/app/assets/javascripts/discourse/components/user-selector.js.es6 +++ b/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.get('element')).val(this.get('usernames')).autocomplete({ - template: template, - + this.$().val(this.get('usernames')).autocomplete({ + template: this.container.lookup('template:user-selector-autocomplete.raw'), 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") }); diff --git a/app/assets/javascripts/discourse/models/group.js b/app/assets/javascripts/discourse/models/group.js index b3fc5798b..c492cce54 100644 --- a/app/assets/javascripts/discourse/models/group.js +++ b/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.Group.create(g.basic_group); + return Discourse.ajax("/groups/" + name + ".json").then(function (result) { + return Discourse.Group.create(result.basic_group); }); } }); diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index d77de3dc5..dca9a5403 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/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 diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index dd9e5f720..0cdf04136 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 95c269f0b..d24229b9c 100644 --- a/config/locales/client.en.yml +++ b/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" diff --git a/config/routes.rb b/config/routes.rb index 70d26b189..3c288b57d 100644 --- a/config/routes.rb +++ b/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" - put "members" => "groups#add_members" + member do + 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" diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 45be241b0..8c83768e0 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/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