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

add more search capabilities to audit endpoint 

This adds more search capabilities to the audit endpoint to give
maximum flexibility to support the new gradebook history page. This
adds the ability to search for course AND any combination of the
following: assignment, grader, student. Permissions were not touched
in this patchset, so only admins can access currently.

closes CNVS-37299

test plan:
 - Create a course with two teachers, two assignments, and two
   student.
 - Grade the students for the assignments and change the grades a few
   times as each teacher
 - Visit the following API endpoints with appropriate ids confirming
   that appropriate data is returned:
    o /api/v1/audit/grade_change/courses/:course_id/assignments/:assignment_id
    o /api/v1/audit/grade_change/courses/:course_id/assignments/:assignment_id/graders/:grader_id
    o /api/v1/audit/grade_change/courses/:course_id/assignments/:assignment_id/graders/:grader_id/students/:student_id
    o /api/v1/audit/grade_change/courses/:course_id/assignments/:assignment_id/students/:student_id
    o /api/v1/audit/grade_change/courses/:course_id/graders/:grader_id
    o /api/v1/audit/grade_change/courses/:course_id/graders/:grader_id/students/:student_id
    o /api/v1/audit/grade_change/courses/:course_id/students/:student_id

Change-Id: I86648896d49d7228ddce04864e80221da9b5f96d
Reviewed-on: https://gerrit.instructure.com/114146
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Tested-by: Jenkins
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
This commit is contained in:
Keith Garner 2017-06-02 18:24:41 -05:00 committed by Keith T. Garner
parent 7207e78d09
commit d441418d7e
10 changed files with 718 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
app/controllers/grade_change_audit_api_controller.rb
View File

@ -136,16 +136,15 @@ class GradeChangeAuditApiController < AuditorApiController
# @returns [GradeChangeEvent]
#
def for_assignment
return render_unauthorized_action unless authorize
@assignment = Assignment.active.find(params[:assignment_id])
unless @assignment.context.root_account == @domain_root_account
raise ActiveRecord::RecordNotFound, "Couldn't find assignment with API id '#{params[:assignment_id]}'"
end
if authorize
events = Auditors::GradeChange.for_assignment(@assignment, query_options)
render_events(events, polymorphic_url([:api_v1, :audit_grade_change, @assignment]))
else
render_unauthorized_action
end
events = Auditors::GradeChange.for_assignment(@assignment, query_options)
render_events(events, polymorphic_url([:api_v1, :audit_grade_change, @assignment]))
end
# @API Query by course.
@ -161,13 +160,11 @@ class GradeChangeAuditApiController < AuditorApiController
# @returns [GradeChangeEvent]
#
def for_course
@course = @domain_root_account.all_courses.active.find(params[:course_id])
if authorize
events = Auditors::GradeChange.for_course(@course, query_options)
render_events(events, polymorphic_url([:api_v1, :audit_grade_change, @course]))
else
render_unauthorized_action
end
return render_unauthorized_action unless authorize
@course = @domain_root_account.all_courses.find(params[:course_id])
events = Auditors::GradeChange.for_course(@course, query_options)
render_events(events, polymorphic_url([:api_v1, :audit_grade_change, @course]))
end
# @API Query by student.
@ -183,16 +180,15 @@ class GradeChangeAuditApiController < AuditorApiController
# @returns [GradeChangeEvent]
#
def for_student
return render_unauthorized_action unless authorize
@student = User.active.find(params[:student_id])
unless @domain_root_account.associated_user?(@student)
raise ActiveRecord::RecordNotFound, "Couldn't find user with API id '#{params[:student_id]}'"
end
if authorize
events = Auditors::GradeChange.for_root_account_student(@domain_root_account, @student, query_options)
render_events(events, api_v1_audit_grade_change_student_url(@student))
else
render_unauthorized_action
end
events = Auditors::GradeChange.for_root_account_student(@domain_root_account, @student, query_options)
render_events(events, api_v1_audit_grade_change_student_url(@student))
end
# @API Query by grader.
@ -208,16 +204,45 @@ class GradeChangeAuditApiController < AuditorApiController
# @returns [GradeChangeEvent]
#
def for_grader
return render_unauthorized_action unless authorize
@grader = User.active.find(params[:grader_id])
unless @domain_root_account.associated_user?(@grader)
raise ActiveRecord::RecordNotFound, "Couldn't find user with API id '#{params[:grader_id]}'"
end
if authorize
events = Auditors::GradeChange.for_root_account_grader(@domain_root_account, @grader, query_options)
render_events(events, api_v1_audit_grade_change_grader_url(@grader))
else
render_unauthorized_action
events = Auditors::GradeChange.for_root_account_grader(@domain_root_account, @grader, query_options)
render_events(events, api_v1_audit_grade_change_grader_url(@grader))
end
def for_course_and_other_parameters
return render_unauthorized_action unless authorize
course = @domain_root_account.all_courses.find(params[:course_id])
args = { course: course }
args[:assignment] = course.assignments.find(params[:assignment_id]) if params[:assignment_id]
args[:grader] = course.all_users.find(params[:grader_id]) if params[:grader_id]
args[:student] = course.all_users.find(params[:student_id]) if params[:student_id]
url_method = if args[:assignment] && args[:grader] && args[:student]
:api_v1_audit_grade_change_course_assignment_grader_student_url
elsif args[:assignment] && args[:grader]
:api_v1_audit_grade_change_course_assignment_grader_url
elsif args[:assignment] && args[:student]
:api_v1_audit_grade_change_course_assignment_student_url
elsif args[:assignment]
:api_v1_audit_grade_change_course_assignment_url
elsif args[:grader] && args[:student]
:api_v1_audit_grade_change_course_grader_student_url
elsif args[:grader]
:api_v1_audit_grade_change_course_grader_url
elsif args[:student]
:api_v1_audit_grade_change_course_student_url
end
events = Auditors::GradeChange.for_course_and_other_arguments(course, args, query_options)
render_events(events, send(url_method, args))
end
private

91
app/models/auditors/grade_change.rb
View File

@ -177,6 +177,59 @@ class Auditors::GradeChange
entry_proc lambda{ |record| [record.root_account, record.student] }
key_proc lambda{ |root_account, student| [root_account.global_id, student.global_id] }
end
add_index :course_assignment do
table :grade_changes_by_course_assignment
entry_proc lambda { |record| [record.course, record.assignment] }
key_proc lambda { |course, assignment| [course.global_id, assignment.global_id] }
end
add_index :course_assignment_grader do
table :grade_changes_by_course_assignment_grader
entry_proc lambda { |record|
[record.course, record.assignment, record.grader] if record.grader && !record.submission.autograded?
}
key_proc lambda { |course, assignment, grader| [course.global_id, assignment.global_id, grader.global_id] }
end
add_index :course_assignment_grader_student do
table :grade_change_by_course_assignment_grader_student
entry_proc lambda { |record|
if record.grader && !record.submission.autograded?
[record.course, record.assignment, record.grader, record.student]
end
}
key_proc lambda { |course, assignment, grader, student|
[course.global_id, assignment.global_id, grader.global_id, student.global_id]
}
end
add_index :course_assignment_student do
table :grade_changes_by_course_assignment_student
entry_proc lambda { |record| [record.course, record.assignment, record.student] }
key_proc lambda { |course, assignment, student| [course.global_id, assignment.global_id, student.global_id] }
end
add_index :course_grader do
table :grade_changes_by_course_grader
entry_proc lambda { |record| [record.course, record.grader] if record.grader && !record.submission.autograded? }
key_proc lambda { |course, grader| [course.global_id, grader.global_id] }
end
add_index :course_grader_student do
table :grade_changes_by_course_grader_student
entry_proc lambda { |record|
[record.course, record.grader, record.student] if record.grader && !record.submission.autograded?
}
key_proc lambda { |course, grader, student| [course.global_id, grader.global_id, student.global_id] }
end
add_index :course_student do
table :grade_changes_by_course_student
entry_proc lambda { |record| [record.course, record.student] }
key_proc lambda { |course, student| [course.global_id, student.global_id] }
end
end
def self.record(submission, event_type=nil)
@ -211,4 +264,42 @@ class Auditors::GradeChange
Auditors::GradeChange::Stream.for_assignment(assignment, options)
end
end
# These are the groupings this method expects to receive:
# course assignment
# course assignment grader
# course assignment grader student
# course assignment student
# course grader
# course grader student
# course student
def self.for_course_and_other_arguments(course, arguments, options={})
course.shard.activate do
if arguments[:assignment] && arguments[:grader] && arguments[:student]
Auditors::GradeChange::Stream.for_course_assignment_grader_student(course,
arguments[:assignment], arguments[:grader], arguments[:student], options)
elsif arguments[:assignment] && arguments[:grader]
Auditors::GradeChange::Stream.for_course_assignment_grader(course, arguments[:assignment],
arguments[:grader], options)
elsif arguments[:assignment] && arguments[:student]
Auditors::GradeChange::Stream.for_course_assignment_student(course, arguments[:assignment],
arguments[:student], options)
elsif arguments[:assignment]
Auditors::GradeChange::Stream.for_course_assignment(course, arguments[:assignment], options)
elsif arguments[:grader] && arguments[:student]
Auditors::GradeChange::Stream.for_course_grader_student(course, arguments[:grader], arguments[:student],
options)
elsif arguments[:grader]
Auditors::GradeChange::Stream.for_course_grader(course, arguments[:grader], options)
elsif arguments[:student]
Auditors::GradeChange::Stream.for_course_student(course, arguments[:student], options)
end
end
end
end

1
app/models/course.rb
View File

@ -117,6 +117,7 @@ class Course < ActiveRecord::Base
has_many :course_account_associations
has_many :non_unique_associated_accounts, -> { order('course_account_associations.depth') }, source: :account, through: :course_account_associations
has_many :users, -> { distinct }, through: :enrollments, source: :user
has_many :all_users, -> { distinct }, through: :all_enrollments, source: :user
has_many :current_users, -> { distinct }, through: :current_enrollments, source: :user
has_many :all_current_users, -> { distinct }, through: :all_current_enrollments, source: :user
has_many :group_categories, -> {where(deleted_at: nil) }, as: :context, inverse_of: :context

14
config/routes.rb
View File

@ -1015,6 +1015,20 @@ CanvasRails::Application.routes.draw do
get 'audit/grade_change/courses/:course_id', action: :for_course, as: 'audit_grade_change_course'
get 'audit/grade_change/students/:student_id', action: :for_student, as: 'audit_grade_change_student'
get 'audit/grade_change/graders/:grader_id', action: :for_grader, as: 'audit_grade_change_grader'
get 'audit/grade_change/courses/:course_id/assignments/:assignment_id',
action: :for_course_and_other_parameters, as: 'audit_grade_change_course_assignment'
get 'audit/grade_change/courses/:course_id/assignments/:assignment_id/graders/:grader_id',
action: :for_course_and_other_parameters, as: 'audit_grade_change_course_assignment_grader'
get 'audit/grade_change/courses/:course_id/assignments/:assignment_id/graders/:grader_id/students/:student_id',
action: :for_course_and_other_parameters, as: 'audit_grade_change_course_assignment_grader_student'
get 'audit/grade_change/courses/:course_id/assignments/:assignment_id/students/:student_id',
action: :for_course_and_other_parameters, as: 'audit_grade_change_course_assignment_student'
get 'audit/grade_change/courses/:course_id/graders/:grader_id',
action: :for_course_and_other_parameters, as: 'audit_grade_change_course_grader'
get 'audit/grade_change/courses/:course_id/graders/:grader_id/students/:student_id',
action: :for_course_and_other_parameters, as: 'audit_grade_change_course_grader_student'
get 'audit/grade_change/courses/:course_id/students/:student_id',
action: :for_course_and_other_parameters, as: 'audit_grade_change_course_student'
end
scope(controller: :course_audit_api) do

63
db/migrate/20170605134354_cassandra_add_additional_grade_change_indexes_for_gradebook_history.rb Normal file
View File

@ -0,0 +1,63 @@
#
# Copyright (C) 2017 - 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 CassandraAddAdditionalGradeChangeIndexesForGradebookHistory < ActiveRecord::Migration[4.2]
tag :predeploy
include Canvas::Cassandra::Migration
def self.cassandra_cluster
'auditors'
end
def self.indexes
%w(
grade_changes_by_course_assignment
grade_changes_by_course_assignment_grader
grade_change_by_course_assignment_grader_student
grade_changes_by_course_assignment_student
grade_changes_by_course_grader
grade_changes_by_course_grader_student
grade_changes_by_course_student
)
end
def self.up
compression_params = if cassandra.db.use_cql3?
"WITH compression = { 'sstable_compression' : 'DeflateCompressor' }"
else
"WITH compression_parameters:sstable_compression='DeflateCompressor'"
end
indexes.each do |index_name|
cassandra.execute %{
CREATE TABLE #{index_name} (
key text,
ordered_id text,
id text,
PRIMARY KEY (key, ordered_id)
) #{compression_params}}
end
end
def self.down
indexes.each do |index_name|
cassandra.execute %{DROP TABLE #{index_name};}
end
end
end

29
db/migrate/20170606220650_init_new_grade_history_audit_log_indexes.rb Normal file
View File

@ -0,0 +1,29 @@
# Copyright (C) 2017 - 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 InitNewGradeHistoryAuditLogIndexes < ActiveRecord::Migration[4.2]
tag :postdeploy
def self.up
DataFixup::InitNewGradeHistoryAuditLogIndexes.send_later_if_production_enqueue_args(
:run, {
priority: Delayed::LOW_PRIORITY,
strand: "init_new_grade_history_audit_log_indexes:#{Shard.current.database_server.id}"
}
)
end
end

140
lib/data_fixup/init_new_grade_history_audit_log_indexes.rb Normal file
View File

@ -0,0 +1,140 @@
#
# Copyright (C) 2017 - 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
class InitNewGradeHistoryAuditLogIndexes
def self.run
new.build_indexes
end
SEARCH_CQL = %{
SELECT id, created_at, context_id, assignment_id, grader_id, student_id
FROM grade_changes
WHERE token(id) > token(?)
LIMIT ?
}.freeze
INDEX_METHODS = [
:add_course_assignment_index,
:add_course_assignment_grader_index,
:add_course_assignment_grader_student_index,
:add_course_assignment_student_index,
:add_course_grader_index,
:add_course_grader_student_index,
:add_course_student_index
].freeze
READ_BATCH_SIZE = 1000
WRITE_BATCH_SIZE = 400
def build_indexes
new_index_entries = []
last_seen_id = nil
done = false
until done
done, last_seen_id = read_and_process_batch(last_seen_id, new_index_entries)
end
write_batch(new_index_entries)
end
private
def read_and_process_batch(starting_key, index_entries)
last_id = nil
result = database.execute(SEARCH_CQL, starting_key, READ_BATCH_SIZE)
return true, nil if result.rows == 0
result.fetch do |row|
last_id = row['id']
INDEX_METHODS.each do |method|
result = self.send(method, row)
index_entries << result if result
end
end
write_in_batches(index_entries)
return false, last_id
end
ResultStruct = Struct.new(:index, :record, :key)
def write_in_batches(batch)
while batch.size >= WRITE_BATCH_SIZE
write_batch(batch.shift(WRITE_BATCH_SIZE))
end
end
def write_batch(batch)
return if batch.empty?
database.batch { batch.each { |r| r.index.insert(r.record, r.key) } }
end
def database
@database ||= Canvas::Cassandra::DatabaseBuilder.from_config(:auditors)
end
def add_course_assignment_index(row)
index = Auditors::GradeChange::Stream.course_assignment_index
key = [row['context_id'], row['assignment_id']]
ResultStruct.new(index, OpenStruct.new(row.to_hash), key)
end
def add_course_assignment_grader_index(row)
return unless row['grader_id']
index = Auditors::GradeChange::Stream.course_assignment_grader_index
key = [row['context_id'], row['assignment_id'], row['grader_id']]
ResultStruct.new(index, OpenStruct.new(row.to_hash), key)
end
def add_course_assignment_grader_student_index(row)
return unless row['grader_id']
index = Auditors::GradeChange::Stream.course_assignment_grader_student_index
key = [row['context_id'], row['assignment_id'], row['grader_id'], row['student_id']]
ResultStruct.new(index, OpenStruct.new(row.to_hash), key)
end
def add_course_assignment_student_index(row)
index = Auditors::GradeChange::Stream.course_assignment_student_index
key = [row['context_id'], row['assignment_id'], row['student_id']]
ResultStruct.new(index, OpenStruct.new(row.to_hash), key)
end
def add_course_grader_index(row)
return unless row['grader_id']
index = Auditors::GradeChange::Stream.course_grader_index
key = [row['context_id'], row['grader_id']]
ResultStruct.new(index, OpenStruct.new(row.to_hash), key)
end
def add_course_grader_student_index(row)
return unless row['grader_id']
index = Auditors::GradeChange::Stream.course_grader_student_index
key = [row['context_id'], row['grader_id'], row['student_id']]
ResultStruct.new(index, OpenStruct.new(row.to_hash), key)
end
def add_course_student_index(row)
index = Auditors::GradeChange::Stream.course_student_index
key = [row['context_id'], row['student_id']]
ResultStruct.new(index, OpenStruct.new(row.to_hash), key)
end
end
end

172
spec/apis/v1/grade_change_audit_api_spec.rb
View File

@ -85,6 +85,51 @@ describe "GradeChangeAudit API", type: :request do
api_call_as_user(@viewing_user, :get, path, arguments, {}, {}, options.slice(:expected_status))
end
def fetch_for_course_and_other_contexts(contexts, options={})
expected_contexts = [:course, :assignment, :grader, :student].freeze
sorted_contexts = contexts.select { |key,_| expected_contexts.include?(key) }.
sort_by { |key, _| expected_contexts.index(key) }
arguments = sorted_contexts.map { |key, value| ["#{key}_id".to_sym, value.id] }.to_h
arguments.merge!({
controller: 'grade_change_audit_api',
action: 'for_course_and_other_parameters',
format: 'json'
})
query_string = []
per_page = options.delete(:per_page)
if per_page
arguments[:per_page] = per_page.to_s
query_string << "per_page=#{arguments[:per_page]}"
end
start_time = options.delete(:start_time)
if start_time
arguments[:start_time] = start_time.iso8601
query_string << "start_time=#{arguments[:start_time]}"
end
end_time = options.delete(:end_time)
if end_time
arguments[:end_time] = end_time.iso8601
query_string << "end_time=#{arguments[:end_time]}"
end
account = options.delete(:account)
if account
arguments[:account_id] = Shard.global_id_for(account).to_s
query_string << "account_id=#{arguments[:account_id]}"
end
path_args = sorted_contexts.map { |key, value| "#{key.to_s.pluralize}/#{value.id}" }.join('/')
path = "/api/v1/audit/grade_change/#{path_args}"
path += "?" + query_string.join('&') if query_string.present?
api_call_as_user(@viewing_user, :get, path, arguments, {}, {}, options.slice(:expected_status))
end
def expect_event_for_context(context, event, options={})
json = options.delete(:json)
json ||= fetch_for_context(context, options)
@ -93,6 +138,14 @@ describe "GradeChangeAudit API", type: :request do
json
end
def expect_event_for_course_and_contexts(contexts, event, options={})
json = options.delete(:json)
json ||= fetch_for_course_and_other_contexts(contexts, options)
expect(json['events'].map{ |e| [e['id'], e['event_type']] })
.to include([event.id, event.event_type])
json
end
def forbid_event_for_context(context, event, options={})
json = options.delete(:json)
json ||= fetch_for_context(context, options)
@ -101,12 +154,48 @@ describe "GradeChangeAudit API", type: :request do
json
end
def forbid_event_for_course_and_contexts(contexts, event, options={})
json = options.delete(:json)
json ||= fetch_for_course_and_contexts(contexts, options)
expect(json['events'].map{ |e| [e['id'], e['event_type']] })
.not_to include([event.id, event.event_type])
json
end
def test_course_and_contexts
# course assignment
contexts = { course: @course, assignment: @assignment }
yield(contexts)
# course assignment grader
contexts[:grader] = @teacher
yield(contexts)
# course assignment grader student
contexts[:student] = @student
yield(contexts)
# course assignment student
contexts.delete(:grader)
yield(contexts)
# course student
contexts.delete(:assignment)
yield(contexts)
# course grader
contexts = { course: @course, grader: @teacher}
yield(contexts)
# course grader student
contexts[:student] = @student
yield(contexts)
end
context "nominal cases" do
it "should include events at context endpoint" do
expect_event_for_context(@assignment, @event)
expect_event_for_context(@course, @event)
expect_event_for_context(@student, @event, type: "student")
expect_event_for_context(@teacher, @event, type: "grader")
test_course_and_contexts do |contexts|
expect_event_for_course_and_contexts(contexts, @event)
end
end
end
@ -121,6 +210,7 @@ describe "GradeChangeAudit API", type: :request do
it "should recognize :start_time" do
json = expect_event_for_context(@assignment, @event, start_time: 12.hours.ago)
forbid_event_for_context(@assignment, @event2, start_time: 12.hours.ago, json: json)
json = expect_event_for_context(@course, @event, start_time: 12.hours.ago)
@ -131,6 +221,11 @@ describe "GradeChangeAudit API", type: :request do
json = expect_event_for_context(@teacher, @event, type: "grader", start_time: 12.hours.ago)
forbid_event_for_context(@teacher, @event2, type: "grader", start_time: 12.hours.ago, json: json)
test_course_and_contexts do |contexts|
json = expect_event_for_course_and_contexts(contexts, @event, start_time: 12.hours.ago)
forbid_event_for_course_and_contexts(contexts, @event2, start_time: 12.hours.ago, json: json)
end
end
it "should recognize :end_time" do
@ -145,6 +240,11 @@ describe "GradeChangeAudit API", type: :request do
json = expect_event_for_context(@teacher, @event2, type: "grader", end_time: 12.hours.ago)
forbid_event_for_context(@teacher, @event, type: "grader", end_time: 12.hours.ago, json: json)
test_course_and_contexts do |contexts|
json = expect_event_for_course_and_contexts(contexts, @event2, end_time: 12.hours.ago)
forbid_event_for_course_and_contexts(contexts, @event, end_time: 12.hours.ago, json: json)
end
end
end
@ -154,9 +254,27 @@ describe "GradeChangeAudit API", type: :request do
fetch_for_context(@assignment, expected_status: 404)
end
it "should 404 for inactive courses" do
it "should allow inactive assignments when used with a course" do
@assignment.destroy
fetcher = lambda do |contexts|
fetch_for_course_and_other_contexts(contexts, expected_status: 200)
end
contexts = {course: @course, assignment: @assignment}
fetcher.call(contexts)
contexts[:grader] = @teacher
fetcher.call(contexts)
contexts[:student] = @student
fetcher.call(contexts)
contexts.delete(:grader)
fetcher.call(contexts)
end
it "should allow inactive courses" do
@course.destroy
fetch_for_context(@course, expected_status: 404)
fetch_for_context(@course, expected_status: 200)
test_course_and_contexts do |contexts|
fetch_for_course_and_other_contexts(contexts, expected_status: 200)
end
end
it "should 404 for inactive students" do
@ -164,10 +282,40 @@ describe "GradeChangeAudit API", type: :request do
fetch_for_context(@student, expected_status: 404, type: "student")
end
it "should allow inactive students when used with a course" do
@student.destroy
fetcher = lambda do |contexts|
fetch_for_course_and_other_contexts(contexts, expected_status: 200)
end
contexts = {course: @course, assignment: @assignment, grader: @teacher, student: @student}
fetcher.call(contexts)
contexts.delete(:grader)
fetcher.call(contexts)
contexts.delete(:assignment)
fetcher.call(contexts)
contexts = {course: @course, student: @student}
fetcher.call(contexts)
end
it "should 404 for inactive grader" do
@teacher.destroy
fetch_for_context(@teacher, expected_status: 404, type: "grader")
end
it "shoudl allow inactive graders when used with a course" do
@teacher.destroy
fetcher = lambda do |contexts|
fetch_for_course_and_other_contexts(contexts, expected_status: 200)
end
contexts = {course: @course, assignment: @assignment, grader: @teacher, student: @student}
fetcher.call(contexts)
contexts.delete(:student)
fetcher.call(contexts)
contexts.delete(:assignment)
fetcher.call(contexts)
contexts[:student] = @student
fetcher.call(contexts)
end
end
describe "permissions" do
@ -178,15 +326,22 @@ describe "GradeChangeAudit API", type: :request do
fetch_for_context(@assignment, expected_status: 401)
fetch_for_context(@student, expected_status: 401, type: "student")
fetch_for_context(@teacher, expected_status: 401, type: "grader")
test_course_and_contexts do |contexts|
fetch_for_course_and_other_contexts(contexts, expected_status: 401)
end
end
it "should not authorize the endpoints with revoking the :view_grade_changes permission" do
RoleOverride.manage_role_override(@account_user.account, @account_user.role, :view_grade_changes.to_s, :override => false)
RoleOverride.manage_role_override(@account_user.account, @account_user.role,
:view_grade_changes.to_s, :override => false)
fetch_for_context(@course, expected_status: 401)
fetch_for_context(@assignment, expected_status: 401)
fetch_for_context(@student, expected_status: 401, type: "student")
fetch_for_context(@teacher, expected_status: 401, type: "grader")
test_course_and_contexts do |contexts|
fetch_for_course_and_other_contexts(contexts, expected_status: 401)
end
end
it "should not allow other account models" do
@ -194,10 +349,13 @@ describe "GradeChangeAudit API", type: :request do
LoadAccount.stubs(:default_domain_root_account).returns(new_root_account)
@viewing_user = user_with_pseudonym(account: new_root_account)
fetch_for_context(@course, expected_status: 404)
fetch_for_context(@assignment, expected_status: 404)
fetch_for_context(@student, expected_status: 404, type: "student")
fetch_for_context(@teacher, expected_status: 404, type: "grader")
fetch_for_context(@course, expected_status: 401)
fetch_for_context(@assignment, expected_status: 401)
fetch_for_context(@student, expected_status: 401, type: "student")
fetch_for_context(@teacher, expected_status: 401, type: "grader")
test_course_and_contexts do |contexts|
fetch_for_course_and_other_contexts(contexts, expected_status: 401)
end
end
context "sharding" do

108
spec/lib/data_fixup/init_new_grade_history_audit_log_indexes_spec.rb Normal file
View File

@ -0,0 +1,108 @@
#
# Copyright (C) 2017 - 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/>.
#
require 'spec_helper'
describe DataFixup::InitNewGradeHistoryAuditLogIndexes do
def insert_cql(use_grader: false)
%{
INSERT INTO grade_changes (id, created_at, context_id, assignment_id,
#{'grader_id, ' if use_grader} student_id)
VALUES (?, ?, ?, ?, #{'?,' if use_grader} ?) USING TTL ?
}
end
def search_table_cql(table_name)
"SELECT id FROM #{table_name}"
end
NO_GRADER_REQUIRED_TABLES = %w(
grade_changes_by_course_assignment
grade_changes_by_course_assignment_student
grade_changes_by_course_student
).freeze
GRADER_REQUIRED_TABLES = %w(
grade_changes_by_course_assignment_grader
grade_change_by_course_assignment_grader_student
grade_changes_by_course_grader
grade_changes_by_course_grader_student
).freeze
INDEX_TABLES = (NO_GRADER_REQUIRED_TABLES + GRADER_REQUIRED_TABLES).freeze
before(:each) do
@database = Canvas::Cassandra::DatabaseBuilder.from_config(:auditors)
@values = [
[
'08d87bfc-a679-4f5d-9315-470a5fc7d7d0',
Time.new(2016, 10, 18, 23, 24, 38).utc,
10000000000018,
10000000000116,
10000000000001,
10000000000006,
1.year
],
[
'fc85afda-538e-4fcb-a7fb-45697c551b71',
Time.new(2017, 6, 26, 16, 40, 5).utc,
10000000000028,
10000000000144,
10000000000003,
1.year
]
]
@values.each do |v|
@database.execute(insert_cql(use_grader: v.size > 6), *v)
end
end
it 'creates all the new indexes for records with grader ids' do
DataFixup::InitNewGradeHistoryAuditLogIndexes.run
INDEX_TABLES.each do |table_name|
cql = search_table_cql(table_name)
ids = []
@database.execute(cql).fetch do |row|
ids << row['id']
end
expect(ids).to include(@values.first.first)
end
end
it 'creates the expected subset of indexes for records without grader ids' do
DataFixup::InitNewGradeHistoryAuditLogIndexes.run
INDEX_TABLES.each do |table_name|
cql = search_table_cql(table_name)
ids = []
@database.execute(cql).fetch do |row|
ids << row['id']
end
if GRADER_REQUIRED_TABLES.include?(table_name)
expect(ids).not_to include(@values.second.first)
else
expect(ids).to include(@values.second.first)
end
end
end
end

63
spec/models/auditors/grade_change_spec.rb
View File

@ -48,13 +48,44 @@ describe Auditors::GradeChange do
Timecop.freeze(@event_time) { @event = Auditors::GradeChange.record(@submission) }
end
def test_course_and_other_contexts
# course assignment
contexts = { assignment: @assignment }
yield contexts
# course assignment grader
contexts[:grader] = @teacher
yield contexts
# course assignment grader student
contexts[:student] = @student
yield contexts
# course assignment student
contexts.delete(:grader)
yield contexts
# course grader
contexts = { grader: @teacher }
yield contexts
# course grader student
contexts[:student] = @student
yield contexts
# course student
contexts.delete(:grader)
yield contexts
end
context "nominal cases" do
it "should include event" do
expect(@event.created_at).to eq @event_time
expect(Auditors::GradeChange.for_assignment(@assignment).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_course(@course).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_root_account_student(@account, @student).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_root_account_grader(@account, @teacher).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_root_account_student(@account, @student).
paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_root_account_grader(@account, @teacher).
paginate(:per_page => 5)).to include(@event)
test_course_and_other_contexts do |contexts|
expect(Auditors::GradeChange.for_course_and_other_arguments(@course, contexts).
paginate(:per_page => 5)).to include(@event)
end
end
it "should include event for nil grader" do
@ -65,7 +96,14 @@ describe Auditors::GradeChange do
expect(Auditors::GradeChange.for_assignment(@assignment).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_course(@course).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_root_account_student(@account, @student).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_root_account_student(@account, @student).
paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_course_and_other_arguments(@course, {assignment: @assignment}).
paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_course_and_other_arguments(@course, {assignment: @assignment,
student: @student}).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_course_and_other_arguments(@course, {student: @student}).
paginate(:per_page => 5)).to include(@event)
end
it "should include event for auto grader" do
@ -77,7 +115,14 @@ describe Auditors::GradeChange do
expect(Auditors::GradeChange.for_assignment(@assignment).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_course(@course).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_root_account_student(@account, @student).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_root_account_student(@account, @student).
paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_course_and_other_arguments(@course, {assignment: @assignment}).
paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_course_and_other_arguments(@course, {assignment: @assignment,
student: @student}).paginate(:per_page => 5)).to include(@event)
expect(Auditors::GradeChange.for_course_and_other_arguments(@course, {student: @student}).
paginate(:per_page => 5)).to include(@event)
end
it "should set request_id" do
@ -92,10 +137,14 @@ describe Auditors::GradeChange do
for_assignment = Auditors::GradeChange.for_assignment(@assignment)
for_course = Auditors::GradeChange.for_course(@course)
for_root_account_student = Auditors::GradeChange.for_root_account_student(@account, @student)
expect(for_assignment.paginate(per_page: 5)).to include(@event)
expect(for_course.paginate(per_page: 5)).to include(@event)
expect(for_root_account_student.paginate(per_page: 5)).to include(@event)
test_course_and_other_contexts do |contexts|
expect(Auditors::GradeChange.for_course_and_other_arguments(@course, contexts).paginate(per_page: 5)).
to include(@event)
end
end
it "reports formerly excused submissions" do
@ -111,6 +160,10 @@ describe Auditors::GradeChange do
expect(for_assignment.paginate(per_page: 5)).to include(@event)
expect(for_course.paginate(per_page: 5)).to include(@event)
expect(for_root_account_student.paginate(per_page: 5)).to include(@event)
test_course_and_other_contexts do |contexts|
expect(Auditors::GradeChange.for_course_and_other_arguments(@course, contexts).paginate(per_page: 5)).
to include(@event)
end
end
it "records excused_before and excused_after as booleans on initial grading" do