From 944bfffc11d99646923d66b19b42310fe1484210 Mon Sep 17 00:00:00 2001 From: Colin Brash Date: Tue, 19 Aug 2025 11:01:57 -0400 Subject: [PATCH 1/7] feat: make city an option field --- sanctions/apps/api/v1/views.py | 6 ++++-- sanctions/apps/sanctions/utils.py | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/sanctions/apps/api/v1/views.py b/sanctions/apps/api/v1/views.py index c2b54b2..7d37dc0 100644 --- a/sanctions/apps/api/v1/views.py +++ b/sanctions/apps/api/v1/views.py @@ -34,7 +34,7 @@ def post(self, request): # Make sure we have the values needed to carry out the request missing_args = [] - for expected_arg in ['lms_user_id', 'full_name', 'city', 'country']: + for expected_arg in ['lms_user_id', 'full_name', 'country']: if not payload.get(expected_arg): missing_args.append(expected_arg) if missing_args: @@ -45,7 +45,9 @@ def post(self, request): lms_user_id = payload.get('lms_user_id') full_name = payload.get('full_name') - city = payload.get('city') + # The city field is optional. Default to an empty string if not provided so that + # downstream checks and SDN API calls continue to work without additional guards. + city = payload.get('city', '') country = payload.get('country') sdn_api_list = settings.SDN_CHECK_API_LIST diff --git a/sanctions/apps/sanctions/utils.py b/sanctions/apps/sanctions/utils.py index 7b1a6ec..15df23c 100644 --- a/sanctions/apps/sanctions/utils.py +++ b/sanctions/apps/sanctions/utils.py @@ -74,8 +74,11 @@ def process_text(text): Returns: text (set): processed text """ - if len(text) == 0: - return '' + # If the input is empty or None, return an empty set so that downstream + # set operations such as ``issubset`` do not raise AttributeError while + # still behaving logically (an empty set is a subset of any other set). + if not text: + return set() # Make lowercase text = text.casefold() From 274a5aeb93021e5adb88f24fbcd726d5082807d2 Mon Sep 17 00:00:00 2001 From: Colin Brash Date: Tue, 19 Aug 2025 12:09:12 -0400 Subject: [PATCH 2/7] feat: do not send city to trade.gov API if we don't have a city --- sanctions/apps/api_client/sdn_client.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/sanctions/apps/api_client/sdn_client.py b/sanctions/apps/api_client/sdn_client.py index 823e16f..02c7527 100644 --- a/sanctions/apps/api_client/sdn_client.py +++ b/sanctions/apps/api_client/sdn_client.py @@ -36,15 +36,24 @@ def search(self, lms_user_id, name, city, country): Returns: dict: SDN API response. """ + # Build the query-string parameters expected by the Trade.gov API. The + # "city" field has become optional for the sanctions service. When the + # caller provides an empty string we omit the parameter entirely to + # broaden the search (an empty value can produce unexpected filtering + # on the API side). params_dict = { 'sources': self.sdn_api_list, 'type': 'individual', 'name': str(name).encode('utf-8'), + 'countries': country, + } + + # Only include the city parameter if one was supplied + if city: # We are using the city as the address parameter value as indicated in the documentation: # https://internationaltradeadministration.github.io/developerportal/consolidated-screening-list.html - 'city': str(city).encode('utf-8'), - 'countries': country - } + params_dict['city'] = str(city).encode('utf-8') + params = urlencode(params_dict) sdn_check_url = '{api_url}?{params}'.format( api_url=self.sdn_api_url, From 27c4632a2b9240d694d409bf1f7672583abf0163 Mon Sep 17 00:00:00 2001 From: Colin Brash Date: Tue, 19 Aug 2025 12:17:01 -0400 Subject: [PATCH 3/7] feat: add explicit comment for fallback functionality when city is not passed in --- sanctions/apps/sanctions/utils.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sanctions/apps/sanctions/utils.py b/sanctions/apps/sanctions/utils.py index 15df23c..90da554 100644 --- a/sanctions/apps/sanctions/utils.py +++ b/sanctions/apps/sanctions/utils.py @@ -36,6 +36,13 @@ def checkSDNFallback(name, city, country): ) records = records.filter(countries__contains=country) processed_name, processed_city = process_text(name), process_text(city) + + # NOTE: If ``city`` is an empty string it becomes an empty set after + # processing. Because an empty set is considered a subset of any other + # set, the address comparison below will automatically evaluate ``True``. + # As a result the fallback check ignores city filtering and will match any + # record whose name matches. + for record in records: record_names, record_addresses = set(record.names.split()), set(record.addresses.split()) if (processed_name.issubset(record_names) and processed_city.issubset(record_addresses)): From db21e9496c1fcef929a2743443ca2e428a582a52 Mon Sep 17 00:00:00 2001 From: Colin Brash Date: Tue, 19 Aug 2025 12:23:33 -0400 Subject: [PATCH 4/7] feat: add tests for when city is omitted --- sanctions/apps/api/v1/tests/test_views.py | 29 ++++++++++++++++++ .../apps/api_client/tests/test_sdn_client.py | 30 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/sanctions/apps/api/v1/tests/test_views.py b/sanctions/apps/api/v1/tests/test_views.py index 840139e..f2b6982 100644 --- a/sanctions/apps/api/v1/tests/test_views.py +++ b/sanctions/apps/api/v1/tests/test_views.py @@ -119,3 +119,32 @@ def test_sdn_check_search_succeeds_despite_DB_issue( assert response.json()['sanctions_check_failure_id'] is None assert SanctionsCheckFailure.objects.count() == 0 + + @mock.patch('sanctions.apps.api.v1.views.checkSDNFallback') + @mock.patch('sanctions.apps.api_client.sdn_client.SDNClient.search') + def test_sdn_check_accepts_request_without_city( + self, + mock_search, + mock_fallback, + ): + """Verify that omitting the optional city field still succeeds.""" + mock_search.return_value = {'total': 0} + post_data = { + 'lms_user_id': self.user.lms_user_id, + 'full_name': 'Din Grogu', + 'country': 'SW', + 'system_identifier': 'a new django IDA' + } + + self.set_jwt_cookie(self.user.id) + response = self.client.post( + self.url, + content_type='application/json', + data=json.dumps(post_data) + ) + + assert response.status_code == 200 + assert response.json()['hit_count'] == 0 + # city omitted, no sanctions record should be created when hit_count == 0 + assert SanctionsCheckFailure.objects.count() == 0 + mock_fallback.assert_not_called() diff --git a/sanctions/apps/api_client/tests/test_sdn_client.py b/sanctions/apps/api_client/tests/test_sdn_client.py index 8cf20ff..adc78f3 100644 --- a/sanctions/apps/api_client/tests/test_sdn_client.py +++ b/sanctions/apps/api_client/tests/test_sdn_client.py @@ -91,3 +91,33 @@ def test_sdn_search_success(self): self.mock_sdn_api_response(json.dumps(sdn_response), status_code=200) response = self.sdn_api_client.search(self.lms_user_id, self.name, self.city, self.country) assert response == sdn_response + + @responses.activate + def test_sdn_search_success_no_city(self): + """Verify that when city is empty, the client omits the city query parameter.""" + sdn_response = {'total': 2} + + # Build expected URL without the city param + params_dict = { + 'sources': self.sdn_api_list, + 'type': 'individual', + 'name': str(self.name).encode('utf-8'), + 'countries': self.country, + } + params = urlencode(params_dict) + sdn_check_url = f'{self.sdn_api_url}?{params}' + auth_header = {'subscription-key': f'{self.sdn_api_key}'} + + responses.add( + responses.GET, + sdn_check_url, + headers=auth_header, + status=200, + body=json.dumps(sdn_response), + content_type='application/json', + ) + + response = self.sdn_api_client.search(self.lms_user_id, self.name, '', self.country) + assert response == sdn_response + # Ensure the mocked endpoint was called exactly once + assert len(responses.calls) == 1 From 13d1b052df276eaddd74e1c9f2fa0d80fba3a4a7 Mon Sep 17 00:00:00 2001 From: Colin Brash Date: Tue, 19 Aug 2025 12:28:44 -0400 Subject: [PATCH 5/7] feat: update README to note that city is optional --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 7ee080f..48c32ba 100644 --- a/README.rst +++ b/README.rst @@ -108,7 +108,7 @@ Example of making a POST request to the `api/v1/sdn-check/` endpoint: 'lms_user_id': user.lms_user_id, 'username': user.username, # optional 'full_name': full_name, - 'city': city, + 'city': city, # optional 'country': country, 'metadata': { # optional, any key/value can be added 'order_identifer': 'EDX-123456', From 6c698b0f75ff0290e8c35bf6271155f4a0eaa242 Mon Sep 17 00:00:00 2001 From: Colin Brash Date: Wed, 20 Aug 2025 08:30:04 -0400 Subject: [PATCH 6/7] fix: re-use self.post_data from setUp() --- sanctions/apps/api/v1/tests/test_views.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/sanctions/apps/api/v1/tests/test_views.py b/sanctions/apps/api/v1/tests/test_views.py index f2b6982..c5d568e 100644 --- a/sanctions/apps/api/v1/tests/test_views.py +++ b/sanctions/apps/api/v1/tests/test_views.py @@ -129,18 +129,15 @@ def test_sdn_check_accepts_request_without_city( ): """Verify that omitting the optional city field still succeeds.""" mock_search.return_value = {'total': 0} - post_data = { - 'lms_user_id': self.user.lms_user_id, - 'full_name': 'Din Grogu', - 'country': 'SW', - 'system_identifier': 'a new django IDA' - } + + # Re-use the default payload created in setUp but remove the city field. + self.post_data.pop('city') # city is optional self.set_jwt_cookie(self.user.id) response = self.client.post( self.url, content_type='application/json', - data=json.dumps(post_data) + data=json.dumps(self.post_data) ) assert response.status_code == 200 From dfdafab00da9ac73169621d320def6a5ce649656 Mon Sep 17 00:00:00 2001 From: Colin Brash Date: Wed, 20 Aug 2025 08:32:04 -0400 Subject: [PATCH 7/7] feat: add a test for when city is present but an empty string --- sanctions/apps/api/v1/tests/test_views.py | 27 +++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/sanctions/apps/api/v1/tests/test_views.py b/sanctions/apps/api/v1/tests/test_views.py index c5d568e..fbcf7b8 100644 --- a/sanctions/apps/api/v1/tests/test_views.py +++ b/sanctions/apps/api/v1/tests/test_views.py @@ -145,3 +145,30 @@ def test_sdn_check_accepts_request_without_city( # city omitted, no sanctions record should be created when hit_count == 0 assert SanctionsCheckFailure.objects.count() == 0 mock_fallback.assert_not_called() + + @mock.patch('sanctions.apps.api.v1.views.checkSDNFallback') + @mock.patch('sanctions.apps.api_client.sdn_client.SDNClient.search') + def test_sdn_check_accepts_request_with_empty_city( + self, + mock_search, + mock_fallback, + ): + """Verify that sending an empty string for city still succeeds.""" + mock_search.return_value = {'total': 0} + + # Use the default payload but set city to an empty string to simulate + # clients that populate the field but have no value. + self.post_data['city'] = '' + + self.set_jwt_cookie(self.user.id) + response = self.client.post( + self.url, + content_type='application/json', + data=json.dumps(self.post_data) + ) + + assert response.status_code == 200 + assert response.json()['hit_count'] == 0 + # empty city should not trigger fallback when hit_count == 0 + assert SanctionsCheckFailure.objects.count() == 0 + mock_fallback.assert_not_called()