From af3fa58f3c8fb3dfe5c69a677c62a2f227f4f107 Mon Sep 17 00:00:00 2001 From: Issam Taboubi Date: Fri, 12 Feb 2016 13:48:00 -0500 Subject: [PATCH] MDL-52755 tool_lp: Preserve competencies sortorder when plan completed --- admin/tool/lp/classes/api.php | 4 + .../tool/lp/classes/user_competency_plan.php | 9 +- admin/tool/lp/db/install.xml | 1 + admin/tool/lp/db/upgrade.php | 15 ++++ admin/tool/lp/tests/api_test.php | 90 +++++++++++++++++++ admin/tool/lp/tests/generator/lib.php | 4 + 6 files changed, 120 insertions(+), 3 deletions(-) diff --git a/admin/tool/lp/classes/api.php b/admin/tool/lp/classes/api.php index e1be28fbd8c..a50227ad768 100644 --- a/admin/tool/lp/classes/api.php +++ b/admin/tool/lp/classes/api.php @@ -3552,6 +3552,7 @@ class api { $competencies = $plan->get_competencies(); $usercompetencies = user_competency::get_multiple($plan->get_userid(), $competencies); + $i = 0; foreach ($competencies as $competency) { $found = false; @@ -3561,6 +3562,7 @@ class api { $ucprecord = $uc->to_record(); $ucprecord->planid = $plan->get_id(); + $ucprecord->sortorder = $i; unset($ucprecord->id); unset($ucprecord->status); unset($ucprecord->reviewerid); @@ -3577,8 +3579,10 @@ class api { if (!$found) { $usercompetencyplan = user_competency_plan::create_relation($plan->get_userid(), $competency->get_id(), $plan->get_id()); + $usercompetencyplan->set_sortorder($i); $usercompetencyplan->create(); } + $i++; } } diff --git a/admin/tool/lp/classes/user_competency_plan.php b/admin/tool/lp/classes/user_competency_plan.php index 09354a92f33..ba69c2ee3bc 100644 --- a/admin/tool/lp/classes/user_competency_plan.php +++ b/admin/tool/lp/classes/user_competency_plan.php @@ -63,6 +63,10 @@ class user_competency_plan extends persistent { 'planid' => array( 'type' => PARAM_INT, ), + 'sortorder' => array( + 'type' => PARAM_INT, + 'default' => null, + ), ); } @@ -171,14 +175,13 @@ class user_competency_plan extends persistent { public static function list_competencies($planid, $userid) { global $DB; - // TODO Fix ordering. The order set in template_competency, or plan_competency is not applied here. - // Perhaps we should have copied the sortorder here as well. $sql = 'SELECT c.* FROM {' . competency::TABLE . '} c JOIN {' . self::TABLE . '} ucp ON ucp.competencyid = c.id AND ucp.userid = :userid - WHERE ucp.planid = :planid'; + WHERE ucp.planid = :planid + ORDER BY ucp.sortorder ASC'; $params = array('userid' => $userid, 'planid' => $planid); $results = $DB->get_recordset_sql($sql, $params); diff --git a/admin/tool/lp/db/install.xml b/admin/tool/lp/db/install.xml index d9e8ed48596..b9aaf57a91b 100755 --- a/admin/tool/lp/db/install.xml +++ b/admin/tool/lp/db/install.xml @@ -191,6 +191,7 @@ + diff --git a/admin/tool/lp/db/upgrade.php b/admin/tool/lp/db/upgrade.php index edc560e1295..b73ba662269 100644 --- a/admin/tool/lp/db/upgrade.php +++ b/admin/tool/lp/db/upgrade.php @@ -713,5 +713,20 @@ function xmldb_tool_lp_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2016020900, 'tool', 'lp'); } + if ($oldversion < 2016020901) { + + // Define field note to be added to tool_lp_user_competency_plan. + $table = new xmldb_table('tool_lp_user_competency_plan'); + $field = new xmldb_field('sortorder', XMLDB_TYPE_INTEGER, '10', null, null, null, null); + + // Conditionally launch add field note. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Lp savepoint reached. + upgrade_plugin_savepoint(true, 2016020901, 'tool', 'lp'); + } + return true; } diff --git a/admin/tool/lp/tests/api_test.php b/admin/tool/lp/tests/api_test.php index eb73096da9f..b37063ec666 100644 --- a/admin/tool/lp/tests/api_test.php +++ b/admin/tool/lp/tests/api_test.php @@ -1509,6 +1509,96 @@ class tool_lp_api_testcase extends advanced_testcase { $this->assertEquals(0, \tool_lp\user_competency_plan::count_records()); } + /** + * Test completing plan does not change the order of competencies. + */ + public function test_complete_plan_doesnot_change_order() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $dg = $this->getDataGenerator(); + $lpg = $this->getDataGenerator()->get_plugin_generator('tool_lp'); + + $syscontext = context_system::instance(); + + // Create users and roles for the test. + $user = $dg->create_user(); + + // Create a framework and assign competencies. + $framework = $lpg->create_framework(); + $c1 = $lpg->create_competency(array('competencyframeworkid' => $framework->get_id())); + $c2 = $lpg->create_competency(array('competencyframeworkid' => $framework->get_id())); + $c3 = $lpg->create_competency(array('competencyframeworkid' => $framework->get_id())); + + // Create two plans and assign competencies. + $plan = $lpg->create_plan(array('userid' => $user->id)); + + $lpg->create_plan_competency(array('planid' => $plan->get_id(), 'competencyid' => $c1->get_id())); + $lpg->create_plan_competency(array('planid' => $plan->get_id(), 'competencyid' => $c2->get_id())); + $lpg->create_plan_competency(array('planid' => $plan->get_id(), 'competencyid' => $c3->get_id())); + + // Changing competencies order in plan competency. + api::reorder_plan_competency($plan->get_id(), $c1->get_id(), $c3->get_id()); + + $competencies = api::list_plan_competencies($plan); + $this->assertEquals($c2->get_id(), $competencies[0]->competency->get_id()); + $this->assertEquals($c3->get_id(), $competencies[1]->competency->get_id()); + $this->assertEquals($c1->get_id(), $competencies[2]->competency->get_id()); + + // Completing plan. + api::complete_plan($plan); + + $competencies = api::list_plan_competencies($plan); + + // Completing plan does not change order. + $this->assertEquals($c2->get_id(), $competencies[0]->competency->get_id()); + $this->assertEquals($c3->get_id(), $competencies[1]->competency->get_id()); + $this->assertEquals($c1->get_id(), $competencies[2]->competency->get_id()); + + // Testing plan based on template. + $template = $lpg->create_template(); + $framework = $lpg->create_framework(); + $c1 = $lpg->create_competency(array('competencyframeworkid' => $framework->get_id())); + $c2 = $lpg->create_competency(array('competencyframeworkid' => $framework->get_id())); + $c3 = $lpg->create_competency(array('competencyframeworkid' => $framework->get_id())); + + $lpg->create_template_competency(array( + 'templateid' => $template->get_id(), + 'competencyid' => $c1->get_id() + )); + $lpg->create_template_competency(array( + 'templateid' => $template->get_id(), + 'competencyid' => $c2->get_id() + )); + $lpg->create_template_competency(array( + 'templateid' => $template->get_id(), + 'competencyid' => $c3->get_id() + )); + // Reorder competencies in template. + api::reorder_template_competency($template->get_id(), $c1->get_id(), $c3->get_id()); + + // Create plan from template. + $plan = api::create_plan_from_template($template->get_id(), $user->id); + + $competencies = api::list_plan_competencies($plan); + + // Completing plan does not change order. + $this->assertEquals($c2->get_id(), $competencies[0]->competency->get_id()); + $this->assertEquals($c3->get_id(), $competencies[1]->competency->get_id()); + $this->assertEquals($c1->get_id(), $competencies[2]->competency->get_id()); + + // Completing plan. + api::complete_plan($plan); + + $competencies = api::list_plan_competencies($plan); + + // Completing plan does not change order. + $this->assertEquals($c2->get_id(), $competencies[0]->competency->get_id()); + $this->assertEquals($c3->get_id(), $competencies[1]->competency->get_id()); + $this->assertEquals($c1->get_id(), $competencies[2]->competency->get_id()); + } + /** * Test remove plan and the managing of archived user competencies. */ diff --git a/admin/tool/lp/tests/generator/lib.php b/admin/tool/lp/tests/generator/lib.php index cc6ad87c845..47df21c50bb 100644 --- a/admin/tool/lp/tests/generator/lib.php +++ b/admin/tool/lp/tests/generator/lib.php @@ -332,6 +332,10 @@ class tool_lp_generator extends component_generator_base { throw new coding_exception('The planid value is required.'); } + if (!isset($record->sortorder)) { + $record->sortorder = 0; + } + $usercompetencyplan = new user_competency_plan(0, $record); $usercompetencyplan->create();