diff --git a/.github/PULL_REQUEST_TEMPLATE/development-to-staging.md b/.github/PULL_REQUEST_TEMPLATE/development-to-staging.md index f8ea009d..dd04ba8a 100644 --- a/.github/PULL_REQUEST_TEMPLATE/development-to-staging.md +++ b/.github/PULL_REQUEST_TEMPLATE/development-to-staging.md @@ -19,5 +19,8 @@ --- +## Contributors + + ## Technical notes diff --git a/.github/PULL_REQUEST_TEMPLATE/staging-to-main.md b/.github/PULL_REQUEST_TEMPLATE/staging-to-main.md index d7eec391..e556e256 100644 --- a/.github/PULL_REQUEST_TEMPLATE/staging-to-main.md +++ b/.github/PULL_REQUEST_TEMPLATE/staging-to-main.md @@ -19,5 +19,8 @@ --- +## Contributors + + ## Technical notes diff --git a/core/admin.py b/core/admin.py index 006e10f3..bffc2ce2 100644 --- a/core/admin.py +++ b/core/admin.py @@ -133,7 +133,12 @@ def has_add_permission(self, request): @admin.action(description="Publish selected announcements") def publish_selected_announcements(_modeladmin, _request, queryset): now = timezone.now() - queryset.update(is_published=True, published_at=now) + # Save individually (not queryset.update()) so Announcement.save() + # broadcasts the "announcement_published" WebSocket event per row. + for announcement in queryset: + announcement.is_published = True + announcement.published_at = now + announcement.save() @admin.action(description="Unpublish selected announcements") diff --git a/core/models.py b/core/models.py index 78b8c990..2dd80537 100644 --- a/core/models.py +++ b/core/models.py @@ -249,9 +249,33 @@ def __str__(self): return self.title def save(self, *args, **kwargs): + was_published = ( + Announcement.objects.filter(pk=self.pk, is_published=True).exists() + if self.pk + else False + ) if self.is_published and self.published_at is None: self.published_at = timezone.now() super().save(*args, **kwargs) + if self.is_published and not was_published: + from django.db import transaction + + transaction.on_commit(self._broadcast_published) + + def _broadcast_published(self): + from asgiref.sync import async_to_sync + + from gameplay.utils import send_group_message + + async_to_sync(send_group_message)( + "online_users", + { + "type": "action", + "action": "announcement_published", + "data": {"id": self.id}, + "success": True, + }, + ) class PlayerAnnouncementState(models.Model): diff --git a/docs/operations/deployment-runbook.md b/docs/operations/deployment-runbook.md index 69941b6a..fc076a72 100644 --- a/docs/operations/deployment-runbook.md +++ b/docs/operations/deployment-runbook.md @@ -50,6 +50,38 @@ This is a lightweight runbook for backend deployment and immediate verification. - Prefer additive schema changes prior to destructive cleanups - Announce deploy windows for higher-risk changes +### Removing a database column safely + +`web`, `celery`, and `celery-beat` (and their `-staging` equivalents) are +separate Render services, each with its own `autoDeployTrigger: commit` and +build queue. Only `web` has a `preDeployCommand` that runs `migrate`; `celery` +and `celery-beat` have none. All three deploy independently off the same +commit, with no guarantee they finish building/restarting at the same time — +`celery`/`celery-beat` builds can lag `web` by anywhere from seconds to a +couple of hours depending on Render's build queue. + +Because of this, a migration that drops a column in the same deploy as the +code change that stops using it creates a window where `web` has already +migrated the (shared) database but `celery`/`celery-beat` are still running +the *old* image — which still references the now-dropped column — until +their own deploys catch up. Any query touching that model from the old +worker code raises `UndefinedColumn` for the whole window. (This is what +happened with `Character.can_link` on 2026-08-14: the migration removing the +column landed in the same deploy as the code that stopped needing it, and +`commute_tick`/`wander_tick` errored on the stale `celery`/`celery-beat` +workers until they finished redeploying.) + +To remove a column without a stale-worker error window, split it across two +separate deploys (expand/contract): + +1. **Deploy 1**: ship the code change that stops reading/writing the column, + but leave the column itself in the DB (no migration removing it yet). +2. Confirm `web`, `celery`, and `celery-beat` (and `-staging` equivalents) + have all finished redeploying and are healthy. +3. **Deploy 2**: add the migration that drops the column, with no + accompanying code change needed (the code already stopped touching it in + deploy 1). + ## Note on PR workflow The rule for which branch a *feature* PR should target (normally `development`, with a documented exception for basing on `staging`) is a development workflow concern, not a deployment-execution step — that lives in `CLAUDE.md`, not here. diff --git a/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.module.scss b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.module.scss index 8c7e4788..4c7de25f 100644 --- a/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.module.scss +++ b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.module.scss @@ -120,7 +120,7 @@ } @include m.respond-to(sm, down) { - .row { + .row:not(.durationRow) { flex-direction: column; } } diff --git a/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.tsx b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.tsx index bd3f6c83..0dae974f 100644 --- a/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.tsx +++ b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.tsx @@ -88,7 +88,7 @@ export default function LogOfflineActivityModal({ onClose }: LogOfflineActivityM Duration * -
+
const { showToast } = useToast(); const { refetch: maintenanceRefetch } = useMaintenanceStatus(); const { setMaintenance } = useMaintenanceContext(); + const queryClient = useQueryClient(); // Set stores message handler callbacks registered by child components const eventHandlersRef = useRef void>>(new Set()); const wsEnabled = Boolean(!authLoading && isAuthenticated && player?.id); @@ -52,14 +58,25 @@ export const WebSocketProvider = ({ children }: ProviderProps): ReactElement => }); }, [loadFromServer, player?.is_premium, freeTimerLimitSeconds]); + const onAnnouncementPublished = useCallback(() => { + queryClient.invalidateQueries({ queryKey: ANNOUNCEMENTS_QUERY_KEY }); + queryClient.invalidateQueries({ queryKey: ANNOUNCEMENT_UNREAD_QUERY_KEY }); + }, [queryClient]); + const onMessage = useCallback((data: IncomingWebSocketMessage) => { if (data.type === 'online_count') { setOnlinePlayerCount(data.count); } //console.log("[WS Provider] showToast:", showToast); - handleGlobalWebSocketEvent(data, { showToast, maintenanceRefetch, setMaintenance, onActivityTimerUpdate }); + handleGlobalWebSocketEvent(data, { + showToast, + maintenanceRefetch, + setMaintenance, + onActivityTimerUpdate, + onAnnouncementPublished, + }); eventHandlersRef.current.forEach((handler) => handler(data)); - }, [showToast, maintenanceRefetch, setMaintenance, setOnlinePlayerCount, onActivityTimerUpdate]); + }, [showToast, maintenanceRefetch, setMaintenance, setOnlinePlayerCount, onActivityTimerUpdate, onAnnouncementPublished]); const onError = useCallback(() => { console.error('WebSocket connection error'); diff --git a/frontend/src/types/timers.ts b/frontend/src/types/timers.ts index 6dca6d7e..6ac00197 100644 --- a/frontend/src/types/timers.ts +++ b/frontend/src/types/timers.ts @@ -157,7 +157,7 @@ export interface WebSocketErrorMessage extends WebSocketMessageBase { /** Server-initiated action message (maintenance refresh, game events) */ export interface WebSocketActionMessage { type: "action"; - action: "refresh" | "load-game" | "activity_timer_update"; + action: "refresh" | "load-game" | "activity_timer_update" | "announcement_published"; message?: string; maintenance_active?: boolean; name?: string; @@ -168,8 +168,12 @@ export interface WebSocketActionMessage { * Present when action is "activity_timer_update" — pushed whenever another * of this player's sessions (tabs/devices) starts, labels, or submits the * activity timer, so every open session can reconcile to server state. + * + * Present when action is "announcement_published" — the id of the + * newly-published Announcement, so callers can invalidate the + * announcements list and unread-count queries. */ - data?: { activity_timer: ActivityTimerApiData }; + data?: { activity_timer: ActivityTimerApiData } | { id: number }; } /** Generic server message (currently unused payload) */ diff --git a/frontend/src/websockets/handleGlobalWebSocketEvent.ts b/frontend/src/websockets/handleGlobalWebSocketEvent.ts index a12599a2..140d54d2 100644 --- a/frontend/src/websockets/handleGlobalWebSocketEvent.ts +++ b/frontend/src/websockets/handleGlobalWebSocketEvent.ts @@ -13,11 +13,22 @@ interface HandleGlobalWebSocketEventOptions { * useActivityTimer's loadFromServer. */ onActivityTimerUpdate?: (activityTimer: ActivityTimerApiData) => void; + /** + * Called when the server announces a newly-published Announcement, so the + * caller can refetch the announcements list / unread-count queries. + */ + onAnnouncementPublished?: () => void; } export async function handleGlobalWebSocketEvent( data: IncomingWebSocketMessage, - { showToast, maintenanceRefetch, setMaintenance, onActivityTimerUpdate }: HandleGlobalWebSocketEventOptions, + { + showToast, + maintenanceRefetch, + setMaintenance, + onActivityTimerUpdate, + onAnnouncementPublished, + }: HandleGlobalWebSocketEventOptions, ): Promise { switch (data.type) { case 'notification': @@ -74,10 +85,13 @@ export async function handleGlobalWebSocketEvent( console.log("[WS] Django consumer 'load-game' message not currently in use."); break; case 'activity_timer_update': - if (data.data?.activity_timer) { + if (data.data && 'activity_timer' in data.data) { onActivityTimerUpdate?.(data.data.activity_timer); } break; + case 'announcement_published': + onAnnouncementPublished?.(); + break; default: console.warn('[WS] Unknown action:', data); } diff --git a/gameworld/admin.py b/gameworld/admin.py index cfdb664e..3ba49685 100644 --- a/gameworld/admin.py +++ b/gameworld/admin.py @@ -15,7 +15,7 @@ class DailySunTimesAdmin(admin.ModelAdmin): ] fields = [ "id", - "name", + "date", ("dawn", "dusk"), ("sunrise", "sunset"), ] diff --git a/locations/services/schedule.py b/locations/services/schedule.py index c2fffbba..783bc86f 100644 --- a/locations/services/schedule.py +++ b/locations/services/schedule.py @@ -14,6 +14,8 @@ # seconds either way, so the whole village doesn't flip home/work in lockstep. MAX_STAGGER_SECONDS = 20 * 60 +_UNSET = object() + def _stagger_offset_seconds(character_id: int) -> int: """Deterministic per-character offset in [-MAX_STAGGER_SECONDS, MAX_STAGGER_SECONDS].""" @@ -21,7 +23,7 @@ def _stagger_offset_seconds(character_id: int) -> int: return (character_id % span) - MAX_STAGGER_SECONDS -def work_hours_for(character) -> tuple[time, time]: +def work_hours_for(character, work_location=_UNSET) -> tuple[time, time]: """The (open, close) window a character should be at work, from their assigned work building's open_time/close_time if set, else the fixed WORK_START/WORK_END constants. Shared by target_role_for (drives @@ -29,17 +31,23 @@ def work_hours_for(character) -> tuple[time, time]: scheduled CharacterActivity blocks), so a character's actual work building's hours - e.g. an inn open until 23:00 - govern both rather than generate_day assuming a fixed 8-17 workday that leaves late - building hours showing as an unrelated leisure/"Relaxing" block.""" + building hours showing as an unrelated leisure/"Relaxing" block. + + `work_location` may be passed in pre-fetched (e.g. by a caller batching + lookups across many characters, such as commute_tick) to avoid a + per-character query. Left unset, it's looked up here as before. + """ from character.models import CharacterLocation work_start, work_end = WORK_START, WORK_END - work_location = ( - CharacterLocation.objects.filter( - character=character, role=CharacterLocation.Role.WORK, is_primary=True + if work_location is _UNSET: + work_location = ( + CharacterLocation.objects.filter( + character=character, role=CharacterLocation.Role.WORK, is_primary=True + ) + .select_related("location") + .first() ) - .select_related("location") - .first() - ) if work_location is not None: building = work_location.location if building.open_time is not None and building.close_time is not None: @@ -47,19 +55,22 @@ def work_hours_for(character) -> tuple[time, time]: return work_start, work_end -def target_role_for(character, now=None) -> str: +def target_role_for(character, now=None, work_location=_UNSET) -> str: """Which role (home/work) a character should currently be at. The work window comes from the character's assigned work building's open_time/close_time if set, else falls back to the fixed WORK_START/ WORK_END constants. A per-character stagger is applied to whichever - window is resolved, so the whole village doesn't flip in lockstep.""" + window is resolved, so the whole village doesn't flip in lockstep. + + `work_location` is forwarded to work_hours_for - see its docstring. + """ from character.models import CharacterLocation now = now or timezone.localtime() seconds_since_midnight = now.hour * 3600 + now.minute * 60 + now.second - work_start, work_end = work_hours_for(character) + work_start, work_end = work_hours_for(character, work_location=work_location) offset = _stagger_offset_seconds(character.id) work_start_seconds = work_start.hour * 3600 + work_start.minute * 60 + offset @@ -70,21 +81,18 @@ def target_role_for(character, now=None) -> str: return CharacterLocation.Role.HOME -_UNSET = object() - - def sync_character_location( - character, target_location=_UNSET, entrance_node=_UNSET + character, target_role=_UNSET, target_location=_UNSET, entrance_node=_UNSET ) -> None: """Compare a character's current/target position against their schedule and, if they should be elsewhere, send them there via the existing Journey/set_destination movement stack. No-op if already there, already heading there, mid-journey, or no matching CharacterLocation/path exists. - `target_location` and `entrance_node` may be passed in pre-fetched (e.g. - by a caller batching lookups across many characters, such as - commute_tick) to avoid a per-character query. Left unset, they're looked - up here as before. + `target_role`, `target_location`, and `entrance_node` may be passed in + pre-fetched (e.g. by a caller batching lookups across many characters, + such as commute_tick) to avoid a per-character query. Left unset, + they're looked up here as before. """ from character.models import CharacterLocation from locations.models import Node @@ -92,7 +100,8 @@ def sync_character_location( if character.is_moving: return - target_role = target_role_for(character) + if target_role is _UNSET: + target_role = target_role_for(character) if target_location is _UNSET: target_location = ( diff --git a/locations/tasks.py b/locations/tasks.py index 8f0dbc6d..e7be9402 100644 --- a/locations/tasks.py +++ b/locations/tasks.py @@ -150,15 +150,32 @@ def commute_tick(): if not characters: return + # Batch every character's primary locations (home + work) in one query, + # rather than letting target_role_for's work_hours_for lookup issue its + # own per-character CharacterLocation query in the loop below (was an + # N+1 flagged by Sentry). + primary_locations = list( + CharacterLocation.objects.filter( + character_id__in=[character.id for character in characters], + is_primary=True, + ).select_related("location") + ) + work_locations_by_character = { + char_location.character_id: char_location + for char_location in primary_locations + if char_location.role == CharacterLocation.Role.WORK + } + target_roles = { - character.id: target_role_for(character) for character in characters + character.id: target_role_for( + character, work_location=work_locations_by_character.get(character.id) + ) + for character in characters } target_locations = { char_location.character_id: char_location - for char_location in CharacterLocation.objects.filter( - character_id__in=target_roles, is_primary=True - ).select_related("location") + for char_location in primary_locations if char_location.role == target_roles[char_location.character_id] } @@ -181,6 +198,7 @@ def commute_tick(): ) sync_character_location( character, + target_role=target_roles[character.id], target_location=target_location, entrance_node=entrance_node, ) diff --git a/locations/views.py b/locations/views.py index 386c2aba..554e1333 100644 --- a/locations/views.py +++ b/locations/views.py @@ -90,6 +90,7 @@ def get(self, request, pk): .select_related("from_node", "to_node") .only( "id", + "geom", "from_node__location", "to_node__location", ) @@ -240,7 +241,7 @@ def get(self, request): paths = ( Path.objects.filter(geom__isnull=False, geom__bboverlaps=bbox) .select_related("from_node", "to_node") - .only("id", "from_node__location", "to_node__location") + .only("id", "geom", "from_node__location", "to_node__location") ) roads = Road.objects.filter(geom__bboverlaps=bbox) characters = (