Conversation
WalkthroughThis update introduces a new image upload feature, including a controller, service, and interface for handling uploads and asset retrieval. It adds user association to asset entities and DTOs, configures dependency injection for the new service, and updates AutoMapper profiles. A database seed operation is also commented out in the startup logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UploadController
participant UploadService
participant CloudinaryService
participant AssetService
Client->>UploadController: POST /api/upload/image (file)
UploadController->>UploadService: UploadImageAsync(file, userId)
UploadService->>CloudinaryService: UploadAsync(file stream)
CloudinaryService-->>UploadService: Uploaded file URL
UploadService->>AssetService: CreateAsset (metadata, userId)
AssetService-->>UploadService: AssetDto
UploadService-->>UploadController: AssetDto
UploadController-->>Client: 200 OK (AssetDto)
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
Dentizone.Infrastructure/DependencyInjection/AddRepositories.cs (1)
16-16: Registration looks correct – keep the list sorted.
IAssetRepository→AssetRepositoryis the right lifetime.
Minor nit: consider alphabetically ordering registrations for easier scanning.Dentizone.Application/AutoMapper/Asset/AssetProfile.cs (1)
18-18: Mapping OK, but watch out for overload proliferation.The extra
Asset ↔ CreateAssetDtomap is valid, just ensure overlapping maps don’t become ambiguous when you add customForMemberrules later.Dentizone.Application/Services/UploadService.cs (1)
18-21:_mapperis injected but never used.If mapping is not required, drop the dependency to reduce coupling; otherwise, use it.
Dentizone.Presentaion/Controllers/UploadController.cs (1)
33-44: REST semantics & naming
- Method name
getAssetByIduses camel-case; keep interface & implementation Pascal-case (GetAssetByIdAsync) for consistency.- Route
get-asset-by-id/{id}is RPC-style and verbose. Prefer/assets/{id}and return200 OKwith the asset representation (or404) instead of a message wrapper.Not blocking, but improves API ergonomics.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Dentizone.Application/AutoMapper/Asset/AssetProfile.cs(1 hunks)Dentizone.Application/DI/Services.cs(2 hunks)Dentizone.Application/Interfaces/IUploadService.cs(1 hunks)Dentizone.Application/Services/UploadService.cs(1 hunks)Dentizone.Infrastructure/DependencyInjection/AddRepositories.cs(1 hunks)Dentizone.Presentaion/Controllers/UploadController.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
Dentizone.Application/DI/Services.cs (2)
Dentizone.Domain/Entity/Asset.cs (1)
Asset(6-18)Dentizone.Application/Services/UploadService.cs (2)
UploadService(16-57)UploadService(22-27)
Dentizone.Application/AutoMapper/Asset/AssetProfile.cs (2)
Dentizone.Domain/Entity/Asset.cs (1)
Asset(6-18)Dentizone.Application/DTOs/Asset/CreateAssetDto.cs (1)
CreateAssetDto(6-11)
Dentizone.Application/Interfaces/IUploadService.cs (2)
Dentizone.Application/Services/UploadService.cs (2)
Task(29-37)Task(39-52)Dentizone.Application/DTOs/Asset/AssetDto.cs (1)
AssetDto(5-11)
Dentizone.Presentaion/Controllers/UploadController.cs (2)
Dentizone.Application/Interfaces/IUploadService.cs (2)
Task(13-13)Task(14-14)Dentizone.Application/Services/UploadService.cs (2)
Task(29-37)Task(39-52)
🔇 Additional comments (1)
Dentizone.Application/DI/Services.cs (1)
27-28: Good addition – services correctly registered.
IUploadServiceandIAssetServiceare scoped like the rest of the application services – LGTM.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces new image upload functionality with two API endpoints: one for uploading images and returning their URLs, and another for checking asset existence by ID, along with necessary service and repository registrations.
- Introduced UploadController with endpoints for uploading images and asset lookup.
- Added UploadService and related DI registrations for handling image uploads and assets.
- Updated AutoMapper configuration to include mapping for asset DTOs.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Dentizone.Presentaion/Controllers/UploadController.cs | New endpoints for image upload and asset lookup. |
| Dentizone.Infrastructure/DependencyInjection/AddRepositories.cs | Added asset repository DI registration. |
| Dentizone.Application/Services/UploadService.cs | Implemented upload functionality and asset check logic. |
| Dentizone.Application/Interfaces/IUploadService.cs | Defined upload service interface for image handling. |
| Dentizone.Application/DI/Services.cs | Registered new services in the DI container. |
| Dentizone.Application/AutoMapper/Asset/AssetProfile.cs | Updated AutoMapper configuration for asset mappings. |
Comments suppressed due to low confidence (3)
Dentizone.Application/Services/UploadService.cs:29
- Method names in C# should follow PascalCase; consider renaming 'getAssetById' to 'GetAssetById'.
public async Task<bool> getAssetById(string id)
Dentizone.Application/Services/UploadService.cs:43
- [nitpick] Variable 'added_image_to_our_DB' should follow camelCase conventions; consider renaming it to 'addedImageToDb'.
var added_image_to_our_DB= await _assetService.CreateAssetAsync(new CreateAssetDto
Dentizone.Application/Interfaces/IUploadService.cs:14
- Interface method names should use PascalCase; consider renaming 'getAssetById' to 'GetAssetById' to maintain consistency with C# conventions.
public Task<bool> getAssetById(string id);
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
Dentizone.Application/Interfaces/IUploadService.cs (1)
8-10: 🛠️ Refactor suggestionAsync-suffix missing & signature drift
FindAssetByIdreturnsTask<AssetDto>but omits theAsyncsuffix → violates .NET naming guidelines and breaks discoverability alongside other async API.-public Task<AssetDto> FindAssetById(string id); +Task<AssetDto> FindAssetByIdAsync(string id);Dentizone.Presentaion/Controllers/UploadController.cs (2)
7-8:⚠️ Potential issueNamespace typo breaks endpoint discovery.
Dentizone.Presentaion→Dentizone.Presentation. The misspelled assembly/route namespace prevents MVC from finding the controller.-namespace Dentizone.Presentaion.Controllers +namespace Dentizone.Presentation.Controllers
11-15: 🛠️ Refactor suggestionMissing
[Consumes]+[FromForm]attributes & incorrect DI-style ctor.
- Multipart binding is fragile without
[Consumes("multipart/form-data")]and[FromForm] IFormFile file.- Primary-ctor syntax for controllers is C# 12 only – confirm LangVersion or switch to classic ctor for wider TFMs.
[HttpPost("image")] +[Consumes("multipart/form-data")] [Authorize] -public async Task<IActionResult> UploadImageAsync(IFormFile file) +public async Task<IActionResult> UploadImageAsync([FromForm] IFormFile file)
🧹 Nitpick comments (3)
Dentizone.Application/DTOs/Asset/CreateAssetDto.cs (1)
27-29: Validation chain: addCascade(CascadeMode.Stop)for clearer errors.If
UserIdis empty the second rule still evaluates. Use cascade to avoid redundant messages.-RuleFor(x => x.UserId) +RuleFor(x => x.UserId) + .Cascade(CascadeMode.Stop)Dentizone.Presentaion/Controllers/UploadController.cs (2)
20-25: Unprofessional exception message.API error strings should be actionable & neutral for clients.
-throw new BadActionException("Why you upload non image man?"); +throw new BadActionException("Only JPG, JPEG, PNG or WEBP images are allowed.");
27-31: Hard-coded size limit: make it configurable & reuse constant.Move
10 * 1024 * 1024to a configuration option orFileSizeLimitsconstant to avoid magic numbers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
Dentizone.Application/DTOs/Asset/CreateAssetDto.cs(2 hunks)Dentizone.Application/Interfaces/IUploadService.cs(1 hunks)Dentizone.Application/Services/UploadService.cs(1 hunks)Dentizone.Domain/Entity/Asset.cs(1 hunks)Dentizone.Infrastructure/Migrations/20250606212156_Update.Designer.cs(0 hunks)Dentizone.Infrastructure/Migrations/20250606212156_Update.cs(0 hunks)Dentizone.Infrastructure/Migrations/20250609100443_UpdateCats.Designer.cs(0 hunks)Dentizone.Infrastructure/Migrations/20250609100443_UpdateCats.cs(0 hunks)Dentizone.Infrastructure/Migrations/20250611212139_Merge.Designer.cs(0 hunks)Dentizone.Infrastructure/Migrations/20250611212139_Merge.cs(0 hunks)Dentizone.Infrastructure/Migrations/AppDbContextModelSnapshot.cs(0 hunks)Dentizone.Presentaion/Controllers/UploadController.cs(1 hunks)Dentizone.Presentaion/Program.cs(1 hunks)
💤 Files with no reviewable changes (7)
- Dentizone.Infrastructure/Migrations/20250609100443_UpdateCats.cs
- Dentizone.Infrastructure/Migrations/AppDbContextModelSnapshot.cs
- Dentizone.Infrastructure/Migrations/20250606212156_Update.Designer.cs
- Dentizone.Infrastructure/Migrations/20250611212139_Merge.Designer.cs
- Dentizone.Infrastructure/Migrations/20250609100443_UpdateCats.Designer.cs
- Dentizone.Infrastructure/Migrations/20250606212156_Update.cs
- Dentizone.Infrastructure/Migrations/20250611212139_Merge.cs
✅ Files skipped from review due to trivial changes (1)
- Dentizone.Presentaion/Program.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Dentizone.Application/Services/UploadService.cs
🧰 Additional context used
🧬 Code Graph Analysis (2)
Dentizone.Application/Interfaces/IUploadService.cs (3)
Dentizone.Domain/Entity/Asset.cs (1)
Asset(6-21)Dentizone.Application/Services/UploadService.cs (2)
Task(13-17)Task(19-32)Dentizone.Application/DTOs/Asset/AssetDto.cs (1)
AssetDto(5-11)
Dentizone.Domain/Entity/Asset.cs (3)
Dentizone.Domain/Entity/UserAsset.cs (1)
UserAsset(6-21)Dentizone.Domain/Entity/PostAsset.cs (1)
PostAsset(5-16)Dentizone.Domain/Entity/AppUser.cs (1)
AppUser(6-32)
🔇 Additional comments (2)
Dentizone.Domain/Entity/Asset.cs (1)
20-20: Navigation property should be nullable or guarded.
public virtual AppUser User { get; set; }is non-nullable but can be null for orphan assets, causing NRT warnings at call-sites.
Either mark it nullable (AppUser?) or enforce required relationship in the DbContext.Dentizone.Application/DTOs/Asset/CreateAssetDto.cs (1)
12-13:UserIdadded here but missing fromAssetDto.The read-model (
AssetDto) no longer mirrors the write-model – callers that rely onAssetDtocannot see the associated user.
Ensure the projection DTO is updated or intentionally omit the field with justification.
gitnasr
left a comment
gitnasr
left a comment
There was a problem hiding this comment.
Approved with No Changes
Summary by CodeRabbit