From 127b9b33028bbcddf0435a694288f20cf46e0b60 Mon Sep 17 00:00:00 2001
From: Simon Williams <simon@instructure.com>
Date: Tue, 15 May 2012 15:50:43 -0600
Subject: [PATCH] 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>
---
 app/controllers/groups_controller.rb          |   6 +-
 app/models/group.rb                           | 142 +++++-----
 app/models/group_category.rb                  |  21 +-
 app/models/group_membership.rb                |   4 +-
 ...add_moderator_flag_to_group_memberships.rb |  11 +
 spec/models/group_category_spec.rb            |  50 ++++
 spec/models/group_spec.rb                     | 254 +++++++++++-------
 7 files changed, 315 insertions(+), 173 deletions(-)
 create mode 100644 db/migrate/20120516185217_add_moderator_flag_to_group_memberships.rb

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 089d6a94108..b3a384cccca 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -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
diff --git a/app/models/group.rb b/app/models/group.rb
index d666b507643..b39b5c08a02 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -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.join_level == 'parent_context_auto_join'
+    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? && 
-      ['parent_context_auto_join', 'parent_context_request'].include?(self.join_level)
+    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)))
+    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)
-    end
+    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
-      can :manage_calendar and
-      can :update and can :delete and can :create and
-      can :manage_wiki
+    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 :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)) }
-      can :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
diff --git a/app/models/group_category.rb b/app/models/group_category.rb
index 1444e0dd970..9c962a6076f 100644
--- a/app/models/group_category.rb
+++ b/app/models/group_category.rb
@@ -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'.
diff --git a/app/models/group_membership.rb b/app/models/group_membership.rb
index b0f72824044..98e5c2049c2 100644
--- a/app/models/group_membership.rb
+++ b/app/models/group_membership.rb
@@ -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
diff --git a/db/migrate/20120516185217_add_moderator_flag_to_group_memberships.rb b/db/migrate/20120516185217_add_moderator_flag_to_group_memberships.rb
new file mode 100644
index 00000000000..041dae91081
--- /dev/null
+++ b/db/migrate/20120516185217_add_moderator_flag_to_group_memberships.rb
@@ -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
diff --git a/spec/models/group_category_spec.rb b/spec/models/group_category_spec.rb
index a6b4d27b076..d51f25469af 100644
--- a/spec/models/group_category_spec.rb
+++ b/spec/models/group_category_spec.rb
@@ -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
 
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 0374d6cf08e..ed1a22baba1 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -34,31 +34,33 @@ 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
   
-  it "should find all peer groups" do
-    context = course_model
-    group_category = context.group_categories.create(:name => "worldCup")
-    other_category = context.group_categories.create(:name => "other category")
-    group1 = Group.create!(:name=>"group1", :group_category => group_category, :context => context)
-    group2 = Group.create!(:name=>"group2", :group_category => group_category, :context => context)
-    group3 = Group.create!(:name=>"group3", :group_category => group_category, :context => context)
-    group4 = Group.create!(:name=>"group4", :group_category => other_category, :context => context)
-    group1.peer_groups.length.should == 2
-    group1.peer_groups.should be_include(group2)
-    group1.peer_groups.should be_include(group3)
-    group1.peer_groups.should_not be_include(group1)
-    group1.peer_groups.should_not be_include(group4)
-  end
-  
-  it "should not find peer groups for student organized groups" do
-    context = course_model
-    group_category = GroupCategory.student_organized_for(context)
-    group1 = Group.create!(:name=>"group1", :group_category=>group_category, :context => context)
-    group2 = Group.create!(:name=>"group2", :group_category=>group_category, :context => context)
-    group1.peer_groups.should be_empty
+  context "#peer_groups" do
+    it "should find all peer groups" do
+      context = course_model
+      group_category = context.group_categories.create(:name => "worldCup")
+      other_category = context.group_categories.create(:name => "other category")
+      group1 = Group.create!(:name=>"group1", :group_category => group_category, :context => context)
+      group2 = Group.create!(:name=>"group2", :group_category => group_category, :context => context)
+      group3 = Group.create!(:name=>"group3", :group_category => group_category, :context => context)
+      group4 = Group.create!(:name=>"group4", :group_category => other_category, :context => context)
+      group1.peer_groups.length.should == 2
+      group1.peer_groups.should be_include(group2)
+      group1.peer_groups.should be_include(group3)
+      group1.peer_groups.should_not be_include(group1)
+      group1.peer_groups.should_not be_include(group4)
+    end
+    
+    it "should not find peer groups for student organized groups" do
+      context = course_model
+      group_category = GroupCategory.student_organized_for(context)
+      group1 = Group.create!(:name=>"group1", :group_category=>group_category, :context => context)
+      group2 = Group.create!(:name=>"group2", :group_category=>group_category, :context => context)
+      group1.peer_groups.should be_empty
+    end
   end
   
   context "atom" 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)
@@ -108,40 +110,29 @@ describe Group do
       group1.reload
       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