Technical debt repayment (#632)

* fix routing from most specific to least for manifests views

* add check that manifest is not draft before saving to RCRAInfo

* refactor CreateManifestView so that all logic is moved into our business logic and it now saves to the database if just a draft or launches a task and saves to RCRAInfo if not a draft

* revert docker and local virtual environment to python 3.11 until python 3.12 is fully supported by Celery

* refactor sync rcrainfo manifest services and tasks

* refactor site services for pulling rcra sites from rcrainfo

* update HandlerSearchView to follow styleguide and use class internal serializer, switch DRF exception handling to match-case

* Cache all GET api views for at least 15 minutes

* misc service method renames and add waste_services for retrieving waste information

* add service function for getting DOT shipping information (hazard class, ID Numbers, Shipping name)

* add examples for DOT option lookup views

* move test_handler_service to sites app

* update test suite and conform quicker sign related function signatures

* add test for waste service functions for retrieveing DOT lookup infromation

* add unit test for ManifestService._filter_mtn method

* remove python 3.10 from supported versions

.github/workflows/test_server.yaml

@@ -27,7 +27,7 @@ jobs:
 
     strategy:
       matrix:
-        python-version: ['3.10', '3.11']
+        python-version: ['3.11']
 
     services:
       postgres:

client/src/components/Manifest/ManifestForm.tsx

@@ -96,23 +96,30 @@ export function ManifestForm({
   const onSubmit: SubmitHandler<Manifest> = (data: Manifest) => {
     console.log('Manifest Submitted', data);
     htApi
-      .post<TaskStatus>('/rcra/manifest', data)
+      .post<TaskStatus | Manifest>('/rcra/manifest', data)
       .then((response) => {
         return response;
       })
       .then((r) => {
-        dispatch(
-          addNotification(
-            // @ts-ignore
-            {
-              uniqueId: r.data.taskId,
-              createdDate: new Date().toISOString(),
-              inProgress: true,
-            }
-          )
-        );
-        setTaskId(r.data.taskId);
-        toggleShowUpdatingRcra();
+        if ('manifestTrackingNumber' in r.data) {
+          console.log("congratulations! it's a manifest!");
+          navigate(`/manifest/${r.data.manifestTrackingNumber}/view`);
+        }
+        if ('taskId' in r.data) {
+          dispatch(
+            addNotification(
+              // @ts-ignore
+              {
+                uniqueId: r.data.taskId,
+                createdDate: new Date().toISOString(),
+                inProgress: true,
+              }
+            )
+          );
+          console.log('r', r);
+          setTaskId(r.data.taskId);
+          toggleShowUpdatingRcra();
+        }
       })
       .catch((r) => console.error(r));
   };

client/src/store/site.slice.ts

@@ -3,7 +3,7 @@ import { RcraSite } from 'components/RcraSite';
 import { htApiBaseQuery } from 'store/baseQuery';
 
 interface RcrainfoSiteSearch {
-  siteType: string;
+  siteType: 'designatedFacility' | 'generator' | 'transporter' | 'broker';
   siteId: string;
 }
 

server/Dockerfile

@@ -1,19 +1,16 @@
 # Builder
-FROM python:3.11.2-alpine3.17 as builder
+FROM python:3.11.6-alpine3.18 as builder
 LABEL maintainer="[email protected]"
-# Starting environment varibles
 ENV APP_DIRECTORY=/app/
 ENV PYTHONDONTWRITEBYTECODE 1
 ENV PYTHONUNBUFFERED 1
 WORKDIR $APP_DIRECTORY
 COPY requirements.txt ./
 RUN mkdir $APP_DIRECTORY/static
 RUN mkdir $APP_DIRECTORY/media
-# Setup virtual environment and install dependencies
 RUN python -m venv /opt/venv \
     && /opt/venv/bin/pip install --upgrade pip \
     && /opt/venv/bin/pip install --no-cache-dir --quiet -r requirements.txt
-# Copy app into image working directory
 COPY . .
 ENV VIRTUAL_ENV="/opt/venv"
 ENV PATH="$VIRTUAL_ENV/bin:$PATH"
@@ -27,7 +24,6 @@ RUN python manage.py makemigrations && \
     python manage.py loaddata dev_data.yaml
 CMD ["python", "manage.py", "runserver", "0.0.0.0:8000"]
 
-
 # Production
 FROM builder as production
 EXPOSE 8000

server/apps/core/exceptions.py

@@ -2,9 +2,18 @@
 from django.core.exceptions import ValidationError as DjangoValidationError
 from django.http import Http404
 from rest_framework import exceptions
+from rest_framework.exceptions import APIException
 from rest_framework.serializers import as_serializer_error
 from rest_framework.views import exception_handler
 
+from apps.sites.services.site_services import SiteServiceError
+
+
+class InternalServer500(APIException):
+    status_code = 500
+    default_detail = "Internal Server Error"
+    default_code = "internal_server_error"
+
 
 def haztrak_exception_handler(exc, context):
     """
@@ -16,14 +25,19 @@ def haztrak_exception_handler(exc, context):
     and the django-styleguide
     https://github.com/HackSoftware/Django-Styleguide#approach-1---use-drfs-default-exceptions-with-very-little-modifications
     """
-    if isinstance(exc, DjangoValidationError):
-        exc = exceptions.ValidationError(as_serializer_error(exc))
-
-    if isinstance(exc, PermissionDenied):
-        exc = exceptions.PermissionDenied()
-
-    if isinstance(exc, Http404):
-        exc = exceptions.NotFound()
+    match exc:
+        case DjangoValidationError():
+            exc = exceptions.ValidationError(as_serializer_error(exc))
+        case PermissionDenied():
+            exc = exceptions.PermissionDenied()
+        case Http404():
+            exc = exceptions.NotFound()
+        case KeyError():
+            exc = exceptions.ParseError()
+        case ValueError():
+            exc = InternalServer500()
+        case SiteServiceError():
+            exc = InternalServer500()
 
     response = exception_handler(exc, context)
 

server/apps/sites/admin.py

@@ -22,7 +22,7 @@ class SiteAdmin(admin.ModelAdmin):
     @admin.display(description="EPA Site")
     def related_handler(self, site: Site) -> str:
         url = (
-            reverse("admin:sites_epasite_changelist")
+            reverse("admin:sites_rcrasite_changelist")
             + "?"
             + urlencode({"epa_id": str(site.rcra_site.epa_id)})
         )

server/apps/sites/services/profile_services.py

@@ -46,7 +46,7 @@ def __repr__(self):
             f"<{self.__class__.__name__}(username='{self.username}', rcrainfo='{self.rcrainfo}')>"
         )
 
-    def pull_rcra_profile(self, *, username: Optional[str] = None) -> None:
+    def update_rcrainfo_profile(self, *, username: Optional[str] = None) -> None:
         """
         This high level function makes several requests to RCRAInfo to pull...
         1. A user's rcrainfo site permissions, it creates a RcraSitePermission for each

server/apps/sites/services/site_services.py

@@ -1,19 +1,25 @@
 import logging
-from typing import Dict, List, Optional
+from datetime import UTC, datetime
+from typing import Dict, Optional, TypedDict
 
-from django.core.cache import cache
+from django.core.cache import CacheKeyWarning, cache
 from django.db import transaction
 from rest_framework.exceptions import ValidationError
 
 from apps.core.services import RcrainfoService  # type: ignore
 from apps.sites.models import RcraSite, Site  # type: ignore
 from apps.sites.serializers import RcraSiteSerializer  # type: ignore
 from apps.trak.services import ManifestService  # type: ignore
-from apps.trak.services.manifest_service import PullManifestsResult  # type: ignore
+from apps.trak.services.manifest_services import PullManifestsResult, TaskResponse  # type: ignore
+from apps.trak.tasks import sync_site_manifests  # type: ignore
 
 logger = logging.getLogger(__name__)
 
 
+class HandlerSearchResults(TypedDict):
+    sites: list[RcraSite]
+
+
 class SiteServiceError(Exception):
     def __init__(self, message: str):
         super().__init__(message)
@@ -33,39 +39,38 @@ def __init__(
     ):
         self.username = username
         self.rcrainfo = rcrainfo or RcrainfoService(api_username=username)
-        if site_id:
-            self.site = Site.objects.get(rcra_site__epa_id=site_id)
+        self.site_id = site_id
 
-    def sync_rcra_manifest(self, *, site_id: Optional[str] = None) -> PullManifestsResult:
-        """
-        Retrieve a site's manifest from Rcrainfo and save to the database.
-
-        Keyword Args:
-            site_id (str): the epa_id to sync with RCRAInfo's manifest. Defaults self.site.
-        """
+    def sync_rcrainfo_manifest(self, *, site_id: Optional[str] = None) -> TaskResponse:
+        """Validate input and Launch a Celery task to sync a site's manifests from RCRAInfo"""
         logger.info(f"{self} sync rcra manifest, site ID {site_id}")
+        task = sync_site_manifests.delay(site_id=site_id, username=self.username)
+        return {"taskId": task.id}
+
+    @transaction.atomic
+    def sync_manifests(self, *, site_id: str) -> PullManifestsResult:
+        """Pull manifests and update the last sync date for a site"""
         try:
-            manifest_service = ManifestService(username=self.username, rcrainfo=self.rcrainfo)
-            site = Site.objects.get(rcra_site__epa_id=site_id or self.site)
-            logger.info(f"site: {site}, manifest_service: {manifest_service}")
-            tracking_numbers: List[str] = manifest_service.search_rcra_mtn(
-                site_id=site_id, start_date=site.last_rcra_sync
-            )
-            # ToDo: uncomment this after we have manifest development fixtures
-            # limit the number of manifest to sync at a time per RCRAInfo rate limits
-            tracking_numbers = tracking_numbers[:30]
-            logger.info(f"Pulling {tracking_numbers} from RCRAInfo")
-            results: PullManifestsResult = manifest_service.pull_manifests(
-                tracking_numbers=tracking_numbers
+            site = Site.objects.get(rcra_site__epa_id=site_id)
+            updated_mtn = self._get_updated_mtn(
+                site_id=site.rcra_site.epa_id, last_sync_date=site.last_rcra_sync
             )
-            # ToDo: uncomment this after we have manifest development fixtures
-            # site.last_rcra_sync = datetime.now().replace(tzinfo=timezone.utc)
+            updated_mtn = updated_mtn[:15]  # temporary limit to 15
+            logger.info(f"Pulling {updated_mtn} from RCRAInfo")
+            manifest = ManifestService(username=self.username, rcrainfo=self.rcrainfo)
+            results: PullManifestsResult = manifest.pull_manifests(tracking_numbers=updated_mtn)
+            site.last_rcra_sync = datetime.now(UTC)
             site.save()
             return results
         except Site.DoesNotExist:
             logger.warning(f"Site Does not exists {site_id}")
             raise SiteServiceError(f"Site Does not exists {site_id}")
 
+    def _get_updated_mtn(self, site_id: str, last_sync_date: datetime) -> list[str]:
+        logger.info(f"retrieving updated MTN for site {site_id}")
+        manifest = ManifestService(username=self.username, rcrainfo=self.rcrainfo)
+        return manifest.search_rcrainfo_mtn(site_id=site_id, start_date=last_sync_date)
+
     @transaction.atomic
     def create_or_update_site(
         self, *, rcra_site: RcraSite, site_name: Optional[str] = None
@@ -102,15 +107,10 @@ def __repr__(self):
             f"rcrainfo='{self.rcrainfo}')>"
         )
 
-    def pull_rcra_site(self, *, site_id: str) -> RcraSite:
-        """
-        Retrieve a site/rcra_site from Rcrainfo and return RcraSiteSerializer
-        """
+    def pull_rcrainfo_site(self, *, site_id: str) -> RcraSite:
+        """Retrieve a site/rcra_site from Rcrainfo and return RcraSiteSerializer"""
         rcra_site_data: Dict = self.rcrainfo.get_site(site_id).json()
-        rcra_site_serializer: RcraSiteSerializer = self._deserialize_rcra_site(
-            rcra_site_data=rcra_site_data
-        )
-        return self._create_or_update_rcra_site(rcra_site_data=rcra_site_serializer.validated_data)
+        return self._update_or_create_rcra_site_from_json(rcra_site_data=rcra_site_data)
 
     def get_or_pull_rcra_site(self, site_id: str) -> RcraSite:
         """
@@ -120,39 +120,29 @@ def get_or_pull_rcra_site(self, site_id: str) -> RcraSite:
         if RcraSite.objects.filter(epa_id=site_id).exists():
             logger.debug(f"using existing rcra_site {site_id}")
             return RcraSite.objects.get(epa_id=site_id)
-        new_rcra_site = self.pull_rcra_site(site_id=site_id)
+        new_rcra_site = self.pull_rcrainfo_site(site_id=site_id)
         logger.debug(f"pulled new rcra_site {new_rcra_site}")
         return new_rcra_site
 
-    def search_rcra_site(self, **search_parameters) -> dict:
+    def search_rcrainfo_handlers(self, **search_parameters) -> HandlerSearchResults:
         """
         Search RCRAInfo for a site by name or EPA ID
         """
+        cache_key = (
+            f'handlerSearch:epaSiteId:{search_parameters["epaSiteId"]}:siteType:'
+            f'{search_parameters["siteType"]}'
+        )
         try:
-            data = cache.get(f'{search_parameters["epaSiteId"]}-{search_parameters["siteType"]}')
+            data = cache.get(cache_key)
             if not data:
-                data = self.rcrainfo.search_sites(**search_parameters).json()
-                cache.set(
-                    f'{search_parameters["epaSiteId"]}-{search_parameters["siteType"]}',
-                    data,
-                    60 * 60 * 24,
-                )
+                data: HandlerSearchResults = self.rcrainfo.search_sites(**search_parameters).json()
+                cache.set(cache_key, data, 60 * 60 * 24)
             return data
-        except KeyError:
-            raise ValidationError("Missing required search parameters")
-
-    def _deserialize_rcra_site(self, *, rcra_site_data: dict) -> RcraSiteSerializer:
-        serializer = RcraSiteSerializer(data=rcra_site_data)
-        if serializer.is_valid():
-            return serializer
-        logger.error(serializer.errors)
-        raise ValidationError(serializer.errors)
+        except CacheKeyWarning:
+            raise ValidationError("Error retrieving data from cache")
 
     @transaction.atomic
-    def _create_or_update_rcra_site(self, *, rcra_site_data: dict) -> RcraSite:
-        epa_id = rcra_site_data.get("epa_id")
-        if RcraSite.objects.filter(epa_id=epa_id).exists():
-            rcra_site = RcraSite.objects.get(epa_id=epa_id)
-            return rcra_site
-        rcra_site = RcraSite.objects.save(**rcra_site_data)
-        return rcra_site
+    def _update_or_create_rcra_site_from_json(self, *, rcra_site_data: dict) -> RcraSite:
+        serializer = RcraSiteSerializer(data=rcra_site_data)
+        serializer.is_valid(raise_exception=True)
+        return serializer.save()

server/apps/sites/tasks/profile_tasks.py

@@ -26,7 +26,7 @@ def sync_user_sites(self: RcraProfileTasks, username: str) -> None:
 
     try:
         profile_service = RcraProfileService(username=username)
-        profile_service.pull_rcra_profile()
+        profile_service.update_rcrainfo_profile()
     except (ConnectionError, RequestException, TimeoutError):
         # ToDo retry if network error, see celery docs
         raise Reject()

server/apps/sites/tasks/site_tasks.py

@@ -12,7 +12,7 @@
 def get_rcra_site(self, *, site_id: str, username: str) -> str:
     try:
         site_service = RcraSiteService(username=username)
-        rcra_site = site_service.pull_rcra_site(site_id=site_id)
+        rcra_site = site_service.pull_rcrainfo_site(site_id=site_id)
         return rcra_site.epa_id
     except Exception as exc:
         self.update_state(state=states.FAILURE, meta=f"Internal Error {exc}")

server/apps/sites/tests/test_epa_site_views.py

@@ -15,20 +15,17 @@ class TestEpaSiteView:
     URL = "/api/rcra/handler"
 
     @pytest.fixture
-    def client(self, rcra_site_factory, api_client_factory):
+    def client(self, api_client_factory):
         return api_client_factory()
 
     @pytest.fixture
-    def generator(self, rcra_site_factory, api_client_factory):
+    def generator(self, rcra_site_factory):
         return rcra_site_factory()
 
     def test_endpoint_returns_json_with_rcra_site(self, client, generator):
         response: Response = client.get(f"{self.URL}/{generator.pk}")
         assert response.headers["Content-Type"] == "application/json"
         assert response.status_code == status.HTTP_200_OK
-
-    def test_returns_serialized_handler(self, client, generator):
-        response: Response = client.get(f"{self.URL}/{generator.pk}")
         assert response.data["epaSiteId"] == generator.epa_id
 
 

server/apps/trak/tests/services/test_handler_services.py → server/apps/sites/tests/test_handler_services.py

@@ -36,6 +36,6 @@ def test_pulls_site_details_from_rcrainfo(self, mock_responses):
             status=200,
         )
         # Act
-        rcra_site = handler_service.pull_rcra_site(site_id=self.epa_id)
+        rcra_site = handler_service.pull_rcrainfo_site(site_id=self.epa_id)
         # Assert
         assert isinstance(rcra_site, RcraSite)