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

Merge pull request #5007 from tgxworld/split_public_settings 

FEATURE: Add group settngs to allow users to leave a group freely.
This commit is contained in:
Guo Xiang Tan 2017-07-28 15:15:33 +09:00 committed by GitHub
parent 5012d46cbd 4620dfe92d
commit e84e310325
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')

14
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}}
{{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

62
spec/services/group_action_logger_spec.rb
View File

@ -36,6 +36,7 @@ RSpec.describe GroupActionLogger do
end
describe '#log_add_user_to_group' do
describe 'as a group owner' do
it 'should create the right record' do
subject.log_add_user_to_group(user)
@ -47,7 +48,36 @@ RSpec.describe GroupActionLogger do
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
describe 'as group owner' do
it 'should create the right record' do
subject.log_remove_user_from_group(user)
@ -59,9 +89,37 @@ RSpec.describe GroupActionLogger do
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

24
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'
);
});
});

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"}
}