magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.55k stars 9.32k forks source link

[Issue] Cannot update Order status in 2.4.7 #38659

Closed Nuranto closed 2 months ago

Nuranto commented 6 months ago

Preconditions and environment

Steps to reproduce

  1. Try to update status from admin order page view

Expected result

Status updated

Actual result

Status not updated

Additional information

I don't understand this change introduced in 2.4.7 : https://github.com/magento/magento2/issues/36562

private function getOrderStatus(string $orderStatus, string $historyStatus): string
    {
        return ($orderStatus === Order::STATE_PROCESSING || $orderStatus === Order::STATUS_FRAUD) ? $historyStatus
            : $orderStatus;
    }

Why would we be able to change status only if current status is processing or fraud ? (Also, Order::STATE_PROCESSING instead of Order::STATUS_PROCESSING )

If I understand correctly the issue #36562, the fix should be something like this instead :

private function getOrderStatus(OrderInterface $order, string $historyStatus): string
    {
        $statuses = $this->orderConfig->getStateStatuses($order->getState());

        if (!in_array($status, array_keys($statuses))) {
            return $order->getStatus();
        }
        return $historyStatus;
    }

Or am I missing something ?

Release note

No response

Triage and priority

m2-assistant[bot] commented 6 months ago

Hi @Nuranto. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

m2-assistant[bot] commented 6 months ago

Hi @engcom-Bravo. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

m2-assistant[bot] commented 6 months ago

Hi @engcom-Dash. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:


engcom-Dash commented 6 months ago

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 6 months ago

Hi @engcom-Dash. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 6 months ago

Hi @engcom-Dash, here is your Magento Instance: https://fe74e6b08484c0858eb929e57e74a885.instances-prod.magento-community.engineering Admin access: https://fe74e6b08484c0858eb929e57e74a885.instances-prod.magento-community.engineering/admin_f188 Login: 64dd48de Password: 68a200a92f49

engcom-Dash commented 6 months ago

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 6 months ago

Hi @engcom-Dash. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 6 months ago

Hi @engcom-Dash, here is your Magento Instance: https://fe74e6b08484c0858eb929e57e74a885.instances-prod.magento-community.engineering Admin access: https://fe74e6b08484c0858eb929e57e74a885.instances-prod.magento-community.engineering/admin_058a Login: 60dbf855 Password: 7a60f5716929

engcom-Dash commented 6 months ago

Hi @Nuranto

Thanks for reporting and collaboration.

Can you please provide detailed steps to reproduce with screenshots which would help us reproduce the issue accurately

Thanks!

Nuranto commented 6 months ago

(https://fe74e6b08484c0858eb929e57e74a885.instances-prod.magento-community.engineering/admin_058a/sales/order/view/order_id/3/)

https://github.com/magento/magento2/assets/1633463/5237d99c-86ec-4281-a216-2c7d84ed5854

engcom-Dash commented 6 months ago

Hi @Nuranto

Verified the issue in 2.4.7 and 2.4 dev instances.

But I am seeing only processing and suspend fraud options in order status after generating invoice.

Please refer to the screenshots. Let us know if we are missing anything.

Screenshot 2024-04-29 at 2 41 49 PM
Nuranto commented 6 months ago

I added a custom status on your temporary env.

As you can see in the code :

private function getOrderStatus(string $orderStatus, string $historyStatus): string
    {
        return ($orderStatus === Order::STATE_PROCESSING || $orderStatus === Order::STATUS_FRAUD) ? $historyStatus
            : $orderStatus;
    }

We can update status, but only if current status is processing or fraud.

engcom-Dash commented 6 months ago

Hi @Nuranto

Thanks for reporting and collaboration.

Verified the issue on magento 2.4 dev and 2.4.7 instances and the issue is reproducible. Hence, we are confirming the issue.

We are able to update the status only if the current status is in a processing or fraud state. If the status is anything other than processing or fraud state we are not able to update the status.

Please refer the attached screen recording.

https://github.com/magento/magento2/assets/60198592/6b72cbc4-fdc1-4b7c-b09c-48d80e667aba

github-jira-sync-bot commented 6 months ago

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-11877 is successfully created for this GitHub issue.

m2-assistant[bot] commented 6 months ago

:white_check_mark: Confirmed by @engcom-Dash. Thank you for verifying the issue.
Issue Available: @engcom-Dash, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

github-jira-sync-bot commented 6 months ago

:x: Cannot export the issue. This GitHub issue is already linked to Jira issue(s): https://jira.corp.adobe.com/browse/AC-11877

redati commented 5 months ago

Related to the order status update, this commit https://github.com/magento/magento2/commit/632f253106e0a2d68cbf0b5a58b12eeeb0a014bf also involves integration with Bling Erp. This integration updates the order via comment.

dennismetz commented 5 months ago

We have the same error in our Magento instances. Is there already a patch?

superdav42 commented 4 months ago

I used a workaround and sharing it so it can be helpful to anyone:

Create app/code/DevStone/Bugfix/Controller/Adminhtml/Order/AddComment.php

<?php

namespace DevStone\Bugfix\Controller\Adminhtml\Order;

use Magento\Framework\App\Action\HttpPostActionInterface;
use Magento\Sales\Model\Order\Email\Sender\OrderCommentSender;

/**
 * Class AddComment
 *
 * Controller responsible for addition of the order comment to the order
 */
class AddComment extends \Magento\Sales\Controller\Adminhtml\Order implements HttpPostActionInterface
{
    /**
     * Authorization level of a basic admin session
     *
     * @see _isAllowed()
     */
    public const ADMIN_RESOURCE = 'Magento_Sales::comment';

    /**
     * ACL resource needed to send comment email notification
     */
    public const ADMIN_SALES_EMAIL_RESOURCE = 'Magento_Sales::emails';

    /**
     * Add order comment action
     *
     * @return \Magento\Framework\Controller\ResultInterface
     */
    public function execute()
    {
        $order = $this->_initOrder();
        if ($order) {
            try {
                $data = $this->getRequest()->getPost('history');
                if (empty($data['comment']) && $data['status'] == $order->getDataByKey('status')) {
                    $error = 'Please provide a comment text or ' .
                             'update the order status to be able to submit a comment for this order.';
                    throw new \Magento\Framework\Exception\LocalizedException(__($error));
                }

                $order->setStatus($data['status']);
                $notify = $data['is_customer_notified'] ?? false;
                $visible = $data['is_visible_on_front'] ?? false;

                if ($notify && !$this->_authorization->isAllowed(self::ADMIN_SALES_EMAIL_RESOURCE)) {
                    $notify = false;
                }

                $comment = trim(strip_tags($data['comment']));
                $history = $order->addStatusHistoryComment($comment, $data['status']);
                $history->setIsVisibleOnFront($visible);
                $history->setIsCustomerNotified($notify);
                $history->save();

                $order->save();
                /** @var OrderCommentSender $orderCommentSender */
                $orderCommentSender = $this->_objectManager
                    ->create(\Magento\Sales\Model\Order\Email\Sender\OrderCommentSender::class);

                $orderCommentSender->send($order, $notify, $comment);

                return $this->resultPageFactory->create();
            } catch (\Magento\Framework\Exception\LocalizedException $e) {
                $response = ['error' => true, 'message' => $e->getMessage()];
            } catch (\Exception $e) {
                $response = ['error' => true, 'message' => __('We cannot add order history.')];
            }
            if (is_array($response)) {
                $resultJson = $this->resultJsonFactory->create();
                $resultJson->setData($response);
                return $resultJson;
            }
        }
        return $this->resultRedirectFactory->create()->setPath('sales/*/');
    }
}

Create app/code/DevStone/Bugfix/etc/di.xml:

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <preference for="Magento\Sales\Controller\Adminhtml\Order\AddComment" type="DevStone\Bugfix\Controller\Adminhtml\Order\AddComment" />
</config>

Of course update the paths and namespace to match an existing module you may have or create a new one with the usual module.xml and registration.php

This will revert the behavior to pre 2.4.7

YiffyToys commented 4 months ago

Issue can be reproduced with 2.4.7-p1

If anyone is trying to find the respective lines in their local Magento instance, it's at the end of magento/vendor/magento/module-sales/Controller/Adminhtml/Order/AddComment.php

YiffyToys commented 4 months ago

Something like this should work too

cat patches/github-issue-38659_cannot-update-orderstatus-magento-2-4-7.diff

diff --git a/vendor/magento/module-sales/Controller/Adminhtml/Order/AddComment.php b/vendor/magento/module-sales/Controller/Adminhtml/Order/AddComment.php
index ea8af5828c..34608db2bd 100644
--- a/vendor/magento/module-sales/Controller/Adminhtml/Order/AddComment.php     2024-06-29 09:53:19.997829986 +0200
+++ b/vendor/magento/module-sales/Controller/Adminhtml/Order/AddComment.php     2024-06-29 10:15:35.983809521 +0200
@@ -91,7 +91,12 @@
      */
     private function getOrderStatus(string $orderStatus, string $historyStatus): string
     {
-        return ($orderStatus === Order::STATE_PROCESSING || $orderStatus === Order::STATUS_FRAUD) ? $historyStatus
-            : $orderStatus;
+           // BUGFIX for https://github.com/magento/magento2/issues/38659
+           return $historyStatus;
+
+
+//        return ($orderStatus === Order::STATE_PROCESSING || $orderStatus === Order::STATUS_FRAUD) ? $historyStatus
+//            : $orderStatus;
+
     }
 }

cat magento/composer.json

...
 "extra": {
        "magento-force": "override",
        "enable-patching": true,
        "composer-exit-on-patch-failure": true,
        "patches": {
...
            "magento/module-sales": {
                "fix Magento 2.4.7 can not set update order status": "patches/github-issue-38659_cannot-update-orderstatus-magento-2-4-7.diff"
            }
        }
    }
YiffyToys commented 4 months ago

Nothing seems to have happend with the pull request #38659 in all of May and all of June. While absolutely basic functionality is broken for all users out there.

Checked out the pull request and basically 1 static code analyser complained that the @param coment wasn't updated to reflect the new method signature and some unit test where $order->config is null failed. Probably because the unit test didn't set it up with proper values. All the other automatic test failures look like the codebase the PR was applied to was already broken.

The initial author hasn't responded in 2 month, no other developer picked it up and to do it yourself Adobe wants you to jump through even more (legal) hoops then you already have to in order to clone the repository, check it out, do your branch and create a new PR. No time for that.

NeoM-dev commented 4 months ago

Having same issue with 2.4.7 - Need solution soon have to send email for every process to update customers.

joshbourke commented 4 months ago

Having same issue. Major functionality broken. Issue been open for nearly 2 months. Fixes like this are taking far too long to review/release. I am not sure why we always need 100's of fixes in each release; it shouldn't be a huge deal to release a 2.4.7.1, 2.4.7.2, etc. with a couple dozen critical fixes.

BeggiNN commented 4 months ago

@magento I am working on this

simonmaass commented 4 months ago

is there any patch or hotfix? this is quite important as many integrations (incl. payment) are failing that are using the comment api for webhooks etc

Quazz commented 3 months ago

The order state should be checked rather than the status to begin with; no clue how this got greenlit.

You can make custom order statuses for a singular order state. With the changes that have been made it's impossible to change order status after it's been set.

You should be able to switch freely between order statuses for the same order state.

This needs to be rectified ASAP.

edit: In fact; since only the statuses for the current state are displayed, it should be impossible to enter an incorrect status...

dennismetz commented 2 months ago

Almost four months since the bug was reported... is there any hope that a patch will be released soon?

Henryrchurchill commented 2 months ago

We are also waiting for a patch for this issue.

redati commented 2 months ago

I haven't seen it yet, but does 2.4.7-p2 solve this?

YiffyToys commented 2 months ago

At least the patch was still applied by Compose.

As you can see here, even the current 2.4 development branch doesn't have a fix yet. https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Sales/Controller/Adminhtml/Order/AddComment.php#L94

redati commented 2 months ago

in a quick solution for an integration here, i just removed this code from that file.

vendor/magento/module-sales/Model/Service/OrderService.php

/**

chittima commented 2 months ago

Internal team is working on this issue

pkd87 commented 2 months ago

Any update on issue? We are using Magento 2.4.7.

chittima commented 2 months ago

The internal team already fixed this issue. Currently, in delivery queue.

YiffyToys commented 2 months ago

The internal team already fixed this issue. Currently, in delivery queue.

So what about tagging this ticket with the milestone/release it is intended for?

o-iegorov commented 2 months ago

The internal team already fixed this issue. Currently, in delivery queue.

So what about tagging this ticket with the milestone/release it is intended for?

we can determine relese when the code changes will be delivered, since they are in delivery process and not delivered yet we can't specify release which will contain the changes

hostep commented 2 months ago

Possibly fixed by https://github.com/magento/magento2/commit/18dee3d979ed8ee68a44324e892e6b7f570c987c?

chittima commented 2 months ago

@hostep Yes

engcom-Bravo commented 2 months ago

Hi,

As per this comment https://github.com/magento/magento2/issues/38917#issuecomment-2348086896 we are closing this issue.

Thanks.

YiffyToys commented 2 months ago

No milestone = release set?

hostep commented 2 months ago

Based on the Jira ticket, the target version is 2.4.8-beta1.

Source: https://github.com/magento/magento2/issues/38917#issuecomment-2348086896

guilhermemap commented 1 month ago

same issue here on 2.4.7-p2