MDL-53645 tool_lp: Address FIXME comments

diff --git a/admin/tool/lp/classes/external.php b/admin/tool/lp/classes/external.php
index 230a009..1335fc9 100644 (file)
--- a/admin/tool/lp/classes/external.php
+++ b/admin/tool/lp/classes/external.php
@@ -1627,11 +1627,10 @@ class external extends external_api {
     public static function list_course_modules_using_competency($competencyid, $courseid) {
         global $PAGE;
 
-        $params = self::validate_parameters(self::list_course_modules_using_competency_parameters(),
-                                            array(
-                                                'competencyid' => $competencyid,
-                                                'courseid' => $courseid,
-                                            ));
+        $params = self::validate_parameters(self::list_course_modules_using_competency_parameters(), array(
+            'competencyid' => $competencyid,
+            'courseid' => $courseid,
+        ));
 
         $coursecontext = context_course::instance($params['courseid']);
         self::validate_context($coursecontext);
@@ -1641,8 +1640,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);
+        $fastmodinfo = get_fast_modinfo($params['courseid']);
 
         foreach ($coursemodules as $cmid) {
             $cminfo = $fastmodinfo->cms[$cmid];

diff --git a/admin/tool/lp/classes/external/exporter.php b/admin/tool/lp/classes/external/exporter.php
index 68ff981..0a71da6 100644 (file)
--- a/admin/tool/lp/classes/external/exporter.php
+++ b/admin/tool/lp/classes/external/exporter.php
@@ -122,8 +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);
+            throw new coding_exception('Cannot override a standard property value.');
         }
         $values += $othervalues;
         $record = (object) $values;

diff --git a/admin/tool/lp/classes/output/template_cohorts_table.php b/admin/tool/lp/classes/output/template_cohorts_table.php
index f2a044a..45a7f47 100644 (file)
--- a/admin/tool/lp/classes/output/template_cohorts_table.php
+++ b/admin/tool/lp/classes/output/template_cohorts_table.php
@@ -98,9 +98,6 @@ 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.
         $cols = array(
             'name' => get_string('name', 'cohort'),

diff --git a/admin/tool/lp/classes/plan.php b/admin/tool/lp/classes/plan.php
index 92320ac..d617406 100644 (file)
--- a/admin/tool/lp/classes/plan.php
+++ b/admin/tool/lp/classes/plan.php
@@ -222,8 +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);
+            $competencies = template_competency::list_competencies($this->get_templateid());
         } else {
             // Get the competencies from the plan.
             $competencies = plan_competency::list_competencies($this->get_id());

diff --git a/admin/tool/lp/lib.php b/admin/tool/lp/lib.php
index abf3bab..0fcde71 100644 (file)
--- a/admin/tool/lp/lib.php
+++ b/admin/tool/lp/lib.php
@@ -69,14 +69,13 @@ function tool_lp_extend_navigation_user($navigation, $user, $usercontext, $cours
     if (\tool_lp\plan::can_read_user($user->id)) {
         $node = $navigation->add(get_string('learningplans', 'tool_lp'),
             new moodle_url('/admin/tool/lp/plans.php', array('userid' => $user->id)));
-    }
 
-    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)));
+        if (\tool_lp\user_evidence::can_read_user($user->id)) {
+            $node->add(get_string('userevidence', 'tool_lp'),
+                new moodle_url('/admin/tool/lp/user_evidence_list.php', array('userid' => $user->id)));
+        }
     }
+
 }
 
 /**

diff --git a/admin/tool/lp/tests/externallib_test.php b/admin/tool/lp/tests/externallib_test.php
index c220a0e..c10fc54 100644 (file)
--- a/admin/tool/lp/tests/externallib_test.php
+++ b/admin/tool/lp/tests/externallib_test.php
@@ -3083,7 +3083,7 @@ class tool_lp_external_testcase extends externallib_advanced_testcase {
         $settings = course_competency_settings::get_by_courseid($course->id);
 
         $this->assertTrue((bool)$settings->get_pushratingstouserplans());
-        
+
         $result = external::update_course_competency_settings($course->id, array('pushratingstouserplans' => false));
 
         $settings = course_competency_settings::get_by_courseid($course->id);
@@ -3094,4 +3094,24 @@ class tool_lp_external_testcase extends externallib_advanced_testcase {
         $this->setExpectedException('required_capability_exception');
         $result = external::update_course_competency_settings($course->id, array('pushratingstouserplans' => true));
     }
+
+    public function test_list_course_modules_using_competency() {
+        $this->setAdminUser();
+
+        $dg = $this->getDataGenerator();
+        $lpg = $dg->get_plugin_generator('tool_lp');
+
+        $course = $dg->create_course();
+        $cm1 = $dg->create_module('assign', array('course' => $course->id));
+
+        $f1 = $lpg->create_framework();
+        $c1 = $lpg->create_competency(array('competencyframeworkid' => $f1->get_id()));
+        $lpg->create_course_competency(array('courseid' => $course->id, 'competencyid' => $c1->get_id()));
+        $lpg->create_course_module_competency(array('cmid' => $cm1->cmid, 'competencyid' => $c1->get_id()));
+
+        $result = external::list_course_modules_using_competency($c1->get_id(), $course->id);
+        $result = external_api::clean_returnvalue(external::list_course_modules_using_competency_returns(), $result);
+        $this->assertCount(1, $result);
+        $this->assertEquals($cm1->cmid, $result[0]['id']);
+    }
 }