From 7c61b319c382fba21afcbee2eff9b8f0e57ea98a Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Wed, 22 May 2024 10:13:41 -0700 Subject: [PATCH 1/5] safe list delete endpoint Signed-off-by: Shaanjot Gill --- notify-api/src/notify_api/models/safe_list.py | 5 +++++ .../src/notify_api/resources/v2/safe_list.py | 17 +++++++++++++++++ .../tests/unit/resources/v2/test_safe_list.py | 13 +++++++++++++ 3 files changed, 35 insertions(+) diff --git a/notify-api/src/notify_api/models/safe_list.py b/notify-api/src/notify_api/models/safe_list.py index 59d6871b..b983e4d5 100644 --- a/notify-api/src/notify_api/models/safe_list.py +++ b/notify-api/src/notify_api/models/safe_list.py @@ -71,3 +71,8 @@ def find_all(cls) -> List[SafeList]: """Return all of the safe emails.""" safe_emails = cls.query.all() return safe_emails + + def delete_email(self): + """delete email from safe list.""" + db.session.delete(self) + db.session.commit() diff --git a/notify-api/src/notify_api/resources/v2/safe_list.py b/notify-api/src/notify_api/resources/v2/safe_list.py index ba3ee1e8..9fc9b5ed 100644 --- a/notify-api/src/notify_api/resources/v2/safe_list.py +++ b/notify-api/src/notify_api/resources/v2/safe_list.py @@ -40,6 +40,23 @@ def safe_list(body: SafeListRequest): # pylint: disable=unused-argument return {}, HTTPStatus.OK +@bp.route('/', methods=['DELETE']) +@jwt.requires_auth +@jwt.has_one_of_roles([Role.SYSTEM.value, Role.STAFF.value]) +@validate() +def delete_email(email: str): + if SafeList.is_in_safe_list(email): + email_to_delete = SafeList(email=email) + try: + email_to_delete.delete_email() + except Exception as err: # NOQA # pylint: disable=broad-except + logger.debug(err) + else: + logger.debug("Email not found in safe list.") + + return {}, HTTPStatus.OK + + @bp.route("/", methods=["OPTIONS"]) @validate() def get_safe_list_preflight(): diff --git a/notify-api/tests/unit/resources/v2/test_safe_list.py b/notify-api/tests/unit/resources/v2/test_safe_list.py index 1003c73e..8a8e1272 100644 --- a/notify-api/tests/unit/resources/v2/test_safe_list.py +++ b/notify-api/tests/unit/resources/v2/test_safe_list.py @@ -13,6 +13,8 @@ # limitations under the License. """Tests to assure the safe list end-point.""" +import json + from notify_api.models.safe_list import SafeList from notify_api.utils.enums import Role from tests.factories.jwt import create_header @@ -29,3 +31,14 @@ def test_safe_list(session, client, jwt): # pylint: disable=unused-argument assert response.json assert len(response.json) == 2 assert response.json[0]["email"] == "hello@gogo.com" + response = client.delete(f'/api/v2/safe_list/{"hello@gogo.com"}', headers=headers) + assert response.status_code == 200 + assert response.json + assert len(response.json) == 1 + assert response.json[0]["email"] == "hello@gogo2.com" + request_json = json.dumps({"email": ["hello@gogo.com"]}) + response = client.post("/api/v2/safe_list", json=request_json, headers=headers) + assert response.status_code == 200 + assert response.json + assert len(response.json) == 2 + assert response.json[0]["email"] == "hello@gogo2.com" \ No newline at end of file From 5100c06369cd954ca872a7216c4e8bd8f26a60e0 Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Wed, 22 May 2024 20:49:17 -0700 Subject: [PATCH 2/5] black fmt changes Signed-off-by: Shaanjot Gill --- notify-api/src/notify_api/resources/v2/safe_list.py | 4 ++-- notify-api/tests/unit/resources/v2/test_safe_list.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/notify-api/src/notify_api/resources/v2/safe_list.py b/notify-api/src/notify_api/resources/v2/safe_list.py index 9fc9b5ed..a1c318ee 100644 --- a/notify-api/src/notify_api/resources/v2/safe_list.py +++ b/notify-api/src/notify_api/resources/v2/safe_list.py @@ -40,7 +40,7 @@ def safe_list(body: SafeListRequest): # pylint: disable=unused-argument return {}, HTTPStatus.OK -@bp.route('/', methods=['DELETE']) +@bp.route("/", methods=["DELETE"]) @jwt.requires_auth @jwt.has_one_of_roles([Role.SYSTEM.value, Role.STAFF.value]) @validate() @@ -52,7 +52,7 @@ def delete_email(email: str): except Exception as err: # NOQA # pylint: disable=broad-except logger.debug(err) else: - logger.debug("Email not found in safe list.") + logger.debug("Email not found in safe list.") return {}, HTTPStatus.OK diff --git a/notify-api/tests/unit/resources/v2/test_safe_list.py b/notify-api/tests/unit/resources/v2/test_safe_list.py index 8a8e1272..a72dc0bc 100644 --- a/notify-api/tests/unit/resources/v2/test_safe_list.py +++ b/notify-api/tests/unit/resources/v2/test_safe_list.py @@ -41,4 +41,4 @@ def test_safe_list(session, client, jwt): # pylint: disable=unused-argument assert response.status_code == 200 assert response.json assert len(response.json) == 2 - assert response.json[0]["email"] == "hello@gogo2.com" \ No newline at end of file + assert response.json[0]["email"] == "hello@gogo2.com" From 59095250bb6bf4b8b3e257a1caa44bfb1d523c96 Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Wed, 22 May 2024 20:57:13 -0700 Subject: [PATCH 3/5] unit test fix Signed-off-by: Shaanjot Gill --- .../tests/unit/resources/v2/test_safe_list.py | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/notify-api/tests/unit/resources/v2/test_safe_list.py b/notify-api/tests/unit/resources/v2/test_safe_list.py index a72dc0bc..16cae6e0 100644 --- a/notify-api/tests/unit/resources/v2/test_safe_list.py +++ b/notify-api/tests/unit/resources/v2/test_safe_list.py @@ -31,14 +31,19 @@ def test_safe_list(session, client, jwt): # pylint: disable=unused-argument assert response.json assert len(response.json) == 2 assert response.json[0]["email"] == "hello@gogo.com" - response = client.delete(f'/api/v2/safe_list/{"hello@gogo.com"}', headers=headers) - assert response.status_code == 200 - assert response.json - assert len(response.json) == 1 - assert response.json[0]["email"] == "hello@gogo2.com" + del_response = client.delete(f'/api/v2/safe_list/{"hello@gogo.com"}', headers=headers) + assert del_response.status_code == 200 + get_response = client.get("/api/v2/safe_list/", headers=headers) + assert get_response.status_code == 200 + assert get_response.json + assert len(get_response.json) == 1 + assert get_response.json[0]["email"] == "hello@gogo2.com" request_json = json.dumps({"email": ["hello@gogo.com"]}) - response = client.post("/api/v2/safe_list", json=request_json, headers=headers) - assert response.status_code == 200 - assert response.json - assert len(response.json) == 2 - assert response.json[0]["email"] == "hello@gogo2.com" + add_response = client.post("/api/v2/safe_list", json=request_json, headers=headers) + assert add_response.status_code == 200 + assert add_response.json == {} + get_response = client.get("/api/v2/safe_list/", headers=headers) + assert get_response.status_code == 200 + assert get_response.json + assert len(get_response.json) == 2 + assert get_response.json[0]["email"] == "hello@gogo2.com" From f14d50b9383ce59d4166dc092f8975c54d54a952 Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Fri, 24 May 2024 09:13:24 -0700 Subject: [PATCH 4/5] feedback changes + unit test update Signed-off-by: Shaanjot Gill --- notify-api/src/notify_api/models/safe_list.py | 6 ++++-- .../src/notify_api/resources/v2/safe_list.py | 14 ++++++-------- .../tests/unit/resources/v2/test_safe_list.py | 14 ++++---------- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/notify-api/src/notify_api/models/safe_list.py b/notify-api/src/notify_api/models/safe_list.py index b983e4d5..9e359834 100644 --- a/notify-api/src/notify_api/models/safe_list.py +++ b/notify-api/src/notify_api/models/safe_list.py @@ -72,7 +72,9 @@ def find_all(cls) -> List[SafeList]: safe_emails = cls.query.all() return safe_emails - def delete_email(self): + @classmethod + def delete_email(cls, email: str): """delete email from safe list.""" - db.session.delete(self) + db_email = SafeList(email=email) + db.session.delete(db_email) db.session.commit() diff --git a/notify-api/src/notify_api/resources/v2/safe_list.py b/notify-api/src/notify_api/resources/v2/safe_list.py index a1c318ee..b207621d 100644 --- a/notify-api/src/notify_api/resources/v2/safe_list.py +++ b/notify-api/src/notify_api/resources/v2/safe_list.py @@ -45,15 +45,13 @@ def safe_list(body: SafeListRequest): # pylint: disable=unused-argument @jwt.has_one_of_roles([Role.SYSTEM.value, Role.STAFF.value]) @validate() def delete_email(email: str): - if SafeList.is_in_safe_list(email): - email_to_delete = SafeList(email=email) - try: - email_to_delete.delete_email() - except Exception as err: # NOQA # pylint: disable=broad-except - logger.debug(err) - else: + if not SafeList.is_in_safe_list(email): logger.debug("Email not found in safe list.") - + return {}, HTTPStatus.OK + try: + SafeList.delete_email(email) + except Exception as err: # NOQA # pylint: disable=broad-except + logger.debug(err) return {}, HTTPStatus.OK diff --git a/notify-api/tests/unit/resources/v2/test_safe_list.py b/notify-api/tests/unit/resources/v2/test_safe_list.py index 16cae6e0..a4127081 100644 --- a/notify-api/tests/unit/resources/v2/test_safe_list.py +++ b/notify-api/tests/unit/resources/v2/test_safe_list.py @@ -33,17 +33,11 @@ def test_safe_list(session, client, jwt): # pylint: disable=unused-argument assert response.json[0]["email"] == "hello@gogo.com" del_response = client.delete(f'/api/v2/safe_list/{"hello@gogo.com"}', headers=headers) assert del_response.status_code == 200 - get_response = client.get("/api/v2/safe_list/", headers=headers) - assert get_response.status_code == 200 - assert get_response.json - assert len(get_response.json) == 1 - assert get_response.json[0]["email"] == "hello@gogo2.com" + assert not SafeList.is_in_safe_list("hello@gogo.com") + assert SafeList.is_in_safe_list("hello@gogo2.com") request_json = json.dumps({"email": ["hello@gogo.com"]}) add_response = client.post("/api/v2/safe_list", json=request_json, headers=headers) assert add_response.status_code == 200 assert add_response.json == {} - get_response = client.get("/api/v2/safe_list/", headers=headers) - assert get_response.status_code == 200 - assert get_response.json - assert len(get_response.json) == 2 - assert get_response.json[0]["email"] == "hello@gogo2.com" + assert SafeList.is_in_safe_list("hello@gogo.com") + assert SafeList.is_in_safe_list("hello@gogo2.com") From fc2035feac949d894dd4b0da97e29722cdb05b40 Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Fri, 24 May 2024 09:18:57 -0700 Subject: [PATCH 5/5] unit test update + other fixes Signed-off-by: Shaanjot Gill --- notify-api/src/notify_api/models/safe_list.py | 11 ++++--- .../src/notify_api/resources/v2/safe_list.py | 3 +- .../tests/unit/models/test_safe_list.py | 21 +++++++++++++ .../tests/unit/resources/v2/test_safe_list.py | 31 +++++++++++-------- 4 files changed, 48 insertions(+), 18 deletions(-) create mode 100644 notify-api/tests/unit/models/test_safe_list.py diff --git a/notify-api/src/notify_api/models/safe_list.py b/notify-api/src/notify_api/models/safe_list.py index 9e359834..b1457bc3 100644 --- a/notify-api/src/notify_api/models/safe_list.py +++ b/notify-api/src/notify_api/models/safe_list.py @@ -66,15 +66,18 @@ def is_in_safe_list(cls, email: str) -> bool: return is_safe + @classmethod + def find_by_email(cls, email: str) -> SafeList: + """return safe list.""" + return (cls.query.filter_by(email=email).all())[0] + @classmethod def find_all(cls) -> List[SafeList]: """Return all of the safe emails.""" safe_emails = cls.query.all() return safe_emails - @classmethod - def delete_email(cls, email: str): + def delete_email(self): """delete email from safe list.""" - db_email = SafeList(email=email) - db.session.delete(db_email) + db.session.delete(self) db.session.commit() diff --git a/notify-api/src/notify_api/resources/v2/safe_list.py b/notify-api/src/notify_api/resources/v2/safe_list.py index b207621d..88fc306d 100644 --- a/notify-api/src/notify_api/resources/v2/safe_list.py +++ b/notify-api/src/notify_api/resources/v2/safe_list.py @@ -49,7 +49,8 @@ def delete_email(email: str): logger.debug("Email not found in safe list.") return {}, HTTPStatus.OK try: - SafeList.delete_email(email) + safe_list = SafeList.find_by_email(email) + safe_list.delete_email() except Exception as err: # NOQA # pylint: disable=broad-except logger.debug(err) return {}, HTTPStatus.OK diff --git a/notify-api/tests/unit/models/test_safe_list.py b/notify-api/tests/unit/models/test_safe_list.py new file mode 100644 index 00000000..b40eb381 --- /dev/null +++ b/notify-api/tests/unit/models/test_safe_list.py @@ -0,0 +1,21 @@ +"""The Unit Test for the SafeList Model.""" + +from notify_api.models.safe_list import SafeList + + +def test_safe_list(): + """Assert the test safe list model vaildation.""" + safelist = SafeList() + safelist.add_email("hello@gogo.com") + safelist.add_email("hello@gogo2.com") + assert safelist.is_in_safe_list("hello@gogo.com") + assert safelist.is_in_safe_list("hello@gogo2.com") + # Test delete email + safelist_to_delete = safelist.find_by_email("hello@gogo.com") + safelist_to_delete.delete_email() + assert not safelist.is_in_safe_list("hello@gogo.com") + assert safelist.is_in_safe_list("hello@gogo2.com") + # Test add email + safelist.add_email("hello@gogo.com") + assert safelist.is_in_safe_list("hello@gogo.com") + assert safelist.is_in_safe_list("hello@gogo2.com") diff --git a/notify-api/tests/unit/resources/v2/test_safe_list.py b/notify-api/tests/unit/resources/v2/test_safe_list.py index a4127081..316e3c1f 100644 --- a/notify-api/tests/unit/resources/v2/test_safe_list.py +++ b/notify-api/tests/unit/resources/v2/test_safe_list.py @@ -13,8 +13,6 @@ # limitations under the License. """Tests to assure the safe list end-point.""" -import json - from notify_api.models.safe_list import SafeList from notify_api.utils.enums import Role from tests.factories.jwt import create_header @@ -22,22 +20,29 @@ def test_safe_list(session, client, jwt): # pylint: disable=unused-argument """Assert that the safe list returns.""" - headers = create_header(jwt, [Role.STAFF.value], **{"Accept-Version": "v2"}) safelist = SafeList() + headers = create_header(jwt, [Role.STAFF.value], **{"Accept-Version": "v2"}) safelist.add_email("hello@gogo.com") safelist.add_email("hello@gogo2.com") response = client.get("/api/v2/safe_list/", headers=headers) assert response.status_code == 200 assert response.json assert len(response.json) == 2 - assert response.json[0]["email"] == "hello@gogo.com" - del_response = client.delete(f'/api/v2/safe_list/{"hello@gogo.com"}', headers=headers) - assert del_response.status_code == 200 - assert not SafeList.is_in_safe_list("hello@gogo.com") - assert SafeList.is_in_safe_list("hello@gogo2.com") - request_json = json.dumps({"email": ["hello@gogo.com"]}) - add_response = client.post("/api/v2/safe_list", json=request_json, headers=headers) + # Test delete endpoint + delete_response = client.delete(f"/api/v2/safe_list/{'hello@gogo.com'}", headers=headers) + assert delete_response.status_code == 200 + response = client.get("/api/v2/safe_list/", headers=headers) + assert response.status_code == 200 + assert response.json + assert len(response.json) == 1 + assert safelist.is_in_safe_list("hello@gogo2.com") + # Test add post endpoint + add_request_data = {"email": ["hello@gogo.com"]} + add_response = client.post("/api/v2/safe_list/", json=add_request_data, headers=headers) assert add_response.status_code == 200 - assert add_response.json == {} - assert SafeList.is_in_safe_list("hello@gogo.com") - assert SafeList.is_in_safe_list("hello@gogo2.com") + response = client.get("/api/v2/safe_list/", headers=headers) + assert response.status_code == 200 + assert response.json + assert len(response.json) == 2 + assert safelist.is_in_safe_list("hello@gogo2.com") + assert safelist.is_in_safe_list("hello@gogo.com")