From 51718b752dc0564afcdf423621c00f1657c09e43 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Fri, 18 Nov 2016 21:36:52 +0800 Subject: [PATCH 1/2] MDL-56902 blocks: Prevent blocks returning when switching themes. Now if the current theme requires some blocks that don't exist, they will be created - but with a new flag that indicates this block should only be returned if the current theme requires it. This allows flipping back and forth between boost and clean without restoring the nav and settings blocks in boost. --- lib/blocklib.php | 71 +++++++++++++++++++++++++++++++++++-- lib/db/install.xml | 3 +- lib/db/upgrade.php | 15 ++++++++ lib/tests/blocklib_test.php | 52 +++++++++++++++++++++++++++ version.php | 2 +- 5 files changed, 139 insertions(+), 4 deletions(-) mode change 100644 => 100755 lib/db/install.xml diff --git a/lib/blocklib.php b/lib/blocklib.php index 0cd4c4610a8..04740029c0b 100644 --- a/lib/blocklib.php +++ b/lib/blocklib.php @@ -234,12 +234,18 @@ class block_manager { return false; } + $undeletableblocks = self::get_undeletable_block_types(); foreach ($this->blockinstances as $region) { foreach ($region as $instance) { if (empty($instance->instance->blockname)) { continue; } if ($instance->instance->blockname == $blockname) { + if ($instance->instance->requiredbytheme) { + if (!in_array($block->name, $undeletableblocks)) { + continue; + } + } return true; } } @@ -579,6 +585,20 @@ class block_manager { return; } + // Exclude auto created blocks if they are not undeletable in this theme. + $undeletable = $this->get_undeletable_block_types(); + $undeletablecheck = ''; + $undeletableparams = array(); + $undeletablenotparams = array(); + if (!empty($undeletable)) { + list($testsql, $undeletableparams) = $DB->get_in_or_equal($undeletable, SQL_PARAMS_NAMED, 'undeletable'); + list($testnotsql, $undeletablenotparams) = $DB->get_in_or_equal($undeletable, SQL_PARAMS_NAMED, 'deletable', false); + $undeletablecheck = 'AND ((bi.blockname ' . $testsql . ' AND bi.requiredbytheme = 1) OR ' . + ' (bi.blockname ' . $testnotsql . ' AND bi.requiredbytheme = 0))'; + } else { + $undeletablecheck = 'AND (bi.requiredbytheme = 0)'; + } + if (is_null($includeinvisible)) { $includeinvisible = $this->page->user_is_editing(); } @@ -626,6 +646,7 @@ class block_manager { bi.parentcontextid, bi.showinsubcontexts, bi.pagetypepattern, + bi.requiredbytheme, bi.subpagepattern, bi.defaultregion, bi.defaultweight, @@ -649,12 +670,15 @@ class block_manager { AND (bi.subpagepattern IS NULL OR bi.subpagepattern = :subpage2) $visiblecheck AND b.visible = 1 + $undeletablecheck ORDER BY COALESCE(bp.region, bi.defaultregion), COALESCE(bp.weight, bi.defaultweight), bi.id"; - $blockinstances = $DB->get_recordset_sql($sql, $params + $parentcontextparams + $pagetypepatternparams); + + $allparams = $params + $parentcontextparams + $pagetypepatternparams + $undeletableparams + $undeletablenotparams; + $blockinstances = $DB->get_recordset_sql($sql, $allparams); $this->birecordsbyregion = $this->prepare_per_region_arrays(); $unknown = array(); @@ -957,6 +981,10 @@ class block_manager { * load_blocks. This is used, for example, to ensure that all blocks get a * chance to initialise themselves via the {@link block_base::specialize()} * method, before any output is done. + * + * It is also used to create any blocks that are "undeletable" by the current theme. + * These blocks that are auto-created have requiredbytheme set on the block instance + * so they are only visible on themes that require them. */ public function create_all_block_instances() { global $PAGE; @@ -977,7 +1005,7 @@ class block_manager { } } if (!$found) { - $this->add_block_at_end_of_default_region($forced); + $this->add_block_required_by_theme($forced); $missing = true; } } @@ -993,6 +1021,45 @@ class block_manager { } + /** + * Add a block that is required by the current theme but has not been + * created yet. This is a special type of block that only shows in themes that + * require it (by listing it in undeletable_block_types). + * + * @param string $blockname the name of the block type. + */ + protected function add_block_required_by_theme($blockname) { + global $DB; + + if (empty($this->birecordsbyregion)) { + // No blocks or block regions exist yet. + return; + } + + $systemcontext = context_system::instance(); + $defaultregion = $this->get_default_region(); + // Add a special system wide block instance only for themes that require it. + $blockinstance = new stdClass; + $blockinstance->blockname = $blockname; + $blockinstance->parentcontextid = $systemcontext->id; + $blockinstance->showinsubcontexts = true; + $blockinstance->requiredbytheme = true; + $blockinstance->pagetypepattern = '*'; + $blockinstance->subpagepattern = null; + $blockinstance->defaultregion = $defaultregion; + $blockinstance->defaultweight = 0; + $blockinstance->configdata = ''; + $blockinstance->id = $DB->insert_record('block_instances', $blockinstance); + + // Ensure the block context is created. + context_block::instance($blockinstance->id); + + // If the new instance was created, allow it to do additional setup. + if ($block = block_instance($blockname, $blockinstance)) { + $block->instance_create(); + } + } + /** * Return an array of content objects from a set of block instances * diff --git a/lib/db/install.xml b/lib/db/install.xml old mode 100644 new mode 100755 index 39bbd58abc4..a26be81259d --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -2459,6 +2459,7 @@ + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 9d68867c165..81f4dd6e42b 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2406,5 +2406,20 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2016110600.00); } + if ($oldversion < 2016111900.00) { + + // Define field requiredbytheme to be added to block_instances. + $table = new xmldb_table('block_instances'); + $field = new xmldb_field('requiredbytheme', XMLDB_TYPE_INTEGER, '4', null, XMLDB_NOTNULL, null, '0', 'showinsubcontexts'); + + // Conditionally launch add field requiredbytheme. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2016111900.00); + } + return true; } diff --git a/lib/tests/blocklib_test.php b/lib/tests/blocklib_test.php index 533eeaa7770..a7be723750c 100644 --- a/lib/tests/blocklib_test.php +++ b/lib/tests/blocklib_test.php @@ -542,6 +542,58 @@ class core_blocklib_testcase extends advanced_testcase { context_block::instance($tokeep); // Would throw an exception if it was deleted. } + public function test_create_all_block_instances() { + global $CFG, $PAGE, $DB; + + $this->resetAfterTest(); + $regionname = 'side-pre'; + $context = context_system::instance(); + + $PAGE->reset_theme_and_output(); + $CFG->theme = 'boost'; + + list($page, $blockmanager) = $this->get_a_page_and_block_manager(array($regionname), + $context, 'page-type'); + $blockmanager->load_blocks(); + $blockmanager->create_all_block_instances(); + $blocks = $blockmanager->get_blocks_for_region($regionname); + $this->assertEmpty($blocks); + // There should be no blocks in the DB. + + $PAGE->reset_theme_and_output(); + // Change to a theme with undeletable blocks. + $CFG->theme = 'clean'; + + list($page, $blockmanager) = $this->get_a_page_and_block_manager(array($regionname), + $context, 'page-type'); + $blockmanager->load_blocks(); + $blockmanager->create_all_block_instances(); + $blocks = $blockmanager->get_blocks_for_region($regionname); + $this->assertCount(2, $blocks); + + $undeletable = block_manager::get_undeletable_block_types(); + foreach ($undeletable as $blockname) { + $instance = $DB->get_record('block_instances', array('blockname' => $blockname)); + $this->assertEquals(1, $instance->requiredbytheme); + } + + // Switch back and those auto blocks should not be returned. + $PAGE->reset_theme_and_output(); + $CFG->theme = 'boost'; + + list($page, $blockmanager) = $this->get_a_page_and_block_manager(array($regionname), + $context, 'page-type'); + $blockmanager->load_blocks(); + $blockmanager->create_all_block_instances(); + $blocks = $blockmanager->get_blocks_for_region($regionname); + $this->assertEmpty($blocks); + // But they should exist in the DB. + foreach ($undeletable as $blockname) { + $count = $DB->count_records('block_instances', array('blockname' => $blockname)); + $this->assertEquals(1, $count); + } + } + } /** diff --git a/version.php b/version.php index 7ad865e6c3f..bd60e050558 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2016111800.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2016111900.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. From 45ed4271b7544f925bef703443f4ff309c649d5e Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 22 Nov 2016 09:42:36 +0800 Subject: [PATCH 2/2] MDL-56902 blocks: no nav / settings in boost Upgraded sites switching to boost should not see nav/settings blocks. --- lib/db/upgrade.php | 12 ++++++++++++ version.php | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 81f4dd6e42b..c6e525fabd3 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2420,6 +2420,18 @@ function xmldb_main_upgrade($oldversion) { // Main savepoint reached. upgrade_main_savepoint(true, 2016111900.00); } + if ($oldversion < 2016111900.02) { + + // Change the existing site level admin and settings blocks to be requiredbytheme which means they won't show in boost. + $context = context_system::instance(); + $params = array('blockname' => 'settings', 'parentcontextid' => $context->id); + $DB->set_field('block_instances', 'requiredbytheme', 1, $params); + + $params = array('blockname' => 'navigation', 'parentcontextid' => $context->id); + $DB->set_field('block_instances', 'requiredbytheme', 1, $params); + // Main savepoint reached. + upgrade_main_savepoint(true, 2016111900.02); + } return true; } diff --git a/version.php b/version.php index bd60e050558..6d7ff0c5623 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2016111900.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2016111900.02; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.