ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

group model prep for communities; refs #8598 

this commit just lays down a bit of ground work for communities. it defines
communities as a protected group category type for accounts which allows
membership in multiple groups. it also generalizes several places where student
organized groups was a special case. it also removes the restriction that
group.is_public always be false. this property will be used for different
visibility levels within communities. finally, it lays some groundwork for
handling group requests/invitations/following.

test plan:
- there should be no functional changes from this commit
- you should be able to run GroupCategory.communities_for(account) at the
  console for an account, and get a category shell for communities for that
  account.

Change-Id: Icff3f0e7fbd4ec3c0a72cdbaa310cbd0e75d0e3e
Reviewed-on: https://gerrit.instructure.com/10780
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Simon Williams 2012-05-15 15:50:43 -06:00
parent 5b04db4372
commit 127b9b3302
7 changed files with 315 additions and 173 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/controllers/groups_controller.rb
View File

@ -74,7 +74,7 @@ class GroupsController < ApplicationController
end
@current_conferences = @group.web_conferences.select{|c| c.active? && c.users.include?(@current_user) } rescue []
@groups = @current_user.group_memberships_for(@group.context) if @current_user
if params[:join] && @group.free_association?(@current_user)
if params[:join] && @group.can_join?(@current_user)
@group.request_user(@current_user)
if !@group.grants_right?(@current_user, session, :read)
render :action => 'membership_pending'
@ -85,7 +85,7 @@ class GroupsController < ApplicationController
return
end
end
if params[:leave] && (@group.free_association?(@current_user) || @group.student_organized?)
if params[:leave] && @group.can_leave?(@current_user)
membership = @group.membership_for_user(@current_user)
if membership
membership.destroy
@ -334,7 +334,7 @@ class GroupsController < ApplicationController
@user_groups = @current_user.group_memberships_for(@context).select{|g| group_ids.include?(g.id) } if @current_user
@user_groups ||= []
@available_groups = (@groups - @user_groups).select{|g| g.free_association?(@current_user) }
@available_groups = (@groups - @user_groups).select{|g| g.can_join?(@current_user) }
if !@context.grants_right?(@current_user, session, :manage_groups)
@groups = @user_groups
end

126
app/models/group.rb
View File

@ -25,8 +25,6 @@ class Group < ActiveRecord::Base
has_many :users, :through => :group_memberships, :conditions => ['users.workflow_state != ?', 'deleted']
has_many :participating_group_memberships, :class_name => "GroupMembership", :conditions => ['group_memberships.workflow_state = ?', 'accepted']
has_many :participating_users, :source => :user, :through => :participating_group_memberships
has_many :invited_group_memberships, :class_name => "GroupMembership", :conditions => ['group_memberships.workflow_state = ?', 'invited']
has_many :invited_users, :source => :user, :through => :invited_group_memberships
belongs_to :context, :polymorphic => true
belongs_to :group_category
belongs_to :account
@ -86,26 +84,33 @@ class Group < ActiveRecord::Base
end
def auto_accept?(user)
self.context.grants_right?(user, :participate_in_groups) &&
self.student_organized? &&
self.group_category &&
self.group_category.available_for?(user) &&
self.group_category.allows_multiple_memberships? &&
self.join_level == 'parent_context_auto_join'
end
def allow_join_request?(user)
self.context.grants_right?(user, :participate_in_groups) &&
self.student_organized? &&
self.group_category &&
self.group_category.available_for?(user) &&
self.group_category.allows_multiple_memberships? &&
['parent_context_auto_join', 'parent_context_request'].include?(self.join_level)
end
def allow_self_signup?(user)
self.context &&
self.context.grants_right?(user, :participate_in_groups) &&
self.group_category &&
(self.group_category.unrestricted_self_signup? ||
(self.group_category.restricted_self_signup? && self.has_common_section_with_user?(user)))
end
def free_association?(user)
allow_join_request?(user) || allow_self_signup?(user)
def can_join?(user)
auto_accept?(user) || allow_join_request?(user) || allow_self_signup?(user)
end
def can_leave?(user)
self.group_category.try(:allows_multiple_memberships?) || self.allow_self_signup?(user)
end
def participants(include_observers=false)
@ -125,6 +130,14 @@ class Group < ActiveRecord::Base
self.group_memberships.find_by_user_id(user && user.id)
end
def has_member?(user)
self.participating_group_memberships.find_by_user_id(user && user.id)
end
def has_moderator?(user)
self.participating_group_memberships.moderators.find_by_user_id(user && user.id)
end
def short_name
name
end
@ -175,11 +188,6 @@ class Group < ActiveRecord::Base
res += (self.context.course_code rescue self.context.name) if self.context
end
def is_public
false
end
def to_atom
Atom::Entry.new do |entry|
entry.title = self.name
@ -190,40 +198,26 @@ class Group < ActiveRecord::Base
end
end
def add_user(user)
def add_user(user, new_record_state=nil)
return nil if !user
unless member = self.group_memberships.find_by_user_id(user.id)
member = self.group_memberships.create(:user=>user)
attrs = { :user => user }
new_record_state ||= case self.join_level
when 'invitation_only' then 'invited'
when 'parent_context_request' then 'requested'
when 'parent_context_auto_join' then 'accepted'
end
attrs[:workflow_state] = new_record_state if new_record_state
member = self.group_memberships.find_by_user_id(user.id)
member ||= self.group_memberships.create(attrs)
return member
end
def invite_user(user)
return nil if !user
res = nil
Group.transaction do
res = self.group_memberships.find_by_user_id(user.id)
unless res
res = self.group_memberships.build(:user => user)
res.workflow_state = 'invited'
res.save
end
end
res
self.add_user(user, 'invited')
end
def request_user(user)
return nil if !user
res = nil
Group.transaction do
res = self.group_memberships.find_by_user_id(user.id)
unless res
res = self.group_memberships.build(:user => user)
res.workflow_state = 'requested'
res.save
end
end
res
self.add_user(user, 'requested')
end
def invitees=(params)
@ -239,10 +233,8 @@ class Group < ActiveRecord::Base
end
def peer_groups
return [] if !self.context || self.student_organized?
category = self.group_category || GroupCategory.student_organized_for(self.context)
return [] unless category
category.groups.find(:all, :conditions => ["id != ?", self.id])
return [] if !self.context || !self.group_category || self.group_category.allows_multiple_memberships?
self.group_category.groups.find(:all, :conditions => ["id != ?", self.id])
end
def migrate_content_links(html, from_course)
@ -274,6 +266,7 @@ class Group < ActiveRecord::Base
self.uuid ||= AutoHandle.generate_securish_uuid
self.group_category ||= GroupCategory.student_organized_for(self.context)
self.join_level ||= 'invitation_only'
self.is_public ||= false
if self.context && self.context.is_a?(Course)
self.account = self.context.account
elsif self.context && self.context.is_a?(Account)
@ -296,34 +289,55 @@ class Group < ActiveRecord::Base
# if you modify this set_policy block, note that we've denormalized this
# permission check for efficiency -- see User#cached_contexts
set_policy do
given { |user| user && self.participating_group_memberships.find_by_user_id(user.id) }
can :read and can :read_roster and can :manage and can :manage_content and can :manage_students and can :manage_admin_users and
can :manage_files and
can :post_to_forum and
can :send_messages and can :create_conferences and
can :create_collaborations and can :read_roster and
given { |user| user && self.has_member?(user) }
can :create and
can :create_collaborations and
can :create_conferences and
can :delete and
can :manage and
can :manage_admin_users and
can :manage_calendar and
can :update and can :delete and can :create and
can :manage_wiki
can :manage_content and
can :manage_files and
can :manage_students and
can :manage_wiki and
can :post_to_forum and
can :read and
can :read_roster and
can :send_messages and
can :update
# if I am a member of this group and I can moderate_forum in the group's context
# (makes it so group members cant edit each other's discussion entries)
given { |user, session| user && self.participating_group_memberships.find_by_user_id(user.id) && (!self.context || self.context.grants_right?(user, session, :moderate_forum)) }
given { |user, session| user && self.has_member?(user) && (!self.context || self.context.grants_right?(user, session, :moderate_forum)) }
can :moderate_forum
given { |user| user && self.invited_users.include?(user) }
can :read
given { |user| user && self.has_moderator?(user) }
can :moderate_forum
given { |user, session| self.context && self.context.grants_right?(user, session, :participate_as_student) && self.context.allow_student_organized_groups }
can :create
given { |user, session| self.context && self.context.grants_right?(user, session, :manage_groups) }
can :read and can :read_roster and can :manage and can :manage_content and can :manage_students and can :manage_admin_users and can :update and can :delete and can :create and can :moderate_forum and can :post_to_forum and can :manage_wiki and can :manage_files and can :create_conferences
can :create and
can :create_conferences and
can :delete and
can :manage and
can :manage_admin_users and
can :manage_content and
can :manage_files and
can :manage_students and
can :manage_wiki and
can :moderate_forum and
can :post_to_forum and
can :read and
can :read_roster and
can :update
given { |user, session| self.context && self.context.grants_right?(user, session, :view_group_pages) }
can :read and can :read_roster
given { |user, session| self.context && self.free_association?(user) }
given { |user| user && self.can_join?(user) }
can :read_roster
end
@ -380,7 +394,7 @@ class Group < ActiveRecord::Base
def self.serialization_excludes; [:uuid]; end
def self.process_migration(data, migration)
groups = data['groups'] ? data['groups']: []
groups = data['groups'] || []
groups.each do |group|
if migration.import_object?("groups", group['migration_id'])
begin

21
app/models/group_category.rb
View File

@ -43,18 +43,23 @@ class GroupCategory < ActiveRecord::Base
role_category_for_context('imported', context)
end
def communities_for(context)
role_category_for_context('communities', context)
end
protected
def name_for_role(role)
case role
when 'student_organized' then t('group_categories.student_organized', "Student Groups")
when 'imported' then t('group_categories.imported', "Imported Groups")
when 'communities' then t('group_categories.communities', "Communities")
end
end
def protected_roles_for_context(context)
case context
when Course then ['student_organized', 'imported']
when Account then ['imported']
when Account then ['communities', 'imported']
else []
end
end
@ -74,6 +79,10 @@ class GroupCategory < ActiveRecord::Base
end
end
def communities?
self.role == 'communities'
end
def student_organized?
self.role == 'student_organized'
end
@ -82,6 +91,16 @@ class GroupCategory < ActiveRecord::Base
self.role.present?
end
def allows_multiple_memberships?
self.student_organized? || self.communities?
end
# Communities can be joined by anyone,
# otherwise defer to the permission on the context
def available_for?(user)
self.communities? || (self.context && self.context.grants_right?(user, :participate_in_groups))
end
# this is preferred over setting self_signup directly. know that if you set
# self_signup directly to anything other than nil (or ''), 'restricted', or
# 'enabled', it will behave as if you used 'enabled'.

4
app/models/group_membership.rb
View File

@ -23,7 +23,7 @@ class GroupMembership < ActiveRecord::Base
belongs_to :group
belongs_to :user
attr_accessible :group, :user
attr_accessible :group, :user, :workflow_state
before_save :ensure_mutually_exclusive_membership
before_save :assign_uuid
@ -40,6 +40,7 @@ class GroupMembership < ActiveRecord::Base
named_scope :include_user, :include => :user
named_scope :active, :conditions => ['group_memberships.workflow_state != ?', 'deleted']
named_scope :moderators, :conditions => { :moderator => true }
set_broadcast_policy do |p|
p.dispatch :new_context_group_membership
@ -111,6 +112,7 @@ class GroupMembership < ActiveRecord::Base
workflow do
state :accepted
state :following
state :invited do
event :reject, :transitions_to => :rejected
event :accept, :transitions_to => :accepted

11
db/migrate/20120516185217_add_moderator_flag_to_group_memberships.rb Normal file
View File

@ -0,0 +1,11 @@
class AddModeratorFlagToGroupMemberships < ActiveRecord::Migration
tag :predeploy
def self.up
add_column :group_memberships, :moderator, :boolean
end
def self.down
remove_column :group_memberships, :moderator
end
end

50
spec/models/group_category_spec.rb
View File

@ -60,6 +60,28 @@ describe GroupCategory do
end
end
context "communities_for" do
it "should be nil in courses" do
course_model
GroupCategory.communities_for(@course).should be_nil
end
it "should be a category belonging to the account with role 'communities'" do
account = Account.default
category = GroupCategory.communities_for(account)
category.should_not be_nil
category.role.should eql('communities')
category.context.should eql(account)
end
it "should be the the same category every time for the same account" do
account = Account.default
category1 = GroupCategory.communities_for(account)
category2 = GroupCategory.communities_for(account)
category1.id.should eql(category2.id)
end
end
context "imported_for" do
it "should be a category belonging to the account with role 'imported' in accounts" do
account = Account.default
@ -94,6 +116,33 @@ describe GroupCategory do
GroupCategory.imported_for(course).should_not be_student_organized
GroupCategory.imported_for(course).should_not be_student_organized
course.group_categories.create(:name => 'Random Category').should_not be_student_organized
GroupCategory.communities_for(account).should_not be_student_organized
end
end
context 'communities?' do
it "should be true iff the role is 'communities', regardless of name" do
account = Account.default
course = course_model
GroupCategory.student_organized_for(course).should_not be_communities
account.group_categories.create(:name => 'Communities').should_not be_communities
GroupCategory.imported_for(course).should_not be_communities
GroupCategory.imported_for(course).should_not be_communities
course.group_categories.create(:name => 'Random Category').should_not be_communities
GroupCategory.communities_for(account).should be_communities
end
end
context 'allows_multiple_memberships?' do
it "should be true iff the category is student organized or communities" do
account = Account.default
course = course_model
GroupCategory.student_organized_for(course).allows_multiple_memberships?.should be_true
account.group_categories.create(:name => 'Student Groups').allows_multiple_memberships?.should be_false
GroupCategory.imported_for(course).allows_multiple_memberships?.should be_false
GroupCategory.imported_for(course).allows_multiple_memberships?.should be_false
course.group_categories.create(:name => 'Random Category').allows_multiple_memberships?.should be_false
GroupCategory.communities_for(account).allows_multiple_memberships?.should be_true
end
end
@ -106,6 +155,7 @@ describe GroupCategory do
GroupCategory.imported_for(course).should be_protected
GroupCategory.imported_for(course).should be_protected
course.group_categories.create(:name => 'Random Category').should_not be_protected
GroupCategory.communities_for(account).should be_protected
end
end

208
spec/models/group_spec.rb
View File

@ -34,10 +34,11 @@ describe Group do
@group.wiki.should eql(WikiNamespace.default_for_context(@group).wiki)
end
it "should not be public" do
it "should be private by default" do
@group.is_public.should be_false
end
context "#peer_groups" do
it "should find all peer groups" do
context = course_model
group_category = context.group_categories.create(:name => "worldCup")
@ -60,6 +61,7 @@ describe Group do
group2 = Group.create!(:name=>"group2", :group_category=>group_category, :context => context)
group1.peer_groups.should be_empty
end
end
context "atom" do
it "should have an atom name as it's own name" do
@ -73,7 +75,7 @@ describe Group do
end
end
context "enrollment" do
context "add_user" do
it "should be able to add a person to the group" do
user_model
pseudonym_model(:user_id => @user.id)
@ -93,7 +95,7 @@ describe Group do
@group.users.count.should == 1
end
it "adding a user should remove that user from peer groups" do
it "should remove that user from peer groups" do
context = course_model
group_category = context.group_categories.create!(:name => "worldCup")
group1 = Group.create!(:name=>"group1", :group_category=>group_category, :context => context)
@ -109,39 +111,28 @@ describe Group do
group1.users.should_not be_include(@user)
end
# it "should be able to add more than one person at a time" do
# user_model
# p1 = pseudonym_model(:user_id => @user.id)
# u1 = p1.user
# user_model
# p2 = pseudonym_model(:user_id => @user.id)
# u2 = p2.user
# @group.add_user([u1, u2])
# @group.users.should be_include(u1)
# @group.users.should be_include(u2)
# end
it "should be able to add a person as a user instead as a pseudonym" do
user_model
@group.add_user(@user)
@group.users.should be_include(@user)
it "should add a user at the right workflow_state by default" do
pending "waiting to turn off auto join functionality"
{
'invitation_only' => 'invited',
'parent_context_request' => 'requested',
'parent_context_auto_join' => 'accepted'
}.each do |join_level, workflow_state|
usr = user_model
grp = group_model(:join_level => join_level)
grp.add_user(usr)
grp.group_memberships.scoped(:conditions => { :workflow_state => workflow_state }).map(&:user_id).should be_include usr.id
end
end
it "should be able to add a person with a user id" do
user_model
@group.add_user(@user)
@group.users.should be_include(@user)
it "should allow specifying a workflow_state" do
pending "waiting to turn off auto join functionality"
[ 'invited', 'requested', 'accepted' ].each do |workflow_state|
usr = user_model
@group.add_user(usr, workflow_state)
@group.group_memberships.scoped(:conditions => { :workflow_state => workflow_state }).map(&:user_id).should be_include usr.id
end
end
# it "should be able to add a person from their communication channel" do
# user_model
# communication_channel_model
# @group.users.should_not be_include(@user)
# @cc.user.should eql(@user)
# @group.add_user(@user)
# @group.users.should be_include(@user)
# end
end
it "should grant manage permissions for associated objects to group managers" do
@ -204,75 +195,130 @@ describe Group do
context "auto_accept?" do
it "should be false unless join level is 'parent_context_auto_join'" do
course_with_teacher
student = user_model
@course.enroll_student(student)
@course.reload
course_with_student
group_category = GroupCategory.student_organized_for(@course)
group = @course.groups.create(:group_category => group_category)
group.auto_accept?(student).should be_false
group1 = @course.groups.create(:group_category => group_category, :join_level => 'parent_context_auto_join')
group2 = @course.groups.create(:group_category => group_category, :join_level => 'parent_context_request')
group3 = @course.groups.create(:group_category => group_category, :join_level => 'invitation_only')
[group1, group2, group3].map{|g| g.auto_accept?(@student)}.should == [true, false, false]
end
it "should be false unless the group is student organized" do
course_with_teacher
student = user_model
@course.enroll_student(student)
@course.reload
it "should be false unless the group is student organized or a community" do
course_with_student
@account = @course.root_account
group_category = @course.group_categories.create(:name => "random category")
group = @course.groups.create(:group_category => group_category, :join_level => 'parent_context_auto_join')
group.auto_accept?(student).should be_false
end
it "should be true otherwise" do
course_with_teacher
student = user_model
@course.enroll_student(student)
@course.reload
group_category = GroupCategory.student_organized_for(@course)
group = @course.groups.create(:group_category => group_category, :join_level => 'parent_context_auto_join')
group.auto_accept?(student).should be_true
jl = 'parent_context_auto_join'
group1 = @course.groups.create(:group_category => @course.group_categories.create(:name => "random category"), :join_level => jl)
group2 = @course.groups.create(:group_category => GroupCategory.student_organized_for(@course), :join_level => jl)
group3 = @account.groups.create(:group_category => GroupCategory.communities_for(@account), :join_level => jl)
[group1, group2, group3].map{|g| g.auto_accept?(@student)}.should == [false, true, true]
end
end
context "allow_join_request?" do
it "should be false unless join level is 'parent_context_auto_join' or 'parent_context_request'" do
course_with_teacher
student = user_model
@course.enroll_student(student)
@course.reload
course_with_student
group_category = GroupCategory.student_organized_for(@course)
group = @course.groups.create(:group_category => group_category)
group.allow_join_request?(student).should be_false
group1 = @course.groups.create(:group_category => group_category, :join_level => 'parent_context_auto_join')
group2 = @course.groups.create(:group_category => group_category, :join_level => 'parent_context_request')
group3 = @course.groups.create(:group_category => group_category, :join_level => 'invitation_only')
[group1, group2, group3].map{|g| g.allow_join_request?(@student)}.should == [true, true, false]
end
it "should be false unless the group is student organized" do
course_with_teacher
student = user_model
@course.enroll_student(student)
@course.reload
it "should be false unless the group is student organized or a community" do
course_with_student
@account = @course.root_account
group_category = @course.group_categories.create(:name => "random category")
group = @course.groups.create(:group_category => group_category, :join_level => 'parent_context_auto_join')
group.allow_join_request?(student).should be_false
jl = 'parent_context_auto_join'
group1 = @course.groups.create(:group_category => @course.group_categories.create(:name => "random category"), :join_level => jl)
group2 = @course.groups.create(:group_category => GroupCategory.student_organized_for(@course), :join_level => jl)
group3 = @account.groups.create(:group_category => GroupCategory.communities_for(@account), :join_level => jl)
[group1, group2, group3].map{|g| g.allow_join_request?(@student)}.should == [false, true, true]
end
end
it "should be true otherwise" do
course_with_teacher
student = user_model
@course.enroll_student(student)
@course.reload
context "allow_self_signup?" do
it "should follow the group category self signup option" do
course_with_student
group_category = GroupCategory.student_organized_for(@course)
group_category.configure_self_signup(true, false)
group_category.save!
group1 = @course.groups.create(:group_category => group_category)
group1.allow_self_signup?(@student).should be_true
group = @course.groups.create(:group_category => group_category, :join_level => 'parent_context_auto_join')
group.allow_join_request?(student).should be_true
group_category.configure_self_signup(true, true)
group_category.save!
group2 = @course.groups.create(:group_category => group_category)
group2.allow_self_signup?(@student).should be_true
group = @course.groups.create(:group_category => group_category, :join_level => 'parent_context_request')
group.allow_join_request?(student).should be_true
group_category.configure_self_signup(false, false)
group_category.save!
group3 = @course.groups.create(:group_category => group_category)
group3.allow_self_signup?(@student).should be_false
end
it "should correctly handle restricted course sections" do
course_with_student
@other_section = @course.course_sections.create!(:name => "Other Section")
@other_student = @course.enroll_student(user_model, {:section => @other_section}).user
group_category = GroupCategory.student_organized_for(@course)
group_category.configure_self_signup(true, true)
group_category.save!
group1 = @course.groups.create(:group_category => group_category)
group1.allow_self_signup?(@student).should be_true
group1.add_user(@student)
group1.reload
group1.allow_self_signup?(@other_student).should be_false
end
end
context "has_member?" do
it "should be true for accepted memberships, regardless of moderator flag" do
@user1 = user_model
@user2 = user_model
@user3 = user_model
@user4 = user_model
@user5 = user_model
@group.add_user(@user1, 'accepted')
@group.add_user(@user2, 'accepted')
@group.add_user(@user3, 'invited')
@group.add_user(@user4, 'requested')
@group.add_user(@user5, 'rejected')
GroupMembership.update_all({:moderator => true}, {:group_id => @group.id, :user_id => @user2.id})
@group.has_member?(@user1).should be_true
@group.has_member?(@user2).should be_true
@group.has_member?(@user3).should be_true # false when we turn auto_join off
@group.has_member?(@user4).should be_true # false when we turn auto_join off
@group.has_member?(@user5).should be_false
end
end
context "has_moderator?" do
it "should be true for accepted memberships, with moderator flag" do
@user1 = user_model
@user2 = user_model
@user3 = user_model
@user4 = user_model
@user5 = user_model
@group.add_user(@user1, 'accepted')
@group.add_user(@user2, 'accepted')
@group.add_user(@user3, 'invited')
@group.add_user(@user4, 'requested')
@group.add_user(@user5, 'rejected')
GroupMembership.update_all({:moderator => true}, {:group_id => @group.id, :user_id => [@user2.id, @user3.id, @user4.id, @user5.id]})
@group.has_moderator?(@user1).should be_false
@group.has_moderator?(@user2).should be_true
@group.has_moderator?(@user3).should be_true # false when we turn auto_join off
@group.has_moderator?(@user4).should be_true # false when we turn auto_join off
@group.has_moderator?(@user5).should be_false
end
end