Skip to content

Commit

Permalink
Merge pull request #916 from uktrade/LTD-1774-refusal-criteria-add-lo…
Browse files Browse the repository at this point in the history
…gging

Add logging calls when creating or updating an advice object
  • Loading branch information
kevincarrogan authored Feb 7, 2022
2 parents 75c42b8 + 1873ea4 commit 0f5ce86
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 4 deletions.
32 changes: 32 additions & 0 deletions api/applications/serializers/advice.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import logging

from django.conf import settings
from rest_framework import serializers
from rest_framework.exceptions import ValidationError

Expand All @@ -19,6 +22,9 @@
from api.users.models import GovUser


denial_reasons_logger = logging.getLogger(settings.DENIAL_REASONS_DELETION_LOGGER)


class GoodField(serializers.Field):
def to_representation(self, instance):
return str(instance.id)
Expand Down Expand Up @@ -159,6 +165,32 @@ def _footnote_fields_setup(self):
self.initial_data[i]["footnote"] = None
self.initial_data[i]["footnote_required"] = None

def create(self, *args, **kwargs):
instance = super().create(*args, **kwargs)

if not instance.denial_reasons.exists():
denial_reasons_logger.warning(
"Creating advice object with no denial reasons: %s (%s)", instance, instance.pk, exc_info=True,
)

return instance

def update(self, instance, validated_data):
previous_denial_reasons = list(instance.denial_reasons.values_list("pk", flat=True))

instance = super().update(instance, validated_data)

if not instance.denial_reasons.exists():
denial_reasons_logger.warning(
"Updating advice object with no denial reasons: %s (%s) - %s",
instance,
instance.pk,
previous_denial_reasons,
exc_info=True,
)

return instance


class CountersignAdviceListSerializer(serializers.ListSerializer):
def update(self, instances, validated_data):
Expand Down
16 changes: 16 additions & 0 deletions api/cases/libraries/advice.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
import logging

from collections import defaultdict

from django.conf import settings

from api.cases.enums import AdviceType
from api.cases.models import Advice
from api.goods.enums import PvGrading


denial_reasons_logger = logging.getLogger(settings.DENIAL_REASONS_DELETION_LOGGER)


def group_advice(case, advice, user, new_level):
advice_entities = {entity_field: defaultdict(list) for entity_field in Advice.ENTITY_FIELDS}

Expand All @@ -29,7 +36,16 @@ def collate_advice(entity_field, new_level, collection, case, user):
setattr(advice, entity_field, key)

advice.save()
previous_denial_reasons = list(advice.denial_reasons.values_list("pk", flat=True))
advice.denial_reasons.set(denial_reasons)
if not denial_reasons:
denial_reasons_logger.warning(
"Removing denial reasons in `collate_advice` for: %s (%s) - %s",
advice,
advice.pk,
previous_denial_reasons,
exc_info=True,
)


def deduplicate_advice(advice_list):
Expand Down
43 changes: 39 additions & 4 deletions api/cases/models.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import logging
import uuid

from collections import defaultdict
from typing import Optional

from django.conf import settings
from django.contrib.contenttypes.fields import GenericRelation
from django.db import models
from django.utils import timezone
Expand Down Expand Up @@ -44,6 +47,9 @@
)


denial_reasons_logger = logging.getLogger(settings.DENIAL_REASONS_DELETION_LOGGER)


class CaseTypeManager(models.Manager):
def get_by_natural_key(self, reference):
return self.get(reference=reference)
Expand Down Expand Up @@ -418,7 +424,7 @@ def save(self, *args, **kwargs):
self.proviso = None
try:
if self.level == AdviceLevel.TEAM:
Advice.objects.get(
old_advice = Advice.objects.get(
case=self.case,
team=self.team,
level=AdviceLevel.TEAM,
Expand All @@ -429,7 +435,17 @@ def save(self, *args, **kwargs):
ultimate_end_user=self.ultimate_end_user,
consignee=self.consignee,
third_party=self.third_party,
).delete()
)
if old_advice.denial_reasons.exists():
denial_reasons = old_advice.denial_reasons.values_list("pk", flat=True)
denial_reasons_logger.warning(
"Deleting advice object that had denial reasons: %s (%s) - %s",
old_advice,
old_advice.pk,
denial_reasons,
exc_info=True,
)
old_advice.delete()
elif self.level == AdviceLevel.FINAL:
old_advice = Advice.objects.get(
case=self.case,
Expand All @@ -444,9 +460,18 @@ def save(self, *args, **kwargs):
)
self.footnote = old_advice.footnote
self.footnote_required = old_advice.footnote_required
if old_advice.denial_reasons.exists():
denial_reasons = old_advice.denial_reasons.values_list("pk", flat=True)
denial_reasons_logger.warning(
"Deleting advice object that had denial reasons: %s (%s) - %s",
old_advice,
old_advice.pk,
denial_reasons,
exc_info=True,
)
old_advice.delete()
elif self.level == AdviceLevel.USER:
Advice.objects.get(
old_advice = Advice.objects.get(
case=self.case,
good=self.good,
user=self.user,
Expand All @@ -457,7 +482,17 @@ def save(self, *args, **kwargs):
ultimate_end_user=self.ultimate_end_user,
consignee=self.consignee,
third_party=self.third_party,
).delete()
)
if old_advice.denial_reasons.exists():
denial_reasons = old_advice.denial_reasons.values_list("pk", flat=True)
denial_reasons_logger.warning(
"Deleting advice object that had denial reasons: %s (%s) - %s",
old_advice,
old_advice.pk,
denial_reasons,
exc_info=True,
)
old_advice.delete()
except Advice.DoesNotExist:
pass

Expand Down
6 changes: 6 additions & 0 deletions api/conf/settings.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
import os
import sys

Expand Down Expand Up @@ -308,6 +309,9 @@
]


DENIAL_REASONS_DELETION_LOGGER = "denial_reasons_deletion_logger"


if "test" not in sys.argv:
LOGGING = {
"version": 1,
Expand All @@ -319,8 +323,10 @@
"handlers": {
"stdout": {"class": "logging.StreamHandler", "formatter": "simple"},
"ecs": {"class": "logging.StreamHandler", "formatter": "ecs_formatter"},
"sentry": {"class": "sentry_sdk.integrations.logging.EventHandler"},
},
"root": {"handlers": ["stdout", "ecs"], "level": env("LOG_LEVEL").upper()},
"loggers": {DENIAL_REASONS_DELETION_LOGGER: {"handlers": ["sentry"], "level": logging.WARNING},},
}
else:
LOGGING = {"version": 1, "disable_existing_loggers": True}
Expand Down

0 comments on commit 0f5ce86

Please sign in to comment.