Handle empty expiration date and make id required in admin banner update cmd (#868)

* Handle empty expiration date

* Makinf id param mandatory for banner update command

* Fixing back compat check

* Minor

* minor refactoring

Author: ishitam8

azure-devops/azext_devops/dev/admin/banner.py

@@ -64,7 +64,7 @@ def banner_add(message, banner_type=None, id=None, expiration=None, organization
     return {id: entries[setting_key]}
 
 
-def banner_update(message=None, banner_type=None, id=None, expiration=None, organization=None, detect=None):  # pylint: disable=redefined-builtin
+def banner_update(id, message=None, banner_type=None, expiration=None, organization=None, detect=None):  # pylint: disable=redefined-builtin
     """Update the message, level, or expiration date for a banner.
     :param message: Message (string) to show in the banner.
     :type message: str
@@ -81,10 +81,10 @@ def banner_update(message=None, banner_type=None, id=None, expiration=None, orga
     if message is None and banner_type is None and expiration is None:
         raise ValueError('At least one of the following arguments need to be supplied: --message, --type, '
                          '--expiration.')
-    if expiration is not None:
+    if expiration is not None and expiration != '':
         expiration_iso8601 = convert_date_string_to_iso8601(value=expiration, argument='expiration')
     else:
-        expiration_iso8601 = None
+        expiration_iso8601 = expiration
     existing_entries = setting_list(user_scope='host',
                                     key=GLOBAL_MESSAGE_BANNERS_KEY,
                                     organization=organization,

scripts/backCompatChecker.py

@@ -16,6 +16,7 @@
 
 
 allowedNewMandatoryArguments = {}
+allowedNewMandatoryArguments['devops admin banner update'] = ['--id']
 
 
 # Do not compare these commands

tests/test_adminBannerTest.py

@@ -6,6 +6,7 @@
 import os
 import unittest
 
+from datetime import datetime
 from azure_devtools.scenario_tests import AllowLargeResponse
 from .utilities.helper import DevopsScenarioTest, disable_telemetry, set_authentication, get_test_org_from_env_variable
 
@@ -23,26 +24,33 @@ def test_admin_banner_addUpdateShowListRemove(self):
         admin_banner_updated_message = 'Sample updated banner message'
         admin_banner_updated_type = 'error'
         admin_banner_id = self.create_random_name(prefix='banner-id-', length=15)
+        admin_banner_expiration_date = datetime.today().strftime('%Y-%m-%d')
 
         try:
             #add a banner to the project
             add_admin_banner_command = ('az devops admin banner add --id ' + admin_banner_id + ' --message "' + admin_banner_message + '" --type ' + admin_banner_type + 
+                ' --expiration ' + admin_banner_expiration_date +
                 ' --output json --detect false --debug')
             add_admin_banner_output = self.cmd(add_admin_banner_command).get_output_in_json()
             assert len(add_admin_banner_output) > 0
             assert add_admin_banner_output[admin_banner_id]["level"] == admin_banner_type
             assert add_admin_banner_output[admin_banner_id]["message"] == admin_banner_message
+            from azext_devops.dev.common.arguments import convert_date_string_to_iso8601
+            iso_date = convert_date_string_to_iso8601(admin_banner_expiration_date)
+            assert add_admin_banner_output[admin_banner_id]["expirationDate"] == iso_date
 
             #Test was failing without adding a sleep here. Though the create was successful when queried after few seconds. 
             self.sleep_in_live_run(5)
 
             #update banner 
             update_admin_banner_command = ('az devops admin banner update --id ' + admin_banner_id + ' --message "' + admin_banner_updated_message + 
-                '" --type ' + admin_banner_updated_type + ' --output json --detect false')
+                '" --expiration ' + '""' +
+                ' --type ' + admin_banner_updated_type + ' --output json --detect false')
             update_admin_banner_output = self.cmd(update_admin_banner_command).get_output_in_json()
             assert len(update_admin_banner_output[admin_banner_id]) > 0
             assert update_admin_banner_output[admin_banner_id]["level"] == admin_banner_updated_type
             assert update_admin_banner_output[admin_banner_id]["message"] == admin_banner_updated_message
+            assert update_admin_banner_output[admin_banner_id]["expirationDate"] == ''
 
             #Test was failing without adding a sleep here. Though the update was successful when queried after few seconds. 
             self.sleep_in_live_run(5)