Skip to content

Conversation

@Strehk
Copy link
Member

@Strehk Strehk commented Oct 30, 2025

Summary by CodeRabbit

  • New Features

    • School-aware UI: school counts, filtering, sorted application lists and a conference-level schools report.
    • Normalize Schools tool: select/merge/rename schools, keyboard shortcut, and inline school edit.
  • Listing improvements

    • Search, refined pagination/filtering, average age & participation badges, motivation/experience length indicators.
  • Table enhancements

    • Selectable rows, configurable row key, click-to-select behavior and checkbox support.
  • Documentation

    • Added comprehensive project guide and new translation keys (EN/DE) for Normalize Schools.
  • Chores

    • Updated base Docker image.

@Strehk Strehk added the PR: Enhancement New feature or request label Oct 30, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

📥 Commits

Reviewing files that changed from the base of the PR and between 6c126c0 and 8cd332e.

📒 Files selected for processing (1)
  • .trivyignore (1 hunks)
 ________________
< CSI: Git Diff. >
 ----------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ

Walkthrough

Adds 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

Cohort / File(s) Summary
Translations (EN/DE)
messages/en.json, messages/de.json
Added translation keys for the "Normalize Schools" UI and enterNewSchoolName (German).
GraphQL schema & resolver
schema.graphql, src/api/resolvers/modules/conference/conference.ts
Added ConferenceSchools type and Conference.schools; added normalizeSchoolsInConference mutation; resolver aggregates school counts/members and performs transactional normalization.
App data & models
src/routes/(authenticated)/assignment-assistant/[projectId]/appData.svelte.ts
Added optional school to Delegation/SingleParticipant schemas; changed getApplications ordering; added getSchools() aggregator; switched getMoreInfoLink to use selected query param.
Assignment UI: sighting & school filter
src/routes/(authenticated)/assignment-assistant/[projectId]/sighting/+page.svelte, .../sighting/SchoolFilter.svelte, .../sighting/Application.svelte
Added SchoolFilter component using getSchools(); introduced filter (multi) and search query params with encode/decode/defaults; revised visibility/pagination logic; Application shows motivation/experience length badges.
Assignment UI: delegation details
src/routes/(authenticated)/assignment-assistant/[projectId]/DelegationCard.svelte, src/routes/(authenticated)/assignment-assistant/+page.svelte
Extended user query to include birthday and conferenceParticipationsCount; show average age and average participations tooltips; imported graphql in page.
Management UI: delegation edit & normalization
src/routes/(authenticated)/management/[conferenceId]/delegations/DelegationDrawer.svelte, src/routes/(authenticated)/management/[conferenceId]/cleanup/NormalizeSchools.svelte, src/routes/(authenticated)/management/[conferenceId]/cleanup/+page.svelte
DelegationDrawer: ChangeDelegationSchool mutation and inline edit flow (prompt → mutation → cache.invalidate); added NormalizeSchools component for selecting/merging schools (query, mutation, keyboard shortcut, toasts, table selection); cleanup page renders component.
DataTable & table checkbox
src/lib/components/DataTable/DataTable.svelte, src/routes/(authenticated)/management/[conferenceId]/cleanup/CheckboxForTable.svelte
DataTable: new props selectOnClick, rowKey, selected and forwards them to underlying table with selected-row styling; added checkbox cell component bound to row.$selected.
Management query adjustments
src/routes/(authenticated)/management/[conferenceId]/assignment/+page.ts
Added school field to findManyDelegations and findManySingleParticipants selections.
Docs
WARP.md
New developer guide and project overview (setup, workflows, architecture, commands, conventions).
Build
Dockerfile
Bumped base image from oven/bun:1.2-slim to oven/bun:1.3-slim.

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
Loading
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
Loading
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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay attention to resolver logic and transaction safety in src/api/resolvers/modules/conference/conference.ts.
  • Verify DataTable prop forwarding and selection behavior in src/lib/components/DataTable/DataTable.svelte.
  • Check getSchools() aggregation and ordering changes in appData.svelte.ts.
  • Validate query param encode/decode and pagination/filter/search interplay in sighting +page.svelte.
  • Review NormalizeSchools selection, mutation payload, Shift+Enter binding, and cache refresh.

Possibly related PRs

Suggested reviewers

  • m1212e

Poem

🐰 I hopped through lists and names today,

toggled schools and cleared the fray,
pencil clicked, a merge begun,
rows aligned and work well done,
tiny badges shining in the sun.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Boy Scout Scope ⚠️ Warning The PR fails the Boy Scout Scope check. The PR description (via CodeRabbit's auto-generated summary) does not contain a dedicated "Boy Scout changes" section documenting opportunistic improvements. While the description mentions "Added comprehensive project guide" and "Updated base Docker image" under "Chores/Documentation," these are presented as features rather than explicitly acknowledged as secondary improvements. Key problematic changes include: (1) Dockerfile base image update from 1.2 to 1.3, which is completely unrelated to assignment-assistant refactoring; (2) WARP.md, a new 225-line project documentation file; (3) DelegationCard age/participation averages, which represent new feature additions rather than fixes; and (4) Application.svelte character length badges. These changes mix primary features (Normalize Schools, school filtering) with unrelated improvements and new features, but lack the required clear categorization and justification in a dedicated "Boy Scout changes" section. To resolve this check failure, add a dedicated "Boy Scout Changes" section to the PR description that explicitly lists and justifies all secondary modifications. Alternatively, extract unrelated changes into separate PRs: move the Dockerfile version bump to its own maintenance PR, and consider moving WARP.md documentation to a separate documentation PR unless it is essential context for this PR. For the DataTable enhancements and DelegationCard badges, document clearly in the Boy Scout section why they are included with this PR (e.g., "DataTable props needed to support NormalizeSchools component selection" vs "DelegationCard feature addition that improves assignment context"). This allows reviewers to distinguish between intentional secondary improvements and main-scope changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Refactor assignment-assistant routing, schemas, and components" is partially related to the changeset. The title accurately describes real modifications to the assignment-assistant route, including schema extensions (adding school fields to DelegationSchema and SingleParticipantSchema), component updates (DelegationCard, Application, sighting page), and new utilities (getSchools function). However, the title does not fully capture the main feature addition, which is the new "Normalize Schools" cleanup functionality introduced via new GraphQL types, mutations, translation keys, and dedicated components (NormalizeSchools.svelte, SchoolFilter.svelte). The emphasis on "refactor" also underrepresents the fact that significant new functionality is being added rather than just existing code being reorganized.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot changed the title @coderabbitai Refactor assignment-assistant routing, schemas, and components Oct 30, 2025
Strehk and others added 3 commits October 30, 2025 03:41
…ty changes (#291)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 graphql import from $houdini is 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 replacing prompt() 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 graphql and queryParameters are 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.school in 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 searchActive and filterActive states instead of checking raw $params for 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

📥 Commits

Reviewing files that changed from the base of the PR and between d8d590d and bc5840b.

📒 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 school field 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 school fields to both DelegationSchema and SingleParticipantSchema correctly 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 handle selected= query parameter.

The management pages for individuals, delegations, supervisors, and participants all correctly read the selected query parameter via queryParam('selected') to highlight or open the appropriate drawer item. The refactored links in getMoreInfoLink are consistent with the existing usage pattern throughout the codebase.


185-194: No changes needed—toSorted is 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 including toSorted. The TypeScript target of ESNext correctly preserves modern syntax without transpilation, aligning perfectly with the runtime environment. The sorting logic is sound.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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, the getSchools function converts them to "No School". This mismatch means:

  1. SchoolFilter displays "No School" with a count
  2. User selects "No School" filter → $params.filter = ["No School"]
  3. This line checks: ["No School"].includes(undefined ?? '')["No School"].includes('')false
  4. 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 groupBy queries already filter for school: { not: null }. TypeScript should infer that delegation.school and singleParticipant.school are 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 graphql import from $houdini is 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

📥 Commits

Reviewing files that changed from the base of the PR and between bc5840b and 42bb289.

📒 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 reusing updateOneConferenceMutationObject is 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 42bb289 and 0fc3ade.

📒 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 res as an empty array
  • Populates it from database results
  • Returns res unconditionally, ensuring an empty array is returned when no schools exist

The [ConferenceSchools!]! signature is properly fulfilled.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

toggleExpanded still falls back to (row as any).rowKey ?? (row as any).id. When callers supply a different rowKey (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 outer rowKey variable 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 return

After the transaction the resolver calls db.conference.findUnique without the active transaction, strips the incoming query, 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 with tx.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

📥 Commits

Reviewing files that changed from the base of the PR and between 0fc3ade and d9d3c79.

📒 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between d9d3c79 and 7c97f33.

📒 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.groupBy has been addressed (line 258). All three data sources (delegations, delegation members, single participants) now properly apply permission checks before aggregation.

@Strehk Strehk merged commit 076b44e into main Nov 2, 2025
3 of 4 checks passed
@Strehk Strehk deleted the 269-assignment-assistant-make-compatible-with-latest-changes branch November 2, 2025 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants