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

API Make use of the new AdminController #151

Merged
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
7 changes: 0 additions & 7 deletions _config/routes.yml
Copy link
Member Author

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

This file was deleted.

2 changes: 1 addition & 1 deletion client/dist/js/BrokenExternalLinksReport.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion client/src/js/BrokenExternalLinksReport.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Member Author

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.

self.buttonReset();
return;
}
Expand Down
40 changes: 16 additions & 24 deletions src/Controllers/CMSExternalLinksController.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,63 +2,54 @@

namespace SilverStripe\ExternalLinks\Controllers;

use SilverStripe\Admin\AdminController;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus;
use SilverStripe\ExternalLinks\Jobs\CheckExternalLinksJob;
use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask;
use SilverStripe\Control\Controller;
use Symbiote\QueuedJobs\Services\QueuedJobService;
use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware;
use SilverStripe\PolyExecution\PolyOutput;
use SilverStripe\Security\Permission;

class CMSExternalLinksController extends Controller
class CMSExternalLinksController extends AdminController
{
private static ?string $url_segment = 'externallinks';

private static string|array $required_permission_codes = [
'CMS_ACCESS_CMSMain',
];
Comment on lines +17 to +19
Copy link
Member Author

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.


private static $allowed_actions = [
'getJobStatus',
'start'
];

/**
* Respond to Ajax requests for info on a running job
*
* @return string JSON string detailing status of the job
*/
public function getJobStatus()
public function getJobStatus(): HTTPResponse
{
if (!Permission::check('CMS_ACCESS_CMSMain')) {
return $this->httpError(403, 'You do not have permission to access this resource');
}
Comment on lines -28 to -30
Copy link
Member Author

Choose a reason for hiding this comment

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

Handled by AdminController::init()

// Set headers
HTTPCacheControlMiddleware::singleton()->setMaxAge(0);
Copy link
Member Author

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')
Comment on lines -33 to -35
Copy link
Member Author

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()

->addHeader('X-Content-Type-Options', 'nosniff');

// Format status
$this->getResponse()->addHeader('X-Content-Type-Options', 'nosniff');
$track = BrokenExternalPageTrackStatus::get_latest();
if ($track) {
return json_encode([
return $this->jsonSuccess(200, [
'TrackID' => $track->ID,
'Status' => $track->Status,
'Completed' => $track->getCompletedPages(),
'Total' => $track->getTotalPages()
]);
}
return $this->jsonSuccess(200, []);
Copy link
Member Author

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.

Copy link
Member Author

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.

}

/**
* Starts a broken external link check
*/
public function start()
public function start(): HTTPResponse
{
if (!Permission::check('CMS_ACCESS_CMSMain')) {
return $this->httpError(403, 'You do not have permission to access this resource');
}
// return if the a job is already running
$status = BrokenExternalPageTrackStatus::get_latest();
if ($status && $status->Status == 'Running') {
return;
return $this->jsonSuccess(200, []);
}

// Create a new job
Expand All @@ -71,5 +62,6 @@ public function start()
$task = CheckExternalLinksTask::create();
$task->runLinksCheck(PolyOutput::create(PolyOutput::FORMAT_HTML));
}
return $this->jsonSuccess(200, []);
}
}
Loading