From ed56f9a976b6a3ae1cfa54bb25c0a973f537c09d Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 15 Apr 2022 18:13:47 +0800 Subject: [PATCH 1/2] MDL-74506 block: Fix block upgrade to use correct parentcontextid --- lib/db/upgradelib.php | 5 +++-- lib/tests/db/upgradelib_test.php | 8 +++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/db/upgradelib.php b/lib/db/upgradelib.php index 4607164c8e1..5374fa7e9a9 100644 --- a/lib/db/upgradelib.php +++ b/lib/db/upgradelib.php @@ -1338,7 +1338,7 @@ function upgrade_block_set_defaultregion( timemodified ) SELECT :selectblockname AS blockname, - :selectparentcontext AS parentcontextid, + c.id AS parentcontextid, 0 AS showinsubcontexts, :selectpagetypepattern AS pagetypepattern, mp.id AS subpagepattern, @@ -1347,6 +1347,7 @@ function upgrade_block_set_defaultregion( :selecttimecreated AS timecreated, :selecttimemodified AS timemodified FROM {my_pages} mp + JOIN {context} c ON c.instanceid = mp.userid AND c.contextlevel = :contextuser WHERE mp.id NOT IN ( SELECT mpi.id FROM {my_pages} mpi JOIN {block_instances} bi @@ -1362,7 +1363,7 @@ function upgrade_block_set_defaultregion( $context = context_system::instance(); $result = $DB->execute($sql, [ 'selectblockname' => $blockname, - 'selectparentcontext' => $context->id, + 'contextuser' => CONTEXT_USER, 'selectpagetypepattern' => $pagetypepattern, 'selectdefaultregion' => $newdefaultregion, 'selecttimecreated' => time(), diff --git a/lib/tests/db/upgradelib_test.php b/lib/tests/db/upgradelib_test.php index 742f3959c50..c7b9821ad54 100644 --- a/lib/tests/db/upgradelib_test.php +++ b/lib/tests/db/upgradelib_test.php @@ -180,7 +180,7 @@ class upgradelib_test extends \advanced_testcase { // Any dashboards which are missing the block will have it created by the operation. upgrade_block_set_defaultregion('calendar_month', '__default', 'my-index', 'content'); - // Each of the dashboards should not have a block instance of the calendar_month block in the 'content' region + // Each of the dashboards should now have a block instance of the calendar_month block in the 'content' region // on 'my-index' only. foreach ($dashboards as $dashboardid) { // Only one block should have been created. @@ -193,6 +193,12 @@ class upgradelib_test extends \advanced_testcase { $this->assertEquals('calendar_month', $theblock->blockname); $this->assertEquals('content', $theblock->defaultregion); $this->assertEquals('my-index', $theblock->pagetypepattern); + + // Fetch the user details. + $dashboard = $DB->get_record('my_pages', ['id' => $dashboardid]); + $usercontext = \context_user::instance($dashboard->userid); + + $this->assertEquals($usercontext->id, $theblock->parentcontextid); } // Enusre that there are no blocks on the mycourses page. From fb0b57f4abb007cd4ce5e9c1fb4c77147c13c986 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 15 Apr 2022 19:57:50 +0800 Subject: [PATCH 2/2] MDL-74506 block: Correct parentcontextid for blocks in dashboard --- blocks/calendar_month/db/upgrade.php | 5 ++ blocks/calendar_month/version.php | 2 +- blocks/myoverview/db/upgrade.php | 5 ++ blocks/myoverview/version.php | 2 +- blocks/recentlyaccesseditems/db/upgrade.php | 5 ++ blocks/recentlyaccesseditems/version.php | 2 +- blocks/timeline/db/upgrade.php | 5 ++ blocks/timeline/version.php | 2 +- lib/db/upgradelib.php | 89 ++++++++++++++++++- lib/tests/db/upgradelib_test.php | 98 ++++++++++++++++++++- 10 files changed, 207 insertions(+), 8 deletions(-) diff --git a/blocks/calendar_month/db/upgrade.php b/blocks/calendar_month/db/upgrade.php index 86b4ab3f438..8eb2ab84580 100644 --- a/blocks/calendar_month/db/upgrade.php +++ b/blocks/calendar_month/db/upgrade.php @@ -67,5 +67,10 @@ function xmldb_block_calendar_month_upgrade($oldversion, $block) { upgrade_block_savepoint(true, 2022030200, 'calendar_month', false); } + if ($oldversion < 2022041901) { + upgrade_block_set_my_user_parent_context('calendar_month', '__default', 'my-index'); + upgrade_block_savepoint(true, 2022041901, 'calendar_month', false); + } + return true; } diff --git a/blocks/calendar_month/version.php b/blocks/calendar_month/version.php index 3efa7374c2a..9bc73984660 100644 --- a/blocks/calendar_month/version.php +++ b/blocks/calendar_month/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2022041900; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2022041901; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2022041200; // Requires this Moodle version. $plugin->component = 'block_calendar_month'; // Full name of the plugin (used for diagnostics) diff --git a/blocks/myoverview/db/upgrade.php b/blocks/myoverview/db/upgrade.php index 45d8da9b028..5f7aed8258e 100644 --- a/blocks/myoverview/db/upgrade.php +++ b/blocks/myoverview/db/upgrade.php @@ -115,5 +115,10 @@ function xmldb_block_myoverview_upgrade($oldversion) { upgrade_block_savepoint(true, 2021052504, 'myoverview', false); } + if ($oldversion < 2022041901) { + upgrade_block_set_my_user_parent_context('myoverview', '__default', 'my-index'); + upgrade_block_savepoint(true, 2022041901, 'myoverview', false); + } + return true; } diff --git a/blocks/myoverview/version.php b/blocks/myoverview/version.php index 25fe0ae9210..75aa74ee424 100644 --- a/blocks/myoverview/version.php +++ b/blocks/myoverview/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2022041900; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2022041901; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2022041200; // Requires this Moodle version. $plugin->component = 'block_myoverview'; // Full name of the plugin (used for diagnostics). diff --git a/blocks/recentlyaccesseditems/db/upgrade.php b/blocks/recentlyaccesseditems/db/upgrade.php index 060319e878f..34c4ac33460 100644 --- a/blocks/recentlyaccesseditems/db/upgrade.php +++ b/blocks/recentlyaccesseditems/db/upgrade.php @@ -83,5 +83,10 @@ function xmldb_block_recentlyaccesseditems_upgrade($oldversion, $block) { upgrade_block_savepoint(true, 2022030200, 'recentlyaccesseditems', false); } + if ($oldversion < 2022041901) { + upgrade_block_set_my_user_parent_context('recentlyaccesseditems', '__default', 'my-index'); + upgrade_block_savepoint(true, 2022041901, 'recentlyaccesseditems', false); + } + return true; } diff --git a/blocks/recentlyaccesseditems/version.php b/blocks/recentlyaccesseditems/version.php index 0580cdd2e23..4e8b38c02a4 100644 --- a/blocks/recentlyaccesseditems/version.php +++ b/blocks/recentlyaccesseditems/version.php @@ -22,6 +22,6 @@ */ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2022041900; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2022041901; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2022041200; // Requires this Moodle version. $plugin->component = 'block_recentlyaccesseditems'; // Full name of the plugin (used for diagnostics). diff --git a/blocks/timeline/db/upgrade.php b/blocks/timeline/db/upgrade.php index a019622a21c..e4e0d3dae7e 100644 --- a/blocks/timeline/db/upgrade.php +++ b/blocks/timeline/db/upgrade.php @@ -66,5 +66,10 @@ function xmldb_block_timeline_upgrade($oldversion, $block) { upgrade_block_savepoint(true, 2022030200, 'timeline', false); } + if ($oldversion < 2022041901) { + upgrade_block_set_my_user_parent_context('timeline', '__default', 'my-index'); + upgrade_block_savepoint(true, 2022041901, 'timeline', false); + } + return true; } diff --git a/blocks/timeline/version.php b/blocks/timeline/version.php index b16f6922400..967e796c5cf 100644 --- a/blocks/timeline/version.php +++ b/blocks/timeline/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2022041900; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2022041901; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2022041200; // Requires this Moodle version. $plugin->component = 'block_timeline'; // Full name of the plugin (used for diagnostics). diff --git a/lib/db/upgradelib.php b/lib/db/upgradelib.php index 5374fa7e9a9..4a39f08ee9e 100644 --- a/lib/db/upgradelib.php +++ b/lib/db/upgradelib.php @@ -1360,8 +1360,7 @@ function upgrade_block_set_defaultregion( AND mp.name = :pagename EOF; - $context = context_system::instance(); - $result = $DB->execute($sql, [ + $DB->execute($sql, [ 'selectblockname' => $blockname, 'contextuser' => CONTEXT_USER, 'selectpagetypepattern' => $pagetypepattern, @@ -1512,3 +1511,89 @@ function upgrade_block_delete_instances( $deleteblockinstances($instanceselect, $params); } + +/** + * Update the block instance parentcontext to point to the correct user context id for the specified block on a my page. + * + * @param string $blockname + * @param string $pagename + * @param string $pagetypepattern + */ +function upgrade_block_set_my_user_parent_context( + string $blockname, + string $pagename, + string $pagetypepattern +): void { + global $DB; + + $subpagepattern = $DB->sql_cast_char2int('bi.subpagepattern'); + // Look for any and all instances of the block in customised /my pages. + $subpageempty = $DB->sql_isnotempty('block_instances', 'bi.subpagepattern', true, false); + + $dbman = $DB->get_manager(); + $temptablename = 'block_instance_context'; + $xmldbtable = new \xmldb_table($temptablename); + $xmldbtable->add_field('instanceid', XMLDB_TYPE_INTEGER, 10, null, XMLDB_NOTNULL, null, null); + $xmldbtable->add_field('contextid', XMLDB_TYPE_INTEGER, 10, null, XMLDB_NOTNULL, null, null); + $xmldbtable->add_key('primary', XMLDB_KEY_PRIMARY, ['instanceid']); + $dbman->create_temp_table($xmldbtable); + + $sql = << c.id + EOF; + + $DB->execute($sql, [ + 'blockname' => $blockname, + 'pagetypepattern' => $pagetypepattern, + 'contextuser' => CONTEXT_USER, + 'pagename' => $pagename, + ]); + + $dbfamily = $DB->get_dbfamily(); + if ($dbfamily === 'mysql') { + // MariaDB and MySQL. + $sql = <<execute($sql); + + $dbman->drop_table($xmldbtable); +} diff --git a/lib/tests/db/upgradelib_test.php b/lib/tests/db/upgradelib_test.php index c7b9821ad54..f989d285ec8 100644 --- a/lib/tests/db/upgradelib_test.php +++ b/lib/tests/db/upgradelib_test.php @@ -238,7 +238,6 @@ class upgradelib_test extends \advanced_testcase { ], 'id')->id; $dashboards = []; - $mycourses = []; $unchanged = []; $unchangedcontexts = []; $unchangedpreferences = []; @@ -309,7 +308,6 @@ class upgradelib_test extends \advanced_testcase { 'name' => '__courses', 'private' => MY_PAGE_PRIVATE, ]); - $mycourses[] = $usermycoursesid; // These are on the my-index above, but are not the block being updated. $userunchangedblocks[] = $this->getDataGenerator()->create_block('online_users', [ @@ -445,4 +443,100 @@ class upgradelib_test extends \advanced_testcase { ])); } } + + /** + * Ensrue that the upgrade_block_set_my_user_parent_context function performs as expected. + * + * @covers ::upgrade_block_set_my_user_parent_context + */ + public function test_upgrade_block_set_my_user_parent_context(): void { + global $DB; + + $this->resetAfterTest(); + $this->preventResetByRollback(); + + $systemcontext = \context_system::instance(); + + $dashboards = []; + $otherblocknames = [ + 'online_users', + 'myoverview', + 'calendar_month', + ]; + $affectedblockname = 'timeline'; + + // Create dashboard pages for a number of users. + while (count($dashboards) < 10) { + $user = $this->getDataGenerator()->create_user(); + $dashboard = $DB->insert_record('my_pages', (object) [ + 'userid' => $user->id, + 'name' => '__default', + 'private' => MY_PAGE_PRIVATE, + ]); + $dashboards[] = $dashboard; + + $mycourse = $DB->insert_record('my_pages', (object) [ + 'userid' => $user->id, + 'name' => '__courses', + 'private' => MY_PAGE_PRIVATE, + ]); + + // These are on the my-index above, but are not the block being updated. + foreach ($otherblocknames as $blockname) { + $unchanged[] = $this->getDataGenerator()->create_block($blockname, [ + 'parentcontextid' => $systemcontext->id, + 'pagetypepattern' => 'my-index', + 'subpagepattern' => $dashboard, + ]); + } + + // This is on a my-index page, and is the affected block, but is on the mycourses page, not the dashboard. + $unchanged[] = $this->getDataGenerator()->create_block($affectedblockname, [ + 'parentcontextid' => $systemcontext->id, + 'pagetypepattern' => 'my-index', + 'subpagepattern' => $mycourse, + ]); + + // This is on the default dashboard, and is the affected block, but not a my-index page. + $unchanged[] = $this->getDataGenerator()->create_block($affectedblockname, [ + 'parentcontextid' => $systemcontext->id, + 'pagetypepattern' => 'not-my-index', + 'subpagepattern' => $dashboard, + ]); + + // This is the match which should be changed. + $changed[] = $this->getDataGenerator()->create_block($affectedblockname, [ + 'parentcontextid' => $systemcontext->id, + 'pagetypepattern' => 'my-index', + 'subpagepattern' => $dashboard, + ]); + } + + // Perform the operation. + // Target all affected blocks matching 'my-index' and correct the context to the relevant user's contexct. + // Only the '__default' dashboard on the 'my-index' my_page should be affected. + upgrade_block_set_my_user_parent_context($affectedblockname, '__default', 'my-index'); + + // Ensure that the relevant blocks remain unchanged. + foreach ($unchanged as $original) { + $block = $DB->get_record('block_instances', ['id' => $original->id]); + $this->assertEquals($original, $block); + } + + // Ensure that only the expected blocks were changed. + foreach ($changed as $original) { + $block = $DB->get_record('block_instances', ['id' => $original->id]); + $this->assertNotEquals($original, $block); + + // Fetch the my page and user details. + $dashboard = $DB->get_record('my_pages', ['id' => $original->subpagepattern]); + $usercontext = \context_user::instance($dashboard->userid); + + // Only the contextid should be updated to the relevant user's context. + // No other changes are expected. + $expected = (object) $original; + $expected->parentcontextid = $usercontext->id; + $this->assertEquals($expected, $block); + } + } }