Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Cart model customizable and easy to exchange #13

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tomhatzer
Copy link

@tomhatzer tomhatzer commented Oct 31, 2019

Dear maintainers,

I've updated the package to allow custom Cart model classes.
This is a nice addition if you want to extend the base Cart model class with more relationships like adding a user and coupon relationship.

I hope my code is ok, otherwise please comment and I'll fix any issues that my occur.

Thank you very much!

Best wishes
Tom

PS: Sorry for the multiple commits...

@gauravmak
Copy link
Member

Hi Tom,

I really appreciate your work here.

This one looks good at the first pass. Let us work on this together to polish it :)

Can you please also update tests accordingly to make sure that users' custom models would also work properly?

Thanks.

@tomhatzer
Copy link
Author

Hi @gauravmak !

Thanks for replying 😃
Sure, I've spent quite some time now on this but I'm uncertain if testing of the custom models is possible at all. Setting/overwriting the config value is possible and writing a test that checks if the config value has changed is possible either.

But for the models themself, there is currently no way to test it as everything is converted to arrays or Core/Cart and Core/CartItem.

The models are only used for inserting data into the database and querying data inside the database.

Do you possibly have anything in mind that could help with this?
One thing that could work is switching from the Core classes to the models classes and use the toArray() methods inside the model classes. But this would be a huge breaking change.

Thank you very much!

Best wishes
Tom

@gauravmak
Copy link
Member

Tom,

I will be able to take a look at it later this week. I will reply here.

@nfunwigabga
Copy link

It would really make sense to be able to customize the model.

@gauravmak
Copy link
Member

gauravmak commented Apr 12, 2023

Yes, it does. Haven't been able to go through this due to time constraints.

This PR is quite old now. Is anyone still up to recreating with the latest code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants