Skip to content
Open
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
2 changes: 1 addition & 1 deletion Security/Service/Implementations/AuthService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class AuthService : IAuthService
private readonly UserManager<ApplicationUser> _userManager;
private readonly ITokenGenerator _tokenGenerator;
private readonly IEmailService _emailService;
private readonly IHttpContextAccessor _httpContextAccessor;
private readonly IHttpContextAccessor _httpContextAccessor;//not needed
private readonly ICurrentUserService _currentUserService;
private readonly IWebHostEnvironment _webHostEnvironment;
private readonly MailTemplatesConstants _mailTemplatesConstants;
Expand Down
1 change: 1 addition & 0 deletions TestingApi/Controllers/AnswerTemplatesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class AnswerTemplatesController : ControllerBase
private readonly IQuestionTmplService _questionTmplService;
private readonly ILogger<AnswersController> _logger;

//logger, questionService and answerService are not needed. also very long string, better to split into several lines
public AnswerTemplatesController(IQuestionService questionService, ILogger<AnswersController> logger, IAnswerService answerService, IAnswerTmplService answerTmplService, IQuestionTmplService questionTmplService)
{
_logger = logger;
Expand Down
2 changes: 1 addition & 1 deletion TestingApi/Data/DataContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ private void OnBeforeSaving()
switch (entry.State)
{
case EntityState.Modified:
baseEntity.ModifiedTimestamp = DateTime.Now;
baseEntity.ModifiedTimestamp = DateTime.Now;//better use utc time. it may cause data inconsistency after deployment to many availability zones
if(_currentUserService.UserId != null)
baseEntity.ModifiedBy = Guid.Parse(_currentUserService.UserId);
break;
Expand Down
2 changes: 1 addition & 1 deletion TestingApi/Dto/FiltersDto.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ public class FiltersDto
{
public string? SearchTerm { get; set; }
public string? SortColumn { get; set; }
public string? SortOrder { get; set; }
public string? SortOrder { get; set; }//it can be boolean

[Range(1, int.MaxValue, ErrorMessage = "Page must be greater than 0")]
public int Page { get; set; }
Expand Down
2 changes: 1 addition & 1 deletion TestingApi/Extensions/RegisterServices.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public static void ConfigureAuth(this IServiceCollection services, IConfiguratio

ValidateIssuerSigningKey = true,
ClockSkew = TimeSpan.Zero,
IssuerSigningKey = new SymmetricSecurityKey(Encoding.ASCII.GetBytes(configuration["Auth:SecretKey"])),
IssuerSigningKey = new SymmetricSecurityKey(Encoding.ASCII.GetBytes(configuration["Auth:SecretKey"])),//better to get the config model. this is safer
};
});
}
Expand Down
6 changes: 4 additions & 2 deletions TestingApi/Services/Implementations/AnswerService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ public AnswerService(DataContext dataContext, ILogger<AnswerService> logger, IMa
_logger = logger;
}

public async Task<AnswerResponseDto?> GetAnswerById(Guid id, CancellationToken cancellationToken = default)
public async Task<AnswerResponseDto?> GetAnswerById(Guid id, CancellationToken cancellationToken = default)//no need to add word 'Answer' into the method name.
//it is already AnswerService
{
var answers = await _dataContext.Answers
.FirstOrDefaultAsync(a => a.Id == id, cancellationToken);

return _mapper.Map<AnswerResponseDto>(answers);
}

public async Task<bool> AnswerExistsAsync(Guid id, CancellationToken cancellationToken = default)
public async Task<bool> AnswerExistsAsync(Guid id, CancellationToken cancellationToken = default)//looks like you need a generic repo to put common logic here
//I see at least several very similar methods in most of the services
{
return await _dataContext.Answers.AnyAsync(a => a.Id == id, cancellationToken);
}
Expand Down
6 changes: 4 additions & 2 deletions TestingApi/Services/Implementations/FileService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ public class FileService : IFileService
{
private readonly IConfiguration _configuration;

public FileService(IConfiguration configuration)
public FileService(IConfiguration configuration)//do not inject configuration. inject model instead
{
_configuration = configuration;
}
public async Task RemoveFilesByNameIfExistsAsync(string fileName, CancellationToken cancellationToken = default)
public async Task RemoveFilesByNameIfExistsAsync(string fileName, CancellationToken cancellationToken = default)//there is no need in this method
//old file is replaced by new one when created
//read FileMode.Create description
Copy link
Owner

Choose a reason for hiding this comment

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

Проблема в тому, що розширення файлів можуть бути різними, чи буде тоді працювати реплейс по імені?

{
var folderName = _configuration["FileStorage:FolderPath"];
var pathToRemoveFiles = Path.Combine(Directory.GetCurrentDirectory(), folderName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public TestStatisticService(DataContext dataContext, IMapper mapper)
_mapper = mapper;
}

public async Task<TestStatisticResponseDto> GetTestStatistic(Guid testId,
public async Task<TestStatisticResponseDto> GetTestStatistic(Guid testId,//lines 29, 32, 36, 47 calls can be done in parallel
CancellationToken cancellationToken = default)
{
var test = await _dataContext.Tests.Where(t => t.Id == testId).FirstAsync(cancellationToken);
Expand Down
3 changes: 2 additions & 1 deletion TestingApi/Services/Implementations/UserService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ public async Task<bool> UserExistsAsync(Guid id, CancellationToken cancellationT
.AnyAsync(e => e.Id == id, cancellationToken);
}

public async Task<UserResponseDto> RegisterUserAsync(UserSignUpDto userSignUpDto,
public async Task<UserResponseDto> RegisterUserAsync(UserSignUpDto userSignUpDto,//this way of handling users is not the best
//better to make security api visible to FE and do all the call directly
CancellationToken cancellationToken = default, bool isAdmin = false)
{
var jsonContent = JsonSerializer.Serialize(userSignUpDto);
Expand Down
2 changes: 1 addition & 1 deletion TestingApi/Services/Implementations/UserTestService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ public async Task<TestResultResponseDto> GetUserTestResults(Guid userId, Guid te
};
}

private IQueryable<QuestionResultResponseDto> GetAnsweredUserQuestionResultQuery(Guid userId, Guid testId)
private IQueryable<QuestionResultResponseDto> GetAnsweredUserQuestionResultQuery(Guid userId, Guid testId)//very big linq. better to split into private methods
{
var userAnswersDetailsQuery = _dataContext.UserAnswers
.Include(ua => ua.Question)
Expand Down