Merge pull request #306 from ropable/master

Better error handling in email functions, make vehicle registration number readonly in admin
dbca-wa · Nov 6, 2024 · 3280dd5 · 3280dd5
2 parents 0d29aaf + 54e7e23
commit 3280dd5
Show file tree

Hide file tree

Showing 9 changed files with 373 additions and 333 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -8,4 +8,4 @@ repos:
       # For running trufflehog in docker, use the following entry instead:
       # entry: bash -c 'docker run --rm -v "$(pwd):/workdir" -i --rm trufflesecurity/trufflehog:latest git file:///workdir --since-commit HEAD --only-verified --fail'
       language: system
-      stages: ["commit", "push"]
+      stages: ["pre-commit", "pre-push"]
diff --git a/kustomize/overlays/prod/kustomization.yaml b/kustomize/overlays/prod/kustomization.yaml
@@ -16,6 +16,8 @@ secretGenerator:
     type: Opaque
     envs:
       - .env
+generatorOptions:
+  disableNameSuffixHash: true
 labels:
   - includeSelectors: true
     pairs:
@@ -26,4 +28,4 @@ patches:
   - path: service_patch.yaml
 images:
   - name: ghcr.io/dbca-wa/resource_tracking
-    newTag: 1.4.18
+    newTag: 1.4.19
diff --git a/kustomize/overlays/uat/kustomization.yaml b/kustomize/overlays/uat/kustomization.yaml
@@ -7,6 +7,7 @@ resources:
   - cronjobs/harvest-email-dplus
   - cronjobs/harvest-email-iriditrak
   - cronjobs/harvest-email-mp70
+  # - cronjobs/harvest-email-spot
   - cronjobs/harvest-tracplus
   - ingress.yaml
   - pdb.yaml
@@ -15,6 +16,8 @@ secretGenerator:
     type: Opaque
     envs:
       - .env
+generatorOptions:
+  disableNameSuffixHash: true
 labels:
   - includeSelectors: true
     pairs:

diff --git a/poetry.lock b/poetry.lock
diff --git a/pyproject.toml b/pyproject.toml
@@ -1,47 +1,47 @@
-[tool.poetry]
-name = "resource_tracking"
-version = "1.4.18"
-description = "DBCA internal corporate application to download and serve data from remote tracking devices."
-authors = ["DBCA OIM <[email protected]>"]
-license = "Apache-2.0"
-package-mode = false
-
-[tool.poetry.dependencies]
-python = "^3.12"
-django = "4.2.16"
-psycopg = { version = "3.2.2", extras = ["binary", "pool"] }
-dbca-utils = "2.0.2"
-python-dotenv = "1.0.1"
-dj-database-url = "2.2.0"
-gunicorn = "23.0.0"
-django-extensions = "3.2.3"
-django-tastypie = "0.14.7"
-django-geojson = "4.1.0"
-unicodecsv = "0.14.1"
-whitenoise = { version = "6.7.0", extras = ["brotli"] }
-azure-storage-blob = "12.23.0"
-sentry-sdk = { version = "2.14.0", extras = ["django"] }
-
-[tool.poetry.group.dev.dependencies]
-ipython = "^8.27.0"
-ipdb = "^0.13.13"
-pre-commit = "^3.8.0"
-mixer = "^7.2.2"
-
-# Reference: https://docs.astral.sh/ruff/configuration/
-[tool.ruff]
-line-length = 120
-
-[tool.ruff.lint]
-ignore = [
-  "E501", # Line too long
-  "E722", # Bare except
-]
-
-# Reference: https://www.djlint.com/docs/configuration/
-[tool.djlint]
-profile = "django"
-
-[build-system]
-requires = ["poetry-core"]
-build-backend = "poetry.masonry.api"
+[tool.poetry]
+name = "resource_tracking"
+version = "1.4.19"
+description = "DBCA internal corporate application to download and serve data from remote tracking devices."
+authors = ["DBCA OIM <[email protected]>"]
+license = "Apache-2.0"
+package-mode = false
+
+[tool.poetry.dependencies]
+python = "^3.12"
+django = "4.2.16"
+psycopg = { version = "3.2.3", extras = ["binary", "pool"] }
+dbca-utils = "2.0.2"
+python-dotenv = "1.0.1"
+dj-database-url = "2.2.0"
+gunicorn = "23.0.0"
+django-extensions = "3.2.3"
+django-tastypie = "0.14.7"
+django-geojson = "4.1.0"
+unicodecsv = "0.14.1"
+whitenoise = { version = "6.7.0", extras = ["brotli"] }
+azure-storage-blob = "12.23.1"
+sentry-sdk = {version = "2.18.0", extras = ["django"]}
+
+[tool.poetry.group.dev.dependencies]
+ipython = "^8.29.0"
+ipdb = "^0.13.13"
+pre-commit = "^4.0.1"
+mixer = "^7.2.2"
+
+# Reference: https://docs.astral.sh/ruff/configuration/
+[tool.ruff]
+line-length = 120
+
+[tool.ruff.lint]
+ignore = [
+  "E501", # Line too long
+  "E722", # Bare except
+]
+
+# Reference: https://www.djlint.com/docs/configuration/
+[tool.djlint]
+profile = "django"
+
+[build-system]
+requires = ["poetry-core"]
+build-backend = "poetry.masonry.api"
diff --git a/tracking/admin.py b/tracking/admin.py
@@ -1,7 +1,8 @@
 from django.conf import settings
-from django.contrib.admin import ModelAdmin, register, AdminSite
-from django.urls import reverse
+from django.contrib.admin import AdminSite, ModelAdmin, register
 from django.http import HttpResponseRedirect
+from django.urls import reverse
+
 from .models import Device
 
 
@@ -10,40 +11,47 @@ class DeviceAdmin(ModelAdmin):
     actions = None
     date_hierarchy = "seen"
     list_display = (
-        "id", "deviceid", "source_device_type", "registration", "callsign", "rin_display", "symbol", "district_display",
-        "seen", "hidden", "internal_only",
+        "id",
+        "deviceid",
+        "source_device_type",
+        "registration",
+        "callsign",
+        "rin_display",
+        "symbol",
+        "district_display",
+        "seen",
+        "hidden",
+        "internal_only",
     )
     list_filter = ("symbol", "district", "source_device_type", "hidden", "internal_only")
     search_fields = ("deviceid", "registration", "callsign_display", "rin_display", "symbol", "district_display")
-    readonly_fields = ("deviceid", "source_device_type", "seen", "point")
+    readonly_fields = ("deviceid", "source_device_type", "seen", "point", "registration")
     fieldsets = (
-        ("Vehicle/Device details", {
-            "description": """<p class="errornote">This is the live tracking database;
+        (
+            "Vehicle/Device details",
+            {
+                "description": """<p class="errornote">This is the live tracking database;
             changes made to these fields will apply to the Device Tracking map in all
             variants of the Spatial Support System.</p>
-            """ if settings.PROD_SCARY_WARNING else "",
-            "fields": (
-                "deviceid",
-                "source_device_type",
-                "seen",
-                "point",
-                "district",
-                "symbol",
-                "callsign",
-                "registration",
-                "rin_number",
-                "fire_use",
-            ),
-        }),
-        ("Crew Details", {
-            "fields": ("current_driver", "usual_driver", "usual_location")
-        }),
-        ("Contractor Details", {
-            "fields": ("contractor_details",)
-        }),
-        ("Other Details", {
-            "fields": ("other_details", "internal_only", "hidden")
-        })
+            """
+                if settings.PROD_SCARY_WARNING
+                else "",
+                "fields": (
+                    "deviceid",
+                    "source_device_type",
+                    "registration",
+                    "seen",
+                    "district",
+                    "symbol",
+                    "callsign",
+                    "rin_number",
+                    "fire_use",
+                ),
+            },
+        ),
+        ("Crew Details", {"fields": ("current_driver", "usual_driver", "usual_location")}),
+        ("Contractor Details", {"fields": ("contractor_details",)}),
+        ("Other Details", {"fields": ("other_details", "internal_only", "hidden")}),
     )
 
     def has_delete_permission(self, request, obj=None):
@@ -61,7 +69,6 @@ def has_change_permission(self, request, obj=None):
 
 
 class DeviceSSSAdmin(DeviceAdmin):
-
     def add_view(self, request, obj=None):
         return HttpResponseRedirect(reverse("sss_admin:tracking_device_changelist"))
 

diff --git a/tracking/email_utils.py b/tracking/email_utils.py
@@ -20,7 +20,7 @@ def get_imap(mailbox="INBOX"):
 
 
 def email_get_unread(imap, from_email_address):
-    """Returns (status, list of UIDs) of unread emails from a sending email address."""
+    """Returns (status, [list of UIDs]) of unread emails from a sending email address."""
     search = '(UNSEEN UNFLAGGED FROM "{}")'.format(from_email_address)
     try:
         status, response = imap.search(None, search)

diff --git a/tracking/harvest.py b/tracking/harvest.py
@@ -33,22 +33,30 @@ def harvest_tracking_email(device_type, purge_email=False):
     start = timezone.now()
     created = 0
     flagged = 0
+    unread_emails = None
 
     if device_type == "iriditrak":
         LOGGER.info("Harvesting IridiTRAK emails")
-        status, uids = email_utils.email_get_unread(imap, settings.EMAIL_IRIDITRAK)
+        unread_emails = email_utils.email_get_unread(imap, settings.EMAIL_IRIDITRAK)
     elif device_type == "dplus":
         LOGGER.info("Harvesting DPlus emails")
-        status, uids = email_utils.email_get_unread(imap, settings.EMAIL_DPLUS)
+        unread_emails = email_utils.email_get_unread(imap, settings.EMAIL_DPLUS)
     elif device_type == "spot":
         LOGGER.info("Harvesting Spot emails")
-        status, uids = email_utils.email_get_unread(imap, settings.EMAIL_SPOT)
+        unread_emails = email_utils.email_get_unread(imap, settings.EMAIL_SPOT)
     elif device_type == "mp70":
         LOGGER.info("Harvesting MP70 emails")
-        status, uids = email_utils.email_get_unread(imap, settings.EMAIL_MP70)
+        unread_emails = email_utils.email_get_unread(imap, settings.EMAIL_MP70)
 
+    if not unread_emails:
+        LOGGER.warning("Mail server status failure")
+        return
+
+    status, uids = unread_emails
     if status != "OK":
-        LOGGER.error(f"Server response failure: {status}")
+        LOGGER.warning(f"Mail server status failure: {status}")
+        return
+
     LOGGER.info(f"Server lists {len(uids)} unread emails")
 
     if uids:
@@ -58,9 +66,14 @@ def harvest_tracking_email(device_type, purge_email=False):
                 uid = uid.decode("utf-8")
 
             # Fetch the email message.
-            status, message = email_utils.email_fetch(imap, uid)
+            email_message = email_utils.email_fetch(imap, uid)
+            if not email_message:
+                LOGGER.warning(f"Mail server status failure on fetching email UID {uid}")
+                continue
+
+            status, message = email_message
             if status != "OK":
-                LOGGER.error(f"Server response failure on fetching email UID {uid}: {status}")
+                LOGGER.warning(f"Mail server status failure on fetching email UID {uid}: {status}")
                 continue
 
             # `result` will be a LoggedPoint, or None
@@ -80,8 +93,8 @@ def harvest_tracking_email(device_type, purge_email=False):
 
             # Optionally mark email as read and flag it for deletion.
             if purge_email:
-                status, response = email_utils.email_mark_read(imap, uid)
-                status, response = email_utils.email_delete(imap, uid)
+                email_utils.email_mark_read(imap, uid)
+                email_utils.email_delete(imap, uid)
 
     LOGGER.info(f"Created {created} tracking points, flagged {flagged} emails")
 
@@ -120,9 +133,9 @@ def save_mp70(message):
 
     try:
         device, created = Device.objects.get_or_create(deviceid=data["device_id"])
-    except:
-        LOGGER.error("Exception during creation/query of MP70 device")
-        LOGGER.error(data)
+    except Exception as e:
+        LOGGER.warning(f"Exception during creation/query of MP70 device: {data}")
+        LOGGER.error(e)
         return False
 
     seen = data["timestamp"]
@@ -168,9 +181,9 @@ def save_spot(message):
 
     try:
         device, created = Device.objects.get_or_create(deviceid=data["device_id"])
-    except:
-        LOGGER.error("Exception during creation/query of Spot device")
-        LOGGER.error(data)
+    except Exception as e:
+        LOGGER.warning(f"Exception during creation/query of Spot device: {data}")
+        LOGGER.error(e)
         return False
 
     seen = data["timestamp"]
@@ -216,9 +229,9 @@ def save_iriditrak(message):
 
     try:
         device, created = Device.objects.get_or_create(deviceid=data["device_id"])
-    except:
-        LOGGER.error("Exception during creation/query of Iriditrak device")
-        LOGGER.error(data)
+    except Exception as e:
+        LOGGER.warning(f"Exception during creation/query of Iriditrak device: {data}")
+        LOGGER.error(e)
         return False
 
     seen = data["timestamp"]
@@ -266,9 +279,9 @@ def save_dplus(message):
 
     try:
         device, created = Device.objects.get_or_create(deviceid=data["device_id"])
-    except:
-        LOGGER.error("Exception during creation/query of DPlus device")
-        LOGGER.error(data)
+    except Exception as e:
+        LOGGER.warning(f"Exception during creation/query of DPlus device: {data}")
+        LOGGER.error(e)
         return False
 
     seen = data["timestamp"]
@@ -373,9 +386,9 @@ def save_dfes_feed():
 
         try:
             device, created = Device.objects.get_or_create(deviceid=data["device_id"])
-        except:
-            LOGGER.error("Exception during creation/query of DFES device")
-            LOGGER.error(data)
+        except Exception as e:
+            LOGGER.warning(f"Exception during creation/query of DFES device: {data}")
+            LOGGER.error(e)
             continue
 
         properties = feature["properties"]
@@ -466,9 +479,9 @@ def save_tracplus_feed():
 
         try:
             device, created = Device.objects.get_or_create(deviceid=data["device_id"])
-        except:
-            LOGGER.error("Exception during creation/query of TracPlus device")
-            LOGGER.error(row)
+        except Exception as e:
+            LOGGER.warning(f"Exception during creation/query of TracPlus device: {row}")
+            LOGGER.error(e)
             skipped_device += 1
             continue
 

diff --git a/tracking/models.py b/tracking/models.py
@@ -134,7 +134,7 @@ class Device(models.Model):
         verbose_name="Device ID",
         help_text="Device unique identifier",
     )
-    registration = models.CharField(max_length=32, default="No Rego", help_text="e.g. 1QBB157")
+    registration = models.CharField(max_length=32, default="No Rego")
     rin_number = models.PositiveIntegerField(
         validators=[MaxValueValidator(999)],
         verbose_name="Resource Identification Number (RIN)",