From fb5ce7d3517cb70ec892edc11f7f3f9aaa85b17e Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Tue, 16 Jul 2013 10:03:38 +0800 Subject: [PATCH] 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 682145960ee..2def3610e95 100644 --- a/cohort/externallib.php +++ b/cohort/externallib.php @@ -258,7 +258,7 @@ class core_cohort_external extends external_api { 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 @@ class core_cohort_external extends external_api { '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 d2ad95560bf..c8137fb28c1 100644 --- a/cohort/tests/externallib_test.php +++ b/cohort/tests/externallib_test.php @@ -203,6 +203,36 @@ class core_cohort_external_testcase extends externallib_advanced_testcase { 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 00000000000..d776e6f5cf4 --- /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 f7f08046533..2c4d3ac17a4 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -197,7 +197,7 @@ class core_enrol_external extends external_api { '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 6b606a49ea1..dc6d2c09f76 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 ===