-
+
+
+
+
+
+ {{#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"}}
+
+{{/d-modal-body}}
+
+
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":{
-