From 6fefe626831c0f8bdd4c8e78c2b820e27a7f569c Mon Sep 17 00:00:00 2001 From: Laetitia Fesselier <laetitia.fesselier@mail.mcgill.ca> Date: Tue, 24 Aug 2021 16:16:13 -0400 Subject: [PATCH] Addressing QA comments --- .../endpoints/candidate/visit/visit.class.inc | 107 ++-- .../candidate/visit/visit_0_0_4_dev.class.inc | 472 ++++++++++++++++-- 2 files changed, 454 insertions(+), 125 deletions(-) diff --git a/modules/api/php/endpoints/candidate/visit/visit.class.inc b/modules/api/php/endpoints/candidate/visit/visit.class.inc index 84deef74bfb..128a0ca1ba0 100644 --- a/modules/api/php/endpoints/candidate/visit/visit.class.inc +++ b/modules/api/php/endpoints/candidate/visit/visit.class.inc @@ -33,28 +33,14 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator * * @var \Candidate */ - protected $_candidate; + private $_candidate; /** * The requested Visit * * @var ?\Timepoint */ - protected $_visit; - - /** - * The visit centerID - * - * @var ?\CenterID - */ - protected $_centerID; - - /** - * The visit Project - * - * @var ?\Project - */ - protected $_project; + private $_visit; /** * Contructor @@ -90,7 +76,7 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator protected function supportedVersions() : array { return [ - 'v0.0.3' + 'v0.0.3', ]; } @@ -108,19 +94,23 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator if (!$this->_visit->isAccessibleBy($user)) { return new \LORIS\Http\Response\JSON\Forbidden('Permission denied'); } + } $pathparts = $request->getAttribute('pathparts'); if (count($pathparts) === 0) { - $method = $request->getMethod(); + switch ($request->getMethod()) { + case 'GET': + return $this->_handleGET(); - if (in_array($method, $this->allowedMethods())) { - $handle = 'handle'.$method; - return $this->$handle($request); - } else if ($method === 'OPTIONS') { + case 'PUT': + return $this->_handlePUT($request); + + case 'OPTIONS': return (new \LORIS\Http\Response()) ->withHeader('Allow', $this->allowedMethods()); - } else { + + default: return new \LORIS\Http\Response\JSON\MethodNotAllowed( $this->allowedMethods() ); @@ -168,7 +158,7 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator * * @return ResponseInterface The outgoing PSR7 response */ - protected function handleGET(): ResponseInterface + private function _handleGET(): ResponseInterface { if ($this->_visit === null) { return new \LORIS\Http\Response\JSON\NotFound('Visit not found'); @@ -181,7 +171,7 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator } /** - * Validate a PUT request + * Handles a PUT request that creates or replace a candidate visit * * TODO: There is no way to validate the the visit_label in the url * fits the json data of the request because it is removed from the @@ -191,13 +181,13 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator * * @param ServerRequestInterface $request The incoming PSR7 request * - * @return ?ResponseInterface The outgoing PSR7 response + * @return ResponseInterface The outgoing PSR7 response */ - protected function validatePUT( - ServerRequestInterface $request - ): ?ResponseInterface { - $user = $request->getAttribute('user'); - $data = json_decode((string) $request->getBody(), true) ?? []; + private function _handlePUT(ServerRequestInterface $request): ResponseInterface + { + $user = $request->getAttribute('user'); + $data = json_decode((string) $request->getBody(), true); + $visitinfo = $data ?? []; $requiredfields = [ 'CandID', @@ -206,7 +196,7 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator 'Battery', 'Project', ]; - $diff = array_diff($requiredfields, array_keys($data)); + $diff = array_diff($requiredfields, array_keys($visitinfo)); if (!empty($diff)) { return new \LORIS\Http\Response\JSON\BadRequest( @@ -215,61 +205,39 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator } try { - $this->_project = \NDB_Factory::singleton()->project($data['Project']); + $project = \NDB_Factory::singleton()->project($visitinfo['Project']); } catch (\NotFound $e) { return new \LORIS\Http\Response\JSON\BadRequest( $e->getMessage() ); } - if (strval($data['CandID']) !== strval($this->_candidate->getCandID())) { + if ($visitinfo['CandID'] !== $this->_candidate->getCandID()) { return new \LORIS\Http\Response\JSON\BadRequest( - 'The CandID field does not match this candidate' + 'CandID does not match this candidate' ); } if (!$this->_candidate->isAccessibleBy($user)) { return new \LORIS\Http\Response\JSON\Forbidden( - "You can't modify that candidate" + "You can't create or modify that candidate" ); } - $centerID = array_search($data['Site'], \Utility::getSiteList()); + $centerid = array_search($visitinfo['Site'], \Utility::getSiteList()); - if ($centerID === false || !in_array( - new \CenterID(strval($centerID)), - $user->getCenterIDs() - ) + if ($centerid === false + || !in_array(new \CenterID("$centerid"), $user->getCenterIDs()) ) { return new \LORIS\Http\Response\JSON\Forbidden( "You can't create or modify candidates visit for the site " . - $centerID + $centerid ); } - // Now that the validation is done, convert to an object. - $this->_centerID = new \CenterID(strval($centerID)); - - return null; - } - - /** - * Handles a PUT request that creates or replace a candidate visit - * - * @param ServerRequestInterface $request The incoming PSR7 request - * - * @return ResponseInterface The outgoing PSR7 response - */ - protected function handlePUT(ServerRequestInterface $request): ResponseInterface - { - $user = $request->getAttribute('user'); - $data = json_decode((string) $request->getBody(), true); - $visitinfo = $data ?? []; - - $response = $this->validatePUT($request); - if ($response !== null) { - return $response; - } + // \Utility::getSiteList key was a string. Now that the + // validation is done, convert to an object. + $centerid = new \CenterID("$centerid"); $subprojectid = array_search( $visitinfo['Battery'], @@ -286,7 +254,6 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator $timepoint = \NDB_Factory::singleton()->timepoint( new \SessionID(strval($sessionid)) ); - if ($timepoint->getCurrentStage() !== 'Not Started') { return new \LORIS\Http\Response\JSON\Conflict( 'This visit is already started' @@ -298,7 +265,7 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator $timepoint->setData( [ - 'CenterID' => $this->_centerID, + 'CenterID' => $centerid, 'Visit_label' => $visitinfo['Visit'], 'SubprojectID' => $subprojectid, 'Active' => 'Y', @@ -307,7 +274,7 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator 'UserID' => $username, 'Date_registered' => $today, 'Testdate' => $today, - 'ProjectID' => $this->_project->getId(), + 'ProjectID' => $project->getId(), ] ); @@ -333,8 +300,8 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator $this->_candidate, $subprojectid, $visitinfo['Visit'], - \Site::singleton($this->_centerID), - $this->_project + \Site::singleton($centerid), + $project ); $link = '/' . $request->getUri()->getPath(); diff --git a/modules/api/php/endpoints/candidate/visit/visit_0_0_4_dev.class.inc b/modules/api/php/endpoints/candidate/visit/visit_0_0_4_dev.class.inc index 66fc951f31e..8bbca2a21bb 100644 --- a/modules/api/php/endpoints/candidate/visit/visit_0_0_4_dev.class.inc +++ b/modules/api/php/endpoints/candidate/visit/visit_0_0_4_dev.class.inc @@ -14,6 +14,9 @@ namespace LORIS\api\Endpoints\Candidate\Visit; use \Psr\Http\Message\ServerRequestInterface; use \Psr\Http\Message\ResponseInterface; +use \LORIS\api\Endpoint; +use \LORIS\StudyEntities\Candidate\CandID; + /** * A class for handling the /candidates/$candidate/$visit_label endpoint. @@ -24,8 +27,49 @@ use \Psr\Http\Message\ResponseInterface; * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 * @link https://github.com/aces/Loris */ -class Visit_0_0_4_Dev extends Visit +class Visit_0_0_4_Dev extends Endpoint implements \LORIS\Middleware\ETagCalculator { + + /** + * The requested Candidate + * + * @var \Candidate + */ + private $_candidate; + + /** + * The requested Visit + * + * @var ?\Timepoint + */ + private $_visit; + + /** + * The visit centerID + * + * @var ?\CenterID + */ + private $_centerID; + + /** + * The visit Project + * + * @var ?\Project + */ + private $_project; + + /** + * Contructor + * + * @param \Candidate $candidate The requested candidate + * @param ?\Timepoint $visit The requested visit; can be null in PUT requests + */ + public function __construct(\Candidate $candidate, ?\Timepoint $visit) + { + $this->_candidate = $candidate; + $this->_visit = $visit; + } + /** * Return which methods are supported by this endpoint. * @@ -54,102 +98,420 @@ class Visit_0_0_4_Dev extends Visit } /** - * Handles a PATCH request. - * For now, only support a NextStageDate argument to start next stage. + * Handles a request that starts with /candidates/$candid * * @param ServerRequestInterface $request The incoming PSR7 request * * @return ResponseInterface The outgoing PSR7 response */ - protected function handlePATCH( - ServerRequestInterface $request - ): ResponseInterface { - $data = json_decode((string) $request->getBody(), true); - $nextStageDate = $data['NextStageDate'] ?? null; + public function handle(ServerRequestInterface $request) : ResponseInterface + { + if ($this->_visit !== null) { + $user = $request->getAttribute('user'); + if (!$this->_visit->isAccessibleBy($user)) { + return new \LORIS\Http\Response\JSON\Forbidden('Permission denied'); + } + } + + $pathparts = $request->getAttribute('pathparts'); + if (count($pathparts) === 0) { + $method = $request->getMethod(); + + if (in_array($method, $this->allowedMethods())) { + $handle = '_handle'.$method; + return $this->$handle($request); + } + + if ($method === 'OPTIONS') { + return (new \LORIS\Http\Response()) + ->withHeader('Allow', $this->allowedMethods()); + } + + return new \LORIS\Http\Response\JSON\MethodNotAllowed( + $this->allowedMethods() + ); + } if ($this->_visit === null) { + // Subendpoint requires a Timepoint. see Constructor comment for $visit return new \LORIS\Http\Response\JSON\NotFound('Visit not found'); } - $response = $this->validatePUT($request); - if ($response !== null) { - return $response; + // Delegate to sub-endpoints + $subendpoint = array_shift($pathparts); + switch($subendpoint) { + case 'instruments': + $handler = new Instruments($this->_visit); + break; + case 'images': + $handler = new Images($this->_visit); + break; + case 'qc': + $handler = new Qc($this->_visit); + break; + case 'dicoms': + $handler = new Dicoms($this->_visit); + break; + case 'recordings': + $handler = new Recordings($this->_visit); + break; + default: + return new \LORIS\Http\Response\JSON\NotFound(); } - if (!isset($data['Visit']) - || $data['Visit'] !== $this->_visit->getVisitLabel() - ) { + $newrequest = $request + ->withAttribute('pathparts', $pathparts); + + return $handler->process( + $newrequest, + $handler + ); + } + + /** + * Generates a response fitting the API specification for this endpoint. + * + * @return ResponseInterface The outgoing PSR7 response + */ + private function _handleGET(): ResponseInterface + { + if ($this->_visit === null) { + return new \LORIS\Http\Response\JSON\NotFound('Visit not found'); + } + + return new \LORIS\Http\Response\JsonResponse( + (new \LORIS\api\Views\Visit($this->_visit)) + ->toArray() + ); + } + + /** + * Validate a PUT request + * + * TODO: There is no way to validate the the visit_label in the url + * fits the json data of the request because it is removed from the + * pathparts in the calling class. The correct way to do it would be + * to pass a "light" timepoint class that contains the visit_label but + * no sessionid in the constructor. + * + * @param ServerRequestInterface $request The incoming PSR7 request + * + * @return ?ResponseInterface The outgoing PSR7 response + */ + private function _validatePUT( + ServerRequestInterface $request + ): ?ResponseInterface { + $user = $request->getAttribute('user'); + $data = json_decode((string) $request->getBody(), true) ?? []; + + $requiredfields = [ + 'CandID', + 'Visit', + 'Site', + 'Battery', + 'Project', + ]; + + $diff = array_diff($requiredfields, array_keys($data)); + if (!empty($diff)) { return new \LORIS\Http\Response\JSON\BadRequest( - 'The Visit field does not match this visit' + 'Field(s) missing: ' . implode(', ', $diff) + ); + } + + try { + $this->_project = \NDB_Factory::singleton()->project($data['Project']); + } catch (\NotFound $e) { + return new \LORIS\Http\Response\JSON\BadRequest( + $e->getMessage() ); } - if (!$nextStageDate) { + if (strval($data['CandID']) !== strval($this->_candidate->getCandID())) { return new \LORIS\Http\Response\JSON\BadRequest( - 'There is no stage date specified' + 'The CandID field does not match this candidate' + ); + } + + if (!$this->_candidate->isAccessibleBy($user)) { + return new \LORIS\Http\Response\JSON\Forbidden( + "You can't modify that candidate" + ); + } + + $centerID = array_search($data['Site'], \Utility::getSiteList()); + + if ($centerID === false || !in_array( + new \CenterID(strval($centerID)), + $user->getCenterIDs() + )) { + return new \LORIS\Http\Response\JSON\Forbidden( + "You can't create or modify candidates' visits for the site " . + $data['Site'] ); } - $date_format = 'Y-m-d'; - $date = \DateTime::createFromFormat($date_format, $nextStageDate); + // Now that the validation is done, convert to an object. + $this->_centerID = new \CenterID(strval($centerID)); + + return null; + } + + /** + * Handles a PUT request that creates or replace a candidate visit + * + * @param ServerRequestInterface $request The incoming PSR7 request + * + * @return ResponseInterface The outgoing PSR7 response + */ + private function _handlePUT(ServerRequestInterface $request): ResponseInterface + { + $user = $request->getAttribute('user'); + $data = json_decode((string) $request->getBody(), true); + $visitinfo = $data ?? []; + + $response = $this->_validatePUT($request); + if ($response !== null) { + return $response; + } + + $subprojectid = array_search( + $visitinfo['Battery'], + \Utility::getSubprojectList() + ); + + $sessionid = array_search( + $visitinfo['Visit'], + $this->_candidate->getListOfVisitLabels() + ); + + if ($sessionid !== false) { + // If the visit label already exists for that candidate + $timepoint = \NDB_Factory::singleton()->timepoint( + new \SessionID(strval($sessionid)) + ); + + if ($timepoint->getCurrentStage() !== 'Not Started') { + return new \LORIS\Http\Response\JSON\Conflict( + 'This visit is already started' + ); + } + + $username = $user->getUsername(); + $today = date("Y-m-d"); - if (!($date && $date->format($date_format) === $nextStageDate)) { + $timepoint->setData( + [ + 'CenterID' => $this->_centerID, + 'Visit_label' => $visitinfo['Visit'], + 'SubprojectID' => $subprojectid, + 'Active' => 'Y', + 'Date_active' => $today, + 'RegisteredBy' => $username, + 'UserID' => $username, + 'Date_registered' => $today, + 'Testdate' => $today, + 'ProjectID' => $this->_project->getId(), + ] + ); + + $link = '/' . $request->getUri()->getPath(); + return (new \LORIS\Http\Response()) + ->withStatus(204) + ->withHeader('Content-Location', $link); + } + + try { + \TimePoint::isValidVisitLabel( + new CandID($visitinfo['CandID']), + $subprojectid, + $visitinfo['Visit'] + ); + } catch (\LorisException $e) { return new \LORIS\Http\Response\JSON\BadRequest( - 'The date specified is invalid. Date must be of the form YYYY-MM-DD' + $e->getMessage() ); } - if ($nextStageDate > date($date_format)) { + \TimePoint::createNew( + $this->_candidate, + $subprojectid, + $visitinfo['Visit'], + \Site::singleton($this->_centerID), + $this->_project + ); + + $link = '/' . $request->getUri()->getPath(); + return (new \LORIS\Http\Response()) + ->withStatus(201) + ->withHeader('Content-Location', $link); + } + + /** + * Handles a PATCH request. + * + * @param ServerRequestInterface $request The incoming PSR7 request + * + * @return ResponseInterface The outgoing PSR7 response + */ + private function _handlePATCH( + ServerRequestInterface $request + ): ResponseInterface { + $data = json_decode((string) $request->getBody(), true); + + if (is_null($data)) { return new \LORIS\Http\Response\JSON\BadRequest( - 'The date specified must be not later than today' + 'The request is not a valid json object' ); } - if ($this->_visit->getCurrentStage() !== 'Not Started') { - return new \LORIS\Http\Response\JSON\Conflict( - 'This visit is already started' + if (is_null($this->_visit)) { + return new \LORIS\Http\Response\JSON\NotFound('Visit not found'); + } + + // a PATCH request should contain the same mandatory fields as a PUT request. + $response = $this->_validatePUT($request); + if ($response !== null) { + return $response; + } + + if ($data['Visit'] !== $this->_visit->getVisitLabel()) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'The Visit field does not match this visit' ); } - $newStage = $this->_visit->getNextStage(); - if (!$newStage) { - return new \LORIS\Http\Response\JSON\Conflict( - "Next stage is null" + if (!isset($data['Stages'])) { + return new \LORIS\Http\Response\JSON\BadRequest( + 'There is no stage data specified' ); } - // start that stage - $this->_visit->startStage($newStage); + $allowedStages = ['Screening', 'Visit', 'Approval']; + $allowedStatuses = ['Pass', 'Failure', 'Withdrawal', 'In Progress']; - // set the date for the new stage - $this->_visit->setData(["Date_".$newStage => $nextStageDate]); + // loop on all stages to validate + foreach ($data['Stages'] as $stage => $stageData) { + if (!in_array($stage, $allowedStages)) { + return new \LORIS\Http\Response\JSON\BadRequest( + "Stage $stage is not valid" + ); + } - // create a new battery object && new battery - $candidate = \Candidate::singleton($this->_visit->getCandID()); + // TODO: Implement and remove + if (in_array($stage, ['Screening', 'Approval'])) { + return new \LORIS\Http\Response\JSON\NotImplemented( + "Stage $stage is not implemented yet" + ); + } - // First visit ? - $firstVisit = false; - try { - if ($candidate->getFirstVisit() == $this->_visit->getVisitLabel()) { - $firstVisit = true; + if (!isset($stageData['Status'])) { + return new \LORIS\Http\Response\JSON\BadRequest( + "There is no $stage stage status specified" + ); + } + + if (!in_array($stageData['Status'], $allowedStatuses)) { + return new \LORIS\Http\Response\JSON\BadRequest( + "Stage $stage status {$stageData['Status']} is not valid" + ); + } + + // TODO: Implement and remove + if (in_array($stageData['Status'], ['Pass', 'Failure', 'Withdrawal'])) { + return new \LORIS\Http\Response\JSON\NotImplemented( + "Stage $stage status transition + {$stageData['Status']} is not implemented yet" + ); + } + + if (!isset($stageData['Date'])) { + return new \LORIS\Http\Response\JSON\BadRequest( + "There is no $stage stage date specified" + ); + } + + $stageData['Date'] = \DateTime::createFromFormat( + 'Y-m-d', + $stageData['Date'] + ); + if (!$stageData['Date']) { + return new \LORIS\Http\Response\JSON\BadRequest( + "The stage date specified for $stage is invalid. + Date must be of the form YYYY-MM-DD" + ); } - } catch (\LorisException $e) { } - $battery = new \NDB_BVL_Battery; - // select a specific time point (sessionID) for the battery - $battery->selectBattery($this->_visit->getSessionID()); + // Passed this point, the request should be valid + // Loop again and process the request + foreach ($data['Stages'] as $stage => $stageData) { + switch ($stage) { + // Only Visit | In progress is supported at the moment + case 'Visit': + switch ($stageData['Status']) { + case 'In Progress': + if ($this->_visit->getCurrentStage() !== 'Not Started') { + return new \LORIS\Http\Response\JSON\Conflict( + "This visit is already started" + ); + } - // add instruments to the time point - $battery->createBattery( - $request->getAttribute("loris"), - $this->_visit->getSubprojectID(), - $newStage, - $this->_visit->getVisitLabel(), - $this->_visit->getCenterID(), - $firstVisit - ); + $newStage = $this->_visit->getNextStage(); + if (!$newStage) { + return new \LORIS\Http\Response\JSON\Conflict( + "Next stage is null" + ); + } + + // start that stage + $this->_visit->startStage($newStage); + + // set the date for the new stage + $this->_visit->setData(["Date_$newStage" => $stageData["Date"]]); + + // create a new battery object && new battery + $candidate = \Candidate::singleton($this->_visit->getCandID()); + $visitLabel = $this->_visit->getVisitLabel(); + + // First visit ? + $firstVisit = false; + try { + if ($candidate->getFirstVisit() == $visitLabel) { + $firstVisit = true; + } + } catch (\LorisException $e) { + } + + $battery = new \NDB_BVL_Battery; + // select a specific time point (sessionID) for the battery + $battery->selectBattery($this->_visit->getSessionID()); - return new \LORIS\Http\Response\JSON\OK(); + // add instruments to the time point + $battery->createBattery( + $request->getAttribute("loris"), + $this->_visit->getSubprojectID(), + $newStage, + $this->_visit->getVisitLabel(), + $this->_visit->getCenterID(), + $firstVisit + ); + + return new \LORIS\Http\Response\JSON\OK(); + } + } + } + } + + /** + * Implements the ETagCalculator interface + * + * @param ServerRequestInterface $request The PSR7 incoming request. + * + * @return string etag summarizing value of this request. + */ + public function ETag(ServerRequestInterface $request) : string + { + return md5(json_encode($this->_visit)); } }