From 00ba6a7e6eb929f7eca7fda2db98b363c7bdf692 Mon Sep 17 00:00:00 2001 From: Jan Date: Sat, 3 Oct 2026 10:44:03 +0200 Subject: [PATCH 1/3] feat(planning): save conflict detection (409) for plans, empires and cx, and per-key preference merge for multi-tab planning - Plan, empire and CX responses carry modified_at; PUT accepts an optional base_modified_at and answers 409 (code "conflict", current modified_at) when the row changed since, checked under a row lock in the saving transaction - Empires get config_modified_at (migration with backfill) as their version, so state syncs and cx-junctions never cause a conflict; read-only in the admin - PATCH /user/preferences/ merges planOverrides per plan uuid; null deletes one - Deleting a plan removes its override from the owner's preferences - Backward compatible: without base_modified_at saves work as before Spec: specs/2026-10-03-multitab-01-backend.md Co-Authored-By: Claude Opus 5.5 --- backend/planning/admin.py | 2 +- backend/planning/api/serializers/__init__.py | 1 + backend/planning/api/serializers/conflict.py | 54 ++++ backend/planning/api/serializers/cx.py | 5 +- backend/planning/api/serializers/empire.py | 28 ++- backend/planning/api/serializers/plan.py | 6 +- backend/planning/api/viewsets/cx_viewset.py | 6 +- .../planning/api/viewsets/empire_viewset.py | 6 +- backend/planning/api/viewsets/plan_viewset.py | 6 +- .../0008_planningempire_config_modified_at.py | 19 ++ ...0009_empire_config_modified_at_backfill.py | 21 ++ backend/planning/models.py | 4 + backend/planning/signals.py | 17 ++ .../planning/api/serializers/test_conflict.py | 238 ++++++++++++++++++ backend/tests/planning/test_planning_admin.py | 6 + .../tests/planning/test_planning_signals.py | 55 +++- backend/tests/user/api/test_user_viewsets.py | 32 ++- backend/user/api/serializer.py | 4 +- backend/user/api/viewsets.py | 23 +- 19 files changed, 511 insertions(+), 22 deletions(-) create mode 100644 backend/planning/api/serializers/conflict.py create mode 100644 backend/planning/migrations/0008_planningempire_config_modified_at.py create mode 100644 backend/planning/migrations/0009_empire_config_modified_at_backfill.py create mode 100644 backend/tests/planning/api/serializers/test_conflict.py diff --git a/backend/planning/admin.py b/backend/planning/admin.py index ead2de6..9b6bd6b 100644 --- a/backend/planning/admin.py +++ b/backend/planning/admin.py @@ -182,7 +182,7 @@ class PlanningEmpireAdmin(SummaryStripMixin, ChangeHeaderMixin[PlanningEmpire], list_filter = [('empire_faction', ChoicesDropdownFilter), 'needs_state_sync', *CREATED_MODIFIED_FILTERS] list_filter_submit = True date_hierarchy = 'created_at' - readonly_fields = ['uuid', 'created_at', 'modified_at'] + readonly_fields = ['uuid', 'created_at', 'modified_at', 'config_modified_at'] autocomplete_fields = ['user', 'cx'] list_sections = [EmpirePlansSection] diff --git a/backend/planning/api/serializers/__init__.py b/backend/planning/api/serializers/__init__.py index 3b5d141..751c34d 100644 --- a/backend/planning/api/serializers/__init__.py +++ b/backend/planning/api/serializers/__init__.py @@ -1,3 +1,4 @@ +from .conflict import * from .cx import * from .empire import * from .plan import * diff --git a/backend/planning/api/serializers/conflict.py b/backend/planning/api/serializers/conflict.py new file mode 100644 index 0000000..af9bcda --- /dev/null +++ b/backend/planning/api/serializers/conflict.py @@ -0,0 +1,54 @@ +from datetime import datetime + +from django.db import models, transaction +from rest_framework import serializers, status +from rest_framework.exceptions import APIException + + +class SaveConflict(APIException): + status_code = status.HTTP_409_CONFLICT + default_detail = 'Changed since it was loaded.' + default_code = 'conflict' + + def __init__(self, modified_at: datetime) -> None: + super().__init__( + { + 'detail': self.default_detail, + 'code': self.default_code, + 'modified_at': serializers.DateTimeField().to_representation(modified_at), + } + ) + + +class PlanningSaveConflictSerializer(serializers.Serializer): + detail = serializers.CharField() + code = serializers.CharField() + modified_at = serializers.DateTimeField() + + +class SaveConflictMixin(serializers.Serializer): + # rejects an update with 409 when base_modified_at (the version the client edited) is no longer the stored one + + base_modified_at = serializers.DateTimeField(write_only=True, required=False, allow_null=True) + + # model field holding the version exposed as modified_at + version_field = 'modified_at' + + def save(self, **kwargs: object) -> models.Model: + # save(), not update(): the subclasses' own create()/update() don't call super() + base: datetime | None = self.validated_data.pop('base_modified_at', None) + instance = self.instance + if instance is None or base is None: + return super().save(**kwargs) + + with transaction.atomic(): + # the row lock makes two saves from the same base end in one success and one conflict + current: datetime = ( + type(instance) + .objects.select_for_update() + .values_list(self.version_field, flat=True) + .get(pk=instance.pk) + ) + if current != base: + raise SaveConflict(current) + return super().save(**kwargs) diff --git a/backend/planning/api/serializers/cx.py b/backend/planning/api/serializers/cx.py index e0c71ea..1c29f07 100644 --- a/backend/planning/api/serializers/cx.py +++ b/backend/planning/api/serializers/cx.py @@ -4,17 +4,18 @@ from planning.schemas.latest_schemas import LATEST_SCHEMA from rest_framework import serializers +from .conflict import SaveConflictMixin from .empire import PlanningEmpireNestedSerializer -class PlanningCXDetailSerializer(serializers.ModelSerializer): +class PlanningCXDetailSerializer(SaveConflictMixin, serializers.ModelSerializer): empires = PlanningEmpireNestedSerializer(source='cxs', many=True, read_only=True) cx_data = PydanticJSONField(pydantic_model=LATEST_SCHEMA['CX_DATA']) class Meta: model = PlanningCX - exclude = ['created_at', 'modified_at', 'schema_version', 'user'] + exclude = ['created_at', 'schema_version', 'user'] @transaction.atomic def create(self, validated_data): diff --git a/backend/planning/api/serializers/empire.py b/backend/planning/api/serializers/empire.py index f9f9aaf..7d9bb53 100644 --- a/backend/planning/api/serializers/empire.py +++ b/backend/planning/api/serializers/empire.py @@ -1,7 +1,9 @@ from django.db import transaction +from django.utils import timezone from planning.models import PlanningCOGCChoices, PlanningEmpire, PlanningFactionChoices from rest_framework import serializers +from .conflict import SaveConflictMixin from .minimal import PlanningCXMinimal, PlanningPlanMinimalSerializer @@ -15,23 +17,37 @@ class Meta: class PlanningEmpireListSerializer(serializers.ModelSerializer): cx = PlanningCXMinimal(read_only=True) + # configuration version: state syncs and cx-junctions don't change it + modified_at = serializers.DateTimeField(source='config_modified_at', read_only=True) class Meta: model = PlanningEmpire - fields = ['uuid', 'empire_name', 'empire_faction', 'empire_permits_used', 'empire_permits_total', 'cx'] - - -class PlanningEmpireDetailSerializer(serializers.ModelSerializer): + fields = [ + 'uuid', + 'empire_name', + 'empire_faction', + 'empire_permits_used', + 'empire_permits_total', + 'cx', + 'modified_at', + ] + + +class PlanningEmpireDetailSerializer(SaveConflictMixin, serializers.ModelSerializer): plans = PlanningPlanMinimalSerializer(many=True, read_only=True) cx = PlanningCXMinimal(read_only=True) + # the model's modified_at moves on every state sync, so the configuration has its own version + modified_at = serializers.DateTimeField(source='config_modified_at', read_only=True) + + version_field = 'config_modified_at' class Meta: model = PlanningEmpire - exclude = ['created_at', 'modified_at', 'user', 'empire_state'] + exclude = ['created_at', 'config_modified_at', 'user', 'empire_state'] @transaction.atomic def update(self, instance, validated_data): - + instance.config_modified_at = timezone.now() instance = super().update(instance, validated_data) return instance diff --git a/backend/planning/api/serializers/plan.py b/backend/planning/api/serializers/plan.py index 9c8307e..424f9ab 100644 --- a/backend/planning/api/serializers/plan.py +++ b/backend/planning/api/serializers/plan.py @@ -10,6 +10,7 @@ from planning.schemas.latest_schemas import LATEST_SCHEMA from rest_framework import serializers +from .conflict import SaveConflictMixin from .cx import PlanningCXDetailSerializer from .empire import PlanningEmpireListSerializer @@ -31,7 +32,7 @@ } -class PlanningPlanDetailSerializer(serializers.ModelSerializer): +class PlanningPlanDetailSerializer(SaveConflictMixin, serializers.ModelSerializer): empires = PlanningEmpireListSerializer(many=True, read_only=True) cx = PlanningCXDetailSerializer(read_only=True) @@ -56,6 +57,8 @@ class Meta: 'empires', 'cx', 'empire_uuid', + 'modified_at', + 'base_modified_at', ] def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: @@ -146,6 +149,7 @@ class Meta(PlanningPlanDetailSerializer.Meta): 'plan_corphq', 'plan_data', 'empire_uuid', + 'modified_at', ] diff --git a/backend/planning/api/viewsets/cx_viewset.py b/backend/planning/api/viewsets/cx_viewset.py index cce9daa..84a24fa 100644 --- a/backend/planning/api/viewsets/cx_viewset.py +++ b/backend/planning/api/viewsets/cx_viewset.py @@ -11,6 +11,7 @@ PlanningCXDetailSerializer, PlanningCXJunctionsSyncErrorSerializer, PlanningCXJunctionUpdateSerializer, + PlanningSaveConflictSerializer, ) from planning.models import PlanningCX, PlanningEmpire from planning.planning_cache_manager import PLANNING @@ -64,7 +65,10 @@ def fetch_data(): def create(self, request, *args, **kwargs): return super().create(request, *args, **kwargs) - @extend_schema(summary='Updates an existing cx preference') + @extend_schema( + summary='Updates an existing cx preference', + responses={200: PlanningCXDetailSerializer, 409: PlanningSaveConflictSerializer}, + ) def update(self, request, *args, **kwargs): return super().update(request, *args, **kwargs) diff --git a/backend/planning/api/viewsets/empire_viewset.py b/backend/planning/api/viewsets/empire_viewset.py index 39106ac..ed649a3 100644 --- a/backend/planning/api/viewsets/empire_viewset.py +++ b/backend/planning/api/viewsets/empire_viewset.py @@ -11,6 +11,7 @@ PlanningEmpireJunctionsSerializer, PlanningEmpirePlanSyncErrorSerializer, PlanningPlanListSerializer, + PlanningSaveConflictSerializer, ) from planning.api.serializers.empire import PlanningEmpireStateUpdateSerializer from planning.models import PlanningEmpire, PlanningEmpirePlan, PlanningPlan @@ -78,7 +79,10 @@ def fetch_data(): return CacheManager.respond(request, PLANNING, 'empire-plans', pk, build=fetch_data, scope=user_id) - @extend_schema(summary='Updates an existing exmpire') + @extend_schema( + summary='Updates an existing exmpire', + responses={200: PlanningEmpireDetailSerializer, 409: PlanningSaveConflictSerializer}, + ) def update(self, request, *args, **kwargs): return super().update(request, *args, **kwargs) diff --git a/backend/planning/api/viewsets/plan_viewset.py b/backend/planning/api/viewsets/plan_viewset.py index 9908e1e..c7aec48 100644 --- a/backend/planning/api/viewsets/plan_viewset.py +++ b/backend/planning/api/viewsets/plan_viewset.py @@ -6,6 +6,7 @@ from drf_spectacular.utils import extend_schema from planning.api.serializers import ( PlanningPlanDetailSerializer, + PlanningSaveConflictSerializer, ) from planning.models import PlanningEmpire, PlanningPlan from planning.planning_cache_manager import PLANNING @@ -60,7 +61,10 @@ def fetch_data() -> dict[str, Any]: def create(self, request, *args, **kwargs): return super().create(request, *args, **kwargs) - @extend_schema(summary='Updates an existing plan') + @extend_schema( + summary='Updates an existing plan', + responses={200: PlanningPlanDetailSerializer, 409: PlanningSaveConflictSerializer}, + ) def update(self, request, *args, **kwargs): return super().update(request, *args, **kwargs) diff --git a/backend/planning/migrations/0008_planningempire_config_modified_at.py b/backend/planning/migrations/0008_planningempire_config_modified_at.py new file mode 100644 index 0000000..fbde00f --- /dev/null +++ b/backend/planning/migrations/0008_planningempire_config_modified_at.py @@ -0,0 +1,19 @@ +# Generated by Django 6.0.8 on 2026-10-03 08:22 + +import django.utils.timezone +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('planning', '0007_planningempire_needs_state_sync'), + ] + + operations = [ + migrations.AddField( + model_name='planningempire', + name='config_modified_at', + field=models.DateTimeField(default=django.utils.timezone.now), + ), + ] diff --git a/backend/planning/migrations/0009_empire_config_modified_at_backfill.py b/backend/planning/migrations/0009_empire_config_modified_at_backfill.py new file mode 100644 index 0000000..e12549c --- /dev/null +++ b/backend/planning/migrations/0009_empire_config_modified_at_backfill.py @@ -0,0 +1,21 @@ +# Generated by Django 6.0.8 on 2026-10-03 08:22 + +from django.db import migrations +from django.db.models import F + + +def backfill(apps, schema_editor): + """Existing empires: last configuration change unknown, start from the last change of any kind.""" + PlanningEmpire = apps.get_model('planning', 'PlanningEmpire') + PlanningEmpire.objects.update(config_modified_at=F('modified_at')) + + +class Migration(migrations.Migration): + + dependencies = [ + ('planning', '0008_planningempire_config_modified_at'), + ] + + operations = [ + migrations.RunPython(backfill, reverse_code=migrations.RunPython.noop), + ] diff --git a/backend/planning/models.py b/backend/planning/models.py index 7f90deb..99426c2 100644 --- a/backend/planning/models.py +++ b/backend/planning/models.py @@ -1,6 +1,7 @@ from core.models import ChangeTrackedModel, UUIDModel from django.core.validators import MaxValueValidator, MinLengthValidator, MinValueValidator from django.db import models +from django.utils import timezone class PlanningFactionChoices(models.TextChoices): @@ -83,6 +84,9 @@ class PlanningEmpire(UUIDModel, ChangeTrackedModel): empire_state = models.JSONField(default=dict, blank=True) needs_state_sync = models.BooleanField(default=False, db_index=True) + # version of the configuration (name, faction, permits) for save conflicts; modified_at also moves on state syncs + config_modified_at = models.DateTimeField(default=timezone.now) + def __str__(self) -> str: return f'{self.empire_name} ({self.uuid})' diff --git a/backend/planning/signals.py b/backend/planning/signals.py index f990c3b..3543a0f 100644 --- a/backend/planning/signals.py +++ b/backend/planning/signals.py @@ -1,6 +1,7 @@ from core.services.cache_manager import CacheManager from django.db.models.signals import post_delete, post_save from django.dispatch import receiver +from user.models import User, UserPreference from planning.models import PlanningCX, PlanningEmpire, PlanningEmpirePlan, PlanningPlan from planning.planning_cache_manager import PLANNING @@ -20,3 +21,19 @@ def invalidate_user_planning_caches( user_id: int = instance.user_id # type: ignore CacheManager.invalidate_on_commit(PLANNING, user_id) + + +@receiver(post_delete, sender=PlanningPlan, dispatch_uid='planning_drop_plan_overrides') +def drop_plan_overrides( + sender: type[PlanningPlan], instance: PlanningPlan, origin: object = None, **kwargs: object +) -> None: + # account deletion removes the preferences anyway + if isinstance(origin, User): + return + + # replaces the frontend's cleanPlanPreferences(), which also dropped plans created in another tab + preference = UserPreference.objects.select_for_update().filter(user_id=instance.user_id).first() # type: ignore + overrides = preference.preferences.get('plan_overrides') if preference else None + if preference and overrides and str(instance.uuid) in overrides: + del overrides[str(instance.uuid)] + preference.save(update_fields=['preferences']) diff --git a/backend/tests/planning/api/serializers/test_conflict.py b/backend/tests/planning/api/serializers/test_conflict.py new file mode 100644 index 0000000..3d23ced --- /dev/null +++ b/backend/tests/planning/api/serializers/test_conflict.py @@ -0,0 +1,238 @@ +import importlib +from dataclasses import dataclass +from datetime import timedelta + +import pytest +from django.apps import apps +from django.db.models import Model +from django.urls import reverse +from django.utils import timezone +from model_bakery import baker +from planning.models import PlanningEmpire, PlanningFactionChoices +from rest_framework.test import APIClient +from tests.fixtures.planning.fxt_plan_vallis import plan_data_vallis +from user.models import User + +pytestmark = pytest.mark.django_db + + +@dataclass(frozen=True) +class Resource: + name: str + model: str + list_url: str + detail_url: str + name_field: str + payload: dict[str, object] + + +RESOURCES = [ + Resource( + 'plan', + 'planning.PlanningPlan', + 'planning:plan', + 'planning:plan-detail', + 'plan_name', + { + 'plan_name': 'Plan', + 'planet_natural_id': 'OT-580b', + 'plan_permits_used': 1, + 'plan_corphq': False, + 'plan_data': plan_data_vallis, + }, + ), + Resource( + 'empire', + 'planning.PlanningEmpire', + 'planning:empire', + 'planning:empire-detail', + 'empire_name', + { + 'empire_name': 'Empire', + 'empire_faction': PlanningFactionChoices.ANTARES, + 'empire_permits_used': 1, + 'empire_permits_total': 2, + }, + ), + Resource( + 'cx', 'planning.PlanningCX', 'planning:cx', 'planning:cx-detail', 'cx_name', {'cx_name': 'CX', 'cx_data': {}} + ), +] + + +@pytest.fixture(params=RESOURCES, ids=lambda r: r.name) +def resource(request: pytest.FixtureRequest) -> Resource: + return request.param + + +@pytest.fixture +def owner() -> User: + return baker.make('user.User') + + +@pytest.fixture +def obj(resource: Resource, owner: User) -> Model: + if resource.name == 'plan': + return baker.make(resource.model, user=owner, plan_data=plan_data_vallis) + return baker.make(resource.model, user=owner) + + +def _client(api_client: APIClient, user: User) -> APIClient: + return api_client.as_user(user) # ty:ignore[unresolved-attribute] + + +def _url(resource: Resource, obj: Model) -> str: + return reverse(resource.detail_url, kwargs={'pk': str(obj.pk)}) + + +def _put(api_client: APIClient, user: User, resource: Resource, obj: Model, name: str, **extra: object): + return _client(api_client, user).put( + _url(resource, obj), data={**resource.payload, resource.name_field: name, **extra}, format='json' + ) + + +class TestModifiedAt: + def test_list_and_detail_carry_modified_at(self, api_client, resource, owner, obj) -> None: + listed = _client(api_client, owner).get(reverse(resource.list_url)).data + detail = _client(api_client, owner).get(_url(resource, obj)).data + + assert listed[0]['modified_at'] == detail['modified_at'] + assert detail['modified_at'] + assert 'base_modified_at' not in detail + + def test_plan_list_of_empire_carries_modified_at(self, api_client, owner) -> None: + empire: PlanningEmpire = baker.make('planning.PlanningEmpire', user=owner) + plan = baker.make('planning.PlanningPlan', user=owner, plan_data=plan_data_vallis) + empire.plans.add(plan, through_defaults={'user': owner}) + + response = _client(api_client, owner).get(reverse('planning:empire-plan-list', kwargs={'pk': str(empire.pk)})) + + assert response.data[0]['modified_at'] + + +class TestConditionalSave: + def test_current_base_saves_and_returns_newer_version(self, api_client, resource, owner, obj) -> None: + base = _client(api_client, owner).get(_url(resource, obj)).data['modified_at'] + + response = _put(api_client, owner, resource, obj, 'Renamed', base_modified_at=base) + + assert response.status_code == 200 + assert response.data[resource.name_field] == 'Renamed' + assert response.data['modified_at'] > base + + def test_outdated_base_is_a_conflict(self, api_client, resource, owner, obj) -> None: + current = _client(api_client, owner).get(_url(resource, obj)).data['modified_at'] + older = (timezone.now() - timedelta(hours=1)).isoformat() + + response = _put(api_client, owner, resource, obj, 'Renamed', base_modified_at=older) + + assert response.status_code == 409 + assert response.data == {'detail': 'Changed since it was loaded.', 'code': 'conflict', 'modified_at': current} + obj.refresh_from_db() + assert getattr(obj, resource.name_field) != 'Renamed' + + def test_second_save_from_same_base_conflicts(self, api_client, resource, owner, obj) -> None: + base = _client(api_client, owner).get(_url(resource, obj)).data['modified_at'] + + first = _put(api_client, owner, resource, obj, 'Tab B', base_modified_at=base) + second = _put(api_client, owner, resource, obj, 'Tab A', base_modified_at=base) + + assert first.status_code == 200 + assert second.status_code == 409 + assert second.data['modified_at'] == first.data['modified_at'] + obj.refresh_from_db() + assert getattr(obj, resource.name_field) == 'Tab B' + + @pytest.mark.parametrize('extra', [{}, {'base_modified_at': None}], ids=['absent', 'null']) + def test_without_base_overwrites(self, api_client, resource, owner, obj, extra) -> None: + _put(api_client, owner, resource, obj, 'Tab B') + + response = _put(api_client, owner, resource, obj, 'Tab A', **extra) + + assert response.status_code == 200 + assert response.data[resource.name_field] == 'Tab A' + + def test_deleted_object_is_404(self, api_client, resource, owner, obj) -> None: + base = _client(api_client, owner).get(_url(resource, obj)).data['modified_at'] + url = _url(resource, obj) + obj.delete() + + response = _client(api_client, owner).put( + url, data={**resource.payload, 'base_modified_at': base}, format='json' + ) + + assert response.status_code == 404 + + def test_create_ignores_base(self, api_client, resource, owner) -> None: + response = _client(api_client, owner).post( + reverse(resource.list_url), + data={**resource.payload, 'base_modified_at': '2020-01-01T00:00:00Z'}, + format='json', + ) + + assert response.status_code == 201 + assert response.data['modified_at'] + + @pytest.mark.usefixtures('locmem_cache') + def test_get_after_save_returns_new_version( + self, api_client, resource, owner, obj, django_capture_on_commit_callbacks + ) -> None: + client = _client(api_client, owner) + base = client.get(_url(resource, obj)).data['modified_at'] + client.get(reverse(resource.list_url)) + assert client.get(_url(resource, obj))['X-Cache-Hit'] == '1' + + with django_capture_on_commit_callbacks(execute=True): + saved = _put(api_client, owner, resource, obj, 'Renamed', base_modified_at=base) + + assert client.get(_url(resource, obj)).data['modified_at'] == saved.data['modified_at'] + assert client.get(reverse(resource.list_url)).data[0]['modified_at'] == saved.data['modified_at'] + + +class TestEmpireConfigVersion: + def test_state_sync_and_cx_junctions_keep_the_version(self, api_client, owner) -> None: + empire: PlanningEmpire = baker.make('planning.PlanningEmpire', user=owner) + cx = baker.make('planning.PlanningCX', user=owner) + client = _client(api_client, owner) + url = reverse('planning:empire-detail', kwargs={'pk': str(empire.pk)}) + base = client.get(url).data['modified_at'] + + state = { + 'metadata': { + 'faction': PlanningFactionChoices.ANTARES, + 'permits_used': 1, + 'permits_total': 2, + 'plan_count': 0, + 'timestamp': '2026-01-01T00:00:00Z', + }, + 'empire_total': {}, + 'plan_details': {}, + } + synced = client.patch( + reverse('planning:empire-sync-state', kwargs={'pk': str(empire.pk)}), data=state, format='json' + ) + junctions = client.post( + reverse('planning:cx-junctions'), + data=[{'cx_uuid': str(cx.pk), 'empires': [{'empire_uuid': str(empire.pk)}]}], + format='json', + ) + + assert synced.status_code == 200 + assert junctions.status_code == 200 + assert synced.data['modified_at'] == base + assert client.get(url).data['modified_at'] == base + + response = client.put(url, data={**RESOURCES[1].payload, 'base_modified_at': base}, format='json') + assert response.status_code == 200 + + +class TestEmpireConfigBackfill: + def test_backfill_copies_modified_at(self, owner) -> None: + migration = importlib.import_module('planning.migrations.0009_empire_config_modified_at_backfill') + empire: PlanningEmpire = baker.make('planning.PlanningEmpire', user=owner) + PlanningEmpire.objects.filter(pk=empire.pk).update(config_modified_at=timezone.now() + timedelta(days=1)) + + migration.backfill(apps, None) + + empire.refresh_from_db() + assert empire.config_modified_at == empire.modified_at diff --git a/backend/tests/planning/test_planning_admin.py b/backend/tests/planning/test_planning_admin.py index c2996f5..a5f06e6 100644 --- a/backend/tests/planning/test_planning_admin.py +++ b/backend/tests/planning/test_planning_admin.py @@ -85,6 +85,12 @@ def test_inlines_use_autocomplete(self, admin_client: Client, seeded: Seeded) -> # the CX inline never renders the empires' JSON state assert 'empire_state' not in cx_page + def test_empire_config_version_is_read_only(self, admin_client: Client, seeded: Seeded) -> None: + # only the API's configuration saves move the save-conflict version + html = admin_client.get(url(PlanningEmpire, 'change', seeded.empire.pk)).content.decode() + + assert 'name="config_modified_at' not in html + def test_autocomplete_endpoint_skips_json(self, admin_client: Client, seeded: Seeded) -> None: with CaptureQueriesContext(connection) as queries: response = admin_client.get( diff --git a/backend/tests/planning/test_planning_signals.py b/backend/tests/planning/test_planning_signals.py index d507c80..ef266bd 100644 --- a/backend/tests/planning/test_planning_signals.py +++ b/backend/tests/planning/test_planning_signals.py @@ -12,10 +12,12 @@ from typing import Protocol, cast import pytest +from django.db import connection +from django.test.utils import CaptureQueriesContext from django.urls import reverse from planning.models import PlanningCX, PlanningEmpire, PlanningPlan from rest_framework.test import APIClient -from user.models import User +from user.models import User, UserPreference pytestmark = [pytest.mark.django_db, pytest.mark.usefixtures('locmem_cache')] @@ -239,3 +241,54 @@ def test_changes_leave_caches_of_users_with_suffix_ids_alone( plan.save() assert _get(api_client, other, url)['X-Cache-Hit'] == '1' + + +class TestPlanDeleteDropsOverrides: + OVERRIDE = {'include_cm': True, 'visitation_material_exclusions': [], 'auto_optimize_habs': False} + + def test_delete_removes_only_that_plans_override(self, api_client, user_factory, plan_factory) -> None: + user, other = user_factory(), user_factory() + plan, kept = plan_factory(user=user), plan_factory(user=user) + other_plan = plan_factory(user=other) + UserPreference.objects.create( + user=user, + preferences={ + 'burn_days_red': 3, + 'plan_overrides': {str(plan.uuid): self.OVERRIDE, str(kept.uuid): self.OVERRIDE}, + }, + ) + UserPreference.objects.create(user=other, preferences={'plan_overrides': {str(plan.uuid): self.OVERRIDE}}) + + response = api_client.as_user(user).delete(reverse('planning:plan-detail', kwargs={'pk': str(plan.uuid)})) + + assert response.status_code == 204 + assert UserPreference.objects.get(user=user).preferences == { + 'burn_days_red': 3, + 'plan_overrides': {str(kept.uuid): self.OVERRIDE}, + } + assert UserPreference.objects.get(user=other).preferences == {'plan_overrides': {str(plan.uuid): self.OVERRIDE}} + assert PlanningPlan.objects.filter(pk=other_plan.pk).exists() + + def test_delete_without_preferences_or_override(self, user_factory, plan_factory) -> None: + user = user_factory() + plan_factory(user=user).delete() + UserPreference.objects.create(user=user, preferences={'plan_overrides': None}) + + plan_factory(user=user).delete() + + assert UserPreference.objects.get(user=user).preferences == {'plan_overrides': None} + + def test_account_deletion_skips_override_cleanup(self, user_factory, plan_factory) -> None: + user = user_factory() + plans = [plan_factory(user=user) for _ in range(5)] + UserPreference.objects.create( + user=user, preferences={'plan_overrides': {str(p.uuid): self.OVERRIDE for p in plans}} + ) + table = UserPreference._meta.db_table + + with CaptureQueriesContext(connection) as ctx: + user.delete() + + # no per-plan preference lookup: the cascade deletes the preferences anyway + assert not [q for q in ctx.captured_queries if q['sql'].startswith('SELECT') and table in q['sql']] + assert not UserPreference.objects.filter(user_id=user.pk).exists() diff --git a/backend/tests/user/api/test_user_viewsets.py b/backend/tests/user/api/test_user_viewsets.py index 2dc2d78..23800b0 100644 --- a/backend/tests/user/api/test_user_viewsets.py +++ b/backend/tests/user/api/test_user_viewsets.py @@ -270,20 +270,46 @@ def test_partial_update_keeps_omitted_keys(self, api_client, user_factory): assert response.data['locale'] == 'de_DE' assert UserPreference.objects.get(user=user).preferences == {**stored, 'burn_days_red': 3} - def test_partial_update_replaces_plan_overrides(self, api_client, user_factory): + def test_partial_update_merges_plan_overrides_per_plan(self, api_client, user_factory): user = user_factory(id=1) override = {'include_cm': False, 'visitation_material_exclusions': [], 'auto_optimize_habs': True} baker.make(UserPreference, user=user, preferences={'plan_overrides': {'p1': override, 'p2': override}}) response = api_client.as_user(user).patch( reverse('user:user_preferences'), - data={'planOverrides': {'p2': {'includeCM': False, 'autoOptimizeHabs': True}}}, + data={'planOverrides': {'p2': {'includeCM': True, 'autoOptimizeHabs': True}}}, format='json', ) + assert response.status_code == 200 + assert list(response.data['planOverrides']) == ['p1', 'p2'] + stored = UserPreference.objects.get(user=user).preferences['plan_overrides'] + assert stored['p1'] == override + assert stored['p2']['include_cm'] is True + + def test_partial_update_null_plan_override_deletes_only_that_plan(self, api_client, user_factory): + user = user_factory(id=1) + override = {'include_cm': False, 'visitation_material_exclusions': [], 'auto_optimize_habs': True} + baker.make(UserPreference, user=user, preferences={'plan_overrides': {'p1': override, 'p2': override}}) + + response = api_client.as_user(user).patch( + reverse('user:user_preferences'), data={'planOverrides': {'p1': None, 'p3': None}}, format='json' + ) + assert response.status_code == 200 assert list(response.data['planOverrides']) == ['p2'] - assert list(UserPreference.objects.get(user=user).preferences['plan_overrides']) == ['p2'] + assert UserPreference.objects.get(user=user).preferences['plan_overrides'] == {'p2': override} + + def test_partial_update_one_key_keeps_the_others(self, api_client, user_factory): + user = user_factory(id=1) + baker.make(UserPreference, user=user, preferences={'burn_days_red': 2, 'burn_days_yellow': 10}) + + response = api_client.as_user(user).patch( + reverse('user:user_preferences'), data={'burnDaysYellow': 7}, format='json' + ) + + assert response.status_code == 200 + assert UserPreference.objects.get(user=user).preferences == {'burn_days_red': 2, 'burn_days_yellow': 7} def test_partial_update_null_clears_default_uuid(self, api_client, user_factory): user = user_factory(id=1) diff --git a/backend/user/api/serializer.py b/backend/user/api/serializer.py index ee55031..e72722c 100644 --- a/backend/user/api/serializer.py +++ b/backend/user/api/serializer.py @@ -209,7 +209,9 @@ class UserPreferenceSerializer(JSONSafeSerializerMixin, serializers.Serializer): colorPalette = serializers.ChoiceField(source='color_palette', choices=COLOR_PALETTES, required=False) planSuggestions = serializers.BooleanField(source='plan_suggestions', required=False) - planOverrides = serializers.DictField(source='plan_overrides', child=PlanOverrideSerializer(), required=False) + planOverrides = serializers.DictField( + source='plan_overrides', child=PlanOverrideSerializer(allow_null=True), required=False + ) def to_representation(self, instance): data_from_db = instance if isinstance(instance, dict) else {} diff --git a/backend/user/api/viewsets.py b/backend/user/api/viewsets.py index 8e133e2..5ee905c 100644 --- a/backend/user/api/viewsets.py +++ b/backend/user/api/viewsets.py @@ -1,4 +1,5 @@ from django.core.cache import cache +from django.db import transaction from drf_spectacular.utils import extend_schema from rest_framework import mixins, status, viewsets from rest_framework.decorators import action @@ -46,10 +47,24 @@ def partial_update(self, request, *args, **kwargs): serializer = self.get_serializer(data=request.data, partial=True) serializer.is_valid(raise_exception=True) - # top-level merge: omitted keys keep their stored value, a sent planOverrides replaces the stored dict - obj, _ = UserPreference.objects.get_or_create(user=request.user) - obj.preferences = {**obj.preferences, **serializer.validated_data} - obj.save() + # top-level merge: omitted keys keep their stored value; planOverrides merges per plan uuid, null deletes one + changes = dict(serializer.validated_data) + overrides: dict[str, dict | None] | None = changes.pop('plan_overrides', None) + + # locked, so another tab's PATCH in between isn't lost + with transaction.atomic(): + obj, _ = UserPreference.objects.select_for_update().get_or_create(user=request.user) + preferences = {**obj.preferences, **changes} + if overrides is not None: + merged = dict(obj.preferences.get('plan_overrides') or {}) + for plan_uuid, override in overrides.items(): + if override is None: + merged.pop(plan_uuid, None) + else: + merged[plan_uuid] = override + preferences['plan_overrides'] = merged + obj.preferences = preferences + obj.save() return Response(self.get_serializer(obj.preferences).data) From 19e83e2e617c4468f2dc83eb245a614fe1538c33 Mon Sep 17 00:00:00 2001 From: Jan Date: Sat, 3 Oct 2026 10:46:59 +0200 Subject: [PATCH 2/3] refactor(planning): import conflict serializers by name Co-Authored-By: Claude Opus 5.5 --- backend/planning/api/serializers/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/planning/api/serializers/__init__.py b/backend/planning/api/serializers/__init__.py index 751c34d..c07533e 100644 --- a/backend/planning/api/serializers/__init__.py +++ b/backend/planning/api/serializers/__init__.py @@ -1,4 +1,4 @@ -from .conflict import * +from .conflict import PlanningSaveConflictSerializer, SaveConflict, SaveConflictMixin from .cx import * from .empire import * from .plan import * From 1d9d7cb09bc04a390a23fc95fc7cad01fed66c03 Mon Sep 17 00:00:00 2001 From: Jan Date: Sat, 3 Oct 2026 10:50:04 +0200 Subject: [PATCH 3/3] refactor(planning): import the conflict serializer from its module Co-Authored-By: Claude Opus 5.5 --- backend/planning/api/serializers/__init__.py | 1 - backend/planning/api/viewsets/cx_viewset.py | 2 +- backend/planning/api/viewsets/empire_viewset.py | 2 +- backend/planning/api/viewsets/plan_viewset.py | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/backend/planning/api/serializers/__init__.py b/backend/planning/api/serializers/__init__.py index c07533e..3b5d141 100644 --- a/backend/planning/api/serializers/__init__.py +++ b/backend/planning/api/serializers/__init__.py @@ -1,4 +1,3 @@ -from .conflict import PlanningSaveConflictSerializer, SaveConflict, SaveConflictMixin from .cx import * from .empire import * from .plan import * diff --git a/backend/planning/api/viewsets/cx_viewset.py b/backend/planning/api/viewsets/cx_viewset.py index 84a24fa..c2d5e8c 100644 --- a/backend/planning/api/viewsets/cx_viewset.py +++ b/backend/planning/api/viewsets/cx_viewset.py @@ -11,8 +11,8 @@ PlanningCXDetailSerializer, PlanningCXJunctionsSyncErrorSerializer, PlanningCXJunctionUpdateSerializer, - PlanningSaveConflictSerializer, ) +from planning.api.serializers.conflict import PlanningSaveConflictSerializer from planning.models import PlanningCX, PlanningEmpire from planning.planning_cache_manager import PLANNING from rest_framework import mixins, status, viewsets diff --git a/backend/planning/api/viewsets/empire_viewset.py b/backend/planning/api/viewsets/empire_viewset.py index ed649a3..906d03b 100644 --- a/backend/planning/api/viewsets/empire_viewset.py +++ b/backend/planning/api/viewsets/empire_viewset.py @@ -11,8 +11,8 @@ PlanningEmpireJunctionsSerializer, PlanningEmpirePlanSyncErrorSerializer, PlanningPlanListSerializer, - PlanningSaveConflictSerializer, ) +from planning.api.serializers.conflict import PlanningSaveConflictSerializer from planning.api.serializers.empire import PlanningEmpireStateUpdateSerializer from planning.models import PlanningEmpire, PlanningEmpirePlan, PlanningPlan from planning.planning_cache_manager import PLANNING diff --git a/backend/planning/api/viewsets/plan_viewset.py b/backend/planning/api/viewsets/plan_viewset.py index c7aec48..7830f56 100644 --- a/backend/planning/api/viewsets/plan_viewset.py +++ b/backend/planning/api/viewsets/plan_viewset.py @@ -6,8 +6,8 @@ from drf_spectacular.utils import extend_schema from planning.api.serializers import ( PlanningPlanDetailSerializer, - PlanningSaveConflictSerializer, ) +from planning.api.serializers.conflict import PlanningSaveConflictSerializer from planning.models import PlanningEmpire, PlanningPlan from planning.planning_cache_manager import PLANNING from rest_framework import mixins, status, viewsets