-
-
Notifications
You must be signed in to change notification settings - Fork 241
Check uploaded product image in Boilerplate using AI (#11004) #11005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes refactor attachment file path generation into a helper method, introduce AI-based validation for uploaded product images to check for cars, update localization resources for error messaging, and adjust authorization and state update logic for image uploads and deletions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AttachmentController
participant AIChatClient
participant FileSystem
participant ProductState
Client->>AttachmentController: UploadAttachment (ProductPrimaryImageMedium)
AttachmentController->>FileSystem: Save uploaded image
alt AIChatClient available
AttachmentController->>AIChatClient: Send image, ask "Is this a car?"
AIChatClient-->>AttachmentController: "Yes" or "No"
alt Response is "No"
AttachmentController-->>Client: Return error "ImageNotCarError"
else Response is "Yes"
AttachmentController->>ProductState: Update product image state
AttachmentController-->>Client: Return success
end
else AIChatClient not available
AttachmentController->>ProductState: Update product image state
AttachmentController-->>Client: Return success
end
Assessment against linked issues
Poem
✨ Finishing Touches🧪 Generate Unit Tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new error message to indicate when an uploaded image does not contain a car and integrates an AI chat client to validate product images. In addition, it updates the resource files with the new message and refactors file path generation in the attachment controller.
- Added "ImageNotCarError" resource text in both English and Farsi resource files.
- Modified the attachment controller to use a helper method (GetFilePath) and integrate a chat client check for car images.
- Updated endpoint attributes to improve access control and parameter handling.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx | Added new error message for images without a car. |
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx | Added Farsi translation for the new error message. |
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs | Refactored file path generation, adjusted endpoint parameters, and integrated AI image validation. |
Comments suppressed due to low confidence (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs:250
- The use of Path.GetExtension(fileName) may throw a NullReferenceException if fileName is null; consider adding a validation or a default value for fileName when the extension is required.
AttachmentKind.ProductPrimaryImageOriginal => $"{AppSettings.ProductImagesDir}{attachmentId}_{kind}{Path.GetExtension(fileName)}",
...rplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs (1)
178-194
: Fix potential null reference in AI validationThe
imageBytes
variable is only assigned when the image needs resizing. IfProductPrimaryImageOriginal
is uploaded (which doesn't need resizing),imageBytes
will be null when passed to the AI validation, causing a null reference exception.byte[]? imageBytes = null; if (imageResizeContext.NeedsResize) { using MagickImage sourceImage = new(file.OpenReadStream()); if (sourceImage.Width < imageResizeContext.Width || sourceImage.Height < imageResizeContext.Height) return BadRequest(Localizer[nameof(AppStrings.ImageTooSmall), imageResizeContext.Width, imageResizeContext.Height, sourceImage.Width, sourceImage.Height].ToString()); sourceImage.Resize(new MagickGeometry(imageResizeContext.Width, imageResizeContext.Height)); await blobStorage.WriteAsync(attachment.Path, imageBytes = sourceImage.ToByteArray(MagickFormat.WebP), cancellationToken: cancellationToken); } else { + using var stream = file.OpenReadStream(); + using var memoryStream = new MemoryStream(); + await stream.CopyToAsync(memoryStream, cancellationToken); + imageBytes = memoryStream.ToArray(); await blobStorage.WriteAsync(attachment.Path, file.OpenReadStream(), cancellationToken: cancellationToken); }Also applies to: 209-209
🧹 Nitpick comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs (1)
200-214
: Consider adding configuration and monitoringFor production readiness, consider:
- Adding a configuration flag to enable/disable AI validation
- Implementing metrics to track AI validation success/failure rates
- Adding a fallback mechanism when AI service is unavailable
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs
(9 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (6)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx (1)
1355-1357
: LGTM!The new error message resource is clear and appropriate for the AI-based car detection feature.
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx (1)
1349-1351
: LGTM!The Persian translation is consistent with the English version and appropriate for the feature.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs (4)
22-24
: Service locator pattern is acceptable hereThe conditional injection of
IServiceProvider
is appropriate given thatIChatClient
may not always be registered. This allows graceful degradation when AI validation is not available.
83-83
: Excellent security improvement!Adding authorization to the
DeleteProductPrimaryImage
endpoint is crucial. Product catalog management operations should always be protected.
200-200
: Logical improvement in state update conditionsChanging the conditions to use the resized image kinds (Medium/Small) instead of Original is correct, as these are the versions that are validated and displayed to users.
Also applies to: 226-226
244-260
: Excellent refactoring!The
GetFilePath
helper method effectively centralizes the file path generation logic, reducing code duplication and improving maintainability. The use ofEnvironment.ExpandEnvironmentVariables
is appropriate for handling environment-specific paths.
closes #11004
Summary by CodeRabbit