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

Fix API error object code and display a flash error message #805

Merged
merged 1 commit into from
Mar 27, 2017

Conversation

AparnaKarve
Copy link
Contributor

After #783 merge, the API error object would be in e.error. Related code was adjusted accordingly.

Also failures from the API need to be shown as a flash messages.

@AparnaKarve
Copy link
Contributor Author

/cc @himdel

@himdel
Copy link
Contributor

himdel commented Mar 24, 2017

Looks good, but I'm wondering if the old e.message still isn't happening somewhere .. any chance this could still be happening in $http requests?

@AparnaKarve AparnaKarve force-pushed the fix_api_error_flash_msg branch from 21f07ac to d9acd65 Compare March 24, 2017 23:58
@AparnaKarve
Copy link
Contributor Author

AparnaKarve commented Mar 25, 2017

any chance this could still be happening in $http requests

Not sure, but won't hurt to have both cases in there.
Updated with the change.

(code is slightly repetitious, but I wanted to use minimum number of conditionals)

@AparnaKarve AparnaKarve force-pushed the fix_api_error_flash_msg branch from d9acd65 to 52fc752 Compare March 25, 2017 00:09
@miq-bot
Copy link
Member

miq-bot commented Mar 25, 2017

Checked commit AparnaKarve@52fc752 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks good. 🏆

@himdel
Copy link
Contributor

himdel commented Mar 27, 2017

@AparnaKarve I was a bit worried that this would make some places flash twice, which is why I didn't do this in #783 ... but looks like you're right, no such place I can see.. works like a charm, thanks! :)

@himdel himdel added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 27, 2017
@himdel himdel merged commit 31bd639 into ManageIQ:master Mar 27, 2017
@himdel
Copy link
Contributor

himdel commented Mar 27, 2017

@AparnaKarve just a note, as part of angular2 compatibility effort, I'm going to try making eslint warn about angular.isDefined, and we should stop using it at some point.

I can do a sweep PR so don't worry about it too much, but.. well, angular.isDefined(foo) is just a long way of writing foo !== undefined so.. we can just do that :). (Unless backporting to darga, I'm not sure about IE9 in this regard.)

Just FYI :)

@AparnaKarve AparnaKarve deleted the fix_api_error_flash_msg branch July 26, 2017 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants