From 2ec1b70bcd9b85e720b24803ca8cfa4f2f02fd75 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Tue, 3 Nov 2015 19:45:41 +0800 Subject: [PATCH] MDL-51866 tool_lp: Framework scale cannot be changed once it is used --- .../tool/lp/classes/competency_framework.php | 44 ++++++++++++++++++- .../lp/classes/form/competency_framework.php | 33 +++++++++----- admin/tool/lp/classes/user_competency.php | 19 ++++++++ admin/tool/lp/lang/en/tool_lp.php | 1 + admin/tool/lp/tests/externallib_test.php | 33 ++++++++++++++ 5 files changed, 117 insertions(+), 13 deletions(-) diff --git a/admin/tool/lp/classes/competency_framework.php b/admin/tool/lp/classes/competency_framework.php index 844390b567b..cdf45ed85ea 100644 --- a/admin/tool/lp/classes/competency_framework.php +++ b/admin/tool/lp/classes/competency_framework.php @@ -62,6 +62,9 @@ class competency_framework extends persistent { /** Taxonomy constant. */ const TAXONOMY_VALUE = 'value'; + /** @var static The object before it was updated. */ + protected $beforeupdate; + /** * Get the context. * @@ -113,6 +116,21 @@ class competency_framework extends persistent { ); } + /** + * Hook to execute before validate. + * + * @return void + */ + protected function before_validate() { + $this->beforeupdate = null; + + // During update. + if ($this->get_id()) { + $this->beforeupdate = new competency_framework($this->get_id()); + } + + } + /** * Return the scale. * @@ -165,6 +183,17 @@ class competency_framework extends persistent { return $taxonomies; } + /** + * Returns true when some competencies of the framework have user competencies. + * + * This is useful to determine if the framework, or part of it, should be locked down. + * + * @return boolean + */ + public function has_user_competencies() { + return user_competency::has_records_for_framework($this->get_id()); + } + /** * Convenience method to set taxonomies from an array or string. * @@ -236,10 +265,23 @@ class competency_framework extends persistent { protected function validate_scaleid($value) { global $DB; + // Always validate that the scale exists. if (!$DB->record_exists_select('scale', 'id = :id', array('id' => $value))) { return new lang_string('invalidscaleid', 'error'); } + // During update. + if ($this->get_id()) { + + // Validate that we can only change the scale when it is not used yet. + if ($this->beforeupdate->get_scaleid() != $value) { + if ($this->beforeupdate->has_user_competencies()) { + return new lang_string('errorscalealreadyused', 'tool_lp'); + } + } + + } + return true; } @@ -262,7 +304,7 @@ class competency_framework extends persistent { $scaleinfo = array_shift($scaleconfigurations); if (empty($scaleinfo) || !isset($scaleinfo->scaleid) || $scaleinfo->scaleid != $this->get('scaleid')) { // This should never happen. - return new lang_string('invaliddata', 'error'); + return new lang_string('errorscaleconfiguration', 'tool_lp'); } // Walk through the array to find proficient and default values. diff --git a/admin/tool/lp/classes/form/competency_framework.php b/admin/tool/lp/classes/form/competency_framework.php index 2d301288f75..7591d073e57 100644 --- a/admin/tool/lp/classes/form/competency_framework.php +++ b/admin/tool/lp/classes/form/competency_framework.php @@ -49,6 +49,11 @@ class competency_framework extends moodleform { $mform = $this->_form; $id = $this->_customdata['id']; $context = $this->_customdata['context']; + $framework = null; + + if ($id) { + $framework = api::read_framework($id);; + } $mform->addElement('hidden', 'id'); $mform->setType('id', PARAM_INT); @@ -69,9 +74,16 @@ class competency_framework extends moodleform { $mform->addRule('idnumber', null, 'required', null, 'client'); $scales = get_scales_menu(); - $mform->addElement('select', 'scaleid', get_string('scale', 'tool_lp'), $scales); + $scaleid = $mform->addElement('select', 'scaleid', get_string('scale', 'tool_lp'), $scales); $mform->setType('scaleid', PARAM_INT); $mform->addHelpButton('scaleid', 'scale', 'tool_lp'); + if ($framework && $framework->has_user_competencies()) { + // The scale is used so we "freeze" the element. Though, the javascript code for the scale + // configuration requires this field so we only disable it. It is fine as setting the value + // as a constant will ensure that nobody can change it. And it's validated in the persistent anyway. + $scaleid->updateAttributes(array('disabled' => 'disabled')); + $mform->setConstant('scaleid', $framework->get_scaleid()); + } $mform->addElement('button', 'scaleconfigbutton', get_string('configurescale', 'tool_lp')); // Add js. @@ -99,16 +111,13 @@ class competency_framework extends moodleform { $this->add_action_buttons(true, get_string('savechanges', 'tool_lp')); - if (!empty($id)) { - if (!$this->is_submitted()) { - $framework = api::read_framework($id); - $record = $framework->to_record(); - // Massage for editor API. - $record->description = array('text' => $record->description, 'format' => $record->descriptionformat); - // New hair cut for taxonomies. - $record->taxonomies = $framework->get_taxonomies(); - $this->set_data($record); - } + if ($framework && !$this->is_submitted()) { + $record = $framework->to_record(); + // Massage for editor API. + $record->description = array('text' => $record->description, 'format' => $record->descriptionformat); + // New hair cut for taxonomies. + $record->taxonomies = $framework->get_taxonomies(); + $this->set_data($record); } } @@ -144,7 +153,7 @@ class competency_framework extends moodleform { $framework = new \tool_lp\competency_framework(0, $data); $errors = $framework->get_errors(); - if (isset($errors['scaleconfiguration'])) { + if (isset($errors['scaleconfiguration']) && !isset($errors['scaleid'])) { $errors['scaleid'] = $errors['scaleconfiguration']; unset($errors['scaleconfiguration']); } diff --git a/admin/tool/lp/classes/user_competency.php b/admin/tool/lp/classes/user_competency.php index 0f260047345..b55fb5e34b0 100644 --- a/admin/tool/lp/classes/user_competency.php +++ b/admin/tool/lp/classes/user_competency.php @@ -238,4 +238,23 @@ class user_competency extends persistent { return parent::get_records_select("userid = :userid AND $sql", $params); } + /** + * Checks if any of the competencies of a framework has a user competency record. + * + * @param int $frameworkid The competency framework ID. + * @return boolean + */ + public static function has_records_for_framework($frameworkid) { + global $DB; + + $sql = "SELECT 'x' + FROM {" . self::TABLE . "} uc + JOIN {" . competency::TABLE . "} c + ON uc.competencyid = c.id + WHERE c.competencyframeworkid = ?"; + $params = array($frameworkid); + + return $DB->record_exists_sql($sql, $params); + } + } diff --git a/admin/tool/lp/lang/en/tool_lp.php b/admin/tool/lp/lang/en/tool_lp.php index 08af2fea377..d4c807377bd 100644 --- a/admin/tool/lp/lang/en/tool_lp.php +++ b/admin/tool/lp/lang/en/tool_lp.php @@ -62,6 +62,7 @@ $string['errorcannotsetduedateinthepast'] = 'The due date cannot be set in the p $string['errorcannotchangeapastduedate'] = 'The due date has passed, it cannot be changed.'; $string['errornocompetency'] = '{$a} competency can not be found'; $string['errorplanstatus'] = 'Learning plans \'{$a}\' status unknown'; +$string['errorscalealreadyused'] = 'The scale cannot be changed, it is already being used.'; $string['errorscaleconfiguration'] = 'You must configure the scale by selecting default and proficient values.'; $string['hidden'] = 'Hidden'; $string['hiddenhint'] = '(hidden)'; diff --git a/admin/tool/lp/tests/externallib_test.php b/admin/tool/lp/tests/externallib_test.php index aec307b7ba8..f467efc084e 100644 --- a/admin/tool/lp/tests/externallib_test.php +++ b/admin/tool/lp/tests/externallib_test.php @@ -462,6 +462,39 @@ class tool_lp_external_testcase extends externallib_advanced_testcase { } } + public function test_update_framework_scale() { + $this->setUser($this->creator); + $lpg = $this->getDataGenerator()->get_plugin_generator('tool_lp'); + + $s1 = $this->getDataGenerator()->create_scale(); + + $f1 = $lpg->create_framework(array('scaleid' => 1)); + $f2 = $lpg->create_framework(array('scaleid' => 1)); + $c1 = $lpg->create_competency(array('competencyframeworkid' => $f1->get_id())); + $c2 = $lpg->create_competency(array('competencyframeworkid' => $f2->get_id())); + + $this->assertEquals(1, $f1->get_scaleid()); + + // Make the scale of f2 being used. + $lpg->create_user_competency(array('userid' => $this->user->id, 'competencyid' => $c2->get_id())); + + // Changing the framework where the scale is not used. + $result = external::update_competency_framework($f1->get_id(), 'a', 'a', 'a', FORMAT_PLAIN, 3, $this->scaleconfiguration3, false); + $result = external_api::clean_returnvalue(external::update_competency_framework_returns(), $result); + + $f1 = new \tool_lp\competency_framework($f1->get_id()); + $this->assertEquals(3, $f1->get_scaleid()); + + // Changing the framework where the scale is used. + try { + $result = external::update_competency_framework($f2->get_id(), 'b', 'b', 'b', FORMAT_PLAIN, 3, $this->scaleconfiguration3, false); + $result = external_api::clean_returnvalue(external::update_competency_framework_returns(), $result); + $this->fail('The scale cannot be changed once used.'); + } catch (\tool_lp\invalid_persistent_exception $e) { + $this->assertRegexp('/scaleid/', $e->getMessage()); + } + } + /** * Test we can update a competency framework with read permissions. */