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

cache quiz_lti on submissions 

This caches/denormalizes the quiz_lti? information from assignments
onto submissions. This makes queries for missing/late Quizzes.Next
quizzes to be easier without having to join three tables. (Future
ticket coming to make this happen.)

closes GRADE-2231

test plan:
 - Have a course with a student and a Quizzes.Next assignment and
   another non-Q.N assignment. (You can fake the quizzes next assignment
   via the rails console)
 - On the rails console check the submission object for each
   assignment. Confirm that the quizzes.next assignment has
   cached_quiz_lti set to true and that the other assignment has it
   set to false
 - Create another assignment with on paper. Save.
 - Switch it to a Quizzes.Next assignment.
 - On the rails console check the submission object for each
   assignment. Confirm that the new quizzes.next assignment has
   cached_quiz_lti set to true
 - For the new assignment, in the rails console, find the ContentTag
   and .destroy it
-  On the rails console check the submission object for each
   assignment. Confirm that the new quizzes.next assignment has
   cached_quiz_lti set to false

Change-Id: I4bfc1d3a282863dde9de9658b335b9a24b2341e5
Reviewed-on: https://gerrit.instructure.com/197811
QA-Review: James Butters <jbutters@instructure.com>
Tested-by: Jenkins
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: James Williams <jamesw@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
This commit is contained in:
Keith T. Garner 2019-06-14 18:13:39 -05:00 committed by Keith Garner
parent 449938d236
commit 536a72f2f8
9 changed files with 250 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
app/models/assignment.rb
View File

@ -2889,10 +2889,7 @@ class Assignment < ActiveRecord::Base
end
def quiz_lti?
external_tool? &&
external_tool_tag.present? &&
external_tool_tag.content.present? &&
external_tool_tag.content.try(:tool_id) == 'Quizzes 2'
external_tool? && !!external_tool_tag&.content&.try(:quiz_lti?)
end
def quiz_lti!

8
app/models/content_tag.rb
View File

@ -54,6 +54,8 @@ class ContentTag < ActiveRecord::Base
after_save :update_could_be_locked
after_save :touch_context_module_after_transaction
after_save :touch_context_if_learning_outcome
after_save :run_due_date_cacher_for_quizzes_next
include CustomValidations
validates_as_url :url
@ -347,6 +349,8 @@ class ContentTag < ActiveRecord::Base
self.workflow_state = 'deleted'
self.save!
run_due_date_cacher_for_quizzes_next(force: true)
# after deleting the last native link to an unaligned outcome, delete the
# outcome. we do this here instead of in LearningOutcome#destroy because
# (a) LearningOutcome#destroy *should* only ever be called from here, and
@ -585,4 +589,8 @@ class ContentTag < ActiveRecord::Base
self.errors.add(:title, "cannot change title - associated content locked by Master Course")
end
end
def run_due_date_cacher_for_quizzes_next(force: false)
DueDateCacher.recompute(context) if content.try(:quiz_lti?) && (force || workflow_state != 'deleted')
end
end

5
app/models/context_external_tool.rb
View File

@ -46,6 +46,7 @@ class ContextExternalTool < ActiveRecord::Base
validate :check_for_xml_error
scope :disabled, -> { where(workflow_state: DISABLED_STATE) }
scope :quiz_lti, -> { where(tool_id: 'Quizzes 2') }
CUSTOM_EXTENSION_KEYS = {
:file_menu => [:accept_media_types].freeze,
@ -812,6 +813,10 @@ end
end
end
def quiz_lti?
tool_id == "Quizzes 2"
end
private
def self.context_id_for(asset, shard)

37
db/migrate/20190614230624_add_cached_quiz_lti_to_submissions.rb Normal file
View File

@ -0,0 +1,37 @@
#
# Copyright (C) 2019 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
class AddCachedQuizLtiToSubmissions < ActiveRecord::Migration[5.1]
tag :predeploy
disable_ddl_transaction!
def up
add_column :submissions, :cached_quiz_lti, :boolean
change_column_default(:submissions, :cached_quiz_lti, false)
DataFixup::BackfillNulls.run(Submission, :cached_quiz_lti, default_value: false)
Shard.current.database_server.unshackle do
connection = Submission.connection
connection.execute("VACUUM ANALYZE #{Submission.quoted_table_name}") if connection.open_transactions == 0
end
change_column_null(:submissions, :cached_quiz_lti, false)
end
def down
remove_column :submissions, :cached_quiz_lti
end
end

36
db/migrate/20190617172509_run_due_date_cacher_for_quiz_lti.rb Normal file
View File

@ -0,0 +1,36 @@
#
# Copyright (C) 2019 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
class RunDueDateCacherForQuizLti < ActiveRecord::Migration[5.1]
tag :postdeploy
disable_ddl_transaction!
def change
Course.find_ids_in_ranges(batch_size: 100_000) do |start_at, end_at|
DataFixup::RunDueDateCacherForQuizLTI.send_later_if_production_enqueue_args(
:run,
{
priority: Delayed::LOW_PRIORITY,
n_strand: ["DataFixup::RunDueDateCacherForQuizLti", Shard.current.database_server.id]
},
start_at,
end_at
)
end
end
end

37
lib/data_fixup/run_due_date_cacher_for_quiz_lti.rb Normal file
View File

@ -0,0 +1,37 @@
#
# Copyright (C) 2019 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
module DataFixup::RunDueDateCacherForQuizLTI
def self.run(start_at, end_at)
# The migration will have us at most a range of 100,000 items,
# we'll break it down to a thousand at a time here.
Course.find_ids_in_ranges(start_at: start_at, end_at: end_at) do |batch_start, batch_end|
courses_to_recompute =
Course.joins(assignments: :external_tool_tag).
joins("INNER JOIN #{ContextExternalTool.quoted_table_name} ON content_tags.content_type='ContextExternalTool' AND context_external_tools.id = content_tags.content_id").
with_enrollments.
not_completed.
where(id: batch_start..batch_end, workflow_state: :available).
merge(ContextExternalTool.quiz_lti).
distinct
courses_to_recompute.each { |c| DueDateCacher.recompute_course(c, run_immediately: true) }
end
end
end

24
lib/due_date_cacher.rb
View File

@ -149,6 +149,15 @@ class DueDateCacher
Set.new
end
# We only care about quiz LTIs, so we'll only snag those. In fact,
# we only care if the assignment *is* a quiz, LTI, so we'll just
# keep a set of those assignment ids.
@quiz_lti_assignments =
ContentTag.joins("INNER JOIN #{ContextExternalTool.quoted_table_name} ON content_tags.content_type='ContextExternalTool' AND context_external_tools.id = content_tags.content_id").
merge(ContextExternalTool.quiz_lti).
where(context_type: 'Assignment', context_id: @assignment_ids).
where.not(workflow_state: 'deleted').distinct.pluck(:context_id).to_set
@user_ids = Array(user_ids)
@update_grades = update_grades
@original_caller = original_caller
@ -174,6 +183,8 @@ class DueDateCacher
create_moderation_selections_for_assignment(assignment_id, student_due_dates.keys, @user_ids)
quiz_lti = @quiz_lti_assignments.include?(assignment_id)
students_without_priors.each do |student_id|
submission_info = student_due_dates[student_id]
due_date = submission_info[:due_at] ? "'#{submission_info[:due_at].iso8601}'::timestamptz" : 'NULL'
@ -182,7 +193,7 @@ class DueDateCacher
anonymous_id = Anonymity.generate_id(existing_ids: existing_anonymous_ids)
existing_anonymous_ids << anonymous_id
sql_ready_anonymous_id = Submission.connection.quote(anonymous_id)
values << [assignment_id, student_id, due_date, grading_period_id, sql_ready_anonymous_id]
values << [assignment_id, student_id, due_date, grading_period_id, sql_ready_anonymous_id, quiz_lti]
end
end
@ -236,14 +247,15 @@ class DueDateCacher
query = <<~SQL
INSERT INTO #{Submission.quoted_table_name}
(assignment_id, user_id, workflow_state, created_at, updated_at, context_code, process_attempts,
cached_due_date, grading_period_id, anonymous_id)
cached_due_date, grading_period_id, anonymous_id, cached_quiz_lti)
SELECT
assignments.id, vals.student_id, 'unsubmitted',
now() AT TIME ZONE 'UTC', now() AT TIME ZONE 'UTC',
assignments.context_code, 0, vals.due_date::timestamptz, vals.grading_period_id::integer,
vals.anonymous_id
vals.anonymous_id,
vals.quiz_lti
FROM (VALUES #{batch_values.join(',')})
AS vals(assignment_id, student_id, due_date, grading_period_id, anonymous_id)
AS vals(assignment_id, student_id, due_date, grading_period_id, anonymous_id, quiz_lti)
INNER JOIN #{Assignment.quoted_table_name} assignments
ON assignments.id = vals.assignment_id
LEFT OUTER JOIN #{Submission.quoted_table_name} submissions
@ -257,6 +269,7 @@ class DueDateCacher
#{INFER_SUBMISSION_WORKFLOW_STATE_SQL}
)),
anonymous_id = COALESCE(submissions.anonymous_id, excluded.anonymous_id),
cached_quiz_lti = excluded.cached_quiz_lti,
updated_at = excluded.updated_at
WHERE
(submissions.cached_due_date IS DISTINCT FROM excluded.cached_due_date) OR
@ -264,7 +277,8 @@ class DueDateCacher
(submissions.workflow_state <> COALESCE(NULLIF(submissions.workflow_state, 'deleted'),
(#{INFER_SUBMISSION_WORKFLOW_STATE_SQL})
)) OR
(submissions.anonymous_id IS DISTINCT FROM COALESCE(submissions.anonymous_id, excluded.anonymous_id))
(submissions.anonymous_id IS DISTINCT FROM COALESCE(submissions.anonymous_id, excluded.anonymous_id)) OR
(submissions.cached_quiz_lti IS DISTINCT FROM excluded.cached_quiz_lti)
SQL
Assignment.connection.execute(query)

25
spec/lib/due_date_cacher_spec.rb
View File

@ -953,6 +953,31 @@ describe DueDateCacher do
end
end
end
describe "cached_quiz_lti" do
let_once(:tool) do
@course.context_external_tools.create!(
name: 'Quizzes.Next',
consumer_key: 'test_key',
shared_secret: 'test_secret',
tool_id: 'Quizzes 2',
url: 'http://example.com/launch'
)
end
it "sets cached_quiz_lti to false if the assignment is not a Quizzes.Next assignment" do
cacher.recompute
expect(submission).to_not be_cached_quiz_lti
end
it "sets cached_quiz_lti to true if the assignment's external tool identifies itself as Quizzes 2" do
@assignment.update!(submission_types: "external_tool")
tool.content_tags.create!(context: @assignment)
cacher.recompute
expect(submission).to be_cached_quiz_lti
end
end
end
describe "AnonymousOrModerationEvent logging" do

83
spec/models/content_tag_spec.rb
View File

@ -632,9 +632,11 @@ describe ContentTag do
end
describe "destroy" do
it "updates completion requirements on its associated ContextModule" do
before do
course_with_teacher(:active_all => true)
end
it "updates completion requirements on its associated ContextModule" do
@module = @course.context_modules.create!(:name => "some module")
@assignment = @course.assignments.create!(:title => "some assignment")
@assignment2 = @course.assignments.create!(:title => "some assignment2")
@ -651,6 +653,85 @@ describe ContentTag do
expect(@module.reload.completion_requirements).to eq [{id: @tag2.id, type: 'must_submit'}]
end
it "runs the due date cacher when the content is Quizzes 2" do
@course.context_external_tools.create!(
name: 'Quizzes.Next',
consumer_key: 'test_key',
shared_secret: 'test_secret',
tool_id: 'Quizzes 2',
url: 'http://example.com/launch'
)
assignment = @course.assignments.create!(title: "some assignment")
assignment.quiz_lti!
assignment.save!
tag = assignment.external_tool_tag
expect(DueDateCacher).to receive(:recompute).with(assignment)
tag.destroy!
end
it "does not run the due date cacher for general content" do
tool = @course.context_external_tools.create!(
name: 'Not Quizzes.Next',
consumer_key: 'test_key',
shared_secret: 'test_secret',
tool_id: 'Not Quizzes 2',
url: 'http://example.com/launch'
)
assignment = @course.assignments.create!(
title: "some assignment",
submission_types: "external_tool",
external_tool_tag_attributes: { content: tool, url: tool.url }
)
tag = assignment.external_tool_tag
expect(DueDateCacher).to_not receive(:recompute).with(assignment)
tag.destroy!
end
end
context "Quizzes 2 calls backs" do
before do
course_with_teacher(active_all: true)
end
it "runs the due date cacher when saved if the content is Quizzes 2" do
tool = @course.context_external_tools.create!(
name: 'Quizzes.Next',
consumer_key: 'test_key',
shared_secret: 'test_secret',
tool_id: 'Quizzes 2',
url: 'http://example.com/launch'
)
assignment = @course.assignments.create!(title: "some assignment", submission_types: 'external_tool')
expect(DueDateCacher).to receive(:recompute).with(assignment)
ContentTag.create!(content: tool, url: tool.url, context: assignment)
end
it "does not run the due date cacher when saved for general content" do
tool = @course.context_external_tools.create!(
name: 'Not Quizzes.Next',
consumer_key: 'test_key',
shared_secret: 'test_secret',
tool_id: 'Not Quizzes 2',
url: 'http://example.com/launch'
)
assignment = @course.assignments.create!(title: "some assignment", submission_types: 'external_tool')
expect(DueDateCacher).to_not receive(:recompute).with(assignment)
ContentTag.create!(content: tool, url: tool.url, context: assignment)
end
end
it "should sync tag published state with attachment locked state" do