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.

Sign up for GitHub

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

Jump to bottom

Catch 400 error and display it in flash message in GOD #4951

Merged
merged 4 commits into from Dec 11, 2018
Merged

Catch 400 error and display it in flash message in GOD #4951

merged 4 commits into from Dec 11, 2018

Conversation

ZitaNemeckova
Copy link
Contributor

@ZitaNemeckova ZitaNemeckova commented Nov 21, 2018 •
edited

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1650085

Go to Automation -> Automate -> Generic Object Definition -> create/edit a button -> give it same name/description that some other button already has

Before:
screen shot 2018-11-21 at 10 19 39 am

After:
screen shot 2018-11-20 at 4 34 21 pm

@miq-bot add_label wip, automation/automate, bug, hammer/yes

@mzazrivec
Copy link
Contributor

@ZitaNemeckova The codecliemate warnings are worth fixing here.

@ZitaNemeckova ZitaNemeckova changed the title Catch 400 error and display it in flash message in GOD [WIP] Catch 400 error and display it in flash message in GOD Nov 21, 2018
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] Catch 400 error and display it in flash message in GOD Catch 400 error and display it in flash message in GOD Nov 27, 2018
@miq-bot miq-bot removed the wip label Nov 27, 2018
himdel
himdel reviewed Dec 5, 2018
View reviewed changes
app/assets/javascripts/components/generic_object/main-custom-button-form-component.js
.then(miqService.redirectBack.bind(vm, saveMsgBtnInGrp, 'success', vm.redirectUrl))
.catch(miqService.handleFailure);
// private functions
function handleErrorMessages(error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing miqSparkleOff()

it never stops sparkling on error

himdel
himdel reviewed Dec 5, 2018
View reviewed changes
app/assets/javascripts/components/generic_object/main-custom-button-form-component.js
@@ -125,13 +125,32 @@ function mainCustomButtonFormController(API, miqService, $q, $http) {
};

vm.saveClicked = function() {
miqService.sparkleOn();
Copy link
Contributor

@himdel himdel Dec 5, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..and we probably need miqService.miqFlashClear(); in both saveClicked and addClicked.

Because otherwise:

  1. fail the validation with duplicate name
    => Name has already been taken appears

  2. fix the name (and also edit the description)

  3. fail the validation with duplicate desciption
    => Desciption has already been taken appears

  4. you're seeing both flash messages, but one is obsolete now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(or you could just replace add_flash with miqService.miqFlash, but that would not work when both are supposed to be shown, so you'd have to merge the messages into one)

@himdel
Copy link
Contributor

himdel commented Dec 5, 2018

Verified adding a new button works,
verified adding a new button when in a group also adds it in the group,
verified editing a button also works.

So just the error path now :).

@himdel himdel self-assigned this Dec 5, 2018
himdel
himdel reviewed Dec 6, 2018
View reviewed changes
app/assets/javascripts/components/generic_object/main-custom-button-form-component.js Outdated
@@ -221,6 +223,7 @@ function mainCustomButtonFormController(API, miqService, $q, $http) {
} else {
return miqService.handleFailure();
}
miqSparkleOff();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong place :) Before the returns please :)

@miq-bot
Copy link
Member

miq-bot commented Dec 10, 2018

Checked commits ZitaNemeckova/manageiq-ui-classic@55d7178~...2035006 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. ⭐

@himdel himdel merged commit 6565204 into ManageIQ:master Dec 11, 2018
@himdel himdel added this to the Sprint 101 Ending Dec 17, 2018 milestone Dec 11, 2018
@ZitaNemeckova ZitaNemeckova deleted the fix_error_message_god_button branch February 12, 2019 13:15
simaishi pushed a commit that referenced this pull request Mar 6, 2019
@himdel @simaishi
Merge pull request #4951 from ZitaNemeckova/fix_error_message_god_button
b42e732 
Catch 400 error and display it in flash message in GOD

(cherry picked from commit 6565204)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1686018
@simaishi
Copy link
Contributor

simaishi commented Mar 6, 2019

Hammer backport details:

$ git log -1
commit b42e732afa66e8754bb3c12408de2e9176320da4
Author: Martin Hradil <himdel@seznam.cz>
Date:   Tue Dec 11 15:22:56 2018 +0100

    Merge pull request #4951 from ZitaNemeckova/fix_error_message_god_button
    
    Catch 400 error and display it in flash message in GOD
    
    (cherry picked from commit 65652049aeabc25426bb09ef1ad941b5eb4d56b9)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1686018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@himdel himdel himdel left review comments

Assignees

@himdel himdel

Labels
automation/automate bug generic objects hammer/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ZitaNemeckova @mzazrivec @himdel @miq-bot @simaishi