Skip to content

feat: реализовал фичу подписки и написал postman тесты#4

Merged
Chernosmaga merged 11 commits intomainfrom
feature_subscriptions
Jan 22, 2024
Merged

feat: реализовал фичу подписки и написал postman тесты#4
Chernosmaga merged 11 commits intomainfrom
feature_subscriptions

Conversation

@Chernosmaga
Copy link
Copy Markdown
Owner

No description provided.

.stream().filter(request -> request.getStatus().equals(CONFIRMED))
.count())).collect(Collectors.toList());
List<EventShortDto> eventsToReturn;
if (sort.equals(VIEWS)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Лучше сделать наоборот - сравнение VIEVS с sort, так не понятно nullable это поле или нет. Может быть нуллпоинтер

Copy link
Copy Markdown
Owner Author

@Chernosmaga Chernosmaga Jan 22, 2024

Choose a reason for hiding this comment

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

По идее, оно не должно прийти null, ведь у меня стоит значение по умолчанию в контроллере VIEWS

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Да, это понятно, но лучше всегда сравнивать константу со значением, чем наоборот

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Понял, исправлю

request.setRequester(user);
State status = event.getRequestModeration() ? PENDING : CONFIRMED;
request.setStatus(event.getParticipantLimit() == 0 ? CONFIRMED : status);
if (request.getStatus().equals(CONFIRMED)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Может быть нуллпоинтер


@Override
public void unfollow(Long userId, Long followerId) {
log.debug("unfollow({}, {})", userId, followerId);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Такие комментарии лучше не оставлять - ниже хороший пример, если пишешь на русском, то и остальные так пиши. Здесь просто выглядит вразнобой

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Мне удалить логи во всем проекте на уровне debug?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Нет, оставь, просто текст можно немного иначе писать - подробнее. Здесь выглядит, будто лог временный для отладки

User user = userSearch(userId);
User follower = userSearch(followerId);
if (!subscriptionRepository.existsByUserAndFollower(user, follower)) {
throw new DataViolationException("Вы не подписаны на пользователя");
Copy link
Copy Markdown

@PolnySkvorets PolnySkvorets Jan 20, 2024

Choose a reason for hiding this comment

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

Хорошо, что валидация на уровне сервиса

Copy link
Copy Markdown

@PolnySkvorets PolnySkvorets left a comment

Choose a reason for hiding this comment

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

Привет! Оставил несколько комментариев, повторяется в нескольких местах. Поправь это

Copy link
Copy Markdown

@PolnySkvorets PolnySkvorets left a comment

Choose a reason for hiding this comment

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

Замечания исправлены

@Chernosmaga Chernosmaga merged commit ef8d845 into main Jan 22, 2024
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