Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: Add group settngs to allow users to leave a group freely. 

https://meta.discourse.org/t/split-join-leave-freely-setting-on-groups/65565
This commit is contained in:
Guo Xiang Tan 2017-07-28 11:37:10 +09:00
parent 5012d46cbd
commit 4620dfe92d
25 changed files with 315 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/assets/javascripts/admin/controllers/admin-group.js.es6
View File

@ -31,10 +31,10 @@ export default Ember.Controller.extend({
];
}.property(),
@computed('model.visibility_level', 'model.public')
disableMembershipRequestSetting(visibility_level, publicGroup) {
@computed('model.visibility_level', 'model.public_admission')
disableMembershipRequestSetting(visibility_level, publicAdmission) {
visibility_level = parseInt(visibility_level);
return (visibility_level !== 0) || publicGroup;
return (visibility_level !== 0) || publicAdmission;
},
@computed('model.visibility_level', 'model.allow_membership_requests')

13
app/assets/javascripts/admin/templates/group.hbs
View File

@ -61,10 +61,19 @@
<div>
<label>
{{input type="checkbox"
checked=model.public
checked=model.public_admission
disabled=disablePublicSetting}}
{{i18n 'groups.public'}}
{{i18n 'groups.public_admission'}}
</label>
</div>
<div>
<label>
{{input type='checkbox'
checked=model.public_exit}}
{{i18n 'groups.public_exit'}}
</label>
</div>

11
app/assets/javascripts/discourse/components/group-membership-button.js.es6
View File

@ -5,9 +5,14 @@ import DiscourseURL from 'discourse/lib/url';
export default Ember.Component.extend({
loading: false,
@computed("model.public")
canJoinGroup(publicGroup) {
return publicGroup;
@computed("model.public_admission", "userIsGroupUser")
canJoinGroup(publicAdmission, userIsGroupUser) {
return publicAdmission && !userIsGroupUser;
},
@computed("model.public_exit", "userIsGroupUser")
canLeaveGroup(publicExit, userIsGroupUser) {
return publicExit && userIsGroupUser;
},
@computed("model.is_group_user", "model.id", "groupUserIds")

3
app/assets/javascripts/discourse/models/group.js.es6
View File

@ -144,7 +144,8 @@ const Group = RestModel.extend({
flair_bg_color: this.get('flairBackgroundHexColor'),
flair_color: this.get('flairHexColor'),
bio_raw: this.get('bio_raw'),
public: this.get('public'),
public_admission: this.get('public_admission'),
public_exit: this.get('public_exit'),
allow_membership_requests: this.get('allow_membership_requests'),
full_name: this.get('full_name'),
default_notification_level: this.get('default_notification_level')

24
app/assets/javascripts/discourse/templates/components/group-membership-button.hbs
View File

@ -1,17 +1,15 @@
{{#if canJoinGroup}}
{{#if userIsGroupUser}}
{{d-button action="leaveGroup"
class="btn-danger group-index-leave"
icon="user-times"
label="groups.leave"
disabled=updatingMembership}}
{{else}}
{{d-button action="joinGroup"
class="group-index-join"
icon="user-plus"
label="groups.join"
disabled=updatingMembership}}
{{/if}}
{{d-button action="joinGroup"
class="group-index-join"
icon="user-plus"
label="groups.join"
disabled=updatingMembership}}
{{else if canLeaveGroup}}
{{d-button action="leaveGroup"
class="btn-danger group-index-leave"
icon="user-times"
label="groups.leave"
disabled=updatingMembership}}
{{else if model.allow_membership_requests}}
{{#if userIsGroupUser}}
{{#if showMembershipStatus}}

19
app/assets/javascripts/discourse/templates/group-edit.hbs
View File

@ -21,11 +21,22 @@
<div class="control-group">
<label>
{{input type='checkbox'
checked=model.public
class="group-edit-public"
checked=model.public_admission
class="group-edit-public-admission"
disabled=model.allow_membership_requests}}
{{i18n 'groups.public'}}
{{i18n 'groups.public_admission'}}
</label>
</div>
<div class="control-group">
<label>
{{input type='checkbox'
checked=model.public_exit
class="group-edit-public-exit"
disabled=model.allow_membership_requests}}
{{i18n 'groups.public_exit'}}
</label>
</div>
@ -34,7 +45,7 @@
{{input type='checkbox'
checked=model.allow_membership_requests
class="group-edit-allow-membership-requests"
disabled=model.public}}
disabled=model.public_admission}}
{{i18n 'groups.allow_membership_requests'}}
</label>

33
app/controllers/admin/groups_controller.rb
View File

@ -67,7 +67,13 @@ class Admin::GroupsController < Admin::AdminController
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
group.public = group_params[:public] if group_params[:public]
%i{public_admission public_exit}.each do |key|
if group_params[key]
group.public_send("#{key}=", group_params[key])
end
end
group.bio_raw = group_params[:bio_raw] if group_params[:bio_raw]
group.full_name = group_params[:full_name] if group_params[:full_name]
@ -169,11 +175,26 @@ class Admin::GroupsController < Admin::AdminController
def group_params
params.require(:group).permit(
:name, :alias_level, :visibility_level, :automatic_membership_email_domains,
:automatic_membership_retroactive, :title, :primary_group,
:grant_trust_level, :incoming_email, :flair_url, :flair_bg_color,
:flair_color, :bio_raw, :public, :allow_membership_requests, :full_name,
:default_notification_level, :usernames, :owner_usernames
:name,
:alias_level,
:visibility_level,
:automatic_membership_email_domains,
:automatic_membership_retroactive,
:title,
:primary_group,
:grant_trust_level,
:incoming_email,
:flair_url,
:flair_bg_color,
:flair_color,
:bio_raw,
:public_admission,
:public_exit,
:allow_membership_requests,
:full_name,
:default_notification_level,
:usernames,
:owner_usernames
)
end
end

11
app/controllers/groups_controller.rb
View File

@ -146,7 +146,7 @@ class GroupsController < ApplicationController
def add_members
group = Group.find(params[:id])
group.public ? ensure_logged_in : guardian.ensure_can_edit!(group)
group.public_admission ? ensure_logged_in : guardian.ensure_can_edit!(group)
users =
if params[:usernames].present?
@ -163,7 +163,7 @@ class GroupsController < ApplicationController
raise Discourse::NotFound if users.blank?
if group.public
if group.public_admission
if !guardian.can_log_group_changes?(group) && current_user != users.first
raise Discourse::InvalidAccess
end
@ -201,7 +201,7 @@ class GroupsController < ApplicationController
def remove_member
group = Group.find(params[:id])
group.public ? ensure_logged_in : guardian.ensure_can_edit!(group)
group.public_exit ? ensure_logged_in : guardian.ensure_can_edit!(group)
user =
if params[:user_id].present?
@ -216,7 +216,7 @@ class GroupsController < ApplicationController
raise Discourse::NotFound unless user
if group.public
if group.public_exit
if !guardian.can_log_group_changes?(group) && current_user != user
raise Discourse::InvalidAccess
end
@ -322,7 +322,8 @@ class GroupsController < ApplicationController
:flair_color,
:bio_raw,
:full_name,
:public,
:public_admission,
:public_exit,
:allow_membership_requests
)
end

3
app/models/group.rb
View File

@ -619,11 +619,12 @@ end
# flair_color :string
# bio_raw :text
# bio_cooked :text
# public :boolean default(FALSE), not null
# public_admission :boolean default(FALSE), not null
# allow_membership_requests :boolean default(FALSE), not null
# full_name :string
# default_notification_level :integer default(3), not null
# visibility_level :integer default(0), not null
# public_exit :boolean default(FALSE), not null
#
# Indexes
#

15
app/models/watched_word.rb
View File

@ -51,3 +51,18 @@ class WatchedWord < ActiveRecord::Base
end
end
# == Schema Information
#
# Table name: watched_words
#
# id :integer not null, primary key
# word :string not null
# action :integer not null
# created_at :datetime
# updated_at :datetime
#
# Indexes
#
# index_watched_words_on_action_and_word (action,word) UNIQUE
#

3
app/serializers/basic_group_serializer.rb
View File

@ -18,7 +18,8 @@ class BasicGroupSerializer < ApplicationSerializer
:flair_color,
:bio_raw,
:bio_cooked,
:public,
:public_admission,
:public_exit,
:allow_membership_requests,
:full_name,
:default_notification_level

4
app/services/group_action_logger.rb
View File

@ -24,7 +24,7 @@ class GroupActionLogger
end
def log_add_user_to_group(target_user)
@group.public || can_edit?
(target_user == @acting_user && @group.public_admission) || can_edit?
GroupHistory.create!(default_params.merge(
action: GroupHistory.actions[:add_user_to_group],
@ -33,7 +33,7 @@ class GroupActionLogger
end
def log_remove_user_from_group(target_user)
@group.public || can_edit?
(target_user == @acting_user && @group.public_exit) || can_edit?
GroupHistory.create!(default_params.merge(
action: GroupHistory.actions[:remove_user_from_group],

3
config/locales/client.en.yml
View File

@ -409,7 +409,8 @@ en:
add_members: "Add Members"
delete_member_confirm: "Remove '%{username}' from the '%{group}' group?"
name_placeholder: "Group name, no spaces, same as username rule"
public: "Allow users to join/leave the group freely (Requires publicly visible group)"
public_admission: "Allow users to join the group freely (Requires publicly visible group)"
public_exit: "Allow users to leave the group freely"
empty:
posts: "There are no posts by members of this group."
members: "There are no members in this group."

4
db/fixtures/002_groups.rb
View File

@ -7,8 +7,8 @@ Group.where(name: 'everyone').update_all(visibility_level: Group.visibility_leve
ColumnDropper.drop(
table: 'groups',
after_migration: 'AddVisibleBackToGroups',
columns: %w[visible],
after_migration: 'RemovePublicFromGroups',
columns: %w[visible public],
on_drop: ->() {
STDERR.puts 'Removing superflous visible group column!'
}

17
db/migrate/20170728012754_split_public_in_groups.rb Normal file
View File

@ -0,0 +1,17 @@
class SplitPublicInGroups < ActiveRecord::Migration
def up
add_column :groups, :public_exit, :boolean, default: false, null: false
add_column :groups, :public_admission, :boolean, default: false, null: false
ActiveRecord::Base.exec_sql <<~SQL
UPDATE groups
SET public_exit = true, public_admission = true
WHERE public = true
SQL
end
def down
remove_column :groups, :public_exit
remove_column :groups, :public_admission
end
end

9
db/migrate/20170728054115_remove_public_from_groups.rb Normal file
View File

@ -0,0 +1,9 @@
class RemovePublicFromGroups < ActiveRecord::Migration
def up
# Defer dropping of the columns until the new application code has been deployed.
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

5
spec/fabricators/group_fabricator.rb
View File

@ -1,3 +1,8 @@
Fabricator(:group) do
name { sequence(:name) { |n| "my_group_#{n}" } }
end
Fabricator(:public_group, from: :group) do
public_admission true
public_exit true
end

42
spec/integration/groups_spec.rb
View File

@ -80,7 +80,14 @@ describe "Groups" do
end
describe "group can be updated" do
let(:group) { Fabricate(:group, name: 'test', users: [user], public: false) }
let(:group) do
Fabricate(:group,
name: 'test',
users: [user],
public_admission: false,
public_exit: false
)
end
before do
sign_in(user)
@ -102,10 +109,11 @@ describe "Groups" do
flair_url: 'fa-adjust',
bio_raw: 'testing',
full_name: 'awesome team',
public: true,
public_admission: true,
public_exit: true,
allow_membership_requests: true
}
end.to change { GroupHistory.count }.by(7)
end.to change { GroupHistory.count }.by(8)
expect(response).to be_success
@ -116,7 +124,8 @@ describe "Groups" do
expect(group.flair_url).to eq('fa-adjust')
expect(group.bio_raw).to eq('testing')
expect(group.full_name).to eq('awesome team')
expect(group.public).to eq(true)
expect(group.public_admission).to eq(true)
expect(group.public_exit).to eq(true)
expect(group.allow_membership_requests).to eq(true)
expect(GroupHistory.last.subject).to eq('allow_membership_requests')
end
@ -225,7 +234,10 @@ describe "Groups" do
context 'public group' do
it 'should be fobidden' do
group.update_attributes!(public: true)
group.update_attributes!(
public_admission: true,
public_exit: true
)
expect { xhr :put, "/groups/#{group.id}/members", usernames: "bob" }
.to raise_error(Discourse::NotLoggedIn)
@ -341,7 +353,10 @@ describe "Groups" do
let(:other_user) { Fabricate(:user) }
before do
group.update!(public: true)
group.update!(
public_admission: true,
public_exit: true
)
end
context 'admin' do
@ -424,11 +439,7 @@ describe "Groups" do
context 'public group' do
let(:other_user) { Fabricate(:user) }
let(:group) { Fabricate(:group, users: [other_user]) }
before do
group.update!(public: true)
end
let(:group) { Fabricate(:public_group, users: [other_user]) }
context "admin" do
it "removes by username" do
@ -484,7 +495,12 @@ describe "Groups" do
context 'public group' do
before do
group.add_owner(user)
group.update_attributes!(public: true)
group.update_attributes!(
public_admission: true,
public_exit: true
)
GroupActionLogger.new(user, group).log_change_group_settings
sign_in(user)
end
@ -497,7 +513,7 @@ describe "Groups" do
result = JSON.parse(response.body)["logs"].first
expect(result["action"]).to eq(GroupHistory.actions[1].to_s)
expect(result["subject"]).to eq('public')
expect(result["subject"]).to eq('public_exit')
expect(result["prev_value"]).to eq('f')
expect(result["new_value"]).to eq('t')
end

2
spec/models/group_spec.rb
View File

@ -531,7 +531,7 @@ describe Group do
let(:category) { Fabricate(:category) }
it "should publish the group's categories to the client" do
group.update!(public: true, categories: [category])
group.update!(public_admission: true, categories: [category])
message = MessageBus.track_publish { group.add(user) }.first

86
spec/services/group_action_logger_spec.rb
View File

@ -36,32 +36,90 @@ RSpec.describe GroupActionLogger do
end
describe '#log_add_user_to_group' do
it 'should create the right record' do
subject.log_add_user_to_group(user)
describe 'as a group owner' do
it 'should create the right record' do
subject.log_add_user_to_group(user)
group_history = GroupHistory.last
group_history = GroupHistory.last
expect(group_history.action).to eq(GroupHistory.actions[:add_user_to_group])
expect(group_history.acting_user).to eq(group_owner)
expect(group_history.target_user).to eq(user)
expect(group_history.action).to eq(GroupHistory.actions[:add_user_to_group])
expect(group_history.acting_user).to eq(group_owner)
expect(group_history.target_user).to eq(user)
end
end
context 'as a normal user' do
subject { described_class.new(user, group) }
describe 'user cannot freely exit group' do
it 'should not be allowed to log the action' do
expect { subject.log_add_user_to_group(user) }
.to raise_error(Discourse::InvalidParameters)
end
end
describe 'user can freely exit group' do
before do
group.update!(public_admission: true)
end
it 'should create the right record' do
subject.log_add_user_to_group(user)
group_history = GroupHistory.last
expect(group_history.action).to eq(GroupHistory.actions[:add_user_to_group])
expect(group_history.acting_user).to eq(user)
expect(group_history.target_user).to eq(user)
end
end
end
end
describe '#log_remove_user_from_group' do
it 'should create the right record' do
subject.log_remove_user_from_group(user)
describe 'as group owner' do
it 'should create the right record' do
subject.log_remove_user_from_group(user)
group_history = GroupHistory.last
group_history = GroupHistory.last
expect(group_history.action).to eq(GroupHistory.actions[:remove_user_from_group])
expect(group_history.acting_user).to eq(group_owner)
expect(group_history.target_user).to eq(user)
expect(group_history.action).to eq(GroupHistory.actions[:remove_user_from_group])
expect(group_history.acting_user).to eq(group_owner)
expect(group_history.target_user).to eq(user)
end
end
context 'as a normal user' do
subject { described_class.new(user, group) }
describe 'user cannot freely exit group' do
it 'should not be allowed to log the action' do
expect { subject.log_remove_user_from_group(user) }
.to raise_error(Discourse::InvalidParameters)
end
end
describe 'user can freely exit group' do
before do
group.update!(public_exit: true)
end
it 'should create the right record' do
subject.log_remove_user_from_group(user)
group_history = GroupHistory.last
expect(group_history.action).to eq(GroupHistory.actions[:remove_user_from_group])
expect(group_history.acting_user).to eq(user)
expect(group_history.target_user).to eq(user)
end
end
end
end
describe '#log_change_group_settings' do
it 'should create the right record' do
group.update_attributes!(public: true, created_at: Time.zone.now)
group.update_attributes!(public_admission: true, created_at: Time.zone.now)
expect { subject.log_change_group_settings }.to change { GroupHistory.count }.by(1)
@ -69,7 +127,7 @@ RSpec.describe GroupActionLogger do
expect(group_history.action).to eq(GroupHistory.actions[:change_group_setting])
expect(group_history.acting_user).to eq(group_owner)
expect(group_history.subject).to eq('public')
expect(group_history.subject).to eq('public_admission')
expect(group_history.prev_value).to eq('f')
expect(group_history.new_value).to eq('t')
end

26
test/javascripts/acceptance/group-edit-test.js.es6
View File

@ -12,7 +12,17 @@ QUnit.test("Editing group", assert => {
assert.ok(find('.group-flair-inputs').length === 1, 'it should display avatar flair inputs');
assert.ok(find('.group-edit-bio').length === 1, 'it should display group bio input');
assert.ok(find('.group-edit-full-name').length === 1, 'it should display group full name input');
assert.ok(find('.group-edit-public').length === 1, 'it should display group public input');
assert.ok(
find('.group-edit-public-admission').length === 1,
'it should display group public admission input'
);
assert.ok(
find('.group-edit-public-exit').length === 1,
'it should display group public exit input'
);
assert.ok(find('.group-edit-allow-membership-requests').length === 1, 'it should display group allow_membership_requets input');
assert.ok(find('.group-members-input .item').length === 7, 'it should display group members');
assert.ok(find('.group-members-input-selector').length === 1, 'it should display input to add group members');
@ -20,14 +30,20 @@ QUnit.test("Editing group", assert => {
});
andThen(() => {
assert.ok(find('.group-edit-allow-membership-requests[disabled]').length === 1, 'it should disable group allow_membership_request input');
assert.ok(
find('.group-edit-allow-membership-requests[disabled]').length === 1,
'it should disable group allow_membership_request input'
);
});
click('.group-edit-public');
click('.group-edit-public-admission');
click('.group-edit-allow-membership-requests');
andThen(() => {
assert.ok(find('.group-edit-public[disabled]').length === 1, 'it should disable group public input');
assert.ok(
find('.group-edit-public-admission[disabled]').length === 1,
'it should disable group public admission input'
);
});
});
@ -37,4 +53,4 @@ QUnit.test("Editing group as an anonymous user", assert => {
andThen(() => {
assert.ok(count('.group-members tr') > 0, "it should redirect to members page for an anonymous user");
});
});
});

46
test/javascripts/components/group-membership-button-test.js.es6
View File

@ -2,18 +2,52 @@ moduleFor('component:group-membership-button');
QUnit.test('canJoinGroup', function(assert) {
this.subject().setProperties({
model: { public: false }
model: { public_admission: false, is_group_user: true }
});
assert.equal(this.subject().get("canJoinGroup"), false, "non public group cannot be joined");
assert.equal(
this.subject().get("canJoinGroup"), false,
"can't join group if public_admission is false"
);
this.subject().set("model.public", true);
this.subject().set("model.public_admission", true);
assert.equal(this.subject().get("canJoinGroup"), true, "public group can be joined");
assert.equal(
this.subject().get("canJoinGroup"), false,
"can't join group if user is already in the group"
);
this.subject().setProperties({ currentUser: null, model: { public: true } });
this.subject().set("model.is_group_user", false);
assert.equal(this.subject().get("canJoinGroup"), true, "can't join group when not logged in");
assert.equal(
this.subject().get("canJoinGroup"), true,
"allowed to join group"
);
});
QUnit.test('canLeaveGroup', function(assert) {
this.subject().setProperties({
model: { public_exit: false, is_group_user: false }
});
assert.equal(
this.subject().get("canLeaveGroup"), false,
"can't leave group if public_exit is false"
);
this.subject().set("model.public_exit", true);
assert.equal(
this.subject().get("canLeaveGroup"), false,
"can't leave group if user is not in the group"
);
this.subject().set("model.is_group_user", true);
assert.equal(
this.subject().get("canLeaveGroup"), true,
"allowed to leave group"
);
});
QUnit.test('userIsGroupUser', function(assert) {

14
test/javascripts/controllers/admin-group-test.js.es6
View File

@ -20,16 +20,22 @@ QUnit.test("disablePublicSetting", function(assert) {
QUnit.test("disableMembershipRequestSetting", function(assert) {
this.subject().setProperties({
model: { visibility_level: 1, public: false, canEveryoneMention: true }
model: { visibility_level: 1, public_admission: false, canEveryoneMention: true }
});
assert.equal(this.subject().get("disableMembershipRequestSetting"), true, "it should disable setting");
this.subject().set("model.visibility_level", 0);
assert.equal(this.subject().get("disableMembershipRequestSetting"), false, "it should enable setting");
assert.equal(
this.subject().get("disableMembershipRequestSetting"), false,
"it should enable setting"
);
this.subject().set("model.public", true);
this.subject().set("model.public_admission", true);
assert.equal(this.subject().get("disableMembershipRequestSetting"), true, "it should disalbe setting");
assert.equal(
this.subject().get("disableMembershipRequestSetting"), true,
"it should disable setting"
);
});

3
test/javascripts/fixtures/group-fixtures.js.es6
View File

@ -8,7 +8,8 @@ export default {
"user_count":8,
"alias_level":99,
"visible":true,
"public":true,
"public_admission":true,
"public_exit":true,
"flair_url": 'fa-adjust',
"is_group_owner":true,
"mentionable":true

2
test/javascripts/fixtures/groups-fixtures.js.es6
View File

@ -1,3 +1,3 @@
export default {
"/groups.json": {"groups":[{"id":41,"automatic":false,"name":"discourse","user_count":0,"alias_level":0,"visible":true,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":false,"title":null,"grant_trust_level":null,"has_messages":false,"flair_url":null,"flair_bg_color":null,"flair_color":null,"bio_raw":"","bio_cooked":null,"public":true,"allow_membership_requests":false,"full_name":"Awesome Team"},{"id":42,"automatic":false,"name":"Macdonald","user_count":0,"alias_level":99,"visible":true,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":false,"title":null,"grant_trust_level":null,"has_messages":false,"flair_url":null,"flair_bg_color":null,"flair_color":null,"bio_raw":null,"bio_cooked":null,"public":false,"allow_membership_requests":true,"full_name":null}],"extras":{"group_user_ids":[]},"total_rows_groups":2,"load_more_groups":"/groups?page=1"}
"/groups.json": {"groups":[{"id":41,"automatic":false,"name":"discourse","user_count":0,"alias_level":0,"visible":true,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":false,"title":null,"grant_trust_level":null,"has_messages":false,"flair_url":null,"flair_bg_color":null,"flair_color":null,"bio_raw":"","bio_cooked":null,"public_admission":true,"allow_membership_requests":false,"full_name":"Awesome Team"},{"id":42,"automatic":false,"name":"Macdonald","user_count":0,"alias_level":99,"visible":true,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":false,"title":null,"grant_trust_level":null,"has_messages":false,"flair_url":null,"flair_bg_color":null,"flair_color":null,"bio_raw":null,"bio_cooked":null,"public_admission":false,"allow_membership_requests":true,"full_name":null}],"extras":{"group_user_ids":[]},"total_rows_groups":2,"load_more_groups":"/groups?page=1"}
}