From 67aa60f9b71bc9725a7e469b4e59087d6f8e907f Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Thu, 10 May 2012 11:41:46 +0200 Subject: [PATCH] MDL-32662 Fixed review problems --- group/externallib.php | 83 +++++++++++++++++++++++++++++-------------- lib/db/services.php | 40 ++++++++++----------- 2 files changed, 77 insertions(+), 46 deletions(-) diff --git a/group/externallib.php b/group/externallib.php index eb3efdb5b1770..0ff3b5157dc7e 100644 --- a/group/externallib.php +++ b/group/externallib.php @@ -552,7 +552,9 @@ public static function delete_group_members_returns() { /** * Returns description of method parameters + * * @return external_function_parameters + * @since Moodle 2.3 */ public static function create_groupings_parameters() { return new external_function_parameters( @@ -572,8 +574,10 @@ public static function create_groupings_parameters() { /** * Create groupings + * * @param array $groupings array of grouping description arrays (with keys groupname and courseid) * @return array of newly created groupings + * @since Moodle 2.3 */ public static function create_groupings($groupings) { global $CFG, $DB; @@ -591,7 +595,7 @@ public static function create_groupings($groupings) { if (trim($grouping->name) == '') { throw new invalid_parameter_exception('Invalid grouping name'); } - if ($DB->get_record('groupings', array('courseid'=>$grouping->courseid, 'name'=>$grouping->name))) { + if ($DB->count_records('groupings', array('courseid'=>$grouping->courseid, 'name'=>$grouping->name))) { throw new invalid_parameter_exception('Grouping with the same name already exists in the course'); } @@ -622,7 +626,9 @@ public static function create_groupings($groupings) { /** * Returns description of method result value + * * @return external_description + * @since Moodle 2.3 */ public static function create_groupings_returns() { return new external_multiple_structure( @@ -639,7 +645,9 @@ public static function create_groupings_returns() { /** * Returns description of method parameters + * * @return external_function_parameters + * @since Moodle 2.3 */ public static function update_groupings_parameters() { return new external_function_parameters( @@ -659,8 +667,10 @@ public static function update_groupings_parameters() { /** * Update groupings + * * @param array $groupings array of grouping description arrays (with keys groupname and courseid) * @return array of newly updated groupings + * @since Moodle 2.3 */ public static function update_groupings($groupings) { global $CFG, $DB; @@ -670,8 +680,6 @@ public static function update_groupings($groupings) { $transaction = $DB->start_delegated_transaction(); - $groupings = array(); - foreach ($params['groupings'] as $grouping) { $grouping = (object)$grouping; @@ -701,52 +709,49 @@ public static function update_groupings($groupings) { // Finally update the grouping. groups_update_grouping($grouping); - $groupings[] = (array)$grouping; } $transaction->allow_commit(); - return $groupings; + return null; } - /** + /** * Returns description of method result value + * * @return external_description + * @since Moodle 2.3 */ public static function update_groupings_returns() { - return new external_multiple_structure( - new external_single_structure( - array( - 'id' => new external_value(PARAM_INT, 'grouping record id'), - 'courseid' => new external_value(PARAM_INT, 'id of course'), - 'name' => new external_value(PARAM_TEXT, 'multilang compatible name, course unique'), - 'description' => new external_value(PARAM_CLEANHTML, 'grouping description text') - ) - ), 'List of grouping object. A grouping has an id, a courseid, a name and a description.' - ); + return null; } /** * Returns description of method parameters + * * @return external_function_parameters + * @since Moodle 2.3 */ public static function get_groupings_parameters() { return new external_function_parameters( array( 'groupingids' => new external_multiple_structure(new external_value(PARAM_INT, 'grouping ID') - ,'List of grouping id. A grouping id is an integer.'), + , 'List of grouping id. A grouping id is an integer.'), ) ); } /** * Get groupings definition specified by ids + * * @param array $groupingids arrays of grouping ids * @return array of grouping objects (id, courseid, name) + * @since Moodle 2.3 */ public static function get_groupings($groupingids) { global $CFG; require_once("$CFG->dirroot/group/lib.php"); + require_once("$CFG->libdir/filelib.php"); $params = self::validate_parameters(self::get_groupings_parameters(), array('groupingids'=>$groupingids)); @@ -780,9 +785,11 @@ public static function get_groupings($groupingids) { return $groupings; } - /** + /** * Returns description of method result value + * * @return external_description + * @since Moodle 2.3 */ public static function get_groupings_returns() { return new external_multiple_structure( @@ -799,7 +806,9 @@ public static function get_groupings_returns() { /** * Returns description of method parameters + * * @return external_function_parameters + * @since Moodle 2.3 */ public static function get_course_groupings_parameters() { return new external_function_parameters( @@ -811,12 +820,15 @@ public static function get_course_groupings_parameters() { /** * Get all groupings in the specified course + * * @param int $courseid id of course * @return array of grouping objects (id, courseid, name, enrolmentkey) + * @since Moodle 2.3 */ public static function get_course_groupings($courseid) { global $CFG; require_once("$CFG->dirroot/group/lib.php"); + require_once("$CFG->libdir/filelib.php"); $params = self::validate_parameters(self::get_course_groupings_parameters(), array('courseid'=>$courseid)); @@ -850,9 +862,11 @@ public static function get_course_groupings($courseid) { return $groupings; } - /** + /** * Returns description of method result value + * * @return external_description + * @since Moodle 2.3 */ public static function get_course_groupings_returns() { return new external_multiple_structure( @@ -869,7 +883,9 @@ public static function get_course_groupings_returns() { /** * Returns description of method parameters + * * @return external_function_parameters + * @since Moodle 2.3 */ public static function delete_groupings_parameters() { return new external_function_parameters( @@ -881,8 +897,10 @@ public static function delete_groupings_parameters() { /** * Delete groupings + * * @param array $groupingids array of grouping ids * @return void + * @since Moodle 2.3 */ public static function delete_groupings($groupingids) { global $CFG, $DB; @@ -893,8 +911,7 @@ public static function delete_groupings($groupingids) { $transaction = $DB->start_delegated_transaction(); foreach ($params['groupingids'] as $groupingid) { - // Validate params. - $groupingid = validate_param($groupingid, PARAM_INTEGER); + if (!$grouping = groups_get_grouping($groupingid, 'id, courseid', IGNORE_MISSING)) { // Silently ignore attempts to delete nonexisting groupings. continue; @@ -918,9 +935,11 @@ public static function delete_groupings($groupingids) { $transaction->allow_commit(); } - /** + /** * Returns description of method result value + * * @return external_description + * @since Moodle 2.3 */ public static function delete_groupings_returns() { return null; @@ -928,7 +947,9 @@ public static function delete_groupings_returns() { /** * Returns description of method parameters + * * @return external_function_parameters + * @since Moodle 2.3 */ public static function assign_grouping_parameters() { return new external_function_parameters( @@ -947,8 +968,10 @@ public static function assign_grouping_parameters() { /** * Assign a group to a grouping + * * @param array $assignments of arrays with keys groupid, groupingid * @return void + * @since Moodle 2.3 */ public static function assign_grouping($assignments) { global $CFG, $DB; @@ -970,7 +993,7 @@ public static function assign_grouping($assignments) { continue; } - // now security checks + // Now security checks. $context = context_course::instance($grouping->courseid); try { self::validate_context($context); @@ -988,9 +1011,11 @@ public static function assign_grouping($assignments) { $transaction->allow_commit(); } - /** + /** * Returns description of method result value + * * @return null + * @since Moodle 2.3 */ public static function assign_grouping_returns() { return null; @@ -998,7 +1023,9 @@ public static function assign_grouping_returns() { /** * Returns description of method parameters + * * @return external_function_parameters + * @since Moodle 2.3 */ public static function unassign_grouping_parameters() { return new external_function_parameters( @@ -1017,8 +1044,10 @@ public static function unassign_grouping_parameters() { /** * Unassign a group from a grouping + * * @param array $unassignments of arrays with keys groupid, groupingid * @return void + * @since Moodle 2.3 */ public static function unassign_grouping($unassignments) { global $CFG, $DB; @@ -1040,7 +1069,7 @@ public static function unassign_grouping($unassignments) { continue; } - // now security checks + // Now security checks. $context = context_course::instance($grouping->courseid); try { self::validate_context($context); @@ -1058,9 +1087,11 @@ public static function unassign_grouping($unassignments) { $transaction->allow_commit(); } - /** + /** * Returns description of method result value + * * @return null + * @since Moodle 2.3 */ public static function unassign_grouping_returns() { return null; diff --git a/lib/db/services.php b/lib/db/services.php index e943e1d7284d3..4a4559f7b74c3 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -188,35 +188,35 @@ ), 'core_group_get_course_groupings' => array( - 'classname' => 'core_group_external', - 'methodname' => 'get_course_groupings', - 'classpath' => 'group/externallib.php', - 'description' => 'Returns all groupings in specified course.', - 'type' => 'read', + 'classname' => 'core_group_external', + 'methodname' => 'get_course_groupings', + 'classpath' => 'group/externallib.php', + 'description' => 'Returns all groupings in specified course.', + 'type' => 'read', ), 'core_group_delete_groupings' => array( - 'classname' => 'core_group_external', - 'methodname' => 'delete_groupings', - 'classpath' => 'group/externallib.php', - 'description' => 'Deletes all specified groupings.', - 'type' => 'write', + 'classname' => 'core_group_external', + 'methodname' => 'delete_groupings', + 'classpath' => 'group/externallib.php', + 'description' => 'Deletes all specified groupings.', + 'type' => 'write', ), 'core_group_assign_grouping' => array( - 'classname' => 'core_group_external', - 'methodname' => 'assign_grouping', - 'classpath' => 'group/externallib.php', - 'description' => 'Assing groups from groupings', - 'type' => 'write', + 'classname' => 'core_group_external', + 'methodname' => 'assign_grouping', + 'classpath' => 'group/externallib.php', + 'description' => 'Assing groups from groupings', + 'type' => 'write', ), 'core_group_unassign_grouping' => array( - 'classname' => 'core_group_external', - 'methodname' => 'unassign_grouping', - 'classpath' => 'group/externallib.php', - 'description' => 'Unassing groups from groupings', - 'type' => 'write', + 'classname' => 'core_group_external', + 'methodname' => 'unassign_grouping', + 'classpath' => 'group/externallib.php', + 'description' => 'Unassing groups from groupings', + 'type' => 'write', ), // === file related functions ===