Feat/report#15
Conversation
WalkthroughThe changes introduce charting and spreadsheet export capabilities to the analytics reports dashboard. New dependencies for chart rendering and Excel export are added. Several new TypeScript interfaces and React hooks standardize report data fetching. The analytics page is fully implemented with data aggregation, visualization, filtering, and exporting to Excel. Configuration files are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AnalyticsPage
participant useReportsActivity/useReportTicket/useRevenueTicket
participant API
participant XLSX
User->>AnalyticsPage: Selects filters/views or navigates tabs
AnalyticsPage->>useReportsActivity/useReportTicket/useRevenueTicket: Fetch report data (with params)
useReportsActivity/useReportTicket/useRevenueTicket->>API: GET /reports/{type}
API-->>useReportsActivity/useReportTicket/useRevenueTicket: Return paginated report data
useReportsActivity/useReportTicket/useRevenueTicket-->>AnalyticsPage: Return parsed data
AnalyticsPage->>AnalyticsPage: Aggregate/process data for charts/tables
AnalyticsPage-->>User: Render charts, tables, and export buttons
User->>AnalyticsPage: Clicks "Export to Excel"
AnalyticsPage->>XLSX: Generate Excel file from current data
XLSX-->>User: Download Excel file
Assessment against linked issues
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
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 5
🧹 Nitpick comments (5)
src/types/ticketReport.d.ts (1)
1-6: New interfaceTicketReportDtois well-defined.
Consider clarifying thedateformat or switching to aDatetype if downstream code performs date operations.src/types/activity.d.ts (1)
1-6: New interfaceActivityDtois well-defined.
You may want to document the format oftotalPrincipalor convert it to a numeric type for calculations if feasible..vscode/settings.json (1)
12-12: Custom spell-check words updated
Good catch addingchartjsand other terms. You might also includexlsxto avoid future spell-check flags when referencing spreadsheet exports.src/app/(dashboard)/analytics-reports/page.tsx (1)
516-519: Remove redundant netIncome assignmentThe
netIncomeproperty is being assigned redundantly. It's already correctly aggregated in line 512.return Object.values(termAggregated).map((item) => ({ ...item, - netIncome: item.netIncome, }));src/lib/hooks/useReport.ts (1)
53-137: Refactor duplicated hook logic to follow DRY principleAll three hooks share nearly identical logic. Consider extracting a generic hook factory to reduce code duplication and improve maintainability.
Create a generic hook factory:
function createReportHook<T extends { data: any[]; meta: any; links: any }>( endpoint: string ) { return (params: { page?: number; limit?: number; sortBy?: string[]; nextUrl?: string; }) => { const query = new URLSearchParams(); if (params.page) query.append('page', params.page.toString()); if (params.limit) query.append('limit', params.limit.toString()); if (params.sortBy) { for (const sort of params.sortBy) { query.append('sortBy', sort); } } const url = params.nextUrl || `${API_URL}${endpoint}?${query.toString()}`; const raw = useFetch<T>(url); return useMemo(() => { return { ...raw, reports: raw.data?.data, meta: raw.data?.meta, links: raw.data?.links, }; }, [raw.data, raw.error, raw.isLoading, raw.isValidating, raw.mutate]); }; } export const useReportsActivity = createReportHook<ReportActivityResponse>('/reports/activity'); export const useReportTicket = createReportHook<ReportTicketResponse>('/reports/ticket'); export const useRevenueTicket = createReportHook<ReportRevenueResponse>('/reports/revenue');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.vscode/settings.json(1 hunks)package.json(1 hunks)src/app/(dashboard)/analytics-reports/page.tsx(1 hunks)src/lib/hooks/useReport.ts(1 hunks)src/types/activity.d.ts(1 hunks)src/types/revenue.d.ts(1 hunks)src/types/ticketReport.d.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/app/(dashboard)/analytics-reports/page.tsx (4)
src/types/activity.d.ts (1)
ActivityDto(1-6)src/types/ticketReport.d.ts (1)
TicketReportDto(1-6)src/types/revenue.d.ts (1)
RevenueReportDto(1-7)src/lib/hooks/useReport.ts (3)
useReportsActivity(53-80)useReportTicket(82-108)useRevenueTicket(110-137)
src/lib/hooks/useReport.ts (5)
src/types/activity.d.ts (1)
ActivityDto(1-6)src/types/ticketReport.d.ts (1)
TicketReportDto(1-6)src/types/revenue.d.ts (1)
RevenueReportDto(1-7)src/constants/api-urls.ts (1)
API_URL(5-7)src/lib/hooks/useFetch.tsx (1)
useFetch(24-44)
🔇 Additional comments (2)
package.json (1)
24-37: Review added dependencies
The new dependencies (chart.js,react-chartjs-2,swr,xlsx) align with the analytics reporting feature. Ensure thatxlsxships with its own TypeScript definitions and that these versions integrate smoothly with your code.src/types/revenue.d.ts (1)
1-7: LGTM!The interface definition is well-structured and follows TypeScript conventions. Using string types for monetary values is appropriate for avoiding floating-point precision issues and maintaining consistency with other DTOs in the codebase.
| "isolatedModules": true, | ||
| "jsx": "preserve", | ||
| "incremental": true, | ||
| "types": ["node", "chart.js"], |
There was a problem hiding this comment.
Restrictive types configuration
By specifying "types": ["node", "chart.js"], the compiler will ignore all other @types packages (e.g., React, React DOM, Next.js). Either remove the types option to include all ambient definitions or explicitly add "react", "react-dom", "next", etc., to this array.
🤖 Prompt for AI Agents
In tsconfig.json at line 18, the "types" array currently restricts TypeScript to
only include "node" and "chart.js" type definitions, excluding others like React
and Next.js. To fix this, either remove the "types" option entirely to include
all available ambient type definitions or explicitly add "react", "react-dom",
"next", and any other needed packages to the array to ensure their types are
included during compilation.
| "@tailwindcss/postcss": "^4", | ||
| "@types/chart.js": "^2.9.41", | ||
| "@types/node": "^20", |
There was a problem hiding this comment.
Mismatched Chart.js type definitions
@types/chart.js@^2.9.41 targets Chart.js v2, but the project uses chart.js@^4.5.0, which already includes its own TS types. Remove this outdated devDependency to prevent type conflicts.
🤖 Prompt for AI Agents
In package.json around lines 42 to 44, remove the devDependency
"@types/chart.js": "^2.9.41" because it targets Chart.js v2, while the project
uses chart.js@^4.5.0 which includes its own TypeScript types. Deleting this
outdated type definition will prevent type conflicts.
| const [dayA, monthA, yearA] = a.split('/').map(Number); | ||
| const [dayB, monthB, yearB] = b.split('/').map(Number); | ||
| return ( | ||
| new Date(yearA, monthA - 1, dayA).getTime() - | ||
| new Date(yearB, monthB - 1, dayB).getTime() | ||
| ); | ||
| } else if (viewMode === 'month') { | ||
| const [monthA, yearA] = a.split('/').map(Number); | ||
| const [monthB, yearB] = b.split('/').map(Number); | ||
| return ( | ||
| new Date(yearA, monthA - 1).getTime() - | ||
| new Date(yearB, monthB - 1).getTime() | ||
| ); | ||
| } | ||
| return a.localeCompare(b); | ||
| }); |
There was a problem hiding this comment.
Fix date separator mismatch in sorting logic
The date keys are created using - as separator (e.g., 01-12-2024) but the sorting logic attempts to split by /. This will cause the sorting to fail for day view mode.
Apply this diff to fix the issue:
-const [dayA, monthA, yearA] = a.split('/').map(Number);
-const [dayB, monthB, yearB] = b.split('/').map(Number);
+const [dayA, monthA, yearA] = a.split('-').map(Number);
+const [dayB, monthB, yearB] = b.split('-').map(Number);And similarly for month view:
-const [monthA, yearA] = a.split('/').map(Number);
-const [monthB, yearB] = b.split('/').map(Number);
+const [monthA, yearA] = a.split('-').map(Number);
+const [monthB, yearB] = b.split('-').map(Number);🤖 Prompt for AI Agents
In src/app/(dashboard)/analytics-reports/page.tsx around lines 367 to 382, the
sorting logic splits date strings using '/' but the actual date keys use '-' as
the separator. Update the split calls to use '-' instead of '/' for both day and
month view modes to correctly parse the date components and fix the sorting.
| return useMemo(() => { | ||
| return { | ||
| ...raw, | ||
| reports: raw.data?.data, | ||
| meta: raw.data?.meta, | ||
| links: raw.data?.links, | ||
| }; | ||
| }, [raw]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix useMemo dependency to prevent unnecessary re-computations
The dependency array [raw] includes the entire object, which may change reference on every render even if the actual data hasn't changed. This could cause unnecessary re-computations.
return useMemo(() => {
return {
...raw,
reports: raw.data?.data,
meta: raw.data?.meta,
links: raw.data?.links,
};
-}, [raw]);
+}, [raw.data, raw.error, raw.isLoading, raw.isValidating, raw.mutate]);📝 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.
| return useMemo(() => { | |
| return { | |
| ...raw, | |
| reports: raw.data?.data, | |
| meta: raw.data?.meta, | |
| links: raw.data?.links, | |
| }; | |
| }, [raw]); | |
| return useMemo(() => { | |
| return { | |
| ...raw, | |
| reports: raw.data?.data, | |
| meta: raw.data?.meta, | |
| links: raw.data?.links, | |
| }; | |
| }, [raw.data, raw.error, raw.isLoading, raw.isValidating, raw.mutate]); |
🤖 Prompt for AI Agents
In src/lib/hooks/useReport.ts around lines 72 to 79, the useMemo hook's
dependency array currently includes the entire 'raw' object, which can cause
unnecessary recomputations due to reference changes. To fix this, update the
dependency array to include only the specific properties of 'raw' that are used
inside useMemo, such as 'raw.data?.data', 'raw.data?.meta', and
'raw.data?.links', ensuring useMemo only recomputes when these actual data
values change.
| interface ReportActivityResponse { | ||
| data: ActivityDto[]; | ||
| meta: { | ||
| itemsPerPage: number; | ||
| sortBy: [string, string][]; | ||
| }; | ||
| links: { | ||
| previous?: string; | ||
| current: string; | ||
| next?: string; | ||
| }; | ||
| } | ||
|
|
||
| interface ReportTicketResponse { | ||
| data: TicketReportDto[]; | ||
| meta: { | ||
| itemsPerPage: number; | ||
| totalItems: number; | ||
| currentPages: number; | ||
| totalPages: number; | ||
| sortBy: [string, string][]; | ||
| }; | ||
| links: { | ||
| previous?: string; | ||
| current: string; | ||
| next?: string; | ||
| }; | ||
| } | ||
|
|
||
| interface ReportRevenueResponse { | ||
| data: RevenueReportDto[]; | ||
| meta: { | ||
| itemsPerPage: number; | ||
| totalItems: number; | ||
| currentPages: number; | ||
| totalPages: number; | ||
| sortBy: [string, string][]; | ||
| }; | ||
| links: { | ||
| previous?: string; | ||
| current: string; | ||
| next?: string; | ||
| }; | ||
| } |
There was a problem hiding this comment.
❓ Verification inconclusive
Fix interface inconsistencies and naming issues
There are two issues with the response interfaces:
ReportActivityResponsehas a different meta structure compared to the other two interfaces - it's missing pagination fields.currentPagesshould becurrentPage(singular) in bothReportTicketResponseandReportRevenueResponse.
Please verify the actual API response structure to ensure these interfaces match:
🏁 Script executed:
#!/bin/bash
# Description: Search for API endpoint definitions or documentation to verify the response structure
# Search for API endpoint implementations
rg -A 10 "reports/(activity|ticket|revenue)" --type ts
# Search for any API documentation or tests that might show the response structure
fd -e md -e ts -e js | xargs rg -l "reports.*activity.*ticket.*revenue" | head -20Length of output: 1906
Fix interface inconsistencies and naming issues
There are two issues with the response interfaces:
ReportActivityResponsehas a different meta structure compared to the other two interfaces - it's missing pagination fields.currentPagesshould becurrentPage(singular) in bothReportTicketResponseandReportRevenueResponse.
Please verify the actual API response structure to ensure these interfaces match:
#!/bin/bash
# Description: Search for API endpoint definitions or documentation to verify the response structure
# Search for API endpoint implementations
rg -A 10 "reports/(activity|ticket|revenue)" --type ts
# Search for any API documentation or tests that might show the response structure
fd -e md -e ts -e js | xargs rg -l "reports.*activity.*ticket.*revenue" | head -20🤖 Prompt for AI Agents
In src/lib/hooks/useReport.ts between lines 8 and 51, fix the interface
inconsistencies by updating ReportActivityResponse's meta to include the missing
pagination fields (totalItems, currentPage, totalPages) to match the other
interfaces. Also, rename the property currentPages to currentPage (singular) in
both ReportTicketResponse and ReportRevenueResponse. Verify these changes
against the actual API response structure to ensure accuracy.
1 participant
Closes #4
Summary by CodeRabbit