[Feat]: Productivity project table view #3616
Conversation
- Create new ProductivityProjectTable component - Group activities by project with dedicated headers - Update activity report interfaces - Implement date-based sorting within project groups - Add visual progress indicators for time usage
- Group activities by both project and date - Combine activities for the same date - Show all unique members and applications per date - Fix type error in duration formatting
- Change totalDuration type to string - Fix string/number type conversions in duration calculations - Ensure consistent type handling throughout the component
- Add hierarchical structure with project, date, and activity levels - Show individual member activities with time and percentage - Add date summaries with total time and combined percentage - Improve visual hierarchy with subtle background colors
- Split into smaller, reusable components - Add proper TypeScript interfaces - Extract data processing logic - Improve code organization and maintainability - Add loading and empty states as separate components
- Split into modular components with proper file organization - Create separate files for types, utils, components, and states - Improve TypeScript types and interfaces - Enhance dark mode support and styling - Add proper component documentation
- Replace if/else and ternary operators with switch statements - Simplify APPS-URLS case to only fetch activity report - Standardize Promise.all usage for default case - Improve code consistency across all fetch operations
- Add conditional rendering based on groupBy type - Show ProductivityProjectTable for project grouping - Show ProductivityTable for date grouping - Improve skeleton loading UI with full-width progress bars - Fix group type handling in switch statement
|
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 12 minutes and 24 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. 📒 Files selected for processing (1)
WalkthroughThe changes update the dashboard’s productivity view. In the Changes
Possibly related PRs
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: 4
🧹 Nitpick comments (8)
apps/web/app/[locale]/dashboard/app-url/components/productivity-project/types.ts (2)
3-9: Consider adding validation constraints to the Employee interface.The interface could benefit from additional type safety:
- Consider making
fullNamea non-empty string- Add validation for image URL format
export interface Employee { id: string; - fullName: string; + fullName: NonEmptyString; // Create a type that ensures non-empty string user?: { - imageUrl?: string; + imageUrl?: URL | string; // Consider using URL type for better validation }; }
16-26: Consider adding validation for project names and dates.The
ProjectGroupsinterface could benefit from stronger typing for project names and dates:export interface ProjectGroups { - [projectName: string]: { - [date: string]: ProjectGroupData; + [projectName: NonEmptyString]: { + [date: ISODateString]: ProjectGroupData; // Create a type for ISO date strings }; }apps/web/app/[locale]/dashboard/app-url/components/productivity-project/states.tsx (2)
25-39: Extract magic number and consider pagination limits.The hardcoded value of 7 rows should be extracted to a constant and consider matching the actual pagination limit:
+const SKELETON_ROWS = 7; // Match this with your pagination limit {[...Array(7)].map((_, i) => (
44-51: Consider internationalizing the text content.The hardcoded text should be internationalized for better maintainability:
export const EmptyState: React.FC = () => ( <Card className="bg-white rounded-md border border-gray-100 dark:border-gray-800 dark:bg-dark--theme-light min-h-[600px] flex items-center justify-center w-full"> <div className="text-center text-gray-500 dark:text-gray-400"> - <p className="text-lg font-medium">No activity data available</p> - <p className="text-sm">Select a different date range or check back later</p> + <p className="text-lg font-medium">{t('no_activity_data')}</p> + <p className="text-sm">{t('select_different_date_range')}</p> </div> </Card> );apps/web/app/[locale]/dashboard/app-url/components/productivity-project/components.tsx (1)
54-56: Remove commented code.The commented code block should either be removed or uncommented if needed.
- {/* <TableCell colSpan={2} className="text-sm text-gray-500 dark:text-gray-400"> - Total for {format(new Date(date), 'MMM dd')} - </TableCell> */}apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1)
136-145: Simplify the switch statement.The switch statement can be simplified as the default case returns the same component as the 'date' case.
- {(() => { - switch (groupByType) { - case 'project': - return <ProductivityProjectTable data={activityReport} isLoading={loadingActivityReport} />; - case 'date': - return <ProductivityTable data={activityReport} isLoading={loadingActivityReport} />; - default: - return <ProductivityTable data={activityReport} isLoading={loadingActivityReport} />; - } - })()} + {groupByType === 'project' ? ( + <ProductivityProjectTable data={activityReport} isLoading={loadingActivityReport} /> + ) : ( + <ProductivityTable data={activityReport} isLoading={loadingActivityReport} /> + )}apps/web/app/hooks/features/useReportActivity.ts (2)
72-72: Consider adding JSDoc documentation for thetypesparameter.The introduction of the
typesparameter is a good pattern for managing different data fetching strategies. Consider adding JSDoc documentation to explain the purpose and impact of each type.+/** + * @param types - Determines the data fetching strategy: + * - 'TEAM-DASHBOARD': Fetches all reports in parallel + * - 'APPS-URLS': Fetches only activity report + */ export function useReportActivity({ types }: { types?: 'TEAM-DASHBOARD' | 'APPS-URLS' }) {Also applies to: 88-88, 91-91
243-254: Consider consistent error handling across switch cases.While both error handling approaches work, consider using a consistent pattern. You could either:
- Use try-catch blocks in both cases, or
- Use
.catch(console.error)consistentlyswitch (types) { case 'APPS-URLS': - fetchActivityReport(newProps).catch(console.error); + try { + await fetchActivityReport(newProps); + } catch (error) { + console.error('Failed to fetch activity report:', error); + } break; default: - Promise.all([ - fetchReportActivity(newProps), - fetchDailyReport(newProps), - fetchStatisticsCounts(newProps) - ]).catch(console.error); + try { + await Promise.all([ + fetchReportActivity(newProps), + fetchDailyReport(newProps), + fetchStatisticsCounts(newProps) + ]); + } catch (error) { + console.error('Failed to fetch reports:', error); + } break; }Also applies to: 273-284
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx(4 hunks)apps/web/app/[locale]/dashboard/app-url/components/ProductivityTable.tsx(2 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-project/components.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-project/index.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-project/states.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-project/types.ts(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-project/utils.ts(1 hunks)apps/web/app/hooks/features/useReportActivity.ts(5 hunks)apps/web/app/interfaces/activity/IActivityReport.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (9)
apps/web/app/interfaces/activity/IActivityReport.ts (1)
127-145: LGTM! Well-structured interfaces for project activity reporting.The new interfaces
IProjectActivityandIActivityReportByProjectare well-designed and properly documented, providing a clear structure for project-based activity reporting.apps/web/app/[locale]/dashboard/app-url/components/productivity-project/components.tsx (1)
8-93: LGTM! Well-structured and reusable components.The components are well-designed with proper type definitions, error handling, and consistent styling. Good use of composition and reusability.
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1)
37-37: LGTM! Good state management implementation.The state management for group by type is well-implemented with proper type safety and state updates.
Also applies to: 107-111
apps/web/app/[locale]/dashboard/app-url/components/ProductivityTable.tsx (1)
45-45: LGTM! Improved UI consistency.The changes to skeleton and progress bar widths improve UI consistency and space utilization.
Also applies to: 145-145
apps/web/app/hooks/features/useReportActivity.ts (5)
112-114: LGTM!The formatting changes improve readability while maintaining the same functionality.
120-127: LGTM!The consistent use of the nullish coalescing operator improves readability and maintains the same fallback behavior.
147-151: LGTM!The union type correctly includes the new
queryActivityReportfunction while maintaining type safety.
292-303: Apply consistent error handling here as well.This section has the same error handling inconsistency as noted in the previous comment.
307-329: LGTM!The return object is well-organized with logical grouping of related values.
apps/web/app/[locale]/dashboard/app-url/components/productivity-project/utils.ts
Show resolved
Hide resolved
apps/web/app/[locale]/dashboard/app-url/components/productivity-project/utils.ts
Outdated
Show resolved
Hide resolved
apps/web/app/[locale]/dashboard/app-url/components/productivity-project/index.tsx
Show resolved
Hide resolved
apps/web/app/[locale]/dashboard/app-url/components/productivity-project/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/web/app/[locale]/dashboard/app-url/components/productivity-project/utils.ts (2)
4-11:⚠️ Potential issueImprove type safety and error handling in formatDuration.
The function could benefit from better input validation and error handling.
-export const formatDuration = (seconds: string): string => { +export const formatDuration = (seconds: string | number): string => { + if (typeof seconds === 'string' && !/^\d+$/.test(seconds)) { + throw new Error('Invalid duration format'); + } const totalSeconds = parseInt(seconds); + if (isNaN(totalSeconds) || totalSeconds < 0) { + throw new Error('Invalid duration value'); + } const hours = Math.floor(totalSeconds / 3600); const minutes = Math.floor((totalSeconds % 3600) / 60); const remainingSeconds = totalSeconds % 60;
25-38:⚠️ Potential issueReplace 'any' types with proper interfaces.
The function uses
anytype which reduces type safety.+interface IEmployeeData { + projects?: Array<{ + activity: IActivityItem[]; + }>; + activity?: IActivityItem[]; + employee?: IEmployee; +} + +interface IActivityItem { + projectId?: string; + duration?: string; +} + +interface IEmployee { + id: string; +} + - dayData.employees.forEach((employeeData: any) => { + dayData.employees.forEach((employeeData: IEmployeeData) => { if (!employeeData) { console.warn('employeeData is undefined'); return; } const activities = employeeData.projects?.[0]?.activity || employeeData.activity || []; if (!Array.isArray(activities)) { console.warn('activities must be an array'); return; } - activities.forEach((activity: any) => { + activities.forEach((activity: IActivityItem) => {
🧹 Nitpick comments (6)
apps/web/app/[locale]/dashboard/app-url/components/productivity-project/states.tsx (2)
12-42: Consider making the number of skeleton rows configurable.The component looks good, but consider making the number of skeleton rows configurable via props for better reusability.
-export const LoadingSkeleton: React.FC = () => ( +interface LoadingSkeletonProps { + rowCount?: number; +} + +export const LoadingSkeleton: React.FC<LoadingSkeletonProps> = ({ rowCount = 7 }) => ( <Card className="bg-white rounded-md border border-gray-100 dark:border-gray-800 dark:bg-dark--theme-light min-h-[600px] w-full"> <Table> <TableHeader> <TableRow className="bg-gray-50 dark:bg-gray-800"> <TableHead className="font-semibold">Date</TableHead> <TableHead className="font-semibold">Member</TableHead> <TableHead className="font-semibold">Application</TableHead> <TableHead className="font-semibold">Time Spent</TableHead> <TableHead className="font-semibold">Percent used</TableHead> </TableRow> </TableHeader> <TableBody> - {[...Array(7)].map((_, i) => ( + {[...Array(rowCount)].map((_, i) => (
48-67: Add i18n support for better internationalization.The component looks good, but consider adding i18n support for the text content to support multiple languages.
+import { useTranslations } from 'next-intl'; + export const EmptyState: React.FC<EmptyStateProps> = ({ selectedDate }) => { + const t = useTranslations('productivity'); const formattedDate = selectedDate ? new Date(selectedDate).toLocaleDateString('en-US', { weekday: 'long', day: '2-digit', month: 'long', year: 'numeric' }) : ''; return ( <Card className="bg-white rounded-md border border-gray-100 dark:border-gray-800 dark:bg-dark--theme-light min-h-[600px] flex items-center justify-center w-full"> <div className="text-center text-gray-500 dark:text-gray-400"> - <p className="text-lg font-medium">No activity data available</p> + <p className="text-lg font-medium">{t('noActivityData')}</p> {selectedDate && ( - <p className="mb-2 text-sm">for {formattedDate}</p> + <p className="mb-2 text-sm">{t('forDate', { date: formattedDate })}</p> )} - <p className="text-sm">Select a different date range or check back later</p> + <p className="text-sm">{t('selectDifferentDate')}</p> </div> </Card> ); };apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1)
139-148: Consider using object literal for conditional rendering.The IIFE with switch statement could be simplified using an object literal for better readability and maintainability.
-{(() => { - switch (groupByType) { - case 'project': - return <ProductivityProjectTable data={activityReport} isLoading={loadingActivityReport} />; - case 'date': - return <ProductivityTable data={activityReport} isLoading={loadingActivityReport} />; - case 'employee': - return <ProductivityEmployeeTable data={activityReport} isLoading={loadingActivityReport} />; - } -})()} +{ + { + 'project': <ProductivityProjectTable data={activityReport} isLoading={loadingActivityReport} />, + 'date': <ProductivityTable data={activityReport} isLoading={loadingActivityReport} />, + 'employee': <ProductivityEmployeeTable data={activityReport} isLoading={loadingActivityReport} /> + }[groupByType] +}apps/web/app/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx (3)
228-228: Remove console.log statements.Development console.log statements should be removed before merging.
- console.log("New data received:", data); - console.log("Grouping data:", localData);Also applies to: 233-233
232-284: Consider extracting data transformation logic.The data grouping logic in the useMemo hook is complex and could be moved to a separate utility function for better maintainability and testability.
+const groupEmployeeData = (data: any[]): Map<string, EmployeeActivityGroup> => { + const employeeMap = new Map<string, EmployeeActivityGroup>(); + if (!Array.isArray(data)) { + console.warn('Invalid data format: array expected'); + return employeeMap; + } + // ... rest of the grouping logic ... + return employeeMap; +}; const groupedData = React.useMemo(() => { - console.log("Grouping data:", localData); - const employeeMap = new Map<string, EmployeeActivityGroup>(); - // ... rest of the current logic ... + return groupEmployeeData(localData); }, [localData]);
290-293: Consider adding a more descriptive empty state message.The empty state could provide more context about why no data is available and what actions the user can take.
- return <EmptyState selectedDate={selectedDate} />; + return ( + <EmptyState + selectedDate={selectedDate} + message="No employee productivity data available for the selected date range." + action="Try selecting a different date range or updating the filters." + /> + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx(5 hunks)apps/web/app/[locale]/dashboard/app-url/components/GroupBySelect.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-project/components.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-project/states.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-project/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/[locale]/dashboard/app-url/components/productivity-project/components.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
apps/web/app/[locale]/dashboard/app-url/components/productivity-project/utils.ts (1)
13-75: Add unit tests for the groupActivitiesByProjectAndDate function.The function has complex logic and error handling that should be covered by unit tests.
Create a new test file
utils.test.tswith the following test cases:import { groupActivitiesByProjectAndDate } from './utils'; import { IActivityReportGroupByDate } from '@/app/interfaces/activity/IActivityReport'; describe('groupActivitiesByProjectAndDate', () => { it('should handle empty input array', () => { const result = groupActivitiesByProjectAndDate([]); expect(result).toEqual({}); }); it('should handle invalid input', () => { const result = groupActivitiesByProjectAndDate(null as any); expect(result).toEqual({}); }); it('should group activities by project and date', () => { const input: IActivityReportGroupByDate[] = [{ date: '2024-02-14', employees: [{ employee: { id: '1', name: 'John' }, projects: [{ activity: [{ projectId: 'project1', duration: '3600' }] }] }] }]; const result = groupActivitiesByProjectAndDate(input); expect(result).toHaveProperty('project1'); expect(result.project1).toHaveProperty('2024-02-14'); expect(result.project1['2024-02-14'].totalDuration).toBe(3600); expect(result.project1['2024-02-14'].members.size).toBe(1); }); it('should handle missing project ID', () => { const input: IActivityReportGroupByDate[] = [{ date: '2024-02-14', employees: [{ employee: { id: '1', name: 'John' }, activity: [{ duration: '3600' }] }] }]; const result = groupActivitiesByProjectAndDate(input); expect(result).toHaveProperty('No project'); }); });apps/web/app/[locale]/dashboard/app-url/components/GroupBySelect.tsx (1)
22-22:❓ Verification inconclusive
Verify the impact of changing the select value from "person" to "employee".
The value change from "person" to "employee" could break existing code that relies on the "person" value. Please ensure all consumers of this component are updated accordingly.
🏁 Script executed:
#!/bin/bash # Search for any remaining usage of "person" value in group by logic rg -i "groupByType.*[\"']person[\"']" -g "*.{ts,tsx,js,jsx}"Length of output: 66
Manual Verification Required: Impact of Value Change in GroupBySelect
The updated select value from "person" to "employee" appears to have been applied, and our automated search using the
rgcommand did not return any references to a "groupBy" logic checking for the "person" value. However, the empty output might not be fully conclusive. Since this change constitutes a breaking change on a public API, please manually verify that no downstream consumers depend on the old "person" value. In particular, confirm that any grouping logic or associated configurations have been updated to work with "employee."
- Verify all usages of the GroupBySelect component across the codebase.
- Ensure that tests covering grouping functionality pass and that no logic explicitly relies on the old "person" API.
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (2)
38-38: LGTM! State management is properly implemented.The groupByType state is correctly initialized with a default value of 'date'.
49-52: LGTM! Handler properly updates state and triggers callback.The handleGroupTypeChange function correctly manages both local state and parent callback.
apps/web/app/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx (1)
113-123: LGTM! Proper use of React.memo for performance optimization.The EmployeeAvatar component is correctly memoized to prevent unnecessary re-renders.
apps/web/app/[locale]/dashboard/app-url/components/productivity-project/utils.ts
Show resolved
Hide resolved
...pp/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/app/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx (2)
21-45: Improve time calculation accuracy and parsing safety.The utility functions have potential issues:
formatDurationdrops seconds, which could lead to inaccurate time reportingcalculateTotalDurationusesparseIntwithout a radix parameterConsider these improvements:
const formatDuration = (seconds: number): string => { const hours = Math.floor(seconds / 3600); const minutes = Math.floor((seconds % 3600) / 60); - return `${hours.toString().padStart(2, '0')}:${minutes.toString().padStart(2, '0')}`; + const remainingSeconds = seconds % 60; + return `${hours.toString().padStart(2, '0')}:${minutes.toString().padStart(2, '0')}:${remainingSeconds.toString().padStart(2, '0')}`; }; const calculateTotalDuration = (activities: IActivityItem[]): number => { return activities.reduce((total, activity) => { - const duration = parseInt(activity.duration || '0'); + const duration = parseInt(activity.duration || '0', 10); return isNaN(duration) ? total : total + duration; }, 0); };
246-307: Optimize component performance and memory usage.The main component has several areas for optimization:
- Missing cleanup in useEffect
- Complex data transformation logic in a single memo
Consider these improvements:
- Add cleanup to useEffect:
React.useEffect(() => { setLocalData(data); + return () => { + setLocalData([]); + }; }, [data]);
- Split the data transformation logic:
const sortByDate = (data: any[]) => [...data].sort((a, b) => new Date(b.date).getTime() - new Date(a.date).getTime()); const createEmployeeGroup = (employee: IEmployee) => ({ employee, dateGroups: new Map() }); const processEmployeeActivities = (projects: any[]) => { const activities = projects?.flatMap((p: any) => p?.activity || []) || []; return { activities, totalDuration: calculateTotalDuration(activities) }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (1)
apps/web/app/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx (1)
68-71: Type the data prop properly to maintain type safety.The
dataprop is typed asany[]which loses type safety benefits. Consider using a proper interface for the data structure.
...pp/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx
Outdated
Show resolved
Hide resolved
...pp/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx
Show resolved
Hide resolved
Description
#3261
Please include a summary of the changes and the related issues.
Type of Change
Checklist
Previous screenshots
Please add here videos or images of the previous status
Current screenshots
Please add here videos or images of the current (new) status
Summary by CodeRabbit
New Features
Style