Use the error modal for API errors as well #1976
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Cc @dclarizio, you asked for it :) |
|
@himdel at least 2 of the codeclimate issues look fixable :) Also, can you suggest a reviewer? Thx, Dan |
Right now, all the API requests need to handle errors where called. That should still be true, but we also want to show an error modal on server errors, including the API. Thus, triggering the error modal when the server response is no 2**.
this is mostly so that we can differentiate between the two kinds of requests and handle their specifics appropriately but also showing it in the modal title
this way, doing `API.get(url)` always triggers the error modal on error, but now this can be skipped by using `API.get.noErrorModal(url)` instead. The same goes for all the other methods.
…tch/$http differences there getting at the headers is diffent, need to store contentType outside also, $http errors contain both the response and the metadata, while fetch errors contain only the metadata and async methods to get at the response itself - thus this changes the error objects coming from the API to contain everything needed
…andle JS-encoded responses
Thanks, fixed all 3 :).
I guess @mzazrivec maybe? :) |
|
Checked commits https://github.com/himdel/manageiq-ui-classic/compare/3c9fe45f8ef0b7d33fe27e5d56bc2a297c9f8692~...d3f005549992c5d6d02faefdf37329568bc7a1fa with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
The code looks good and things seem to be working nicely in the UI. Than you @himdel for being a wonderful human being (and coder). |
This was referenced Sep 4, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now, we have an error modal component, which we trigger when an angular ajax request fails - but only for requests made via angular's $http.
Now that we're moving to use the API more and more, we also need to see those errors if they're coming from the API.
Thus, this enhances the error modal to:
noErrorModalAPIvs$http)To prevent the error modal on API request when such is not desired (for example, when a 404 is a valid response that the client code can handle), this makes it possible to call those methods as
API.get.noErrorModal(url)instead ofAPI.get(url)and not see the error. (This works for all the request methods, not just get.)An API request failing (this is new):
A
$httprequest failing (this worked before, but didn't show the url (or the type)):