feat: add associations management#40
Open
BIA3IA wants to merge 50 commits into
Open
Conversation
WalkthroughThis PR adds an association management feature with a new dashboard page, client components for viewing/editing associations and their links, supporting types/constants, and role-gated server actions. It also adds sidebar navigation data, button/dialog UI additions, a getInitials fix, and config/dependency updates. ChangesAssociation Management Feature
Supporting UI Components and Infrastructure
Sequence Diagram(s)sequenceDiagram
participant Page as WebAssociationsIndex
participant View as AssociationsView
participant Card as CardAssociation
participant Dialog as AssociationLinksDialog
participant Action as Server Actions
Page->>View: initialAssociations
View->>Card: render with callbacks
Card->>Card: enter edit mode, edit fields/logo
Card->>Action: onSave (createAssociation/editAssociation)
Action->>Action: requireRole, build FormData
Action->>View: association or error
View->>View: toast, update state, router.refresh()
Card->>Dialog: open links dialog
Dialog->>Card: onSave normalized links
Card->>Action: onSaveLinks (editAssociationLinks)
Action->>View: links result or error
View->>View: toast, update links, router.refresh()
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
src/components/ui/sidebar.tsx (1)
76-89: 💤 Low valueRefactor
setOpencallback to avoid unnecessary re-creation.Including
openin the dependency array causes thesetOpencallback to be recreated every time the sidebar state changes, which reduces the performance benefit ofuseCallback. Consider restructuring to use functional updates consistently:const setOpen = React.useCallback( (value: boolean | ((value: boolean) => boolean)) => { const newOpenState = typeof value === "function" ? (setOpenProp ?? _setOpen) : undefined if (typeof value === "function") { // Use functional form to avoid needing `open` in closure if (setOpenProp) { setOpenProp((prev) => { const next = value(prev) document.cookie = `${SIDEBAR_COOKIE_NAME}=${next}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}` return next }) } else { _setOpen((prev) => { const next = value(prev) document.cookie = `${SIDEBAR_COOKIE_NAME}=${next}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}` return next }) } } else { if (setOpenProp) { setOpenProp(value) } else { _setOpen(value) } document.cookie = `${SIDEBAR_COOKIE_NAME}=${value}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}` } }, [setOpenProp] )This removes
openfrom the dependency array, preventing recreation on every state change.🤖 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 `@src/components/ui/sidebar.tsx` around lines 76 - 89, The setOpen callback closes over open causing it to be recreated on every state change; refactor setOpen (created with React.useCallback) to avoid using open in the closure by using functional updates: when value is a function, call either setOpenProp or _setOpen with a functional updater that computes next from prev, writes the cookie using SIDEBAR_COOKIE_NAME and SIDEBAR_COOKIE_MAX_AGE inside that updater, and returns next; when value is a boolean, call setOpenProp or _setOpen directly and write the cookie with that boolean; finally update the useCallback dependency array to only [setOpenProp].src/app/dashboard/(active)/layout.tsx (2)
41-41: ⚡ Quick winRemove commented-out dead code.
The commented Separator component should be removed to keep the codebase clean.
-// <Separator orientation="vertical" className="mr-2 data-vertical:h-4 data-vertical:self-auto" />🤖 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 `@src/app/dashboard/`(active)/layout.tsx at line 41, Remove the commented-out Separator JSX line (the "// <Separator orientation="vertical" className="mr-2 data-vertical:h-4 data-vertical:self-auto" />" comment) from the layout component so the dead code is gone; also check for and remove any now-unused import or reference to the Separator symbol in the same file (layout.tsx) to avoid unused-import lint warnings, then run the linter/build to confirm no remaining references.
31-34: 💤 Low valueConsider a more semantic approach for header layout.
The empty
<div></div>on line 34 is used to achieve three-column flexbox layout withjustify-betweento center the breadcrumb. While this works, it's a layout hack that could be clearer.💡 Alternative approaches
Option 1: Add a clarifying comment:
<SidebarTrigger className="-ml-1" size="icon" /> <Breadcrumb className="absolute left-1/2 -translate-x-1/2" /> - <div></div> + <div>{/* Spacer for flex layout */}</div>Option 2: Use CSS Grid for more explicit three-column layout:
- <header className="flex h-16 shrink-0 relative items-center justify-between gap-2 border-b mb-8"> + <header className="grid grid-cols-3 h-16 shrink-0 relative items-center gap-2 border-b mb-8"> <SidebarTrigger className="-ml-1" size="icon" /> - <Breadcrumb className="absolute left-1/2 -translate-x-1/2" /> - <div></div> + <Breadcrumb className="justify-self-center" /> + <div>{/* Reserved for future header actions */}</div>🤖 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 `@src/app/dashboard/`(active)/layout.tsx around lines 31 - 34, The header currently uses an empty <div></div> as a layout hack to center Breadcrumb between SidebarTrigger and the right side; update layout.tsx to use a clearer semantic approach: either switch the header's container (the element with className "flex h-16 ...") to a CSS grid with three explicit columns and place SidebarTrigger, Breadcrumb, and a semantic spacer element (e.g., a visually-hidden or aria-hidden div) into the three grid cells, or replace the empty <div> with a purposefully named spacer element (e.g., <div aria-hidden="true" className="header-spacer" />) and add a brief comment explaining its role; ensure Breadcrumb and SidebarTrigger keep their existing props and classes so behavior doesn’t change.src/app/dashboard/(active)/breadcrumb.tsx (1)
72-74: ⚡ Quick winImprove UUID/ID detection heuristic for breadcrumbs.
The current implementation treats any numeric string as an ID (line 73:
!Number.isNaN(Number(segment))), which may produce misleading breadcrumb labels. For example,/dashboard/web/associations/2024would display "Details" instead of "2024".Consider using a more specific pattern to detect actual UUIDs or database IDs:
♻️ More robust UUID/ID detection
function isUUIDorId(segment: string) { - return !Number.isNaN(Number(segment)) || segment.length > 20 // Regex custom a seconda dei tuoi ID + // Match UUID v4 format or numeric IDs that look like database IDs + const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + const numericIdPattern = /^\d{6,}$/ // 6+ digit numeric IDs + return uuidPattern.test(segment) || numericIdPattern.test(segment) || segment.length > 30 }This approach:
- Explicitly matches UUID v4 format
- Only treats longer numeric strings (6+ digits) as IDs, avoiding false positives on year numbers
- Increases the length threshold to 30 to reduce false positives
🤖 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 `@src/app/dashboard/`(active)/breadcrumb.tsx around lines 72 - 74, The isUUIDorId heuristic in breadcrumb.tsx is too permissive (treats any numeric string as an ID); update the isUUIDorId(segment: string) function to match a UUID v4 regex for UUID detection, treat purely-numeric segments as IDs only when they are sufficiently long (e.g., >= 6 digits) to avoid years like "2024", and raise the non-numeric length threshold (e.g., to 30) to reduce false positives; keep the function name isUUIDorId and operate on the incoming segment string so breadcrumbs show real IDs vs human-readable labels.src/components/dashboard-sidebar/main-nav.tsx (1)
64-87: ⚡ Quick winRemove unnecessary
undefinedcheck foropenstate.The condition
open !== undefinedon line 64 is always true because:
openis typed asbooleanin theuseState<boolean>declaration (line 57)- The initial value always resolves to a boolean via the
?? falsefallbacksetOpenis only ever called with boolean valuesThe
<Skeleton />branch is unreachable dead code.♻️ Simplify the conditional rendering
- return open !== undefined ? ( + return ( <Collapsible render={<SidebarMenuItem />} open={open} onOpenChange={handleOpenChange} className="group/collapsible"> <CollapsibleTrigger render={ <SidebarMenuButton className="font-medium"> {category.icon} {category.title} <ChevronRight className="ml-auto transition-transform ease-linear rotate-0 group-data-open/collapsible:rotate-90" /> </SidebarMenuButton> } /> <CollapsibleContent> {category.items?.length ? ( <SidebarMenuSub className="px-1.5"> {category.items.map((item) => ( <DSMenuItem key={item.url === "#" ? item.title : item.url} item={item} /> ))} </SidebarMenuSub> ) : null} </CollapsibleContent> </Collapsible> - ) : ( - <Skeleton /> )🤖 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 `@src/components/dashboard-sidebar/main-nav.tsx` around lines 64 - 87, The ternary checking "open !== undefined" is unnecessary because "open" is a boolean from useState<boolean> (with ?? false fallback) so the Skeleton branch is dead; replace the entire conditional return with a direct return of the <Collapsible ...> block (keeping Collapsible, CollapsibleTrigger, CollapsibleContent, SidebarMenuItem/DSMenuItem, SidebarMenuSub, handleOpenChange and the open prop intact) and remove the unreachable <Skeleton /> branch and the ternary expression so the component always renders the Collapsible.src/app/dashboard/(active)/web/associations/constants.ts (1)
30-41: 💤 Low valueConsider distinct icons for TikTok and Spotify.
Both TikTok (line 35) and Spotify (line 40) use the
Musicicon, which reduces visual distinction in the UI. Consider using platform-specific icons or different generic icons to improve differentiation.🤖 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 `@src/app/dashboard/`(active)/web/associations/constants.ts around lines 30 - 41, LINK_FIELDS currently assigns the same Music icon to ASSOCIATION_LINK.TIKTOK and ASSOCIATION_LINK.SPOTIFY which hurts visual distinction; update the entry for ASSOCIATION_LINK.TIKTOK to use a TikTok-specific icon (e.g., Tiktok) and the entry for ASSOCIATION_LINK.SPOTIFY to use a Spotify-specific icon (e.g., Spotify) or two distinct generic icons, ensuring the imports at the top include the chosen icon components and that the LINK_FIELDS array entries for ASSOCIATION_LINK.TIKTOK and ASSOCIATION_LINK.SPOTIFY are updated to reference those new icon symbols.
🤖 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 `@src/app/dashboard/`(active)/layout.tsx:
- Around line 7-14: parseCookie currently returns the raw JSON which is
implicitly any; update parseCookie to return a safe Record<string, boolean> for
DashboardSidebar by treating JSON.parse result as unknown, performing runtime
validation and coercion: ensure the parsed value is an object, iterate its keys
and keep only entries whose values are booleans (or coerce string/"true"/"false"
and numeric 0/1 to booleans), and return that sanitized Record<string, boolean>;
keep the same function name parseCookie so callers continue to work and return
{} on invalid input.
In `@src/app/dashboard/`(active)/web/associations/associations-view.tsx:
- Around line 45-68: The handler in associations-view.tsx expects
deleteAssociation to return a standardized result object with an error string
(e.g., { error: "UNAUTHORIZED" | "NOT_FOUND" } or no error), so change the
server-side action deleteAssociation (in src/server/actions/web.ts) to map the
raw TRPC response into a consistent shape before returning: catch TRPC errors
and return { error: "UNAUTHORIZED" } or { error: "NOT_FOUND" } as appropriate,
and return a success object (e.g., {}) on success; ensure the exported function
name deleteAssociation is the one that performs this mapping so the front-end
checks (result.error) work reliably.
- Around line 19-33: Replace the fragile Date.now() temporary id in handleAdd
with a deterministic client-side draft id strategy: introduce a component-level
negative counter (e.g., nextTempIdRef or nextTempId variable) or generate a UUID
and use that value for association.id, then continue to call setAssociations,
setEditingAssociationId and setDraftAssociationIds with that new id; update any
code relying on numeric ids (if using negative counter) or ensure Set membership
works for string ids (if using UUID) so draft ids are clearly distinguished from
server-assigned ids.
In `@src/app/dashboard/`(active)/web/associations/card-association.tsx:
- Around line 37-41: The handleIconUpload function currently assigns raw
uploaded SVG text via setLogoSvg and is later rendered with
dangerouslySetInnerHTML (risking XSS); change it to validate and sanitize
uploads: verify the file's type/extension and size, read the file as text only
if it's an SVG, then pass the text through a trusted sanitizer (e.g.,
isomorphic-dompurify / DOMPurify) configured to strip scripts, event handlers,
external references and disallowed tags/attributes before calling setLogoSvg;
ensure any failed validation rejects the upload and does not update the state.
In `@src/hooks/use-cookie-storage.tsx`:
- Around line 28-46: The setValue callback in use-cookie-storage captures
storedValue in its closure, so function updaters call value(storedValue) and can
get stale state; change setValue to use the functional updater pattern: call
setStoredValue(prev => { const next = typeof value === 'function' ? (value as
any)(prev) : value; /* persist next via setCookie/deleteCookie using key and
mergedOptions */ return next; }); remove storedValue from the useCallback
dependency array and keep key and mergedOptions only; ensure
deleteCookie/setCookie use the computed next value when persisting.
In `@src/server/actions/web.ts`:
- Around line 40-45: The deleteAssociation action returns the raw TRPC result
but should match the other actions' { association, error } shape; update
deleteAssociation to call trpc.web.associations.deleteAssociation.mutate({ id })
and then return an object like { association: result.association, error:
result.error } (or map the mutation's response fields to association and error),
keeping the existing authorization check via requireRole; reference the function
deleteAssociation and the call trpc.web.associations.deleteAssociation.mutate to
locate and adjust the return shape to match createAssociation, editAssociation,
and editAssociationLinks.
---
Nitpick comments:
In `@src/app/dashboard/`(active)/breadcrumb.tsx:
- Around line 72-74: The isUUIDorId heuristic in breadcrumb.tsx is too
permissive (treats any numeric string as an ID); update the isUUIDorId(segment:
string) function to match a UUID v4 regex for UUID detection, treat
purely-numeric segments as IDs only when they are sufficiently long (e.g., >= 6
digits) to avoid years like "2024", and raise the non-numeric length threshold
(e.g., to 30) to reduce false positives; keep the function name isUUIDorId and
operate on the incoming segment string so breadcrumbs show real IDs vs
human-readable labels.
In `@src/app/dashboard/`(active)/layout.tsx:
- Line 41: Remove the commented-out Separator JSX line (the "// <Separator
orientation="vertical" className="mr-2 data-vertical:h-4
data-vertical:self-auto" />" comment) from the layout component so the dead code
is gone; also check for and remove any now-unused import or reference to the
Separator symbol in the same file (layout.tsx) to avoid unused-import lint
warnings, then run the linter/build to confirm no remaining references.
- Around line 31-34: The header currently uses an empty <div></div> as a layout
hack to center Breadcrumb between SidebarTrigger and the right side; update
layout.tsx to use a clearer semantic approach: either switch the header's
container (the element with className "flex h-16 ...") to a CSS grid with three
explicit columns and place SidebarTrigger, Breadcrumb, and a semantic spacer
element (e.g., a visually-hidden or aria-hidden div) into the three grid cells,
or replace the empty <div> with a purposefully named spacer element (e.g., <div
aria-hidden="true" className="header-spacer" />) and add a brief comment
explaining its role; ensure Breadcrumb and SidebarTrigger keep their existing
props and classes so behavior doesn’t change.
In `@src/app/dashboard/`(active)/web/associations/constants.ts:
- Around line 30-41: LINK_FIELDS currently assigns the same Music icon to
ASSOCIATION_LINK.TIKTOK and ASSOCIATION_LINK.SPOTIFY which hurts visual
distinction; update the entry for ASSOCIATION_LINK.TIKTOK to use a
TikTok-specific icon (e.g., Tiktok) and the entry for ASSOCIATION_LINK.SPOTIFY
to use a Spotify-specific icon (e.g., Spotify) or two distinct generic icons,
ensuring the imports at the top include the chosen icon components and that the
LINK_FIELDS array entries for ASSOCIATION_LINK.TIKTOK and
ASSOCIATION_LINK.SPOTIFY are updated to reference those new icon symbols.
In `@src/components/dashboard-sidebar/main-nav.tsx`:
- Around line 64-87: The ternary checking "open !== undefined" is unnecessary
because "open" is a boolean from useState<boolean> (with ?? false fallback) so
the Skeleton branch is dead; replace the entire conditional return with a direct
return of the <Collapsible ...> block (keeping Collapsible, CollapsibleTrigger,
CollapsibleContent, SidebarMenuItem/DSMenuItem, SidebarMenuSub, handleOpenChange
and the open prop intact) and remove the unreachable <Skeleton /> branch and the
ternary expression so the component always renders the Collapsible.
In `@src/components/ui/sidebar.tsx`:
- Around line 76-89: The setOpen callback closes over open causing it to be
recreated on every state change; refactor setOpen (created with
React.useCallback) to avoid using open in the closure by using functional
updates: when value is a function, call either setOpenProp or _setOpen with a
functional updater that computes next from prev, writes the cookie using
SIDEBAR_COOKIE_NAME and SIDEBAR_COOKIE_MAX_AGE inside that updater, and returns
next; when value is a boolean, call setOpenProp or _setOpen directly and write
the cookie with that boolean; finally update the useCallback dependency array to
only [setOpenProp].
🪄 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: CHILL
Plan: Pro
Run ID: 67a0ff49-3909-424b-9df4-1ae5178ad04d
📒 Files selected for processing (48)
src/app/dashboard/(active)/account/loading.tsxsrc/app/dashboard/(active)/account/page.tsxsrc/app/dashboard/(active)/azure/members/loading.tsxsrc/app/dashboard/(active)/azure/members/page.tsxsrc/app/dashboard/(active)/azure/page.tsxsrc/app/dashboard/(active)/breadcrumb.tsxsrc/app/dashboard/(active)/complete-profile.tsxsrc/app/dashboard/(active)/layout.tsxsrc/app/dashboard/(active)/page.tsxsrc/app/dashboard/(active)/telegram/grants/loading.tsxsrc/app/dashboard/(active)/telegram/grants/page.tsxsrc/app/dashboard/(active)/telegram/groups/group-row.tsxsrc/app/dashboard/(active)/telegram/groups/loading.tsxsrc/app/dashboard/(active)/telegram/groups/page.tsxsrc/app/dashboard/(active)/telegram/page.tsxsrc/app/dashboard/(active)/telegram/user-details/page.tsxsrc/app/dashboard/(active)/telegram/user-details/remove-role.tsxsrc/app/dashboard/(active)/telegram/user-list/loading.tsxsrc/app/dashboard/(active)/telegram/user-list/page.tsxsrc/app/dashboard/(active)/web/associations/association-links.tsxsrc/app/dashboard/(active)/web/associations/associations-view.tsxsrc/app/dashboard/(active)/web/associations/card-association.tsxsrc/app/dashboard/(active)/web/associations/constants.tssrc/app/dashboard/(active)/web/associations/page.tsxsrc/app/dashboard/(active)/web/associations/types.tssrc/app/dashboard/layout.tsxsrc/app/layout.tsxsrc/app/page.tsxsrc/components/admin-header/index.tsxsrc/components/admin-header/right-nav.tsxsrc/components/dashboard-sidebar/data.tsxsrc/components/dashboard-sidebar/index.tsxsrc/components/dashboard-sidebar/main-nav.tsxsrc/components/dashboard-sidebar/user-nav.tsxsrc/components/delete-dialog.tsxsrc/components/ui/alert.tsxsrc/components/ui/button.tsxsrc/components/ui/sidebar.tsxsrc/components/web-header.tsxsrc/constants.tssrc/hooks/use-cookie-storage.tsxsrc/hooks/use-session-storage.tsxsrc/index.csssrc/lib/i18n.tssrc/lib/utils/index.tssrc/server/actions/web.tssrc/utils/cookies.tstsconfig.json
💤 Files with no reviewable changes (5)
- src/lib/i18n.ts
- src/app/dashboard/(active)/telegram/page.tsx
- src/components/admin-header/index.tsx
- src/components/admin-header/right-nav.tsx
- src/app/dashboard/(active)/azure/page.tsx
BIA3IA
added
the
status: blocked
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/server/actions/web.ts (1)
62-67: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
deleteAssociationreturn type is inconsistent with other actions.This was flagged in a previous review and remains unresolved.
deleteAssociationreturns the raw tRPC result, whilecreateAssociation,editAssociation, andeditAssociationLinksall return{ association, error }shapes. Downstream callers expecting a uniform interface will break.♻️ Standardize the return type
export async function deleteAssociation(id: number) { const { allowed } = await requireRole(["owner", "direttivo", "president"]) if (!allowed) return { association: null, error: "UNAUTHORIZED" } - return await trpc.web.associations.deleteAssociation.mutate({ id }) + const result = await trpc.web.associations.deleteAssociation.mutate({ id }) + if ("error" in result) return { association: null, error: result.error } + + return { association: result, error: null } }🤖 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 `@src/server/actions/web.ts` around lines 62 - 67, The deleteAssociation action currently returns the raw tRPC mutation result, unlike createAssociation, editAssociation, and editAssociationLinks which return a consistent { association, error } shape. Update deleteAssociation to follow the same interface by using the trpc.web.associations.deleteAssociation.mutate call but wrapping its result into the same standardized object format expected by downstream callers, and keep the unauthorized branch aligned with that return shape as well.
🧹 Nitpick comments (1)
src/app/dashboard/(active)/web/associations/card-association.tsx (1)
75-83: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
logoandlinksin save values are ignored by the consumer.
saveChangespasseslogo: nullandlinkstoonSave, but the downstream consumer (associations-view.tsxhandleSave) only destructuresname,descriptionIt,descriptionEn, andlogoFile. Links are saved separately viahandleSaveLinks. These dead fields are misleading — consider omitting them or documenting that they're intentionally ignored.♻️ Proposed refactor
const saved = await item.onSave({ id: item.id, name, logoFile, descriptionIt, descriptionEn, - links, - logo: null, })🤖 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 `@src/app/dashboard/`(active)/web/associations/card-association.tsx around lines 75 - 83, The save payload in `saveChanges` includes `logo` and `links`, but `associations-view.tsx`’s `handleSave` ignores them and `links` are handled separately by `handleSaveLinks`. Update `card-association.tsx` so `item.onSave` only receives the fields actually consumed downstream (`id`, `name`, `logoFile`, `descriptionIt`, `descriptionEn`), or otherwise document the intentional no-op fields if they must remain.
🤖 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 `@next.config.ts`:
- Around line 22-26: Add client-side file size validation to match the 2MB
server action limit: update handleIconUpload in card-association.tsx to check
the selected file’s size before upload and reject anything over 2MB with a clear
user-facing message. Keep the next.config.ts serverActions bodySizeLimit as-is,
and ensure the validation is applied at the upload entry point so oversized
files are blocked before reaching the server action.
In `@src/app/dashboard/`(active)/web/associations/card-association.tsx:
- Around line 39-44: The handleIconUpload flow in card-association.tsx currently
accepts any selected file and only discovers oversized uploads later in the
server action. Add a client-side size check in handleIconUpload before calling
setLogoFile and setLogoPreview, using the same 2mb limit configured in
next.config.ts, and prevent preview/state updates when the file exceeds that
limit. Keep the existing file selection logic in handleIconUpload and only
proceed with URL.createObjectURL after the size validation passes.
In `@src/server/actions/web.ts`:
- Around line 29-32: The createAssociation flow is treating the raw result from
trpc.web.associations.addAssociation.mutate as a successful association without
checking for an error response. Update createAssociation in web.ts to mirror
editAssociation by inspecting the mutation result for an error field and
returning it via the error property when present, otherwise return the
association data; keep handleSave and the createAssociation return shape
consistent so invalid results are not written into state.
---
Duplicate comments:
In `@src/server/actions/web.ts`:
- Around line 62-67: The deleteAssociation action currently returns the raw tRPC
mutation result, unlike createAssociation, editAssociation, and
editAssociationLinks which return a consistent { association, error } shape.
Update deleteAssociation to follow the same interface by using the
trpc.web.associations.deleteAssociation.mutate call but wrapping its result into
the same standardized object format expected by downstream callers, and keep the
unauthorized branch aligned with that return shape as well.
---
Nitpick comments:
In `@src/app/dashboard/`(active)/web/associations/card-association.tsx:
- Around line 75-83: The save payload in `saveChanges` includes `logo` and
`links`, but `associations-view.tsx`’s `handleSave` ignores them and `links` are
handled separately by `handleSaveLinks`. Update `card-association.tsx` so
`item.onSave` only receives the fields actually consumed downstream (`id`,
`name`, `logoFile`, `descriptionIt`, `descriptionEn`), or otherwise document the
intentional no-op fields if they must remain.
🪄 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: CHILL
Plan: Pro
Run ID: 56ed62d1-28d7-4c3d-a7df-7c5a5fddabb3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
next.config.tspackage.jsonsrc/app/dashboard/(active)/web/associations/associations-view.tsxsrc/app/dashboard/(active)/web/associations/card-association.tsxsrc/app/dashboard/(active)/web/associations/types.tssrc/components/dashboard-sidebar/data.tsxsrc/server/actions/web.ts
✅ Files skipped from review due to trivial changes (2)
- package.json
- src/app/dashboard/(active)/web/associations/types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/dashboard-sidebar/data.tsx
- src/app/dashboard/(active)/web/associations/associations-view.tsx
Comment on lines
22
to
+26
| experimental: { | ||
| reactCompiler: true, | ||
| serverActions: { | ||
| bodySizeLimit: "2mb", | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
bodySizeLimit should be paired with client-side file size validation.
The 2MB limit is set server-side, but handleIconUpload in card-association.tsx (lines 39-44) performs no size check. Files exceeding 2MB will fail at the server action level with an unclear error. See the cross-file comment on card-association.tsx for the client-side fix.
🤖 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 `@next.config.ts` around lines 22 - 26, Add client-side file size validation to
match the 2MB server action limit: update handleIconUpload in
card-association.tsx to check the selected file’s size before upload and reject
anything over 2MB with a clear user-facing message. Keep the next.config.ts
serverActions bodySizeLimit as-is, and ensure the validation is applied at the
upload entry point so oversized files are blocked before reaching the server
action.
Comment on lines
+39
to
+44
| async function handleIconUpload(event: ChangeEvent<HTMLInputElement>) { | ||
| const file = event.target.files?.[0] | ||
| if (!file) return | ||
| setLogoFile(file) | ||
| setLogoPreview(URL.createObjectURL(file)) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add client-side file size validation before upload.
The bodySizeLimit in next.config.ts is set to "2mb". Without a client-side check, uploading a larger file will fail at the server action with an opaque error. The previous XSS concern (SVG + dangerouslySetInnerHTML) is resolved since SVGs are now rendered via next/image (<img>), which does not execute embedded JavaScript.
🛡️ Proposed fix
async function handleIconUpload(event: ChangeEvent<HTMLInputElement>) {
const file = event.target.files?.[0]
if (!file) return
+ if (file.size > 2 * 1024 * 1024) {
+ toast.error("File size must be under 2MB.")
+ return
+ }
setLogoFile(file)
setLogoPreview(URL.createObjectURL(file))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function handleIconUpload(event: ChangeEvent<HTMLInputElement>) { | |
| const file = event.target.files?.[0] | |
| if (!file) return | |
| setLogoFile(file) | |
| setLogoPreview(URL.createObjectURL(file)) | |
| } | |
| async function handleIconUpload(event: ChangeEvent<HTMLInputElement>) { | |
| const file = event.target.files?.[0] | |
| if (!file) return | |
| if (file.size > 2 * 1024 * 1024) { | |
| toast.error("File size must be under 2MB.") | |
| return | |
| } | |
| setLogoFile(file) | |
| setLogoPreview(URL.createObjectURL(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 `@src/app/dashboard/`(active)/web/associations/card-association.tsx around
lines 39 - 44, The handleIconUpload flow in card-association.tsx currently
accepts any selected file and only discovers oversized uploads later in the
server action. Add a client-side size check in handleIconUpload before calling
setLogoFile and setLogoPreview, using the same 2mb limit configured in
next.config.ts, and prevent preview/state updates when the file exceeds that
limit. Keep the existing file selection logic in handleIconUpload and only
proceed with URL.createObjectURL after the size validation passes.
Comment on lines
+29
to
+32
| return { | ||
| association: await trpc.web.associations.addAssociation.mutate(formdata), | ||
| error: null, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
createAssociation doesn't check for errors from the tRPC mutation.
Unlike editAssociation (line 44 checks if ("error" in result)), createAssociation wraps the raw mutation result directly as association with error: null. If addAssociation.mutate returns an error object, it will be silently treated as a successful association, and downstream code in handleSave will set it as the association state — corrupting the UI.
🐛 Proposed fix: add error checking consistent with `editAssociation`
const formdata = getAssociationFormData(input)
formdata.set("createdBy", String(telegramId))
- return {
- association: await trpc.web.associations.addAssociation.mutate(formdata),
- error: null,
- }
+ const result = await trpc.web.associations.addAssociation.mutate(formdata)
+ if ("error" in result) return { association: null, error: result.error }
+
+ return { association: result, error: null }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return { | |
| association: await trpc.web.associations.addAssociation.mutate(formdata), | |
| error: null, | |
| } | |
| const result = await trpc.web.associations.addAssociation.mutate(formdata) | |
| if ("error" in result) return { association: null, error: result.error } | |
| return { association: result, error: null } |
🧰 Tools
🪛 GitHub Check: Typecheck and Lint
[failure] 30-30:
Argument of type 'FormData' is not assignable to parameter of type '{ name: string; descriptionIt: string; descriptionEn: string; logoSvg: string | null; createdBy: number; }'.
🤖 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 `@src/server/actions/web.ts` around lines 29 - 32, The createAssociation flow
is treating the raw result from trpc.web.associations.addAssociation.mutate as a
successful association without checking for an error response. Update
createAssociation in web.ts to mirror editAssociation by inspecting the mutation
result for an error field and returning it via the error property when present,
otherwise return the association data; keep handleSave and the createAssociation
return shape consistent so invalid results are not written into state.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Blocked by #39 and by PoliNetworkOrg/backend#36