[Fix]: Update Timesheet Functionality and Load All Projects #3413
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe changes in this pull request involve modifications to multiple components within the timesheet management system. Key updates include the replacement of the Changes
Suggested labels
Suggested reviewers
Poem
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: 3
🧹 Outside diff range and nitpick comments (23)
apps/web/lib/components/Kanban.tsx (4)
Line range hint
235-296: Complete the internationalization of all stringsWhile "CREATE_TASK" has been translated, other strings like "Collapse Column" and "Edit Status" remain hardcoded. This creates an inconsistent user experience when the language is changed.
Apply this diff to complete the translations:
- <div className="hover:font-medium p-1.5 text-sm cursor-pointer" onClick={() => toggleColumn(title, false)} > - Collapse Column + {t('common.COLLAPSE_COLUMN')} </div> <div className="hover:font-medium p-1.5 text-sm cursor-pointer" onClick={editOpenModal} > - Edit Status + {t('common.EDIT_STATUS')} </div>
Line range hint
380-420: Maintain consistency in string translationsSimilar to the previous component, some strings remain untranslated while others use the translation system. This inconsistency should be addressed.
Apply this diff to complete the translations:
<div className="hover:font-medium p-1.5 text-sm cursor-pointer" onClick={() => toggleColumn(title, true)} > - Collapse Column + {t('common.COLLAPSE_COLUMN')} </div> <div className="hover:font-medium p-1.5 text-sm cursor-pointer" onClick={openModal} > - Edit Status + {t('common.EDIT_STATUS')} </div>
Line range hint
1-565: Consider architectural improvements for better maintainabilityThe current implementation could benefit from several architectural improvements:
- Extract style constants to a theme configuration
- Consider using a modal management system to reduce duplicate modal state logic
- Move common translation keys to a shared constant file
Here's a suggested approach:
- Create a theme configuration file:
// theme/kanban.ts export const KANBAN_THEME = { columnWidth: '355px', defaultHeight: '320px', headerPadding: '15px 7px', borderRadius: 'rounded-lg', // ... other style values };
- Create a shared translations constant file:
// constants/translations.ts export const KANBAN_TRANSLATIONS = { CREATE_TASK: 'common.CREATE_TASK', COLLAPSE_COLUMN: 'common.COLLAPSE_COLUMN', EDIT_STATUS: 'common.EDIT_STATUS', // ... other translation keys };
- Consider implementing a modal management hook:
// hooks/useModalManager.ts export const useModalManager = () => { const [modals, setModals] = useState<Record<string, boolean>>({}); const openModal = (modalId: string) => setModals(prev => ({ ...prev, [modalId]: true })); const closeModal = (modalId: string) => setModals(prev => ({ ...prev, [modalId]: false })); return { modals, openModal, closeModal }; };
Line range hint
1-565: Optimize performance with memoizationSeveral opportunities for performance optimization through memoization:
- Memoize the column height calculation
- Memoize the items array to prevent unnecessary re-renders
- Memoize callback functions passed as props
Consider applying these optimizations:
// 1. Memoize column height calculation const getKanbanColumnHeight = useMemo(() => (itemsLength: number, containerRef: RefObject<HTMLDivElement> | undefined): string => { // ... existing logic }, [/* dependencies */] ); // 2. Memoize items array const memoizedItems = useMemo(() => items, [items]); // 3. Memoize callbacks const handleCreateTask = useCallback(() => { openModal(); }, [openModal]);apps/web/app/[locale]/timesheet/[memberId]/page.tsx (2)
Line range hint
40-62: Consider enhancing error handling and loading state managementWhile the initialization of useOrganizationProjects and the useEffect setup are functionally correct, consider these improvements:
- Add error handling for the getOrganizationProjects call
- Track and handle the loading state
- Consider memoizing getOrganizationProjects if it changes frequently
Here's a suggested implementation:
- const { getOrganizationProjects } = useOrganizationProjects(); + const { getOrganizationProjects, loading: projectsLoading, error: projectsError } = useOrganizationProjects(); React.useEffect(() => { - getOrganizationProjects(); + try { + getOrganizationProjects(); + } catch (error) { + console.error('Failed to fetch organization projects:', error); + // Consider showing an error notification to the user + } }, [getOrganizationProjects]); + // Handle loading state + if (projectsLoading) { + // Consider showing a loading indicator + } + // Handle error state + if (projectsError) { + // Consider showing an error message to the user + }
Line range hint
65-77: Consider optimizing filtering performance for large datasetsThe current filtering implementation, while correct, might face performance issues with large datasets due to nested filtering operations. Consider these optimizations:
- Index the data for faster searching
- Implement debounced search
- Consider pagination or virtual scrolling for large lists
Here's a suggested optimization:
+ const debouncedSearch = useDebounce(search, 300); + const filterDataTimesheet = useMemo(() => { + if (!debouncedSearch) return timesheet; + + const searchTerms = debouncedSearch.toLowerCase().split(' '); const filteredTimesheet = timesheet .filter((v) => v.tasks.some( (task) => { + const searchableText = [ + task.task?.title, + task.employee?.fullName, + task.project?.name + ].map(text => text?.toLowerCase() ?? '').join(' '); + + return searchTerms.every(term => searchableText.includes(term)); } ) ); return filteredTimesheet; - }, [timesheet, lowerCaseSearch]); + }, [timesheet, debouncedSearch]);apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetCard.tsx (1)
Line range hint
1-192: Consider enhancing type safety, accessibility, and performance.While the current implementation works well, here are some suggestions for improvement:
Type Safety:
- Add null checks when accessing
data?.PENDINGin the groupByDate call- Consider using TypeScript's non-null assertion operator more judiciously
Accessibility:
- Add aria-label to the accordion items to describe their content
- Consider adding aria-expanded state to accordion triggers
Performance:
- Memoize the results of
getStatusTimesheetandgroupByDateusing useMemo- Consider implementing React.memo for child components that receive stable props
Example implementation for performance optimization:
+ import { useMemo } from 'react'; export const TimesheetCardDetail = ({ data }: { data?: Record<TimesheetStatus, TimesheetLog[]> }) => { const { getStatusTimesheet, groupByDate } = useTimesheet({}); const { timesheetGroupByDays } = useTimelogFilterOptions(); - const timesheetGroupByDate = groupByDate(data?.PENDING || []) + const timesheetGroupByDate = useMemo(() => + groupByDate(data?.PENDING || []), + [data?.PENDING, groupByDate] + );apps/web/app/[locale]/timesheet/[memberId]/components/TimeSheetFilterPopover.tsx (2)
100-102: Simplify the project selection implementation.The current implementation of
itemToStringhas unnecessary complexity and redundant checks.Consider simplifying the code:
<MultiSelect localStorageKey="timesheet-select-filter-projects" removeItems={shouldRemoveItems} - items={organizationProjects ?? []} + items={organizationProjects || []} itemToString={(project) => - (organizationProjects && project ? project.name : '') || '' + project?.name || '' } itemId={(item) => item.id} onValueChange={(selectedItems) => setProjectState(selectedItems as any)} multiSelect={true} triggerClassName="dark:border-gray-700" />Improvements:
- Simplified null check using logical OR
- Removed unnecessary organizationProjects check in itemToString
- Used optional chaining for cleaner null handling
Line range hint
21-22: Improve type safety by removing 'any' type assertions.The state setters use type 'any' which reduces type safety. Consider defining proper types for the state values.
Example improvement:
interface TimelogFilterState { employee: Member[]; project: Project[]; task: Task[]; statusState: StatusOption[]; } const setProjectState = (selectedItems: Project[]) => { // Implementation };apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx (3)
51-51: Improved state management and data handlingGood improvements in state management:
- Default value for isBillable with nullish coalescing
- Updated project data source to use organizationProjects
- Proper handling of undefined selectedValues
Consider adding type validation for the project data structure.
- selectedValues={selectedValues ?? []} + selectedValues={selectedValues ?? {} as Record<string, Item>}Also applies to: 56-56, 234-234
310-318: Good UI/UX improvements with loading stateThe loading state implementation is good, but consider these accessibility improvements:
<button disabled={loadingUpdateTimesheet} + aria-busy={loadingUpdateTimesheet} type="submit" className={clsxm( 'bg-primary dark:bg-primary-light h-[2.3rem] w-[5.5rem] justify-center font-normal flex items-center text-white px-2 rounded-lg', + loadingUpdateTimesheet && 'cursor-not-allowed opacity-70' )}> {loadingUpdateTimesheet && ( - <ReloadIcon className="w-4 h-4 mr-2 animate-spin" /> + <ReloadIcon className="w-4 h-4 mr-2 animate-spin" aria-hidden="true" /> )} - {loadingUpdateTimesheet ? "Processing..." : t('common.SAVE')} + <span>{loadingUpdateTimesheet ? "Processing..." : t('common.SAVE')}</span> </button>
Line range hint
72-83: Consider using a form validation libraryThe current manual validation works but could be improved:
- Time format validation is basic
- Error messages are shown through alert()
Consider using a form validation library like
react-hook-formwithzodfor better validation:
- Type-safe validation
- Better error handling
- More maintainable validation rules
- Improved user experience with inline error messages
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (2)
144-146: Consider consolidating duplicated stylesThe TaskNameInfoDisplay styling is duplicated between the CalendarDataView and BaseCalendarDataView components. Consider extracting these common styles into a shared constant or utility function to maintain consistency and reduce duplication.
+ // Add at the top of the file + const taskNameDisplayStyles = { + container: 'rounded-sm h-auto !px-[0.3312rem] py-[0.2875rem] shadow-[0px_0px_15px_0px_#e2e8f0] dark:shadow-transparent', + title: 'text-sm text-ellipsis overflow-hidden !text-[#293241] dark:!text-white' + }; // Then in both components: <TaskNameInfoDisplay task={task.task} - className={cn( - 'rounded-sm h-auto !px-[0.3312rem] py-[0.2875rem] shadow-[0px_0px_15px_0px_#e2e8f0] dark:shadow-transparent' - )} - taskTitleClassName={cn( - 'text-sm text-ellipsis overflow-hidden !text-[#293241] dark:!text-white' - )} + className={cn(taskNameDisplayStyles.container)} + taskTitleClassName={cn(taskNameDisplayStyles.title)} showSize={true} dash taskNumberClassName="text-sm" />Also applies to: 238-241
Line range hint
22-63: Simplify render logic and improve loading state handlingThe current implementation has nested ternary operators and doesn't effectively use the loading prop. Consider restructuring for better readability and loading state handling.
export function CalendarView({ data, loading, user }: { data?: GroupedTimesheet[], loading: boolean, user?: IUser | undefined }) { const t = useTranslations(); const { timesheetGroupByDays } = useTimelogFilterOptions(); const defaultDaysLabels = [/* ... */]; + const renderContent = () => { + if (loading) { + return ( + <div className="flex items-center justify-center h-full"> + <p>{t('pages.timesheet.LOADING')}</p> + </div> + ); + } + + if (!data || data.length === 0) { + return ( + <div className="flex items-center justify-center h-full min-h-[280px]"> + <p>{t('pages.timesheet.NO_ENTRIES_FOUND')}</p> + </div> + ); + } + + const viewComponents = { + Monthly: MonthlyCalendarDataView, + Weekly: WeeklyCalendarDataView, + }; + + const ViewComponent = viewComponents[timesheetGroupByDays as keyof typeof viewComponents]; + return ViewComponent ? ( + <ViewComponent data={data} daysLabels={defaultDaysLabels} t={t} /> + ) : ( + <CalendarDataView data={data} t={t} /> + ); + }; return ( <div className="grow h-full bg-[#FFFFFF] dark:bg-dark--theme"> - {data ? ( - data.length > 0 ? ( - <> - {timesheetGroupByDays === 'Monthly' ? ( - <MonthlyCalendarDataView data={data} daysLabels={defaultDaysLabels} t={t} /> - ) : timesheetGroupByDays === 'Weekly' ? ( - <WeeklyCalendarDataView data={data} daysLabels={defaultDaysLabels} t={t} /> - ) : ( - <CalendarDataView data={data} t={t} /> - )} - </> - ) : ( - <div className="flex items-center justify-center h-full min-h-[280px]"> - <p>{t('pages.timesheet.NO_ENTRIES_FOUND')}</p> - </div> - ) - ) : ( - <div className="flex items-center justify-center h-full"> - <p>{t('pages.timesheet.LOADING')}</p> - </div> - )} + {renderContent()} </div> ); }apps/web/app/[locale]/timesheet/[memberId]/components/AddTaskModal.tsx (2)
Line range hint
249-268: Add time zone handling and validate work hoursThe time calculation logic has several potential issues:
- No time zone handling which could cause issues with international teams
- Missing validation for maximum work hours per shift
- Complex time calculation could be simplified
Consider these improvements:
const calculateTotalHoursHour = React.useCallback( (start: string, end: string): string => { if (!start || !end) return '00:00h'; const startMinutes = convertToMinutesHour(start); const endMinutes = convertToMinutesHour(end); const totalMinutes = endMinutes >= startMinutes ? endMinutes - startMinutes : 1440 - startMinutes + endMinutes; + // Add validation for maximum work hours (e.g., 24 hours) + if (totalMinutes > 1440) { + throw new Error('Shift duration cannot exceed 24 hours'); + } const hours = Math.floor(totalMinutes / 60); const minutes = totalMinutes % 60; return `${String(hours).padStart(2, '0')}:${String(minutes).padStart(2, '0')}h`; }, [] );
Line range hint
171-181: Implement form submission and validationThe form is missing critical functionality:
- No form submission handler
- No validation for required fields before submission
- No loading states during submission
Consider implementing form handling:
+const [isSubmitting, setIsSubmitting] = React.useState(false); +const [errors, setErrors] = React.useState<Record<string, string>>({}); +const validateForm = () => { + const newErrors: Record<string, string> = {}; + if (!task) newErrors.task = 'Task is required'; + if (!notes.trim()) newErrors.notes = 'Notes are required'; + if (!selectedValues.Project) newErrors.project = 'Project is required'; + setErrors(newErrors); + return Object.keys(newErrors).length === 0; +}; +const handleSubmit = async () => { + if (!validateForm()) return; + setIsSubmitting(true); + try { + // Add your submission logic here + closeModal(); + } catch (error) { + console.error('Failed to submit:', error); + } finally { + setIsSubmitting(false); + } +}; <button type="submit" + disabled={isSubmitting} className={clsxm( 'bg-primary dark:bg-primary-light h-[2.3rem] w-[5.5rem] justify-center font-normal flex items-center text-white px-2 rounded-lg', + isSubmitting && 'opacity-50 cursor-not-allowed' )} + onClick={handleSubmit} > - {t('common.SAVE')} + {isSubmitting ? t('common.SAVING') : t('common.SAVE')} </button>apps/web/app/[locale]/timesheet/[memberId]/components/FilterWithStatus.tsx (5)
Line range hint
8-19: Fix inconsistent status naming and improve type safetyThe component has inconsistencies in status naming and type safety issues:
FilterStatustype uses 'Rejected' while the code accessesDENIEDin the data- String literal
'IN REVIEW'bypasses TypeScript's type checkingConsider applying these changes:
-export type FilterStatus = 'All Tasks' | 'Pending' | 'Approved' | 'In review' | 'Draft' | 'Rejected'; +export type FilterStatus = 'All Tasks' | 'Pending' | 'Approved' | 'In Review' | 'Draft' | 'Rejected'; export function FilterWithStatus({ activeStatus, onToggle, className, data }: Readonly<{ activeStatus: FilterStatus; - data?: Record<TimesheetStatus, TimesheetLog[]> + data?: Partial<Record<TimesheetStatus, TimesheetLog[]>> onToggle: (status: FilterStatus) => void; className?: HTMLAttributes<HTMLDivElement>; }>) {
Line range hint
35-42: Optimize memo dependencies and fix status accessThe
buttonDatamemo uses the translation function inside but doesn't include it in dependencies. Also, the status access needs to be aligned with TimesheetStatus type.Apply these changes:
const buttonData = React.useMemo(() => { const counts = { [t('pages.timesheet.ALL_TASKS')]: Object.values(data ?? {}).reduce((total, tasks) => total + (tasks?.length ?? 0), 0), [t('pages.timesheet.PENDING')]: data?.PENDING?.length ?? 0, [t('pages.timesheet.APPROVED')]: data?.APPROVED?.length ?? 0, - [t('pages.timesheet.IN_REVIEW')]: data?.['IN REVIEW']?.length ?? 0, + [t('pages.timesheet.IN_REVIEW')]: data?.IN_REVIEW?.length ?? 0, [t('pages.timesheet.DRAFT')]: data?.DRAFT?.length ?? 0, - [t('pages.timesheet.REJECTED')]: data?.DENIED?.length ?? 0, + [t('pages.timesheet.REJECTED')]: data?.REJECTED?.length ?? 0, }; return Object.entries(counts).map(([label, count]) => ({ label: label as FilterStatus, count, icon: <i className={statusIcons[label as FilterStatus]} />, })); -}, [data]); +}, [data, t]);
Line range hint
55-77: Improve accessibility and button renderingThe current implementation has accessibility issues and uses index as key which is an anti-pattern in React.
Consider these improvements:
{buttonData.map(({ label, count, icon }, index) => ( <Button - key={index} + key={label} className={clsxm( 'group flex items-center justify-start h-[2.2rem] rounded-xl w-full', 'dark:bg-gray-800 dark:border-primary-light bg-transparent text-[#71717A] w-[80px]', activeStatus === label && 'text-primary bg-white shadow-2xl dark:text-primary-light font-bold border' )} onClick={() => onToggle(label)} + aria-label={`Filter by ${label.toLowerCase()} status. ${count} items`} + aria-pressed={activeStatus === label} >
Line range hint
21-32: Consider using TypeScript enum or const assertions for status iconsThe status icons mapping could benefit from stronger typing to prevent potential mismatches.
Consider this improvement:
-const statusIcons: Record<FilterStatus, string> = { +const statusIcons = { 'All Tasks': 'icon-all', Pending: 'icon-pending', Approved: 'icon-approved', 'In review': 'icon-rejected', Draft: 'icon-approved', Rejected: 'icon-rejected', -}; +} as const;
Status naming inconsistencies found in the timesheet implementation
There are several inconsistencies in how timesheet statuses are handled across the codebase:
DENIEDvsREJECTED: The backend usesDENIEDbut the UI displays it asREJECTEDIN REVIEWvsIN_REVIEW: Mixed usage of space and underscore in the status name- The
FilterStatustype uses display-friendly names ('In review') while the data object uses uppercase keys ('IN REVIEW')- Inconsistent casing: Some components use lowercase for valid statuses (e.g., in
timer/timesheet/status/route.ts)Recommendations:
- Standardize on either
DENIEDorREJECTEDacross the entire codebase- Use consistent casing (preferably uppercase) for status values
- Standardize the format of multi-word statuses (either with space or underscore)
- Consider using the enum
TimesheetStatusconsistently instead of string literals🔗 Analysis chain
Line range hint
1-77: Verify status naming consistency across the codebaseLet's ensure that the status naming changes are consistent across all components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for inconsistent status naming across the codebase # Check for different variations of status names echo "Checking for status name variations..." rg -i "denied|rejected" --type tsx --type ts # Check TimesheetStatus enum definition echo -e "\nChecking TimesheetStatus definition..." rg "enum\s+TimesheetStatus" --type ts # Check for 'IN REVIEW' vs 'IN_REVIEW' usage echo -e "\nChecking review status variations..." rg -i "('IN[\s_]REVIEW'|\"IN[\s_]REVIEW\")" --type tsx --type tsLength of output: 482
Script:
#!/bin/bash # Let's try a different approach without file type restrictions # Check for TimesheetStatus enum definition echo "Checking TimesheetStatus enum definition..." ast-grep --pattern 'enum TimesheetStatus { $$$ }' # Check for status name variations in the codebase echo -e "\nChecking status name variations..." rg -i "status.*=.*('DENIED'|'REJECTED'|'IN REVIEW'|'IN_REVIEW')" # Check for data access patterns echo -e "\nChecking data access patterns..." rg "data\?\.(DENIED|REJECTED|IN_REVIEW|'IN REVIEW'|DRAFT|PENDING|APPROVED)" # Check for translations related to these statuses echo -e "\nChecking translation keys..." rg "'pages\.timesheet\.(DENIED|REJECTED|IN_REVIEW|DRAFT|PENDING|APPROVED)'"Length of output: 4188
apps/web/app/hooks/features/useTimesheet.ts (2)
Line range hint
165-176: Improve error handling consistency across async operationsThe error handling patterns are inconsistent across the async operations:
updateTimesheetthrows the errorcreateTimesheetswallows the errorupdateTimesheetStatuslogs the error but continues silentlyThis inconsistency could make debugging difficult and lead to silent failures.
Consider standardizing error handling:
const createTimesheet = useCallback( async ({ ...timesheetParams }: UpdateTimesheet) => { if (!user) { throw new Error("User not authenticated"); } try { const response = await queryCreateTimesheet(timesheetParams); setTimesheet((prevTimesheet) => [ response.data, ...(prevTimesheet || []) ]); + } catch (error) { - console.error('Error:', error); + console.error('Error creating timesheet:', error); + throw error; } }, [queryCreateTimesheet, setTimesheet, user] ); const updateTimesheetStatus = useCallback( async ({ status, ids }: { status: TimesheetStatus; ids: ID[] | ID }) => { - if (!user) return; + if (!user) { + throw new Error("User not authenticated"); + } const idsArray = Array.isArray(ids) ? ids : [ids]; try { // ... existing code ... } catch (error) { console.error('Error updating timesheet status:', error); + throw error; } }, [queryUpdateTimesheetStatus, setTimesheet, user] );Also applies to: 246-259, 277-289
Line range hint
224-244: Optimize performance of status mapping and grouping operationsThe status mapping and grouping operations could be optimized to prevent unnecessary recalculations:
STATUS_MAPis recreated on every call togetStatusTimesheet- The grouping functions aren't memoized
Consider these optimizations:
+const STATUS_MAP: Record<TimesheetStatus, TimesheetLog[]> = { + PENDING: [], + APPROVED: [], + DENIED: [], + DRAFT: [], + 'IN REVIEW': [] +}; const getStatusTimesheet = (items: TimesheetLog[] = []) => { - const STATUS_MAP: Record<TimesheetStatus, TimesheetLog[]> = { - PENDING: [], - APPROVED: [], - DENIED: [], - DRAFT: [], - 'IN REVIEW': [] - }; + const result = JSON.parse(JSON.stringify(STATUS_MAP)); return items.reduce((acc, item) => { const status = item.timesheet.status; if (isTimesheetStatus(status)) { acc[status].push(item); } else { console.warn(`Invalid timesheet status: ${status}`); } return acc; - }, STATUS_MAP); + }, result); } +const memoizedGroupByDate = useMemo(() => groupByDate, []); +const memoizedGroupByWeek = useMemo(() => groupByWeek, []); +const memoizedGroupByMonth = useMemo(() => groupByMonth, []);Also applies to: 268-276
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
apps/web/app/[locale]/timesheet/[memberId]/components/AddTaskModal.tsx(2 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx(3 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/CompactTimesheetComponent.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx(7 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/FilterWithStatus.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/TimeSheetFilterPopover.tsx(3 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetCard.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/page.tsx(3 hunks)apps/web/app/hooks/features/useTimesheet.ts(1 hunks)apps/web/lib/components/Kanban.tsx(4 hunks)
🔇 Additional comments (9)
apps/web/app/[locale]/timesheet/[memberId]/components/CompactTimesheetComponent.tsx (1)
33-33: LGTM: Improved loading spinner proportions
The increased spinner size (w-5/h-5) provides better visual balance within its container (w-6/h-6).
apps/web/app/[locale]/timesheet/[memberId]/page.tsx (2)
11-11: LGTM: Clean import addition
The useOrganizationProjects hook import is properly grouped with other custom hooks.
Line range hint 1-1: Verify complete migration from useTeamTasks
Let's verify that all instances of useTeamTasks have been replaced with useOrganizationProjects across the codebase.
apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetCard.tsx (1)
154-154: LGTM! Consider verifying responsive behavior.
The styling update aligns with the broader UI consistency improvements across the timesheet components. The change from shadow effects to rounded corners with specific padding maintains visual hierarchy while potentially improving readability.
Please verify the visual appearance of task names across different screen sizes and text lengths. Run the following script to find related components that might need similar styling updates:
✅ Verification successful
Style update looks good and consistent with component usage
The styling change to TaskNameInfoDisplay is appropriate as it's used within a timesheet task row where consistent spacing and visual hierarchy are important. The component is rendered with proper padding and rounded corners, maintaining visual consistency with the parent accordion item's design. The shadow effect modification (shadow-[0px_0px_15px_0px_#e2e8f0] dark:shadow-transparent) provides subtle depth in light mode while staying flat in dark mode, which is consistent with modern UI practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other TaskNameInfoDisplay usages to ensure consistent styling
rg "TaskNameInfoDisplay.*className" --type tsx
Length of output: 76
Script:
#!/bin/bash
# Search for TaskNameInfoDisplay component usage with broader file patterns
rg "TaskNameInfoDisplay.*className" --type ts --type tsx --type jsx --type js
# Find all files containing TaskNameInfoDisplay to understand its usage
rg "TaskNameInfoDisplay" --type ts --type tsx --type jsx --type js
Length of output: 202
Script:
#!/bin/bash
# First, find all TypeScript/React files that might contain TaskNameInfoDisplay
fd "\.(tsx|ts|jsx|js)$"
# Then search for TaskNameInfoDisplay in all files
rg "TaskNameInfoDisplay"
Length of output: 71167
Script:
#!/bin/bash
# Find the file content to understand TaskNameInfoDisplay usage
fd "TimesheetCard.tsx" | xargs cat
Length of output: 9985
apps/web/app/[locale]/timesheet/[memberId]/components/TimeSheetFilterPopover.tsx (1)
2-2: LGTM! Verify hook usage across the application.
The addition of useOrganizationProjects hook aligns with the PR objective to load all projects and maintains consistency with other components.
Let's verify the consistent usage of this hook across the application:
Also applies to: 16-16
✅ Verification successful
Based on the search results, I can see that the useOrganizationProjects hook is consistently used across multiple components in the application, including:
- TimeSheetFilterPopover.tsx
- AddTaskModal.tsx
- EditTaskModal.tsx
- task-all-status-type.tsx
- create-project-modal.tsx
- integration-setting.tsx
- page.tsx (timesheet)
Let's verify the hook's implementation and its usage pattern:
Based on the search results, I can now provide a definitive response about the hook usage:
✓ The hook usage is consistent and correct across the application
The useOrganizationProjects hook is properly used across multiple components with consistent patterns:
- Components that need project data use
organizationProjectsfor listing/filtering - Components that need to modify projects use specific actions like
editOrganizationProject,createOrganizationProject - Components that need to look up projects use
getOrganizationProject
The hook's implementation in useOrganizationProjects.ts follows standard patterns with proper state management using atoms.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of useOrganizationProjects hook
# Expected: Should find usage in AddTaskModal, EditTaskModal, and page.tsx as mentioned in the AI summary
# Search for useOrganizationProjects usage
rg "useOrganizationProjects" --type tsx
Length of output: 67
Script:
#!/bin/bash
# Let's try with a more generic search approach to find TypeScript/React files
# Search for useOrganizationProjects usage across ts and tsx files
rg "useOrganizationProjects" -t ts -t tsx
# Also search for the hook definition
rg "export.*useOrganizationProjects" -t ts -t tsx
Length of output: 149
Script:
#!/bin/bash
# Search for useOrganizationProjects usage without file type restrictions
rg "useOrganizationProjects"
# Let's also find all files with .ts or .tsx extension to understand the structure
fd "\.(ts|tsx)$"
Length of output: 67872
Script:
#!/bin/bash
# Check the hook implementation
rg -A 5 "export function useOrganizationProjects"
# Check how the hook is used in these components
rg -A 2 "const.*useOrganizationProjects"
Length of output: 3307
apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx (1)
9-9: Verify the impact of switching from useTeamTasks to useOrganizationProjects
The switch to useOrganizationProjects aligns with the PR objectives to load all projects. However, we should verify that this change doesn't affect existing functionality.
Also applies to: 25-25
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (1)
105-108: Verify status terminology change across the codebase
The status "DENIED" is being replaced with "REJECTED". While this change improves clarity, we should ensure this terminology update is consistent across the entire application.
✅ Verification successful
Status terminology change is part of a broader standardization effort
The change from 'DENIED' to 'REJECTED' appears to be part of a systematic update across the application. The search results show:
- Multiple components are already using the 'DENIED' to 'REJECTED' conversion pattern
- Both terms exist in the codebase, with 'REJECTED' being used in newer components and translations
- The change is being handled consistently with conditional rendering:
status === 'DENIED' ? 'REJECTED' : status - Translation files in multiple languages support both terms
This change appears to be a deliberate standardization effort, and the current implementation correctly handles the transition.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of 'DENIED' status
rg -i "('|\")DENIED('|\")" --type typescript --type javascript
# Search for current usage of 'REJECTED' status
rg -i "('|\")REJECTED('|\")" --type typescript --type javascript
Length of output: 209
Script:
#!/bin/bash
# Search for any remaining instances of 'DENIED' status using ripgrep
rg -i "('|\")DENIED('|\")" --type ts --type tsx --type js --type jsx
# Search for current usage of 'REJECTED' status
rg -i "('|\")REJECTED('|\")" --type ts --type tsx --type js --type jsx
Length of output: 207
Script:
#!/bin/bash
# Search for any remaining instances of 'DENIED' status using ripgrep without file type filters
rg -i "('|\")DENIED('|\")"
# Search for current usage of 'REJECTED' status
rg -i "('|\")REJECTED('|\")"
Length of output: 10904
apps/web/app/[locale]/timesheet/[memberId]/components/AddTaskModal.tsx (1)
2-2: Verify impact of switching from team-based to organization-based projects
The change from useTeamTasks to useOrganizationProjects represents a significant architectural shift in how projects are loaded. While this aligns with the PR objective, we should ensure this change doesn't affect existing team-specific functionality.
Also applies to: 20-20
✅ Verification successful
No impact from switching to organization-based projects
The change from useTeamTasks to useOrganizationProjects in AddTaskModal.tsx is safe. The useOrganizationProjects hook provides organization-wide project data through proper API calls and state management, while maintaining backward compatibility. The hook's implementation shows it handles project data at the organization level without breaking existing functionality.
Key findings:
useOrganizationProjectsproperly manages organization projects through tenant-aware API calls- No direct dependencies on team-specific functionality in the implementation
- The change aligns with the codebase's architecture where projects are managed at organization level
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining team-specific project references
rg "team.*project" --type typescript
# Check for any components still using useTeamTasks
rg "useTeamTasks" --type typescript
Length of output: 144
Script:
#!/bin/bash
# Check for any remaining team-specific project references
rg "team.*project"
# Check for any components still using useTeamTasks
rg "useTeamTasks"
# Check the implementation of useOrganizationProjects to understand the change
ast-grep --pattern 'export function useOrganizationProjects() { $$$ }'
# Check if there are any other components using useOrganizationProjects
rg "useOrganizationProjects"
Length of output: 34727
apps/web/app/hooks/features/useTimesheet.ts (1)
170-173: Verify the timesheet structure change across the codebase
The change from item.timesheet.id to item.id suggests a structural modification in how timesheets are identified. This could be a breaking change if other components still expect the old nested structure.
apps/web/app/[locale]/timesheet/[memberId]/components/CompactTimesheetComponent.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx (2)
310-318: Enhance loading state UXGood addition of loading state, but consider these improvements:
- Disable all interactive elements during submission
- Show loading indicator on the entire modal
<form onSubmit={handleUpdateSubmit} className="flex flex-col w-full"> + <div className={clsxm("relative", loadingUpdateTimesheet && "pointer-events-none opacity-70")}> {/* ... existing form content ... */} + </div> </form>
163-164: Enhance accessibility for task information displayConsider adding these accessibility improvements:
- Add aria-label to task information section
- Add screen reader text for task status
<TaskNameInfoDisplay task={dataTimesheet.task} + aria-label={`Task: ${dataTimesheet.task?.title}`} className={clsxm('rounded-sm h-auto !px-[0.3312rem] py-[0.2875rem] shadow-[0px_0px_15px_0px_#e2e8f0] dark:shadow-transparent')} taskTitleClassName={clsxm('text-sm text-ellipsis overflow-hidden')} showSize={true} dash taskNumberClassName="text-sm" />Also applies to: 232-232
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx(7 hunks)
🔇 Additional comments (3)
apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx (3)
9-9: LGTM: Import changes enhance error handling and UX
The replacement of team-based hooks with organization-level hooks and addition of toast notifications improve the component's functionality.
Also applies to: 15-17
51-51: LGTM: Improved state initialization and data handling
Good improvements:
- Proper fallback for isBillable with nullish coalescing
- Consistent use of organizationProjects
- Proper initialization of selectedValues with project data
Also applies to: 56-56, 65-65
99-132: Improve error handling and modal behavior
While the toast notifications are a good addition, there are still some issues that need to be addressed:
- Raw error exposure in the error message
- Modal closes on both success and error states
The previous review comment's suggestions are still valid and should be implemented:
- Use a user-friendly error message
- Keep the modal open on error to allow retry
Innocent-Akim
added
feature
Ever Teams
Bug fix
Don't Merge
Description
Please include a summary of the changes and the related issue.
Type of Change
Checklist
Previous screenshots
Please add here videos or images of previous status
Current screenshots
Please add here videos or images of previous status
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores