Update workshop export #1619
Update workshop export #1619
Conversation
|
| if (dto.ParentWorkshopId.HasValue && !await Exists((Guid)dto.ParentWorkshopId).ConfigureAwait(false)) | ||
| { | ||
| var errorMessage = $"The main workshop (with id = {dto.MemberOfWorkshopId}) for the workshop being created was not found."; | ||
| var errorMessage = $"The main workshop (with id = {dto.ParentWorkshopId}) for the workshop being created was not found."; | ||
| throw new InvalidOperationException(errorMessage); | ||
| } | ||
|
|
||
| if (dto.MemberOfWorkshopId.HasValue && (await workshopRepository.GetById((Guid)dto.MemberOfWorkshopId).ConfigureAwait(false)).MemberOfWorkshopId.HasValue) | ||
| if (dto.ParentWorkshopId.HasValue && (await workshopRepository.GetById((Guid)dto.ParentWorkshopId).ConfigureAwait(false)).ParentWorkshopId.HasValue) |
There was a problem hiding this comment.
Call to Exists() followed by GetById(). Could there be better iplementation from performance perspective?
| { | ||
| try | ||
| { | ||
| logger.LogInformation("Getting all updated providers started"); |
There was a problem hiding this comment.
LogTrace/Debug maybe?
There was a problem hiding this comment.
Same for other methods
| if (providers == null) | ||
| { | ||
| logger.LogError("Failed to retrieve updated providers. The provider list is null"); | ||
| return new SearchResult<ProviderInfoBaseDto>(); | ||
| } |
There was a problem hiding this comment.
Curious if it is ever a case? Do your repositories return null instead of empty list sometimes?
| [JsonConverter(typeof(JsonStringEnumConverter))] | ||
| public enum WorkshopType | ||
| { | ||
| None |
There was a problem hiding this comment.
Some TODO comment should be present here, otherwise this enum looks pretty simple
|
|
||
| public uint SpecialNeedsId { get; set; } = 0; | ||
| public SpecialNeedsType SpecialNeedsType { get; set; } = SpecialNeedsType.None; |
There was a problem hiding this comment.
Curios, why this and other enum property is not covered with EnumType attribute, as it was in Dto classes
| query = updatedAfter == default | ||
| ? query.Where(workshop => !workshop.IsDeleted) | ||
| : query.Where(workshop => workshop.UpdatedAt > updatedAfter || workshop.DeleteDate > updatedAfter); |
There was a problem hiding this comment.
Logic doesn't correspond name. Could either be changed?
| return query.Skip(from).Take(size).ToListAsync(); | ||
| } | ||
|
|
||
| public Task<int> CountWithDeleted(DateTime updatedAfter) |
There was a problem hiding this comment.
Same name vs logic issue. WithDeleted in name but !workshop.IsDeleted in logic
Workshopentity to include stub of future dictionaries as enums.