Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions backend/src/Taskdeck.Api/Controllers/NotificationsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@ public async Task<IActionResult> MarkAsRead(Guid id, CancellationToken cancellat
return result.IsSuccess ? Ok(result.Value) : result.ToErrorActionResult();
}

[HttpPost("mark-all-read")]
public async Task<IActionResult> MarkAllAsRead(
[FromQuery] Guid? boardId = null,
CancellationToken cancellationToken = default)
{
if (!TryGetCurrentUserId(out var userId, out var errorResult))
return errorResult!;

var result = await _notificationService.MarkAllAsReadAsync(userId, boardId, cancellationToken);
return result.IsSuccess ? Ok(new { markedCount = result.Value }) : result.ToErrorActionResult();
}

[HttpGet("preferences")]
public async Task<IActionResult> GetPreferences(CancellationToken cancellationToken = default)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,9 @@ Task<IEnumerable<Notification>> GetByUserIdAsync(
Guid userId,
string deduplicationKey,
CancellationToken cancellationToken = default);

Task<IEnumerable<Notification>> GetUnreadByUserIdAsync(
Guid userId,
Guid? boardId = null,
CancellationToken cancellationToken = default);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ Task<Result<NotificationDto>> MarkAsReadAsync(
Guid notificationId,
CancellationToken cancellationToken = default);

Task<Result<int>> MarkAllAsReadAsync(
Guid userId,
Guid? boardId = null,
CancellationToken cancellationToken = default);

Task<Result<NotificationPreferenceDto>> GetPreferencesAsync(
Guid userId,
CancellationToken cancellationToken = default);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ public Task<Result<NotificationDto>> MarkAsReadAsync(
return Task.FromResult(Result.Failure<NotificationDto>(ErrorCodes.NotFound, "Notification not found"));
}

public Task<Result<int>> MarkAllAsReadAsync(
Guid userId,
Guid? boardId = null,
CancellationToken cancellationToken = default)
{
return Task.FromResult(Result.Success(0));
}

public Task<Result<NotificationPreferenceDto>> GetPreferencesAsync(
Guid userId,
CancellationToken cancellationToken = default)
Expand Down
41 changes: 41 additions & 0 deletions backend/src/Taskdeck.Application/Services/NotificationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,43 @@ public async Task<Result<NotificationDto>> MarkAsReadAsync(
return Result.Success(MapToDto(notification));
}

public async Task<Result<int>> MarkAllAsReadAsync(
Guid userId,
Guid? boardId = null,
CancellationToken cancellationToken = default)
{
if (userId == Guid.Empty)
return Result.Failure<int>(ErrorCodes.ValidationError, "User ID cannot be empty");

if (boardId.HasValue && _authorizationService is not null)
{
var boardPermission = await _authorizationService.CanReadBoardAsync(userId, boardId.Value);
if (!boardPermission.IsSuccess || !boardPermission.Value)
{
return Result.Failure<int>(
ErrorCodes.Forbidden,
"You do not have access to notifications for this board");
}
}

var unreadNotifications = await _unitOfWork.Notifications.GetUnreadByUserIdAsync(
userId, boardId, cancellationToken);

var count = 0;
foreach (var notification in unreadNotifications)
{
notification.MarkAsRead();
count++;
}
Comment on lines +106 to +114
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of MarkAllAsReadAsync fetches all unread notifications into memory and iterates over them to update each one individually. For users with a large volume of unread notifications, this can lead to significant memory pressure and performance issues. Consider implementing a bulk update operation in the repository (e.g., using EF Core's ExecuteUpdateAsync) to perform this change directly in the database.


if (count > 0)
{
await _unitOfWork.SaveChangesAsync(cancellationToken);
}

return Result.Success(count);
}

public async Task<Result<NotificationPreferenceDto>> GetPreferencesAsync(
Guid userId,
CancellationToken cancellationToken = default)
Expand Down Expand Up @@ -218,6 +255,8 @@ private static bool TryResolveCadence(
NotificationType.Mention => preference.MentionImmediateEnabled,
NotificationType.Assignment => preference.AssignmentImmediateEnabled,
NotificationType.ProposalOutcome => preference.ProposalOutcomeImmediateEnabled,
NotificationType.BoardChange => true,
NotificationType.System => true,
_ => false
};

Expand All @@ -226,6 +265,8 @@ private static bool TryResolveCadence(
NotificationType.Mention => preference.MentionDigestEnabled,
NotificationType.Assignment => preference.AssignmentDigestEnabled,
NotificationType.ProposalOutcome => preference.ProposalOutcomeDigestEnabled,
NotificationType.BoardChange => false,
NotificationType.System => false,
_ => false
};

Expand Down
4 changes: 3 additions & 1 deletion backend/src/Taskdeck.Domain/Entities/Notification.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ public enum NotificationType
{
Mention,
Assignment,
ProposalOutcome
ProposalOutcome,
BoardChange,
System
}

public enum NotificationCadence
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,20 @@ public async Task<IEnumerable<Notification>> GetByUserIdAsync(
n => n.UserId == userId && n.DeduplicationKey == deduplicationKey,
cancellationToken);
}

public async Task<IEnumerable<Notification>> GetUnreadByUserIdAsync(
Guid userId,
Guid? boardId = null,
CancellationToken cancellationToken = default)
{
var query = _dbSet
.Where(n => n.UserId == userId && !n.IsRead);

if (boardId.HasValue)
{
query = query.Where(n => n.BoardId == boardId.Value);
}

return await query.ToListAsync(cancellationToken);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Fetching all unread notifications without a limit can be dangerous for performance if a user has accumulated a very large number of notifications. Since this method is primarily used for batch updates, consider if a bulk update approach would be more appropriate, or at least apply a reasonable maximum limit to the query.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,66 @@ public async Task GetNotificationsAsync_ShouldReturnForbidden_WhenBoardLookupFai
result.ErrorMessage.Should().Be("You do not have access to notifications for this board");
}

[Fact]
public async Task MarkAllAsReadAsync_ShouldMarkAllUnread_WhenNotificationsExist()
{
var userId = Guid.NewGuid();
var n1 = new Notification(userId, NotificationType.Mention, NotificationCadence.Immediate, "N1", "Message 1");
var n2 = new Notification(userId, NotificationType.Assignment, NotificationCadence.Immediate, "N2", "Message 2");

_notificationRepositoryMock
.Setup(r => r.GetUnreadByUserIdAsync(userId, null, default))
.ReturnsAsync(new[] { n1, n2 });

var result = await _service.MarkAllAsReadAsync(userId);

result.IsSuccess.Should().BeTrue();
result.Value.Should().Be(2);
n1.IsRead.Should().BeTrue();
n2.IsRead.Should().BeTrue();
_unitOfWorkMock.Verify(u => u.SaveChangesAsync(default), Times.Once);
}

[Fact]
public async Task MarkAllAsReadAsync_ShouldReturnZero_WhenNoUnreadNotifications()
{
var userId = Guid.NewGuid();
_notificationRepositoryMock
.Setup(r => r.GetUnreadByUserIdAsync(userId, null, default))
.ReturnsAsync(Array.Empty<Notification>());

var result = await _service.MarkAllAsReadAsync(userId);

result.IsSuccess.Should().BeTrue();
result.Value.Should().Be(0);
_unitOfWorkMock.Verify(u => u.SaveChangesAsync(default), Times.Never);
}

[Fact]
public async Task MarkAllAsReadAsync_ShouldReturnForbidden_WhenBoardAccessDenied()
{
var userId = Guid.NewGuid();
var boardId = Guid.NewGuid();

_authorizationServiceMock
.Setup(s => s.CanReadBoardAsync(userId, boardId))
.ReturnsAsync(Result.Success(false));

var result = await _service.MarkAllAsReadAsync(userId, boardId);

result.IsSuccess.Should().BeFalse();
result.ErrorCode.Should().Be(ErrorCodes.Forbidden);
}

[Fact]
public async Task MarkAllAsReadAsync_ShouldReturnValidationError_WhenUserIdEmpty()
{
var result = await _service.MarkAllAsReadAsync(Guid.Empty);

result.IsSuccess.Should().BeFalse();
result.ErrorCode.Should().Be(ErrorCodes.ValidationError);
}

[Fact]
public async Task PublishAsync_ShouldAvoidDuplicatesWithinSameUnitOfWork_WhenPreferenceIsNotPersistedYet()
{
Expand Down
7 changes: 7 additions & 0 deletions frontend/taskdeck-web/src/api/notificationsApi.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import http from './http'
import { buildQueryString } from '../utils/queryBuilder'
import type {
MarkAllReadResponse,
NotificationItem,
NotificationPreference,
NotificationQuery,
Expand All @@ -18,6 +19,12 @@ export const notificationsApi = {
return data
},

async markAllRead(boardId?: string): Promise<MarkAllReadResponse> {
const qs = boardId ? `?boardId=${encodeURIComponent(boardId)}` : ''
const { data } = await http.post<MarkAllReadResponse>(`/notifications/mark-all-read${qs}`)
return data
},

async getPreferences(): Promise<NotificationPreference> {
const { data } = await http.get<NotificationPreference>('/notifications/preferences')
return data
Expand Down
Loading
Loading