Merge pull request #47 from cfpb/46_delete_institution_bug

46 delete institution bug

src/regtech_cleanup_api/entities/repos/institution_repo.py

@@ -30,7 +30,7 @@ def delete_sbl_type_by_lei(session: Session, lei: str) -> List[SblTypeMappingDao
         logger.warning(f"No Domain(s) for LEI {lei} found.")
 
 
-def delete_institution(session: Session, lei: str) -> FinancialInstitutionDao | None:
+def delete_institution(session: Session, lei: str) -> dict[str, bool] | None:
     stmt = session.query(FinancialInstitutionDao).filter(FinancialInstitutionDao.lei == lei)
     fi = session.execute(stmt)
 
@@ -40,5 +40,6 @@ def delete_institution(session: Session, lei: str) -> FinancialInstitutionDao |
         del_hist_stmt = text("DELETE from financial_institutions_history where lei = :lei")
         session.execute(del_hist_stmt, {"lei": lei})
         session.commit()
+        return {"institution_removed": True}
     else:
         logger.warning(f"No sbl type(s) for LEI {lei} found.")

src/regtech_cleanup_api/routers/institution_cleanup.py

@@ -47,14 +47,13 @@ def delete_institution(request: Request, lei: str):
 
 
 def delete_helper(lei: str, session: Session):
-
     try:
         repo.delete_domains_by_lei(session, lei)
     except Exception as e:
         raise RegTechHttpException(
             status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
             name="Domains Delete Failed",
-            detail="Failed to delete domains",
+            detail=f"Failed to delete domains for LEI {lei}",
         ) from e
 
     try:
@@ -63,7 +62,7 @@ def delete_helper(lei: str, session: Session):
         raise RegTechHttpException(
             status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
             name="Sbl Type Delete Failed",
-            detail="Failed to delete sbl_types",
+            detail=f"Failed to delete sbl_types for LEI {lei}",
         ) from e
 
     res = repo.delete_institution(session, lei)

tests/api/routers/test_filing_cleanup.py

@@ -27,8 +27,10 @@ def test_delete_filing(app_fixture: FastAPI, mocker: MockerFixture, authed_user_
     # Test with errors
     delete_helper_mock.side_effect = IOError("Test")
     res = client.delete("/v1/cleanup/filing/123456E2ETESTBANK123/2024")
+    res_text = json.loads(res.text)
     assert res.status_code == 500
-    assert json.loads(res.text)["error_name"] == "Delete Filing Server Error"
+    assert res_text["error_name"] == "Delete Filing Server Error"
+    assert res_text["error_detail"] == "Server error while trying to delete filing for LEI 123456E2ETESTBANK123."
 
 
 def test_filing_delete_helper(app_fixture: FastAPI, mocker: MockerFixture):
@@ -63,6 +65,7 @@ def test_filing_delete_helper(app_fixture: FastAPI, mocker: MockerFixture):
         delete_helper("123456E2ETESTBANK123", "2024", session_mock)
     assert isinstance(e.value, RegTechHttpException)
     assert e.value.name == "Contact Info Delete Failed"
+    assert e.value.detail == "Failed to delete contact info for LEI 123456E2ETESTBANK123"
 
     # Get User Action IDs Fail
     delete_contact_info_mock.side_effect = None
@@ -71,6 +74,7 @@ def test_filing_delete_helper(app_fixture: FastAPI, mocker: MockerFixture):
         delete_helper("123456E2ETESTBANK123", "2024", session_mock)
     assert isinstance(e.value, RegTechHttpException)
     assert e.value.name == "Missing User Action Data"
+    assert e.value.detail == "Failed to get user action data for LEI 123456E2ETESTBANK123"
 
     # Delete Submissions Fail
     user_action_ids_mock.side_effect = None
@@ -79,6 +83,7 @@ def test_filing_delete_helper(app_fixture: FastAPI, mocker: MockerFixture):
         delete_helper("123456E2ETESTBANK123", "2024", session_mock)
     assert isinstance(e.value, RegTechHttpException)
     assert e.value.name == "Submission Delete Failed"
+    assert e.value.detail == "Failed to delete submission data for LEI 123456E2ETESTBANK123"
 
     # Delete Filing Fail
     delete_submissions_mock.side_effect = None
@@ -87,6 +92,7 @@ def test_filing_delete_helper(app_fixture: FastAPI, mocker: MockerFixture):
         delete_helper("123456E2ETESTBANK123", "2024", session_mock)
     assert isinstance(e.value, RegTechHttpException)
     assert e.value.name == "Filing Delete Failed"
+    assert e.value.detail == "Failed to delete filing data for LEI 123456E2ETESTBANK123"
 
     # Delete User Actions Fail
     delete_filing_mock.side_effect = None
@@ -95,6 +101,7 @@ def test_filing_delete_helper(app_fixture: FastAPI, mocker: MockerFixture):
         delete_helper("123456E2ETESTBANK123", "2024", session_mock)
     assert isinstance(e.value, RegTechHttpException)
     assert e.value.name == "User Action Delete Failed"
+    assert e.value.detail == "Failed to delete user action data for LEI 123456E2ETESTBANK123"
 
 
 def test_unauthed_delete_submissions(app_fixture: FastAPI):
@@ -127,19 +134,25 @@ def test_delete_submissions(app_fixture: FastAPI, mocker: MockerFixture, authed_
     # Get User Action IDs fail
     user_action_ids_mock.side_effect = test_error
     res = client.delete("/v1/cleanup/filing/submissions/123456E2ETESTBANK123/2024")
+    res_text = json.loads(res.text)
     assert res.status_code == 500
-    assert json.loads(res.text)["error_name"] == "Missing User Action Data"
+    assert res_text["error_name"] == "Missing User Action Data"
+    assert res_text["error_detail"] == "Failed to get user action data for LEI 123456E2ETESTBANK123"
 
     # Delete Submissions Fail
     user_action_ids_mock.side_effect = None
     delete_submissions_mock.side_effect = test_error
     res = client.delete("/v1/cleanup/filing/submissions/123456E2ETESTBANK123/2024")
+    res_text = json.loads(res.text)
     assert res.status_code == 500
-    assert json.loads(res.text)["error_name"] == "Submission Delete Failed"
+    assert res_text["error_name"] == "Submission Delete Failed"
+    assert res_text["error_detail"] == "Failed to delete submission data for LEI 123456E2ETESTBANK123"
 
     # Delete User Actions Fail
     delete_submissions_mock.side_effect = None
     delete_user_actions_mock.side_effect = test_error
     res = client.delete("/v1/cleanup/filing/submissions/123456E2ETESTBANK123/2024")
+    res_text = json.loads(res.text)
     assert res.status_code == 500
-    assert json.loads(res.text)["error_name"] == "User Action Delete Failed"
+    assert res_text["error_name"] == "User Action Delete Failed"
+    assert res_text["error_detail"] == "Failed to delete user action data for LEI 123456E2ETESTBANK123"

tests/api/routers/test_institution_cleanup.py

@@ -27,13 +27,12 @@ def test_delete_institutions(app_fixture: FastAPI, mocker: MockerFixture, authed
 
 def test_institution_delete_helper(app_fixture: FastAPI, mocker: MockerFixture, caplog):
     session_mock = Mock()
-    ok_res = {"OK": True}
     delete_domains_mock = mocker.patch("regtech_cleanup_api.routers.institution_cleanup.repo.delete_domains_by_lei")
-    delete_domains_mock.return_value = ok_res
+    delete_domains_mock.return_value = None
     delete_sbl_type_mock = mocker.patch("regtech_cleanup_api.routers.institution_cleanup.repo.delete_sbl_type_by_lei")
-    delete_sbl_type_mock.return_value = ok_res
+    delete_sbl_type_mock.return_value = None
     delete_institution_mock = mocker.patch("regtech_cleanup_api.routers.institution_cleanup.repo.delete_institution")
-    delete_institution_mock.return_value = ok_res
+    delete_institution_mock.return_value = {"institution_removed": True}
     delete_group_mock = mocker.patch("regtech_cleanup_api.routers.institution_cleanup.oauth2_admin.delete_group")
     delete_group_mock.return_value = {"123456E2ETESTBANK123": "test"}
 
@@ -45,28 +44,36 @@ def test_institution_delete_helper(app_fixture: FastAPI, mocker: MockerFixture,
     delete_group_mock.assert_called_once_with("123456E2ETESTBANK123")
 
     # Delete Domains Fail
-    delete_domains_mock.return_value = None
-    delete_helper("123456E2ETESTBANK123", session_mock)
-    assert "Domain(s) for LEI 123456E2ETESTBANK123 not deleted." in caplog.text
+    delete_domains_mock.side_effect = IOError("Test")
+    with pytest.raises(Exception) as e:
+        delete_helper("123456E2ETESTBANK123", session_mock)
+    assert isinstance(e.value, RegTechHttpException)
+    assert e.value.name == "Domains Delete Failed"
+    assert e.value.detail == "Failed to delete domains for LEI 123456E2ETESTBANK123"
 
     # Delete SBL Type Fail
-    delete_domains_mock.return_value = ok_res
-    delete_sbl_type_mock.return_value = None
-    delete_helper("123456E2ETESTBANK123", session_mock)
-    assert "sbl type(s) for LEI 123456E2ETESTBANK123 not deleted." in caplog.text
+    delete_domains_mock.side_effect = None
+    delete_sbl_type_mock.side_effect = IOError("Test")
+    with pytest.raises(Exception) as e:
+        delete_helper("123456E2ETESTBANK123", session_mock)
+    assert isinstance(e.value, RegTechHttpException)
+    assert e.value.name == "Sbl Type Delete Failed"
+    assert e.value.detail == "Failed to delete sbl_types for LEI 123456E2ETESTBANK123"
 
     # Delete Institution Fail
-    delete_sbl_type_mock.return_value = ok_res
+    delete_sbl_type_mock.side_effect = None
     delete_institution_mock.return_value = None
     with pytest.raises(Exception) as e:
         delete_helper("123456E2ETESTBANK123", session_mock)
     assert isinstance(e.value, RegTechHttpException)
-    assert e.value.name == "Institution to be deleted Not Found"
+    assert e.value.name == "Institution Delete Failed"
+    assert e.value.detail == "Institution LEI 123456E2ETESTBANK123 not found."
 
     # Delete Group Fail
-    delete_institution_mock.return_value = ok_res
+    delete_institution_mock.return_value = {"institution_removed": True}
     delete_group_mock.side_effect = IOError("test")
     with pytest.raises(Exception) as e:
         delete_helper("123456E2ETESTBANK123", session_mock)
     assert isinstance(e.value, RegTechHttpException)
-    assert e.value.name == "Group Not Found"
+    assert e.value.name == "Group Delete Failed"
+    assert e.value.detail == "The group associated with LEI 123456E2ETESTBANK123 not found."