Issue #906 backend админ панель раздел интервью

[AlexVin11]

Реализовал логику для интервью в разделе админ

[alexey4050]

@RequestMapping("/{locale}/admin/interview")
Наставник нам предложила делать проверку чужого кода. Я попробую. {locale} решили не использовать, устанавливаться будет во фронте (язык они устанавливают). Еще admin/interview возникает вопрос где лучше размещать эндпоинт в AdminController или каждый в своем, если оставлять, так как возникает ошибка если идет дублирование пути. P.S.: я свой marketing буду переделывать (такая же ошибка)

[alexey4050]

@RequestParam(required = false) String interviewSearchWord,
                                        @RequestParam(defaultValue = DEFAULT_PAGE_TO_SHOW) int page,
                                        @RequestParam(defaultValue = DEFAULT_NUMBER_OF_INTERVIEW_ON_PAGE) int size,

Мы мигрировали логику через application.yml c использованием Pageable (напрямую в параметрах метода)

[alexey4050]

`catch (InterviewNotFoundException ex) {

        Map<String, Object> errorProps = flashPropsService.buildProps(locale, request);

        errorProps.put("status", 404);
        errorProps.put("message", ex.getMessage());
        errorProps.put("interviewId", id);
        errorProps.put("locale", locale);

        ResponseEntity<?> inertiaResponse = inertia.render("Error/InterviewNotFound", errorProps);` 

Дублирование кода в контроллере.

[alexey4050]

public void updateEntityWithUpdateDto(InterviewUpdateDTO dto, Interview model) {
        if (dto.getTitle() != null && dto.getTitle().isPresent()) {
            String titleFromUpdateDto = dto.getTitle().get();
            if (titleFromUpdateDto == null) {
                throw new IllegalArgumentException("Title in InterviewUpdateDTO cannot be null");
            }

JsonNullable в DTO .get() никогда не вернет null если isPresent() == true (мне так кажется) не только в этом методе

[alexey4050]

public InterviewDTO create(InterviewCreateDTO createDTO) {
        Interview model = interviewConverter.convertCreateDtoToEntity(createDTO);
        interviewRepository.save(model);
        InterviewDTO dto = interviewConverter.convertEntityToDto(model);

        return dto;
    }

interviewService.create(createDTO);
        redirectAttributes.addFlashAttribute("success", "Интервью успешно создано");

        return inertia.redirect("/" + locale + "/admin/interview");

У тебя сервис создает интервью и возвращает данные, но контроллер эти данные теряет и делает редирект( нужен редирект на создание интервью, а не на весь список) там надо + createdInterview.get..... И у тебя POST HTTP. статус другой 302 Found у тебя CREATE. Это моё мнение может быть не прав

[alexey4050]

speakerIdFromDto != null
                            ? userRepository.findById(speakerIdFromDto)
                                    .orElseThrow(() -> new ResourceNotFoundException("User with id "
                                            + speakerIdFromDto + " not found."))

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

catch (InterviewNotFoundException ex) {

            Map<String, Object> errorProps = flashPropsService.buildProps(locale, request);

[AlexVin11]

@RequestParam(required = false) String interviewSearchWord,
                                        @RequestParam(defaultValue = DEFAULT_PAGE_TO_SHOW) int page,
                                        @RequestParam(defaultValue = DEFAULT_NUMBER_OF_INTERVIEW_ON_PAGE) int size,

Мы мигрировали логику через application.yml c использованием Pageable (напрямую в параметрах метода)

Не понял о чем тут речь. Зашел глянуть на application.yml ы в main ветке, но ничего связанного с Pageable не вижу

[ann-p-1320]

локаль в запросы не передаем, интерфейс будет локализовываться на фронте

[ann-p-1320]

логику принято держать в сервисах, перенеси, пжл, вот этот кусок:

        Pageable pageable = PageRequest.of(page, safeSize, Sort.by("createdAt").descending());

        Page<InterviewDTO> interviewPage = StringUtils.hasText(interviewSearchWord)
                ? interviewService.search(interviewSearchWord, pageable)
                : interviewService.getAll(pageable);

[ann-p-1320]

Вероятно, эти константы больше не нужны, так как в application.yml добавили

  data:
    web:
      pageable:
        default-page-size: 10
        max-page-size: 100

[ann-p-1320]

Можно сделать передачу параметров как в LearningProgressController:

public Object getProgress(@PageableDefault(size = 10) Pageable pageable) 

Заодно потести, пжл, можно ли обойтись вообще без @PageableDefault(size = 10), весь в ямле вроде указали дефолт default-page-size: 10

[ann-p-1320]

Обработку ошибок лучше вынести в GlobalExceptionHandler

[ann-p-1320]

Локализованные константы убираем с бэкенда, лучше типа такого:
redirectAttributes.addFlashAttribute("success", "update.success");

[ann-p-1320]

если код не нужен, лучше удалить

[ann-p-1320]

Пожалуйста, опиши более подробно, что именно и как реализовано, по аналогии с тем, как сделано здесь:
#935

[ann-p-1320]

для полей входящих дто можно использовать валидацию с помощью аннотаций, например @notblank, тогда не придется валидировать в конвертере

[ann-p-1320]

метод получения сущности по id должен быть в сервисе, а не в конвертере

[ann-p-1320]

первым делом проверить на null и вернуть то, что нужно, и дальше уже писать не в ветке else, а просто

[ann-p-1320]

Лучше через билдер

[ann-p-1320]

Это поле будет приходить с фронта при создании сущности? По идее не должно, и тогда его лучше из дто убрать

[ann-p-1320]

Мапперы только перекладывают готовые поля из одних дто в другие, в них не нужно ходить в базу

[ann-p-1320]

Расскажи, пожалуйста, как ты видишь дальнейшее использование этого базового класса?

[ann-p-1320]

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

[ann-p-1320]

вместо конкатенации можно использовать builder.like

[ann-p-1320]

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

[ann-p-1320]

Если код не нужен, лучше удалить