Refactor get_configdocs_status Method

This is a follow-up patch set for [0] to address the comments
to break up the get_configdocs_status method into smaller
methods.

Also made a minor update to the check_reformat_versions method
based on comments in [0]

[0] https://review.gerrithub.io/#/c/att-comdev/shipyard/+/409839/

Change-Id: Id13fcaa25e48cc2f4267a08db1ccb4561b260197

src/bin/shipyard_airflow/shipyard_airflow/control/helpers/configdocs_helper.py

@@ -176,78 +176,12 @@ class ConfigdocsHelper(object):
         """
         configdocs_status = []
 
-        # Default ordering
-        def_order = [SUCCESSFUL_SITE_ACTION,
-                     LAST_SITE_ACTION,
-                     COMMITTED,
-                     BUFFER]
+        # Get ordered versions
+        ordered_versions = self._get_ordered_versions(versions)
 
-        # Defaults to COMMITTED and BUFFER
-        if versions is None:
-            versions = [COMMITTED, BUFFER]
-
-        elif not len(versions) == 2:
-            raise AppError(
-                title='Incorrect number of versions for comparison',
-                description=(
-                    'User must pass in 2 valid versions for comparison'),
-                status=falcon.HTTP_400,
-                retry=False)
-
-        elif versions[0] == versions[1]:
-            raise AppError(
-                title='Versions must be different for comparison',
-                description=(
-                    'Versions must be unique in order to perform comparison'),
-                status=falcon.HTTP_400,
-                retry=False)
-
-        for version in versions:
-            if version not in def_order:
-                raise AppError(
-                    title='Invalid version detected',
-                    description=(
-                        '{} is not a valid version, which include: '
-                        '{}'.format(version, ', '.join(def_order))),
-                    status=falcon.HTTP_400,
-                    retry=False)
-
-        # Higher index in the def_order list will mean that it is a newer
-        # version. We will swap the order and sort the version if need be.
-        if def_order.index(versions[0]) > def_order.index(versions[1]):
-            sorted_versions = list(reversed(versions))
-        else:
-            sorted_versions = versions
-
-        # Get version id
-        old_version_id = self.get_revision_id(sorted_versions[0])
-        new_version_id = self.get_revision_id(sorted_versions[1])
-
-        # Get revision name
-        old_version_name = sorted_versions[0]
-        new_version_name = sorted_versions[1]
-
-        # Check that revision id of LAST_SITE_ACTION and SUCCESSFUL_SITE_ACTION
-        # is not None
-        for name, rev_id in [(old_version_name, old_version_id),
-                             (new_version_name, new_version_id)]:
-            if (name in [LAST_SITE_ACTION, SUCCESSFUL_SITE_ACTION] and
-                    rev_id is None):
-                raise AppError(
-                    title='Version does not exist',
-                    description='{} version does not exist'.format(name),
-                    status=falcon.HTTP_404,
-                    retry=False)
-
-        # Set to 0 if there is no committed version
-        if old_version_name == COMMITTED and old_version_id is None:
-            old_version_id = 0
-            new_version_id = self.get_revision_id(BUFFER) or 0
-
-        # Set new_version_id if None
-        if new_version_id is None:
-            new_version_id = (
-                self.get_revision_id(BUFFER) or old_version_id or 0)
+        # Get version name and id
+        old_version_name, new_version_name, old_version_id, new_version_id = (
+            self._get_versions_name_id(ordered_versions))
 
         try:
             diff = self.deckhand.get_diff(
@@ -873,3 +807,86 @@ def _format_validations_to_status(val_msgs, error_count):
         },
         "code": code
     }
+
+    def _get_ordered_versions(self, versions=None):
+        """returns a list of ordered versions"""
+
+        # Default ordering
+        def_order = [SUCCESSFUL_SITE_ACTION,
+                     LAST_SITE_ACTION,
+                     COMMITTED,
+                     BUFFER]
+
+        # Defaults to COMMITTED and BUFFER
+        if versions is None:
+            versions = [COMMITTED, BUFFER]
+
+        elif not len(versions) == 2:
+            raise AppError(
+                title='Incorrect number of versions for comparison',
+                description=(
+                    'User must pass in 2 valid versions for comparison'),
+                status=falcon.HTTP_400,
+                retry=False)
+
+        elif versions[0] == versions[1]:
+            raise AppError(
+                title='Versions must be different for comparison',
+                description=(
+                    'Versions must be unique in order to perform comparison'),
+                status=falcon.HTTP_400,
+                retry=False)
+
+        for version in versions:
+            if version not in def_order:
+                raise AppError(
+                    title='Invalid version detected',
+                    description=(
+                        '{} is not a valid version, which include: '
+                        '{}'.format(version, ', '.join(def_order))),
+                    status=falcon.HTTP_400,
+                    retry=False)
+
+        # Higher index in the def_order list will mean that it is a newer
+        # version. We will swap the order and sort the version if need be.
+        if def_order.index(versions[0]) > def_order.index(versions[1]):
+            ordered_versions = list(reversed(versions))
+        else:
+            ordered_versions = versions
+
+        return ordered_versions
+
+    def _get_versions_name_id(self, ordered_versions):
+
+        # Get version id
+        old_version_id = self.get_revision_id(ordered_versions[0])
+        new_version_id = self.get_revision_id(ordered_versions[1])
+
+        # Get revision name
+        old_version_name = ordered_versions[0]
+        new_version_name = ordered_versions[1]
+
+        # Check that revision id of LAST_SITE_ACTION and SUCCESSFUL_SITE_ACTION
+        # is not None
+        for name, rev_id in [(old_version_name, old_version_id),
+                             (new_version_name, new_version_id)]:
+            if (name in [LAST_SITE_ACTION, SUCCESSFUL_SITE_ACTION] and
+                    rev_id is None):
+                raise AppError(
+                    title='Version does not exist',
+                    description='{} version does not exist'.format(name),
+                    status=falcon.HTTP_404,
+                    retry=False)
+
+        # Set to 0 if there is no committed version
+        if old_version_name == COMMITTED and old_version_id is None:
+            old_version_id = 0
+            new_version_id = self.get_revision_id(BUFFER) or 0
+
+        # Set new_version_id if None
+        if new_version_id is None:
+            new_version_id = (
+                self.get_revision_id(BUFFER) or old_version_id or 0)
+
+        return (
+            old_version_name, new_version_name, old_version_id, new_version_id)

src/bin/shipyard_airflow/tests/unit/control/test_configdocs_helper.py

@@ -60,6 +60,9 @@ REV_BUFFER_DICT = {
 
 DIFF_BUFFER_DICT = {'mop': 'unmodified', 'chum': 'created', 'slop': 'deleted'}
 
+ORDERED_VER = ['committed', 'buffer']
+REV_NAME_ID = ('committed', 'buffer', 3, 5)
+
 REV_BUFF_EMPTY_DICT = {
     'committed': {
         'id': 3,
@@ -297,6 +300,8 @@ def test_is_buffer_valid_for_bucket():
 def test_get_configdocs_status():
     helper = ConfigdocsHelper(CTX)
     helper._get_revision_dict = lambda: REV_BUFFER_DICT
+    helper._get_ordered_versions = lambda versions: ORDERED_VER
+    helper._get_versions_name_id = lambda ordered_versions: REV_NAME_ID
     helper.deckhand.get_diff = (
         lambda old_revision_id, new_revision_id: DIFF_BUFFER_DICT)
     result = helper.get_configdocs_status()

src/bin/shipyard_client/shipyard_client/cli/input_checks.py

@@ -89,7 +89,7 @@ def check_reformat_versions(ctx, buffer, committed, last_site_action,
         versions.append('successful_site_action')
 
     if len(versions) == 0:
-        return None
+        return ['committed', 'buffer']
 
     elif len(versions) == 2:
         return versions

src/bin/shipyard_client/tests/unit/cli/get/test_get_commands.py

@@ -66,7 +66,7 @@ def test_get_configdocs_without_passing_collection(*args):
     runner = CliRunner()
     with patch.object(GetConfigdocsStatus, '__init__') as mock_method:
         runner.invoke(shipyard, [auth_vars, 'get', 'configdocs'])
-    mock_method.assert_called_once_with(ANY, None)
+    mock_method.assert_called_once_with(ANY, ['committed', 'buffer'])
 
 
 def test_get_configdocs_negative(*args):