fix(api): connect uploaded instruments when a group is created - #1519
Merged
Conversation
In demo mode the manage group page is read-only, so a group whose accessibleInstrumentIds is empty leaves the administer-instrument and remote-assignment pages with nothing to select and no way to fix it. Fall back to the non-repo instruments when isDemo is set, and pre-select them on the read-only manage page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dropping the PROD guard locked the manage page in the test and dev builds too, so group-manage.spec.ts could no longer uncheck an instrument. Restore the guard and gate the demo pre-selection on the same condition, so an editable page never shows boxes ticked that the group's stored selection does not contain. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GroupsService.create selected the instruments to connect with `sourceRepoId: null`. InstrumentsService.create never writes that field, so on an instrument that was never imported from a repository the key is absent rather than null, and Prisma's null filter does not match an absent key. Every group was therefore created with nothing connected: demo groups had no instruments to administer or assign, and because the manage page is read-only in demo there was no way to fix it from the UI. Match an absent field as well as a null one, as the sibling seriesGroupId clause already did. group-manage.spec.ts could not catch this: `uncheck()` is a no-op on an already-unchecked box and the later assertion is `not.toBeChecked()`, so the test passed whether or not the group had anything accessible. Assert the box starts checked. Reverts the client-side demo workaround from 5e58703 and cf0521c. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
thomasbeaudry
changed the title
The api.createGroup fixture PATCHes accessible instruments in after creating a group, so no spec exercised what creation itself connects. Assert it directly against a raw POST /groups. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
joshunrau
approved these changes
Sep 1, 2026
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
2 participants
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.
On a fresh clone with demo mode enabled, the administer-instrument and remote-assignment pages offer no instruments to select. The manage group page is read-only in demo, so there is no way to fix it from the UI either.
Cause
GroupsService.createpicks the instruments to connect withsourceRepoId: null.InstrumentsService.createnever writes that field, so on an instrument that was never imported from a repository the key is absent rather than null — and Prisma'snullfilter does not match an absent key. The query returned nothing, so every group was created with nothing connected.Confirmed against a freshly seeded demo database:
The 7 with an absent key are exactly the demo instruments, and both groups got none of them. Querying the same collection directly:
{sourceRepoId: null}matches 7 in raw MongoDB,{$eq: null, $exists: true}matches 0, and the replacement filter matches 7.This is not demo-specific — it affects every group created since the repo feature landed, including admin-created groups, which silently receive no manually-uploaded instruments.
Fix
Match an absent field as well as a null one, as the sibling
seriesGroupIdclause already did.Why CI never caught it
The
api.createGrouptest fixture PATCHes accessible instruments in immediately after creating a group, so no spec ever exercised what creation connects on its own.This PR adds one that bypasses the fixture: a raw
POST /groups, then an assertion that the new group came back with a non-emptyaccessibleInstrumentIds.Verified rather than assumed. Running
mainplus only that test, with the service fix left out, CI fails on it:With the fix, CI is green.
Existing instances
A code fix only affects groups created after it. Instances already carrying groups with an empty list need those instruments re-selected on the manage page, or a re-run of setup. Demo instances are re-seeded by setup, so they recover on their own.
🤖 Generated with Claude Code