From df5ccae4c5743258c94c218b893038f9aaa2946c Mon Sep 17 00:00:00 2001 From: David McDonald Date: Fri, 15 May 2020 17:34:30 +0100 Subject: [PATCH 1/2] Add in positive logging case for purge command This is useful so we can see that it's doing things, which case is being hit and know that an empty terminal for an hour isn't a bad thing --- app/commands.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/commands.py b/app/commands.py index b120838cd..14692c86d 100644 --- a/app/commands.py +++ b/app/commands.py @@ -119,9 +119,11 @@ def purge_functional_test_data(user_email_prefix): else: services = dao_fetch_all_services_by_user(usr.id) if services: + print(f"Deleting user {usr.id} which has related services") for service in services: delete_service_and_all_associated_db_objects(service) else: + print(f"Deleting user {usr.id} which does not have related services") delete_user_verify_codes(usr) delete_model_user(usr) From 2f0b3a96364bdaaffdc9dac68a38ef3dd6461ba2 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 18 May 2020 10:30:28 +0100 Subject: [PATCH 2/2] Fix edge case in func test data purging for created_by_id When running the purge command I found about 4 users who could not be deleted because their user id was still referenced in the services table as they had created the service yet they were not a member of that service anymore. I have fixed this by checking that if they are not a member but created the service then we also delete the service for them. Note, I've followed the previous convention of no tests for this function. I've run it locally and executed the code path so there should be no major flaws in the code. There is a small chance I wasn't able to exactly replicate the state that existed in preview on my local but hopefully it was close enough to be accurate. --- app/commands.py | 14 ++++++++++++-- app/dao/services_dao.py | 10 ++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/app/commands.py b/app/commands.py index 14692c86d..766bc9f2d 100644 --- a/app/commands.py +++ b/app/commands.py @@ -38,6 +38,7 @@ from app.dao.service_callback_api_dao import get_service_delivery_status_callbac from app.dao.services_dao import ( delete_service_and_all_associated_db_objects, dao_fetch_all_services_by_user, + dao_fetch_all_services_created_by_user, dao_fetch_service_by_id, dao_update_service ) @@ -119,11 +120,20 @@ def purge_functional_test_data(user_email_prefix): else: services = dao_fetch_all_services_by_user(usr.id) if services: - print(f"Deleting user {usr.id} which has related services") + print(f"Deleting user {usr.id} which is part of services") for service in services: delete_service_and_all_associated_db_objects(service) else: - print(f"Deleting user {usr.id} which does not have related services") + services_created_by_this_user = dao_fetch_all_services_created_by_user(usr.id) + if services_created_by_this_user: + # user is not part of any services but may still have been the one to create the service + # sometimes things get in this state if the tests fail half way through + # Remove the service they created (but are not a part of) so we can then remove the user + print(f"Deleting services created by {usr.id}") + for service in services_created_by_this_user: + delete_service_and_all_associated_db_objects(service) + + print(f"Deleting user {usr.id} which is not part of any services") delete_user_verify_codes(usr) delete_model_user(usr) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index cf2541f60..8a5e35101 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -234,6 +234,16 @@ def dao_fetch_all_services_by_user(user_id, only_active=False): return query.all() +def dao_fetch_all_services_created_by_user(user_id): + query = Service.query.filter_by( + created_by_id=user_id + ).order_by( + asc(Service.created_at) + ) + + return query.all() + + @transactional @version_class( VersionOptions(ApiKey, must_write_history=False),