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

Delayed Notifications #298

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Delayed Notifications #298

wants to merge 23 commits into from

Conversation

BakhorikovEgor
Copy link
Contributor

No description provided.

.UseSqlServerStorage(Configuration.GetConnectionString("HangfireConnection")));

// Add the processing server as IHostedService
services.AddHangfireServer();
Copy link
Contributor

Choose a reason for hiding this comment

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

это точно нужно?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

у hangfire клиент - серверная архитектура, где сервер и клиент общаются с общей бд
сервер нужно добавить, чтобы он брал задачи из бд и выполнял их время от времени

using HwProj.Repositories;


namespace HwProj.NotificationsService.API.Repositories;
Copy link
Contributor

Choose a reason for hiding this comment

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

namespace надо сделать block-scoped

using Microsoft.EntityFrameworkCore;

namespace HwProj.NotificationsService.API.Models
{
public sealed class NotificationsContext : DbContext
{
public DbSet<Notification> Notifications { get; set; }
public DbSet<ScheduleWork> ScheduleWorks { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public DbSet<ScheduleWork> ScheduleWorks { get; set; }
public DbSet<ScheduleJob> ScheduleJobs { get; set; }

@@ -2,7 +2,7 @@

namespace HwProj.Repositories
{
public interface IEntity<TKey>
public interface IEntity<TKey>
Copy link
Contributor

Choose a reason for hiding this comment

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

нужно отформатировать

{
public class UpdateTaskEvent : Event
{
public CourseDTO Course { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

можем ли мы из курса взять только необходимые данные?

Comment on lines 11 to 20
public string TaskTitle { get; set; }

public long TaskId { get; set; }

//закладываюсь, что дату публикации не будут менять после публикации
public DateTime PreviousPublicationDate { get; set; }

public DateTime PublicationDate { get; set; }

public DateTime? Deadline { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Возможно, мы хотим старую задачу и новую задачу, а не набор отдельных полей?


namespace HwProj.Models.NotificationsService
{
public class ScheduleWork : IEntity<string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public class ScheduleWork : IEntity<string>
public class ScheduleJob : IEntity<string>


public override async Task HandleAsync(DeleteTaskEvent @event)
{
var id = ScheduleWorkIdBuilder.Build(@event, @event.TaskId);
Copy link
Contributor

Choose a reason for hiding this comment

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

По эвенту удаления не должны создаваться ключи других эвентов, тем более, что отменяемых эвентов может быть несколько

{
_ when eventType == typeof(NewTaskEvent) || eventType == typeof(UpdateTaskEvent) ||
eventType == typeof(DeleteTaskEvent)
=> "Task",
Copy link
Contributor

Choose a reason for hiding this comment

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

для дедлайнов и публикаций ключи должны отличаться

Comment on lines 17 to 19
var userId = context.HttpContext.User.Claims
.FirstOrDefault(claim => claim.Type.ToString() == "_id")
?.Value;;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var userId = context.HttpContext.User.Claims
.FirstOrDefault(claim => claim.Type.ToString() == "_id")
?.Value;;
var userId = context.HttpContext.User.FindFirst("_id");

Comment on lines 8 to 9
Homeworks,
Tasks
Copy link
Contributor

Choose a reason for hiding this comment

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

Думаю, нам не нужно это изменение

var course = await _coursesServiceClient.GetCourseById(@event.Course.Id);
if (course == null) return;

var studentIds = course.CourseMates.Select(t => t.StudentId).ToArray();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var studentIds = course.CourseMates.Select(t => t.StudentId).ToArray();
var studentIds = course.AcceptedStudents.Select(t => t.StudentId).ToArray();

Comment on lines 87 to 88
await EventHandlerExtensions<NewTaskEvent>.DeleteScheduleJobAsync(@event, @event.TaskId,
_scheduleJobsRepository);
Copy link
Contributor

Choose a reason for hiding this comment

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

Давай будем удалять джобу только тогда, когда её создаем: вынесем этот код из AddNotificationsAsync

Copy link
Contributor

Choose a reason for hiding this comment

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

Также нужно удалять джобы просто из репозитория

var accountsData = await _authServiceClient.GetAccountsData(studentIds);

var url = _configuration["Url"];
var isTaskPublished = @event.PreviousEvent.PublicationDate < DateTimeUtils.GetMoscowNow();
Copy link
Contributor

Choose a reason for hiding this comment

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

если задача опубликована, мы можем использовать логику из NewHomeworkTaskEventHandler

Comment on lines 26 to 30
var predicate = (ScheduleJob scheduleJob) =>
{
var id = ScheduleJobIdHelper.ParseId(scheduleJob.Id);
return id.CategoryId == categoryId && id.Category.Equals(category);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Этот код не транслируется в SQL. Нужно или делать фильтрацию по объектному ключу и его полям, или составлять список всех буквальных ключей, и искать записи в бд по ним

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants