Skip to content

Commit 82e46a9

Browse files
committed
refactor progression deep_evaluate
test plan: * simple regression on student module progression closes CNVS-11483 Change-Id: I58cfd4f71d8fb2fe597f9f0d72523e07cc04de75 Reviewed-on: https://gerrit.instructure.com/30906 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Jeremy Stanley <jeremy@instructure.com> Product-Review: Jeremy Stanley <jeremy@instructure.com> QA-Review: Matt Fairbourn <mfairbourn@instructure.com>
1 parent 13e348e commit 82e46a9

3 files changed

Lines changed: 91 additions & 44 deletions

File tree

app/models/context_module.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ def locked_for?(user, opts={})
149149
def available_for?(user, opts={})
150150
return true if self.active? && !self.to_be_unlocked && self.prerequisites.blank? && !self.require_sequential_progress
151151
if self.grants_right?(user, nil, :update)
152-
return true
152+
return true
153153
elsif !self.active?
154154
return false
155155
end
@@ -417,7 +417,7 @@ def re_evaluate_for(users, skip_confirm_valid_requirements=false)
417417
users = Array(users)
418418
users.each{|u| u.clear_cached_lookups }
419419
progressions = self.find_or_create_progressions(users)
420-
progressions.each{|p| p.workflow_state = 'locked' }
420+
progressions.each(&:mark_as_dirty)
421421
@already_confirmed_valid_requirements = true if skip_confirm_valid_requirements
422422
progressions.each do |progression|
423423
self.evaluate_for(progression, true, true)
@@ -542,7 +542,7 @@ def evaluate_for(user, recursive_check=false, deep_check=false)
542542
elsif ['min_score', 'max_score', 'must_submit'].include?(req[:type]) && !tag.scoreable?
543543
res = true
544544
else
545-
progression.deep_evaluate(self) if deep_check
545+
progression.evaluate_requirements_met if deep_check
546546
res = progression.requirements_met.any?{|r| r[:id] == req[:id] && r[:type] == req[:type] } #include?(req)
547547
if req[:type] == 'min_score'
548548
progression.requirements_met = progression.requirements_met.select{|r| r[:id] != req[:id] || r[:type] != req[:type]}

app/models/context_module_progression.rb

Lines changed: 86 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -46,54 +46,101 @@ def uncollapse!
4646
self.save
4747
end
4848

49-
def deep_evaluate(mod)
50-
mod = nil if mod && mod.id != self.context_module_id
51-
mod ||= self.context_module
52-
return if mod.completion_requirements.blank?
53-
tags_hash = mod.content_tags_hash
49+
def evaluate_requirements_met
5450
met = self.requirements_met || []
5551
orig_reqs = met.map{|r| "#{r[:id]}_#{r[:type]}"}.sort
56-
(mod.completion_requirements || []).each do |req|
57-
next if met.any?{|r| r[:id] == req[:id] && r[:type] == req[:type]}
58-
tag = tags_hash[req[:id]]
59-
if !req[:id] || !tag
60-
met << req
52+
53+
met = evaluate_uncompleted_requirements(met)
54+
new_reqs = met.map{|r| "#{r[:id]}_#{r[:type]}"}.sort
55+
56+
if orig_reqs != new_reqs
57+
self.requirements_met = met
58+
self.save
59+
end
60+
end
61+
62+
def evaluate_uncompleted_requirements(met)
63+
met = met.dup
64+
uncompleted_view_reqs = []
65+
context_module.completion_requirements.each do |req|
66+
next if requirement_met?(req, met)
67+
68+
req_met = nil
69+
tag = context_module.content_tags_hash[req[:id]]
70+
if !tag
71+
req_met = req
6172
elsif req[:type] == "must_view"
62-
view = false
63-
view = true if met.any?{|r| r[:id] == req[:id] }
64-
met << req if view
73+
uncompleted_view_reqs << req
6574
elsif req[:type] == "must_contribute"
6675

6776
elsif req[:type] == "must_submit"
68-
obj = tag.content
69-
if obj.is_a?(DiscussionTopic)
70-
obj = obj.assignment
71-
end
72-
if obj.is_a?(Assignment)
73-
met << req if self.user.submitted_submission_for(obj.id)
74-
elsif obj.is_a?(Quizzes::Quiz)
75-
met << req if self.user.attempted_quiz_submission_for(obj.id)
76-
end
77+
req_met = evaluate_submit_requirement_met(req, tag)
7778
elsif req[:type] == "max_score" || req[:type] == "min_score"
78-
obj = tag.content
79-
sub = nil
80-
if obj.is_a?(Assignment)
81-
sub = self.user.submitted_submission_for(obj.id)
82-
score = sub.try(:score)
83-
elsif obj.is_a?(Quizzes::Quiz)
84-
sub = self.user.attempted_quiz_submission_for(obj.id)
85-
score = sub.try(:kept_score)
86-
end
87-
if req[:type == "max_score"]
88-
met << req if score && score <= req[:max_score].to_f
89-
else
90-
met << req if score && score >= req[:min_score].to_f
91-
end
79+
req_met = evaluate_score_requirement_met(req, tag)
9280
end
81+
82+
met << req_met if req_met
83+
end
84+
85+
uncompleted_view_reqs.each do |req|
86+
met << req if other_requirement_met?(req, met)
9387
end
94-
new_reqs = met.map{|r| "#{r[:id]}_#{r[:type]}"}.sort
95-
self.requirements_met = met
96-
self.save if orig_reqs != new_reqs
88+
89+
met
90+
end
91+
private :evaluate_uncompleted_requirements
92+
93+
def requirement_met?(req, met_reqs)
94+
met_reqs.any? {|r| r[:id] == req[:id] && r[:type] == req[:type] }
95+
end
96+
97+
def other_requirement_met?(req, met_reqs)
98+
met_reqs.any? {|r| r[:id] == req[:id] }
99+
end
100+
101+
def get_submission_score(tag_content)
102+
if tag_content.is_a?(Assignment)
103+
submission = self.user.submitted_submission_for(tag_content.id)
104+
submission.try(:score)
105+
elsif tag_content.is_a?(Quizzes::Quiz)
106+
submission = self.user.attempted_quiz_submission_for(tag_content.id)
107+
submission.try(:kept_score)
108+
end
109+
end
110+
private :get_submission_score
111+
112+
def evaluate_score_requirement_met(requirement, tag)
113+
score = get_submission_score(tag.content)
114+
if requirement[:type] == "max_score"
115+
requirement if score && score <= requirement[:max_score].to_f
116+
else
117+
requirement if score && score >= requirement[:min_score].to_f
118+
end
119+
end
120+
private :evaluate_score_requirement_met
121+
122+
def evaluate_submit_requirement_met(requirement, tag)
123+
content = tag.content
124+
content = content.assignment if content.is_a?(DiscussionTopic)
125+
if content.is_a?(Assignment)
126+
requirement if self.user.submitted_submission_for(content.id)
127+
elsif content.is_a?(Quizzes::Quiz)
128+
requirement if self.user.attempted_quiz_submission_for(content.id)
129+
end
130+
end
131+
private :evaluate_submit_requirement_met
132+
133+
def mark_as_dirty
134+
self.workflow_state = 'locked'
135+
nil
136+
end
137+
138+
def mark_as_dirty!
139+
mark_as_dirty
140+
Shackles.activate(:master) do
141+
self.save if self.workflow_state_changed?
142+
end
143+
nil
97144
end
98145

99146
def trigger_completion_events

app/models/user.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,13 +1140,13 @@ def show_in_inbox?(type)
11401140
end
11411141
end
11421142

1143-
# only used by ContextModuleProgression#deep_evaluate
1143+
# only used by ContextModuleProgression#evaluate_uncompleted_requirements
11441144
def submitted_submission_for(assignment_id)
11451145
@submissions ||= self.submissions.having_submission.except(:includes).select([:id, :score, :assignment_id]).all
11461146
@submissions.detect{|s| s.assignment_id == assignment_id }
11471147
end
11481148

1149-
# only used by ContextModuleProgression#deep_evaluate
1149+
# only used by ContextModuleProgression#evaluate_uncompleted_requirements
11501150
def attempted_quiz_submission_for(quiz_id)
11511151
@quiz_submissions ||= self.quiz_submissions.select([:id, :kept_score, :quiz_id, :workflow_state]).select{|s| !s.settings_only? }
11521152
@quiz_submissions.detect{|qs| qs.quiz_id == quiz_id }

0 commit comments

Comments
 (0)