remove all_courses call in users set_policy

fixes: CNVS-3669

Changes the given blocks to look for the sought right in the
current courses first and if its not found look through the
concluded courses.  The current courses should catch the majority
of calls and fall back to the concluded courses if required.

* Note:  This will not check in other course states then completed and
         current.

Test Plan:

- Create a course with a student and admin/teacher.
- Enable user notes for the course.
- Check to make sure the teacher can read the students user notes.
  - Make sure the user notes button on the students profile is visible.
- Conclude the course.
- Ensure the teacher can still read the students user notes and the
  button is still visible.

- As an admin do the same as above but for manage user notes.

- Make sure the teacher/admin can run reports on the student for a
  concluded and current course.

Change-Id: I5eade42253c24c6ecad5e1d654695662ebb3afde
Reviewed-on: https://gerrit.instructure.com/36770
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Nick Cloward <ncloward@instructure.com>

app/models/user.rb

@@ -943,6 +943,16 @@ class User < ActiveRecord::Base
     result
   end
 
+  def check_courses_right?(user, sought_right)
+    # Look through the currently enrolled courses first.  This should
+    # catch most of the calls.  If none of the current courses grant
+    # the right then look at the concluded courses.
+    user && sought_right && (
+      self.courses.any?{ |c| c.grants_right?(user, sought_right) } ||
+      self.concluded_courses.any?{ |c| c.grants_right?(user, sought_right) }
+    )
+  end
+
   set_policy do
     given { |user| user == self }
     can :read and can :manage and can :manage_content and can :manage_files and can :manage_calendar and can :send_messages and can :update_avatar and can :manage_feature_flags
@@ -953,22 +963,16 @@ class User < ActiveRecord::Base
     given {|user| self.courses.any?{|c| c.user_is_instructor?(user)}}
     can :rename and can :create_user_notes and can :read_user_notes
 
-    given do |user|
-      user && (
-        # by default this means that the user we are given is an administrator
-        # of an account of one of the courses that this user is enrolled in, or
-        # an admin (teacher/ta/designer) in the course
-        self.all_courses.any? { |c| c.grants_right?(user, :read_reports) }
-      )
-    end
+    # by default this means that the user we are given is an administrator
+    # of an account of one of the courses that this user is enrolled in, or
+    # an admin (teacher/ta/designer) in the course
+    given { |user| self.check_courses_right?(user, :read_reports) }
     can :rename and can :remove_avatar and can :read_reports
 
-    given do |user|
-      user && self.all_courses.any? { |c| c.grants_right?(user, :manage_user_notes) }
-    end
+    given { |user| self.check_courses_right?(user, :manage_user_notes) }
     can :create_user_notes and can :read_user_notes
 
-    given { |user| user && self.all_courses.any? { |c| c.grants_right?(user, :read_user_notes) } }
+    given { |user| self.check_courses_right?(user, :read_user_notes) }
     can :read_user_notes
 
     given do |user|

spec/models/conversation_message_spec.rb

@@ -134,8 +134,8 @@ describe ConversationMessage do
   context "generate_user_note" do
     it "should add a user note under nominal circumstances" do
       Account.default.update_attribute :enable_user_notes, true
-      course_with_teacher
-      student = student_in_course.user
+      course_with_teacher(active_all: true)
+      student = student_in_course(active_all: true).user
       conversation = @teacher.initiate_conversation([student])
       conversation.add_message("reprimanded!", :generate_user_note => true)
       student.user_notes.size.should be(1)
@@ -147,8 +147,8 @@ describe ConversationMessage do
 
     it "should fail if notes are disabled on the account" do
       Account.default.update_attribute :enable_user_notes, false
-      course_with_teacher
-      student = student_in_course.user
+      course_with_teacher(active_all: true)
+      student = student_in_course(active_all: true).user
       conversation = @teacher.initiate_conversation([student])
       conversation.add_message("reprimanded!", :generate_user_note => true)
       student.user_notes.size.should be(0)
@@ -156,9 +156,9 @@ describe ConversationMessage do
 
     it "should allow user notes on more than one recipient" do
       Account.default.update_attribute :enable_user_notes, true
-      course_with_teacher
-      student1 = student_in_course.user
-      student2 = student_in_course.user
+      course_with_teacher(active_all: true)
+      student1 = student_in_course(active_all: true).user
+      student2 = student_in_course(active_all: true).user
       conversation = @teacher.initiate_conversation([student1, student2])
       conversation.add_message("reprimanded!", :generate_user_note => true)
       student1.user_notes.size.should be(1)

spec/models/user_spec.rb

@@ -699,6 +699,31 @@ describe User do
     end
   end
 
+  context "check_courses_right?" do
+    before do
+      course_with_teacher(:active_all => true)
+      @student = user_model
+      @course.stubs(:grants_right?).returns(true)
+    end
+
+    it "should require parameters" do
+      @student.check_courses_right?(nil, :some_right).should be_false
+      @student.check_courses_right?(@teacher, nil).should be_false
+    end
+
+    it "should check current courses" do
+      @student.expects(:courses).once.returns([@course])
+      @student.expects(:concluded_courses).never
+      @student.check_courses_right?(@teacher, :some_right).should be_true
+    end
+
+    it "should check concluded courses" do
+      @student.expects(:courses).once.returns([])
+      @student.expects(:concluded_courses).once.returns([@course])
+      @student.check_courses_right?(@teacher, :some_right).should be_true
+    end
+  end
+
   context "search_messageable_users" do
     before(:each) do
       @admin = user_model