Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues #16160 #16258

Open
wants to merge 3 commits into
base: 1.13
Choose a base branch
from
Open

Conversation

akyoscommunication
Copy link

Updated the 'isStockSufficient' function in the AvailabilityChecker class to include an optional argument for deferring payment check. Also adjusted the logic in PaymentPreCompleteListener accordingly.

Q A
Branch? 1.13
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets fixes #16160
License MIT

During an order, the requested quantity of products is deducted from the available stock and set aside. However, when proceeding to payment via the admin interface (payment plugins do not trigger the same event), the stock verification function subtracts the reserved stock from the total stock to determine if there is enough left to fulfill the order. This approach yields an incorrect value (false) when the available stock is just sufficient to cover the order, as the quantity to be paid for is already deducted from the total stock. To address this issue, here is a proposal.

To reproduce the error on the demo:

  • Add a product with a stock of 10.
  • Place an order for 10 units of this product with deferred payment (as payment plugins do not trigger the same event).
  • Verify the stock of the product changing to 0, with 10 units reserved.
  • As an administrator, validate the payment.
  • A message indicates that there is not enough stock remaining, as the calculation performed is as follows: requested quantity (10) <= total stock (10) - reserved stock (10), which is 10 <= 0. However, the 10 reserved units are those for which the payment is validated and should not be subtracted.

At this stage, it is sufficient to verify that there are still a total of 10 units in stock, without needing to check the reserved units, as this verification has already been performed.

Updated the 'isStockSufficient' function in the AvailabilityChecker class to include an optional argument for deferring payment check. Also adjusted the logic in PaymentPreCompleteListener accordingly.
@akyoscommunication akyoscommunication requested review from a team as code owners May 14, 2024 14:29
Copy link

github-actions bot commented May 14, 2024

Bunnyshell Preview Environment deployment failed

Check https://github.com/Sylius/Sylius/actions/runs/9091685728 for details.

Available commands:

  • /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

Copy link
Contributor

Choose a reason for hiding this comment

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

This new code flow should be tested in the AvailabilityCheckerSpec test class.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the review. I have added tests as requested

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants