Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core: Module] Fix errors in 500-reponse logic #4193

Merged
merged 3 commits into from
Dec 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions modules/instrument_list/php/instrument_list.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ class Instrument_List extends \NDB_Menu_Filter
$user =& \User::singleton();
$timePoint = $this->timePoint;
if ($timePoint === null) {
throw new \LorisException("Page requires a timepoint to load");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were being caught by https://github.com/aces/Loris/pull/4163/files#diff-b9919f169806c905a8606fd31442044fR304, preventing instrument_list from loading.

throw new \NotFound("Page requires a timepoint to load");
}
$candidate = $this->candidate;
if ($candidate === null) {
throw new \LorisException("Page requires a candidate to load");
throw new \NotFound("Page requires a candidate to load");
}

//These variable are only for checking the permissions.
Expand Down
25 changes: 14 additions & 11 deletions modules/instrument_list/php/module.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,29 @@ class Module extends \Module
* The instrument_list module falls back to the instrument_list page regardless
* of what the page loaded was. This ensures that routes which use the CandID
* (and not the word "instrument_list") are still able to be loaded and not
* 404 errors.
* give 404 errors.
*
* @param ServerRequestInterface $request The incoming PSR7 request
*
* @return ResponseInterface The outgoing PSR7 response
*/
public function handle(ServerRequestInterface $request) : ResponseInterface
{
try {
$resp = parent::handle($request);
// hasAccess from the parent returns a 403
if ($resp->getStatusCode() != 404) {
return $resp;
}
} catch (\LorisException $e) {
// Calling hasAccess from the loader would have failed with
// an exception saying it needs a timepoint/candidate if loading
// the instrument_list page, which we set below.
$resp = parent::handle($request);
// hasAccess from the parent returns a 403
if ($resp->getStatusCode() != 404) {
return $resp;
}

/* If the response code above was a 404, it could have been caused by
* the \NotFound exception in instrument_list.class.inc. This means
* that the route used to access the module contained either a candID
* or a timepoint but not both.
* The code below extracts this information from the server so that the
* module can load properly even in cases like the above where
* "instrument_list" isn't actually in the URL path.
*/

// Falling back to instrument_list, ensure that the CandID
// and SessionID are valid, and if so attach the models to
// the request.
Expand Down
5 changes: 3 additions & 2 deletions php/libraries/Module.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ class Module extends \LORIS\Router\PrefixRouter implements RequestHandlerInterfa
}

try {
$user = $request->getAttribute("user") ?? new \LORIS\AnonymousUser();
$page = $this->loadPage($pagename);
// FIXME: Hack required for breadcrumbs. This should be removed,
// but some tests depend on it.
Expand All @@ -258,7 +259,7 @@ class Module extends \LORIS\Router\PrefixRouter implements RequestHandlerInterfa

if ($page->_hasAccess() !== true) {
return (new \LORIS\Middleware\PageDecorationMiddleware(
$request->getAttribute("user") ?? new \LORIS\AnonymousUser()
$user
))->process(
$request,
new \LORIS\Router\NoopResponder(
Expand All @@ -272,7 +273,7 @@ class Module extends \LORIS\Router\PrefixRouter implements RequestHandlerInterfa
}
} catch (\NotFound $e) {
return (new \LORIS\Middleware\PageDecorationMiddleware(
$request->getAttribute("user") ?? new \LORIS\AnonymousUser()
$user
))->process(
$request,
new \LORIS\Router\NoopResponder(
Expand Down