Respect resource groups in Fenom storefront snippets

[Ibochkarev]

Описание

Классическая Fenom-витрина (ms3_products и смежные сниппеты) не учитывала ACL групп ресурсов MODX. Условие видимости берётся из того же CatalogResourceGroupVisibility, что и публичный Web API (#666). В ms3_products фрагмент кладётся в INNER JOIN … ON, а не в $where[], чтобы pdoTools additionalConditions() не глушил &resources / &context.

Depends on #666. После merge #666 — retarget на beta.

Настройка: переиспользуем ms3_web_catalog_respect_resource_groups (без переименования). Листинг для Fenom — anonymous-safe (как Web API MVP); видимость для участников групп — #669.

Тип изменений

• Исправление бага (non-breaking change)
• Новая функциональность (non-breaking change)
• Breaking change (изменение, ломающее обратную совместимость)
• Рефакторинг (без изменения функциональности)
• Документация
• Другое (опишите):

Связанные Issues

Closes #670

Как это было протестировано?

php -l core/components/minishop3/src/Services/Catalog/CatalogResourceGroupVisibility.php
# exit 0
php -l core/components/minishop3/elements/snippets/ms3_products.php
# exit 0
php core/components/minishop3/tests/CatalogResourceGroupVisibilityTest.php
# EXIT:0 — OK CatalogResourceGroupVisibilityTest
cd core/components/minishop3 && ./vendor/bin/phpunit tests/Unit/Catalog/CatalogResourceGroupVisibilitySqlTest.php
# EXIT:0 — 8 tests, 13 assertions

• Ручное тестирование
• Автоматические тесты (composer ci:php / composer test, npm run lint:ci, composer stan / GitHub Actions CI)
• Тестирование на разных версиях PHP/MODX

Конфигурация тестирования:

• MiniShop3: ветка PR (поверх feat(web-api): учитывать ACL групп ресурсов в публичном каталоге #666)
• MODX: n/a (smoke/SQLite unit)
• PHP: локальный CLI 8.4

Скриншоты (если применимо)

До	После

Чеклист

• Код соответствует стилю проекта
• Добавлены/обновлены комментарии в сложных местах
• Изменения не ломают существующую функциональность
• Лексиконы добавлены на двух языках (ru/en)
• PHPStan проходит без новых ошибок (composer stan / CI job PHPStan)
• ESLint проходит без ошибок (npm run lint:ci для Vue)
• Обновлён CHANGELOG.md (для значимых изменений)

Дополнительные заметки

• Аудит: ms3_gallery / ms3_options / ms3_product_options закрыты через isVisible() при &product=<id>. ms3_cart / order-сниппеты — вне scope (данные сессии; вход в корзину через CartItemManager::validateProduct → apply(), как в feat(web-api): учитывать ACL групп ресурсов в публичном каталоге #666; feat(web-api): msCustomerGroup и ACL групп ресурсов для авторизованных #677 поднимает до applyForRequest()).
• pdoResources / pdoMenu для категорий — вне компонента MiniShop3.
• Суффикс _rg… к cacheKey убран: Fetch::run() его не читает. Инвалидация штатного ресурсного кэша MODX при смене ACL — отдельная задача, здесь не решена.
• Порядок мержа с feat(web-api): msCustomerGroup и ACL групп ресурсов для авторизованных #677: сначала feat(web-api): учитывать ACL групп ресурсов в публичном каталоге #666 → Respect resource groups in Fenom storefront snippets #681 (Fenom + buildWhereFragment/isVisible) → feat(web-api): msCustomerGroup и ACL групп ресурсов для авторизованных #677 (расширяет apply до request-aware API).

[biz87]

Реализация сделана аккуратно, и переиспользование подтверждено не на словах: apply() из #666 теперь делегирует в новый buildWhereFragment(), а тест проверяет побайтовое совпадение SQL из обоих путей. Второй реализации не возникло — это был главный критерий приёмки #670.

Охват четырёх сниппетов обоснован, в том числе неочевидной деталью. У ms3_gallery, ms3_options и ms3_product_options гейт стоит только когда товар передан явным &product=<id>; когда товар — текущий ресурс, проверки нет. Это правильно: ядро само зовёт checkPolicy('view') и отдаёт sendUnauthorizedPage() ещё до рендера сниппетов, дублировать незачем.

INNER JOIN вместо where[] — тоже не произвол, а обход бага Fetch::additionalConditions(), где численный ключ с SQL-строкой глушит параметры &resources/&context. Покрыто тестом.

Числа: мерж с beta чистый, smoke 100/100, PHPUnit 322 / 1397 assertions, PHPStan чисто.

Но две вещи нужно решить.

Суффикс кэша не работает — его никто не читает

ms3_products.php добавляет к scriptProperties['cacheKey']/['cache_key'] суффикс _rg{0|1}_{context}. Коммит называется «Extend pdoTools cache keys with RG+context», в описании PR это записано как закрытый пункт.

Проверил: ModxPro\PdoTools\Fetch::run() не вызывает ни getCache(), ни setCache() — в src/Fetch.php ноль таких вызовов. Кэш выборки в pdoTools задействован только в snippet.pdomenu.php, snippet.pdopage.php и snippet.pdositemap.php. А ms3_products.php работает именно через Fetch::run().

То есть добавленный суффикс — мёртвый код. Он ни на что не влияет.

Это хуже, чем отсутствие фикса: следующий читатель увидит «ключ кэша учитывает видимость» и будет на это полагаться.

При этом реальный риск устаревания есть, только другой — штатный ресурсный кэш MODX. По документации некэшируемый вызов через ! обязателен только для сессионных сниппетов; ms3_products можно звать кэшируемо. Значит закэшированная страница может отдавать открытый листинг после того, как товар закрыли, — особенно если ACL правили через Безопасность → Права доступа, а не пересохранением ресурса.

Прошу либо убрать мёртвый суффикс и формулировки о нём, честно записав, что этот пункт не решён, либо решить его по-настоящему — тогда речь про инвалидацию ресурсного кэша при изменении ACL, и это отдельная работа, скорее следующим PR.

Конфликт с #677 — семантический, не механический

Вы оба стоите на ветке #666 и по-разному переписываете один и тот же метод CartItemManager::validateProduct():

• Respect resource groups in Fenom storefront snippets #681 переводит его на isVisible($productId) — новый метод, который сам документирован как anonymous-only и не принимает список разрешённых групп;
• feat(web-api): msCustomerGroup и ACL групп ресурсов для авторизованных #677 переводит на applyForRequest(), который как раз учитывает группу авторизованного покупателя.

Взять одну версию целиком нельзя ни в каком порядке. Если победит isVisible() — авторизованный покупатель увидит товар в каталоге, но не сможет положить его в корзину. Если победит только правка #677 — потеряется то, ради чего #681 трогал этот метод.

То же касается самого CatalogResourceGroupVisibility: #681 добавляет buildWhereFragment() и isVisible(), #677 добавляет четвёртый параметр к apply() плюс applyForRequest() и buildVisibilitySql(). Написаны они независимо, друг про друга не знают.

Нужно решение, как свести apply(), applyForRequest(), buildWhereFragment() и isVisible() в одну согласованную точку входа, и в каком порядке мержить пару. Сам не резолвлю — это решение о поведении.

И третье: база тоже уходит на доработку

В #666 нашёлся баг: ресурс, состоящий в нескольких группах ресурсов, скрывается, даже если одна из групп открыта анониму. Ваш buildWhereFragment() оборачивает именно этот SQL, так что после исправления базы этот PR потребует ребейза в любом случае.

Подробности в комментарии к #666.

[Ibochkarev]

Ответ на ревью — 901fc0c7 (ветка ребейзнута на #666 с multi-group SQL).

1. Суффикс кэша

Убран. Fetch::run() действительно не читает cacheKey/cache_key. Формулировки про «pdoTools cache key» из PR убраны. Риск устаревания через штатный ресурсный кэш MODX при правке ACL без пересохранения ресурса не закрыт — это отдельная работа (инвалидация), не в этом PR.

2. Конфликт с #677

Решение по точкам входа:

API	Роль в #681	#677
buildWhereFragment / apply	общий SQL (anonymous)	расширяет apply до allowed ids + applyForRequest
isVisible	Fenom &product=<id> (gallery/options)	можно позже принять allowed ids, сейчас anonymous-safe ок
CartItemManager::validateProduct	снова на apply() (как #666), не isVisible	однострочная замена на applyForRequest()

Порядок мержа: #666 → #681 → #677.

3. База #666

Ребейз на 8bd5f3e8 (document-level OR). SQL-тесты: 8/13, including multi-group.

php tests/CatalogResourceGroupVisibilityTest.php  # OK
vendor/bin/phpunit tests/Unit/Catalog/CatalogResourceGroupVisibilitySqlTest.php  # 8 tests, 13 assertions