From b22a091b24805c3ea1b279044ab573d6ecfe961c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ducoulombier?= Date: Tue, 26 Nov 2019 09:20:07 +0100 Subject: [PATCH 1/7] Minor - Coding conventions --- main/webservices/api/tests/V2TestCase.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/main/webservices/api/tests/V2TestCase.php b/main/webservices/api/tests/V2TestCase.php index 07d1a87269..d0e94395f4 100644 --- a/main/webservices/api/tests/V2TestCase.php +++ b/main/webservices/api/tests/V2TestCase.php @@ -82,6 +82,7 @@ abstract class V2TestCase extends TestCase $this->assertSame(200, $response->getStatusCode()); $decodedResponse = json_decode($response->getBody()->getContents()); $this->assertNotNull($decodedResponse); + return $decodedResponse; } @@ -102,6 +103,7 @@ abstract class V2TestCase extends TestCase $this->assertTrue(property_exists($decodedResponse, 'message')); $message = $decodedResponse->message; $this->assertIsString($message); + return $message; } @@ -117,9 +119,11 @@ abstract class V2TestCase extends TestCase $this->assertIsObject($decodedResponse); $this->assertTrue( property_exists($decodedResponse, 'data'), - 'response data property is missing: '.print_r($decodedResponse, true)); + 'response data property is missing: '.print_r($decodedResponse, true) + ); $data = $decodedResponse->data; $this->assertIsArray($data); + return $data; } @@ -133,6 +137,7 @@ abstract class V2TestCase extends TestCase { $data = $this->dataArray($parameters); $this->assertSame(1, count($data)); + return $data[0]; } @@ -146,6 +151,7 @@ abstract class V2TestCase extends TestCase { $value = $this->singleElementValue($parameters); $this->assertIsInt($value); + return $value; } @@ -159,6 +165,7 @@ abstract class V2TestCase extends TestCase { $value = $this->singleElementValue($parameters); $this->assertIsBool($value); + return $value; } } From c9ba3f38bec10c6de415b04c7bbd45871c701289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ducoulombier?= Date: Tue, 26 Nov 2019 15:33:01 +0100 Subject: [PATCH 2/7] Minor - Coding conventions --- main/webservices/api/tests/TestCreateSessionFromModel.php | 2 ++ main/webservices/api/tests/TestGetSessionFromExtraField.php | 2 ++ main/webservices/api/tests/TestSaveUser.php | 2 ++ .../api/tests/TestSubscribeUserToSessionFromUsername.php | 2 ++ main/webservices/api/tests/TestUpdateUserFromUsername.php | 2 ++ 5 files changed, 10 insertions(+) diff --git a/main/webservices/api/tests/TestCreateSessionFromModel.php b/main/webservices/api/tests/TestCreateSessionFromModel.php index 138126ac80..fdd7877e32 100644 --- a/main/webservices/api/tests/TestCreateSessionFromModel.php +++ b/main/webservices/api/tests/TestCreateSessionFromModel.php @@ -1,4 +1,6 @@ Date: Tue, 26 Nov 2019 15:33:26 +0100 Subject: [PATCH 3/7] Minor - test case enriched --- main/webservices/api/tests/TestUpdateUserFromUsername.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/main/webservices/api/tests/TestUpdateUserFromUsername.php b/main/webservices/api/tests/TestUpdateUserFromUsername.php index 620fcb6e36..43139722e3 100644 --- a/main/webservices/api/tests/TestUpdateUserFromUsername.php +++ b/main/webservices/api/tests/TestUpdateUserFromUsername.php @@ -78,10 +78,10 @@ class TestUpdateUserFromUsername extends V2TestCase $userManager = UserManager::getManager(); $user = $userManager->find($userId); $userManager->reloadUser($user); -/* $this->assertSame($newFirstName, $user->newFirstname()); - $this->assertSame($newLastName, $user->newLastName()); - $this->assertSame($newStatus, $user->newStatus()); - $this->assertSame($newEmail, $user->newEmail());*/ + $this->assertSame($newFirstName, $user->getFirstname()); + $this->assertSame($newLastName, $user->getLastname()); + $this->assertSame($newStatus, $user->getStatus()); + $this->assertSame($newEmail, $user->getEmail()); // assert extra field values have been updated $extraFieldValueModel = new ExtraFieldValue('user'); From 34211fe64c3c242ef4181f37b99f79225f9abb14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ducoulombier?= Date: Tue, 26 Nov 2019 16:37:35 +0100 Subject: [PATCH 4/7] Minor - Rest new methods return values, not arrays --- main/inc/lib/webservices/Rest.php | 25 +++++++++++++------------ main/webservices/api/v2.php | 8 ++++---- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/main/inc/lib/webservices/Rest.php b/main/inc/lib/webservices/Rest.php index 7aab405bb2..8fd0e00415 100644 --- a/main/inc/lib/webservices/Rest.php +++ b/main/inc/lib/webservices/Rest.php @@ -1604,15 +1604,16 @@ class Rest extends WebService * @param $startDate * @param $endDate * @param array $extraFields - * @return array containing an int, the id of the new session + * @return int, the id of the new session * @throws Exception */ public function createSessionFromModel($modelSessionId, $sessionName, $startDate, $endDate, array $extraFields = []) { - if (empty($_POST['modelSessionId']) - || empty($_POST['sessionName']) - || empty($_POST['startDate']) - || empty($_POST['endDate'])) { + if (empty($modelSessionId) + || empty($sessionName) + || empty($startDate) + || empty($endDate) + ) { throw new Exception(get_lang('NoData')); } @@ -1694,7 +1695,7 @@ class Rest extends WebService UrlManager::add_session_to_url($newSessionId, 1); } - return [$newSessionId]; + return $newSessionId; } /** @@ -1702,7 +1703,7 @@ class Rest extends WebService * * @param int $sessionId the session id * @param string $loginName the user's login name - * @return array containing a boolean, whether it worked + * @return boolean, whether it worked * @throws Exception */ public function subscribeUserToSessionFromUsername($sessionId, $loginName) @@ -1725,7 +1726,7 @@ class Rest extends WebService throw new Exception(get_lang('UserNotSubscribed')); } - return [true]; + return true; } /** @@ -1733,7 +1734,7 @@ class Rest extends WebService * * @param $fieldName * @param $fieldValue - * @return array containing an int, the matching session id + * @return int, the matching session id * @throws Exception when no session matched or more than one session matched */ public function getSessionFromExtraField($fieldName, $fieldValue) @@ -1754,14 +1755,14 @@ class Rest extends WebService } // return sessionId - return [ intval($sessionIdList[0]['item_id']) ]; + return intval($sessionIdList[0]['item_id']); } /** * updates a user identified by its login name * * @param array $parameters - * @return array containing the boolean true on success + * @return boolean, true on success * @throws Exception on failure */ public function updateUserFromUserName($parameters) @@ -1949,6 +1950,6 @@ class Rest extends WebService } } - return [ true ]; + return true; } } diff --git a/main/webservices/api/v2.php b/main/webservices/api/v2.php index 63ee7447bc..b0825a30a4 100644 --- a/main/webservices/api/v2.php +++ b/main/webservices/api/v2.php @@ -298,25 +298,25 @@ try { $_POST['startDate'], $_POST['endDate'], isset($_POST['extraFields']) ? $_POST['extraFields'] : []); - $restResponse->setData($newSessionId); + $restResponse->setData([$newSessionId]); break; case Rest::SUBSCRIBE_USER_TO_SESSION_FROM_USERNAME: if (empty($_POST['sessionId']) || empty($_POST['loginName'])) { throw new Exception(get_lang('NoData')); } $subscribed = $restApi->subscribeUserToSessionFromUsername($_POST['sessionId'], $_POST['loginName']); - $restResponse->setData($subscribed); + $restResponse->setData([$subscribed]); break; case Rest::GET_SESSION_FROM_EXTRA_FIELD; if (empty($_POST['field_name']) || empty($_POST['field_value'])) { throw new Exception(get_lang('NoData')); } $idSession = $restApi->getSessionFromExtraField($_POST['field_name'], $_POST['field_value']); - $restResponse->setData($idSession); + $restResponse->setData([$idSession]); break; case Rest::UPDATE_USER_FROM_USERNAME: $data = $restApi->updateUserFromUserName($_POST); - $restResponse->setData($data); + $restResponse->setData([$data]); break; default: throw new Exception(get_lang('InvalidAction')); From bcb3580c6bf6ab94b5974afc1bbf012e80f3be96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ducoulombier?= Date: Tue, 26 Nov 2019 16:39:15 +0100 Subject: [PATCH 5/7] Minor - using session as object in test --- .../api/tests/TestCreateSessionFromModel.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/main/webservices/api/tests/TestCreateSessionFromModel.php b/main/webservices/api/tests/TestCreateSessionFromModel.php index fdd7877e32..be543b9549 100644 --- a/main/webservices/api/tests/TestCreateSessionFromModel.php +++ b/main/webservices/api/tests/TestCreateSessionFromModel.php @@ -47,14 +47,19 @@ class TestCreateSessionFromModel extends V2TestCase ); // assert the session was created and given the returned session id - $session = (new SessionManager)->fetch($newSessionId); - $this->assertIsArray($session); + $entityManager = Database::getManager(); + $repository = $entityManager->getRepository('ChamiloCoreBundle:Session'); + $newSession = $repository->find($newSessionId); + $this->assertIsObject($newSession); // assert the new session got the right data - $this->assertSame($name, $session['name']); - // FIXME time shift (one or two hours back) - // $this->assertSame($startDate, $session['display_start_date']); - // $this->assertSame($endDate, $session['display_end_date']); + $this->assertSame($name, $newSession->getName()); + // FIXME account for UTC / local timezone shift + // $this->assertSame($endDate, $newSession->getDisplayEndDate()); + // $this->assertSame($startDate, $newSession->getAccessStartDate()); + // $this->assertSame($endDate, $newSession->getAccessEndDate()); + // $this->assertSame($startDate, $newSession->getCoachAccessStartDate()); + // $this->assertSame($endDate, $newSession->getCoachAccessEndDate()); // clean up SessionManager::delete($modelSessionId); From 053d4deeaf31421d40aebbe823ef0c8af97dbe05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ducoulombier?= Date: Tue, 26 Nov 2019 17:11:46 +0100 Subject: [PATCH 6/7] README tells how to launch all WebService API tests --- ...eSessionFromModel.php => CreateSessionFromModelTest.php} | 6 +++--- ...nFromExtraField.php => GetSessionFromExtraFieldTest.php} | 6 +++--- main/webservices/api/tests/README | 2 ++ .../api/tests/{TestSaveUser.php => SaveUserTest.php} | 6 +++--- ...rname.php => SubscribeUserToSessionFromUsernameTest.php} | 6 +++--- ...eUserFromUsername.php => UpdateUserFromUsernameTest.php} | 6 +++--- 6 files changed, 17 insertions(+), 15 deletions(-) rename main/webservices/api/tests/{TestCreateSessionFromModel.php => CreateSessionFromModelTest.php} (98%) rename main/webservices/api/tests/{TestGetSessionFromExtraField.php => GetSessionFromExtraFieldTest.php} (96%) create mode 100644 main/webservices/api/tests/README rename main/webservices/api/tests/{TestSaveUser.php => SaveUserTest.php} (95%) rename main/webservices/api/tests/{TestSubscribeUserToSessionFromUsername.php => SubscribeUserToSessionFromUsernameTest.php} (92%) rename main/webservices/api/tests/{TestUpdateUserFromUsername.php => UpdateUserFromUsernameTest.php} (96%) diff --git a/main/webservices/api/tests/TestCreateSessionFromModel.php b/main/webservices/api/tests/CreateSessionFromModelTest.php similarity index 98% rename from main/webservices/api/tests/TestCreateSessionFromModel.php rename to main/webservices/api/tests/CreateSessionFromModelTest.php index be543b9549..078115d1fa 100644 --- a/main/webservices/api/tests/TestCreateSessionFromModel.php +++ b/main/webservices/api/tests/CreateSessionFromModelTest.php @@ -1,17 +1,17 @@ Date: Tue, 26 Nov 2019 23:40:25 +0100 Subject: [PATCH 7/7] Exercise: Fix CSV/XLS results export not including incomplete exercises - refs BT#16383 --- main/exercise/exercise_result.class.php | 43 ++++++++++++++----------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/main/exercise/exercise_result.class.php b/main/exercise/exercise_result.class.php index 7fd0266dda..ff51f28817 100755 --- a/main/exercise/exercise_result.class.php +++ b/main/exercise/exercise_result.class.php @@ -81,7 +81,8 @@ class ExerciseResult te.exe_duration as duration, te.orig_lp_id as orig_lp_id, tlm.name as lp_name, - user.username + user.username, + te.status as exstatus FROM $TBL_EXERCISES AS ce INNER JOIN $TBL_TRACK_EXERCISES AS te ON (te.exe_exo_id = ce.id) @@ -91,7 +92,6 @@ class ExerciseResult ON (tlm.id = te.orig_lp_id AND tlm.c_id = ce.c_id) WHERE ce.c_id = $course_id AND - te.status != 'incomplete' AND te.c_id = ce.c_id $user_id_and $session_id_and AND ce.active <>-1"; } else { @@ -112,7 +112,8 @@ class ExerciseResult ce.results_disabled as exdisabled, te.orig_lp_id as orig_lp_id, tlm.name as lp_name, - user.username + user.username, + te.status as exstatus FROM $TBL_EXERCISES AS ce INNER JOIN $TBL_TRACK_EXERCISES AS te ON (te.exe_exo_id = ce.id) @@ -122,7 +123,6 @@ class ExerciseResult ON (tlm.id = te.orig_lp_id AND tlm.c_id = ce.c_id) WHERE ce.c_id = $course_id AND - te.status != 'incomplete' AND te.c_id = ce.c_id $user_id_and $session_id_and AND ce.active <>-1 AND ORDER BY userpart2, te.c_id ASC, ce.title ASC, te.exe_date DESC"; @@ -180,25 +180,29 @@ class ExerciseResult if (is_array($results)) { $i = 0; foreach ($results as $result) { - $revised = false; - //revised or not - $sql_exe = "SELECT exe_id - FROM $TBL_TRACK_ATTEMPT_RECORDING - WHERE - author != '' AND - exe_id = ".Database::escape_string($result['exid'])." - LIMIT 1"; - $query = Database::query($sql_exe); - - if (Database:: num_rows($query) > 0) { - $revised = true; + $revised = 0; + if ($result['exstatus'] === 'incomplete') { + $revised = -1; + } else { + //revised or not + $sql_exe = "SELECT exe_id + FROM $TBL_TRACK_ATTEMPT_RECORDING + WHERE + author != '' AND + exe_id = ".intval($result['exid'])." + LIMIT 1"; + $query = Database::query($sql_exe); + + if (Database:: num_rows($query) > 0) { + $revised = 1; + } } - if ($filter_by_not_revised && $revised) { + if ($filter_by_not_revised && $revised === 1) { continue; } - if ($filter_by_revised && !$revised) { + if ($filter_by_revised && $revised < 1) { continue; } @@ -222,7 +226,8 @@ class ExerciseResult $return[$i]['duration'] = $result['duration']; $return[$i]['result'] = $result['exresult']; $return[$i]['max'] = $result['exweight']; - $return[$i]['status'] = $revised ? get_lang('Validated') : get_lang('NotValidated'); + // Revised: 1 = revised, 0 = not revised, -1 = not even finished by user + $return[$i]['status'] = $revised === 1 ? get_lang('Validated') : ($revised === 0 ? get_lang('NotValidated') : get_lang('Unclosed') ); $return[$i]['lp_id'] = $result['orig_lp_id']; $return[$i]['lp_name'] = $result['lp_name'];