UI for add/remove interface on network router #60

sseago · 2017-01-04T03:32:37Z

This commit adds UI code for adding and removing router interfaces.

BZ for this is:

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

Euwe backport depends on merging ManageIQ/manageiq#13005

sseago · 2017-01-04T04:23:09Z

So whoever reviews this, regarding the rubocop output, are all of those hash-related comments in the haml files false positives? They don't seem to match well to the actual content, and the content here is very similar to other haml files that have succeeded in the past.

mzazrivec · 2017-01-05T09:48:13Z

app/assets/javascripts/controllers/network_router/network_router_form_controller.js

@@ -41,6 +46,30 @@ ManageIQ.angular.app.controller('networkRouterFormController', ['$http', '$scope
    miqService.miqAjaxButton(url, $scope.networkRouterModel, { complete: false });
  };

+  $scope.addInterfaceClicked = function() {
+    miqService.sparkleOn();
+    var url = '/network_router/add_interface/' + networkRouterFormId + '?button=addInterface';


Why addInterface when the info is already contained in the path? This should be just:

var url = '/network_router/add_interface/' + networkRouterFormId + '?button=add';

Mostly I was just following convention elsewhere for similar add/remove association functionality. I can change it to just 'add'. I need to verify that this won't cause any ambiguity with the normal create 'add' form, but from a quick glance I think it will be fine. I'll update and re-test to make sure it's working with the change, though.

mzazrivec · 2017-01-05T09:48:22Z

app/assets/javascripts/controllers/network_router/network_router_form_controller.js

+
+  $scope.removeInterfaceClicked = function() {
+    miqService.sparkleOn();
+    var url = '/network_router/remove_interface/' + networkRouterFormId + '?button=removeInterface';


Same as above.

mzazrivec · 2017-01-05T09:52:59Z

app/views/network_router/add_interface_select.html.haml

+      %td{:align => 'right'}
+        #buttons_on
+          = render :partial => "layouts/angular/x_custom_form_buttons_angular",
+                   :locals  => {:button_label           => N_("Add"),


Not N_("Add"), that wouldn't work. You need to use _("Add") here.

Sure. I just noticed that you made this same change last month to the code that I modeled this from -- I'll make the same change here.

mzazrivec · 2017-01-05T09:53:21Z

app/views/network_router/remove_interface_select.html.haml

+      %td{:align => 'right'}
+        #buttons_on
+          = render :partial => "layouts/angular/x_custom_form_buttons_angular",
+                   :locals  => {:button_label           => N_("Remove"),


_("Remove")

sseago · 2017-01-05T15:20:31Z

@mzazrivec OK the suggested changes have been made and the PR updated.

mzazrivec · 2017-01-06T13:34:08Z

app/views/network_router/remove_interface_select.html.haml

+        #buttons_on
+          = render :partial => "layouts/angular/x_custom_form_buttons_angular",
+                   :locals  => {:button_label           => _("Remove"),
+                                :button_click           => "removeInterfaceClicked()"}


@sseago I know this part with %table ... was copied from other places in our views,
but I fixed that recently -- we don't want to use table for styling here, so please fix
this similarly to what I did here:

https://github.com/ManageIQ/manageiq-ui-classic/pull/78/files#diff-64b58413d95ff03ddc058038d06cae9dR17

mzazrivec · 2017-01-06T13:42:23Z

app/views/network_router/add_interface_select.html.haml

+        #buttons_on
+          = render :partial => "layouts/angular/x_custom_form_buttons_angular",
+                   :locals  => {:button_label           => _("Add"),
+                                :button_click           => "addInterfaceClicked()"}


Same as below.

mzazrivec · 2017-01-09T12:38:22Z

app/assets/javascripts/controllers/network_router/network_router_form_controller.js

+    var url = '/network_router/remove_interface/' + networkRouterFormId + '?button=cancel';
+    miqService.miqAjaxButton(url);
+  };
+


Where are the cancelAddInterfaceClicked() and cancelRemoveInterfaceClicked() routines called from?

Ahh, it looks like this is not needed as a result of earlier refactoring. More specifically, this PR was based off the add/remove host functionality on HostAggregate, so this came from cancelAddHostClicked and cancelRemoveHostClicked, both of which were originally called in the UI form with manually-created buttons but should have been removed when I created x_custom_form_buttons_angular.

I'll remove these from network_router_form_controller, although I guess those two functions in host_aggregate_form_controller.js should also be removed (probably not as part of this PR, though)

mzazrivec · 2017-01-10T14:11:15Z

app/views/network_router/add_interface_select.html.haml

@@ -0,0 +1,23 @@
+%form#form_div{:name => "angularForm", 'ng-controller' => "networkRouterFormController"}


Please add :novalidate => true to the list of options for the %form here, so that the browser won't render that
annoying "Please Select an Item from the List." popup when you click Cancel in the add form. We do use
that in other angular forms as well.

mzazrivec · 2017-01-10T14:12:16Z

app/views/network_router/remove_interface_select.html.haml

@@ -0,0 +1,23 @@
+%form#form_div{:name => "angularForm", 'ng-controller' => "networkRouterFormController"}


Same as above with novalidate.

sseago · 2017-01-10T15:45:18Z

@mzazrivec OK the novalidate changes have been made.

mzazrivec · 2017-01-11T12:08:27Z

Failing CI, because of backend changes, it seems.

This commit adds UI code for adding and removing router interfaces. BZ for this is: https://bugzilla.redhat.com/show_bug.cgi?id=1394284 Euwe backport depends on merging ManageIQ/manageiq#13005

miq-bot · 2017-01-11T15:08:29Z

sseago · 2017-01-11T15:08:56Z

@mzazrivec Looks like a recent change in the main repo (method names changed) broke my spec test. Fixed and the test works locally now, so hopefully the build goes through now.

tzumainn · 2017-01-11T20:26:27Z

@miq-bot add_label euwe/yes, blocker, bug

This commit adds UI code for adding and removing router interfaces. BZ for this is: https://bugzilla.redhat.com/show_bug.cgi?id=1394284 This is the Euwe-only backport PR corresponding to ManageIQ/manageiq-ui-classic#60

simaishi · 2017-01-16T14:48:33Z

Euwe backport (to manageiq repo) details:

$ git log -1
commit 228f8c3069a446b8692351b3d8a0573179b5b01e
Author: Milan Zázrivec <[email protected]>
Date:   Fri Jan 13 09:19:53 2017 +0100

    Merge pull request #60 from sseago/router-interfaces-ui
    
    UI for add/remove interface on network router
    (cherry picked from commit bc890e0e029e4ea0e0b4cc109fe01bdb5b592086)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1413212

sseago force-pushed the router-interfaces-ui branch from 58ebed1 to 5838d94 Compare January 4, 2017 04:04

mzazrivec self-assigned this Jan 4, 2017

sseago force-pushed the router-interfaces-ui branch from 5838d94 to e977e11 Compare January 4, 2017 15:19

mzazrivec reviewed Jan 5, 2017

View reviewed changes

sseago force-pushed the router-interfaces-ui branch from e977e11 to aa50b80 Compare January 5, 2017 15:19

mzazrivec reviewed Jan 6, 2017

View reviewed changes

sseago force-pushed the router-interfaces-ui branch from aa50b80 to 380be9f Compare January 7, 2017 04:44

mzazrivec reviewed Jan 9, 2017

View reviewed changes

sseago force-pushed the router-interfaces-ui branch from 380be9f to a4f83dd Compare January 9, 2017 14:24

mzazrivec reviewed Jan 10, 2017

View reviewed changes

sseago force-pushed the router-interfaces-ui branch from a4f83dd to 23f6d91 Compare January 10, 2017 15:44

UI for add/remove interface on network router

b91242e

This commit adds UI code for adding and removing router interfaces. BZ for this is: https://bugzilla.redhat.com/show_bug.cgi?id=1394284 Euwe backport depends on merging ManageIQ/manageiq#13005

sseago force-pushed the router-interfaces-ui branch from 23f6d91 to b91242e Compare January 11, 2017 15:07

miq-bot added blocker bug euwe/yes labels Jan 11, 2017

mzazrivec added this to the Sprint 52 Ending Jan 16, 2017 milestone Jan 13, 2017

mzazrivec merged commit bc890e0 into ManageIQ:master Jan 13, 2017

sseago mentioned this pull request Jan 15, 2017

[EUWE] UI for add/remove interface on network router ManageIQ/manageiq#13500

Closed

simaishi added euwe/backported and removed euwe/yes labels Jan 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI for add/remove interface on network router #60

UI for add/remove interface on network router #60

sseago commented Jan 4, 2017

sseago commented Jan 4, 2017 •

edited

Loading

mzazrivec Jan 5, 2017

sseago Jan 5, 2017

mzazrivec Jan 5, 2017

mzazrivec Jan 5, 2017

sseago Jan 5, 2017

mzazrivec Jan 5, 2017

sseago commented Jan 5, 2017

mzazrivec Jan 6, 2017

mzazrivec Jan 6, 2017

mzazrivec Jan 9, 2017

sseago Jan 9, 2017

sseago Jan 9, 2017

mzazrivec Jan 10, 2017

mzazrivec Jan 10, 2017

sseago commented Jan 10, 2017

mzazrivec commented Jan 11, 2017

miq-bot commented Jan 11, 2017

sseago commented Jan 11, 2017

tzumainn commented Jan 11, 2017

simaishi commented Jan 16, 2017

		@@ -0,0 +1,23 @@
		%form#form_div{:name => "angularForm", 'ng-controller' => "networkRouterFormController"}

UI for add/remove interface on network router #60

UI for add/remove interface on network router #60

Conversation

sseago commented Jan 4, 2017

sseago commented Jan 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sseago commented Jan 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sseago commented Jan 10, 2017

mzazrivec commented Jan 11, 2017

miq-bot commented Jan 11, 2017

sseago commented Jan 11, 2017

tzumainn commented Jan 11, 2017

simaishi commented Jan 16, 2017

sseago commented Jan 4, 2017 •

edited

Loading