From f749a5d44d1402e0f8021b913dc98bfdc2c9b05b Mon Sep 17 00:00:00 2001
From: AlanCoding <arominge@redhat.com>
Date: Wed, 13 Feb 2019 12:23:35 -0500
Subject: [PATCH] Surface empty groups as children of all group

---
 awx/main/models/inventory.py                  | 32 +++++++++++--------
 .../tests/functional/models/test_inventory.py | 25 +++++++++++----
 2 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py
index df04d5b1ad..781ed9b70b 100644
--- a/awx/main/models/inventory.py
+++ b/awx/main/models/inventory.py
@@ -289,23 +289,27 @@ class Inventory(CommonModelNameNotUnique, ResourceMixin, RelatedJobsMixin):
                 group_children = group_children_map.setdefault(to_group_id, [])
                 group_children.append(from_group_name)
 
-            # Now use in-memory maps to build up group info.
-            for group in self.groups.only('name', 'id', 'variables'):
-                group_info = dict()
-                group_info['hosts'] = group_hosts_map.get(group.id, [])
-                group_info['children'] = group_children_map.get(group.id, [])
-                group_info['vars'] = group.variables_dict
-                data[group.name] = group_info
-
             # Add ungrouped hosts to all group
             all_group['hosts'] = [host.name for host in hosts if host.name not in grouped_hosts]
 
-        # Remove any empty groups
-        for group_name in list(data.keys()):
-            if group_name == 'all':
-                continue
-            if not (data.get(group_name, {}).get('hosts', []) or data.get(group_name, {}).get('children', [])):
-                data.pop(group_name)
+            # Now use in-memory maps to build up group info.
+            all_group_names = []
+            for group in self.groups.only('name', 'id', 'variables'):
+                group_info = dict()
+                if group.id in group_hosts_map:
+                    group_info['hosts'] = group_hosts_map[group.id]
+                if group.id in group_children_map:
+                    group_info['children'] = group_children_map[group.id]
+                group_vars = group.variables_dict
+                if group_vars:
+                    group_info['vars'] = group_vars
+                if group_info:
+                    data[group.name] = group_info
+                all_group_names.append(group.name)
+
+            # add all groups as children of all group, includes empty groups
+            if all_group_names:
+                all_group['children'] = all_group_names
 
         if hostvars:
             data.setdefault('_meta', dict())
diff --git a/awx/main/tests/functional/models/test_inventory.py b/awx/main/tests/functional/models/test_inventory.py
index 1519c3b28e..b970bc34fc 100644
--- a/awx/main/tests/functional/models/test_inventory.py
+++ b/awx/main/tests/functional/models/test_inventory.py
@@ -43,13 +43,28 @@ class TestInventoryScript:
         data = inventory.get_script_data()
         assert 'all' in data
         assert data['all'] == {
-            'hosts': [],
-            'children': [],
             'vars': {
                 'a1': 'a1'
             }
         }
 
+    def test_empty_group(self, inventory):
+        inventory.groups.create(name='ghost')
+        data = inventory.get_script_data()
+        # canonical behavior same as ansible-inventory
+        # group not provided top-level to avoid host / group confusion
+        # still list group as a child of the all group
+        assert 'ghost' not in data
+        assert 'ghost' in data['all']['children']
+
+    def test_empty_group_with_vars(self, inventory):
+        inventory.groups.create(name='ghost2', variables={'foo': 'bar'})
+        data = inventory.get_script_data()
+        # must be top-level key so group vars can be provided
+        assert 'ghost2' in data
+        assert data['ghost2']['vars'] == {'foo': 'bar'}
+        assert 'ghost2' in data['all']['children']
+
     def test_grandparent_group(self, inventory):
         g1 = inventory.groups.create(name='g1', variables={'v1': 'v1'})
         g2 = inventory.groups.create(name='g2', variables={'v2': 'v2'})
@@ -62,13 +77,11 @@ class TestInventoryScript:
         assert 'g1' in data
         assert 'g2' in data
         assert data['g1'] == {
-            'hosts': [],
             'children': ['g2'],
             'vars': {'v1': 'v1'}
         }
         assert data['g2'] == {
             'hosts': ['h1'],
-            'children': [],
             'vars': {'v2': 'v2'}
         }
 
@@ -93,10 +106,10 @@ class TestInventoryScript:
             g2.hosts.add(host)
         for i in range(3):
             expected_data = {
-                'contains_all_hosts': {'hosts': ['host{}'.format(i)], 'children': [], 'vars': {}},
+                'contains_all_hosts': {'hosts': ['host{}'.format(i)]},
             }
             if i < 2:
-                expected_data['contains_two_hosts'] = {'hosts': ['host{}'.format(i)], 'children': [], 'vars': {}}
+                expected_data['contains_two_hosts'] = {'hosts': ['host{}'.format(i)]}
             data = inventory.get_script_data(slice_number=i + 1, slice_count=3)
             data.pop('all')
             assert data == expected_data