From 31acd311e59684e4399bbd54d295e2fd88a6fa52 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 29 Nov 2016 16:25:02 +0800 Subject: [PATCH] FEATURE: Allow group owners to edit group name and avatar flair. --- .../admin/controllers/admin-group.js.es6 | 39 ----------- .../javascripts/admin/templates/group.hbs | 58 +--------------- .../discourse/components/avatar-flair.js.es6 | 20 ++++++ .../components/group-flair-inputs.js.es6 | 50 ++++++++++++++ .../discourse/controllers/edit-group.js.es6 | 20 ++++++ .../discourse/controllers/group-index.js.es6 | 13 +--- .../discourse/controllers/group.js.es6 | 5 ++ .../javascripts/discourse/models/group.js.es6 | 10 ++- .../javascripts/discourse/routes/group.js.es6 | 8 +++ .../components/group-flair-inputs.hbs | 47 +++++++++++++ .../components/user-card-contents.hbs | 6 +- .../javascripts/discourse/templates/group.hbs | 30 +++++--- .../discourse/templates/modal/edit-group.hbs | 10 +++ .../stylesheets/common/admin/admin_base.scss | 32 --------- .../stylesheets/common/base/groups.scss | 50 ++++++++++++-- app/controllers/admin/groups_controller.rb | 43 +++++++----- app/controllers/groups_controller.rb | 34 +++++++-- app/models/group.rb | 6 +- app/serializers/group_show_serializer.rb | 18 ++++- config/locales/client.en.yml | 25 ++++--- .../admin/groups_controller_spec.rb | 8 +-- spec/integration/groups_spec.rb | 69 ++++++++++++++++--- .../serializers/group_show_serializer_spec.rb | 31 +++++++++ .../javascripts/acceptance/groups-test.js.es6 | 8 ++- .../javascripts/controllers/group-test.js.es6 | 19 +++++ .../fixtures/group-fixtures.js.es6 | 3 +- 26 files changed, 453 insertions(+), 209 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/avatar-flair.js.es6 create mode 100644 app/assets/javascripts/discourse/components/group-flair-inputs.js.es6 create mode 100644 app/assets/javascripts/discourse/controllers/edit-group.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/components/group-flair-inputs.hbs create mode 100644 app/assets/javascripts/discourse/templates/modal/edit-group.hbs create mode 100644 spec/serializers/group_show_serializer_spec.rb create mode 100644 test/javascripts/controllers/group-test.js.es6 diff --git a/app/assets/javascripts/admin/controllers/admin-group.js.es6 b/app/assets/javascripts/admin/controllers/admin-group.js.es6 index e7d53800e7d..660fbafd3b6 100644 --- a/app/assets/javascripts/admin/controllers/admin-group.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-group.js.es6 @@ -1,7 +1,5 @@ import { popupAjaxError } from 'discourse/lib/ajax-error'; import { propertyEqual } from 'discourse/lib/computed'; -import { escapeExpression } from 'discourse/lib/utilities'; -import computed from 'ember-addons/ember-computed-decorators'; export default Ember.Controller.extend({ adminGroupsType: Ember.inject.controller(), @@ -37,43 +35,6 @@ export default Ember.Controller.extend({ ]; }.property(), - @computed - demoAvatarUrl() { - return Discourse.getURL('/images/avatar.png'); - }, - - @computed('model.flair_url') - flairPreviewIcon() { - return this.get('model.flair_url') && this.get('model.flair_url').substr(0,3) === 'fa-'; - }, - - @computed('flairPreviewIcon') - flairPreviewImage() { - return this.get('model.flair_url') && !this.get('flairPreviewIcon'); - }, - - @computed('flairPreviewImage', 'model.flair_url', 'model.flairBackgroundHexColor', 'model.flairHexColor') - flairPreviewStyle() { - var style = ''; - if (this.get('flairPreviewImage')) { - style += 'background-image: url(' + escapeExpression(this.get('model.flair_url')) + '); '; - } - if (this.get('model.flairBackgroundHexColor')) { - style += 'background-color: #' + this.get('model.flairBackgroundHexColor') + ';'; - } - if (this.get('model.flairHexColor')) { - style += 'color: #' + this.get('model.flairHexColor') + ';'; - } - return style; - }, - - @computed('model.flairBackgroundHexColor') - flairPreviewClasses() { - if (this.get('model.flairBackgroundHexColor')) { - return 'rounded'; - } - }, - actions: { next() { if (this.get("showingLast")) { return; } diff --git a/app/assets/javascripts/admin/templates/group.hbs b/app/assets/javascripts/admin/templates/group.hbs index 0d53645023e..b8852002aab 100644 --- a/app/assets/javascripts/admin/templates/group.hbs +++ b/app/assets/javascripts/admin/templates/group.hbs @@ -4,8 +4,8 @@ {{#if model.automatic}}

{{model.name}}

{{else}} - - {{text-field name="name" value=model.name placeholderKey="admin.groups.name_placeholder"}} + + {{text-field name="name" value=model.name placeholderKey="group.name_placeholder"}} {{/if}} @@ -101,59 +101,7 @@ {{/unless}} {{#unless model.automatic}} -
-
-
- - {{text-field name="flair_url" value=model.flair_url placeholderKey="admin.groups.flair_url_placeholder"}} -
- -
- - {{text-field name="flair_bg_color" class="flair-bg-color" value=model.flair_bg_color placeholderKey="admin.groups.flair_bg_color_placeholder"}} -
- - {{#if flairPreviewIcon}} -
- - {{text-field name="flair_color" class="flair-color" value=model.flair_color placeholderKey="admin.groups.flair_color_placeholder"}} -
- {{/if}} - -
-
- {{i18n 'admin.groups.flair_note'}} -
-
- - {{#if flairPreviewIcon}} -
- -
-
- -
-
- -
-
-
- {{/if}} - - {{#if flairPreviewImage}} -
- -
-
- -
-
-
-
- {{/if}} - -
-
+ {{group-flair-inputs model=model}} {{/unless}}
diff --git a/app/assets/javascripts/discourse/components/avatar-flair.js.es6 b/app/assets/javascripts/discourse/components/avatar-flair.js.es6 new file mode 100644 index 00000000000..4beaa9cdf5f --- /dev/null +++ b/app/assets/javascripts/discourse/components/avatar-flair.js.es6 @@ -0,0 +1,20 @@ +import { observes } from 'ember-addons/ember-computed-decorators'; +import MountWidget from 'discourse/components/mount-widget'; + +export default MountWidget.extend({ + widget: 'avatar-flair', + + @observes('flairURL', 'flairBgColor', 'flairColor') + _rerender() { + this.queueRerender(); + }, + + buildArgs() { + return { + primary_group_flair_url: this.get('flairURL'), + primary_group_flair_bg_color: this.get('flairBgColor'), + primary_group_flair_color: this.get('flairColor'), + primary_group_name: this.get('groupName') + }; + } +}); diff --git a/app/assets/javascripts/discourse/components/group-flair-inputs.js.es6 b/app/assets/javascripts/discourse/components/group-flair-inputs.js.es6 new file mode 100644 index 00000000000..f14d45da630 --- /dev/null +++ b/app/assets/javascripts/discourse/components/group-flair-inputs.js.es6 @@ -0,0 +1,50 @@ +import computed from 'ember-addons/ember-computed-decorators'; +import { escapeExpression } from 'discourse/lib/utilities'; + +export default Ember.Component.extend({ + + classNames: ['group-flair-inputs'], + + @computed + demoAvatarUrl() { + return Discourse.getURL('/images/avatar.png'); + }, + + @computed('model.flair_url') + flairPreviewIcon(flairURL) { + return flairURL && flairURL.substr(0,3) === 'fa-'; + }, + + @computed('model.flair_url', 'flairPreviewIcon') + flairPreviewImage(flairURL, flairPreviewIcon) { + return flairURL && !flairPreviewIcon; + }, + + @computed('model.flair_url', 'flairPreviewImage', 'model.flairBackgroundHexColor', 'model.flairHexColor') + flairPreviewStyle(flairURL, flairPreviewImage, flairBackgroundHexColor, flairHexColor) { + let style = ''; + + if (flairPreviewImage) { + style += `background-image: url(${escapeExpression(flairURL)});`; + } + + if (flairBackgroundHexColor) { + style += `background-color: #${flairBackgroundHexColor};`; + } + + if (flairHexColor) style += `color: #${flairHexColor};`; + + return style; + }, + + @computed('model.flairBackgroundHexColor') + flairPreviewClasses(flairBackgroundHexColor) { + if (flairBackgroundHexColor) return 'rounded'; + }, + + @computed('flairPreviewImage') + flairPreviewLabel(flairPreviewImage) { + const key = flairPreviewImage ? 'image' : 'icon'; + return I18n.t(`group.flair_preview_${key}`); + } +}); diff --git a/app/assets/javascripts/discourse/controllers/edit-group.js.es6 b/app/assets/javascripts/discourse/controllers/edit-group.js.es6 new file mode 100644 index 00000000000..01db8c7ad47 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/edit-group.js.es6 @@ -0,0 +1,20 @@ +import { popupAjaxError } from 'discourse/lib/ajax-error'; + +export default Ember.Controller.extend({ + saving: false, + + actions: { + save() { + this.set('saving', true); + + this.get('model').save().then(() => { + this.transitionToRoute('group', this.get('model.name')); + this.send('closeModal'); + }).catch(error => { + popupAjaxError(error); + }).finally(() => { + this.set('saving', false); + }); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/group-index.js.es6 b/app/assets/javascripts/discourse/controllers/group-index.js.es6 index 306ff8aa80b..162ac3e8984 100644 --- a/app/assets/javascripts/discourse/controllers/group-index.js.es6 +++ b/app/assets/javascripts/discourse/controllers/group-index.js.es6 @@ -1,22 +1,11 @@ import { popupAjaxError } from 'discourse/lib/ajax-error'; -import computed from 'ember-addons/ember-computed-decorators'; import Group from 'discourse/models/group'; export default Ember.Controller.extend({ loading: false, limit: null, offset: null, - - @computed('model.owners.[]') - isOwner(owners) { - if (this.get('currentUser.admin')) { - return true; - } - const currentUserId = this.get('currentUser.id'); - if (currentUserId) { - return !!owners.findBy('id', currentUserId); - } - }, + isOwner: Ember.computed.alias('model.is_group_owner'), actions: { removeMember(user) { diff --git a/app/assets/javascripts/discourse/controllers/group.js.es6 b/app/assets/javascripts/discourse/controllers/group.js.es6 index b7382ed98c4..f46a3bfe55a 100644 --- a/app/assets/javascripts/discourse/controllers/group.js.es6 +++ b/app/assets/javascripts/discourse/controllers/group.js.es6 @@ -23,6 +23,11 @@ export default Ember.Controller.extend({ Tab.create({ name: 'messages', requiresMembership: true }) ], + @computed('model.is_group_owner', 'model.automatic') + canEditGroup(isGroupOwner, automatic) { + return !automatic && isGroupOwner; + }, + @computed('model.name') groupName(name) { return name.capitalize(); diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6 index 299cda4b686..68550d2c2b5 100644 --- a/app/assets/javascripts/discourse/models/group.js.es6 +++ b/app/assets/javascripts/discourse/models/group.js.es6 @@ -119,13 +119,19 @@ const Group = Discourse.Model.extend({ create() { var self = this; - return ajax("/admin/groups", { type: "POST", data: this.asJSON() }).then(function(resp) { + return ajax("/admin/groups", { type: "POST", data: { group: this.asJSON() } }).then(function(resp) { self.set('id', resp.basic_group.id); }); }, save() { - return ajax("/admin/groups/" + this.get('id'), { type: "PUT", data: this.asJSON() }); + const id = this.get('id'); + const url = this.get('is_group_owner') ? `/groups/${id}` : `/admin/groups/${id}`; + + return ajax(url, { + type: "PUT", + data: { group: this.asJSON() } + }); }, destroy() { diff --git a/app/assets/javascripts/discourse/routes/group.js.es6 b/app/assets/javascripts/discourse/routes/group.js.es6 index 3df20f794bf..51731bccb68 100644 --- a/app/assets/javascripts/discourse/routes/group.js.es6 +++ b/app/assets/javascripts/discourse/routes/group.js.es6 @@ -1,4 +1,5 @@ import Group from 'discourse/models/group'; +import showModal from 'discourse/lib/show-modal'; export default Discourse.Route.extend({ @@ -16,5 +17,12 @@ export default Discourse.Route.extend({ setupController(controller, model) { controller.setProperties({ model, counts: this.get('counts') }); + }, + + actions: { + showGroupEditor() { + showModal('edit-group'); + this.controllerFor('edit-group').set('model', this.modelFor('group')); + } } }); diff --git a/app/assets/javascripts/discourse/templates/components/group-flair-inputs.hbs b/app/assets/javascripts/discourse/templates/components/group-flair-inputs.hbs new file mode 100644 index 00000000000..1e75c9655f7 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/group-flair-inputs.hbs @@ -0,0 +1,47 @@ +
+
+ + {{text-field name="flair_url" + value=model.flair_url + placeholderKey="group.flair_url_placeholder"}} +
+ +
+ + {{text-field name="flair_bg_color" + class="group-flair-bg-color" + value=model.flair_bg_color + placeholderKey="group.flair_bg_color_placeholder"}} +
+ + {{#if flairPreviewIcon}} +
+ + {{text-field name="flair_color" + class="group-flair-color" + value=model.flair_color + placeholderKey="group.flair_color_placeholder"}} +
+ {{/if}} + +
+ {{i18n 'group.flair_note'}} +
+
+ +
+ +
+
+ +
+ + {{#if flairPreviewImage}} +
+ {{else}} +
+ +
+ {{/if}} +
+
diff --git a/app/assets/javascripts/discourse/templates/components/user-card-contents.hbs b/app/assets/javascripts/discourse/templates/components/user-card-contents.hbs index 641351cc00d..c88b7cc9d60 100644 --- a/app/assets/javascripts/discourse/templates/components/user-card-contents.hbs +++ b/app/assets/javascripts/discourse/templates/components/user-card-contents.hbs @@ -4,7 +4,11 @@
{{bound-avatar avatar "huge"}} {{#if user.primary_group_name}} - {{mount-widget widget="avatar-flair" args=user}} + {{avatar-flair + flairURL=user.primary_group_flair_url + flairBgColor=user.primary_group_flair_bg_color + flairColor=user.primary_group_flair_color + groupName=user.primary_group_name}} {{/if}}
diff --git a/app/assets/javascripts/discourse/templates/group.hbs b/app/assets/javascripts/discourse/templates/group.hbs index f3f96d4efe4..c7be3e1fef2 100644 --- a/app/assets/javascripts/discourse/templates/group.hbs +++ b/app/assets/javascripts/discourse/templates/group.hbs @@ -16,15 +16,27 @@
-
-

- {{#if model.flair_url}} - - {{mount-widget widget="avatar-flair" args=avatarFlairAttributes}} - - {{/if}} - {{groupName}} -

+
+ +

+ {{#if model.flair_url}} + + {{avatar-flair + flairURL=model.flair_url + flairBgColor=model.flair_bg_color + flairColor=model.flair_color + groupName=model.name}} + + {{/if}} + {{groupName}} +

+ + + {{#if canEditGroup}} + + {{d-button action="showGroupEditor" class="group-edit-btn btn-small" icon="pencil"}} + + {{/if}}
{{outlet}} diff --git a/app/assets/javascripts/discourse/templates/modal/edit-group.hbs b/app/assets/javascripts/discourse/templates/modal/edit-group.hbs new file mode 100644 index 00000000000..f467e5e94e2 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/edit-group.hbs @@ -0,0 +1,10 @@ +{{#d-modal-body title="group.edit.title" class="edit-group groups"}} +
+ {{group-flair-inputs model=model}} +
+{{/d-modal-body}} + +
+ {{d-button action="save" class="btn-primary" disabled=saving label="save"}} + {{i18n 'cancel'}} +
diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index 4a93fedb9a7..13893d7cc23 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -696,38 +696,6 @@ section.details { width: 100%; border-color: dark-light-choose(scale-color($primary, $lightness: 75%), scale-color($secondary, $lightness: 25%)); } - .avatar-flair-preview { - position: relative; - width: 45px; - - .avatar-wrapper { - background-color: #f4f4f4; - } - } - .form-horizontal { - .flair-inputs { - margin-top: 30px; - margin-bottom: 30px; - - .flair-left { - float: left; - width: 60%; - input[name=flair_url] { - width: 90%; - } - } - - .flair-right { - float: left; - margin-left: 30px; - } - } - } -} -.row.groups { - input[type='text'].flair-bg-color, input[type='text'].flair-color { - width: 200px; - } } // Customise area diff --git a/app/assets/stylesheets/common/base/groups.scss b/app/assets/stylesheets/common/base/groups.scss index 9146b4bc074..82b79388ac5 100644 --- a/app/assets/stylesheets/common/base/groups.scss +++ b/app/assets/stylesheets/common/base/groups.scss @@ -1,9 +1,12 @@ .groups { - .group-header { + .group-header, .group-details { display: table; - } - .group-avatar-flair { + span { + display: table-cell; + vertical-align: middle; + } + .avatar-flair { $size: 40px; @@ -17,8 +20,43 @@ } } - .group-avatar-flair, .group-name { - display: table-cell; - vertical-align: middle; + .group-edit-btn { + margin-left: 5px; + } + + .form-horizontal { + label { + font-weight: bold; + } + + input[type="text"] { + width: 80% !important; + margin-bottom: 10px; + } + + .group-flair-inputs { + display: inline-block; + margin-top: 30px; + margin-bottom: 30px; + + .group-flair-left { + float: left; + } + + .group-flair-right { + float: left; + margin-left: 30px; + } + } + + .avatar-flair-preview { + position: relative; + width: 45px; + + .avatar-wrapper { + background-color: #f4f4f4; + } + } } } + diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index daca3101247..e363aff74a9 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -36,7 +36,7 @@ class Admin::GroupsController < Admin::AdminController def create group = Group.new - group.name = (params[:name] || '').strip + group.name = (group_params[:name] || '').strip save_group(group) end @@ -44,29 +44,29 @@ class Admin::GroupsController < Admin::AdminController group = Group.find(params[:id]) # group rename is ignored for automatic groups - group.name = params[:name] if params[:name] && !group.automatic + group.name = group_params[:name] if group_params[:name] && !group.automatic save_group(group) end def save_group(group) - group.alias_level = params[:alias_level].to_i if params[:alias_level].present? - group.visible = params[:visible] == "true" - grant_trust_level = params[:grant_trust_level].to_i + group.alias_level = group_params[:alias_level].to_i if group_params[:alias_level].present? + group.visible = group_params[:visible] == "true" + grant_trust_level = group_params[:grant_trust_level].to_i group.grant_trust_level = (grant_trust_level > 0 && grant_trust_level <= 4) ? grant_trust_level : nil - group.automatic_membership_email_domains = params[:automatic_membership_email_domains] unless group.automatic - group.automatic_membership_retroactive = params[:automatic_membership_retroactive] == "true" unless group.automatic + group.automatic_membership_email_domains = group_params[:automatic_membership_email_domains] unless group.automatic + group.automatic_membership_retroactive = group_params[:automatic_membership_retroactive] == "true" unless group.automatic - group.primary_group = group.automatic ? false : params["primary_group"] == "true" + group.primary_group = group.automatic ? false : group_params["primary_group"] == "true" - group.incoming_email = group.automatic ? nil : params[:incoming_email] + group.incoming_email = group.automatic ? nil : group_params[:incoming_email] - title = params[:title] if params[:title].present? + title = group_params[:title] if group_params[:title].present? group.title = group.automatic ? nil : title - group.flair_url = params[:flair_url].presence - group.flair_bg_color = params[:flair_bg_color].presence - group.flair_color = params[:flair_color].presence + group.flair_url = group_params[:flair_url].presence + group.flair_bg_color = group_params[:flair_bg_color].presence + group.flair_color = group_params[:flair_color].presence if group.save Group.reset_counters(group.id, :group_users) @@ -124,7 +124,18 @@ class Admin::GroupsController < Admin::AdminController protected - def can_not_modify_automatic - render json: {errors: I18n.t('groups.errors.can_not_modify_automatic')}, status: 422 - end + def can_not_modify_automatic + render json: {errors: I18n.t('groups.errors.can_not_modify_automatic')}, status: 422 + end + + private + + def group_params + params.require(:group).permit( + :name, :alias_level, :visible, :automatic_membership_email_domains, + :automatic_membership_retroactive, :title, :primary_group, + :grant_trust_level, :incoming_email, :flair_url, :flair_bg_color, + :flair_color + ) + end end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 92dec3cce67..918a113848a 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -1,12 +1,28 @@ class GroupsController < ApplicationController - before_filter :ensure_logged_in, only: [:set_notifications, :mentionable] + before_filter :ensure_logged_in, only: [ + :set_notifications, + :mentionable, + :update + ] + skip_before_filter :preload_json, :check_xhr, only: [:posts_feed, :mentions_feed] def show render_serialized(find_group(:id), GroupShowSerializer, root: 'basic_group') end + def update + group = Group.find(params[:id]) + guardian.ensure_can_edit!(group) + + if group.update_attributes(group_params) + render json: success_json + else + render_json_error(group) + end + end + def posts group = find_group(:group_id) posts = group.posts_for(guardian, params[:before_post_id]).limit(20) @@ -152,11 +168,15 @@ class GroupsController < ApplicationController private - def find_group(param_name) - name = params.require(param_name) - group = Group.find_by("lower(name) = ?", name.downcase) - guardian.ensure_can_see!(group) - group - end + def group_params + params.require(:group).permit(:flair_url, :flair_bg_color, :flair_color) + end + + def find_group(param_name) + name = params.require(param_name) + group = Group.find_by("lower(name) = ?", name.downcase) + guardian.ensure_can_see!(group) + group + end end diff --git a/app/models/group.rb b/app/models/group.rb index 9d488ac7622..c0c487f4d23 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -349,7 +349,11 @@ class Group < ActiveRecord::Base end def add_owner(user) - self.group_users.create(user_id: user.id, owner: true) + if group_user = self.group_users.find_by(user: user) + group_user.update_attributes!(owner: true) if !group_user.owner + else + GroupUser.create!(user: user, group: self, owner: true) + end end def self.find_by_email(email) diff --git a/app/serializers/group_show_serializer.rb b/app/serializers/group_show_serializer.rb index 58358c1bac1..dc9e0c7649a 100644 --- a/app/serializers/group_show_serializer.rb +++ b/app/serializers/group_show_serializer.rb @@ -1,11 +1,25 @@ class GroupShowSerializer < BasicGroupSerializer - attributes :is_group_user + attributes :is_group_user, :is_group_owner def include_is_group_user? scope.authenticated? end def is_group_user - object.users.include?(scope.user) + !!fetch_group_user + end + + def include_is_group_owner? + scope.authenticated? + end + + def is_group_owner + scope.is_admin? || fetch_group_user&.owner + end + + private + + def fetch_group_user + @group_user ||= object.group_users.find_by(user: scope.user) end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e65c6f928cc..486748c2154 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1845,6 +1845,21 @@ en: title: "Show the raw source diffs side-by-side" button: ' Raw' + group: + edit: + title: 'Edit Group' + name: "Name" + name_placeholder: "Group name, no spaces, same as username rule" + flair_url: "Avatar Flair Image" + flair_url_placeholder: "(Optional) Image URL or Font Awesome class" + flair_bg_color: "Avatar Flair Background Color" + flair_bg_color_placeholder: "(Optional) Hex color value" + flair_color: "Avatar Flair Color" + flair_color_placeholder: "(Optional) Hex color value" + flair_preview_icon: "Preview Icon" + flair_preview_image: "Preview Image" + flair_note: "Note: Flair will only show for a user's primary group." + category: can: 'can… ' none: '(no category)' @@ -2451,7 +2466,6 @@ en: refresh: "Refresh" new: "New" selector_placeholder: "enter username" - name_placeholder: "Group name, no spaces, same as username rule" about: "Edit your group membership and names here" group_members: "Group members" delete: "Delete" @@ -2459,7 +2473,6 @@ en: delete_failed: "Unable to delete group. If this is an automatic group, it cannot be destroyed." delete_member_confirm: "Remove '%{username}' from the '%{group}' group?" delete_owner_confirm: "Remove owner privilege for '%{username}'?" - name: "Name" add: "Add" add_members: "Add members" custom: "Custom" @@ -2476,14 +2489,6 @@ en: add_owners: Add owners incoming_email: "Custom incoming email address" incoming_email_placeholder: "enter email address" - flair_url: "Avatar Flair Image" - flair_url_placeholder: "(Optional) Image URL or Font Awesome class" - flair_bg_color: "Avatar Flair Background Color" - flair_bg_color_placeholder: "(Optional) Hex color value" - flair_color: "Avatar Flair Color" - flair_color_placeholder: "(Optional) Hex color value" - flair_preview: "Preview" - flair_note: "Note: Flair will only show for a user's primary group." api: generate_master: "Generate Master API Key" diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 6971184dd4a..39fc436e3a8 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -66,7 +66,7 @@ describe Admin::GroupsController do context ".create" do it "strip spaces on the group name" do - xhr :post, :create, name: " bob " + xhr :post, :create, { group: { name: " bob " } } expect(response.status).to eq(200) @@ -81,7 +81,7 @@ describe Admin::GroupsController do context ".update" do it "ignore name change on automatic group" do - xhr :put, :update, id: 1, name: "WAT", visible: "true" + xhr :put, :update, { id: 1, group: { name: "WAT", visible: "true" } } expect(response).to be_success group = Group.find(1) @@ -92,14 +92,14 @@ describe Admin::GroupsController do it "doesn't launch the 'automatic group membership' job when it's not retroactive" do Jobs.expects(:enqueue).never group = Fabricate(:group) - xhr :put, :update, id: group.id, automatic_membership_retroactive: "false" + xhr :put, :update, { id: group.id, group: { automatic_membership_retroactive: "false" } } expect(response).to be_success end it "launches the 'automatic group membership' job when it's retroactive" do group = Fabricate(:group) Jobs.expects(:enqueue).with(:automatic_group_membership, group_id: group.id) - xhr :put, :update, id: group.id, automatic_membership_retroactive: "true" + xhr :put, :update, { id: group.id, group: { automatic_membership_retroactive: "true" } } expect(response).to be_success end diff --git a/spec/integration/groups_spec.rb b/spec/integration/groups_spec.rb index 396d7df9f41..e7f4e9eb7d8 100644 --- a/spec/integration/groups_spec.rb +++ b/spec/integration/groups_spec.rb @@ -1,22 +1,22 @@ require 'rails_helper' describe "Groups" do - describe "checking if a group can be mentioned" do - let(:password) { 'somecomplicatedpassword' } - let(:email_token) { Fabricate(:email_token, confirmed: true) } - let(:user) { email_token.user } - let(:group) { Fabricate(:group, name: 'test', users: [user]) } + let(:password) { 'somecomplicatedpassword' } + let(:email_token) { Fabricate(:email_token, confirmed: true) } + let(:user) { email_token.user } - before do - user.update_attributes!(password: password) - end + before do + user.update_attributes!(password: password) + post "/session.json", { login: user.username, password: password } + expect(response).to be_success + end + + describe "checking if a group can be mentioned" do + let(:group) { Fabricate(:group, name: 'test', users: [user]) } it "should return the right response" do group - post "/session.json", { login: user.username, password: password } - expect(response).to be_success - get "/groups/test/mentionable.json", { name: group.name } expect(response).to be_success @@ -33,4 +33,51 @@ describe "Groups" do expect(response_body["mentionable"]).to eq(true) end end + + describe "group can be updated" do + let(:group) { Fabricate(:group, name: 'test', users: [user]) } + + context "when user is group owner" do + before do + group.add_owner(user) + end + + it "should be able update the group" do + xhr :put, "/groups/#{group.id}", { group: { + flair_bg_color: 'FFF', + flair_color: 'BBB', + flair_url: 'fa-adjust' + } } + + expect(response).to be_success + + group.reload + + expect(group.flair_bg_color).to eq('FFF') + expect(group.flair_color).to eq('BBB') + expect(group.flair_url).to eq('fa-adjust') + end + end + + context "when user is group admin" do + before do + user.update_attributes!(admin: true) + end + + it 'should be able to update the group' do + xhr :put, "/groups/#{group.id}", { group: { flair_color: 'BBB' } } + + expect(response).to be_success + expect(group.reload.flair_color).to eq('BBB') + end + end + + context "when user is not a group owner or admin" do + it 'should not be able to update the group' do + xhr :put, "/groups/#{group.id}", { group: { name: 'testing' } } + + expect(response.status).to eq(403) + end + end + end end diff --git a/spec/serializers/group_show_serializer_spec.rb b/spec/serializers/group_show_serializer_spec.rb new file mode 100644 index 00000000000..5641ccd8d09 --- /dev/null +++ b/spec/serializers/group_show_serializer_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +describe GroupShowSerializer do + context 'admin user' do + let(:user) { Fabricate(:admin) } + let(:group) { Fabricate(:group, users: [user]) } + + it 'should return the right attributes' do + json = GroupShowSerializer.new(group, scope: Guardian.new(user)).as_json + + expect(json[:group_show][:is_group_owner]).to eq(true) + expect(json[:group_show][:is_group_user]).to eq(true) + end + end + + context 'group owner' do + let(:user) { Fabricate(:user) } + let(:group) { Fabricate(:group) } + + before do + group.add_owner(user) + end + + it 'should return the right attributes' do + json = GroupShowSerializer.new(group, scope: Guardian.new(user)).as_json + + expect(json[:group_show][:is_group_owner]).to eq(true) + expect(json[:group_show][:is_group_user]).to eq(true) + end + end +end diff --git a/test/javascripts/acceptance/groups-test.js.es6 b/test/javascripts/acceptance/groups-test.js.es6 index 3b01016a94b..53c1b157453 100644 --- a/test/javascripts/acceptance/groups-test.js.es6 +++ b/test/javascripts/acceptance/groups-test.js.es6 @@ -32,7 +32,7 @@ test("Browsing Groups", () => { }); }); -test("Messages tab", () => { +test("Admin Browsing Groups", () => { logIn(); Discourse.reset(); @@ -41,4 +41,10 @@ test("Messages tab", () => { andThen(() => { ok($('.action-list li').length === 5, 'it should show messages tab if user is admin'); }); + + click('.group-edit-btn'); + + andThen(() => { + ok(find('.group-flair-inputs').length === 1, 'it should display avatar flair inputs'); + }); }); diff --git a/test/javascripts/controllers/group-test.js.es6 b/test/javascripts/controllers/group-test.js.es6 new file mode 100644 index 00000000000..822b14dd424 --- /dev/null +++ b/test/javascripts/controllers/group-test.js.es6 @@ -0,0 +1,19 @@ +moduleFor("controller:group"); + +test("canEditGroup", function() { + const GroupController = this.subject(); + + GroupController.setProperties({ + model: { is_group_owner: true, automatic: true } + }); + + equal(GroupController.get("canEditGroup"), false, "automatic groups cannot be edited"); + + GroupController.set("model.automatic", false); + + equal(GroupController.get("canEditGroup"), true, "owners can edit groups"); + + GroupController.set("model.is_group_owner", false); + + equal(GroupController.get("canEditGroup"), false, "normal users cannot edit groups"); +}); diff --git a/test/javascripts/fixtures/group-fixtures.js.es6 b/test/javascripts/fixtures/group-fixtures.js.es6 index d6ea20a239c..38dd8d69d8d 100644 --- a/test/javascripts/fixtures/group-fixtures.js.es6 +++ b/test/javascripts/fixtures/group-fixtures.js.es6 @@ -7,7 +7,8 @@ export default { "user_count":8, "alias_level":0, "visible":true, - "flair_url": 'fa-adjust' + "flair_url": 'fa-adjust', + "is_group_owner":true } }, "/groups/discourse/counts.json":{