Skip to content

feat(grid): inline edit for category products table#134

Merged
biz87 merged 4 commits intobetafrom
feat/inline-edit-category-products
Mar 16, 2026
Merged

feat(grid): inline edit for category products table#134
biz87 merged 4 commits intobetafrom
feat/inline-edit-category-products

Conversation

@Ibochkarev
Copy link
Copy Markdown
Member

Описание

Реализовано быстрое редактирование данных прямо в таблице товаров категории (Category Products): по двойному клику ячейка переходит в режим редактирования (текст, число или чекбокс для boolean), сохранение по blur/Enter. Редактируемые поля и тип редактора настраиваются в «Утилиты → Поля таблицы» для грида «Товары категории». На бэкенде обновление идёт через PUT /api/mgr/product-data/{id} с белым списком полей; поле published обновляет ресурс msProduct.

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

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

Связанные Issues

Closes #116

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

  • Включение «Редактируемое поле» и выбор типа редактора (текст/число) в настройках полей таблицы для грида «Товары категории».

  • Двойной клик по ячейке — появление инпута/чекбокса, ввод значения, сохранение по blur или Enter; при отсутствии изменений запрос не отправляется и тост «Изменения сохранены» не показывается.

  • Редактирование поля published (чекбокс) — переключение публикации товара с обновлением на бэкенде (msProduct).

  • Ручное тестирование

  • Автоматические тесты (PHPStan, ESLint)

  • Тестирование на разных версиях PHP/MODX

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

  • MiniShop3: (указать при необходимости)
  • MODX: (указать при необходимости)
  • PHP: (указать при необходимости)

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

До После
Редактирование только в форме товара Редактирование в ячейке таблицы по двойному клику

Чеклист

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

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

  • Редактируемые поля задаются в «Утилиты → Поля таблицы» при выборе грида «Товары категории» (колонка «Редактируемое поле» и «Тип редактора»).
  • Поле published обновляется в ресурсе msProduct (published, publishedon, publishedby); остальные — в msProductData через существующий whitelist.

- GridFieldsConfig: editable + editor_type (text, number, boolean) for category-products
- GridConfigService: persist editable/editor_type in grid config
- CategoryProductsGrid: double-click to edit, save on blur/Enter, skip save when unchanged
- ProductDataService: allowedUpdateFields whitelist, allowedResourceFields for published
- Lexicons ru/en: field_editable, editor_type, inline_edit_*, inline_edit_hint

Fixes #116
@Ibochkarev Ibochkarev requested a review from biz87 March 9, 2026 07:42
@Ibochkarev Ibochkarev self-assigned this Mar 9, 2026
@Ibochkarev Ibochkarev marked this pull request as ready for review March 9, 2026 07:42
Copy link
Copy Markdown
Member

@biz87 biz87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: PR #134

Общее впечатление

PR хорошо структурирован. Whitelist в ProductDataService — важное улучшение безопасности (старый код передавал raw input в fromArray() без фильтрации). Lifecycle инлайн-редактирования продуман: double-click → edit → blur/Enter saves, Escape cancels, unchanged values не шлют запрос.


Баг: autofocus не работает на динамически вставленных элементах

CategoryProductsGrid.vue:1029, 1040

<InputText ... autofocus />
<InputNumber ... autofocus />

HTML-атрибут autofocus работает только при загрузке страницы. При динамической вставке элемента через v-if он не получит фокус. Пользователь двойным кликом откроет редактор, но курсор в инпуте не окажется — придётся кликать ещё раз.

Фикс: использовать Vue-директиву или ref + nextTick:

// В startInlineEdit(), после установки editingCell:
nextTick(() => {
  const input = document.querySelector('.inline-edit-cell input')
  if (input) input.focus()
})

Или кастомная директива v-focus.


Баг: двойной вызов saveInlineEdit (Enter + blur)

CategoryProductsGrid.vue:1030-1031

@blur="saveInlineEdit(product, column)"
@keydown.enter="saveInlineEdit(product, column)"

Когда пользователь нажимает Enter:

  1. @keydown.enter вызывает saveInlineEdit()clearInlineEditState()editingCell = null
  2. Vue убирает <InputText> из DOM (условие v-else-if больше не true)
  3. Удаление из DOM вызывает blur → второй вызов saveInlineEdit()

Guard в начале saveInlineEdit (if (!editingCell.value || ...)) защищает от повторного API-вызова — второй вызов вернётся рано. Но при ошибке API (catch-блок делает return без clearInlineEditState), editingCell остаётся, и blur после Enter снова попытается сохранить.

Рекомендация: в @keydown.enter сначала сделать $event.target.blur() и убрать отдельный @keydown.enter handler, либо добавить флаг isSaving чтобы исключить параллельные вызовы.


Отсутствие MODX-событий при publish/unpublish

ProductDataService.php:501-512applyPublishedToResource()

Метод напрямую меняет published, publishedon, publishedby через set() + save(). MODX-события OnDocPublished / OnDocUnPublished не вызываются. Если плагины зависят от этих событий (кеширование, индексация, уведомления), они не сработают.

Рекомендация: Использовать $product->set('published', ...) + $product->save() с ручным вызовом событий, или как минимум оставить // TODO комментарий что события пропущены осознанно.


editor_options в whitelist но не используется

GridConfigService.php:190 — ключ editor_options добавлен в $configKeys. В лексиконах есть editor_type_select и editor_options. Но в UI select-редактор не реализован — editorTypeOptions содержит только text/number.

Не баг, но мёртвый конфиг-ключ. Если select планируется позже — стоит добавить // TODO: select editor комментарий.


Нет индикации загрузки при сохранении

CategoryProductsGrid.vue:420-428saveInlineEdit() делает await request.put(...) без визуального feedback'а. При медленном соединении пользователь может подумать что ничего не происходит и кликнуть снова.

Рекомендация (не блокер): добавить кратковременный loading-индикатор или disabled-состояние на ячейку.


Что сделано хорошо

  • Whitelist $allowedUpdateFields — критическое улучшение безопасности. Старый код передавал сырой input напрямую в fromArray()
  • Разделение resource/productData полейpublished обновляется через отдельный метод с правильной обработкой publishedon/publishedby
  • isInlineValueUnchanged() — грамотная оптимизация, пропускает API-вызов и тост если значение не изменилось
  • Guard в saveInlineEdit — проверка editingCell в начале защищает от гонок
  • Лексиконы на двух языках — ru + en
  • isCategoryProductsGrid — inline edit показывается только для релевантного грида

Резюме

Замечание Серьёзность
autofocus не работает динамически Баг (UX)
Enter + blur двойной вызов (edge case при ошибке) Низкая
Нет MODX-событий при publish/unpublish Средняя
editor_options мёртвый ключ Косметика
Нет loading-индикатора Nice to have

…oductsGrid

- Added loading state to prevent multiple save requests during inline editing.
- Improved focus handling for inline edit inputs after DOM updates.
- Updated saveInlineEdit function to handle saving state and prevent double invocation.
- Adjusted UI to reflect saving state with visual cues.

This update enhances user experience by ensuring smoother inline editing interactions.
@Ibochkarev Ibochkarev requested a review from biz87 March 10, 2026 13:40
Copy link
Copy Markdown
Member

@biz87 biz87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: inline edit for category products table

+367 / -16 | 6 файлов | 2 коммита


Общая оценка

Фича добавляет инлайн-редактирование в таблице товаров категории (двойной клик по ячейке). Включает: UI в гриде, конфигурацию editable/editor_type в настройках колонок, whitelist-фильтрацию полей на бэкенде, обработку published через ивенты MODX.


1. Безопасность: whitelist — хорошо, но есть пробел

ProductDataService.php — до этого PR метод updateProductData() делал $productData->fromArray($data) без фильтрации. Теперь есть $allowedUpdateFields + array_intersect_key(). Это правильно и важно.

Однако: нет проверки прав на редактирование. Метод вызывается через /api/mgr/product-data/{id} — менеджерский API, доступ ограничен авторизацией. Но пользователь с view без save_document сможет редактировать. Стоит проверять $this->modx->hasPermission('save_document') или $product->checkPolicy('save').


2. Порядок сохранения: race condition между resource и productData

// Сначала сохраняется resource (published)
$updatedResource = $this->applyPublishedToResource($product, $published);

// Потом productData
if (!$productData->save()) {
    return null;  // resource уже сохранён!
}

Если productData->save() упадёт, published уже изменён и сохранён. Это рассинхронизация — юзер получит ошибку, но published фактически изменился. Нужно сначала сохранять productData, потом resource. Или оба в транзакции.


3. Ответ API не используется на фронте

await request.put(`/api/mgr/product-data/${product.id}`, { [column.name]: value })
product[column.name] = value  // локальное обновление нормализованным значением

Ответ сервера игнорируется. Если бэкенд трансформирует значение (округление цены, приведение типа), локальное состояние будет отличаться от серверного. Лучше:

const result = await request.put(...)
Object.assign(product, result.data)  // или хотя бы product[column.name] = result.data[column.name]

4. DOM-запрос для фокуса — хрупко

nextTick(() => {
  const input = document.querySelector('.inline-edit-cell input')
  if (input) input.focus()
})

Глобальный document.querySelector найдёт первый .inline-edit-cell input на странице. Для PrimeVue InputNumber реальный <input> может быть вложен глубже. Надёжнее — ref на компоненте:

<InputText ref="inlineInput" ... />
nextTick(() => inlineInput.value?.$el?.focus())

5. @blur на InputNumber — потенциальная проблема PrimeVue

PrimeVue InputNumber оборачивает <input> в <span>. Событие @blur на компоненте может не пробрасываться корректно с внутреннего <input>. Нужно проверить, что save действительно вызывается при клике вне ячейки. Если нет — нужен @blur с .native или обработка через onBlur prop.

Аналогично @keydown.escape — может не долетать от вложенного input до компонента.


6. editor_options и editor_type_select — мёртвый код

'editor_options', // TODO: select editor - not yet implemented in UI

Лексикон editor_type_select добавлен, editor_options сохраняется в конфиге, но UI для select-редактора нет. Это нормально для итеративной разработки, но стоит либо убрать до реализации, либо добавить issue. Сейчас это dead code + TODO в продакшн-коде.


7. Нет валидации значений на бэкенде

updateProductData() принимает данные и ставит через fromArray(). xPDO не валидирует значения — можно установить отрицательную цену, нечисловой stock и т.д. Минимальная валидация:

  • price / old_price >= 0
  • stock — integer
  • weight >= 0

8. Мелочи

  • (int)(bool)$resourceData['published'] — двойное приведение. (int)$resourceData['published'] достаточно если нужно 0/1. Или $resourceData['published'] ? 1 : 0 для ясности.
  • Лексикон inline_edit_hint — длинный текст в подсказке. Проверить что не ломает layout при узких экранах.
  • cursor: text на .editable-cell — подсказывает юзеру, но визуально не отличает editable от non-editable ячеек. Лёгкий hover-эффект (background) был бы полезнее.

Итого

Категория Оценка
Идея / UX ✅ Хорошо — инлайн-редактирование удобно
Безопасность ⚠️ Whitelist — плюс. Нет проверки permissions — минус
Надёжность ⚠️ Race condition published/productData, ответ API не используется
Код ⚠️ DOM querySelector хрупкий, PrimeVue blur под вопросом

Рекомендация: исправить пункты 2, 3, 4, добавить проверку permissions (п.1) и минимальную валидацию (п.7).

- Implemented validation for price, old_price, stock, and weight fields to ensure they meet expected criteria before updates.
- Enhanced updateProductData method to validate filtered data prior to saving.
- Updated inline editing functionality in CategoryProductsGrid to improve focus handling for input elements.
Copy link
Copy Markdown
Member

@biz87 biz87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Хорошая фича, реализация в целом рабочая. Но есть ряд проблем разной степени критичности.


Критичные проблемы

1. Контроллер не различает причины ошибки

updateProductData() возвращает null для 5 разных ситуаций: нет прав, не найден продукт, не найдены данные, не прошла валидация, ошибка save. Контроллер на всё отвечает 500 "Failed to save". Клиент не может отличить ошибку прав (403) от невалидных данных (422) или отсутствия продукта (404).

2. Валидация stock не проверяет отрицательные значения

if (isset($filtered['stock']) && (int)$filtered['stock'] != $filtered['stock']) {
    return false;
}

Проверяет только «целое ли число», но stock = -10 пройдёт валидацию. При этом price, old_price и weight проверяются на < 0.

3. Частичное обновление без отката

Если productData->save() успешен, но applyPublishedToResource() падает — данные продукта уже сохранены, а published нет. Метод вернёт null, фронтенд покажет ошибку, хотя часть данных обновлена.


Архитектурные замечания

4. Ответ API — проверить структуру

Контроллер оборачивает результат в Response::success(['updated' => true, 'data' => $result]). Фронтенд делает:

const res = await request.put(...)
if (res?.data) Object.assign(product, res.data)

Нужно убедиться, что res.data — это именно данные продукта, а не { updated: true, data: {...} }. Если обёртка лишняя, Object.assign перезапишет объект продукта полями updated и data.


Проблемы фронтенда

5. Race condition при быстром переключении ячеек

startInlineEdit не проверяет inlineEditSaving. Сценарий: blur на ячейке A запускает async-сохранение → пользователь dblclick на ячейку B → clearInlineEditState() из завершения сохранения A закроет редактирование B.

6. @keydown на InputNumber

PrimeVue InputNumber не всегда пробрасывает keydown. Нужно проверить, что Escape и Enter реально работают.

7. Object.assign(product, res.data) — мутация

Прямая мутация элемента массива через Object.assign может не вызвать реактивное обновление Vue в некоторых кейсах. Безопаснее обновлять через индекс массива.


Мелкие замечания

8. Неиспользуемые лексиконы

editor_type_select и editor_options добавлены в оба лексикона, но select-editor не реализован. Лучше не добавлять неиспользуемый код заранее.

9. isBooleanColumn — мёртвая ветка

column.editor_type === 'boolean'

В UI нельзя выбрать editor_type = 'boolean' (только text/number). Эта проверка никогда не сработает.


Резюме

Основное перед мержем:

  1. Нормальные коды ошибок из сервиса → контроллер (403/404/422)
  2. Проверить структуру ответа API на фронтенде (res.data vs res.data.data)
  3. Добавить stock < 0 в валидацию
  4. Защита от race condition при переключении ячеек
  5. Убрать неиспользуемые лексиконы

…uct updates

- Enhanced the updateProductData method to return detailed error messages and codes for various failure scenarios.
- Updated validation logic for stock to ensure it cannot be negative.
- Adjusted inline editing functionality in CategoryProductsGrid to prevent race conditions during save operations.
- Removed unused lexicon entries related to editor types in English and Russian translations.

This update improves the robustness of product data handling and user experience during inline editing.
@Ibochkarev Ibochkarev requested a review from biz87 March 16, 2026 07:23
Copy link
Copy Markdown
Member

@biz87 biz87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review (повторный)

Все 9 замечаний из первого ревью исправлены в 4-м коммите.

Что исправлено

  1. HTTP-коды ошибок — контроллер возвращает 403/404/422/500 через structured response
  2. stock < 0 — валидация добавлена
  3. Rollback при ошибке published$oldValues сохраняются, откатываются при сбое
  4. Ответ APIResponse::success($result['data']) напрямую, request.js корректно извлекает data
  5. Race conditionstartInlineEdit блокируется при inlineEditSaving
  6. InputNumber keydown — wrapper div с @keydown.enter.capture.prevent / @keydown.escape.capture.prevent
  7. Реактивное обновлениеfindIndex + spread вместо Object.assign
  8. Лексиконыeditor_type_select, editor_options удалены
  9. isBooleanColumn — только column.type === 'boolean'

Мелкое замечание (некритичное)

В updateProductData() переменная $fieldsToUpdate создаётся через array_intersect_key($filtered, array_flip(self::$allowedUpdateFields)), но $filtered уже отфильтрована тем же whitelist. Это дублирование — можно использовать $filtered напрямую.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants