From f2dbb96e847be609ce339c26d12871c2cb3db541 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Tue, 20 Oct 2015 12:48:26 +1030 Subject: [PATCH] Improve client XHR error handling The default XHR error handler produce an alert which is appropriate to the response status code. It can be overridden per-request (by specifying the `errorHandler` option) so that the alert can be suppressed or displayed in a different position (e.g. inside a modal). ref #118 --- .../js/admin/src/components/EditGroupModal.js | 11 ++- .../js/admin/src/components/SettingsModal.js | 7 +- .../js/forum/src/components/AvatarEditor.js | 1 + .../forum/src/components/ChangeEmailModal.js | 14 +--- .../src/components/ChangePasswordModal.js | 4 +- .../js/forum/src/components/ComposerBody.js | 8 ++ .../src/components/DiscussionComposer.js | 6 +- .../js/forum/src/components/DiscussionPage.js | 2 +- .../forum/src/components/EditPostComposer.js | 12 +-- .../js/forum/src/components/EditUserModal.js | 25 +++--- .../src/components/ForgotPasswordModal.js | 22 ++---- .../js/forum/src/components/LogInModal.js | 36 ++++----- .../forum/src/components/NotificationList.js | 13 ++-- .../js/forum/src/components/ReplyComposer.js | 9 +-- .../js/forum/src/components/SignUpModal.js | 11 +-- .../core/js/forum/src/components/UserBio.js | 5 +- framework/core/js/lib/App.js | 76 ++++++++++++++----- framework/core/js/lib/Model.js | 14 ++-- framework/core/js/lib/Session.js | 7 +- framework/core/js/lib/Store.js | 7 +- framework/core/js/lib/components/Modal.js | 29 +++---- .../js/lib/components/RequestErrorModal.js | 4 - framework/core/js/lib/utils/RequestError.js | 13 +++- .../src/Api/Controller/TokenController.php | 9 +-- .../core/src/Core/Command/EditUserHandler.php | 9 ++- .../src/Forum/Controller/LoginController.php | 13 ++-- 26 files changed, 192 insertions(+), 175 deletions(-) diff --git a/framework/core/js/admin/src/components/EditGroupModal.js b/framework/core/js/admin/src/components/EditGroupModal.js index 744bf804f..056cde4ab 100644 --- a/framework/core/js/admin/src/components/EditGroupModal.js +++ b/framework/core/js/admin/src/components/EditGroupModal.js @@ -85,13 +85,12 @@ export default class EditGroupModal extends Modal { namePlural: this.namePlural(), color: this.color(), icon: this.icon() - }).then( - () => this.hide(), - (response) => { + }, {errorHandler: this.onerror.bind(this)}) + .then(this.hide.bind(this)) + .catch(() => { this.loading = false; - this.handleErrors(response); - } - ); + m.redraw(); + }); } deleteGroup() { diff --git a/framework/core/js/admin/src/components/SettingsModal.js b/framework/core/js/admin/src/components/SettingsModal.js index fc8b10206..f683fe14f 100644 --- a/framework/core/js/admin/src/components/SettingsModal.js +++ b/framework/core/js/admin/src/components/SettingsModal.js @@ -68,11 +68,8 @@ export default class SettingsModal extends Modal { this.loading = true; saveSettings(this.dirty()).then( - () => this.hide(), - () => { - this.loading = false; - m.redraw(); - } + this.hide.bind(this), + this.loaded.bind(this) ); } } diff --git a/framework/core/js/forum/src/components/AvatarEditor.js b/framework/core/js/forum/src/components/AvatarEditor.js index 718c2b3a8..35acab59c 100644 --- a/framework/core/js/forum/src/components/AvatarEditor.js +++ b/framework/core/js/forum/src/components/AvatarEditor.js @@ -163,5 +163,6 @@ export default class AvatarEditor extends Component { */ failure() { this.loading = false; + m.redraw(); } } diff --git a/framework/core/js/forum/src/components/ChangeEmailModal.js b/framework/core/js/forum/src/components/ChangeEmailModal.js index 14f10f740..320d9313b 100644 --- a/framework/core/js/forum/src/components/ChangeEmailModal.js +++ b/framework/core/js/forum/src/components/ChangeEmailModal.js @@ -83,16 +83,8 @@ export default class ChangeEmailModal extends Modal { this.loading = true; - app.session.user.save({email: this.email()}).then( - () => { - this.loading = false; - this.success = true; - m.redraw(); - }, - response => { - this.loading = false; - this.handleErrors(response); - } - ); + app.session.user.save({email: this.email()}, {errorHandler: this.onerror.bind(this)}) + .then(() => this.success = true) + .finally(this.loaded.bind(this)); } } diff --git a/framework/core/js/forum/src/components/ChangePasswordModal.js b/framework/core/js/forum/src/components/ChangePasswordModal.js index 976bd306c..82ebc4a0d 100644 --- a/framework/core/js/forum/src/components/ChangePasswordModal.js +++ b/framework/core/js/forum/src/components/ChangePasswordModal.js @@ -42,8 +42,8 @@ export default class ChangePasswordModal extends Modal { url: app.forum.attribute('apiUrl') + '/forgot', data: {email: app.session.user.email()} }).then( - () => this.hide(), - () => this.loading = false + this.hide.bind(this), + this.loaded.bind(this) ); } } diff --git a/framework/core/js/forum/src/components/ComposerBody.js b/framework/core/js/forum/src/components/ComposerBody.js index 2a1404104..5d485023f 100644 --- a/framework/core/js/forum/src/components/ComposerBody.js +++ b/framework/core/js/forum/src/components/ComposerBody.js @@ -102,4 +102,12 @@ export default class ComposerBody extends Component { */ onsubmit() { } + + /** + * Stop loading. + */ + loaded() { + this.loading = false; + m.redraw(); + } } diff --git a/framework/core/js/forum/src/components/DiscussionComposer.js b/framework/core/js/forum/src/components/DiscussionComposer.js index a8230b2bf..f3efa8567 100644 --- a/framework/core/js/forum/src/components/DiscussionComposer.js +++ b/framework/core/js/forum/src/components/DiscussionComposer.js @@ -109,11 +109,7 @@ export default class DiscussionComposer extends ComposerBody { app.cache.discussionList.addDiscussion(discussion); m.route(app.route.discussion(discussion)); }, - response => { - this.loading = false; - m.redraw(); - app.alertErrors(response.errors); - } + this.loaded.bind(this) ); } } diff --git a/framework/core/js/forum/src/components/DiscussionPage.js b/framework/core/js/forum/src/components/DiscussionPage.js index 7f0975348..e859fd409 100644 --- a/framework/core/js/forum/src/components/DiscussionPage.js +++ b/framework/core/js/forum/src/components/DiscussionPage.js @@ -128,7 +128,7 @@ export default class DiscussionPage extends Page { // component for the first time on page load, then any calls to m.redraw // will be ineffective and thus any configs (scroll code) will be run // before stuff is drawn to the page. - setTimeout(this.show.bind(this, preloadedDiscussion)); + setTimeout(this.show.bind(this, preloadedDiscussion), 0); } else { const params = this.requestParams(); diff --git a/framework/core/js/forum/src/components/EditPostComposer.js b/framework/core/js/forum/src/components/EditPostComposer.js index 8eaacdb33..7341bd26b 100644 --- a/framework/core/js/forum/src/components/EditPostComposer.js +++ b/framework/core/js/forum/src/components/EditPostComposer.js @@ -37,7 +37,7 @@ export default class EditPostComposer extends ComposerBody { items.add('title', (

- {icon('pencil')}{' '} + {icon('pencil')} {' '} {app.trans('core.forum.composer_edit_post_link', {number: post.number(), discussion: post.discussion().title()})} @@ -64,14 +64,8 @@ export default class EditPostComposer extends ComposerBody { const data = this.data(); this.props.post.save(data).then( - () => { - app.composer.hide(); - m.redraw(); - }, - () => { - this.loading = false; - m.redraw(); - } + () => app.composer.hide(), + this.loaded.bind(this) ); } } diff --git a/framework/core/js/forum/src/components/EditUserModal.js b/framework/core/js/forum/src/components/EditUserModal.js index 8d963a9db..e409ea68b 100644 --- a/framework/core/js/forum/src/components/EditUserModal.js +++ b/framework/core/js/forum/src/components/EditUserModal.js @@ -102,11 +102,7 @@ export default class EditUserModal extends Modal { ); } - onsubmit(e) { - e.preventDefault(); - - this.loading = true; - + data() { const groups = Object.keys(this.groups) .filter(id => this.groups[id]()) .map(id => app.store.getById('groups', id)); @@ -121,12 +117,19 @@ export default class EditUserModal extends Modal { data.password = this.password(); } - this.props.user.save(data).then( - () => this.hide(), - response => { + return data; + } + + onsubmit(e) { + e.preventDefault(); + + this.loading = true; + + this.props.user.save(this.data(), {errorHandler: this.onerror.bind(this)}) + .then(this.hide.bind(this)) + .catch(() => { this.loading = false; - this.handleErrors(response); - } - ); + m.redraw(); + }); } } diff --git a/framework/core/js/forum/src/components/ForgotPasswordModal.js b/framework/core/js/forum/src/components/ForgotPasswordModal.js index 586ef26c3..456186277 100644 --- a/framework/core/js/forum/src/components/ForgotPasswordModal.js +++ b/framework/core/js/forum/src/components/ForgotPasswordModal.js @@ -85,24 +85,12 @@ export default class ForgotPasswordModal extends Modal { app.request({ method: 'POST', url: app.forum.attribute('apiUrl') + '/forgot', - data: {email: this.email()}, - handlers: { - 404: () => { - this.alert = new Alert({type: 'warning', message: 'That email wasn\'t found in our database.'}); - throw new Error(); - } - } - }).then( - () => { - this.loading = false; + data: {email: this.email()} + }) + .then(() => { this.success = true; this.alert = null; - m.redraw(); - }, - response => { - this.loading = false; - this.handleErrors(response); - } - ); + }) + .finally(this.loaded.bind(this)); } } diff --git a/framework/core/js/forum/src/components/LogInModal.js b/framework/core/js/forum/src/components/LogInModal.js index 5999e115c..c4acab352 100644 --- a/framework/core/js/forum/src/components/LogInModal.js +++ b/framework/core/js/forum/src/components/LogInModal.js @@ -124,25 +124,25 @@ export default class LogInModal extends Modal { const email = this.email(); const password = this.password(); - app.session.login(email, password).then( - null, - response => { - this.loading = false; + app.session.login(email, password, {errorHandler: this.onerror.bind(this)}) + .catch(this.loaded.bind(this)); + } - if (response && response.code === 'confirm_email') { - this.alert = Alert.component({ - children: app.trans('core.forum.log_in_confirmation_required_message', {email: response.email}) - }); - } else { - this.alert = Alert.component({ - type: 'error', - children: app.trans('core.forum.log_in_invalid_login_message') - }); - } + onerror(error) { + switch (error.status) { + case 401: + error.alert.props.children = app.trans('core.forum.log_in_confirmation_required_message', {email: error.response.emailConfirmationRequired}); + delete error.alert.props.type; + break; - m.redraw(); - this.onready(); - } - ); + case 404: + error.alert.props.children = app.trans('core.forum.log_in_invalid_login_message'); + break; + + default: + // no default + } + + super.onerror(error); } } diff --git a/framework/core/js/forum/src/components/NotificationList.js b/framework/core/js/forum/src/components/NotificationList.js index 84843ec81..53e97b1e6 100644 --- a/framework/core/js/forum/src/components/NotificationList.js +++ b/framework/core/js/forum/src/components/NotificationList.js @@ -115,13 +115,12 @@ export default class NotificationList extends Component { this.loading = true; m.redraw(); - app.store.find('notifications').then(notifications => { - app.session.user.pushAttributes({newNotificationsCount: 0}); - app.cache.notifications = notifications.sort((a, b) => b.time() - a.time()); - - this.loading = false; - m.redraw(); - }); + app.store.find('notifications') + .then(notifications => { + app.session.user.pushAttributes({newNotificationsCount: 0}); + app.cache.notifications = notifications.sort((a, b) => b.time() - a.time()); + }) + .finally(this.loaded.bind(this)); } /** diff --git a/framework/core/js/forum/src/components/ReplyComposer.js b/framework/core/js/forum/src/components/ReplyComposer.js index e23a62882..5682a993e 100644 --- a/framework/core/js/forum/src/components/ReplyComposer.js +++ b/framework/core/js/forum/src/components/ReplyComposer.js @@ -36,7 +36,8 @@ export default class ReplyComposer extends ComposerBody { items.add('title', (

- {icon('reply')}{' '}{discussion.title()} + {icon('reply')} {' '} + {discussion.title()}

)); @@ -93,11 +94,7 @@ export default class ReplyComposer extends ComposerBody { app.composer.hide(); }, - response => { - this.loading = false; - m.redraw(); - app.alertErrors(response.errors); - } + this.loaded.bind(this) ); } } diff --git a/framework/core/js/forum/src/components/SignUpModal.js b/framework/core/js/forum/src/components/SignUpModal.js index edb98bb33..f30946140 100644 --- a/framework/core/js/forum/src/components/SignUpModal.js +++ b/framework/core/js/forum/src/components/SignUpModal.js @@ -178,7 +178,8 @@ export default class SignUpModal extends Modal { app.request({ url: app.forum.attribute('baseUrl') + '/register', method: 'POST', - data + data, + errorHandler: this.onerror.bind(this) }).then( payload => { const user = app.store.pushPayload(payload); @@ -190,14 +191,10 @@ export default class SignUpModal extends Modal { window.location.reload(); } else { this.welcomeUser = user; - this.loading = false; - m.redraw(); + this.loaded(); } }, - response => { - this.loading = false; - this.handleErrors(response); - } + this.loaded.bind(this) ); } diff --git a/framework/core/js/forum/src/components/UserBio.js b/framework/core/js/forum/src/components/UserBio.js index e8ebdab3f..29fe670cc 100644 --- a/framework/core/js/forum/src/components/UserBio.js +++ b/framework/core/js/forum/src/components/UserBio.js @@ -91,10 +91,7 @@ export default class UserBio extends Component { if (user.bio() !== value) { this.loading = true; - user.save({bio: value}).then(() => { - this.loading = false; - m.redraw(); - }); + user.save({bio: value}).finally(this.loaded.bind(this)); } this.editing = false; diff --git a/framework/core/js/lib/App.js b/framework/core/js/lib/App.js index 08bf62f8c..e136e257e 100644 --- a/framework/core/js/lib/App.js +++ b/framework/core/js/lib/App.js @@ -206,10 +206,14 @@ export default class App { try { return JSON.parse(responseText); } catch (e) { - throw new RequestError(e.message, responseText); + throw new RequestError(500, responseText); } }); + options.errorHandler = options.errorHandler || (error => { + throw error; + }); + // When extracting the data from the response, we can check the server // response code and show an error message to the user if something's gone // awry. @@ -225,26 +229,56 @@ export default class App { const status = xhr.status; - if (status >= 500 && status <= 599) { - throw new RequestError('Internal Server Error', responseText); + if (status < 200 || status > 299) { + throw new RequestError(status, responseText, xhr); } return responseText; }; - this.alerts.dismiss(this.requestErrorAlert); + if (this.requestError) this.requestError.hideAlert(); // Now make the request. If it's a failure, inspect the error that was // returned and show an alert containing its contents. return m.request(options).then(null, error => { - if (error instanceof RequestError) { - this.alerts.show(this.requestErrorAlert = new Alert({ - type: 'error', - children: 'Oops! Something went wrong. Please reload the page and try again.', - controls: app.forum.attribute('debug') ? [ - - ] : undefined - })); + this.requestError = error; + + let children; + + switch (error.status) { + case 422: + children = error.response.errors + .map(error => [error.detail,
]) + .reduce((a, b) => a.concat(b), []) + .slice(0, -1); + break; + + case 401: + case 403: + children = 'You do not have permission to do that.'; + break; + + case 404: + case 410: + children = 'The requested resource was not found.'; + break; + + default: + children = 'Oops! Something went wrong. Please reload the page and try again.'; + } + + error.alert = new Alert({ + type: 'error', + children, + controls: app.forum.attribute('debug') ? [ + + ] : undefined + }); + + try { + options.errorHandler(error); + } catch (error) { + this.alerts.show(error.alert); } throw error; @@ -264,16 +298,18 @@ export default class App { /** * Show alert error messages for each error returned in an API response. * - * @param {Array} errors + * @param {Object} response * @public */ - alertErrors(errors) { - errors.forEach(error => { - this.alerts.show(new Alert({ - type: 'error', - children: error.detail - })); - }); + alertErrors(response) { + if (response.errors) { + response.errors.forEach(error => { + this.alerts.show(new Alert({ + type: 'error', + children: error.detail + })); + }); + } } /** diff --git a/framework/core/js/lib/Model.js b/framework/core/js/lib/Model.js index 6d7fa0ca2..ad16ceb9b 100644 --- a/framework/core/js/lib/Model.js +++ b/framework/core/js/lib/Model.js @@ -117,10 +117,11 @@ export default class Model { * * @param {Object} attributes The attributes to save. If a 'relationships' key * exists, it will be extracted and relationships will also be saved. + * @param {Object} [options] * @return {Promise} * @public */ - save(attributes) { + save(attributes, options = {}) { const data = { type: this.data.type, id: this.data.id, @@ -153,11 +154,11 @@ export default class Model { this.pushData(data); - return app.request({ + return app.request(Object.assign({ method: this.exists ? 'PATCH' : 'POST', url: app.forum.attribute('apiUrl') + this.apiEndpoint(), data: {data} - }).then( + }, options)).then( // If everything went well, we'll make sure the store knows that this // model exists now (if it didn't already), and we'll push the data that // the API returned into the store. @@ -181,17 +182,18 @@ export default class Model { * Send a request to delete the resource. * * @param {Object} data Data to send along with the DELETE request. + * @param {Object} [options] * @return {Promise} * @public */ - delete(data) { + delete(data, options = {}) { if (!this.exists) return m.deferred.resolve().promise; - return app.request({ + return app.request(Object.assign({ method: 'DELETE', url: app.forum.attribute('apiUrl') + this.apiEndpoint(), data - }).then(() => { + }, options)).then(() => { this.exists = false; this.store.remove(this); }); diff --git a/framework/core/js/lib/Session.js b/framework/core/js/lib/Session.js index a6c121b49..5053c955d 100644 --- a/framework/core/js/lib/Session.js +++ b/framework/core/js/lib/Session.js @@ -26,15 +26,16 @@ export default class Session { * * @param {String} identification The username/email. * @param {String} password + * @param {Object} [options] * @return {Promise} * @public */ - login(identification, password) { - return app.request({ + login(identification, password, options = {}) { + return app.request(Object.assign({ method: 'POST', url: app.forum.attribute('baseUrl') + '/login', data: {identification, password} - }) + }, options)) .then(() => window.location.reload()); } diff --git a/framework/core/js/lib/Store.js b/framework/core/js/lib/Store.js index b159a8e87..43b86541e 100644 --- a/framework/core/js/lib/Store.js +++ b/framework/core/js/lib/Store.js @@ -79,10 +79,11 @@ export default class Store { * Alternatively, if an object is passed, it will be handled as the * `query` parameter. * @param {Object} [query] + * @param {Object} [options] * @return {Promise} * @public */ - find(type, id, query = {}) { + find(type, id, query = {}, options = {}) { let data = query; let url = app.forum.attribute('apiUrl') + '/' + type; @@ -94,11 +95,11 @@ export default class Store { url += '/' + id; } - return app.request({ + return app.request(Object.assign({ method: 'GET', url, data - }).then(this.pushPayload.bind(this)); + }, options)).then(this.pushPayload.bind(this)); } /** diff --git a/framework/core/js/lib/components/Modal.js b/framework/core/js/lib/components/Modal.js index 84f56082b..6f76bdf40 100644 --- a/framework/core/js/lib/components/Modal.js +++ b/framework/core/js/lib/components/Modal.js @@ -109,27 +109,28 @@ export default class Modal extends Component { } /** - * Show an alert describing errors returned from the API, and give focus to + * Stop loading. + */ + loaded() { + this.loading = false; + m.redraw(); + } + + /** + * Show an alert describing an error returned from the API, and give focus to * the first relevant field. * - * @param {Object} response + * @param {RequestError} error */ - handleErrors(response) { - const errors = response && response.errors; - - if (errors) { - this.alert = new Alert({ - type: 'error', - children: errors.map((error, k) => [error.detail, k < errors.length - 1 ? m('br') : '']) - }); - } + onerror(error) { + this.alert = error.alert; m.redraw(); - if (errors) { - this.$('form [name=' + errors[0].source.pointer.replace('/data/attributes/', '') + ']').select(); + if (error.status === 422 && error.response.errors) { + this.$('form [name=' + error.response.errors[0].source.pointer.replace('/data/attributes/', '') + ']').select(); } else { - this.$('form :input:first').select(); + this.onready(); } } } diff --git a/framework/core/js/lib/components/RequestErrorModal.js b/framework/core/js/lib/components/RequestErrorModal.js index ed84ec7b8..5ef8468c0 100644 --- a/framework/core/js/lib/components/RequestErrorModal.js +++ b/framework/core/js/lib/components/RequestErrorModal.js @@ -5,10 +5,6 @@ export default class RequestErrorModal extends Modal { return 'RequestErrorModal Modal--large'; } - title() { - return this.props.error.message; - } - content() { let responseText; diff --git a/framework/core/js/lib/utils/RequestError.js b/framework/core/js/lib/utils/RequestError.js index f4ee196fb..d3f9b30d4 100644 --- a/framework/core/js/lib/utils/RequestError.js +++ b/framework/core/js/lib/utils/RequestError.js @@ -1,6 +1,15 @@ export default class RequestError { - constructor(message, responseText) { - this.message = message; + constructor(status, responseText, xhr) { + this.status = status; this.responseText = responseText; + this.xhr = xhr; + + try { + this.response = JSON.parse(responseText); + } catch (e) { + this.response = null; + } + + this.alert = null; } } diff --git a/framework/core/src/Api/Controller/TokenController.php b/framework/core/src/Api/Controller/TokenController.php index 9f229c780..9bb2de6bd 100644 --- a/framework/core/src/Api/Controller/TokenController.php +++ b/framework/core/src/Api/Controller/TokenController.php @@ -12,11 +12,11 @@ namespace Flarum\Api\Controller; use Flarum\Api\Command\GenerateAccessToken; use Flarum\Core\Repository\UserRepository; -use Flarum\Core\Exception\PermissionDeniedException; use Flarum\Event\UserEmailChangeWasRequested; use Flarum\Http\Controller\ControllerInterface; use Illuminate\Contracts\Bus\Dispatcher as BusDispatcher; use Illuminate\Contracts\Events\Dispatcher as EventDispatcher; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Psr\Http\Message\ServerRequestInterface; use Zend\Diactoros\Response\JsonResponse; @@ -62,16 +62,13 @@ class TokenController implements ControllerInterface $user = $this->users->findByIdentification($identification); if (! $user || ! $user->checkPassword($password)) { - throw new PermissionDeniedException; + throw new ModelNotFoundException; } if (! $user->is_activated) { $this->events->fire(new UserEmailChangeWasRequested($user, $user->email)); - return new JsonResponse([ - 'code' => 'confirm_email', - 'email' => $user->email - ], 401); + return new JsonResponse(['emailConfirmationRequired' => $user->email], 401); } $token = $this->bus->dispatch( diff --git a/framework/core/src/Core/Command/EditUserHandler.php b/framework/core/src/Core/Command/EditUserHandler.php index 83fb0298d..46a7ed64f 100644 --- a/framework/core/src/Core/Command/EditUserHandler.php +++ b/framework/core/src/Core/Command/EditUserHandler.php @@ -63,6 +63,7 @@ class EditUserHandler $attributes = array_get($data, 'attributes', []); $relationships = array_get($data, 'relationships', []); + $validate = []; if (isset($attributes['username'])) { $this->assertPermission($canEdit); @@ -72,6 +73,10 @@ class EditUserHandler if (isset($attributes['email'])) { if ($isSelf) { $user->requestEmailChange($attributes['email']); + + if ($attributes['email'] !== $user->email) { + $validate['email'] = $attributes['email']; + } } else { $this->assertPermission($canEdit); $user->changeEmail($attributes['email']); @@ -81,6 +86,8 @@ class EditUserHandler if (isset($attributes['password'])) { $this->assertPermission($canEdit); $user->changePassword($attributes['password']); + + $validate['password'] = $attributes['password']; } if (isset($attributes['bio'])) { @@ -127,7 +134,7 @@ class EditUserHandler new UserWillBeSaved($user, $actor, $data) ); - $this->validator->assertValid(array_merge($user->getDirty(), array_only($attributes, ['password', 'email']))); + $this->validator->assertValid(array_merge($user->getDirty(), $validate)); $user->save(); diff --git a/framework/core/src/Forum/Controller/LoginController.php b/framework/core/src/Forum/Controller/LoginController.php index ea12bc50e..3ca89af69 100644 --- a/framework/core/src/Forum/Controller/LoginController.php +++ b/framework/core/src/Forum/Controller/LoginController.php @@ -55,12 +55,11 @@ class LoginController implements ControllerInterface $actor = $request->getAttribute('actor'); $params = array_only($request->getParsedBody(), ['identification', 'password']); - $data = json_decode($this->apiClient->send($controller, $actor, [], $params)->getBody()); + $response = $this->apiClient->send($controller, $actor, [], $params); + + if ($response->getStatusCode() === 200) { + $data = json_decode($response->getBody()); - // TODO: The client needs to pass through exceptions(?) or the whole - // response so we can look at the response code. For now if there isn't - // any useful data we just assume it's a 401. - if (isset($data->userId)) { // Extend the token's expiry to 2 weeks so that we can set a // remember cookie AccessToken::where('id', $data->token)->update(['expires_at' => new DateTime('+2 weeks')]); @@ -68,11 +67,11 @@ class LoginController implements ControllerInterface event(new UserLoggedIn($this->users->findOrFail($data->userId), $data->token)); return $this->withRememberCookie( - new JsonResponse($data), + $response, $data->token ); } else { - return new EmptyResponse(401); + return $response; } } }