From 00be36ad166e09c78352fe0b72a4a61b8b018652 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Mon, 18 May 2015 18:13:16 +0930 Subject: [PATCH] Better API error handling --- .../js/forum/src/components/composer-reply.js | 3 +- .../js/forum/src/components/login-modal.js | 14 ++++-- framework/core/js/lib/components/alert.js | 2 +- framework/core/js/lib/components/alerts.js | 2 +- framework/core/js/lib/utils/app.js | 9 ++++ .../core/src/Api/Actions/DeleteAction.php | 2 +- .../core/src/Api/Actions/JsonApiAction.php | 43 +++++++++++++++++++ .../core/src/Api/Actions/SerializeAction.php | 22 ++-------- .../core/src/Api/Actions/TokenAction.php | 7 ++- 9 files changed, 74 insertions(+), 30 deletions(-) create mode 100644 framework/core/src/Api/Actions/JsonApiAction.php diff --git a/framework/core/js/forum/src/components/composer-reply.js b/framework/core/js/forum/src/components/composer-reply.js index 632be4ce9..3b8b3eeab 100644 --- a/framework/core/js/forum/src/components/composer-reply.js +++ b/framework/core/js/forum/src/components/composer-reply.js @@ -93,9 +93,10 @@ export default class ComposerReply extends ComposerBody { }) ); } - }, (response) => { + }, errors => { this.loading(false); m.redraw(); + app.handleApiErrors(errors); }); } } diff --git a/framework/core/js/forum/src/components/login-modal.js b/framework/core/js/forum/src/components/login-modal.js index a82c48994..09c1563f9 100644 --- a/framework/core/js/forum/src/components/login-modal.js +++ b/framework/core/js/forum/src/components/login-modal.js @@ -1,6 +1,7 @@ import Component from 'flarum/component'; import LoadingIndicator from 'flarum/components/loading-indicator'; import SignupModal from 'flarum/components/signup-modal'; +import Alert from 'flarum/components/alert'; import icon from 'flarum/helpers/icon'; export default class LoginModal extends Component { @@ -15,7 +16,7 @@ export default class LoginModal extends Component { view() { return m('div.modal-dialog.modal-sm.modal-login', [ m('div.modal-content', [ - m('button.btn.btn-icon.btn-link.close.back-control', {onclick: app.modal.close.bind(app.modal)}, icon('times')), + m('button.btn.btn-icon.btn-link.close.back-control', {onclick: this.hide.bind(this)}, icon('times')), m('form', {onsubmit: this.login.bind(this)}, [ m('div.modal-header', m('h3.title-control', 'Log In')), this.props.message ? m('div.modal-alert.alert', this.props.message) : '', @@ -49,15 +50,22 @@ export default class LoginModal extends Component { $modal.find('[name=email]').focus(); } + hide() { + app.modal.close(); + app.alerts.dismiss(this.errorAlert); + } + login(e) { e.preventDefault(); this.loading(true); app.session.login(this.email(), this.password()).then(() => { - app.modal.close(); + this.hide(); this.props.callback && this.props.callback(); - }, (response) => { + }, response => { this.loading(false); m.redraw(); + app.alerts.dismiss(this.errorAlert); + app.alerts.show(this.errorAlert = new Alert({ type: 'warning', message: 'Invalid credentials.' })) }); } } diff --git a/framework/core/js/lib/components/alert.js b/framework/core/js/lib/components/alert.js index 9445ece86..cc10d6de3 100644 --- a/framework/core/js/lib/components/alert.js +++ b/framework/core/js/lib/components/alert.js @@ -13,7 +13,7 @@ export default class Alert extends Component { var message = attrs.message; delete attrs.message; - var controlItems = attrs.controls.slice() || []; + var controlItems = attrs.controls ? attrs.controls.slice() : []; delete attrs.controls; if (attrs.dismissible || attrs.dismissible === undefined) { diff --git a/framework/core/js/lib/components/alerts.js b/framework/core/js/lib/components/alerts.js index bf336a014..2434f770e 100644 --- a/framework/core/js/lib/components/alerts.js +++ b/framework/core/js/lib/components/alerts.js @@ -23,8 +23,8 @@ export default class Alerts extends Component { var index = this.components.indexOf(component); if (index !== -1) { this.components.splice(index, 1); + m.redraw(); } - m.redraw(); } clear() { diff --git a/framework/core/js/lib/utils/app.js b/framework/core/js/lib/utils/app.js index b4b2f0856..300bdb220 100644 --- a/framework/core/js/lib/utils/app.js +++ b/framework/core/js/lib/utils/app.js @@ -1,4 +1,5 @@ import ItemList from 'flarum/utils/item-list'; +import Alert from 'flarum/components/alert'; class App { constructor() { @@ -14,6 +15,14 @@ class App { document.title = (title ? title+' - ' : '')+this.config['forum_title']; } + handleApiErrors(response) { + this.alerts.clear(); + + response.errors.forEach(error => + this.alerts.show(new Alert({ type: 'warning', message: error.detail })) + ); + } + route(name, params) { var url = this.routes[name][0].replace(/:([^\/]+)/g, function(m, t) { var value = params[t]; diff --git a/framework/core/src/Api/Actions/DeleteAction.php b/framework/core/src/Api/Actions/DeleteAction.php index b247e7bd5..d327d14ef 100644 --- a/framework/core/src/Api/Actions/DeleteAction.php +++ b/framework/core/src/Api/Actions/DeleteAction.php @@ -12,7 +12,7 @@ abstract class DeleteAction implements ActionInterface * @param \Flarum\Api\Request $request * @return \Flarum\Api\Response */ - public function handle(Request $request) + public function respond(Request $request) { $this->delete($request, $response = new Response('', 204)); diff --git a/framework/core/src/Api/Actions/JsonApiAction.php b/framework/core/src/Api/Actions/JsonApiAction.php new file mode 100644 index 000000000..95a790999 --- /dev/null +++ b/framework/core/src/Api/Actions/JsonApiAction.php @@ -0,0 +1,43 @@ +respond($request); + } catch (ValidationFailureException $e) { + $errors = []; + foreach ($e->getErrors()->getMessages() as $field => $messages) { + $errors[] = [ + 'detail' => implode("\n", $messages), + 'path' => $field + ]; + } + return new JsonResponse(['errors' => $errors], 422); + } catch (PermissionDeniedException $e) { + return new JsonResponse(null, 401); + } + } + + /** + * Handle an API request and return an API response. + * + * @param \Flarum\Api\Request $request + * @return \Flarum\Api\Response + */ + abstract protected function respond(Request $request); +} diff --git a/framework/core/src/Api/Actions/SerializeAction.php b/framework/core/src/Api/Actions/SerializeAction.php index 732486636..fe26a45fd 100644 --- a/framework/core/src/Api/Actions/SerializeAction.php +++ b/framework/core/src/Api/Actions/SerializeAction.php @@ -3,14 +3,11 @@ use Flarum\Api\Request; use Flarum\Api\JsonApiRequest; use Flarum\Api\JsonApiResponse; -use Flarum\Core\Exceptions\ValidationFailureException; -use Flarum\Core\Exceptions\PermissionDeniedException; use Tobscure\JsonApi\SerializerInterface; use Tobscure\JsonApi\Criteria; use Illuminate\Http\Response; -use Illuminate\Http\JsonResponse; -abstract class SerializeAction implements ActionInterface +abstract class SerializeAction extends JsonApiAction { /** * The name of the serializer class to output results with. @@ -68,24 +65,11 @@ abstract class SerializeAction implements ActionInterface * @param \Flarum\Api\Request $request * @return \Flarum\Api\Response */ - public function handle(Request $request) + public function respond(Request $request) { $request = static::buildJsonApiRequest($request); - try { - $data = $this->data($request, $response = new JsonApiResponse); - } catch (ValidationFailureException $e) { - $errors = []; - foreach ($e->getErrors()->getMessages() as $field => $messages) { - $errors[] = [ - 'detail' => implode("\n", $messages), - 'path' => $field - ]; - } - return new JsonResponse(['errors' => $errors], 422); - } catch (PermissionDeniedException $e) { - return new JsonResponse(null, 401); - } + $data = $this->data($request, $response = new JsonApiResponse); $serializer = new static::$serializer($request->actor, $request->include, $request->link); diff --git a/framework/core/src/Api/Actions/TokenAction.php b/framework/core/src/Api/Actions/TokenAction.php index ca5ae01d3..1836b8cb2 100644 --- a/framework/core/src/Api/Actions/TokenAction.php +++ b/framework/core/src/Api/Actions/TokenAction.php @@ -7,7 +7,7 @@ use Flarum\Core\Exceptions\PermissionDeniedException; use Illuminate\Http\JsonResponse; use Illuminate\Contracts\Bus\Dispatcher; -class TokenAction implements ActionInterface +class TokenAction extends JsonApiAction { protected $users; @@ -25,7 +25,7 @@ class TokenAction implements ActionInterface * @param \Flarum\Api\Request $request * @return \Flarum\Api\Response */ - public function handle(Request $request) + public function respond(Request $request) { $identification = $request->get('identification'); $password = $request->get('password'); @@ -33,8 +33,7 @@ class TokenAction implements ActionInterface $user = $this->users->findByIdentification($identification); if (! $user || ! $user->checkPassword($password)) { - // throw new PermissionDeniedException; - return new JsonResponse(null, 401); + throw new PermissionDeniedException; } $token = $this->bus->dispatch(