Этап
Готова
Тип
Рефакторнинг
Приоритет
Обычный
Трудоемкость
Обычная
Создана
Дата создания
4 года назад
Назначена
Обновлена
2 года назад

Это начиная с 6.2 стандартный механизм, который странно не использовать. Кроме того часто загружаемая контроллером основная сущность участвует в определении полномочий, и получается, что её сначала грузит по аннотации IsGranted, а потом мы грузим повторно и повторно проверяем. Тогда если сущности нет ругается isGranted, а не наша проверка, которая может выдать более понятную и правильную ошибку.

Когда переделаем на ParamConverter, он, я надеюсь окажется в Request и тогда сборщики меню или хлебных крошек смогу не получать его из репозитория

Здесь же создать аннотацию @NeedProjectContext проверяющую наличие контекста проекта. Чтобы не догадываться почему не сработало полномочие или неявно запретило действие. уже сделали

Панель управления

Комментарии могут оставлять только авторизованные пользователи
 demius 2 года назад

И, вроде бы на этом все. Ну, стоит посмотреть что там с UserController но там вроде это не критично.

 demius 2 года назад

В итоге реализовали вариант 3

 demius 2 года назад

Решение 3: совсем не явное

  • Делаем, декоратор к новому Symfony\Bridge\Doctrine\ArgumentResolver\EntityValueResolver или переопределяем его, добавляя ссылку на сущность в реквест.

и тут в реквест может приехать много лишних сущностей, которые не то что бы там нужны. (хотя с другой стороны, раз уж мы их в контроллере уже зарезолвили, почему нет?). Вариант решения в https://github.com/symfony/symfony/issues/48935#issuecomment-1377942737

 demius 2 года назад

Решение 2: менее явное

  • расширяем InProjectContextRequestSubscriber (или делаем аналогичные), так чтобы они клали не только проект, но и текущую сущность по разделам, будь то Task, Doc или что еще

и нам нужно будет помнить, какие автоматические найденные в контроллере сущности были загружены в request, а какие нет

 demius 2 года назад

Решение 1: явное и многословное - делаем наследников базовых евентов на те, кто умеют рисовать меню в таких разделах, например для страниц внутри задачи BreadcrumbTaskEvent extend BreadcrumbEvent - делаем базовые шаблоны для таких разделов и в них переопределяем блок хлебных крошек тем, которые вызовут евент с этим объектом {% block breadcrumbs %}{{ breadcrumbs(task) }}{% endblock %} - в слушателях делаем instanceof и забираем task из шаблона.

 demius 2 года назад

Итак. Начиная с Symfony 6.2 @ParamConverter как и весь бандл, который его предоставляет объявлен deprecated. Вместо него введен #[MapEntity]. Все хорошо, он работает из коробки, даже без указание, через autowiring, но он не кладет полученную сущность в request. И если #[IsGranted] берет её из атрибутов метода, то листенеры собирающие меню так не могут.

Обсуждение проблемы: https://github.com/symfony/symfony/issues/48935

 demius 2 года назад

По идее добавить-то весьма не сложно и не долго. Если оно запустится сразу, из коробки, давайте сделаем вместе с tndt-141, чтобы там не делать лишний вызов к репозиторию. Если то оставим на другой релиз

 demius 2 года назад

А может и не стоит прятать получение сущности в аннотации, хотя это может быть нужно проверки полномочий. Сейчас проверке нужен максимум проект, в котором работает пользователь, но какому-нибудь будущему полномочию PERM_REOPEN_CLOSED_TASK понадобиться и сама задача, так что перед tndt-131 надо бы реализовать.

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

 demius 3 года назад

Это что же, для каждой сущности делать свой листенер загружающей её в request? Или это касается только основных проектных Task, Doc и всю тройку грузить в в одном листенере?

 demius 3 года назад

Надо посмотреть, актуально ли еще

 demius 4 года назад

Перевод загрузки основной сущности, давай-те уже все же здесь доделаем, ибо и так релиз задерживаем.

Для задач как минимум стоит использовать @Entity("task", expr="repository.findByTaskId(taskId)") и да, TaskRepository::getByTaskId() стоит переименовать в findByTaskId(), ведь она вполне может и не найти, и для репозитория это не исключительная ситуация, семантически это поиск с неясным результатом.

 demius 4 года назад

NeedProjectContext уже реализовали в рамках tndt-3 как аннотацию @InProjectContext, так как без неё не было видно, что система ошибок работает с нашими исключениями