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

When canceling order with OrderService, the cancel method always saves the order and returns true, even if the order can not be canceled. #10803

Closed
ekuusela opened this issue Sep 7, 2017 · 6 comments
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@ekuusela
Copy link
Contributor

ekuusela commented Sep 7, 2017

I'm assuming the cancel method in OrderService is supposed to return true only if the cancellation was a success.

But, since the cancel method in Order always returns the order itself, the service method ends up always saving and returning true as if the order was cancelled.

public function cancel()

This is confusing if not broken. The meaning of the boolean return value should be documented and if it is indeed meant as an indicator of successful cancelation, the service method should be fixed.

I'll include the boilerplate below to prevent this issue from getting auto-marked as not helpful.

Preconditions

  1. Fresh Magento 2 version (see links to functions in develop branch above)

Steps to reproduce

  1. Look at phpdoc in OrderService's cancel method and try to understand it's return value.

Expected result

  1. A clear understanding of the return value AND
  2. Return value corresponding to reality, for example, true only if order was canceled.

Actual result

  1. Confusion. Looks like the return value might indicate cancelation success, but it does not.
@magento-engcom-team magento-engcom-team added G1 Passed Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed labels Sep 7, 2017
@magento-engcom-team
Copy link
Contributor

Hi @ekuusela
Could you please create a Pull Request for this issue?

@magento-engcom-team magento-engcom-team added Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed and removed G1 Passed labels Sep 11, 2017
@okorshenko okorshenko added the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Sep 14, 2017
@freakphp
Copy link
Contributor

@freakphp

@magento-engcom-team
Copy link
Contributor

@ekuusela thank you for your bug report.
We've created internal ticket MAGETWO-75370 to track progress on the issue.

@magento-engcom-team magento-engcom-team added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Sep 19, 2017
magento-team pushed a commit that referenced this issue Sep 23, 2017
…e for cancel me… #10919

 - Merge Pull Request #10919 from freakphp/magento2:10803
 - Merged commits:
   1. db79dbf
   2. d4cddf7
magento-team pushed a commit that referenced this issue Sep 23, 2017
@magento-team
Copy link
Contributor

Internal ticket to track issue progress: MAGETWO-75327

@orlangur orlangur added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Sep 23, 2017
@strell
Copy link
Contributor

strell commented Sep 30, 2017

I would like to work on this issue

strell added a commit to strell/magento2 that referenced this issue Sep 30, 2017
From db79dbf Mon Sep 17 00:00:00 2001
From: freakphp <[email protected]>
Date: Sun, 17 Sep 2017 12:52:00 +0200
Subject: [PATCH 1/2] magento#10803 update OrderService to return correct bool value for cancel method

From d4cddf7 Mon Sep 17 00:00:00 2001
From: Ievgen Shakhsuvarov <[email protected]>
Date: Mon, 18 Sep 2017 12:22:00 +0200
Subject: [PATCH 2/2] magento#10919: update OrderService to return correct bool value
@okorshenko
Copy link
Contributor

@ekuusela thank you for your report. The issue is already fixed in 2.2-develop branch (PR: #11160 by @strell) and will be available in the Magento 2.2.2 release

@okorshenko okorshenko added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

9 participants