From 292dcf047f6ab7253e4d9b55f886e4236800f317 Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Mon, 31 Mar 2014 11:46:18 +1300 Subject: [PATCH] MDL-41551 blocks: added tracking and recognition of custom block regions --- lib/ajax/blocks.php | 5 +-- lib/blocklib.php | 46 ++++++++++++++++++++---- lib/outputlib.php | 2 +- lib/tests/blocklib_test.php | 70 +++++++++++++++++++++++++++++++++---- 4 files changed, 105 insertions(+), 18 deletions(-) diff --git a/lib/ajax/blocks.php b/lib/ajax/blocks.php index 7b2c82065f4..545b82fae08 100644 --- a/lib/ajax/blocks.php +++ b/lib/ajax/blocks.php @@ -56,17 +56,14 @@ $PAGE->set_context(context::instance_by_id($contextid)); // Setting layout to replicate blocks configuration for the page we edit. $PAGE->set_pagelayout($pagelayout); $PAGE->set_subpage($subpage); +$PAGE->blocks->add_custom_regions_for_pagetype($pagetype); $pagetype = explode('-', $pagetype); switch ($pagetype[0]) { case 'my': - // My Home page needs to have 'content' block region set up. $PAGE->set_blocks_editing_capability('moodle/my:manageblocks'); - $PAGE->blocks->add_region('content'); break; case 'user': if ($pagelayout == 'mydashboard') { - // User profile pages also need the 'content' block region set up. - $PAGE->blocks->add_region('content'); // If it's not the current user's profile, we need a different capability. if ($PAGE->context->contextlevel == CONTEXT_USER && $PAGE->context->instanceid != $USER->id) { $PAGE->set_blocks_editing_capability('moodle/user:manageblocks'); diff --git a/lib/blocklib.php b/lib/blocklib.php index 07f31a5bb9d..fbf37acef4a 100644 --- a/lib/blocklib.php +++ b/lib/blocklib.php @@ -394,12 +394,30 @@ class block_manager { /** * Add a region to a page * - * @param string $region add a named region where blocks may appear on the - * current page. This is an internal name, like 'side-pre', not a string to - * display in the UI. + * @param string $region add a named region where blocks may appear on the current page. + * This is an internal name, like 'side-pre', not a string to display in the UI. + * @param bool $custom True if this is a custom block region, being added by the page rather than the theme layout. */ - public function add_region($region) { + public function add_region($region, $custom = true) { + global $SESSION; $this->check_not_yet_loaded(); + if ($custom) { + if (array_key_exists($region, $this->regions)) { + // This here is EXACTLY why we should not be adding block regions into a page. It should + // ALWAYS be done in a theme layout. + debugging('A custom region conflicts with a block region in the theme.', DEBUG_DEVELOPER); + } + // We need to register this custom region against the page type being used. + // This allows us to check, when performing block actions, that unrecognised regions can be worked with. + $type = $this->page->pagetype; + if (!isset($SESSION->custom_block_regions)) { + $SESSION->custom_block_regions = array($type => array($region)); + } else if (!isset($SESSION->custom_block_regions[$type])) { + $SESSION->custom_block_regions[$type] = array($region); + } else if (!in_array($region, $SESSION->custom_block_regions[$type])) { + $SESSION->custom_block_regions[$type][] = $region; + } + } $this->regions[$region] = 1; } @@ -409,9 +427,23 @@ class block_manager { * * @param array $regions this utility method calls add_region for each array element. */ - public function add_regions($regions) { + public function add_regions($regions, $custom = true) { foreach ($regions as $region) { - $this->add_region($region); + $this->add_region($region, $custom); + } + } + + /** + * Finds custom block regions associated with a page type and registers them with this block manager. + * + * @param string $pagetype + */ + public function add_custom_regions_for_pagetype($pagetype) { + global $SESSION; + if (isset($SESSION->custom_block_regions[$pagetype])) { + foreach ($SESSION->custom_block_regions[$pagetype] as $customregion) { + $this->add_region($customregion, false); + } } } @@ -746,7 +778,7 @@ class block_manager { * @param string $subpagepattern optional. Passed to @see add_block() */ public function add_blocks($blocks, $pagetypepattern = NULL, $subpagepattern = NULL, $showinsubcontexts=false, $weight=0) { - $this->add_regions(array_keys($blocks)); + $this->add_regions(array_keys($blocks), false); foreach ($blocks as $region => $regionblocks) { $weight = 0; foreach ($regionblocks as $blockname) { diff --git a/lib/outputlib.php b/lib/outputlib.php index 7a1a2ec1c51..9327c9decbd 100644 --- a/lib/outputlib.php +++ b/lib/outputlib.php @@ -1852,7 +1852,7 @@ class theme_config { public function setup_blocks($pagelayout, $blockmanager) { $layoutinfo = $this->layout_info_for_page($pagelayout); if (!empty($layoutinfo['regions'])) { - $blockmanager->add_regions($layoutinfo['regions']); + $blockmanager->add_regions($layoutinfo['regions'], false); $blockmanager->set_default_region($layoutinfo['defaultregion']); } } diff --git a/lib/tests/blocklib_test.php b/lib/tests/blocklib_test.php index fbadf5d299b..49b064a77a7 100644 --- a/lib/tests/blocklib_test.php +++ b/lib/tests/blocklib_test.php @@ -43,6 +43,7 @@ class core_blocklib_testcase extends advanced_testcase { parent::setUp(); $this->testpage = new moodle_page(); $this->testpage->set_context(context_system::instance()); + $this->testpage->set_pagetype('phpunit-block-test'); $this->blockmanager = new testable_block_manager($this->testpage); } @@ -69,7 +70,7 @@ class core_blocklib_testcase extends advanced_testcase { public function test_add_region() { // Exercise SUT. - $this->blockmanager->add_region('a-region-name'); + $this->blockmanager->add_region('a-region-name', false); // Validate. $this->assertEquals(array('a-region-name'), $this->blockmanager->get_regions()); } @@ -78,15 +79,15 @@ class core_blocklib_testcase extends advanced_testcase { // Set up fixture. $regions = array('a-region', 'another-region'); // Exercise SUT. - $this->blockmanager->add_regions($regions); + $this->blockmanager->add_regions($regions, false); // Validate. $this->assertEquals($regions, $this->blockmanager->get_regions(), '', 0, 10, true); } public function test_add_region_twice() { // Exercise SUT. - $this->blockmanager->add_region('a-region-name'); - $this->blockmanager->add_region('another-region'); + $this->blockmanager->add_region('a-region-name', false); + $this->blockmanager->add_region('another-region', false); // Validate. $this->assertEquals(array('a-region-name', 'another-region'), $this->blockmanager->get_regions(), '', 0, 10, true); } @@ -95,6 +96,63 @@ class core_blocklib_testcase extends advanced_testcase { * @expectedException coding_exception */ public function test_cannot_add_region_after_loaded() { + // Set up fixture. + $this->blockmanager->mark_loaded(); + // Exercise SUT. + $this->blockmanager->add_region('too-late', false); + } + + /** + * Testing adding a custom region. + */ + public function test_add_custom_region() { + global $SESSION; + // Exercise SUT. + $this->blockmanager->add_region('a-custom-region-name'); + // Validate. + $this->assertEquals(array('a-custom-region-name'), $this->blockmanager->get_regions()); + $this->assertTrue(isset($SESSION->custom_block_regions)); + $this->assertArrayHasKey('phpunit-block-test', $SESSION->custom_block_regions); + $this->assertTrue(in_array('a-custom-region-name', $SESSION->custom_block_regions['phpunit-block-test'])); + + } + + /** + * Test adding two custom regions using add_regions method. + */ + public function test_add_custom_regions() { + global $SESSION; + // Set up fixture. + $regions = array('a-region', 'another-custom-region'); + // Exercise SUT. + $this->blockmanager->add_regions($regions); + // Validate. + $this->assertEquals($regions, $this->blockmanager->get_regions(), '', 0, 10, true); + $this->assertTrue(isset($SESSION->custom_block_regions)); + $this->assertArrayHasKey('phpunit-block-test', $SESSION->custom_block_regions); + $this->assertTrue(in_array('another-custom-region', $SESSION->custom_block_regions['phpunit-block-test'])); + } + + /** + * Test adding two custom block regions. + */ + public function test_add_custom_region_twice() { + // Exercise SUT. + $this->blockmanager->add_region('a-custom-region-name'); + $this->blockmanager->add_region('another-custom-region'); + // Validate. + $this->assertEquals( + array('a-custom-region-name', 'another-custom-region'), + $this->blockmanager->get_regions(), + '', 0, 10, true + ); + } + + /** + * Test to ensure that we cannot add a region after the blocks have been loaded. + * @expectedException coding_exception + */ + public function test_cannot_add_custom_region_after_loaded() { // Set up fixture. $this->blockmanager->mark_loaded(); // Exercise SUT. @@ -103,7 +161,7 @@ class core_blocklib_testcase extends advanced_testcase { public function test_set_default_region() { // Set up fixture. - $this->blockmanager->add_region('a-region-name'); + $this->blockmanager->add_region('a-region-name', false); // Exercise SUT. $this->blockmanager->set_default_region('a-region-name'); // Validate. @@ -149,7 +207,7 @@ class core_blocklib_testcase extends advanced_testcase { $page->set_subpage($subpage); $blockmanager = new testable_block_manager($page); - $blockmanager->add_regions($regions); + $blockmanager->add_regions($regions, false); $blockmanager->set_default_region($regions[0]); return array($page, $blockmanager); -- 2.17.1