-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Refactor]: Optimize DeleteTimesheet and UpdateTimesheetStatus #3375
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request introduce an asynchronous POST handler for managing timesheet updates, enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning Rate limit exceeded@Innocent-Akim has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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/hooks/features/useTimesheet.tsOops! 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. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (7)
apps/web/app/stores/time-logs.ts (1)
19-22
: Consider consolidating timesheet operation states.The current implementation has separate atoms for different timesheet operations (
timesheetDeleteState
,timesheetUpdateStatus
,timesheetUpdateState
). This separation might lead to synchronization challenges when multiple operations are in progress.Consider consolidating these into a single atom with a more comprehensive state structure:
type TimesheetOperationState = { deleting: string[]; statusUpdates: UpdateTimesheetStatus[]; currentUpdate: TimesheetLog | null; isLoading: boolean; error: Error | null; }; export const timesheetOperationsState = atom<TimesheetOperationState>({ deleting: [], statusUpdates: [], currentUpdate: null, isLoading: false, error: null });This approach would:
- Centralize timesheet operation states
- Make it easier to track loading and error states
- Prevent potential race conditions between operations
- Simplify state management in components
apps/web/app/services/server/requests/timesheet.ts (1)
112-120
: Add JSDoc documentation for consistency.Consider adding JSDoc documentation to match the documentation style of other functions in this file (e.g., see ITimesheetProps documentation above).
Here's a suggested documentation:
+/** + * Creates a new timesheet entry + * @param params - Timesheet update parameters + * @param bearer_token - Authentication token + * @returns Promise<TimesheetLog> - Created timesheet entry + */ export function createTimesheetRequest(params: UpdateTimesheet, bearer_token: string) {apps/web/app/interfaces/timer/ITimerLog.ts (2)
73-73
: Consider aligning status types across interfacesWhile using
TimesheetStatus
type improves type safety, there's an inconsistency with theUpdateTimesheetStatus
interface which still uses string literals. Consider usingTimesheetStatus
consistently across both interfaces.
Documentation update needed for new timesheet interfaces
The codebase lacks specific documentation for the new timesheet interfaces. While there are some JSDoc comments in the interfaces directory, the new timesheet-related interfaces are not documented. Please:
- Add JSDoc comments to document the
TimesheetLog
,UpdateTimesheetStatus
, andUpdateTimesheet
interfaces- Consider adding usage examples in the web app's README.md or create a dedicated documentation file for timesheet features
- Document the relationship between these interfaces and their role in the new async POST handler
🔗 Analysis chain
Line range hint
1-158
: Verify documentation updatesThe interface changes look good and align with the PR objectives. However, since this is marked as a new feature in the PR description, please ensure that the documentation is updated to reflect these interface changes.
Let's check for documentation files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for relevant documentation files fd -e md -e mdx | grep -i "timesheet\|timer"Length of output: 46
Script:
#!/bin/bash # Let's try a broader search for documentation files fd -t f "README|CONTRIBUTING|docs|documentation" -i # Also search for any documentation references in the codebase rg -i "documentation" --type ts --type tsx --type js --type jsx -g "!node_modules" # Search for comments or JSDoc that might document these interfaces rg -i "@interface|/\*\*.*timesheet|/\*\*.*timer" --type tsLength of output: 806
Script:
#!/bin/bash # Let's check the content of the web app README since it's most relevant cat apps/web/README.md # Let's also check if there are any API documentation or type definitions in the web app rg -i "timesheet|timer" apps/web/README.md # Search for any TypeScript interface documentation in the interfaces directory rg -i "/\*\*" apps/web/app/interfaces/ -A 5Length of output: 2802
apps/web/lib/features/integrations/calendar/table-time-sheet.tsx (2)
494-497
: Consider simplifying the timesheet prop typeThe nested
timesheet.timesheet.id
access suggests a potentially redundant data structure. Consider flattening the TimesheetLog interface to avoid double nesting.
Line range hint
524-541
: Implement billable status functionalityThe billable status menu items are present but lack implementation for updating the billable status.
Would you like help implementing the billable status update functionality?
apps/web/app/hooks/features/useTimesheet.ts (1)
150-150
: Replace 'console.log' with appropriate feedback mechanismUsing
console.log
is not recommended in production code. Consider replacingconsole.log
with a proper logging mechanism or providing user feedback through the UI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
apps/web/app/api/timer/timesheet/time-log/route.ts
(1 hunks)apps/web/app/hooks/features/useTimesheet.ts
(6 hunks)apps/web/app/interfaces/timer/ITimerLog.ts
(4 hunks)apps/web/app/services/client/api/timer/timer-log.ts
(2 hunks)apps/web/app/services/server/requests/timesheet.ts
(2 hunks)apps/web/app/stores/time-logs.ts
(1 hunks)apps/web/lib/features/integrations/calendar/table-time-sheet.tsx
(3 hunks)
🔇 Additional comments (7)
apps/web/app/api/timer/timesheet/time-log/route.ts (2)
1-4
: LGTM! Clean and well-organized imports
The imports are properly structured and demonstrate good separation of concerns between authentication, interfaces, and request handling.
6-6
: LGTM! Proper Next.js API route handler signature
The handler follows Next.js API route conventions with correct async/await usage.
apps/web/app/services/server/requests/timesheet.ts (2)
4-4
: LGTM! Import changes are appropriate.
The addition of UpdateTimesheet
type import aligns with the new functionality being introduced.
112-120
: Verify endpoint usage consistency.
The function implementation looks good, following the established patterns. Let's verify that the endpoint '/timesheet/time-log' is used consistently across the codebase.
✅ Verification successful
Endpoint usage is consistent across the codebase
The verification shows that the /timesheet/time-log
endpoint is used consistently throughout the codebase, appearing in both server and client-side implementations with the same path structure. All usages follow similar patterns for GET, POST, and DELETE operations.
apps/web/app/services/server/requests/timesheet.ts
: Base implementationapps/web/app/services/client/api/timer/manual-time.ts
: Client-side manual time loggingapps/web/app/services/client/api/timer/timer-log.ts
: Client-side timer operationsapps/web/app/services/client/api/activity/time-logs.ts
: Activity reporting
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of the timesheet endpoint
# Expected: Should show all usages of the timesheet/time-log endpoint to verify consistency
# Search for timesheet endpoint usage patterns
echo "Checking timesheet endpoint usage patterns..."
rg -A 2 "timesheet/time-log" --type ts
# Search for potential alternate endpoint definitions
echo "Checking for potential alternate endpoint definitions..."
rg -A 2 "TIMESHEET.*ENDPOINT|TIMELOG.*ENDPOINT|TIME_LOG.*ENDPOINT" --type ts
Length of output: 2224
apps/web/app/interfaces/timer/ITimerLog.ts (1)
138-158
: Well-structured type definition using TypeScript utilities
The UpdateTimesheet
interface is well-designed:
- Good use of
Pick
utility type to ensure type safety - Clear separation between optional and required fields
- Proper reuse of existing types
apps/web/app/services/client/api/timer/timer-log.ts (1)
1-2
: LGTM: Import changes are appropriate
The new imports are correctly added to support the new functionality.
apps/web/app/hooks/features/useTimesheet.ts (1)
287-287
: Verify the definition of 'puTimesheetStatus'
The variable puTimesheetStatus
is included in the returned object, but it may not be correctly defined or might be a typo. Please verify that puTimesheetStatus
is properly defined and intended to be included.
Run the following script to search for the definition and usage of puTimesheetStatus
in the codebase:
✅ Verification successful
Let me search for the definition of timesheetUpdateStatus
atom to fully verify this.
Let me check the type definition of UpdateTimesheetStatus
to complete the verification.
Let me try searching for interface or type definition with a broader pattern.
Let me fix the regex syntax and try again.
puTimesheetStatus
is properly defined and used correctly
The variable is correctly defined and used in the codebase:
- It's defined as a Jotai atom of type
UpdateTimesheetStatus[]
instores/time-logs.ts
- The type
UpdateTimesheetStatus
is properly defined ininterfaces/timer/ITimerLog.ts
- It's used consistently across the timesheet-related hooks and components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for definitions and usages of 'puTimesheetStatus' in TypeScript files
rg 'puTimesheetStatus' --type ts -A 5 -B 5
Length of output: 3861
Script:
#!/bin/bash
# Search for the definition of timesheetUpdateStatus atom
rg "timesheetUpdateStatus.*=.*atom" --type ts -A 2 -B 2
Length of output: 450
Script:
#!/bin/bash
# Search for UpdateTimesheetStatus type definition
rg "type UpdateTimesheetStatus" --type ts -A 5 -B 1
Length of output: 53
Script:
#!/bin/bash
# Search for UpdateTimesheetStatus interface or type definition
rg "UpdateTimesheetStatus(\s+{|^interface|^type)" --type ts -A 5 -B 1
Length of output: 224
Script:
#!/bin/bash
# Search for UpdateTimesheetStatus interface or type definition
rg "UpdateTimesheetStatus" --type ts -A 5 -B 1
Length of output: 7107
apps/web/lib/features/integrations/calendar/table-time-sheet.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
apps/web/app/hooks/features/useTimesheet.ts (2)
164-191
: Enhance error handling and user feedback in updateTimesheetStatusThe function has been improved with better state management, but could benefit from these enhancements:
async ({ status, ids }: { status: TimesheetStatus; ids: ID[] | ID }) => { if (!user) return; const idsArray = Array.isArray(ids) ? ids : [ids]; + if (idsArray.length === 0) { + throw new Error("No timesheet IDs provided"); + } try { const response = await queryUpdateTimesheetStatus({ ids: idsArray, status }); + if (!Array.isArray(response.data)) { + throw new Error("Invalid response data"); + } const responseMap = new Map(response.data.map(item => [item.id, item])); setTimesheet(prevTimesheet => prevTimesheet.map(item => { const updatedItem = responseMap.get(item.timesheet.id); if (updatedItem) { return { ...item, timesheet: { ...item.timesheet, status: updatedItem.status } }; } return item; }) ); - console.log('Timesheet status updated successfully!'); + // TODO: Use a toast notification instead of console.log } catch (error) { - console.error('Error updating timesheet status:', error); + const errorMessage = error instanceof Error ? error.message : 'Unknown error'; + throw new Error(`Failed to update timesheet status: ${errorMessage}`); } },Changes:
- Added validation for empty idsArray
- Added response data type validation
- Improved error handling with proper error message propagation
- Suggested moving success message to a toast notification
248-256
: Consider optimizing state updates in deleteTaskTimesheetThe state update logic has been improved, but consider these enhancements:
-setTimesheet(prevTimesheet => - prevTimesheet.filter(item => !logIds.includes(item.timesheet.id)) -); +setTimesheet(prevTimesheet => { + // Optimize for large arrays by creating a Set + const logIdSet = new Set(logIds); + return prevTimesheet.filter(item => !logIdSet.has(item.timesheet.id)); +});This optimization uses a Set for O(1) lookups instead of Array.includes() which is O(n), improving performance when dealing with large arrays.
apps/web/lib/features/integrations/calendar/table-time-sheet.tsx (2)
224-224
: Consider keeping the selection count in the dialogThe removal of
${selectTimesheet.length}
from the dialog description makes it less informative for bulk deletions. Users should know how many items they're about to delete.-description={`This action is irreversible. All related data will be lost.`} +description={`This action is irreversible. All related data will be lost. (${selectTimesheet.length} ${selectTimesheet.length === 1 ? 'item' : 'items'})`}
Line range hint
499-521
: Improve error handling in StatusTask componentThe error handling implementation is incomplete and lacks user feedback.
Apply these improvements:
onClick={async () => { try { await updateTimesheetStatus({ status: status.label as TimesheetStatus, ids: [timesheet.timesheet.id] }); + // Add success notification } catch (error) { - console.error('Failed to update timesheet status:'); + console.error('Failed to update timesheet status:', error); + // Add error notification } }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
apps/web/app/hooks/features/useTimesheet.ts
(6 hunks)apps/web/app/interfaces/timer/ITimerLog.ts
(4 hunks)apps/web/app/services/client/api/timer/timer-log.ts
(2 hunks)apps/web/app/stores/time-logs.ts
(2 hunks)apps/web/lib/features/integrations/calendar/table-time-sheet.tsx
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/interfaces/timer/ITimerLog.ts
- apps/web/app/stores/time-logs.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/app/hooks/features/useTimesheet.ts
[error] 154-154: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🔇 Additional comments (9)
apps/web/app/services/client/api/timer/timer-log.ts (3)
1-2
: LGTM: Import changes are appropriate
The new imports (UpdateTimesheet
and post
) are correctly added to support the new timesheet creation functionality.
132-143
: 🛠️ Refactor suggestion
Enhance error handling and add documentation
While the basic error handling is good, consider these improvements:
+/**
+ * Creates a new timesheet log entry
+ * @param {UpdateTimesheet} data - The timesheet data to create
+ * @returns {Promise<TimesheetLog>} The created timesheet log
+ * @throws {Error} If required parameters are missing or API call fails
+ */
-export function createTimesheetFromApi(data: UpdateTimesheet) {
+export async function createTimesheetFromApi(data: UpdateTimesheet): Promise<TimesheetLog> {
const organizationId = getOrganizationIdCookie();
const tenantId = getTenantIdCookie();
if (!organizationId || !tenantId) {
throw new Error('Required parameters missing: organizationId and tenantId are required');
}
+
+ // Validate required fields in data
+ if (!data || !data.duration || !data.startedAt) {
+ throw new Error('Required timesheet data missing: duration and startedAt are required');
+ }
+
try {
- return post<TimesheetLog>('/timesheet/time-log', { ...data, organizationId }, { tenantId })
+ return await post<TimesheetLog>('/timesheet/time-log', { ...data, organizationId }, { tenantId });
} catch (error) {
- throw new Error('Failed to create timesheet log');
+ const message = error instanceof Error ? error.message : 'Unknown error';
+ throw new Error(`Failed to create timesheet log: ${message}`);
}
}
Changes include:
- Added JSDoc documentation
- Made the function explicitly async and added return type
- Added validation for required timesheet data fields
- Enhanced error message with original error details
- Added proper semicolon for consistency
Likely invalid or redundant comment.
139-139
: Verify endpoint path and response type handling
Let's ensure the endpoint path and response type handling are consistent across the codebase.
✅ Verification successful
Let me gather more information about the TimesheetLog type usage to ensure type consistency.
Endpoint and type usage are consistent
The verification shows that:
- The endpoint '/timesheet/time-log' is consistently used in both client and server implementations
- The TimesheetLog type is properly defined as an interface in
apps/web/app/interfaces/timer/ITimerLog.ts
and is consistently used across the codebase - The POST response type handling matches the interface definition and is used consistently in related API calls
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check endpoint path consistency and response type handling
# Check for other usages of the timesheet/time-log endpoint
echo "Checking endpoint usage patterns:"
rg -A 2 "'/timesheet/time-log'" ./apps/web/
# Check TimesheetLog type usage in POST responses
echo "Checking response type handling:"
ast-grep --pattern 'post<$_>("/timesheet/time-log"'
Length of output: 819
Script:
#!/bin/bash
# Check TimesheetLog type definition and usage
echo "Checking TimesheetLog type definition:"
rg -p "type TimesheetLog" ./apps/web/
echo "Checking TimesheetLog interface definition:"
rg -p "interface TimesheetLog" ./apps/web/
echo "Checking other usages of TimesheetLog type:"
rg "TimesheetLog" ./apps/web/
Length of output: 5496
apps/web/app/hooks/features/useTimesheet.ts (3)
6-6
: LGTM: Import and state initialization changes
The new imports and loading state are properly added to support the timesheet creation functionality.
Also applies to: 8-8, 107-107
285-287
: LGTM: Hook return value updates
The new return values are properly added to expose the timesheet creation functionality to consumers of this hook.
142-158
: 🛠️ Refactor suggestion
Enhance error handling and state management in createTimesheet
While the implementation is functional, there are several improvements that could be made:
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) {
- throw error;
- }
+ const response = await queryCreateTimesheet(timesheetParams);
+ if (!response.data) {
+ throw new Error("Invalid response data");
+ }
+ setTimesheet((prevTimesheet) => [
+ response.data,
+ ...(Array.isArray(prevTimesheet) ? prevTimesheet : [])
+ ]);
},
[queryCreateTimesheet, setTimesheet, user]
);
Changes:
- Removed unnecessary try-catch block that just rethrows
- Added response data validation
- Added type checking for prevTimesheet
Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 154-154: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
apps/web/lib/features/integrations/calendar/table-time-sheet.tsx (3)
155-155
: LGTM: Hook usage is properly updated
The addition of updateTimesheetStatus
to the destructured hooks aligns with the PR's optimization objectives.
476-478
:
Add onClick handler for Delete action
The Delete menu item is missing the onClick handler to trigger the deletion dialog.
Apply this improvement:
-<DropdownMenuItem className="text-red-600 hover:!text-red-600 cursor-pointer">
+<DropdownMenuItem
+ className="text-red-600 hover:!text-red-600 cursor-pointer"
+ onClick={() => setIsDialogOpen(true)}
+>
{t('common.DELETE')}
</DropdownMenuItem>
199-207
:
Add error handling and state management
The async status update implementation needs improvements:
- Error handling is missing
- No user feedback on success/failure
- State management after successful update is missing
Consider applying this improvement:
const handleButtonClick = async (action: StatusAction) => {
switch (action) {
case 'Approved':
if (selectTimesheet.length > 0) {
+ try {
await updateTimesheetStatus({
status: 'APPROVED',
ids: selectTimesheet
- })
+ });
+ setSelectTimesheet([]); // Reset selection
+ // Add success notification
+ } catch (error) {
+ console.error('Failed to update timesheet status:', error);
+ // Add error notification
+ }
}
break;
Likely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/web/app/hooks/features/useTimesheet.ts (1)
164-189
: Consider these improvements to updateTimesheetStatusWhile the implementation is solid, here are some suggested improvements:
- Add error handling for unauthenticated user (similar to other functions)
- Add type annotation for response data
- Consider removing or replacing console.log with proper logging
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 { - const response = await queryUpdateTimesheetStatus({ ids: idsArray, status }); + const response: { data: Array<{ id: string; status: TimesheetStatus }> } = + await queryUpdateTimesheetStatus({ ids: idsArray, status }); const responseMap = new Map(response.data.map(item => [item.id, item])); setTimesheet(prevTimesheet => prevTimesheet.map(item => { const updatedItem = responseMap.get(item.timesheet.id); if (updatedItem) { return { ...item, timesheet: { ...item.timesheet, status: updatedItem.status } }; } return item; }) ); - console.log('Timesheet status updated successfully!'); + return response.data; } catch (error) { console.error('Error updating timesheet status:', error); + throw error; } }, [queryUpdateTimesheetStatus, setTimesheet, user] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/app/hooks/features/useTimesheet.ts
(6 hunks)
🔇 Additional comments (3)
apps/web/app/hooks/features/useTimesheet.ts (3)
6-6
: LGTM: Import statement follows established patterns
The new import for createTimesheetFromApi
is correctly placed with other API imports and follows the established naming convention.
248-251
: LGTM: Efficient state update implementation
The state update implementation is correct and efficient:
- Uses functional update pattern for state management
- Correctly filters out deleted items
- Maintains immutability
142-158
:
Enhance error handling in createTimesheet function
The error handling needs improvement:
- The error is only logged but not propagated
- The function's return type and value are undefined
- Success/failure state is not communicated to the caller
Apply this diff to improve 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 || [])
]);
+ return response.data;
} catch (error) {
console.error('Error:', error);
+ throw error;
}
},
[queryCreateTimesheet, setTimesheet, user]
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
apps/web/app/hooks/features/useTimesheet.ts (3)
164-191
: Enhance error handling and add return valueThe implementation is good but could be improved with better error handling and success tracking.
async ({ status, ids }: { status: TimesheetStatus; ids: ID[] | ID }) => { if (!user) return; const idsArray = Array.isArray(ids) ? ids : [ids]; try { const response = await queryUpdateTimesheetStatus({ ids: idsArray, status }); const responseMap = new Map(response.data.map(item => [item.id, item])); setTimesheet(prevTimesheet => prevTimesheet.map(item => { const updatedItem = responseMap.get(item.timesheet.id); if (updatedItem) { return { ...item, timesheet: { ...item.timesheet, status: updatedItem.status } }; } return item; }) ); console.log('Timesheet status updated successfully!'); + return true; } catch (error) { - console.error('Error updating timesheet status:', error); + const errorMessage = error instanceof Error ? error.message : 'Unknown error occurred'; + console.error(`Error updating timesheet status: ${errorMessage}`); + return false; } },
234-247
: Add success logging and return valueThe error handling is good, but let's add success logging and a return value for consistency.
await queryDeleteTimesheet({ organizationId: user.employee.organizationId, tenantId: user.tenantId ?? "", logIds }); setTimesheet(prevTimesheet => prevTimesheet.filter(item => !logIds.includes(item.timesheet.id)) ); +console.log('Timesheets deleted successfully:', logIds); +return true; } catch (error) { console.error('Failed to delete timesheets:', error); throw error; }
Line range hint
1-280
: Consider splitting the hook for better maintainabilityThe hook is handling multiple responsibilities (CRUD operations, status management, grouping logic). Consider splitting it into smaller, more focused hooks:
useTimesheetOperations
for CRUD operationsuseTimesheetStatus
for status managementuseTimesheetGrouping
for grouping logicThis would improve maintainability and make the code easier to test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/app/hooks/features/useTimesheet.ts
(6 hunks)
🔇 Additional comments (4)
apps/web/app/hooks/features/useTimesheet.ts (4)
6-6
: LGTM: Import changes are well-organized
The new imports are properly structured and align with the added functionality.
Also applies to: 8-8
263-263
: LGTM: Proper dependency management
Good addition of timesheetGroupByDays
to the dependency array to ensure proper re-fetching when grouping changes.
142-158
: 🛠️ Refactor suggestion
Enhance error handling and logging in createTimesheet
While the implementation is good, there are a few improvements needed:
const createTimesheet = useCallback(
async ({ ...timesheetParams }: UpdateTimesheet) => {
if (!user) {
throw new Error("User not authenticated");
}
try {
const response = await queryCreateTimesheet(timesheetParams);
+ console.log('Timesheet created successfully:', response.data);
setTimesheet((prevTimesheet) => [
response.data,
...(prevTimesheet || [])
]);
+ return response.data;
} catch (error) {
console.error('Error:', error);
+ throw error;
}
},
[queryCreateTimesheet, setTimesheet, user]
);
276-278
: Verify the usage of new return values
Let's verify that the new return values are properly used in the consuming components.
✅ Verification successful
New return values are properly integrated in the codebase
The verification shows that all new return values are properly used within the codebase:
createTimesheet
is defined inuseTimesheet.ts
and used in API-related operationsloadingCreateTimesheet
is correctly derived from theuseQuery
hook and exposedpuTimesheetStatus
is properly integrated with the atom state management and filter options
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using the new return values
echo "Searching for createTimesheet usage..."
rg "createTimesheet" --type ts --type tsx
echo "Searching for loadingCreateTimesheet usage..."
rg "loadingCreateTimesheet" --type ts --type tsx
echo "Searching for puTimesheetStatus usage..."
rg "puTimesheetStatus" --type ts --type tsx
Length of output: 497
Script:
#!/bin/bash
# Search for components using the new return values with correct file type flags
echo "Searching for createTimesheet usage..."
rg "createTimesheet" -t typescript -t tsx
echo "Searching for loadingCreateTimesheet usage..."
rg "loadingCreateTimesheet" -t typescript -t tsx
echo "Searching for puTimesheetStatus usage..."
rg "puTimesheetStatus" -t typescript -t tsx
Length of output: 518
Script:
#!/bin/bash
# Search for components using the new return values without file type flags
echo "Searching for createTimesheet usage..."
rg "createTimesheet"
echo "Searching for loadingCreateTimesheet usage..."
rg "loadingCreateTimesheet"
echo "Searching for puTimesheetStatus usage..."
rg "puTimesheetStatus"
Length of output: 2073
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (7)
apps/web/app/services/server/requests/timesheet.ts (2)
112-120
: Consider adding input validation and sanitizationWhile TypeScript provides type safety, consider adding runtime validation for the request payload:
- Validate required fields before making the request
- Sanitize input data to prevent potential injection attacks
- Consider picking only the necessary fields instead of spreading the entire params object
Example improvement:
export function createTimesheetRequest(params: UpdateTimesheet, bearer_token: string) { + const { id, tenantId, organizationId, duration, ...timeLogData } = params; + if (!tenantId || !organizationId) { + throw new Error('Missing required fields: tenantId and organizationId are required'); + } return serverFetch<TimesheetLog>({ path: '/timesheet/time-log', method: 'POST', - body: { ...params }, + body: { organizationId, tenantId, ...timeLogData }, bearer_token, tenantId: params.tenantId }) }
110-130
: Add JSDoc documentation for new functionsConsider adding JSDoc comments for the new functions to maintain consistency with existing documentation (see
ITimesheetProps
documentation above). Include:
- Function description
- Parameter descriptions
- Return type description
- Example usage if applicable
Example:
/** * Creates a new timesheet entry * @param params - Timesheet creation parameters * @param bearer_token - Authentication token * @returns Promise resolving to the created TimesheetLog */ export function createTimesheetRequest(params: UpdateTimesheet, bearer_token: string)apps/web/app/hooks/features/useTimesheet.ts (5)
6-6
: Consider splitting long import line for better readabilityThe import line is quite long and could be split for better maintainability.
-import { deleteTaskTimesheetLogsApi, getTaskTimesheetLogsApi, updateStatusTimesheetFromApi, createTimesheetFromApi, updateTimesheetFromAPi } from '@/app/services/client/api/timer/timer-log'; +import { + deleteTaskTimesheetLogsApi, + getTaskTimesheetLogsApi, + updateStatusTimesheetFromApi, + createTimesheetFromApi, + updateTimesheetFromAPi +} from '@/app/services/client/api/timer/timer-log';
136-152
: Add return type and consider error propagationThe function implementation looks good, but could benefit from explicit typing and consistent error handling.
- const createTimesheet = useCallback( + const createTimesheet = useCallback<(params: UpdateTimesheet) => Promise<void>>( 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); + throw error; } }, [queryCreateTimesheet, setTimesheet, user] );
167-194
: Consider removing console.log and returning success statusThe implementation is good, but success feedback should be handled by the caller rather than logged to console.
const updateTimesheetStatus = useCallback( - async ({ status, ids }: { status: TimesheetStatus; ids: ID[] | ID }) => { + async ({ status, ids }: { status: TimesheetStatus; ids: ID[] | ID }): Promise<boolean> => { if (!user) return; const idsArray = Array.isArray(ids) ? ids : [ids]; try { const response = await queryUpdateTimesheetStatus({ ids: idsArray, status }); const responseMap = new Map(response.data.map(item => [item.id, item])); setTimesheet(prevTimesheet => prevTimesheet.map(item => { const updatedItem = responseMap.get(item.timesheet.id); if (updatedItem) { return { ...item, timesheet: { ...item.timesheet, status: updatedItem.status } }; } return item; }) ); - console.log('Timesheet status updated successfully!'); + return true; } catch (error) { console.error('Error updating timesheet status:', error); + return false; } }, [queryUpdateTimesheetStatus, setTimesheet, user] );
237-245
: Remove unnecessary whitespace and improve error messageThe implementation is good, but could use some minor cleanup.
await queryDeleteTimesheet({ organizationId: user.employee.organizationId, tenantId: user.tenantId ?? "", logIds }); setTimesheet(prevTimesheet => prevTimesheet.filter(item => !logIds.includes(item.timesheet.id)) - ); - + ); } catch (error) { - console.error('Failed to delete timesheets:', error); + console.error(`Failed to delete ${logIds.length} timesheet(s):`, error); throw error; }
278-284
: Add explicit return type for the hookConsider adding an explicit return type interface for better type safety and documentation.
interface UseTimesheetReturn { loadingTimesheet: boolean; timesheet: GroupedTimesheet[]; getTaskTimesheet: (params: TimesheetParams) => void; loadingDeleteTimesheet: boolean; deleteTaskTimesheet: () => Promise<void>; getStatusTimesheet: (items?: TimesheetLog[]) => Record<TimesheetStatus, TimesheetLog[]>; timesheetGroupByDays: string; statusTimesheet: Record<TimesheetStatus, TimesheetLog[]>; updateTimesheetStatus: (params: { status: TimesheetStatus; ids: ID[] | ID }) => Promise<boolean>; loadingUpdateTimesheetStatus: boolean; puTimesheetStatus: any; // TODO: Add proper type createTimesheet: (params: UpdateTimesheet) => Promise<void>; loadingCreateTimesheet: boolean; updateTimesheet: (params: UpdateTimesheet) => Promise<void>; loadingUpdateTimesheet: boolean; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
apps/web/app/api/timer/timesheet/time-log/[id]/route.ts
(1 hunks)apps/web/app/hooks/features/useTimesheet.ts
(6 hunks)apps/web/app/interfaces/timer/ITimerLog.ts
(4 hunks)apps/web/app/services/client/api/timer/timer-log.ts
(2 hunks)apps/web/app/services/server/requests/timesheet.ts
(2 hunks)apps/web/lib/features/integrations/calendar/table-time-sheet.tsx
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/interfaces/timer/ITimerLog.ts
- apps/web/lib/features/integrations/calendar/table-time-sheet.tsx
🔇 Additional comments (6)
apps/web/app/api/timer/timesheet/time-log/[id]/route.ts (1)
1-4
: LGTM! Imports are clean and relevant.
All imports are being used and are appropriate for the functionality.
apps/web/app/services/server/requests/timesheet.ts (2)
4-4
: LGTM: Import changes are well-organized
The addition of UpdateTimesheet
type to the existing imports is appropriate and maintains code organization.
122-130
: Verify timesheet existence before update
The function should handle cases where the timesheet with the given ID doesn't exist. Consider:
- Adding error handling for 404 responses
- Verifying the timesheet exists before attempting update
Let's check how timesheet existence is verified in the codebase:
apps/web/app/services/client/api/timer/timer-log.ts (3)
1-1
: LGTM: Import changes are appropriate
The addition of UpdateTimesheet
type is consistent with the new functionality being added.
132-143
: 🛠️ Refactor suggestion
Enhance error handling and add documentation
The error handling could be improved for better debugging:
+/**
+ * Creates a new timesheet log entry
+ * @param data - The timesheet data to be created
+ * @returns Promise<TimesheetLog>
+ * @throws Error if organizationId or tenantId are missing
+ * @throws Error if the API request fails
+ */
export function createTimesheetFromApi(data: UpdateTimesheet) {
const organizationId = getOrganizationIdCookie();
const tenantId = getTenantIdCookie();
if (!organizationId || !tenantId) {
throw new Error('Required parameters missing: organizationId and tenantId are required');
}
try {
return post<TimesheetLog>('/timesheet/time-log', { ...data, organizationId }, { tenantId })
} catch (error) {
- throw new Error('Failed to create timesheet log');
+ throw new Error(`Failed to create timesheet log: ${error instanceof Error ? error.message : 'Unknown error'}`);
}
}
132-157
: Verify API endpoint configuration
Let's verify that the API endpoints for creating and updating timesheet logs are properly configured.
✅ Verification successful
Let me gather more information about the API endpoint configuration by checking the server-side implementation.
API endpoints are properly configured
The verification shows that both endpoints are properly configured and consistent with the codebase:
- The
/timesheet/time-log
POST endpoint for creating timesheets is defined inapps/web/app/services/server/requests/timesheet.ts
- The timesheet interfaces and types are properly defined in
apps/web/app/interfaces/timer/ITimerLog.ts
- The error handling pattern is consistent with other timesheet-related operations
- The endpoints follow the same pattern as other timesheet endpoints (timer/start, timer/stop, etc.)
- Required parameters (organizationId, tenantId) are handled consistently
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify API endpoint configuration for timesheet operations
# Check for route definitions
echo "Checking route definitions for timesheet endpoints..."
rg -A 5 "'/timesheet/time-log'" ./apps/web/
# Check for corresponding backend handlers
echo "Checking backend handlers..."
rg -A 5 "timesheet.*controller" ./apps/web/
# Check for consistent error handling patterns across timesheet-related functions
echo "Checking error handling patterns..."
ast-grep --pattern 'try {
$$$
return (post|put)<TimesheetLog>($$$)
$$$
} catch (error) {
$$$
}'
Length of output: 1405
Script:
#!/bin/bash
# Check for timesheet-related route configurations and controllers
echo "Checking for timesheet route configurations..."
rg -A 5 "timesheet.*router" ./apps/web/
# Check for timesheet-related API handlers or services
echo "Checking for timesheet API handlers..."
rg -A 5 "class.*TimesheetController" ./apps/web/
# Check for any other timesheet-related endpoints
echo "Checking for other timesheet endpoints..."
rg -A 5 "'/timesheet/.*'" ./apps/web/
# Check for timesheet-related types and interfaces
echo "Checking for timesheet types..."
rg -A 5 "interface.*Timesheet" ./apps/web/
Length of output: 11076
… for better readability
60ab188
to
474f18b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (7)
apps/web/app/services/server/requests/timesheet.ts (3)
Line range hint
71-79
: Fix malformed URL in getTaskTimesheetRequestThere's a syntax error in the URL construction. The
activityLevel?
appears to be incorrectly placed, resulting in a malformed URL:`/timesheet/time-log?activityLevel?${queries.toString()}`
Apply this fix:
- path: `/timesheet/time-log?activityLevel?${queries.toString()}`, + path: `/timesheet/time-log?${queries.toString()}`,
112-120
: Add JSDoc documentation for createTimesheetRequestThe function implementation looks good, but adding documentation would improve maintainability.
Add JSDoc documentation:
+/** + * Creates a new timesheet entry + * @param params - Timesheet creation parameters + * @param bearer_token - Authentication token + * @returns Promise<TimesheetLog> - Created timesheet entry + */ export function createTimesheetRequest(params: UpdateTimesheet, bearer_token: string) {
122-130
: Add JSDoc documentation for updateTimesheetRequestThe function implementation looks good, but adding documentation would improve maintainability.
Add JSDoc documentation:
+/** + * Updates an existing timesheet entry + * @param params - Timesheet update parameters including the timesheet ID + * @param bearer_token - Authentication token + * @returns Promise<TimesheetLog> - Updated timesheet entry + */ export function updateTimesheetRequest(params: UpdateTimesheet, bearer_token: string) {apps/web/lib/features/integrations/calendar/table-time-sheet.tsx (1)
231-231
: Consider including the count of items being deletedThe generic description loses context about the number of items being deleted, which could be important information for the user.
Consider:
-description={`This action is irreversible. All related data will be lost.`} +description={`This action is irreversible. ${selectTimesheet.length} item(s) and all related data will be lost.`}apps/web/app/hooks/features/useTimesheet.ts (3)
6-6
: Correct the inconsistent casing in 'updateTimesheetFromAPi'The import
updateTimesheetFromAPi
has inconsistent casing in 'APi'. Consider renaming it toupdateTimesheetFromApi
for consistency with naming conventions.Apply this diff to fix the casing:
-import { deleteTaskTimesheetLogsApi, getTaskTimesheetLogsApi, updateStatusTimesheetFromApi, createTimesheetFromApi, updateTimesheetFromAPi } from '@/app/services/client/api/timer/timer-log'; +import { deleteTaskTimesheetLogsApi, getTaskTimesheetLogsApi, updateStatusTimesheetFromApi, createTimesheetFromApi, updateTimesheetFromApi } from '@/app/services/client/api/timer/timer-log';
101-101
: Ensure consistent casing in 'updateTimesheetFromAPi'The variable
updateTimesheetFromAPi
used inuseQuery
has inconsistent casing. Rename it toupdateTimesheetFromApi
to match the corrected import.Apply this diff:
const { loading: loadingUpdateTimesheet, queryCall: queryUpdateTimesheet } = useQuery( - updateTimesheetFromAPi + updateTimesheetFromApi );
Line range hint
211-217
: Simplify the 'isTimesheetStatus' functionThe
isTimesheetStatus
function can be simplified by removing the unnecessaryObject.values
call sincetimesheetStatusValues
is already an array.Apply this diff:
function isTimesheetStatus(status: unknown): status is TimesheetStatus { const timesheetStatusValues: TimesheetStatus[] = [ "DRAFT", "PENDING", "IN REVIEW", "DENIED", "APPROVED" ]; - return Object.values(timesheetStatusValues).includes(status as TimesheetStatus); + return timesheetStatusValues.includes(status as TimesheetStatus); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
apps/web/app/api/timer/timesheet/time-log/[id]/route.ts
(1 hunks)apps/web/app/api/timer/timesheet/time-log/route.ts
(1 hunks)apps/web/app/hooks/features/useTimesheet.ts
(6 hunks)apps/web/app/interfaces/timer/ITimerLog.ts
(4 hunks)apps/web/app/services/client/api/timer/timer-log.ts
(2 hunks)apps/web/app/services/server/requests/timesheet.ts
(2 hunks)apps/web/app/stores/time-logs.ts
(2 hunks)apps/web/lib/features/integrations/calendar/table-time-sheet.tsx
(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/app/api/timer/timesheet/time-log/[id]/route.ts
- apps/web/app/api/timer/timesheet/time-log/route.ts
- apps/web/app/interfaces/timer/ITimerLog.ts
- apps/web/app/stores/time-logs.ts
🔇 Additional comments (10)
apps/web/app/services/server/requests/timesheet.ts (2)
4-4
: LGTM: Import statement is properly structured
The import statement correctly includes the new UpdateTimesheet
type needed for the new functions.
Line range hint 81-90
: Verify DELETE endpoint's handling of multiple IDs
The current implementation joins all IDs with commas, which might hit URL length limits for bulk deletions. We should verify the API's limitations.
apps/web/app/services/client/api/timer/timer-log.ts (4)
1-2
: LGTM: Import statements are correctly updated
The new imports UpdateTimesheet
and post
are appropriately added to support the new timesheet management functions.
132-143
: 🛠️ Refactor suggestion
Enhance error handling and add documentation
The function needs the following improvements:
- Add JSDoc documentation
- Validate the
data
parameter - Include the original error message in the catch block
Apply this diff:
+/**
+ * Creates a new timesheet log entry
+ * @param data - The timesheet data to be created
+ * @returns Promise<TimesheetLog>
+ * @throws Error if organizationId or tenantId are missing
+ * @throws Error if the API request fails
+ */
export function createTimesheetFromApi(data: UpdateTimesheet) {
+ if (!data) {
+ throw new Error('Required parameter missing: data is required');
+ }
const organizationId = getOrganizationIdCookie();
const tenantId = getTenantIdCookie();
if (!organizationId || !tenantId) {
throw new Error('Required parameters missing: organizationId and tenantId are required');
}
try {
return post<TimesheetLog>('/timesheet/time-log', { ...data, organizationId }, { tenantId })
} catch (error) {
- throw new Error('Failed to create timesheet log');
+ throw new Error(`Failed to create timesheet log: ${error instanceof Error ? error.message : 'Unknown error'}`);
}
}
145-157
:
Fix function naming and error handling issues
The function has several issues that need to be addressed:
- Inconsistent naming (
APi
vsApi
) - Incorrect error message (mentions "create" instead of "update")
- Missing validation for the required
id
parameter - Missing JSDoc documentation
These issues were previously identified in past review comments. Please refer to the existing review comment for the suggested fixes.
132-157
: Verify integration with consuming components
Let's ensure that components using these new functions properly handle errors and required parameters.
✅ Verification successful
Integration verified with proper error handling and parameter usage
The code inspection reveals that the new timesheet functions are properly integrated:
- The
useTimesheet
hook correctly implements both functions with proper error handling and required parameters - The functions are used within a proper try-catch block with error logging
- Required parameters (organizationId, tenantId) are properly handled through the authenticated user context
- The state management is properly implemented using Jotai atoms
- The component layer (TimesheetAction) properly handles the various states and actions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the usage and error handling of the new timesheet functions
# Check for components using these functions
echo "Checking usage of new timesheet functions..."
rg -t typescript -t tsx "createTimesheetFromApi|updateTimesheetFromAPi" ./apps/web/
# Check error handling in components
echo "Checking error handling implementation..."
ast-grep --pattern 'try {
$$$
(createTimesheetFromApi|updateTimesheetFromAPi)($$$)
$$$
} catch (error) {
$$$
}'
# Check for potential breaking changes
echo "Checking for potential breaking changes..."
rg -t typescript -t tsx "timesheet.*api" ./apps/web/
Length of output: 619
Script:
#!/bin/bash
# Let's try with correct file extensions and simpler patterns
# Check usage of the new functions
echo "Checking usage of timesheet functions..."
rg "createTimesheetFromApi|updateTimesheetFromAPi" --type ts --type tsx
# Check for timesheet-related API calls
echo "Checking timesheet API patterns..."
rg "timesheet.*api" --type ts --type tsx
# Look for files that might use these functions
echo "Looking for timesheet-related files..."
fd -e ts -e tsx timesheet
Length of output: 1295
Script:
#!/bin/bash
# Let's examine the content of relevant files
# Check the hook implementation
echo "Checking useTimesheet hook..."
cat "apps/web/app/hooks/features/useTimesheet.ts"
# Check task timesheet store
echo "Checking task timesheet store..."
cat "apps/web/app/stores/task-timesheet.ts"
# Check timesheet components for API usage
echo "Checking TimesheetAction component..."
cat "apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetAction.tsx"
# Check timesheet interfaces
echo "Checking timesheet interfaces..."
cat "apps/web/app/interfaces/ITaskTimesheet.ts"
Length of output: 15679
apps/web/lib/features/integrations/calendar/table-time-sheet.tsx (2)
489-491
: Missing onClick handler for Delete action
The Delete menu item lacks an onClick handler to trigger the deletion dialog.
207-215
:
Add error handling for the async operation
The async operation lacks error handling which could lead to unhandled promise rejections.
Consider implementing try-catch:
const handleButtonClick = async (action: StatusAction) => {
switch (action) {
case 'Approved':
if (selectTimesheet.length > 0) {
+ try {
await updateTimesheetStatus({
status: 'APPROVED',
ids: selectTimesheet
})
+ } catch (error) {
+ console.error('Failed to update timesheet status:', error);
+ // Add error notification/feedback to user
+ }
}
break;
Likely invalid or redundant comment.
apps/web/app/hooks/features/useTimesheet.ts (2)
147-149
: Enhance error handling in 'createTimesheet' function
In the createTimesheet
function, errors are only logged to the console without notifying the caller. Consider throwing the error after logging so that the calling code can handle it appropriately.
Apply this diff:
} catch (error) {
console.error('Error:', error);
+ throw error;
}
156-163
: Add error handling and fix potential state update issues in 'updateTimesheet' function
The updateTimesheet
function lacks error handling and might cause duplicates in the state. Include a try-catch block and update the state to prevent duplicates.
Apply this diff:
const updateTimesheet = useCallback(async ({ ...timesheet }: UpdateTimesheet) => {
- if (!user) return;
+ if (!user) {
+ throw new Error("User not authenticated");
+ }
+ try {
const response = await queryUpdateTimesheet(timesheet);
- setTimesheet(prevTimesheet => [
- response.data,
- ...prevTimesheet,
- ])
+ setTimesheet(prevTimesheet =>
+ prevTimesheet.map(item =>
+ item.timesheet.id === response.data.timesheet.id
+ ? response.data
+ : item
+ )
+ );
+ } catch (error) {
+ console.error('Error updating timesheet:', error);
+ throw error;
+ }
}, [queryUpdateTimesheet, setTimesheet, user]);
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
Release Notes
New Features
StatusTask
component to directly accept timesheet data, improving clarity and functionality.Bug Fixes
Documentation
These changes enhance user experience by streamlining timesheet operations and improving data handling within the application.