feat!: db changed #323
feat!: db changed #323
Conversation
WalkthroughThis pull request makes extensive configuration and code changes across both the frontend and backend. On the frontend side, new SonarLint settings and environment variable updates were applied along with modifications to event handling, URL construction, and data flow in several Vue components and pages. On the backend, repository calls have been replaced with service functions across multiple API endpoints and stores, and new service modules and a Redis-based plugin have been added. Type definitions and workspace dependencies were also updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Cart Component
participant Channel as Channel Store
U->>C: Select product variant
C->>Channel: getProductByProductVariant(variantId)
Channel-->>C: Return product data
C->>Channel: getMenuCategoryByProduct(productId)
Channel-->>C: Return category data
C->>U: Render productUrl as /catalog/{category.slug}/{product.slug}
sequenceDiagram
participant Client as Client
participant API as API Endpoint
participant PMService as Payment Method Service
participant ChannelService as Channel Service
Client->>API: Request deletion of payment method (id)
API->>PMService: deletePaymentMethod(id)
PMService-->>API: Acknowledgement
API->>ChannelService: setChannelAsUpdated(channelId)
ChannelService-->>API: Channel updated
API->>Client: Send success response
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 8
🔭 Outside diff range comments (1)
apps/web-app/app/components/Form/CreateChannelPaymentMethod.vue (1)
72-94: 💡 Verification agent🧩 Analysis chain
Consider handling disconnection between modal and error states.
Since the modal will now close immediately upon submission (based on the AI summary), users might miss error notifications if the API call fails. Consider adding a mechanism to handle errors more visibly after the modal closes.
🏁 Script executed:
#!/bin/bash # Check how errors are handled in the modal component fd "CreateChannelPaymentMethod.vue" apps/web-app/app/components/Modal --type f | xargs catLength of output: 433
Revisit Modal Closing Behavior on Submission
The current flow immediately closes the modal on the
submittedevent—even before the API call completes. Our verification confirms that the modal component inapps/web-app/app/components/Modal/CreateChannelPaymentMethod.vuecloses the modal on both@submittedand@successevents. This means that if the API call fails (as handled in the form’sonSubmitfunction inapps/web-app/app/components/Form/CreateChannelPaymentMethod.vue), the error toast is shown only after the modal has already closed, which may cause users to miss the error notification.Recommendations:
- Delay Modal Closure: Remove or adjust the immediate
modal.close()call from the@submittedevent. Consider closing the modal only after a successful API call so that error notifications remain visible.- Error State Handling: Alternatively, if keeping the modal open on error isn’t feasible, introduce a mechanism (such as re-opening the modal or displaying persistent error messages) to ensure users acknowledge API call failures.
🧹 Nitpick comments (43)
apps/web-app/app/components/Form/DeleteProductVariant.vue (1)
8-8: Correct event binding for button elementThe change from
@submitto@clickis appropriate since submit events are typically handled on form elements, while buttons should use click events. This change aligns with proper Vue event handling practices.However, consider reviewing if
type="submit"is still needed on line 3, as this attribute is meant for buttons within forms that should trigger form submission.apps/web-app/app/components/Form/DeleteChannelPaymentMethod.vue (1)
8-8: Correct event binding for button elementThe change from
@submitto@clickis appropriate since submit events are typically handled on form elements, while buttons should use click events. This change aligns with proper Vue event handling practices.However, consider reviewing if
type="submit"is still needed on line 3, as this attribute is meant for buttons within forms that should trigger form submission.apps/web-app/app/components/Form/DeleteProduct.vue (1)
8-8: Correct event binding for button elementThe change from
@submitto@clickis appropriate since submit events are typically handled on form elements, while buttons should use click events. This change aligns with proper Vue event handling practices.However, consider reviewing if
type="submit"is still needed on line 3, as this attribute is meant for buttons within forms that should trigger form submission.apps/web-app/app/components/Form/CreateChannelPaymentMethod.vue (1)
73-74: Good implementation of the submitted event.Emitting the 'submitted' event at the beginning of the submission process ensures that parent components can react immediately, before waiting for the async operations to complete.
However, consider whether users might benefit from a loading state during the API call to provide visual feedback that something is happening after the modal closes.
async function onSubmit(event: FormSubmitEvent<ChannelPaymentMethodCreateSchema>) { emit('submitted') + const isLoading = ref(true) const { data, error } = await useAsyncData( 'create-payment-method', () => $fetch('/api/channel/payment-method', { method: 'POST', body: event.data, }), ) + isLoading.value = falsepackages/core/server/plugins/01.db.ts (1)
1-9: Redis storage driver integration looks good, but consider adding error handlingThe plugin effectively integrates Redis as a storage driver with Nitro, making it available under the key 'db'. This aligns well with the new environment variable in
.env.example.Consider enhancing the plugin with error handling and logging:
import redisDriver from 'unstorage/drivers/redis' export default defineNitroPlugin(() => { const { redisUrl: url } = useRuntimeConfig() + if (!url) { + console.warn('Redis URL not provided, db storage might not function correctly') + } + const driver = redisDriver({ url }) + + // Optional: Add event listeners for connection status + driver.on?.('error', (err) => { + console.error('Redis connection error:', err) + }) useStorage().mount('db', driver) })packages/core/server/services/db/index.ts (1)
10-26: Consider optimizing key filtering for large datasetsThe implementation is clean and straightforward, but performs multiple iterations over the
allKeysarray (once for each filter). While likely not an issue for most cases, this could be optimized to a single-pass approach if dealing with a large number of keys.export async function getKeys(): Promise<Keys> { const allKeys = await useStorage('db').getKeys() - const menuKeys = allKeys.filter((key) => key.startsWith('menu:')) - const productKeys = allKeys.filter((key) => key.startsWith('product:')) - const warehouseKeys = allKeys.filter((key) => key.startsWith('warehouse:')) - const paymentMethodKeys = allKeys.filter((key) => key.startsWith('payment:method:')) - const userKeys = allKeys.filter((key) => key.startsWith('user:')) + const menuKeys: string[] = [] + const productKeys: string[] = [] + const warehouseKeys: string[] = [] + const paymentMethodKeys: string[] = [] + const userKeys: string[] = [] + + for (const key of allKeys) { + if (key.startsWith('menu:')) menuKeys.push(key) + else if (key.startsWith('product:')) productKeys.push(key) + else if (key.startsWith('warehouse:')) warehouseKeys.push(key) + else if (key.startsWith('payment:method:')) paymentMethodKeys.push(key) + else if (key.startsWith('user:')) userKeys.push(key) + } return { allKeys, menuKeys, productKeys, warehouseKeys, paymentMethodKeys, userKeys, } }apps/web-app/app/components/Cart/Line.vue (1)
39-40: Improved category lookup method for product URLs.The change to use
getMenuCategoryByProductinstead of directly accessing product category data is a good improvement. It centralizes category retrieval logic in the channel store and allows for more flexibility in how categories are determined.However, you should handle the potential undefined value from
category?.slugmore robustly.-const category = channel.getMenuCategoryByProduct(product.value?.id ?? '') -const productUrl = computed(() => `/catalog/${category?.slug}/${product.value?.slug}`) +const category = computed(() => channel.getMenuCategoryByProduct(product.value?.id ?? '')) +const productUrl = computed(() => { + const categorySlug = category.value?.slug || 'uncategorized' + const productSlug = product.value?.slug || '' + return `/catalog/${categorySlug}/${productSlug}` +})apps/web-app/app/pages/command-center/system/index.vue (1)
94-94: Improve time display for clarity or localization.
Rendering{{ workingDay.open }} - {{ workingDay.close }}is a good start, but consider using date/time formatting utilities to handle locales, AM/PM, or 24-hour format.apps/web-app/stores/checkout.ts (1)
14-38: Streamlined fetch approach with improved error handling.
Switching fromuseFetchto$fetchis straightforward and the try/catch block is an improvement. However, ensure that ignoring'NotFound'errors is truly intentional—consider logging them or throwing a custom error to avoid hiding unexpected mistakes.packages/core/server/services/db/media.ts (1)
5-12: Return the newly created object directly to avoid an additional read.
Currently, you write to storage and then perform a separate call togetMedia()to retrieve the just-created object. If you wish to optimize, you can return the object directly as populated in-memory.export async function createMedia(data: Omit<Media, 'createdAt' | 'updatedAt'>): Promise<Media | null> { const now = new Date().toISOString() const newMedia = { ...data, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), + createdAt: now, + updatedAt: now, } await useStorage('db').setItem(`media:${data.id}`, newMedia) - return getMedia(data.id) + return newMedia }packages/core/server/services/db/menu.ts (1)
1-43: LGTM: Well-structured menu service implementation.The new
getMenusservice function is clearly written with good error handling and appropriate use of async/await patterns. The comments explaining the key formats are helpful for readability.For a minor performance optimization, consider batching the awaits for storage operations. Currently, the Map stores promises which are then awaited individually, but you could initialize the Map and then await all promises at once to potentially improve performance.
- const menuStorage = new Map<string, unknown>(keys.map((key) => [key, useStorage('db').getItem(key)])) + const storagePromises = keys.map(key => [key, useStorage('db').getItem(key)]) + const resolvedPromises = await Promise.all(storagePromises.map(([_, promise]) => promise)) + const menuStorage = new Map<string, unknown>( + storagePromises.map(([key], index) => [key, resolvedPromises[index]]) + )apps/web-app/app/pages/command-center/product/[id]/index.vue (1)
10-10: Need to handle potential undefined values.The current display format could show "//" if both
categoryandproductslugs are undefined. Consider adding a conditional check to avoid displaying empty slashes.- /{{ category?.slug }}/{{ product?.slug }} + {{ category?.slug ? `/${category.slug}` : '' }}{{ product?.slug ? `/${product?.slug}` : '' }}apps/web-app/app/components/CategoryBlock.vue (1)
39-39: Consider performance optimization for product filtering.The current implementation fetches all products and filters them in memory. For large product catalogs, this could impact performance.
Consider implementing server-side filtering or pagination if the product catalog grows significantly:
-const products = channel.getProductsInCategory(categoryId).value.filter((p) => p.isAvailableForPurchase && p.variants.length).slice(0, 8) +const products = computed(() => { + const allProducts = channel.getProductsInCategory(categoryId).value + return allProducts.filter((p) => p.isAvailableForPurchase && p.variants.length).slice(0, 8) +})This change makes
productsreactive and will recalculate if the underlying data changes.packages/core/server/api/channel/working-day/index.patch.ts (1)
18-20: Simplified loop with better error handling needed.The loop has been simplified by delegating logic to the service layer, but consider adding transaction handling for multiple updates.
Consider implementing transaction handling to ensure atomicity of updates:
+ // Consider implementing transaction handling here + let hasErrors = false; + const errors = []; for (const day of data) { - await patchWorkingDay(day.day, day) + try { + await patchWorkingDay(day.day, day) + } catch (error) { + hasErrors = true; + errors.push(`Failed to update ${day.day}: ${error.message}`); + } } + if (hasErrors) { + throw createError({ + statusCode: 500, + statusMessage: `Failed to update some working days: ${errors.join(', ')}`, + }); + }packages/core/server/services/db/channel.ts (4)
1-3: Add input validation for channel ID.Consider adding validation to check if the
idparameter is valid before querying the storage. Also, ensure thatuseStorageandChanneltype are properly imported (not visible in this snippet).export async function getChannel(id: string): Promise<Channel | null> { + if (!id) { + return null + } return useStorage('db').getItem<Channel>(`channel:${id}`) }
5-20: Implementation looks good, consider consistency in return pattern.The function properly handles cases where the channel doesn't exist and correctly updates the timestamp. However, I noticed that unlike other DB methods in this codebase, this function returns the in-memory object rather than fetching it again from storage.
Consider adopting a consistent approach across all DB service methods - either return the in-memory object or fetch it again from storage.
22-29: Add existence check before creating a channel.The function correctly sets timestamps, but it doesn't check if a channel with the same ID already exists, which could lead to unintentional overwrites.
export async function createChannel(data: Omit<Channel, 'createdAt' | 'updatedAt'>): Promise<Channel | null> { + const existingChannel = await getChannel(data.id) + if (existingChannel) { + return null // or throw an error + } await useStorage('db').setItem(`channel:${data.id}`, { ...data, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), }) return getChannel(data.id) }
31-33: Redundant function implementation.The
setChannelAsUpdatedfunction is redundant sincepatchChannelalready updates theupdatedAttimestamp automatically. Consider refactoring for simplicity.export async function setChannelAsUpdated(id: string): Promise<void> { - await patchChannel(id, { updatedAt: new Date().toISOString() }) + await patchChannel(id, {}) // This will update the timestamp automatically }packages/core/server/services/db/warehouse.ts (4)
1-3: Add input validation for warehouse ID.Consider adding validation to check if the
idparameter is valid before querying the storage.export async function getWarehouse(id: string): Promise<Warehouse | null> { + if (!id) { + return null + } return useStorage('db').getItem<Warehouse>(`warehouse:${id}`) }
5-19: Clarify parameter naming and improve code formatting.The parameter is named
keysbut it's not clear if these are full storage keys or just warehouse IDs. Also, there's a minor formatting issue with the closing parenthesis.-export async function getWarehouses(keys: string[]): Promise<Warehouse[]> { +export async function getWarehouses(warehouseIds: string[]): Promise<Warehouse[]> { const warehouses: Warehouse[] = [] await Promise.all( - keys.map(async (key) => { + warehouseIds.map(async (id) => { - const warehouse = await useStorage('db').getItem<Warehouse>(key) + const warehouse = await useStorage('db').getItem<Warehouse>(`warehouse:${id}`) if (!warehouse) { return } warehouses.push(warehouse) - })) + }) + ) return warehouses }
21-28: Add existence check before creating a warehouse.Similar to the channel service, consider adding a check to prevent overwriting existing warehouses.
export async function createWarehouse(data: Omit<Warehouse, 'createdAt' | 'updatedAt'>): Promise<Warehouse | null> { + const existingWarehouse = await getWarehouse(data.id) + if (existingWarehouse) { + return null // or throw an error + } await useStorage('db').setItem(`warehouse:${data.id}`, { ...data, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), }) return getWarehouse(data.id) }
30-45: Consider consistency in return pattern across DB services.Unlike the
patchChannelfunction that returns the in-memory object, this function fetches the warehouse again from storage after updating it. Adopt a consistent approach across all DB service methods.export async function patchWarehouse(id: string, data: Partial<Warehouse>): Promise<Warehouse | null> { const warehouse = await getWarehouse(id) if (!warehouse) { return null } const newWarehouse = { ...warehouse, ...data, updatedAt: new Date().toISOString(), } await useStorage('db').setItem(`warehouse:${id}`, newWarehouse) - return getWarehouse(id) + return newWarehouse }packages/core/server/api/channel/payment-method/index.post.ts (1)
19-26: Add error handling for service function calls.The code correctly uses the new service functions, but it lacks specific error handling if either
createPaymentMethodorsetChannelAsUpdatedfails.- await createPaymentMethod({ - id: createId(), - name: data.name, - type: data.type, - }) - - await setChannelAsUpdated(channelId) + const paymentMethodId = createId() + const paymentMethod = await createPaymentMethod({ + id: paymentMethodId, + name: data.name, + type: data.type, + }) + + if (!paymentMethod) { + throw createError({ + statusCode: 500, + statusMessage: 'Failed to create payment method', + }) + } + + try { + await setChannelAsUpdated(channelId) + } catch (error) { + console.error('Failed to update channel timestamp:', error) + // Continue execution as this is non-critical + }packages/core/server/services/db/work.ts (2)
5-19: Consider returning a value for error cases.The function silently returns undefined in error cases, which might make debugging harder. Consider returning a more explicit response.
export async function changeWorkingDayActivity(day: DayCode, isActive: boolean): Promise<void> { const workingDays = await getWorkingDays() if (!workingDays) { - return + return undefined } const workingDay = workingDays.find((workingDay) => workingDay.day === day) if (!workingDay) { - return + return undefined } workingDay.isActive = isActive await useStorage('db').setItem('working-time', workingDays) + return undefined }
39-42: Add validation for WorkingWeek data.Consider adding validation to ensure the provided
WorkingWeekdata has a valid structure before saving it to storage.export async function createWorkingDays(data: WorkingWeek): Promise<WorkingWeek | null> { + if (!Array.isArray(data) || data.length === 0) { + return null + } + + // Validate that each working day has required properties + for (const day of data) { + if (!day.day || typeof day.isActive !== 'boolean') { + return null + } + } await useStorage('db').setItem('working-time', data) return getWorkingDays() }apps/web-app/app/pages/checkout/index.vue (2)
222-236: Consider distinct handling for empty cart vs maintenance mode.Using a single
<template v-else>covers both an empty cart and maintenance mode. If your intent is to differentiate these states, you may want to show a maintenance message separately rather than reusing the “empty cart” label.
276-276: Review maximum phone length limit.Using
17as a hard-coded limit might not account for varying country formats or future expansions. Consider referencing a config-based constant or a library for robust phone validation.packages/core/server/api/channel/master.put.ts (1)
48-49: Consider credential collision checks.Creating user credentials with a fresh ID is good practice. Ensure that if two masters are created concurrently (though unlikely), or a login collision occurs, the system fails gracefully and informs the caller.
packages/core/server/services/db/payment.ts (2)
5-19: Parallel retrieval of multiple payment methods is fine; watch for concurrency.Pushing into
paymentMethodsinsidemap(async...)is acceptable in single-threaded environments, but in highly concurrent situations, external concurrency or merges might need more robust handling to avoid race conditions.
21-29:createPaymentMethodsets timestamps automatically, but confirm uniqueness.Automatically populating
createdAtandupdatedAtis convenient, though consider verifying thatpayment:method:${data.id}does not already exist. Failing gracefully if an ID collision occurs might improve reliability.apps/web-app/app/components/Checkout/Line.vue (1)
46-48: Consider null or undefined handling for product and category.If
productVariant.value?.idis empty or invalid, this could lead tochannel.getProductByProductVariantreturning null, and similarlygetMenuCategoryByProductmight also return null. In these scenarios, buildingproductUrlcould result in broken or empty URLs such as/catalog/undefined/undefined. You may want to conditionally compute or provide a fallback route if the product or category is not found.apps/web-app/app/components/Form/UpdateWorkingDays.vue (1)
48-48: Check reactivity approach for workingDays.Setting
workingDays = ref(channel.workingDays ?? [])might not capture future reactive changes ifchannel.workingDaysupdates. If you intend for the local array to track changes fromchannel.workingDays, consider either watchingchannel.workingDaysor migrating to a fully reactive approach (e.g., using computed properties or watchers).packages/core/server/services/db/user.ts (2)
3-5: Consider adding error handling or logs for getUser.The function returns
nullif a user is not found, which is fine. However, you might want to log attempts to fetch invalid user IDs or handle cases where the user is unexpectedly missing.
11-32: Parallelize or streamline retrieval in getMaster.Currently, the code builds a Map of promises without properly awaiting them in parallel (the for-loop awaits each entry sequentially). For a large user set, consider using
Promise.allto improve performance, or confirm this logic is acceptable for your data volume.packages/core/server/api/channel/index.put.ts (2)
27-40: Initialize optional fields or track usage.Fields like
description,conditions,phone, andminAmountForDeliveryare set tonull. Confirm that your downstream logic can handlenullvalues and does not break if these fields remain unset. If needed, consider default strings or numbers to avoid potential null-pointer checks.
44-93: Consider custom or user-defined working day schedules.Currently, all working days are initialized with
"00:00". If you intend these defaults as placeholders, great. Otherwise, you might want to adopt a user-configurable approach or store partial availability. This ensures the data is meaningful if the channel is truly open or closed on certain days or times.packages/core/server/api/channel/time-slots.get.ts (1)
26-27: Consider efficiency of working day retrievalThe implementation now fetches all working days and then filters, instead of finding one specific day. While this works for a small dataset of working days, consider adding a dedicated
getWorkingDayByIndexfunction if this pattern is repeated in many places.packages/core/server/services/db/product.ts (2)
7-49: Efficient product retrieval with variantsThe getProducts function efficiently handles fetching multiple products along with their variants in a single pass. A couple of suggestions:
- Consider adding error handling for unexpected key formats
- Document the expected key format in comments for maintainability
136-147: Improve delete operation efficiencySimilar to getProductVariant, the deleteProductVariant function could be more efficient if you directly delete the variant when you know both product ID and variant ID, rather than searching through all keys.
apps/web-app/stores/channel.ts (3)
93-95: Naming clash risk in category’sproducts.
category.productsis merely{id: string}[], which differs fromProduct[]. Consider renaming it or clarifying in docstrings to avoid confusion.
114-131: Potential multi-category edge case in query results.
getMenuCategoryByProductonly returns the first matching category. If products reside in multiple categories, consider how to handle them all.
132-148: Similar multi-category consideration for top searches.
Same logic asgetProductsByQuery; products assigned to a single found category.packages/core/types/index.d.ts (1)
48-56: Fixed-lengthWorkingWeektype.
Represents all seven days. If partial or variable schedules are allowed, consider a more flexible structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (64)
.vscode/settings.json(1 hunks)apps/web-app/.env.example(1 hunks)apps/web-app/app/components/Cart/Line.vue(1 hunks)apps/web-app/app/components/CategoryBlock.vue(2 hunks)apps/web-app/app/components/Checkout/Line.vue(1 hunks)apps/web-app/app/components/CommandCenter/ProductCard.vue(1 hunks)apps/web-app/app/components/DeliveryInfoBlock.vue(1 hunks)apps/web-app/app/components/Form/CreateChannelPaymentMethod.vue(2 hunks)apps/web-app/app/components/Form/DeleteChannelPaymentMethod.vue(1 hunks)apps/web-app/app/components/Form/DeleteProduct.vue(1 hunks)apps/web-app/app/components/Form/DeleteProductVariant.vue(1 hunks)apps/web-app/app/components/Form/UpdateWorkingDays.vue(3 hunks)apps/web-app/app/components/Modal/CreateChannelPaymentMethod.vue(1 hunks)apps/web-app/app/components/ProductCard.vue(1 hunks)apps/web-app/app/components/SearchBlock.vue(2 hunks)apps/web-app/app/middleware/02.staff.ts(1 hunks)apps/web-app/app/pages/catalog/[categorySlug]/[productSlug]/index.vue(1 hunks)apps/web-app/app/pages/catalog/[categorySlug]/index.vue(2 hunks)apps/web-app/app/pages/checkout/index.vue(3 hunks)apps/web-app/app/pages/command-center/product/[id]/index.vue(2 hunks)apps/web-app/app/pages/command-center/system/index.vue(1 hunks)apps/web-app/nuxt.config.ts(1 hunks)apps/web-app/package.json(1 hunks)apps/web-app/stores/channel.ts(3 hunks)apps/web-app/stores/checkout.ts(1 hunks)packages/core/nuxt.config.ts(1 hunks)packages/core/package.json(1 hunks)packages/core/server/api/channel/index.get.ts(3 hunks)packages/core/server/api/channel/index.patch.ts(2 hunks)packages/core/server/api/channel/index.put.ts(3 hunks)packages/core/server/api/channel/master.put.ts(4 hunks)packages/core/server/api/channel/payment-method/[id].delete.ts(2 hunks)packages/core/server/api/channel/payment-method/[id].patch.ts(2 hunks)packages/core/server/api/channel/payment-method/index.post.ts(2 hunks)packages/core/server/api/channel/receiving-method/index.post.ts(2 hunks)packages/core/server/api/channel/time-slots.get.ts(4 hunks)packages/core/server/api/channel/working-day/activity.post.ts(2 hunks)packages/core/server/api/channel/working-day/index.patch.ts(2 hunks)packages/core/server/api/product/[id]/image.post.ts(2 hunks)packages/core/server/api/product/[id]/index.delete.ts(2 hunks)packages/core/server/api/product/[id]/index.get.ts(0 hunks)packages/core/server/api/product/[id]/index.patch.ts(2 hunks)packages/core/server/api/product/index.post.ts(2 hunks)packages/core/server/api/product/slug/[slug].get.ts(0 hunks)packages/core/server/api/product/variant/[id].delete.ts(2 hunks)packages/core/server/api/product/variant/[id].patch.ts(2 hunks)packages/core/server/api/product/variant/index.post.ts(2 hunks)packages/core/server/api/user/[id]/index.get.ts(2 hunks)packages/core/server/api/warehouse/[id]/index.get.ts(0 hunks)packages/core/server/api/warehouse/[id]/index.patch.ts(2 hunks)packages/core/server/api/warehouse/index.post.ts(2 hunks)packages/core/server/plugins/01.db.ts(1 hunks)packages/core/server/services/db/channel.ts(1 hunks)packages/core/server/services/db/index.ts(1 hunks)packages/core/server/services/db/media.ts(1 hunks)packages/core/server/services/db/menu.ts(1 hunks)packages/core/server/services/db/payment.ts(1 hunks)packages/core/server/services/db/product.ts(1 hunks)packages/core/server/services/db/user.ts(1 hunks)packages/core/server/services/db/warehouse.ts(1 hunks)packages/core/server/services/db/work.ts(1 hunks)packages/core/shared/services/workingDay.ts(1 hunks)packages/core/types/index.d.ts(5 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (3)
- packages/core/server/api/product/[id]/index.get.ts
- packages/core/server/api/warehouse/[id]/index.get.ts
- packages/core/server/api/product/slug/[slug].get.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (118)
apps/web-app/app/components/Modal/CreateChannelPaymentMethod.vue (1)
6-6: Verify behavior when form submission failsAdding
@submitted="modal.close()"will close the modal as soon as the form is submitted, regardless of success or failure. This is different from the existing behavior where the modal only closes on success.Make sure this is the intended behavior and that users will still see error messages if the submission fails. If the intent is to keep the modal open on errors to display validation messages, this change might prevent that from happening.
apps/web-app/app/components/Form/CreateChannelPaymentMethod.vue (1)
47-47: Good addition of the 'submitted' event to the component's API.Enhancing the component's events improves its flexibility. This addition of the 'submitted' event allows the parent component to react to form submissions immediately, rather than only on success.
packages/core/package.json (1)
16-16:✅ Verification successful
Added cuid2 library for unique ID generation.
The addition of
@paralleldrive/cuid2introduces a collision-resistant ID generation library that's suitable for distributed systems. This is likely part of the database changes mentioned in the PR title.
🌐 Web query:
What are the advantages of using cuid2 over other ID generation methods?💡 Result:
Cuid2 offers several significant advantages over traditional ID generation methods like UUIDs, legacy CUID, and NanoID, particularly for modern distributed systems requiring high security and collision resistance:
Enhanced Security
- Cryptographic hashing using SHA3 ensures generated IDs don't leak system/time data[1][9]
- Random salt per ID thwarts pattern recognition attacks[1]
- Multiple entropy sources (system time, random values, session counter, host fingerprint) make brute-forcing impractical[1][7]
Collision Resistance
- Default configuration provides
1.62e+37entropy - 50% collision probability requires generating 4 quintillion IDs[1]- Far surpasses UUIDv4's
3.4e+38theoretical maximum while being more practical[3][9]Operational Advantages
- Horizontal scaling without coordination between systems[1][3]
- Deterministic length (default 24 chars) vs variable-length legacy CUID[1][7]
- Base36 encoding makes URLs/database keys cleaner than UUID's hyphens[1][3]
// Example Cuid2 generation import { createId } from '@paralleldrive/cuid2'; const id = createId(); // y3cfw1hafbtezzflns334sb2
Feature Cuid2 UUIDv4 NanoID Entropy Source 5 independent Single PRNG Single PRNG Hash Algorithm SHA3 None None Default Length 24 chars 36 chars 21 chars Collision Risk 1 in 4e+18 1 in 1e+38 1 in 1e+27 Sortable No No No Modern System Optimization
- Avoids database index fragmentation through non-sequential design[3][9]
- Client-side generation capability reduces server roundtrips[1][3]
- Tiny footprint (<5KB gzipped) suitable for browser apps[1]
For applications requiring maximum security in distributed environments (like financial systems or healthcare platforms), Cuid2 provides a robust balance of collision resistance and cryptographic safety that outperforms alternatives. However, simpler use cases might prefer NanoID for raw speed or UUIDv7 for time-sortable needs[3][9].
Citations:
- 1: https://github.com/paralleldrive/cuid2
- 2: https://www.uuid.lol/cuid2
- 3: https://www.wisp.blog/blog/uuid-vs-cuid-vs-nanoid-choosing-the-right-id-generator-for-your-application
- 4: https://www.uuid.lol/cuid
- 5: https://www.bennadel.com/blog/4388-cuid2-for-coldfusion-cfml.htm
- 6: https://matthewtolman.com/article/overview-of-globally-unique-identifiers
- 7: https://docs.rs/cuid2
- 8: https://dev.to/turck/comparing-uuid-cuid-and-nanoid-a-developers-guide-50c
- 9: Comparison to others? paralleldrive/cuid2#7
- 10: https://crates.io/crates/cuid2
Cuid2 Library Integration Verified and Approved
The addition of the
@paralleldrive/cuid2package is a robust choice for generating collision-resistant IDs. As confirmed by recent research, Cuid2 leverages SHA3 cryptographic hashing, employs a random salt per ID, and combines multiple entropy sources—ensuring enhanced security and extremely low collision probability. Its deterministic fixed-length (24 characters) and base36 encoding further support clean URL/database key designs, making it ideal for distributed systems and horizontal scaling.
- Location:
packages/core/package.json(line 16)- Impact: Aligns well with the database changes referenced in the PR title and supports the goals of high security and performance.
No further modifications are required based on this verification.
pnpm-workspace.yaml (1)
44-44:✅ Verification successful
Added ioredis to workspace catalog.
The addition of
ioredis: ^5.5.0to the workspace catalog ensures consistent versioning across the monorepo. Version 5.5.0 is appropriate for the needed Redis functionality.
🌐 Web query:
What is the latest stable version of ioredis and are there any known issues with version 5.5.0?💡 Result:
The latest stable version of ioredis is v5.5.0, released on February 7, 2025[1][8]. This release focuses on performance improvements and stability enhancements while maintaining backward compatibility with minimal breaking changes[1][2].
Known Issues in v5.5.0
No major issues are explicitly documented in the release notes or changelogs for this version[1][2][8]. However, developers should note:
ESM Import Syntax: While v5.x introduced support for ESM imports via
import { Redis } from 'ioredis', older syntax (import Redis from 'ioredis') remains valid but will be deprecated in future major versions[1].Cluster Reconnection: Some edge cases in cluster mode may require manual handling of transient connection errors during failover scenarios[8].
Key Features in Recent Versions
- Native Promise support (no Bluebird dependency)[2]
- Improved TypeScript declarations[2][8]
- Redis v7 command support[2]
- Enhanced auto-pipelining performance[2]
For production environments using Redis clusters or sentinels, ensure your retry strategy accounts for AWS ElastiCache's specific failover behaviors when using
reconnectOnErrorconfigurations[8].Citations:
- 1: https://github.com/redis/ioredis/releases
- 2: https://github.com/redis/ioredis/blob/main/CHANGELOG.md
- 3: https://github.com/redis/node-redis/blob/master/CHANGELOG.md
- 4: https://www.npmjs.com/package/ioredis/v/1.0.11
- 5: https://unpkg.com/browse/ioredis@4.9.4/Changelog.md
- 6: https://tyk.io/docs/developer-support/release-notes/gateway/
- 7: https://ably.com/blog/migrating-from-node-redis-to-ioredis
- 8: https://www.npmjs.com/package/ioredis
- 9: https://docs.streamsets.com/platform-datacollector/latest/datacollector/UserGuide/ReleaseNotes/ReleaseNotes.html
- 10: https://github.com/redis/ioredis
Version 5.5.0 Confirmed: ioredis is Up-to-Date
The inclusion of
ioredis: ^5.5.0in the workspace catalog has been verified. Version 5.5.0 is the latest stable release, and no major issues are documented. This change ensures consistent versioning across the monorepo and meets our Redis functionality requirements. Note that while there are minor deprecation notes regarding ESM syntax and some cluster reconnection edge cases, these do not impact our current usage..vscode/settings.json (1)
54-57: SonarLint integration looks goodThe addition of SonarLint connected mode configuration will help enforce consistent code quality standards across the project.
apps/web-app/.env.example (1)
17-18:❓ Verification inconclusive
Redis configuration setup looks good
The Redis URL configuration is properly documented as being used for both database and cache functionality, which represents a significant architectural choice.
Since this represents a shift to using Redis as the main database, ensure this is adequately documented in your project:
🏁 Script executed:
#!/bin/bash # Check if there's any migration documentation for this database change fd -t f "migration|README|CHANGELOG" -E node_modules # Check if there are any existing database implementations that might need updating rg -t ts "useStorage\\(\\)\\.getItem\\(" --no-ignore-vcsLength of output: 178
Attention: Verify Documentation Updates for Redis as Main Database
The Redis URL configuration in
apps/web-app/.env.examplenow sets Redis as the primary database and cache. The configuration itself looks good; however, since this is an architectural shift, please double-check that the project documentation reflects this change. In particular:
- Verify that files like
CHANGELOG.md,README.md, and any migration guides inpackages/core/README.mdorpackages/ui/README.mdinclude notes or instructions regarding the switch to Redis.- Confirm that no outdated database implementations (e.g., code using
useStorage().getItem() remain in the codebase.Given the current automated searches produced limited output, a manual review of the documentation is recommended.
apps/web-app/nuxt.config.ts (2)
12-12:❓ Verification inconclusive
Verify impact of channelId change
Changing the channelId from 'burger' to 'web-app' might impact how the application identifies itself or references data.
Make sure this change is consistent with any related code:
🏁 Script executed:
#!/bin/bash # Check for references to the old channelId value rg -t ts -t vue "burger" --no-ignore-vcs # Check for places where channelId is used that might need updating rg -t ts -t vue "channelId" --no-ignore-vcsLength of output: 140
Double-check channelId Update Consistency
The change from'burger'to'web-app'inapps/web-app/nuxt.config.tsmust be verified for consistency across the codebase. The initial search encountered an error due to the unrecognized file type "vue." Please re-run the above commands (which now correctly add Vue as a recognized type) to confirm that no outdated references to'burger'remain and that allchannelIdusages are consistent.
🏁 Script executed:
#!/bin/bash # Re-run search for references to the old channelId value "burger" rg --type-add 'vue:*.vue' -t ts -t vue "burger" --no-ignore-vcs # Re-run search for usages of "channelId" rg --type-add 'vue:*.vue' -t ts -t vue "channelId" --no-ignore-vcsLength of output: 11071
Action Required: Verify & Align
channelIdConfiguration Across Modules
- In
apps/web-app/nuxt.config.ts, thechannelIdhas been updated to'web-app'as expected.- However, the search reveals that
packages/core/nuxt.config.tsstill setschannelIdto'burger'.- Please verify whether this divergence is intentional. If these environments should use a unified channel identifier, update the configuration in
packages/core/nuxt.config.tsaccordingly.- Additionally, review downstream usages (e.g., repository functions and API endpoints consuming
channelIdviauseRuntimeConfig()) to ensure they behave as expected with the new value.
13-16:✅ Verification successful
Check for impacts from locale removal
The locale property has been removed from the public runtime config, which aligns with the removal of NUXT_PUBLIC_LOCALE from .env.example.
Make sure this doesn't break locale-dependent functionality:
🏁 Script executed:
#!/bin/bash # Check for references to the public locale configuration rg -t ts -t vue "config\\.public\\.locale" --no-ignore-vcs rg -t ts -t vue "useRuntimeConfig\\(\\)\\.public\\.locale" --no-ignore-vcsLength of output: 187
Below is a revised shell script that uses glob patterns instead of file types so that we include Vue files in the search. Please execute this script and verify that no references to the removed locale configuration exist:
Once verified, let us know if you still see any references so we can reassess the potential locale-dependent functionality impacts.
🏁 Script executed:
[run_scripts] #!/bin/bash echo "Searching for references to 'config.public.locale' in TS and Vue files..." rg -g "*.ts" -g "*.vue" "config\.public\.locale" --no-ignore-vcs echo "Searching for references to 'useRuntimeConfig().public.locale' in TS and Vue files..." rg -g "*.ts" -g "*.vue" "useRuntimeConfig\(\)\.public\.locale" --no-ignore-vcsLength of output: 619
Locale Configuration Removal Verified
The search for references to
config.public.localeanduseRuntimeConfig().public.localein both TS and Vue files returned no results. This confirms that removing the locale property from the public runtime configuration (and consequentlyNUXT_PUBLIC_LOCALEfrom the environment) does not leave any lingering dependencies or break locale-dependent functionality.No further changes are required for this removal.
packages/core/server/api/warehouse/index.post.ts (3)
1-3: Good adoption of service pattern importsThe imports now correctly reflect the transition from repository pattern to service-oriented architecture, which aligns with the overall DB restructuring of this PR.
13-16: Appropriate warehouse creation implementationThe warehouse creation now uses
createWarehousefunction with a unique ID generated viacreateId(). This is a cleaner approach than using repository methods directly.
18-19: Good data consistency practiceAdding
setChannelAsUpdatedensures the channel is marked as updated after warehouse creation, maintaining data integrity across related entities.packages/core/server/api/user/[id]/index.get.ts (2)
1-1: Clean service function importThe import now correctly uses the service function rather than the repository, aligning with the overall architecture transition.
13-13: Appropriate repository replacementThe user retrieval now uses the
getUserservice function instead ofrepository.user.find, maintaining the same functionality while following the new architectural pattern.packages/core/server/services/db/index.ts (1)
1-8: Well-structured Keys interfaceThis interface provides a clear contract for the different types of keys in the database, which helps with type safety and code organization.
packages/core/shared/services/workingDay.ts (1)
7-11:❓ Verification inconclusive
Breaking schema change - ensure consumers are updated
The change from a record-based structure to an array-based structure for working days is more flexible but represents a breaking change. This shift requires updates to all code that consumes this schema.
🏁 Script executed:
#!/bin/bash # Check for places that might be using the old schema structure rg -A 10 "workingDaysUpdateSchema" --type tsLength of output: 2374
Action: Confirm all consumers handle the new array schema correctly
The schema for working days has been refactored from a record-based structure to an array-based one. This is a breaking change, so please ensure that every consumer of
workingDaysUpdateSchemahas been updated accordingly. For example, in the file atpackages/core/server/api/channel/working-day/index.patch.ts, the schema is used to parse the request body and then iterated over as an array. Verify that all other usages expect an array and that the client-side code (or any other upstream consumers) is also updated to send data in the new format.
- Review in
packages/core/server/api/channel/working-day/index.patch.ts: It usesworkingDaysUpdateSchema.parse(body)and correctly iterates over the resulting array. Confirm that similar adaptations exist elsewhere.- Ensure tests are updated: Any tests or integrations relying on the old record structure should now validate against the new array structure.
apps/web-app/app/pages/catalog/[categorySlug]/index.vue (2)
11-11: Improved product iteration and navigation with category slug.The changes to the ProductCard component usage are well-implemented. Moving from direct category filtering to using a pre-filtered products array improves template readability, and adding the category slug enables proper URL construction in child components.
Also applies to: 14-14
32-33: Good separation of concerns.Moving the product filtering logic from the template to the script section is a solid improvement that follows Vue best practices. The filtering criteria correctly ensures that only available products with variants are displayed.
packages/core/server/api/channel/index.patch.ts (1)
1-1: Service-oriented refactoring improves modularity.The migration from direct repository usage to a service function approach enhances code organization and follows a more maintainable architectural pattern. The explicit type casting to
Partial<Channel>also adds clarity about the expected data structure.Also applies to: 17-17
packages/core/server/api/channel/payment-method/[id].delete.ts (1)
1-2: Improved separation of concerns with service functions.The refactoring to use dedicated service functions (
deletePaymentMethodandsetChannelAsUpdated) instead of direct repository access enhances modularity and maintainability. The addition of channel update notification ensures data consistency across the system.Also applies to: 6-6, 15-16
apps/web-app/app/components/DeliveryInfoBlock.vue (1)
25-25: Simplified time formatting logic.The refactoring to use the pre-formatted
closeproperty directly rather than manually constructing the time string reduces code complexity. This change is more maintainable and less error-prone, assuming theworkingDay.closeproperty is properly formatted.apps/web-app/app/components/CommandCenter/ProductCard.vue (1)
46-46: Improved null safety with additional optional chainingAdding the optional chaining operator to
variantsis a good defensive programming practice. It ensures the code gracefully handles cases wherevariantsmight be undefined, preventing potential runtime errors.packages/core/server/api/product/index.post.ts (3)
1-3: Good architectural shift to dedicated service functionsThe change in imports properly separates concerns by:
- Using a standardized external library (
@paralleldrive/cuid2) for ID generation- Importing specific service functions for database operations instead of using a repository directly
This refactoring improves modularity and maintainability.
13-20: Enhanced product creation with explicit default valuesThe implementation now properly initializes all required product fields with appropriate default values:
isAvailableForPurchase: truemediaId: nullvariants: []This makes the contract between the API and service layer clearer and more explicit.
22-22: Consistent usage of service function for channel updatesUsing the dedicated
setChannelAsUpdatedservice function maintains consistency with other changes in this PR and centralizes the channel update logic.apps/web-app/app/pages/catalog/[categorySlug]/[productSlug]/index.vue (1)
137-138: Improved category retrieval logicUsing
channel.getMenuCategoryByProductto retrieve the category centralizes the logic and follows the service-oriented approach of this PR.packages/core/server/api/product/[id]/index.patch.ts (3)
1-2: Good separation of concerns with dedicated service importsImporting specific service functions for database operations enhances code maintainability and readability.
20-20: Consistent usage of product service functionUsing the dedicated
patchProductservice function aligns with the architectural changes in this PR and improves separation of concerns.
22-22: Consistent usage of channel service functionUsing the dedicated
setChannelAsUpdatedservice function maintains consistency with other changes and centralizes the channel update logic.packages/core/server/api/product/[id]/index.delete.ts (2)
1-2: Good architectural improvement: Moving to service-oriented design.The shift from direct repository calls to service functions creates better separation of concerns and encapsulates database operations in dedicated service modules.
17-18:❓ Verification inconclusive
Refactoring looks good, verify service implementation details.
The code properly replaces direct repository calls with service functions. This change improves maintainability and testability of the codebase.
Ensure the service implementations properly handle:
- Error conditions
- Transaction management (if multiple operations need to be atomic)
🏁 Script executed:
#!/bin/bash # Check implementation of the deleteProduct and setChannelAsUpdated services echo "Checking deleteProduct implementation:" fd "deleteProduct" --type file --exec grep -A 10 "deleteProduct" {} \; echo "Checking setChannelAsUpdated implementation:" fd "setChannelAsUpdated" --type file --exec grep -A 10 "setChannelAsUpdated" {} \; echo "Checking for transaction handling:" fd "transaction|tx" --type file --exec grep -A 5 -B 5 "transaction|tx" {} \;Length of output: 502
Manual Verification Required for Service Implementations
The refactoring looks good as it replaces direct repository calls with service functions. However, our automated checks did not return any evidence of the implementations of
deleteProductandsetChannelAsUpdated, nor did they confirm transaction handling. Please manually verify that the service implementations:
- Properly handle error conditions.
- Include transaction management where multiple operations need to be atomic.
packages/core/server/api/product/variant/[id].delete.ts (2)
1-2: Good architectural change: Moving to service-oriented design.The shift from direct repository access to service functions improves separation of concerns and code maintainability, aligning with the changes in other API endpoints.
16-17:❓ Verification inconclusive
Implementation looks good, verify service implementation details.
The refactoring from direct repository calls to service functions is consistent with the changes in other endpoints and improves the architecture of the codebase.
Ensure the service implementations properly handle:
- Error conditions
- Transaction management (if multiple database operations need to be atomic)
🏁 Script executed:
#!/bin/bash # Check implementation of the deleteProductVariant service echo "Checking deleteProductVariant implementation:" fd "deleteProductVariant" --type file --exec grep -A 10 "deleteProductVariant" {} \; echo "Checking if deleteProductVariant handles any product relationships:" fd "deleteProductVariant|productVariant" --type file --exec grep -B 10 -A 10 "delete.*ProductVariant|productVariant.*delete" {} \;Length of output: 467
Please Manually Verify Service Error Handling & Transaction Management
The refactoring toward service functions in this endpoint is consistent and improves the codebase architecture. However, our automated checks did not yield sufficient details on the internal implementation of thedeleteProductVariant(and by extension,setChannelAsUpdated) service functions. Please manually review their implementations to confirm that they correctly handle:
- Error conditions (e.g., proper try/catch usage and error propagation)
- Transaction management when multiple database operations must be atomic
packages/core/server/api/warehouse/[id]/index.patch.ts (2)
1-2: These new imports align well with the service-based refactor.
Adopting thepatchWarehouseandsetChannelAsUpdatedservices helps centralize your logic and improves maintainability.
7-7: ValidatechannelIdand handle service errors carefully.
RetrievingchannelIdfrom the runtime config is fine; however, consider gracefully handling scenarios wherechannelIdmight be missing or invalid before callingsetChannelAsUpdated(channelId). Additionally, confirm thatpatchWarehousethrows (or handles) descriptive errors when invalid inputs are passed.Would you like a script to verify that
channelIdis consistently defined in your environment?Also applies to: 19-19, 21-21
apps/web-app/app/pages/command-center/system/index.vue (2)
88-88: Watch out for non-unique keys.
UsingworkingDay.dayas the key may cause collisions if multiple entries share the same day. Ensure eachdayfield is guaranteed unique across the array.
91-91: Cleaner prop usage.
Directly passingworkingDay.daysimplifies usage. This is fine as long as every workingDay object has a validdayproperty.packages/core/server/api/channel/payment-method/[id].patch.ts (2)
1-2: Service-based approach looks consistent.
Switching from the repository topatchPaymentMethodandsetChannelAsUpdatedkeeps your codebase aligned with the new architecture.
7-7: ConfirmchannelIdvalidity.
IfchannelIdis undefined or invalid,setChannelAsUpdated(channelId)could fail silently or throw. Consider adding a safeguard or fallback for missing values.Also applies to: 19-20
packages/core/server/services/db/media.ts (2)
1-3: Consider validating the media ID.
Although this logic is straightforward, ensure there’s always a valididinput. Ifidis incorrectly formatted or empty, this function could silently returnnull.
14-16: Confirm whether a non-existent item needs special handling.
If the item does not exist, this function will silently succeed. Verify whether you need to throw or otherwise handle the not-found case.packages/core/server/api/product/variant/[id].patch.ts (3)
1-2: Service-based approach looks clean.
Importing these dedicated service functions rather than calling repository methods directly helps maintain separation of concerns.
20-20: Ensure proper handling if the patch operation returns nothing.
Double-check thatpatchProductVariant(id, data)always returns a valid result. If it can returnnull, consider throwing an error or handling it gracefully.
22-22: Channel update looks good.
Marking the channel as updated makes sense here, and the surroundingtry/catchhandles potential failures.packages/core/server/api/channel/working-day/activity.post.ts (4)
1-2: Aligns with the service-based architecture.
These imports are consistent with the broader shift away from repository calls, improving readability and maintainability.
18-19: Distinguish between no data vs. empty array.
IfgetWorkingDays()returns an empty array (rather thannull), the code will still skip throwing. Confirm that is the intended behavior.
26-27: Double-check behavior if the day is not found.
You’re toggling!day?.isActive, which defaults totruefor a missing day. Decide whether you want to create a new entry, throw an error, or keep this behavior.
29-29: Channel update is appropriate.
Great job ensuring the channel state is marked as updated after changing working day activity. The error handling via the parenttry/catchis sufficient.packages/core/server/api/channel/receiving-method/index.post.ts (3)
1-1: LGTM: Clean service import.The refactoring from repository pattern to service functions looks good, with clear imports of the specific functions needed.
17-17: LGTM: Successfully migrated to service function.The code has been correctly updated to use the new
getChannelservice function.
20-22: LGTM: Update operations properly refactored.The channel update operations have been successfully migrated to use the
patchChannelservice function while maintaining the same functionality.Also applies to: 26-28
apps/web-app/app/components/SearchBlock.vue (2)
3-3: Added null safety with optional chaining.Good defensive programming practice to prevent potential runtime errors if
searchQueryis null or undefined.
41-41: Improved separation of concerns.Moving the top results logic to the channel store (
getTopSearchedProducts()) instead of directly manipulating the data in the component is a better architectural approach.apps/web-app/app/components/ProductCard.vue (3)
45-48: LGTM: Added categorySlug prop.Adding the categorySlug as a required prop improves component reusability and makes data flow clearer.
54-55: Verify variant selection behavior change.The logic for selecting the smallest variant has changed. Previously, it likely found the variant with the lowest price, but now it always selects the first variant in the array. This could change the displayed price for products with multiple variants.
Make sure this change aligns with the intended business logic for displaying product prices.
60-60: LGTM: Improved URL construction.Using the provided categorySlug prop directly for URL construction is more direct and reliable than extracting it from the product object.
apps/web-app/app/pages/command-center/product/[id]/index.vue (1)
141-143: Good refactoring of data access pattern.Moving from direct property access to a service method improves maintainability and decouples the component from the underlying data structure.
apps/web-app/app/components/CategoryBlock.vue (1)
22-22: Template now uses precomputed products list.Moving the filtering logic from the template to the script section improves readability and performance. The addition of the category-slug prop to ProductCard is consistent with the data access pattern changes.
Also applies to: 25-25
packages/core/server/api/product/variant/index.post.ts (3)
1-3: Good architectural change to service-based approach.The transition from repository patterns to service functions improves code organization and maintainability.
12-21: Explicit default values improve code clarity.Setting explicit default values for all fields ensures consistent data structure. The use of
createId()for generating unique IDs is a good practice.One suggestion - consider using nullish coalescing for all properties consistently:
const variant = await createProductVariant({ ...data, id: createId(), sku: data.sku ?? null, net: data.net ?? null, calories: data.calories ?? null, protein: data.protein ?? null, fat: data.fat ?? null, carbohydrate: data.carbohydrate ?? null, })
23-23: Good separation of concerns with channel update.Moving the channel update logic to a dedicated service function improves code organization.
packages/core/server/api/channel/working-day/index.patch.ts (2)
1-2: Good architectural change to service-based approach.The import of dedicated service functions improves code organization and follows the same pattern as other changes in this PR.
22-22: Good separation of concerns with channel update.Moving the channel update logic outside the loop and to a dedicated service function is a good practice.
packages/core/server/api/channel/payment-method/index.post.ts (1)
1-3: Good use of service modules for improved architecture.The imports demonstrate a shift from a repository pattern to a service-oriented architecture, which is a good design decision for maintainability.
packages/core/server/services/db/work.ts (1)
21-37: Immutable updates are correctly implemented.Good job implementing immutable updates by creating a new array with the updated day instead of modifying the existing array directly. This helps prevent unexpected side effects.
apps/web-app/app/pages/checkout/index.vue (1)
2-220: Use<template>for cleaner conditional rendering, but confirm maintenance state handling.Switching to
<template v-if>is a good move to avoid adding extra DOM elements. However, consider whether you intend to show a distinct message or UI whenchannel.isOnMaintenance. Currently, the same "cart empty" screen is displayed in that scenario, which might confuse users.packages/core/server/api/channel/master.put.ts (3)
1-3: Imports appear valid.The new import statements for
createId,createUser,createUserCredentials, andgetMasterlook consistent and properly structured. No concerns here.
17-17: Validate whether channel-specific master retrieval is required.
const master = await getMaster()might be insufficient if you ever support multiple channels. Verify that retrieving a single global master user is correct. Otherwise, consider scoping the master to achannelId.
28-38: Confirm active-but-unconfirmed user logic.Creating the master user with
isActive: trueyetisConfirmed: falsemay be intentional, but can be confusing. Additionally, settingnullcould cause issues in areas expecting a valid email. Double-check flows that rely on an email or a confirmed status.packages/core/server/services/db/payment.ts (3)
1-3:getPaymentMethodhandles single retrieval well.The function is straightforward, but note it returns
nullif not found. Be sure callers handle the null case properly.
30-45:patchPaymentMethodmerges partial updates cleanly.Merging existing fields with
...datais good. Ensure thatupdatedAtis consistently used by the rest of the codebase for out-of-band sync or auditing, if needed.
47-49:deletePaymentMethodstraightforwardly removes by key.Method looks good for your storage approach. Confirm whether failing to find a method is an error or not. Right now, removing a non-existent key silently succeeds.
apps/web-app/app/components/Form/UpdateWorkingDays.vue (2)
3-16: Validate existence of day.index and potential off-by-one issues.You're indexing
workingDaysusingday.indexin the template (v-model="workingDays[day.index].open"andv-model="workingDays[day.index].close"). Ensure eachdayobject actually has a valid, zero-basedindexproperty in sync with the array. Otherwise, you might encounter out-of-range errors or unintentional data mutations.
65-65: Ensure body is a plain object or array instead of a Ref.When passing
body: workingDaysto$fetch, you're passing a Ref object. If your framework does not automatically unwrap Refs, consider usingworkingDays.valueto send a plain array as the request body (unless you explicitly rely on auto-unwrap behavior).packages/core/server/services/db/user.ts (3)
7-9: Clarify usage of getUserCredentials.Similar to getUser, this also returns
nullif not found. Ensure callers handle the null case appropriately (e.g., throwing an error or returning a 404 to the client).
34-41: Check for overwrites or unique constraints when creating a new user.The code does not guard against overwriting an existing user ID in storage. If you rely on globally unique IDs, ensure collisions are handled or validated before writing to storage.
43-50: Add error handling or duplication checks for createUserCredentials.Similar to createUser, there's no check for existing credentials. Consider whether duplicates with the same ID should be disallowed or gracefully handled.
packages/core/server/api/channel/index.put.ts (2)
1-2: Imports look good.The shift from repository calls to service-based functions (
getChannel,createChannel, andcreateWorkingDays) aligns well with a service-oriented architecture.
16-16: Verify channel existence logic.Early-stopping if a channel already exists is reasonable. Ensure you handle scenarios where the
channelIdmight be malformed or not string-based, although the existing check is fine for standard usage.packages/core/server/api/product/[id]/image.post.ts (6)
3-5: Code architecture improvement with service functionsThe refactoring from repository pattern to dedicated service functions improves code organization and maintainability. Each service function has a clear purpose, making the code easier to understand and test.
75-75: Function usage looks goodCorrectly replaced repository.media.create with the new createMedia service function.
77-77: Function usage looks goodCorrectly replaced repository.product.find with the new getProduct service function.
85-85: Function usage looks goodCorrectly replaced repository.media.delete with the new deleteMedia service function.
88-88: Function usage looks goodCorrectly replaced repository.product.patch with the new patchProduct service function.
90-90: Function usage looks goodCorrectly replaced repository.channel.setAsUpdated with the new setChannelAsUpdated service function.
packages/core/server/api/channel/time-slots.get.ts (5)
2-3: Clean service-based importsGood choice to import specific service functions rather than importing the entire repository, improving code clarity.
15-15: Function usage looks goodCorrectly replaced repository.channel.find with the new getChannel service function.
36-40: Improved time handling with string parsingThe transition to a string-based time format (e.g., "10:30") and parsing it is a more standard approach than using separate hour and minute fields. This makes the data format more human-readable and consistent with common time representations.
42-45: Consistent approach for close timeGood consistency in handling both open and close times with the same string parsing approach.
48-48: Updated conditional for midnight handlingCorrectly updated to use timeCloseHours variable to match the new time format.
packages/core/server/api/channel/index.get.ts (6)
2-9: Well-organized service importsThe imports are now more specific and targeted to the service functions needed, which improves maintainability and provides better clarity on dependencies.
35-35: Function usage looks goodCorrectly replaced repository.channel.find with the new getChannel service function in the shouldInvalidateCache method.
54-54: Function usage looks goodCorrectly replaced repository.channel.find with the new getChannel service function in the main handler.
62-67: Enhanced data retrieval with service functionsGood use of service functions to fetch related entities. This approach centralizes data access logic in the service layer.
69-74: Simplified working day identificationThe code now fetches all working days once and then filters by index, which is cleaner than making separate calls for each working day.
77-86: Enriched response dataThe return object now includes more data (menus, products, warehouses, paymentMethods), providing a more comprehensive context for the client. This could reduce the number of API calls needed from the frontend.
packages/core/server/services/db/product.ts (3)
3-5: Clean and concise implementationThe getProduct function is straightforward and follows good separation of concerns.
51-66: Consistent pattern for update operationsGood pattern of fetch-modify-save-return for both product and variant updates. The code correctly handles the case where the entity doesn't exist and updates the timestamp.
Also applies to: 110-125
68-75: Consistent pattern for create operationsGood pattern for creating both products and variants with proper timestamp handling.
Also applies to: 127-134
apps/web-app/stores/channel.ts (15)
7-9: Check for multi-category product scenario.
Currently,ProductWithCategoryassociates only oneMenuCategoryper product. If products can belong to multiple categories, consider using an array forcategoryor handling the multi-category use case elsewhere.
27-29: Configuration refs look good.
DeclaringworkingDays,menus, andproductswith clear types enhances readability and maintainability.
34-37: Computed properties are straightforward.
These computed values improve clarity for commonly accessed store data.
42-73: Validate the fetched data shape or consider partial updates.
This function relies on multiple properties indata. If the backend's contract changes or the response is partial, potential undefined errors could appear.Would you like a script to scan for any usage of these properties that might be missing null checks?
75-86: Consistent time-slot fetching.
Error handling aligns well with early return on emptydata. Implementation looks good.
87-89: Simple getter is fine.
Returning aComputedRef<Menu | undefined>ensures reactive updates.
90-92: No issues with category lookup.
Returning a computed reference is aligned with the store’s reactive nature.
96-98: Active category retrieval lgtm.
Returning a computed reference matches the pattern used in other get functions.
99-101: Slug-based category retrieval is consistent.
Implementation aligns with prior getters—no issues found.
102-103: Direct product finder is fitting.
Simple logic, minimal overhead.
105-107: Slug-based product lookup is good.
No concerns with the straightforward find approach.
108-109: Variant-to-product relationship is clear.
Looks correct for bridging a product variant ID to its product.
112-113: Aggregation approach for product variants is valid.
Flattening variants is a common approach—no issues found.
150-153: Category-based product filter is clean.
Using a computed ref ensures reactivity for the displayed products.
189-198: Exporting new methods for global usability.
Adding these references in the returned object makes them straightforward to consume across the app.packages/core/types/index.d.ts (5)
4-5: Verify string-based timestamps.
Switching fromDatetostringmight require parsing or time-zone handling wherever these timestamps are used. Ensure consistency across the codebase.Also applies to: 60-61, 97-98, 116-117, 199-200, 213-214
39-42: SimplifiedWorkingDaystructure.
The newindex,day,open, andcloseproperties are easier to maintain. Verify that legacy usage of removed properties (e.g.,createdAt) is fully updated.
46-46:DayCodeenumeration.
A clear type for days of the week—this improves type safety.
80-80:products: { id: string }[]field.
Storing just the product ID might suffice for referencing. If you need more product data here, consider a nestedProductor a dedicated relationship type.
206-206: Expanded user permissions.
Storing an array ofPermissionCodeoffers flexible role-based access control.
| @@ -16,6 +16,7 @@ export default defineNuxtConfig({ | |||
| accessKeyId: '', | |||
| secretAccessKey: '', | |||
| }, | |||
| redisUrl: '', | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Redis integration added to runtime configuration.
The addition of redisUrl to the runtime configuration enables Redis connection support. This is a significant architectural change that aligns with the PR's database focus.
Make sure you've also updated documentation to reflect this new configuration option. Users will need to know how to properly configure this for different environments.
🏁 Script executed:
#!/bin/bash
# Check if there's documentation for the new Redis configuration
fd -t f "README|\.md$" | xargs grep -l "redisUrl\|REDIS_URL" || echo "No documentation found for Redis configuration"Length of output: 165
Documentation Update Required: Add Redis Configuration Guidance
The integration of redisUrl into the runtime configuration has been approved and aligns with the PR's database focus. However, the verification indicates that the new Redis configuration option is not documented. Please update the relevant documentation (e.g., README.md or other markdown files) to explain how users should configure redisUrl (or REDIS_URL) for various environments.
- Ensure the documentation includes instructions on setting the configuration for different deployment scenarios.
- Update any existing configuration guides to include this Redis integration.
| @@ -18,6 +18,7 @@ | |||
| "@next-orders/database": "workspace:*", | |||
| "@next-orders/ui": "workspace:*", | |||
| "@pinia/nuxt": "catalog:", | |||
| "ioredis": "catalog:", | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Added ioredis client dependency.
The addition of ioredis provides the application with a Redis client, which is consistent with the Redis URL configuration added in the Nuxt config.
Check that the Redis implementation has proper error handling:
🏁 Script executed:
#!/bin/bash
# Look for Redis client implementation
fd -t f ".ts$|.js$" | xargs grep -l "new Redis\|createClient" | xargs grep -l "try\|catch\|error" || echo "Potential missing error handling in Redis client implementation"Length of output: 236
Action Required: Enhance Redis Client Error Handling
The addition of the ioredis dependency is consistent with the Redis URL configuration in the Nuxt config. However, our verification indicates that the Redis client implementation is missing proper error handling (i.e., try/catch or error event listeners around the client instantiation/use).
- Issue Identified: No try/catch blocks or other error handling mechanisms were found where the Redis client is initialized.
- Recommendation: Review the Redis client instantiation code and add appropriate error handling. For instance, wrap the client creation and connection logic within try/catch blocks, and consider handling error events as necessary.
| const breadcrumbs = computed(() => [ | ||
| { label: t('common.home'), icon: 'food:home', to: '/' }, | ||
| { | ||
| label: product.value?.category.name ?? '', | ||
| to: `/catalog/${product.value?.category?.slug}`, | ||
| label: category?.name ?? '', | ||
| to: `/catalog/${category?.slug}`, | ||
| }, | ||
| ]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential issue with missing fallback for category slug
While there's a fallback for the category name with category?.name ?? '', there isn't a similar fallback for the category slug in the URL construction. If category is undefined, the URL would include "undefined" as part of the path.
Consider adding a fallback for the slug:
- to: `/catalog/${category?.slug}`,
+ to: `/catalog/${category?.slug ?? ''}`,📝 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 breadcrumbs = computed(() => [ | |
| { label: t('common.home'), icon: 'food:home', to: '/' }, | |
| { | |
| label: product.value?.category.name ?? '', | |
| to: `/catalog/${product.value?.category?.slug}`, | |
| label: category?.name ?? '', | |
| to: `/catalog/${category?.slug}`, | |
| }, | |
| ]) | |
| const breadcrumbs = computed(() => [ | |
| { label: t('common.home'), icon: 'food:home', to: '/' }, | |
| { | |
| label: category?.name ?? '', | |
| to: `/catalog/${category?.slug ?? ''}`, | |
| }, | |
| ]) |
| const { error } = await useFetch(`/api/user/${user.value.id}`) | ||
| if (error.value) { |
There was a problem hiding this comment.
Potential error with optional chaining removal.
The change removed optional chaining (?.) when accessing user.value.id, which could lead to runtime errors if user.value is undefined. While line 4 seems to check for user.value?.id, a race condition or state change could still make this risky.
- const { error } = await useFetch(`/api/user/${user.value.id}`)
- if (error.value) {
+ const { error } = await useFetch(`/api/user/${user.value?.id}`)
+ if (error.value) {The broadening of the error condition from checking a specific message to just checking for any error is a good change for more robust error handling.
📝 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 { error } = await useFetch(`/api/user/${user.value.id}`) | |
| if (error.value) { | |
| const { error } = await useFetch(`/api/user/${user.value?.id}`) | |
| if (error.value) { |
| const category = channel.getMenuCategoryByProduct(product.value?.id ?? '') | ||
| const menu = channel.menus.find((menu) => menu.categories.some((category) => category.id === category?.id)) | ||
|
|
||
| const menuPageUrl = computed(() => `/command-center/menu/${menu?.id}`) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure menu is defined before constructing URL.
The menu?.id could be undefined if no menu is found, which would result in an invalid URL like /command-center/menu/undefined.
-const menuPageUrl = computed(() => `/command-center/menu/${menu?.id}`)
+const menuPageUrl = computed(() => menu?.id ? `/command-center/menu/${menu.id}` : '/command-center/menu')📝 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 menuPageUrl = computed(() => `/command-center/menu/${menu?.id}`) | |
| const menuPageUrl = computed(() => menu?.id ? `/command-center/menu/${menu.id}` : '/command-center/menu') |
| export async function getWorkingDays(): Promise<WorkingWeek | null> { | ||
| return useStorage('db').getItem<WorkingWeek>('working-time') | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure types and utilities are imported.
The function looks good, but ensure that WorkingWeek type and useStorage utility are properly imported (not visible in this snippet).
🏁 Script executed:
#!/bin/bash
# Check if the WorkingWeek type and useStorage utility are imported
grep -A 5 "import.*WorkingWeek" packages/core/server/services/db/work.ts || echo "WorkingWeek type import not found"
grep -A 5 "import.*useStorage" packages/core/server/services/db/work.ts || echo "useStorage utility import not found"Length of output: 306
Missing Import Declarations for WorkingWeek and useStorage
The current file does not import either the WorkingWeek type or the useStorage utility. Please add the appropriate import statements at the beginning of the file. For example:
- Add an import for
WorkingWeek, e.g.,import { WorkingWeek } from 'path/to/types';
- Add an import for
useStorage, e.g.,import { useStorage } from 'path/to/storage';
Ensure that the import paths match your project’s structure.
| export async function getProductVariant(id: string): Promise<ProductVariant | null> { | ||
| const { productKeys } = await getKeys() | ||
|
|
||
| for (const key of productKeys) { | ||
| const [, productId, variant, variantId] = key.split(':') | ||
|
|
||
| // product:id:variant:id | ||
| if (productId && variant === 'variant' && variantId === id) { | ||
| return useStorage('db').getItem<ProductVariant>(key) | ||
| } | ||
| } | ||
|
|
||
| return null | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize variant retrieval
The getProductVariant function iterates through all product keys, which could be inefficient as the number of products grows. Consider adding a way to directly access variants if you know both product ID and variant ID.
export async function getProductVariant(id: string): Promise<ProductVariant | null> {
- const { productKeys } = await getKeys()
-
- for (const key of productKeys) {
- const [, productId, variant, variantId] = key.split(':')
-
- // product:id:variant:id
- if (productId && variant === 'variant' && variantId === id) {
- return useStorage('db').getItem<ProductVariant>(key)
- }
- }
-
- return null
+ // If we already know the product ID for this variant, we could use:
+ // return useStorage('db').getItem<ProductVariant>(`product:${productId}:variant:${id}`)
+
+ // Without product ID, we still need to search:
+ const { productKeys } = await getKeys()
+
+ // Filter keys first before fetching data
+ const variantKey = productKeys.find(key => {
+ const [, , variant, variantId] = key.split(':')
+ return variant === 'variant' && variantId === id
+ })
+
+ if (variantKey) {
+ return useStorage('db').getItem<ProductVariant>(variantKey)
+ }
+
+ return null
}📝 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.
| export async function getProductVariant(id: string): Promise<ProductVariant | null> { | |
| const { productKeys } = await getKeys() | |
| for (const key of productKeys) { | |
| const [, productId, variant, variantId] = key.split(':') | |
| // product:id:variant:id | |
| if (productId && variant === 'variant' && variantId === id) { | |
| return useStorage('db').getItem<ProductVariant>(key) | |
| } | |
| } | |
| return null | |
| } | |
| export async function getProductVariant(id: string): Promise<ProductVariant | null> { | |
| // If we already know the product ID for this variant, we could use: | |
| // return useStorage('db').getItem<ProductVariant>(`product:${productId}:variant:${id}`) | |
| // Without product ID, we still need to search: | |
| const { productKeys } = await getKeys() | |
| // Filter keys first before fetching data | |
| const variantKey = productKeys.find(key => { | |
| const [, , variant, variantId] = key.split(':') | |
| return variant === 'variant' && variantId === id | |
| }) | |
| if (variantKey) { | |
| return useStorage('db').getItem<ProductVariant>(variantKey) | |
| } | |
| return null | |
| } |
| export async function deleteProduct(id: string): Promise<void> { | ||
| const { productKeys } = await getKeys() | ||
|
|
||
| for (const key of productKeys) { | ||
| const [, productId, variant, variantId] = key.split(':') | ||
|
|
||
| // product:id | ||
| if (productId === id && !variant) { | ||
| await useStorage('db').removeItem(key) | ||
| } | ||
|
|
||
| // product:id:variant:id | ||
| if (productId === id && variant === 'variant' && variantId) { | ||
| await useStorage('db').removeItem(key) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider transaction safety for delete operations
The deleteProduct function doesn't use transactions, which could lead to inconsistent data if an error occurs during deletion of product variants. Consider implementing a way to ensure atomicity (all-or-nothing operations) if your storage mechanism supports it.
🌐 Web query:
Does Redis support transactions for atomic operations?
💡 Result:
Yes, Redis supports atomic operations through transactional commands (MULTI, EXEC, DISCARD, WATCH) and Lua scripting. Here's how it works:
1. MULTI/EXEC Transactions
Redis groups commands into atomic blocks using MULTI (to start) and EXEC (to execute):
-
Atomic Execution: All commands between
MULTIandEXECare queued and executed sequentially as a single unit[2][4]. -
Example:
> MULTI OK > INCR counter QUEUED > SET status "done" QUEUED > EXEC 1) (integer) 1 2) OK
This increments
counterand setsstatusatomically[2]. -
Error Handling:
- Commands with syntax errors (e.g., invalid arguments) abort the transaction before execution[2].
- Runtime errors (e.g., wrong data type) after
EXECdon’t roll back—only failed commands are skipped[2][4].
-
Optimistic Locking with
WATCH:
Monitors keys for changes before committing:WATCH balance balance = GET balance MULTI SET balance $balance-10 EXEC # Fails if "balance" changed after WATCH
If
balanceis modified by another client beforeEXEC, the transaction aborts[2][4].
2. Lua Scripting
Redis executes Lua scripts atomically on the server:
- Guaranteed Atomicity: Scripts run without interruption, combining multiple operations into one[3].
- Example (conditional update):
Executed via
local key = KEYS[1] local value = ARGV[1] if redis.call("GET", key) == value then return redis.call("INCR", "success_count") end return nil
EVALorEVALSHA:// Jedis example in Java String script = "return redis.call('SET', KEYS[1], ARGV[1])"; jedis.eval(script, 1, "key", "value"); // Atomic SET[3]
Key Differences Between Methods
| Feature | MULTI/EXEC | Lua Scripting |
|---|---|---|
| Atomicity | Commands queued then executed[2] | Entire script runs atomically[3] |
| Error Handling | Partial failures possible[2] | All-or-nothing execution[3] |
| Network Overhead | Multiple roundtrips | Single request/response[3] |
| Complex Logic | Limited to queued commands | Supports conditionals/loops[3] |
When to Use Each
- Use MULTI/EXEC for simple command batches with optimistic locking via
WATCH. - Use Lua scripting for complex logic requiring server-side execution or strict atomicity across conditional steps[3][4].
Redis transactions lack traditional rollbacks but provide atomicity through these mechanisms[2][4].
Citations:
- 1: https://docs.servicestack.net/redis/transactions
- 2: https://redis.io/docs/latest/develop/interact/transactions/
- 3: https://codesignal.com/learn/courses/mastering-redis-transactions-and-efficiency-with-java-and-jedis-1/lessons/redis-lua-scripting-for-atomic-transactions-in-java
- 4: https://stackexchange.github.io/StackExchange.Redis/Transactions.html
- 5: https://groups.google.com/g/redis-db/c/KEfJ4Dp58tQ
**Atomic Deletion Consideration for Product and Variant Keys **
The current implementation in packages/core/server/services/db/product.ts (lines 77–93) individually removes each key. This exposes you to possible data inconsistency if an error occurs midway during the deletion process. Since your storage mechanism (Redis) supports transactions via MULTI/EXEC and Lua scripts—which provide atomic execution—it is advisable to wrap these removal operations in a transactional block. This approach would ensure that either all deletions succeed or none do, safeguarding against partial deletion states.
Please review and update the deletion logic to leverage Redis transactional commands (or Lua scripting for stricter atomicity), thereby ensuring all-or-nothing execution of product and variant deletions.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (15)
packages/core/server/api/auth/sign-in.post.ts (1)
21-21: Consider clarifying password field namingThe code now compares against
credentials.passwordrather than the previouscredentials.passwordHash. Even if the value is still a hash, naming it "password" could be misleading to other developers who might assume it contains plaintext.Consider renaming this field to
passwordHashin both the service function and database schema to make it clear that it stores a hashed value, not plaintext.packages/core/server/services/db/index.ts (1)
11-29: Efficient key retrieval and categorization.The
getKeysfunction efficiently retrieves all keys and categorizes them by prefix. This approach simplifies key management throughout the application and supports the new service-oriented architecture.One observation: This implementation will scan all keys in the database on each call. For large databases, consider adding caching with appropriate invalidation if performance becomes an issue.
packages/core/server/services/db/menu.ts (2)
7-49: Menu and categories batch retrieval implementation.The
getMenusfunction efficiently retrieves multiple menus and their categories in a single operation. It uses a Map for storage and handles various edge cases like missing menus or categories.However, there's a potential inefficiency in the nested loop (lines 22-29) that iterates through all keys for each menu to find its categories.
Consider reorganizing the key parsing to avoid the nested loop. You could pre-process all keys to categorize them by type and relationship:
- const menuCategoriesKeys: string[] = [] - for (const k of keys) { - const [, menuId, category, categoryId] = k.split(':') - - // menu:id:category:id - if (menuId === menu.id && category === 'category' && categoryId) { - menuCategoriesKeys.push(k) - } - } + // Assuming we pre-processed keys into a map of menu IDs to category keys + const menuCategoriesKeys = menuCategoryKeysMap.get(menu.id) || []This would require an initial pass to build the map but would avoid O(n²) complexity.
91-104: Menu category retrieval function implementation.The
getMenuCategoryfunction retrieves a category by ID but has to iterate through all menu keys to find it. This could be inefficient for a large number of menus and categories.Consider a more direct key lookup pattern based on category ID, perhaps using a secondary index or a more structured key format:
export async function getMenuCategory(id: string): Promise<MenuCategory | null> { // If you know the menuId, you could use a more direct approach: // return useStorage('db').getItem<MenuCategory>(`menu:${menuId}:category:${id}`) // Alternatively, consider creating a category index: // const categoryIndex = await useStorage('db').getItem<Record<string, string>>('category_index') // if (categoryIndex && categoryIndex[id]) { // return useStorage('db').getItem<MenuCategory>(categoryIndex[id]) // } // Current implementation (potentially inefficient for large datasets) const { menuKeys } = await getKeys() // ...rest of the implementation }packages/core/server/api/checkout/add/index.post.ts (3)
36-59: Validate checkout creation parameters.
When creating a new checkout, ensure that required fields match business requirements (e.g., shippingPrice ≥ 0, time not empty if needed, etc.). Consider applying validations or domain constraints here.
114-114: Consider a dedicated increment function.
Patching the line to increase quantity by 1 is correct. However, a specialized helper (e.g.,incrementCheckoutLineQuantity) could be more expressive and encapsulate additional validations (like maximum quantity).
117-117: Recheck performance for recalculation.
Recalculating the entire checkout after adding a single product line might be suboptimal if the system scales or lines become numerous. Consider batching or deferring this recalculation if performance becomes an issue.packages/core/server/api/checkout/index.patch.ts (3)
33-33: Validate checkout existence before finalizing.
Fetching again ensures the checkout is up-to-date, but consider concurrency or rate-limiting if repeated calls occur in quick succession.
46-50: Ensure patch data alignment with domain logic.
Patchingcheckoutwith fields likechangeconverted to string could have domain implications. Validate that all transformations (e.g. for currency or numeric fields) are consistent throughout the system.
89-89: Consider lazy retrieval vs. single consolidated query.
If the system can unify calls to payment and warehouse keys, consider returning them in a single query or object to reduce overhead and keep logic simpler.packages/core/server/services/db/checkout.ts (5)
3-5: Consider caching or indexing.
getCheckoutRegistryItemsrepeatedly fetches the entire list of checkouts. If usage is frequent, you might optimize by maintaining sorted or cached structures.
11-37: Check for potential concurrency issues.
getLatestFinishedCheckoutsconcurrently queries items from storage, then re-fetches them. If your system grows large, you might face performance bottlenecks or race conditions. A more direct approach (e.g., an indexed structure) could be beneficial.
48-61: Patch approach fosters flexibility.
Merging partial data is a nice pattern. If necessary, add validations to ensure that incorrectly typed partial data does not corrupt the existing checkout structure.
96-100: Finalize with caution.
Setting checkout status to 'FINISHED' might require additional domain-level validations (e.g., verify payment status). Ensure usage is consistent with the rest of the order lifecycle.
131-152: Partial patching fosters code readability.
Updating a specific line with minimal code duplication is beneficial. For advanced concurrency, consider implementing an optimistic concurrency control strategy or versioning to avoid accidental overwrites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
apps/web-app/app/components/CommandCenter/CheckoutLine.vue(2 hunks)apps/web-app/app/pages/checkout/index.vue(4 hunks)apps/web-app/stores/checkout.ts(3 hunks)packages/core/server/api/auth/sign-in.post.ts(3 hunks)packages/core/server/api/category/[id]/index.patch.ts(2 hunks)packages/core/server/api/category/index.post.ts(2 hunks)packages/core/server/api/category/slug/[slug].get.ts(0 hunks)packages/core/server/api/channel/index.patch.ts(1 hunks)packages/core/server/api/channel/index.put.ts(2 hunks)packages/core/server/api/channel/master.put.ts(3 hunks)packages/core/server/api/channel/payment-method/index.post.ts(1 hunks)packages/core/server/api/channel/receiving-method/index.post.ts(1 hunks)packages/core/server/api/channel/time-slots.get.ts(3 hunks)packages/core/server/api/channel/working-day/activity.post.ts(1 hunks)packages/core/server/api/channel/working-day/index.patch.ts(1 hunks)packages/core/server/api/checkout/add/index.post.ts(5 hunks)packages/core/server/api/checkout/id/[id].get.ts(2 hunks)packages/core/server/api/checkout/index.get.ts(2 hunks)packages/core/server/api/checkout/index.patch.ts(6 hunks)packages/core/server/api/checkout/line/[id].post.ts(3 hunks)packages/core/server/api/checkout/list/index.get.ts(1 hunks)packages/core/server/api/health/index.get.ts(2 hunks)packages/core/server/api/menu/[id]/activity.post.ts(2 hunks)packages/core/server/api/menu/[id]/index.patch.ts(2 hunks)packages/core/server/api/menu/index.post.ts(2 hunks)packages/core/server/services/db/checkout.ts(1 hunks)packages/core/server/services/db/index.ts(1 hunks)packages/core/server/services/db/menu.ts(1 hunks)packages/core/server/services/db/receiver.ts(1 hunks)packages/core/server/services/db/user.ts(1 hunks)packages/core/server/services/db/work.ts(1 hunks)packages/core/shared/services/checkout.ts(1 hunks)packages/core/types/index.d.ts(7 hunks)
💤 Files with no reviewable changes (1)
- packages/core/server/api/category/slug/[slug].get.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/core/server/services/db/work.ts
- packages/core/server/api/channel/payment-method/index.post.ts
- apps/web-app/app/pages/checkout/index.vue
- apps/web-app/stores/checkout.ts
- packages/core/server/services/db/user.ts
- packages/core/types/index.d.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (90)
apps/web-app/app/components/CommandCenter/CheckoutLine.vue (5)
6-6: Template now uses computed property for product name.The template now accesses the product name directly from the
productcomputed property instead of through the nested path. This approach is cleaner and follows better separation of concerns.
10-10: Template now references productVariant computed property.Accessing the variant name through the dedicated computed property improves readability and maintainability.
13-13: Updated weight display to use computed property.The template now correctly uses the
productVariantcomputed property to access weight information, providing a more consistent data access pattern throughout the component.
41-41: Simplified totalAmount calculation.Using
line.value.totalPricedirectly is cleaner than calculating it manually from gross price and quantity. This change ensures consistency with backend calculations.
13-13: Missing function reference.The template uses
getWeightLocalizedUnit()but this function isn't defined in the component.Please ensure that this function is imported or defined elsewhere in the component. If it's a global utility, make sure it's properly registered.
packages/core/shared/services/checkout.ts (1)
20-20:✅ Verification successful
Good addition of type definition for better type safety.
The new
CheckoutDraftexport type enhances type safety by providing a concrete type derived from the schema definition. This allows for more consistent usage of checkout data throughout the application and better IDE support.
🏁 Script executed:
#!/bin/bash # Search for places where the new CheckoutDraft type is used echo "Checking usages of the new CheckoutDraft type..." rg "CheckoutDraft" --glob "*.{ts,tsx,vue,js,jsx}" # Check for potential places where the type should be updated echo "Checking for potential places where Partial<Checkout> might need updating..." rg "Partial<Checkout>" --glob "*.{ts,tsx,vue,js,jsx}"Length of output: 1427
CheckoutDraft type integration verified and approved
The new
CheckoutDrafttype is successfully integrated across the codebase and enhances type safety as expected. Our search confirms its usage in the web-app pages, stores, and database repositories, ensuring consistent and improved IDE support.
- Usage verified in:
apps/web-app/app/pages/checkout/index.vueapps/web-app/stores/checkout.tspackages/database/src/types.tsand related repository files- Additional note:
- There's one instance of
Partial<Checkout>inpackages/core/server/services/db/checkout.ts. Please double-check that its usage is intentional and doesn’t conflict with the adoption ofCheckoutDraftelsewhere.Overall, the changes are solid and the type addition is a clear win for type safety.
packages/core/server/api/auth/sign-in.post.ts (5)
2-2: Architectural shift to service functionsThis change represents a transition from a repository pattern to using direct service functions, which aligns with the overall PR objective mentioned in the title "feat!: db changed".
16-16: Service function replaces repository callThe change from repository.userCredential.findByLogin to a dedicated service function follows the new architectural pattern and improves code readability.
26-26: Service function replaces user repository callThe change to using the
getUserservice function is consistent with the new architectural approach and maintains the same functionality.
36-36: Simplified permissions assignmentThe direct assignment of user permissions to the session object is cleaner than the previous approach which may have involved mapping over the permissions array.
2-36:❓ Verification inconclusive
Verify consistent implementation across dependent modules
The architectural change from repositories to services is significant and should be implemented consistently across related modules.
🏁 Script executed:
#!/bin/bash # Check for any remaining repository.user or repository.userCredential references # which might indicate inconsistent architectural patterns echo "Checking for remaining repository.user references:" rg "repository\.user\." --type ts echo "Checking for remaining repository.userCredential references:" rg "repository\.userCredential\." --type tsLength of output: 323
Ensure consistent service usage across modules
The recent search for
repository.userandrepository.userCredentialreferences in TypeScript files returned no matches, indicating that the migration from repositories to services appears to have been applied consistently in the parts of the codebase we checked. However, given the absence of output from the grep-based search and the possibility of variations in reference patterns, please perform a manual review of all dependent modules to ensure that no legacy repository calls remain.
- Verify that all modules previously using repository calls (e.g., for user or user credential access) now correctly use the corresponding service functions.
- Ensure that no alternative patterns or indirect references to repository usage exist.
packages/core/server/api/checkout/id/[id].get.ts (1)
1-1: Repository pattern replaced with service function call.The code has been updated to use a dedicated service function
getCheckoutinstead of directly accessing the repository. This change aligns with the broader shift to a service-oriented architecture mentioned in the PR description.Also applies to: 13-13
packages/core/server/api/menu/[id]/activity.post.ts (1)
1-1: Repository calls replaced with service functions.The code has been updated to use dedicated service functions (
getMenuandsetMenuAsActive) instead of directly accessing the repository. This change is consistent with the database changes mentioned in the PR title.Also applies to: 13-13, 21-21
packages/core/server/services/db/receiver.ts (1)
1-18: New service function implements efficient receiver retrieval.The
getCheckoutReceiversfunction effectively retrieves and filters checkout receivers. Good approach to skip invalid receivers with the continue statement rather than including null values.packages/core/server/services/db/index.ts (1)
1-9: Well-structured Keys interface definition.The interface clearly defines all the necessary key categories that will be used throughout the application.
packages/core/server/api/channel/index.patch.ts (1)
1-1: Architecture shift from repository pattern to service functions.The import has been updated to use a dedicated service function instead of the repository. This is part of a larger architectural change across the codebase to use service functions instead of direct repository access.
packages/core/server/api/checkout/index.get.ts (2)
1-1: Architecture shift from repository pattern to service functions.The import has been updated to use a dedicated service function instead of direct repository access. This is consistent with the architectural changes being made across the codebase.
13-13: Simplified checkout retrieval.The checkout retrieval logic has been simplified by directly using the
getCheckoutservice function. The function no longer needs to validatechannelIdas it's not using the channel repository.packages/core/server/api/checkout/list/index.get.ts (4)
1-1: Architecture shift from repository pattern to service functions.The import has been updated to use a dedicated service function instead of direct repository access. This aligns with the broader architectural changes in the codebase.
5-6: Simplified checkout list retrieval with proper error handling.The checkout retrieval has been simplified by directly calling the service function. The error handling has been updated to check if
checkoutsis falsy rather than checking forchannelId.
8-10: Status code and message have been updated.The error status code is now explicitly set to 404 and the message has been updated to "No checkouts". This provides more accurate error information to the client.
13-13: Direct return of service function result.The function now directly returns the result from the service function rather than making another repository call. This simplifies the code and reduces potential for inconsistencies.
packages/core/server/api/category/[id]/index.patch.ts (3)
1-2: Architecture shift from repository pattern to service functions.The imports have been updated to use dedicated service functions for channel and menu category operations. This is consistent with the architectural changes throughout the codebase.
20-20: Service function now handles menu category patching.The code now uses
patchMenuCategoryservice function instead of directly accessing the repository. The function also captures the returned category for inclusion in the response.
22-22: Channel update now uses service function.The code now uses
setChannelAsUpdatedservice function instead of directly accessing the repository to mark the channel as updated.packages/core/server/api/health/index.get.ts (2)
1-1: Import change aligns with architectural shift from repositories to services.The change from importing the repository directly to using a local service function aligns with the broader architectural changes indicated in the PR title "feat!: db changed".
12-13: Health check implementation changed to use key-based validation.The implementation now checks for the existence of keys rather than using a repository health check method. This appears to be more specific but ensure that
getKeys()properly handles all possible database failure scenarios.Does the
getKeys()function handle connection failures appropriately? Make sure it returnsnullor a falsy value forallKeyswhen the database is unavailable to ensure this health check properly detects all failure modes.packages/core/server/api/channel/working-day/activity.post.ts (4)
1-2: Import statement updates align with new service architecture.The imports have been updated to use service-based functions from the local codebase rather than direct repository access, which is consistent with the database architecture changes in this PR.
12-13: Working day retrieval now uses service function.The implementation now fetches all working days and then filters them in-memory, rather than directly querying for a specific working day by ID.
Consider whether fetching all working days is efficient here. If there are many working days, a direct query for the specific day might be more performant. Does
getWorkingDays()have filtering capabilities that could be used?
20-22: Working day activity toggle logic might have an edge case.The code toggles the activity state of a working day based on its current state, but there's a potential issue if the day is not found.
If
dayis undefined (not found inworkingDays), then!day?.isActivewill betrue, which might not be the intended behavior. Consider adding an explicit check to handle the case when the day is not found.- const day = workingDays.find((workingDay) => workingDay.day === data.day) - await changeWorkingDayActivity(data.day, !day?.isActive) + const day = workingDays.find((workingDay) => workingDay.day === data.day) + if (!day) { + throw createError({ + statusCode: 404, + statusMessage: 'Specific working day not found', + }) + } + await changeWorkingDayActivity(data.day, !day.isActive)
23-23: Channel update notification added.Good addition of
setChannelAsUpdatedto notify that the channel has been modified, which is important for tracking changes and triggering any necessary updates.packages/core/server/api/menu/[id]/index.patch.ts (3)
1-2: Import statements aligned with service architecture.The imports have been updated to use service-based functions rather than direct repository access, which is consistent with the architectural changes in this PR.
7-7: Runtime configuration retrieval added.Added retrieval of
channelIdfrom runtime configuration for use in the channel update notification.
19-21: Menu patching now uses service function with channel update.The implementation has been updated to use the
patchMenuservice function instead of direct repository access, and now includes notifying that the channel has been updated.Does the
patchMenufunction handle validation and error cases in the same way as the previous repository implementation? Ensure consistency in error handling to avoid breaking changes in behavior.packages/core/server/api/menu/index.post.ts (3)
1-3: Import statements updated for service architecture.The imports have been modified to use the ID generation utility and service-based functions rather than direct repository access.
14-20: Menu creation parameters changed.The implementation now sets default values for
isActiveandcategories, and no longer passeschannelIddirectly.Make sure that the
createMenufunction doesn't requirechannelIdanymore. If the menu is supposed to be associated with a channel, verify that this relationship is established correctly through other means or that it's intentionally removed.
22-23: Channel update notification added.Good addition of
setChannelAsUpdatedto notify that the channel has been modified, which is important for tracking changes and triggering any necessary updates.packages/core/server/api/channel/receiving-method/index.post.ts (4)
1-1: Clean service-oriented import.The code now imports specific service functions rather than a repository object, following a service-oriented approach. This is a good architectural change that promotes better separation of concerns.
11-11: Improved data access with service function.Replacing repository access with a dedicated service function improves code maintainability and follows the service-oriented architecture pattern.
14-16: Consistent service function usage for updates.Replacing the repository update with the
patchChannelservice function maintains consistent data access patterns throughout the application.
20-22: Consistent service function usage for updates.Replacing the repository update with the
patchChannelservice function maintains consistent data access patterns throughout the application.packages/core/server/api/category/index.post.ts (3)
1-3: Improved imports with proper dependency management.The code now uses a standard library for ID creation and imports specific service functions for database operations. This is a good practice that follows the principle of using specialized modules for specific tasks.
14-20: Improved category creation with pre-initialized properties.The code now initializes the
productsarray during category creation, which prevents potential null/undefined issues when accessing the property later. This is a good defensive programming practice.
22-22: Consistent service function usage for channel updates.Using a dedicated service function for updating the channel status improves code consistency and maintainability.
packages/core/server/api/channel/working-day/index.patch.ts (3)
1-2: Clean service-oriented imports.The code now imports specific service functions rather than a repository object, following a service-oriented approach.
12-14: Simplified and improved working day update loop.The loop has been simplified to iterate directly over the data array and use a service function, improving readability and maintainability. The complex parsing logic has been moved to the service layer, which is a better separation of concerns.
16-17: Efficient channel update.Moving the channel update outside the loop ensures it only happens once after all working days are updated, improving performance.
packages/core/server/api/checkout/line/[id].post.ts (5)
1-1: Clean service-oriented import.The code now imports specific service functions rather than a repository object, following a service-oriented approach.
26-26: Consistent service function usage for checkout retrieval.Replacing repository access with a dedicated service function improves code maintainability and follows the service-oriented architecture pattern.
51-51: Explicit and improved checkout line update.The code now explicitly sets the new quantity value when incrementing, which is more readable and maintainable than using a specialized increment method.
53-53: Explicit and improved checkout line update.The code now explicitly sets the new quantity value when decrementing, which is more readable and maintainable than using a specialized decrement method.
56-56: Consistent service function usage for checkout recalculation.Using a dedicated service function for recalculating the checkout improves code consistency and maintainability.
packages/core/server/api/channel/master.put.ts (5)
1-4: Dependencies updated to use service functions instead of direct repository access.The file now imports service functions from dedicated modules, which is a good practice as it encapsulates database interactions and promotes better separation of concerns.
23-33: Enhanced user creation with explicit properties and a unique ID.The user creation now uses a service function with more explicit properties and generates a unique ID using
createId(). This is a good practice as it ensures all required fields are properly set.
43-48: Improved user credentials creation with a unique ID.The user credentials creation now uses a service function and generates a unique ID. This approach is better than relying on auto-generated IDs from the database as it provides more control and predictability.
50-50: Channel update notification added.A new function call
setChannelAsUpdated(channelId)has been added to ensure the channel state is refreshed after user creation, which is important for maintaining data consistency.
12-12:❓ Verification inconclusive
Verify channel-specific master user retrieval.
The
getMaster()function no longer takes a channelId parameter, which differs from the previous repository call that was channel-specific.
🏁 Script executed:
#!/bin/bash # Check implementation of getMaster function to ensure it properly handles channel context rg -A 3 "export (async )?function getMaster" --glob "packages/core/server/**/*.ts"Length of output: 450
Attention: Confirm Intended Master User Retrieval Behavior
The implementation in
packages/core/server/services/db/user.tsshows thatgetMaster()now returns a global master user without accepting a channel-specific parameter. In the currentpackages/core/server/api/channel/master.put.tsfile, this global retrieval (viaawait getMaster()) is being used. Please verify that this change in behavior—from a previously channel-specific call to a global call—is intentional for the channel API. If a channel-specific master user is still required, consider updating the implementation accordingly.packages/core/server/api/channel/index.put.ts (4)
1-2: Dependencies updated to use service functions.The imports now use dedicated service functions from specific modules, which improves code organization and maintainability.
10-10: Channel retrieval updated to use service function.The function now uses
getChannel(channelId)instead of directly accessing the repository, which is a good practice for encapsulation.
21-35: Enhanced channel creation with explicit properties.The channel creation now includes more explicit fields with default values, making the intent clearer and ensuring all required fields are set.
38-87: Improved working days creation with a more explicit approach.The previous implementation likely used a loop with a generic day array, whereas this version explicitly defines each day with its properties. This is more maintainable and less error-prone as it clearly shows the configuration for each day.
packages/core/server/api/channel/time-slots.get.ts (6)
2-3: Dependencies updated to use service functions.The imports now use dedicated service functions, which aligns with the architectural changes across the codebase.
9-9: Channel retrieval updated to use service function.Consistent with other files, channel retrieval now uses the service function
getChannel.
36-39: Similar time parsing changes for closing time.The same string parsing approach is used for closing time. This change maintains consistency with the opening time parsing.
42-42: Condition check updated for midnight closing time.The check for midnight closing time now uses the parsed hour value instead of directly accessing a property.
20-21:❓ Verification inconclusive
Working day retrieval approach changed.
Instead of fetching a specific working day by day and channelId, the code now fetches all working days and finds the relevant one by index. This approach may have performance implications if there are many working days.
🏁 Script executed:
#!/bin/bash # Check implementation of getWorkingDays to understand if it's filtering by channelId rg -A 5 "export (async )?function getWorkingDays" --glob "packages/core/server/**/*.ts"Length of output: 613
Actionable: Review Working Day Retrieval Performance and Channel-Specific Filtering
The updated code now retrieves all working days via
getWorkingDays(located inpackages/core/server/services/db/work.ts) and then selects the needed day by matching its index, rather than performing a channel-specific fetch. SincegetWorkingDayssimply fetches the complete working time data from storage without filtering by channelId, please verify that the working time dataset remains limited (e.g., typically seven days). If the dataset could grow large or if channel-specific filtering is important for performance, consider reintroducing direct filtering to maintain efficiency.
- Location:
packages/core/server/api/channel/time-slots.get.tslines 20-21- Function Involved:
getWorkingDays(see implementation inpackages/core/server/services/db/work.ts)
30-34:❓ Verification inconclusive
Time parsing has changed to use string splitting.
The time parsing now extracts hours and minutes from strings in the format "HH:MM" instead of using separate fields. Ensure that the time values are consistently formatted throughout the application to avoid parsing errors.
🏁 Script executed:
#!/bin/bash # Check for any inconsistent time formats in the codebase rg "open\s*=\s*['\"]" --glob "packages/core/server/**/*.ts" | grep -v "[0-9][0-9]:[0-9][0-9]" rg "close\s*=\s*['\"]" --glob "packages/core/server/**/*.ts" | grep -v "[0-9][0-9]:[0-9][0-9]"Length of output: 196
Time Parsing Update Verified – Please Manually Confirm Consistency Across the Codebase
The changes in
packages/core/server/api/channel/time-slots.get.ts(lines 30–34) correctly extract hours and minutes via string splitting using the"HH:MM"format. A regex check for time values using the command:#!/bin/bash rg "open\s*=\s*['\"]" --glob "packages/core/server/**/*.ts" | grep -v "[0-9][0-9]:[0-9][0-9]" rg "close\s*=\s*['\"]" --glob "packages/core/server/**/*.ts" | grep -v "[0-9][0-9]:[0-9][0-9]"did not produce any output, which suggests that no immediate inconsistencies were detected. However, since the automated check returned no results (possibly due to pattern limitations), please perform a manual verification to ensure that all time string assignments across the application strictly follow the
"HH:MM"format to avoid any parsing errors.packages/core/server/services/db/menu.ts (7)
1-2: Importing utility function for key retrieval.The file imports a helper function
getKeysfrom the parent directory, which is good for reusing common functionality.
3-6: Menu retrieval function implementation.The
getMenufunction provides a straightforward way to retrieve a menu by ID, with proper typing and null handling.
51-58: Menu creation function implementation.The
createMenufunction properly handles the creation of a menu with timestamps. It returns the created menu for immediate use, which is a good practice.
60-75: Menu update function implementation.The
patchMenufunction includes proper error handling by checking if the menu exists before updating it. It also preserves all existing properties while updating only the specified ones.
77-89: Menu activation function implementation.The
setMenuAsActivefunction ensures only one menu is active at a time by deactivating all other menus first. This maintains data consistency and prevents having multiple active menus.
106-113: Menu category creation function implementation.The
createMenuCategoryfunction properly handles the creation of a menu category with timestamps. It returns the created category, which allows for immediate use.
115-130: Menu category update function implementation.The
patchMenuCategoryfunction includes proper error handling by checking if the category exists before updating it. It also preserves all existing properties while updating only the specified ones.packages/core/server/api/checkout/add/index.post.ts (3)
1-3: Adopted service-based approach looks good.
The transition from direct repository calls to named service imports promotes modularity and maintainability.
18-18: Add error handling for missing or invalid channel data.
While channel activity checks exist, ifgetChannel(channelId)fails (e.g., returns null due to an invalid ID) before usage, it might cause an unexpected error.
81-81: Confirm concurrency handling.
Calls togetCheckoutcan potentially be stale if multiple services or requests update the same checkout simultaneously. Verify that upstream usage either locks or re-checks relevant data to avoid collisions.packages/core/server/api/checkout/index.patch.ts (8)
2-5: Approach to centralizing business logic is solid.
Replacing direct DB calls with service-layer imports increases flexibility and consistency.
26-26: Handle invalid channel scenarios.
Same as in theaddendpoint, consider verifying thatchannelis not null or undefined after fetching, to avoid potential runtime errors.
28-28: Confirm recalc necessity.
InvokingrecalculateCheckoutbefore finalizing changes is logical, but confirm with business rules if partial updates require an immediate recalculation.
54-54: Audit finalization timing.
Upon callingsetCheckoutAsFinished, confirm that the external dependencies (like payment or inventory) have already cleared. Otherwise, you risk data inconsistencies if called prematurely.
79-79: Validate checkout data once again.
Fetching the checkout after finalization is valuable; ensure you handle an already-finished or invalid checkout gracefully.
84-84: Check channel references in final stage.
If the channel became inactive or changed mid-checkout, confirm if the system needs to handle that scenario carefully.
91-92: Guard for absent payment methods.
If the payment method ID is invalid,paymentMethodNamecould be undefined. Provide a default or handle the missing scenario to avoid potential runtime errors.
94-95: Similar retrieval approach for warehouse.
Like payment methods, confirm that a missing warehouse does not cause unexpected behavior or crashes down the line.packages/core/server/services/db/checkout.ts (5)
7-9: Simple get function is straightforward.
Returningnullfor a nonexistent checkout is a clean approach. Just ensure trying to update a null checkout is handled gracefully downstream.
39-46: Enhance error handling upon creation.
IfuseStorage('db').setItemfails (due to network or other issues), consider capturing or surfacing these errors more explicitly.
63-94: Comprehensive recalculation flow.
Recomputing the line totals is accurate. However, removing lines withquantity <= 0within the same loop modifies the iteration target. This is valid if you rely on immediate side effects, but watch for potential iteration pitfalls (e.g., usingfor ... of ...while mutating).
102-109: Simplicity is good.
Fetching a line from the lines array is straightforward. Just confirm that the line references remain correct if lines are manipulated concurrently by other processes.
154-163: Cleanup logic is appropriate.
Removing a line by filtering is valid. Confirm if there's a need to ensure reference integrity for related entities, e.g., discount codes or stock-limited items, that might link to that line.
| const productVariant = channel.getProductVariant(line.value?.productVariantId ?? '') | ||
| const product = channel.getProduct(productVariant.value?.productId ?? '') |
There was a problem hiding this comment.
Missing computed property reactivity.
These properties are defined as constants but should be reactive computed properties to ensure they update when dependencies change.
Apply this change to ensure proper reactivity:
-const productVariant = channel.getProductVariant(line.value?.productVariantId ?? '')
-const product = channel.getProduct(productVariant.value?.productId ?? '')
+const productVariant = computed(() => channel.getProductVariant(line.value?.productVariantId ?? ''))
+const product = computed(() => channel.getProduct(productVariant.value?.productId ?? ''))📝 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 productVariant = channel.getProductVariant(line.value?.productVariantId ?? '') | |
| const product = channel.getProduct(productVariant.value?.productId ?? '') | |
| const productVariant = computed(() => channel.getProductVariant(line.value?.productVariantId ?? '')) | |
| const product = computed(() => channel.getProduct(productVariant.value?.productId ?? '')) |
| @@ -13,6 +13,8 @@ export const checkoutUpdateSchema = z.object({ | |||
| addressNote: z.string().max(250).optional(), | |||
| paymentMethodId: z.string().max(50).optional(), | |||
| time: z.number().optional(), | |||
| change: z.string().max(10).optional(), | |||
| change: z.number().optional(), | |||
There was a problem hiding this comment.
❓ Verification inconclusive
Breaking change detected: field type changed from string to number.
The change field has been modified from z.string().max(10) to z.number(). This is a breaking change that will affect data validation and processing throughout the application. Ensure that:
- All places where this field is used have been updated to handle a number instead of a string
- Database schemas or APIs consuming this data are updated accordingly
- Frontend components are updated to input numbers rather than strings
🏁 Script executed:
#!/bin/bash
# Search for usages of the 'change' field in the codebase
echo "Searching for usages of the 'change' field in checkout..."
rg -A 3 -B 3 "change.*Checkout" --glob "*.{ts,tsx,vue,js,jsx}"Length of output: 1118
Action Required: Validate 'change' Field Type Transition
The change field in packages/core/shared/services/checkout.ts now uses z.number().optional() instead of the previous z.string().max(10). This change remains breaking and impacts data validation and processing. Our search shows:
- Frontend Usage: In
apps/web-app/app/pages/checkout/index.vue, the field is accessed asremainingCheckout.value.change. Verify that this component now correctly handles a number. - Store Update: In
apps/web-app/stores/checkout.ts, an async function namedchangeexists, suggesting additional logic may need adjustments to work with a numeric type.
Please ensure that all consuming components, database schemas, and API endpoints have been reviewed and updated to accommodate the new numeric type for change.
|
|
||
| const body = await readBody(event) | ||
| const data = channelUpdateSchema.parse(body) | ||
|
|
||
| await repository.channel.patch(channelId, data) | ||
| await patchChannel(channelId, data as Partial<Channel>) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validation for channelId has been removed.
The previous implementation likely checked if channelId exists before trying to patch the channel. Now the code assumes channelId will always be present in the configuration. Ensure that patchChannel service handles the case when channelId is undefined or that this validation happens elsewhere.
🏁 Script executed:
#!/bin/bash
# Check if patchChannel handles validation for channelId
cd packages/core/server/services/db
grep -r "function patchChannel" --include="*.ts" .
# If found, check the implementation to see if it validates channelIdLength of output: 201
Action Required: Validate channelId or Handle Undefined Scenario
The updated code now directly calls:
await patchChannel(channelId, data as Partial<Channel>)which assumes that a valid channelId is always provided. However, the implementation of patchChannel in packages/core/server/services/db/channel.ts has the signature:
export async function patchChannel(id: string, data: Partial<Channel>): Promise<Channel | null> {There’s no evidence within patchChannel that it explicitly validates whether id is defined. Please ensure that one of the following is true:
- The validation for
channelId(i.e., ensuring it is defined and valid) is performed upstream before this call. - Or, the
patchChannelservice is updated to gracefully handle cases wherechannelIdmight beundefined.
This check is important to avoid potential runtime errors if an undefined value is inadvertently passed.
| export async function createCheckoutLine(data: Omit<CheckoutLine, 'createdAt' | 'updatedAt'>): Promise<CheckoutLine | null> { | ||
| const checkout = await getCheckout(data.checkoutId) | ||
| if (!checkout) { | ||
| return null | ||
| } | ||
|
|
||
| await patchCheckout(data.checkoutId, { | ||
| lines: [ | ||
| ...checkout.lines, | ||
| { | ||
| ...data, | ||
| createdAt: new Date().toISOString(), | ||
| updatedAt: new Date().toISOString(), | ||
| }, | ||
| ], | ||
| }) | ||
|
|
||
| return getCheckoutLine(data.checkoutId, data.id) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Creation logic is consistent with patch usage.
Creating a new line then patching the checkout is a neat approach. For high concurrency, consider atomic operations or version checks to prevent overwriting lines with stale data.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/nuxt.config.ts (1)
16-16: Consider adding a sensible default or error handling for Redis connectionThe new
redisUrlconfiguration has an empty string as the default value. If this configuration is required for the application to function correctly after this database change, this could lead to runtime errors when not explicitly set.Consider one of these approaches:
- Provide a sensible default (e.g.,
'redis://localhost:6379')- Implement proper error handling to detect missing configuration
- Make Redis optional with a clear fallback mechanism
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (40)
apps/web-app/package.json(1 hunks)docker/.env.example(0 hunks)docker/docker-compose.yml(0 hunks)eslint.config.mjs(1 hunks)package.json(0 hunks)packages/core/nuxt.config.ts(1 hunks)packages/core/package.json(1 hunks)packages/core/server/api/auth/session.post.ts(1 hunks)packages/core/server/preset.ts(0 hunks)packages/core/server/tasks/db/migrate.ts(0 hunks)packages/core/shared/utils/date.ts(0 hunks)packages/database/.env.example(0 hunks)packages/database/build.config.ts(0 hunks)packages/database/drizzle.config.ts(0 hunks)packages/database/migrations/0000_aspiring_sprite.sql(0 hunks)packages/database/migrations/meta/0000_snapshot.json(0 hunks)packages/database/migrations/meta/_journal.json(0 hunks)packages/database/package.json(0 hunks)packages/database/src/connection.ts(0 hunks)packages/database/src/database.ts(0 hunks)packages/database/src/index.ts(0 hunks)packages/database/src/repository/channel.ts(0 hunks)packages/database/src/repository/checkout.ts(0 hunks)packages/database/src/repository/checkoutLine.ts(0 hunks)packages/database/src/repository/checkoutReceiver.ts(0 hunks)packages/database/src/repository/index.ts(0 hunks)packages/database/src/repository/media.ts(0 hunks)packages/database/src/repository/menu.ts(0 hunks)packages/database/src/repository/menuCategory.ts(0 hunks)packages/database/src/repository/paymentMethod.ts(0 hunks)packages/database/src/repository/product.ts(0 hunks)packages/database/src/repository/productVariant.ts(0 hunks)packages/database/src/repository/user.ts(0 hunks)packages/database/src/repository/userCredential.ts(0 hunks)packages/database/src/repository/userPermission.ts(0 hunks)packages/database/src/repository/warehouse.ts(0 hunks)packages/database/src/repository/workingDay.ts(0 hunks)packages/database/src/tables.ts(0 hunks)packages/database/src/types.ts(0 hunks)packages/database/tsconfig.json(0 hunks)
💤 Files with no reviewable changes (35)
- packages/database/migrations/meta/_journal.json
- packages/database/build.config.ts
- packages/core/server/tasks/db/migrate.ts
- packages/database/package.json
- docker/.env.example
- packages/database/src/repository/userPermission.ts
- package.json
- packages/database/src/repository/checkoutReceiver.ts
- packages/database/src/repository/menuCategory.ts
- packages/database/src/repository/warehouse.ts
- packages/database/.env.example
- packages/database/src/database.ts
- packages/database/drizzle.config.ts
- packages/database/migrations/meta/0000_snapshot.json
- packages/database/src/repository/paymentMethod.ts
- packages/database/src/repository/media.ts
- packages/database/src/connection.ts
- packages/database/src/repository/product.ts
- packages/database/src/repository/checkout.ts
- packages/core/shared/utils/date.ts
- packages/database/tsconfig.json
- packages/database/src/repository/channel.ts
- packages/database/src/repository/userCredential.ts
- packages/database/src/repository/user.ts
- packages/database/src/index.ts
- packages/database/src/repository/workingDay.ts
- packages/database/src/repository/checkoutLine.ts
- docker/docker-compose.yml
- packages/database/src/tables.ts
- packages/database/src/repository/menu.ts
- packages/database/src/types.ts
- packages/core/server/preset.ts
- packages/database/migrations/0000_aspiring_sprite.sql
- packages/database/src/repository/index.ts
- packages/database/src/repository/productVariant.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/package.json
🔇 Additional comments (6)
apps/web-app/package.json (1)
20-20: Ensure Redis client implementation has proper error handling.The addition of
ioredisas a dependency is consistent with the broader database changes indicated in the PR title. Since Redis will now be part of your infrastructure, make sure to implement proper error handling in your Redis client code.#!/bin/bash # Search for Redis client implementation and check for error handling echo "Looking for Redis client implementations and their error handling..." grep -r --include="*.{ts,js,vue}" "new Redis\|createClient" ./apps ./packages 2>/dev/null | grep -v "node_modules" echo -e "\nChecking for error handling in Redis-related files..." # Find files that mention Redis and check if they have error handling grep -r --include="*.{ts,js,vue}" -l "Redis\|redis" ./apps ./packages 2>/dev/null | grep -v "node_modules" | xargs grep -l "try\|catch\|error\|.on('error'" 2>/dev/null || echo "No error handling found in Redis-related files"packages/core/server/api/auth/session.post.ts (2)
3-23: Function logic remains sound.The event handler logic is robust with proper error handling and a clear flow. The function correctly uses the new
createId()from the updated import to generate user IDs.
1-1:✅ Verification successful
Import source change is appropriate.
Good change replacing the database import with a direct dependency on
@paralleldrive/cuid2. This aligns with the broader database architecture changes indicated in the PR and removes the dependency on the internal database package.
🏁 Script executed:
#!/bin/bash # Verify that @paralleldrive/cuid2 is a dependency in the project echo "Checking for @paralleldrive/cuid2 in package.json files:" grep -r --include="package.json" "@paralleldrive/cuid2" ./packages ./apps 2>/dev/null || echo "Warning: @paralleldrive/cuid2 dependency not found in any package.json" echo -e "\nChecking other createId imports to ensure consistency:" grep -r --include="*.ts" "import.*createId" ./packages ./apps 2>/dev/null | grep -v "node_modules" | grep -v "@paralleldrive/cuid2"Length of output: 506
Import Update Verified: @paralleldrive/cuid2 Dependency Is Consistent
The change to import
createIddirectly from@paralleldrive/cuid2has been verified. The dependency is declared in the project'spackage.json(e.g., in./packages/core/package.json), and no additional inconsistentcreateIdimports were found in the rest of the codebase. The adjustments align with the broader architectural updates regarding database changes.packages/core/nuxt.config.ts (2)
1-36: Documentation Update Required: Add Redis Configuration GuidanceThe integration of
redisUrlinto the runtime configuration has been approved and aligns with the PR's database focus. However, the previous verification indicated that the new Redis configuration option is not documented.
1-36:❓ Verification inconclusive
Verify all dependent components are updated to work without channelId
According to the summary,
channelId: 'burger'has been removed from the runtime config. This is a breaking change that could affect components that rely on this value.
🏁 Script executed:
#!/bin/bash # Search for usage of channelId in components that might be affected echo "Searching for references to channelId in the codebase..." rg "channelId|channel[^a-zA-Z]" --type vue --type tsLength of output: 205
Manual Verification Required: Confirm Removal of
channelIdUsageThe initial search did not return expected results due to an unrecognized file type for Vue files. Please re-run the search with updated parameters to include
.vuefiles, for example:rg --type-add "vue:*.vue" --type vue "channelId"If the refined search confirms that there are no remaining references to
channelIdin your TypeScript or Vue files, then we can be confident that all dependent components have been updated accordingly following the removal ofchannelId: 'burger'from the runtime config.eslint.config.mjs (1)
24-24:❓ Verification inconclusive
Database migration files will now be linted
The
'packages/database/migrations/**'path has been removed from the ESLint ignore list. This means database migration files will now be subject to linting rules, which could enforce better code quality and consistency in migrations.To understand the potential impact, check if existing migration files comply with current linting rules:
🏁 Script executed:
#!/bin/bash # Check if existing migration files have linting issues if [ -d "packages/database/migrations" ]; then echo "Checking migration files for linting issues..." npx eslint packages/database/migrations --ext .js,.ts,.mjs else echo "Migration directory not found at expected path" # Find where migration files might be located echo "Searching for migration files in the repository..." find . -path "*/migrations/*" -type f -name "*.js" -o -name "*.ts" -o -name "*.mjs" | grep -v "node_modules" fiLength of output: 4595
Attention: Database Migrations Will Now Be Linted, but None Were Found in the Expected Location
The ESLint configuration update removes the exclusion of
'packages/database/migrations/**', so any migration files added to that path in the future will be subject to linting—improving code quality and consistency for migrations.Verification Details:
- A shell script check confirmed that the directory
packages/database/migrationsdoes not exist in the repository.- No migration files were located under the expected path. (We did find similar files elsewhere, but none matching this migration directory.)
Action Items:
- Confirm whether database migration files are maintained in a different location or if they are expected to be added under
packages/database/migrations.- If migration files are used in a different folder, update the ESLint ignore configuration accordingly to ensure the correct files are linted.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/core/server/services/db/checkout.ts (3)
35-39: Consider retrieving checkouts in parallel usingPromise.all.Currently, the code initiates multiple asynchronous fetches and stores promises in a Map before awaiting them in a loop. For improved clarity and performance, consider fetching them in parallel using
Promise.all:-const storage = new Map<string, unknown>(keys.map((key) => [key, useStorage('db').getItem(key)])) -const checkouts: Checkout[] = [] -for (const key of keys) { - const checkout = await storage.get(key) as Checkout - if (!checkout) continue - checkouts.push(checkout) -} +const storageFetches = keys.map((key) => useStorage('db').getItem<Checkout>(key)) +const checkouts = await Promise.all(storageFetches) +return checkouts.filter((c): c is Checkout => Boolean(c))
50-57: Optimize by returning the created checkout object directly.The current approach sets the item in storage, then performs a subsequent
getCheckoutcall. Since the newly created data is already known, you can directly return it without the extra read:export async function createCheckout(data: Omit<Checkout, 'createdAt' | 'updatedAt'>): Promise<Checkout | null> { const now = new Date().toISOString() const newCheckout: Checkout = { ...data, createdAt: now, updatedAt: now, } await useStorage('db').setItem(`checkout:${data.id}`, newCheckout) - return getCheckout(data.id) + return newCheckout }
134-152: Trigger checkout total price recalculation after adding a checkout line.When adding a new checkout line, the total price isn't automatically recalculated. Consider calling
recalculateCheckout(data.checkoutId)after creating the new line to ensure the stored total price remains accurate and up-to-date.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web-app/app/components/CommandCenter/CheckoutCard.vue(1 hunks)apps/web-app/app/components/CommandCenter/WarehouseCard.vue(1 hunks)apps/web-app/app/pages/checkout/index.vue(4 hunks)apps/web-app/app/pages/finish/index.vue(1 hunks)packages/core/server/services/db/checkout.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/web-app/app/components/CommandCenter/CheckoutCard.vue
- apps/web-app/app/components/CommandCenter/WarehouseCard.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web-app/app/pages/checkout/index.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (7)
packages/core/server/services/db/checkout.ts (1)
74-109: Implement version checks or locking for concurrency consistency.If multiple users or processes recalculate the same checkout simultaneously, updates can overwrite one another. Adopting version checks or a locking mechanism will prevent inconsistencies when modifying items under high concurrency.
apps/web-app/app/pages/finish/index.vue (6)
6-6: CSS variable adoption for background styling.The background styling has been updated from explicit color classes to a CSS variable (
--ui-bg-muted), which improves theme consistency and maintainability.
12-14: Updated text styling to use CSS variables.Text color has been updated to use the
--ui-text-mutedCSS variable, improving theme consistency across the application.
54-57: Consistent styling update for section headings.The heading for the ordered items section now uses the same text styling as other section headings.
68-71: Consistent styling update for total section heading.The heading for the order total section now uses the same text styling as other section headings.
80-84: Enhanced button styling with gradient variant.The home button has been updated to include a gradient variant, which improves visual appeal while maintaining functionality.
11-52:❓ Verification inconclusive
Simplified component structure by removing ClientOnly wrapper.
The ClientOnly wrapper has been removed, and content is now directly rendered in a standard div. This simplifies the markup, but ensure that there are no hydration mismatches if this content depends on client-side only data.
🏁 Script executed:
#!/bin/bash # Check if there are any client-side only libraries or browser APIs used in this component rg -i "window\.|document\.|navigator\." apps/web-app/app/pages/finish/index.vue # Look for other client-specific code that might require the ClientOnly wrapper rg -i "process\.client" apps/web-app/app/pages/finish/index.vueLength of output: 147
Action: Verify Hydration Consistency After Removing ClientOnly Wrapper
The ClientOnly wrapper has been removed to simplify the markup. Our automated checks did not find direct usages of browser-specific globals (e.g. window, document, navigator) or explicit client-only conditions (e.g. process.client) in this component. However, be aware that dynamic content—such as the localized date formatting via
new Date(...).toLocaleString(...)—might behave differently between server and client renderings. Please manually verify that these differences do not lead to hydration mismatches in your testing environments.
Summary by CodeRabbit
New Features
Improvements
Refactor