Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cpappas/rev 3765 2 #15

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
59 changes: 54 additions & 5 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 Down Expand Up @@ -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
Loading