-
Notifications
You must be signed in to change notification settings - Fork 12
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
API Make use of the new AdminController #151
API Make use of the new AdminController #151
Conversation
'TrackID' => $track->ID, | ||
'Status' => $track->Status, | ||
'Completed' => $track->getCompletedPages(), | ||
'Total' => $track->getTotalPages() | ||
]); | ||
} | ||
return $this->jsonSuccess(200, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning nothing (by returning $this->jsonSuccess(200);
) resulted in a "parsererror" toast message. So I had to include the []
there.
Looks like the toast was being triggered by something upstream of the js in this module itself, so I couldn't just solve it there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing with start()
below.
c091616
to
4be1e71
Compare
if (!Permission::check('CMS_ACCESS_CMSMain')) { | ||
return $this->httpError(403, 'You do not have permission to access this resource'); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handled by AdminController::init()
return $this->httpError(403, 'You do not have permission to access this resource'); | ||
} | ||
// Set headers | ||
HTTPCacheControlMiddleware::singleton()->setMaxAge(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This middleware is disabled in AdminController::init()
$this->response | ||
->addHeader('Content-Type', 'application/json') | ||
->addHeader('Content-Encoding', 'UTF-8') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UTF-8 encoding is either not needed anymore or is done by default - works without that line.
The content type is set by AdminController::jsonSuccess()
@@ -99,7 +99,7 @@ | |||
async: true, | |||
success(data) { | |||
// No report, so let user create one | |||
if (!data) { | |||
if (!data || (typeof data === 'object' && data.length < 1)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no data is no option anymore, but I kept it there as defensive programming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay we don't need this silly routing rule anymore
private static string|array $required_permission_codes = [ | ||
'CMS_ACCESS_CMSMain', | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taken from the explicit permission checks that used to be in the actions.
Make
CMSExternalLinksController
a subclass ofAdminController
introduced in silverstripe/silverstripe-admin#1836Issue
admin/*
routing logic fromLeftAndMain
into its own abstract class silverstripe-admin#1761