[Improvement]: CartInterface::setCheckoutData should accept null values as data

[aashan10]

Improvement description

The AbstractCartCheckoutData class accepts null values as data attribute. In fact, by default the value is null

ecommerce-framework-bundle/src/CartManager/AbstractCartCheckoutData.php

Lines 19 to 27 in 82a567e

	abstract class AbstractCartCheckoutData extends \Pimcore\Model\AbstractModel
	{
	protected string $key;
	protected array|string|null $data = null;
	protected ?CartInterface $cart = null;
	public function setCart(CartInterface $cart): void

By this logic, we should be able to set null to cart data using the implementation in CartInterface

But, CartInterface method signature does not allow this.

ecommerce-framework-bundle/src/CartManager/CartInterface.php

Lines 154 to 159 in 82a567e

	/**
	* Set custom checkout data for cart.
	* can be used for delivery information, ...
	*
	*/
	public function setCheckoutData(string $key, string $data): void;

[jdreesen]

It seems like CartInterface::setCheckoutData() only allows string values for $data while AbstractCartCheckoutData::$data also allows null and array.

[aashan10]

@jdreesen there is no api to remove checkout data from the cart. As far as I am concerned, either there should be a way to remove checkout data from the cart or setCheckoutData should accept null as the value param (to invalidate the previously set checkout data).

I think a better example would be something like this:

• Let's assume Discount vouchers are stored as checkout data
• A customer applies a discount voucher on the cart
• Customer removes the discount voucher (now there is no way of removing the checkout data without resetting the entire cart)

what do you think?

[jdreesen]

It should definitely be possible to reset/remove the data.

In our current project, we are still using Pimcore 10, where the types are still annotated in the PHPDoc. There we pass null in some places to remove the data again. So this would actually be a blocker for us that would prevent the upgrade to Pimcore 11 if it were not possible (and we would like to update soon).

What I meant in my comment was that AbstractCartCheckoutData::$data is typed as array|string|null so we should probably not only allow null but also array. Or remove the array from the allowed types of the property if it's not possible to set it anyway.

[jdreesen]

In fact, looking at the code, I'm not sure if it currently works when an array is passed, because when the data is written to the DB, arrays, and even objects are serialized. But it does not seem that deserialization happens when it is read again. 🤔.

What we currently do in our project is the manual serialization of such values in JSON before and deserialization after. But this is not very convenient.

[github-actions]

Thanks a lot for reporting the issue. We did not consider the issue as "Pimcore:Priority", "Pimcore:ToDo" or "Pimcore:Backlog", so we're not going to work on that anytime soon. Please create a pull request to fix the issue if this is a bug report. We'll then review it as quickly as possible. If you're interested in contributing a feature, please contact us first here before creating a pull request. We'll then decide whether we'd accept it or not. Thanks for your understanding.