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

Exclude outcome results from muted asgmts/quizzes 

closes OUT-2304

performance:
Indices are used throughout the scoped query.

Shard.current.id => 1773

base query:
LearningOutcomeResult.active.where(context_code:'course_1079845',user_id:3306819,learning_outcome_id:1397026)

without scope:
----------------------------------------------------------------------------------------------------------------------------------
 Nested Loop  (cost=4.86..8.20 rows=1 width=268)
   ->  Bitmap Heap Scan on learning_outcome_results  (cost=4.42..5.54 rows=1 width=268)
         Recheck Cond: ((user_id = 3306819) AND (learning_outcome_id = 1397026))
         Filter: ((context_code)::text = 'course_1079845'::text)
         ->  BitmapAnd  (cost=4.42..4.42 rows=1 width=0)
               ->  Bitmap Index Scan on index_learning_outcome_results_association  (cost=0.00..1.73 rows=27 width=0)
                     Index Cond: (user_id = 3306819)
               ->  Bitmap Index Scan on index_learning_outcome_results_on_learning_outcome_id  (cost=0.00..2.44 rows=123 width=0)
                     Index Cond: (learning_outcome_id = 1397026)
   ->  Index Scan using content_tags_pkey on content_tags  (cost=0.43..2.66 rows=1 width=8)
         Index Cond: (id = learning_outcome_results.content_tag_id)
         Filter: ((workflow_state)::text <> 'deleted'::text)

with scope (`exclude_muted_associations`):
----------------------------------------------------------------------------------------------------------------------------------------------------------
 Nested Loop Left Join  (cost=6.99..20.54 rows=1 width=268)
   Join Filter: ((learning_outcome_results.association_type)::text = 'Assignment'::text)
   Filter: (((ra.muted IS NULL) AND (qa.muted IS NULL) AND (sa.muted IS NULL)) OR (ra.muted IS FALSE) OR (qa.muted IS FALSE) OR (sa.muted IS FALSE))
   ->  Nested Loop Left Join  (cost=6.56..17.88 rows=1 width=270)
         ->  Nested Loop Left Join  (cost=6.13..15.72 rows=1 width=277)
               Join Filter: ((learning_outcome_results.association_type)::text = 'Quizzes::Quiz'::text)
               ->  Nested Loop Left Join  (cost=5.71..13.06 rows=1 width=269)
                     Join Filter: (((rassoc.association_type)::text = 'Assignment'::text) AND ((rassoc.purpose)::text = 'grading'::text))
                     ->  Nested Loop Left Join  (cost=5.28..10.85 rows=1 width=293)
                           Join Filter: ((learning_outcome_results.association_type)::text = 'RubricAssociation'::text)
                           ->  Nested Loop  (cost=4.86..8.20 rows=1 width=268)
                                 ->  Bitmap Heap Scan on learning_outcome_results  (cost=4.42..5.54 rows=1 width=268)
                                       Recheck Cond: ((user_id = 3306819) AND (learning_outcome_id = 1397026))
                                       Filter: ((context_code)::text = 'course_1079845'::text)
                                       ->  BitmapAnd  (cost=4.42..4.42 rows=1 width=0)
                                             ->  Bitmap Index Scan on index_learning_outcome_results_association  (cost=0.00..1.73 rows=27 width=0)
                                                   Index Cond: (user_id = 3306819)
                                             ->  Bitmap Index Scan on index_learning_outcome_results_on_learning_outcome_id  (cost=0.00..2.44 rows=123 width=0)
                                                   Index Cond: (learning_outcome_id = 1397026)
                                 ->  Index Scan using content_tags_pkey on content_tags  (cost=0.43..2.66 rows=1 width=8)
                                       Index Cond: (id = learning_outcome_results.content_tag_id)
                                       Filter: ((workflow_state)::text <> 'deleted'::text)
                           ->  Index Scan using rubric_associations_pkey on rubric_associations rassoc  (cost=0.42..2.64 rows=1 width=33)
                                 Index Cond: (id = learning_outcome_results.association_id)
                     ->  Index Scan using assignments_pkey on assignments ra  (cost=0.43..2.19 rows=1 width=9)
                           Index Cond: (id = rassoc.association_id)
               ->  Index Scan using quizzes_pkey on quizzes  (cost=0.43..2.65 rows=1 width=16)
                     Index Cond: (id = learning_outcome_results.association_id)
         ->  Index Scan using assignments_pkey on assignments qa  (cost=0.43..2.15 rows=1 width=9)
               Index Cond: (id = quizzes.assignment_id)
   ->  Index Scan using assignments_pkey on assignments sa  (cost=0.43..2.65 rows=1 width=9)
         Index Cond: (id = learning_outcome_results.association_id)

test plan:
  - create two course-level outcomes
  - create an assignment with a single question, and align the 1st outcome via a rubric
  - create a quiz bank with a single auto-gradeable question (e.g. true/false), and
    align the 2nd outcome to it
  - create a quiz that pulls the single question from the quiz bank above
  - as a student, submit to the assignment and the quiz
  - as a teacher, assess the assignment, providing a score to the rubric
  - as a teacher, confirm:
    * both outcomes have results in the LMGB
    * both outcomes have results in the sLMGB
  - as a student, confirm:
    * both outcomes have results in the sLMGB
  - as a teacher, mute the assignment in the gradebook:
    https://community.canvaslms.com/docs/DOC-12961-4152724339
  - as a teacher, confirm:
    * both outcomes have results in the LMGB
    * both outcomes have results in the sLMGB
  - as a student, confirm:
    * only the outcome associated with the quiz bank has results in the sLMGB
  - as a teacher, mute the quiz in the gradebook
  - as a teacher, confirm:
    * both outcomes have results in the LMGB
    * both outcomes have results in the sLMGB
  - as a student, confirm:
    * no outcomes should have results in the sLMGB
  - as a teacher, unmute the assignment in the gradebook
  - as a teacher, confirm:
    * both outcomes have results in the LMGB
    * both outcomes have results in the sLMGB
  - as a student, confirm:
    * only the outcome associated with the assignment has results in the sLMGB

Change-Id: I0ea05eedd29383501cc9306bcedcfa67aee4cd67
Reviewed-on: https://gerrit.instructure.com/155210
Tested-by: Jenkins
Reviewed-by: Neil Gupta <ngupta@instructure.com>
Product-Review: Neil Gupta <ngupta@instructure.com>
QA-Review: Neil Gupta <ngupta@instructure.com>
This commit is contained in:
Augusto Callejas 2018-06-21 12:52:49 -10:00
parent b17ef3ae7f
commit c47fb7a26f
6 changed files with 168 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
app/controllers/outcome_results_controller.rb
View File

@ -228,10 +228,7 @@ class OutcomeResultsController < ApplicationController
# outcome_results: [OutcomeResult]
# }
def index
@results = find_outcome_results(
users: @users,
context: @context,
outcomes: @outcomes,
@results = find_results(
include_hidden: value_to_boolean(params[:include_hidden])
)
@results = Api.paginate(@results, self, api_v1_course_outcome_results_url)
@ -318,8 +315,12 @@ class OutcomeResultsController < ApplicationController
private
def user_rollups(opts = {})
@results = find_outcome_results(users: @users, context: @context, outcomes: @outcomes).preload(:user)
def find_results(opts = {})
find_outcome_results(@current_user, users: @users, context: @context, outcomes: @outcomes, **opts)
end
def user_rollups(_opts = {})
@results = find_results.preload(:user)
outcome_results_rollups(@results, @users)
end
@ -333,7 +334,7 @@ class OutcomeResultsController < ApplicationController
def aggregate_rollups_json
# calculating averages for all users in the context and only returning one
# rollup, so don't paginate users in this method.
@results = find_outcome_results(users: @users, context: @context, outcomes: @outcomes)
@results = find_results
aggregate_rollups = [aggregate_outcome_results_rollup(@results, @context, params[:aggregate_stat])]
json = aggregate_outcome_results_rollups_json(aggregate_rollups)
# no pagination, so no meta field

10
app/models/learning_outcome_result.rb
View File

@ -123,6 +123,16 @@ class LearningOutcomeResult < ActiveRecord::Base
scope :for_association, lambda { |association| where(:association_type => association.class.to_s, :association_id => association.id) }
scope :for_associated_asset, lambda { |associated_asset| where(:associated_asset_type => associated_asset.class.to_s, :associated_asset_id => associated_asset.id) }
scope :active, lambda { where("content_tags.workflow_state <> 'deleted'").joins(:alignment) }
# rubocop:disable Metrics/LineLength
scope :exclude_muted_associations, -> {
joins("LEFT JOIN #{RubricAssociation.quoted_table_name} rassoc ON rassoc.id = learning_outcome_results.association_id AND learning_outcome_results.association_type = 'RubricAssociation'").
joins("LEFT JOIN #{Assignment.quoted_table_name} ra ON ra.id = rassoc.association_id AND rassoc.association_type = 'Assignment' AND rassoc.purpose = 'grading'").
joins("LEFT JOIN #{Quizzes::Quiz.quoted_table_name} ON quizzes.id = learning_outcome_results.association_id AND learning_outcome_results.association_type = 'Quizzes::Quiz'").
joins("LEFT JOIN #{Assignment.quoted_table_name} qa ON qa.id = quizzes.assignment_id").
joins("LEFT JOIN #{Assignment.quoted_table_name} sa ON sa.id = learning_outcome_results.association_id AND learning_outcome_results.association_type = 'Assignment'").
where('(ra.muted IS NULL AND qa.muted IS NULL AND sa.muted IS NULL) OR ra.muted IS FALSE OR qa.muted IS FALSE OR sa.muted IS FALSE')
}
# rubocop:enable Metrics/LineLength
private

6
lib/outcomes/result_analytics.rb
View File

@ -24,6 +24,7 @@ module Outcomes
# Public: Queries learning_outcome_results for rollup.
#
# user - User requesting results.
# opts - The options for the query. In a later version of ruby, these would
# be named parameters.
# :users - The users to lookup results for (required)
@ -31,7 +32,7 @@ module Outcomes
# :outcomes - The outcomes to lookup results for (required)
#
# Returns a relation of the results, suitably ordered.
def find_outcome_results(opts)
def find_outcome_results(user, opts)
required_opts = [:users, :context, :outcomes]
required_opts.each { |p| raise "#{p} option is required" unless opts[p] }
users, context, outcomes = opts.values_at(*required_opts)
@ -40,6 +41,9 @@ module Outcomes
user_id: users.map(&:id),
learning_outcome_id: outcomes.map(&:id),
)
unless context.grants_any_right?(user, :manage_grades, :view_all_grades)
results = results.exclude_muted_associations
end
unless opts[:include_hidden]
results = results.where(hidden: false)
end

49
spec/controllers/outcome_results_controller_spec.rb
View File

@ -59,6 +59,25 @@ describe OutcomeResultsController do
create_outcome_rubric_association
end
let_once(:outcome_result) do
rubric_association = outcome_rubric.associate_with(outcome_assignment, outcome_course, purpose: 'grading')
LearningOutcomeResult.new(
user_id: @student.id,
alignment: ContentTag.create!({
title: 'content',
context: outcome_course,
learning_outcome: @outcome,
content_type: 'Assignment',
content_id: outcome_assignment.id
})
).tap do |lor|
lor.association_object = rubric_association
lor.context = outcome_course
lor.save!
end
end
let(:outcome_criterion) do
find_outcome_criterion
end
@ -151,5 +170,35 @@ describe OutcomeResultsController do
format: "json"
expect(response).not_to be_success
end
context 'with muted assignment' do
before do
outcome_assignment.mute!
end
it 'teacher should see result' do
user_session(@teacher)
get 'index', params: {:context_id => @course.id,
:course_id => @course.id,
:context_type => "Course",
:user_ids => [@student.id],
:outcome_ids => [@outcome.id]},
format: "json"
json = JSON.parse(response.body.gsub("while(1);", ""))
expect(json['outcome_results'].length).to eq 1
end
it 'student should not see result' do
user_session(@student)
get 'index', params: {:context_id => @course.id,
:course_id => @course.id,
:context_type => "Course",
:user_ids => [@student.id],
:outcome_ids => [@outcome.id]},
format: "json"
json = JSON.parse(response.body.gsub("while(1);", ""))
expect(json['outcome_results'].length).to eq 0
end
end
end
end

34
spec/lib/outcomes/result_analytics_spec.rb
View File

@ -81,22 +81,42 @@ describe Outcomes::ResultAnalytics do
describe "#find_outcome_results" do
before(:once) do
course_with_student
outcome_with_rubric context: @course
assignment_model
alignment = @outcome.align(@assignment, @course)
LearningOutcomeResult.create! context: @course, learning_outcome: @outcome, user: @student, alignment: alignment
LearningOutcomeResult.create! context: @course, learning_outcome: @outcome, user: @student, alignment: alignment, hidden: true
course_with_teacher(course: @course)
rubric = outcome_with_rubric context: @course
@assignment = assignment_model
@alignment = @outcome.align(@assignment, @course)
@rubric_association = rubric.associate_with(@assignment, @course, purpose: 'grading')
lor
lor(hidden: true)
end
def lor(opts = {})
LearningOutcomeResult.create!(
context: @course,
learning_outcome: @outcome,
user: @student,
alignment: @alignment,
association_type: 'RubricAssociation',
association_id: @rubric_association.id,
**opts
)
end
it 'does not return hidden outcome results' do
results = ra.find_outcome_results(users: [@student], context: @course, outcomes: [@outcome])
results = ra.find_outcome_results(@teacher, users: [@student], context: @course, outcomes: [@outcome])
expect(results.length).to eq 1
end
it 'returns hidden outcome results when include_hidden is true' do
results = ra.find_outcome_results(users: [@student], context: @course, outcomes: [@outcome], include_hidden: true)
results = ra.find_outcome_results(@teacher, users: [@student], context: @course, outcomes: [@outcome], include_hidden: true)
expect(results.length).to eq 2
end
it 'does not return muted assignment results' do
@assignment.mute!
results = ra.find_outcome_results(@student, users: [@student], context: @course, outcomes: [@outcome])
expect(results.length).to eq 0
end
end
describe '#rollup_user_results' do

69
spec/models/learning_outcome_result_spec.rb
View File

@ -20,11 +20,15 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
describe LearningOutcomeResult do
let_once :learning_outcome_result do
create_and_associate_lor(quiz_model)
let_once :quiz do
quiz_model(assignment: assignment_model)
end
def create_and_associate_lor(association_object)
let_once :learning_outcome_result do
create_and_associate_lor(quiz)
end
def create_and_associate_lor(association_object, associated_asset = nil)
assignment_model
outcome = @course.created_learning_outcomes.create!(title: 'outcome')
@ -32,11 +36,12 @@ describe LearningOutcomeResult do
alignment: ContentTag.create!({
title: 'content',
context: @course,
learning_outcome: outcome})
learning_outcome: outcome
})
).tap do |lor|
lor.association_object = association_object
lor.context = @course
lor.associated_asset = association_object
lor.associated_asset = associated_asset || association_object
lor.save!
end
end
@ -59,7 +64,61 @@ describe LearningOutcomeResult do
})
expect(learning_outcome_result.submitted_or_assessed_at).to eq(@assessed_at)
end
end
describe 'exclude_muted_associations scope' do
shared_examples_for 'muting assignment' do
context 'unmuted assignment' do
it 'includes assignment result' do
total = LearningOutcomeResult.count
expect(LearningOutcomeResult.exclude_muted_associations.count).to eq total
end
end
context 'muted assignment' do
before do
assignment.mute!
end
it 'excludes assignment result' do
total = LearningOutcomeResult.count
expect(LearningOutcomeResult.exclude_muted_associations.count).to eq(total-1)
end
end
end
context 'with quiz assignment' do
let_once :assignment do
quiz.assignment
end
include_examples 'muting assignment'
end
context 'with assignment result' do
let_once :assignment do
assignment_model
end
let_once :assignment_learning_outcome_result do
create_and_associate_lor(assignment)
end
include_examples 'muting assignment'
end
context 'with rubric association result' do
let_once :assignment do
assignment_model
end
let_once :assignment_learning_outcome_result do
rubric_assessment_model(user: @user, context: @course, association_object: assignment, purpose: 'grading')
create_and_associate_lor(@rubric_association, assignment)
end
include_examples 'muting assignment'
end
end
describe "active scope" do