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 oVirt metrics DB name validation #1379

Merged
merged 1 commit into from
May 19, 2017
Merged

Fix oVirt metrics DB name validation #1379

merged 1 commit into from
May 19, 2017

Conversation

jhernand
Copy link
Contributor

@jhernand jhernand commented May 18, 2017

Currently the 'Database name' field of the 'C & U Database' tab of the
authentication form is always mandatory. In addition the presence check
is done using the 'required' attribute. The result of that is that the
provider can't be edited unless that field is filled, even if the
metrics aren't enabled, and the fact that the validation failed isn't
explicitly shown in the 'C & U Database' tab header. To fix these issues
this patch changes the form to use 'ng-required' instead of 'required',
and to make the field mandatory only when the host name is not empty.

https://bugzilla.redhat.com/1451301

Currently the 'Database name' field of the 'C & U Database' tab of the
authentication for is always mandatory. In addition the presence check
is done using the 'required' attribute. The result of that is that the
provider can't be edited unless that field is filled, even if the
metrics aren't enabled, and the fact that the validation failed isn't
explicitly shown in the 'C & U Database' tab header. To fix these issues
this patch changes the form to use 'ng-required' instead of 'required',
and to make the field mandatory only when the host name is not empty.

https://bugzilla.redhat.com/1451301
@jhernand
Copy link
Contributor Author

@borod108 @masayag please review.

@miq-bot
Copy link
Member

miq-bot commented May 18, 2017

Checked commit https://github.com/jhernand/manageiq-ui-classic/commit/944f4960d09549afbed0bea45d2238adf97a64aa with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@oourfali
Copy link

Looks good to me.
@mpovolny can you review?

@oourfali
Copy link

@martinpovolny can you please review?

@oourfali
Copy link

@miq-bot add_label fine/yes

Copy link

@borod108 borod108 left a comment

Choose a reason for hiding this comment

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

LGTM

@h-kataria
Copy link
Contributor

looks good

@h-kataria h-kataria added this to the Sprint 61 Ending May 22, 2017 milestone May 19, 2017
@h-kataria h-kataria merged commit 76f80da into ManageIQ:master May 19, 2017
simaishi pushed a commit that referenced this pull request Jun 8, 2017
@simaishi
Copy link
Contributor

simaishi commented Jun 8, 2017

Fine backport details:

$ git log -1
commit e2feabdbfd6596551bf5d01babd6885d223fefcd
Author: Harpreet Kataria <hkataria@redhat.com>
Date:   Fri May 19 14:20:46 2017 -0400

    Merge pull request #1379 from jhernand/fix_ovirt_metrics_db_name_validation
    
    Fix oVirt metrics DB name validation
    (cherry picked from commit 76f80da3c2bfdca0eedbe54541934d05a6060d30)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1460002

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.

6 participants