CoreShop icon indicating copy to clipboard operation
CoreShop copied to clipboard

Reimplement User Cart Revise Cancellation

Open solverat opened this issue 2 years ago • 12 comments

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 avatar Jan 03 '23 16:01 solverat

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

solverat avatar Jan 19 '23 08:01 solverat

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

dpfaffenbauer avatar Jan 19 '23 08:01 dpfaffenbauer

@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:

  • use injected services
  • use the sessionKeyName from the compiled (storageList) parameters

BTW: While testing this, I found some orphans:

  • Parametercoreshop.session.cart: Unused
  • CoreShop\Bundle\OrderBundle\EventListener\SessionCartSubscriber: Unused

solverat avatar Jan 20 '23 11:01 solverat

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

  • What about missing Order Numbers? Eg. O1 get's reset, then O2 get's created, O1 is never visible as Order again
  • What about some other culprits that might be connected to the Order Workflow that do stuff on the transition?

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

dpfaffenbauer avatar Jan 20 '23 19:01 dpfaffenbauer

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.

solverat avatar Jan 21 '23 14:01 solverat

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.

dpfaffenbauer avatar Jan 23 '23 06:01 dpfaffenbauer

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

solverat avatar Jan 23 '23 09:01 solverat

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

hSpille avatar Aug 21 '23 13:08 hSpille

@hSpille has not been implemented on our end

dpfaffenbauer avatar Aug 22 '23 05:08 dpfaffenbauer

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?

m0nken avatar Aug 22 '23 09:08 m0nken

@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.

dpfaffenbauer avatar Aug 22 '23 09:08 dpfaffenbauer

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

m0nken avatar Aug 22 '23 09:08 m0nken