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

Add fallback hostname form input fields for Nuage Network Provider #2531

Merged
merged 1 commit into from Oct 30, 2017

Conversation

gasper-vrhovsek
Copy link
Contributor

Added two fallback fields in add and edit network provider forms in
case a Nuage network provider is being added.

@miq-bot add_label enhancement

nuage_add_network_fields

@gasper-vrhovsek gasper-vrhovsek force-pushed the feature/nuage-events-ui branch 2 times, most recently from 7473645 to d604f8f Compare October 27, 2017 08:18
@gberginc
Copy link
Contributor

This PR is related to the Nuage provider PR and is required to allow adding 1 AMQP endpoint or three (1 main + two fallback); two endpoints are never used by this network provider.

/cc @chargio, @blomquisg

@gberginc
Copy link
Contributor

@miq-bot add_label networks

@gberginc
Copy link
Contributor

@miq-bot assign @AparnaKarve

@miq-bot
Copy link
Member

miq-bot commented Oct 27, 2017

This pull request is not mergeable. Please rebase and repush.

Added two fallback fields in add and edit network provider forms in
case a Nuage network provider is being added.
@miq-bot
Copy link
Member

miq-bot commented Oct 29, 2017

Checked commit gasper-vrhovsek@829d3a9 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@gberginc
Copy link
Contributor

@AparnaKarve I've rebased this PR to include the changes introduced recently. The backend PR is not yet merged, however it was already reviewed and got LGTM (ManageIQ/manageiq-providers-nuage#30).

@blomquisg
Copy link
Member

@gberginc, @AparnaKarve: backend PR is merged ManageIQ/manageiq-providers-nuage#30

@gberginc
Copy link
Contributor

Thanks @blomquisg!!!

@AparnaKarve, if you have any questions, please reach out to me on Gitter so that I can provide responses quicker.

%div{"ng-if" => (defined?(hostname_hide) ? false : true) && (defined?(fallback_hostnames) ? fallback_hostnames : false)}
.form-group{"ng-class" => "{'has-error': angularForm.#{prefix}_fallback_hostname1.$invalid}"}
%label.col-md-2.control-label{"for" => "#{prefix}_fallback_hostname1"}
= _('Fallback Hostname 1')
Copy link
Member

Choose a reason for hiding this comment

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

Can this be 'Fallback AMQP Hostname 1'? (Same for 'Fallback Hostname 2' below).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @blomquisg. As discussed in gitter, the reason for Fallback Hostname 1 was because this is already on the AMQP tab and the main input field is entitled Hostname.

The reason for adding the hostname, ipv4 or ipv6 as a placeholder text is that with this long name, the field label is broken into two rows which looks ugly.

@AparnaKarve
Copy link
Contributor

I did not get a chance to test this in the UI, but overall the code looks good since it follows an existing pattern.

LGTM

/cc @martinpovolny @h-kataria

@h-kataria h-kataria added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 30, 2017
@h-kataria h-kataria merged commit 5760f28 into ManageIQ:master Oct 30, 2017
@gberginc
Copy link
Contributor

Thanks @AparnaKarve and @h-kataria!

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.

None yet

6 participants