MDL-41551 blocks: added tracking and recognition of custom block regions
authorSam Hemelryk <sam@moodle.com>
Sun, 30 Mar 2014 22:46:18 +0000 (11:46 +1300)
committerSam Hemelryk <sam@moodle.com>
Sun, 6 Apr 2014 03:17:47 +0000 (15:17 +1200)
lib/ajax/blocks.php
lib/blocklib.php
lib/outputlib.php
lib/tests/blocklib_test.php

index 7b2c820..545b82f 100644 (file)
@@ -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);
 // 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':
 $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->set_blocks_editing_capability('moodle/my:manageblocks');
-        $PAGE->blocks->add_region('content');
         break;
     case 'user':
         if ($pagelayout == 'mydashboard') {
         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');
             // 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');
index 07f31a5..fbf37ac 100644 (file)
@@ -394,12 +394,30 @@ class block_manager {
     /**
      * Add a region to a page
      *
     /**
      * 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();
         $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;
     }
 
         $this->regions[$region] = 1;
     }
 
@@ -409,9 +427,23 @@ class block_manager {
      *
      * @param array $regions this utility method calls add_region for each array element.
      */
      *
      * @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) {
         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) {
      * @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) {
         foreach ($blocks as $region => $regionblocks) {
             $weight = 0;
             foreach ($regionblocks as $blockname) {
index 7a1a2ec..9327c9d 100644 (file)
@@ -1852,7 +1852,7 @@ class theme_config {
     public function setup_blocks($pagelayout, $blockmanager) {
         $layoutinfo = $this->layout_info_for_page($pagelayout);
         if (!empty($layoutinfo['regions'])) {
     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']);
         }
     }
             $blockmanager->set_default_region($layoutinfo['defaultregion']);
         }
     }
index fbadf5d..49b064a 100644 (file)
@@ -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());
         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);
     }
 
         $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.
 
     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());
     }
         // 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.
         // 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.
         // 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);
     }
         // 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() {
      * @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.
         // 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.
 
     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.
         // 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);
         $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);
         $blockmanager->set_default_region($regions[0]);
 
         return array($page, $blockmanager);