ya-pomogau / frontend

5 stars 7 forks source link

Проверить раздел "Контакты" (редактирование, вывод) #193

Open INextYP opened 1 month ago

INextYP commented 1 month ago

В рамках задачи требуется проверить работу раздела "Контакты". Описать:

Важно: Контакты выглядят единым образом для неавторизованных пользователей, волонтеров, реципиентов и администраторов, но только Главный администратор может менять их. Обязательно нужно проверить этот кейс.

Правки никакие не пушим. Задачи по исправлению ошибок/ доработкам будут в следующем спринте.

Ветка, где проверяем: develop;

Все баги, предложения и т.д. указываем в комментариях к этой карточки

IvannaBalanyuk commented 1 month ago

Данные о контактах сейчас статичны:

Image

При сохранении формы после редактирования никуда не уходят:

Image

Network тоже проверяла - после сохранения никакой запрос не уходит.

IvannaBalanyuk commented 1 month ago

Процесс редактирования на фронте возможно нуждается в доработке. Сейчас оба блока используют один коллбэк handleEnableEdit и поэтому после нажатия Изменить данные в одном поле редактирование открывается и для второго тоже.

IvannaBalanyuk commented 1 month ago

Возможность редактирования контактов открыта для пользователя с ролью Admin с нужными разрешениями:

Image

Image

У нашего тестового admin4 в permissions пусто, поэтому для проверки вёрстки "выключала" roleChecker.

IvannaBalanyuk commented 1 month ago

По верстке - много расхождений с макетом. В первую очередь обратил на себя внимание header (компонент SmartHeader), но это за рамками таски, поэтому рассказываю в предпоследнем комменте)

Начну с десктопа. Блоки с контактами сверстаны с использованием множества margin-отступов. В паре мест нижний margin одного блока и верхний другого схлопываются, из-за чего в итоге отступы не соответствуют макету.

Вот такие отступы в макете:

Image

А так сейчас выглядит верстка:

Image

"Потерянные" расстояния:

Image

Для form написаны стили, но не подключены:

Image

IvannaBalanyuk commented 1 month ago

Тексту Изменить данные нужно добавить стили (сейчас всё, кроме цвета, по дефолту):

Image

Сейчас вот так:

Image

А надо так:

Image

IvannaBalanyuk commented 1 month ago

Предлагаемая верстка:

Image

Image

Image

IvannaBalanyuk commented 1 month ago

В стили вносила следующие изменения (в песочнице):

Image Image Image Image

В последнем - добавила gap, чтобы отступы задавал контейнер.

IvannaBalanyuk commented 1 month ago

Про SmartHeader. У этого компонента высота на 10px больше, чем в макете, + паддинги не те. Из-за этого контент компонента расположен со смещением вверх от центра (в макете контент центрирован по вертикали) + сам блок вылезает вниз визуально (ощущение, что по задумке нижняя граница SmartHeader должна находиться на одном уровне с верхней границей цветастого блока с данными юзера):

Image

Вот так в макете:

Image

В мобильной версии проблема с высотой SmartHeader выражена ещё ярче - в макете 60px, а в верстке сейчас всё те же 100px. Скрины про мобилку - в следующем комментарии.

IvannaBalanyuk commented 1 month ago

В мобилке по верстке все те же проблемы, что и в десктопе. Дополнительно - в мобильной версии блок Изменить данные по макету находится вовне блока с данными (видно по расположению бордера). В целом медиа-запросов в стилях нет, т.е. адаптив как таковой, видимо, не реализовывался.

Так в макете:

Image

Так сверстано сейчас:

Image Image

IvannaBalanyuk commented 1 month ago

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

Image

kspshnik commented 1 month ago

У нашего тестового admin4 в permissions пусто, поэтому для проверки "выключала" roleChecker.

Кстати, здорово ещё убедиться, что usePermission() грамотно реагирует на role === UserRole.ADMIN && isRoot === true, выдавая все права, т.к. суперпользователь (root, главный администратор) по определению может всё и не видит преград :)

IvannaBalanyuk commented 1 month ago

Кстати, здорово ещё убедиться, что usePermission() грамотно реагирует на role === UserRole.ADMIN && isRoot === true, выдавая все права

Т.е. вот тут в массиве

Image

Должны быть перечислены все разрешения, предусмотренные для всемогущего? В CreateAdminDto содержатся такие:

Image

Image

Значит в массив должны попасть следующие элементы:

Image

Работа usePermission() действительно вызывает вопросы в контексте данного кейса. Хук сверяет переданный ему массив разрешений с разрешениями в user.data.permissions . И сейчас хук вернёт положительный результат в случае, если хотя бы один элемент совпадет:

Image

А к редактированию контактов можно допускать (если я теперь всё правильно поняла) только Главного администратора, а это такой, у которого есть ВСЕ разрешения.

Проверки на isRoot === true в хуке нет.

Т.е. в хук нужно добавить проверку на isRoot === true и в случае успеха возвращать isAllowed === true (нужна ли тогда сверка разрешений на предмет "есть ВСЕ"?).

kspshnik commented 1 month ago

А к редактированию контактов можно допускать (если я теперь всё правильно поняла) только Главного администратора, а это такой, у которого есть ВСЕ разрешения.

Не совсем ;) Рута допускаем ВООБЩЕ КО ВСЕМУ без дополнительных проверок, остальных - если есть права ;)

Проверки на isRoot === true в хуке нет.

Т.е. в хук нужно добавить проверку на isRoot === true и в случае успеха возвращать isAllowed === true (нужна ли тогда сверка разрешений на предмет "есть ВСЕ"?).

Да ;) И руту, ес-сно, больше ничего проверять нинада - Главный администратор может всё ;)

INextYP commented 1 month ago

Отличная работа, умничка :)