MDL-58957 Global search: Add time fields to block_instances

reskit · Jun 29, 2017 · 557554f · 557554f
1 parent 5a651b4
commit 557554f
Show file tree

Hide file tree

Showing 13 changed files with 225 additions and 7 deletions.
diff --git a/backup/moodle2/backup_stepslib.php b/backup/moodle2/backup_stepslib.php
@@ -1410,8 +1410,9 @@ protected function define_structure() {
         // Define each element separated
 
         $block = new backup_nested_element('block', array('id', 'contextid', 'version'), array(
-            'blockname', 'parentcontextid', 'showinsubcontexts', 'pagetypepattern',
-            'subpagepattern', 'defaultregion', 'defaultweight', 'configdata'));
+                'blockname', 'parentcontextid', 'showinsubcontexts', 'pagetypepattern',
+                'subpagepattern', 'defaultregion', 'defaultweight', 'configdata',
+                'timecreated', 'timemodified'));
 
         $positions = new backup_nested_element('block_positions');
 

diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php
@@ -3937,6 +3937,14 @@ public function process_block($data) {
             $data->configdata = base64_encode(serialize((object)$configdata));
         }
 
+        // Set timecreated, timemodified if not included (older backup).
+        if (empty($data->timecreated)) {
+            $data->timecreated = time();
+        }
+        if (empty($data->timemodified)) {
+            $data->timemodified = $data->timecreated;
+        }
+
         // Create the block instance
         $newitemid = $DB->insert_record('block_instances', $data);
         // Save the mapping (with restorefiles support)

diff --git a/backup/moodle2/tests/moodle2_test.php b/backup/moodle2/tests/moodle2_test.php
@@ -464,9 +464,10 @@ public function test_restore_frontpage() {
      *
      * @param stdClass $course Course object to backup
      * @param int $newdate If non-zero, specifies custom date for new course
+     * @param callable|null $inbetween If specified, function that is called before restore
      * @return int ID of newly restored course
      */
-    protected function backup_and_restore($course, $newdate = 0) {
+    protected function backup_and_restore($course, $newdate = 0, $inbetween = null) {
         global $USER, $CFG;
 
         // Turn off file logging, otherwise it can't delete the file (Windows).
@@ -481,6 +482,10 @@ protected function backup_and_restore($course, $newdate = 0) {
         $bc->execute_plan();
         $bc->destroy();
 
+        if ($inbetween) {
+            $inbetween($backupid);
+        }
+
         // Do restore to new course with default settings.
         $newcourseid = restore_dbops::create_new_course(
                 $course->fullname, $course->shortname . '_2', $course->category);
@@ -802,4 +807,64 @@ public function test_restore_with_users_with_enrolments_deleting() {
         $enrolment = reset($enrolments);
         $this->assertEquals('self', $enrolment->enrol);
     }
+
+    /**
+     * Test the block instance time fields (timecreated, timemodified) through a backup and restore.
+     */
+    public function test_block_instance_times_backup() {
+        global $DB;
+        $this->resetAfterTest();
+
+        $this->setAdminUser();
+        $generator = $this->getDataGenerator();
+
+        // Create course and add HTML block.
+        $course = $generator->create_course();
+        $context = context_course::instance($course->id);
+        $page = new moodle_page();
+        $page->set_context($context);
+        $page->set_course($course);
+        $page->set_pagelayout('standard');
+        $page->set_pagetype('course-view');
+        $page->blocks->load_blocks();
+        $page->blocks->add_block_at_end_of_default_region('html');
+
+        // Update (hack in database) timemodified and timecreated to specific values for testing.
+        $blockdata = $DB->get_record('block_instances',
+                ['blockname' => 'html', 'parentcontextid' => $context->id]);
+        $originalblockid = $blockdata->id;
+        $blockdata->timecreated = 12345;
+        $blockdata->timemodified = 67890;
+        $DB->update_record('block_instances', $blockdata);
+
+        // Do backup and restore.
+        $newcourseid = $this->backup_and_restore($course);
+
+        // Confirm that values were transferred correctly into HTML block on new course.
+        $newcontext = context_course::instance($newcourseid);
+        $blockdata = $DB->get_record('block_instances',
+                ['blockname' => 'html', 'parentcontextid' => $newcontext->id]);
+        $this->assertEquals(12345, $blockdata->timecreated);
+        $this->assertEquals(67890, $blockdata->timemodified);
+
+        // Simulate what happens with an older backup that doesn't have those fields, by removing
+        // them from the backup before doing a restore.
+        $before = time();
+        $newcourseid = $this->backup_and_restore($course, 0, function($backupid) use($originalblockid) {
+            global $CFG;
+            $path = $CFG->dataroot . '/temp/backup/' . $backupid . '/course/blocks/html_' .
+                    $originalblockid . '/block.xml';
+            $xml = file_get_contents($path);
+            $xml = preg_replace('~<timecreated>.*?</timemodified>~s', '', $xml);
+            file_put_contents($path, $xml);
+        });
+        $after = time();
+
+        // The fields not specified should default to current time.
+        $newcontext = context_course::instance($newcourseid);
+        $blockdata = $DB->get_record('block_instances',
+                ['blockname' => 'html', 'parentcontextid' => $newcontext->id]);
+        $this->assertTrue($before <= $blockdata->timecreated && $after >= $blockdata->timecreated);
+        $this->assertTrue($before <= $blockdata->timemodified && $after >= $blockdata->timemodified);
+    }
 }
diff --git a/blocks/html/lib.php b/blocks/html/lib.php
@@ -104,7 +104,8 @@ function block_html_global_db_replace($search, $replace) {
         $config = unserialize(base64_decode($instance->configdata));
         if (isset($config->text) and is_string($config->text)) {
             $config->text = str_replace($search, $replace, $config->text);
-            $DB->set_field('block_instances', 'configdata', base64_encode(serialize($config)), array('id' => $instance->id));
+            $DB->update_record('block_instances', ['id' => $instance->id,
+                    'configdata' => base64_encode(serialize($config)), 'timemodified' => time()]);
         }
     }
     $instances->close();

diff --git a/blocks/moodleblock.class.php b/blocks/moodleblock.class.php
@@ -474,8 +474,8 @@ function instance_allow_multiple() {
      */
     function instance_config_save($data, $nolongerused = false) {
         global $DB;
-        $DB->set_field('block_instances', 'configdata', base64_encode(serialize($data)),
-                array('id' => $this->instance->id));
+        $DB->update_record('block_instances', ['id' => $this->instance->id,
+                'configdata' => base64_encode(serialize($data)), 'timemodified' => time()]);
     }
 
     /**

diff --git a/blocks/upgrade.txt b/blocks/upgrade.txt
@@ -1,6 +1,12 @@
 This files describes API changes in /blocks/* - activity modules,
 information provided here is intended especially for developers.
 
+=== 3.4 ===
+
+* The block_instances table now contains fields timecreated and timemodified. If third-party code
+  creates or updates these rows (without using the standard API), it should be modified to set
+  these fields as appropriate.
+
 === 3.3 ===
 
 * block_manager::get_required_by_theme_block_types() is no longer static.

diff --git a/lib/blocklib.php b/lib/blocklib.php
@@ -833,6 +833,8 @@ public function add_block($blockname, $region, $weight, $showinsubcontexts, $pag
         $blockinstance->defaultregion = $region;
         $blockinstance->defaultweight = $weight;
         $blockinstance->configdata = '';
+        $blockinstance->timecreated = time();
+        $blockinstance->timemodified = $blockinstance->timecreated;
         $blockinstance->id = $DB->insert_record('block_instances', $blockinstance);
 
         // Ensure the block context is created.
@@ -940,6 +942,7 @@ public function reposition_block($blockinstanceid, $newregion, $newweight) {
             $newbi->id = $bi->id;
             $newbi->defaultregion = $newregion;
             $newbi->defaultweight = $newweight;
+            $newbi->timemodified = time();
             $DB->update_record('block_instances', $newbi);
 
             if ($bi->blockpositionid) {
@@ -1162,6 +1165,8 @@ protected function add_block_required_by_theme($blockname) {
         $blockinstance->defaultregion = $defaultregion;
         $blockinstance->defaultweight = 0;
         $blockinstance->configdata = '';
+        $blockinstance->timecreated = time();
+        $blockinstance->timemodified = $blockinstance->timecreated;
         $blockinstance->id = $DB->insert_record('block_instances', $blockinstance);
 
         // Ensure the block context is created.
@@ -1723,6 +1728,7 @@ public function process_url_edit() {
 
             $bi->defaultregion = $data->bui_defaultregion;
             $bi->defaultweight = $data->bui_defaultweight;
+            $bi->timemodified = time();
             $DB->update_record('block_instances', $bi);
 
             if (!empty($block->config)) {

diff --git a/lib/db/install.xml b/lib/db/install.xml
@@ -2493,13 +2493,16 @@
         <FIELD NAME="defaultregion" TYPE="char" LENGTH="16" NOTNULL="true" SEQUENCE="false" COMMENT="Which block region this block should appear in on each page, in the absence of a specific position in the block_positions table."/>
         <FIELD NAME="defaultweight" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Used to order the blocks within a block region. Again, may be overridden by the block_positions table for a specific page where this block appears."/>
         <FIELD NAME="configdata" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="A serialized blob of configuration data for this block instance."/>
+        <FIELD NAME="timecreated" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Time at which this block instance was originally created"/>
+        <FIELD NAME="timemodified" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Time at which block instance was last modified."/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
         <KEY NAME="parentcontextid" TYPE="foreign" FIELDS="parentcontextid" REFTABLE="context" REFFIELDS="id"/>
       </KEYS>
       <INDEXES>
         <INDEX NAME="parentcontextid-showinsubcontexts-pagetypepattern-subpagepattern" UNIQUE="false" FIELDS="parentcontextid, showinsubcontexts, pagetypepattern, subpagepattern"/>
+        <INDEX NAME="timemodified" UNIQUE="false" FIELDS="timemodified"/>
       </INDEXES>
     </TABLE>
     <TABLE NAME="block_positions" COMMENT="Stores the position of a sticky block_instance on a another page than the one where it was added.">

diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php
@@ -2887,5 +2887,58 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2017061301.00);
     }
 
+    if ($oldversion < 2017062900.01) {
+
+        // Define field timemodified to be added to block_instances.
+        $table = new xmldb_table('block_instances');
+        $field = new xmldb_field('timemodified', XMLDB_TYPE_INTEGER, '10', null, null,
+                null, null, 'configdata');
+
+        // Conditionally launch add field timemodified.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+
+            // Set field to current time.
+            $DB->set_field('block_instances', 'timemodified', time());
+
+            // Changing nullability of field timemodified on table block_instances to not null.
+            $field = new xmldb_field('timemodified', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL,
+                    null, null, 'configdata');
+
+            // Launch change of nullability for field timemodified.
+            $dbman->change_field_notnull($table, $field);
+
+            // Define index timemodified (not unique) to be added to block_instances.
+            $index = new xmldb_index('timemodified', XMLDB_INDEX_NOTUNIQUE, array('timemodified'));
+
+            // Conditionally launch add index timemodified.
+            if (!$dbman->index_exists($table, $index)) {
+                $dbman->add_index($table, $index);
+            }
+        }
+
+        // Define field timecreated to be added to block_instances.
+        $field = new xmldb_field('timecreated', XMLDB_TYPE_INTEGER, '10', null, null,
+                null, null, 'configdata');
+
+        // Conditionally launch add field timecreated.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+
+            // Set field to current time.
+            $DB->set_field('block_instances', 'timecreated', time());
+
+            // Changing nullability of field timecreated on table block_instances to not null.
+            $field = new xmldb_field('timecreated', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL,
+                    null, null, 'configdata');
+
+            // Launch change of nullability for field timecreated.
+            $dbman->change_field_notnull($table, $field);
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2017062900.01);
+    }
+
     return true;
 }
diff --git a/lib/testing/generator/block_generator.php b/lib/testing/generator/block_generator.php
@@ -122,6 +122,13 @@ public function create_instance($record = null, $options = array()) {
         $this->preprocess_record($record, $options);
         $record = $this->prepare_record($record);
 
+        if (empty($record->timecreated)) {
+            $record->timecreated = time();
+        }
+        if (empty($record->timemodified)) {
+            $record->timemodified = time();
+        }
+
         $id = $DB->insert_record('block_instances', $record);
         context_block::instance($id);
 

diff --git a/lib/tests/blocklib_test.php b/lib/tests/blocklib_test.php
@@ -622,6 +622,72 @@ public function test_create_all_block_instances() {
         }
     }
 
+    /**
+     * Test the block instance time fields (timecreated, timemodified).
+     */
+    public function test_block_instance_times() {
+        global $DB;
+
+        $this->purge_blocks();
+
+        // Set up fixture.
+        $regionname = 'a-region';
+        $blockname = 'html';
+        $context = context_system::instance();
+
+        list($page, $blockmanager) = $this->get_a_page_and_block_manager(array($regionname),
+                $context, 'page-type');
+
+        // Add block to page.
+        $before = time();
+        $blockmanager->add_block($blockname, $regionname, 0, false);
+        $after = time();
+
+        // Check database table to ensure it contains created/modified times.
+        $blockdata = $DB->get_record('block_instances', ['blockname' => 'html']);
+        $this->assertTrue($blockdata->timemodified >= $before && $blockdata->timemodified <= $after);
+        $this->assertTrue($blockdata->timecreated >= $before && $blockdata->timecreated <= $after);
+
+        // Get block from manager.
+        $blockmanager->load_blocks();
+        $blocks = $blockmanager->get_blocks_for_region($regionname);
+        $block = reset($blocks);
+
+        // Wait until at least the next second.
+        while (time() === $after) {
+            usleep(100000);
+        }
+
+        // Update block settings.
+        $this->setAdminUser();
+        $data = (object)['text' => ['text' => 'New text', 'itemid' => 0, 'format' => FORMAT_HTML]];
+        $before = time();
+        $block->instance_config_save($data);
+        $after = time();
+
+        // Check time modified updated, but time created didn't.
+        $newblockdata = $DB->get_record('block_instances', ['blockname' => 'html']);
+        $this->assertTrue(
+                $newblockdata->timemodified >= $before &&
+                $newblockdata->timemodified <= $after &&
+                $newblockdata->timemodified > $blockdata->timemodified);
+        $this->assertEquals($blockdata->timecreated, $newblockdata->timecreated);
+
+        // Also try repositioning the block.
+        while (time() === $after) {
+            usleep(100000);
+        }
+        $before = time();
+        $blockmanager->reposition_block($blockdata->id, $regionname, 10);
+        $after = time();
+        $blockdata = $newblockdata;
+        $newblockdata = $DB->get_record('block_instances', ['blockname' => 'html']);
+        $this->assertTrue(
+                $newblockdata->timemodified >= $before &&
+                $newblockdata->timemodified <= $after &&
+                $newblockdata->timemodified > $blockdata->timemodified);
+        $this->assertEquals($blockdata->timecreated, $newblockdata->timecreated);
+    }
 }
 
 /**

diff --git a/my/lib.php b/my/lib.php
@@ -86,6 +86,8 @@ function my_copy_page($userid, $private=MY_PAGE_PRIVATE, $pagetype='my-index') {
         unset($instance->id);
         $instance->parentcontextid = $usercontext->id;
         $instance->subpagepattern = $page->id;
+        $instance->timecreated = time();
+        $instance->timemodified = $instance->timecreated;
         $instance->id = $DB->insert_record('block_instances', $instance);
         $newblockinstanceids[$originalid] = $instance->id;
         $blockcontext = context_block::instance($instance->id);  // Just creates the context record

diff --git a/version.php b/version.php
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2017062900.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2017062900.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.