feat: admin user table improvements, config tabs, and participant view#320
Open
feat: admin user table improvements, config tabs, and participant view#320
Conversation
…ant view - Allow double delegations: change committeeMember/conferenceMember user relations from one-to-one to one-to-many so multiple users can share a seat - Add admin config table with TanStack Table: sortable columns, global faceted search, paginated with ellipsis, edit modal for role and seat assignment - Show OIDC name and committee columns in the admin user table - Increase Rumble defaultLimit from 300 to 1000 to support larger conferences - Add participant view with identity card and committee pages - Add self-add to speakers list functionality - Add i18n strings for new features (en + de) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…legations, and NSAs Restructure the config page into 5 tabs (General, Users, Committees, Delegations, Non-State Actors). Add backend mutations for create/update/delete on conferences, committees, representations, committee members, and conference members. Extract existing user management into UsersTab and build new tab components for managing conference settings, committees, delegations (with bulk country-code add and committee seat editing), and NSA/UN actors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Position 0 (currently speaking) now shows "You're up!" badge instead of "#1", and queue numbering starts at the actual position. Badge and remove button are now stacked full width. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a comprehensive participant-facing feature set with committee member management capabilities. It adds new GraphQL mutations for creating, updating, and deleting committees, representations, and member assignments; refactors database relationships to support plural users per member record; reorganizes the mission-control UI into a tab-based interface; and implements a new participant portal with committee views, identity cards, and self-managed speakers list capabilities. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/handlers/committee.ts (1)
196-218:⚠️ Potential issue | 🟠 Major
updateCommitteeauthorization is inconsistent with new create/delete policy.
createCommittee/deleteCommitteeuseassertConferenceAdmin, butupdateCommitteestill gates byctx.abilities.committee.filter('update')(Line [216]). This can block conference admins from updating committees while still allowing them to create/delete.Proposed fix
resolve: async (query, root, args, ctx, info) => { + const committee = await db.query.committee.findFirst({ + where: { id: args.id } + }); + if (!committee) { + throw new GraphQLError('Committee not found'); + } + await assertConferenceAdmin(ctx, committee.conferenceId); + await db .update(schema.committee) .set({ name: args.name ?? undefined, abbreviation: args.abbreviation ?? undefined, whiteboardContent: args.whiteboardContent ?? undefined, showWhiteboard: args.showWhiteboard ?? undefined, status: args.status ?? undefined, statusHeadline: args.statusHeadline ?? undefined, statusUntil: args.statusUntil ?? undefined, stateOfDebate: args.stateOfDebate ?? undefined, activeAgendaItemId: args.activeAgendaItemId ?? undefined, lastResolutionAdoptionDate: args.lastResolutionAdoptionDate ?? undefined, allowDelegationsToAddThemselvesToSpeakersList: args.allowDelegationsToAddThemselvesToSpeakersList ?? undefined }) - .where( - and( - eq(schema.committee.id, args.id), - ctx.abilities.committee.filter('update').sql.where - ) - ); + .where(eq(schema.committee.id, args.id));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/handlers/committee.ts` around lines 196 - 218, The updateCommittee resolver currently authorizes via ctx.abilities.committee.filter('update') which is inconsistent with createCommittee/deleteCommittee that call assertConferenceAdmin; updateCommittee should perform the same conference-admin check before running the DB update. Replace the use of ctx.abilities.committee.filter('update') in the WHERE clause of the db.update in the resolve function with a prior call to the shared admin assertion (e.g., invoke ctx.auth.assertConferenceAdmin(...) or the same helper used by createCommittee/deleteCommittee using args.conferenceId) and then run the update constrained only by eq(schema.committee.id, args.id); remove the ability-filter expression so conference admins are allowed to update.
🧹 Nitpick comments (2)
src/routes/app/[conferenceId]/mission-control/config/+page.svelte (1)
51-92: Add ARIA selected-state to tabs for assistive tech parity.Line 53 to Line 89 uses
role="tab"but does not exposearia-selected/focus state, so screen-reader tab semantics are incomplete.Suggested accessibility adjustment
<button role="tab" class="tab" + aria-selected={activeTab === 'general'} + tabindex={activeTab === 'general' ? 0 : -1} class:tab-active={activeTab === 'general'} onclick={() => (activeTab = 'general')} >Apply the same pattern to the other tab buttons.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/app/`[conferenceId]/mission-control/config/+page.svelte around lines 51 - 92, The tab buttons with role="tab" (the ones using activeTab and labels like m.general(), m.users(), m.committees(), m.delegations(), m.nonStateActors()) need explicit ARIA selected/focus state: add aria-selected={activeTab === '...'} for each button (true when its key matches activeTab, false otherwise) and set tabindex={activeTab === '...' ? 0 : -1} so keyboard and screen-reader users see which tab is active; apply this pattern to all tab buttons and keep the existing onclick handlers that set activeTab.src/routes/app/[conferenceId]/mission-control/config/DelegationsTab.svelte (1)
90-107: Bulk add is fully sequential; consider batching to reduce latency and partial-stop behavior.The current loop waits per country and aborts the rest on first rejection. For large imports, this is noticeably slow.
Refactor option (continue-through-failures)
async function handleAddCountries( countries: { alpha2Code: string; alpha3Code: string; name: string }[] ) { - for (const country of countries) { - await toast.promise( - CreateRepresentationMutation.mutate({ - conferenceId, - type: 'DELEGATION', - alpha2Code: country.alpha2Code, - alpha3Code: country.alpha3Code, - name: country.name - }), - promiseToastStrings(m.delegations(), 'create') - ); - } + const results = await Promise.allSettled( + countries.map((country) => + CreateRepresentationMutation.mutate({ + conferenceId, + type: 'DELEGATION', + alpha2Code: country.alpha2Code, + alpha3Code: country.alpha3Code, + name: country.name + }) + ) + ); + + const failed = results.filter((r) => r.status === 'rejected').length; + if (failed > 0) { + toast.error(`${failed} ${m.delegations()} failed to create`); + } else { + toast.success(m.delegations()); + } + cache.markStale(); await invalidateAll(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/app/`[conferenceId]/mission-control/config/DelegationsTab.svelte around lines 90 - 107, The handleAddCountries function currently awaits each CreateRepresentationMutation.mutate serially which is slow and stops on first failure; change it to send mutations in parallel (with controlled concurrency if needed) by mapping countries to mutation promises and using Promise.allSettled (or chunked Promise.allSettled for large lists) so failures don't abort the rest, then process results to show success/error toasts per country, and only after all settle call cache.markStale() and await invalidateAll(); keep references to CreateRepresentationMutation.mutate, promiseToastStrings, cache.markStale, and invalidateAll for locating and updating the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/en.json`:
- Line 237: Update the tooltip copy for the simple majority message: replace the
current value of "simpleMajorityTooltip" with clearer text such as "Number of
votes needed for a simple majority." Also find and update the duplicate
occurrence noted at the other location (the second "simpleMajorityTooltip"
instance) to the same corrected copy so both entries match.
In `@src/api/handlers/agendaItem.ts`:
- Around line 46-49: The current rule using
abilityBuilder.agendaItem.allow('read').when(({ mustBeLoggedIn }) => {
mustBeLoggedIn(); return 'allow'; }) grants read to any authenticated user and
must be replaced with a scoped Rumble DSL rule that enforces conference-level
authorization; update the handler to use the Rumble DSL ability (e.g.,
abilityBuilder.agendaItem.allow('read').when or the equivalent Rumble helper) to
verify the user is a member/organizer of the relevant conference (check
conferenceId on the agenda item and the caller’s roles/membership) instead of
calling mustBeLoggedIn() alone so only users with proper conference
membership/role can read the agenda item.
In `@src/api/handlers/committeeMember.ts`:
- Around line 19-22: The current rule
abilityBuilder.committeeMember.allow('read').when(({ mustBeLoggedIn }) => {
mustBeLoggedIn(); return 'allow'; }) grants any logged-in user read access;
replace it with a Rumble DSL ability that checks both authentication and
conference scoping/role before allowing read (i.e., require mustBeLoggedIn()
plus validate the actor’s conferenceId or specific role/permission matches the
CommitteeMember record's conferenceId using the Rumble ability check for the
CommitteeMember resource). Locate the abilityBuilder.committeeMember rule and
change it to use Rumble-style checks that enforce conference-level ownership or
an explicit permission (e.g., conferenceId equality or a conferenceAdmin
permission) instead of unconditional allow.
- Around line 43-48: The insert into committeeMember
(db.insert(schema.committeeMember).values(...)) doesn't verify the
representation belongs to the same conference as the committee; before
inserting, fetch the representation by args.representationId and compare its
conferenceId to the committee.conferenceId (the committee was looked up
earlier), and only proceed with the insert if they match—otherwise throw or
return a validation error; update the handler around the committee lookup/insert
to perform this guard using the representation record's conferenceId.
In `@src/api/handlers/conference.ts`:
- Around line 84-109: In the resolve function, avoid publishing a pubsub.updated
event when no row was changed and reject empty patches: after calling
db.update(schema.conference).set(...).where(...), capture the update result/row
count (from the update execution/returning API) and only call
pubsub.updated(args.id) if the update affected >0 rows; additionally validate
that at least one of args.title, args.pressWebsite, or args.hasModeratedCaucus
is provided (not undefined/null) and throw a validation error before performing
the update if the patch is empty. Use the existing resolve, db.update(...).set,
and pubsub.updated symbols to locate and implement these checks.
In `@src/api/handlers/conferenceMember.ts`:
- Around line 34-39: Before inserting into conferenceMember, verify that the
provided args.representationId actually belongs to the target args.conferenceId
by querying schema.representation for a row with id = args.representationId and
conferenceId = args.conferenceId; if that lookup returns no row, abort and
return/throw a validation error instead of performing the insert into
schema.conferenceMember. Use the same db client (the code around
db.insert(schema.conferenceMember) and args.representationId/args.conferenceId)
and reference schema.representation for the guard query so you prevent
cross-conference links.
- Around line 19-22: The current rule
abilityBuilder.conferenceMember.allow('read') only calls mustBeLoggedIn(),
granting any logged-in user global read access; replace this with a Rumble DSL
ability check that enforces scoped access (e.g., only the conference organizer,
a member of the same conference, or a system admin can read). Update the rule so
instead of relying solely on mustBeLoggedIn() it uses the Rumble DSL predicate
to verify ownership/role (use the Rumble ability check for conference membership
or admin role) within the allow('read') when-clause so reads are limited to
permitted actors.
In `@src/api/handlers/conferenceUser.ts`:
- Around line 20-23: The current abilityBuilder.conferenceUser.allow('read')
rule calls mustBeLoggedIn() but lacks conference scoping, allowing any logged-in
user to read ConferenceUser (which contains userEmail) across conferences;
update the when(...) predicate for conferenceUser.read to accept the resource
and context, call mustBeLoggedIn(), then enforce that the target ConferenceUser
belongs to the same conference as the requester (e.g., compare
resource.conferenceId to context.conferenceId) or that the requester has an
explicit conference-level role (e.g.,
context.isConferenceAdmin(resource.conferenceId)) before returning 'allow' so
reads are limited to the correct conference.
In `@src/api/handlers/representation.ts`:
- Around line 42-69: Wrap the multi-step create/delete flows in a single DB
transaction so partial state can't be left behind: for the representation create
flow, run the
insert(schema.representation).returning().then(assertFirstEntryExists) and the
subsequent committee findMany + insert(schema.committeeMember) inside the same
transaction (use the library transactional API, e.g., db.transaction or
equivalent) and use the transaction handle (tx) in place of db for the dependent
queries; do the same for the delete flow referenced (the code around the 103-114
block) so deletion and cleanup are executed atomically and will roll back on
error.
In `@src/api/handlers/speakerOnList.ts`:
- Around line 213-223: The conferenceUser lookup in the db.transaction blocks
(using tx.query.conferenceUser.findFirst) is only filtering by userEmail and
must be scoped to the target conference to prevent cross-conference mutations;
update the queries in the functions that create/delete speaker-on-list entries
(the transaction callbacks that set createdId and the similar blocks at the
other noted occurrences) to include a where clause that also filters by the
intended conference id (use the conferenceId or targetConference variable/param
from the request/payload) and ensure any related with: committeeMember/committee
and conferenceMember joins remain the same; apply the same change to the other
similar tx.query.conferenceUser.findFirst calls in this file so all lookups are
scoped to the correct conference.
- Around line 25-28: The current rule in
abilityBuilder.speakerOnList.allow('read') only calls mustBeLoggedIn() and
returns allow for any authenticated user; change it to also verify conference
membership before allowing read: call mustBeLoggedIn(), extract the conference
identifier from the resolver context/params (e.g., params.conferenceId or
context.conferenceId), then check membership via the existing membership helper
(e.g., isConferenceMember(user, conferenceId) or
user.conferences.includes(conferenceId)); only return 'allow' when that
membership check passes, otherwise return 'deny' (or do not allow). Ensure you
update the ability rule that references
abilityBuilder.speakerOnList.allow('read') and keep mustBeLoggedIn() as the
first call.
In `@src/api/handlers/speakersList.ts`:
- Around line 59-62: The current rule in
abilityBuilder.speakersList.allow('read').when(({ mustBeLoggedIn }) => {
mustBeLoggedIn(); return 'allow'; }) grants read to any logged-in user; change
it to enforce the original whitelist/conference-bound restriction by calling the
login check and then validating conference membership or whitelist for the
requested speakers list (e.g., use the existing context helpers like
checkConferenceWhitelist or mustBeInConference and the request
params/conferenceId) and return 'allow' only when that whitelist/membership
check passes, otherwise deny.
In `@src/routes/app/`[conferenceId]/mission-control/config/DelegationsTab.svelte:
- Line 128: The table header "Alpha-3" in DelegationsTab.svelte is hardcoded;
replace the literal <th>Alpha-3</th> with the app's localization call (e.g. use
the project's translate helper used elsewhere in this component such as t('...')
or $t('...') — match the pattern used in DelegationsTab.svelte) and reference a
new key like "alpha3" (or "table.alpha3") in your translation JSONs; add the
corresponding entries to all locale files so the header is translated
consistently across languages.
In
`@src/routes/app/`[conferenceId]/mission-control/config/EditConferenceUserModal.svelte:
- Around line 73-79: The effect only syncs form state when `user` changes, so
reopening the modal for the same user leaves previous unsaved edits; add a
dedicated reset function (e.g. resetForm()) that sets `selectedRole =
user.conferenceUserType as RoleType`, `selectedCommitteeMemberId =
user.committeeMember?.id ?? null`, and `selectedConferenceMemberId =
user.conferenceMember?.id ?? null`, then call that reset from the existing
$effect and also whenever the modal visibility toggles (hook into the modal open
prop/state — e.g. `open`, `isOpen`, or the component event that shows the modal)
so the form is reinitialized on each open even if `user` hasn't changed.
In `@src/routes/app/`[conferenceId]/mission-control/config/UsersTab.svelte:
- Around line 401-406: The search input in UsersTab.svelte currently relies on
placeholder text only; add an explicit accessible label by either adding a
<label> tied to the input's id or adding an aria-label/aria-labelledby attribute
on the input so screen readers announce its purpose; reference the input that
binds to globalFilter and uses placeholder={m.searchUsers()} and update that
element accordingly to include the new id/label or aria-label value (use
m.searchUsers() or a descriptive string).
- Around line 343-352: The current for-loop calling
CreateConferenceUserMutation.mutate inside await toast.promise aborts on the
first failing email; wrap each per-email await toast.promise call in its own
try/catch so a single mutation failure doesn’t break the loop (use the existing
CreateConferenceUserMutation.mutate, toast.promise and promiseToastStrings calls
inside the try), collect successes and failures (e.g., arrays or counters) as
you iterate, and after the loop surface an aggregated result or summary toast
for the user so all valid emails are attempted even if some fail.
In `@src/routes/app/`[conferenceId]/participant/[committeeId]/+page.svelte:
- Around line 199-202: The primary action button inside the participant list
card (the button that calls handleSelfAdd(list.id) and renders m.addMeToList())
should be made full-width on mobile by adding the appropriate utility class
(e.g., w-full) to the button's class list so it matches other controls and
provides a consistent touch target; update the <button> element that triggers
handleSelfAdd to include w-full (and adjust spacing classes if needed to keep
icon/text layout).
In `@src/routes/app/`[conferenceId]/participant/[committeeId]/+page.ts:
- Around line 5-7: The ParticipantCommitteeQuery currently filters committees
only by committeeId; update the GraphQL query signature to accept $conferenceId:
ID! and scope the where filter on findFirstCommittee to include conferenceId
(e.g. where: { id: $committeeId, conferenceId: $conferenceId }); likewise update
the other similar query instance (the second occurrence of the same committee
lookup) to take and pass $conferenceId, and ensure the code that calls these
queries in +page.ts supplies the route's conferenceId variable when invoking the
query.
- Around line 13-14: The participant payload currently includes
whiteboardContent unconditionally; change the server-side logic that builds the
response in the route handling (the code that produces showWhiteboard and
whiteboardContent) to only add whiteboardContent when showWhiteboard is true (or
move whiteboardContent retrieval into a separate, access-controlled endpoint);
locate the variables/fields named showWhiteboard and whiteboardContent in
+page.ts and gate the data fetch/assignment so hidden whiteboard data is never
included in the participant response unless explicitly allowed.
In
`@src/routes/app/`[conferenceId]/participant/[committeeId]/committeeSubscription.ts:
- Around line 4-14: The subscription ParticipantCommitteeSubscription currently
queries findFirstCommittee by just ($id) and unconditionally streams
whiteboardContent; tighten scope by adding conference context (accept and use a
$conferenceId variable and include it in the findFirstCommittee where clause so
the subscription is conference-scoped) and remove or stop broadcasting
whiteboardContent to all participant clients (return only non-sensitive fields
like showWhiteboard and whiteboard metadata in the subscription payload, and
expose whiteboardContent only via a separate authorized query or via server-side
filtering/authorization in the resolver). Target symbols:
ParticipantCommitteeSubscription, findFirstCommittee, whiteboardContent,
showWhiteboard.
---
Outside diff comments:
In `@src/api/handlers/committee.ts`:
- Around line 196-218: The updateCommittee resolver currently authorizes via
ctx.abilities.committee.filter('update') which is inconsistent with
createCommittee/deleteCommittee that call assertConferenceAdmin; updateCommittee
should perform the same conference-admin check before running the DB update.
Replace the use of ctx.abilities.committee.filter('update') in the WHERE clause
of the db.update in the resolve function with a prior call to the shared admin
assertion (e.g., invoke ctx.auth.assertConferenceAdmin(...) or the same helper
used by createCommittee/deleteCommittee using args.conferenceId) and then run
the update constrained only by eq(schema.committee.id, args.id); remove the
ability-filter expression so conference admins are allowed to update.
---
Nitpick comments:
In `@src/routes/app/`[conferenceId]/mission-control/config/+page.svelte:
- Around line 51-92: The tab buttons with role="tab" (the ones using activeTab
and labels like m.general(), m.users(), m.committees(), m.delegations(),
m.nonStateActors()) need explicit ARIA selected/focus state: add
aria-selected={activeTab === '...'} for each button (true when its key matches
activeTab, false otherwise) and set tabindex={activeTab === '...' ? 0 : -1} so
keyboard and screen-reader users see which tab is active; apply this pattern to
all tab buttons and keep the existing onclick handlers that set activeTab.
In `@src/routes/app/`[conferenceId]/mission-control/config/DelegationsTab.svelte:
- Around line 90-107: The handleAddCountries function currently awaits each
CreateRepresentationMutation.mutate serially which is slow and stops on first
failure; change it to send mutations in parallel (with controlled concurrency if
needed) by mapping countries to mutation promises and using Promise.allSettled
(or chunked Promise.allSettled for large lists) so failures don't abort the
rest, then process results to show success/error toasts per country, and only
after all settle call cache.markStale() and await invalidateAll(); keep
references to CreateRepresentationMutation.mutate, promiseToastStrings,
cache.markStale, and invalidateAll for locating and updating the logic.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
messages/de.jsonmessages/en.jsonpackage.jsonschema.graphqlsrc/api/db/relations.tssrc/api/handlers/agendaItem.tssrc/api/handlers/committee.tssrc/api/handlers/committeeMember.tssrc/api/handlers/conference.tssrc/api/handlers/conferenceMember.tssrc/api/handlers/conferenceUser.tssrc/api/handlers/representation.tssrc/api/handlers/speakerOnList.tssrc/api/handlers/speakersList.tssrc/api/rumble.tssrc/lib/components/CommitteeGrid.sveltesrc/lib/components/Majorities.sveltesrc/routes/app/(launcher)/+page.sveltesrc/routes/app/(launcher)/+page.tssrc/routes/app/[conferenceId]/[committeeId]/(chairs)/+layout.tssrc/routes/app/[conferenceId]/[committeeId]/(chairs)/committeeSubscription.tssrc/routes/app/[conferenceId]/[committeeId]/(chairs)/setup/+page.sveltesrc/routes/app/[conferenceId]/mission-control/DownloadPresenceData.sveltesrc/routes/app/[conferenceId]/mission-control/config/+page.sveltesrc/routes/app/[conferenceId]/mission-control/config/+page.tssrc/routes/app/[conferenceId]/mission-control/config/CommitteesTab.sveltesrc/routes/app/[conferenceId]/mission-control/config/DelegationsTab.sveltesrc/routes/app/[conferenceId]/mission-control/config/EditConferenceUserModal.sveltesrc/routes/app/[conferenceId]/mission-control/config/EditDelegationModal.sveltesrc/routes/app/[conferenceId]/mission-control/config/GeneralTab.sveltesrc/routes/app/[conferenceId]/mission-control/config/NsaTab.sveltesrc/routes/app/[conferenceId]/mission-control/config/UsersTab.sveltesrc/routes/app/[conferenceId]/participant/+layout.sveltesrc/routes/app/[conferenceId]/participant/+layout.tssrc/routes/app/[conferenceId]/participant/+page.sveltesrc/routes/app/[conferenceId]/participant/+page.tssrc/routes/app/[conferenceId]/participant/ParticipantIdentityCard.sveltesrc/routes/app/[conferenceId]/participant/[committeeId]/+page.sveltesrc/routes/app/[conferenceId]/participant/[committeeId]/+page.tssrc/routes/app/[conferenceId]/participant/[committeeId]/committeeSubscription.ts
| "short_sleek_snake_hint": "Committee", | ||
| "showOfHandsVoting": "Vote by Show of Hands", | ||
| "simpleMajority": "Simple", | ||
| "simpleMajorityTooltip": "Needed notes for simple majority", |
There was a problem hiding this comment.
Fix tooltip copy typos in majority messages.
These two strings currently contain spelling/wording errors.
✍️ Suggested copy fix
- "simpleMajorityTooltip": "Needed notes for simple majority",
+ "simpleMajorityTooltip": "Needed votes for simple majority",
...
- "twoThirdsMajorityTooltip": "Needed votes for two-thrids majority",
+ "twoThirdsMajorityTooltip": "Needed votes for two-thirds majority",Also applies to: 274-274
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/en.json` at line 237, Update the tooltip copy for the simple
majority message: replace the current value of "simpleMajorityTooltip" with
clearer text such as "Number of votes needed for a simple majority." Also find
and update the duplicate occurrence noted at the other location (the second
"simpleMajorityTooltip" instance) to the same corrected copy so both entries
match.
Comment on lines
+46
to
+49
| abilityBuilder.agendaItem.allow('read').when(({ mustBeLoggedIn }) => { | ||
| mustBeLoggedIn(); | ||
| return 'allow'; | ||
| }); |
There was a problem hiding this comment.
Unscoped read permission opens agenda items to any authenticated account.
Line 46 to Line 49 returns 'allow' for every logged-in user, which bypasses conference-level authorization boundaries and creates a direct authz exposure.
As per coding guidelines, src/api/handlers/*.ts must use Rumble DSL with ability-based access control.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/handlers/agendaItem.ts` around lines 46 - 49, The current rule using
abilityBuilder.agendaItem.allow('read').when(({ mustBeLoggedIn }) => {
mustBeLoggedIn(); return 'allow'; }) grants read to any authenticated user and
must be replaced with a scoped Rumble DSL rule that enforces conference-level
authorization; update the handler to use the Rumble DSL ability (e.g.,
abilityBuilder.agendaItem.allow('read').when or the equivalent Rumble helper) to
verify the user is a member/organizer of the relevant conference (check
conferenceId on the agenda item and the caller’s roles/membership) instead of
calling mustBeLoggedIn() alone so only users with proper conference
membership/role can read the agenda item.
Comment on lines
+19
to
+22
| abilityBuilder.committeeMember.allow('read').when(({ mustBeLoggedIn }) => { | ||
| mustBeLoggedIn(); | ||
| return 'allow'; | ||
| }); |
There was a problem hiding this comment.
committeeMember read authorization is over-broadened.
Line 19 to Line 22 allows read for any logged-in account, which can leak committee-member data across conferences.
As per coding guidelines, src/api/handlers/*.ts must use Rumble DSL with ability-based access control.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/handlers/committeeMember.ts` around lines 19 - 22, The current rule
abilityBuilder.committeeMember.allow('read').when(({ mustBeLoggedIn }) => {
mustBeLoggedIn(); return 'allow'; }) grants any logged-in user read access;
replace it with a Rumble DSL ability that checks both authentication and
conference scoping/role before allowing read (i.e., require mustBeLoggedIn()
plus validate the actor’s conferenceId or specific role/permission matches the
CommitteeMember record's conferenceId using the Rumble ability check for the
CommitteeMember resource). Locate the abilityBuilder.committeeMember rule and
change it to use Rumble-style checks that enforce conference-level ownership or
an explicit permission (e.g., conferenceId equality or a conferenceAdmin
permission) instead of unconditional allow.
Comment on lines
+43
to
+48
| const result = await db | ||
| .insert(schema.committeeMember) | ||
| .values({ | ||
| committeeId: args.committeeId, | ||
| representationId: args.representationId | ||
| }) |
There was a problem hiding this comment.
Ensure representation and committee belong to the same conference before creating membership.
Line 43 to Line 48 inserts directly after committee lookup, but does not verify the representation is from committee.conferenceId. That can create invalid cross-conference associations.
Proposed guard
await assertConferenceAdmin(ctx, committee.conferenceId);
+ const representation = await db.query.representation.findFirst({
+ where: {
+ id: args.representationId,
+ conferenceId: committee.conferenceId
+ }
+ });
+ if (!representation) {
+ throw new GraphQLError('Representation not found in this conference');
+ }
const result = await db
.insert(schema.committeeMember)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const result = await db | |
| .insert(schema.committeeMember) | |
| .values({ | |
| committeeId: args.committeeId, | |
| representationId: args.representationId | |
| }) | |
| await assertConferenceAdmin(ctx, committee.conferenceId); | |
| const representation = await db.query.representation.findFirst({ | |
| where: { | |
| id: args.representationId, | |
| conferenceId: committee.conferenceId | |
| } | |
| }); | |
| if (!representation) { | |
| throw new GraphQLError('Representation not found in this conference'); | |
| } | |
| const result = await db | |
| .insert(schema.committeeMember) | |
| .values({ | |
| committeeId: args.committeeId, | |
| representationId: args.representationId | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/handlers/committeeMember.ts` around lines 43 - 48, The insert into
committeeMember (db.insert(schema.committeeMember).values(...)) doesn't verify
the representation belongs to the same conference as the committee; before
inserting, fetch the representation by args.representationId and compare its
conferenceId to the committee.conferenceId (the committee was looked up
earlier), and only proceed with the insert if they match—otherwise throw or
return a validation error; update the handler around the committee lookup/insert
to perform this guard using the representation record's conferenceId.
Comment on lines
+84
to
+109
| resolve: async (query, root, args, ctx, info) => { | ||
| await assertConferenceAdmin(ctx, args.id); | ||
|
|
||
| await db | ||
| .update(schema.conference) | ||
| .set({ | ||
| title: args.title ?? undefined, | ||
| pressWebsite: args.pressWebsite ?? undefined, | ||
| hasModeratedCaucus: args.hasModeratedCaucus ?? undefined | ||
| }) | ||
| .where(eq(schema.conference.id, args.id)); | ||
|
|
||
| pubsub.updated(args.id); | ||
|
|
||
| return db.query.conference | ||
| .findFirst( | ||
| query( | ||
| ctx.abilities.conference.filter('read', { | ||
| inject: { | ||
| where: { id: args.id } | ||
| } | ||
| }).query.single | ||
| ) | ||
| ) | ||
| .then(assertFindFirstExists); | ||
| } |
There was a problem hiding this comment.
Confirm a successful update before publishing pubsub.updated.
Line [96] publishes an update event even when no row was changed (e.g., bad id). This can create phantom realtime updates. Also, this mutation currently accepts an empty patch payload.
Proposed fix
+import { GraphQLError } from 'graphql';
resolve: async (query, root, args, ctx, info) => {
await assertConferenceAdmin(ctx, args.id);
- await db
+ if (
+ args.title === undefined &&
+ args.pressWebsite === undefined &&
+ args.hasModeratedCaucus === undefined
+ ) {
+ throw new GraphQLError('At least one field must be provided');
+ }
+
+ const updated = await db
.update(schema.conference)
.set({
title: args.title ?? undefined,
pressWebsite: args.pressWebsite ?? undefined,
hasModeratedCaucus: args.hasModeratedCaucus ?? undefined
})
- .where(eq(schema.conference.id, args.id));
+ .where(eq(schema.conference.id, args.id))
+ .returning({ id: schema.conference.id });
- pubsub.updated(args.id);
+ if (updated.length === 0) {
+ throw new GraphQLError('Conference not found');
+ }
+
+ pubsub.updated(updated[0].id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/handlers/conference.ts` around lines 84 - 109, In the resolve
function, avoid publishing a pubsub.updated event when no row was changed and
reject empty patches: after calling
db.update(schema.conference).set(...).where(...), capture the update result/row
count (from the update execution/returning API) and only call
pubsub.updated(args.id) if the update affected >0 rows; additionally validate
that at least one of args.title, args.pressWebsite, or args.hasModeratedCaucus
is provided (not undefined/null) and throw a validation error before performing
the update if the patch is empty. Use the existing resolve, db.update(...).set,
and pubsub.updated symbols to locate and implement these checks.
Comment on lines
+401
to
+406
| <input | ||
| type="text" | ||
| class="input input-bordered w-full max-w-xs" | ||
| placeholder={m.searchUsers()} | ||
| bind:value={globalFilter} | ||
| /> |
There was a problem hiding this comment.
Add an explicit accessible label for the search field.
Line 401 to Line 406 relies on placeholder text only; add a label or aria-label so assistive technologies announce the input purpose consistently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/app/`[conferenceId]/mission-control/config/UsersTab.svelte around
lines 401 - 406, The search input in UsersTab.svelte currently relies on
placeholder text only; add an explicit accessible label by either adding a
<label> tied to the input's id or adding an aria-label/aria-labelledby attribute
on the input so screen readers announce its purpose; reference the input that
binds to globalFilter and uses placeholder={m.searchUsers()} and update that
element accordingly to include the new id/label or aria-label value (use
m.searchUsers() or a descriptive string).
Comment on lines
+199
to
+202
| <button class="btn btn-primary btn-sm" onclick={() => handleSelfAdd(list.id)}> | ||
| <i class="fas fa-plus mr-1"></i> | ||
| {m.addMeToList()} | ||
| </button> |
There was a problem hiding this comment.
Make the self-add button full-width for consistent mobile controls.
This button is the only primary action in the card that is not w-full, which causes inconsistent touch targets.
🎯 Suggested change
- <button class="btn btn-primary btn-sm" onclick={() => handleSelfAdd(list.id)}>
+ <button class="btn btn-primary btn-sm w-full" onclick={() => handleSelfAdd(list.id)}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button class="btn btn-primary btn-sm" onclick={() => handleSelfAdd(list.id)}> | |
| <i class="fas fa-plus mr-1"></i> | |
| {m.addMeToList()} | |
| </button> | |
| <button class="btn btn-primary btn-sm w-full" onclick={() => handleSelfAdd(list.id)}> | |
| <i class="fas fa-plus mr-1"></i> | |
| {m.addMeToList()} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/app/`[conferenceId]/participant/[committeeId]/+page.svelte around
lines 199 - 202, The primary action button inside the participant list card (the
button that calls handleSelfAdd(list.id) and renders m.addMeToList()) should be
made full-width on mobile by adding the appropriate utility class (e.g., w-full)
to the button's class list so it matches other controls and provides a
consistent touch target; update the <button> element that triggers handleSelfAdd
to include w-full (and adjust spacing classes if needed to keep icon/text
layout).
Comment on lines
+5
to
+7
| query ParticipantCommitteeQuery($committeeId: ID!) { | ||
| findFirstCommittee(where: { id: $committeeId }) { | ||
| id |
There was a problem hiding this comment.
Scope committee query by conference to prevent cross-conference reads.
The query currently trusts committeeId alone. Since this route is conference-scoped, bind the fetch to conferenceId too.
🛡️ Suggested fix
-export const _houdini_load = graphql(`
- query ParticipantCommitteeQuery($committeeId: ID!) {
- findFirstCommittee(where: { id: $committeeId }) {
+export const _houdini_load = graphql(`
+ query ParticipantCommitteeQuery($conferenceId: ID!, $committeeId: ID!) {
+ findFirstCommittee(where: { id: $committeeId, conference: { id: $conferenceId } }) {
id
...
}
}
`);
export const _ParticipantCommitteeQueryVariables: ParticipantCommitteeQueryVariables = (event) => {
return {
+ conferenceId: event.params.conferenceId,
committeeId: event.params.committeeId
};
};Also applies to: 77-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/app/`[conferenceId]/participant/[committeeId]/+page.ts around
lines 5 - 7, The ParticipantCommitteeQuery currently filters committees only by
committeeId; update the GraphQL query signature to accept $conferenceId: ID! and
scope the where filter on findFirstCommittee to include conferenceId (e.g.
where: { id: $committeeId, conferenceId: $conferenceId }); likewise update the
other similar query instance (the second occurrence of the same committee
lookup) to take and pass $conferenceId, and ensure the code that calls these
queries in +page.ts supplies the route's conferenceId variable when invoking the
query.
Comment on lines
+13
to
+14
| showWhiteboard | ||
| whiteboardContent |
There was a problem hiding this comment.
Avoid sending hidden whiteboard content in participant payloads.
Fetching whiteboardContent regardless of visibility exposes data in network responses. Gate this server-side or fetch content only in a dedicated allowed path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/app/`[conferenceId]/participant/[committeeId]/+page.ts around
lines 13 - 14, The participant payload currently includes whiteboardContent
unconditionally; change the server-side logic that builds the response in the
route handling (the code that produces showWhiteboard and whiteboardContent) to
only add whiteboardContent when showWhiteboard is true (or move
whiteboardContent retrieval into a separate, access-controlled endpoint); locate
the variables/fields named showWhiteboard and whiteboardContent in +page.ts and
gate the data fetch/assignment so hidden whiteboard data is never included in
the participant response unless explicitly allowed.
Comment on lines
+4
to
+14
| subscription ParticipantCommitteeSubscription($id: ID!) { | ||
| findFirstCommittee(where: { id: $id }) { | ||
| id | ||
| abbreviation | ||
| name | ||
| status | ||
| statusHeadline | ||
| statusUntil | ||
| showWhiteboard | ||
| whiteboardContent | ||
| allowDelegationsToAddThemselvesToSpeakersList |
There was a problem hiding this comment.
Tighten subscription scope and payload sensitivity.
This subscription should be conference-scoped (not id-only) and should not stream whiteboardContent unconditionally to participant clients.
🛡️ Suggested direction
-subscription ParticipantCommitteeSubscription($id: ID!) {
- findFirstCommittee(where: { id: $id }) {
+subscription ParticipantCommitteeSubscription($conferenceId: ID!, $id: ID!) {
+ findFirstCommittee(where: { id: $id, conference: { id: $conferenceId } }) {
...
- showWhiteboard
- whiteboardContent
+ showWhiteboard
+ # fetch whiteboardContent only via a separately authorized path
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| subscription ParticipantCommitteeSubscription($id: ID!) { | |
| findFirstCommittee(where: { id: $id }) { | |
| id | |
| abbreviation | |
| name | |
| status | |
| statusHeadline | |
| statusUntil | |
| showWhiteboard | |
| whiteboardContent | |
| allowDelegationsToAddThemselvesToSpeakersList | |
| subscription ParticipantCommitteeSubscription($conferenceId: ID!, $id: ID!) { | |
| findFirstCommittee(where: { id: $id, conference: { id: $conferenceId } }) { | |
| id | |
| abbreviation | |
| name | |
| status | |
| statusHeadline | |
| statusUntil | |
| showWhiteboard | |
| # fetch whiteboardContent only via a separately authorized path | |
| allowDelegationsToAddThemselvesToSpeakersList |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/routes/app/`[conferenceId]/participant/[committeeId]/committeeSubscription.ts
around lines 4 - 14, The subscription ParticipantCommitteeSubscription currently
queries findFirstCommittee by just ($id) and unconditionally streams
whiteboardContent; tighten scope by adding conference context (accept and use a
$conferenceId variable and include it in the findFirstCommittee where clause so
the subscription is conference-scoped) and remove or stop broadcasting
whiteboardContent to all participant clients (return only non-sensitive fields
like showWhiteboard and whiteboard metadata in the subscription payload, and
expose whiteboardContent only via a separate authorized query or via server-side
filtering/authorization in the resolver). Target symbols:
ParticipantCommitteeSubscription, findFirstCommittee, whiteboardContent,
showWhiteboard.
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
1 participant
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.
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Localization