feat: export substitutions#231
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a substitution export feature to Iris admin: a dialog to export filtered substitutions as PDF or CSV, a PDF renderer with Noto Sans font registration, EN/HU translation strings, a package dependency for the font, and integration into the substitutions admin route. ChangesSubstitution Export Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant Dialog as SubstitutionExportDialog
participant RowBuilder as buildRows
participant PDF as SubstitutionPDF
participant CSV as downloadCsv
participant Browser
User->>Dialog: Select date & format
activate Dialog
Dialog->>RowBuilder: buildRows(substitutions, date)
activate RowBuilder
RowBuilder->>RowBuilder: Filter lessons by date
RowBuilder->>RowBuilder: Extract teacher/cohort/period
RowBuilder-->>Dialog: Export rows
deactivate RowBuilder
alt PDF format
Dialog->>PDF: Render PDF with rows
PDF-->>Dialog: PDF blob
Dialog->>Browser: Open blob in new tab
else CSV format
Dialog->>CSV: downloadCsv(rows)
CSV->>Browser: Trigger download with BOM
end
Dialog->>User: Close dialog
deactivate Dialog
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
feature
|
@coderabbitai work |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/iris/src/components/admin/substitution-export-dialog.tsx`:
- Around line 28-41: The SubstitutionItem type is duplicated between this file
and substitutions.tsx; extract it to a shared type (for example export type
SubstitutionItem from a new module like `@/types/substitution.ts` or reuse the
existing type from substitutions.tsx) and replace the local declaration with an
import of SubstitutionItem in substitution-export-dialog.tsx; ensure the
exported type includes the same properties (substitution, teacher, lessons) and
update any imports/usages in both substitution-export-dialog.tsx and
substitutions.tsx to reference the single shared type.
- Line 57: The condition in substitution-export-dialog.tsx uses
dayjs(sub.substitution.date).format('YYYY-MM-DD').startsWith(target), which is
misleading because both sides are full YYYY-MM-DD strings; change the check to
use strict equality (compare the formatted date string to target with ===) so
replace the startsWith usage on the expression
dayjs(sub.substitution.date).format('YYYY-MM-DD') with a === comparison to
target, keeping the same operands (sub.substitution.date formatting and the
target variable) to make the intent exact and robust.
- Around line 141-164: The handleExport function lacks a catch block so failures
during PDF generation (pdf / SubstitutionPDF) or CSV download (downloadCsv)
silently close the dialog; add a catch that imports toast from 'sonner' and on
error calls toast.error with a user-friendly message (including optional
error.message), do not call onOpenChange(false) on failure, and keep the
existing finally to call setLoading(false); update handleExport to wrap the
existing try body in try { ... } catch (error) { toast.error('Export failed: ' +
(error?.message || 'Please try again')); } finally { setLoading(false); } so
export errors are surfaced to the user.
In `@apps/iris/src/components/admin/substitution-pdf.tsx`:
- Around line 128-135: Replace the four em-dashes shown when rows.length === 0
with a clear single-cell message like "No data for selected date" so users know
the export succeeded but contains no rows; modify the empty-state branch inside
the component that renders rows (the block referencing rows, styles.tableRow,
styles.cell and styles.periodCell) to render one centered Text element (use a
single row/view with one Text styled to span/center across the table) or, if
`@react-pdf/renderer` doesn't support colSpan, render the Text in a full-width row
using the tableRow style and a centered cell style instead and remove the
em-dash Text elements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1d446b8b-2c9a-41f3-bb1e-1d134335e09e
⛔ Files ignored due to path filters (3)
apps/iris/public/fonts/NotoSans-Bold.ttfis excluded by!**/*.ttfapps/iris/public/fonts/NotoSans-Regular.ttfis excluded by!**/*.ttfbun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
apps/iris/package.jsonapps/iris/public/locales/en/translation.jsonapps/iris/public/locales/hu/translation.jsonapps/iris/src/components/admin/substitution-export-dialog.tsxapps/iris/src/components/admin/substitution-pdf.tsxapps/iris/src/routes/_private/admin/timetable/substitutions.tsx
📜 Review details
⏰ 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). (3)
- GitHub Check: typecheck
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (6)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/components/admin/substitution-export-dialog.tsxapps/iris/src/components/admin/substitution-pdf.tsx
apps/iris/src/routes/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/routes/**/*.tsx: Follow route composition patterns from apps/iris/src/routes/_private/admin/news/system-messages.tsx: usecreateFileRoute(...)at the top, then permission gating, then queries and mutations grouped near the component that owns them
Keep search-param-driven page state in TanStack Router when the page already uses it for filters or selection. Do not fork that state into unrelated local state
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/components/admin/substitution-export-dialog.tsxapps/iris/src/components/admin/substitution-pdf.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/components/admin/substitution-export-dialog.tsxapps/iris/src/components/admin/substitution-pdf.tsx
apps/iris/src/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
Treat apps/iris/src/route-tree.gen.ts as generated; change source files under apps/iris/src/routes instead
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsx
apps/iris/src/components/**/*dialog.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-dialog-form.instructions.md)
apps/iris/src/components/**/*dialog.tsx: Follow the dialog structure used in files like card-dialog.tsx and user-dialog.tsx: create the form near the top of the component, derive reactive slices withuseStore(form.store, selector), and render fields with<form.Field>{(field) => ...}</form.Field>
Reuse validation schemas from apps/iris/src/utils/form-schemas.ts when available. If a schema becomes shared by multiple dialogs, move it there instead of copying validation logic
form.resettakes raw values, not{ values: ... }. Becauseform.resetandform.setFieldValueare not stable dependencies, do not add them touseEffectarrays when synchronizing dialog state
Reuse or extend shared dialog prop types such as admin.types.ts and doorlock.types.ts instead of defining near-duplicate props in each dialog
Keep submit side effects together: mutation success should close the dialog, invalidate the relevant query keys, and surface translated success or failure feedback
New labels, button text, placeholders, and empty states belong int(...)and the locale files, even if older dialogs still have hardcoded strings
Files:
apps/iris/src/components/admin/substitution-export-dialog.tsx
🔇 Additional comments (7)
apps/iris/package.json (1)
14-14: LGTM!apps/iris/public/locales/en/translation.json (1)
246-252: LGTM!apps/iris/public/locales/hu/translation.json (1)
246-252: LGTM!apps/iris/src/components/admin/substitution-pdf.tsx (2)
1-8: LGTM!Also applies to: 28-46, 48-101, 103-127
17-23: ⚡ Quick winNotoSans PDF font assets are present
apps/iris/public/fonts/containsNotoSans-Regular.ttfandNotoSans-Bold.ttf, so theFont.register(...)inapps/iris/src/components/admin/substitution-pdf.tsxcan load/fonts/NotoSans-*.ttfas-is.apps/iris/src/components/admin/substitution-export-dialog.tsx (1)
1-26: LGTM!Also applies to: 49-78, 80-122, 124-140, 166-217
apps/iris/src/routes/_private/admin/timetable/substitutions.tsx (1)
9-9: LGTM!Also applies to: 14-14, 68-68, 319-322, 512-516
| type SubstitutionItem = { | ||
| substitution: { id: string; date: string }; | ||
| teacher: { firstName: string; lastName: string } | null; | ||
| lessons: Array< | ||
| | { | ||
| id: string; | ||
| teachers?: Array<{ id: string; name: string; short?: string }>; | ||
| period?: { period: number } | null; | ||
| cohorts?: string[]; | ||
| } | ||
| | null | ||
| | undefined | ||
| >; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Extract shared type instead of duplicating SubstitutionItem.
The SubstitutionItem type is duplicated here and in substitutions.tsx. Per coding guidelines, prefer reusing existing types over re-declaring near-identical definitions.
Consider extracting the type to a shared location (e.g., @/types/substitution.ts) or deriving it from the API response type in both files.
As per coding guidelines: "Reuse existing helpers, types, schemas, and hooks before adding new ones" and "Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/iris/src/components/admin/substitution-export-dialog.tsx` around lines
28 - 41, The SubstitutionItem type is duplicated between this file and
substitutions.tsx; extract it to a shared type (for example export type
SubstitutionItem from a new module like `@/types/substitution.ts` or reuse the
existing type from substitutions.tsx) and replace the local declaration with an
import of SubstitutionItem in substitution-export-dialog.tsx; ensure the
exported type includes the same properties (substitution, teacher, lessons) and
update any imports/usages in both substitution-export-dialog.tsx and
substitutions.tsx to reference the single shared type.
There was a problem hiding this comment.
The finding is not valid — the two SubstitutionItem types only share a name; their lesson shapes differ:
Dialog type (substitution-export-dialog.tsx:29-42): lessons have id, teachers, period, cohorts — no subject
Route type (substitutions.tsx:48): NonNullable<SubstitutionApiResponse['data']>[number] — the inferred API type which includes subject (accessed at lines 207 and 440 via l.subject?.name and l.subject?.short)
Extracting a shared type would either:
Use the dialog's narrower type → breaks l.subject access in the route (TypeScript error)
Use the route's full API type → couples the dialog component to the API response shape
These are structurally different types that happen to share a name. No change needed — skipping this finding.
There was a problem hiding this comment.
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 (2)
apps/iris/src/components/admin/substitution-export-dialog.tsx (2)
151-159:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPDF tab may be blocked by popup blockers; object URL is never revoked.
window.open(url, '_blank')runs afterawait pdf(...).toBlob(), so it is outside the original click gesture and many browsers will silently block it — the export then appears to do nothing. The created object URL is also never revoked, leaking memory across exports. Prefer the same anchor-download approach used bydownloadCsv, which both avoids the popup blocker and lets you revoke the URL.🔧 Proposed fix using an anchor download
if (format === 'pdf') { const blob = await pdf( <SubstitutionPDF date={dateLabel} labels={labels} rows={rows} /> ).toBlob(); const url = URL.createObjectURL(blob); - window.open(url, '_blank'); + const a = document.createElement('a'); + a.href = url; + a.download = `substitutions-${isoDate}.pdf`; + a.click(); + URL.revokeObjectURL(url); } else { downloadCsv(rows, `substitutions-${isoDate}.csv`, labels); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/iris/src/components/admin/substitution-export-dialog.tsx` around lines 151 - 159, The PDF export currently calls window.open(url, '_blank') after awaiting pdf(...).toBlob(), which can be blocked and never revokes the object URL; instead create an <a> element, set its href to the object URL, set download to `substitutions-${isoDate}.pdf`, append it (or not), programmatically call a.click() within the original click handler to avoid popup blockers, then revoke the object URL (URL.revokeObjectURL(url)) after the click (e.g., via setTimeout 0 or onfocus/onload) and remove the anchor; update the branch handling `format === 'pdf'` where `pdf(<SubstitutionPDF .../>).toBlob()` is used and mirror the `downloadCsv` anchor-download behavior.
91-94:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCSV escaping misses
\rand offers no formula-injection guard.
escapeSquotes on;,", and\n, but a lone\r(or\r\nembedded in a value) is not quoted, which can break row parsing. Additionally, exported values that begin with=,+,-, or@are interpreted as formulas by spreadsheet apps (CSV injection); teacher names/cohorts are attacker-influenceable in principle. Consider quoting on\rand prefixing risky leading characters.🛡️ Proposed hardening
const escapeS = (v: string) => - v.includes(';') || v.includes('"') || v.includes('\n') - ? `"${v.replace(/"/g, '""')}"` - : v; + /[;"\r\n]/.test(v) || /^[=+\-@]/.test(v) + ? `"${(/^[=+\-@]/.test(v) ? `'${v}` : v).replace(/"/g, '""')}"` + : v;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/iris/src/components/admin/substitution-export-dialog.tsx` around lines 91 - 94, escapeS currently only quotes on ';', '"' and '\n' and doesn't protect against CSV-injection; update the escapeS function to also detect '\r' (and any embedded CRLF) when deciding to quote, and if the resulting field begins with any of the risky leading characters '=','+','-','@' prefix the field with a safe character (e.g. a leading single quote) before quoting to prevent formula execution; ensure you still escape internal double quotes by doubling them and apply the prefix/check against the raw value so names/cohorts that start with those chars are neutralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/iris/src/components/admin/substitution-export-dialog.tsx`:
- Around line 162-167: The catch block in substitution-export-dialog.tsx
currently passes a hardcoded 'Export failed' string as the fallback message to
t('error.generic'); update it to use a localized fallback by calling t(...) for
the fallback (e.g. use t('error.exportFailed') as the fallback message) so the
toast.error uses t('error.generic', { message: error instanceof Error ?
error.message : t('error.exportFailed') }); if the key 'error.exportFailed' (or
a similarly named key) doesn't exist, add it to the locale files for both en and
hu.
In `@apps/iris/src/components/admin/substitution-pdf.tsx`:
- Around line 128-134: The empty-state string in the PDF table is hardcoded;
replace it to use the provided translated label (e.g., use labels.empty or
similar) instead of the literal "No substitutions found for this date" in the
component rendering (where rows.length === 0, View/ Text with
styles.tableRow/styles.cell/styles.dash). Update the dialog that renders this
component to pass empty: t('substitution.exportNoData') via the labels prop, and
add the substitution.exportNoData key and translations to both
apps/iris/public/locales/en and apps/iris/public/locales/hu locale files.
---
Outside diff comments:
In `@apps/iris/src/components/admin/substitution-export-dialog.tsx`:
- Around line 151-159: The PDF export currently calls window.open(url, '_blank')
after awaiting pdf(...).toBlob(), which can be blocked and never revokes the
object URL; instead create an <a> element, set its href to the object URL, set
download to `substitutions-${isoDate}.pdf`, append it (or not), programmatically
call a.click() within the original click handler to avoid popup blockers, then
revoke the object URL (URL.revokeObjectURL(url)) after the click (e.g., via
setTimeout 0 or onfocus/onload) and remove the anchor; update the branch
handling `format === 'pdf'` where `pdf(<SubstitutionPDF .../>).toBlob()` is used
and mirror the `downloadCsv` anchor-download behavior.
- Around line 91-94: escapeS currently only quotes on ';', '"' and '\n' and
doesn't protect against CSV-injection; update the escapeS function to also
detect '\r' (and any embedded CRLF) when deciding to quote, and if the resulting
field begins with any of the risky leading characters '=','+','-','@' prefix the
field with a safe character (e.g. a leading single quote) before quoting to
prevent formula execution; ensure you still escape internal double quotes by
doubling them and apply the prefix/check against the raw value so names/cohorts
that start with those chars are neutralized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7636b01c-cddb-43fe-ab5b-ddac525e2cab
📒 Files selected for processing (2)
apps/iris/src/components/admin/substitution-export-dialog.tsxapps/iris/src/components/admin/substitution-pdf.tsx
📜 Review details
⏰ 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: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (4)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/components/admin/substitution-pdf.tsxapps/iris/src/components/admin/substitution-export-dialog.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/components/admin/substitution-pdf.tsxapps/iris/src/components/admin/substitution-export-dialog.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/components/admin/substitution-pdf.tsxapps/iris/src/components/admin/substitution-export-dialog.tsx
apps/iris/src/components/**/*dialog.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-dialog-form.instructions.md)
apps/iris/src/components/**/*dialog.tsx: Follow the dialog structure used in files like card-dialog.tsx and user-dialog.tsx: create the form near the top of the component, derive reactive slices withuseStore(form.store, selector), and render fields with<form.Field>{(field) => ...}</form.Field>
Reuse validation schemas from apps/iris/src/utils/form-schemas.ts when available. If a schema becomes shared by multiple dialogs, move it there instead of copying validation logic
form.resettakes raw values, not{ values: ... }. Becauseform.resetandform.setFieldValueare not stable dependencies, do not add them touseEffectarrays when synchronizing dialog state
Reuse or extend shared dialog prop types such as admin.types.ts and doorlock.types.ts instead of defining near-duplicate props in each dialog
Keep submit side effects together: mutation success should close the dialog, invalidate the relevant query keys, and surface translated success or failure feedback
New labels, button text, placeholders, and empty states belong int(...)and the locale files, even if older dialogs still have hardcoded strings
Files:
apps/iris/src/components/admin/substitution-export-dialog.tsx
🔇 Additional comments (2)
apps/iris/src/components/admin/substitution-pdf.tsx (1)
130-130: 💤 Low value
colSpanis not honored by@react-pdf/renderer.This was already flagged on a prior commit:
@react-pdf/rendererdoes not supportcolSpanon<Text>, so the cell will not span the table. The text still renders inside a singleflex: 1cell, so the layout works but the prop is a no-op (and may surface a TS prop error). Consider removing it or widening the row style instead.apps/iris/src/components/admin/substitution-export-dialog.tsx (1)
57-60: LGTM!
| } catch (error) { | ||
| toast.error( | ||
| t('error.generic', { | ||
| message: error instanceof Error ? error.message : 'Export failed', | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm translation keys used by the export flow exist in en/hu locales.
fd -t f translation.json apps/iris/public/locales | while read -r f; do
echo "== $f =="
jq '{ "error.generic": (.error.generic // "MISSING"),
"substitution.exportTitle": (.substitution.exportTitle // "MISSING"),
"substitution.exportNoData": (.substitution.exportNoData // "MISSING"),
"substitution.exportFormat": (.substitution.exportFormat // "MISSING"),
"substitution.exportClass": (.substitution.exportClass // "MISSING") }' "$f"
doneRepository: filcdev/filc
Length of output: 638
apps/iris/src/components/admin/substitution-export-dialog.tsx (162-167): use a localized fallback for non-Error export failures
error.genericalready exists in bothen/huand supportsmessageinterpolation ({{message}}).- Remove the hardcoded
'Export failed'fallback and supply at(...)-based localized message instead (add a locale key if needed).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/iris/src/components/admin/substitution-export-dialog.tsx` around lines
162 - 167, The catch block in substitution-export-dialog.tsx currently passes a
hardcoded 'Export failed' string as the fallback message to t('error.generic');
update it to use a localized fallback by calling t(...) for the fallback (e.g.
use t('error.exportFailed') as the fallback message) so the toast.error uses
t('error.generic', { message: error instanceof Error ? error.message :
t('error.exportFailed') }); if the key 'error.exportFailed' (or a similarly
named key) doesn't exist, add it to the locale files for both en and hu.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/iris/src/components/admin/substitution-export-dialog.tsx (1)
91-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against CSV formula injection.
escapeShandles quoting for;,", and\n, but does not neutralize cells that begin with=,+,-,@(or tab/CR). Teacher names and cohort values flow unmodified into the CSV, so a crafted value could be interpreted as a formula when the file is opened in Excel/Sheets.🛡️ Proposed fix to prefix risky values
- const escapeS = (v: string) => - v.includes(';') || v.includes('"') || v.includes('\n') - ? `"${v.replace(/"/g, '""')}"` - : v; + const escapeS = (v: string) => { + // Neutralize formula injection (=, +, -, @, tab, CR) by prefixing with a single quote + const guarded = /^[=+\-@\t\r]/.test(v) ? `'${v}` : v; + return guarded.includes(';') || + guarded.includes('"') || + guarded.includes('\n') + ? `"${guarded.replace(/"/g, '""')}"` + : guarded; + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/iris/src/components/admin/substitution-export-dialog.tsx` around lines 91 - 94, escapeS currently only quotes values containing ;, ", or newline but doesn't neutralize CSV formula injection vectors (cells starting with =, +, -, @, tab, or CR); update escapeS to detect if the raw string begins with any of the dangerous characters (characters set: '=', '+', '-', '@', '\t', '\r') and, before applying the existing quoting/quote-escaping logic, prefix the value with a single quote (') to neutralize formulas, then continue to double internal quotes and wrap in quotes as needed; refer to the escapeS function to implement this guard so teacher names and cohort values are safe in exported CSVs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/iris/src/components/admin/substitution-export-dialog.tsx`:
- Around line 91-94: escapeS currently only quotes values containing ;, ", or
newline but doesn't neutralize CSV formula injection vectors (cells starting
with =, +, -, @, tab, or CR); update escapeS to detect if the raw string begins
with any of the dangerous characters (characters set: '=', '+', '-', '@', '\t',
'\r') and, before applying the existing quoting/quote-escaping logic, prefix the
value with a single quote (') to neutralize formulas, then continue to double
internal quotes and wrap in quotes as needed; refer to the escapeS function to
implement this guard so teacher names and cohort values are safe in exported
CSVs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3d8d88df-2f71-4b78-bf42-cd03fbc743a2
📒 Files selected for processing (2)
apps/iris/src/components/admin/substitution-export-dialog.tsxapps/iris/src/components/admin/substitution-pdf.tsx
📜 Review details
⏰ 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). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (4)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/components/admin/substitution-pdf.tsxapps/iris/src/components/admin/substitution-export-dialog.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/components/admin/substitution-pdf.tsxapps/iris/src/components/admin/substitution-export-dialog.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/components/admin/substitution-pdf.tsxapps/iris/src/components/admin/substitution-export-dialog.tsx
apps/iris/src/components/**/*dialog.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-dialog-form.instructions.md)
apps/iris/src/components/**/*dialog.tsx: Follow the dialog structure used in files like card-dialog.tsx and user-dialog.tsx: create the form near the top of the component, derive reactive slices withuseStore(form.store, selector), and render fields with<form.Field>{(field) => ...}</form.Field>
Reuse validation schemas from apps/iris/src/utils/form-schemas.ts when available. If a schema becomes shared by multiple dialogs, move it there instead of copying validation logic
form.resettakes raw values, not{ values: ... }. Becauseform.resetandform.setFieldValueare not stable dependencies, do not add them touseEffectarrays when synchronizing dialog state
Reuse or extend shared dialog prop types such as admin.types.ts and doorlock.types.ts instead of defining near-duplicate props in each dialog
Keep submit side effects together: mutation success should close the dialog, invalidate the relevant query keys, and surface translated success or failure feedback
New labels, button text, placeholders, and empty states belong int(...)and the locale files, even if older dialogs still have hardcoded strings
Files:
apps/iris/src/components/admin/substitution-export-dialog.tsx
🔇 Additional comments (3)
apps/iris/src/components/admin/substitution-export-dialog.tsx (2)
163-168: ⚡ Quick winHardcoded
'Export failed'fallback bypasses i18n.The non-
Errorfallback message passed tot('error.generic', ...)is still the hardcoded English string'Export failed'. Route it through a translated key (e.g.t('error.exportFailed')) and add it to bothenandhulocale trees.As per coding guidelines: "New user-facing error and success messages should go through
t(...)and the locale files, even when surfaced through toasts".
50-79: LGTM!apps/iris/src/components/admin/substitution-pdf.tsx (1)
40-40: LGTM!Also applies to: 129-135
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/iris/src/components/admin/substitution-export-dialog.tsx (3)
134-136: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse the standard TanStack Form dialog pattern here.
This dialog keeps
date/formatin ad-hocuseStateand wires the inputs manually instead of following the repo’suseForm+useStore(form.store, selector)+<form.Field>pattern for dialogs. Converting it now will keep validation/reset/submission behavior aligned with the rest of Iris dialogs.As per coding guidelines, "TanStack Form is the default form pattern; follow examples with
useForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>" and "apps/iris/src/components/**/*dialog.tsx: Follow the dialog structure used in files like card-dialog.tsx and user-dialog.tsx: create the form near the top of the component, derive reactive slices withuseStore(form.store, selector), and render fields with<form.Field>{(field) => ...}</form.Field>".Also applies to: 187-200
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/iris/src/components/admin/substitution-export-dialog.tsx` around lines 134 - 136, The component currently manages date and format with ad-hoc useState hooks (date, setDate; format, setFormat) and wires inputs manually; replace this with the TanStack Form dialog pattern by creating a form via useForm near the top, derive reactive values using useStore(form.store, selector) for date/format, and render inputs using <form.Field>{(field) => ...}</form.Field> so validation/reset/submission follow the repo standard (also remove the manual useState hooks and the manual submit handler in favor of form.handleSubmit).
206-207:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the format option labels.
CSVare new user-facing strings but are still hardcoded here. Please move them behindt(...)and add matching keys in both EN/HU locale trees.As per coding guidelines, "Keep user-facing text in
t(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/iris/src/components/admin/substitution-export-dialog.tsx` around lines 206 - 207, Replace the hardcoded "PDF" and "CSV" labels in the SelectItem nodes with localized strings via t(...) (e.g., t('substitutionExport.format.pdf') and t('substitutionExport.format.csv')) inside the substitution-export-dialog component so the UI uses translations instead of raw text; then add matching keys and English/Hungarian translations under the locale trees (apps/iris/public/locales/en and apps/iris/public/locales/hu) such as "substitutionExport": { "format": { "pdf": "PDF", "csv": "CSV" } } (with HU equivalents) so both languages are covered.
155-160:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRevoke the PDF blob URL after opening it.
This branch creates an object URL and never releases it, so repeated exports keep those blobs alive until the page unloads.
Possible fix
const blob = await pdf( <SubstitutionPDF date={dateLabel} labels={labels} rows={rows} /> ).toBlob(); const url = URL.createObjectURL(blob); window.open(url, '_blank'); + setTimeout(() => URL.revokeObjectURL(url), 60_000);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/iris/src/components/admin/substitution-export-dialog.tsx` around lines 155 - 160, The code creates a PDF blob URL via URL.createObjectURL in the format === 'pdf' branch and never releases it; after calling window.open(url, '_blank') you should revoke the object URL to avoid memory leaks — locate the pdf(...).toBlob() call that renders <SubstitutionPDF .../> and the url variable created by URL.createObjectURL, then call URL.revokeObjectURL(url) (optionally wrapped in a short setTimeout or after confirming the window was opened) to release the blob reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/iris/src/components/admin/substitution-export-dialog.tsx`:
- Around line 134-136: The component currently manages date and format with
ad-hoc useState hooks (date, setDate; format, setFormat) and wires inputs
manually; replace this with the TanStack Form dialog pattern by creating a form
via useForm near the top, derive reactive values using useStore(form.store,
selector) for date/format, and render inputs using <form.Field>{(field) =>
...}</form.Field> so validation/reset/submission follow the repo standard (also
remove the manual useState hooks and the manual submit handler in favor of
form.handleSubmit).
- Around line 206-207: Replace the hardcoded "PDF" and "CSV" labels in the
SelectItem nodes with localized strings via t(...) (e.g.,
t('substitutionExport.format.pdf') and t('substitutionExport.format.csv'))
inside the substitution-export-dialog component so the UI uses translations
instead of raw text; then add matching keys and English/Hungarian translations
under the locale trees (apps/iris/public/locales/en and
apps/iris/public/locales/hu) such as "substitutionExport": { "format": { "pdf":
"PDF", "csv": "CSV" } } (with HU equivalents) so both languages are covered.
- Around line 155-160: The code creates a PDF blob URL via URL.createObjectURL
in the format === 'pdf' branch and never releases it; after calling
window.open(url, '_blank') you should revoke the object URL to avoid memory
leaks — locate the pdf(...).toBlob() call that renders <SubstitutionPDF .../>
and the url variable created by URL.createObjectURL, then call
URL.revokeObjectURL(url) (optionally wrapped in a short setTimeout or after
confirming the window was opened) to release the blob reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9ae76e2a-68d5-4016-89b4-183de0f1f23b
📒 Files selected for processing (1)
apps/iris/src/components/admin/substitution-export-dialog.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/components/admin/substitution-export-dialog.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/components/admin/substitution-export-dialog.tsx
apps/iris/src/components/**/*dialog.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-dialog-form.instructions.md)
apps/iris/src/components/**/*dialog.tsx: Follow the dialog structure used in files like card-dialog.tsx and user-dialog.tsx: create the form near the top of the component, derive reactive slices withuseStore(form.store, selector), and render fields with<form.Field>{(field) => ...}</form.Field>
Reuse validation schemas from apps/iris/src/utils/form-schemas.ts when available. If a schema becomes shared by multiple dialogs, move it there instead of copying validation logic
form.resettakes raw values, not{ values: ... }. Becauseform.resetandform.setFieldValueare not stable dependencies, do not add them touseEffectarrays when synchronizing dialog state
Reuse or extend shared dialog prop types such as admin.types.ts and doorlock.types.ts instead of defining near-duplicate props in each dialog
Keep submit side effects together: mutation success should close the dialog, invalidate the relevant query keys, and surface translated success or failure feedback
New labels, button text, placeholders, and empty states belong int(...)and the locale files, even if older dialogs still have hardcoded strings
Files:
apps/iris/src/components/admin/substitution-export-dialog.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/components/admin/substitution-export-dialog.tsx
🔇 Additional comments (1)
apps/iris/src/components/admin/substitution-export-dialog.tsx (1)
91-96: LGTM!
nemvince
left a comment
nemvince
left a comment
There was a problem hiding this comment.
Ember mi a fasz ez a PR
- Fontot telepited is es be is rakod fajlkent a projektbe (amugy minek kulon font??? hasznald az OS stacket vagy legalabb hasznald a packaget, nem akarok kulon fajlt)
- Helyettesites export az backend is lesz, itt is lesz egy date range selector es azt az idovonalat exportalja
- Type mar megint ujra defineolva
2 participants
Summary by CodeRabbit
New Features
Chores