[PM-10563] Notification Center API #26
Conversation
# Conflicts: # test/Core.Test/NotificationCenter/AutoFixture/NotificationFixtures.cs
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
21 file(s) reviewed, 31 comment(s)
Edit PR Review Bot Settings | Greptile
| notificationStatusDetailsPagedResult.ContinuationToken); | ||
| } | ||
|
|
||
| [HttpPatch("{id}/delete")] |
There was a problem hiding this comment.
style: Consider using HttpPut instead of HttpPatch for idempotent operations
| await _markNotificationDeletedCommand.MarkDeletedAsync(id); | ||
| } | ||
|
|
||
| [HttpPatch("{id}/read")] |
There was a problem hiding this comment.
style: Consider using HttpPut instead of HttpPatch for idempotent operations
| /// <summary> | ||
| /// A cursor for use in pagination. | ||
| /// </summary> | ||
| [StringLength(9)] |
There was a problem hiding this comment.
style: Consider increasing the maximum length to accommodate larger page numbers
| [Range(10, 1000)] | ||
| public int PageSize { get; set; } = 10; |
There was a problem hiding this comment.
style: Consider allowing a smaller minimum page size, such as 1, for more flexibility
| if (!string.IsNullOrWhiteSpace(ContinuationToken) && | ||
| (!int.TryParse(ContinuationToken, out var pageNumber) || pageNumber <= 0)) |
There was a problem hiding this comment.
style: Use long.TryParse instead of int.TryParse to handle larger page numbers
| RevisionDate = DateTime.UtcNow - TimeSpan.FromMinutes(3), | ||
| ReadDate = DateTime.UtcNow - TimeSpan.FromMinutes(1), | ||
| DeletedDate = DateTime.UtcNow, |
There was a problem hiding this comment.
style: Use fixed dates instead of DateTime.UtcNow for more predictable tests
| } | ||
| } | ||
|
|
||
| public class NotificationStatusDetailsListCustomization(int count) : ICustomization |
There was a problem hiding this comment.
style: Consider adding XML documentation for this new class to improve code clarity
| public class NotificationStatusDetailsListCustomizeAttribute(int count) : BitCustomizeAttribute | ||
| { | ||
| public override ICustomization GetCustomization() => new NotificationStatusDetailsListCustomization(count); | ||
| } |
There was a problem hiding this comment.
style: Add XML documentation for this new attribute class
| LEFT JOIN [dbo].[OrganizationUserView] ou ON n.[OrganizationId] = ou.[OrganizationId] | ||
| AND ou.[UserId] = @UserId | ||
| WHERE (n.[NotificationStatusUserId] IS NULL OR n.[NotificationStatusUserId] = @UserId) | ||
| AND [ClientType] IN (0, CASE WHEN @ClientType != 0 THEN @ClientType END) |
There was a problem hiding this comment.
logic: this logic might allow unintended client types if @ClientType is 0
| AND ((@Read IS NULL AND @Deleted IS NULL) | ||
| OR (n.[NotificationStatusUserId] IS NOT NULL | ||
| AND (@Read IS NULL | ||
| OR IIF((@Read = 1 AND n.[ReadDate] IS NOT NULL) OR | ||
| (@Read = 0 AND n.[ReadDate] IS NULL), | ||
| 1, 0) = 1) | ||
| AND (@Deleted IS NULL | ||
| OR IIF((@Deleted = 1 AND n.[DeletedDate] IS NOT NULL) OR | ||
| (@Deleted = 0 AND n.[DeletedDate] IS NULL), | ||
| 1, 0) = 1))) |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-10563
📔 Objective
Notification API:
MarkNotificationDeletedCommandandMarkNotificationReadCommandduring create operation - now UTC format📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changesGreptile Summary
This pull request introduces a Notification Center API with pagination and status management features for the Bitwarden server. Key changes include:
NotificationsControllerwith endpoints for listing, marking as read, and marking as deleted notificationsGetNotificationStatusDetailsForUserQueryand repository interfacesMarkNotificationDeletedCommandandMarkNotificationReadCommandto use UTC time consistentlyNotification_ReadByUserIdAndStatusto support pagination