diff --git a/cohort/classes/reportbuilder/audience/cohortmember.php b/cohort/classes/reportbuilder/audience/cohortmember.php index d2e83318f15..52e6d421806 100644 --- a/cohort/classes/reportbuilder/audience/cohortmember.php +++ b/cohort/classes/reportbuilder/audience/cohortmember.php @@ -56,11 +56,10 @@ class cohortmember extends base { public function get_sql(string $usertablealias): array { global $DB; - $cm = database::generate_alias(); $cohorts = $this->get_configdata()['cohorts']; - $prefix = database::generate_param_name() . '_'; - [$insql, $inparams] = $DB->get_in_or_equal($cohorts, SQL_PARAMS_NAMED, $prefix); + [$insql, $inparams] = $DB->get_in_or_equal($cohorts, SQL_PARAMS_NAMED, database::generate_param_name('_')); + $cm = database::generate_alias(); $join = "JOIN {cohort_members} {$cm} ON ({$cm}.userid = {$usertablealias}.id)"; diff --git a/reportbuilder/classes/local/filters/autocomplete.php b/reportbuilder/classes/local/filters/autocomplete.php index d0d5fe24a88..24db56d9a69 100644 --- a/reportbuilder/classes/local/filters/autocomplete.php +++ b/reportbuilder/classes/local/filters/autocomplete.php @@ -70,8 +70,7 @@ class autocomplete extends base { return ['', []]; } - $paramprefix = database::generate_param_name() . '_'; - [$insql, $inparams] = $DB->get_in_or_equal($invalues, SQL_PARAMS_NAMED, $paramprefix); + [$insql, $inparams] = $DB->get_in_or_equal($invalues, SQL_PARAMS_NAMED, database::generate_param_name('_')); return ["{$fieldsql} $insql", array_merge($params, $inparams)]; } diff --git a/reportbuilder/classes/local/filters/course_selector.php b/reportbuilder/classes/local/filters/course_selector.php index 6a4747ac6fa..5c789cc0a53 100644 --- a/reportbuilder/classes/local/filters/course_selector.php +++ b/reportbuilder/classes/local/filters/course_selector.php @@ -62,8 +62,7 @@ class course_selector extends base { return ['', []]; } - $paramprefix = database::generate_param_name() . '_'; - [$courseselect, $courseparams] = $DB->get_in_or_equal($courseids, SQL_PARAMS_NAMED, $paramprefix); + [$courseselect, $courseparams] = $DB->get_in_or_equal($courseids, SQL_PARAMS_NAMED, database::generate_param_name('_')); return ["{$fieldsql} $courseselect", array_merge($params, $courseparams)]; } diff --git a/reportbuilder/classes/local/filters/tags.php b/reportbuilder/classes/local/filters/tags.php index f2239cb657b..a0730d24c2c 100644 --- a/reportbuilder/classes/local/filters/tags.php +++ b/reportbuilder/classes/local/filters/tags.php @@ -116,13 +116,13 @@ class tags extends base { $select = "{$fieldsql} IS NULL"; } else if ($operator === self::EQUAL_TO && !empty($tags)) { [$tagselect, $tagselectparams] = $DB->get_in_or_equal($tags, SQL_PARAMS_NAMED, - database::generate_param_name() . '_'); + database::generate_param_name('_')); $select = "{$fieldsql} {$tagselect}"; $params = array_merge($params, $tagselectparams); } else if ($operator === self::NOT_EQUAL_TO && !empty($tags)) { [$tagselect, $tagselectparams] = $DB->get_in_or_equal($tags, SQL_PARAMS_NAMED, - database::generate_param_name() . '_', false); + database::generate_param_name('_'), false); // We should also return those elements that aren't tagged at all. $select = "COALESCE({$fieldsql}, 0) {$tagselect}"; diff --git a/reportbuilder/classes/local/filters/user.php b/reportbuilder/classes/local/filters/user.php index ed311606eb0..91d7b037e5a 100644 --- a/reportbuilder/classes/local/filters/user.php +++ b/reportbuilder/classes/local/filters/user.php @@ -107,8 +107,14 @@ class user extends base { $params[$paramuserid] = $USER->id; break; case self::USER_SELECT: - $paramuserid = database::generate_param_name(); - [$useridselect, $useridparams] = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED, "{$paramuserid}_", true, null); + [$useridselect, $useridparams] = $DB->get_in_or_equal( + $userids, + SQL_PARAMS_NAMED, + database::generate_param_name('_'), + true, + null, + ); + $sql = "{$fieldsql} {$useridselect}"; $params = array_merge($params, $useridparams); break; diff --git a/reportbuilder/classes/local/helpers/audience.php b/reportbuilder/classes/local/helpers/audience.php index 65290e0951f..fbca62cc694 100644 --- a/reportbuilder/classes/local/helpers/audience.php +++ b/reportbuilder/classes/local/helpers/audience.php @@ -130,8 +130,7 @@ class audience { } // Get all sql audiences. - $prefix = database::generate_param_name() . '_'; - [$select, $params] = $DB->get_in_or_equal($allowedreports, SQL_PARAMS_NAMED, $prefix); + [$select, $params] = $DB->get_in_or_equal($allowedreports, SQL_PARAMS_NAMED, database::generate_param_name('_')); $sql = "{$reporttablealias}.id {$select}"; return [$sql, $params]; @@ -182,15 +181,21 @@ class audience { // If user can't view all reports, limit the returned list to those reports they can see. if (!has_capability('moodle/reportbuilder:editall', $context, $userid)) { - $reports = self::user_reports_list($userid); - [$paramprefix, $paramuserid] = database::generate_param_names(2); - [$reportselect, $params] = $DB->get_in_or_equal($reports, SQL_PARAMS_NAMED, "{$paramprefix}_", true, null); + // Select all reports accessible to the user based on audience. + [$reportselect, $params] = $DB->get_in_or_equal( + self::user_reports_list($userid), + SQL_PARAMS_NAMED, + database::generate_param_name('_'), + true, + null, + ); $where = "{$reporttablealias}.id {$reportselect}"; // User can also see any reports that they can edit. if (has_capability('moodle/reportbuilder:edit', $context, $userid)) { + $paramuserid = database::generate_param_name(); $where = "({$reporttablealias}.usercreated = :{$paramuserid} OR {$where})"; $params[$paramuserid] = $userid ?? $USER->id; } diff --git a/reportbuilder/classes/local/helpers/database.php b/reportbuilder/classes/local/helpers/database.php index a19694c12b7..bb1e5a8ef1d 100644 --- a/reportbuilder/classes/local/helpers/database.php +++ b/reportbuilder/classes/local/helpers/database.php @@ -39,47 +39,49 @@ class database { /** * Generates unique table/column alias that must be used in generated SQL * + * @param string $suffix Optional string to append to alias * @return string */ - public static function generate_alias(): string { + public static function generate_alias(string $suffix = ''): string { static $aliascount = 0; - return static::GENERATE_ALIAS_PREFIX . ($aliascount++); + return static::GENERATE_ALIAS_PREFIX . ($aliascount++) . $suffix; } /** * Generate multiple unique table/column aliases, see {@see generate_alias} for info * * @param int $count + * @param string $suffix * @return string[] */ - public static function generate_aliases(int $count): array { - return array_map([ - static::class, 'generate_alias' - ], array_fill(0, $count, null)); + public static function generate_aliases(int $count, string $suffix = ''): array { + return array_map([static::class, 'generate_alias'], array_fill(0, $count, $suffix)); } /** * Generates unique parameter name that must be used in generated SQL * + * When passing the returned value to {@see \moodle_database::get_in_or_equal} it's recommended to define the suffix + * + * @param string $suffix Optional string to append to parameter name * @return string */ - public static function generate_param_name(): string { + public static function generate_param_name(string $suffix = ''): string { static $paramcount = 0; - return static::GENERATE_PARAM_PREFIX . ($paramcount++); + return static::GENERATE_PARAM_PREFIX . ($paramcount++) . $suffix; } /** * Generate multiple unique parameter names, see {@see generate_param_name} for info * * @param int $count + * @param string $suffix * @return string[] */ - public static function generate_param_names(int $count): array { - return array_map([ - static::class, 'generate_param_name' - ], array_fill(0, $count, null)); + public static function generate_param_names(int $count, string $suffix = ''): array { + return array_map([static::class, 'generate_param_name'], array_fill(0, $count, $suffix)); } /** diff --git a/reportbuilder/classes/reportbuilder/audience/admins.php b/reportbuilder/classes/reportbuilder/audience/admins.php index ac03632fd87..fb0d7576ab2 100644 --- a/reportbuilder/classes/reportbuilder/audience/admins.php +++ b/reportbuilder/classes/reportbuilder/audience/admins.php @@ -50,7 +50,7 @@ class admins extends base { global $CFG, $DB; $siteadmins = array_map('intval', explode(',', $CFG->siteadmins)); - [$select, $params] = $DB->get_in_or_equal($siteadmins, SQL_PARAMS_NAMED, database::generate_param_name() . '_'); + [$select, $params] = $DB->get_in_or_equal($siteadmins, SQL_PARAMS_NAMED, database::generate_param_name('_')); return ['', "{$usertablealias}.id {$select}", $params]; } diff --git a/reportbuilder/classes/reportbuilder/audience/manual.php b/reportbuilder/classes/reportbuilder/audience/manual.php index baee0317b53..16bee3be4ac 100644 --- a/reportbuilder/classes/reportbuilder/audience/manual.php +++ b/reportbuilder/classes/reportbuilder/audience/manual.php @@ -63,8 +63,7 @@ class manual extends base { global $DB; $users = $this->get_configdata()['users']; - $prefix = database::generate_param_name() . '_'; - [$insql, $inparams] = $DB->get_in_or_equal($users, SQL_PARAMS_NAMED, $prefix); + [$insql, $inparams] = $DB->get_in_or_equal($users, SQL_PARAMS_NAMED, database::generate_param_name('_')); return ['', "{$usertablealias}.id $insql", $inparams]; } diff --git a/reportbuilder/classes/reportbuilder/audience/systemrole.php b/reportbuilder/classes/reportbuilder/audience/systemrole.php index 7dff0bf88c5..d58f4ef4eda 100644 --- a/reportbuilder/classes/reportbuilder/audience/systemrole.php +++ b/reportbuilder/classes/reportbuilder/audience/systemrole.php @@ -54,8 +54,7 @@ class systemrole extends base { global $DB; $roles = $this->get_configdata()['roles']; - $prefix = database::generate_param_name() . '_'; - [$insql, $inparams] = $DB->get_in_or_equal($roles, SQL_PARAMS_NAMED, $prefix); + [$insql, $inparams] = $DB->get_in_or_equal($roles, SQL_PARAMS_NAMED, database::generate_param_name('_')); // Ensure parameter names and aliases are unique, as the same audience type can be added multiple times to a report. $paramcontextid = database::generate_param_name(); diff --git a/reportbuilder/tests/local/helpers/database_test.php b/reportbuilder/tests/local/helpers/database_test.php index 22df8614a49..a7a89f84f3c 100644 --- a/reportbuilder/tests/local/helpers/database_test.php +++ b/reportbuilder/tests/local/helpers/database_test.php @@ -37,6 +37,9 @@ class database_test extends advanced_testcase { */ public function test_generate_alias(): void { $this->assertMatchesRegularExpression('/^rbalias(\d+)$/', database::generate_alias()); + + // Specify a suffix. + $this->assertMatchesRegularExpression('/^rbalias(\d+)_$/', database::generate_alias('_')); } /** @@ -48,10 +51,20 @@ class database_test extends advanced_testcase { $this->assertCount(3, $aliases); [$aliasone, $aliastwo, $aliasthree] = $aliases; + $this->assertMatchesRegularExpression('/^rbalias(\d+)$/', $aliasone); + $this->assertMatchesRegularExpression('/^rbalias(\d+)$/', $aliastwo); + $this->assertMatchesRegularExpression('/^rbalias(\d+)$/', $aliasthree); + // Ensure they are different. $this->assertNotEquals($aliasone, $aliastwo); $this->assertNotEquals($aliasone, $aliasthree); $this->assertNotEquals($aliastwo, $aliasthree); + + // Specify a suffix. + [$aliasfour, $aliasfive] = database::generate_aliases(2, '_'); + $this->assertNotEquals($aliasfour, $aliasfive); + $this->assertMatchesRegularExpression('/^rbalias(\d+)_$/', $aliasfour); + $this->assertMatchesRegularExpression('/^rbalias(\d+)_$/', $aliasfive); } /** @@ -59,6 +72,9 @@ class database_test extends advanced_testcase { */ public function test_generate_param_name(): void { $this->assertMatchesRegularExpression('/^rbparam(\d+)$/', database::generate_param_name()); + + // Specify a suffix. + $this->assertMatchesRegularExpression('/^rbparam(\d+)_$/', database::generate_param_name('_')); } /** @@ -70,10 +86,20 @@ class database_test extends advanced_testcase { $this->assertCount(3, $params); [$paramone, $paramtwo, $paramthree] = $params; + $this->assertMatchesRegularExpression('/^rbparam(\d+)$/', $paramone); + $this->assertMatchesRegularExpression('/^rbparam(\d+)$/', $paramtwo); + $this->assertMatchesRegularExpression('/^rbparam(\d+)$/', $paramthree); + // Ensure they are different. $this->assertNotEquals($paramone, $paramtwo); $this->assertNotEquals($paramone, $paramthree); $this->assertNotEquals($paramtwo, $paramthree); + + // Specify a suffix. + [$paramfour, $paramfive] = database::generate_param_names(2, '_'); + $this->assertNotEquals($paramfour, $paramfive); + $this->assertMatchesRegularExpression('/^rbparam(\d+)_$/', $paramfour); + $this->assertMatchesRegularExpression('/^rbparam(\d+)_$/', $paramfive); } /** diff --git a/reportbuilder/upgrade.txt b/reportbuilder/upgrade.txt index 6df9844f42c..415a027a209 100644 --- a/reportbuilder/upgrade.txt +++ b/reportbuilder/upgrade.txt @@ -1,6 +1,11 @@ This file describes API changes in /reportbuilder/* Information provided here is intended especially for developers. +=== 4.4 === + +* The database helper `generate_alias[es]` and `generate_param_name[s]` methods now accept an optional `$suffix` argument for + appending additional string to the generated value + === 4.3 === * New external methods for retrieving system report data: