Skip to content

Commit 903e349

Browse files
committed
refactor evaluate/deep_evaluate logic
test plan: * full regression of all module progression functionality closes CNVS-11505 Change-Id: I63f542d5d42f950cf9c8117e12ecce4518e2ccf4 Reviewed-on: https://gerrit.instructure.com/30983 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Simon Williams <simon@instructure.com> QA-Review: Matt Fairbourn <mfairbourn@instructure.com> Reviewed-by: Jeremy Stanley <jeremy@instructure.com> Product-Review: Jeremy Stanley <jeremy@instructure.com>
1 parent 55f75e8 commit 903e349

9 files changed

Lines changed: 305 additions & 283 deletions

app/controllers/context_modules_controller.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ def reevaluate_modules_if_locked(tag)
7676
if tag.content && tag.content.respond_to?(:locked_for?)
7777
locked = tag.content.locked_for?(@current_user, :context => @context)
7878
if locked
79-
@context.context_modules.active.each { |m| m.evaluate_for(@current_user, true, true) }
79+
@context.context_modules.active.each { |m| m.evaluate_for(@current_user, true) }
8080
if tag.content.respond_to?(:clear_locked_cache)
8181
tag.content.clear_locked_cache(@current_user)
8282
end
@@ -362,7 +362,7 @@ def progressions
362362
if request.format == :json
363363
if @context.context_modules.scoped.new.grants_right?(@current_user, session, :update)
364364
if params[:user_id] && @user = @context.students.find(params[:user_id])
365-
@progressions = @context.context_modules.active.map{|m| m.evaluate_for(@user, true, true) }
365+
@progressions = @context.context_modules.active.map{|m| m.evaluate_for(@user, true) }
366366
else
367367
if @context.large_roster
368368
@progressions = []

app/models/content_tag.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,10 @@ def content_type_quiz?
346346
Quizzes::Quiz.class_names.include?(self.content_type)
347347
end
348348

349+
def content_type_discussion?
350+
'DiscussionTopic' == self.content_type
351+
end
352+
349353
def context_module_action(user, action, points=nil)
350354
self.context_module.update_for(user, action, self, points) if self.context_module
351355
end

app/models/context_module.rb

Lines changed: 15 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ def update_student_progressions(user=nil)
126126
:context_type => self.context_type, :context_id => self.context_id, :workflow_state => 'active')
127127
students = user ? [user] : self.context.students
128128
modules.each do |mod|
129-
mod.re_evaluate_for(students, true)
129+
mod.re_evaluate_for(students)
130130
end
131131
end
132132

@@ -163,7 +163,7 @@ def available_for?(user, opts={})
163163
res = progression && !progression.locked? && progression.current_position && progression.current_position >= tag.position
164164
end
165165
if !res && opts[:deep_check_if_needed]
166-
progression = self.evaluate_for(user, true, true)
166+
progression = self.evaluate_for(user, true)
167167
if tag && tag.context_module_id == self.id && self.require_sequential_progress
168168
res = progression && !progression.locked? && progression.current_position && progression.current_position >= tag.position
169169
end
@@ -339,7 +339,7 @@ def add_item(params, added_item=nil, opts={})
339339

340340
def update_for(user, action, tag, points=nil)
341341
return nil unless self.context.users.include?(user)
342-
return nil unless self.prerequisites_satisfied?(user)
342+
return nil unless ContextModuleProgression.prerequisites_satisfied?(user, self)
343343
progression = self.find_or_create_progression(user)
344344
return unless progression
345345
progression.requirements_met ||= []
@@ -377,54 +377,34 @@ def self.requirement_description(req)
377377
nil
378378
end
379379
end
380-
381-
382-
def prerequisites_satisfied?(user)
383-
unlocked = (self.active_prerequisites || []).all? do |pre|
384-
if pre[:type] == 'context_module'
385-
prog = user.module_progression_for(pre[:id])
386-
if prog
387-
prog.completed?
388-
elsif pre[:id].present?
389-
if prereq = self.context.context_modules.active.find_by_id(pre[:id])
390-
prog = prereq.evaluate_for(user, true)
391-
prog.completed?
392-
else
393-
true
394-
end
395-
else
396-
true
397-
end
398-
else
399-
true
400-
end
401-
end
402-
unlocked
403-
end
404380

405381
def active_prerequisites
406382
return [] unless self.prerequisites.any?
407383
prereq_ids = self.prerequisites.select{|pre|pre[:type] == 'context_module'}.map{|pre| pre[:id] }
408384
active_ids = self.context.context_modules.active.where(:id => prereq_ids).pluck(:id)
409385
self.prerequisites.select{|pre| pre[:type] == 'context_module' && active_ids.member?(pre[:id])}
410386
end
387+
388+
def reload
389+
clear_cached_lookups
390+
super
391+
end
411392

412393
def clear_cached_lookups
413-
@cached_tags = nil
394+
@cached_active_tags = nil
414395
end
415396

416-
def cached_tags
417-
@cached_tags ||= self.content_tags.active
397+
def cached_active_tags
398+
@cached_active_tags ||= self.content_tags.active
418399
end
419400

420-
def re_evaluate_for(users, skip_confirm_valid_requirements=false)
401+
def re_evaluate_for(users)
421402
users = Array(users)
422403
users.each{|u| u.clear_cached_lookups }
423404
progressions = self.find_or_create_progressions(users)
424405
progressions.each(&:mark_as_dirty)
425-
@already_confirmed_valid_requirements = true if skip_confirm_valid_requirements
426406
progressions.each do |progression|
427-
self.evaluate_for(progression, true, true)
407+
self.evaluate_for(progression, true)
428408
end
429409
end
430410

@@ -491,15 +471,8 @@ def find_or_create_progression(user)
491471
def find_or_create_progression_with_multiple_lookups(user)
492472
user.module_progression_for(self.id) || self.find_or_create_progression(user)
493473
end
494-
495-
def content_tags_hash
496-
return @tags_hash if @tags_hash
497-
@tags_hash = {}
498-
self.content_tags.each{|t| @tags_hash[t.id] = t }
499-
@tags_hash
500-
end
501474

502-
def evaluate_for(user_or_progression, recursive_check=false, deep_check=false)
475+
def evaluate_for(user_or_progression, force_evaluate_requirements=false)
503476
if user_or_progression.is_a?(ContextModuleProgression)
504477
progression, user = [user_or_progression, user_or_progression.user]
505478
else
@@ -509,7 +482,7 @@ def evaluate_for(user_or_progression, recursive_check=false, deep_check=false)
509482

510483
progression.context_module = self if progression.context_module_id == self.id
511484
progression.user = user if progression.user_id == user.id
512-
progression.evaluate(recursive_check, deep_check)
485+
progression.evaluate(force_evaluate_requirements)
513486
progression
514487
end
515488

0 commit comments

Comments
 (0)