feat!: media with media items#381
Conversation
|
""" WalkthroughThis update refactors error handling across several API endpoints by standardizing error objects to use the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant DB
participant MediaService
Client->>API: POST /api/product/[id]/image
API->>MediaService: Generate image variants (JPG, WebP)
MediaService->>API: Return image variant metadata (items)
API->>DB: createMedia(media, items)
DB-->>API: Store media and items
API-->>Client: Respond with media info including items
sequenceDiagram
participant Client
participant API
participant DB
Client->>API: GET /api/products
API->>DB: getProducts(keys)
DB->>API: Fetch products, variants, and media
API-->>Client: Return products with variants and media objects
Possibly related PRs
Suggested labels
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/essence/server/utils/error.ts (1)
21-24: Consider standardizing the 500 error format as well.While validation errors now use
message, the 500 error still usesstatusMessage. Consider updating this for complete consistency across all error types.return createError({ statusCode: 500, - statusMessage: 'Internal server error', + message: 'Internal server error', })packages/core/server/api/product/[id]/image.post.ts (2)
16-21: Consider updating error messages for consistencyWhile the changes you've made are good, I noticed that the error handling in these sections still uses
statusMessagerather thanmessage, which appears to be the new standard based on other files and your changes.- statusMessage: 'Missing id', + message: 'Missing id',Similar changes should be applied to all error objects in this file for consistency.
Also applies to: 26-30, 40-44, 47-51, 53-58
63-87: Consider adding error handling in the nested loopsThe image processing and storage operations could benefit from additional error handling. If an error occurs during these operations, resources might not be properly cleaned up.
// Every size +try { for (const size of IMAGE_SIZES) { // Every format for (const format of ['jpg', 'webp'] as const) { let buffer: unknown = await sharpStream .resize({ width: size, height: size }) .toFormat(format, { quality: 75 }) .toBuffer() await storage.setItemRaw(`${productsDirectory}/${mediaId}/${size}.${format}`, buffer) items.push({ id: createId(), createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), mediaId, size, format, url: `${mediaUrl}${productsDirectory}/${mediaId}/${size}.${format}`, }) // Clear buffer = null } } +} finally { + sharpStream.destroy() +} -sharpStream.destroy()
📜 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 (28)
apps/essence/server/api/product/[productId]/index.delete.ts(1 hunks)apps/essence/server/api/product/[productId]/index.get.ts(1 hunks)apps/essence/server/api/product/[productId]/index.patch.ts(1 hunks)apps/essence/server/api/product/variant/[id].delete.ts(1 hunks)apps/essence/server/api/product/variant/[id].patch.ts(2 hunks)apps/essence/server/api/product/variant/index.post.ts(1 hunks)apps/essence/server/utils/error.ts(1 hunks)apps/web-app/app/components/Cart/Line.vue(1 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/CommandCenter/ProductVariantCard.vue(1 hunks)apps/web-app/app/components/ProductCard.vue(1 hunks)apps/web-app/app/components/ProductImage.vue(2 hunks)apps/web-app/app/pages/catalog/[categorySlug]/[productSlug]/index.vue(1 hunks)apps/web-app/app/pages/command-center/product/[productId]/index.vue(1 hunks)apps/web-app/app/pages/command-center/product/index.vue(4 hunks)apps/web-app/app/utils/helpers.ts(1 hunks)apps/web-app/nuxt.config.ts(0 hunks)apps/web-app/stores/channel.ts(4 hunks)packages/core/nuxt.config.ts(1 hunks)packages/core/package.json(1 hunks)packages/core/server/api/product/[id]/image.post.ts(4 hunks)packages/core/server/api/product/index.post.ts(0 hunks)packages/core/server/services/db/media.ts(1 hunks)packages/core/server/services/db/product.ts(2 hunks)packages/core/types/food.ts(1 hunks)packages/core/types/index.d.ts(0 hunks)packages/schema/src/types/food.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/web-app/nuxt.config.ts
- packages/core/server/api/product/index.post.ts
- packages/core/types/index.d.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/core/types/food.ts (1)
packages/schema/src/types/food.ts (4)
Media(20-20)MediaItem(34-34)Product(46-46)ProductVariant(64-64)
packages/core/server/services/db/product.ts (4)
packages/schema/src/types/food.ts (1)
Product(46-46)packages/core/types/food.ts (1)
ProductWithVariantsAndMedia(7-10)apps/essence/server/services/db/tables.ts (1)
products(6-15)packages/core/server/services/db/media.ts (1)
getMedia(4-6)
packages/core/server/services/db/media.ts (2)
packages/core/types/food.ts (1)
MediaWithItems(3-5)packages/schema/src/types/food.ts (2)
Media(20-20)MediaItem(34-34)
apps/web-app/stores/channel.ts (3)
packages/core/types/food.ts (1)
ProductWithVariantsAndMedia(7-10)apps/essence/server/services/db/tables.ts (1)
products(6-15)packages/core/server/services/db/product.ts (1)
getProduct(7-9)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (59)
packages/core/nuxt.config.ts (1)
15-17: Appropriate addition ofmediaUrlin runtime config.The addition of a
mediaUrlproperty in the public runtime config is a good enhancement to support the new media handling functionality across the project. This will enable both server-side and client-side code to construct media URLs consistently.apps/web-app/app/utils/helpers.ts (1)
1-2: Good migration to schema-based types.The import of
WeightUnitfrom@nextorders/schemaaligns well with the overall refactoring effort to move from global type declarations to schema-based types for better maintainability and type safety.packages/core/package.json (1)
12-12: Correctly added schema dependency.Adding the
@nextorders/schemadependency with the workspace reference is necessary to support the migration to schema-based types and ensures consistency across the monorepo.apps/web-app/app/pages/command-center/product/[productId]/index.vue (1)
7-7:Details
❓ Verification inconclusive
Good enhancement to support richer media handling.
Adding the media object prop alongside the existing ID prop supports the transition from ID-based to object-based media rendering, enabling richer media content display with different sizes and formats.
Verify that the
ProductImagecomponent properly handles both props and has clear precedence rules:
🏁 Script executed:
#!/bin/bash # Check how ProductImage component handles both id and media props fd ProductImage --exec cat {} \; | grep -A 30 "props" | grep -B 5 -A 15 "media\|id"Length of output: 86
🏁 Script executed:
#!/bin/bash # Locate and inspect ProductImage component file file=$(fd -e vue -i productimage | head -n 1) if [ -z "$file" ]; then echo "Component file not found" 1>&2 exit 1 fi echo "Inspecting: $file" # Print the props definition block sed -n '/props:/,/^ *}/p' "$file"Length of output: 402
#!/bin/bash set -e # Locate the page to see how ProductImage is imported and used INDEX="apps/web-app/app/pages/command-center/product/\\[productId\\]/index.vue" echo "=== Import Statements for ProductImage ===" grep -nE "import .*ProductImage" -n "$INDEX" || echo "No import of ProductImage found" echo -e "\n=== Usage of ProductImage in index.vue ===" grep -nC5 "ProductImage" "$INDEX" || echo "No usage of ProductImage found"
Ensure ProductImage Handles Both
idandmediaProps with Clear PrecedenceGreat enhancement to support richer media handling. To avoid regressions, please verify in your
ProductImage.vuecomponent that:
- Both
id(string/number) andmedia(object) props are declared with the correct types.- When both props are passed,
mediatakes precedence overid.- If
mediais not provided, the component falls back to rendering byid.- Appropriate default values, prop validations, and unit tests are in place to cover each scenario.
apps/web-app/app/components/Cart/Line.vue (1)
6-6: Props updated to support richer media objects.The change from
:id="product?.mediaId"to:media="product?.media"properly implements the new media handling approach, allowing the ProductImage component to work with complete media objects rather than just IDs.apps/web-app/app/components/Checkout/Line.vue (1)
5-5: Props updated to support richer media objects.The change from
:id="product?.mediaId"to:media="product?.media"properly implements the new media handling approach, allowing the ProductImage component to work with complete media objects rather than just IDs.apps/web-app/app/pages/catalog/[categorySlug]/[productSlug]/index.vue (1)
8-8: Props updated to support richer media objects.The change from
:id="product?.mediaId"to:media="product?.media"properly implements the new media handling approach, allowing the ProductImage component to work with complete media objects rather than just IDs.apps/essence/server/api/product/[productId]/index.get.ts (1)
9-9: Error handling standardization.The change from
statusMessagetomessageproperly implements the standardized error handling pattern, ensuring consistency across API endpoints.apps/essence/server/api/product/variant/[id].delete.ts (1)
9-9: Standardized error handling with 'message' propertyThis change from
statusMessagetomessagealigns with the standardization of error handling across the codebase, creating a more consistent API error response format.apps/essence/server/api/product/[productId]/index.delete.ts (1)
9-9: Standardized error handling with 'message' propertyThis change from
statusMessagetomessagealigns with the standardization of error handling across the codebase, creating a more consistent API error response format.apps/web-app/app/components/CommandCenter/ProductCard.vue (1)
11-11: Enhanced media handling with rich media objectsThe component now passes the entire
mediaobject toProductImageinstead of just the ID, enabling more sophisticated image selection based on size, format, and other attributes. This aligns with the broader refactoring of media handling throughout the application.apps/web-app/app/components/CommandCenter/ProductVariantCard.vue (1)
34-35: Improved type safety with explicit schema importsExplicitly importing the
ProductVarianttype from@nextorders/schemainstead of relying on global type declarations improves code maintainability and clarity. This change follows TypeScript best practices and aligns with the broader type system improvements throughout the codebase.apps/essence/server/api/product/variant/[id].patch.ts (2)
10-12: Good job on standardizing error messaging format.The change from
statusMessagetomessagealigns with the broader error handling refactoring across the application, making error objects more consistent.
24-26: Good job maintaining consistent error structure.Consistently using
messageinstead ofstatusMessagefor error objects makes error handling more predictable across the system.apps/essence/server/utils/error.ts (1)
8-11: Good job standardizing validation error format.Updated error structure for
type.errorsto usemessageinstead ofstatusMessagecreates consistency with other error handlers.apps/web-app/app/components/ProductCard.vue (2)
9-13: Correctly upgraded ProductImage to use full media object.Passing the complete
mediaobject instead of just the ID aligns with the enhanced media handling system. This change allows the ProductImage component to access richer media metadata and select appropriate image variants.
52-54:Details
❓ Verification inconclusive
Verify channel store type compatibility.
Ensure that the
channel.getProduct()method now returns products with the full media object structure, not just a mediaId, to maintain compatibility with the updated ProductImage component.
🏁 Script executed:
#!/bin/bash # Check store typing and product retrieval implementation echo "Checking channel store implementation..." rg -A 5 "getProduct" apps/web-app/stores/Length of output: 2281
🏁 Script executed:
#!/bin/bash # Locate and print definition of ProductWithVariantsAndMedia echo "Searching for ProductWithVariantsAndMedia type..." rg -n "type ProductWithVariantsAndMedia" -C5 apps/web-app rg -n "interface ProductWithVariantsAndMedia" -C5 apps/web-appLength of output: 233
🏁 Script executed:
#!/bin/bash # Display import statements to locate the source of ProductWithVariantsAndMedia echo "=== Imports (first 50 lines) ===" sed -n '1,50p' apps/web-app/stores/channel.ts # Show where `products` is initialized and its type echo -e "\n=== products declaration ===" rg -A3 "products\s*=" apps/web-app/stores/channel.ts # Show the defineStore block for context echo -e "\n=== defineStore context ===" rg -A5 "defineStore" apps/web-app/stores/channel.tsLength of output: 3144
Verify full media object in ProductWithVariantsAndMedia type
The store’s getProduct method correctly returns a ComputedRef and products is typed as ProductWithVariantsAndMedia[]. You now need to confirm that ProductWithVariantsAndMedia (from @nextorders/core/types/food) includes the full media object structure (not just a mediaId) to satisfy the updated ProductImage component.
• apps/web-app/stores/channel.ts
const products = ref<ProductWithVariantsAndMedia[]>([])function getProduct(id: string): ComputedRef<ProductWithVariantsAndMedia | undefined>Please review the external type definition and ensure it provides the complete media objects.
apps/essence/server/api/product/[productId]/index.patch.ts (3)
10-12: Good job standardizing error message format.The change from
statusMessagetomessagealigns with the broader error handling refactoring across the application.
18-20: Good job maintaining consistent error structure.Using
messageconsistently for error objects improves predictability for error handling throughout the system.
32-36:Details
❓ Verification inconclusive
Verify media handling in repository update method.
Since this PR updates product handling to support richer media objects, verify that the repository's
product.updatemethod correctly handles the potentially new media structure when updating products.
🏁 Script executed:
#!/bin/bash # Check product repository implementation for media handling echo "Checking product repository implementation..." rg -A 10 "product\.update" apps/essence/server/services/db/Length of output: 162
Verify media handling in repository update method.
I couldn’t locate theproduct.updateimplementation in the DB layer to confirm it handles our new richermediastructure. Please ensure that this method:
- Properly maps the incoming
mediapayload into nested writes (e.g. ORM‑levelconnect,disconnect,update,create).- Doesn’t simply overwrite existing media entries.
- Is fully covered by tests for creating, updating and removing media.
apps/essence/server/api/product/variant/index.post.ts (1)
14-20: Added product existence check before creating a variantThis is a good addition that prevents creating orphaned variant records by verifying the product exists before proceeding. The error message is clear and matches the standardization efforts in the codebase (using
messageinstead ofstatusMessage).packages/core/server/api/product/[id]/image.post.ts (5)
1-1: Import of MediaItem type from the schema packageGood addition to ensure type safety for the newly introduced media items functionality.
12-12: Added mediaUrl to runtime config extractionCorrect extraction of the mediaUrl from the public runtime config to support the URL construction for media items.
61-61: Added MediaItem array initializationThis array will store metadata for each generated image variant, supporting the enhanced media structure.
74-82: Added media item metadata collectionGood implementation that creates detailed metadata for each image variant, including unique ID, timestamps, size, format, and properly constructed URL.
91-91: Updated createMedia call to include media itemsProperly passes both the media record and the collected items array to the createMedia function.
packages/core/types/food.ts (1)
1-10: Added well-structured interfaces for enhanced product and media dataThe new interfaces
MediaWithItemsandProductWithVariantsAndMediaeffectively extend the base types to support richer media and product data structures. This provides proper typing for products that include detailed variant and media information, which is a great improvement for type safety throughout the application.packages/core/server/services/db/product.ts (6)
1-4: Updated imports to support new functionalityGood addition of imports for the new types and the
getMediafunction to support the enhanced product data structure.
11-13: Updated return type and array declarationCorrectly changed the return type of
getProductsto useProductWithVariantsAndMedia[]and updated the products array declaration to match, ensuring type safety throughout the function.
20-20: Updated type casting to use enriched product typeCorrectly updated the type casting to use
ProductWithVariantsAndMedia.
26-26: Using the new helper function for variant keysGood refactoring to use the new
prepareProductVariantKeyshelper function for better code organization.
40-43: Added media data fetching and attachmentGood addition that ensures media data is fetched and attached to products when available.
However, consider explicitly setting
product.media = nullwhen there's nomediaIdfor consistency:// media if (product.mediaId) { product.media = await getMedia(product.mediaId) +} else { + product.media = null }
52-64: Added helper function for collecting product variant keysGood extraction of the logic for collecting product variant keys into a separate helper function, which improves code organization and reusability.
packages/core/server/services/db/media.ts (4)
1-2: Good type imports from the centralized schema package.This is a good practice to import types from a centralized schema package rather than using global types. It improves type safety and maintainability.
4-5: Updated return type to support richer media structure.Changing the return type from
Media | nulltoMediaWithItems | nullproperly reflects that the function now returns a media object with an array of media items.
8-8: Function signature enhanced to support media items collection.The
createMediafunction now accepts an optionalitemsparameter, allowing multiple media items to be associated with a single media record. This is a good enhancement of the API.
13-13: Media items stored with media record.The media object now includes the items array when stored, which aligns with the
MediaWithItemsinterface structure.apps/web-app/app/pages/command-center/product/index.vue (4)
132-133: Updated imports to use the new type system.Importing types from their appropriate packages enhances code organization and type safety.
157-157: Enhanced type safety with more specific column typing.The columns reference now correctly uses
ProductWithVariantsAndMediatype instead of justProduct, properly reflecting the richer product structure that includes variants and media objects.
161-161: Updated accessor key to match new data structure.The accessor key was appropriately changed from 'mediaId' to 'media', reflecting that products now contain full media objects rather than just references.
74-77: Updated template to use media objects instead of IDs.The template slot name has been changed to
media-celland the ProductImage component now receives a fullmediaobject instead of just an ID. This aligns with the broader refactoring to use complete media objects.apps/web-app/app/components/ProductImage.vue (6)
21-23: Imports for new media type system.Importing the appropriate media types from their respective packages aligns with the broader refactoring of the type system.
24-28: Props updated to use media objects.The component now accepts a
mediaprop of typeMediaWithItems | nullinstead of an ID, which provides access to richer media data including multiple image variants.
31-35: Improved size mapping using numeric values.The sizes are now represented as numeric values instead of strings, which is more appropriate for the size comparison logic used in image selection.
37-38: Enhanced image URL computation based on size and format.The component now intelligently computes image URLs by finding the nearest matching media item by size and format, which improves the user experience by serving appropriately sized images.
40-50: Well-implemented helper function for image selection.The
getNearestImageBySizeAndFormatfunction efficiently finds the media item with the closest size for a given format, implementing a good algorithm for selecting the most appropriate image.
2-10: Updated rendering logic for multiple media formats.The component now properly renders different image formats based on the available media items, with proper fallback handling when no items are available.
packages/schema/src/types/food.ts (5)
15-20: Good base Media schema definition.The Media schema provides a solid foundation for media entities with essential fields like ID and timestamps.
22-24: Clear media format type definition.Explicitly defining supported media formats ('jpg' and 'webp') as a schema ensures consistency and type safety throughout the application.
25-34: Comprehensive MediaItem schema with format support.The MediaItem schema is well-defined with all necessary properties including URL, size, and format. This enables the application to handle different image variants effectively.
36-46: Potential inconsistency in Product schema.The Product schema still includes a
mediaIdfield of typestring | null, but other changes suggest products should now reference full media objects. This might be an inconsistency with theProductWithVariantsAndMediatype used elsewhere.Could you clarify how products reference media in the broader codebase? If products now use full media objects, consider updating this schema to use a media property instead of mediaId.
48-64: Comprehensive ProductVariant schema.The ProductVariant schema is well-defined with all necessary properties including nutritional information, which provides a solid foundation for variant handling throughout the application.
apps/web-app/stores/channel.ts (7)
1-2: Update imports to new product and variant types
The store now importsProductWithVariantsAndMediafrom the core types andProductVariantfrom the schema package, aligning with the consolidated type definitions across the codebase.
10-12: Extend search result type with enriched product-media type
ProductWithCategorycorrectly extendsProductWithVariantsAndMediaby adding acategoryfield, ensuring search results carry both media/variant data and category context.
36-36: Annotate products ref with enhanced product type
Updatingproductstoref<ProductWithVariantsAndMedia[]>guarantees that the store holds the full variant and media information for each product.
43-43: Update computed activeProducts to use enriched type
TheactiveProductscomputed now accurately reflects theProductWithVariantsAndMedia[]shape, preserving type safety when consuming this list.
121-123: Use enriched product type in getProduct
getProductis updated to returnComputedRef<ProductWithVariantsAndMedia | undefined>, matching the new store payload structure.
124-126: Use enriched product type in getProductBySlug
getProductBySlugnow returns the fullProductWithVariantsAndMediaobject for a given slug, ensuring variants and media are available.
146-149: Use enriched product type in getProductsInCategory
getProductsInCategorycorrectly filters and returnsProductWithVariantsAndMedia[]for the specified category, preserving the enriched structure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/core/server/api/product/[id]/image.post.ts (2)
67-85: Improved media processing with unique IDs.The implementation now correctly:
- Generates unique IDs for each media variant
- Stores files with proper paths
- Creates complete metadata for each variant
- Builds proper URLs using the configured mediaUrl
However, there is a minor optimization opportunity:
Consider using a single timestamp for both createdAt and updatedAt:
+const now = new Date().toISOString(); items.push({ id, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), + createdAt: now, + updatedAt: now, mediaId, size, format, url: `${mediaUrl}${productsDirectory}/${mediaId}/${id}.${format}`, });
11-115: Consider adding try-finally for resource cleanup.While the code properly attempts to clean up resources with
sharpStream.destroy(), these cleanup steps might not execute if an error occurs during processing.Consider using a try-finally pattern to ensure resources are properly released:
export default defineEventHandler(async (event) => { + let sharpStream; try { const { productsDirectory, public: { mediaUrl } } = useRuntimeConfig() const storage = useStorage('s3') // ... validation code ... sharp.cache(false) sharp.concurrency(1) - const sharpStream = sharp(file.data.buffer as ArrayBuffer) + sharpStream = sharp(file.data.buffer as ArrayBuffer) // ... rest of the code ... - sharpStream.destroy() // ... final operations ... return { ok: true } } catch (error) { throw errorResolver(error) + } finally { + if (sharpStream) { + sharpStream.destroy() + } } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/server/api/product/[id]/image.post.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (9)
packages/core/server/api/product/[id]/image.post.ts (9)
1-1: Good addition of schema-based typing.The import of the
MediaItemtype from the schema package follows best practices by leveraging centralized type definitions.
4-4: Properly updated imports to include getMedia.The import statement has been correctly updated to include the
getMediafunction, which is needed for the enhanced media handling logic.
8-9: Good type safety improvement.Adding the
as consttype assertion to IMAGE_FORMATS provides better type safety, ensuring the array is treated as a readonly tuple of specific string literals.
13-13: Good extraction of mediaUrl from runtime config.Destructuring the mediaUrl from public runtime config is a good practice for handling environment-specific URLs.
20-20: Consistent error message standardization.The change from
statusMessagetomessagealigns with standardized error object format across the codebase.Also applies to: 29-29, 43-43, 50-50, 57-57
40-40: Complete validation of image format and dimensions.The validation check is thorough, ensuring the image has a valid format and dimensions before processing.
62-62: Good initialization of MediaItem array.Properly initializing the items array to store multiple media variants follows the new schema design.
94-94: Updated createMedia call to match new signature.The call to createMedia now correctly passes both the media record and the items array, matching the updated service API.
98-106: More robust cleanup of existing media items.The improved approach to cleaning up existing media:
- Fetches the complete media record with all items
- Iterates through each item to properly remove all variants
- Only then deletes the media record itself
This is significantly more thorough than the previous approach.
|
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
messageproperty.Refactor
Chores