coreshop / CoreShop

CoreShop - Pimcore enhanced eCommerce
http://www.coreshop.org
Other
277 stars 157 forks source link

Reimplement User Cart Revise Cancellation #2166

Open solverat opened 1 year ago

solverat commented 1 year ago
Q A
Bug report? yes
Feature request? no
BC Break report? /no
RFC? no

Now

https://github.com/coreshop/CoreShop/blob/c4ddac02c7df4a94f4ee363912ddc9a1322913f3/src/CoreShop/Bundle/FrontendBundle/Controller/OrderController.php#L66

Original

https://github.com/coreshop/CoreShop/blob/100b1b3b90fbc876ce22610232b38549b5be5586/src/CoreShop/Bundle/FrontendBundle/Controller/OrderController.php#L75-L100

solverat commented 1 year ago

@dpfaffenbauer Is there a reason why this has been removed? Otherwise, we could provide an PR which implements the original behavior.

dpfaffenbauer commented 1 year ago

@solverat It never was implemented with the new Order Flow. How would you implement it?

solverat commented 1 year ago

@dpfaffenbauer: I just tested it via this roughly implemented snippet:

if ($cancelButton instanceof ClickableInterface && $form->isSubmitted() && $cancelButton->isClicked()) {

    if ($order instanceof Concrete) {
        $this->get(HistoryLogger::class)->log(
            $order,
            'User Cart Revise Cancellation'
        );
    }

    $order->setSaleState(OrderSaleStates::STATE_CART);
    $this->get('coreshop.cart.manager')->persistCart($order);

    $request->getSession()->set(
        sprintf('%s.%s', 'coreshop.cart', $order->getStore()->getId()),
        $order->getId()
    );

    return $this->redirectToRoute('coreshop_cart_summary');
}

Which just will bring back the user's cart.

Of course, this snippet should:

BTW: While testing this, I found some orphans:

dpfaffenbauer commented 1 year ago

Let's think about the consequences of reseting the state instead of creating a new cart:

Not sure if it woudl be better to create a new cart and populate it with the same items. Sort of similar like it was with CS 2

solverat commented 1 year ago

In CS2, the original cart object has been re-assigned, so it was a bit easier. However, creating a new cart object leads to the same problem you've mentioned (second item of your list). What if the cart has been populated with other data (additional fields during the checkout, for example)?

I still think, the re-assignment is the better way to go. I admit, the Order-Number could be an issue, so resetting that field would solve that. Everything else can be ignored: The order is still invalid at this point. Also: The workflow will trigger again.

dpfaffenbauer commented 1 year ago

the Order-Number could be an issue, so resetting that field would solve that

No, that is not what I meant. The O1 Number will never be reassigned and there is no order then. Once a Order has a Order Number it should stay an Order.

My suggestion: Copy the Order and make it a cart. Then additional Data is also copied.

solverat commented 1 year ago

Ah, I forgot about the sequence generator. Well, yes, cloning the object is the only way to go.

hSpille commented 1 year ago

Same Problem here. Is there a solution for this yet?

dpfaffenbauer commented 1 year ago

@hSpille has not been implemented on our end

m0nken commented 1 year ago

Maybe the order could just have its state switched back to OrderSaleStates::STATE_CART and keep its assigned order number. Then of course the OrderCommitter would have to be adapted in function commitOrder(OrderInterface $order) so that it would only generate a new order number if the order does not already have an order number. Or am I missing something here?

dpfaffenbauer commented 1 year ago

@m0nken I don't really like that, since the Order should get cancelled so the number isn't missing. What if the User never checks out? The Order O0010 would then be missing and never be found.

It honestly shouldn't be too difficult, just take the OrderItems, Copy them, and attach them to a new Cart.

m0nken commented 1 year ago

@dpfaffenbauer Good point! Just wanted to check if that could have been an option.