fix account level rubric assessments showing in wrong context

fixes OUT-1348

test plan:
- create an account level rubric
- create two courses with two different teacher users
- create a student in Course A
- import the rubric into both courses by attaching them to
  an assignment
- as the student in Course A, submit to the assignment
- grade the assignment in speedgrader under 'view rubric'
- as the teacher in Course A, hit the following endpoint:
  /api/v1/courses/{course_id}/rubrics/{rubric_id}?include=assessments
  (note: you can easily get the rubric id by hovering or inspecting the
  'edit' icon on the assignment's show page)
- as the teacher in Course A, the assessment should be returned in the
  api response
- log in as the teacher for Course B, and hit the same endpoint (just
  updating the course and rubric ids for Course B)
- only the rubric should be included in the api response, there should
  be no rubric assessments returned

Change-Id: I12121703dcf92d301c706e3d0e5d64cf315b1e63
Reviewed-on: https://gerrit.instructure.com/118915
Reviewed-by: Augusto Callejas <[email protected]>
QA-Review: Andrew Porter <[email protected]>
Product-Review: Michael Brewer-Davis <[email protected]>
Tested-by: Jenkins

app/controllers/rubrics_api_controller.rb

@@ -162,9 +162,10 @@ def index
   def show
     return unless authorized_action(@context, @current_user, :manage_rubrics)
     if !@context.errors.present?
-      assessments = get_rubric_assessment(params[:include])
+      assessments = rubric_assessments
       render json: rubric_json(@rubric, @current_user, session,
-                  assessments: assessments, style: params[:style])
+             assessments: assessments,
+             style: params[:style])
     else
       render json: @context.errors, status: :bad_request
     end
@@ -176,14 +177,19 @@ def find_rubric
     @rubric = Rubric.find(params[:id])
   end
 
-  def get_rubric_assessment(type)
-    case type
-      when 'assessments'
-        RubricAssessment.where(rubric_id: @rubric.id)
-      when 'graded_assessments'
-        RubricAssessment.where(rubric_id: @rubric.id, assessment_type: 'grading')
-      when 'peer_assessments'
-        RubricAssessment.where(rubric_id: @rubric.id, assessment_type: 'peer_review')
+  def rubric_assessments
+    scope = if @context.is_a? Course
+              RubricAssessment.for_course_context(@context.id).where(rubric_id: @rubric.id)
+            else
+              RubricAssessment.where(rubric_id: @rubric.id)
+            end
+    case params[:include]
+    when 'assessments'
+      scope
+    when 'graded_assessments'
+      scope.where(assessment_type: 'grading')
+    when 'peer_assessments'
+      scope.where(assessment_type: 'peer_review')
     end
   end
 

app/models/rubric_assessment.rb

@@ -210,6 +210,10 @@ def update_artifact
   scope :for_submissions, -> { where(:artifact_type => "Submission")}
   scope :for_provisional_grades, -> { where(:artifact_type => "ModeratedGrading::ProvisionalGrade")}
 
+  scope :for_course_context, lambda { |course_id|
+    joins(:rubric_association).where(rubric_associations: {context_id: course_id, context_type: "Course"})
+  }
+
   def methods_for_serialization(*methods)
     @serialization_methods = methods
   end

spec/apis/v1/rubrics_api_spec.rb

@@ -150,22 +150,22 @@ def paginate_call(context, type)
     expect(json.length).to eq 3
     links = response.headers['Link'].split(",")
     expect(links.all?{ |l| l =~ /api\/v1\/#{type}s\/#{context.id}\/rubrics/ }).to be_truthy
-    expect(links.find{ |l| l.match(/rel="next"/)}).to match /page=2/
-    expect(links.find{ |l| l.match(/rel="first"/)}).to match /page=1/
-    expect(links.find{ |l| l.match(/rel="last"/)}).to match /page=3/
+    expect(links.find{ |l| l.match(/rel="next"/)}).to match(/page=2/)
+    expect(links.find{ |l| l.match(/rel="first"/)}).to match(/page=1/)
+    expect(links.find{ |l| l.match(/rel="last"/)}).to match(/page=3/)
 
     # get the last page
     json = rubrics_api_call(context, {:per_page => '3', :page => '3'}, type)
 
     expect(json.length).to eq 2
     links = response.headers['Link'].split(",")
     expect(links.all?{ |l| l =~ /api\/v1\/#{type}s\/#{context.id}\/rubrics/ }).to be_truthy
-    expect(links.find{ |l| l.match(/rel="prev"/)}).to match /page=2/
-    expect(links.find{ |l| l.match(/rel="first"/)}).to match /page=1/
-    expect(links.find{ |l| l.match(/rel="last"/)}).to match /page=3/
+    expect(links.find{ |l| l.match(/rel="prev"/)}).to match(/page=2/)
+    expect(links.find{ |l| l.match(/rel="first"/)}).to match(/page=1/)
+    expect(links.find{ |l| l.match(/rel="last"/)}).to match(/page=3/)
   end
 
-  describe "in a course" do
+  describe "course level rubrics" do
     describe "index action" do
       before :once do
         course_with_teacher active_all: true
@@ -389,7 +389,7 @@ def paginate_call(context, type)
     end
   end
 
-  describe "in an account" do
+  describe "account level rubrics" do
     describe "index action" do
       before :once do
         @user = account_admin_user
@@ -455,6 +455,15 @@ def paginate_call(context, type)
           expect(response["assessments"].length).to eq 2
         end
 
+        it "returns only rubric assessments a user has permission to read" do
+          course_with_teacher active_all: true
+          assignment = assignment_model(context: @course)
+          ra_params = rubric_association_params_for_assignment(assignment)
+          RubricAssociation.generate(@teacher, @rubric, @course, ra_params)
+          response = rubric_api_call(@course, {include: "assessments"})
+          expect(response).not_to have_key "assessments"
+        end
+
         it "returns any rubric assessments used for grading when passed 'graded_assessments'" do
           response = rubric_api_call(@account, {include: "graded_assessments"}, 'account')
           expect(response["assessments"][0]["assessment_type"]).to eq "grading"