1
0
mirror of https://github.com/bitwarden/server.git synced 2025-07-02 16:42:50 -05:00

[PM-20041] Deleting Notifications when Task is completed (#5896)

* mark all notifications associated with a security task as deleted when the task is completed

* fix spelling

* formatting

* refactor "Active" to "NonDeleted"

* refactor "Active" to "NonDeleted" for stored procedure

* only send notifications per user for each notification

* move notification status updates into the DB layer to save on multiple queries and insertions from the C#

* Only return UserIds from db layer

* omit userId from `MarkTaskAsCompletedCommand` query.

The userId from the notification will be used

* update UserIds

* consistency in comments regarding `taskId` and `UserId`
This commit is contained in:
Nick Krantz
2025-06-27 16:04:47 -05:00
committed by GitHub
parent c441fa27dd
commit 69b7600eab
10 changed files with 198 additions and 2 deletions

View File

@ -32,4 +32,13 @@ public interface INotificationRepository : IRepository<Notification, Guid>
/// </returns> /// </returns>
Task<PagedResult<NotificationStatusDetails>> GetByUserIdAndStatusAsync(Guid userId, ClientType clientType, Task<PagedResult<NotificationStatusDetails>> GetByUserIdAndStatusAsync(Guid userId, ClientType clientType,
NotificationStatusFilter? statusFilter, PageOptions pageOptions); NotificationStatusFilter? statusFilter, PageOptions pageOptions);
/// <summary>
/// Marks notifications as deleted by a taskId.
/// </summary>
/// <param name="taskId">The unique identifier of the task.</param>
/// <returns>
/// A collection of UserIds for the notifications that are now marked as deleted.
/// </returns>
Task<IEnumerable<Guid>> MarkNotificationsAsDeletedByTask(Guid taskId);
} }

View File

@ -0,0 +1,11 @@
namespace Bit.Core.Vault.Commands.Interfaces;
public interface IMarkNotificationsForTaskAsDeletedCommand
{
/// <summary>
/// Marks notifications associated with a given taskId as deleted.
/// </summary>
/// <param name="taskId">The unique identifier of the task to complete</param>
/// <returns>A task representing the async operation</returns>
Task MarkAsDeletedAsync(Guid taskId);
}

View File

@ -0,0 +1,32 @@
using Bit.Core.NotificationCenter.Repositories;
using Bit.Core.Platform.Push;
using Bit.Core.Vault.Commands.Interfaces;
namespace Bit.Core.Vault.Commands;
public class MarkNotificationsForTaskAsDeletedCommand : IMarkNotificationsForTaskAsDeletedCommand
{
private readonly INotificationRepository _notificationRepository;
private readonly IPushNotificationService _pushNotificationService;
public MarkNotificationsForTaskAsDeletedCommand(
INotificationRepository notificationRepository,
IPushNotificationService pushNotificationService)
{
_notificationRepository = notificationRepository;
_pushNotificationService = pushNotificationService;
}
public async Task MarkAsDeletedAsync(Guid taskId)
{
var userIds = await _notificationRepository.MarkNotificationsAsDeletedByTask(taskId);
// For each user associated with the notifications, send a push notification so local tasks can be updated.
var uniqueUserIds = userIds.Distinct();
foreach (var id in uniqueUserIds)
{
await _pushNotificationService.PushPendingSecurityTasksAsync(id);
}
}
}

View File

@ -14,15 +14,19 @@ public class MarkTaskAsCompletedCommand : IMarkTaskAsCompleteCommand
private readonly ISecurityTaskRepository _securityTaskRepository; private readonly ISecurityTaskRepository _securityTaskRepository;
private readonly IAuthorizationService _authorizationService; private readonly IAuthorizationService _authorizationService;
private readonly ICurrentContext _currentContext; private readonly ICurrentContext _currentContext;
private readonly IMarkNotificationsForTaskAsDeletedCommand _markNotificationsForTaskAsDeletedAsync;
public MarkTaskAsCompletedCommand( public MarkTaskAsCompletedCommand(
ISecurityTaskRepository securityTaskRepository, ISecurityTaskRepository securityTaskRepository,
IAuthorizationService authorizationService, IAuthorizationService authorizationService,
ICurrentContext currentContext) ICurrentContext currentContext,
IMarkNotificationsForTaskAsDeletedCommand markNotificationsForTaskAsDeletedAsync)
{ {
_securityTaskRepository = securityTaskRepository; _securityTaskRepository = securityTaskRepository;
_authorizationService = authorizationService; _authorizationService = authorizationService;
_currentContext = currentContext; _currentContext = currentContext;
_markNotificationsForTaskAsDeletedAsync = markNotificationsForTaskAsDeletedAsync;
} }
/// <inheritdoc /> /// <inheritdoc />
@ -46,5 +50,8 @@ public class MarkTaskAsCompletedCommand : IMarkTaskAsCompleteCommand
task.RevisionDate = DateTime.UtcNow; task.RevisionDate = DateTime.UtcNow;
await _securityTaskRepository.ReplaceAsync(task); await _securityTaskRepository.ReplaceAsync(task);
// Mark all notifications related to this task as deleted
await _markNotificationsForTaskAsDeletedAsync.MarkAsDeletedAsync(taskId);
} }
} }

View File

@ -55,7 +55,7 @@ public interface ICipherRepository : IRepository<Cipher, Guid>
Guid userId); Guid userId);
/// <summary> /// <summary>
/// Returns the users and the cipher ids for security tawsks that are applicable to them. /// Returns the users and the cipher ids for security tasks that are applicable to them.
/// ///
/// Security tasks are actionable when a user has manage access to the associated cipher. /// Security tasks are actionable when a user has manage access to the associated cipher.
/// </summary> /// </summary>

View File

@ -24,5 +24,6 @@ public static class VaultServiceCollectionExtensions
services.AddScoped<IGetSecurityTasksNotificationDetailsQuery, GetSecurityTasksNotificationDetailsQuery>(); services.AddScoped<IGetSecurityTasksNotificationDetailsQuery, GetSecurityTasksNotificationDetailsQuery>();
services.AddScoped<ICreateManyTaskNotificationsCommand, CreateManyTaskNotificationsCommand>(); services.AddScoped<ICreateManyTaskNotificationsCommand, CreateManyTaskNotificationsCommand>();
services.AddScoped<ICreateManyTasksCommand, CreateManyTasksCommand>(); services.AddScoped<ICreateManyTasksCommand, CreateManyTasksCommand>();
services.AddScoped<IMarkNotificationsForTaskAsDeletedCommand, MarkNotificationsForTaskAsDeletedCommand>();
} }
} }

View File

@ -56,4 +56,21 @@ public class NotificationRepository : Repository<Notification, Guid>, INotificat
ContinuationToken = data.Count < pageOptions.PageSize ? null : (pageNumber + 1).ToString() ContinuationToken = data.Count < pageOptions.PageSize ? null : (pageNumber + 1).ToString()
}; };
} }
public async Task<IEnumerable<Guid>> MarkNotificationsAsDeletedByTask(Guid taskId)
{
await using var connection = new SqlConnection(ConnectionString);
var results = await connection.QueryAsync<Guid>(
"[dbo].[Notification_MarkAsDeletedByTask]",
new
{
TaskId = taskId,
},
commandType: CommandType.StoredProcedure);
var data = results.ToList();
return data;
}
} }

View File

@ -74,4 +74,53 @@ public class NotificationRepository : Repository<Core.NotificationCenter.Entitie
ContinuationToken = results.Count < pageOptions.PageSize ? null : (pageNumber + 1).ToString() ContinuationToken = results.Count < pageOptions.PageSize ? null : (pageNumber + 1).ToString()
}; };
} }
public async Task<IEnumerable<Guid>> MarkNotificationsAsDeletedByTask(Guid taskId)
{
await using var scope = ServiceScopeFactory.CreateAsyncScope();
var dbContext = GetDatabaseContext(scope);
var notifications = await dbContext.Notifications
.Where(n => n.TaskId == taskId)
.ToListAsync();
var notificationIds = notifications.Select(n => n.Id).ToList();
var statuses = await dbContext.Set<NotificationStatus>()
.Where(ns => notificationIds.Contains(ns.NotificationId))
.ToListAsync();
var now = DateTime.UtcNow;
// Update existing statuses and add missing ones
foreach (var notification in notifications)
{
var status = statuses.FirstOrDefault(s => s.NotificationId == notification.Id);
if (status != null)
{
if (status.DeletedDate == null)
{
status.DeletedDate = now;
}
}
else if (notification.UserId.HasValue)
{
dbContext.Set<NotificationStatus>().Add(new NotificationStatus
{
NotificationId = notification.Id,
UserId = (Guid)notification.UserId,
DeletedDate = now
});
}
}
await dbContext.SaveChangesAsync();
var userIds = notifications
.Select(n => n.UserId)
.Where(u => u.HasValue)
.ToList();
return (IEnumerable<Guid>)userIds;
}
} }

View File

@ -0,0 +1,35 @@
CREATE PROCEDURE [dbo].[Notification_MarkAsDeletedByTask]
@TaskId UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON;
-- Collect UserIds as they are altered
DECLARE @UserIdsForAlteredNotifications TABLE (
UserId UNIQUEIDENTIFIER
);
-- Update existing NotificationStatus as deleted
UPDATE ns
SET ns.DeletedDate = GETUTCDATE()
OUTPUT inserted.UserId INTO @UserIdsForAlteredNotifications
FROM NotificationStatus ns
INNER JOIN Notification n ON ns.NotificationId = n.Id
WHERE n.TaskId = @TaskId
AND ns.DeletedDate IS NULL;
-- Insert NotificationStatus records for notifications that don't have one yet
INSERT INTO NotificationStatus (NotificationId, UserId, DeletedDate)
OUTPUT inserted.UserId INTO @UserIdsForAlteredNotifications
SELECT n.Id, n.UserId, GETUTCDATE()
FROM Notification n
LEFT JOIN NotificationStatus ns
ON n.Id = ns.NotificationId
WHERE n.TaskId = @TaskId
AND ns.NotificationId IS NULL;
-- Return the UserIds associated with the altered notifications
SELECT u.UserId
FROM @UserIdsForAlteredNotifications u;
END
GO

View File

@ -0,0 +1,35 @@
CREATE OR ALTER PROCEDURE [dbo].[Notification_MarkAsDeletedByTask]
@TaskId UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON;
-- Collect UserIds as they are altered
DECLARE @UserIdsForAlteredNotifications TABLE (
UserId UNIQUEIDENTIFIER
);
-- Update existing NotificationStatus as deleted
UPDATE ns
SET ns.DeletedDate = GETUTCDATE()
OUTPUT inserted.UserId INTO @UserIdsForAlteredNotifications
FROM NotificationStatus ns
INNER JOIN Notification n ON ns.NotificationId = n.Id
WHERE n.TaskId = @TaskId
AND ns.DeletedDate IS NULL;
-- Insert NotificationStatus records for notifications that don't have one yet
INSERT INTO NotificationStatus (NotificationId, UserId, DeletedDate)
OUTPUT inserted.UserId INTO @UserIdsForAlteredNotifications
SELECT n.Id, n.UserId, GETUTCDATE()
FROM Notification n
LEFT JOIN NotificationStatus ns
ON n.Id = ns.NotificationId
WHERE n.TaskId = @TaskId
AND ns.NotificationId IS NULL;
-- Return the UserIds associated with the altered notifications
SELECT u.UserId
FROM @UserIdsForAlteredNotifications u;
END
GO