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

retrieve conditional release rule information natively 

also add native support for choosing from
multiple assignment sets

test plan:
* enable native conditional release (see g/239985)
* set up a course with assignments assigned to "Mastery Paths"
 so they're only visible to students with overrides
* use the editor to create a trigger assignment that can
 release the hidden assignments, setting up a
 scoring range that has an "or" option (by clicking the "&"
 between multiple assignments) that won't automatically
 assign students but rather present a choice
* as a student, submit to the trigger assignment
* as a teacher, grade the trigger assignment with a
score that will trigger the "or" path
* as the student, confirm that the modules page reveals
a link to a page that allows the student to choose
one assignment set or the other
* confirm that choosing an assignment set will
 reveal its assignments to the student

* confirm that as a teacher, viewing the trigger
 assignment page shows a "Mastery Paths Breakdown"
 that can be interacted with to show which students
 fell into each scoring range

* all other native conditional release functionality
 not listed here should be in-line with current functionality
 via the full service (apart from course copy support)

closes #LS-1065

Change-Id: Iede1689b3896380d4da155fa7ae325796d85f934
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/240607
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
James Williams 2020-06-18 11:53:16 -06:00
parent 1f7b069951
commit a848924b57
19 changed files with 439 additions and 179 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
app/controllers/conditional_release/rules_controller.rb
View File

@ -27,7 +27,7 @@ module ConditionalRelease
# GET /api/rules
def index
rules = get_rules
rules = rules.preload(Rule.all_includes) if include_param.include?('all')
rules = rules.preload(Rule.preload_associations) if include_param.include?('all')
rules = rules.with_assignments if value_to_boolean(params[:active])
render json: rules.as_json(include: json_includes, include_root: false, except: [:root_account_id, :deleted_at])
@ -52,7 +52,7 @@ module ConditionalRelease
rule = Rule.new(create_params)
if rule.save
render json: rule.as_json(include: all_includes, include_root: false, except: [:root_account_id, :deleted_at])
render json: rule.as_json(include: Rule.includes_for_json, include_root: false, except: [:root_account_id, :deleted_at])
else
render json: rule.errors, status: :bad_request
end
@ -70,7 +70,7 @@ module ConditionalRelease
:assignment_set_associations
)
if rule.update(update_params)
render json: rule.as_json(include: all_includes, include_root: false, except: [:root_account_id, :deleted_at])
render json: rule.as_json(include: Rule.includes_for_json, include_root: false, except: [:root_account_id, :deleted_at])
else
render json: rule.errors, status: :bad_request
end
@ -99,19 +99,8 @@ module ConditionalRelease
Array.wrap(params[:include])
end
def all_includes
{ scoring_ranges: {
include: {
assignment_sets: {
include: {assignment_set_associations: {except: [:root_account_id, :deleted_at]}},
except: [:root_account_id, :deleted_at]
} },
except: [:root_account_id, :deleted_at]
} }
end
def json_includes
return all_includes if include_param.include? 'all'
return Rule.includes_for_json if include_param.include? 'all'
end
def add_ordering_to(attrs)

27
app/controllers/context_module_items_api_controller.rb
View File

@ -276,7 +276,7 @@ class ContextModuleItemsApiController < ApplicationController
# Get conditionally released objects if requested
# TODO: Document in API when out of beta
if includes.include?("mastery_paths")
opts[:conditional_release_rules] = ConditionalRelease::Service.rules_for(@context, @student, items, session)
opts[:conditional_release_rules] = ConditionalRelease::Service.rules_for(@context, @student, session)
end
render :json => items.map { |item| module_item_json(item, @student || @current_user, session, mod, prog, includes, opts) }
end
@ -542,18 +542,25 @@ class ContextModuleItemsApiController < ApplicationController
assignment = @item.assignment
return render json: { message: 'requested item is not an assignment' }, status: :bad_request unless assignment
response = ConditionalRelease::Service.select_mastery_path(
@context,
@current_user,
@student,
assignment,
params[:assignment_set_id],
session)
if ConditionalRelease::Service.natively_enabled_for_account?(@context.root_account)
assignment_ids = ConditionalRelease::OverrideHandler.handle_assignment_set_selection(@student, assignment, params[:assignment_set_id])
request_failed = false
else
response = ConditionalRelease::Service.select_mastery_path(
@context,
@current_user,
@student,
assignment,
params[:assignment_set_id],
session)
request_failed = response[:code] != '200'
assignment_ids = response[:body]['assignments'].map {|a| a['assignment_id'].try(&:to_i) } unless request_failed
end
if response[:code] != '200'
if request_failed
render json: response[:body], status: response[:code]
else
assignment_ids = response[:body]['assignments'].map {|a| a['assignment_id'].try(&:to_i) }
# assignment occurs in delayed job, may not be fully visible to user until job completes
assignments = @context.assignments.published.where(id: assignment_ids).
preload(Api::V1::Assignment::PRELOADS)

6
app/controllers/context_modules_controller.rb
View File

@ -47,7 +47,7 @@ class ContextModulesController < ApplicationController
collection_cache_key(@modules), Time.zone, Digest::MD5.hexdigest([visible_assignments, @section_visibility].join("/"))]
cache_key = cache_key_items.join('/')
cache_key = add_menu_tools_to_cache_key(cache_key)
cache_key = add_mastery_paths_to_cache_key(cache_key, @context, @modules, @current_user)
cache_key = add_mastery_paths_to_cache_key(cache_key, @context, @current_user)
end
end
@ -139,7 +139,7 @@ class ContextModulesController < ApplicationController
item = @context.context_module_tags.not_deleted.find(params[:id])
if item.present? && item.published? && item.context_module.published?
rules = ConditionalRelease::Service.rules_for(@context, @current_user, item, session)
rules = ConditionalRelease::Service.rules_for(@context, @current_user, session)
rule = conditional_release_rule_for_module_item(item, conditional_release_rules: rules)
# locked assignments always have 0 sets, so this check makes it not return 404 if locked
@ -151,7 +151,7 @@ class ContextModulesController < ApplicationController
setId: set[:id]
}
option[:assignments] = set[:assignments].map { |a|
option[:assignments] = (set[:assignments] || set[:assignment_set_associations]).map { |a|
assg = assignment_json(a[:model], @current_user, session)
assg[:assignmentId] = a[:assignment_id]
assg

11
app/helpers/context_modules_helper.rb
View File

@ -40,7 +40,7 @@ module ContextModulesHelper
true, Time.zone, Digest::MD5.hexdigest([visible_assignments, @section_visibility].join("/"))]
cache_key = cache_key_items.join('/')
cache_key = add_menu_tools_to_cache_key(cache_key)
cache_key = add_mastery_paths_to_cache_key(cache_key, context, context_module, user)
cache_key = add_mastery_paths_to_cache_key(cache_key, context, user)
cache(cache_key, {}, &block)
else
yield
@ -55,13 +55,10 @@ module ContextModulesHelper
cache_key
end
def add_mastery_paths_to_cache_key(cache_key, context, module_or_modules, user)
def add_mastery_paths_to_cache_key(cache_key, context, user)
if user && cyoe_enabled?(context)
if context.user_is_student?(user)
items = Rails.cache.fetch("visible_content_tags_for/#{cache_key}") do
Array.wrap(module_or_modules).map{ |m| m.content_tags_visible_to(user, :is_teacher => false) }.flatten
end
rules = cyoe_rules(context, user, items, @session)
rules = cyoe_rules(context, user, @session)
else
rules = ConditionalRelease::Service.active_rules(context, user, @session)
end
@ -120,7 +117,7 @@ module ContextModulesHelper
}
if cyoe_enabled?(@context)
rules = cyoe_rules(@context, current_user, module_data[:items], session) || []
rules = cyoe_rules(@context, current_user, session) || []
end
items_data = {}

22
app/helpers/cyoe_helper.rb
View File

@ -31,12 +31,12 @@ module CyoeHelper
ConditionalRelease::Service.enabled_in_context?(context)
end
def cyoe_rules(context, current_user, items, session)
ConditionalRelease::Service.rules_for(context, current_user, items, session)
def cyoe_rules(context, current_user, session)
ConditionalRelease::Service.rules_for(context, current_user, session)
end
def conditional_release_rule_for_module_item(content_tag, opts = {})
rules = opts[:conditional_release_rules] || cyoe_rules(opts[:context], opts[:user], [], opts[:session])
rules = opts[:conditional_release_rules] || cyoe_rules(opts[:context], opts[:user], opts[:session])
assignment_id = content_tag.assignment.try(:id)
path_data = conditional_release_assignment_set(rules, assignment_id) if rules.present? && assignment_id.present?
if path_data.present? && opts[:is_student]
@ -48,7 +48,7 @@ module CyoeHelper
end
def conditional_release_assignment_set(rules, id)
result = rules.find { |rule| rule[:trigger_assignment].to_s == id.to_s }
result = rules.find { |rule| rule[:trigger_assignment].to_s == id.to_s || rule[:trigger_assignment_id] == id }
return if result.blank?
result.slice(:locked, :assignment_sets, :selected_set_id)
end
@ -57,8 +57,9 @@ module CyoeHelper
result = conditional_release_rule_for_module_item(content_tag, opts)
return if result.blank?
result[:assignment_sets].each do |as|
next if as[:assignments].blank?
as[:assignments].each do |a|
associations = as[:assignments] || as[:assignment_set_associations]
next if associations.blank?
associations.each do |a|
a[:model] = assignment_json(a[:model], user, nil) if a[:model]
end
end
@ -76,8 +77,11 @@ module CyoeHelper
def check_if_processing(data)
if !data[:awaiting_choice] && data[:assignment_sets].length == 1
vis_assignments = AssignmentStudentVisibility.visible_assignment_ids_for_user(@current_user.id, @context.id) || []
selected_set_assignment_ids = data[:assignment_sets][0][:assignments].map{ |a| a[:assignment_id].to_i } || []
vis_assignments = RequestCache.cache('visible_assignment_ids_in_course', @current_user, @context) do
AssignmentStudentVisibility.visible_assignment_ids_for_user(@current_user.id, @context.id) || []
end
set = data[:assignment_sets][0]
selected_set_assignment_ids = (set[:assignments] || set[:assignment_set_associations]).map{ |a| a[:assignment_id].to_i } || []
data[:still_processing] = (selected_set_assignment_ids - vis_assignments).present?
end
end
@ -94,4 +98,4 @@ module CyoeHelper
modules_tab_disabled: modules_disabled
})
end
end
end

6
app/jsx/shared/conditional_release/CyoeHelper.js
View File

@ -25,17 +25,17 @@ const parseEnvData = () => {
(ENV.CONDITIONAL_RELEASE_ENV && ENV.CONDITIONAL_RELEASE_ENV.active_rules) || []
return {
triggerAssignments: activeRules.reduce((triggers, rule) => {
triggers[rule.trigger_assignment] = rule
triggers[rule.trigger_assignment || rule.trigger_assignment_id] = rule
return triggers
}, {}),
releasedAssignments: activeRules.reduce((released, rule) => {
rule.scoring_ranges.forEach(range => {
range.assignment_sets.forEach(set => {
set.assignments.forEach(asg => {
;(set.assignments || set.assignment_set_associations).forEach(asg => {
const id = asg.assignment_id
released[id] = released[id] || []
released[id].push({
assignment_id: rule.trigger_assignment,
assignment_id: rule.trigger_assignment || rule.trigger_assignment_id,
assignment: rule.trigger_assignment_model,
upper_bound: range.upper_bound,
lower_bound: range.lower_bound

2
app/jsx/shared/helpers/assignment-categories.js
View File

@ -71,7 +71,7 @@ Categories.getCategory = assg => {
_.find(assg.submission_types, sub => cat.submissionTypes.indexOf(sub) !== -1)
)
})
return category || Categories.OTHER
return category || OTHER
}
export default Categories

3
app/models/assignment.rb
View File

@ -108,6 +108,7 @@ class Assignment < ActiveRecord::Base
dependent: :destroy,
inverse_of: :assignment
has_many :conditional_release_rules, class_name: "ConditionalRelease::Rule", dependent: :destroy, foreign_key: 'trigger_assignment_id', inverse_of: :trigger_assignment
has_many :conditional_release_associations, class_name: "ConditionalRelease::AssignmentSetAssociation", dependent: :destroy, inverse_of: :assignment
scope :anonymous, -> { where(anonymous_grading: true) }
@ -1213,6 +1214,8 @@ class Assignment < ActiveRecord::Base
self.save!
each_submission_type { |submission| submission.destroy if submission && !submission.deleted? }
self.conditional_release_rules.destroy_all
self.conditional_release_associations.destroy_all
refresh_course_content_participation_counts
ScheduledSmartAlert.where(context_type: 'Assignment', context_id: self.id).destroy_all

11
app/models/conditional_release/assignment_set_association.rb
View File

@ -32,11 +32,22 @@ module ConditionalRelease
has_one :rule, through: :assignment_set
belongs_to :root_account, :class_name => "Account"
after_save :clear_caches
before_create :set_root_account_id
def set_root_account_id
self.root_account_id ||= assignment_set.root_account_id
end
def clear_caches
if self.saved_change_to_deleted_at? && self.assignment.deleted?
# normally this will be cleared by the rule, but not after assignment deletion
self.class.connection.after_transaction_commit do
self.assignment.context.clear_cache_key(:conditional_release)
end
end
end
delegate :course_id, to: :rule, allow_nil: true
private

18
app/models/conditional_release/override_handler.rb
View File

@ -29,6 +29,24 @@ module ConditionalRelease
student_id: submission.user_id, actor_id: submission.grader_id, source: 'grade_change')
end
def handle_assignment_set_selection(student, trigger_assignment, assignment_set_id)
# just going to raise a 404 if the choice is invalid because i'm too lazy to return more specific errors
rule_ids = trigger_assignment.conditional_release_rules.active.pluck(:id)
submission = trigger_assignment.submissions.for_user(student).in_workflow_state(:graded).posted.take!
relative_score = ConditionalRelease::Stats.percent_from_points(submission.score, trigger_assignment.points_possible)
assignment_set = AssignmentSet.active.where(id: assignment_set_id,
scoring_range: ScoringRange.active.where(:rule_id => rule_ids).for_score(relative_score)).take!
other_assignment_sets = assignment_set.scoring_range.assignment_sets - [assignment_set]
previous_set_ids = AssignmentSetAction.current_assignments(student.id, other_assignment_sets).pluck(:assignment_set_id)
sets_to_unassign = other_assignment_sets.select{|set| previous_set_ids.include?(set.id)}
set_assignment_overrides(submission.user_id, [assignment_set], sets_to_unassign)
ConditionalRelease::AssignmentSetAction.create_from_sets([assignment_set], sets_to_unassign,
student_id: submission.user_id, source: 'select_assignment_set')
assignment_set.assignment_set_associations.map(&:assignment_id)
end
def find_assignment_sets(submission)
rules = submission.course.conditional_release_rules.active.where(:trigger_assignment_id => submission.assignment).preload(:assignment_sets).to_a
relative_score = ConditionalRelease::Stats.percent_from_points(submission.score, submission.assignment.points_possible)

29
app/models/conditional_release/rule.rb
View File

@ -31,7 +31,7 @@ module ConditionalRelease
has_many :assignment_set_associations, -> { active.order(position: :asc) }, through: :scoring_ranges
accepts_nested_attributes_for :scoring_ranges, allow_destroy: true
after_save :clear_trigger_assignment_cache
after_save :clear_caches
before_create :set_root_account_id
def set_root_account_id
@ -45,20 +45,37 @@ module ConditionalRelease
end
scope :with_assignments, -> do
having_assignments = joins(all_includes).group(Arel.sql("conditional_release_rules.id"))
preload(all_includes).where(id: having_assignments.pluck(:id))
having_assignments = joins(preload_associations).group(Arel.sql("conditional_release_rules.id"))
preload(preload_associations).where(id: having_assignments.pluck(:id))
end
def self.all_includes
def self.preload_associations
{ scoring_ranges: { assignment_sets: :assignment_set_associations } }
end
def self.includes_for_json
{
scoring_ranges: {
include: {
assignment_sets: {
include: {assignment_set_associations: {except: [:root_account_id, :deleted_at]}},
except: [:root_account_id, :deleted_at]
}
},
except: [:root_account_id, :deleted_at]
}
}
end
def assignment_sets_for_score(score)
AssignmentSet.active.where(scoring_range: scoring_ranges.for_score(score))
end
def clear_trigger_assignment_cache
self.trigger_assignment.clear_cache_key(:conditional_release)
def clear_caches
self.class.connection.after_transaction_commit do
self.trigger_assignment.clear_cache_key(:conditional_release)
self.course.clear_cache_key(:conditional_release)
end
end
def self.is_trigger_assignment?(assignment)

116
app/models/conditional_release/service.rb
View File

@ -56,7 +56,6 @@ module ConditionalRelease
cyoe_env[:course_id] = context.id
cyoe_env[:stats_url] = "/api/v1/courses/#{context.id}/mastery_paths/stats"
end
# TODO: add rules and whatnot
else
cyoe_env = {
jwt: jwt_for(context, user, domain, session: session, real_user: real_user),
@ -67,10 +66,10 @@ module ConditionalRelease
base_url: base_url,
context_id: context.id
}
cyoe_env[:rule] = rule_triggered_by(assignment, user, session) if includes.include? :rule
cyoe_env[:active_rules] = active_rules(context, user, session) if includes.include? :active_rules
end
cyoe_env[:rule] = rule_triggered_by(assignment, user, session) if includes.include? :rule
cyoe_env[:active_rules] = active_rules(context, user, session) if includes.include? :active_rules
env.merge(CONDITIONAL_RELEASE_ENV: cyoe_env)
end
@ -89,9 +88,9 @@ module ConditionalRelease
)
end
def self.rules_for(context, student, content_tags, session)
def self.rules_for(context, student, session)
return unless enabled_in_context?(context)
rules_data(context, student, Array.wrap(content_tags), session)
rules_data(context, student, session)
end
def self.clear_active_rules_cache(course)
@ -201,36 +200,13 @@ module ConditionalRelease
rules = active_rules(assignment.context, current_user, session)
return nil unless rules
rules.find {|r| r['trigger_assignment'] == assignment.id.to_s}
end
def self.rules_assigning(assignment, current_user, session = nil)
reverse_lookup = Rails.cache.fetch(active_rules_reverse_cache_key(assignment.context)) do
all_rules = active_rules(assignment.context, current_user, session)
return nil unless all_rules
lookup = {}
all_rules.each do |rule|
(rule['scoring_ranges'] || []).each do |sr|
(sr['assignment_sets'] || []).each do |as|
(as['assignments'] || []).each do |a|
if a['assignment_id'].present?
lookup[a['assignment_id']] ||= []
lookup[a['assignment_id']] << rule
end
end
end
end
end
lookup.each {|_id, rules| rules.uniq!}
lookup
end
reverse_lookup[assignment.id.to_s]
rules.find {|r| r['trigger_assignment'] == assignment.id.to_s || r['trigger_assignment_id'] == assignment.id}
end
def self.active_rules(course, current_user, session)
return unless enabled_in_context?(course)
return unless course.grants_any_right?(current_user, session, :read, :manage_assignments)
return native_active_rules(course) if natively_enabled_for_account?(course.root_account)
Rails.cache.fetch(active_rules_cache_key(course)) do
headers = headers_for(course, current_user, domain_for(course), session)
@ -263,6 +239,25 @@ module ConditionalRelease
[]
end
def self.native_active_rules(course)
rules_data = Rails.cache.fetch_with_batched_keys('conditional_release_active_rules', batch_object: course, batched_keys: :conditional_release) do
rules = course.conditional_release_rules.active.with_assignments.to_a
rules.as_json(include: Rule.includes_for_json, include_root: false, except: [:root_account_id, :deleted_at])
end
trigger_ids = rules_data.map { |rule| rule['trigger_assignment_id'] }
trigger_assgs = course.assignments.preload(:grading_standard).where(id: trigger_ids).each_with_object({}) do |a, assgs|
assgs[a.id] = {
points_possible: a.points_possible,
grading_type: a.grading_type,
grading_scheme: a.uses_grading_standard ? a.grading_scheme : nil,
}
end
rules_data.each do |rule|
rule['trigger_assignment_model'] = trigger_assgs[rule['trigger_assignment_id']]
end
rules_data
end
class << self
private
def config_file
@ -328,8 +323,12 @@ module ConditionalRelease
end
end
def rules_data(context, student, content_tags = [], session = {})
def rules_data(context, student, session = {})
return [] if context.blank? || student.blank?
if natively_enabled_for_account?(context.root_account)
return native_rules_data_for_student(context, student)
end
cached = rules_cache(context, student)
assignments = assignments_for(cached[:rules]) if cached
force_cache = rules_cache_expired?(context, cached)
@ -349,6 +348,59 @@ module ConditionalRelease
[]
end
def native_rules_data_for_student(course, student)
rules_data =
::Rails.cache.fetch(['conditional_release_rules_for_student', student.cache_key(:submissions), course.cache_key(:conditional_release)].cache_key) do
rules = course.conditional_release_rules.active.preload(Rule.preload_associations).to_a
trigger_assignments = course.assignments.where(:id => rules.map(&:trigger_assignment_id)).to_a.index_by(&:id)
trigger_submissions = course.submissions.where(:assignment_id => trigger_assignments.keys).
for_user(student).in_workflow_state(:graded).posted.to_a.index_by(&:assignment_id)
assigned_set_ids = ConditionalRelease::AssignmentSetAction.current_assignments(
student, rules.flat_map(&:scoring_ranges).flat_map(&:assignment_sets)).pluck(:assignment_set_id)
rules.map do |rule|
trigger_assignment = trigger_assignments[rule.trigger_assignment_id]
trigger_sub = trigger_submissions[trigger_assignment.id]
if trigger_sub&.score
relative_score = ConditionalRelease::Stats.percent_from_points(trigger_sub.score, trigger_assignment.points_possible)
assignment_sets = rule.scoring_ranges.select{|sr| sr.contains_score(relative_score)}.flat_map(&:assignment_sets)
selected_set_id =
if assignment_sets.length == 1
assignment_sets.first.id
else
(assignment_sets.map(&:id) & assigned_set_ids).first
end
end
assignment_sets_data = (assignment_sets || []).as_json(
include_root: false, except: [:root_account_id, :deleted_at],
include: {assignment_set_associations: {except: [:root_account_id, :deleted_at]}}
).map(&:deep_symbolize_keys)
rule.as_json(include_root: false, except: [:root_account_id, :deleted_at]).merge(
locked: relative_score.blank?,
selected_set_id: selected_set_id,
assignment_sets: assignment_sets_data
)
end
end
# TODO: do something less weird than mixing AR records into json
# to get the assignment data in when we're not maintaining back compat
referenced_assignment_ids = rules_data.map do |rule_hash|
rule_hash[:assignment_sets].map do |set_hash|
set_hash[:assignment_set_associations].map{|assoc_hash| assoc_hash[:assignment_id]}
end
end.flatten
referenced_assignments = course.assignments.where(:id => referenced_assignment_ids).to_a.index_by(&:id)
rules_data.each do |rule_hash|
rule_hash[:assignment_sets].each do |set_hash|
set_hash[:assignment_set_associations].each do |assoc_hash|
assoc_hash[:model] = referenced_assignments[assoc_hash[:assignment_id]]
end
end
end
rules_data
end
def rules_cache(context, student, force: false, &block)
Rails.cache.fetch(rules_cache_key(context, student), force: force, &block)
end

8
app/models/conditional_release/stats.rb
View File

@ -15,6 +15,8 @@
# 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_dependency 'conditional_release/assignment_set_action'
module ConditionalRelease
module Stats
class << self
@ -78,7 +80,9 @@ module ConditionalRelease
end
end
end
ranges.each do |r|
r[:scoring_range] = r[:scoring_range].as_json(include_root: false, except: [:root_account_id, :deleted_at]) # can't rely on normal json serialization
end
{ rule: rule, ranges: ranges, enrolled: users_by_id.count }
end
@ -120,7 +124,7 @@ module ConditionalRelease
def assignment_detail(assignment, submission, trend_score: nil)
score = submission ? percent_from_points(submission.score, assignment.points_possible) : nil
detail = {
assignment: {id: assignment.id, name: assignment.title, submission_types: assignment.submission_types},
assignment: {id: assignment.id, name: assignment.title, submission_types: assignment.submission_types_array, grading_type: assignment.grading_type},
submission: {id: submission.id, score: submission.score, grade: submission.grade, submitted_at: submission.submitted_at},
score: score
}

2
lib/canvas/cache_register.rb
View File

@ -28,7 +28,7 @@ module Canvas
ALLOWED_TYPES = {
'Account' => %w{account_chain role_overrides global_navigation feature_flags},
'Course' => %w{account_associations},
'Course' => %w{account_associations conditional_release},
'User' => %w{enrollments groups account_users todo_list submissions user_services},
'Assignment' => %w{availability conditional_release},
'Quizzes::Quiz' => %w{availability}

10
spec/apis/v1/context_module_items_api_spec.rb
View File

@ -1036,6 +1036,16 @@ describe "Module Items API", type: :request do
call_select_mastery_path @assignment_tag, 100, @student.id, expected_status: 404
end
it 'should call the native selection path if configured' do
expect(ConditionalRelease::Service).to receive(:natively_enabled_for_account?).and_return(true)
assignment_ids = create_assignments([@course.id], 2)
expect(ConditionalRelease::OverrideHandler).to receive(:handle_assignment_set_selection).
with(@student, @assignment, "100").and_return(assignment_ids)
json = call_select_mastery_path @assignment_tag, "100", @student.id, expected_status: 200
expect(json['assignments'].map {|a| a['id']}).to eq assignment_ids
end
context 'successful' do
def cyoe_returns(assignment_ids)
cyoe_ids = assignment_ids.map {|id| { 'assignment_id' => "#{id}" }} # cyoe ids in strings

43
spec/conditional_release_spec_helper.rb
View File

@ -43,3 +43,46 @@ RSpec.shared_examples 'a soft-deletable model' do
expect { copy.save! }.to_not raise_error
end
end
module ConditionalRelease
module SpecHelper
def setup_course_with_native_conditional_release
Account.default.tap do |ra|
ra.settings[:use_native_conditional_release] = true
ra.save!
end
# set up a trigger assignment with rules and whatnot
course_with_student(:active_all => true)
@trigger_assmt = @course.assignments.create!(:points_possible => 10, submission_types: "online_text_entry")
@sub = @trigger_assmt.submit_homework(@student, body: "hi")
@set1_assmt1 = @course.assignments.create!(:only_visible_to_overrides => true) # one in one set
@set2_assmt1 = @course.assignments.create!(:only_visible_to_overrides => true)
@set2_assmt2 = @course.assignments.create!(:only_visible_to_overrides => true) # two in one set
@set3a_assmt = @course.assignments.create!(:only_visible_to_overrides => true) # two sets in one range - will have to choose
@set3b_assmt = @course.assignments.create!(:only_visible_to_overrides => true)
ranges = [
ScoringRange.new(:lower_bound => 0.7, :upper_bound => 1.0, :assignment_sets => [
AssignmentSet.new(:assignment_set_associations => [AssignmentSetAssociation.new(:assignment_id => @set1_assmt1.id)])
]),
ScoringRange.new(:lower_bound => 0.4, :upper_bound => 0.7, :assignment_sets => [
AssignmentSet.new(:assignment_set_associations => [
AssignmentSetAssociation.new(:assignment_id => @set2_assmt1.id),
AssignmentSetAssociation.new(:assignment_id => @set2_assmt2.id)
])
]),
ScoringRange.new(:lower_bound => 0, :upper_bound => 0.4, :assignment_sets => [
AssignmentSet.new(:assignment_set_associations => [AssignmentSetAssociation.new(:assignment_id => @set3a_assmt.id)]),
AssignmentSet.new(:assignment_set_associations => [AssignmentSetAssociation.new(:assignment_id => @set3b_assmt.id)])
])
]
@rule = @course.conditional_release_rules.create!(:trigger_assignment => @trigger_assmt, :scoring_ranges => ranges)
@course.enable_feature!(:conditional_release)
end
end
end
RSpec.configure do |config|
config.include ConditionalRelease::SpecHelper
end

18
spec/helpers/context_modules_helper_spec.rb
View File

@ -222,15 +222,15 @@ describe ContextModulesHelper do
it "does not affect cache keys unless mastery paths enabled" do
allow(ConditionalRelease::Service).to receive(:enabled_in_context?).and_return(false)
student_in_course(course: t_course, active_all: true)
cache = add_mastery_paths_to_cache_key('foo', t_course, t_module, @student)
cache = add_mastery_paths_to_cache_key('foo', t_course, @student)
expect(cache).to eq 'foo'
end
it "creates the same key for the same mastery paths rules for a student" do
s1 = student_in_course(course: t_course, active_all: true)
s2 = student_in_course(course: t_course, active_all: true)
cache1 = add_mastery_paths_to_cache_key('foo', t_course, t_module, s1.user)
cache2 = add_mastery_paths_to_cache_key('foo', t_course, t_module, s2.user)
cache1 = add_mastery_paths_to_cache_key('foo', t_course, s1.user)
cache2 = add_mastery_paths_to_cache_key('foo', t_course, s2.user)
expect(cache1).not_to eq 'foo'
expect(cache1).to eq cache2
end
@ -238,17 +238,17 @@ describe ContextModulesHelper do
it "creates different keys for different mastery paths rules for a student" do
s1 = student_in_course(course: t_course, active_all: true)
s2 = student_in_course(course: t_course, active_all: true)
cache1 = add_mastery_paths_to_cache_key('foo', t_course, t_module, s1.user)
cache1 = add_mastery_paths_to_cache_key('foo', t_course, s1.user)
allow(ConditionalRelease::Service).to receive(:rules_for).and_return([3, 2, 1])
cache2 = add_mastery_paths_to_cache_key('foo', t_course, t_module, s2.user)
cache2 = add_mastery_paths_to_cache_key('foo', t_course, s2.user)
expect(cache1).not_to eq cache2
end
it "creates the same key for the same mastery paths rules for a teacher" do
t1 = teacher_in_course(course: t_course)
t2 = teacher_in_course(course: t_course)
cache1 = add_mastery_paths_to_cache_key('foo', t_course, t_module, t1.user)
cache2 = add_mastery_paths_to_cache_key('foo', t_course, t_module, t2.user)
cache1 = add_mastery_paths_to_cache_key('foo', t_course, t1.user)
cache2 = add_mastery_paths_to_cache_key('foo', t_course, t2.user)
expect(cache1).not_to eq 'foo'
expect(cache1).to eq cache2
end
@ -256,9 +256,9 @@ describe ContextModulesHelper do
it "creates different keys for different mastery paths rules for a teacher" do
t1 = teacher_in_course(course: t_course)
t2 = teacher_in_course(course: t_course)
cache1 = add_mastery_paths_to_cache_key('foo', t_course, t_module, t1.user)
cache1 = add_mastery_paths_to_cache_key('foo', t_course, t1.user)
allow(ConditionalRelease::Service).to receive(:active_rules).and_return([3, 2, 1])
cache2 = add_mastery_paths_to_cache_key('foo', t_course, t_module, t2.user)
cache2 = add_mastery_paths_to_cache_key('foo', t_course, t2.user)
expect(cache1).not_to eq cache2
end
end

167
spec/models/conditional_release/conditional_release_service_spec.rb
View File

@ -16,7 +16,7 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
require_relative '../../conditional_release_spec_helper'
require File.expand_path(File.dirname(__FILE__) + '/../../sharding_spec_helper')
describe ConditionalRelease::Service do
@ -493,32 +493,6 @@ describe ConditionalRelease::Service do
Service.rule_triggered_by(@a1, @teacher, nil)
end
end
describe 'rules_assigning' do
before(:each) do
allow(Service).to receive(:active_rules).and_return(default_rules)
end
it 'caches the calculation of the reverse index' do
enable_cache do
expect do
Service.rules_assigning(@a1, @teacher, nil)
allow(Service).to receive(:active_rules).and_raise 'should not refetch rules'
Service.rules_assigning(@a2, @teacher, nil)
end.to_not raise_error
end
end
it 'returns all rules which matched assignments' do
expect(Service.rules_assigning(@a2, @teacher, nil).map{|r| r['id']}).to eq [1]
expect(Service.rules_assigning(@a3, @teacher, nil).map{|r| r['id']}).to eq [1, 2]
end
it 'returns nil if no rules matched assignments' do
expect(Service.rules_assigning(@a1, @teacher, nil)).to eq nil
expect(Service.rules_assigning(@a4, @teacher, nil)).to eq nil
end
end
end
describe 'rules_for' do
@ -550,7 +524,7 @@ describe ConditionalRelease::Service do
expect(CanvasHttp).to receive(:post).once.and_return(response)
end
let(:rules) { Service.rules_for(@course, @student, [], nil) }
let(:rules) { Service.rules_for(@course, @student, nil) }
let(:assignments0) { rules[0][:assignment_sets][0][:assignments] }
let(:models0) { assignments0.map{|a| a[:model]} }
@ -601,55 +575,55 @@ describe ConditionalRelease::Service do
it 'uses the cache' do
enable_cache do
expect_cyoe_request '200', @a1
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
Service.rules_for(@course, @student, nil)
end
end
it 'does not use the cache if cache cleared manually' do
enable_cache do
expect_cyoe_request '200', @a1
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
Service.clear_rules_cache_for(@course, @student)
expect_cyoe_request '200', @a1
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
end
it 'does not use the cache if assignments updated' do
enable_cache do
expect_cyoe_request '200', @a1
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
@a1.title = 'updated'
@a1.save!
expect_cyoe_request '200', @a1
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
end
it 'does not use the cache if assignments are saved' do
enable_cache do
expect_cyoe_request '200', @a1
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
@a1.save!
expect_cyoe_request '200', @a1
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
end
it 'does not store an error response in the cache' do
enable_cache do
expect_cyoe_request '404'
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
expect_cyoe_request '404'
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
end
end
@ -686,7 +660,7 @@ describe ConditionalRelease::Service do
submission_model(course: @course, user: @student)
@submission.assignment.grade_student(@student, grade: 10, grader: @teacher)
expect_request_rules(@submission.reload)
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
end
end
@ -699,7 +673,7 @@ describe ConditionalRelease::Service do
s2 = graded_submission_model(course: @course, user: @student)
s2.assignment.grade_student(@student, grade: 10, grader: @teacher)
expect_request_rules(s2.reload)
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
it 'includes only completely graded submissions' do
@ -707,7 +681,7 @@ describe ConditionalRelease::Service do
s1.assignment.grade_student(@student, grade: 10, grader: @teacher)
_s2 = submission_model(course: @course, user: @student)
expect_request_rules(s1.reload)
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
it 'includes only non-muted assignments' do
@ -715,11 +689,11 @@ describe ConditionalRelease::Service do
enable_cache do
@submission.assignment.mute!
expect_request_rules([])
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
@submission.assignment.unmute!
expect_request_rules(@submission)
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
end
@ -735,7 +709,7 @@ describe ConditionalRelease::Service do
enable_cache do
@submission.assignment.post_submissions(submission_ids: [@submission.id])
expect_request_rules(@submission.reload)
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
end
@ -752,7 +726,110 @@ describe ConditionalRelease::Service do
@submission.assignment.post_submissions
@submission.assignment.hide_submissions(submission_ids: [@submission.id])
expect_request_rules([])
Service.rules_for(@course, @student, [], nil)
Service.rules_for(@course, @student, nil)
end
end
end
end
context "native conditional release" do
before :once do
setup_course_with_native_conditional_release
end
context "active_rules" do
it "should show all the rules in the course to teachers" do
data = Service.active_rules(@course, @teacher, nil)
# basically it's just the same thing as returned by the api
expect(data.first["scoring_ranges"].first["assignment_sets"].first["assignment_set_associations"].first["assignment_id"]).to eq @set1_assmt1.id
expect(data.first["trigger_assignment_model"]["points_possible"]).to eq @trigger_assmt.points_possible
end
context "caching" do
specs_require_cache(:redis_cache_store)
it "should cache across admins" do
old_teacher = @teacher
teacher_in_course(:course => @course)
data = Service.active_rules(@course, old_teacher, nil)
@course.conditional_release_rules.update_all(:deleted_at => Time.now.utc) # skip callbacks
expect(Service.active_rules(@course, @teacher, nil)).to eq data # doesn't matter who accesses it if they have rights
end
it "should invalidate cache when a rule is saved" do
Service.active_rules(@course, @teacher, nil)
@rule.update_attribute(:deleted_at, Time.now.utc)
expect(Service.active_rules(@course, @teacher, nil)).to eq []
end
it "should invalidate cache when a trigger assignment is deleted" do
Service.active_rules(@course, @teacher, nil)
@trigger_assmt.destroy
expect(Service.active_rules(@course, @teacher, nil)).to eq []
end
it "should invalidate cache when a releasable assignment is deleted" do
old_data = Service.active_rules(@course, @teacher, nil)
@set1_assmt1.destroy
data = Service.active_rules(@course, @teacher, nil)
expect(data).to_not eq old_data
expect(data.first["scoring_ranges"].first["assignment_sets"].first["assignment_set_associations"]).to eq []
end
end
end
context "rules_for" do
it "should return no assignment set data for unreleased rules" do
data = Service.rules_for(@course, @student, nil)
expect(data.count).to eq 1
rule_hash = data.first
expect(rule_hash['trigger_assignment_id']).to eq @trigger_assmt.id
expect(rule_hash['locked']).to eq true
expect(rule_hash['selected_set_id']).to eq nil
expect(rule_hash['assignment_sets']).to eq []
end
it "should return data about released assignment sets" do
@trigger_assmt.grade_student(@student, grade: 9, grader: @teacher)
rule_hash = Service.rules_for(@course, @student, nil).first
expect(rule_hash['trigger_assignment_id']).to eq @trigger_assmt.id
expect(rule_hash['locked']).to eq false
released_set = @set1_assmt1.conditional_release_associations.first.assignment_set
expect(rule_hash['selected_set_id']).to eq released_set.id
expect(rule_hash['assignment_sets'].count).to eq 1
set_hash = rule_hash['assignment_sets'].first
expect(set_hash['assignment_set_associations'].first['model']).to eq @set1_assmt1
end
it "should return data about multiple assignment set choices" do
@trigger_assmt.grade_student(@student, grade: 2, grader: @teacher) # has two choices now
rule_hash = Service.rules_for(@course, @student, nil).first
expect(rule_hash['trigger_assignment_id']).to eq @trigger_assmt.id
expect(rule_hash['locked']).to eq false
expect(rule_hash['selected_set_id']).to eq nil # neither one was picked yet
expect(rule_hash['assignment_sets'].count).to eq 2
expect(rule_hash['assignment_sets'].map{|s| s['assignment_set_associations'].first['model']}).to match_array([@set3a_assmt, @set3b_assmt])
end
context "caching" do
specs_require_cache(:redis_cache_store)
it "should cache" do
data = Service.rules_for(@course, @student, nil)
@course.conditional_release_rules.update_all(:deleted_at => Time.now.utc) # skip callbacks
expect(Service.rules_for(@course, @student, nil)).to eq data
end
it "should invalidate cache on rule change" do
data = Service.rules_for(@course, @student, nil)
@rule.update_attribute(:deleted_at, Time.now.utc)
expect(Service.rules_for(@course, @student, nil)).to eq []
end
it "should invalidate cache on submission change" do
data = Service.rules_for(@course, @student, nil)
@trigger_assmt.grade_student(@student, grade: 8, grader: @teacher)
expect(Service.rules_for(@course, @student, nil)).to_not eq data
end
end
end

100
spec/models/conditional_release/override_handler_spec.rb
View File

@ -21,43 +21,11 @@ require_dependency "conditional_release/override_handler"
module ConditionalRelease
describe OverrideHandler do
before :once do
setup_course_with_native_conditional_release
end
context 'handle_grade_change' do
before :once do
# set up a trigger assignment with rules and whatnot
course_with_student(:active_all => true)
@trigger_assmt = @course.assignments.create!(:points_possible => 10, submission_types: "online_text_entry")
@sub = @trigger_assmt.submit_homework(@student, body: "hi")
@set1_assmt1 = @course.assignments.create!(:only_visible_to_overrides => true) # one in one set
@set2_assmt1 = @course.assignments.create!(:only_visible_to_overrides => true)
@set2_assmt2 = @course.assignments.create!(:only_visible_to_overrides => true) # two in one set
@set3a_assmt = @course.assignments.create!(:only_visible_to_overrides => true) # two sets in one range - will have to choose
@set3b_assmt = @course.assignments.create!(:only_visible_to_overrides => true)
ranges = [
ScoringRange.new(:lower_bound => 0.7, :upper_bound => 1.0, :assignment_sets => [
AssignmentSet.new(:assignment_set_associations => [AssignmentSetAssociation.new(:assignment_id => @set1_assmt1.id)])
]),
ScoringRange.new(:lower_bound => 0.4, :upper_bound => 0.7, :assignment_sets => [
AssignmentSet.new(:assignment_set_associations => [
AssignmentSetAssociation.new(:assignment_id => @set2_assmt1.id),
AssignmentSetAssociation.new(:assignment_id => @set2_assmt2.id)
])
]),
ScoringRange.new(:lower_bound => 0, :upper_bound => 0.4, :assignment_sets => [
AssignmentSet.new(:assignment_set_associations => [AssignmentSetAssociation.new(:assignment_id => @set3a_assmt.id)]),
AssignmentSet.new(:assignment_set_associations => [AssignmentSetAssociation.new(:assignment_id => @set3b_assmt.id)])
])
]
@rule = @course.conditional_release_rules.create!(:trigger_assignment => @trigger_assmt, :scoring_ranges => ranges)
Account.default.tap do |ra|
ra.settings[:use_native_conditional_release] = true
ra.save!
end
@course.enable_feature!(:conditional_release)
end
it "should require native conditional release" do
expect(ConditionalRelease::Service).to receive(:natively_enabled_for_account?).and_return(false).once
expect(ConditionalRelease::OverrideHandler).to_not receive(:handle_grade_change)
@ -106,5 +74,65 @@ module ConditionalRelease
expect(visible_assmts).to_not include(@set3b_assmt)
end
end
context 'handle_assignment_set_selection' do
before :once do
@trigger_assmt.grade_student(@student, grade: 2, grader: @teacher) # set up the choice
@set_a = @set3a_assmt.conditional_release_associations.first.assignment_set
@set_b = @set3b_assmt.conditional_release_associations.first.assignment_set
@invalid_set = @set1_assmt1.conditional_release_associations.first.assignment_set
end
it "should check that a rule exists for the assignment" do
@rule.destroy!
expect {
ConditionalRelease::OverrideHandler.handle_assignment_set_selection(@student, @trigger_assmt, @set_a.id)
}.to raise_error(ActiveRecord::RecordNotFound)
end
it "should check that the submission is actually graded" do
Submission.where(:id => @sub).update_all(:posted_at => nil)
expect {
ConditionalRelease::OverrideHandler.handle_assignment_set_selection(@student, @trigger_assmt, @set_a.id)
}.to raise_error(ActiveRecord::RecordNotFound)
end
it "should check that the assignment set is valid for the submissions core" do
expect {
ConditionalRelease::OverrideHandler.handle_assignment_set_selection(@student, @trigger_assmt, @invalid_set.id)
}.to raise_error(ActiveRecord::RecordNotFound)
end
it "should create the assignment override" do
assignment_ids = ConditionalRelease::OverrideHandler.handle_assignment_set_selection(@student, @trigger_assmt, @set_a.id)
expect(assignment_ids).to eq [@set3a_assmt.id]
visible_assmts = DifferentiableAssignment.scope_filter(@course.assignments, @student, @course).to_a
expect(visible_assmts).to include(@set3a_assmt)
expect(visible_assmts).to_not include(@set3b_assmt)
end
it "should be able to switch" do
ConditionalRelease::OverrideHandler.handle_assignment_set_selection(@student, @trigger_assmt, @set_a.id)
ConditionalRelease::OverrideHandler.handle_assignment_set_selection(@student, @trigger_assmt, @set_b.id)
visible_assmts = DifferentiableAssignment.scope_filter(@course.assignments, @student, @course).to_a
expect(visible_assmts).to include(@set3b_assmt)
expect(visible_assmts).to_not include(@set3a_assmt)
end
it "should reuse an existing override when assigning (and leave it be when unassigning)" do
old_student = @student
ConditionalRelease::OverrideHandler.handle_assignment_set_selection(old_student, @trigger_assmt, @set_a.id)
student_in_course(:course => @course, :active_all => true)
@trigger_assmt.grade_student(@student, grade: 3, grader: @teacher)
ConditionalRelease::OverrideHandler.handle_assignment_set_selection(@student, @trigger_assmt, @set_a.id)
expect(@set3a_assmt.assignment_overrides.count).to eq 1
expect(@set3a_assmt.assignment_overrides.first.assignment_override_students.count).to eq 2
ConditionalRelease::OverrideHandler.handle_assignment_set_selection(@student, @trigger_assmt, @set_b.id) # now unassign
expect(DifferentiableAssignment.scope_filter(@course.assignments, @student, @course).to_a).to_not include(@set3a_assmt)
expect(DifferentiableAssignment.scope_filter(@course.assignments, old_student, @course).to_a).to include(@set3a_assmt)
end
end
end
end