From a7a8a2ad465ed65155c471cf2693ee1a2ba04c95 Mon Sep 17 00:00:00 2001 From: dyatlov-a Date: Tue, 10 Dec 2024 00:09:42 +0300 Subject: [PATCH] Field HasConcreteReviewer was removed from TaskForReview --- .../2024_12_09_0_ChangeReviewer.cs | 41 ++++++++++++++ .../ITeamAccessor.cs | 1 + .../EditDraft/EditDraftCommandHandler.cs | 9 +-- .../MoveToReviewCommandHandler.cs | 38 ++++++------- .../NeedReview/NeedReviewCommandHandler.cs | 22 ++++---- .../Contracts/TaskForReviewHistory.cs | 2 +- .../TaskForReviewHistoryConverter.cs | 3 +- .../ServiceCollectionExtensions.cs | 2 + .../Services/NextReviewerStrategyFactory.cs | 55 +++++++++++++++++++ .../Services/ReassignReviewService.cs | 33 ++++++----- .../Services/ReviewMessageBuilder.cs | 4 +- .../ReviewerSettingSectionProvider.cs | 4 +- .../TaskForReviewReader.cs | 2 +- .../TaskForReviewRepository.cs | 11 +--- .../DraftTaskForReview.cs | 17 ++---- .../INextReviewerStrategy.cs | 2 +- .../INextReviewerStrategyFactory.cs | 13 +++++ .../RandomReviewerStrategy.cs | 39 +++++++++---- .../RoundRobinReviewerStrategy.cs | 42 ++++++++++---- .../TargetReviewerStrategy.cs | 13 +++++ .../NextReviewerStrategyFactory.cs | 23 -------- .../NextReviewerType.cs | 3 +- .../TaskForReview.cs | 46 ++-------------- .../Services/ReviewMetricsProviderTests.cs | 5 +- .../RandomReviewerStrategyTests.cs | 48 +++++++++++++--- .../RoundRobinReviewerStrategyTests.cs | 25 ++++++--- 26 files changed, 315 insertions(+), 188 deletions(-) create mode 100644 src/Inc.TeamAssistant.Migrations/2024_12_09_0_ChangeReviewer.cs create mode 100644 src/Inc.TeamAssistant.Reviewer.Application/Services/NextReviewerStrategyFactory.cs create mode 100644 src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/INextReviewerStrategyFactory.cs create mode 100644 src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/TargetReviewerStrategy.cs delete mode 100644 src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategyFactory.cs diff --git a/src/Inc.TeamAssistant.Migrations/2024_12_09_0_ChangeReviewer.cs b/src/Inc.TeamAssistant.Migrations/2024_12_09_0_ChangeReviewer.cs new file mode 100644 index 00000000..cbfc4ff0 --- /dev/null +++ b/src/Inc.TeamAssistant.Migrations/2024_12_09_0_ChangeReviewer.cs @@ -0,0 +1,41 @@ +using FluentMigrator; + +namespace Inc.TeamAssistant.Migrations; + +[Migration(2024_12_09_0)] +public sealed class ChangeReviewer : Migration +{ + public override void Up() + { + Execute.Sql( + """ + UPDATE review.task_for_reviews AS t + SET strategy = 3 + WHERE t.has_concrete_reviewer + """, + "Set strategy by has_concrete_reviewer"); + + Delete + .Column("has_concrete_reviewer") + .FromTable("task_for_reviews") + .InSchema("review"); + } + + public override void Down() + { + Create + .Column("has_concrete_reviewer") + .OnTable("task_for_reviews") + .InSchema("review") + .AsBoolean() + .NotNullable() + .SetExistingRowsTo(false); + + Execute.Sql( + """ + UPDATE review.task_for_reviews AS t + SET has_concrete_reviewer = t.strategy = 3; + """, + "Set has_concrete_reviewer by strategy"); + } +} \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Primitives/ITeamAccessor.cs b/src/Inc.TeamAssistant.Primitives/ITeamAccessor.cs index b5c77e90..94d47f82 100644 --- a/src/Inc.TeamAssistant.Primitives/ITeamAccessor.cs +++ b/src/Inc.TeamAssistant.Primitives/ITeamAccessor.cs @@ -1,3 +1,4 @@ +using Inc.TeamAssistant.Primitives.Commands; using Inc.TeamAssistant.Primitives.Languages; namespace Inc.TeamAssistant.Primitives; diff --git a/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/EditDraft/EditDraftCommandHandler.cs b/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/EditDraft/EditDraftCommandHandler.cs index c51b47fa..234a9559 100644 --- a/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/EditDraft/EditDraftCommandHandler.cs +++ b/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/EditDraft/EditDraftCommandHandler.cs @@ -27,12 +27,9 @@ public async Task Handle(EditDraftCommand command, CancellationTo if (draft is not null) { - draft.WithDescription(command.Description); - - if (command.MessageContext.TargetPersonId.HasValue) - draft.WithTargetPerson(command.MessageContext.TargetPersonId.Value); - else - draft.WithoutTargetPerson(); + draft + .WithDescription(command.Description) + .SetTargetPerson(command.MessageContext.TargetPersonId); var notification = await _reviewMessageBuilder.Build(draft, command.MessageContext.LanguageId, token); diff --git a/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/MoveToReview/MoveToReviewCommandHandler.cs b/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/MoveToReview/MoveToReviewCommandHandler.cs index 95937c0a..75c315b2 100644 --- a/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/MoveToReview/MoveToReviewCommandHandler.cs +++ b/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/MoveToReview/MoveToReviewCommandHandler.cs @@ -1,4 +1,3 @@ -using Inc.TeamAssistant.Holidays.Extensions; using Inc.TeamAssistant.Primitives; using Inc.TeamAssistant.Primitives.Bots; using Inc.TeamAssistant.Primitives.Commands; @@ -7,6 +6,7 @@ using Inc.TeamAssistant.Reviewer.Application.Contracts; using Inc.TeamAssistant.Reviewer.Application.Services; using Inc.TeamAssistant.Reviewer.Domain; +using Inc.TeamAssistant.Reviewer.Domain.NextReviewerStrategies; using Inc.TeamAssistant.Reviewer.Model.Commands.MoveToReview; using MediatR; @@ -17,31 +17,32 @@ internal sealed class MoveToReviewCommandHandler : IRequestHandler Handle(MoveToReviewCommand command, CancellationToken token) @@ -60,29 +61,22 @@ public async Task Handle(MoveToReviewCommand command, Cancellatio if (!teammates.Any()) throw new TeamAssistantUserException(Messages.Reviewer_TeamWithoutUsers, draft.TeamId); + var nextReviewerStrategy = await _nextReviewerStrategyFactory.Create( + draft.TeamId, + draft.OwnerId, + draft.GetStrategy(), + draft.TargetPersonId, + teammates.Select(t => t.Id).ToArray(), + excludePersonId: null, + token); var taskForReview = new TaskForReview( Guid.NewGuid(), draft, command.MessageContext.Bot.Id, DateTimeOffset.UtcNow, botContext.GetNotificationIntervals(), - targetTeam.ChatId); - - if (draft.TargetPersonId.HasValue) - taskForReview.SetConcreteReviewer(draft.TargetPersonId.Value); - else - { - var lastReviewerId = await _taskForReviewRepository.FindLastReviewer( - taskForReview.TeamId, - taskForReview.OwnerId, - token); - var history = await _taskForReviewReader.GetHistory( - taskForReview.TeamId, - DateTimeOffset.UtcNow.GetLastDayOfWeek(DayOfWeek.Monday), - token); - - taskForReview.DetectReviewer(teammates.Select(t => t.Id).ToArray(), history, lastReviewerId); - } + targetTeam.ChatId, + nextReviewerStrategy.GetReviewer()); var reviewer = await _teamAccessor.FindPerson(taskForReview.ReviewerId, token); if (reviewer is null) diff --git a/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/NeedReview/NeedReviewCommandHandler.cs b/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/NeedReview/NeedReviewCommandHandler.cs index 377e9e7c..65bd5ee1 100644 --- a/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/NeedReview/NeedReviewCommandHandler.cs +++ b/src/Inc.TeamAssistant.Reviewer.Application/CommandHandlers/NeedReview/NeedReviewCommandHandler.cs @@ -22,19 +22,17 @@ public NeedReviewCommandHandler( public async Task Handle(NeedReviewCommand command, CancellationToken token) { ArgumentNullException.ThrowIfNull(command); - + var draft = new DraftTaskForReview( - Guid.NewGuid(), - command.TeamId, - command.MessageContext.Person.Id, - Enum.Parse(command.Strategy), - command.MessageContext.ChatMessage.ChatId, - command.MessageContext.ChatMessage.MessageId, - command.Description, - DateTimeOffset.UtcNow); - - if (command.MessageContext.TargetPersonId.HasValue) - draft.WithTargetPerson(command.MessageContext.TargetPersonId.Value); + Guid.NewGuid(), + command.TeamId, + command.MessageContext.Person.Id, + command.MessageContext.ChatMessage.ChatId, + command.MessageContext.ChatMessage.MessageId, + command.Description, + DateTimeOffset.UtcNow, + Enum.Parse(command.Strategy)) + .SetTargetPerson(command.MessageContext.TargetPersonId); await _repository.Upsert(draft, token); diff --git a/src/Inc.TeamAssistant.Reviewer.Application/Contracts/TaskForReviewHistory.cs b/src/Inc.TeamAssistant.Reviewer.Application/Contracts/TaskForReviewHistory.cs index 8a9df691..3b1255c3 100644 --- a/src/Inc.TeamAssistant.Reviewer.Application/Contracts/TaskForReviewHistory.cs +++ b/src/Inc.TeamAssistant.Reviewer.Application/Contracts/TaskForReviewHistory.cs @@ -6,6 +6,7 @@ public sealed record TaskForReviewHistory( Guid Id, DateTimeOffset Created, TaskForReviewState State, + NextReviewerType Strategy, Guid BotId, string Description, IReadOnlyCollection ReviewIntervals, @@ -15,6 +16,5 @@ public sealed record TaskForReviewHistory( long OwnerId, string OwnerName, string? OwnerUserName, - bool HasConcreteReviewer, bool IsOriginalReviewer) : ITaskForReviewStats; \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Application/QueryHandlers/GetLastTasks/Converters/TaskForReviewHistoryConverter.cs b/src/Inc.TeamAssistant.Reviewer.Application/QueryHandlers/GetLastTasks/Converters/TaskForReviewHistoryConverter.cs index aa8f640e..daf9428c 100644 --- a/src/Inc.TeamAssistant.Reviewer.Application/QueryHandlers/GetLastTasks/Converters/TaskForReviewHistoryConverter.cs +++ b/src/Inc.TeamAssistant.Reviewer.Application/QueryHandlers/GetLastTasks/Converters/TaskForReviewHistoryConverter.cs @@ -1,5 +1,6 @@ using Inc.TeamAssistant.Reviewer.Application.Contracts; using Inc.TeamAssistant.Reviewer.Application.Services; +using Inc.TeamAssistant.Reviewer.Domain; using Inc.TeamAssistant.Reviewer.Model.Queries.GetLastTasks; namespace Inc.TeamAssistant.Reviewer.Application.QueryHandlers.GetLastTasks.Converters; @@ -34,7 +35,7 @@ public async Task ConvertTo(TaskForReviewHistory item, Cancell item.OwnerId, item.OwnerName, item.OwnerUserName, - item.HasConcreteReviewer, + HasConcreteReviewer: item.Strategy == NextReviewerType.Target, item.IsOriginalReviewer); } } \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Application/ServiceCollectionExtensions.cs b/src/Inc.TeamAssistant.Reviewer.Application/ServiceCollectionExtensions.cs index 0035cf43..10140465 100644 --- a/src/Inc.TeamAssistant.Reviewer.Application/ServiceCollectionExtensions.cs +++ b/src/Inc.TeamAssistant.Reviewer.Application/ServiceCollectionExtensions.cs @@ -14,6 +14,7 @@ using Inc.TeamAssistant.Reviewer.Application.Handlers; using Inc.TeamAssistant.Reviewer.Application.QueryHandlers.GetLastTasks.Converters; using Inc.TeamAssistant.Reviewer.Application.Services; +using Inc.TeamAssistant.Reviewer.Domain.NextReviewerStrategies; using Microsoft.Extensions.DependencyInjection; namespace Inc.TeamAssistant.Reviewer.Application; @@ -25,6 +26,7 @@ public static IServiceCollection AddReviewerApplication(this IServiceCollection ArgumentNullException.ThrowIfNull(services); services + .AddSingleton() .AddSingleton() .AddScoped() .AddScoped() diff --git a/src/Inc.TeamAssistant.Reviewer.Application/Services/NextReviewerStrategyFactory.cs b/src/Inc.TeamAssistant.Reviewer.Application/Services/NextReviewerStrategyFactory.cs new file mode 100644 index 00000000..02e331d0 --- /dev/null +++ b/src/Inc.TeamAssistant.Reviewer.Application/Services/NextReviewerStrategyFactory.cs @@ -0,0 +1,55 @@ +using Inc.TeamAssistant.Holidays.Extensions; +using Inc.TeamAssistant.Primitives.Exceptions; +using Inc.TeamAssistant.Reviewer.Application.Contracts; +using Inc.TeamAssistant.Reviewer.Domain; +using Inc.TeamAssistant.Reviewer.Domain.NextReviewerStrategies; + +namespace Inc.TeamAssistant.Reviewer.Application.Services; + +internal sealed class NextReviewerStrategyFactory : INextReviewerStrategyFactory +{ + private readonly ITaskForReviewReader _taskForReviewReader; + private readonly ITaskForReviewRepository _taskForReviewRepository; + + public NextReviewerStrategyFactory( + ITaskForReviewReader taskForReviewReader, + ITaskForReviewRepository taskForReviewRepository) + { + _taskForReviewReader = taskForReviewReader ?? throw new ArgumentNullException(nameof(taskForReviewReader)); + _taskForReviewRepository = + taskForReviewRepository ?? throw new ArgumentNullException(nameof(taskForReviewRepository)); + } + + public async Task Create( + Guid teamId, + long ownerId, + NextReviewerType nextReviewerType, + long? targetPersonId, + IReadOnlyCollection teammates, + long? excludePersonId, + CancellationToken token) + { + ArgumentNullException.ThrowIfNull(teammates); + + var lastReviewerId = await _taskForReviewRepository.FindLastReviewer( + teamId, + ownerId, + token); + var history = await _taskForReviewReader.GetHistory( + teamId, + DateTimeOffset.UtcNow.GetLastDayOfWeek(DayOfWeek.Monday), + token); + + var excludedPersonIds = excludePersonId.HasValue + ? new[] { ownerId, excludePersonId.Value } + : [ownerId]; + + return nextReviewerType switch + { + NextReviewerType.RoundRobin => new RoundRobinReviewerStrategy(teammates, excludedPersonIds, lastReviewerId), + NextReviewerType.Random => new RandomReviewerStrategy(teammates, history, excludedPersonIds, lastReviewerId), + NextReviewerType.Target when targetPersonId.HasValue => new TargetReviewerStrategy(targetPersonId.Value), + _ => throw new TeamAssistantException($"NextReviewerType {nextReviewerType} was not supported.") + }; + } +} \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Application/Services/ReassignReviewService.cs b/src/Inc.TeamAssistant.Reviewer.Application/Services/ReassignReviewService.cs index 28ef21d9..f618b7f1 100644 --- a/src/Inc.TeamAssistant.Reviewer.Application/Services/ReassignReviewService.cs +++ b/src/Inc.TeamAssistant.Reviewer.Application/Services/ReassignReviewService.cs @@ -1,9 +1,10 @@ -using Inc.TeamAssistant.Holidays.Extensions; using Inc.TeamAssistant.Primitives; using Inc.TeamAssistant.Primitives.Bots; using Inc.TeamAssistant.Primitives.Exceptions; using Inc.TeamAssistant.Primitives.Notifications; using Inc.TeamAssistant.Reviewer.Application.Contracts; +using Inc.TeamAssistant.Reviewer.Domain; +using Inc.TeamAssistant.Reviewer.Domain.NextReviewerStrategies; namespace Inc.TeamAssistant.Reviewer.Application.Services; @@ -12,20 +13,21 @@ internal sealed class ReassignReviewService private readonly ITaskForReviewRepository _taskForReviewRepository; private readonly ITeamAccessor _teamAccessor; private readonly IReviewMessageBuilder _reviewMessageBuilder; - private readonly ITaskForReviewReader _taskForReviewReader; + private readonly INextReviewerStrategyFactory _nextReviewerStrategyFactory; public ReassignReviewService( ITaskForReviewRepository taskForReviewRepository, ITeamAccessor teamAccessor, IReviewMessageBuilder reviewMessageBuilder, - ITaskForReviewReader taskForReviewReader) + INextReviewerStrategyFactory nextReviewerStrategyFactory) { _taskForReviewRepository = taskForReviewRepository ?? throw new ArgumentNullException(nameof(taskForReviewRepository)); _teamAccessor = teamAccessor ?? throw new ArgumentNullException(nameof(teamAccessor)); _reviewMessageBuilder = reviewMessageBuilder ?? throw new ArgumentNullException(nameof(reviewMessageBuilder)); - _taskForReviewReader = taskForReviewReader ?? throw new ArgumentNullException(nameof(taskForReviewReader)); + _nextReviewerStrategyFactory = + nextReviewerStrategyFactory ?? throw new ArgumentNullException(nameof(nextReviewerStrategyFactory)); } public async Task> ReassignReview( @@ -38,19 +40,20 @@ public async Task> ReassignReview( if (!task.CanAccept()) return Array.Empty(); - var history = await _taskForReviewReader.GetHistory( - task.TeamId, - DateTimeOffset.UtcNow.GetLastDayOfWeek(DayOfWeek.Monday), - token); var teammates = await _teamAccessor.GetTeammates(task.TeamId, DateTimeOffset.UtcNow, token); - var lastReviewerId = await _taskForReviewRepository.FindLastReviewer(task.TeamId, task.OwnerId, token); - - task.Reassign( - DateTimeOffset.UtcNow, + var nextReviewerType = task.Strategy == NextReviewerType.Target + ? NextReviewerType.Random + : task.Strategy; + var nextReviewerStrategy = await _nextReviewerStrategyFactory.Create( + task.TeamId, + task.OwnerId, + nextReviewerType, + targetPersonId: null, teammates.Select(t => t.Id).ToArray(), - history, - task.ReviewerId, - lastReviewerId); + excludePersonId: task.ReviewerId, + token); + + task.Reassign(DateTimeOffset.UtcNow, nextReviewerStrategy.GetReviewer()); var owner = await _teamAccessor.FindPerson(task.OwnerId, token); if (owner is null) diff --git a/src/Inc.TeamAssistant.Reviewer.Application/Services/ReviewMessageBuilder.cs b/src/Inc.TeamAssistant.Reviewer.Application/Services/ReviewMessageBuilder.cs index 8310c774..82b1e653 100644 --- a/src/Inc.TeamAssistant.Reviewer.Application/Services/ReviewMessageBuilder.cs +++ b/src/Inc.TeamAssistant.Reviewer.Application/Services/ReviewMessageBuilder.cs @@ -210,10 +210,10 @@ private async Task MessageForTeam( Func attachPersons = n => n; var languageId = await _teamAccessor.GetClientLanguage(task.BotId, owner.Id, token); - var reviewerTargetMessageKey = (task.OriginalReviewerId.HasValue, task.HasConcreteReviewer) switch + var reviewerTargetMessageKey = (task.OriginalReviewerId.HasValue, task.Strategy) switch { (true, _) => Messages.Reviewer_TargetReassigned, - (_, true) => Messages.Reviewer_TargetManually, + (_, NextReviewerType.Target) => Messages.Reviewer_TargetManually, (_, _) => Messages.Reviewer_TargetAutomatically }; diff --git a/src/Inc.TeamAssistant.Reviewer.Application/Services/ReviewerSettingSectionProvider.cs b/src/Inc.TeamAssistant.Reviewer.Application/Services/ReviewerSettingSectionProvider.cs index a0582161..3c9d4433 100644 --- a/src/Inc.TeamAssistant.Reviewer.Application/Services/ReviewerSettingSectionProvider.cs +++ b/src/Inc.TeamAssistant.Reviewer.Application/Services/ReviewerSettingSectionProvider.cs @@ -5,7 +5,7 @@ namespace Inc.TeamAssistant.Reviewer.Application.Services; internal sealed class ReviewerSettingSectionProvider : ISettingSectionProvider { - private readonly IReadOnlyDictionary _storyType = new Dictionary + private readonly IReadOnlyDictionary _nextReviewerType = new Dictionary { [NextReviewerType.RoundRobin] = "FormSectionSetSettingsRoundRobinDescription", [NextReviewerType.Random] = "FormSectionSetSettingsRandomDescription" @@ -44,7 +44,7 @@ public IReadOnlyCollection GetSections() private IEnumerable GetValuesForNextReviewerType() { foreach (var item in Enum.GetValues()) - if (_storyType.TryGetValue(item, out var value)) + if (_nextReviewerType.TryGetValue(item, out var value)) yield return new SelectValue(value, item.ToString()); } diff --git a/src/Inc.TeamAssistant.Reviewer.DataAccess/TaskForReviewReader.cs b/src/Inc.TeamAssistant.Reviewer.DataAccess/TaskForReviewReader.cs index 5969fc82..dc8735fd 100644 --- a/src/Inc.TeamAssistant.Reviewer.DataAccess/TaskForReviewReader.cs +++ b/src/Inc.TeamAssistant.Reviewer.DataAccess/TaskForReviewReader.cs @@ -202,6 +202,7 @@ public async Task> GetLastTasks( t.id AS id, t.created AS created, t.state AS state, + t.strategy AS strategy, t.bot_id AS botid, t.description AS description, t.review_intervals AS reviewintervals, @@ -211,7 +212,6 @@ public async Task> GetLastTasks( o.id AS ownerid, o.name AS ownername, o.username AS ownerusername, - t.has_concrete_reviewer AS hasconcretereviewer, t.original_reviewer_id IS NULL AS isoriginalreviewer FROM review.task_for_reviews AS t JOIN connector.persons AS r ON r.id = t.reviewer_id diff --git a/src/Inc.TeamAssistant.Reviewer.DataAccess/TaskForReviewRepository.cs b/src/Inc.TeamAssistant.Reviewer.DataAccess/TaskForReviewRepository.cs index 11771144..48ea8c6b 100644 --- a/src/Inc.TeamAssistant.Reviewer.DataAccess/TaskForReviewRepository.cs +++ b/src/Inc.TeamAssistant.Reviewer.DataAccess/TaskForReviewRepository.cs @@ -40,7 +40,6 @@ public async Task> Get( t.accept_date AS acceptdate, t.message_id AS messageid, t.chat_id AS chatid, - t.has_concrete_reviewer AS hasconcretereviewer, t.original_reviewer_id AS originalreviewerid, t.review_intervals AS reviewintervals, t.accepted_with_comments AS acceptedwithcomments @@ -80,7 +79,6 @@ public async Task GetById(Guid taskForReviewId, CancellationToken t.accept_date AS acceptdate, t.message_id AS messageid, t.chat_id AS chatid, - t.has_concrete_reviewer AS hasconcretereviewer, t.original_reviewer_id AS originalreviewerid, t.review_intervals AS reviewintervals, t.accepted_with_comments AS acceptedwithcomments @@ -118,7 +116,6 @@ INSERT INTO review.task_for_reviews ( accept_date, message_id, chat_id, - has_concrete_reviewer, original_reviewer_id, review_intervals, accepted_with_comments) @@ -138,7 +135,6 @@ INSERT INTO review.task_for_reviews ( @accept_date, @message_id, @chat_id, - @has_concrete_reviewer, @original_reviewer_id, @review_intervals::jsonb, @accepted_with_comments) @@ -157,7 +153,6 @@ ON CONFLICT (id) DO UPDATE SET accept_date = excluded.accept_date, message_id = excluded.message_id, chat_id = excluded.chat_id, - has_concrete_reviewer = excluded.has_concrete_reviewer, original_reviewer_id = excluded.original_reviewer_id, review_intervals = excluded.review_intervals, accepted_with_comments = excluded.accepted_with_comments;", @@ -178,7 +173,6 @@ ON CONFLICT (id) DO UPDATE SET owner_message_id = taskForReview.OwnerMessageId, reviewer_id = taskForReview.ReviewerId, reviewer_message_id = taskForReview.ReviewerMessageId, - has_concrete_reviewer = taskForReview.HasConcreteReviewer, original_reviewer_id = taskForReview.OriginalReviewerId, review_intervals = reviewIntervals, accepted_with_comments = taskForReview.AcceptedWithComments @@ -197,14 +191,15 @@ ON CONFLICT (id) DO UPDATE SET SELECT t.reviewer_id AS reviewerid FROM review.task_for_reviews AS t - WHERE t.team_id = @team_id AND t.owner_id = @owner_id AND NOT t.has_concrete_reviewer + WHERE t.team_id = @team_id AND t.owner_id = @owner_id AND t.strategy != @next_reviewer_type__target ORDER BY t.created DESC OFFSET 0 LIMIT 1;", new { team_id = teamId, - owner_id = ownerId + owner_id = ownerId, + next_reviewer_type__target = (int)NextReviewerType.Target }, flags: CommandFlags.None, cancellationToken: token); diff --git a/src/Inc.TeamAssistant.Reviewer.Domain/DraftTaskForReview.cs b/src/Inc.TeamAssistant.Reviewer.Domain/DraftTaskForReview.cs index 9df0a571..f0fb5d7d 100644 --- a/src/Inc.TeamAssistant.Reviewer.Domain/DraftTaskForReview.cs +++ b/src/Inc.TeamAssistant.Reviewer.Domain/DraftTaskForReview.cs @@ -23,11 +23,11 @@ public DraftTaskForReview( Guid id, Guid teamId, long ownerId, - NextReviewerType strategy, long chatId, int messageId, string description, - DateTimeOffset now) + DateTimeOffset now, + NextReviewerType strategy) : this() { ArgumentException.ThrowIfNullOrWhiteSpace(description); @@ -35,11 +35,11 @@ public DraftTaskForReview( Id = id; TeamId = teamId; OwnerId = ownerId; - Strategy = strategy; ChatId = chatId; MessageId = messageId; Description = description; Created = now; + Strategy = strategy; } public DraftTaskForReview WithDescription(string description) @@ -51,20 +51,13 @@ public DraftTaskForReview WithDescription(string description) return this; } - public DraftTaskForReview WithTargetPerson(long personId) + public DraftTaskForReview SetTargetPerson(long? personId) { TargetPersonId = personId; return this; } - public DraftTaskForReview WithoutTargetPerson() - { - TargetPersonId = null; - - return this; - } - public DraftTaskForReview WithPreviewMessage(int messageId) { PreviewMessageId = messageId; @@ -79,4 +72,6 @@ public DraftTaskForReview CheckRights(long personId) return this; } + + public NextReviewerType GetStrategy() => TargetPersonId.HasValue ? NextReviewerType.Target : Strategy; } \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Domain/INextReviewerStrategy.cs b/src/Inc.TeamAssistant.Reviewer.Domain/INextReviewerStrategy.cs index ed9a7564..25de20d2 100644 --- a/src/Inc.TeamAssistant.Reviewer.Domain/INextReviewerStrategy.cs +++ b/src/Inc.TeamAssistant.Reviewer.Domain/INextReviewerStrategy.cs @@ -2,5 +2,5 @@ namespace Inc.TeamAssistant.Reviewer.Domain; public interface INextReviewerStrategy { - long Next(IReadOnlyCollection excludedPersonIds, long? lastReviewerId); + long GetReviewer(); } \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/INextReviewerStrategyFactory.cs b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/INextReviewerStrategyFactory.cs new file mode 100644 index 00000000..d4eb1810 --- /dev/null +++ b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/INextReviewerStrategyFactory.cs @@ -0,0 +1,13 @@ +namespace Inc.TeamAssistant.Reviewer.Domain.NextReviewerStrategies; + +public interface INextReviewerStrategyFactory +{ + Task Create( + Guid teamId, + long ownerId, + NextReviewerType nextReviewerType, + long? targetPersonId, + IReadOnlyCollection teammates, + long? excludePersonId, + CancellationToken token); +} \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/RandomReviewerStrategy.cs b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/RandomReviewerStrategy.cs index 6a583b08..4e565479 100644 --- a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/RandomReviewerStrategy.cs +++ b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/RandomReviewerStrategy.cs @@ -1,32 +1,49 @@ namespace Inc.TeamAssistant.Reviewer.Domain.NextReviewerStrategies; -internal sealed class RandomReviewerStrategy : INextReviewerStrategy +public sealed class RandomReviewerStrategy : INextReviewerStrategy { private const int ReviewerWeight = 100; private readonly IReadOnlyCollection _teammates; private readonly IReadOnlyDictionary _history; + private readonly IReadOnlyCollection _excludedPersonIds; + private readonly long? _lastReviewerId; - public RandomReviewerStrategy(IReadOnlyCollection teammates, IReadOnlyDictionary history) + public RandomReviewerStrategy( + IReadOnlyCollection teammates, + IReadOnlyDictionary history, + IReadOnlyCollection excludedPersonIds, + long? lastReviewerId) { _teammates = teammates ?? throw new ArgumentNullException(nameof(teammates)); _history = history ?? throw new ArgumentNullException(nameof(history)); + _excludedPersonIds = excludedPersonIds ?? throw new ArgumentNullException(nameof(excludedPersonIds)); + _lastReviewerId = lastReviewerId; } - - public long Next(IReadOnlyCollection excludedPersonIds, long? lastReviewerId) + + public long GetReviewer() + { + var targetPlayers = GetTargetPlayers(); + return targetPlayers.Any() + ? FromTargetPlayers(targetPlayers) + : _teammates.First(); + } + + private IReadOnlyCollection GetTargetPlayers() { - ArgumentNullException.ThrowIfNull(excludedPersonIds); + var excludedTeammates = _lastReviewerId.HasValue + ? _excludedPersonIds.Append(_lastReviewerId.Value) + : _excludedPersonIds; - var excludedTeammates = lastReviewerId.HasValue - ? excludedPersonIds.Append(lastReviewerId.Value) - : excludedPersonIds; - var targetPlayers = _teammates + return _teammates .Where(t => !excludedTeammates.Contains(t)) .OrderBy(t => t) .ToArray(); + } - if (!targetPlayers.Any()) - return _teammates.First(); + private long FromTargetPlayers(IReadOnlyCollection targetPlayers) + { + ArgumentNullException.ThrowIfNull(targetPlayers); var seeding = targetPlayers .Select(t => (PersonId: t, Count: 1d / _history.GetValueOrDefault(t, 1) * ReviewerWeight)) diff --git a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/RoundRobinReviewerStrategy.cs b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/RoundRobinReviewerStrategy.cs index 588a195d..50848014 100644 --- a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/RoundRobinReviewerStrategy.cs +++ b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/RoundRobinReviewerStrategy.cs @@ -1,28 +1,46 @@ namespace Inc.TeamAssistant.Reviewer.Domain.NextReviewerStrategies; -internal sealed class RoundRobinReviewerStrategy : INextReviewerStrategy +public sealed class RoundRobinReviewerStrategy : INextReviewerStrategy { private readonly IReadOnlyCollection _teammates; + private readonly IReadOnlyCollection _excludedPersonIds; + private readonly long? _lastReviewerId; - public RoundRobinReviewerStrategy(IReadOnlyCollection teammates) + public RoundRobinReviewerStrategy( + IReadOnlyCollection teammates, + IReadOnlyCollection excludedPersonIds, + long? lastReviewerId) { _teammates = teammates ?? throw new ArgumentNullException(nameof(teammates)); + _excludedPersonIds = excludedPersonIds ?? throw new ArgumentNullException(nameof(excludedPersonIds)); + _lastReviewerId = lastReviewerId; } - - public long Next(IReadOnlyCollection excludedPersonIds, long? lastReviewerId) + + public long GetReviewer() { - ArgumentNullException.ThrowIfNull(excludedPersonIds); - - var otherTeammates = _teammates - .Where(t => !excludedPersonIds.Contains(t)) + var otherTeammates = GetOtherTeammates(); + return otherTeammates.Any() + ? FromOtherTeammates(otherTeammates) + : _teammates.First(); + } + + private IReadOnlyCollection GetOtherTeammates() + { + return _teammates + .Where(t => !_excludedPersonIds.Contains(t)) .OrderBy(t => t) .ToArray(); + } - if (!otherTeammates.Any()) - return _teammates.First(); - - var nextReviewers = otherTeammates.Where(ot => lastReviewerId is null || ot > lastReviewerId).ToArray(); + private long FromOtherTeammates(IReadOnlyCollection otherTeammates) + { + ArgumentNullException.ThrowIfNull(otherTeammates); + + var nextReviewers = otherTeammates + .Where(ot => _lastReviewerId is null || ot > _lastReviewerId) + .ToArray(); var targets = nextReviewers.Any() ? nextReviewers : otherTeammates; + return targets.MinBy(t => t); } } \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/TargetReviewerStrategy.cs b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/TargetReviewerStrategy.cs new file mode 100644 index 00000000..4ca52046 --- /dev/null +++ b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategies/TargetReviewerStrategy.cs @@ -0,0 +1,13 @@ +namespace Inc.TeamAssistant.Reviewer.Domain.NextReviewerStrategies; + +public sealed class TargetReviewerStrategy : INextReviewerStrategy +{ + private readonly long _targetPersonId; + + public TargetReviewerStrategy(long targetPersonId) + { + _targetPersonId = targetPersonId; + } + + public long GetReviewer() => _targetPersonId; +} \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategyFactory.cs b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategyFactory.cs deleted file mode 100644 index 1f1f5e99..00000000 --- a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerStrategyFactory.cs +++ /dev/null @@ -1,23 +0,0 @@ -using Inc.TeamAssistant.Primitives.Exceptions; -using Inc.TeamAssistant.Reviewer.Domain.NextReviewerStrategies; - -namespace Inc.TeamAssistant.Reviewer.Domain; - -internal static class NextReviewerStrategyFactory -{ - public static INextReviewerStrategy Create( - NextReviewerType nextReviewerType, - IReadOnlyCollection teammates, - IReadOnlyDictionary history) - { - ArgumentNullException.ThrowIfNull(teammates); - ArgumentNullException.ThrowIfNull(history); - - return nextReviewerType switch - { - NextReviewerType.RoundRobin => new RoundRobinReviewerStrategy(teammates), - NextReviewerType.Random => new RandomReviewerStrategy(teammates, history), - _ => throw new TeamAssistantException($"Strategy {nextReviewerType} was not supported.") - }; - } -} \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerType.cs b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerType.cs index 16d15d46..6afd3e26 100644 --- a/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerType.cs +++ b/src/Inc.TeamAssistant.Reviewer.Domain/NextReviewerType.cs @@ -3,5 +3,6 @@ namespace Inc.TeamAssistant.Reviewer.Domain; public enum NextReviewerType { RoundRobin = 1, - Random = 2 + Random = 2, + Target = 3 } \ No newline at end of file diff --git a/src/Inc.TeamAssistant.Reviewer.Domain/TaskForReview.cs b/src/Inc.TeamAssistant.Reviewer.Domain/TaskForReview.cs index 909c80ef..ad478fcf 100644 --- a/src/Inc.TeamAssistant.Reviewer.Domain/TaskForReview.cs +++ b/src/Inc.TeamAssistant.Reviewer.Domain/TaskForReview.cs @@ -17,7 +17,6 @@ public sealed class TaskForReview : ITaskForReviewStats public DateTimeOffset NextNotification { get; private set; } public DateTimeOffset? AcceptDate { get; private set; } public int? MessageId { get; private set; } - public bool HasConcreteReviewer { get; private set; } public long? OriginalReviewerId { get; private set; } public IReadOnlyCollection ReviewIntervals { get; private set; } public bool AcceptedWithComments { get; private set; } @@ -33,7 +32,8 @@ public TaskForReview( Guid botId, DateTimeOffset now, NotificationIntervals notificationIntervals, - long chatId) + long chatId, + long reviewerId) : this() { ArgumentNullException.ThrowIfNull(draft); @@ -41,7 +41,7 @@ public TaskForReview( Id = id; BotId = botId; - Strategy = draft.Strategy; + Strategy = draft.GetStrategy(); Created = now; TeamId = draft.TeamId; OwnerId = draft.OwnerId; @@ -50,6 +50,7 @@ public TaskForReview( State = TaskForReviewState.New; SetNextNotificationTime(now, notificationIntervals); + SetReviewer(reviewerId); } public void AttachMessage(MessageType messageType, int messageId) @@ -136,49 +137,14 @@ private void AddReviewInterval(long userId, DateTimeOffset end) ReviewIntervals = ReviewIntervals.Append(new ReviewInterval(State, end, userId)).ToArray(); } - public TaskForReview SetConcreteReviewer(long reviewerId) - { - SetReviewer(reviewerId); - HasConcreteReviewer = true; - - return this; - } - - public TaskForReview Reassign( - DateTimeOffset now, - IReadOnlyCollection teammates, - IReadOnlyDictionary history, - long excludedPersonId, - long? lastReviewerId) + public void Reassign(DateTimeOffset now, long reviewerId) { - ArgumentNullException.ThrowIfNull(teammates); - ArgumentNullException.ThrowIfNull(history); - AddReviewInterval(ReviewerId, now); NextNotification = now; ReviewerMessageId = null; - - return DetectReviewer(teammates, history, lastReviewerId, excludedPersonId); - } - - public TaskForReview DetectReviewer( - IReadOnlyCollection teammates, - IReadOnlyDictionary history, - long? lastReviewerId, - long? excludedPersonId = null) - { - ArgumentNullException.ThrowIfNull(teammates); - ArgumentNullException.ThrowIfNull(history); - - var excludedPersonIds = excludedPersonId.HasValue - ? new[] { OwnerId, excludedPersonId.Value } - : [OwnerId]; - var reviewerStrategy = NextReviewerStrategyFactory.Create(Strategy, teammates, history); - SetReviewer(reviewerStrategy.Next(excludedPersonIds, lastReviewerId)); - - return this; + SetReviewer(reviewerId); } public int? GetAttempts() diff --git a/tests/Inc.TeamAssistant.Reviewer.ApplicationTests/Services/ReviewMetricsProviderTests.cs b/tests/Inc.TeamAssistant.Reviewer.ApplicationTests/Services/ReviewMetricsProviderTests.cs index 468c3f8e..53b3cf0c 100644 --- a/tests/Inc.TeamAssistant.Reviewer.ApplicationTests/Services/ReviewMetricsProviderTests.cs +++ b/tests/Inc.TeamAssistant.Reviewer.ApplicationTests/Services/ReviewMetricsProviderTests.cs @@ -152,17 +152,18 @@ private TaskForReview CreateAndSetupTaskForReview( _fixture.Create(), teamId, _fixture.Create(), - _fixture.Create(), _fixture.Create(), _fixture.Create(), _fixture.Create(), - start); + start, + _fixture.Create()); var taskForReview = new TaskForReview( _fixture.Create(), draft, _fixture.Create(), start, _fixture.Create(), + _fixture.Create(), _fixture.Create()); var operationStart = start; diff --git a/tests/Inc.TeamAssistant.Reviewer.DomainTests/RandomReviewerStrategyTests.cs b/tests/Inc.TeamAssistant.Reviewer.DomainTests/RandomReviewerStrategyTests.cs index 1ea019a0..67e6ae55 100644 --- a/tests/Inc.TeamAssistant.Reviewer.DomainTests/RandomReviewerStrategyTests.cs +++ b/tests/Inc.TeamAssistant.Reviewer.DomainTests/RandomReviewerStrategyTests.cs @@ -7,7 +7,6 @@ namespace Inc.TeamAssistant.Reviewer.DomainTests; public sealed class RandomReviewerStrategyTests { private readonly IReadOnlyCollection _teammates; - private readonly RandomReviewerStrategy _target; private readonly Fixture _fixture = new(); public RandomReviewerStrategyTests() @@ -20,13 +19,16 @@ public RandomReviewerStrategyTests() _fixture.Create(), _fixture.Create() }; - _target = new RandomReviewerStrategy(_teammates, new Dictionary()); } [Fact] public void Constructor_TeamIsNull_ThrowsException() { - RandomReviewerStrategy Action() => new(teammates: null!, new Dictionary()); + RandomReviewerStrategy Action() => new( + teammates: null!, + new Dictionary(), + excludedPersonIds: Array.Empty(), + lastReviewerId: null); Assert.Throws(Action); } @@ -34,7 +36,23 @@ public void Constructor_TeamIsNull_ThrowsException() [Fact] public void Constructor_HistoryIsNull_ThrowsException() { - RandomReviewerStrategy Action() => new(_teammates, null!); + RandomReviewerStrategy Action() => new( + _teammates, + null!, + excludedPersonIds: Array.Empty(), + lastReviewerId: null); + + Assert.Throws(Action); + } + + [Fact] + public void Constructor_ExcludedPersonIdsIsNull_ThrowsException() + { + RandomReviewerStrategy Action() => new( + _teammates, + new Dictionary(), + excludedPersonIds: null!, + lastReviewerId: null); Assert.Throws(Action); } @@ -43,8 +61,13 @@ public void Constructor_HistoryIsNull_ThrowsException() public void Next_Team_ShouldNotOwner() { var ownerId = _teammates.First(); + var target = new RandomReviewerStrategy( + _teammates, + new Dictionary(), + [ownerId], + lastReviewerId: null); - var reviewerId = _target.Next([ownerId], lastReviewerId: null); + var reviewerId = target.GetReviewer(); Assert.NotEqual(ownerId, reviewerId); } @@ -54,8 +77,13 @@ public void Next_Team_ShouldNotLastReviewerId() { var ownerId = _teammates.First(); var lastReviewerId = _teammates.Skip(1).First(); + var target = new RandomReviewerStrategy( + _teammates, + new Dictionary(), + [ownerId], + lastReviewerId); - var reviewerId = _target.Next([ownerId], lastReviewerId); + var reviewerId = target.GetReviewer(); Assert.NotEqual(lastReviewerId, reviewerId); } @@ -65,9 +93,13 @@ public void Next_Team_ShouldNotLastReviewerId() public void Next_MultipleIterations_MustRandomReviewer(int iterationCount, int reviewerCountByPlayer) { var owner = _teammates.First(); - + var target = new RandomReviewerStrategy( + _teammates, + new Dictionary(), + [owner], + lastReviewerId: null); var reviewerIds = Enumerable.Range(0, iterationCount) - .Select(_ => _target.Next([owner], lastReviewerId: null)) + .Select(_ => target.GetReviewer()) .GroupBy(i => i) .ToDictionary(i => i, i => i.Count()); diff --git a/tests/Inc.TeamAssistant.Reviewer.DomainTests/RoundRobinReviewerStrategyTests.cs b/tests/Inc.TeamAssistant.Reviewer.DomainTests/RoundRobinReviewerStrategyTests.cs index b2c7300a..1be4481b 100644 --- a/tests/Inc.TeamAssistant.Reviewer.DomainTests/RoundRobinReviewerStrategyTests.cs +++ b/tests/Inc.TeamAssistant.Reviewer.DomainTests/RoundRobinReviewerStrategyTests.cs @@ -7,7 +7,6 @@ namespace Inc.TeamAssistant.Reviewer.DomainTests; public sealed class RoundRobinReviewerStrategyTests { private readonly IReadOnlyCollection _teammates; - private readonly RoundRobinReviewerStrategy _target; private readonly Fixture _fixture = new(); public RoundRobinReviewerStrategyTests() @@ -20,13 +19,15 @@ public RoundRobinReviewerStrategyTests() _fixture.Create(), _fixture.Create() }; - _target = new RoundRobinReviewerStrategy(_teammates); } [Fact] public void Constructor_TeamIsNull_ThrowsException() { - RandomReviewerStrategy Action() => new(teammates: null!, new Dictionary()); + RoundRobinReviewerStrategy Action() => new( + teammates: null!, + excludedPersonIds: Array.Empty(), + lastReviewerId: null); Assert.Throws(Action); } @@ -34,7 +35,10 @@ public void Constructor_TeamIsNull_ThrowsException() [Fact] public void Constructor_HistoryIsNull_ThrowsException() { - RandomReviewerStrategy Action() => new(_teammates, null!); + RoundRobinReviewerStrategy Action() => new( + _teammates, + null!, + lastReviewerId: null); Assert.Throws(Action); } @@ -43,8 +47,9 @@ public void Constructor_HistoryIsNull_ThrowsException() public void Next_Team_ShouldNotOwner() { var ownerId = _teammates.First(); - - var reviewerId = _target.Next([ownerId], lastReviewerId: null); + var target = new RoundRobinReviewerStrategy(_teammates, [ownerId], lastReviewerId: null); + + var reviewerId = target.GetReviewer(); Assert.NotEqual(ownerId, reviewerId); } @@ -54,8 +59,9 @@ public void Next_Team_ShouldNotLastReviewerId() { var ownerId = _teammates.First(); var lastReviewerId = _teammates.Skip(1).First(); - - var reviewerId = _target.Next([ownerId], lastReviewerId); + var target = new RoundRobinReviewerStrategy(_teammates, [ownerId], lastReviewerId); + + var reviewerId = target.GetReviewer(); Assert.NotEqual(lastReviewerId, reviewerId); } @@ -72,7 +78,8 @@ public void Next_MultipleIterations_ShouldBeRoundRobin() long? lastReviewerId = null; foreach (var otherPlayerId in otherPlayerIds.Concat(otherPlayerIds)) { - var reviewerId = _target.Next([ownerId], lastReviewerId); + var target = new RoundRobinReviewerStrategy(_teammates, [ownerId], lastReviewerId); + var reviewerId = target.GetReviewer(); lastReviewerId = reviewerId; Assert.Equal(otherPlayerId, reviewerId); }