Save instace_id inside Associate Floating IP workflow

Also, rename instance_id field to proper port_id.

Before this change, when instance_id was passed from the Instances
page in the URL, the information would be lost on form submission,
and if the form contained an error, the it would be redisplayed with
the port drop-down containing all ports from all instances. This also
made submitting this form slow, as the drop-down would be populated
with all ports on form validation.

It also creates a bug, where if there are more instances than Nova's
pagination allows, the ports from newer instances would no longer
appear in the drop-down, and the form would fail to validate, even
though the choice of port on initial form was correct.

Closes-bug: #1920010
Change-Id: I3ab26c19dc9ea1ed23fcff790d0db919039099eb
(cherry picked from commit 3d82d57353d1a29fb5b568ffaa93db86998e9d94)

3 files changed, 22 insertions, 16 deletions

diff --git a/openstack_dashboard/dashboards/project/floating_ips/tests.py b/openstack_dashboard/dashboards/project/floating_ips/tests.py
index d900b5c13..12a259a9a 100644
--- a/openstack_dashboard/dashboards/project/floating_ips/tests.py
+++ b/openstack_dashboard/dashboards/project/floating_ips/tests.py
@@ -139,7 +139,7 @@ class FloatingIpViewTests(test.TestCase):
             self._get_fip_targets()
         self.mock_floating_ip_associate.return_value = None
 
-        form_data = {'instance_id': port_target_id,
+        form_data = {'port_id': port_target_id,
                      'ip_id': floating_ip.id}
         url = reverse('%s:associate' % NAMESPACE)
         res = self.client.post(url, form_data)
@@ -168,7 +168,7 @@ class FloatingIpViewTests(test.TestCase):
         self.mock_floating_ip_associate.return_value = None
 
         next = reverse("horizon:project:instances:index")
-        form_data = {'instance_id': port_target_id,
+        form_data = {'port_id': port_target_id,
                      'next': next,
                      'ip_id': floating_ip.id}
         url = reverse('%s:associate' % NAMESPACE)
@@ -197,7 +197,7 @@ class FloatingIpViewTests(test.TestCase):
             self._get_fip_targets()
         self.mock_floating_ip_associate.side_effect = self.exceptions.nova
 
-        form_data = {'instance_id': port_target_id,
+        form_data = {'port_id': port_target_id,
                      'ip_id': floating_ip.id}
         url = reverse('%s:associate' % NAMESPACE)
         res = self.client.post(url, form_data)
diff --git a/openstack_dashboard/dashboards/project/floating_ips/workflows.py b/openstack_dashboard/dashboards/project/floating_ips/workflows.py
index b3a0e0338..8f573ec16 100644
--- a/openstack_dashboard/dashboards/project/floating_ips/workflows.py
+++ b/openstack_dashboard/dashboards/project/floating_ips/workflows.py
@@ -36,7 +36,11 @@ class AssociateIPAction(workflows.Action):
         coerce=filters.get_int_or_uuid,
         empty_value=None
     )
-    instance_id = forms.ThemableChoiceField(
+    instance_id = forms.CharField(
+        widget=forms.widgets.HiddenInput(),
+        required=False,
+    )
+    port_id = forms.ThemableChoiceField(
         label=_("Port to be associated")
     )
 
@@ -53,7 +57,8 @@ class AssociateIPAction(workflows.Action):
         # an association target is not an instance but a port, so we need
         # to get an association target based on a received instance_id
         # and set the initial value of instance_id ChoiceField.
-        q_instance_id = self.request.GET.get('instance_id')
+        q_instance_id = self.data.get('instance_id',
+                                      self.request.GET.get('instance_id'))
         q_port_id = self.request.GET.get('port_id')
 
         if policy.check((("network", "create_floatingip"),),
@@ -61,20 +66,21 @@ class AssociateIPAction(workflows.Action):
             self.fields['ip_id'].widget.add_item_link = ALLOCATE_URL
 
         if q_instance_id:
+            self.initial['instance_id'] = q_instance_id
             targets = self._get_target_list(q_instance_id)
             # Setting the initial value here is required to avoid a situation
             # where instance_id passed in the URL is used as the initial value
             # unexpectedly. (This always happens if the form is invoked from
             # the instance table.)
             if targets:
-                self.initial['instance_id'] = targets[0].id
+                self.initial['port_id'] = targets[0].id
             else:
-                self.initial['instance_id'] = ''
+                self.initial['port_id'] = ''
         elif q_port_id:
             targets = self._get_target_list()
             for target in targets:
                 if target.port_id == q_port_id:
-                    self.initial['instance_id'] = target.id
+                    self.initial['port_id'] = target.id
                     break
 
     def populate_ip_id_choices(self, request, context):
@@ -112,9 +118,9 @@ class AssociateIPAction(workflows.Action):
                               redirect=redirect)
         return targets
 
-    # TODO(amotoki): [drop-nova-network] Rename instance_id to port_id
-    def populate_instance_id_choices(self, request, context):
-        q_instance_id = self.request.GET.get('instance_id')
+    def populate_port_id_choices(self, request, context):
+        q_instance_id = self.data.get('instance_id',
+                                      self.initial.get('instance_id'))
         # The reason of specifying an empty tuple when q_instance_id is None
         # is to make memoized_method _get_target_list work. Two calls of
         # _get_target_list from here and __init__ must have a same arguments.
@@ -132,7 +138,7 @@ class AssociateIPAction(workflows.Action):
 
 class AssociateIP(workflows.Step):
     action_class = AssociateIPAction
-    contributes = ("ip_id", "instance_id", "ip_address")
+    contributes = ("ip_id", "instance_id", "ip_address", "port_id")
 
     def contribute(self, data, context):
         context = super(AssociateIP, self).contribute(data, context)
@@ -163,7 +169,7 @@ class IPAssociationWorkflow(workflows.Workflow):
         try:
             api.neutron.floating_ip_associate(request,
                                               data['ip_id'],
-                                              data['instance_id'])
+                                              data['port_id'])
         except neutron_exc.Conflict:
             msg = _('The requested instance port is already'
                     ' associated with another floating IP.')
diff --git a/openstack_dashboard/test/integration_tests/pages/project/network/floatingipspage.py b/openstack_dashboard/test/integration_tests/pages/project/network/floatingipspage.py
index 0a8fe9fa6..287f9cb0f 100644
--- a/openstack_dashboard/test/integration_tests/pages/project/network/floatingipspage.py
+++ b/openstack_dashboard/test/integration_tests/pages/project/network/floatingipspage.py
@@ -25,7 +25,7 @@ from openstack_dashboard.test.integration_tests.regions import tables
 class FloatingIPTable(tables.TableRegion):
     name = 'floating_ips'
     FLOATING_IP_ASSOCIATIONS = (
-        ("ip_id", "instance_id"))
+        ("ip_id", "port_id"))
 
     @tables.bind_table_action('allocate')
     def allocate_ip(self, allocate_button):
@@ -93,8 +93,8 @@ class FloatingipsPage(basepage.BaseNavigationPage):
                              instance_ip=None):
         row = self._get_row_with_floatingip(floatingip)
         floatingip_form = self.floatingips_table.associate_ip(row)
-        floatingip_form.instance_id.text = "{}: {}".format(instance_name,
-                                                           instance_ip)
+        floatingip_form.port_id.text = "{}: {}".format(instance_name,
+                                                       instance_ip)
         floatingip_form.submit()
 
     def disassociate_floatingip(self, floatingip):