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

Add bookings #4

Merged
merged 5 commits into from
Aug 5, 2024
Merged

Add bookings #4

merged 5 commits into from
Aug 5, 2024

Conversation

Nikolaev-Java
Copy link
Owner

@Nikolaev-Java Nikolaev-Java commented Aug 2, 2024

Привет. Некоторые тесты закомичены, чтобы ошибки не вылезали. Я просто посмотрел что в следующем ТЗ Тесты писать будем, и решил так сделать а то и так уже по срокам не успеваю.
Еще по выводу бронирования при запросе списка вещей (getAllOfOwner)... Что-то я не додумался как его сделать оптимально, с учетом минимизации запросов к БД. Может что-то посоветуешь?

@Setter
@Getter
@Builder
static class ItemDtoResponse {

Choose a reason for hiding this comment

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

может тут использовать поле с типом ItemDto просто?

Copy link
Owner Author

Choose a reason for hiding this comment

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

дело в том что в тестах постман проверяются поля item.id и item.name. То есть в json должен объект с такими полями.

Choose a reason for hiding this comment

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

ну так оно и будет, если сделать ItemDto item

Copy link
Owner Author

Choose a reason for hiding this comment

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

ну в ItemDto есть еще поля description и available, а они в выводе не нужны, поэтому и внутренний класс в котором полей меньше. Я просто сделал дополнительные ДТО с меньшим количеством полей, только те которые проверяются в постман тестах

Choose a reason for hiding this comment

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

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

@Setter
@Getter
@AllArgsConstructor
static class UserDtoResponse {

Choose a reason for hiding this comment

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

а вместо этого long userId

Copy link
Owner Author

Choose a reason for hiding this comment

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

здесь так же по тестам проверяется booker.id.

Choose a reason for hiding this comment

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

а тут тогда BookerDto booker
Проверь, должно работать

@Component
public class BookingMapper {

public Booking toNewBooking(NewBookingDtoRequest dto, Item item, User booker) {

Choose a reason for hiding this comment

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

хорошо бы проверять на null входящие в метод параметры

Copy link
Owner Author

Choose a reason for hiding this comment

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

если проверю дто на нулл, этого же достаточно будет? item и booker если будет нулл то NPE не должно быть, методы их не вызываем.

Choose a reason for hiding this comment

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

да достаточно проверить объект

@Alexander-Bolotov
Copy link

Привет. Некоторые тесты закомичены, чтобы ошибки не вылезали. Я просто посмотрел что в следующем ТЗ Тесты писать будем, и решил так сделать а то и так уже по срокам не успеваю. Еще по выводу бронирования при запросе списка вещей (getAllOfOwner)... Что-то я не додумался как его сделать оптимально, с учетом минимизации запросов к БД. Может что-то посоветуешь?

ну что бы упростить сам метод, это "усложнить" сам sql запрос, т.е. вытаскивать данные прямо в нем. А поля lastBooking и nextBooking нужны в дто?

@Nikolaev-Java
Copy link
Owner Author

да last и nextbooking нужны в Дто. C одной вещью можно ограничить список в запросе. А как для списка это сделать не понятно, приходится вот в стримах фильтровать как то. Или можно в цикле для каждого item запрос в бд делать чтобы вытащить брони, но я так понимаю что не стоит в цикле запросы к бд делать.

@Alexander-Bolotov
Copy link

да last и nextbooking нужны в Дто. C одной вещью можно ограничить список в запросе. А как для списка это сделать не понятно, приходится вот в стримах фильтровать как то. Или можно в цикле для каждого item запрос в бд делать чтобы вытащить брони, но я так понимаю что не стоит в цикле запросы к бд делать.

да в цикле запросы делать это bad practice)

@Nikolaev-Java
Copy link
Owner Author

я переписал запрос к БД должен вроде как работать. Теперь только в хэшмапу перевожу список и все.
постман тесты не проверяют этот функционал.

@Nikolaev-Java Nikolaev-Java merged commit f63eb44 into main Aug 5, 2024
2 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.

2 participants