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

Feat/week06 #6

Merged
merged 13 commits into from
Dec 29, 2023
Merged

Feat/week06 #6

merged 13 commits into from
Dec 29, 2023

Conversation

ekhvalov
Copy link
Owner

@ekhvalov ekhvalov commented Dec 21, 2023

Manager Events

  • Реализован manager.events.swagger.yml
  • manager.events.swagger.yml добавлен в debug server
  • manager.events.swagger.yml добавлен в Swagger UI
  • NewMessageEvent: обработана ситуация, когда клиент пишет в чат, но с другой стороны ещё нет менеджера
  • NewChatEvent.CanTakeMoreProblems выставляется с помощью services/manager-load
  • NewChatEvent.RequestID равен initial_request_id первого видимого менеджеру сообщения в чате
  • managerevents.Adapter покрыт юнит-тестами

Manager Scheduling

  • Реализован services/manager-scheduler
  • Менеджеру не прилетают "пустые" для него чаты

Manager API

  • Реализованы и покрыты тестами на каждом из слоёв функциональности:
  1. Получение менеджером чатов
  2. Получение менеджером истории чата
  3. Отправка менеджерских сообщений
  4. Закрытие менеджером чата
  • Для ускорения получения менеджерских чатов в таблицу problems добавлен индекс

Validation

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

Testing

  • E2E тест "Manager Scheduling Smoke" расширен двумя новыми шагами
  • E2E тест "Error Responses" расширен новым шагом
  • [Optional] Поддержан E2E user agent
  • Юнит-тесты проходят (task tests)
  • Интеграционные тесты проходят (task tests:integration)
  • E2E тесты проходят (task tests:e2e)
  • Открываешь UI и понимаешь, что НАКОНЕЦ-ТО ВСЁ РАБОТАЕТ!!!

@MysterySuperhero MysterySuperhero self-assigned this Dec 23, 2023
Copy link
Collaborator

@MysterySuperhero MysterySuperhero left a comment

Choose a reason for hiding this comment

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

🏅 🏅 🏅 Отличная работа! 🏅 🏅 🏅

problem.ResolvedAtIsNil(),
),
).
Order(chat.ByCreatedAt(sql.OrderDesc())).
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick, зацепился глаз: мы наоборот решили отсортировать, т.к. посчитали, что лучше первый обратившийся будет светиться первым в списке чатов у менеджера 🙂

Copy link
Owner Author

Choose a reason for hiding this comment

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

Исправил

func (r *Repo) GetOpenProblemChatsForManager(ctx context.Context, managerID types.UserID) ([]Chat, error) {
chats, err := r.db.Chat(ctx).Query().
Where(
chat.HasProblemsWith(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Лайк 👍

@@ -53,3 +54,102 @@ func (s *ChatsRepoSuite) Test_CreateIfNotExists() {
s.Equal(chat.ID, chatID)
})
}

func (s *ChatsRepoSuite) Test_GetOpenProblemChatsForManager() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Лайк

@@ -15,6 +17,18 @@ type Repo struct {
Options
}

func (r *Repo) GetClientIDByChatID(ctx context.Context, chatID types.ChatID) (types.UserID, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не туда затесался метод – его бы в api.go + тест

Copy link
Owner Author

Choose a reason for hiding this comment

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

Исправил.


var managerID types.UserID
for _, problemID := range problemIDs {
managerID, err = s.mngrPool.Get(ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

На всякий напоминаем про кейс с зомби-менеджером 🙂

Copy link
Owner Author

Choose a reason for hiding this comment

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

Что за кейс? Менеджер, которому не досталось проблемы?
Добавил несколько тестов, теперь все описанные в задании случаи тестируются.

return fmt.Errorf("get manager from pool: %v", err)
}

problem, err := s.problemsRepo.AssignManagerToProblem(ctx, managerID, problemID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Академический интерес: а почему назначение проблемы на менеджера тоже не добавили в транзакцию?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Исправил.

message.RequestID,
canManagerTakeProblem,
)
err = j.eventStream.Publish(ctx, pl.ManagerID, newChatEvent)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Здесь тоже можно, как и в других джобах, распараллелить отправку

Copy link
Owner Author

Choose a reason for hiding this comment

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

Исправил.

@ekhvalov ekhvalov merged commit 01bd8ed into main Dec 29, 2023
3 checks passed
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.

None yet

2 participants