Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix comments & improve performance #1620

Merged
merged 2 commits into from
Dec 26, 2024
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
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
using System.ComponentModel.DataAnnotations;

namespace OutOfSchool.BusinessLogic.Models.Exported;
namespace OutOfSchool.BusinessLogic.Models.Exported;

public class AddressInfoDto
{
public long Id { get; set; }

public string Street { get; set; } = string.Empty;

public string BuildingNumber { get; set; } = string.Empty;

public CodeficatorAddressInfoDto CodeficatorAddressDto { get; set; }
public CodeficatorAddressInfoDto CodeficatorAddress { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

public class CodeficatorAddressInfoDto
{
public int Id { get; set; }

public string Region { get; set; }

public string District { get; set; }
Expand All @@ -13,4 +11,6 @@ public class CodeficatorAddressInfoDto
public string Settlement { get; set; }

public string CityDistrict { get; set; }

public string Code { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public class ProviderInfoDto : ProviderInfoBaseDto, IExternalRatingInfo
[MaxLength(Constants.MaxProviderFounderLength)]
public string Founder { get; set; } = string.Empty;

public ProviderTypeDto Type { get; set; }
public string Type { get; set; }

[Required]
[EnumDataType(typeof(ProviderStatus), ErrorMessage = Constants.EnumErrorMessage)]
Expand All @@ -91,7 +91,7 @@ public class ProviderInfoDto : ProviderInfoBaseDto, IExternalRatingInfo

public AddressInfoDto ActualAddress { get; set; }

public InstitutionDto Institution { get; set; }
public string Institution { get; set; }

[Required]
[EnumDataType(typeof(InstitutionType), ErrorMessage = Constants.EnumErrorMessage)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class WorkshopInfoDto : WorkshopInfoBaseDto, IExternalRatingInfo

public Guid? ParentWorkshopId { get; set; }

public uint TakenSeats { get; set; } = 0;
public int TakenSeats { get; set; } = 0;

public float Rating { get; set; }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using AutoMapper;
using System.Linq.Expressions;
using AutoMapper;
using OutOfSchool.BusinessLogic.Models;
using OutOfSchool.BusinessLogic.Models.Exported;
using OutOfSchool.BusinessLogic.Services.AverageRatings;
Expand All @@ -8,21 +9,31 @@ namespace OutOfSchool.BusinessLogic.Services;

public class ExternalExportService : IExternalExportService
{
private const string ProviderIncludes =
"ProviderSectionItems,Images,Institution,ActualAddress,ActualAddress.CATOTTG.Parent.Parent.Parent.Parent,LegalAddress,LegalAddress.CATOTTG.Parent.Parent.Parent.Parent,Type";

private const string WorkshopIncludes =
"WorkshopDescriptionItems,Tags,Address,Address.CATOTTG.Parent.Parent.Parent.Parent,Images,DateTimeRanges,Teachers,InstitutionHierarchy,InstitutionHierarchy.Institution,InstitutionHierarchy.Directions,DefaultTeacher";

private readonly IProviderRepository providerRepository;
private readonly IWorkshopRepository workshopRepository;
private readonly IApplicationRepository applicationRepository;
private readonly IAverageRatingService averageRatingService;
private readonly IMapper mapper;
private readonly ILogger<ExternalExportService> logger;

public ExternalExportService(
IProviderRepository providerRepository,
IWorkshopRepository workshopRepository,
IApplicationRepository applicationRepository,
IAverageRatingService averageRatingService,
IMapper mapper,
ILogger<ExternalExportService> logger)
{
this.providerRepository = providerRepository ?? throw new ArgumentNullException(nameof(providerRepository));
this.workshopRepository = workshopRepository ?? throw new ArgumentNullException(nameof(workshopRepository));
this.applicationRepository =
applicationRepository ?? throw new ArgumentNullException(nameof(applicationRepository));
this.averageRatingService =
averageRatingService ?? throw new ArgumentNullException(nameof(averageRatingService));
this.mapper = mapper ?? throw new ArgumentNullException(nameof(mapper));
Expand All @@ -34,25 +45,29 @@ public async Task<SearchResult<ProviderInfoBaseDto>> GetProviders(DateTime updat
{
try
{
logger.LogInformation("Getting all updated providers started");
logger.LogDebug("Getting all updated providers started");
offsetFilter ??= new OffsetFilter();
var providers = await providerRepository
.GetAllWithDeleted(updatedAfter, offsetFilter.From, offsetFilter.Size)
.ConfigureAwait(false);

if (providers == null)
{
logger.LogError("Failed to retrieve updated providers. The provider list is null");
return new SearchResult<ProviderInfoBaseDto>();
}
Expression<Func<Provider, bool>> filterExpression = updatedAfter == default
? provider => !provider.IsDeleted
: provider => provider.UpdatedAt > updatedAfter ||
provider.Workshops.Any(w => w.UpdatedAt > updatedAfter);

var providers = await providerRepository.Get(
offsetFilter.From,
offsetFilter.Size,
ProviderIncludes,
filterExpression)
.ToListAsync()
.ConfigureAwait(false);

var providersDto = providers
.Select(MapToInfoProviderDto)
.ToList();

await FillRatingsForType(providersDto).ConfigureAwait(false);

var count = await providerRepository.CountWithDeleted(updatedAfter);
var count = await providerRepository.Count(filterExpression);

var searchResult = new SearchResult<ProviderInfoBaseDto>
{
Expand All @@ -65,32 +80,35 @@ public async Task<SearchResult<ProviderInfoBaseDto>> GetProviders(DateTime updat
catch (Exception ex)
{
logger.LogError(ex, "An unexpected error occurred while processing providers");
return new SearchResult<ProviderInfoBaseDto>();
throw;
}
}

public async Task<SearchResult<WorkshopInfoBaseDto>> GetWorkshops(DateTime updatedAfter, OffsetFilter offsetFilter)
{
try
{
logger.LogInformation("Getting all updated providers started");
logger.LogDebug("Getting all updated providers started");
offsetFilter ??= new OffsetFilter();

var workshops = await workshopRepository
.GetAllWithDeleted(updatedAfter, offsetFilter.From, offsetFilter.Size)
.ConfigureAwait(false);
Expression<Func<Workshop, bool>> filterExpression = updatedAfter == default
? workshop => !workshop.IsDeleted
: workshop => workshop.UpdatedAt > updatedAfter || workshop.DeleteDate > updatedAfter;

if (workshops == null)
{
logger.LogError("Failed to retrieve updated workshops. The workshop list is null");
return new SearchResult<WorkshopInfoBaseDto>();
}
var workshops = await workshopRepository.Get(
offsetFilter.From,
offsetFilter.Size,
WorkshopIncludes,
filterExpression)
.ToListAsync()
.ConfigureAwait(false);

var workshopsDto = workshops.Select(MapToInfoWorkshopDto).ToList();

await FillRatingsForType(workshopsDto);
await FillRatingsForType(workshopsDto).ConfigureAwait(false);
await FillTakenSeats(workshopsDto).ConfigureAwait(false);

var count = await workshopRepository.CountWithDeleted(updatedAfter);
var count = await workshopRepository.Count(filterExpression).ConfigureAwait(false);

return new SearchResult<WorkshopInfoBaseDto>
{
Expand All @@ -101,7 +119,7 @@ public async Task<SearchResult<WorkshopInfoBaseDto>> GetWorkshops(DateTime updat
catch (Exception ex)
{
logger.LogError(ex, "An unexpected error occurred while processing workshops");
return new SearchResult<WorkshopInfoBaseDto>();
throw;
}
}

Expand Down Expand Up @@ -134,4 +152,16 @@ private async Task FillRatingsForType<T>(List<T> dtos)
dto.NumberOfRatings = averageRatingsForProvider?.RateQuantity ?? 0;
}
}

private async Task FillTakenSeats(List<WorkshopInfoBaseDto> dtos)
{
var fullDtos = dtos.OfType<WorkshopInfoDto>().ToList();
var ids = fullDtos.Select(w => w.Id).ToList();
var takenSeats = await applicationRepository.CountTakenSeatsForWorkshops(ids).ConfigureAwait(false);
foreach (var dto in fullDtos)
{
var takenSeat = takenSeats?.SingleOrDefault(w => w.WorkshopId == dto.Id)?.TakenSeats;
dto.TakenSeats = takenSeat ?? 0;
}
}
}
6 changes: 4 additions & 2 deletions OutOfSchool/OutOfSchool.BusinessLogic/Util/MappingProfile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ public MappingProfile()
.ForMember(dest => dest.CodeficatorAddressDto, opt => opt.MapFrom(src => src.CATOTTG));

CreateMap<Address, AddressInfoDto>()
.ForMember(dest => dest.CodeficatorAddressDto, opt => opt.MapFrom(src => src.CATOTTG));
.ForMember(dest => dest.CodeficatorAddress, opt => opt.MapFrom(src => src.CATOTTG));

/// <summary>
/// The localization is done outside the mapping
Expand Down Expand Up @@ -391,7 +391,7 @@ public MappingProfile()
opt => opt.MapFrom(src => src.WorkshopDescriptionItems.Where(x => !x.IsDeleted)))
.ForMember(dest => dest.Price, opt => opt.MapFrom(src => src.Price))
.ForMember(dest => dest.PayRate, opt => opt.MapFrom(src => src.PayRate))
.ForMember(dest => dest.TakenSeats, opt => opt.MapFrom(src => src.Applications.TakenSeats()))
.ForMember(dest => dest.TakenSeats, opt => opt.Ignore())
.ForMember(dest => dest.ImageIds, opt => opt.MapFrom(src => src.Images.Select(x => x.ExternalStorageId)))
.ForMember(dest => dest.Tags, opt => opt.MapFrom(src => src.Tags.Select(x => x.Name)))
.ForMember(dest => dest.LanguageOfEducation, opt => opt.Ignore())
Expand All @@ -402,6 +402,8 @@ public MappingProfile()

CreateMap<Provider, ProviderInfoDto>()
.IncludeBase<Provider, ProviderInfoBaseDto>()
.ForMember(dest => dest.Type, opt => opt.MapFrom(src => src.Type.Name))
.ForMember(dest => dest.Institution, opt => opt.MapFrom(src => src.Institution.Title))
.ForMember(dest => dest.ImageIds, opt => opt.MapFrom(src => src.Images.Select(x => x.ExternalStorageId)))
.ForMember(dest => dest.Rating, opt => opt.Ignore())
.ForMember(dest => dest.NumberOfRatings, opt => opt.Ignore());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace OutOfSchool.Common.Enums.Workshop;

// TODO: Add new types when customer specifies them

Check warning on line 5 in OutOfSchool/OutOfSchool.Common/Enums/Workshop/WorkshopType.cs

View workflow job for this annotation

GitHub Actions / SonarCloud

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
[JsonConverter(typeof(JsonStringEnumConverter))]
public enum WorkshopType
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using OutOfSchool.Services.Models;
using OutOfSchool.Services.Repository.Base.Api;
using OutOfSchool.Services.Util;

namespace OutOfSchool.Services.Repository.Api;

Expand All @@ -14,4 +16,6 @@ public interface IApplicationRepository : IEntityRepositorySoftDeleted<Guid, App
Task<int> UpdateAllApprovedApplications();

Task DeleteChildApplications(Guid childId);

Task<List<WorkshopTakenSeats>> CountTakenSeatsForWorkshops(List<Guid> workshopIds);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ public interface IProviderRepository : ISensitiveEntityRepositorySoftDeleted<Pro

Task<Provider> GetWithNavigations(Guid id);

Task<List<Provider>> GetAllWithDeleted(DateTime updatedAfter, int from, int size);

Task<int> CountWithDeleted(DateTime updatedAfter);

Task<List<int>> CheckExistsByEdrpous(Dictionary<int, string> edrpous);

Task<List<int>> CheckExistsByEmails(Dictionary<int, string> emails);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,4 @@ public interface IWorkshopRepository : IEntityRepositorySoftDeleted<Guid, Worksh
/// <returns>Amount of available seats for the specified workshop.</returns>
/// <exception cref="InvalidOperationException">It can throw exception when method get workshopId but Workshop doesn't exist.</exception>
Task<uint> GetAvailableSeats(Guid workshopId);

Task<List<Workshop>> GetAllWithDeleted(DateTime updatedAfter, int from, int size);

Task<int> CountWithDeleted(DateTime updatedAfter);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using OutOfSchool.Services.Models;
using OutOfSchool.Services.Repository.Api;
using OutOfSchool.Services.Repository.Base;
using OutOfSchool.Services.Util;

namespace OutOfSchool.Services.Repository;

Expand Down Expand Up @@ -98,4 +99,20 @@ public async Task DeleteChildApplications(Guid childId)
await dbContext.SaveChangesAsync().ConfigureAwait(false);
}
}

public Task<List<WorkshopTakenSeats>> CountTakenSeatsForWorkshops(List<Guid> workshopIds)
{
return dbSet
.Where(x =>
(x.Status == ApplicationStatus.Approved || x.Status == ApplicationStatus.StudyingForYears)
&& !x.IsDeleted
&& x.Child != null
&& !x.Child.IsDeleted
&& x.Parent != null
&& !x.Parent.IsDeleted
&& workshopIds.Contains(x.WorkshopId))
.GroupBy(a => a.WorkshopId)
.Select(g => new WorkshopTakenSeats(g.Key, g.Count()))
.ToListAsync();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,28 +84,6 @@ public async Task<Provider> GetWithNavigations(Guid id)
.SingleOrDefaultAsync(provider => !provider.IsDeleted && provider.Id == id);
}

public Task<List<Provider>> GetAllWithDeleted(DateTime updatedAfter, int from, int size)
{
IQueryable<Provider> query = db.Providers;

query = updatedAfter == default
? query.Where(provider => !provider.IsDeleted)
: query.Where(provider => provider.UpdatedAt > updatedAfter || provider.Workshops.Any(w => w.UpdatedAt > updatedAfter));

return query.Skip(from).Take(size).ToListAsync();
}

public Task<int> CountWithDeleted(DateTime updatedAfter)
{
IQueryable<Provider> query = dbSet;

query = updatedAfter == default
? query.Where(provider => !provider.IsDeleted)
: query.Where(provider => provider.UpdatedAt > updatedAfter || provider.Workshops.Any(w => w.UpdatedAt > updatedAfter));

return query.CountAsync();
}

public async Task<List<int>> CheckExistsByEdrpous(Dictionary<int, string> edrpous)
{
var existingEdrpouIpn = await db.Providers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,26 +96,4 @@ public override async Task<Workshop> Create(Workshop workshop)

return await Task.FromResult(workshop).ConfigureAwait(false);
}

public Task<List<Workshop>> GetAllWithDeleted(DateTime updatedAfter, int from, int size)
{
IQueryable<Workshop> query = dbSet;

query = updatedAfter == default
? query.Where(workshop => !workshop.IsDeleted)
: query.Where(workshop => workshop.UpdatedAt > updatedAfter || workshop.DeleteDate > updatedAfter);

return query.Skip(from).Take(size).ToListAsync();
}

public Task<int> CountWithDeleted(DateTime updatedAfter)
{
IQueryable<Workshop> query = dbSet;

query = updatedAfter == default
? query.Where(workshop => !workshop.IsDeleted)
: query.Where(workshop => workshop.UpdatedAt > updatedAfter || workshop.DeleteDate > updatedAfter);

return query.CountAsync();
}
}
5 changes: 5 additions & 0 deletions OutOfSchool/OutOfSchool.DataAccess/Util/WorkshopTakenSeats.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
using System;

namespace OutOfSchool.Services.Util;

public record WorkshopTakenSeats(Guid WorkshopId, int TakenSeats);
Loading
Loading