Skip to content

Commit

Permalink
REV-3765 | make certain fields optional, check api creates record on …
Browse files Browse the repository at this point in the history
…hit, check api is admin only (#14)

* feat: prepare sanctions API and model prepare sanctions API and model for use

* fix: user the lms_user_id provided in the payload
  • Loading branch information
christopappas authored Nov 14, 2023
1 parent 34b2ad0 commit 5fba044
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 12 deletions.
58 changes: 57 additions & 1 deletion sanctions/apps/api/v1/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
import json
from unittest import mock

from django.db.utils import OperationalError
from requests.exceptions import HTTPError
from rest_framework.reverse import reverse

from sanctions.apps.sanctions.models import SanctionsCheckFailure
from test_utils import APITest


Expand All @@ -21,9 +23,12 @@ def setUp(self):
'full_name': 'Din Grogu',
'city': 'Jedi Temple',
'country': 'SW',
'system_identifier': 'a new django IDA'
}
self.user.is_staff = True
self.user.save()

def test_sdn_check_missing_args(self):
def test_sdn_check_missing_args_returns_400(self):
self.set_jwt_cookie(self.user.id)
response = self.client.post(self.url)
assert response.status_code == 400
Expand All @@ -43,6 +48,18 @@ def test_sdn_check_search_fails_uses_fallback(self, mock_search, mock_fallback):
assert response.json()['hit_count'] == 0
assert mock_fallback.is_called()

def test_sdn_check_no_jwt_returns_401(self):
response = self.client.post(self.url)
assert response.status_code == 401

def test_sdn_check_non_staff_returns_403(self):
self.user.is_staff = False
self.user.save()

self.set_jwt_cookie(self.user.id)
response = self.client.post(self.url)
assert response.status_code == 403

@mock.patch('sanctions.apps.api.v1.views.checkSDNFallback')
@mock.patch('sanctions.apps.api_client.sdn_client.SDNClient.search')
def test_sdn_check_search_succeeds(
Expand All @@ -61,3 +78,42 @@ def test_sdn_check_search_succeeds(
assert response.json()['hit_count'] == 4
assert response.json()['sdn_response'] == {'total': 4}
mock_fallback.assert_not_called()

assert SanctionsCheckFailure.objects.count() == 1
failure_record = SanctionsCheckFailure.objects.first()
assert failure_record.full_name == 'Din Grogu'
assert failure_record.username is None
assert failure_record.lms_user_id == self.user.lms_user_id
assert failure_record.city == 'Jedi Temple'
assert failure_record.country == 'SW'
assert failure_record.sanctions_type == 'ISN,SDN'
assert failure_record.system_identifier == 'a new django IDA'
assert failure_record.metadata == {}
assert failure_record.sanctions_response == {'total': 4}

@mock.patch('sanctions.apps.api.v1.views.SanctionsCheckFailure.objects.create')
@mock.patch('sanctions.apps.api_client.sdn_client.SDNClient.search')
def test_sdn_check_search_succeeds_despite_DB_issue(
self,
mock_search,
mock_sanction_record_create,
):
"""
In the case that we are unable to create a DB object, we should log
and error and return the results from the SDN API to the caller.
"""
mock_search.return_value = {'total': 4}
self.set_jwt_cookie(self.user.id)
mock_sanction_record_create.side_effect = OperationalError('Connection timed out')

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'] == 4
assert response.json()['sdn_response'] == {'total': 4}

assert SanctionsCheckFailure.objects.count() == 0
61 changes: 55 additions & 6 deletions sanctions/apps/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from rest_framework import permissions, views

from sanctions.apps.api_client.sdn_client import SDNClient
from sanctions.apps.sanctions.models import SanctionsCheckFailure
from sanctions.apps.sanctions.utils import checkSDNFallback

logger = logging.getLogger(__name__)
Expand All @@ -20,7 +21,7 @@ class SDNCheckView(views.APIView):
View for external services to run SDN/ISN checks against.
"""
http_method_names = ['post']
permission_classes = (permissions.IsAuthenticated,)
permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser)
authentication_classes = (JwtAuthentication,)

def post(self, request):
Expand All @@ -42,7 +43,7 @@ def post(self, request):
}
return JsonResponse(json_data, status=400)

lms_user_id = request.user.lms_user_id
lms_user_id = payload.get('lms_user_id')
full_name = payload.get('full_name')
city = payload.get('city')
country = payload.get('country')
Expand All @@ -59,7 +60,7 @@ def post(self, request):
'SDNCheckView: calling the SDN Client for SDN check for user %s.',
lms_user_id
)
response = sdn_check.search(lms_user_id, full_name, city, country)
sdn_check_response = sdn_check.search(lms_user_id, full_name, city, country)
except (HTTPError, Timeout) as e:
logger.info(
'SDNCheckView: SDN API call received an error: %s.'
Expand All @@ -73,15 +74,63 @@ def post(self, request):
city,
country
)
response = {'total': sdn_fallback_hit_count}
sdn_check_response = {'total': sdn_fallback_hit_count}

hit_count = response['total']
hit_count = sdn_check_response['total']
if hit_count > 0:
logger.info(
'SDNCheckView request received for lms user [%s]. It received %d hit(s).',
lms_user_id,
hit_count,
)
# write record to our DB that we've had a positive hit, including
# any metadata provided in the payload
metadata = payload.get('metadata', {})
username = payload.get('username')
system_identifier = payload.get('system_identifier')
sanctions_type = 'ISN,SDN'
# This try/except is here to make us fault tolerant. Callers of this
# API should not be held up if we are having DB troubles. Log the error
# and continue through the code to reply to them.
try:
SanctionsCheckFailure.objects.create(
full_name=full_name,
username=username,
lms_user_id=lms_user_id,
city=city,
country=country,
sanctions_type=sanctions_type,
system_identifier=system_identifier,
metadata=metadata,
sanctions_response=sdn_check_response,
)
except Exception as err: # pylint: disable=broad-exception-caught
error_message = (
'Encountered error creating SanctionsCheckFailure. %s '
'Data dump follows to capture information on the hit: '
'lms_user_id: %s '
'username: %s '
'full_name: %s '
'city: %s '
'country: %s '
'sanctions_type: %s '
'system_identifier: %s '
'metadata: %s '
'sanctions_response: %s '
)
logger.exception(
error_message,
err,
lms_user_id,
username,
full_name,
city,
country,
sanctions_type,
system_identifier,
metadata,
sdn_check_response,
)
else:
logger.info(
'SDNCheckView request received for lms user [%s]. It did not receive a hit.',
Expand All @@ -90,7 +139,7 @@ def post(self, request):

json_data = {
'hit_count': hit_count,
'sdn_response': response,
'sdn_response': sdn_check_response,
}

return JsonResponse(json_data, status=200)
61 changes: 61 additions & 0 deletions sanctions/apps/sanctions/migrations/0003_auto_20231109_2121.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Generated by Django 3.2.22 on 2023-11-09 21:21

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('sanctions', '0002_rename_fallback_models'),
]

operations = [
migrations.RemoveField(
model_name='historicalsanctionscheckfailure',
name='sdn_check_response',
),
migrations.RemoveField(
model_name='sanctionscheckfailure',
name='sdn_check_response',
),
migrations.AddField(
model_name='historicalsanctionscheckfailure',
name='sanctions_response',
field=models.JSONField(null=True),
),
migrations.AddField(
model_name='sanctionscheckfailure',
name='sanctions_response',
field=models.JSONField(null=True),
),
migrations.AlterField(
model_name='historicalsanctionscheckfailure',
name='metadata',
field=models.JSONField(null=True),
),
migrations.AlterField(
model_name='historicalsanctionscheckfailure',
name='system_identifier',
field=models.CharField(max_length=255, null=True),
),
migrations.AlterField(
model_name='historicalsanctionscheckfailure',
name='username',
field=models.CharField(max_length=255, null=True),
),
migrations.AlterField(
model_name='sanctionscheckfailure',
name='metadata',
field=models.JSONField(null=True),
),
migrations.AlterField(
model_name='sanctionscheckfailure',
name='system_identifier',
field=models.CharField(max_length=255, null=True),
),
migrations.AlterField(
model_name='sanctionscheckfailure',
name='username',
field=models.CharField(max_length=255, null=True),
),
]
8 changes: 4 additions & 4 deletions sanctions/apps/sanctions/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ class SanctionsCheckFailure(TimeStampedModel):
"""
history = HistoricalRecords()
full_name = models.CharField(max_length=255)
username = models.CharField(max_length=255)
username = models.CharField(null=True, max_length=255)
lms_user_id = models.IntegerField(null=True, db_index=True)
city = models.CharField(max_length=32, default='')
country = models.CharField(max_length=2)
sanctions_type = models.CharField(max_length=255)
system_identifier = models.CharField(max_length=255)
metadata = models.JSONField()
sdn_check_response = models.JSONField()
system_identifier = models.CharField(null=True, max_length=255)
metadata = models.JSONField(null=True)
sanctions_response = models.JSONField(null=True)

class Meta:
verbose_name = 'Sanctions Check Failure'
Expand Down
2 changes: 1 addition & 1 deletion sanctions/apps/sanctions/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def test_unicode(self):
sanctions_type='ISN,SDN',
system_identifier='commerce-coordinator',
metadata={'order_identifer': 'EDX-123456', 'purchase_type': 'program', 'order_total': '989.00'},
sdn_check_response=self.sdn_check_response
sanctions_response=self.sdn_check_response
)
expected = 'Sanctions check failure [{username}]'.format(
username=self.username
Expand Down

0 comments on commit 5fba044

Please sign in to comment.