Merge pull request #11914 from ansible/instances_list_filtering

add ID as default filter if no other filtering criteria is provided as well as some tests that should cover order integrity for future scenarios

awx/api/filters.py

@@ -398,11 +398,11 @@ class OrderByBackend(BaseFilterBackend):
                         order_by = value.split(',')
                     else:
                         order_by = (value,)
-            if order_by is None:
-                order_by = self.get_default_ordering(view)
+            default_order_by = self.get_default_ordering(view)
+            # glue the order by and default order by together so that the default is the backup option
+            order_by = list(order_by or []) + list(default_order_by or [])
             if order_by:
                 order_by = self._validate_ordering_fields(queryset.model, order_by)
-
                 # Special handling of the type field for ordering. In this
                 # case, we're not sorting exactly on the type field, but
                 # given the limited number of views with multiple types,

awx/api/views/__init__.py

@@ -365,6 +365,7 @@ class InstanceList(ListAPIView):
     model = models.Instance
     serializer_class = serializers.InstanceSerializer
     search_fields = ('hostname',)
+    ordering = ('id',)
 
 
 class InstanceDetail(RetrieveUpdateAPIView):

awx/main/tests/functional/api/test_inventory.py

@@ -76,6 +76,22 @@ def test_inventory_host_name_unique(scm_inventory, post, admin_user):
     assert "A Group with that name already exists." in json.dumps(resp.data)
 
 
+@pytest.mark.django_db
+def test_inventory_host_list_ordering(scm_inventory, get, admin_user):
+    # create 3 hosts, hit the inventory host list view 3 times and get the order visible there each time and compare
+    inv_src = scm_inventory.inventory_sources.first()
+    host1 = inv_src.hosts.create(name='1', inventory=scm_inventory)
+    host2 = inv_src.hosts.create(name='2', inventory=scm_inventory)
+    host3 = inv_src.hosts.create(name='3', inventory=scm_inventory)
+    expected_ids = [host1.id, host2.id, host3.id]
+    resp = get(
+        reverse('api:inventory_hosts_list', kwargs={'pk': scm_inventory.id}),
+        admin_user,
+    ).data['results']
+    host_list = [host['id'] for host in resp]
+    assert host_list == expected_ids
+
+
 @pytest.mark.django_db
 def test_inventory_group_name_unique(scm_inventory, post, admin_user):
     inv_src = scm_inventory.inventory_sources.first()
@@ -94,6 +110,24 @@ def test_inventory_group_name_unique(scm_inventory, post, admin_user):
     assert "A Host with that name already exists." in json.dumps(resp.data)
 
 
+@pytest.mark.django_db
+def test_inventory_group_list_ordering(scm_inventory, get, put, admin_user):
+    # create 3 groups, hit the inventory groups list view 3 times and get the order visible there each time and compare
+    inv_src = scm_inventory.inventory_sources.first()
+    group1 = inv_src.groups.create(name='1', inventory=scm_inventory)
+    group2 = inv_src.groups.create(name='2', inventory=scm_inventory)
+    group3 = inv_src.groups.create(name='3', inventory=scm_inventory)
+    expected_ids = [group1.id, group2.id, group3.id]
+    group_ids = {}
+    for x in range(3):
+        resp = get(
+            reverse('api:inventory_groups_list', kwargs={'pk': scm_inventory.id}),
+            admin_user,
+        ).data['results']
+        group_ids[x] = [group['id'] for group in resp]
+    assert group_ids[0] == group_ids[1] == group_ids[2] == expected_ids
+
+
 @pytest.mark.parametrize("role_field,expected_status_code", [(None, 403), ('admin_role', 200), ('update_role', 403), ('adhoc_role', 403), ('use_role', 403)])
 @pytest.mark.django_db
 def test_edit_inventory(put, inventory, alice, role_field, expected_status_code):

awx/main/tests/functional/api/test_organizations.py

@@ -71,6 +71,17 @@ def test_organization_list_integrity(organization, get, admin, alice):
         assert field in res.data['results'][0]
 
 
+@pytest.mark.django_db
+def test_organization_list_order_integrity(organizations, get, admin):
+    # check that the order of the organization list retains integrity.
+    orgs = organizations(4)
+    org_ids = {}
+    for x in range(3):
+        res = get(reverse('api:organization_list'), user=admin).data['results']
+        org_ids[x] = [org['id'] for org in res]
+    assert org_ids[0] == org_ids[1] == org_ids[2] == [orgs[0].id, orgs[1].id, orgs[2].id, orgs[3].id]
+
+
 @pytest.mark.django_db
 def test_organization_list_visibility(organizations, get, admin, alice):
     orgs = organizations(2)
@@ -127,6 +138,18 @@ def test_organization_inventory_list(organization, inventory_factory, get, alice
     get(reverse('api:organization_inventories_list', kwargs={'pk': organization.id}), user=rando, expect=403)
 
 
+@pytest.mark.django_db
+def test_organization_inventory_list_order_integrity(organization, admin, inventory_factory, get):
+    inv1 = inventory_factory('inventory')
+    inv2 = inventory_factory('inventory2')
+    inv3 = inventory_factory('inventory3')
+    inv_ids = {}
+    for x in range(3):
+        res = get(reverse('api:organization_inventories_list', kwargs={'pk': organization.id}), user=admin).data['results']
+        inv_ids[x] = [inv['id'] for inv in res]
+    assert inv_ids[0] == inv_ids[1] == inv_ids[2] == [inv1.id, inv2.id, inv3.id]
+
+
 @pytest.mark.django_db
 def test_create_organization(post, admin, alice):
     new_org = {'name': 'new org', 'description': 'my description'}

awx/main/tests/functional/test_projects.py

@@ -408,3 +408,46 @@ def test_project_delete(delete, organization, admin_user):
         ),
         admin_user,
     )
+
+
+@pytest.mark.parametrize(
+    'order_by, expected_names, expected_ids',
+    [
+        ('name', ['alice project', 'bob project', 'shared project'], [1, 2, 3]),
+        ('-name', ['shared project', 'bob project', 'alice project'], [3, 2, 1]),
+    ],
+)
+@pytest.mark.django_db
+def test_project_list_ordering_by_name(get, order_by, expected_names, expected_ids, organization_factory):
+    'ensure sorted order of project list is maintained correctly when the requested order is invalid or not applicable'
+    objects = organization_factory(
+        'org1',
+        projects=['alice project', 'bob project', 'shared project'],
+        superusers=['admin'],
+    )
+    project_names = []
+    project_ids = []
+    # TODO: ask for an order by here that doesn't apply
+    results = get(reverse('api:project_list'), objects.superusers.admin, QUERY_STRING='order_by=%s' % order_by).data['results']
+    for x in range(len(results)):
+        project_names.append(results[x]['name'])
+        project_ids.append(results[x]['id'])
+    assert project_names == expected_names and project_ids == expected_ids
+
+
+@pytest.mark.parametrize('order_by', ('name', '-name'))
+@pytest.mark.django_db
+def test_project_list_ordering_with_duplicate_names(get, order_by, organization_factory):
+    # why? because all the '1' mean that all the names are the same, you can't sort based on that,
+    # meaning you have to fall back on the default sort order, which in this case, is ID
+    'ensure sorted order of project list is maintained correctly when the project names the same'
+    objects = organization_factory(
+        'org1',
+        projects=['1', '1', '1', '1', '1'],
+        superusers=['admin'],
+    )
+    project_ids = {}
+    for x in range(3):
+        results = get(reverse('api:project_list'), objects.superusers.admin, QUERY_STRING='order_by=%s' % order_by).data['results']
+        project_ids[x] = [proj['id'] for proj in results]
+    assert project_ids[0] == project_ids[1] == project_ids[2] == [1, 2, 3, 4, 5]