Feature/delete item#47
Conversation
| import org.springframework.web.bind.annotation.RequestParam; | ||
| import org.springframework.web.bind.annotation.ResponseStatus; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
| import org.springframework.web.bind.annotation.*; |
There was a problem hiding this comment.
В одном из прошлых ПР был коммент от Андрея, что "не приветствуется импорт со *. Мол может подгрузиться много лишнего". Не блокер, конечно, но надо нам разобраться в команде, как лучше делать, чтобы было в едином стиле. То же самое в классе ItemServiceImplTest. А так, по-моему, всё супер
There was a problem hiding this comment.
Сейчас есть правило в конфигурации checkstyle для этого, так что это нужно исправить для прохождения пайплайна. Перед пушем можно локально выполнить в терминале make checkstyle и подстветятся рекомендации для исправления
| @ResponseStatus(HttpStatus.NO_CONTENT) | ||
| @PreAuthorize("hasRole('ADMIN')") | ||
| public void deleteItem(@PathVariable Long itemId) { | ||
| itemService.deleteItem(itemId); |
There was a problem hiding this comment.
В ТЗ метод называется "softDelete". Как понимаю подразумевает не удаление, а скрытие товара
| PageResponse<ItemResponse> getItems(String sort, String order, String category, String search, int page, int size); | ||
|
|
||
| /** | ||
| * Удаляет товар по указанному идентификатору. |
There was a problem hiding this comment.
Может заменить на "Скрывает" или "Скрывает из выдачи"?
| } | ||
|
|
||
| return item; | ||
| public void deleteItem(Long itemId) { |
There was a problem hiding this comment.
По ТЗ: "ItemService.softDelete(id) — ставит is_active = false". Получается мы не удаляем товар, а скрываем его из выдачи. То есть товар у нас в системе остается и его Stock тоже. Поправь меня, если не прав.
There was a problem hiding this comment.
Там двояко получается, думаю админ имеет право удалять товар.
В общем логику переделал, сейчас находит товар и деактивирует его.
|
|
||
| return item; | ||
| public void deleteItem(Long itemId) { | ||
| itemRepository.findById(itemId).orElseThrow(() -> { |
There was a problem hiding this comment.
И соответственно при повторном удалении (проверке на isActive = false) пробрасываем EntityNotFoundException
|
|
||
| /** | ||
| * Удаляет товар по указанному идентификатору. | ||
| * Перед удалением товара удаляются связанные записи об остатках на складе. |
There was a problem hiding this comment.
Ну и если мы оставляем существовать остатки, то строка не актуальна
| public void deleteItem(Long itemId) { | ||
| itemRepository.findById(itemId).orElseThrow(() -> { | ||
| log.warn("Item с id={} не найден", itemId); | ||
| return new EntityNotFoundException("Item с таким id = " + itemId + " не найден"); |
There was a problem hiding this comment.
Можно использовать EntityNotFoundException.forId("Item", itemId).
# Conflicts: # src/main/java/com/warehouse/controller/ItemController.java # src/main/java/com/warehouse/repository/ItemRepository.java # src/main/java/com/warehouse/repository/StockRepository.java # src/main/java/com/warehouse/service/item/ItemService.java # src/main/java/com/warehouse/service/item/ItemServiceImpl.java # src/test/java/com/warehouse/service/ItemServiceImplTest.java
Andrey-Potokin
left a comment
Andrey-Potokin
left a comment
There was a problem hiding this comment.
Только по удаленному методу вопрос
There was a problem hiding this comment.
Удалил метод "getItem()"? В main ветке он есть
5 participants
Добавлена функциональность удаление товара