Revert temporary upgrade to Django 5.2a1

This reverts commits cf7ed27.
and fabb4bc.

Author: bmispelon

.pre-commit-config.yaml

@@ -36,7 +36,6 @@ repos:
       rev: "1.22.2"
       hooks:
           - id: django-upgrade
-            # TODO: Update to 5.2 when django-upgrade supports it
             args: [--target-version, "5.1"]
     - repo: https://github.com/psf/black
       rev: 24.10.0

djangoproject/settings/common.py

@@ -172,6 +172,7 @@
 SESSION_COOKIE_HTTPONLY = True
 
 SILENCED_SYSTEM_CHECKS = [
+    "fields.W342",  # tracdb has ForeignKey(unique=True) in lieu of multi-col PKs
     "security.W008",  # SSL redirect is handled by nginx
     "security.W009",  # SECRET_KEY is setup through Ansible secrets
 ]

requirements/common.txt

@@ -7,7 +7,7 @@ django-push @ git+https://github.com/brutasse/django-push.git@22fda99641cfbd2f30
 django-read-only==1.18.0
 django-recaptcha==4.0.0
 django-registration-redux==2.13
-Django==5.2a1
+Django==5.1.5
 docutils==0.21.2
 feedparser==6.0.11
 Jinja2==3.1.5

tracdb/models.py

@@ -9,6 +9,38 @@
     * All the session and permission tables: they're just not needed.
     * Enum: I don't know what this is or what it's for.
     * NodeChange: Ditto.
+
+These models are far from perfect but are Good Enough(tm) to get some useful data out.
+
+One important mismatch between these models and the Trac database has to do with
+composite primary keys. Trac uses them for several tables, but Django does not support
+them yet (ticket #373).
+
+These are the tables that use them:
+
+    * ticket_custom (model TicketCustom)
+    * ticket_change (model TicketChange)
+    * wiki (model Wiki)
+    * attachment (model Attachment)
+
+To make these work with Django (for some definition of the word "work") we mark only
+one of their field as being the primary key (primary_key=True).
+This is obviously incorrect but — somewhat suprisingly — it doesn't break **everything**
+and the little that does actually work is good enough for what we're trying to do:
+
+    * Model.objects.create(...) correctly creates the object in the db
+    * Most queryset/manager methods work (in particular filter(), exclude(), all()
+      and count())
+
+On the other hand, here's what absolutely DOES NOT work (the list is sadly not
+exhaustive):
+
+    * Updating a model instance with save() will update ALL ROWS that happen to share
+      the value for the field used as the "fake" primary key if they exist (resulting
+      in a DBError)
+    * The admin won't work (the "pk" field shortcut can't be used reliably since it can
+      return multiple rows)
+
 """
 
 from datetime import date
@@ -129,11 +161,11 @@ def __str__(self):
 
 
 class TicketCustom(models.Model):
-    pk = models.CompositePrimaryKey("ticket", "name")
     ticket = models.ForeignKey(
         Ticket,
         related_name="custom_fields",
         db_column="ticket",
+        primary_key=True,  # XXX See note at the top about composite pk
         on_delete=models.DO_NOTHING,
     )
     name = models.TextField()
@@ -148,11 +180,11 @@ def __str__(self):
 
 
 class TicketChange(models.Model):
-    pk = models.CompositePrimaryKey("ticket", "_time", "field")
     ticket = models.ForeignKey(
         Ticket,
         related_name="changes",
         db_column="ticket",
+        primary_key=True,  # XXX See note at the top about composite pk
         on_delete=models.DO_NOTHING,
     )
     author = models.TextField()
@@ -260,8 +292,9 @@ def __str__(self):
 
 
 class Wiki(models.Model):
-    pk = models.CompositePrimaryKey("name", "version")
-    name = models.TextField()
+    name = models.TextField(
+        primary_key=True
+    )  # XXX See note at the top about composite pk
     version = models.IntegerField()
     _time = models.BigIntegerField(db_column="time")
     time = time_property("_time")
@@ -279,9 +312,10 @@ def __str__(self):
 
 
 class Attachment(models.Model):
-    pk = models.CompositePrimaryKey("type", "id", "filename")
     type = models.TextField()
-    id = models.TextField()
+    id = models.TextField(
+        primary_key=True
+    )  # XXX See note at the top about composite pk
     filename = models.TextField()
     size = models.IntegerField()
     _time = models.BigIntegerField(db_column="time")

tracdb/testutils.py

@@ -1,5 +1,60 @@
+from copy import deepcopy
+
 from django.apps import apps
-from django.db import connections
+from django.db import connections, models
+
+# There's more models with a fake composite pk, but we don't test them at the moment.
+_MODELS_WITH_FAKE_COMPOSITE_PK = {"ticketcustom"}
+
+
+def _get_pk_field(model):
+    """
+    Return the pk field for the given model.
+    Raise ValueError if none or more than one is found.
+    """
+    pks = [field for field in model._meta.get_fields() if field.primary_key]
+    if len(pks) == 0:
+        raise ValueError(f"No primary key field found for model {model._meta.label}")
+    elif len(pks) > 1:
+        raise ValueError(
+            f"Found more than one primary key field for model {model._meta.label}"
+        )
+    else:
+        return pks[0]
+
+
+def _replace_primary_key_field_with_autofield(model, schema_editor):
+    """
+    See section about composite pks in the docstring for models.py to get some context
+    for this.
+
+    In short, some models define a field as `primary_key=True` but that field is not
+    actually a primary key. In particular that field is not supposed to be unique, which
+    interferes with our tests.
+
+    For those models, we remove the `primary_key` flag from the field, and we add a
+    new `testid` autofield. This makes the models easier to manipulate in the tests.
+    """
+    old_pk_field = _get_pk_field(model)
+    del old_pk_field.unique
+    new_pk_field = deepcopy(old_pk_field)
+    new_pk_field.primary_key = False
+    schema_editor.alter_field(
+        model=model, old_field=old_pk_field, new_field=new_pk_field
+    )
+
+    autofield = models.AutoField(primary_key=True)
+    autofield.set_attributes_from_name("testid")
+    schema_editor.add_field(model=model, field=autofield)
+
+
+def _create_db_table_for_model(model, schema_editor):
+    """
+    Use the schema editor API to create the db table for the given (unmanaged) model.
+    """
+    schema_editor.create_model(model)
+    if model._meta.model_name in _MODELS_WITH_FAKE_COMPOSITE_PK:
+        _replace_primary_key_field_with_autofield(model, schema_editor)
 
 
 def create_db_tables_for_unmanaged_models(schema_editor):
@@ -10,7 +65,7 @@ def create_db_tables_for_unmanaged_models(schema_editor):
     for model in appconfig.get_models():
         if model._meta.managed:
             continue
-        schema_editor.create_model(model)
+        _create_db_table_for_model(model, schema_editor)
 
 
 def destroy_db_tables_for_unmanaged_models(schema_editor):