Conversation
WalkthroughThe changes introduce two new financial metrics: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
participant DB
User->>Frontend: Load dashboard
Frontend->>Backend: GET /metrics?metric=dashboard
Backend->>DB: Query unpaid authorised invoices (for totalReceivables)
Backend->>DB: Query latest atoSuperOwing value
DB-->>Backend: Return results
Backend-->>Frontend: Respond with dashboard metrics (including totalReceivables, atoSuperOwing)
Frontend-->>User: Display widgets for Unpaid Invoices and ATO Super Owing
Poem
✨ Finishing Touches
🪧 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: 1
🧹 Nitpick comments (3)
packages/backend/src/services/metricsAggregation.receivables.test.ts (1)
27-43: Test case correctly verifies receivables aggregation logicThis test case effectively validates that the
aggregateMetricfunction correctly sums only unpaid invoices with the 'AUTHORISED' status. The test data is well-structured with both paid and unpaid invoices, ensuring the function properly filters out paid invoices.However, you might want to add more test cases to cover edge cases:
+ it('handles empty result set', async () => { + const db = setupDb(); + const start = Date.parse('2024-06-01'); + const end = Date.parse('2024-06-30'); + + const result = await aggregateMetric({ db: db as any, metric: 'totalReceivables', start, end }); + + expect(result).toEqual({ value: 0 }); + }); + + it('excludes invoices with zero amount due', async () => { + const db = setupDb(); + const now = Date.now(); + await db.insert(xero_invoices).values([ + { id: '1', invoiceId: '1', contactId: 'A', date: '2024-06-05', dueDate: null, total: 100, status: 'AUTHORISED', amountDue: 0, lastUpdated: now, customFields: null, jobId: null, clientId: null }, + ]); + + const start = Date.parse('2024-06-01'); + const end = Date.parse('2024-06-30'); + + const result = await aggregateMetric({ db: db as any, metric: 'totalReceivables', start, end }); + + expect(result).toEqual({ value: 0 }); + });packages/frontend/components/today-widgets/TotalReceivablesWidget.tsx (2)
42-44: Consider making tooltip description more detailedThe tooltip description could be enhanced to provide more context about what "unpaid invoices" represent in the business context.
- <div>{getDescription("Total Receivables")}</div> - <div className="mt-1 text-xs text-gray-400">Outstanding sales invoices.<br/>Source: Xero, updated daily.</div> + <div>{getDescription("Total Receivables")}</div> + <div className="mt-1 text-xs text-gray-400">Sum of all outstanding sales invoices that have been authorized but not yet paid by customers.<br/>Source: Xero, updated daily.</div>
48-48: Use more robust currency formattingThe current implementation uses a simple dollar sign with
toLocaleString(). Consider usingIntl.NumberFormatfor more robust currency formatting that respects locale settings, similar to what's used in theformatMetricValuefunction in dashboard-metrics.tsx.- <div className="mt-3 text-2xl font-bold text-white">${value.toLocaleString()}</div> + <div className="mt-3 text-2xl font-bold text-white"> + {new Intl.NumberFormat("en-AU", { + style: "currency", + currency: "AUD", + maximumFractionDigits: 0, + }).format(value)} + </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (10)
docs/data-model.md(1 hunks)packages/backend/src/routes/metrics.ts(1 hunks)packages/backend/src/services/metricsAggregation.receivables.test.ts(1 hunks)packages/backend/src/services/metricsAggregation.ts(4 hunks)packages/frontend/components/dashboard-metrics.tsx(2 hunks)packages/frontend/components/metrics-explorer.tsx(1 hunks)packages/frontend/components/today-overview.tsx(10 hunks)packages/frontend/components/today-widgets/TotalReceivablesWidget.tsx(1 hunks)packages/frontend/lib/api.ts(2 hunks)packages/frontend/lib/today-widgets.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/backend/src/services/metricsAggregation.ts (1)
packages/backend/dist/services/metricsAggregation.js (7)
db(283-285)db(290-292)isoStart(19-19)isoStart(23-23)isoEnd(20-20)isoEnd(24-24)total(303-303)
packages/frontend/components/today-overview.tsx (1)
packages/frontend/components/today-widgets/WidgetContainer.tsx (1)
WidgetContainer(17-40)
🔇 Additional comments (26)
packages/backend/src/routes/metrics.ts (1)
36-37: LGTM! New metrics correctly integrated in isEmpty check.The addition of 'totalReceivables' and 'atoSuperOwing' to the mainKeys array ensures the API properly handles these new financial metrics when determining if a dashboard result is empty.
packages/frontend/lib/today-widgets.ts (2)
7-7: LGTM! New receivables widget properly added.The 'receivables' widget with label 'Unpaid Invoices' is correctly positioned between the existing widgets.
10-10: LGTM! Widget label updated to accurately reflect its content.Renaming the 'tax' widget label from "Tax & Super" to "ATO Super Owing" properly aligns with the newly implemented metric.
docs/data-model.md (1)
119-123: LGTM! Clear documentation of the new metrics.The documentation provides clear definitions for both new metrics:
- totalReceivables: Sum of amount_due for authorised sales invoices with outstanding balances
- atoSuperOwing: Latest balance of GST, PAYG and Super accounts from Xero
This documentation will help team members understand how these metrics are calculated.
packages/frontend/components/metrics-explorer.tsx (2)
23-23: LGTM! Metric mapping added for 'total receivables'.The mapping from the user-facing 'total receivables' metric name to the 'receivables' widget ID is correctly implemented.
26-26: LGTM! Metric mapping added for 'ato super owing'.The mapping from the user-facing 'ato super owing' metric name to the 'tax' widget ID is correctly implemented, while maintaining the existing mapping for 'currently owing to ato and super'.
packages/frontend/lib/api.ts (3)
12-13: LGTM! API schema correctly updated for new metrics.The MetricsResponse schema has been updated to include the new 'atoSuperOwing' and 'totalReceivables' fields as nullable, optional numbers, which ensures proper type safety throughout the application.
142-142: LGTM! Mock data properly updated for ATO super owing.The mock dashboard metrics now use 'atoSuperOwing' instead of the previous 'taxAndSuper' field, maintaining consistency with the schema changes.
139-139:Details
✅ Verification successful
Verify totalReceivables in mock data.
I notice that line 139 includes 'totalReceivables' without being marked as a new line. Please verify that this field was either previously present but unused, or that it should have been marked as a new addition.
🏁 Script executed:
#!/bin/bash # Check if totalReceivables existed in the previous version of this file git show HEAD^:packages/frontend/lib/api.ts | grep -n "totalReceivables"Length of output: 110
totalReceivables field is not new
Verified viagit show HEAD^:packages/frontend/lib/api.tsthattotalReceivableswas already present at line 139 in the previous commit. No change needed.packages/backend/src/services/metricsAggregation.receivables.test.ts (2)
1-7: Well-structured test file with appropriate imports.The test file is properly set up with the necessary imports for testing and database interaction.
8-25: Good database setup functionThe
setupDbfunction creates an in-memory SQLite database with a schema that matches the productionxero_invoicestable. This is a good practice for unit testing as it isolates the tests from the actual database.packages/frontend/components/dashboard-metrics.tsx (2)
137-140: Good explicit metric name mappingAdding explicit mappings for the new metrics ensures consistent naming throughout the application. This approach is better than relying solely on the generic camelCase-to-Title Case conversion logic.
178-178: Correct description added for atoSuperOwingThe description matches the existing
atoAndSupermetric description, ensuring consistency across the application.packages/backend/src/services/metricsAggregation.ts (5)
107-120: Properly implemented metric aggregation for new metricsThe implementation of
totalReceivablesandatoSuperOwingfollows the established patterns in the file. The SQL queries correctly filter for:
totalReceivables: AUTHORISED invoices with amountDue > 0 within the date rangeatoSuperOwing: Latest recorded value from the 'xero_ato_super_owing' metricThe code properly handles null/undefined results by defaulting to 0.
305-306: Good inclusion of new metrics in result objectThe new metrics are properly added to the result object to be returned from the dashboard aggregation function.
328-328: Updated expected keys list to include new metricsThe expectedKeys array is updated to include the new metrics, ensuring they are properly validated and defaulted to 0 if missing.
515-532: Well-implemented standalone metric handlersThe standalone metric handlers for
totalReceivablesandatoSuperOwingfollow the established pattern in the file and reuse the same SQL query logic as in the dashboard aggregation, which is good for consistency and maintainability.
535-536: Updated TODO comment to remove implemented metricsThe TODO comment has been updated to remove the now-implemented metrics, which is a good practice to keep documentation accurate.
packages/frontend/components/today-widgets/TotalReceivablesWidget.tsx (2)
1-12: Well-defined component interfaceThe component imports the necessary dependencies and defines a clear props interface. The props follow the pattern used by other widget components in the application.
13-50: Well-structured component implementationThe component is implemented as a functional component using React best practices. It properly extends the BaseWidget component and includes appropriate styling, hover effects, and an accessible tooltip.
The "Unpaid Invoices" label is consistent with the mapping defined in dashboard-metrics.tsx.
packages/frontend/components/today-overview.tsx (6)
26-26: Looks good - appropriate TotalReceivablesWidget import addedThe import of the new
TotalReceivablesWidgetcomponent has been correctly added to support displaying the new receivables metric in the dashboard.
66-66: Proper type interface updates for the new metricsThe
MetricsStateinterface has been appropriately updated to replacetaxAndSuperwithatoSuperOwingand addtotalReceivables, maintaining proper type safety for the new financial metrics.Also applies to: 86-86
113-113: Good addition to component mapThe
TotalReceivablesWidgethas been properly registered in theCOMPONENT_MAP, allowing it to be used in the dashboard.
130-130: State initialization correctly updatedThe initial state values are properly initialized for both the renamed
atoSuperOwingand the newtotalReceivablesmetrics, ensuring the component state matches the updated interface.Also applies to: 150-150
200-200: Properly retrieving new metrics from API responseThe data fetching logic correctly sets the state with the new metrics values from the API response, using proper null-checking with the nullish coalescing operator.
Also applies to: 220-220
520-520: Correctly updated TaxAndSuperWidget propThe
TaxAndSuperWidgetcomponent now correctly receives the renamedatoSuperOwingmetric instead of the previoustaxAndSupervalue, ensuring proper data flow.
| <WidgetContainer | ||
| id="receivables" | ||
| visible={visibleWidgets.receivables} | ||
| gridPosition={{ x: 0, y: 2, w: 1, h: 1 }} | ||
| gridItemStyle={gridStackStyles.gridItem} | ||
| > | ||
| <TotalReceivablesWidget | ||
| value={metrics.totalReceivables} | ||
| hovered={hoveredCard === "receivables"} | ||
| onHover={(v: boolean) => setHoveredCard(v ? "receivables" : null)} | ||
| getDescription={getMetricDescription} | ||
| /> | ||
| </WidgetContainer> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Check for widget position conflict
The new receivables widget has been added with position coordinates (0, 2), which appears to conflict with the survival time widget that also uses position (0, 2) at lines 425-437.
Verify that this positioning is intentional or that widget layouts are dynamically adjusted based on visibility. If both widgets are visible simultaneously, they would overlap in the current configuration.
🏁 Script executed:
#!/bin/bash
# Check for other widgets at the same grid position
echo "Checking for widgets with position (0, 2)..."
grep -A 5 "gridPosition.*x: 0.*y: 2" packages/frontend/components/today-overview.tsx
echo "Checking for position handling in GridStackWrapper..."
grep -A 10 "GridStackWrapper" packages/frontend/components/today-widgets/GridStackWrapper.tsx || echo "File not found or no matching pattern"Length of output: 1409
Resolve overlapping widget positions
Both the SurvivalTimeWidget (lines 425–437) and the TotalReceivablesWidget (lines 483–495) are defined with the same gridPosition { x: 0, y: 2, w: 1, h: 1 }, which will cause them to render on top of each other when both are visible.
Please address this by either:
- Assigning a unique
gridPositionto one of the widgets, or - Enhancing the layout logic in GridStackWrapper to detect and resolve overlapping placements dynamically.
• packages/frontend/components/today-overview.tsx
– SurvivalTimeWidget: gridPosition={{ x: 0, y: 2, w: 1, h: 1 }}
– TotalReceivablesWidget: gridPosition={{ x: 0, y: 2, w: 1, h: 1 }}
🤖 Prompt for AI Agents
In packages/frontend/components/today-overview.tsx around lines 425-437 and
483-495, the SurvivalTimeWidget and TotalReceivablesWidget share the same
gridPosition { x: 0, y: 2, w: 1, h: 1 }, causing them to overlap when both are
visible. To fix this, assign a unique gridPosition to one of these widgets by
changing either the x or y coordinate to a non-conflicting value, or
alternatively, update the GridStackWrapper layout logic to detect and resolve
overlapping widget positions dynamically.
1 participant
Summary
totalReceivablesandatoSuperOwingin metrics aggregationTesting
npm run lint(fails: ESLint config missing)npm run typechecknpm testnpm run build(fails to fetch fonts during Next.js build)Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Tests