Permalink
Browse files

MDL-53645 tool_lp: Various trivial bugs identified during review

  • Loading branch information...
danpoltawski authored and FMCorz committed Mar 24, 2016
1 parent 4a4bda6 commit d846a535a6c05064e5c04619ca79b26579f8af59
Showing with 26 additions and 50 deletions.
  1. +0 −4 admin/tool/lp/backup/moodle2/restore_tool_lp_plugin.class.php
  2. +0 −1 admin/tool/lp/classes/api.php
  3. +1 −1 admin/tool/lp/classes/course_competency.php
  4. +0 −2 admin/tool/lp/classes/course_competency_rule_form_element.php
  5. +2 −2 admin/tool/lp/classes/course_module_competency.php
  6. +2 −2 admin/tool/lp/classes/external.php
  7. +0 −3 admin/tool/lp/classes/external/course_module_summary_exporter.php
  8. +1 −0 admin/tool/lp/classes/external/exporter.php
  9. +0 −1 admin/tool/lp/classes/external/user_competency_summary_exporter.php
  10. +0 −1 admin/tool/lp/classes/external/user_summary_exporter.php
  11. +1 −3 admin/tool/lp/classes/form/competency.php
  12. +0 −1 admin/tool/lp/classes/form/user_evidence.php
  13. +0 −2 admin/tool/lp/classes/output/plan_page.php
  14. +2 −2 admin/tool/lp/classes/output/template_cohorts_table.php
  15. +1 −2 admin/tool/lp/classes/output/template_plans_table.php
  16. +1 −4 admin/tool/lp/classes/output/user_competency_course_navigation.php
  17. +0 −2 admin/tool/lp/classes/output/user_competency_summary.php
  18. +1 −1 admin/tool/lp/classes/output/user_competency_summary_in_course.php
  19. +1 −1 admin/tool/lp/classes/output/user_competency_summary_in_plan.php
  20. +3 −3 admin/tool/lp/classes/page_helper.php
  21. +3 −2 admin/tool/lp/classes/plan.php
  22. +1 −1 admin/tool/lp/classes/plan_competency.php
  23. +0 −1 admin/tool/lp/classes/template_cohort.php
  24. +3 −3 admin/tool/lp/classes/template_competency.php
  25. +0 −2 admin/tool/lp/classes/user_competency_plan.php
  26. +1 −0 admin/tool/lp/classes/user_evidence_competency.php
  27. +0 −1 admin/tool/lp/editcompetency.php
  28. +0 −1 admin/tool/lp/editplan.php
  29. +0 −1 admin/tool/lp/edittemplate.php
  30. +2 −0 admin/tool/lp/lib.php
@@ -90,8 +90,6 @@ public function process_course_competency_settings($data) {
* @param array $data The data.
*/
public function process_course_competency($data) {
global $DB;
$data = (object) $data;
// Mapping the competency by ID numbers.
@@ -128,8 +126,6 @@ public function process_course_competency($data) {
* @param array $data The data.
*/
public function process_course_module_competency($data) {
global $DB;
$data = (object) $data;
// Mapping the competency by ID numbers.
@@ -3895,7 +3895,6 @@ public static function delete_user_evidence_competency($userevidenceorid, $compe
* @return bool
*/
public static function request_review_of_user_evidence_linked_competencies($id) {
$onlyidle = false;
$userevidence = new user_evidence($id);
$context = $userevidence->get_context();
$userid = $userevidence->get_userid();
@@ -180,7 +180,7 @@ public static function get_ruleoutcome_name($ruleoutcome) {
$strname = 'complete';
break;
default:
throw new \moodle_exception('errorcoursecompetencyrule', 'tool_lp', '', $rule);
throw new \moodle_exception('errorcoursecompetencyrule', 'tool_lp', '', $ruleoutcome);
break;
}
@@ -51,8 +51,6 @@ class tool_lp_course_competency_rule_form_element extends MoodleQuickForm_select
* @param mixed $attributes Either a typical HTML attribute string or an associative array.
*/
public function __construct($elementName=null, $elementLabel=null, $options=array(), $attributes=null) {
global $OUTPUT;
if ($elementName == null) {
// This is broken quickforms messing with the constructors.
return;
@@ -126,7 +126,7 @@ public static function get_ruleoutcome_name($ruleoutcome) {
$strname = 'complete';
break;
default:
throw new \moodle_exception('errorcompetencyrule', 'tool_lp', '', $rule);
throw new \moodle_exception('errorcompetencyrule', 'tool_lp', '', $ruleoutcome);
break;
}
@@ -248,7 +248,7 @@ public static function get_competency($cmid, $competencyid) {
$result = $DB->get_record_sql($sql, $params);
if (!$result) {
throw new coding_exception('The competency does not belong to this course module: ' . $competencyid . ', ' . $cmid);
throw new \coding_exception('The competency does not belong to this course module: ' . $competencyid . ', ' . $cmid);
}
return new competency(0, $result);
@@ -1568,7 +1568,7 @@ public static function list_course_module_competencies($cmid) {
$result = array();
foreach ($apiresult as $cmrecord) {
$one = new stdClass();
$one = new \stdClass();
$exporter = new competency_exporter($cmrecord['competency']);
$one->competency = $exporter->export($output);
$exporter = new course_module_competency_exporter($cmrecord['coursemodulecompetency']);
@@ -1641,6 +1641,7 @@ public static function list_course_modules_using_competency($competencyid, $cour
$coursemodules = api::list_course_modules_using_competency($params['competencyid'], $params['courseid']);
$result = array();
// FIXME: Test this code and find it broken.
$fastmodinfo = get_fast_modinfo($cm->course);
foreach ($coursemodules as $cmid) {
@@ -2610,7 +2611,6 @@ public static function count_competencies_in_template_parameters() {
* @return int
*/
public static function count_competencies_in_template($templateid) {
global $PAGE;
$params = self::validate_parameters(self::count_competencies_in_template_parameters(),
array(
'id' => $templateid,
@@ -25,7 +25,6 @@
defined('MOODLE_INTERNAL') || die();
use renderer_base;
use moodle_url;
/**
@@ -41,8 +40,6 @@ protected static function define_related() {
}
protected function get_other_values(renderer_base $output) {
global $CFG;
$context = $this->related['cm']->context;
return array(
@@ -122,6 +122,7 @@ public function __construct($data, $related = array()) {
$othervalues = $this->get_other_values($output);
if (array_intersect_key($values, $othervalues)) {
// Attempt to replace a standard property.
//FIXME: property is not defined.
throw new coding_exception('Cannot override a standard property value: ' . $property);
}
$values += $othervalues;
@@ -95,7 +95,6 @@ protected function get_other_values(renderer_base $output) {
));
$result->competency = $exporter->export($output);
$context = context_user::instance($this->related['user']->id);
$result->cangrade = user_competency::can_grade_user($this->related['user']->id);
if ($this->related['user']) {
$exporter = new user_summary_exporter($this->related['user']);
@@ -48,7 +48,6 @@ protected function get_other_values(renderer_base $output) {
$profileurl = (new moodle_url('/user/profile.php', array('id' => $this->data->id)))->out(false);
$identityfields = array_flip(explode(',', $CFG->showuseridentity));
$identity = '';
$data = $this->data;
foreach ($identityfields as $field => $index) {
if (!empty($data->$field)) {
@@ -82,17 +82,15 @@ public function definition() {
$mform->setType('idnumber', PARAM_TEXT);
$mform->addRule('idnumber', null, 'required', null, 'client');
$frameworkscale = $framework->get_scale();
$scales = array(null => get_string('inheritfromframework', 'tool_lp')) + get_scales_menu();
$scaleid = $mform->addElement('select', 'scaleid', get_string('scale', 'tool_lp'), $scales);
$mform->setType('scaleid', PARAM_INT);
$mform->addHelpButton('scaleid', 'scale', 'tool_lp');
$mform->addElement('hidden', 'scaleconfiguration', '', array('id' => 'tool_lp_scaleconfiguration'));
$mform->setType('scaleconfiguration', PARAM_RAW);
$scaleconfig = $mform->addElement('button', 'scaleconfigbutton', get_string('configurescale', 'tool_lp'));
$mform->addElement('button', 'scaleconfigbutton', get_string('configurescale', 'tool_lp'));
$PAGE->requires->js_call_amd('tool_lp/scaleconfig', 'init', array('#id_scaleid',
'#tool_lp_scaleconfiguration', '#id_scaleconfigbutton'));
@@ -52,7 +52,6 @@ public function definition() {
$mform->addRule('name', null, 'required', null, 'client');
$mform->addElement('editor', 'description', get_string('userevidencedescription', 'tool_lp'), array('rows' => 10));
// TODO MDL-52454 Make PARAM_RAW.
$mform->setType('description', PARAM_RAW);
$mform->addElement('url', 'url', get_string('userevidenceurl', 'tool_lp'), array(), array('usefilepicker' => false));
@@ -103,8 +103,6 @@ public function export_for_template(\renderer_base $output) {
}
$scale = $scales[$framework->get_scaleid()];
$context = $framework->get_context();
// Prepare the data.
$record = new stdClass();
$exporter = new competency_exporter($comp, array('context' => $framework->get_context()));
@@ -57,12 +57,11 @@ class template_cohorts_table extends table_sql {
* @param \tool_lp\template $template The template.
*/
public function __construct($uniqueid, \tool_lp\template $template) {
global $CFG;
parent::__construct($uniqueid);
// This object should not be used without the right permissions.
if (!$template->can_read()) {
throw new required_capability_exception($template->get_context(), 'tool/lp:templateview', 'nopermissions', '');
throw new \required_capability_exception($template->get_context(), 'tool/lp:templateview', 'nopermissions', '');
}
// Set protected properties.
@@ -99,6 +98,7 @@ protected function col_actions($row) {
* Setup the headers for the table.
*/
protected function define_table_columns() {
// FIXME: Should we use these?
$extrafields = get_extra_user_fields($this->context);
// Define headers and columns.
@@ -57,12 +57,11 @@ class template_plans_table extends table_sql {
* @param \tool_lp\template $template The template.
*/
public function __construct($uniqueid, \tool_lp\template $template) {
global $CFG;
parent::__construct($uniqueid);
// This object should not be used without the right permissions.
if (!$template->can_read()) {
throw new required_capability_exception($template->get_context(), 'tool/lp:templateview', 'nopermissions', '');
throw new \required_capability_exception($template->get_context(), 'tool/lp:templateview', 'nopermissions', '');
}
// Set protected properties.
@@ -74,7 +74,7 @@ public function __construct($userid, $competencyid, $courseid, $baseurl) {
* @return stdClass
*/
public function export_for_template(renderer_base $output) {
global $CFG, $DB, $SESSION, $PAGE;
global $CFG, $DB, $PAGE;
$context = context_course::instance($this->courseid);
@@ -97,9 +97,6 @@ public function export_for_template(renderer_base $output) {
$showonlyactiveenrol = get_user_preferences('grade_report_showonlyactiveenrol', $defaultgradeshowactiveenrol);
$showonlyactiveenrol = $showonlyactiveenrol || !has_capability('moodle/course:viewsuspendedusers', $context);
// Fetch current active group.
$groupmode = groups_get_course_groupmode($course);
$users = get_enrolled_users($context, 'tool/lp:coursecompetencygradable', $currentgroup,
'u.*', null, 0, 0, $showonlyactiveenrol);
@@ -64,8 +64,6 @@ public function __construct(user_competency $usercompetency, array $related = ar
* @return stdClass
*/
public function export_for_template(renderer_base $output) {
global $DB;
if (!isset($related['user'])) {
$related['user'] = core_user::get_user($this->usercompetency->get_userid());
}
@@ -73,7 +73,7 @@ public function export_for_template(renderer_base $output) {
$usercompetencycourse = api::get_user_competency_in_course($this->courseid, $this->userid, $this->competencyid);
$competency = $usercompetencycourse->get_competency();
if (empty($usercompetencycourse) || empty($competency)) {
throw new invalid_parameter_exception('Invalid params. The competency does not belong to the course.');
throw new \invalid_parameter_exception('Invalid params. The competency does not belong to the course.');
}
$relatedcompetencies = api::list_related_competencies($competency->get_id());
@@ -71,7 +71,7 @@ public function export_for_template(\renderer_base $output) {
$usercompetencyplan = $pc->usercompetencyplan;
if (empty($competency)) {
throw new invalid_parameter_exception('Invalid params. The competency does not belong to the plan.');
throw new \invalid_parameter_exception('Invalid params. The competency does not belong to the plan.');
}
$relatedcompetencies = api::list_related_competencies($competency->get_id());
@@ -60,7 +60,7 @@ class page_helper {
* - Return URL (course competencies page)
*/
public static function setup_for_course(moodle_url $url, $course, $subtitle = '') {
global $PAGE, $SITE;
global $PAGE;
$context = context_course::instance($course->id);
@@ -191,7 +191,7 @@ public static function setup_for_plan($userid, moodle_url $url, $plan = null, $s
// Check that the user is a valid user.
$user = core_user::get_user($userid);
if (!$user || !core_user::is_real_user($userid)) {
throw new moodle_exception('invaliduser', 'error');
throw new \moodle_exception('invaliduser', 'error');
}
$context = context_user::instance($user->id);
@@ -265,7 +265,7 @@ public static function setup_for_user_evidence($userid, moodle_url $url, $eviden
// Check that the user is a valid user.
$user = core_user::get_user($userid);
if (!$user || !core_user::is_real_user($userid)) {
throw new moodle_exception('invaliduser', 'error');
throw new \moodle_exception('invaliduser', 'error');
}
$context = context_user::instance($user->id);
@@ -194,7 +194,7 @@ public function get_comment_object() {
require_once($CFG->dirroot . '/comment/lib.php');
if (!$this->get_id()) {
throw new coding_exception('The plan must exist.');
throw new \coding_exception('The plan must exist.');
}
$comment = new comment((object) array(
@@ -222,6 +222,7 @@ public function get_competencies() {
$competencies = user_competency_plan::list_competencies($this->get_id(), $this->get_userid());
} else if ($this->is_based_on_template()) {
// Get the competencies from the template.
// FIXME: why are you passing a second param?
$competencies = template_competency::list_competencies($this->get_templateid(), true);
} else {
// Get the competencies from the plan.
@@ -580,7 +581,7 @@ public static function update_multiple_from_template(template $template) {
global $DB;
if (!$template->is_valid()) {
// As we will bypass this model's validation we rely on the template being validated.
throw new coding_exception('The template must be validated before updating plans.');
throw new \coding_exception('The template must be validated before updating plans.');
}
$params = array(
@@ -104,7 +104,7 @@ public static function get_competency($planid, $competencyid) {
$result = $DB->get_record_sql($sql, $params);
if (!$result) {
throw new coding_exception('The competency does not belong to this plan: ' . $competencyid . ', ' . $planid);
throw new \coding_exception('The competency does not belong to this plan: ' . $competencyid . ', ' . $planid);
}
return new competency(0, $result);
@@ -79,7 +79,6 @@ protected function validate_cohortid($value) {
* @return true|lang_string
*/
protected function validate_templateid($value) {
global $DB;
if (!template::record_exists($value)) {
return new lang_string('invaliddata', 'error');
}
@@ -180,7 +180,7 @@ public static function get_competency($templateid, $competencyid) {
$result = $DB->get_record_sql($sql, $params);
if (!$result) {
throw new coding_exception('The competency does not belong to this template: ' . $competencyid . ', ' . $templateid);
throw new \coding_exception('The competency does not belong to this template: ' . $competencyid . ', ' . $templateid);
}
return new competency(0, $result);
@@ -245,7 +245,7 @@ protected function before_validate() {
*/
protected function validate_competencyid($value) {
if (!competency::record_exists($value)) {
return new lang_string('invaliddata', 'error');
return new \lang_string('invaliddata', 'error');
}
return true;
}
@@ -258,7 +258,7 @@ protected function validate_competencyid($value) {
*/
protected function validate_templateid($value) {
if (!template::record_exists($value)) {
return new lang_string('invaliddata', 'error');
return new \lang_string('invaliddata', 'error');
}
return true;
}
@@ -149,8 +149,6 @@ protected function validate_grade($value) {
* @return true|lang_string
*/
protected function validate_planid($value) {
global $DB;
if (!plan::record_exists($value) ) {
return new lang_string('invalidplan', 'tool_lp');
}
@@ -28,6 +28,7 @@
defined('MOODLE_INTERNAL') || die();
use stdClass;
use lang_string;
/**
* User evidence competency persistent class.
@@ -89,7 +89,6 @@
// Get form data.
$data = $form->get_data();
if ($data) {
require_sesskey();
if (empty($competency)) {
\tool_lp\api::create_competency($data);
$returnmsg = get_string('competencycreated', 'tool_lp');
@@ -79,7 +79,6 @@
$data = $form->get_data();
if ($data) {
require_sesskey();
if (empty($data->id)) {
$plan = \tool_lp\api::create_plan($data);
$returnurl = new moodle_url('/admin/tool/lp/plan.php', ['id' => $plan->get_id()]);
@@ -68,7 +68,6 @@
$data = $form->get_data();
if ($data) {
require_sesskey();
if (empty($data->id)) {
$template = \tool_lp\api::create_template($data);
$returnurl = new moodle_url('/admin/tool/lp/templatecompetencies.php', [
View
@@ -72,6 +72,8 @@ function tool_lp_extend_navigation_user($navigation, $user, $usercontext, $cours
}
if (\tool_lp\user_evidence::can_read_user($user->id)) {
// FIXME: this should be in the above if statment, or fixed, don't rely on the
// first condition always being true.
$node->add(get_string('userevidence', 'tool_lp'),
new moodle_url('/admin/tool/lp/user_evidence_list.php', array('userid' => $user->id)));
}

0 comments on commit d846a53

Please sign in to comment.