Closed dblaichinger closed 5 months ago
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅
Hey @dblaichinger
Could you plase sign cla and then re-trigger the check?
Tnx
recheck
I have read the CLA Document and I hereby sign the CLA
Can we do that without changing an interface? As it is now it would be a bc break
Hi @fashxp,
I've removed the interface change in this PR and added a method_exists
check instead.
However, I still think it's cleaner to extend the interface, so I've added a new https://github.com/pimcore/ecommerce-framework-bundle/pull/146. Can you please add the BC-BREAK
label there and remove it from this one?
Thank you!
@dblaichinger thx very much. added some docs, can you please double check? also thought that it might be really cool, if the custom properties would be stored automatically and extending Cart/CartItem isn't even necessary. But that might require a bit more refactoring.
Hi @fashxp
added some docs, can you please double check?
I've added some details, please check https://github.com/pimcore/ecommerce-framework-bundle/pull/143/commits/b43fa91b230f4891bfa1d4d388705e76a899caa4
also thought that it might be really cool, if the custom properties would be stored automatically and extending Cart/CartItem isn't even necessary.
Can this be achieved without changing CartItemInterface
and breaking compatibility?
I've added $customProperties
to AbstractCartItem
in the BC-BREAK
tagged PR: https://github.com/pimcore/ecommerce-framework-bundle/pull/146/files. Maybe that's a better approach when already breaking compatibility.
Thanks!
I've added $customProperties to AbstractCartItem in the BC-BREAK tagged PR: https://github.com/pimcore/ecommerce-framework-bundle/pull/146/files. Maybe that's a better approach when already breaking compatibility.
not sure if we really need to add an additional parameter, of if we just add a generic functionality to the existing cart implementations, which persists the data passed via $params
parameter to their storage. Let's continue the discussion over there in the other PR.
This one looks good now. Thx very much!!
Overview
The suggested change would make it easier to extend
CartItem
class and provide the property inCart::addItem()
as$params
afterwards.Extend CartItem
The
$params
parameter inAbstractCart::addItem
is currently unused in this method, but the doc-block suggests that it's possible to add additional data.In
src/CartManager/CartInterface.php
:Also see: