Skip to content
This repository has been archived by the owner on Nov 28, 2022. It is now read-only.

Error handling refactor #65

Merged
merged 2 commits into from Jul 11, 2016

Conversation

kevinansfield
Copy link
Collaborator

@kevinansfield kevinansfield commented Jun 14, 2016

closes TryGhost/Ghost#6974

  • update "change password" fields/process to use inline validations
  • remove notifications.showErrors and update all uses of it to showAPIError
  • display multiple API errors as alerts rather than toaster notifications
  • refactor notifications.showAPIError
    • remove notifications.showErrors, use a loop in showAPIError instead
    • properly determine the message from AjaxError or AdapterError objects
    • determine a unique key if possible so that we don't lose multiple different alerts
  • add ServerUnreachable error for when we get a status code of 0 (eg, when the ghost service has been shut down)
  • simplify error messages for our custom ajax errors

TODO:

  • update password change validation to use inline validation
    • update tests
  • remove all calls to notifications.showErrors (rename to _showErrors to signify it being an internal method)
    • update ._showErrors to use alerts instead of toaster notifications
  • handle standard ember-ajax errors in notifications.showAPIError
  • handle server-returned error messages in notifications.showAPIError
  • update error display tests
  • remove/improve use of getRequestErrorMessage
  • no error message when attempting to edit a post when server is offline

@kevinansfield kevinansfield force-pushed the error-handling-refactor branch 2 times, most recently from ce9f9b8 to e0942fb Compare June 15, 2016 17:17
import {isBlank} from 'ember-utils';
import {A as emberA, isEmberArray} from 'ember-array/utils';
import {filter} from 'ember-computed';
import {dasherize} from 'ember-string';

This comment was marked as abuse.

This comment was marked as abuse.

@kevinansfield kevinansfield force-pushed the error-handling-refactor branch 2 times, most recently from b394137 to 948115a Compare June 16, 2016 16:54
@kevinansfield kevinansfield changed the title [WIP] Error handling refactor Error handling refactor Jun 16, 2016
@kevinansfield
Copy link
Collaborator Author

kevinansfield commented Jun 16, 2016

Ready for review 😃

Found some more edge cases while working on the version mismatch handling 😞

@kevinansfield kevinansfield changed the title Error handling refactor [WIP] Error handling refactor Jun 17, 2016
@kevinansfield kevinansfield force-pushed the error-handling-refactor branch 3 times, most recently from d3d8b70 to c24f503 Compare July 7, 2016 11:11
refs TryGhost/Ghost#6949

Handle version mismatch errors by:
- displaying an alert asking the user to copy any data and refresh
- disabling navigation so that unsaved data is not accidentally lost

Detailed changes:
- add `error` action to application route for global route-based error handling
- remove 404-handler mixin, move logic into app route error handler
- update `.catch` in validation-engine so that promises are rejected with the
  original error objects
- add `VersionMismatchError` and `isVersionMismatchError` to ajax service
- add `upgrade-status` service
  - has a method to trigger the alert and toggle the "upgrade required" mode
  - is injected into all routes by default so that it can be checked before
    transitioning
- add `Route` override
  - updates the `willTransition` hook to check the `upgrade-status` service
    and abort the transition if we're in "upgrade required" mode
- update notifications `showAPIError` method to handle version mismatch errors
- update any areas where we were catching ajax errors manually so that the
  version mismatch error handling is obeyed
- fix redirect tests in editor acceptance test
- fix mirage's handling of 404s for unknown posts in get post requests
- adjust alert z-index to to appear above modal backgrounds
closes TryGhost/Ghost#6974
- update "change password" fields/process to use inline validations
- remove `notifications.showErrors` and update all uses of it to `showAPIError`
- display multiple API errors as alerts rather than toaster notifications
- refactor `notifications.showAPIError`
  - remove `notifications.showErrors`, use a loop in `showAPIError` instead
  - properly determine the message from `AjaxError` or `AdapterError` objects
  - determine a unique key if possible so that we don't lose multiple different alerts
- add `ServerUnreachable` error for when we get a status code of 0 (eg, when the ghost service has been shut down)
- simplify error messages for our custom ajax errors
@kevinansfield kevinansfield changed the title [WIP] Error handling refactor Error handling refactor Jul 8, 2016
@acburdine acburdine merged commit 690dc9d into TryGhost:master Jul 11, 2016
@kevinansfield kevinansfield deleted the error-handling-refactor branch February 4, 2017 10:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants