ERA-12077: Realtime handling for subject create/deactivate#1620
Open
jeslefcourt wants to merge 5 commits into
Open
ERA-12077: Realtime handling for subject create/deactivate#1620jeslefcourt wants to merge 5 commits into
jeslefcourt wants to merge 5 commits into
Conversation
Consumes new_subject / delete_subject realtime socket messages: - subjectStore insert (with name=title||name normalization, null last_position guarded) / remove. - mapSubjects.subjects add/remove so the map selector surfaces/hides the subject. - subjectGroups recursive insert into nodes in subject_group_ids and recursive remove on delete; helpers are non-mutating and preserve references for untouched subtrees. - config maps new_subject/delete_subject to their action creators. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Companion backend PR: PADAS/das#4046 |
🚀 PR Environment Deployed
Access: https://era-12077.dev.pamdas.org |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds realtime support for subject lifecycle changes so newly created and deactivated subjects appear/disappear immediately in the UI (map + subject-group sidebar) by handling new socket message types and updating the relevant Redux slices.
Changes:
- Added
new_subject/delete_subjectsocket message mappings to dispatch new subject actions. - Implemented reducers/action creators to insert/remove subjects across
subjectStore,mapSubjects.subjects, andsubjectGroups. - Added Jest coverage for the new socket config mappings and reducer behaviors (insert/remove/idempotency/nested groups).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/withSocketConnection/useRealTimeImplementation/config.js | Maps new_subject/delete_subject socket messages to new subject action creators + socket health updates. |
| src/withSocketConnection/useRealTimeImplementation/config.test.js | Verifies the new socket message types map to the correct handlers. |
| src/ducks/subjects.js | Adds new socket action types and reducer handling for subject insert/remove across subject store, map subjects list, and subject group tree. |
| src/ducks/subjects.test.js | Adds reducer/action creator tests for new subject insert/remove behavior across all affected stores. |
…dates Addresses code-review feedback: the new reducer paths allocated fresh objects/arrays even when a realtime message wouldn't change state, which defeats memoized selectors and causes avoidable re-renders. - addSubjectIdToGroups now returns the original group/array references when the subject isn't appended anywhere (mirrors removeSubjectIdFromGroups). - subjectStore SOCKET_DELETE_SUBJECT returns the original state when the id is absent (true no-op). - mapSubjects SOCKET_NEW_SUBJECT/SOCKET_DELETE_SUBJECT short-circuit to the existing state when membership wouldn't change. - Adds reference-equality tests for each no-op path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rst subject_status updateSubjectLastPositionFromSocketStatusUpdate dereferenced subject.last_position.radio_state, which threw for a subject inserted via new_subject with last_position: null. Guarded the null access (and coordinateProperties), so the first subject_status populates last_position and the map marker renders. Adds merge + integration regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
luixlive
approved these changes
Jul 17, 2026
luixlive
left a comment
luixlive
left a comment
Contributor
There was a problem hiding this comment.
A couple minor improvement suggestions 👍
| (group) => group.id === groupId || isGroupIdInTree(group.subgroups, groupId) | ||
| ); | ||
|
|
||
| const socketNewSubjectAction = (payload) => ({ type: SOCKET_NEW_SUBJECT, payload }); |
Contributor
There was a problem hiding this comment.
This one liner is not being reused or abstracting anything really, we could keep it as:
dispatch({ type: SOCKET_NEW_SUBJECT, payload });| return lastKnownMapSubjectValue; | ||
| }, INITIAL_MAP_SUBJECT_STATE); | ||
|
|
||
| // Recursively removes a subject ID from every node in the groups tree. |
Contributor
There was a problem hiding this comment.
This and the next comments are good for clarifying what the method does, but are too verbose and get deep in details. Let's keep our comments concise 👍
// Recursively removes a subject ID from every node in the groups tree.
// Returns the original array/group reference when nothing changed.
// Recursively appends subjectId to the subjects array of any group node whose
// id is in targetGroupIds. Idempotent. Returns the original array/group
// reference when nothing changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ERA-12077 — Realtime handling for subject create / deactivate (UI)
Handles two new realtime socket messages so subjects appear/disappear live on the map and in the subject-group sidebar, without waiting for the next REST refetch. Mirrors existing
new_event/subject_statushandling.Companion backend PR: PADAS/das#ERA-12077 (link added after creation).
What changed
withSocketConnection/useRealTimeImplementation/config.js— mapsnew_subject→socketNewSubject,delete_subject→socketDeleteSubject.ducks/subjects.js— new action types/creators and reducer handling across three stores:subjectStore— insert (withname = title || namenormalization matching the fetch path, null-last_positionguarded) / remove.mapSubjects.subjects— add/remove the id so the map selector surfaces/hides the subject.subjectGroups— recursive insert into the nodes named insubject_group_ids, recursive remove on delete. Helpers are non-mutating;removeSubjectIdFromGroupspreserves references for untouched subtrees to keep selector memoization intact.Message contract
Design note — divergence from the AC (needs reviewer/PM sign-off)
The ticket AC suggested reusing the existing
subject_statusmessage + anis_activecheck. This PR consumes dedicatednew_subject/delete_subjectmessages instead (see the backend PR for rationale). Behaviour delivered is equivalent.Testing
ducks/subjects.test.js— insert/remove across all three stores; idempotency; delete-of-unknown no-op; missing/emptysubject_group_ids; unknown/unloaded group ids skipped without crash; nested subgroup handling;name-from-titlenormalization incl. nulllast_position; mutation-safety assertions; reference-preservation for untouched subtrees.withSocketConnection/useRealTimeImplementation/config.test.js— the two new message types map to the correct action creators.🤖 Generated with Claude Code