allow admins to message anyone in their accounts

Change-Id: I47586c69662619b5f73ab43c5873732c7edcf2a1
Reviewed-on: https://gerrit.instructure.com/4885
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Hudson <hudson@instructure.com>

app/models/user.rb

@@ -1505,6 +1505,7 @@ class User < ActiveRecord::Base
   def messageable_users(options = {})
     course_ids = courses.map(&:id) + concluded_courses.map(&:id)
     group_ids = groups.map(&:id)
+    account_ids = []
 
     if options[:context]
       limited_course_ids = []
@@ -1518,8 +1519,16 @@ class User < ActiveRecord::Base
       end
       course_ids &= limited_course_ids
       group_ids &= limited_group_ids
+    else
+      # if we're not searching with context(s) in mind, include any users we
+      # have admin access to know about
+      account_ids = associated_accounts.select{ |a| a.grants_right?(self, nil, :read_roster) }.map(&:id)
+      account_ids &= options[:account_ids] if options[:account_ids]
     end
-    return [] if course_ids.empty? && group_ids.empty? && !options[:ids] || options[:ids] == []
+
+    # if :ids is present but empty (different than just not present), don't
+    # bother doing a query that's guaranteed to return no results.
+    return [] if options[:ids] && options[:ids].empty?
 
     user_condition_sql = "users.id <> #{self.id}"
     user_condition_sql << " AND users.id IN (#{options[:ids].map(&:to_i).join(', ')})" if options[:ids].present?
@@ -1549,6 +1558,14 @@ class User < ActiveRecord::Base
         AND #{user_condition_sql}
     SQL
 
+    user_sql << <<-SQL if account_ids.present?
+      SELECT #{MESSAGEABLE_USER_COLUMN_SQL}, null AS course_id, null AS group_id
+      FROM users, user_account_associations
+      WHERE user_account_associations.account_id IN (#{account_ids.join(',')})
+        AND user_account_associations.user_id = users.id
+        AND #{user_condition_sql}
+    SQL
+
     if options[:ids]
       # provides a way for this user to start a conversation with someone
       # that isn't normally messageable (requires that they already be in a
@@ -1570,6 +1587,9 @@ class User < ActiveRecord::Base
       end
     end
 
+    # if none of our potential sources was included, we're done
+    return [] if user_sql.empty?
+
     users = User.find_by_sql(<<-SQL)
       SELECT #{MESSAGEABLE_USER_COLUMN_SQL},
         #{connection.func(:group_concat, :course_id)} AS common_course_ids,

spec/controllers/conversations_controller_spec.rb

@@ -47,6 +47,17 @@ describe ConversationsController do
       response.should be_success
       assigns[:conversations].map{|c|c[:id]}.should == @user.conversations.map(&:conversation_id)
     end
+
+    it "should work for an admin as well" do
+      course
+      account_admin_user
+      user_session(@user)
+      conversation
+
+      get 'index'
+      response.should be_success
+      assigns[:conversations].map{|c|c[:id]}.should == @user.conversations.map(&:conversation_id)
+    end
   end
 
   describe "GET 'show'" do

spec/factories/user_factory.rb

@@ -20,6 +20,10 @@ def user_model(opts={})
   @user = factory_with_protected_attributes(User, valid_user_attributes.merge(opts))
 end
 
+def tie_user_to_account(user, opts={})
+  user.account_users.create(:account => opts[:account] || Account.default, :membership_type => opts[:membership_type] || 'AccountAdmin')
+end
+
 def valid_user_attributes
   {
     :name => 'value for name',

spec/models/user_spec.rb

@@ -374,4 +374,37 @@ describe User do
       @site_admin.grants_right?(user, nil, :become_user).should be_false
     end
   end
+
+  context "messageable_users" do
+    before(:each) do
+      @admin = user_model
+      @student = user_model
+      tie_user_to_account(@admin, :membership_type => 'AccountAdmin')
+      tie_user_to_account(@student, :membership_type => 'Student')
+    end
+
+    it "should include users with no shared contexts iff admin" do
+      @admin.messageable_users(:ids => [@user.id]).should_not be_empty
+      @user.messageable_users(:ids => [@admin.id]).should be_empty
+    end
+
+    it "should not do admin catch-all if specific contexts requested" do
+      course1 = course_model
+      course2 = course_model
+      course2.offer!
+
+      enrollment = course2.enroll_teacher(@admin)
+      enrollment.workflow_state = 'active'
+      enrollment.save
+      @admin.reload
+
+      enrollment = course2.enroll_student(@student)
+      enrollment.workflow_state = 'active'
+      enrollment.save
+
+      @admin.messageable_users(:context => ["course_#{course1.id}"], :ids => [@student.id]).should be_empty
+      @admin.messageable_users(:context => ["course_#{course2.id}"], :ids => [@student.id]).should_not be_empty
+      @student.messageable_users(:context => ["course_#{course2.id}"], :ids => [@admin.id]).should_not be_empty
+    end
+  end
 end