feat(evidence): Add department and assignee filtering to evidence tasks#81
Conversation
- Enhance evidence tasks list with department and assignee filtering - Update Prisma schema to support assignee and department for organization evidence - Implement new filter options in EvidenceList and data table columns - Add assignee and department columns to evidence tasks view - Improve file preview and section components with better loading and preview handling - Update hooks and server actions to support new filtering parameters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces several new actions, components, hooks, and database migrations to enhance organization evidence management. New server-side functions retrieve organization administrators and update evidence details such as department and frequency. UI components are expanded with filters for department and assignee, along with dedicated sections for assignment and review management. Additionally, hooks are introduced to fetch organization admin data, and the database schema is updated with new columns, indices, and foreign key constraints to support evidence assignments and departmental categorization. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant A as AssigneeSection
participant S as assignEvidence Action
participant DB as Database
U->>A: Select new assignee
A->>S: Invoke assignEvidence(evidenceId, assigneeId)
S->>DB: Verify user organization and fetch evidence
DB-->>S: Return evidence record
S->>DB: Validate assignee’s admin privileges
DB-->>S: Return admin details
S->>DB: Update evidence with new assigneeId
DB-->>S: Return updated evidence
S-->>A: Send success response
A-->>U: Update UI with new assignment
sequenceDiagram
participant U as User
participant G as getOrganizationAdmins
participant DB as Database
U->>G: Request organization admins
G->>DB: Query for admin/owner members
DB-->>G: Return user details
G-->>U: Provide formatted admin list
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/FileSection.tsx (1)
79-94: Automatic preview loading is a nice UX improvementThis useEffect hook automatically loads file previews when the component mounts or when fileUrls change, which improves the user experience by making content immediately available.
One potential enhancement:
- const loadPreviews = async () => { - for (const url of filesToLoad) { - await handlePreviewClick(url); - } - }; + const loadPreviews = async () => { + // Load first 3 previews immediately, then load others in background + const initialBatch = filesToLoad.slice(0, 3); + const remainingFiles = filesToLoad.slice(3); + + // Load initial batch sequentially + for (const url of initialBatch) { + await handlePreviewClick(url); + } + + // Load remaining files in background + if (remainingFiles.length > 0) { + setTimeout(() => { + remainingFiles.forEach(url => handlePreviewClick(url)); + }, 100); + } + };This would provide a more responsive experience when there are many files by showing some previews immediately while loading others in the background.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Actions/updateEvidenceFrequency.ts (1)
49-62: Consider adding an optimistic check before updatingThe function currently updates the frequency even if it's the same as the existing value. Consider adding a check to avoid unnecessary database operations.
+ // Skip update if frequency hasn't changed + if (evidence.frequency === frequency) { + return { + success: true, + data: evidence, + }; + } // Update the evidence frequency const updatedEvidence = await db.organizationEvidence.update({apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/AssigneeSection.tsx (1)
58-70: Consider refactoring to reduce duplicated logic.The handleAssigneeChange function duplicates some of the same logic that exists in the useEffect hook on lines 49-56. Consider extracting the common functionality into a separate function.
+ const updateSelectedAdmin = (newAssigneeId: string | null) => { + if (newAssigneeId && admins) { + const admin = admins.find((a) => a.id === newAssigneeId); + setSelectedAdmin(admin || null); + } else { + setSelectedAdmin(null); + } + }; useEffect(() => { if (admins && assigneeId) { - const admin = admins.find((a) => a.id === assigneeId); - setSelectedAdmin(admin || null); + updateSelectedAdmin(assigneeId); } else { setSelectedAdmin(null); } }, [admins, assigneeId]); const handleAssigneeChange = (value: string) => { const newAssigneeId = value === "none" ? null : value; setAssigneeId(newAssigneeId); - if (newAssigneeId && admins) { - const admin = admins.find((a) => a.id === newAssigneeId); - setSelectedAdmin(admin || null); - } else { - setSelectedAdmin(null); - } + updateSelectedAdmin(newAssigneeId); updateAssignee({ id: evidenceId, assigneeId: newAssigneeId }); };apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/ReviewSection.tsx (2)
11-11: Remove unused Pencil icon import.The Pencil icon from lucide-react is imported but never used in the component.
-import { CalendarClock, Pencil, RefreshCw, Building } from "lucide-react"; +import { CalendarClock, RefreshCw, Building } from "lucide-react";
35-35: Consider memoizing the calculateNextReview result.If the calculateNextReview function becomes computationally expensive in the future, consider memoizing the result with useMemo to prevent unnecessary recalculations.
- const reviewInfo = calculateNextReview(lastPublishedAt, frequency); + const reviewInfo = useMemo( + () => calculateNextReview(lastPublishedAt, frequency), + [lastPublishedAt, frequency] + ); // Don't forget to add the import // import { useMemo } from "react";apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceList.tsx (2)
41-53: Consider fetching enum values instead of hardcoding.While hardcoding enum values works, consider fetching these values from an API or a shared constants file to ensure the UI stays in sync with backend changes.
93-93: Remove console.log statement before production.There's a console.log statement that should be removed before deploying to production.
-console.log({ evidenceTasks, error });apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/DepartmentSection.tsx (1)
31-43: Enhance error handling for better debugging.While the error handling works, consider providing more specific error messages based on the error type.
onError: (error) => { console.error("Error updating department:", error); - toast.error("Failed to update department"); + toast.error(`Failed to update department: ${error.message || "Unknown error"}`); },apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/hooks/useOrganizationAdmins.ts (2)
6-12: Consider using a more specific type for the role field.The role field is currently typed as string, but if there are specific roles like "admin" or "owner", consider using a union type or enum for better type safety.
- role: string; + role: "admin" | "owner" | "member";
33-38: Consider adding cache time configuration.Since the list of organization admins likely doesn't change frequently, you might want to add a dedupingInterval or other cache configuration to optimize performance.
export function useOrganizationAdmins() { return useSWR<Admin[]>("organization-admins", fetchOrganizationAdmins, { revalidateOnFocus: false, revalidateOnReconnect: false, + dedupingInterval: 60000, // Cache for 1 minute }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Actions/getOrganizationAdmins.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Actions/getOrganizationEmployees.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Actions/getOrganizationEvidence.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Actions/getOrganizationEvidenceTasks.ts(5 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceList.tsx(11 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/columns.tsx(3 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/types.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Actions/assignEvidence.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Actions/updateEvidenceDepartment.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Actions/updateEvidenceFrequency.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/AssigneeSection.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/DepartmentSection.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/EvidenceDetails.tsx(4 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/FileCard.tsx(4 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/FileSection.tsx(3 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/FrequencySection.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/ReviewSection.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/hooks/useOrganizationAdmins.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/hooks/useEvidenceTasks.ts(2 hunks)packages/db/prisma/migrations/20250225193214_add_evidence_assignee/migration.sql(1 hunks)packages/db/prisma/migrations/20250225193719_update_evidence_assignee_to_user/migration.sql(1 hunks)packages/db/prisma/migrations/20250225204820_add_department_to_evidence/migration.sql(1 hunks)packages/db/prisma/schema.prisma(3 hunks)
🔇 Additional comments (75)
packages/db/prisma/migrations/20250225193719_update_evidence_assignee_to_user/migration.sql (1)
1-5: Good database constraint handling for assignee relationships.The migration appropriately updates the foreign key constraint for the assigneeId column. Setting
ON DELETE SET NULLis a good choice as it ensures evidence tasks remain in the system even if an assigned user is deleted. TheON UPDATE CASCADEmaintains referential integrity if user IDs change.apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Actions/getOrganizationAdmins.ts (1)
1-71: Well-structured server action for retrieving organization adminsThe implementation follows good practices with proper error handling, appropriate database querying, and clean data transformation. The action correctly verifies the user's organization context before executing the query.
A few suggestions for future enhancements:
- Consider adding pagination support if organizations might have many admins
- You might want to add caching for this data since admin lists don't change frequently
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/FileSection.tsx (1)
49-76: Good optimization to prevent redundant preview loadingThe added check prevents duplicate preview loading operations when a preview is already loading or has been loaded.
packages/db/prisma/migrations/20250225193214_add_evidence_assignee/migration.sql (1)
1-9:Details
✅ Verification successful
Foreign key references Employee table rather than User
The migration adds the assigneeId column with a foreign key constraint to the Employee table. However, based on the PR objective and the
getOrganizationAdmins.tsfile which returns User records, this should likely reference the User table instead.
🏁 Script executed:
#!/bin/bash # Check if there are any subsequent migrations that might correct this fd --type f ".*migration.sql$" packages/db/prisma/migrations | grep -A 5 "20250225193214" | sort # Check the Prisma schema to see the current relationship grep -A 5 "assigneeId" packages/db/prisma/schema.prismaLength of output: 634
Action Required: Confirm Migration Order and Final Schema Consistency
The initial migration (
20250225193214_add_evidence_assignee/migration.sql) adds theassigneeIdcolumn with a foreign key referencing the Employee table. However, as intended by the PR objective, the subsequent migration (20250225193719_update_evidence_assignee_to_user/migration.sql) updates the foreign key to reference the User table. This is verified by the Prisma schema that now shows the relationship as:
assigneeId String?assignee User? @relation(fields: [assigneeId], references: [id], onDelete: SetNull)Please ensure that the intended migration ordering is maintained in all deployment environments to avoid any inconsistency during rollouts. If the historical migration cannot be changed, consider adding a comment to the update migration for clarity to future maintainers.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Actions/updateEvidenceFrequency.ts (2)
8-11: Good schema validation using zodThe schema correctly validates that the id is a string and that frequency is a valid enum value or null.
26-47: Proper authorization and data validationThe action correctly verifies the user's organization context and ensures the evidence exists and belongs to the user's organization before proceeding with the update.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/EvidenceDetails.tsx (4)
12-12: Component replacement looks goodThe
ReviewSectioncomponent import replaces the previousReviewDateCard, which aligns with the PR objective to enhance evidence tasks with department and assignee filtering capabilities.
45-53: Enhanced loading state UIThe loading state UI has been improved with a better structured skeleton layout that properly aligns with the actual rendered component.
84-107: Improved layout for publish status and buttonThe restructuring of the publish status and button improves the UI organization and maintains the conditional rendering functionality.
109-116: Review section implementationThe
ReviewSectioncomponent properly receives all necessary props to handle evidence review information including the new department and assignee capabilities.apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Actions/assignEvidence.ts (6)
8-21: Well-structured server action setupThe server action is well-defined with proper schema validation and metadata for tracking purposes.
26-31: Organization validationGood security check to ensure the user has an associated organization before proceeding.
33-47: Evidence validationProper validation to ensure the evidence exists and belongs to the user's organization.
49-67: Assignee validationGood security check to verify that the assignee exists and has the appropriate admin privileges.
69-78: Evidence update implementationThe code properly updates the evidence with the new assignee and sets the updated timestamp.
84-90: Error handlingComprehensive error handling with appropriate logging and user-friendly error messages.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Actions/getOrganizationEvidenceTasks.ts (5)
4-4: Added department type importThe import of
Departmentsaligns with the PR objective to add department filtering capability.
29-30: New filter parametersThe schema has been properly extended with new optional filters for department and assigneeId.
44-52: Enhanced input destructuringThe destructuring of parsedInput has been reorganized to include the new filter parameters.
70-74: Filter implementationThe whereClause construction has been properly extended to include filtering by department and assigneeId.
117-125: Assignee data inclusionThe query now includes the assignee data with appropriate field selection, which aligns with the PR objective to support assignee filtering.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Actions/getOrganizationEmployees.ts (5)
7-14: Well-structured server action setupThe server action is well-defined with proper metadata for tracking purposes.
18-23: Organization validationGood security check to ensure the user has an associated organization before proceeding.
26-48: Query implementationThe database query is well-structured to retrieve organization members with admin or owner roles, including appropriate field selection and sorting.
51-56: Data transformationGood transformation of the database results into a simpler format for the frontend.
62-68: Error handlingComprehensive error handling with appropriate logging and user-friendly error messages.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Actions/updateEvidenceDepartment.ts (1)
1-71: Well-structured server action implementation with proper validation and security checks.The action correctly validates input, performs proper authorization checks, and includes comprehensive error handling. It ensures the user has access to the organization's evidence before allowing the update.
Good practices observed:
- Using zod for input validation
- Checking organization membership
- Verifying evidence ownership
- Proper error handling with specific error messages
- Appropriate logging
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/hooks/useEvidenceTasks.ts (4)
13-14: LGTM: New filter properties added correctly.The department and assigneeId properties are correctly typed and properly added to the interface.
51-59: LGTM: Updated destructuring to include new properties.The destructuring properly includes the new department and assigneeId properties with the existing ones.
62-71: LGTM: Updated SWR cache key with new filter parameters.The SWR cache key array has been correctly updated to include the new filter parameters, ensuring proper cache invalidation when these values change.
73-81: LGTM: Updated fetchEvidenceTasks call with new parameters.The fetchEvidenceTasks function call correctly passes the new department and assigneeId parameters.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/AssigneeSection.tsx (5)
18-22: LGTM: Clear and well-typed props interface.The props interface is well-defined with appropriate types for each property.
29-33: LGTM: Proper state initialization.The state is properly initialized with the current assignee ID, falling back to null if not provided.
35-43: LGTM: Well-structured action hook with success and error handling.The useAction hook implementation includes proper success and error feedback through toast notifications.
72-89: LGTM: Good UX with loading and error states.The component properly handles loading and error states, providing appropriate feedback to the user.
96-151: LGTM: Well-designed select component with avatar integration.The select implementation is well-designed, providing both a visual representation (avatar) and the name of each assignee. The placeholder for unassigned status is also a nice touch.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/ReviewSection.tsx (2)
37-106: LGTM: Well-organized layout with clear sections.The component has a well-structured layout with clear sections for different types of information (department, frequency, next review). The responsive design (using flex-col/flex-row) ensures good display across different device sizes.
92-101: LGTM: Clear conditional rendering for next review date.The conditional rendering for the next review date is implemented well, showing "ASAP" in red when no review information is available and the days until review with the formatted date otherwise.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/FrequencySection.tsx (1)
1-79: Clean and functional frequency selection component with proper state management.This component is well-structured and follows React best practices. It correctly manages state, handles side effects with useEffect, and provides appropriate feedback to users through toast notifications. The component is also properly typed with a clear interface.
I particularly like how the component:
- Synchronizes its local state with the incoming prop using useEffect
- Uses the next-safe-action pattern for handling server actions
- Provides visual feedback during execution with the disabled state
- Has clean error handling with user-friendly messages
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/columns.tsx (2)
66-88: Well-implemented department column with proper null handling.The department column implementation is clean with good formatting logic. It properly handles cases where the department is null or "none" by displaying a fallback message. The visual formatting with the Building icon and text transformation enhances readability.
134-164: Robust assignee column with proper fallback handling.The assignee column implementation is well-executed. It handles the unassigned state appropriately and includes a nicely formatted display with an avatar. The avatar component has proper fallback handling for when an image is not available. The text truncation is also a good detail for handling long names in a limited space.
packages/db/prisma/schema.prisma (2)
1057-1058: Good default value for department field.Setting a default value of "none" for the department field ensures there won't be null values to handle in the UI. This is consistent with how the department is handled in the UI components.
1070-1075: Well-structured relationship for assignee with appropriate cascade behavior.The assignee relationship is properly implemented with an index for query optimization. Using
onDelete: SetNullis a good choice here as it ensures that if a user is deleted, the evidence won't be deleted with it, but the assignee reference will be cleared.apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/FileCard.tsx (7)
3-3: Ensure all imports are necessary.The import statement now includes useState which is properly utilized for the newly added preview loading state.
6-11: LGTM - Dialog components properly imported.The DialogTitle component is now correctly imported and used in the file preview dialog.
29-32: LGTM - Icons and accessibility components added.Added Loader2 and Maximize2 icons for visual indicators, and VisuallyHidden for accessibility.
60-74: Great user experience improvement with automatic preview loading.The automatic preview loading functionality enhances user experience by loading the preview as soon as the component mounts, rather than requiring a user action.
78-150: Well-structured preview rendering with improved UX.The preview rendering has been significantly improved with:
- Clear loading states
- Better handling of different file types
- Direct preview in card
- Maximize buttons for expandability
- Proper tooltips for file names
This implementation provides better immediate feedback to users and a more intuitive interface.
152-154: LGTM - Added file name in dialog title.Adding the file name in the DialogTitle improves context awareness when viewing the expanded preview.
233-233: Minor icon size adjustment for consistency.Size adjustment for the Trash icon ensures visual consistency in the UI.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceList.tsx (14)
17-19: LGTM - Icons imported for new filter UI elements.The Building and User icons are appropriately imported for use in the department and assignee filter UI.
30-31: LGTM - Type imports properly updated.The Departments type is now imported from @bubba/db for proper type checking of the department filter.
39-40: LGTM - Avatar components imported for assignee display.Avatar components are imported for better visual representation of assignees in the filter dropdown.
59-60: LGTM - New query state variables for filtering.New query state variables for department and assignee filtering are correctly implemented.
78-79: LGTM - Updated API parameters for filtering.The useOrganizationEvidenceTasks hook is properly updated to include the new filter parameters.
95-103: LGTM - Improved memo handling for filter options.Using predefined constants for frequencies and departments improves performance by avoiding recalculations on each render.
105-129: Well-structured assignee data handling.The approach to building a map of unique assignees is clean and efficient. Using Map and providing proper sorting ensures a consistent UI experience.
135-136: LGTM - Updated clearFilters to include new filters.The clearFilters function correctly resets the new department and assignee filters.
154-158: LGTM - Updated filter check logic.The hasActiveFilters check is properly updated to include the new filter states.
172-176: LGTM - Helper for selected assignee display.Finding the selected assignee name for display is well implemented.
197-200: LGTM - Updated badge count logic.The active filter badge count is correctly updated to include the new filters.
248-295: Well-structured department and assignee filter UI.The new filter UI components are well-organized and maintain consistency with the existing filtering pattern:
- Clear visual indicators with icons
- Consistent interaction patterns
- Proper state handling
- Reset to first page when filters change
The Avatar implementation for assignees adds a nice visual touch.
341-367: LGTM - Filter badges for new filters.New filter badges for department and assignee match the existing pattern and provide clear visual feedback about active filters.
402-416: LGTM - Pagination button adjustments.Minor adjustments to pagination button styling maintain UI consistency.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/Components/DepartmentSection.tsx (7)
1-15: LGTM - Well-structured imports and client directive.The component correctly uses the client directive and imports all necessary dependencies.
16-20: LGTM - Clear interface definition.The DepartmentSectionProps interface is well-defined with appropriate types.
22-29: LGTM - Proper state initialization.State is correctly initialized from props with appropriate fallback.
45-48: LGTM - Sync state with prop changes.The useEffect correctly syncs the internal state when the currentDepartment prop changes.
49-53: LGTM - Clean value handling.The department change handler properly manages the "none" value case and updates both local state and remote data.
55-57: LGTM - Good filter implementation.Filtering out the "none" option from the dropdown and handling it separately improves the UX.
59-77: LGTM - Well-structured and accessible UI.The Select component implementation is clean with:
- Proper disabled state during execution
- Clear placeholder
- Consistent formatting of department names
- Proper "None" option handling
The UI provides a good user experience for department selection.
packages/db/prisma/migrations/20250225204820_add_department_to_evidence/migration.sql (1)
1-2: SQL migration looks good.The migration correctly adds a department column to the OrganizationEvidence table with a default value of 'none'. This aligns with the PR objective to support department filtering.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Actions/getOrganizationEvidence.ts (1)
37-40: Approved: Added assignee inclusion to query.Including the assignee data is necessary for the new filtering functionality and properly extends the query result shape.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/types.ts (1)
7-12: Type extension looks good.The assignee property is properly defined with all necessary fields and correctly marked as optional. This supports the assignee filtering functionality required by the PR.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/[id]/hooks/useOrganizationAdmins.ts (1)
14-31: Robust error handling in the fetcher function.The fetchOrganizationAdmins function has comprehensive error handling and appropriate fallbacks. Good job!
…ilter components - Extract filter-related components into separate files for better organization - Create EvidenceTableProvider to centralize state management for evidence tasks - Implement modular components for search, filtering, and pagination - Move constants to a dedicated file - Simplify EvidenceList component by delegating responsibilities to new components - Improve code readability and maintainability
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/constants.ts (2)
4-11: Consider future-proofing department definitions.While the current implementation is clean and appropriate, consider implementing a mechanism to ensure these constants stay in sync with the actual enum definitions in the database schema. If the
Departmentsenum in@bubba/dbchanges, this array would need manual updates.
14-14: Consistent formatting for arrays.For consistency with the
ALL_DEPARTMENTSarray, consider formatting theALL_FREQUENCIESarray with each value on a separate line, especially if more frequencies might be added in the future.-export const ALL_FREQUENCIES: Frequency[] = ["monthly", "quarterly", "yearly"]; +export const ALL_FREQUENCIES: Frequency[] = [ + "monthly", + "quarterly", + "yearly", +];apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/ActiveFilterBadges.tsx (2)
57-68: Consider extracting department name formatting to a utility functionThe department name formatting logic (replacing underscores with spaces and converting to uppercase) might be needed elsewhere in the application. Consider extracting this to a utility function for reusability.
- Department: {department.replace(/_/g, " ").toUpperCase()} + Department: {formatDepartmentName(department)}And then create a utility function:
// utils/formatting.ts export function formatDepartmentName(department: string): string { return department.replace(/_/g, " ").toUpperCase(); }
69-80: Simplify assignee badge conditionThe condition check for both
assigneeIdandassigneeNameis redundant since you're already handling the "Unknown" case when finding the assignee name.- {assigneeId && assigneeName && ( + {assigneeId && (apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/PaginationControls.tsx (1)
41-46: Consider making page size options configurableThe page size options are currently hardcoded. Consider making these configurable through props or a constant to improve flexibility and maintainability.
- {[5, 10, 20, 50, 100].map((size) => ( + {PAGE_SIZE_OPTIONS.map((size) => (And define the constant:
// Either as a constant in the same file or in a constants file const PAGE_SIZE_OPTIONS = [5, 10, 20, 50, 100]; // Or through props interface PaginationControlsProps { pageSizeOptions?: number[]; } export function PaginationControls({ pageSizeOptions = [5, 10, 20, 50, 100] }: PaginationControlsProps) { // ... }apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/SearchInput.tsx (1)
21-28: Consider optimizing the pagination resetThe component always resets the page to "1" on every debounce update, even when clearing the search input. This might cause unnecessary rerenders if a user frequently clears the input without typing new content.
useEffect(() => { if (debouncedValue === "") { setSearch(null); + if (search !== null) { + setPage("1"); // Only reset page when actually clearing a search + } } else { setSearch(debouncedValue); + setPage("1"); // Reset to first page when searching } - setPage("1"); // Reset to first page when searching }, [debouncedValue, setSearch, setPage, search]);apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/FilterDropdown.tsx (1)
107-109: Consider parameterizing the department format transformation.The code assumes departments are stored with underscores that should be displayed as spaces and in uppercase. If the formatting requirements change, you'd need to update this in multiple places.
Consider extracting this to a utility function:
-<span>{dept.replace(/_/g, " ").toUpperCase()}</span> +<span>{formatDepartmentName(dept)}</span>And define the function elsewhere to be reused across the application.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceList.tsx (1)
19-19: Consider adding an empty state message instead of returning null.When there are no evidence tasks and the component is not loading, it returns null. This might be confusing for users.
Consider adding an empty state message to guide users:
-if (!evidenceTasks.length && !isLoading) return null; +if (!evidenceTasks.length && !isLoading) { + return ( + <div className="w-full py-10 text-center"> + <p className="text-muted-foreground">No evidence tasks found. Try adjusting your filters.</p> + </div> + ); +}apps/app/src/app/[locale]/(app)/(dashboard)/evidence/hooks/useEvidenceTableContext.tsx (2)
94-101: Simplify data transformation if possible.The transformation from rawEvidenceTasks to evidenceTasks only adds an evidence object with a name property that duplicates the existing name property.
If this transformation isn't required by the DataTable component, consider simplifying:
- const evidenceTasks = useMemo(() => { - return rawEvidenceTasks?.map((task) => ({ - ...task, - evidence: { - name: task.name, - }, - })) as EvidenceTaskRow[] | undefined; - }, [rawEvidenceTasks]); + const evidenceTasks = rawEvidenceTasks as EvidenceTaskRow[] | undefined;If it is required, consider adding a comment explaining why this transformation is needed.
108-129: Consider fetching all assignees separately.The current implementation only shows assignees who are assigned to tasks in the current filtered view, which might limit filtering options.
Consider fetching all possible assignees from a separate API call to ensure users can filter by any assignee regardless of current view state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/ActiveFilterBadges.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/FilterDropdown.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/PaginationControls.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/SearchInput.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/index.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceList.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/index.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/index.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/constants.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/hooks/useEvidenceTableContext.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/page.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/index.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/index.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/index.ts
🔇 Additional comments (11)
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/constants.ts (1)
1-14: Constants align with the new filtering requirements and follow good practices.The file properly defines type-safe constants for departments and frequencies that will be used for filtering evidence tasks, which aligns with the PR objective of adding department filtering functionality. The constants are well-organized with descriptive comments.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/page.tsx (1)
2-2: Proper implementation of context provider patternThe addition of the
EvidenceTableProviderwrapper is a clean implementation of the context provider pattern. This is a good architectural decision that centralizes state management for evidence filtering functionality.Also applies to: 5-9
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/ActiveFilterBadges.tsx (1)
7-30: Well-structured filter badge implementationThe component correctly handles displaying active filters with a clean implementation for finding the assignee name and returning null when no filters are active.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/PaginationControls.tsx (1)
14-22: Good pagination implementation with proper error handlingThe component properly handles missing pagination data by returning null and correctly parses the current page.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/SearchInput.tsx (1)
13-29: Efficient implementation of debounced searchGood use of debounce to optimize performance while typing. The effect correctly handles both setting and clearing the search, and resets pagination when search changes.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceFilters/FilterDropdown.tsx (3)
85-88: Good use of pagination reset during filtering.Resetting to the first page when a filter changes is good UX practice - this prevents confusion that might occur if the user is on a page that no longer exists after filtering.
126-136: Graceful handling of missing assignee images.Good implementation of the Avatar component with proper fallbacks for when images are missing or null.
140-152: Clean implementation of "Clear all filters" button.The conditional rendering of the clear filters button only when filters are active is a good UX pattern. The ghost variant is appropriate for this type of secondary action.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceList.tsx (2)
25-36: Well-organized filter UI components.The separation of filter components into distinct, reusable parts (SearchInput, FilterDropdown, ActiveFilterBadges) improves code organization and maintainability.
43-43: Simplified data table implementation.The DataTable now directly receives the evidenceTasks array without additional transformation, suggesting good standardization of the data structure.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/hooks/useEvidenceTableContext.tsx (1)
196-200: Good context validation.Throwing an error when the hook is used outside of its provider prevents subtle bugs and makes the dependency clear to developers.
| status: status as "published" | "draft" | null, | ||
| frequency: frequency as any, | ||
| department: department as any, | ||
| assigneeId, | ||
| page: currentPage, | ||
| pageSize: currentPageSize, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using 'any' type assertions.
Type assertions to 'any' bypass TypeScript's type checking, potentially leading to runtime errors. This is especially concerning when passing these values to API calls.
Replace the 'any' type assertions with proper types:
- frequency: frequency as any,
- department: department as any,
+ frequency: frequency as Frequency | null,
+ department: department as Departments | 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.
| status: status as "published" | "draft" | null, | |
| frequency: frequency as any, | |
| department: department as any, | |
| assigneeId, | |
| page: currentPage, | |
| pageSize: currentPageSize, | |
| }); | |
| status: status as "published" | "draft" | null, | |
| frequency: frequency as Frequency | null, | |
| department: department as Departments | null, | |
| assigneeId, | |
| page: currentPage, | |
| pageSize: currentPageSize, | |
| }); |
- Implement server action `getEvidenceTasksStats` to calculate evidence task statuses - Create `useEvidenceTasksStats` hook for fetching and managing task statistics - Add `EvidenceSummaryCards` component to display task status overview - Enhance `EvidenceList` with summary cards and improved empty state handling - Improve error logging and handling in evidence tasks data fetching - Add detailed logging for debugging evidence tasks data
1 participant
Summary by CodeRabbit