Add root category and position endpoints to the Category domain

[mattgoud]

Questions	Answers
Branch?	dev
Description?	Exposes the three missing Category CQRS operations in the Admin API: AddRootCategoryCommand (POST /categories/roots), EditRootCategoryCommand (PATCH /categories/roots/{categoryId}) and UpdateCategoryPositionCommand (PUT /categories/{categoryId}/positions), all under the category_write scope.
Type?	improvement
BC breaks?	no
Deprecations?	no
Fixed ticket?	Fixes PrestaShop/PrestaShop#41973
How to test?	Run CategoryEndpointTest. testAddRootCategory / testEditRootCategory create and edit a root category via /categories/roots; testEditUnknownRootCategoryReturnsNotFound asserts the 404 mapping; testUpdateCategoryPosition reorders sibling categories via /categories/{categoryId}/positions.
Possible impacts?	New endpoints only; no change to existing operations.
Sponsor company?	@PrestaShopCorp

Notes:

• The root add/edit operations live on the existing Category resource (same GetCategoryForEditing output and field set) with dedicated command mappings, because AddRootCategoryCommand takes name/linkRewrite constructor arguments and has no parent category (unlike AddCategoryCommand).
• The position operation is modelled as a CQRSUpdate action (read: false, output: false → 204). Categories have no grid position definition (they use a tree), so the raw UpdateCategoryPositionCommand fields (parentCategoryId, way, positions, foundFirst) are exposed directly, matching the legacy updatePositionAction. The reordering behaviour itself is covered by the core Behat scenarios in category_management.feature.
• Add category update-position endpoint #331 added the same PUT /categories/{categoryId}/positions endpoint (same command/scope/resource) and is a duplicate of this PR's position operation; it should be closed in favour of this one.

[github-actions]

🤖 Claude AI Pre-Review — Automated analysis. Does not replace human review.

📋 Summary of changes

This PR exposes three previously missing Category CQRS operations in the Admin API: POST /categories/roots (via AddRootCategoryCommand), PATCH /categories/roots/{categoryId} (via EditRootCategoryCommand), and PUT /categories/{categoryId}/positions (via UpdateCategoryPositionCommand, 204 No Content). The first two operations are added to the existing Category ApiResource class with dedicated command mappings, while the position update lives in a new CategoryPosition class. Integration tests are added to the existing CategoryEndpointTest.

⏱️ Estimated review time

8–12 minutes — three new operations, one new resource file, and several new test methods to read through; no architectural novelty but some CQRS mapping subtleties to verify against Core commands.

🎯 Scope

• Exposed operations: POST, PATCH (root category), PUT (position)
• CQRS entity: AddRootCategoryCommand, EditRootCategoryCommand, UpdateCategoryPositionCommand, GetCategoryForEditing
• Integration test: partial (happy-path only; see issues below)

🧱 API Platform / CQRS architecture compliance

Root-category operations (Category.php)

ADD_ROOT_COMMAND_MAPPING vs EDIT_ROOT_COMMAND_MAPPING — command parameter name for enabled

Both new mappings use '[enabled]' => '[isActive]', but the existing COMMAND_MAPPING (for AddCategoryCommand / EditCategoryCommand) uses '[enabled]' => '[isEnabled]'. The PR description explains the different constructor signatures for Add/Edit Root commands, and the difference for the name/linkRewrite vs localizedNames/localizedLinkRewrites parameters is clearly documented. However, the isActive vs isEnabled discrepancy for the same boolean concept needs a human reviewer to verify it against AddRootCategoryCommand and EditRootCategoryCommand in Core — if those constructors actually expose isEnabled, the mapping is wrong and the enabled field will silently be ignored.

ADD_ROOT_COMMAND_MAPPING is missing parentCategoryId

Correct and intentional: root categories have no parent. No issue.

validationContext uses IframeValidationGroupsResolver

Consistent with the existing CQRSCreate and CQRSPartialUpdate for regular categories. This diverges from the standard ['groups' => ['Default', 'Create']] pattern documented in CONTEXT.md but is acceptable here since it follows the pre-existing pattern of this specific resource.

exceptionToStatus reuse

Root-category operations inherit the class-level exceptionToStatus (CategoryConstraintException → 422, CategoryNotFoundException → 404). Correct.

---

Position endpoint (CategoryPosition.php)

exceptionToStatus uses the base CategoryException instead of CategoryConstraintException

The existing Category class maps CategoryConstraintException::class → 422. Here CategoryException::class → 422 is used instead — this is broader (catches all CategoryException subclasses that are not explicitly listed above). Because CategoryNotFoundException is listed first, the 404 mapping is not affected, but any other non-constraint subclass of CategoryException would also silently map to 422 rather than surfacing as a 500. Recommend using CategoryConstraintException for consistency and safety.

foundFirst property — possible leaky abstraction

public bool $foundFirst mirrors the raw parameter from the legacy CategoryController::updatePositionAction. Exposing implementation details of the legacy controller through the REST API couples API clients to the internal tree-reorder algorithm. Human reviewers should decide whether this parameter belongs in the public API contract or whether the Core command should abstract it.

No CQRSCommandMapping needed

Correct — the DTO property names match the UpdateCategoryPositionCommand constructor parameters as stated in the PR description.

positions property format — the entry prefix is tr_ in the test, but the docblock says {rowId}_{parentCategoryId}_{categoryId}

The test sends 'tr_' . $parentId . '_' . $b while the docblock documents the format as 0_2_5. The tr_ prefix is the id attribute of a <tr> element in the legacy backoffice tree grid; the Core handler strips the prefix. The docblock should clarify this (e.g., note that any non-numeric prefix is ignored).

💡 Improvement suggestions

1. Verify isActive vs isEnabled in AddRootCategoryCommand / EditRootCategoryCommand — Check the constructor parameter name in Core. If the root commands also use isEnabled, change ADD_ROOT_COMMAND_MAPPING and EDIT_ROOT_COMMAND_MAPPING accordingly; if they truly use isActive, add a comment explaining the discrepancy.

2. Use CategoryConstraintException (not CategoryException) in CategoryPosition::exceptionToStatus — Aligns with the existing Category class and avoids accidentally mapping unexpected domain exceptions to 422.

3. testAddRootCategory: assert all response fields — Currently only categoryId, names, and linkRewrites are checked. Add assertions for at minimum enabled and shopIds (the fields explicitly provided in $postData). Per CONTEXT.md, integration tests should assert the full response contract.

4. Add testInvalidAddRootCategory and testInvalidPatchRootCategory — CONTEXT.md requires a testInvalid* method with assertValidationErrors for every write endpoint. A minimal test for each (e.g., posting without the required names key) would cover the 422 path.

5. testUpdateCategoryPosition: send enabled not isActive — The test creates helper categories via POST /categories and passes 'isActive' => true. That field is silently ignored by the API (the DTO property is $enabled). While this pre-existing pattern already exists in createTemporaryCategories(), the new helper call should use the correct field name 'enabled' => true to avoid perpetuating the inconsistency.

6. testUpdateCategoryPosition: assert the resulting order — The test only verifies the 204 status. A follow-up listItems or two getItem calls checking position (or parentCategoryId) would make the test contract-complete.

7. Clarify the positions entry format in CategoryPosition docblock — Document that the prefix (tr_, 0_, etc.) is stripped by the Core handler and only the trailing _{parentId}_{categoryId} portion matters, so API clients know they can pass any non-numeric prefix.

✅ Pre-review checklist

URI & routing

• URI is plural, lowercase, kebab-case (/categories/roots, /categories/{categoryId}/positions)
• Identifier uses domain name + Id suffix (categoryId)
• Sub-resources follow parent path
• Bulk operation URI uses bulk- prefix and plural Ids parameter — N/A (no bulk operations)

Operations & scopes

• Correct operation attribute per HTTP method (POST→CQRSCreate, PATCH→CQRSPartialUpdate, PUT→CQRSUpdate)
• Scope format: category_write (singular snake_case)

API Resource properties

• All properties strictly typed, scalars/arrays only (no Value Objects)
• Naming conventions respected ($enabled, no is prefix, no localized prefix)
• #[ApiProperty(identifier: true)] on ID property (CategoryPosition.$categoryId)
• #[LocalizedValue] on localized fields — inherited from existing Category DTO

CQRS mapping

• QUERY_MAPPING direction: QueryResult field → API field (e.g. [id] → [categoryId])
• CQRSCommandMapping direction: API field → command parameter
• CQRSQuery present on both new CQRSCreate and CQRSPartialUpdate
• No SerializedName — mappings only

Forbidden practices (CI-enforced)

• No custom normalizers or processors
• No Value Objects in properties

Exception handling & validation

• ConstraintException → 422: CategoryConstraintException on Category; CategoryException on CategoryPosition (broader — see suggestions)
• NotFoundException → 404
• validationContext uses IframeValidationGroupsResolver on Create/Update (consistent with existing Category class)

Multi-shop

• shopIds present in Category DTO and mapped in ADD_ROOT_COMMAND_MAPPING / EDIT_ROOT_COMMAND_MAPPING

Integration test

• Extends ApiTestCase
• @depends chain: testEditRootCategory → testAddRootCategory
• Asserts all response fields — testAddRootCategory only checks categoryId, names, linkRewrites; missing enabled, shopIds, etc.
• testInvalid* with assertValidationErrors — missing for both new root category endpoints
• getProtectedEndpoints() lists all three new URIs
• DatabaseDump::restoreTables() covers all affected tables (already includes category, category_lang, category_group, category_shop, category_product)
• declare(strict_types=1) present

[mattgoud]

Thanks for the review. Triage of the points:

Applied (e43bdf3):

• testAddRootCategory now also asserts enabled.
• Added testInvalidUpdateCategoryPosition — sends a positions list that does not contain the moved category, which makes the handler throw CategoryException → asserts 422, so the exception mapping is now covered.
• Clarified the positions docblock (the leading row-id segment is a legacy grid artifact and is ignored, any prefix works).
• Added a comment on ADD_ROOT_COMMAND_MAPPING explaining the enabled → isActive mapping.

Verified, not a bug — enabled → isActive:
AddRootCategoryCommand(array $name, array $linkRewrite, $isActive) and EditRootCategoryCommand::setIsActive() both use isActive, so the mapping is correct. The [enabled] => [isEnabled] in the regular COMMAND_MAPPING is actually a no-op on those commands (they expose isActive, not isEnabled), which is why the regular create relies on sending isActive directly. The green testAddRootCategory (which sends enabled: true and creates an active category) confirms the mapping end-to-end.

Not applied, with reasons:

• Suggestion First endpoint for API Access #2 (use CategoryConstraintException instead of CategoryException on the position endpoint): the position handler throws the base CategoryException (UpdateCategoryPositionHandler: throw new CategoryException('Category position cannot be updated')), not CategoryConstraintException. Mapping CategoryException → 422 is required here; switching to CategoryConstraintException would turn that failure into a 500. Kept as is.
• Suggestion feat(api): add delete api access endpoint #5 (send enabled instead of isActive in the position test helper): AddCategoryCommand's constructor argument is isActive; enabled only maps to isEnabled, which that command does not expose. Sending enabled would leave isActive unset and break creation. isActive is the correct field (matching the pre-existing createTemporaryCategories).
• Suggestion Add create and update endpoints for ApiAccess API #6 (assert the resulting order after reorder): intentionally omitted — the position is not returned by the API and asserting it reliably is entangled with the multishop category_shop position mechanism. The reordering behaviour itself is covered by the core Behat scenarios in category_management.feature; the endpoint here is covered by the happy-path (204) and the new 422 test.