-
Notifications
You must be signed in to change notification settings - Fork 4
Refactor assignment-assistant routing, schemas, and components #300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor assignment-assistant routing, schemas, and components #300
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
WalkthroughAdds school metadata and aggregation, UI for filtering and normalizing school names, inline delegation school editing, DataTable selection props, GraphQL schema/resolver for Conference.schools and normalizeSchoolsInConference, translation entries (en/de), docs (WARP.md), and a minor Docker base bump. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SightingUI as Sighting Page
participant AppData
participant GraphQL
User->>SightingUI: open sighting page
SightingUI->>AppData: getSchools()
AppData->>GraphQL: query delegations & singleParticipants (with school)
GraphQL-->>AppData: return rows with school
AppData-->>SightingUI: aggregated schools list
SightingUI->>SightingUI: render SchoolFilter and applications
User->>SightingUI: toggle school(s)
SightingUI->>SightingUI: compute inFilter / inSearch / inCurrentPage
SightingUI->>SightingUI: render filtered/paginated list
sequenceDiagram
participant User
participant Drawer as DelegationDrawer
participant GraphQL
participant Cache as HoudiniCache
User->>Drawer: click pencil on school
Drawer->>User: prompt for new school name
User->>Drawer: submit new name
Drawer->>GraphQL: ChangeDelegationSchool mutation
GraphQL-->>Drawer: return updated delegation
Drawer->>Cache: markStale() / invalidateAll()
Drawer->>User: show toast success
Note over Drawer: on error -> log & show error toast
sequenceDiagram
participant User
participant Normalize as NormalizeSchools UI
participant GraphQL
participant Cache
User->>Normalize: select one or many schools
Normalize->>Normalize: set newSchoolName (auto-fill if single)
User->>Normalize: trigger normalize (button or Shift+Enter)
Normalize->>GraphQL: normalizeSchoolsInConference mutation
GraphQL-->>Normalize: mutation result (Conference)
Normalize->>Cache: refresh conference query
Normalize->>User: show success/failure toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
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 |
…ty changes (#291) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/routes/(authenticated)/assignment-assistant/+page.svelte (1)
5-5: Remove unused import.The
graphqlimport from$houdiniis not used anywhere in this file and should be removed to keep the imports clean.Apply this diff:
- import { graphql } from '$houdini';src/api/resolvers/modules/conference/conference.ts (1)
219-282: Remove commented-out code.This 64-line commented block should be removed entirely. Use version control (git) to preserve code history rather than leaving commented code in the codebase. If this functionality is needed, it should be properly implemented and tested; otherwise, it should be deleted.
Apply this diff:
- // schools: t.field({ - // type: [ - // builder.simpleObject('ConferenceSchools', { - // fields: (t) => ({ - // school: t.string(), - // count: t.int() - // }) - // }) - // ], - // resolve: async (conference, _, ctx) => { - // const delegations = await db.delegation.groupBy({ - // where: { - // conferenceId: conference.id, - // school: { - // not: null - // }, - // applied: { - // equals: true - // }, - // AND: [ctx.permissions.allowDatabaseAccessTo('list').Delegation] - // }, - // by: 'school', - // _count: { - // school: true - // } - // }); - // - // const singleParticipants = await db.singleParticipant.groupBy({ - // where: { - // conferenceId: conference.id, - // school: { - // not: null - // }, - // applied: { - // equals: true - // }, - // AND: [ctx.permissions.allowDatabaseAccessTo('list').SingleParticipant] - // }, - // by: 'school', - // _count: { - // school: true - // } - // }); - // - // const res: { school: string; count: number }[] = []; - // - // for (const delegation of delegations) { - // if (!delegation.school) continue; - // res.push({ school: delegation.school, count: delegation._count.school }); - // } - // - // for (const singleParticipant of singleParticipants) { - // if (!singleParticipant.school) continue; - // const existing = res.find((r) => r.school === singleParticipant.school); - // if (existing) { - // existing.count += singleParticipant._count.school; - // } else { - // res.push({ school: singleParticipant.school, count: singleParticipant._count.school }); - // } - // } - // - // return res; - // } - // })src/routes/(authenticated)/management/[conferenceId]/delegations/DelegationDrawer.svelte (1)
158-175: Consider replacingprompt()with a modal dialog.Using the native browser
prompt()for editing the school name has several UX limitations:
- Basic appearance that doesn't match application styling
- Limited accessibility
- No input validation UI
- Cannot use proper form controls
Consider implementing a modal dialog similar to the head delegate selection modal (lines 426-470) for a more consistent and accessible user experience.
Also applies to: 229-236
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/SchoolFilter.svelte (2)
2-3: Remove unused imports.Both
graphqlandqueryParametersare imported but never used in this component. Remove them to keep the imports clean.Apply this diff:
- import { graphql } from '$houdini'; - import { queryParameters } from 'sveltekit-search-params'; import { getSchools } from '../appData.svelte';
13-50: LGTM with minor suggestion.The filter UI implementation is clear and functional. The toggle logic correctly manages the filter array, and the badges provide useful at-a-glance metrics.
Minor suggestion: Consider adding optional chaining or a null check for
school.schoolin the key and filter operations to prevent potential runtime errors if the data structure is unexpected.Example:
{#each getSchools() as school (school.school ?? 'unknown')} <button ... onclick={() => { const schoolName = school.school; if (!schoolName) return; if (filter.includes(schoolName)) { filter = filter.filter((s) => s !== schoolName); } else { filter = [...filter, schoolName]; } }} >src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte (1)
108-112: Consider consistent pagination across filter and search modes.The current logic paginates search results but not filter results. This creates inconsistency:
- Lines 108-112: Pagination shown when searching (no filter)
- Lines 127-139: Pagination + page size shown when neither filtering nor searching
- Filters: No pagination at all
Large filter result sets could benefit from pagination. Also consider using the derived
searchActiveandfilterActivestates instead of checking raw$paramsfor cleaner code.Example refactor:
- {#if $params.filter.length === 0 && searchActive} + {#if !filterActive} <div class="flex items-center justify-center"> <Pagination active={page} total={Math.ceil(getApplications().length / pageSize)} {setPage} /> </div> {/if} {#each getApplications() as application, index} {@const inCurrentPage = index >= (page - 1) * pageSize && index < page * pageSize} {@const filterOrSearchActive = filterActive || searchActive} {@const inFilter = filterActive && $params.filter.includes(application.school ?? 'No School')} {@const inSearch = searchActive && (codenmz(application.id).toLowerCase().includes($params.search.toLowerCase()) || application.school?.toLowerCase()?.includes($params.search.toLowerCase()))} {#if (!filterOrSearchActive && inCurrentPage) || inFilter || inSearch} <Application {application} startConference={conference?.startConference ?? new Date()} /> {/if} {/each} - {#if $params.filter.length === 0 && $params.search === ''} + {#if !filterActive && !searchActive} <div class="flex flex-col items-center justify-center gap-4"> <Pagination active={page} total={Math.ceil(getApplications().length / pageSize)} {setPage} /> <div class="flex items-center gap-4"> <div>Pro Seite:</div> <select class="select select-bordered" bind:value={$params.pageSize}> <option value="10" selected>10</option> <option value="20">20</option> <option value="50">50</option> </select> </div> </div> {/if}Also applies to: 127-139
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
messages/de.json(1 hunks)src/api/resolvers/modules/conference/conference.ts(1 hunks)src/routes/(authenticated)/assignment-assistant/+page.svelte(1 hunks)src/routes/(authenticated)/assignment-assistant/[projectId]/DelegationCard.svelte(4 hunks)src/routes/(authenticated)/assignment-assistant/[projectId]/appData.svelte.ts(4 hunks)src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte(3 hunks)src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/Application.svelte(2 hunks)src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/SchoolFilter.svelte(1 hunks)src/routes/(authenticated)/management/[conferenceId]/assignment/+page.ts(2 hunks)src/routes/(authenticated)/management/[conferenceId]/delegations/DelegationDrawer.svelte(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/api/resolvers/modules/conference/conference.ts (3)
src/api/resolvers/modules/conferenceSupervisor.ts (1)
t(23-41)src/api/resolvers/modules/delegation.ts (2)
t(314-325)t(310-386)src/api/resolvers/modules/singleParticipant.ts (1)
t(29-49)
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte (1)
src/routes/(authenticated)/assignment-assistant/[projectId]/applications.svelte.ts (2)
Application(5-21)id(36-71)
src/routes/(authenticated)/assignment-assistant/[projectId]/DelegationCard.svelte (2)
src/lib/services/ageChecker.ts (1)
getAgeAtConference(3-21)src/api/resolvers/modules/user.ts (1)
t(38-120)
src/routes/(authenticated)/management/[conferenceId]/delegations/DelegationDrawer.svelte (1)
src/api/resolvers/modules/delegation.ts (4)
query(170-289)t(89-154)t(314-325)t(310-386)
src/routes/(authenticated)/management/[conferenceId]/assignment/+page.ts (2)
src/api/resolvers/modules/delegation.ts (3)
query(170-289)t(314-325)t(310-386)src/api/resolvers/modules/singleParticipant.ts (1)
t(29-49)
src/routes/(authenticated)/assignment-assistant/[projectId]/appData.svelte.ts (3)
src/routes/(authenticated)/assignment-assistant/[projectId]/applications.svelte.ts (2)
getApplications(32-34)DelegationApplication(23-25)src/api/resolvers/modules/delegation.ts (4)
query(170-289)t(314-325)root(331-383)t(310-386)src/api/resolvers/modules/singleParticipant.ts (1)
t(29-49)
messages/de.json (1)
scripts/add_new_translation.ts (2)
singleAdd(39-67)saveToTranslationFile(13-37)
🪛 GitHub Actions: Build and publish container image
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte
[error] 43-46: Cannot find name 'getConference'. Did you mean 'conference'?
[error] 46-46: Cannot find name 'loadProjects'.
[error] 110-114: Cannot find name 'getApplications'. Did you mean 'Application'?
🔇 Additional comments (8)
src/routes/(authenticated)/management/[conferenceId]/assignment/+page.ts (1)
9-9: LGTM!The addition of the
schoolfield to both delegation and single participant queries is consistent with the schema and enables the school-based filtering features introduced in this PR.Also applies to: 44-44
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/Application.svelte (1)
226-226: LGTM!Adding character count badges for motivation and experience fields provides useful at-a-glance metrics for reviewing application quality.
Also applies to: 238-238
messages/de.json (1)
321-321: LGTM!The new translation key for prompting school name input is clear and properly placed in the file, supporting the inline school editing feature.
src/routes/(authenticated)/assignment-assistant/[projectId]/DelegationCard.svelte (1)
4-4: LGTM!The additions provide useful derived metrics (average age and conference participation count) for delegation review. The calculations properly handle edge cases with fallback values and division-by-zero protection, and both metrics are appropriately wrapped in LoadingData components.
Also applies to: 8-8, 74-75, 127-142, 204-215
src/routes/(authenticated)/management/[conferenceId]/delegations/DelegationDrawer.svelte (1)
149-156: LGTM!The mutation definition is correct and aligns with the server-side resolver capabilities shown in the code snippets. Server-side permissions will be properly enforced.
src/routes/(authenticated)/assignment-assistant/[projectId]/appData.svelte.ts (3)
98-98: LGTM: Schema changes align with backend.The addition of optional
schoolfields to bothDelegationSchemaandSingleParticipantSchemacorrectly aligns with the backend schema definitions shown in the relevant code snippets, where school is nullable.Also applies to: 112-112
286-293: Verified: Management pages properly handleselected=query parameter.The management pages for individuals, delegations, supervisors, and participants all correctly read the
selectedquery parameter viaqueryParam('selected')to highlight or open the appropriate drawer item. The refactored links ingetMoreInfoLinkare consistent with the existing usage pattern throughout the codebase.
185-194: No changes needed—toSortedis fully supported in the target environment.The codebase uses Node.js 24.x (specified in Dockerfile via
setup_24.x) and Bun runtime, both of which fully support ES2023 features includingtoSorted. The TypeScript target ofESNextcorrectly preserves modern syntax without transpilation, aligning perfectly with the runtime environment. The sorting logic is sound.
src/routes/(authenticated)/assignment-assistant/[projectId]/appData.svelte.ts
Show resolved
Hide resolved
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte
Show resolved
Hide resolved
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte (1)
118-118: Major: Inconsistent undefined school handling breaks filtering.This line converts undefined schools to empty string (
''), but according to the past review comment, thegetSchoolsfunction converts them to"No School". This mismatch means:
- SchoolFilter displays "No School" with a count
- User selects "No School" filter →
$params.filter = ["No School"]- This line checks:
["No School"].includes(undefined ?? '')→["No School"].includes('')→false- Applications with undefined schools won't match the filter
Update the fallback to match:
- {@const inFilter = filterActive && $params.filter?.includes(application.school ?? '')} + {@const inFilter = filterActive && $params.filter?.includes(application.school ?? 'No School')}
🧹 Nitpick comments (2)
src/api/resolvers/modules/conference/conference.ts (1)
264-276: Remove redundant null checks after groupBy with non-null filter.The null checks on lines 265 and 270 are unnecessary because the
groupByqueries already filter forschool: { not: null }. TypeScript should infer thatdelegation.schoolandsingleParticipant.schoolare non-nullable at these points.Apply this diff to simplify:
for (const delegation of delegations) { - if (!delegation.school) continue; res.push({ school: delegation.school, count: delegation._count.school }); } for (const singleParticipant of singleParticipants) { - if (!singleParticipant.school) continue; const existing = res.find((r) => r.school === singleParticipant.school); if (existing) { existing.count += singleParticipant._count.school; } else { res.push({ school: singleParticipant.school, count: singleParticipant._count.school }); } }src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte (1)
8-8: Remove unused import.The
graphqlimport from$houdiniis not used anywhere in this file.Apply this diff:
- import { graphql } from '$houdini';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/api/resolvers/modules/conference/conference.ts(2 hunks)src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte (1)
src/routes/(authenticated)/assignment-assistant/[projectId]/applications.svelte.ts (1)
Application(5-21)
src/api/resolvers/modules/conference/conference.ts (3)
src/api/resolvers/builder.ts (1)
builder(26-66)src/api/resolvers/modules/singleParticipant.ts (1)
t(29-49)src/api/resolvers/modules/delegation.ts (1)
t(89-154)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and publish container images
🔇 Additional comments (8)
src/api/resolvers/modules/conference/conference.ts (1)
567-615: Verify that reusingupdateOneConferenceMutationObjectis appropriate.This mutation updates delegations and single participants, not the conference itself, yet it uses
updateOneConferenceMutationObject(t)as its base. While this works for returning a Conference type, it's semantically misleading.Consider whether a custom field definition would be clearer:
normalizeSchoolsInConference: t.prismaField({ type: 'Conference', args: { conferenceId: t.arg.string({ required: true }), schoolsToMerge: t.arg.stringList(), newSchoolName: t.arg.string({ required: true }) }, // ... rest of implementation })src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte (7)
11-11: LGTM! Missing imports are now resolved.This addresses the critical issue flagged in the previous review.
15-36: LGTM! Query parameter handling is well-structured.The filter and search parameters are properly configured with appropriate encode/decode functions and default values.
38-39: LGTM! Derived state logic is sound.The mutual exclusivity between filter and search (search takes precedence) is a sensible UX decision.
44-48: LGTM! Initialization logic is correct.Conference state and project loading are properly implemented.
101-107: LGTM! UI layout is clean and properly bound.The SchoolFilter and search input are correctly wired to query parameters.
119-122: LGTM! Optional chaining issue is fixed.The double optional chaining (
application.school?.toLowerCase()?.includes()) correctly handles undefined schools and prevents TypeErrors. This resolves the critical issue from the previous review.
128-140: LGTM! Bottom pagination logic is correct.The condition properly hides pagination when filtering or searching is active, and the page size control is well-implemented.
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
schema.graphql (1)
1983-1983: Mutation signature is correct and follows schema conventions.The mutation parameters are well-typed. The return type of
Conference(nullable) is consistent with other mutations in the schema, though it may return more data than necessary after normalization.If performance is a concern, consider returning only the updated schools array rather than the entire Conference object:
- normalizeSchoolsInConference(conferenceId: String!, newSchoolName: String!, schoolsToMerge: [String!]!): Conference + normalizeSchoolsInConference(conferenceId: String!, newSchoolName: String!, schoolsToMerge: [String!]!): [ConferenceSchools!]!However, the current approach is fine if consistency with other mutations is preferred.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
schema.graphql(3 hunks)src/routes/(authenticated)/management/[conferenceId]/cleanup/+page.svelte(2 hunks)src/routes/(authenticated)/management/[conferenceId]/cleanup/NormalizeSchools.svelte(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and publish container images
🔇 Additional comments (3)
src/routes/(authenticated)/management/[conferenceId]/cleanup/+page.svelte (1)
6-6: LGTM! Clean integration of the NormalizeSchools component.The import and usage are correct, and the component is appropriately placed in the cleanup section.
Also applies to: 129-129
schema.graphql (2)
652-655: LGTM! ConferenceSchools type is well-defined.The type correctly represents aggregated school data with count and name.
323-323: Field declaration is correct and backend resolver satisfies the non-nullable contract.The resolver implementation (lines 218-283 in
src/api/resolvers/modules/conference/conference.ts) always returns an array:
- Initializes
resas an empty array- Populates it from database results
- Returns
resunconditionally, ensuring an empty array is returned when no schools existThe
[ConferenceSchools!]!signature is properly fulfilled.
src/routes/(authenticated)/management/[conferenceId]/cleanup/NormalizeSchools.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/DataTable/DataTable.svelte (1)
62-73: Fix expanded row key lookup
toggleExpandedstill falls back to(row as any).rowKey ?? (row as any).id. When callers supply a differentrowKey(e.g."school"in the new Normalize Schools table) the lookup never finds a value, so expand/collapse stops working. Use the prop—e.g.const key = (row as any)[rowKey] ?? (row as any).id;—and avoid shadowing the outerrowKeyvariable so expanded rows keep functioning with custom keys.
♻️ Duplicate comments (1)
src/api/resolvers/modules/conference/conference.ts (1)
647-651: Respect query + permissions on mutation returnAfter the transaction the resolver calls
db.conference.findUniquewithout the active transaction, strips the incomingquery, and omits the permission filter. That reintroduces the critical issue previously flagged: field selections are ignored and access control is bypassed. Return from the transaction withtx.conference.findUniqueOrThrow({ ...query, where: { id: args.conferenceId, AND: [ctx.permissions.allowDatabaseAccessTo('read').Conference] } })(or reuse an existing helper) so we stay inside the transaction, honour the caller’s selection set, and enforce permissions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
WARP.md(1 hunks)messages/de.json(2 hunks)messages/en.json(1 hunks)schema.graphql(3 hunks)src/api/resolvers/modules/conference/conference.ts(2 hunks)src/lib/components/DataTable/DataTable.svelte(3 hunks)src/routes/(authenticated)/management/[conferenceId]/cleanup/+page.svelte(3 hunks)src/routes/(authenticated)/management/[conferenceId]/cleanup/CheckboxForTable.svelte(1 hunks)src/routes/(authenticated)/management/[conferenceId]/cleanup/NormalizeSchools.svelte(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- WARP.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(authenticated)/management/[conferenceId]/cleanup/+page.svelte
- messages/de.json
🧰 Additional context used
🧬 Code graph analysis (3)
src/api/resolvers/modules/conference/conference.ts (4)
src/api/resolvers/builder.ts (1)
builder(26-66)prisma/db.ts (1)
db(31-33)src/api/resolvers/modules/singleParticipant.ts (1)
t(29-49)src/api/resolvers/modules/delegation.ts (1)
t(89-154)
messages/en.json (1)
scripts/add_new_translation.ts (2)
bulkAdd(69-103)saveToTranslationFile(13-37)
schema.graphql (2)
src/api/resolvers/modules/delegation.ts (2)
t(314-325)t(89-154)src/api/resolvers/modules/assignments.ts (1)
t(37-346)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and publish container images
src/routes/(authenticated)/management/[conferenceId]/cleanup/NormalizeSchools.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Dockerfile(1 hunks)src/api/resolvers/modules/conference/conference.ts(2 hunks)src/routes/(authenticated)/management/[conferenceId]/cleanup/NormalizeSchools.svelte(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Dockerfile
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(authenticated)/management/[conferenceId]/cleanup/NormalizeSchools.svelte
🧰 Additional context used
🧬 Code graph analysis (1)
src/api/resolvers/modules/conference/conference.ts (3)
src/api/resolvers/builder.ts (1)
builder(26-66)prisma/db.ts (1)
db(31-33)src/api/resolvers/modules/conferenceSupervisor.ts (3)
t(302-337)t(142-146)t(23-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and publish container images
🔇 Additional comments (1)
src/api/resolvers/modules/conference/conference.ts (1)
218-322: Permission filters correctly applied.The previous concern about missing permission filters on
delegationMember.groupByhas been addressed (line 258). All three data sources (delegations, delegation members, single participants) now properly apply permission checks before aggregation.
Summary by CodeRabbit
New Features
Listing improvements
Table enhancements
Documentation
Chores