Hexlet / hexlet-correction

Typo Reporter
https://fixit.hexlet.io/
GNU Affero General Public License v3.0
51 stars 85 forks source link

Bugfix/#260 #269

Closed d1z3d closed 5 months ago

d1z3d commented 6 months ago

Добрый день! Проблема заключается в том, что не удается заполнить данные при формировании страницы по шаблону workspace/wks-users (например, wrkInfo). В итоге возникает ошибка. Изменения залил на https://hexlet-correction-ac3h.onrender.com

fey commented 5 months ago

@bazilval есть желание сделать код ревью? :_)

bazilval commented 5 months ago

@fey гляну!

bazilval commented 5 months ago

@d1z3d привет! Мне не очень нравится дублирование тут, весь блок повторяет код из метода getWorkspaceUsersPage()

Может как-то вынести это в какой-то общий метод, типа renderUserPage(), чтобы переиспользовать его и там, и там?

Ну и по мелочи: вот тут неудачное название для переменной, лучше так и назвать, что это ACCOUNT_INCORRECT_EMAIL, чтобы в тесте было понятно что мы подставили

bazilval commented 5 months ago

Ещё можно разметку заодно сделать поаккуратнее в части вывода ошибки: image

Ну либо это можно уже в рамках #267 решить

d1z3d commented 5 months ago

@bazilval, привет! Подготовку данных вынес в отдельный метод. Наименование сделал немного другое, но добавил java doc, чтобы было понятно для чего нужен метод. Пофиксил отображение. По какой-то причине у параграфа был margin bottom в 1rem, поэтому параграф отображался криво. Изменения раскатил на render.

Снимок экрана 2024-06-09 в 18 24 05
d1z3d commented 5 months ago

@bazilval, @fey, добрый день! Вижу будет конфликт при мерже пр. Мне необходимо внести правки? Здесь не очень представляю процесс, если такая проблема возникает.

UPD: поправил конфликт. Когда склонил проект, то выполнил команды по инструкции - https://ru.hexlet.io/blog/posts/Open-Source-github. По какой-то причине не подтянулись более свежие изменения.

fey commented 5 months ago

@d1z3d тесты упали, посмотрите. пожалуйста, в чем может быть дело.

bazilval commented 5 months ago

@d1z3d кавычки около емейла потеряли в тесте Плюс надо убедиться, что там действительно значение емейла подставляется, а не что-то другое Сейчас посмотрел на скрины ваши, там "The email "Email" is not valid"

Кстати, а как так вышло, на локальном тесты проходили или вы не проверяли?

d1z3d commented 5 months ago

@bazilval, @fey, добрый день! Docker hub был заблокирован на тот момент. Выкачивал с амазона образ postgres + правил docker-compose, чтобы локально запустить проект, но в тестах используются тест контейнеры и не получилось их переделать для запуска тестов. Рассчитывал, что при билде на рендере будут запущены тесты и если будет что-то не так, то там поймаю ошибку, т.к. сервер находится не в России. На рендер деплой прошел без проблем.

bazilval commented 5 months ago

@d1z3d сделаете фикс? Ну и по хорошему разобраться почему на рендере тесты прошли

d1z3d commented 5 months ago

@bazilval, добрый день!

Мне нужно создать отдельную задачу для правки теста?

Посмотрел лог на рендере, после build. Не происходит вызов тестов. В dockerfile билд происходит с флагом “-x test”. Он исключает выполнение тестов (https://www.baeldung.com/gradle-skip-tests). Поэтому у меня задеплоилось без ошибок. Хочу дополнительно проверить и пришлю лог.

d1z3d commented 5 months ago

Добрый день! Кажется, что не получится отказаться от "-x test". По крайней мере, я не смог найти решение. Похожая проблема описана в этой статье на stackoverflow.

bazilval commented 5 months ago

@d1z3d Да, отдельным пулл реквестом поправьте тест, чтобы деплой перестали падать

По поводу тест-контейнеров ничего не будем делать, печально, что не получается запустить локально и проверить сразу

d1z3d commented 5 months ago

@bazilval Добрый день!

Создал новый драфт пр, т.к. есть вопрос о необходимости изменении других dto - https://github.com/Hexlet/hexlet-correction/pull/278