From 40133895aefb9a8151e9466ada99133cab82d967 Mon Sep 17 00:00:00 2001 From: Mitch Dawson Date: Fri, 13 Dec 2024 19:59:18 +0000 Subject: [PATCH 1/2] initial commit --- feedback/models.py | 18 ++++++++++++++++++ feedback/service.py | 6 ++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/feedback/models.py b/feedback/models.py index 5998bacc..977cac11 100644 --- a/feedback/models.py +++ b/feedback/models.py @@ -1,3 +1,5 @@ +from urllib.parse import quote, urlparse, urlunparse + from django.conf import settings from django.core.validators import MinLengthValidator from django.db import models @@ -55,3 +57,19 @@ class IssueChoices(models.TextChoices): entity_name = models.CharField(max_length=250) entity_url = models.CharField(max_length=250) data_custodian_email = models.CharField(max_length=250) + + @property + def formatted_entity_url(self): + parsed_url = urlparse(self.entity_url) + encoded_path = quote(parsed_url.path) + formatted_entity_url = urlunparse( + ( + parsed_url.scheme, + parsed_url.netloc, + encoded_path, + parsed_url.params, + parsed_url.query, + parsed_url.fragment, + ) + ) + return formatted_entity_url diff --git a/feedback/service.py b/feedback/service.py index 859963c1..a520ed0e 100644 --- a/feedback/service.py +++ b/feedback/service.py @@ -28,12 +28,14 @@ def send( personalisation = { "assetOwner": ( - issue.data_custodian_email if issue.data_custodian_email else "Data Catalog Team" + issue.data_custodian_email + if issue.data_custodian_email + else "Data Catalog Team" ), "userEmail": issue.created_by.email if issue.created_by else "", "assetName": issue.entity_name, "userMessage": issue.additional_info, - "assetUrl": issue.entity_url, + "assetUrl": issue.formatted_entity_url, } reference = str(issue.id) From 7c080221065f1b24fbd7031eb25d46a32eee3ae0 Mon Sep 17 00:00:00 2001 From: Mitch Dawson Date: Mon, 16 Dec 2024 10:17:56 +0000 Subject: [PATCH 2/2] Add test for entity url encoding --- feedback/models.py | 12 ++++++------ feedback/service.py | 2 +- tests/feedback/test_notify_service.py | 21 ++++++++++++++++++++- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/feedback/models.py b/feedback/models.py index 977cac11..49d7d3e9 100644 --- a/feedback/models.py +++ b/feedback/models.py @@ -1,4 +1,4 @@ -from urllib.parse import quote, urlparse, urlunparse +from urllib.parse import ParseResult, quote, urlparse, urlunparse from django.conf import settings from django.core.validators import MinLengthValidator @@ -59,10 +59,10 @@ class IssueChoices(models.TextChoices): data_custodian_email = models.CharField(max_length=250) @property - def formatted_entity_url(self): - parsed_url = urlparse(self.entity_url) - encoded_path = quote(parsed_url.path) - formatted_entity_url = urlunparse( + def encoded_entity_url(self): + parsed_url: ParseResult = urlparse(self.entity_url) + encoded_path: str = quote(parsed_url.path) + encoded_entity_url: str = urlunparse( ( parsed_url.scheme, parsed_url.netloc, @@ -72,4 +72,4 @@ def formatted_entity_url(self): parsed_url.fragment, ) ) - return formatted_entity_url + return encoded_entity_url diff --git a/feedback/service.py b/feedback/service.py index a520ed0e..49c34fc2 100644 --- a/feedback/service.py +++ b/feedback/service.py @@ -35,7 +35,7 @@ def send( "userEmail": issue.created_by.email if issue.created_by else "", "assetName": issue.entity_name, "userMessage": issue.additional_info, - "assetUrl": issue.formatted_entity_url, + "assetUrl": issue.encoded_entity_url, } reference = str(issue.id) diff --git a/tests/feedback/test_notify_service.py b/tests/feedback/test_notify_service.py index b342fd20..5f2db783 100644 --- a/tests/feedback/test_notify_service.py +++ b/tests/feedback/test_notify_service.py @@ -24,7 +24,9 @@ def test_send_all_notifications(mock_notifications_client, reporter): @pytest.mark.django_db -def test_send_notifications_no_data_custodian_email(mock_notifications_client, reporter): +def test_send_notifications_no_data_custodian_email( + mock_notifications_client, reporter +): data = { "reason": "Other", "additional_info": "This is some additional information.", @@ -76,3 +78,20 @@ def test_send_all_notifications_no_reporter_no_data_custodian_email( send(issue=issue, client=mock_notifications_client, send_email_to_reporter=False) assert mock_notifications_client.send_email_notification.call_count == 1 + + +@pytest.mark.django_db +def test_entity_url_encoding(reporter): + data = { + "reason": "Other", + "additional_info": "This is some additional information.", + "entity_name": "my_entity", + "entity_url": "http://localhost:8000/details/table/urn:li:dataset:(urn:li:dataPlatform:dbt,cadet.awsdatacatalog.derived_oasys_dim.dim_ref_question,PROD)", # noqa: E501 + "created_by": reporter, + } + + encoded_entity_url = "http://localhost:8000/details/table/urn%3Ali%3Adataset%3A%28urn%3Ali%3AdataPlatform%3Adbt%2Ccadet.awsdatacatalog.derived_oasys_dim.dim_ref_question%2CPROD%29" # noqa: E501 + + issue = Issue.objects.create(**data) + assert issue + assert issue.encoded_entity_url == encoded_entity_url