From 557554f99fda6992ff48e96eafc99cd1c3497cea Mon Sep 17 00:00:00 2001 From: sam marshall Date: Tue, 16 May 2017 14:49:52 +0100 Subject: [PATCH] MDL-58957 Global search: Add time fields to block_instances --- backup/moodle2/backup_stepslib.php | 5 +- backup/moodle2/restore_stepslib.php | 8 +++ backup/moodle2/tests/moodle2_test.php | 67 ++++++++++++++++++++++- blocks/html/lib.php | 3 +- blocks/moodleblock.class.php | 4 +- blocks/upgrade.txt | 6 ++ lib/blocklib.php | 6 ++ lib/db/install.xml | 3 + lib/db/upgrade.php | 53 ++++++++++++++++++ lib/testing/generator/block_generator.php | 7 +++ lib/tests/blocklib_test.php | 66 ++++++++++++++++++++++ my/lib.php | 2 + version.php | 2 +- 13 files changed, 225 insertions(+), 7 deletions(-) diff --git a/backup/moodle2/backup_stepslib.php b/backup/moodle2/backup_stepslib.php index 2ea524856bda8..b0c6e899866c8 100644 --- 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 index f88649614f97f..00132f6123067 100644 --- 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 index a5f531c829fc7..21773b902b8b5 100644 --- 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('~.*?~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 index 531a32a5dbfc4..a86b827efc732 100644 --- 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 index 57ae2b346f5e5..a81f7ac71bd77 100644 --- 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 index 0fa83c71d5ca2..f34b35d995056 100644 --- 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 index 09bc9478c0f1d..acdaab3b28d22 100644 --- 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 index ce5dda2b38c4f..2e80c6fd669fa 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -2493,6 +2493,8 @@ + + @@ -2500,6 +2502,7 @@ + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 7798fecf76014..ef36f5d97e0fb 100644 --- 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 index 1305ee1ac6a1f..ce5da1fc99a82 100644 --- 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 index 6eb0fed8177fc..1dcbb3c17bed6 100644 --- 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 index b9b523439b97c..85ea1e8555988 100644 --- 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 index b984e3c76a59d..b75f2b4c9ee5b 100644 --- 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.