From 8e45322b093d4cfc5cadff47a3b57c1b92ab0fd4 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 14 Mar 2017 11:44:52 +0800 Subject: [PATCH] FIX: Only group admins can see group edit page. --- .../discourse/controllers/group.js.es6 | 17 ++++++------ .../javascripts/discourse/models/group.js.es6 | 10 ++++--- .../javascripts/discourse/models/user.js.es6 | 5 +++- .../discourse/routes/group-edit.js.es6 | 8 ++++++ .../acceptance/group-edit-test.js.es6 | 17 +++++++++--- test/javascripts/models/user-test.js.es6 | 27 ++++++++++++++++--- 6 files changed, 63 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/group.js.es6 b/app/assets/javascripts/discourse/controllers/group.js.es6 index 126f8911dc8..c32a5bf2acd 100644 --- a/app/assets/javascripts/discourse/controllers/group.js.es6 +++ b/app/assets/javascripts/discourse/controllers/group.js.es6 @@ -1,4 +1,5 @@ import { default as computed, observes } from 'ember-addons/ember-computed-decorators'; +import Group from 'discourse/models/group'; var Tab = Em.Object.extend({ @computed('name') @@ -53,18 +54,18 @@ export default Ember.Controller.extend({ this.get('tabs')[0].set('count', this.get('model.user_count')); }, - @computed('model.is_group_user', 'model.is_group_owner', 'model.automatic') - getTabs(isGroupUser, isGroupOwner, automatic) { + @computed('model.is_group_owner', 'model.automatic') + getTabs(isGroupOwner, automatic) { return this.get('tabs').filter(t => { - let display = true; + let canSee = true; - if (this.currentUser && t.get('requiresGroupAdmin')) { - display = automatic ? false : (this.currentUser.admin || isGroupOwner); - } else if (t.get('requiresGroupAdmin')) { - display = false; + if (this.currentUser && t.requiresGroupAdmin) { + canSee = this.currentUser.canManageGroup(this.get('model')); + } else if (t.requiresGroupAdmin) { + canSee = false; } - return display; + return canSee; }); } }); diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6 index 99c95caf61a..7d1ab55e706 100644 --- a/app/assets/javascripts/discourse/models/group.js.es6 +++ b/app/assets/javascripts/discourse/models/group.js.es6 @@ -17,9 +17,10 @@ const Group = RestModel.extend({ return Em.isEmpty(value) ? "" : value; }, - type: function() { - return this.get("automatic") ? "automatic" : "custom"; - }.property("automatic"), + @computed('automatic') + type(automatic) { + return automatic ? "automatic" : "custom"; + }, @computed('user_count') userCountDisplay(userCount) { @@ -93,6 +94,7 @@ const Group = RestModel.extend({ }); }, + @computed('flair_bg_color') flairBackgroundHexColor() { return this.get('flair_bg_color') ? this.get('flair_bg_color').replace(new RegExp("[^0-9a-fA-F]", "g"), "") : null; @@ -224,7 +226,7 @@ Group.reopenClass({ mentionable(name) { return ajax(`/groups/${name}/mentionable`, { data: { name } }); - }, + } }); export default Group; diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index d4e24f5af83..5086c54cbc5 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -500,8 +500,11 @@ const User = RestModel.extend({ return summary; }); - } + }, + canManageGroup(group) { + return group.get('automatic') ? false : (this.get('admin') || group.get('is_group_owner')); + } }); User.reopenClass(Singleton, { diff --git a/app/assets/javascripts/discourse/routes/group-edit.js.es6 b/app/assets/javascripts/discourse/routes/group-edit.js.es6 index 8608e6ecf3b..9aab78aafb3 100644 --- a/app/assets/javascripts/discourse/routes/group-edit.js.es6 +++ b/app/assets/javascripts/discourse/routes/group-edit.js.es6 @@ -1,3 +1,5 @@ +import Group from 'discourse/models/group'; + export default Ember.Route.extend({ titleToken() { return I18n.t('groups.edit.title'); @@ -7,6 +9,12 @@ export default Ember.Route.extend({ return this.modelFor('group'); }, + afterModel(group) { + if (!this.currentUser || !this.currentUser.canManageGroup(group)) { + this.transitionTo("group.members", group); + } + }, + setupController(controller, model) { this.controllerFor('group-edit').setProperties({ model }); this.controllerFor("group").set("showing", 'edit'); diff --git a/test/javascripts/acceptance/group-edit-test.js.es6 b/test/javascripts/acceptance/group-edit-test.js.es6 index e4498bf793a..6fd097e1de3 100644 --- a/test/javascripts/acceptance/group-edit-test.js.es6 +++ b/test/javascripts/acceptance/group-edit-test.js.es6 @@ -1,10 +1,11 @@ -import { acceptance } from "helpers/qunit-helpers"; +import { acceptance, logIn } from "helpers/qunit-helpers"; -acceptance("Editing Group", { - loggedIn: true -}); +acceptance("Editing Group"); test("Editing group", () => { + logIn(); + Discourse.reset(); + visit("/groups/discourse/edit"); andThen(() => { @@ -29,3 +30,11 @@ test("Editing group", () => { ok(find('.group-edit-public[disabled]').length === 1, 'it should disable group public input'); }); }); + +test("Editing group as an anonymous user", () => { + visit("/groups/discourse/edit"); + + andThen(() => { + ok(count('.group-members tr') > 0, "it should redirect to members page for an anonymous user"); + }); +}) diff --git a/test/javascripts/models/user-test.js.es6 b/test/javascripts/models/user-test.js.es6 index 0414ef44e36..ea845d582f2 100644 --- a/test/javascripts/models/user-test.js.es6 +++ b/test/javascripts/models/user-test.js.es6 @@ -1,7 +1,10 @@ -module("Discourse.User"); +import User from 'discourse/models/user'; +import Group from 'discourse/models/group'; + +module("model:user"); test('staff', function(){ - var user = Discourse.User.create({id: 1, username: 'eviltrout'}); + var user = User.create({id: 1, username: 'eviltrout'}); ok(!user.get('staff'), "user is not staff"); @@ -13,15 +16,31 @@ test('staff', function(){ }); test('searchContext', function() { - var user = Discourse.User.create({id: 1, username: 'EvilTrout'}); + var user = User.create({id: 1, username: 'EvilTrout'}); deepEqual(user.get('searchContext'), {type: 'user', id: 'eviltrout', user: user}, "has a search context"); }); test("isAllowedToUploadAFile", function() { - var user = Discourse.User.create({ trust_level: 0, admin: true }); + var user = User.create({ trust_level: 0, admin: true }); ok(user.isAllowedToUploadAFile("image"), "admin can always upload a file"); user.setProperties({ admin: false, moderator: true }); ok(user.isAllowedToUploadAFile("image"), "moderator can always upload a file"); }); + +test('canMangeGroup', function() { + let user = User.create({ admin: true }); + let group = Group.create({ automatic: true }); + + equal(user.canManageGroup(group), false, "automatic groups cannot be managed."); + + group.set("automatic", false); + + equal(user.canManageGroup(group), true, "an admin should be able to manage the group"); + + user.set('admin', false); + group.setProperties({ is_group_owner: true }); + + equal(user.canManageGroup(group), true, "a group owner should be able to manage the group"); +});