Skip to content

Commit c8d2f39

Browse files
author
Yuliya Bozhko
committed
MDL-42965 badges: Improve badge criteria review performance
1 parent 9b37cd7 commit c8d2f39

10 files changed

Lines changed: 356 additions & 95 deletions

badges/criteria/award_criteria.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,20 @@ public function config_form_criteria($data) {
236236
* Review this criteria and decide if the user has completed
237237
*
238238
* @param int $userid User whose criteria completion needs to be reviewed.
239+
* @param bool $filtered An additional parameter indicating that user list
240+
* has been reduced and some expensive checks can be skipped.
241+
*
239242
* @return bool Whether criteria is complete
240243
*/
241-
abstract public function review($userid);
244+
abstract public function review($userid, $filtered = false);
245+
246+
/**
247+
* Returns array with sql code and parameters returning all ids
248+
* of users who meet this particular criterion.
249+
*
250+
* @return array list($join, $where, $params)
251+
*/
252+
abstract public function get_completed_criteria_sql();
242253

243254
/**
244255
* Mark this criteria as complete for a user

badges/criteria/award_criteria_activity.php

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,20 @@ class award_criteria_activity extends award_criteria {
3737
public $criteriatype = BADGE_CRITERIA_TYPE_ACTIVITY;
3838

3939
private $courseid;
40+
private $coursestartdate;
4041

4142
public $required_param = 'module';
4243
public $optional_params = array('bydate');
4344

4445
public function __construct($record) {
46+
global $DB;
4547
parent::__construct($record);
46-
$this->courseid = self::get_course();
48+
49+
$course = $DB->get_record_sql('SELECT b.courseid, c.startdate
50+
FROM {badge} b INNER JOIN {course} c ON b.courseid = c.id
51+
WHERE b.id = :badgeid ', array('badgeid' => $this->badgeid));
52+
$this->courseid = $course->courseid;
53+
$this->coursestartdate = $course->startdate;
4754
}
4855

4956
/**
@@ -95,17 +102,6 @@ public function get_details($short = '') {
95102
}
96103
}
97104

98-
/**
99-
* Return course ID for activities
100-
*
101-
* @return int
102-
*/
103-
private function get_course() {
104-
global $DB;
105-
$courseid = $DB->get_field('badge', 'courseid', array('id' => $this->badgeid));
106-
return $courseid;
107-
}
108-
109105
/**
110106
* Add appropriate new criteria options to the form
111107
*
@@ -184,14 +180,17 @@ public function get_options(&$mform) {
184180
* Review this criteria and decide if it has been completed
185181
*
186182
* @param int $userid User whose criteria completion needs to be reviewed.
183+
* @param bool $filtered An additional parameter indicating that user list
184+
* has been reduced and some expensive checks can be skipped.
185+
*
187186
* @return bool Whether criteria is complete
188187
*/
189-
public function review($userid) {
190-
global $DB;
188+
public function review($userid, $filtered = false) {
191189
$completionstates = array(COMPLETION_COMPLETE, COMPLETION_COMPLETE_PASS);
192-
$course = $DB->get_record('course', array('id' => $this->courseid));
190+
$course = new stdClass();
191+
$course->id = $this->courseid;
193192

194-
if ($course->startdate > time()) {
193+
if ($this->coursestartdate > time()) {
195194
return false;
196195
}
197196

@@ -217,7 +216,7 @@ public function review($userid) {
217216
} else {
218217
return false;
219218
}
220-
} else if ($this->method == BADGE_CRITERIA_AGGREGATION_ANY) {
219+
} else {
221220
if (in_array($data->completionstate, $completionstates) && $check_date) {
222221
return true;
223222
} else {
@@ -229,4 +228,44 @@ public function review($userid) {
229228

230229
return $overall;
231230
}
231+
232+
/**
233+
* Returns array with sql code and parameters returning all ids
234+
* of users who meet this particular criterion.
235+
*
236+
* @return array list($join, $where, $params)
237+
*/
238+
public function get_completed_criteria_sql() {
239+
$join = '';
240+
$where = '';
241+
$params = array();
242+
243+
if ($this->method == BADGE_CRITERIA_AGGREGATION_ANY) {
244+
foreach ($this->params as $param) {
245+
$moduledata[] = " cmc.coursemoduleid = :completedmodule{$param['module']} ";
246+
$params["completedmodule{$param['module']}"] = $param['module'];
247+
}
248+
if (!empty($moduledata)) {
249+
$extraon = implode(' OR ', $moduledata);
250+
$join = " JOIN {course_modules_completion} cmc ON cmc.userid = u.id AND
251+
( cmc.completionstate = :completionpass OR cmc.completionstate = :completioncomplete ) AND ({$extraon})";
252+
$params["completionpass"] = COMPLETION_COMPLETE_PASS;
253+
$params["completioncomplete"] = COMPLETION_COMPLETE;
254+
}
255+
return array($join, $where, $params);
256+
} else {
257+
foreach ($this->params as $param) {
258+
$join .= " LEFT JOIN {course_modules_completion} cmc{$param['module']} ON
259+
cmc{$param['module']}.userid = u.id AND
260+
cmc{$param['module']}.coursemoduleid = :completedmodule{$param['module']} AND
261+
( cmc{$param['module']}.completionstate = :completionpass{$param['module']} OR
262+
cmc{$param['module']}.completionstate = :completioncomplete{$param['module']} )";
263+
$where .= " AND cmc{$param['module']}.coursemoduleid IS NOT NULL ";
264+
$params["completedmodule{$param['module']}"] = $param['module'];
265+
$params["completionpass{$param['module']}"] = COMPLETION_COMPLETE_PASS;
266+
$params["completioncomplete{$param['module']}"] = COMPLETION_COMPLETE;
267+
}
268+
return array($join, $where, $params);
269+
}
270+
}
232271
}

badges/criteria/award_criteria_course.php

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,23 @@ class award_criteria_course extends award_criteria {
3838
/* @var int Criteria [BADGE_CRITERIA_TYPE_COURSE] */
3939
public $criteriatype = BADGE_CRITERIA_TYPE_COURSE;
4040

41+
private $courseid;
42+
private $coursestartdate;
43+
4144
public $required_param = 'course';
4245
public $optional_params = array('grade', 'bydate');
4346

47+
public function __construct($record) {
48+
global $DB;
49+
parent::__construct($record);
50+
51+
$course = $DB->get_record_sql('SELECT b.courseid, c.startdate
52+
FROM {badge} b INNER JOIN {course} c ON b.courseid = c.id
53+
WHERE b.id = :badgeid ', array('badgeid' => $this->badgeid));
54+
$this->courseid = $course->courseid;
55+
$this->coursestartdate = $course->startdate;
56+
}
57+
4458
/**
4559
* Add appropriate form elements to the criteria form
4660
*
@@ -151,18 +165,22 @@ public function get_options(&$mform) {
151165
* Review this criteria and decide if it has been completed
152166
*
153167
* @param int $userid User whose criteria completion needs to be reviewed.
168+
* @param bool $filtered An additional parameter indicating that user list
169+
* has been reduced and some expensive checks can be skipped.
170+
*
154171
* @return bool Whether criteria is complete
155172
*/
156-
public function review($userid) {
157-
global $DB;
158-
foreach ($this->params as $param) {
159-
$course = $DB->get_record('course', array('id' => $param['course']));
173+
public function review($userid, $filtered = false) {
174+
$course = new stdClass();
175+
$course->id = $this->courseid;
160176

161-
if ($course->startdate > time()) {
162-
return false;
163-
}
177+
if ($this->coursestartdate > time()) {
178+
return false;
179+
}
164180

165-
$info = new completion_info($course);
181+
$info = new completion_info($course);
182+
183+
foreach ($this->params as $param) {
166184
$check_grade = true;
167185
$check_date = true;
168186

@@ -171,7 +189,7 @@ public function review($userid) {
171189
$check_grade = ($grade->grade >= $param['grade']);
172190
}
173191

174-
if (isset($param['bydate'])) {
192+
if (!$filtered && isset($param['bydate'])) {
175193
$cparams = array(
176194
'userid' => $userid,
177195
'course' => $course->id,
@@ -188,4 +206,27 @@ public function review($userid) {
188206

189207
return false;
190208
}
191-
}
209+
210+
/**
211+
* Returns array with sql code and parameters returning all ids
212+
* of users who meet this particular criterion.
213+
*
214+
* @return array list($join, $where, $params)
215+
*/
216+
public function get_completed_criteria_sql() {
217+
// We have only one criterion here, so taking the first one.
218+
$coursecriteria = reset($this->params);
219+
220+
$join = " LEFT JOIN {course_completions} cc ON cc.userid = u.id AND cc.timecompleted > 0";
221+
$where = ' AND cc.course = :courseid ';
222+
$params['courseid'] = $this->courseid;
223+
224+
// Add by date parameter.
225+
if (isset($param['bydate'])) {
226+
$where .= ' AND cc.timecompleted <= :completebydate';
227+
$params['completebydate'] = $coursecriteria['bydate'];
228+
}
229+
230+
return array($join, $where, $params);
231+
}
232+
}

badges/criteria/award_criteria_courseset.php

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,17 @@ public function get_options(&$mform) {
202202
/**
203203
* Review this criteria and decide if it has been completed
204204
*
205+
* @param int $userid User whose criteria completion needs to be reviewed.
206+
* @param bool $filtered An additional parameter indicating that user list
207+
* has been reduced and some expensive checks can be skipped.
208+
*
205209
* @return bool Whether criteria is complete
206210
*/
207-
public function review($userid) {
208-
global $DB;
211+
public function review($userid, $filtered = false) {
209212
foreach ($this->params as $param) {
210-
$course = $DB->get_record('course', array('id' => $param['course']));
213+
$course = new stdClass();
214+
$course->id = $param['course'];
215+
211216
$info = new completion_info($course);
212217
$check_grade = true;
213218
$check_date = true;
@@ -217,7 +222,7 @@ public function review($userid) {
217222
$check_grade = ($grade->grade >= $param['grade']);
218223
}
219224

220-
if (isset($param['bydate'])) {
225+
if (!$filtered && isset($param['bydate'])) {
221226
$cparams = array(
222227
'userid' => $userid,
223228
'course' => $course->id,
@@ -235,7 +240,7 @@ public function review($userid) {
235240
} else {
236241
return false;
237242
}
238-
} else if ($this->method == BADGE_CRITERIA_AGGREGATION_ANY) {
243+
} else {
239244
if ($info->is_course_complete($userid) && $check_grade && $check_date) {
240245
return true;
241246
} else {
@@ -247,4 +252,39 @@ public function review($userid) {
247252

248253
return $overall;
249254
}
255+
256+
/**
257+
* Returns array with sql code and parameters returning all ids
258+
* of users who meet this particular criterion.
259+
*
260+
* @return array list($join, $where, $params)
261+
*/
262+
public function get_completed_criteria_sql() {
263+
$join = '';
264+
$where = '';
265+
$params = array();
266+
267+
if ($this->method == BADGE_CRITERIA_AGGREGATION_ANY) {
268+
foreach ($this->params as $param) {
269+
$coursedata[] = " cc.course = :completedcourse{$param['course']} ";
270+
$params["completedcourse{$param['course']}"] = $param['course'];
271+
}
272+
if (!empty($coursedata)) {
273+
$extraon = implode(' OR ', $coursedata);
274+
$join = " JOIN {course_completions} cc ON cc.userid = u.id AND
275+
cc.timecompleted > 0 AND ({$extraon})";
276+
}
277+
return array($join, $where, $params);
278+
} else {
279+
foreach ($this->params as $param) {
280+
$join .= " LEFT JOIN {course_completions} cc{$param['course']} ON
281+
cc{$param['course']}.userid = u.id AND
282+
cc{$param['course']}.course = :completedcourse{$param['course']} AND
283+
cc{$param['course']}.timecompleted > 0 ";
284+
$where .= " AND cc{$param['course']}.course IS NOT NULL ";
285+
$params["completedcourse{$param['course']}"] = $param['course'];
286+
}
287+
return array($join, $where, $params);
288+
}
289+
}
250290
}

badges/criteria/award_criteria_manual.php

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,19 @@ public function get_details($short = '') {
142142
* Review this criteria and decide if it has been completed
143143
*
144144
* @param int $userid User whose criteria completion needs to be reviewed.
145+
* @param bool $filtered An additional parameter indicating that user list
146+
* has been reduced and some expensive checks can be skipped.
147+
*
145148
* @return bool Whether criteria is complete
146149
*/
147-
public function review($userid) {
150+
public function review($userid, $filtered = false) {
148151
global $DB;
149152

153+
// Users were already filtered by criteria completion.
154+
if ($filtered) {
155+
return true;
156+
}
157+
150158
$overall = false;
151159
foreach ($this->params as $param) {
152160
$crit = $DB->get_record('badge_manual_award', array('issuerrole' => $param['role'], 'recipientid' => $userid, 'badgeid' => $this->badgeid));
@@ -157,7 +165,7 @@ public function review($userid) {
157165
$overall = true;
158166
continue;
159167
}
160-
} else if ($this->method == BADGE_CRITERIA_AGGREGATION_ANY) {
168+
} else {
161169
if (!$crit) {
162170
$overall = false;
163171
continue;
@@ -169,6 +177,41 @@ public function review($userid) {
169177
return $overall;
170178
}
171179

180+
/**
181+
* Returns array with sql code and parameters returning all ids
182+
* of users who meet this particular criterion.
183+
*
184+
* @return array list($join, $where, $params)
185+
*/
186+
public function get_completed_criteria_sql() {
187+
$join = '';
188+
$where = '';
189+
$params = array();
190+
191+
if ($this->method == BADGE_CRITERIA_AGGREGATION_ANY) {
192+
foreach ($this->params as $param) {
193+
$roledata[] = " bma.issuerrole = :issuerrole{$param['role']} ";
194+
$params["issuerrole{$param['role']}"] = $param['role'];
195+
}
196+
if (!empty($roledata)) {
197+
$extraon = implode(' OR ', $roledata);
198+
$join = " JOIN {badge_manual_award} bma ON bma.recipientid = u.id
199+
AND bma.badgeid = :badgeid{$this->badgeid} AND ({$extraon})";
200+
$params["badgeid{$this->badgeid}"] = $this->badgeid;
201+
}
202+
return array($join, $where, $params);
203+
} else {
204+
foreach ($this->params as $param) {
205+
$join .= " LEFT JOIN {badge_manual_award} bma{$param['role']} ON
206+
bma{$param['role']}.recipientid = u.id AND
207+
bma{$param['role']}.issuerrole = :issuerrole{$param['role']} ";
208+
$where .= " AND bma{$param['role']}.issuerrole IS NOT NULL ";
209+
$params["issuerrole{$param['role']}"] = $param['role'];
210+
}
211+
return array($join, $where, $params);
212+
}
213+
}
214+
172215
/**
173216
* Delete this criterion
174217
*

0 commit comments

Comments
 (0)