From a040c02a911ab3e880575761736258e5afce190d Mon Sep 17 00:00:00 2001
From: pengyuesheng <pengyuesheng@gohighsec.com>
Date: Wed, 3 Apr 2019 16:48:15 +0800
Subject: [PATCH] Do not call handle() when AZ is required

On "Edit Host Aggregation" form under Host Aggregates panel,
if the availability zone is cleared, both the error message and
the success message will appear.

In the aggregate API, nova does not allow to clear AZ once
it is set. This commit moves the check for availability_zone field
to the form definition. handle() method is now never called for
such case and the error message will be shown around the field.

Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>
Change-Id: I74e03a47cee6915e9c24c6cf5c3264bf8b0e39b8
Closes-Bug: #1822971
---
 .../dashboards/admin/aggregates/forms.py      | 33 ++++++++++++-------
 .../dashboards/admin/aggregates/tests.py      | 16 +++++++--
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/openstack_dashboard/dashboards/admin/aggregates/forms.py b/openstack_dashboard/dashboards/admin/aggregates/forms.py
index bf636b29a3..ac238a39c7 100644
--- a/openstack_dashboard/dashboards/admin/aggregates/forms.py
+++ b/openstack_dashboard/dashboards/admin/aggregates/forms.py
@@ -23,26 +23,35 @@ INDEX_URL = constants.AGGREGATES_INDEX_URL
 
 
 class UpdateAggregateForm(forms.SelfHandlingForm):
+    use_required_attribute = False
+
     name = forms.CharField(label=_("Name"),
                            max_length=255)
-    availability_zone = forms.CharField(label=_("Availability Zone"),
-                                        required=False,
-                                        max_length=255)
+    availability_zone = forms.CharField(
+        label=_("Availability Zone"),
+        max_length=255,
+        # This message is used when the initial value is non-empty.
+        # Once AZ is set, nova API does not allow us to clear it.
+        error_messages={
+            'required': _("The new availability zone can't be empty"),
+        },
+    )
+
+    def __init__(self, request, *args, **kwargs):
+        super(UpdateAggregateForm, self).__init__(request, *args, **kwargs)
+        old_availability_zone = self.initial['availability_zone']
+        if not old_availability_zone:
+            self.fields['availability_zone'].required = False
 
     def handle(self, request, data):
         id = self.initial['id']
-        old_availability_zone = self.initial['availability_zone']
         name = data['name']
         availability_zone = data['availability_zone']
+
         aggregate = {'name': name}
-        try:
-            if availability_zone:
-                aggregate['availability_zone'] = availability_zone
-            elif old_availability_zone:
-                raise ValueError
-        except Exception:
-            exceptions.handle(request,
-                              _('The new availability zone can\'t be empty'))
+        if availability_zone:
+            aggregate['availability_zone'] = availability_zone
+
         try:
             api.nova.aggregate_update(request, id, aggregate)
             message = (_('Successfully updated aggregate: "%s."')
diff --git a/openstack_dashboard/dashboards/admin/aggregates/tests.py b/openstack_dashboard/dashboards/admin/aggregates/tests.py
index 15da9e4eb2..c93f3d2c29 100644
--- a/openstack_dashboard/dashboards/admin/aggregates/tests.py
+++ b/openstack_dashboard/dashboards/admin/aggregates/tests.py
@@ -266,13 +266,25 @@ class AggregatesViewTests(test.BaseAdminViewTests):
 
         self._test_generic_update_aggregate(form_data, aggregate)
 
-    def test_update_aggregate_fails_missing_fields(self):
+    def test_update_aggregate_fails_missing_name_field(self):
         aggregate = self.aggregates.first()
-        form_data = {'id': aggregate.id}
+        form_data = {'id': aggregate.id,
+                     'name': '',
+                     'availability_zone': aggregate.availability_zone}
 
         self._test_generic_update_aggregate(form_data, aggregate, 1,
                                             u'This field is required')
 
+    def test_update_aggregate_fails_missing_az_field(self):
+        aggregate = self.aggregates.first()
+        form_data = {'id': aggregate.id,
+                     'name': aggregate.name,
+                     'availability_zone': ''}
+
+        self._test_generic_update_aggregate(
+            form_data, aggregate, 1,
+            u'The new availability zone can&#39;t be empty')
+
 
 class ManageHostsTests(test.BaseAdminViewTests):