MDL-53645 tool_lp: Various trivial bugs identified during review

This commit is contained in:
Dan Poltawski 2016-03-24 09:35:51 +08:00 committed by Frederic Massart
parent 4a4bda6d2d
commit d846a535a6
30 changed files with 26 additions and 50 deletions

View File

@ -90,8 +90,6 @@ class restore_tool_lp_plugin extends restore_tool_plugin {
* @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 @@ class restore_tool_lp_plugin extends restore_tool_plugin {
* @param array $data The data.
*/
public function process_course_module_competency($data) {
global $DB;
$data = (object) $data;
// Mapping the competency by ID numbers.

View File

@ -3895,7 +3895,6 @@ class api {
* @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();

View File

@ -180,7 +180,7 @@ class course_competency extends persistent {
$strname = 'complete';
break;
default:
throw new \moodle_exception('errorcoursecompetencyrule', 'tool_lp', '', $rule);
throw new \moodle_exception('errorcoursecompetencyrule', 'tool_lp', '', $ruleoutcome);
break;
}

View File

@ -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;

View File

@ -126,7 +126,7 @@ class course_module_competency extends persistent {
$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 @@ class course_module_competency extends persistent {
$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);

View File

@ -1568,7 +1568,7 @@ class external extends external_api {
$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 @@ class external extends external_api {
$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 @@ class external extends external_api {
* @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,

View File

@ -25,7 +25,6 @@ namespace tool_lp\external;
defined('MOODLE_INTERNAL') || die();
use renderer_base;
use moodle_url;
/**
@ -41,8 +40,6 @@ class course_module_summary_exporter extends exporter {
}
protected function get_other_values(renderer_base $output) {
global $CFG;
$context = $this->related['cm']->context;
return array(

View File

@ -122,6 +122,7 @@ abstract class exporter {
$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;

View File

@ -95,7 +95,6 @@ class user_competency_summary_exporter extends exporter {
));
$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']);

View File

@ -48,7 +48,6 @@ class user_summary_exporter extends exporter {
$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)) {

View File

@ -82,9 +82,7 @@ class competency extends persistent {
$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');
@ -92,7 +90,7 @@ class competency extends persistent {
$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'));

View File

@ -52,7 +52,6 @@ class user_evidence extends persistent {
$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));

View File

@ -103,8 +103,6 @@ class plan_page implements renderable, templatable {
}
$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()));

View File

@ -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 @@ class template_cohorts_table extends table_sql {
* 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.

View File

@ -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.

View File

@ -74,7 +74,7 @@ class user_competency_course_navigation implements renderable, templatable {
* @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 @@ class user_competency_course_navigation implements renderable, templatable {
$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);

View File

@ -64,8 +64,6 @@ class user_competency_summary implements renderable, templatable {
* @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());
}

View File

@ -73,7 +73,7 @@ class user_competency_summary_in_course implements renderable, templatable {
$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());

View File

@ -71,7 +71,7 @@ class user_competency_summary_in_plan implements renderable, templatable {
$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());

View File

@ -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 @@ class page_helper {
// 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 @@ class page_helper {
// 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);

View File

@ -194,7 +194,7 @@ class plan extends persistent {
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 @@ class plan extends persistent {
$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 @@ class plan extends persistent {
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(

View File

@ -104,7 +104,7 @@ class plan_competency extends persistent {
$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);

View File

@ -79,7 +79,6 @@ class template_cohort extends persistent {
* @return true|lang_string
*/
protected function validate_templateid($value) {
global $DB;
if (!template::record_exists($value)) {
return new lang_string('invaliddata', 'error');
}

View File

@ -180,7 +180,7 @@ class template_competency extends persistent {
$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 @@ class template_competency extends persistent {
*/
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 @@ class template_competency extends persistent {
*/
protected function validate_templateid($value) {
if (!template::record_exists($value)) {
return new lang_string('invaliddata', 'error');
return new \lang_string('invaliddata', 'error');
}
return true;
}

View File

@ -149,8 +149,6 @@ class user_competency_plan extends persistent {
* @return true|lang_string
*/
protected function validate_planid($value) {
global $DB;
if (!plan::record_exists($value) ) {
return new lang_string('invalidplan', 'tool_lp');
}

View File

@ -28,6 +28,7 @@ namespace tool_lp;
defined('MOODLE_INTERNAL') || die();
use stdClass;
use lang_string;
/**
* User evidence competency persistent class.

View File

@ -89,7 +89,6 @@ if ($form->is_cancelled()) {
// 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');

View File

@ -79,7 +79,6 @@ if ($form->is_cancelled()) {
$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()]);

View File

@ -68,7 +68,6 @@ if ($form->is_cancelled()) {
$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 File

@ -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)));
}