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

Custom button group - validate required fields before enabling buttons #3424

Merged
merged 2 commits into from
Feb 21, 2018
Merged

Custom button group - validate required fields before enabling buttons #3424

merged 2 commits into from
Feb 21, 2018

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Feb 19, 2018

Automation > Automate > Customization, Buttons accordion, select object type, Configuration > Add Button Group

Right now, it's apparently possible to submit the form before all the input changes made it to the server.

This enables server-side validation for the required fields.
Fixing this properly will probably mean an angular/react rewrite of the screen.

(+ a fix for the error message when it goes wrong :))

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

comes from ManageIQ/manageiq#6578,
looks like all other errors are error_message
to catch the problem of buttons being enabled before the server knows about the inputs

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1536670
@himdel
Copy link
Contributor Author

himdel commented Feb 19, 2018

Testing:

right now, this has to be tested with

diff --git a/app/views/shared/buttons/_group_form.html.haml b/app/views/shared/buttons/_group_form.html.haml
index f43fa0b30..4f04bc6cd 100644
--- a/app/views/shared/buttons/_group_form.html.haml
+++ b/app/views/shared/buttons/_group_form.html.haml
@@ -15,7 +15,7 @@
                           "data-miq_observe" => {:interval => '.5', :url => url}.to_json)
           .input-group-addon
             %label.checkbox-inline
-              - display = @record.set_data.key?(:display) && @record.set_data[:display] == '1'
+              - display = @edit[:new][:display]
               = check_box_tag("display", "1", display,
                               "data-miq_observe_checkbox" => {:url => url}.to_json)
               = _('Display on Button')

(because of #3383 (comment))

EDIT: fixed in #3431

@himdel
Copy link
Contributor Author

himdel commented Feb 19, 2018

@dclarizio Does this make sense as a fix for https://bugzilla.redhat.com/show_bug.cgi?id=1536670 ?

@miq-bot
Copy link
Member

miq-bot commented Feb 19, 2018

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/6af4e3189ceb979d48602680978b19bc483225a2~...3a7e8715729abf0719294c89fa4febc6a3a75572 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

@mzazrivec mzazrivec self-assigned this Feb 21, 2018
@mzazrivec mzazrivec added this to the Sprint 80 Ending Feb 26, 2018 milestone Feb 21, 2018
@mzazrivec mzazrivec merged commit 873ebff into ManageIQ:master Feb 21, 2018
@himdel himdel deleted the custom-group-bz1536670 branch February 21, 2018 16:35
simaishi pushed a commit that referenced this pull request Mar 8, 2018
Custom button group - validate required fields before enabling buttons
(cherry picked from commit 873ebff)

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

simaishi commented Mar 8, 2018

Gaprindashvili backport details:

$ git log -1
commit eb9e5ff5d6136e29cdc504e40e87eabaac69c4f5
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Wed Feb 21 07:57:36 2018 +0100

    Merge pull request #3424 from himdel/custom-group-bz1536670
    
    Custom button group - validate required fields before enabling buttons
    (cherry picked from commit 873ebffda9a23809abac52b7ad08412f263184a6)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553244

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.

4 participants