diff --git a/awx/api/filters.py b/awx/api/filters.py index 10fc488006..90f499d671 100644 --- a/awx/api/filters.py +++ b/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, diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index 7c840d1f21..58b221fcac 100644 --- a/awx/api/views/__init__.py +++ b/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): diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index fb4cc19cda..5bb80bdbbd 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/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): diff --git a/awx/main/tests/functional/api/test_organizations.py b/awx/main/tests/functional/api/test_organizations.py index 3db0e619fd..f86963ecc6 100644 --- a/awx/main/tests/functional/api/test_organizations.py +++ b/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'} diff --git a/awx/main/tests/functional/test_projects.py b/awx/main/tests/functional/test_projects.py index b8471fda5d..0459aaab49 100644 --- a/awx/main/tests/functional/test_projects.py +++ b/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]