Skip to content

Merge the Cart endpoints into one domain PR - #424

Open
PrestaEdit wants to merge 8 commits into
PrestaShop:devfrom
PrestaEdit:domain/cart
Open

PrestaEdit wants to merge 8 commits into
PrestaShop:devfrom
PrestaEdit:domain/cart

Conversation

@PrestaEdit

Copy link
Copy Markdown
Contributor
Questions Answers
Branch? dev
Description? Consolidates the six pending Cart PRs into one domain PR, merging the three cart-line classes and moving every assertion onto the cart details endpoint
Type? new feature
BC breaks? no
Deprecations? no
Fixed ticket? Related to PrestaShop/PrestaShop#39630
How to test? See below
Sponsor company PrestaEdit

What this PR does

Merges #299, #309, #310, #312, #313 and #318 into one PR, following the mutualisation done on the Product domain in #410.

Endpoints

Method URI CQRS Scope
GET /carts/{cartId}/details GetCartForViewing cart_read
PUT /carts/{cartId}/products AddProductToCartCommand cart_write
PUT /carts/{cartId}/products/{productId} UpdateProductQuantityInCartCommand cart_write
DELETE /carts/{cartId}/products/{productId} RemoveProductFromCartCommand cart_write
PUT /carts/{cartId}/currencies UpdateCartCurrencyCommand cart_write
PUT /carts/{cartId}/languages UpdateCartLanguageCommand cart_write
PUT /carts/{cartId}/process-order-emails SendProcessOrderEmailCommand order_write

Six resource classes become four, six test classes become one.

Design decisions

Three classes for one cart line. CartProduct (add), CartProductQuantity (update quantity) and CartProductRemoval (remove) declared identical properties, and the last two declared the same URI — /carts/{cartId}/products/{productId} — from two different resource classes. They are now one CartProduct with three operations, using explicit uriVariables so the add operation binds only cartId.

The cart line deliberately returns 204. There is no query to replay: the cart line has no read side of its own, and GetCartForViewing builds the whole CartDetails view rather than a single line. Returning that view from a CartProduct operation would mean denormalizing it into a class that does not have its fields. The result of a write is observed through GET /carts/{cartId}/details, which is what the tests now do.

Tests

Six test classes become one CartEndpointTest, and this is where the per-endpoint split had really accumulated:

  • createCart() existed in five copies — one per test class, with small differences between them. It is now a single documented helper.
  • The removal test seeded its own cart line with SQL:
    // Seed the cart line directly so the test does not depend on the add-product flow.
    \Db::getInstance()->insert('cart_product', [...]);
    That reasoning made sense for a standalone PR and makes none for a domain PR. Add → change quantity → remove is now one chained lifecycle test going through the API at every step.
  • Assertions moved off raw SQL and off the legacy object:
Was Now
SELECT quantity FROM ps_cart_product WHERE ... the cart_quantity of cartSummary.products in the details
SELECT COUNT(*) FROM ps_cart_product WHERE ... after removal the same products array, now empty
(new \Cart($cartId))->id_currency cartCurrencyId in the details

On fixtures: carts are created by the front office and by the order-creation flow — the Cart domain exposes no "add cart" command, so createCart() still uses the legacy object. Two SQL lookups also remain and are marked as such in the code: finding an active, in-stock, combination-less product (the products listing cannot express that filter), and finding a second installed currency (the Currency domain has no listing endpoint on dev). Neither creates anything.

testUpdateCartLanguage only asserts the command is accepted: GetCartForViewing does not expose the cart language, so that write has no read side to check against.

One question for the reviewers

PUT /carts/{cartId}/process-order-emails declares the order_write scope, while every other /carts/... endpoint — including the already merged CartEmail (#314) — uses cart_write. #318 is approved as it stands so the scope is left untouched here, but if that was an oversight rather than a deliberate choice, this is the PR to fix it in.

How to test

GET    /carts/{id}/details                -> 200 {cartId, cartCurrencyId, customerInformation, orderInformation, cartSummary}
PUT    /carts/{id}/products                -> 204, then the details list the product
PUT    /carts/{id}/products/{productId}    -> 204, then the details show the new quantity
DELETE /carts/{id}/products/{productId}    -> 204, then the details list no product
PUT    /carts/{id}/currencies              -> 204, then the details show the new cartCurrencyId
PUT    /carts/{id}/languages               -> 204
PUT    /carts/{id}/process-order-emails    -> 204   (422 on cartId=0)

Covered by CartEndpointTest.

Supersedes

All six will be closed once the CI is green here.

PrestaEdit and others added 8 commits August 20, 2026 11:00
Expose GetCartForViewing as GET /carts/{cartId}/details (scope cart_read).
Mirrors the CustomerDetails heavy-aggregate read: the CartView result
(customer information, order information, cart summary, currency) is mapped to
array/scalar properties. 404 maps from CartNotFoundException.

Related to PrestaShop/PrestaShop#39630

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fill two Cart update commands not covered by the cart resource:
- PUT /carts/{cartId}/currencies (UpdateCartCurrencyCommand)
- PUT /carts/{cartId}/languages (UpdateCartLanguageCommand)

Both scoped cart_write. Missing carts map to 404. Standalone resource file.

Related to PrestaShop/PrestaShop#39630

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fill the AddProductToCartCommand gap: PUT /carts/{cartId}/products (scope cart_write)
to add a product (optionally a combination) with a quantity to a cart.

Related to PrestaShop/PrestaShop#39630

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fill the RemoveProductFromCartCommand gap: DELETE /carts/{cartId}/products/{productId}
(scope cart_write) to remove a product line from a cart.

Related to PrestaShop/PrestaShop#39630

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fill the UpdateProductQuantityInCartCommand gap: PUT /carts/{cartId}/products/{productId}
(scope cart_write) to set the quantity of a product in a cart.

Related to PrestaShop/PrestaShop#39630

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fill the SendProcessOrderEmailCommand gap: PUT /carts/{cartId}/process-order-emails
(scope order_write) to send the process-order email to the cart customer. The test
disables real mail sending (PS_MAIL_METHOD) so Mail::send() succeeds without SMTP.

Related to PrestaShop/PrestaShop#39630

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add CartConstraintException => HTTP_UNPROCESSABLE_ENTITY to
  exceptionToStatus. cartId=0 satisfies the URI \d+ requirement but
  `new CartId(0)` throws CartConstraintException, which used to fall
  through to a 500. It now surfaces as 422.
- Add testSendProcessOrderEmailWithInvalidCartIdReturns422 covering
  that case.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Consolidates PrestaShop#299, PrestaShop#309, PrestaShop#310, PrestaShop#312, PrestaShop#313 and PrestaShop#318. Six resource classes become
four, six test classes become one.

- CartProduct (add), CartProductQuantity (update) and CartProductRemoval (remove)
  all described the same structure, and the last two even declared the same URI,
  /carts/{cartId}/products/{productId}, from two different classes. They are now
  one CartProduct resource with three operations and explicit uriVariables
- the product operations keep returning 204: the cart line has no read side of its
  own, the cart is read through GetCartForViewing which builds the whole CartDetails
  view, so there is no query to replay. The result is observed through
  GET /carts/{cartId}/details

Tests, where the split cost the most:

- createCart() existed in FIVE copies. It is now one documented helper: carts are
  created by the front office and the order-creation flow, the Cart domain has no
  add command, so it is the only fixture that cannot come from the API
- the removal test seeded its cart line with an INSERT INTO ps_cart_product "so the
  test does not depend on the add-product flow". Add, re-quantify and remove are now
  chained through the API in one lifecycle test
- every assertion moved off raw SQL and off the legacy object:
  SELECT quantity FROM ps_cart_product and (new Cart($id))->id_currency are replaced
  by the products and cartCurrencyId of the cart details

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#[ApiResource(
operations: [
new CQRSGet(
uriTemplate: '/carts/{cartId}/details',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be

Suggested change
uriTemplate: '/carts/{cartId}/details',
uriTemplate: '/carts/{cartId}',

as on the other domains

Comment on lines +59 to +61
public const QUERY_MAPPING = [
'[cartId]' => '[cartId]',
];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can be removed as the mapping has the same origin and destination properties

),
],
exceptionToStatus: [
CartNotFoundException::class => Response::HTTP_NOT_FOUND,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To be added: CartConstraintException

Comment on lines +140 to +145
$this->updateItem(
'/carts/' . $cartId . '/products',
['productId' => $productId, 'quantity' => 2],
['cart_write'],
Response::HTTP_NO_CONTENT
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you make this call twice (eventually with different quantities, and update the expected result? It is supposed to show the quantity is added.

Comment on lines +125 to +128
public function testGetNonExistentCartDetails(): void
{
$this->requestApi('GET', '/carts/999999/details', null, ['cart_read'], Response::HTTP_NOT_FOUND);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add a test with "/carts/0/details" ?

@ps-jarvis ps-jarvis moved this from Ready for review to Waiting for author in PR Dashboard Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Waiting for author

Development

Successfully merging this pull request may close these issues.

3 participants