Discource-C/discourse
1
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-02-22 12:17:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

FEATURE: add support for group visibility level

Browse Source 
There are 4 visibility levels

- public (default)
- members only
- staff
- owners

Note, admins and group owners ALWAYS have visibility to groups

Migration treated old "non public" as "members only"
This commit is contained in:
Sam 2017-07-03 15:26:46 -04:00
parent c79418d334
commit 845170bd6b
17 changed files with 248 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -15,6 +15,15 @@ export default Ember.Controller.extend({
];
}.property(),
visibilityLevelOptions: function() {
return [
{ name: I18n.t("groups.visibility_levels.public"), value: 0 },
{ name: I18n.t("groups.visibility_levels.members"), value: 1 },
{ name: I18n.t("groups.visibility_levels.staff"), value: 2 },
{ name: I18n.t("groups.visibility_levels.owners"), value: 3 }
];
}.property(),
trustLevelOptions: function() {
return [
{ name: I18n.t("groups.trust_levels.none"), value: 0 },
@ -22,14 +31,16 @@ export default Ember.Controller.extend({
];
}.property(),
@computed('model.visible', 'model.public')
disableMembershipRequestSetting(visible, publicGroup) {
return !visible || publicGroup;
@computed('model.visibility_level', 'model.public')
disableMembershipRequestSetting(visibility_level, publicGroup) {
visibility_level = parseInt(visibility_level);
return (visibility_level !== 0) || publicGroup;
},
@computed('model.visible', 'model.allow_membership_requests')
disablePublicSetting(visible, allowMembershipRequests) {
return !visible || allowMembershipRequests;
@computed('model.visibility_level', 'model.allow_membership_requests')
disablePublicSetting(visibility_level, allowMembershipRequests) {
visibility_level = parseInt(visibility_level);
return (visibility_level !== 0) || allowMembershipRequests;
},
actions: {

2
app/assets/javascripts/admin/routes/admin-group.js.es6
View File

@ -4,7 +4,7 @@ export default Discourse.Route.extend({
model(params) {
if (params.name === 'new') {
return Group.create({ automatic: false, visible: true });
return Group.create({ automatic: false, visibility_level: 0 });
}
const group = this.modelFor('adminGroupsType').findBy('name', params.name);

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

@ -43,10 +43,8 @@
{{/if}}
<div>
<label>
{{input type="checkbox" checked=model.visible}}
{{i18n 'groups.visible'}}
</label>
<label for="visiblity">{{i18n 'groups.visibility_levels.title'}}</label>
{{combo-box name="alias" valueAttribute="value" value=model.visibility_level content=visibilityLevelOptions}}
</div>
{{#unless model.automatic}}

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

@ -113,23 +113,27 @@ const Group = RestModel.extend({
return aliasLevel === '99';
},
@observes("visible", "canEveryoneMention")
@observes("visibility_level", "canEveryoneMention")
_updateAllowMembershipRequests() {
if (!this.get('visible') || !this.get('canEveryoneMention')) {
if (this.get('visibility_level') !== 0 || !this.get('canEveryoneMention')) {
this.set ('allow_membership_requests', false);
}
},
@observes("visible")
@observes("visibility_level")
_updatePublic() {
if (!this.get('visible')) this.set('public', false);
let visibility_level = parseInt(this.get('visibility_level'));
if (visibility_level !== 0) {
this.set('public', false);
this.set('allow_membership_requests', false);
}
},
asJSON() {
return {
name: this.get('name'),
alias_level: this.get('alias_level'),
visible: !!this.get('visible'),
visibility_level: this.get('visibility_level'),
automatic_membership_email_domains: this.get('emailDomains'),
automatic_membership_retroactive: !!this.get('automatic_membership_retroactive'),
title: this.get('title'),

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

@ -59,7 +59,7 @@ class Admin::GroupsController < Admin::AdminController
def save_group(group)
group.alias_level = group_params[:alias_level].to_i if group_params[:alias_level].present?
group.visible = group_params[:visible] == "true"
group.visibility_level = group_params[:visibility_level]
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
@ -160,7 +160,7 @@ class Admin::GroupsController < Admin::AdminController
def group_params
params.require(:group).permit(
:name, :alias_level, :visible, :automatic_membership_email_domains,
: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,

6
app/controllers/groups_controller.rb
View File

@ -20,6 +20,12 @@ class GroupsController < ApplicationController
page = params[:page]&.to_i || 0
groups = Group.visible_groups(current_user)
unless guardian.is_staff?
# hide automatic groups from all non stuff to de-clutter page
groups = groups.where(automatic: false)
end
count = groups.count
groups = groups.offset(page * page_size).limit(page_size)

69
app/models/group.rb
View File

@ -1,3 +1,7 @@
# frozen_string_literal: true
require_dependency 'enum'
class Group < ActiveRecord::Base
include HasCustomFields
include AnonCacheInvalidator
@ -62,17 +66,56 @@ class Group < ActiveRecord::Base
:everyone => 99
}
def self.visibility_levels
@visibility_levels = Enum.new(
public: 0,
members: 1,
staff: 2,
owners: 3
)
end
validates :alias_level, inclusion: { in: ALIAS_LEVELS.values}
scope :visible_groups, ->(user) {
groups = Group.order(name: :asc).where("groups.id > 0")
if !user || !user.admin
owner_group_ids = GroupUser.where(user: user, owner: true).pluck(:group_id)
unless user&.admin
sql = <<~SQL
groups.id IN (
SELECT g.id FROM groups g WHERE g.visibility_level = :public
UNION ALL
SELECT g.id FROM groups g
JOIN group_users gu ON gu.group_id = g.id AND
gu.user_id = :user_id
WHERE g.visibility_level = :members
UNION ALL
SELECT g.id FROM groups g
LEFT JOIN group_users gu ON gu.group_id = g.id AND
gu.user_id = :user_id AND
gu.owner
WHERE g.visibility_level = :staff AND (gu.id IS NOT NULL OR :is_staff)
UNION ALL
SELECT g.id FROM groups g
JOIN group_users gu ON gu.group_id = g.id AND
gu.user_id = :user_id AND
gu.owner
WHERE g.visibility_level = :owners
)
SQL
groups = groups.where(
sql,
Group.visibility_levels.to_h.merge(user_id: user&.id, is_staff: !!user&.staff?)
)
groups = groups.where("
(groups.automatic = false AND groups.visible = true) OR groups.id IN (?)
", owner_group_ids)
end
groups
@ -191,7 +234,7 @@ class Group < ActiveRecord::Base
# the everyone group is special, it can include non-users so there is no
# way to have the membership in a table
if name == :everyone
group.visible = false
group.visibility_level = Group.visibility_levels[:owners]
group.save!
return group
end
@ -282,7 +325,7 @@ class Group < ActiveRecord::Base
end
def self.search_group(name)
Group.where(visible: true).where(
Group.where(visibility_level: visibility_levels[:public]).where(
"name ILIKE :term_like OR full_name ILIKE :term_like", term_like: "#{name}%"
)
end
@ -487,11 +530,11 @@ class Group < ActiveRecord::Base
return if new_record? && !self.primary_group?
if self.primary_group_changed?
sql = <<SQL
UPDATE users
/*set*/
/*where*/
SQL
sql = <<~SQL
UPDATE users
/*set*/
/*where*/
SQL
builder = SqlBuilder.new(sql)
builder.where("
@ -539,7 +582,6 @@ end
# automatic :boolean default(FALSE), not null
# user_count :integer default(0), not null
# alias_level :integer default(0)
# visible :boolean default(TRUE), not null
# automatic_membership_email_domains :text
# automatic_membership_retroactive :boolean default(FALSE)
# primary_group :boolean default(FALSE), not null
@ -556,6 +598,7 @@ end
# allow_membership_requests :boolean default(FALSE), not null
# full_name :string
# default_notification_level :integer default(3), not null
# visibility_level :integer default(0)
#
# Indexes
#

2
app/serializers/basic_group_serializer.rb
View File

@ -5,7 +5,7 @@ class BasicGroupSerializer < ApplicationSerializer
:display_name,
:user_count,
:alias_level,
:visible,
:visibility_level,
:automatic_membership_email_domains,
:automatic_membership_retroactive,
:primary_group,

9
app/serializers/user_serializer.rb
View File

@ -129,13 +129,8 @@ class UserSerializer < BasicUserSerializer
end
def groups
groups = object.groups.order(:id)
if scope.is_admin? || object.id == scope.user.try(:id)
groups
else
groups.where(visible: true)
end
object.groups.order(:id)
.visible_groups(scope.user)
end
def group_users

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

@ -408,7 +408,7 @@ 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 group to be visible)"
public: "Allow users to join/leave the group freely (Requires publicly visible group)"
empty:
posts: "There are no posts by members of this group."
members: "There are no members in this group."
@ -430,7 +430,6 @@ en:
bio: "About Group"
selector_placeholder: "Add members"
owner: "owner"
visible: "Group is visible to all users"
index:
title: "Groups"
empty: "There are no visible groups."
@ -444,6 +443,12 @@ en:
mentions: "Mentions"
messages: "Messages"
notification_level: "Default notification level for group messages"
visibility_levels:
title: "Who can see this group?"
public: "Everyone"
members: "Group owners, members and admins"
staff: "Group owners and staff"
owners: "Group owners and admins"
alias_levels:
title: "Who can message and @mention this group?"
nobody: "Nobody"

3
db/fixtures/002_groups.rb
View File

@ -2,4 +2,5 @@ Group.ensure_automatic_groups!
if g = Group.find_by(name: 'trust_level_5', id: 15)
g.destroy!
end
Group.where(name: 'everyone').update_all(visible: false)
Group.where(name: 'everyone').update_all(visibility_level: Group.visibility_levels[:owners])

12
db/migrate/20170703144855_add_visibility_level_to_groups.rb Normal file
View File

@ -0,0 +1,12 @@
class AddVisibilityLevelToGroups < ActiveRecord::Migration
def change
add_column :groups, :visibility_level, :integer, default: 0, null: false
execute <<~SQL
UPDATE groups
SET visibility_level = 1
WHERE NOT visible
SQL
remove_column :groups, :visible
end
end

15
lib/guardian.rb
View File

@ -135,10 +135,21 @@ class Guardian
def can_see_group?(group)
return false if group.blank?
return true if is_admin? || group.visible?
return true if group.visibility_level == Group.visibility_levels[:public]
return true if is_admin?
return true if is_staff? && group.visibility_level == Group.visibility_levels[:staff]
return false if user.blank?
group.group_users.where(user_id: user.id).exists?
membership = GroupUser.find_by(group_id: group.id, user_id: user.id)
return false unless membership
if !membership.owner
return false if group.visibility_level == Group.visibility_levels[:owners]
return false if group.visibility_level == Group.visibility_levels[:staff]
end
true
end

88
spec/components/guardian_spec.rb
View File

@ -434,31 +434,6 @@ describe Guardian do
expect(Guardian.new.can_see?(nil)).to be_falsey
end
describe 'a Group' do
let(:group) { Group.new }
let(:invisible_group) { Group.new(visible: false, name: 'invisible') }
it "returns true when the group is visible" do
expect(Guardian.new.can_see?(group)).to be_truthy
end
it "returns true when the group is invisible but the user is an admin" do
admin = Fabricate.build(:admin)
expect(Guardian.new(admin).can_see?(invisible_group)).to be_truthy
end
it "returns true when the group is invisible but the user is a member" do
invisible_group.save!
member = Fabricate.build(:user)
GroupUser.create(group: invisible_group, user: member)
expect(Guardian.new(member).can_see?(invisible_group)).to be_truthy
end
it "returns false when the group is invisible" do
expect(Guardian.new.can_see?(invisible_group)).to be_falsey
end
end
describe 'a Category' do
it 'allows public categories' do
@ -2436,6 +2411,69 @@ describe Guardian do
end
end
describe(:can_see_group) do
it 'Correctly handles owner visibile groups' do
group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:owners])
member = Fabricate(:user)
group.add(member)
group.save!
owner = Fabricate(:user)
group.add_owner(owner)
group.reload
expect(Guardian.new(admin).can_see_group?(group)).to eq(true)
expect(Guardian.new(moderator).can_see_group?(group)).to eq(false)
expect(Guardian.new(member).can_see_group?(group)).to eq(false)
expect(Guardian.new.can_see_group?(group)).to eq(false)
expect(Guardian.new(owner).can_see_group?(group)).to eq(true)
end
it 'Correctly handles staff visibile groups' do
group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:staff])
member = Fabricate(:user)
group.add(member)
group.save!
owner = Fabricate(:user)
group.add_owner(owner)
group.reload
expect(Guardian.new(member).can_see_group?(group)).to eq(false)
expect(Guardian.new(admin).can_see_group?(group)).to eq(true)
expect(Guardian.new(moderator).can_see_group?(group)).to eq(true)
expect(Guardian.new(owner).can_see_group?(group)).to eq(true)
expect(Guardian.new.can_see_group?(group)).to eq(false)
end
it 'Correctly handles member visibile groups' do
group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:members])
member = Fabricate(:user)
group.add(member)
group.save!
owner = Fabricate(:user)
group.add_owner(owner)
group.reload
expect(Guardian.new(moderator).can_see_group?(group)).to eq(false)
expect(Guardian.new.can_see_group?(group)).to eq(false)
expect(Guardian.new(admin).can_see_group?(group)).to eq(true)
expect(Guardian.new(member).can_see_group?(group)).to eq(true)
expect(Guardian.new(owner).can_see_group?(group)).to eq(true)
end
it 'Correctly handles public groups' do
group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:public])
expect(Guardian.new.can_see_group?(group)).to eq(true)
end
end
context 'topic featured link category restriction' do
before { SiteSetting.topic_featured_link_enabled = true }
let(:guardian) { Guardian.new }

8
spec/controllers/admin/groups_controller_spec.rb
View File

@ -29,7 +29,7 @@ describe Admin::GroupsController do
"user_count"=>1,
"automatic"=>false,
"alias_level"=>0,
"visible"=>true,
"visibility_level"=>0,
"automatic_membership_email_domains"=>nil,
"automatic_membership_retroactive"=>false,
"title"=>nil,
@ -100,15 +100,17 @@ describe Admin::GroupsController do
expect do
xhr :put, :update, { id: group.id, group: {
visible: "false",
visibility_level: Group.visibility_levels[:owners],
allow_membership_requests: "true"
} }
end.to change { GroupHistory.count }.by(2)
expect(response).to be_success
group.reload
expect(group.visible).to eq(false)
expect(group.visibility_level).to eq( Group.visibility_levels[:owners])
expect(group.allow_membership_requests).to eq(true)
end

17
spec/integration/groups_spec.rb
View File

@ -2,16 +2,11 @@ require 'rails_helper'
describe "Groups" do
let(:user) { Fabricate(:user) }
let(:group) { Fabricate(:group, users: [user]) }
let!(:group) { Fabricate(:group, users: [user]) }
describe 'viewing groups' do
let(:other_group) do
Fabricate(:group, name: '0000', visible: true, automatic: false)
end
before do
other_group
group.update_attributes!(automatic: true, visible: true)
let!(:staff_group) do
Fabricate(:group, name: '0000', visibility_level: Group.visibility_levels[:staff])
end
context 'when group directory is disabled' do
@ -33,8 +28,8 @@ describe "Groups" do
group_ids = response_body["groups"].map { |g| g["id"] }
expect(response_body["extras"]["group_user_ids"]).to eq([])
expect(group_ids).to include(other_group.id)
expect(group_ids).to_not include(group.id)
expect(group_ids).to include(group.id)
expect(group_ids).to_not include(staff_group.id)
expect(response_body["load_more_groups"]).to eq("/groups?page=1")
expect(response_body["total_rows_groups"]).to eq(1)
end
@ -54,7 +49,7 @@ describe "Groups" do
group_ids = response_body["groups"].map { |g| g["id"] }
expect(response_body["extras"]["group_user_ids"]).to eq([group.id])
expect(group_ids).to include(group.id, other_group.id)
expect(group_ids).to include(group.id, staff_group.id)
expect(response_body["load_more_groups"]).to eq("/groups?page=1")
expect(response_body["total_rows_groups"]).to eq(10)
end

78
spec/models/group_spec.rb
View File

@ -180,7 +180,7 @@ describe Group do
describe '.refresh_automatic_group!' do
it "makes sure the everyone group is not visible" do
g = Group.refresh_automatic_group!(:everyone)
expect(g.visible).to eq(false)
expect(g.visibility_level).to eq(Group.visibility_levels[:owners])
end
it "uses the localized name if name has not been taken" do
@ -432,44 +432,54 @@ describe Group do
end
describe ".visible_groups" do
let(:group) { Fabricate(:group, visible: false) }
let(:group_2) { Fabricate(:group, visible: true) }
let(:admin) { Fabricate(:admin) }
let(:user) { Fabricate(:user) }
before do
group
group_2
def can_view?(user, group)
Group.visible_groups(user).where(id: group.id).exists?
end
describe 'when user is an admin' do
it 'should return the right groups' do
expect(Group.visible_groups(admin).pluck(:id).sort)
.to eq([group.id, group_2.id].concat(Group::AUTO_GROUP_IDS.keys - [0]).sort)
end
it 'correctly restricts group visibility' do
group = Fabricate.build(:group, visibility_level: Group.visibility_levels[:owners])
member = Fabricate(:user)
group.add(member)
group.save!
owner = Fabricate(:user)
group.add_owner(owner)
moderator = Fabricate(:user, moderator: true)
admin = Fabricate(:user, admin: true)
expect(can_view?(admin, group)).to eq(true)
expect(can_view?(owner, group)).to eq(true)
expect(can_view?(moderator, group)).to eq(false)
expect(can_view?(member, group)).to eq(false)
expect(can_view?(nil, group)).to eq(false)
group.update_columns(visibility_level: Group.visibility_levels[:staff])
expect(can_view?(admin, group)).to eq(true)
expect(can_view?(owner, group)).to eq(true)
expect(can_view?(moderator, group)).to eq(true)
expect(can_view?(member, group)).to eq(false)
expect(can_view?(nil, group)).to eq(false)
group.update_columns(visibility_level: Group.visibility_levels[:members])
expect(can_view?(admin, group)).to eq(true)
expect(can_view?(owner, group)).to eq(true)
expect(can_view?(moderator, group)).to eq(false)
expect(can_view?(member, group)).to eq(true)
expect(can_view?(nil, group)).to eq(false)
group.update_columns(visibility_level: Group.visibility_levels[:public])
expect(can_view?(admin, group)).to eq(true)
expect(can_view?(owner, group)).to eq(true)
expect(can_view?(moderator, group)).to eq(true)
expect(can_view?(member, group)).to eq(true)
expect(can_view?(nil, group)).to eq(true)
end
describe 'when user is owner of a group' do
it 'should return the right groups' do
group.add_owner(user)
expect(Group.visible_groups(user).pluck(:id).sort)
.to eq([group.id, group_2.id])
end
end
describe 'when user is not the owner of any group' do
it 'should return the right groups' do
expect(Group.visible_groups(user).pluck(:id).sort)
.to eq([group_2.id])
end
end
describe 'user is nil' do
it 'should return the right groups' do
expect(Group.visible_groups(nil).pluck(:id).sort).to eq([group_2.id])
end
end
end
describe '#add' do