Skip to content
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

[Feat]: Add new API endpoint for daily time log report charts #3533

Merged
merged 5 commits into from
Jan 22, 2025

Conversation

Innocent-Akim
Copy link
Contributor

@Innocent-Akim Innocent-Akim commented Jan 21, 2025

Description

Please include a summary of the changes and the related issues.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

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

Based on the comprehensive summary, here are the updated release notes:

  • New Features

    • Added a new Apps & URLs page in the dashboard.
    • Introduced a Date Range Picker for selecting time ranges.
    • Created a new Export Dialog for confirming export operations.
    • Added a daily chart report API endpoint for time logs.
    • Implemented a new hook for managing daily time log reports.
  • Improvements

    • Enhanced sidebar navigation with dynamic team-specific links.
    • Updated task filtering and input components with refined styling.
    • Improved dark mode support across various dashboard components.
    • Adjusted layout and styling of the Team Stats Chart for better visual consistency.
  • Bug Fixes

    • Refined layout and responsiveness of dashboard components.
    • Corrected text color and alignment in team stats grid.

Copy link
Contributor

coderabbitai bot commented Jan 21, 2025

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

apps/web/app/services/client/api/timer/timer-log.ts

Oops! 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.

Walkthrough

This pull request introduces several enhancements to the web application's dashboard and timesheet functionality. Key changes include a new AppUrls component for team-specific URL management, a refined DateRangePicker for improved date selection, updates to team dashboard components, and a new API route for generating daily chart reports. The modifications focus on improving user interface, navigation, and data visualization across various dashboard sections, with particular attention to layout refinement and dark mode support.

Changes

File Path Change Summary
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx Added new AppUrls component with team-specific routing and authentication
apps/web/app/[locale]/dashboard/app-url/page.tsx Removed previous generic AppUrls component
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx New DateRangePicker component with advanced date selection functionality
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-chart.tsx Minor styling updates for chart layout and dark mode support
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-grid.tsx Added dark mode text color support
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/page.tsx Layout and breadcrumb refinements
apps/web/app/api/timesheet/time-log/report/daily-chart/route.ts New API route for generating daily chart reports
apps/web/app/services/server/requests/timesheet.ts Added interface and request function for daily chart data
apps/web/components/app-sidebar.tsx Updated sidebar navigation structure
apps/web/components/ui/export-dialog.tsx New ExportDialog component for export confirmation
apps/web/lib/features/task/task-filters.tsx Enhanced task filtering logic and layout
apps/web/lib/features/task/task-input.tsx Styling class name reordering
apps/web/app/hooks/features/useReportActivity.ts New hook for managing daily time log reports
apps/web/app/interfaces/timer/ITimerLog.ts Added new interfaces for timer values and daily logs
apps/web/app/services/client/api/timer/timer-log.ts New function for fetching daily chart report data
apps/web/app/stores/time-logs.ts New atom for managing daily log state

Possibly related PRs

Suggested Labels

feature, WEB, Ever Teams, Improvement

Suggested Reviewers

  • evereq
  • Cedric921

Poem

🐰 Hopping through code with glee,
Dashboard updates, smooth as can be!
Dates dance, charts sing bright,
Sidebar navigation takes flight!
A rabbit's code review delight! 🚀

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Innocent-Akim Innocent-Akim self-assigned this Jan 21, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🔭 Outside diff range comments (2)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-grid.tsx (1)

Line range hint 4-40: Consider replacing hardcoded statistics with dynamic data.

The statistics array is currently hardcoded with static values, which may not reflect real-time analytics data. Consider fetching this data from an API or props to ensure accurate time tracking analytics.

-const stats = [
-	{
-		title: "Members worked",
-		value: "17",
-		type: "number"
-	},
-	// ... other stats
-];
+interface TeamStats {
+	title: string;
+	value: string;
+	type: "number" | "time";
+	color?: string;
+	progress?: number;
+	progressColor?: string;
+}
+
+interface TeamStatsGridProps {
+	stats: TeamStats[];
+}
+
+export function TeamStatsGrid({ stats }: TeamStatsGridProps) {
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/page.tsx (1)

Line range hint 23-36: Add error handling for team not found.

Similar to the AppUrls component, this component should handle cases where activeTeam is undefined.

 function TeamDashboard() {
 	const { activeTeam, isTrackingEnabled } = useOrganizationTeams();
 	const router = useRouter();
 	const t = useTranslations();
 	const fullWidth = useAtomValue(fullWidthState);
 	const paramsUrl = useParams<{ locale: string }>();
 	const currentLocale = paramsUrl?.locale;
+
+	if (!activeTeam) {
+		return (
+			<div className="flex items-center justify-center h-screen">
+				<p className="text-gray-500">Team not found</p>
+			</div>
+		);
+	}

 	const breadcrumbPath = useMemo(
🧹 Nitpick comments (7)
apps/web/app/api/timesheet/time-log/report/daily-chart/route.ts (2)

29-31: LGTM! Authentication is properly implemented.

Consider enhancing the error message to provide more context about why the request was unauthorized.

- if (!user) return $res('Unauthorized');
+ if (!user) return $res('Unauthorized: Valid authentication credentials are required');

33-55: Enhance error handling and add request timeout.

The current error handling could be improved in several ways:

  1. Add request timeout
  2. Differentiate between different types of errors
  3. Provide more specific error messages when safe to do so
 try {
+    const controller = new AbortController();
+    const timeoutId = setTimeout(() => controller.abort(), 30000); // 30s timeout
+
     const { data } = await getTimeLogReportDailyChartRequest({
         activityLevel: {
             start: parseInt(activityLevelStart || '0'),
             end: parseInt(activityLevelEnd || '100')
         },
         organizationId,
         tenantId,
         startDate,
         endDate,
         timeZone: timeZone || 'Etc/UTC',
         groupBy: groupBy || 'date'
-    }, access_token);
+    }, access_token, { signal: controller.signal });
+
+    clearTimeout(timeoutId);
     return NextResponse.json(data);
 } catch (error) {
     console.error('Error fetching daily chart data:', error);
+    if (error.name === 'AbortError') {
+        return NextResponse.json(
+            { error: 'Request timeout exceeded' },
+            { status: 408 }
+        );
+    }
+    if (error.response?.status === 404) {
+        return NextResponse.json(
+            { error: 'No data found for the specified parameters' },
+            { status: 404 }
+        );
+    }
     return NextResponse.json(
         { error: 'Failed to fetch daily chart data' },
         { status: 500 }
     );
 }
apps/web/app/services/server/requests/timesheet.ts (1)

132-143: Add JSDoc documentation to the interface.

The interface should be documented to improve code maintainability.

+/**
+ * Parameters for daily chart report API requests
+ * @property activityLevel - Activity level range configuration
+ * @property activityLevel.start - Minimum activity level (0-100)
+ * @property activityLevel.end - Maximum activity level (0-100)
+ * @property organizationId - Organization identifier
+ * @property tenantId - Tenant identifier
+ * @property startDate - Start date for the report period
+ * @property endDate - End date for the report period
+ * @property timeZone - Optional timezone for date calculations (defaults to UTC)
+ * @property groupBy - Optional grouping parameter (defaults to 'date')
+ */
 export interface ITimeLogReportDailyChartProps {
apps/web/components/ui/export-dialog.tsx (1)

21-43: Consider adding error handling and loading states.

The dialog handles successful exports well but could be enhanced with:

  1. Error state handling for failed exports
  2. Loading state during download operation
 interface ExportDialogProps {
   isOpen: boolean;
   onClose: () => void;
   onDownload: () => void;
+  status: 'idle' | 'loading' | 'success' | 'error';
+  error?: string;
 }

 export function ExportDialog({
   isOpen,
   onClose,
   onDownload,
+  status,
+  error
 }: ExportDialogProps) {
   return (
     <Dialog open={isOpen} onOpenChange={onClose}>
       <DialogContent className="sm:max-w-md">
         <DialogHeader className="flex flex-col items-center gap-y-4">
-          <CheckCircle2 className="h-12 w-12 text-primary" />
+          {status === 'success' && <CheckCircle2 className="h-12 w-12 text-primary" />}
+          {status === 'error' && <XCircle className="h-12 w-12 text-destructive" />}
+          {status === 'loading' && <Loader2 className="h-12 w-12 text-primary animate-spin" />}
           <DialogTitle className="text-xl text-center">
-            Export Successful!
+            {status === 'success' && 'Export Successful!'}
+            {status === 'error' && 'Export Failed'}
+            {status === 'loading' && 'Exporting...'}
           </DialogTitle>
           <DialogDescription className="text-center">
-            Your export is complete. Click below to download your file.
+            {status === 'success' && 'Your export is complete. Click below to download your file.'}
+            {status === 'error' && error || 'An error occurred during export.'}
+            {status === 'loading' && 'Please wait while we prepare your file...'}
           </DialogDescription>
         </DialogHeader>
         <DialogFooter className="flex flex-col sm:flex-row gap-3 sm:gap-2">
           <Button variant="outline" onClick={onClose}>
             Cancel
           </Button>
-          <Button onClick={onDownload}>
+          <Button
+            onClick={onDownload}
+            disabled={status === 'loading' || status === 'error'}
+          >
             Download
           </Button>
         </DialogFooter>
       </DialogContent>
     </Dialog>
   );
 }
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1)

27-27: Use translation for 'Apps & URLs' string.

The breadcrumb title should use the translation system for consistency.

-{ title: 'Apps & URLs', href: `/${currentLocale}/dashboard/app-url` }
+{ title: t('pages.appUrls.BREADCRUMB'), href: `/${currentLocale}/dashboard/app-url` }
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/page.tsx (1)

32-32: Fix inconsistent capitalization.

The breadcrumb shows 'Team-Dashboard' while the displayName shows 'Team-dashboard'. Maintain consistent capitalization.

-			{ title: 'Team-Dashboard', href: `/${currentLocale}/dashboard/team-dashboard` }
+			{ title: 'Team Dashboard', href: `/${currentLocale}/dashboard/team-dashboard` }

 export default withAuthentication(TeamDashboard, {
-	displayName: 'Team-dashboard',
+	displayName: 'Team Dashboard',
 	showPageSkeleton: true
 });

Also applies to: 74-76

apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx (1)

24-27: Consider enhancing the component props interface.

The current props interface could be improved by:

  • Adding validation for the date range
  • Including accessibility props (aria-label, etc.)
  • Adding a prop for custom predefined ranges
 interface DateRangePickerProps {
   className?: string;
   onDateRangeChange?: (range: DateRange | undefined) => void;
+  ariaLabel?: string;
+  minDate?: Date;
+  maxDate?: Date;
+  customRanges?: Array<{
+    label: string;
+    range: DateRange;
+  }>;
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f40a8bf and 40facae.

📒 Files selected for processing (14)
  • apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1 hunks)
  • apps/web/app/[locale]/dashboard/app-url/page.tsx (0 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx (1 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-chart.tsx (1 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-grid.tsx (1 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/page.tsx (3 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/components/date-range-picker.tsx (0 hunks)
  • apps/web/app/api/timesheet/time-log/report/daily-chart/route.ts (1 hunks)
  • apps/web/app/services/server/requests/timesheet.ts (1 hunks)
  • apps/web/components/app-sidebar.tsx (2 hunks)
  • apps/web/components/ui/export-dialog.tsx (1 hunks)
  • apps/web/lib/features/task/task-filters.tsx (3 hunks)
  • apps/web/lib/features/task/task-input.tsx (7 hunks)
  • apps/web/lib/features/task/task-status.tsx (1 hunks)
💤 Files with no reviewable changes (2)
  • apps/web/app/[locale]/dashboard/app-url/page.tsx
  • apps/web/app/[locale]/dashboard/team-dashboard/components/date-range-picker.tsx
✅ Files skipped from review due to trivial changes (2)
  • apps/web/lib/features/task/task-status.tsx
  • apps/web/lib/features/task/task-input.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: deploy
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (12)
apps/web/app/api/timesheet/time-log/report/daily-chart/route.ts (1)

1-5: LGTM! Clean imports and proper function signature.

The imports are well-organized and the function signature follows Next.js API route conventions.

apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-grid.tsx (1)

51-51: LGTM: Dark mode text color implementation.

The addition of dark:text-white with proper fallback handling improves dark mode compatibility.

apps/web/components/ui/export-dialog.tsx (1)

15-19: LGTM: Well-structured props interface.

The TypeScript interface clearly defines the component's contract with required props.

apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1)

52-52: Add content to MainLayout.

The MainLayout is empty. Consider adding the actual Apps & URLs content or a placeholder if the feature is under development.

apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-chart.tsx (3)

35-35: Verify chart readability with reduced height.

The chart height has been reduced from 300px to 250px. Please ensure that this reduction doesn't compromise the chart's readability, especially for datasets with high variance.


37-37: LGTM! Improved chart margins.

The consistent 20px margins on all sides will prevent axis labels from being cut off and improve the overall layout.


39-42: LGTM! Enhanced grid styling with dark mode support.

Good improvements:

  • Added dashed grid lines with strokeDasharray="3 3" for better visual separation
  • Added dark mode support with dark:stroke-gray-700 for better accessibility
apps/web/components/app-sidebar.tsx (2)

135-148: LGTM! Improved favorite task styling.

Good improvements in the favorite task styling:

  • Better class organization for flex layout
  • Enhanced dark mode support
  • Improved text truncation and spacing

99-113: Verify the new dashboard navigation URLs.

The dashboard navigation has been restructured with sub-items. Please verify:

  1. The URL patterns for team dashboard and apps & URLs pages
  2. The URL encoding of the username parameter

Run the following script to verify the URL patterns:

apps/web/lib/features/task/task-filters.tsx (3)

270-270: LGTM! Enhanced responsive layout.

Good improvements in the flex layout:

  • Added flex-wrap-reverse for better mobile layout
  • Improved spacing with items-center
  • Added responsive breakpoints for better adaptability

396-396: LGTM! Improved navigation styling.

Good improvements in the navigation:

  • Better center alignment on mobile
  • Improved spacing at different breakpoints
  • Added margin adjustments for better layout

448-448: LGTM! Enhanced filter layout.

Good improvements in the filter layout:

  • Better flex wrapping for responsive design
  • Improved spacing between filter items
  • Added proper justification for different screen sizes

@Innocent-Akim Innocent-Akim changed the title feat: implement time tracking analytics improvements [Feat]: Add new API endpoint for daily time log report charts Jan 21, 2025
@Innocent-Akim Innocent-Akim marked this pull request as draft January 21, 2025 18:18
@evereq
Copy link
Member

evereq commented Jan 21, 2025

@Innocent-Akim please fix merge conflicts with develop branch

@Innocent-Akim Innocent-Akim force-pushed the feat/daily-time-log-report branch from 40facae to 24fb75e Compare January 21, 2025 20:14
@Innocent-Akim Innocent-Akim marked this pull request as ready for review January 21, 2025 20:15
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx (1)

154-230: 🛠️ Refactor suggestion

Enhance accessibility for screen readers and keyboard navigation.

The component needs accessibility improvements as mentioned in the previous review.

Add ARIA attributes and keyboard navigation:

 <Button
 	variant="outline"
+	aria-label="Select date range"
+	aria-expanded={isPopoverOpen}
+	aria-haspopup="dialog"
 	className={cn(
 		'justify-between gap-2 px-3 py-2 min-w-[240px] text-center flex items-center',
 		!dateRange && 'text-muted-foreground',
 		className
 	)}
+	onKeyDown={(e) => {
+		if (e.key === 'Enter' || e.key === ' ') {
+			e.preventDefault();
+			setIsPopoverOpen(true);
+		}
+	}}
 >
🧹 Nitpick comments (3)
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1)

47-59: Consider extracting header component for reusability.

The header section contains complex markup that could be reused across similar dashboard pages.

Consider extracting to a separate component:

interface DashboardHeaderProps {
  onBack: () => void;
  breadcrumbPaths: { title: string; href: string; }[];
  fullWidth?: boolean;
}

function DashboardHeader({ onBack, breadcrumbPaths, fullWidth }: DashboardHeaderProps) {
  return (
    <div className="flex flex-col py-4 bg-gray-100 dark:bg-dark--theme">
      <Container fullWidth={fullWidth} className={cn('flex gap-4 items-center w-full')}>
        <div className="flex items-center pt-6 w-full dark:bg-dark--theme">
          <button
            onClick={onBack}
            className="p-1 rounded-full transition-colors hover:bg-gray-100"
          >
            <ArrowLeftIcon className="text-dark dark:text-[#6b7280] h-6 w-6" />
          </button>
          <Breadcrumb paths={breadcrumbPaths} className="text-sm" />
        </div>
      </Container>
    </div>
  );
}
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx (2)

24-27: Add validation for date range boundaries.

The DateRangePickerProps interface should include optional min/max date constraints.

Add date constraints to the props:

 interface DateRangePickerProps {
 	className?: string;
 	onDateRangeChange?: (range: DateRange | undefined) => void;
+	minDate?: Date;
+	maxDate?: Date;
 }

196-208: Add aria-label to Calendar component for better screen reader support.

The Calendar component should have an accessible label.

 <Calendar
 	className="min-w-[240px]"
+	aria-label="Date range calendar"
 	mode="range"
 	selected={dateRange}
 	onSelect={handleDateRangeChange}
 	numberOfMonths={2}
 	month={currentMonth}
 	onMonthChange={setCurrentMonth}
 	showOutsideDays={false}
 	fixedWeeks
 	ISOWeek
 	initialFocus
 />
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40facae and 46efb8b.

📒 Files selected for processing (13)
  • apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1 hunks)
  • apps/web/app/[locale]/dashboard/app-url/page.tsx (0 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx (1 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-chart.tsx (1 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-grid.tsx (1 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/page.tsx (3 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/components/date-range-picker.tsx (0 hunks)
  • apps/web/app/api/timesheet/time-log/report/daily-chart/route.ts (1 hunks)
  • apps/web/app/services/server/requests/timesheet.ts (4 hunks)
  • apps/web/components/app-sidebar.tsx (2 hunks)
  • apps/web/components/ui/export-dialog.tsx (1 hunks)
  • apps/web/lib/features/task/task-filters.tsx (3 hunks)
  • apps/web/lib/features/task/task-input.tsx (7 hunks)
💤 Files with no reviewable changes (2)
  • apps/web/app/[locale]/dashboard/app-url/page.tsx
  • apps/web/app/[locale]/dashboard/team-dashboard/components/date-range-picker.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-chart.tsx
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-stats-grid.tsx
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/page.tsx
  • apps/web/app/services/server/requests/timesheet.ts
  • apps/web/components/app-sidebar.tsx
  • apps/web/app/api/timesheet/time-log/report/daily-chart/route.ts
  • apps/web/components/ui/export-dialog.tsx
  • apps/web/lib/features/task/task-filters.tsx
  • apps/web/lib/features/task/task-input.tsx
🔇 Additional comments (1)
apps/web/app/[locale]/dashboard/app-url/[teamId]/page.tsx (1)

26-28: ⚠️ Potential issue

Verify JSON parsing of translation string.

The breadcrumb path uses JSON.parse(t('pages.home.BREADCRUMB')) which could throw an error if the translation string is not valid JSON.

Add error handling:

-			{ title: JSON.parse(t('pages.home.BREADCRUMB')), href: '/' },
+			{ 
+				title: (() => {
+					try {
+						return JSON.parse(t('pages.home.BREADCRUMB'));
+					} catch (error) {
+						console.error('Invalid JSON in translation:', error);
+						return t('pages.home.BREADCRUMB');
+					}
+				})(),
+				href: '/' 
+			},

@Innocent-Akim Innocent-Akim requested a review from evereq January 22, 2025 14:11
- Move report activity state to global store using timeLogsRapportChartState
- Remove local state management in useReportActivity hook
- Improve error handling and data reset logic
- Add better error logging
- Simplify state updates and data flow
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
apps/web/app/interfaces/timer/ITimerLog.ts (3)

161-166: Consider using camelCase for property names in ITimerValue.

The property names are in UPPERCASE which typically indicates constants in JavaScript/TypeScript. Since these are value properties, they should follow camelCase naming convention.

 export interface ITimerValue {
-    TRACKED: number;
-    MANUAL: number;
-    IDLE: number;
-    RESUMED: number;
+    tracked: number;
+    manual: number;
+    idle: number;
+    resumed: number;
 }

168-171: Consider using a more specific type for the date property.

Using a generic string type for dates can lead to inconsistencies. Consider using a more specific type like ISO 8601 string type.

 export interface ITimerDailyLog {
-    date: string;
+    date: `${number}-${number}-${number}`; // YYYY-MM-DD format
     value: ITimerValue;
 }

173-190: Enhance type safety and consistency in ITimeLogReportDailyChartProps.

  1. Fix indentation inconsistency.
  2. Consider using readonly arrays for immutability.
  3. Add stricter typing for timeZone.
 export interface ITimeLogReportDailyChartProps {
     activityLevel: {
         start: number;
         end: number;
     };
     start: number;
     end: number;
     organizationId: string;
     tenantId: string;
     startDate: string;
     endDate: string;
-    timeZone?: string;
+    timeZone?: `${string}/${string}`; // Region/City format
     projectIds?: readonly string[];
-	employeeIds?: string[];
+    employeeIds?: readonly string[];
-    logType?: string[];
+    logType?: readonly string[];
-    teamIds?:string[];
+    teamIds?: readonly string[];
     groupBy?: string;
 }
apps/web/app/services/client/api/timer/timer-log.ts (1)

159-160: Enhance timezone handling with fallback.

The default timezone function could fail in environments where Intl API is not available.

-const getDefaultTimezone = () => Intl.DateTimeFormat().resolvedOptions().timeZone;
+const getDefaultTimezone = () => {
+    try {
+        return Intl.DateTimeFormat().resolvedOptions().timeZone;
+    } catch {
+        return 'UTC';
+    }
+};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 46efb8b and 1db71cb.

📒 Files selected for processing (7)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsx (1 hunks)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx (1 hunks)
  • apps/web/app/hooks/features/useReportActivity.ts (1 hunks)
  • apps/web/app/interfaces/timer/ITimerLog.ts (1 hunks)
  • apps/web/app/services/client/api/timer/timer-log.ts (2 hunks)
  • apps/web/app/services/server/requests/timesheet.ts (5 hunks)
  • apps/web/app/stores/time-logs.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/web/app/services/server/requests/timesheet.ts
  • apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx
⏰ 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/stores/time-logs.ts (1)

25-25: LGTM!

The atom is well-typed and follows jotai's best practices for state management.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
apps/web/app/services/client/api/timer/timer-log.ts (1)

192-203: Simplify array parameter handling using URLSearchParams.

The current implementation using a separate helper function could be simplified by directly using URLSearchParams.

-const addArrayParams = (params: Record<string, string>, key: string, values: string[]) => {
-    values.forEach((value, index) => {
-        params[`${key}[${index}]`] = value;
-    });
-};
-
-const queryParams = { ...baseParams };
-if (projectIds.length) addArrayParams(queryParams, 'projectIds', projectIds);
-if (employeeIds.length) addArrayParams(queryParams, 'employeeIds', employeeIds);
-if (logType.length) addArrayParams(queryParams, 'logType', logType);
-if (teamIds.length) addArrayParams(queryParams, 'teamIds', teamIds);
+const params = new URLSearchParams(baseParams);
+projectIds.forEach(id => params.append('projectIds[]', id));
+employeeIds.forEach(id => params.append('employeeIds[]', id));
+logType.forEach(type => params.append('logType[]', type));
+teamIds.forEach(id => params.append('teamIds[]', id));
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1db71cb and 4c6bcbc.

📒 Files selected for processing (1)
  • apps/web/app/services/client/api/timer/timer-log.ts (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: deploy
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
apps/web/app/services/client/api/timer/timer-log.ts (2)

1-1: LGTM: Import changes are well-organized.

The new type imports are correctly placed alongside related types.


159-159: Consider environment compatibility for timezone detection.

The Intl.DateTimeFormat() approach works well in browsers but might fail in non-browser environments. Consider adding a fallback for server-side contexts.

Comment on lines +185 to +190
if (!organizationId || !tenantId || !startDate || !endDate) {
throw new Error('Required parameters missing: organizationId, tenantId, startDate, and endDate are required');
}
if (activityLevel.start < 0 || activityLevel.end > 100 || activityLevel.start >= activityLevel.end) {
throw new Error('Invalid activity level range');
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add date validation to prevent invalid API calls.

While the function validates required parameters and activity level, it should also validate date formats and ranges.

 if (!organizationId || !tenantId || !startDate || !endDate) {
   throw new Error('Required parameters missing: organizationId, tenantId, startDate, and endDate are required');
 }
 if (activityLevel.start < 0 || activityLevel.end > 100 || activityLevel.start >= activityLevel.end) {
   throw new Error('Invalid activity level range');
 }
+const start = new Date(startDate);
+const end = new Date(endDate);
+if (isNaN(start.getTime()) || isNaN(end.getTime())) {
+  throw new Error('Invalid date format provided');
+}
+if (start >= end) {
+  throw new Error('Start date must be before end date');
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!organizationId || !tenantId || !startDate || !endDate) {
throw new Error('Required parameters missing: organizationId, tenantId, startDate, and endDate are required');
}
if (activityLevel.start < 0 || activityLevel.end > 100 || activityLevel.start >= activityLevel.end) {
throw new Error('Invalid activity level range');
}
if (!organizationId || !tenantId || !startDate || !endDate) {
throw new Error('Required parameters missing: organizationId, tenantId, startDate, and endDate are required');
}
if (activityLevel.start < 0 || activityLevel.end > 100 || activityLevel.start >= activityLevel.end) {
throw new Error('Invalid activity level range');
}
const start = new Date(startDate);
const end = new Date(endDate);
if (isNaN(start.getTime()) || isNaN(end.getTime())) {
throw new Error('Invalid date format provided');
}
if (start >= end) {
throw new Error('Start date must be before end date');
}

Comment on lines +206 to +209
return get<ITimerDailyLog[]>(
`/timesheet/time-log/report/daily-chart?${queryString}`,
{ tenantId }
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for the API call.

The function should handle API errors gracefully and provide meaningful error messages.

-    return get<ITimerDailyLog[]>(
-        `/timesheet/time-log/report/daily-chart?${queryString}`,
-        { tenantId }
-    );
+    try {
+        return await get<ITimerDailyLog[]>(
+            `/timesheet/time-log/report/daily-chart?${queryString}`,
+            { tenantId }
+        );
+    } catch (error) {
+        throw new Error('Failed to fetch daily chart report: ' + (error instanceof Error ? error.message : 'Unknown error'));
+    }

Committable suggestion skipped: line range outside the PR's diff.

@evereq evereq merged commit 1be1d7c into develop Jan 22, 2025
13 checks passed
@evereq evereq deleted the feat/daily-time-log-report branch January 22, 2025 17:27
CREDO23 pushed a commit that referenced this pull request Jan 24, 2025
* feat: implement time tracking analytics improvements

* fix: code rabbit

* fix: code rabbit

* refactor: improve report activity state management

- Move report activity state to global store using timeLogsRapportChartState
- Remove local state management in useReportActivity hook
- Improve error handling and data reset logic
- Add better error logging
- Simplify state updates and data flow

* feat: Add input validation for required fields.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants