Uncomment NotificationController and fix DTO references#103
Uncomment NotificationController and fix DTO references#103
Conversation
Uncommented the NotificationController and updated it to follow current architectural patterns, including API versioning, proper DTOs from Contracts namespace, and BaseController inheritance. Fixed repository interface inconsistencies by changing Delete method parameter from int to string to match entity ID type. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
There was a problem hiding this comment.
Pull Request Overview
This PR uncomments and modernizes the NotificationController to align with current architectural patterns. The changes include implementing API versioning, using proper DTOs from the Contracts namespace, inheriting from BaseController, and fixing a type inconsistency in the repository's Delete method parameter.
- Updated NotificationController to follow current patterns with API versioning, BaseController inheritance, and proper DTOs
- Fixed repository interface type mismatch by changing Delete method parameter from
inttostring - Added NotificationResponse.Id property and corresponding API endpoints
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| NotificationController.cs | Uncommented and modernized controller with API versioning, BaseController inheritance, proper DTOs, and all CRUD operations |
| ApiEndpoints.cs | Added Notifications endpoint constants for routing consistency |
| NotificationRepository.cs | Updated Delete method parameter type from int to string to match entity ID type |
| INotificationRepository.cs | Updated Delete method signature to use string instead of int |
| NotificationResponse.cs | Added Id property to response DTO |
| IsRead = request.IsRead, | ||
| UserId = "temp-user-id", // TODO: Get from authenticated user context | ||
| CreatedAt = DateTime.UtcNow, | ||
| UpdatedAt = DateTime.UtcNow |
There was a problem hiding this comment.
The property name should be 'LastUpdatedAt' to match the BaseEntity property. The current code sets 'UpdatedAt' which doesn't exist on NotificationEntity (which inherits from BaseEntity with 'LastUpdatedAt' property).
| UpdatedAt = DateTime.UtcNow | |
| LastUpdatedAt = DateTime.UtcNow |
| UpdatedAt = DateTime.UtcNow | ||
| }; | ||
|
|
||
| await _notificationRepository.AddAsync(newNotification); |
There was a problem hiding this comment.
Missing SaveChangesAsync call after AddAsync. The repository pattern used here doesn't auto-save changes. Based on IntegrationController and AdminController patterns in this codebase, you should inject IUnitOfWork and call 'await _unitOfWork.SaveChangesAsync(cancellationToken);' after AddAsync to persist the changes to the database.
| NotificationEntity? notification = await _notificationRepository.GetByIdAsync(id); | ||
| if (notification is null) return NotFound(new { Message = $"Notification with ID {id} not found." }); | ||
|
|
||
| _notificationRepository.Delete(id); |
There was a problem hiding this comment.
Missing SaveChangesAsync call after Delete. The repository pattern used here doesn't auto-save changes. Based on IntegrationController and AdminController patterns in this codebase, you should inject IUnitOfWork and call 'await _unitOfWork.SaveChangesAsync(cancellationToken);' after Delete to persist the changes to the database.
Uncommented the NotificationController and updated it to follow current architectural patterns, including API versioning, proper DTOs from Contracts namespace, and BaseController inheritance. Fixed repository interface inconsistencies by changing Delete method parameter from int to string to match entity ID type.
🤖 Generated with Claude Code
Summary & Motivation
A brief description of the changes in this pull request explaining why these changes are necessary. Please delete this paragraph.
Checklist for Pull Request - Merge from development into main (production) branch