chore: some math#27
Conversation
WalkthroughThis set of changes updates the kitchen revenue reporting system to support and process an additional metric, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend (KitchenRevenue.vue)
participant Backend API
participant Database
User->>Frontend (KitchenRevenue.vue): Load kitchen revenue chart
Frontend (KitchenRevenue.vue)->>Backend API: Fetch revenue data (including total, commonTotal)
Backend API->>Database: Query revenue records
Database-->>Backend API: Return records with total, checks, commonTotal, etc.
Backend API-->>Frontend (KitchenRevenue.vue): Return data
Frontend (KitchenRevenue.vue)->>Frontend (KitchenRevenue.vue): Render two line series (total, commonTotal)
User->>Frontend (KitchenRevenue.vue): Hover chart
Frontend (KitchenRevenue.vue)->>Frontend (KitchenRevenue.vue): Show tooltip with total, commonTotal, and averages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 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: 2
🧹 Nitpick comments (3)
packages/database/src/repository/kitchen.ts (1)
68-70: Simplify the unnecessaryor()wrapper.The filtering logic change to focus only on
commonAverageChecklooks correct, but theor()wrapper is now unnecessary since there's only one condition.Apply this diff to simplify:
- where: (revenues, { eq, or }) => or( - eq(revenues.commonAverageCheck, 0), - ), + where: (revenues, { eq }) => eq(revenues.commonAverageCheck, 0),apps/web-app/app/components/chart/KitchenRevenue.client.vue (2)
101-101: Consider using different colors for better line distinction.Both lines use the same color, which might make them difficult to distinguish. Consider using different colors for better visual clarity.
Apply this diff for better visual distinction:
-const color = (_: DataRecord, i: number) => ['var(--ui-secondary)', 'var(--ui-secondary)'][i] +const color = (_: DataRecord, i: number) => ['var(--ui-secondary)', 'var(--ui-primary)'][i]
124-124: Consider clarifying the "Средняя" label in tooltip.The tooltip shows
commonTotalas "Средняя" (Average), which might be confusing since it represents the average total across all kitchens, not the average per transaction. Consider a clearer label like "Общая средняя" (Common Average).Apply this diff for clarity:
-const template = (d: DataRecord) => `${formatDate(d.date)}, ${format(d.date, 'eeeeee', { locale: ru })}: ${d.checks} ${pluralizationRu(d.checks, ['чек', 'чека', 'чеков'])}<br> Выручка: ${formatNumber(d.total)}<br> Средняя: ${formatNumber(d.commonTotal)}` +const template = (d: DataRecord) => `${formatDate(d.date)}, ${format(d.date, 'eeeeee', { locale: ru })}: ${d.checks} ${pluralizationRu(d.checks, ['чек', 'чека', 'чеков'])}<br> Выручка: ${formatNumber(d.total)}<br> Общая средняя: ${formatNumber(d.commonTotal)}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web-app/app/components/chart/KitchenRevenue.client.vue(4 hunks)apps/web-app/nuxt.config.ts(1 hunks)apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts(3 hunks)apps/web-app/server/tasks/kitchen/average-update.ts(0 hunks)apps/web-app/server/tasks/kitchen/revenue-update.ts(1 hunks)packages/database/src/repository/kitchen.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web-app/server/tasks/kitchen/average-update.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts (1)
apps/web-app/server/tasks/kitchen/rating-update.ts (1)
run(10-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (14)
apps/web-app/nuxt.config.ts (1)
40-40: LGTM! Task scheduling configuration updated appropriately.The removal of
'kitchen:average-update'from the minutely scheduled tasks aligns with the broader changes to the kitchen revenue reporting system mentioned in the AI summary. This reduces unnecessary background processing frequency.apps/web-app/server/tasks/kitchen/revenue-update.ts (2)
23-23: LGTM! Rounding revenue to integer makes sense.The addition of
Math.round()ensures weekly revenue is stored as an integer, which is appropriate for currency values and prevents precision issues.
24-26: LGTM! Good optimization to prevent unnecessary database writes.This early exit when
revenueForThisWeek === kitchen.revenueForThisWeekis an excellent optimization that reduces database load by skipping updates when the revenue value hasn't changed.apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts (5)
124-124: LGTM! Type extension correctly adds new revenue metrics.The addition of
averageCheck,commonAverageCheck, andcommonTotalfields to the parsed kitchens type is well-structured and properly typed.
141-144: LGTM! Proper initialization of new revenue fields.The initialization of new fields with calculated
averageCheckand placeholder zeros for common values is correct, as the common values are calculated later.
151-154: LGTM! Correctly assigns common values to all kitchens.The assignment of calculated common values to each kitchen object is appropriate since these represent aggregated metrics that should be consistent across all kitchens.
172-174: LGTM! Database creation includes all new revenue fields.The inclusion of
averageCheck,commonAverageCheck, andcommonTotalin the revenue creation ensures complete data persistence.
180-182: LGTM! Database update mirrors creation with all new fields.The update operation correctly includes all new revenue fields, maintaining consistency with the creation operation.
apps/web-app/app/components/chart/KitchenRevenue.client.vue (6)
59-59: LGTM! Type extension correctly adds commonTotal field.The addition of
commonTotal: numberto the DataRecord type is consistent with existing field types.
65-65: LGTM! Props extension aligns with new data structure.The addition of
commonTotalto the values prop correctly reflects the enhanced data model from the backend.
89-89: LGTM! Data mapping follows consistent pattern.The mapping of
commonTotalwith a fallback to 0 is consistent with the existing pattern used for other fields.
95-98: LGTM! Y accessor array enables dual line visualization.Converting the y accessor to an array of functions correctly enables displaying both
totalandcommonTotalas separate lines on the chart.
99-99: LGTM! Area fill correctly uses only primary total metric.Using a separate
yAreaaccessor for just thetotalvalues is a good design choice that prevents visual confusion while maintaining dual line functionality.
102-102: LGTM! Dash pattern provides good visual differentiation.The
lineDashArraycorrectly differentiates the second line with dashed styling while keeping the first line solid.
| continue | ||
| } | ||
|
|
||
| const averageCheck = Math.round(total / checks) |
There was a problem hiding this comment.
Add validation for division by zero.
The average check calculation could fail if checks is 0, leading to Infinity or NaN values.
Add validation to handle zero checks:
+ if (checks === 0) {
+ continue
+ }
+
const averageCheck = Math.round(total / checks)📝 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.
| const averageCheck = Math.round(total / checks) | |
| if (checks === 0) { | |
| continue | |
| } | |
| const averageCheck = Math.round(total / checks) |
🤖 Prompt for AI Agents
In apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts at line 135, the
calculation of averageCheck divides total by checks without checking if checks
is zero, which can cause Infinity or NaN results. Add a validation before the
division to check if checks is zero; if so, set averageCheck to zero or an
appropriate default value to avoid invalid calculations.
| const commonAverageCheck = Math.round(parsedKitchens.reduce((acc, curr) => acc + curr.averageCheck, 0) / parsedKitchens.length) | ||
| const commonTotal = Math.round(parsedKitchens.reduce((acc, curr) => acc + curr.total, 0) / parsedKitchens.length) |
There was a problem hiding this comment.
Add validation for empty kitchen arrays.
The common value calculations could fail with division by zero if parsedKitchens.length is 0.
Add validation to handle empty arrays:
+ if (parsedKitchens.length === 0) {
+ return { rowsUpdated: 0, errors: ['No valid kitchen data found'] }
+ }
+
// Update common data
const commonAverageCheck = Math.round(parsedKitchens.reduce((acc, curr) => acc + curr.averageCheck, 0) / parsedKitchens.length)
const commonTotal = Math.round(parsedKitchens.reduce((acc, curr) => acc + curr.total, 0) / parsedKitchens.length)📝 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.
| const commonAverageCheck = Math.round(parsedKitchens.reduce((acc, curr) => acc + curr.averageCheck, 0) / parsedKitchens.length) | |
| const commonTotal = Math.round(parsedKitchens.reduce((acc, curr) => acc + curr.total, 0) / parsedKitchens.length) | |
| if (parsedKitchens.length === 0) { | |
| return { rowsUpdated: 0, errors: ['No valid kitchen data found'] } | |
| } | |
| // Update common data | |
| const commonAverageCheck = Math.round( | |
| parsedKitchens.reduce((acc, curr) => acc + curr.averageCheck, 0) / parsedKitchens.length | |
| ) | |
| const commonTotal = Math.round( | |
| parsedKitchens.reduce((acc, curr) => acc + curr.total, 0) / parsedKitchens.length | |
| ) |
🤖 Prompt for AI Agents
In apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts around lines 148
to 149, the code calculates averages by dividing sums by parsedKitchens.length
without checking if the array is empty, which can cause division by zero errors.
Add a validation before these calculations to check if parsedKitchens.length is
greater than zero; if it is zero, set commonAverageCheck and commonTotal to a
safe default value like 0 to avoid runtime errors.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores