Conversation
Working on adding functionality to allow user to do the typical REST actions to a clothing item (get, post, put, delete). This means I neeeded to reuse a lot of old code and move things around. Many tests are probably broken too now.
…re deleted items" feature)
I LOVE VALIDATION
…d warning suppressions for CS8618
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# | Feb 28, 2026 3:08a.m. | Review ↗ | |
| Test coverage | Feb 28, 2026 3:09a.m. | Review ↗ |
Code Coverage Summary
| Language | Line Coverage (New Code) | Line Coverage (Overall) |
|---|---|---|
| Aggregate | 16.6% [⤫ below threshold] |
6% [⤫ below threshold] |
| C# | 16.6% |
6% |
➟ Additional coverage metrics may have been reported. See full coverage report ↗
There was a problem hiding this comment.
Pull request overview
This PR (“US-003”) expands wardrobe functionality end-to-end by introducing new clothing DTOs and validation, adding API endpoints/service methods for clothing CRUD (plus image persistence/deletion), and updating the Blazor Presentation UI/navigation to support viewing and adding wardrobe items.
Changes:
- Added shared DTOs/enums/constants and FluentValidation-backed validation utilities for creating clothing items.
- Implemented API clothing endpoints/services, DB migration for
Size, and image save/delete behavior. - Added/updated Presentation view models/pages/navigation and moved typography utilities into Tailwind.
Reviewed changes
Copilot reviewed 58 out of 60 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| WardrobeManager.Shared/WardrobeManager.Shared.csproj | Adds FluentValidation + suppresses CS8618. |
| WardrobeManager.Shared/StaticResources/StaticValidators.cs | Introduces static validator registry + Validate<T>. |
| WardrobeManager.Shared/StaticResources/ProjectConstants.cs | Adds max image size fallback constant. |
| WardrobeManager.Shared/Services/MiscMethods.cs | Refactors to DI-based IMiscMethods implementation. |
| WardrobeManager.Shared/Services/IMiscMethods.cs | Adds interface for misc utilities. |
| WardrobeManager.Shared/Models/Result.cs | Adds generic result wrapper type. |
| WardrobeManager.Shared/Models/NewOrEditedClothingItemDTO.cs | Removes legacy DTO. |
| WardrobeManager.Shared/Models/ClientClothingItem.cs | Removes legacy model. |
| WardrobeManager.Shared/Enums/ClothingSize.cs | Adds clothing size enum. |
| WardrobeManager.Shared/DTOs/NewClothingItemDTO.cs | Adds DTO for create-clothing flow. |
| WardrobeManager.Shared/DTOs/EditedUserDTO.cs | Moves DTO into Shared.DTOs namespace. |
| WardrobeManager.Shared/DTOs/ClothingItemDTO.cs | Adds DTO for clothing items returned to client. |
| WardrobeManager.Shared.Tests/WardrobeManager.Shared.Tests.csproj | Adds StaticResources folder entry. |
| WardrobeManager.Shared.Tests/StaticResources/MiscMethodsTests.cs | Removes old static MiscMethods tests. |
| WardrobeManager.Presentation/wwwroot/index.html | Stops including deleted custom.css. |
| WardrobeManager.Presentation/wwwroot/css/sysinfocus-styles.css | Comments out h1-h4 overrides. |
| WardrobeManager.Presentation/wwwroot/css/custom.css | Deletes old typography utility CSS. |
| WardrobeManager.Presentation/tailwind.css | Replaces custom typography CSS with Tailwind utilities/theme. |
| WardrobeManager.Presentation/WardrobeManager.Presentation.csproj | Adds UserSecretsId. |
| WardrobeManager.Presentation/ViewModels/WardrobeViewModel.cs | Adds wardrobe listing + delete dialog state. |
| WardrobeManager.Presentation/ViewModels/SignupViewModel.cs | Removes unused DI parameter. |
| WardrobeManager.Presentation/ViewModels/NavMenuViewModel.cs | Initializes UsersName to empty string. |
| WardrobeManager.Presentation/ViewModels/LoginViewModel.cs | Removes unused DI params and EditForm reference. |
| WardrobeManager.Presentation/ViewModels/HomeViewModel.cs | Removes unused DI parameters. |
| WardrobeManager.Presentation/ViewModels/DashboardViewModel.cs | Removes unused DI parameters. |
| WardrobeManager.Presentation/ViewModels/AddClothingItemViewModel.cs | Adds create-clothing + image upload flow. |
| WardrobeManager.Presentation/Services/Interfaces/IApiService.cs | Adds clothing API methods + onboarding docs. |
| WardrobeManager.Presentation/Services/Implementation/ApiService.cs | Implements clothing API calls. |
| WardrobeManager.Presentation/Program.cs | Replaces CookieHandler with CustomHttpMessageHandler; registers IMiscMethods. |
| WardrobeManager.Presentation/Pages/Public/Login.razor | Removes EditForm @ref usage. |
| WardrobeManager.Presentation/Pages/Authenticated/Wardrobe.razor | Adds wardrobe page + delete dialog UI. |
| WardrobeManager.Presentation/Pages/Authenticated/Clothing.razor | Removes old placeholder page. |
| WardrobeManager.Presentation/Pages/Authenticated/AddClothingItem.razor | Builds add-item form UI + image preview. |
| WardrobeManager.Presentation/Layout/NavMenu.razor | Updates authorized nav links and popover formatting. |
| WardrobeManager.Presentation/Identity/CustomAuthorizationMessageHandler.cs | Moves handler into Identity namespace. |
| WardrobeManager.Presentation/Identity/CookieHandler.cs | Removes old cookie delegating handler. |
| WardrobeManager.Presentation/Identity/CookieAuthenticationStateProvider.cs | Removes unused auth state field/mutations. |
| WardrobeManager.Presentation/CustomHttpMessageHandler.cs | Adds delegating handler w/ cookie include + notifications. |
| WardrobeManager.Presentation/Components/Shared/Notifications.razor | Minor markup tweak. |
| WardrobeManager.Presentation/Components/Shared/Image.razor | Adds lazy loading; changes ImageGuid param type to string. |
| WardrobeManager.Presentation/Components/Identity/NotAuthenticated.razor | Improves layout/typography classes. |
| WardrobeManager.Api/WardrobeManager.Api.csproj | Adds CS8618 suppression + formatting. |
| WardrobeManager.Api/Services/Interfaces/IUserService.cs | Adds DTO import for edited user. |
| WardrobeManager.Api/Services/Interfaces/IFileService.cs | Adds image deletion; renames parameter. |
| WardrobeManager.Api/Services/Interfaces/IDataDirectoryService.cs | Adds deleted uploads directory accessor. |
| WardrobeManager.Api/Services/Interfaces/IClothingService.cs | Switches to DTO-returning interface; adds add/delete methods. |
| WardrobeManager.Api/Services/Implementation/UserService.cs | Imports DTO namespace. |
| WardrobeManager.Api/Services/Implementation/FileService.cs | Fixes max size calc; adds DeleteImage + deleted folder behavior. |
| WardrobeManager.Api/Services/Implementation/DataDirectoryService.cs | Implements deleted uploads directory. |
| WardrobeManager.Api/Services/Implementation/ClothingService.cs | Adds DTO mapping + add/delete (incl. image save/delete). |
| WardrobeManager.Api/Repositories/Implementation/ClothingRepository.cs | Simplifies constructor to standard DI pattern. |
| WardrobeManager.Api/Program.cs | Adds AutoMapper maps; registers IClothingRepository + IMiscMethods. |
| WardrobeManager.Api/Migrations/DatabaseContextModelSnapshot.cs | Updates snapshot for ClothingItem + Size. |
| WardrobeManager.Api/Migrations/20260227172446_AddSizeToClothingItem.cs | Adds Size column migration. |
| WardrobeManager.Api/Migrations/20260227172446_AddSizeToClothingItem.Designer.cs | Adds migration designer output. |
| WardrobeManager.Api/Middleware/UserCreationMiddleware.cs | Populates HttpContext.Items["user"] from claims. |
| WardrobeManager.Api/Endpoints/UserEndpoints.cs | Imports DTO namespace. |
| WardrobeManager.Api/Endpoints/ClothingEndpoints.cs | Adds /clothing add/delete endpoints + validation call. |
| WardrobeManager.Api/Database/Entities/ClothingItem.cs | Adds Size property; removes local warning suppression. |
| WardrobeManager.Api.Tests/Services/ClothingServiceTests.cs | Updates tests for DTO mapping + add/delete behaviors. |
Files not reviewed (1)
- WardrobeManager.Api/Migrations/20260227172446_AddSizeToClothingItem.Designer.cs: Language not supported
Comments suppressed due to low confidence (13)
WardrobeManager.Api/Endpoints/ClothingEndpoints.cs:41
- This endpoint uses
Debug.Assert(user != null)and then dereferencesuser.Id. In Release builds the assert is removed, so a missing user inHttpContext.Itemswill become a null dereference and 500. Return an explicit 401/403 (or problem response) when the middleware did not populate the user.
User? user = context.Items["user"] as User;
Debug.Assert(user != null, "Cannot get user");
var clothes = await clothingService.GetAllClothingAsync(user.Id);
WardrobeManager.Api/Endpoints/ClothingEndpoints.cs:48
IMapper mapperis injected into this handler but never used. Remove the unused parameter to avoid unnecessary DI resolution and to keep the handler signature focused.
public static async Task<IResult> AddNewClothingItem(
[FromBody] NewClothingItemDTO newClothingItem,
HttpContext context, IClothingService clothingService, IMapper mapper
)
WardrobeManager.Presentation/ViewModels/AddClothingItemViewModel.cs:55
UploadImageallocates aMemoryStreambut never disposes it. Since this can hold multi‑MB buffers, useusing/await usingto dispose the stream after converting to base64.
WardrobeManager.Api/Services/Implementation/FileService.cs:44- The file size exception message divides by 1024 but labels the result as MB, which will report KB values as MB. Convert bytes to MB using
1024 * 1024(or adjust the label) so operators/users get accurate diagnostics.
if (imageBytes.Length > maxFileSizeNum)
{
throw new Exception(
$"File size too large! Received file size: {imageBytes.Length / 1024} MB. Max file size: {maxFileSizeNum / 1024} MB");
}
WardrobeManager.Api/Endpoints/ClothingEndpoints.cs:66
IMapper mapperis also injected here but unused. Remove it from this handler signature as well.
public static async Task<IResult> DeleteClothingItem(
[FromBody] int itemId,
HttpContext context, IClothingService clothingService, IMapper mapper
)
WardrobeManager.Api/Services/Implementation/FileService.cs:73
DeleteImageis declaredasyncbut has no awaits (all operations are synchronous). Consider removingasyncand returningTask.CompletedTask(or making the file move async) so the signature accurately reflects behavior.
public async Task DeleteImage(Guid givenGuid)
{
var guid = ParseGuid(givenGuid);
string path = Path.Combine(dataDirectoryService.GetUploadsDirectory(), guid);
string deletePath = Path.Combine(dataDirectoryService.GetDeletedUploadsDirectory(), guid);
WardrobeManager.Presentation/ViewModels/AddClothingItemViewModel.cs:5
- Avoid referencing
CommunityToolkit.Mvvm.ComponentModel.__Internals; it's not a public API surface and can break on package updates. Remove this using (and any dependent usage) and rely on the publicCommunityToolkit.Mvvm.ComponentModelAPIs instead.
WardrobeManager.Presentation/ViewModels/AddClothingItemViewModel.cs:65 - The config value
WM_MAX_IMAGE_UPLOAD_SIZE_IN_MBis parsed withConvert.ToInt32but onlyIOExceptionis caught. If the value is missing/invalid (e.g., non-numeric), this will throw and bypass the notification path. Handle parsing failures (e.g.,int.TryParse) and/or broaden the catch to show a user-friendly error.
WardrobeManager.Presentation/Layout/NavMenu.razor:57 - Wrapping a
<Button>inside an<a href>creates nested interactive elements (invalid HTML) and can cause accessibility and click/keyboard issues. PreferNavLinkor have the button trigger navigation directly instead of<a><Button/></a>.
<a href="/wardrobe">
<Button Type="ButtonType.Outline"
Text="My Wardrobe"
Icon="Person"/>
</a>
WardrobeManager.Api/Services/Implementation/FileService.cs:5
SQLitePCLis imported here but not used. Remove the unused using to avoid IDE/analysis noise and keep dependencies clear.
#region
using SQLitePCL;
using WardrobeManager.Api.Services.Interfaces;
WardrobeManager.Api/Middleware/UserCreationMiddleware.cs:30
NameIdentifieris defaulted to an empty string and then passed intoGetUser, which can cause unnecessary lookups and can hide missing-claim issues. Skip the lookup when the claim is null/empty socontext.Items["user"]is only set when you have a valid user id.
var userService = context.RequestServices.GetRequiredService<IUserService>();
var userId = context.User.FindFirst(ClaimTypes.NameIdentifier)?.Value ?? "";
var user = await userService.GetUser(userId);
if (user != null)
WardrobeManager.Shared/StaticResources/StaticValidators.cs:20
StaticValidators.Validateadds new shared validation behavior used across layers, but there are no unit tests covering null input, missing validators, or aggregated error messages. Add tests inWardrobeManager.Shared.Teststo lock down this behavior and avoid regressions.
WardrobeManager.Presentation/Services/Implementation/ApiService.cs:30- There are existing unit tests for
ApiService, but the newly added clothing methods are not covered. Add tests for the new GET/POST clothing calls (including error responses) to keep coverage consistent for this service.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.