Fix #1620 - Add last_accessed column to APIUser

basket/news/admin.py

@@ -55,7 +55,8 @@ class NewsletterGroupAdmin(admin.ModelAdmin):
 
 @admin.register(APIUser)
 class APIUserAdmin(admin.ModelAdmin):
-    list_display = ("name", "enabled")
+    list_display = ("name", "enabled", "created", "last_accessed")
+    readonly_fields = ("api_key", "created", "last_accessed")
 
 
 @admin.register(Newsletter)

basket/news/migrations/0002_apiuser_created_apiuser_last_accessed.py

@@ -0,0 +1,25 @@
+# Generated by Django 4.2.17 on 2025-01-14 23:41
+
+import datetime
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("news", "0001_squashed_0001_to_0032"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="apiuser",
+            name="created",
+            field=models.DateTimeField(auto_now_add=True, default=datetime.datetime(2025, 1, 1, 0, 0)),
+            preserve_default=False,
+        ),
+        migrations.AddField(
+            model_name="apiuser",
+            name="last_accessed",
+            field=models.DateTimeField(blank=True, null=True),
+        ),
+    ]

basket/news/models.py

@@ -6,6 +6,7 @@
 
 import sentry_sdk
 
+from basket import metrics
 from basket.base.rq import get_enqueue_kwargs, get_queue
 from basket.news.fields import LocaleField
 
@@ -140,6 +141,8 @@ class APIUser(models.Model):
     name = models.CharField(max_length=256, help_text="Descriptive name of this user")
     api_key = models.CharField(max_length=40, default=get_uuid, db_index=True)
     enabled = models.BooleanField(default=True)
+    created = models.DateTimeField(auto_now_add=True)
+    last_accessed = models.DateTimeField(null=True, blank=True)
 
     class Meta:
         verbose_name = "API User"
@@ -148,8 +151,28 @@ def __str__(self):  # pragma: no cover
         return f"{self.name} ({self.api_key})"
 
     @classmethod
-    def is_valid(cls, api_key):
-        return cls.objects.filter(api_key=api_key, enabled=True).exists()
+    def is_valid(cls, api_key: str) -> bool:
+        """
+        Checks if the API key is valid and enabled.
+
+        Updates the `last_accessed` field if the key is valid.
+
+        Returns:
+            bool: True if the API key is valid and enabled, False otherwise.
+
+        """
+        try:
+            obj = cls.objects.get(api_key=api_key)
+            if obj.enabled:
+                obj.last_accessed = now()
+                obj.save(update_fields=["last_accessed"])
+                metrics.incr("api.key.is_valid", tags=["value:true"])
+                return True
+        except APIUser.DoesNotExist:
+            pass
+
+        metrics.incr("api.key.is_valid", tags=["value:false"])
+        return False
 
 
 def _is_query_dict(arg):

basket/news/tests/test_models.py

@@ -3,6 +3,9 @@
 from django.test import TestCase
 from django.test.utils import override_settings
 
+import pytest
+
+from basket.base.utils import is_valid_uuid
 from basket.news import models
 
 
@@ -80,7 +83,8 @@ def test_retry(self, mock_enqueue, mock_random):
         assert mock_enqueue.call_args.kwargs["retry"].intervals == [60, 90]
 
 
-class NewsletterTest:
+@pytest.mark.django_db
+class TestNewsletter:
     def test_newsletter_strips_languages(self):
         n = models.Newsletter.objects.create(
             slug="slug",
@@ -89,3 +93,45 @@ def test_newsletter_strips_languages(self):
         )
         obj = models.Newsletter.objects.get(id=n.id)
         assert obj.languages == "en,fr,de"
+
+
+@pytest.mark.django_db
+class TestAPIUser:
+    def _add_api_user(self, name=None):
+        name = name or "The Dude"
+        return models.APIUser.objects.create(name=name)
+
+    def test_api_user(self):
+        user = self._add_api_user()
+        assert str(user) == f"{user.name} ({user.api_key})"
+        assert user.api_key is not None
+        assert is_valid_uuid(user.api_key)
+        assert user.enabled is True
+        assert user.created is not None
+        assert user.last_accessed is None
+
+    def test_api_user_generate_key_unique(self):
+        user1 = self._add_api_user()
+        user2 = self._add_api_user()
+        assert user1.api_key != user2.api_key
+
+    def test_api_is_valid(self, metricsmock):
+        user = self._add_api_user()
+        assert user.last_accessed is None
+        assert models.APIUser.is_valid(user.api_key) is True
+        metricsmock.assert_incr_once("api.key.is_valid", tags=["value:true"])
+        # Test `is_valid` also updates `last_accessed`.
+        user.refresh_from_db()
+        assert user.last_accessed is not None
+
+    def test_api_is_valid_disabled(self, metricsmock):
+        user = self._add_api_user()
+        user.enabled = False
+        user.save()
+        assert models.APIUser.is_valid(user.api_key) is False
+        metricsmock.assert_incr_once("api.key.is_valid", tags=["value:false"])
+        assert user.last_accessed is None
+
+    def test_api_is_valid_invalid(self, metricsmock):
+        assert models.APIUser.is_valid("invalid") is False
+        metricsmock.assert_incr_once("api.key.is_valid", tags=["value:false"])