search

Sylius / ShopApiPlugin

Shop API for Sylius.
https://sylius.com
129 stars 89 forks source link

Delete incompatibility message as it isnt the case #625

Closed remy-theroux closed 4 years ago

remy-theroux commented 4 years ago

Ass discuss in the slack of Sylius and as i see in many places, ShopApi is compatible with the ShopBundle so it's better to delete this confusing message. I am new to Sylius and i take time to search why it's not compatible. It would be helpfull for others to know there is no problem.

mamazu commented 4 years ago

In general I agree, we have solved the problem that was responsible for putting the error message into the documentation. However there is still no guarantee that everything will work perfectly fine (we might have not discovered it yet). See: https://github.com/Sylius/ShopApiPlugin/issues/241

The next problem is that in our test suite we only test the shopapi functions (with the shop as a dependency). What we should really do is also check if the shop is working (aka running the test suit from the sylius/sylius package).

remy-theroux commented 4 years ago

Ok i will try to find time to automate that if it's a convenient solution for you. I am very new to Sylius. We could just say, there may be incompatibilities instead of it is not compatible in a first time ?

mamazu commented 4 years ago

Sure you can try to do that. Testing the integration would be a great thing to have.

The thing is no matter how you want to word it. ShopApi and the shop use different mechanics and different routes for the same thing. (eg. with the checkout) So there will be incompatibilities. And therefore it is currently not recommended to use the shop and the api together. If the compatibility issues are huge or small is a different question. You can try to work around them but this is at your own risk.

BTW: If the checkout process is buggy, which in my opionion is the most important part of an online shop, then it would be justified to call it "not compatible" :D

GSadee commented 4 years ago

I am aware of one problem that this record is still needed for. This plugin overwrites the service (CartBlamerListener) by which the cart in Shop is not assigned to the customer (in the database) after logging in. After solving that issue, we will probably be able to remove that message I guess.

mamazu commented 4 years ago

I'd vote to close this pull request as it is still not compatible.

remy-theroux commented 4 years ago

I close it, i have not enough experience in Sylius so you are probably right.