[Feat]: Apps Url Activity Grouping #3620
Conversation
WalkthroughThe changes update how the application manages and persists the grouping state. A new hook, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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]/dashboard/app-url/components/GroupBySelect.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. apps/web/app/[locale]/dashboard/app-url/[teamId]/page.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. apps/web/app/[locale]/dashboard/app-url/components/index.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.
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (14)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
apps/web/app/[locale]/dashboard/app-url/components/GroupBySelect.tsx (1)
24-24: Commented-out application option.The "application" option is no longer available to users, but it's still here as a comment. Confirm whether it's planned for future usage or consider removing it to keep the codebase clean.
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1)
122-122: Added left padding (pl-8).Visually aligns the content in the card’s header area. Verify that this spacing meets design specifications for smaller screens if responsiveness is a concern.
apps/web/app/hooks/features/useReportActivity.ts (1)
178-178: Helpful console warning.Alerting non-array data is crucial for debugging. Consider turning it into an error if this case should never happen in production.
apps/web/app/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx (5)
16-18: Consider displaying seconds in the duration.
Currently, theformatDurationfunction omits leftover seconds. If you want to showcase a finer time granularity, you could add another segment for seconds.
29-35: Using a fixed locale might limit i18n support.
Depending on requirements, you may consider using the user’s preferred locale to format dates rather than hardcoding'en-US'.
39-42: Specify the radix in parseInt for clarity.
To avoid unexpected behavior in some environments, it’s best practice to explicitly use base 10:- const duration = parseInt(activity.duration || '0'); + const duration = parseInt(activity.duration || '0', 10);
70-81: Consider storing numeric durations directly.
Definingdurationas a string requires parsing in multiple places. Switching to a numeric field could simplify logic and help avoid conversion errors.
258-330: Enhance type safety and error handling for data grouping.
- Casting
employeeDataandproject/activitytounknown | anyreduces type safety. Consider stricter type checks or type guards to avoid potential runtime errors.- You could unify console logs (warn/error) and decide whether to notify the user differently if grouping fails.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx(3 hunks)apps/web/app/[locale]/dashboard/app-url/components/GroupBySelect.tsx(2 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx(3 hunks)apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsx(3 hunks)apps/web/app/hooks/features/useReportActivity.ts(5 hunks)
🧰 Additional context used
🪛 GitHub Check: Cspell
apps/web/app/hooks/features/useReportActivity.ts
[warning] 318-318:
Unknown word (backoff)
🪛 GitHub Actions: Check Spelling and Typos with cspell
apps/web/app/hooks/features/useReportActivity.ts
[warning] 318-318: Unknown word (backoff)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (35)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsx (2)
18-20: Nicely introduced optionalgroupByTypeprop.Declaring
groupByTypeas optional is fine, but ensure you handle the undefined case gracefully if no value is passed.
30-43: Good integration withGroupBySelect.Passing
groupByTypetoGroupBySelectensures a consistent grouping state. This approach allows for a clear separation of concerns and is easy to maintain.apps/web/app/[locale]/dashboard/app-url/components/GroupBySelect.tsx (3)
8-8: OptionalgroupByTypeprop looks good.Keeping this prop optional enables broader flexibility if a default grouping is needed in certain contexts.
11-11: Updated function signature aligns well with the new interface.Invoking
groupByTypewithin the component is straightforward and should enhance clarity when reading the code.
13-13: Dynamic default selection.Replacing the hardcoded
"date"withgroupByTypeimproves consistency across the app, particularly when state is persisted or changed elsewhere.apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (2)
23-23: New import foruseLocalStorageState.Great choice for persistent state management, ensuring
groupByTypeis retained across sessions.
39-39: Replaced local state with localStorage-based state.This enhances user experience by persisting the group setting. The code is straightforward and readable.
apps/web/app/hooks/features/useReportActivity.ts (6)
70-70: ExtendedGroupByTypewith'application'.This addition is consistent with the rest of the code. Ensure your UI and usage logic handle this new option to avoid unimplemented paths.
148-148: Improved documentation.Providing clear comments about error handling and type safety makes the function’s purpose easier to understand.
160-160: Setting data to an empty array when user or props are missing.Safeguarding against null references or incomplete data is good practice. This fallback helps avoid runtime errors.
168-172: Response validation step is beneficial.By explicitly confirming
responseis an object, you reduce the chance of hard-to-diagnose errors downstream. This proactive check is commendable.
190-191: Re-throwing errors enables retry logic.This design pattern ensures upper layers can handle retries or alternative flows. Good for overall resilience.
296-299: Retry logic variables successfully introduced.Defining
maxRetriesandretryCountis a clear approach for controlling the number of fetch attempts.apps/web/app/[locale]/dashboard/app-url/components/productivity-employee/ProductivityEmployeeTable.tsx (22)
12-12: Looks good!
No concerns with the updated table header text.
22-26: All good here!
This logic for generating initials is concise and handles empty split segments gracefully.
45-50: No issues found.
This interface is straightforward and sufficiently descriptive.
52-59: Well-defined user interface!
It covers the typical user fields without any evident omissions.
63-68: All good.
The additional fields (isActive,isTrackingEnabled) look appropriate for tracking employee status.
82-90: Interface looks comprehensive.
Enumeration ofIDateGroupfields is straightforward and clear.
92-94: Props interface is fine.
Havingdata?andisLoading?with proper typing is clear and flexible.
97-133: Skeleton loading component is well-structured.
This block is well-suited to show repetitive placeholders.
136-141: Employee avatar implementation looks good.
UsinggetInitialsfor fallback is a nice touch.
146-147: No issues.
A simple placeholder for a project logo.
151-171: ActivityBar is clear and user-friendly.
The tooltip highlights usage percentages effectively.
175-184: Interface for ActivityRowProps is properly typed.
No potential pitfalls detected.
187-234: ActivityRow component logic is consistent.
Props are well-integrated with the existing formatting utilities.
239-251: MemoizedActivityRow usage is appropriate.
Memoization helps optimize rendering for repeated activity entries.
253-253: Matches the typed props.
ReturningActivityRowinline is concise and maintainable.
335-336: Empty state logic is straightforward.
Displays a friendly placeholder when there’s no data.
348-348: Table header rendering is clear.
No issues with mapping the constant headers.
355-357: Date sorting for descending order is correct.
Straightforward approach usingnew Date(b).getTime() - new Date(a).getTime().
365-367: Displays the employee’s full name effectively.
One-liner is easy to manage.
369-374: Total time calculation is integrated neatly.
Good use offormatDurationto show overall time.
380-380: Iteration over dates is well-structured.
Makes for a clean, nested rendering approach.
387-387: Simple boolean check forisFirstOfDay.
No issues found.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/app/[locale]/dashboard/app-url/components/productivity-application/ProductivityApplicationTable.tsx (1)
61-81: Effective data transformation logic.The data grouping logic effectively transforms the raw activity data into a structure organized by applications. However, there's a type issue with the employee property:
The
employee: anytype in the record could be replaced with a more specific type to improve type safety. Consider defining an interface for the employee structure or using the appropriate existing type.- employee: any + employee: IEmployeeWithUser // or the appropriate interface
📜 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(4 hunks)apps/web/app/[locale]/dashboard/app-url/components/GroupBySelect.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/index.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-application/ProductivityApplicationTable.tsx(1 hunks)apps/web/app/[locale]/dashboard/app-url/components/productivity-application/activity-bar.tsx(1 hunks)apps/web/app/hooks/features/useReportActivity.ts(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/[locale]/dashboard/app-url/components/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/[locale]/dashboard/app-url/components/GroupBySelect.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (14)
apps/web/app/[locale]/dashboard/app-url/components/productivity-application/activity-bar.tsx (1)
1-35: Well-structured visual component for displaying activity percentages.The ActivityBar component is well implemented with good separation of concerns. It correctly parses and rounds the percentage value, and includes helpful tooltips to provide additional context on hover.
Some noteworthy aspects:
- Good use of TypeScript interface for prop typing
- Appropriate rounding of percentage values
- Proper component naming with displayName for debugging
apps/web/app/hooks/features/useReportActivity.ts (4)
70-70: Enhanced grouping options with the new 'application' type.The GroupByType has been properly extended to include 'application' as a new value, which enables the application grouping functionality.
148-195: Improved error handling and response validation.The fetchReport function has been enhanced with proper response validation and error handling. The code now checks if the response is a valid object and logs warnings when the data is not an array.
I also notice the error is re-thrown after setting empty data, which enables the new retry logic to work properly.
268-268: Proper handling for the new 'application' group type.The function correctly maps 'application' grouping to 'date' when making the API call, ensuring compatibility with the backend while enabling new frontend functionality.
295-320: Improved data fetching with retry logic and exponential backoff.The implementation of retry logic with exponential backoff is well-executed. This makes the application more resilient to temporary network issues.
Note: The term "backoff" has been flagged in previous reviews. While the implementation is correct, you might want to consider fixing the spelling to avoid CI issues.
#!/bin/bash # Check if "backoff" is in the cspell dictionary to avoid CI failures grep -r "backoff" .cspell.json || echo "Consider adding 'backoff' to the cspell dictionary"apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (5)
21-22: Good organization of imports with the new components.The imports have been properly organized, adding the new
useLocalStorageStatehook and grouping the productivity table components.
38-38: Improved state persistence with local storage.Replacing React.useState with useLocalStorageState is a good enhancement that will persist the user's grouping preference across sessions. The key 'group-by-type' is descriptive and the default value 'date' is sensible.
117-117: Properly forwarding group state to child components.The groupByType state is now correctly passed to the DashboardHeader component, allowing it to reflect the current grouping state.
121-121: Minor UI enhancement with padding.Added left padding improves the layout spacing in the header section.
148-149: Support for the new application grouping view.The switch statement has been properly extended to handle the 'application' case, rendering the new ProductivityApplicationTable component when this grouping is selected.
apps/web/app/[locale]/dashboard/app-url/components/productivity-application/ProductivityApplicationTable.tsx (4)
1-16: Well-organized imports and component setup.The imports are properly organized and include all necessary UI components and type definitions.
17-59: Good handling of loading and empty states.The component correctly handles loading states with a skeleton UI and displays helpful messages when no data is available. The translations are properly integrated for internationalization support.
82-159: Well-structured table rendering with clear organization.The table implementation is well-structured, with clear separation between application headers and their related activities. The visual representation with avatars and progress bars provides a good user experience.
161-166: Useful helper function for duration formatting.The formatDuration function is well implemented, handling both string and number inputs and properly formatting the output as hours and minutes.
|
@Innocent-Akim can you fix merge conflicts? |
- Add application grouping type to activity reports - Improve data structure with proper TypeScript interfaces - Enhance error handling and data validation in ProductivityEmployeeTable - Add local storage persistence for group-by selection - Update UI components for better code organization and readability - Fix date formatting and duration calculations - Improve dark mode compatibility - Add proper type definitions for employee and activity data - Format code according to project style guidelines
- Add automatic data fetching on component mount - Enhance data validation in ProductivityEmployeeTable - Improve error handling and logging - Add fallbacks for missing data fields - Optimize useEffect dependencies in useReportActivity hook
- Add explicit unknown | any type annotations for dateGroup and project parameters - Enhance type safety in array mapping and filtering operations - Add type guards for project and activity objects - Maintain data processing functionality while improving type checking
- Implement application-based grouping in productivity table - Add proper project name display with fallback options - Fix TypeScript type issues in activity data handling - Clean up unused imports and improve code organization
aa33a54 to
80d86bb
Compare
Description
#3260
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
ProductivityApplicationTablecomponent displays productivity data based on activity reports.ActivityBarcomponent visually represents activity percentages with tooltips.Bug Fixes
Refactor