From 057b4ee7a5c5a0a67f7b0a7a57a13d4033e331e2 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 30 May 2022 14:20:41 +0100 Subject: [PATCH] Simplify tests for get_(user)_template_folders This will make it easier to understand the diff when we move these tests to operate via TemplateList. Despite the verbosity, the only attribute we were actually checking here was the name, as a way of identifying which folders had been returned. Looking at the three tests: 1. The first is checking we can correctly filter all folders that a user can access, which involves appending the names of any parent folders the user doesn't have direct access to. 2. The second is checking the same thing but also that we filter the set of folders to just the children of a parent. 3. The third is just checking the filtering of child folders, without any user filtering or name aggregation applied. I've adapted tests (2) and (3) to make it clearer what is tested, focussing the tests on a specific folder and its contents. --- tests/app/models/test_service.py | 110 ++++++++++--------------------- 1 file changed, 35 insertions(+), 75 deletions(-) diff --git a/tests/app/models/test_service.py b/tests/app/models/test_service.py index 908b49037..8a7454673 100644 --- a/tests/app/models/test_service.py +++ b/tests/app/models/test_service.py @@ -82,39 +82,20 @@ def test_get_user_template_folders_only_returns_folders_visible_to_user( ): mock_get_template_folders.return_value = _get_all_folders(active_user_with_permissions) service = Service(service_one) - result = service.get_user_template_folders(User(active_user_with_permissions)) - assert result == [ - { - 'name': ["Parent 1 - invisible", "1's Visible child"], - 'id': mocker.ANY, - 'parent_id': None, - 'users_with_permission': [active_user_with_permissions['id']], - }, - { - 'name': ["Parent 1 - invisible", ["1's Invisible child", "1's Visible grandchild"]], - 'id': mocker.ANY, - 'parent_id': None, - 'users_with_permission': [active_user_with_permissions['id']], - }, - { - 'name': "2's Visible child", - 'id': mocker.ANY, - 'parent_id': VIS_PARENT_FOLDER_ID, - 'users_with_permission': [active_user_with_permissions['id']], - }, - { - 'name': ["2's Invisible child", "2's Visible grandchild"], - 'id': mocker.ANY, - 'parent_id': VIS_PARENT_FOLDER_ID, - 'users_with_permission': [active_user_with_permissions['id']], - }, - { - 'name': "Parent 2 - visible", - 'id': VIS_PARENT_FOLDER_ID, - 'parent_id': None, - 'users_with_permission': [active_user_with_permissions['id']], - }, - ] + user = User(active_user_with_permissions) + + result_folder_names = tuple( + result['name'] for result in + service.get_user_template_folders(user) + ) + + assert result_folder_names == ( + ["Parent 1 - invisible", "1's Visible child"], + ["Parent 1 - invisible", ["1's Invisible child", "1's Visible grandchild"]], + "2's Visible child", + ["2's Invisible child", "2's Visible grandchild"], + "Parent 2 - visible", + ) def test_get_template_folders_shows_user_folders_when_user_id_passed_in( @@ -126,27 +107,17 @@ def test_get_template_folders_shows_user_folders_when_user_id_passed_in( ): mock_get_template_folders.return_value = _get_all_folders(active_user_with_permissions) service = Service(service_one) - result = service.get_template_folders(user=User(active_user_with_permissions)) - assert result == [ - { - 'name': ["Parent 1 - invisible", "1's Visible child"], - 'id': mocker.ANY, - 'parent_id': None, - 'users_with_permission': [active_user_with_permissions['id']] - }, - { - 'name': ["Parent 1 - invisible", ["1's Invisible child", "1's Visible grandchild"]], - 'id': mocker.ANY, - 'parent_id': None, - 'users_with_permission': [active_user_with_permissions['id']] - }, - { - 'name': "Parent 2 - visible", - 'id': VIS_PARENT_FOLDER_ID, - 'parent_id': None, - 'users_with_permission': [active_user_with_permissions['id']] - }, - ] + user = User(active_user_with_permissions) + + result_folder_names = tuple( + result['name'] for result in + service.get_template_folders(parent_folder_id=VIS_PARENT_FOLDER_ID, user=user) + ) + + assert result_folder_names == ( + "2's Visible child", + ["2's Invisible child", "2's Visible grandchild"], + ) def test_get_template_folders_shows_all_folders_when_user_id_not_passed_in( @@ -157,27 +128,16 @@ def test_get_template_folders_shows_all_folders_when_user_id_not_passed_in( ): mock_get_template_folders.return_value = _get_all_folders(active_user_with_permissions) service = Service(service_one) - result = service.get_template_folders() - assert result == [ - { - 'name': "Invisible folder", - 'id': mocker.ANY, - 'parent_id': None, - 'users_with_permission': [] - }, - { - 'name': "Parent 1 - invisible", - 'id': INV_PARENT_FOLDER_ID, - 'parent_id': None, - 'users_with_permission': [] - }, - { - 'name': "Parent 2 - visible", - 'id': VIS_PARENT_FOLDER_ID, - 'parent_id': None, - 'users_with_permission': [active_user_with_permissions['id']], - } - ] + + result_folder_names = tuple( + result['name'] for result in + service.get_template_folders(parent_folder_id=VIS_PARENT_FOLDER_ID) + ) + + assert result_folder_names == ( + "2's Invisible child", + "2's Visible child", + ) def test_organisation_type_when_services_organisation_has_no_org_type(mocker, service_one):