Добавить таблицу для учета комментариев и действий пользователя для вывода статистики по ChangeLog #30

Open
on.nemtina wants to merge 24 commits from feature/#956-change-log-table-comment into master
Owner
https://project.ddrilling.ru/projects/persistance-service/work_packages/956/activity
on.nemtina added 10 commits 2025-02-18 10:56:47 +05:00
on.nemtina added 1 commit 2025-02-18 11:01:39 +05:00
on.nemtina added 1 commit 2025-02-18 11:07:54 +05:00
on.nemtina added 1 commit 2025-02-18 11:17:17 +05:00
on.nemtina requested review from ng.frolov 2025-02-18 11:17:32 +05:00
ng.frolov was assigned by on.nemtina 2025-02-18 11:17:37 +05:00
on.nemtina self-assigned this 2025-02-18 11:17:38 +05:00
on.nemtina changed title from Добавить таблицу для учета комментариев и действий пользователя для вывода статистики по ChangeLog to WIP: Добавить таблицу для учета комментариев и действий пользователя для вывода статистики по ChangeLog 2025-02-18 11:17:43 +05:00
ng.frolov requested changes 2025-02-18 12:15:02 +05:00
@ -19,0 +22,4 @@
/// </summary>
/// <param name="service"></param>
/// <param name="repository"></param>
public ChangeLogController(ChangeLogService service, IChangeLogRepository repository)
Collaborator

Должен остаться только один

Должен остаться только один
@ -25,10 +33,12 @@ public class ChangeLogController : ControllerBase, IChangeLogApi
public async Task<IActionResult> Add(
Collaborator

Скорее всего этот метод лишний.

Скорее всего этот метод лишний.
@ -42,3 +53,3 @@
{
var userId = User.GetUserId<Guid>();
var result = await repository.AddRange(userId, idDiscriminator, dtos, token);
var changeLogCommit = new ChangeLogCommitDto(userId, comment, dtos);
Collaborator

Скорее всего в ChangeLogCommitDto dtos - лишнее, и помешаются в методе delete например.
Отношение строчек с данными к коммиту делается уже внутри сервиса.

Скорее всего в ChangeLogCommitDto dtos - лишнее, и помешаются в методе delete например. Отношение строчек с данными к коммиту делается уже внутри сервиса.
@ -38,1 +35,4 @@
public required IDictionary<string, object> Value { get; set; }
[Required, Comment("Id коммита")]
public Guid IdCommit { get; set; }
Collaborator

Для устаревших записей у нас есть 2 коммита: один при создании записи и еще один при устаревании.
Кстати туда же можно унести инфо о пользователях и датах.

Для устаревших записей у нас есть 2 коммита: один при создании записи и еще один при устаревании. Кстати туда же можно унести инфо о пользователях и датах.
on.nemtina added 3 commits 2025-02-18 17:21:39 +05:00
on.nemtina added 1 commit 2025-02-19 17:44:58 +05:00
on.nemtina added 1 commit 2025-02-20 12:04:17 +05:00
on.nemtina changed title from WIP: Добавить таблицу для учета комментариев и действий пользователя для вывода статистики по ChangeLog to Добавить таблицу для учета комментариев и действий пользователя для вывода статистики по ChangeLog 2025-02-20 12:44:15 +05:00
on.nemtina added 1 commit 2025-02-20 14:40:33 +05:00
ng.frolov requested changes 2025-02-20 16:45:34 +05:00
@ -0,0 +34,4 @@
}
/// <summary>
/// Создание или чтение данных коммита
Collaborator

Тут мы не читаем данные коммита. Только получаем его Id.
Не правильное название и описание.

Тут мы не читаем данные коммита. Только получаем его Id. Не правильное название и описание.
@ -18,4 +18,0 @@
/// <summary>
/// Редактор
/// </summary>
public Guid? IdEditor { get; set; }
Collaborator

Свойства не должны быть реализованы в интерфейсе.
А почему автор коммита устаревания удален, а автор коммита создания нет?

Свойства не должны быть реализованы в интерфейсе. А почему автор коммита устаревания удален, а автор коммита создания нет?
@ -72,3 +77,3 @@
{
entity.Obsolete = updateTime;
entity.IdEditor = idEditor;
entity.DiscriminatorId = idCommit;
Collaborator

Это не правда

Это не правда
@ -79,2 +84,3 @@
public async Task<int> ClearAndAddRange(Guid idAuthor, Guid idDiscriminator, IEnumerable<ChangeLogValuesDto> dtos, CancellationToken token)
public async Task<int> ClearAndAddRange(Guid idDiscriminator, ChangeLogCommitDto commitDto, IEnumerable<ChangeLogValuesDto> dtos, CancellationToken token)
Collaborator

Этот метод должен помечать все записи относящиеся к дискриминатору как удаленные и добавлять новые.

Этот метод должен помечать все записи относящиеся к дискриминатору как удаленные и добавлять новые.
@ -101,7 +109,6 @@ public class ChangeLogRepository : IChangeLogRepository
.Where(s => updatedIds.Contains(s.Id))
.ToDictionary(s => s.Id);
Collaborator

Linq очень не оптимально материализует в словари.
Тут лучше материализовать сперва в массив, а затем массив в словарь.

  • Используй асинхронные методы материализации в асинхронных методах репозиториев
Linq очень не оптимально материализует в словари. Тут лучше материализовать сперва в массив, а затем массив в словарь. + Используй асинхронные методы материализации в асинхронных методах репозиториев
@ -0,0 +8,4 @@
/// <summary>
/// Id
/// </summary>
public Guid Id { get; set; }
Collaborator

По правильному стоит разделить эту Dto на 2. Сделать отдельную dto для создания нового коммита, так как это поле генерируется внутри репозитория.

По правильному стоит разделить эту Dto на 2. Сделать отдельную dto для создания нового коммита, так как это поле генерируется внутри репозитория.
@ -0,0 +22,4 @@
[Fact]
public async Task AddRange()
{
var discriminatorId = Uuid7.Guid();
Collaborator

Расставь пож. во всех тестах комментарии "//act" и "//assert" чтобы было понятнее где подготовительные операции, где проверяемое действие и где начинаются проверки

Расставь пож. во всех тестах комментарии "//act" и "//assert" чтобы было понятнее где подготовительные операции, где проверяемое действие и где начинаются проверки
on.nemtina added 1 commit 2025-02-20 18:31:07 +05:00
on.nemtina added 1 commit 2025-02-21 12:25:32 +05:00
on.nemtina added 1 commit 2025-02-21 12:27:17 +05:00
on.nemtina added 1 commit 2025-02-21 12:28:40 +05:00
ng.frolov requested changes 2025-02-21 12:36:40 +05:00
@ -14,3 +14,3 @@
public class ChangeLogController : ControllerBase, IChangeLogApi
{
private readonly IChangeLogRepository repository;
private ChangeLogService service { get; }
Collaborator

Не ошибка, но почему не private readonly поле как везде? +Свойства == синтаксический сахар для методов Get_..() Set_..(), А значит они должны называться с большой буквы.

Не ошибка, но почему не private readonly поле как везде? +Свойства == синтаксический сахар для методов Get_..() Set_..(), А значит они должны называться с большой буквы.
@ -21,3 +24,4 @@
this.service = service;
}
[HttpPost("{idDiscriminator}")]
Collaborator

Было бы здорова дать методам контроллеров текстовое описание для сваггера.

Было бы здорова дать методам контроллеров текстовое описание для сваггера.
on.nemtina added 1 commit 2025-02-21 14:22:46 +05:00
ng.frolov requested changes 2025-02-21 14:58:32 +05:00
@ -0,0 +39,4 @@
/// <param name="commitDto"></param>
/// <param name="token"></param>
/// <returns></returns>
private async Task<Guid> GetOrCreateCommitAsync(CreateChangeLogCommitRequest commitDto, CancellationToken token)
Collaborator

Давай в кеше хранить комит целиком не только ID

Давай в кеше хранить комит целиком не только ID
@ -26,4 +22,2 @@
public Guid? IdEditor { get; set; }
[Comment("Дата создания записи")]
public DateTimeOffset Creation { get; set; }
Collaborator

Комментарий про денормализацию БД

Комментарий про денормализацию БД
@ -0,0 +20,4 @@
this.db = db;
}
public async Task<Guid> Add(CreateChangeLogCommitRequest commitRequestDto, CancellationToken token)
Collaborator

commitRequestDto -> commitRequest или request

commitRequestDto -> commitRequest или request
@ -34,3 +43,3 @@
}
public async Task<int> MarkAsDeleted(Guid idEditor, IEnumerable<Guid> ids, CancellationToken token)
public async Task<int> MarkAsDeleted(Guid idCommit, IEnumerable<Guid> ids, DateTimeOffset updateTime, CancellationToken token)
Collaborator

Как будто и idCommit и updateTime есть в ChangeLogCommitDto

Как будто и idCommit и updateTime есть в ChangeLogCommitDto
@ -82,2 +87,4 @@
var result = 0;
var changeLogIds = dtos.Select(c => c.Id);
var comment = commitDto.Comment;
Collaborator

эта логкальная переменная дальше нигде не используется. Зачем она здесь?

эта логкальная переменная дальше нигде не используется. Зачем она здесь?
@ -85,3 +93,2 @@
result += await MarkAsDeleted(idAuthor, idDiscriminator, token);
result += await AddRange(idAuthor, idDiscriminator, dtos, token);
result += await MarkAsDeleted(commitDto.Id, changeLogIds, commitDto.Creation, token);
Collaborator

Это не то.
Покрой этот метод интеграционным тестом плиз.

// arrange
add some data winth 2 discriminators
// act replace data for 1 discriminaqtor
// assert
check thar othe data with othe discr

Это не то. Покрой этот метод интеграционным тестом плиз. // arrange add some data winth 2 discriminators // act replace data for 1 discriminaqtor // assert check thar othe data with othe discr
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feature/#956-change-log-table-comment:feature/#956-change-log-table-comment
git checkout feature/#956-change-log-table-comment
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: on.nemtina/persistence#30
No description provided.