From 8f9b57d5d37ac7a5817c19433319c729e5b1c3cf Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 8 Aug 2014 12:32:19 +0100 Subject: [PATCH] MDL-46739 user preferences: change value column to TEXT. Other similar columns config.value and config_plugins.value are TEXT, so it is surprising that this column is different, and that has lead to bugs in the past, so we should make it consistent. --- .upgradenotes/MDL-46739-2024091010555321.yml | 10 ++++++++++ blocks/online_users/classes/fetcher.php | 4 ++-- .../task/send_new_user_passwords_task.php | 19 ++++++++++++++----- lib/db/install.xml | 4 ++-- lib/db/upgrade.php | 12 ++++++++++++ lib/moodlelib.php | 4 ---- lib/tests/moodlelib_test.php | 16 ++++++++-------- version.php | 2 +- 8 files changed, 49 insertions(+), 22 deletions(-) create mode 100644 .upgradenotes/MDL-46739-2024091010555321.yml diff --git a/.upgradenotes/MDL-46739-2024091010555321.yml b/.upgradenotes/MDL-46739-2024091010555321.yml new file mode 100644 index 00000000000..01e9741392d --- /dev/null +++ b/.upgradenotes/MDL-46739-2024091010555321.yml @@ -0,0 +1,10 @@ +issueNumber: MDL-46739 +notes: + core: + - message: >- + The {user_preferences}.value database field is now TEXT instead of CHAR. + This means that any queries with a condition on this field in a WHERE or + JOIN statement will need updating to use `$DB->sql_compare_text()`. See + the `$newusers` query in + `\core\task\send_new_users_password_task::execute` for an example. + type: changed diff --git a/blocks/online_users/classes/fetcher.php b/blocks/online_users/classes/fetcher.php index 4fcef20511e..7c3ff39765e 100644 --- a/blocks/online_users/classes/fetcher.php +++ b/blocks/online_users/classes/fetcher.php @@ -80,7 +80,7 @@ class fetcher { $uservisibilityselect = ""; if ($CFG->block_online_users_onlinestatushiding) { $uservisibility = ", up.value AS uservisibility"; - $uservisibilityselect = "AND (" . $DB->sql_cast_char2int('up.value') . " = 1 + $uservisibilityselect = "AND (" . $DB->sql_cast_char2int('up.value', true) . " = 1 OR up.value IS NULL OR u.id = :userid)"; } @@ -97,7 +97,7 @@ class fetcher { $lastaccess = ", MAX(u.lastaccess) AS lastaccess"; $timeaccess = ", MAX(ul.timeaccess) AS lastaccess"; if ($CFG->block_online_users_onlinestatushiding) { - $uservisibility = ", MAX(up.value) AS uservisibility"; + $uservisibility = ", MAX(" . $DB->sql_cast_char2int('up.value', true) . ") AS uservisibility"; } $params['currentgroup'] = $currentgroup; } diff --git a/lib/classes/task/send_new_user_passwords_task.php b/lib/classes/task/send_new_user_passwords_task.php index 678b29a40c7..57002c2c5a8 100644 --- a/lib/classes/task/send_new_user_passwords_task.php +++ b/lib/classes/task/send_new_user_passwords_task.php @@ -45,7 +45,13 @@ class send_new_user_passwords_task extends scheduled_task { global $DB; // Generate new password emails for users - ppl expect these generated asap. - if ($DB->count_records('user_preferences', array('name' => 'create_password', 'value' => '1'))) { + if ( + $DB->record_exists_select( + 'user_preferences', + 'name = ? AND ' . $DB->sql_compare_text('value', 2) . ' = ?', + ['create_password', '1'] + ) + ) { mtrace('Creating passwords for new users...'); $userfieldsapi = \core_user\fields::for_name(); $usernamefields = $userfieldsapi->get_sql('u', false, '', '', false)->selects; @@ -54,10 +60,13 @@ class send_new_user_passwords_task extends scheduled_task { $usernamefields, u.username, u.lang, p.id as prefid FROM {user} u - JOIN {user_preferences} p ON u.id=p.userid - WHERE p.name='create_password' AND p.value='1' AND - u.email !='' AND u.suspended = 0 AND - u.auth != 'nologin' AND u.deleted = 0"); + JOIN {user_preferences} p ON u.id = p.userid + WHERE p.name = 'create_password' + AND " . $DB->sql_compare_text('p.value', 2) . " = '1' + AND u.email <> '' + AND u.suspended = 0 + AND u.auth <> 'nologin' + AND u.deleted = 0"); // Note: we can not send emails to suspended accounts. foreach ($newusers as $newuser) { diff --git a/lib/db/install.xml b/lib/db/install.xml index b7bd829f0ac..6802df9e2a5 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -939,7 +939,7 @@ - + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index f040a03a1d7..d3fb644f409 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -1447,5 +1447,17 @@ function xmldb_main_upgrade($oldversion) { // Automatically generated Moodle v4.5.0 release upgrade line. // Put any upgrade step following this. + if ($oldversion < 2024102500.01) { + // Changing type of field value on table user_preferences to text. + $table = new xmldb_table('user_preferences'); + $field = new xmldb_field('value', XMLDB_TYPE_TEXT, null, null, XMLDB_NOTNULL, null, null, 'name'); + + // Launch change of type for field value. + $dbman->change_field_type($table, $field); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2024102500.01); + } + return true; } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 1c24f74c5ec..8e2f9d029dc 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -1490,11 +1490,7 @@ function set_user_preference($name, $value, $user = null) { } else if (is_array($value)) { throw new coding_exception('Invalid value in set_user_preference() call, arrays are not allowed'); } - // Value column maximum length is 1333 characters. $value = (string)$value; - if (core_text::strlen($value) > 1333) { - throw new coding_exception('Invalid value in set_user_preference() call, value is is too long for the value column'); - } if (is_null($user)) { $user = $USER; diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index 45b4f7c0b33..da788a2cfa0 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -1495,14 +1495,14 @@ final class moodlelib_test extends \advanced_testcase { $this->assertEquals($longvalue, $DB->get_field('user_preferences', 'value', array('userid' => $USER->id, 'name' => '_test_long_user_preference'))); - // Test > 1333 char values, coding_exception expected. - $longvalue = str_repeat('a', 1334); - try { - set_user_preference('_test_long_user_preference', $longvalue); - $this->fail('Exception expected - longer than 1333 chars not allowed as preference value'); - } catch (\moodle_exception $ex) { - $this->assertInstanceOf('coding_exception', $ex); - } + // Larger preference values are allowed as of MDL-46739. + $longervalue = str_repeat('a', 1334); + set_user_preference('_test_long_user_preference', $longervalue); + $this->assertEquals($longervalue, get_user_preferences('_test_long_user_preference')); + $this->assertEquals( + $longervalue, + $DB->get_field('user_preferences', 'value', ['userid' => $USER->id, 'name' => '_test_long_user_preference']) + ); // Test invalid params. try { diff --git a/version.php b/version.php index c1b67266d40..30ebc91d3b6 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2024102500.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2024102500.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '5.0dev (Build: 20241025)'; // Human-friendly version name