From 4f2424ca1cc9d5cc840d7500560adb669255bb2b Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Tue, 15 Sep 2015 11:17:21 +0800 Subject: [PATCH] MDL-51408 badges: fix criteria for custom profile fields --- badges/criteria/award_criteria_profile.php | 57 ++++++++++------------ badges/tests/badgeslib_test.php | 29 ++++++++++- 2 files changed, 53 insertions(+), 33 deletions(-) diff --git a/badges/criteria/award_criteria_profile.php b/badges/criteria/award_criteria_profile.php index 18915a0afd2..65c2ddaadfe 100644 --- a/badges/criteria/award_criteria_profile.php +++ b/badges/criteria/award_criteria_profile.php @@ -170,33 +170,31 @@ class award_criteria_profile extends award_criteria { } $join = ''; - $where = ''; + $whereparts = array(); $sqlparams = array(); $rule = ($this->method == BADGE_CRITERIA_AGGREGATION_ANY) ? ' OR ' : ' AND '; foreach ($this->params as $param) { if (is_numeric($param['field'])) { - $infodata[] = " uid.fieldid = :fieldid{$param['field']} "; - $sqlparams["fieldid{$param['field']}"] = $param['field']; + // This is a custom field. + $idx = count($whereparts) + 1; + $join .= " LEFT JOIN {user_info_data} uid{$idx} ON uid{$idx}.userid = u.id AND uid{$idx}.fieldid = :fieldid{$idx} "; + $sqlparams["fieldid{$idx}"] = $param['field']; + $whereparts[] = "uid{$idx}.id IS NOT NULL"; } else { - $userdata[] = $DB->sql_isnotempty('u', "u.{$param['field']}", false, true); + // This is a field from {user} table. + $whereparts[] = $DB->sql_isnotempty('u', "u.{$param['field']}", false, true); } } - // Add user custom field parameters if there are any. - if (!empty($infodata)) { - $extraon = implode($rule, $infodata); - $join = " LEFT JOIN {user_info_data} uid ON uid.userid = u.id AND ({$extraon})"; - } - - // Add user table field parameters if there are any. - if (!empty($userdata)) { - $extraon = implode($rule, $userdata); - $where = " AND ({$extraon})"; - } - $sqlparams['userid'] = $userid; - $sql = "SELECT u.* FROM {user} u " . $join . " WHERE u.id = :userid " . $where; + + if ($whereparts) { + $where = " AND (" . implode($rule, $whereparts) . ")"; + } else { + $where = ''; + } + $sql = "SELECT 1 FROM {user} u " . $join . " WHERE u.id = :userid $where"; $overall = $DB->record_exists_sql($sql, $sqlparams); return $overall; @@ -212,29 +210,26 @@ class award_criteria_profile extends award_criteria { global $DB; $join = ''; - $where = ''; + $whereparts = array(); $params = array(); $rule = ($this->method == BADGE_CRITERIA_AGGREGATION_ANY) ? ' OR ' : ' AND '; foreach ($this->params as $param) { if (is_numeric($param['field'])) { - $infodata[] = " uid.fieldid = :fieldid{$param['field']} "; - $params["fieldid{$param['field']}"] = $param['field']; + // This is a custom field. + $idx = count($whereparts); + $join .= " LEFT JOIN {user_info_data} uid{$idx} ON uid{$idx}.userid = u.id AND uid{$idx}.fieldid = :fieldid{$idx} "; + $params["fieldid{$idx}"] = $param['field']; + $whereparts[] = "uid{$idx}.id IS NOT NULL"; } else { - $userdata[] = $DB->sql_isnotempty('u', "u.{$param['field']}", false, true); + $whereparts[] = $DB->sql_isnotempty('u', "u.{$param['field']}", false, true); } } - // Add user custom fields if there are any. - if (!empty($infodata)) { - $extraon = implode($rule, $infodata); - $join = " LEFT JOIN {user_info_data} uid ON uid.userid = u.id AND ({$extraon})"; - } - - // Add user table fields if there are any. - if (!empty($userdata)) { - $extraon = implode($rule, $userdata); - $where = " AND ({$extraon})"; + if ($whereparts) { + $where = " AND (" . implode($rule, $whereparts) . ")"; + } else { + $where = ''; } return array($join, $where, $params); } diff --git a/badges/tests/badgeslib_test.php b/badges/tests/badgeslib_test.php index b6284316023..2de11228a56 100644 --- a/badges/tests/badgeslib_test.php +++ b/badges/tests/badgeslib_test.php @@ -344,6 +344,11 @@ class core_badges_badgeslib_testcase extends advanced_testcase { $criteria_overall = award_criteria::build(array('criteriatype' => BADGE_CRITERIA_TYPE_ACTIVITY, 'badgeid' => $badge->id)); $criteria_overall->save(array('agg' => BADGE_CRITERIA_AGGREGATION_ANY, 'module_'.$this->module->cmid => $this->module->cmid)); + // Assert the badge will not be issued to the user as is. + $badge = new badge($this->coursebadge); + $badge->review_all_criteria(); + $this->assertFalse($badge->is_issued($this->user->id)); + // Set completion for forum activity. $c = new completion_info($this->course); $activities = $c->get_activities(); @@ -379,6 +384,11 @@ class core_badges_badgeslib_testcase extends advanced_testcase { $ccompletion = new completion_completion(array('course' => $this->course->id, 'userid' => $this->user->id)); + // Assert the badge will not be issued to the user as is. + $badge = new badge($this->coursebadge); + $badge->review_all_criteria(); + $this->assertFalse($badge->is_issued($this->user->id)); + // Mark course as complete. $sink = $this->redirectEmails(); $ccompletion->mark_complete(); @@ -394,18 +404,33 @@ class core_badges_badgeslib_testcase extends advanced_testcase { * Test badges observer when user_updated event is fired. */ public function test_badges_observer_profile_criteria_review() { + global $CFG, $DB; + require_once($CFG->dirroot.'/user/profile/lib.php'); + + // Add a custom field of textarea type. + $customprofileid = $DB->insert_record('user_info_field', array( + 'shortname' => 'newfield', 'name' => 'Description of new field', 'categoryid' => 1, + 'datatype' => 'textarea')); + $this->preventResetByRollback(); // Messaging is not compatible with transactions. $badge = new badge($this->coursebadge); - $this->assertFalse($badge->is_issued($this->user->id)); $criteria_overall = award_criteria::build(array('criteriatype' => BADGE_CRITERIA_TYPE_OVERALL, 'badgeid' => $badge->id)); $criteria_overall->save(array('agg' => BADGE_CRITERIA_AGGREGATION_ANY)); $criteria_overall1 = award_criteria::build(array('criteriatype' => BADGE_CRITERIA_TYPE_PROFILE, 'badgeid' => $badge->id)); - $criteria_overall1->save(array('agg' => BADGE_CRITERIA_AGGREGATION_ALL, 'field_address' => 'address', 'field_aim' => 'aim')); + $criteria_overall1->save(array('agg' => BADGE_CRITERIA_AGGREGATION_ALL, 'field_address' => 'address', 'field_aim' => 'aim', + 'field_' . $customprofileid => $customprofileid)); + // Assert the badge will not be issued to the user as is. + $badge = new badge($this->coursebadge); + $badge->review_all_criteria(); + $this->assertFalse($badge->is_issued($this->user->id)); + + // Set the required fields and make sure the badge got issued. $this->user->address = 'Test address'; $this->user->aim = '999999999'; $sink = $this->redirectEmails(); + profile_save_data((object)array('id' => $this->user->id, 'profile_field_newfield' => 'X')); user_update_user($this->user, false); $this->assertCount(1, $sink->get_messages()); $sink->close();