reorganize changes module into overlay and operations

src/c3nav/editor/api/endpoints.py

@@ -9,7 +9,7 @@ from c3nav.editor.api.base import api_etag_with_update_cache_key
 from c3nav.editor.api.geometries import get_level_geometries_result, get_space_geometries_result
 from c3nav.editor.api.schemas import EditorGeometriesElemSchema, EditorID, GeometryStylesSchema, UpdateCacheKey, \
     EditorBeaconsLookup
-from c3nav.editor.views.base import editor_etag_func, use_changeset_mapdata
+from c3nav.editor.views.base import editor_etag_func, accesses_mapdata
 from c3nav.mapdata.api.base import api_etag
 from c3nav.mapdata.models import Source
 from c3nav.mapdata.schemas.responses import WithBoundsSchema
@@ -63,7 +63,7 @@ def geometrystyles(request):
                                  **auth_permission_responses},
                        openapi_extra={"security": [{"APIKeyAuth": ["editor_access"]}]})
 @api_etag_with_update_cache_key(etag_func=editor_etag_func)
-@use_changeset_mapdata
+@accesses_mapdata
 def space_geometries(request, space_id: EditorID, update_cache_key: UpdateCacheKey = None, **kwargs):
     # newapi_etag_with_update_cache_key does the following, don't let it confuse you:
     # - update_cache_key becomes the actual update_cache_key, not the one supplied be the user
@@ -83,7 +83,7 @@ def space_geometries(request, space_id: EditorID, update_cache_key: UpdateCacheK
                                  **auth_permission_responses},
                        openapi_extra={"security": [{"APIKeyAuth": ["editor_access"]}]})
 @api_etag_with_update_cache_key(etag_func=editor_etag_func)
-@use_changeset_mapdata
+@accesses_mapdata
 def level_geometries(request, level_id: EditorID, update_cache_key: UpdateCacheKey = None, **kwargs):
     # newapi_etag_with_update_cache_key does the following, don't let it confuse you:
     # - update_cache_key becomes the actual update_cache_key, not the one supplied be the user

src/c3nav/editor/apps.py

@@ -2,17 +2,16 @@ from django.apps import AppConfig
 from django.contrib.auth import user_logged_in
 from django.db.models.signals import m2m_changed, post_delete, post_save, pre_save, pre_delete
 
-from c3nav.editor import changes
-
 
 class EditorConfig(AppConfig):
     name = 'c3nav.editor'
 
     def ready(self):
         from c3nav.editor.signals import set_changeset_author_on_login
-        pre_save.connect(changes.handle_pre_change_instance)
+        from c3nav.editor import overlay
-        pre_delete.connect(changes.handle_pre_change_instance)
+        pre_save.connect(overlay.handle_pre_change_instance)
-        post_save.connect(changes.handle_post_save)
+        pre_delete.connect(overlay.handle_pre_change_instance)
-        post_delete.connect(changes.handle_post_delete)
+        post_save.connect(overlay.handle_post_save)
-        m2m_changed.connect(changes.handle_m2m_changed)
+        post_delete.connect(overlay.handle_post_delete)
+        m2m_changed.connect(overlay.handle_m2m_changed)
         user_logged_in.connect(set_changeset_author_on_login)

src/c3nav/editor/migrations/0004_changeset_rewrite_2024.py

@@ -1,12 +1,10 @@
-# Generated by Django 5.0.8 on 2024-08-22 17:03
+# Generated by Django 5.0.8 on 2024-08-26 09:46
-import c3nav.editor.changes
+
-import c3nav.editor.models.changeset
+import c3nav.editor.operations
 import django.core.serializers.json
 import django_pydantic_field.fields
 from django.db import migrations
 
-import c3nav.editor.changes
-
 
 class Migration(migrations.Migration):
 
@@ -22,7 +20,7 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name='changeset',
             name='changes',
-            field=django_pydantic_field.fields.PydanticSchemaField(config=None, default=c3nav.editor.changes.ChangeSetChanges, encoder=django.core.serializers.json.DjangoJSONEncoder, schema=c3nav.editor.changes.ChangeSetChanges),
+            field=django_pydantic_field.fields.PydanticSchemaField(config=None, default=c3nav.editor.operations.CollectedChanges, encoder=django.core.serializers.json.DjangoJSONEncoder, schema=c3nav.editor.operations.CollectedChanges),
         ),
         migrations.DeleteModel(
             name='ChangedObject',

src/c3nav/editor/models/changeset.py

@@ -13,7 +13,7 @@ from django.utils.translation import gettext_lazy as _
 from django.utils.translation import ngettext_lazy
 from django_pydantic_field import SchemaField
 
-from c3nav.editor.changes import ChangeSetChanges
+from c3nav.editor.operations import CollectedChanges
 from c3nav.editor.tasks import send_changeset_proposed_notification
 from c3nav.mapdata.models import LocationSlug, MapUpdate
 from c3nav.mapdata.models.locations import LocationRedirect
@@ -45,7 +45,7 @@ class ChangeSet(models.Model):
                                     related_name='assigned_changesets', verbose_name=_('assigned to'))
     map_update = models.OneToOneField(MapUpdate, null=True, related_name='changeset',
                                       verbose_name=_('map update'), on_delete=models.PROTECT)
-    changes: ChangeSetChanges = SchemaField(schema=ChangeSetChanges, default=ChangeSetChanges)
+    changes: CollectedChanges = SchemaField(schema=CollectedChanges, default=CollectedChanges)
 
     class Meta:
         verbose_name = _('Change Set')

src/c3nav/editor/operations.py

@@ -0,0 +1,74 @@
+import datetime
+from typing import TypeAlias, Any, Annotated, Literal, Union
+
+from django.db import models
+from django.utils import timezone
+from pydantic.config import ConfigDict
+from pydantic.fields import Field
+from pydantic.types import Discriminator
+
+from c3nav.api.schema import BaseSchema
+
+FieldValuesDict: TypeAlias = dict[str, Any]
+
+
+class ObjectReference(BaseSchema):
+    model_config = ConfigDict(frozen=True)
+    model: str
+    id: int
+
+    @classmethod
+    def from_instance(cls, instance: models.Model):
+        """
+        This method will not convert the ID yet!
+        """
+        return cls(model=instance._meta.model_name, id=instance.pk)
+
+
+class BaseOperation(BaseSchema):
+    obj: ObjectReference
+    datetime: Annotated[datetime.datetime, Field(default_factory=timezone.now)]
+
+
+class CreateObjectOperation(BaseOperation):
+    type: Literal["create"] = "create"
+    fields: FieldValuesDict
+
+
+class UpdateObjectOperation(BaseOperation):
+    type: Literal["update"] = "update"
+    fields: FieldValuesDict
+
+
+class DeleteObjectOperation(BaseOperation):
+    type: Literal["delete"] = "delete"
+
+
+class UpdateManyToManyOperation(BaseOperation):
+    type: Literal["m2m_add"] = "m2m_update"
+    field: str
+    add_values: set[int] = set()
+    remove_values: set[int] = set()
+
+
+class ClearManyToManyOperation(BaseOperation):
+    type: Literal["m2m_clear"] = "m2m_clear"
+    field: str
+
+
+DatabaseOperation = Annotated[
+    Union[
+        CreateObjectOperation,
+        UpdateObjectOperation,
+        DeleteObjectOperation,
+        UpdateManyToManyOperation,
+        ClearManyToManyOperation,
+    ],
+    Discriminator("type"),
+]
+
+
+class CollectedChanges(BaseSchema):
+    prev_reprs: dict[ObjectReference, str] = {}
+    prev_values: dict[ObjectReference, FieldValuesDict] = {}
+    operations: list[DatabaseOperation] = []

src/c3nav/editor/overlay.py

@@ -1,18 +1,15 @@
-import datetime
 import json
 from contextlib import contextmanager
 from dataclasses import dataclass, field
-from typing import TypeAlias, Literal, Annotated, Union, Type, Any
+from typing import Type
 
 from django.core import serializers
 from django.db import transaction
 from django.db.models import Model
-from django.db.models.fields.related import OneToOneField, ForeignKey, ManyToManyField
+from django.db.models.fields.related import ManyToManyField
-from django.utils import timezone
-from pydantic import ConfigDict, Discriminator
-from pydantic.fields import Field
 
-from c3nav.api.schema import BaseSchema
+from c3nav.editor.operations import DatabaseOperation, ObjectReference, FieldValuesDict, CreateObjectOperation, \
+    UpdateObjectOperation, DeleteObjectOperation, ClearManyToManyOperation, UpdateManyToManyOperation, CollectedChanges
 
 try:
     from asgiref.local import Local as LocalContext
@@ -20,71 +17,6 @@ except ImportError:
     from threading import local as LocalContext
 
 
-FieldValuesDict: TypeAlias = dict[str, Any]
-
-
-class ObjectReference(BaseSchema):
-    model_config = ConfigDict(frozen=True)
-    model: str
-    id: int
-
-    @classmethod
-    def from_instance(cls, instance: Model):
-        """
-        This method will not convert the ID yet!
-        """
-        return cls(model=instance._meta.model_name, id=instance.pk)
-
-
-class BaseChange(BaseSchema):
-    obj: ObjectReference
-    datetime: Annotated[datetime.datetime, Field(default_factory=timezone.now)]
-
-
-class CreateObjectChange(BaseChange):
-    type: Literal["create"] = "create"
-    fields: FieldValuesDict
-
-
-class UpdateObjectChange(BaseChange):
-    type: Literal["update"] = "update"
-    fields: FieldValuesDict
-
-
-class DeleteObjectChange(BaseChange):
-    type: Literal["delete"] = "delete"
-
-
-class UpdateManyToManyChange(BaseSchema):
-    type: Literal["m2m_add"] = "m2m_update"
-    field: str
-    add_values: set[int] = set()
-    remove_values: set[int] = set()
-
-
-class ClearManyToManyChange(BaseSchema):
-    type: Literal["m2m_clear"] = "m2m_clear"
-    field: str
-
-
-ChangeSetChange = Annotated[
-    Union[
-        CreateObjectChange,
-        UpdateObjectChange,
-        DeleteObjectChange,
-        UpdateManyToManyChange,
-        ClearManyToManyChange,
-    ],
-    Discriminator("type"),
-]
-
-
-class ChangeSetChanges(BaseSchema):
-    prev_reprs: dict[ObjectReference, str] = {}
-    prev_values: dict[ObjectReference, FieldValuesDict] = {}
-    changes: list[ChangeSetChange] = []
-
-
 overlay_state = LocalContext()
 
 
@@ -92,28 +24,32 @@ class InterceptAbortTransaction(Exception):
     pass
 
 
-@contextmanager
-def enable_changeset_overlay(changeset):
-    try:
-        with transaction.atomic():
-            manager = ChangesetOverlayManager(changeset.changes)
-            overlay_state.manager = manager
-            # todo: apply changes so far
-            yield
-            raise InterceptAbortTransaction
-    except InterceptAbortTransaction:
-        if manager:
-            print(manager.new_changes)
-    finally:
-        overlay_state.manager = None
-
-
 @dataclass
-class ChangesetOverlayManager:
+class DatabaseOverlayManager:
-    changes: ChangeSetChanges
+    changes: CollectedChanges
-    new_changes: list[ChangeSetChange] = field(default_factory=list)
+    new_operations: list[DatabaseOperation] = field(default_factory=list)
     pre_change_values: dict[ObjectReference, FieldValuesDict] = field(default_factory=dict)
 
+    @classmethod
+    @contextmanager
+    def enable(cls, changes: CollectedChanges | None, commit: bool):
+        if getattr(overlay_state, 'manager', None) is not None:
+            raise TypeError
+        if changes is None:
+            changes = CollectedChanges()
+        try:
+            with transaction.atomic():
+                manager = DatabaseOverlayManager(changes)
+                overlay_state.manager = manager
+                # todo: apply changes so far
+                yield manager
+                if not commit:
+                    raise InterceptAbortTransaction
+        except InterceptAbortTransaction:
+            pass
+        finally:
+            overlay_state.manager = None
+
     @staticmethod
     def get_model_field_values(instance: Model) -> FieldValuesDict:
         return json.loads(serializers.serialize("json", [instance]))[0]["fields"]
@@ -143,17 +79,17 @@ class ChangesetOverlayManager:
         ref = self.get_ref_and_pre_change_values(instance)
 
         if created:
-            self.new_changes.append(CreateObjectChange(obj=ref, fields=field_values))
+            self.new_operations.append(CreateObjectOperation(obj=ref, fields=field_values))
             return
 
         if update_fields:
             field_values = {name: value for name, value in field_values.items() if name in update_fields}
 
-        self.new_changes.append(UpdateObjectChange(obj=ref, fields=field_values))
+        self.new_operations.append(UpdateObjectOperation(obj=ref, fields=field_values))
 
     def handle_post_delete(self, instance: Model, **kwargs):
         ref = self.get_ref_and_pre_change_values(instance)
-        self.new_changes.append(DeleteObjectChange(obj=ref))
+        self.new_operations.append(DeleteObjectOperation(obj=ref))
 
     def handle_m2m_changed(self, sender: Type[Model], instance: Model, action: str, model: Type[Model],
                            pk_set: set | None, reverse: bool, **kwargs):
@@ -165,7 +101,6 @@ class ChangesetOverlayManager:
 
         for field in instance._meta.get_fields():
             if isinstance(field, ManyToManyField) and field.remote_field.through == sender:
-                print("this is it!", field)
                 break
         else:
             raise ValueError
@@ -173,12 +108,12 @@ class ChangesetOverlayManager:
         ref = self.get_ref_and_pre_change_values(instance)
 
         if action == "post_clear":
-            self.new_changes.append(ClearManyToManyChange(obj=ref, field=field.name))
+            self.new_operations.append(ClearManyToManyOperation(obj=ref, field=field.name))
             return
 
-        if self.new_changes:
+        if self.new_operations:
-            last_change = self.new_changes[-1]
+            last_change = self.new_operations[-1]
-            if isinstance(last_change, UpdateManyToManyChange) and last_change == ref and last_change == field.name:
+            if isinstance(last_change, UpdateManyToManyOperation) and last_change == ref and last_change == field.name:
                 if action == "post_add":
                     last_change.add_values.update(pk_set)
                     last_change.remove_values.difference_update(pk_set)
@@ -188,15 +123,15 @@ class ChangesetOverlayManager:
                 return
 
         if action == "post_add":
-            self.new_changes.append(UpdateManyToManyChange(obj=ref, field=field.name, add_values=list(pk_set)))
+            self.new_operations.append(UpdateManyToManyOperation(obj=ref, field=field.name, add_values=list(pk_set)))
         else:
-            self.new_changes.append(UpdateManyToManyChange(obj=ref, field=field.name, remove_values=list(pk_set)))
+            self.new_operations.append(UpdateManyToManyOperation(obj=ref, field=field.name, remove_values=list(pk_set)))
 
 
 def handle_pre_change_instance(sender: Type[Model], **kwargs):
     if sender._meta.app_label != 'mapdata':
         return
-    manager: ChangesetOverlayManager = getattr(overlay_state, 'manager', None)
+    manager: DatabaseOverlayManager = getattr(overlay_state, 'manager', None)
     if manager:
         manager.handle_pre_change_instance(sender=sender, **kwargs)
 
@@ -204,7 +139,7 @@ def handle_pre_change_instance(sender: Type[Model], **kwargs):
 def handle_post_save(sender: Type[Model], **kwargs):
     if sender._meta.app_label != 'mapdata':
         return
-    manager: ChangesetOverlayManager = getattr(overlay_state, 'manager', None)
+    manager: DatabaseOverlayManager = getattr(overlay_state, 'manager', None)
     if manager:
         manager.handle_post_save(sender=sender, **kwargs)
 
@@ -212,7 +147,7 @@ def handle_post_save(sender: Type[Model], **kwargs):
 def handle_post_delete(sender: Type[Model], **kwargs):
     if sender._meta.app_label != 'mapdata':
         return
-    manager: ChangesetOverlayManager = getattr(overlay_state, 'manager', None)
+    manager: DatabaseOverlayManager = getattr(overlay_state, 'manager', None)
     if manager:
         manager.handle_post_delete(sender=sender, **kwargs)
 
@@ -220,6 +155,6 @@ def handle_post_delete(sender: Type[Model], **kwargs):
 def handle_m2m_changed(sender: Type[Model], **kwargs):
     if sender._meta.app_label != 'mapdata':
         return
-    manager: ChangesetOverlayManager = getattr(overlay_state, 'manager', None)
+    manager: DatabaseOverlayManager = getattr(overlay_state, 'manager', None)
     if manager:
         manager.handle_m2m_changed(sender=sender, **kwargs)

src/c3nav/editor/views/base.py

@@ -15,22 +15,21 @@ from django.utils.cache import patch_vary_headers
 from django.utils.translation import get_language
 from django.utils.translation import gettext_lazy as _
 
-from c3nav.editor.changes import enable_changeset_overlay
 from c3nav.editor.models import ChangeSet
+from c3nav.editor.overlay import DatabaseOverlayManager
 from c3nav.mapdata.models.access import AccessPermission
 from c3nav.mapdata.models.base import SerializableMixin
 from c3nav.mapdata.utils.user import can_access_editor
 
 
-def use_changeset_mapdata(func):
+def accesses_mapdata(func):
     @wraps(func)
     def wrapped(request, *args, **kwargs):
-        print('USE CHANGESET MAPDATA')
+        changes = None if request.changeset.direct_editing is None else request.changeset.changes
-        if request.changeset.direct_editing:
+        with DatabaseOverlayManager.enable(changes, commit=request.changeset.direct_editing) as manager:
-            return func(request, *args, **kwargs)
+            result = func(request, *args, **kwargs)
-
+            print("operations", manager.new_operations)
-        with enable_changeset_overlay(request.changeset):
+            return result
-            return func(request, *args, **kwargs)
 
     return wrapped
 

src/c3nav/editor/views/edit.py

@@ -19,7 +19,7 @@ from c3nav.editor.forms import GraphEdgeSettingsForm, GraphEditorActionForm, get
 from c3nav.editor.utils import DefaultEditUtils, LevelChildEditUtils, SpaceChildEditUtils
 from c3nav.editor.views.base import (APIHybridError, APIHybridFormTemplateResponse, APIHybridLoginRequiredResponse,
                                      APIHybridMessageRedirectResponse, APIHybridTemplateContextResponse,
-                                     editor_etag_func, sidebar_view, use_changeset_mapdata)
+                                     editor_etag_func, sidebar_view, accesses_mapdata)
 from c3nav.mapdata.models import Level, Space, LocationGroupCategory, GraphNode, GraphEdge
 from c3nav.mapdata.models.access import AccessPermission
 from c3nav.mapdata.utils.user import can_access_editor
@@ -44,7 +44,7 @@ def child_model(request, model: typing.Union[str, models.Model], kwargs=None, pa
 
 
 @etag(editor_etag_func)
-@use_changeset_mapdata
+@accesses_mapdata
 @sidebar_view(api_hybrid=True)
 def main_index(request):
     return APIHybridTemplateContextResponse('editor/index.html', {
@@ -69,7 +69,7 @@ def main_index(request):
 
 
 @etag(editor_etag_func)
-@use_changeset_mapdata
+@accesses_mapdata
 @sidebar_view(api_hybrid=True)
 def level_detail(request, pk):
     qs = Level.objects.filter(Level.q_for_request(request))
@@ -98,7 +98,7 @@ def level_detail(request, pk):
 
 
 @etag(editor_etag_func)
-@use_changeset_mapdata
+@accesses_mapdata
 @sidebar_view(api_hybrid=True)
 def space_detail(request, level, pk):
     # todo: HOW TO GET DATA
@@ -132,7 +132,7 @@ def get_changeset_exceeded(request):
 
 
 @etag(editor_etag_func)
-@use_changeset_mapdata
+@accesses_mapdata
 @sidebar_view(api_hybrid=True)
 def edit(request, pk=None, model=None, level=None, space=None, on_top_of=None, explicit_edit=False):
     if isinstance(model, str):
@@ -585,7 +585,7 @@ def connect_nodes(request, active_node, clicked_node, edge_settings_form):
 
 
 @etag(editor_etag_func)
-@use_changeset_mapdata
+@accesses_mapdata
 @sidebar_view
 def graph_edit(request, level=None, space=None):
     if not request.user_permissions.can_access_base_mapdata: