Skip to content

Commit

Permalink
MDL-32662 Fixed review problems
Browse files Browse the repository at this point in the history
  • Loading branch information
@jleyva
jleyva committed May 10, 2012
1 parent 7ce2359 commit 67aa60f
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 46 deletions.
83 changes: 57 additions & 26 deletions group/externallib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Expand All @@ -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');
}

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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));

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -918,17 +935,21 @@ 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;
}

/**
* Returns description of method parameters
*
* @return external_function_parameters
* @since Moodle 2.3
*/
public static function assign_grouping_parameters() {
return new external_function_parameters(
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -988,17 +1011,21 @@ 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;
}

/**
* Returns description of method parameters
*
* @return external_function_parameters
* @since Moodle 2.3
*/
public static function unassign_grouping_parameters() {
return new external_function_parameters(
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand Down
40 changes: 20 additions & 20 deletions lib/db/services.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ===
Expand Down

0 comments on commit 67aa60f

Please sign in to comment.