From e9831af3c26d933c3cecf2f9bb4ecaffba705dfe Mon Sep 17 00:00:00 2001 From: Serge Gauthier Date: Fri, 16 Oct 2015 14:25:23 -0400 Subject: [PATCH] MDL-51233 tool_lp: Remove unused ID number from learning plan templates Conflicts: admin/tool/lp/version.php --- admin/tool/lp/classes/external.php | 23 +----- admin/tool/lp/classes/form/template.php | 3 - admin/tool/lp/classes/template.php | 4 -- admin/tool/lp/db/install.xml | 3 +- admin/tool/lp/db/upgrade.php | 15 ++++ .../templates/manage_templates_page.mustache | 4 +- admin/tool/lp/tests/externallib_test.php | 70 +++++++++---------- admin/tool/lp/version.php | 2 +- 8 files changed, 52 insertions(+), 72 deletions(-) mode change 100755 => 100644 admin/tool/lp/db/install.xml diff --git a/admin/tool/lp/classes/external.php b/admin/tool/lp/classes/external.php index 0c74171a709..87ed02ecb88 100644 --- a/admin/tool/lp/classes/external.php +++ b/admin/tool/lp/classes/external.php @@ -2251,10 +2251,6 @@ class external extends external_api { PARAM_TEXT, 'Short name for the learning plan template' ); - $idnumber = new external_value( - PARAM_TEXT, - 'If provided, must be a unique string to identify this learning plan template' - ); $duedate = new external_value( PARAM_INT, 'The default due date for instances of this plan.' @@ -2298,7 +2294,6 @@ class external extends external_api { $returns = array( 'id' => $id, 'shortname' => $shortname, - 'idnumber' => $idnumber, 'duedate' => $duedate, 'duedateformatted' => $duedateformatted, 'description' => $description, @@ -2324,12 +2319,6 @@ class external extends external_api { 'Short name for the learning plan template.', VALUE_REQUIRED ); - $idnumber = new external_value( - PARAM_TEXT, - 'If provided, must be a unique string to identify this learning plan template.', - VALUE_DEFAULT, - '' - ); $duedate = new external_value( PARAM_INT, 'The default due date for instances of this plan', @@ -2356,7 +2345,6 @@ class external extends external_api { $params = array( 'shortname' => $shortname, - 'idnumber' => $idnumber, 'duedate' => $duedate, 'description' => $description, 'descriptionformat' => $descriptionformat, @@ -2386,11 +2374,10 @@ class external extends external_api { * @param array $context The context info. * @return \stdClass Record of new template. */ - public static function create_template($shortname, $idnumber, $duedate, $description, $descriptionformat, $visible, $context) { + public static function create_template($shortname, $duedate, $description, $descriptionformat, $visible, $context) { $params = self::validate_parameters(self::create_template_parameters(), array( 'shortname' => $shortname, - 'idnumber' => $idnumber, 'duedate' => $duedate, 'description' => $description, 'descriptionformat' => $descriptionformat, @@ -2542,11 +2529,6 @@ class external extends external_api { 'Short name for the learning plan template.', VALUE_REQUIRED ); - $idnumber = new external_value( - PARAM_TEXT, - 'If provided, must be a unique string to identify this learning plan template.', - VALUE_REQUIRED - ); $duedate = new external_value( PARAM_INT, 'Default due date for instances of this plan', @@ -2570,7 +2552,6 @@ class external extends external_api { $params = array( 'id' => $id, 'shortname' => $shortname, - 'idnumber' => $idnumber, 'duedate' => $duedate, 'description' => $description, 'descriptionformat' => $descriptionformat, @@ -2601,7 +2582,6 @@ class external extends external_api { */ public static function update_template($id, $shortname, - $idnumber, $duedate, $description, $descriptionformat, @@ -2611,7 +2591,6 @@ class external extends external_api { array( 'id' => $id, 'shortname' => $shortname, - 'idnumber' => $idnumber, 'duedate' => $duedate, 'description' => $description, 'descriptionformat' => $descriptionformat, diff --git a/admin/tool/lp/classes/form/template.php b/admin/tool/lp/classes/form/template.php index afec9d27533..12785faf82f 100644 --- a/admin/tool/lp/classes/form/template.php +++ b/admin/tool/lp/classes/form/template.php @@ -59,9 +59,6 @@ class template extends moodleform { $mform->addElement('editor', 'description', get_string('description', 'tool_lp'), array('rows' => 4)); $mform->setType('description', PARAM_TEXT); - $mform->addElement('text', 'idnumber', - get_string('idnumber', 'tool_lp')); - $mform->setType('idnumber', PARAM_TEXT); $mform->addElement('selectyesno', 'visible', get_string('visible', 'tool_lp')); $mform->addElement('date_time_selector', diff --git a/admin/tool/lp/classes/template.php b/admin/tool/lp/classes/template.php index 49dbbf3a021..716510c0945 100644 --- a/admin/tool/lp/classes/template.php +++ b/admin/tool/lp/classes/template.php @@ -59,10 +59,6 @@ class template extends persistent { 'type' => PARAM_INT, 'default' => FORMAT_HTML ), - 'idnumber' => array( - 'default' => '', - 'type' => PARAM_TEXT, - ), 'duedate' => array( 'default' => 0, 'type' => PARAM_INT, diff --git a/admin/tool/lp/db/install.xml b/admin/tool/lp/db/install.xml old mode 100755 new mode 100644 index 52e21d67bac..916a64bb5c5 --- a/admin/tool/lp/db/install.xml +++ b/admin/tool/lp/db/install.xml @@ -1,5 +1,5 @@ - @@ -93,7 +93,6 @@ - diff --git a/admin/tool/lp/db/upgrade.php b/admin/tool/lp/db/upgrade.php index c53b0241d92..654a02b995f 100644 --- a/admin/tool/lp/db/upgrade.php +++ b/admin/tool/lp/db/upgrade.php @@ -167,5 +167,20 @@ function xmldb_tool_lp_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2015052414, 'tool', 'lp'); } + if ($oldversion < 2015052416) { + + // Define field idnumber to be dropped from tool_lp_template. + $table = new xmldb_table('tool_lp_template'); + $field = new xmldb_field('idnumber'); + + // Conditionally launch drop field idnumber. + if ($dbman->field_exists($table, $field)) { + $dbman->drop_field($table, $field); + } + + // Lp savepoint reached. + upgrade_plugin_savepoint(true, 2015052416, 'tool', 'lp'); + } + return true; } diff --git a/admin/tool/lp/templates/manage_templates_page.mustache b/admin/tool/lp/templates/manage_templates_page.mustache index 208851b5cf7..cab28a428d2 100644 --- a/admin/tool/lp/templates/manage_templates_page.mustache +++ b/admin/tool/lp/templates/manage_templates_page.mustache @@ -28,7 +28,7 @@ * data-action = deletetemplate Context variables required for this template: - * templates - array of objects containing id, shortname, idnumber, visible, canmanage, contextname + * templates - array of objects containing id, shortname, visible, canmanage, contextname * navigation - array of strings containing buttons for navigation }}
@@ -44,7 +44,7 @@ {{#templates}} - {{shortname}} {{idnumber}} {{^visible}}{{#str}}hiddenhint, tool_lp{{/str}}{{/visible}} + {{shortname}} {{^visible}}{{#str}}hiddenhint, tool_lp{{/str}}{{/visible}} {{contextname}} {{#canmanage}} diff --git a/admin/tool/lp/tests/externallib_test.php b/admin/tool/lp/tests/externallib_test.php index de377656f1e..25ace4609a7 100644 --- a/admin/tool/lp/tests/externallib_test.php +++ b/admin/tool/lp/tests/externallib_test.php @@ -1214,7 +1214,7 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { $syscontext = context_system::instance(); // Create a template. - $template = external::create_template('shortname', 'idnumber', time(), 'description', FORMAT_HTML, true, + $template = external::create_template('shortname', time(), 'description', FORMAT_HTML, true, array('contextid' => $syscontext->id)); $template = (object) external_api::clean_returnvalue(external::create_template_returns(), $template); @@ -1251,7 +1251,7 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { $syscontext = context_system::instance(); // Create a template. - $template = external::create_template('shortname', 'idnumber', time(), 'description', FORMAT_HTML, true, + $template = external::create_template('shortname', time(), 'description', FORMAT_HTML, true, array('contextid' => $syscontext->id)); $template = (object) external_api::clean_returnvalue(external::create_template_returns(), $template); @@ -1293,7 +1293,7 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { $syscontext = context_system::instance(); // Create a template. - $template = external::create_template('shortname', 'idnumber', time(), 'description', FORMAT_HTML, true, + $template = external::create_template('shortname', time(), 'description', FORMAT_HTML, true, array('contextid' => $syscontext->id)); $template = (object) external_api::clean_returnvalue(external::create_template_returns(), $template); @@ -1355,7 +1355,7 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { $syscontext = context_system::instance(); // Create a template. - $template = external::create_template('shortname', 'idnumber', time(), 'description', FORMAT_HTML, true, + $template = external::create_template('shortname', time(), 'description', FORMAT_HTML, true, array('contextid' => $syscontext->id)); $template = (object) external_api::clean_returnvalue(external::create_template_returns(), $template); @@ -1438,7 +1438,7 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // A user without permission. $this->setUser($this->user); try { - $result = external::create_template('shortname', 'idnumber', 0, 'description', FORMAT_HTML, true, + $result = external::create_template('shortname', 0, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); $this->fail('Invalid permissions'); } catch (required_capability_exception $e) { @@ -1447,7 +1447,7 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // A user without permission in a category. $this->setUser($this->catuser); try { - $result = external::create_template('shortname', 'idnumber', 0, 'description', FORMAT_HTML, true, + $result = external::create_template('shortname', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); $this->fail('Invalid permissions'); } catch (required_capability_exception $e) { @@ -1455,14 +1455,14 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // A user with permissions in the system. $this->setUser($this->creator); - $result = external::create_template('shortname', 'idnumber', 0, 'description', FORMAT_HTML, true, + $result = external::create_template('shortname', 0, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); $result = external_api::clean_returnvalue(external::create_template_returns(), $result); $this->assertEquals('shortname', $result['shortname']); $this->assertEquals($syscontextid, $result['contextid']); $this->assertNotEmpty($result['id']); - $result = external::create_template('catshortname', 'catid', 0, 'description', FORMAT_HTML, true, + $result = external::create_template('catshortname', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); $result = external_api::clean_returnvalue(external::create_template_returns(), $result); $this->assertEquals('catshortname', $result['shortname']); @@ -1472,13 +1472,13 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // A user with permissions in the category. $this->setUser($this->catcreator); try { - $result = external::create_template('sysshortname', 'sysidnumber', 0, 'description', FORMAT_HTML, true, + $result = external::create_template('sysshortname', 0, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); $this->fail('Invalid permissions'); } catch (required_capability_exception $e) { } - $result = external::create_template('catshortname2', 'catid2', 0, 'description', FORMAT_HTML, true, + $result = external::create_template('catshortname2', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); $result = external_api::clean_returnvalue(external::create_template_returns(), $result); $this->assertEquals('catshortname2', $result['shortname']); @@ -1500,9 +1500,9 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // Creating two templates. $this->setUser($this->creator); - $systemplate = external::create_template('sys', 'sysid', $duedate, 'description', FORMAT_HTML, true, + $systemplate = external::create_template('sys', $duedate, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); - $cattemplate = external::create_template('cat', 'catid', $duedate, 'description', FORMAT_HTML, true, + $cattemplate = external::create_template('cat', $duedate, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); // User without permissions to read in system. @@ -1537,7 +1537,6 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { $result = external_api::clean_returnvalue(external::read_template_returns(), $result); $this->assertEquals($cattemplate->id, $result['id']); $this->assertEquals('cat', $result['shortname']); - $this->assertEquals('catid', $result['idnumber']); $this->assertEquals('description', $result['description']); $this->assertEquals(FORMAT_HTML, $result['descriptionformat']); $this->assertEquals(true, $result['visible']); @@ -1552,7 +1551,6 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { $result = external_api::clean_returnvalue(external::read_template_returns(), $result); $this->assertEquals($systemplate->id, $result['id']); $this->assertEquals('sys', $result['shortname']); - $this->assertEquals('sysid', $result['idnumber']); $this->assertEquals('description', $result['description']); $this->assertEquals(FORMAT_HTML, $result['descriptionformat']); $this->assertEquals(true, $result['visible']); @@ -1563,7 +1561,6 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { $result = external_api::clean_returnvalue(external::read_template_returns(), $result); $this->assertEquals($cattemplate->id, $result['id']); $this->assertEquals('cat', $result['shortname']); - $this->assertEquals('catid', $result['idnumber']); $this->assertEquals('description', $result['description']); $this->assertEquals(FORMAT_HTML, $result['descriptionformat']); $this->assertEquals(true, $result['visible']); @@ -1585,21 +1582,21 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // Creating two templates. $this->setUser($this->creator); - $systemplate = external::create_template('sys', 'sysid', $duedate, 'description', FORMAT_HTML, true, + $systemplate = external::create_template('sys', $duedate, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); - $cattemplate = external::create_template('cat', 'catid', $duedate, 'description', FORMAT_HTML, true, + $cattemplate = external::create_template('cat', $duedate, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); // Trying to update in a without permissions. $this->setUser($this->user); try { - external::update_template($systemplate->id, 'a', 'b', 1234, 'c', FORMAT_MARKDOWN, false); + external::update_template($systemplate->id, 'a', 1234, 'c', FORMAT_MARKDOWN, false); $this->fail('Invalid permissions'); } catch (required_capability_exception $e) { } try { - external::update_template($cattemplate->id, 'a', 'b', 1234, 'c', FORMAT_MARKDOWN, false); + external::update_template($cattemplate->id, 'a', 1234, 'c', FORMAT_MARKDOWN, false); $this->fail('Invalid permissions'); } catch (required_capability_exception $e) { } @@ -1607,7 +1604,7 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // User with permissions to update in category. $this->setUser($this->catcreator); try { - external::update_template($systemplate->id, 'a', 'b', 1234, 'c', FORMAT_MARKDOWN, false); + external::update_template($systemplate->id, 'a', 1234, 'c', FORMAT_MARKDOWN, false); $this->fail('Invalid permissions'); } catch (required_capability_exception $e) { } @@ -1616,14 +1613,13 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { $date->modify('+1 year'); $duedateupdated = $date->getTimestamp(); - $result = external::update_template($cattemplate->id, 'a', 'b', $duedateupdated, 'c', FORMAT_MARKDOWN, false); + $result = external::update_template($cattemplate->id, 'a', $duedateupdated, 'c', FORMAT_MARKDOWN, false); $result = external_api::clean_returnvalue(external::update_template_returns(), $result); $this->assertTrue($result); $result = external::read_template($cattemplate->id); $result = external_api::clean_returnvalue(external::read_template_returns(), $result); $this->assertEquals($cattemplate->id, $result['id']); $this->assertEquals('a', $result['shortname']); - $this->assertEquals('b', $result['idnumber']); $this->assertEquals('c', $result['description']); $this->assertEquals(FORMAT_MARKDOWN, $result['descriptionformat']); $this->assertEquals(0, $result['visible']); @@ -1632,28 +1628,26 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // User with permissions to update in the system. $this->setUser($this->creator); - $result = external::update_template($systemplate->id, 'x1', 'y1', $duedateupdated, 'z1', FORMAT_PLAIN, false); + $result = external::update_template($systemplate->id, 'x1', $duedateupdated, 'z1', FORMAT_PLAIN, false); $result = external_api::clean_returnvalue(external::update_template_returns(), $result); $this->assertTrue($result); $result = external::read_template($systemplate->id); $result = external_api::clean_returnvalue(external::read_template_returns(), $result); $this->assertEquals($systemplate->id, $result['id']); $this->assertEquals('x1', $result['shortname']); - $this->assertEquals('y1', $result['idnumber']); $this->assertEquals('z1', $result['description']); $this->assertEquals(FORMAT_PLAIN, $result['descriptionformat']); $this->assertEquals(0, $result['visible']); $this->assertEquals($duedateupdated, $result['duedate']); $this->assertEquals(userdate($duedateupdated), $result['duedateformatted']); - $result = external::update_template($cattemplate->id, 'x2', 'y2', $duedateupdated, 'z2', FORMAT_PLAIN, true); + $result = external::update_template($cattemplate->id, 'x2', $duedateupdated, 'z2', FORMAT_PLAIN, true); $result = external_api::clean_returnvalue(external::update_template_returns(), $result); $this->assertTrue($result); $result = external::read_template($cattemplate->id); $result = external_api::clean_returnvalue(external::read_template_returns(), $result); $this->assertEquals($cattemplate->id, $result['id']); $this->assertEquals('x2', $result['shortname']); - $this->assertEquals('y2', $result['idnumber']); $this->assertEquals('z2', $result['description']); $this->assertEquals(FORMAT_PLAIN, $result['descriptionformat']); $this->assertEquals(1, $result['visible']); @@ -1671,11 +1665,11 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // Creating a few templates. $this->setUser($this->creator); - $sys1 = external::create_template('sys1', 'sysid1', 0, 'description', FORMAT_HTML, true, + $sys1 = external::create_template('sys1', 0, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); - $cat1 = external::create_template('cat1', 'catid1', 0, 'description', FORMAT_HTML, true, + $cat1 = external::create_template('cat1', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); - $cat2 = external::create_template('cat2', 'catid2', 0, 'description', FORMAT_HTML, true, + $cat2 = external::create_template('cat2', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); $this->assertTrue($DB->record_exists('tool_lp_template', array('id' => $sys1->id))); $this->assertTrue($DB->record_exists('tool_lp_template', array('id' => $cat1->id))); @@ -1728,13 +1722,13 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // Creating a few templates. $this->setUser($this->creator); - $sys1 = external::create_template('sys1', 'sysid1', 0, 'description', FORMAT_HTML, true, + $sys1 = external::create_template('sys1', 0, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); - $sys2 = external::create_template('sys2', 'sysid2', 0, 'description', FORMAT_HTML, true, + $sys2 = external::create_template('sys2', 0, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); - $cat1 = external::create_template('cat1', 'catid1', 0, 'description', FORMAT_HTML, true, + $cat1 = external::create_template('cat1', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); - $cat2 = external::create_template('cat2', 'catid2', 0, 'description', FORMAT_HTML, true, + $cat2 = external::create_template('cat2', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); // User without permission. @@ -1774,15 +1768,15 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { // Creating a few templates. $this->setUser($this->creator); - $sys1 = external::create_template('sys1', 'sysid1', 0, 'description', FORMAT_HTML, true, + $sys1 = external::create_template('sys1', 0, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); - $sys2 = external::create_template('sys2', 'sysid2', 0, 'description', FORMAT_HTML, true, + $sys2 = external::create_template('sys2', 0, 'description', FORMAT_HTML, true, array('contextid' => $syscontextid)); - $cat1 = external::create_template('cat1', 'catid1', 0, 'description', FORMAT_HTML, true, + $cat1 = external::create_template('cat1', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); - $cat2 = external::create_template('cat2', 'catid2', 0, 'description', FORMAT_HTML, true, + $cat2 = external::create_template('cat2', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); - $cat3 = external::create_template('cat3', 'catid3', 0, 'description', FORMAT_HTML, true, + $cat3 = external::create_template('cat3', 0, 'description', FORMAT_HTML, true, array('contextid' => $catcontextid)); // User without permission. diff --git a/admin/tool/lp/version.php b/admin/tool/lp/version.php index 96da15d8ee1..02852292866 100644 --- a/admin/tool/lp/version.php +++ b/admin/tool/lp/version.php @@ -25,7 +25,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2015052415; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2015052416; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2014110400; // Requires this Moodle version. $plugin->component = 'tool_lp'; // Full name of the plugin (used for diagnostics).