Cbrash/city optional

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',

sanctions/apps/api/v1/tests/test_views.py

@@ -119,3 +119,56 @@ 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}
+
+        # 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(self.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()
+
+    @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()

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
 

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,

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

sanctions/apps/sanctions/utils.py

@@ -34,11 +34,18 @@
     records = SDNFallbackData.get_current_records_and_filter_by_source_and_type(
         'Specially Designated Nationals (SDN) - Treasury Department', 'Individual'
     )
     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)):
             hit_count = hit_count + 1
     return hit_count
 
@@ -74,8 +81,11 @@
     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()