feat: upload several files#22
Conversation
WalkthroughThe code refactors the kitchen revenue upload feature to support multiple Excel files instead of a single file. It introduces dynamic column identification from a dictionary row for kitchen name and revenue, processes each uploaded file to update or create revenue records, and consolidates results and errors. The frontend and validation schema were updated accordingly to handle multiple file uploads. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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)
✨ 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: 0
🧹 Nitpick comments (1)
apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts (1)
53-68: Remove redundant dictionary validation.The dictionary validation is performed twice - once on line 54 and again on line 63. The second check is redundant since the dictionary has already been validated.
Apply this diff to remove the redundant check:
const indexOfName = dictionary.indexOf('Группа') const indexOfTotal = dictionary.indexOf('Сумма со скидкой, р. Всего') -if (!dictionary || indexOfName < 0 || indexOfTotal < 0) { +if (indexOfName < 0 || indexOfTotal < 0) { throw createError({ statusCode: 400, message: 'Invalid dictionary', }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts(2 hunks)
⏰ 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 (1)
apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts (1)
101-102: LGTM! Excellent improvement to use dynamic column indices.This change makes the code more resilient to changes in the Excel file structure by using dynamically determined column indices instead of hardcoded values. The implementation correctly uses the indices found from the dictionary row.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts (1)
12-16: Consider eliminating code duplication.These constants duplicate the values from the shared file schema. Consider importing them from the shared service to maintain DRY principle.
+import { MAX_FILE_SIZE, ACCEPTED_FILE_TYPES } from '~~/shared/services/file' -const MAX_FILE_SIZE = 20 * 1024 * 1024 // 20MB -const ACCEPTED_FILE_TYPES = [ - 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', - 'application/vnd.ms-excel', -]Note: You'll need to export these constants from the shared file service.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web-app/app/components/form/UploadKitchenRevenue.vue(4 hunks)apps/web-app/i18n/locales/ru-RU.json(1 hunks)apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts(1 hunks)apps/web-app/shared/services/file.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web-app/i18n/locales/ru-RU.json
⏰ 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 (6)
apps/web-app/shared/services/file.ts (1)
14-14: LGTM! Clean schema update for multiple file support.The change from single file to array of files is correctly implemented, maintaining the same validation constraints for each file while enabling multiple uploads.
apps/web-app/app/components/form/UploadKitchenRevenue.vue (2)
77-77: LGTM! Correct state initialization for multiple files.The state is properly initialized as an empty array to handle multiple files.
91-93: LGTM! Proper FormData handling for multiple files.The iteration through files and appending each to FormData under the 'files' key is correctly implemented.
apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts (3)
31-35: LGTM! Good aggregation of results across multiple files.The logic properly accumulates rows updated and errors from all processed files, providing a comprehensive result to the client.
83-89: LGTM! Improved flexibility with dynamic column detection.Using the dictionary row to determine column indices is much better than hardcoded values, making the parser more resilient to changes in Excel file structure.
91-98: Verify Excel column names consistencyNo references to the headers ‘Группа’ or ‘Сумма со скидкой, р. Всего’ were found in the repository (docs, test data or code). Please ensure that:
- All incoming Excel files use these exact column names.
- You surface a sample/template spreadsheet or document these headers (e.g. in
docs/or atest/data/folder).- Consider adding a unit or integration test that validates the header row to catch any future changes.
| for (const kitchen of parsedKitchens) { | ||
| const found = kitchens.find((k) => k.iikoAlias === kitchen.name) | ||
| if (found) { | ||
| // Create or update | ||
| const revenue = await repository.kitchen.findRevenueByKitchenAndDate(found.id, date) | ||
| if (!revenue) { | ||
| await repository.kitchen.createRevenue({ | ||
| kitchenId: found.id, | ||
| date: dateOnly, | ||
| total: kitchen.total, | ||
| }) | ||
| } else { | ||
| await repository.kitchen.updateRevenue(revenue.id, { | ||
| total: kitchen.total, | ||
| }) | ||
| } | ||
|
|
||
| logger.warn(`Kitchen "${kitchen.name}" from file not found`) | ||
| errors.push(`"${kitchen.name}" не найдена.`) | ||
| rowsUpdated++ | ||
| continue | ||
| } | ||
|
|
||
| logger.log(rowsUpdated, date, parsedKitchens) | ||
| logger.warn(`Kitchen "${kitchen.name}" from file not found`) | ||
| errors.push(`"${kitchen.name}" не найдена.`) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding database transaction support for better error handling.
Each file's database operations are not wrapped in a transaction. If an error occurs partway through processing a file's data, some records might be updated while others aren't, leading to inconsistent state.
Consider wrapping the database operations for each file in a transaction:
for (const kitchen of parsedKitchens) {
const found = kitchens.find((k) => k.iikoAlias === kitchen.name)
if (found) {
+ // Wrap in transaction if supported by your repository
+ await repository.transaction(async (tx) => {
const revenue = await repository.kitchen.findRevenueByKitchenAndDate(found.id, date)
if (!revenue) {
await repository.kitchen.createRevenue({
kitchenId: found.id,
date: dateOnly,
total: kitchen.total,
})
} else {
await repository.kitchen.updateRevenue(revenue.id, {
total: kitchen.total,
})
}
+ })
rowsUpdated++
continue
}📝 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.
| for (const kitchen of parsedKitchens) { | |
| const found = kitchens.find((k) => k.iikoAlias === kitchen.name) | |
| if (found) { | |
| // Create or update | |
| const revenue = await repository.kitchen.findRevenueByKitchenAndDate(found.id, date) | |
| if (!revenue) { | |
| await repository.kitchen.createRevenue({ | |
| kitchenId: found.id, | |
| date: dateOnly, | |
| total: kitchen.total, | |
| }) | |
| } else { | |
| await repository.kitchen.updateRevenue(revenue.id, { | |
| total: kitchen.total, | |
| }) | |
| } | |
| logger.warn(`Kitchen "${kitchen.name}" from file not found`) | |
| errors.push(`"${kitchen.name}" не найдена.`) | |
| rowsUpdated++ | |
| continue | |
| } | |
| logger.log(rowsUpdated, date, parsedKitchens) | |
| logger.warn(`Kitchen "${kitchen.name}" from file not found`) | |
| errors.push(`"${kitchen.name}" не найдена.`) | |
| } | |
| for (const kitchen of parsedKitchens) { | |
| const found = kitchens.find((k) => k.iikoAlias === kitchen.name) | |
| if (found) { | |
| // Wrap in transaction if supported by your repository | |
| await repository.transaction(async (tx) => { | |
| const revenue = await repository.kitchen.findRevenueByKitchenAndDate(found.id, date) | |
| if (!revenue) { | |
| await repository.kitchen.createRevenue({ | |
| kitchenId: found.id, | |
| date: dateOnly, | |
| total: kitchen.total, | |
| }) | |
| } else { | |
| await repository.kitchen.updateRevenue(revenue.id, { | |
| total: kitchen.total, | |
| }) | |
| } | |
| }) | |
| rowsUpdated++ | |
| continue | |
| } | |
| logger.warn(`Kitchen "${kitchen.name}" from file not found`) | |
| errors.push(`"${kitchen.name}" не найдена.`) | |
| } |
🤖 Prompt for AI Agents
In apps/web-app/server/api/kitchen/revenue/iiko-daily.post.ts around lines 149
to 172, the database operations for each kitchen are not wrapped in a
transaction, risking partial updates and inconsistent state if an error occurs.
To fix this, wrap the entire loop or the processing of each file's data in a
database transaction using your repository or ORM's transaction API. Begin a
transaction before processing, commit it after all operations succeed, and roll
back if any error occurs to ensure atomicity and consistency.
|
1 participant
Summary by CodeRabbit