From 4f60c4af77dcdedf36977588616158900d63d255 Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Mon, 15 Jul 2013 16:10:15 +0800 Subject: [PATCH 1/2] MDL-40666 cohort: remove duplicated code from tests --- cohort/tests/externallib_test.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cohort/tests/externallib_test.php b/cohort/tests/externallib_test.php index 39c2ef7191675..d2ad95560bf23 100644 --- a/cohort/tests/externallib_test.php +++ b/cohort/tests/externallib_test.php @@ -147,10 +147,6 @@ public function test_get_cohorts() { // Check we retrieve the good total number of enrolled cohorts + no error on capability. $this->assertEquals(2, count($returnedcohorts)); - // Call the external function. - $returnedcohorts = core_cohort_external::get_cohorts(array( - $cohort1->id, $cohort2->id)); - foreach ($returnedcohorts as $enrolledcohort) { if ($enrolledcohort['idnumber'] == $cohort1->idnumber) { $this->assertEquals($cohort1->name, $enrolledcohort['name']); From fb5ce7d3517cb70ec892edc11f7f3f9aaa85b17e Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Tue, 16 Jul 2013 10:03:38 +0800 Subject: [PATCH 2/2] MDL-40666 enrol|cohort: PARAM_NUMBER -> PARAM_INT PARAM_NUMBER is a float, our id's are int's * Fix core_cohort_get_cohorts to specify correct return type on cohort id * Fix core_enrol_get_enrolled_users_with_capability to specify correct correct return type on user id * Fix core_cohort_update_cohorts to specify the right parameter type (int) on cohort id * Add tests to verify the bug on core_cohort_update_cohorts param type. --- cohort/externallib.php | 4 ++-- cohort/tests/externallib_test.php | 30 ++++++++++++++++++++++++++++++ cohort/upgrade.txt | 9 +++++++++ enrol/externallib.php | 2 +- enrol/upgrade.txt | 3 +++ 5 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 cohort/upgrade.txt diff --git a/cohort/externallib.php b/cohort/externallib.php index 682145960eed0..2def3610e95c9 100644 --- a/cohort/externallib.php +++ b/cohort/externallib.php @@ -258,7 +258,7 @@ public static function get_cohorts_returns() { return new external_multiple_structure( new external_single_structure( array( - 'id' => new external_value(PARAM_NUMBER, 'ID of the cohort'), + 'id' => new external_value(PARAM_INT, 'ID of the cohort'), 'name' => new external_value(PARAM_RAW, 'cohort name'), 'idnumber' => new external_value(PARAM_RAW, 'cohort idnumber'), 'description' => new external_value(PARAM_RAW, 'cohort description'), @@ -280,7 +280,7 @@ public static function update_cohorts_parameters() { 'cohorts' => new external_multiple_structure( new external_single_structure( array( - 'id' => new external_value(PARAM_NUMBER, 'ID of the cohort'), + 'id' => new external_value(PARAM_INT, 'ID of the cohort'), 'categorytype' => new external_single_structure( array( 'type' => new external_value(PARAM_TEXT, 'the name of the field: id (numeric value diff --git a/cohort/tests/externallib_test.php b/cohort/tests/externallib_test.php index d2ad95560bf23..c8137fb28c1a8 100644 --- a/cohort/tests/externallib_test.php +++ b/cohort/tests/externallib_test.php @@ -203,6 +203,36 @@ public function test_update_cohorts() { core_cohort_external::update_cohorts(array($cohort1)); } + /** + * Verify handling of 'id' param. + */ + public function test_update_cohorts_invalid_id_param() { + $this->resetAfterTest(true); + $cohort = self::getDataGenerator()->create_cohort(); + + $cohort1 = array( + 'id' => 'THIS IS NOT AN ID', + 'name' => 'Changed cohort name', + 'categorytype' => array('type' => 'id', 'value' => '1'), + 'idnumber' => $cohort->idnumber, + ); + + try { + core_cohort_external::update_cohorts(array($cohort1)); + $this->fail('Expecting invalid_parameter_exception exception, none occured'); + } catch (invalid_parameter_exception $e1) { + $this->assertContains('Invalid external api parameter: the value is "THIS IS NOT AN ID"', $e1->debuginfo); + } + + $cohort1['id'] = 9.999; // Also not a valid id of a cohort. + try { + core_cohort_external::update_cohorts(array($cohort1)); + $this->fail('Expecting invalid_parameter_exception exception, none occured'); + } catch (invalid_parameter_exception $e2) { + $this->assertContains('Invalid external api parameter: the value is "9.999"', $e2->debuginfo); + } + } + /** * Test update_cohorts without permission on the dest category. */ diff --git a/cohort/upgrade.txt b/cohort/upgrade.txt new file mode 100644 index 0000000000000..d776e6f5cf4a7 --- /dev/null +++ b/cohort/upgrade.txt @@ -0,0 +1,9 @@ +This files describes API changes in /cohort/ information provided here is intended +especially for developers. + +=== 2.6 === +* Webservice core_cohort_update_cohorts was incorrectly specifiying float as the parameter type + for cohort id. This field is actually int and input is now reported and processed as such. +* Webservice core_cohort_get_cohorts was incorrectly specifiying float as the return + type for cohort id. The actual return type is int and is now reported as such. + diff --git a/enrol/externallib.php b/enrol/externallib.php index f7f08046533ac..2c4d3ac17a4d4 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -197,7 +197,7 @@ public static function get_enrolled_users_with_capability_returns() { 'users' => new external_multiple_structure( new external_single_structure( array( - 'id' => new external_value(PARAM_NUMBER, 'ID of the user'), + 'id' => new external_value(PARAM_INT, 'ID of the user'), 'username' => new external_value(PARAM_RAW, 'Username', VALUE_OPTIONAL), 'firstname' => new external_value(PARAM_NOTAGS, 'The first name(s) of the user', VALUE_OPTIONAL), 'lastname' => new external_value(PARAM_NOTAGS, 'The family name of the user', VALUE_OPTIONAL), diff --git a/enrol/upgrade.txt b/enrol/upgrade.txt index 6b606a49ea103..dc6d2c09f765c 100644 --- a/enrol/upgrade.txt +++ b/enrol/upgrade.txt @@ -6,6 +6,9 @@ information provided here is intended especially for developers. * Enrolment plugin which supports self enrolment should implement can_self_enrol() * Enrolment plugin should implement get_enrol_info() to expose instance information with webservice or external interface. +* Webservice core_enrol_get_enrolled_users_with_capability was incorrectly specifing + float as the return type for user id. int is the actual returned type and is now + reported as such. === 2.5 ===