Skip to content

Commit cf32143

Browse files
committed
Merge branch 'hotfix'
2 parents c99978c + 250a0b6 commit cf32143

File tree

12 files changed

+301
-98
lines changed

12 files changed

+301
-98
lines changed

app/controllers/v1_controller.php

+107-16
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
class V1Controller extends Controller {
1212

1313
public $name = 'V1';
14-
public $uses = array('User', 'RolesUser',
14+
public $uses = array(
15+
'User', 'RolesUser',
1516
'Group', 'Course', 'Event', 'EvaluationSimple', 'EvaluationRubric',
1617
'EvaluationMixeval', 'OauthClient', 'OauthNonce', 'OauthToken',
1718
'GroupsMembers', 'GroupEvent', 'Department', 'Role', 'CourseDepartment',
@@ -250,7 +251,7 @@ public function beforeFilter() {
250251
if (!$this->RequestHandler->isGet()) {
251252
$this->body = trim(file_get_contents('php://input'), true);
252253
}
253-
$this->log('Got API request '. print_r($_REQUEST, true)."\nBody: ".$this->body, 'debug');
254+
$this->log('Got API request: '. $this->getRequestInfo($_REQUEST, $this->body), 'api');
254255

255256
// use oauth=0 paramter in url to bypass oauth checking
256257
if (!(Configure::read('debug') != 0 && isset($_REQUEST['oauth'])) &&
@@ -325,6 +326,8 @@ public function users($id = null) {
325326
if (isset($decode['username'])) {
326327
$role = array('Role' => array('RolesUser' => array('role_id' => $decode['role_id'])));
327328
unset($decode['role_id']);
329+
// do some clean up before we insert the values
330+
array_walk($decode, create_function('&$val', '$val = trim($val);'));
328331
$user = array('User' => $decode);
329332
$user = $user + $role;
330333

@@ -346,6 +349,8 @@ public function users($id = null) {
346349
foreach ($decode as $person) {
347350
$pRole = array('Role' => array('RolesUser' => array('role_id' => $person['role_id'])));
348351
unset($person['role_id']);
352+
// do some clean up before we insert the values
353+
array_walk($person, create_function('&$val', '$val = trim($val);'));
349354
$pUser = array('User' => $person);
350355
$data[] = $pUser + $pRole;
351356
}
@@ -357,14 +362,14 @@ public function users($id = null) {
357362
if ($ret) {
358363
$statusCode = 'HTTP/1.1 201 Created';
359364
$sUser[] = $decode[$key]['username'];
360-
$this->log('User created successful: '. $decode[$key]['username'], 'debug');
365+
$this->log('User created successful: '. $decode[$key]['username'], 'api');
361366
} else {
362367
$temp = array();
363368
$temp['username'] = $decode[$key]['username'];
364369
$temp['first_name'] = $decode[$key]['first_name'];
365370
$temp['last_name'] = $decode[$key]['last_name'];
366371
$uUser[] = $temp;
367-
$this->log('User created failed: '. $decode[$key]['username'], 'debug');
372+
$this->log('User created failed: '. $decode[$key]['username'], 'api');
368373
}
369374
}
370375
$sbody = $this->User->find('all', array(
@@ -625,20 +630,52 @@ public function groupMembers()
625630
// add the list of users to the given group
626631
$ret = $this->body;
627632
$users = json_decode($ret, true);
633+
$group = $this->Group->findById($groupId);
634+
$courseId = $group['Group']['course_id'];
635+
$students = $this->UserEnrol->find('list', array('conditions' => array('course_id' => $courseId), 'fields' => array('user_id')));
636+
$tutors = $this->UserTutor->find('list', array('conditions' => array('course_id' => $courseId), 'fields' => array('user_id')));
637+
$members = array_merge($students, $tutors);
638+
$inClass = $this->User->find('list', array('conditions' => array('User.id' => $members), 'fields' => array('User.username')));
639+
$userIds = array();
628640
$status = 'HTTP/1.1 200 OK';
629641
foreach ($users as $user) {
642+
if (!in_array($user['username'], $inClass)) {
643+
$this->log('User '.$user['username'].' is not in the course '.$courseId, 'debug');
644+
continue;
645+
}
630646
$userId = $this->User->field('id',
631647
array('username' => $user['username']));
648+
// skip the non-existing users
649+
if (!$userId) {
650+
continue;
651+
}
652+
$userIds[] = $userId;
632653
$tmp = array('group_id' => $groupId, 'user_id' => $userId);
633-
// try to add this user to group
634-
$this->GroupsMembers->create();
635-
if ($this->GroupsMembers->save($tmp)) {
636-
$userId = $this->GroupsMembers->read('user_id');
637-
$this->GroupsMembers->id = null;
654+
$inGroup = $this->GroupsMembers->field('id', $tmp);
655+
// user already in group
656+
if ($inGroup) {
638657
$groupMembers[] = $user;
658+
// tries to add the user to the group
659+
} else {
660+
// try to add this user to group
661+
$this->GroupsMembers->create();
662+
if ($this->GroupsMembers->save($tmp)) {
663+
$userId = $this->GroupsMembers->read('user_id');
664+
$this->GroupsMembers->id = null;
665+
$groupMembers[] = $user;
666+
} else {
667+
$status = 'HTTP/1.1 500 Internal Server Error';
668+
break;
669+
}
670+
}
671+
}
672+
$origMembers = Set::extract('/Member/id', $group);
673+
$toRemove = array_diff($origMembers, $userIds);
674+
if (!empty($toRemove)) {
675+
if ($this->GroupsMembers->deleteAll(array('user_id' => $toRemove, 'group_id' => $groupId))) {
676+
$this->log('Removed users '.implode(', ', $toRemove).' from group '.$groupId, 'debug');
639677
} else {
640-
$status = 'HTTP/1.1 500 Internal Server Error';
641-
break;
678+
$this->log('Failed to remove users '.implode(', ', $toRemove).' from group '.$groupId, 'debug');
642679
}
643680
}
644681
} else if ($this->RequestHandler->isDelete()) {
@@ -912,27 +949,36 @@ public function enrolment() {
912949
$students = $this->UserEnrol->find('list', array('conditions' => array('course_id' => $courseId), 'fields' => array('user_id')));
913950
$tutors = $this->UserTutor->find('list', array('conditions' => array('course_id' => $courseId), 'fields' => array('user_id')));
914951
$instructors = $this->UserCourse->find('list', array('conditions' => array('course_id' => $courseId), 'fields' => array('user_id')));
915-
$members = $students + $tutors + $instructors;
916-
$inClass = $this->User->find('list', array('conditions' => array('User.id' => $members), 'fields' => array('User.username')));
952+
$members = array_merge($students, $tutors, $instructors);
953+
$inClass = array();
954+
if (!empty($members)) {
955+
$inClass = $this->User->find('list', array('conditions' => array('User.id' => $members), 'fields' => array('User.username')));
956+
}
917957
$result = array();
918958

919959
foreach ($users as $user) {
960+
$this->log('processing user '.$user['username'].' to course '.$courseId, 'api');
920961
// check if the user is already in the course using case
921962
// insensitive username
922963
if (count(preg_grep("/^".$user['username']."$/i", $inClass)) == 0) {
923964
$userId = $this->User->field('id',
924965
array('username' => $user['username']));
966+
// skip users not in the system
967+
if (empty($userId)) {
968+
$this->log('Username '.$user['username'].' does not exist in the system.', 'debug');
969+
continue;
970+
}
925971
$role = $this->Role->getRoleName($user['role_id']);
926972
$table = null;
927973
if ($role == 'student') {
928974
$ret = $this->User->addStudent($userId, $courseId);
929-
$this->log('Adding student '.$user['username'].' to course '.$courseId, 'debug');
975+
$this->log('Adding student '.$user['username'].' to course '.$courseId, 'api');
930976
} else if ($role == 'instructor') {
931977
$ret = $this->User->addInstructor($userId, $courseId);
932-
$this->log('Adding instructor '.$user['username'].' to course '.$courseId, 'debug');
978+
$this->log('Adding instructor '.$user['username'].' to course '.$courseId, 'api');
933979
} else if ($role == 'tutor') {
934980
$ret = $this->User->addTutor($userId, $courseId);
935-
$this->log('Adding tutor '.$user['username'].' to course '.$courseId, 'debug');
981+
$this->log('Adding tutor '.$user['username'].' to course '.$courseId, 'api');
936982
} else {
937983
$this->set('error', array('code' => 400, 'message' => 'Unsupported role for '.$user['username']));
938984
$this->render('error');
@@ -951,6 +997,35 @@ public function enrolment() {
951997
$result[] = $user;
952998
}
953999
}
1000+
// unenrol students that are no longer in the class, this will become a problem if
1001+
// only a fraction of the class list is given because the rest of the class will
1002+
// be unenrolled. One example is the api being used to only enrol one user.
1003+
$users = Set::extract('/username', $users);
1004+
$unEnrol = array_diff($inClass, $users);
1005+
foreach ($unEnrol as $user) {
1006+
$userId = $this->User->field('id', array('username' => $user));
1007+
$role = $this->User->getRoleName($userId);
1008+
if ($role == 'student') {
1009+
$ret = $this->User->removeStudent($userId, $courseId);
1010+
$this->log('Removing student '.$user['username'].' from course '.$courseId, 'debug');
1011+
} else if ($role == 'instructor') {
1012+
$ret = $this->User->removeInstructor($userId, $courseId);
1013+
$this->log('Removing instructor '.$user['username'].' from course '.$courseId, 'debug');
1014+
} else if ($role == 'tutor') {
1015+
$ret = $this->User->removeTutor($userId, $courseId);
1016+
$this->log('Removing tutor '.$user['username'].' from course '.$courseId, 'debug');
1017+
} else {
1018+
$this->set('error', array('code' => 400, 'message' => 'Unsupported role for '.$user['username'].'. Could not unenrol.'));
1019+
$this->render('error');
1020+
return;
1021+
}
1022+
if (!$ret) {
1023+
$this->set('error', array('code' => 401, 'message' => 'Fail to unenrol ' . $user['username']));
1024+
$this->render('error');
1025+
return;
1026+
}
1027+
}
1028+
9541029
$this->set('result', $result);
9551030
} else if ($this->RequestHandler->isDelete()) {
9561031
$this->set('statusCode', 'HTTP/1.1 200 OK');
@@ -982,4 +1057,20 @@ public function enrolment() {
9821057
}
9831058
$this->render('json');
9841059
}
1060+
1061+
protected function getRequestInfo($request, $body)
1062+
{
1063+
$ret = '';
1064+
$ret .= $_SERVER['REQUEST_METHOD'] . ' ' . $request['url']. "\n";
1065+
$ret .= "Params: \n";
1066+
foreach ($request as $key => $value) {
1067+
if ($key == 'url') {
1068+
continue;
1069+
}
1070+
$ret .= " ".$key.": ".$value."\n";
1071+
}
1072+
$ret .= "Body: ".$body;
1073+
1074+
return $ret;
1075+
}
9851076
}

0 commit comments

Comments
 (0)