feat: add associations management#40
Open
BIA3IA wants to merge 49 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 42 minutes and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR modernizes the admin dashboard by introducing a new sidebar navigation system with cookie-persisted category state and breadcrumb routing, removes legacy index pages, adds a complete association management CRUD feature, standardizes page layouts, and provides supporting UI components and storage utilities. ChangesDashboard Sidebar & Navigation Architecture
Association Management Feature
Dashboard Page Styling and Layout Cleanup
Supporting UI Components and Infrastructure
Possibly Related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
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