Skip to content

Reject non-positive cart item quantities in REST API - #41430

Open
shin4141 wants to merge 6 commits into
magento:2.4-developfrom
shin4141:codex/reject-nonpositive-rest-cart-qty
Open

shin4141 wants to merge 6 commits into
magento:2.4-developfrom
shin4141:codex/reject-nonpositive-rest-cart-qty

Conversation

@shin4141

@shin4141 shin4141 commented Oct 5, 2026 •

Copy link
Copy Markdown

Description (*)

REST cart item input is validated before Quote\Item::setQty() can normalize a non-positive value to 1. This keeps the existing quote item behavior used by other cart flows and makes invalid REST quantities return a validation error.

The new REST functional cases cover guest cart POST with 0 and -2, and PUT with 0 after a successful update to quantity 3. The PUT case verifies the stored quantity remains 3.

Related Pull Requests

None.

Fixed Issues (if relevant)

  1. Fixes REST API: cart item with qty 0 or negative returns 200 and qty is set to 1 #41429

Manual testing scenarios (*)

  1. Create a guest cart and POST an in-stock item with qty: 0. Expect HTTP 400 with qty identified as invalid, and no item added.
  2. Repeat with qty: -2. Expect the same result.
  3. Add an item with qty: 3, then PUT that item with qty: 0. Expect HTTP 400; GET the cart items and confirm the quantity remains 3.
  4. POST or PUT a positive quantity. Expect the normal successful response.

Tests

  • Vanilla 2.4-develop at 5b3d233df732d5c573cb93e78576b9b86278573d: before the change, all three invalid requests returned HTTP 200 and saved quantity 1. After the change, all three return HTTP 400; the rejected PUT leaves quantity 3.
  • New REST functional cases: 3 passed, 12 assertions. Existing positive add case passed.
  • Validator unit tests: 6 passed. Quote Item unit tests: 42 passed.
  • CartItemPersister integration tests: 6 passed. Checkout cart integration tests: 6 passed. Selected admin order integration tests: 2 passed. Multishipping integration tests: 9 passed.
  • Full GuestCartItemRepositoryTest.php: 19 of 20 passed locally. The remaining stock fixture case expects There are no source items with the in stock status, while this local vanilla installation returns Product Simple Product One is out of stock. for its positive quantity request. This case does not exercise the new non-positive quantity validation. CI remains the authoritative check for that environment difference.

Questions or comments

This addresses the reported S3 functional inconsistency. It does not change Quote\Item::_prepareQty() or global quote item quantity semantics.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit and API functional tests
  • No module README section needs updating
  • All automated tests passed successfully (CI pending; local stock fixture message mismatch noted above)

@m2-assistant

m2-assistant Bot commented Oct 5, 2026

Copy link
Copy Markdown

Hi @shin4141. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:
  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.


For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

@engcom-Bravo engcom-Bravo added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Oct 6, 2026
@ilnytskyi

Copy link
Copy Markdown
Member

@shin4141 Thanks for your contribution. Please sign the [Adobe CLA] as required by the contribution guidelines.

*/
public function validateEntityValue(object $entity, string $propertyName, $value): void
{
if ($entity instanceof CartItemInterface

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.

it's probably safer to check item against \Magento\Quote\Model\Quote\Item\AbstractItem as multishipping carts may accept address items that do not implement CartItemInterface

@shin4141

Copy link
Copy Markdown
Author

Thanks for pointing out the address item type. The proposed follow-up checks both CartItemInterface and AbstractItem, so quote address items are covered while implementations of CartItemInterface remain covered. It adds unit cases for address-item quantities of 0, -2, a nonnumeric value, and positive values. The validator tests passed locally (11/11); the related quote item (42/42), multishipping checkout (13/13), multishipping repository plugin (2/2), and guest cart repository (3/3) unit tests also passed. These new cases exercise the validator directly. I have not established a standard REST input route that constructs an address item, and I have not rerun the HTTP or database suites for this follow-up.

@ilnytskyi

Copy link
Copy Markdown
Member

@magento run all tests

<type name="Magento\Framework\Webapi\Validator\CompositeServiceInputValidator">
<arguments>
<argument name="validators" xsi:type="array">
<item name="cartItemQtyValidator" xsi:type="object">Magento\Quote\Model\Webapi\CartItemQtyValidator</item>

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 area config will remove validators registered by global di.xml

worth take that into account as was done in this PR: https://github.com/magento/magento2/pull/40742/changes where all validators were moved to module level global area

sortOrder="100" />
</type>
<type name="Magento\Framework\Webapi\Validator\CompositeServiceInputValidator">
<plugin name="cartItemQtyValidator" type="Magento\Quote\Model\Webapi\CartItemQtyValidator" />

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 clarify my earlier comment, I meant following the relocation used in merged #40742, rather than introducing a plugin around the composite.

Please move the existing entityArrayValidator and searchCriteriaValidator registrations from app/etc/di.xml into Magento_Webapi/etc/di.xml

Then register cartItemQtyValidator through the validators argument in Magento_Quote/etc/di.xml.

All entries would then merge at the module-global stage.

* @return void
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function beforeValidateEntityValue(

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.

Please remove the plugin registration and beforeValidateEntityValue(), keeping the class as a validator implementation as mentioned in previous comment.

@ilnytskyi

Copy link
Copy Markdown
Member

@magento run all tests

@ilnytskyi

Copy link
Copy Markdown
Member

@magento run WebAPI Tests

@ilnytskyi ilnytskyi left a comment •

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.

The fix rejects non-positive quantities

Baseline a385d02 / PR changes 6ea04c8, independently installed

With a salable simple product and a fresh guest cart, POST quantities 0 and -2, then check persisted items; use a separate positive request as control:

Supporting valid-cart check Baseline PR changes
POST 0 / -2 Accepted; saved qty 1 Quantity error; no item
POST 3 Saved qty 3 Saved qty 3

PR regression results:

  • Magento\Quote\Api\GuestCartItemRepositoryTest::testUpdateItemRejectsZeroQtyWithoutChangingExistingItem: baseline FAIL because no validation error occurs; PR PASS, preserving qty 3.
  • Magento\Quote\Api\GuestCartItemRepositoryTest::testAddItemRejectsNonPositiveQty, datasets zero / negative: baseline FAIL because cart-not-found lacks qty; PR PASS (2 cases, 6 assertions).

The PR's composite checks also pass, including the existing input limits.

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

Labels

Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: pending review Progress: ready for testing

Projects

Status: Ready for Testing

Development

Successfully merging this pull request may close these issues.

REST API: cart item with qty 0 or negative returns 200 and qty is set to 1

3 participants