From 415023cefc2b7c32e0eea6c6ebb38d6ca2e2b93c Mon Sep 17 00:00:00 2001
From: Adam Olley <adam.olley@netspot.com.au>
Date: Tue, 10 May 2016 14:15:35 +0930
Subject: [PATCH] MDL-54120 competency: Don't assume there are no existing
 scales

Every time a unit test for competencies adds a scale, it's creating them
with hardcoded id's - which should always be avoided. So this change has
the unit test keep track of the scale id's created without hardcoding
them.
---
 competency/tests/api_test.php      | 12 +++---
 competency/tests/external_test.php | 64 +++++++++++++++++++-----------
 2 files changed, 47 insertions(+), 29 deletions(-)

diff --git a/competency/tests/api_test.php b/competency/tests/api_test.php
index d0f86ebd7a33b..1d322f5adf93f 100644
--- a/competency/tests/api_test.php
+++ b/competency/tests/api_test.php
@@ -2686,17 +2686,17 @@ public function test_validate_grade_in_user_competency() {
         $lpg = $this->getDataGenerator()->get_plugin_generator('core_competency');
         $user = $dg->create_user();
 
-        $dg->create_scale(array("id" => "1", "scale" => "value1, value2"));
-        $dg->create_scale(array("id" => "2", "scale" => "value3, value4, value5, value6"));
+        $s1 = $dg->create_scale(array("scale" => "value1, value2"));
+        $s2 = $dg->create_scale(array("scale" => "value3, value4, value5, value6"));
 
-        $scaleconfiguration1 = '[{"scaleid":"1"},{"name":"value1","id":1,"scaledefault":1,"proficient":0},' .
+        $scaleconfiguration1 = '[{"scaleid":"'.$s1->id.'"},{"name":"value1","id":1,"scaledefault":1,"proficient":0},' .
                 '{"name":"value2","id":2,"scaledefault":0,"proficient":1}]';
-        $scaleconfiguration2 = '[{"scaleid":"2"},{"name":"value3","id":1,"scaledefault":1,"proficient":0},'
+        $scaleconfiguration2 = '[{"scaleid":"'.$s2->id.'"},{"name":"value3","id":1,"scaledefault":1,"proficient":0},'
                 . '{"name":"value4","id":2,"scaledefault":0,"proficient":1}]';
 
         // Create a framework with scale configuration1.
         $frm = array(
-            'scaleid' => 1,
+            'scaleid' => $s1->id,
             'scaleconfiguration' => $scaleconfiguration1
         );
         $framework = $lpg->create_framework($frm);
@@ -2704,7 +2704,7 @@ public function test_validate_grade_in_user_competency() {
 
         // Create competency with its own scale configuration.
         $c2 = $lpg->create_competency(array('competencyframeworkid' => $framework->get_id(),
-                                            'scaleid' => 2,
+                                            'scaleid' => $s2->id,
                                             'scaleconfiguration' => $scaleconfiguration2
                                         ));
 
diff --git a/competency/tests/external_test.php b/competency/tests/external_test.php
index f280e5f98fc22..6be6bf0775df3 100644
--- a/competency/tests/external_test.php
+++ b/competency/tests/external_test.php
@@ -69,6 +69,18 @@ class core_competency_external_testcase extends externallib_advanced_testcase {
     /** @var int User role id */
     protected $userrole = null;
 
+    /** @var stdClass $scale1 Scale */
+    protected $scale1 = null;
+
+    /** @var stdClass $scale2 Scale */
+    protected $scale2 = null;
+
+    /** @var stdClass $scale3 Scale */
+    protected $scale3 = null;
+
+    /** @var stdClass $scale4 Scale */
+    protected $scale4 = null;
+
     /** @var string scaleconfiguration */
     protected $scaleconfiguration1 = null;
 
@@ -153,31 +165,36 @@ protected function setUp() {
         $this->category = $category;
         $this->othercategory = $othercategory;
 
-        $this->getDataGenerator()->create_scale(array("id" => "1", "scale" => "value1, value2"));
-        $this->getDataGenerator()->create_scale(array("id" => "2", "scale" => "value3, value4"));
-        $this->getDataGenerator()->create_scale(array("id" => "3", "scale" => "value5, value6"));
-        $this->getDataGenerator()->create_scale(array("id" => "4", "scale" => "value7, value8"));
+        $this->scale1 = $this->getDataGenerator()->create_scale(array("scale" => "value1, value2"));
+        $this->scale2 = $this->getDataGenerator()->create_scale(array("scale" => "value3, value4"));
+        $this->scale3 = $this->getDataGenerator()->create_scale(array("scale" => "value5, value6"));
+        $this->scale4 = $this->getDataGenerator()->create_scale(array("scale" => "value7, value8"));
 
-        $this->scaleconfiguration1 = '[{"scaleid":"1"},{"name":"value1","id":1,"scaledefault":1,"proficient":0},' .
+        $this->scaleconfiguration1 = '[{"scaleid":"'.$this->scale1->id.'"},' .
+                '{"name":"value1","id":1,"scaledefault":1,"proficient":0},' .
                 '{"name":"value2","id":2,"scaledefault":0,"proficient":1}]';
-        $this->scaleconfiguration2 = '[{"scaleid":"2"},{"name":"value3","id":1,"scaledefault":1,"proficient":0},' .
+        $this->scaleconfiguration2 = '[{"scaleid":"'.$this->scale2->id.'"},' .
+                '{"name":"value3","id":1,"scaledefault":1,"proficient":0},' .
                 '{"name":"value4","id":2,"scaledefault":0,"proficient":1}]';
-        $this->scaleconfiguration3 = '[{"scaleid":"3"},{"name":"value5","id":1,"scaledefault":1,"proficient":0},' .
+        $this->scaleconfiguration3 = '[{"scaleid":"'.$this->scale3->id.'"},' .
+                '{"name":"value5","id":1,"scaledefault":1,"proficient":0},' .
                 '{"name":"value6","id":2,"scaledefault":0,"proficient":1}]';
-        $this->scaleconfiguration4 = '[{"scaleid":"4"},{"name":"value8","id":1,"scaledefault":1,"proficient":0},' .
+        $this->scaleconfiguration4 = '[{"scaleid":"'.$this->scale4->id.'"},'.
+                '{"name":"value8","id":1,"scaledefault":1,"proficient":0},' .
                 '{"name":"value8","id":2,"scaledefault":0,"proficient":1}]';
         accesslib_clear_all_caches_for_unit_testing();
     }
 
 
     protected function create_competency_framework($number = 1, $system = true) {
+        $scalename = 'scale' . $number;
         $scalepropname = 'scaleconfiguration' . $number;
         $framework = array(
             'shortname' => 'shortname' . $number,
             'idnumber' => 'idnumber' . $number,
             'description' => 'description' . $number,
             'descriptionformat' => FORMAT_HTML,
-            'scaleid' => $number,
+            'scaleid' => $this->$scalename->id,
             'scaleconfiguration' => $this->$scalepropname,
             'visible' => true,
             'contextid' => $system ? context_system::instance()->id : context_coursecat::instance($this->category->id)->id
@@ -241,6 +258,7 @@ protected function update_plan($planid, $number, $userid, $templateid, $status,
     }
 
     protected function update_competency_framework($id, $number = 1, $system = true) {
+        $scalename = 'scale' . $number;
         $scalepropname = 'scaleconfiguration' . $number;
         $framework = array(
             'id' => $id,
@@ -248,7 +266,7 @@ protected function update_competency_framework($id, $number = 1, $system = true)
             'idnumber' => 'idnumber' . $number,
             'description' => 'description' . $number,
             'descriptionformat' => FORMAT_HTML,
-            'scaleid' => $number,
+            'scaleid' => $this->$scalename->id,
             'scaleconfiguration' => $this->$scalepropname,
             'visible' => true,
             'contextid' => $system ? context_system::instance()->id : context_coursecat::instance($this->category->id)->id
@@ -316,7 +334,7 @@ public function test_create_competency_frameworks_with_manage_permissions() {
         $this->assertEquals('idnumber1', $result->idnumber);
         $this->assertEquals('description1', $result->description);
         $this->assertEquals(FORMAT_HTML, $result->descriptionformat);
-        $this->assertEquals(1, $result->scaleid);
+        $this->assertEquals($this->scale1->id, $result->scaleid);
         $this->assertEquals($this->scaleconfiguration1, $result->scaleconfiguration);
         $this->assertEquals(true, $result->visible);
     }
@@ -335,7 +353,7 @@ public function test_create_competency_frameworks_with_manage_permissions_in_cat
         $this->assertEquals('idnumber1', $result->idnumber);
         $this->assertEquals('description1', $result->description);
         $this->assertEquals(FORMAT_HTML, $result->descriptionformat);
-        $this->assertEquals(1, $result->scaleid);
+        $this->assertEquals($this->scale1->id, $result->scaleid);
         $this->assertEquals($this->scaleconfiguration1, $result->scaleconfiguration);
         $this->assertEquals(true, $result->visible);
 
@@ -358,7 +376,7 @@ public function test_create_competency_frameworks_with_nasty_data() {
             'idnumber' => 'id;"number',
             'description' => 'de<>\\..scription',
             'descriptionformat' => FORMAT_HTML,
-            'scaleid' => 1,
+            'scaleid' => $this->scale1->id,
             'scaleconfiguration' => $this->scaleconfiguration1,
             'visible' => true,
             'contextid' => context_system::instance()->id
@@ -384,7 +402,7 @@ public function test_read_competency_frameworks_with_manage_permissions() {
         $this->assertEquals('idnumber1', $result->idnumber);
         $this->assertEquals('description1', $result->description);
         $this->assertEquals(FORMAT_HTML, $result->descriptionformat);
-        $this->assertEquals(1, $result->scaleid);
+        $this->assertEquals($this->scale1->id, $result->scaleid);
         $this->assertEquals($this->scaleconfiguration1, $result->scaleconfiguration);
         $this->assertEquals(true, $result->visible);
     }
@@ -410,7 +428,7 @@ public function test_read_competency_frameworks_with_manage_permissions_in_categ
         $this->assertEquals('idnumber2', $result->idnumber);
         $this->assertEquals('description2', $result->description);
         $this->assertEquals(FORMAT_HTML, $result->descriptionformat);
-        $this->assertEquals(2, $result->scaleid);
+        $this->assertEquals($this->scale2->id, $result->scaleid);
         $this->assertEquals($this->scaleconfiguration2, $result->scaleconfiguration);
         $this->assertEquals(true, $result->visible);
 
@@ -444,7 +462,7 @@ public function test_read_competency_frameworks_with_read_permissions() {
         $this->assertEquals('idnumber1', $result->idnumber);
         $this->assertEquals('description1', $result->description);
         $this->assertEquals(FORMAT_HTML, $result->descriptionformat);
-        $this->assertEquals(1, $result->scaleid);
+        $this->assertEquals($this->scale1->id, $result->scaleid);
         $this->assertEquals($this->scaleconfiguration1, $result->scaleconfiguration);
         $this->assertEquals(true, $result->visible);
     }
@@ -470,7 +488,7 @@ public function test_read_competency_frameworks_with_read_permissions_in_categor
         $this->assertEquals('idnumber2', $result->idnumber);
         $this->assertEquals('description2', $result->description);
         $this->assertEquals(FORMAT_HTML, $result->descriptionformat);
-        $this->assertEquals(2, $result->scaleid);
+        $this->assertEquals($this->scale2->id, $result->scaleid);
         $this->assertEquals($this->scaleconfiguration2, $result->scaleconfiguration);
         $this->assertEquals(true, $result->visible);
 
@@ -579,12 +597,12 @@ public function test_update_framework_scale() {
 
         $s1 = $this->getDataGenerator()->create_scale();
 
-        $f1 = $lpg->create_framework(array('scaleid' => 1));
-        $f2 = $lpg->create_framework(array('scaleid' => 1));
+        $f1 = $lpg->create_framework(array('scaleid' => $s1->id));
+        $f2 = $lpg->create_framework(array('scaleid' => $s1->id));
         $c1 = $lpg->create_competency(array('competencyframeworkid' => $f1->get_id()));
         $c2 = $lpg->create_competency(array('competencyframeworkid' => $f2->get_id()));
 
-        $this->assertEquals(1, $f1->get_scaleid());
+        $this->assertEquals($s1->id, $f1->get_scaleid());
 
         // Make the scale of f2 being used.
         $lpg->create_user_competency(array('userid' => $this->user->id, 'competencyid' => $c2->get_id()));
@@ -593,7 +611,7 @@ public function test_update_framework_scale() {
         $result = $this->update_competency_framework($f1->get_id(), 3, true);
 
         $f1 = new \core_competency\competency_framework($f1->get_id());
-        $this->assertEquals(3, $f1->get_scaleid());
+        $this->assertEquals($this->scale3->id, $f1->get_scaleid());
 
         // Changing the framework where the scale is used.
         try {
@@ -645,7 +663,7 @@ public function test_list_and_count_competency_frameworks_with_manage_permission
         $this->assertEquals('idnumber1', $result->idnumber);
         $this->assertEquals('description1', $result->description);
         $this->assertEquals(FORMAT_HTML, $result->descriptionformat);
-        $this->assertEquals(1, $result->scaleid);
+        $this->assertEquals($this->scale1->id, $result->scaleid);
         $this->assertEquals($this->scaleconfiguration1, $result->scaleconfiguration);
         $this->assertEquals(true, $result->visible);
     }
@@ -732,7 +750,7 @@ public function test_list_and_count_competency_frameworks_with_read_permissions(
         $this->assertEquals('idnumber1', $result->idnumber);
         $this->assertEquals('description1', $result->description);
         $this->assertEquals(FORMAT_HTML, $result->descriptionformat);
-        $this->assertEquals(1, $result->scaleid);
+        $this->assertEquals($this->scale1->id, $result->scaleid);
         $this->assertEquals($this->scaleconfiguration1, $result->scaleconfiguration);
         $this->assertEquals(true, $result->visible);
     }