search

coopcycle / coopcycle-app

CoopCycle native app
MIT License
150 stars 33 forks source link

Multiple carts with multiple instances is not working #1418

Open alexsegura opened 1 year ago

alexsegura commented 1 year ago

It is because the carts are indexed by restaurant ID (the @id property). But when switching between instances in the same city, 2 restaurants may have the same @id.

r0xsh commented 1 year ago

I'm a bit confused about this code @lucasferraro Why don't you dispatch an @app/SET_BASE_URL event in _renderRestaurantsForTab() function ? This has the side effect of not changing/disconnecting the user connected to the application. Is this an intended effect ?

https://github.com/coopcycle/coopcycle-app/blob/da848d073ec76aa02889d63b04d2180cb66c3f58/src/navigation/checkout/Search.js#L158-L165

https://github.com/coopcycle/coopcycle-app/blob/da848d073ec76aa02889d63b04d2180cb66c3f58/src/navigation/checkout/Search.js#L144-L150

lucasferraro commented 1 year ago

I'm a bit confused about this code @lucasferraro Why don't you dispatch an @app/SET_BASE_URL event in _renderRestaurantsForTab() function ? This has the side effect of not changing/disconnecting the user connected to the application. Is this an intended effect ?

@r0xsh the code that make the change you are talking about is the following:

https://github.com/coopcycle/coopcycle-app/blob/da848d073ec76aa02889d63b04d2180cb66c3f58/src/navigation/checkout/Search.js#L88-L105

https://github.com/coopcycle/coopcycle-app/blob/da848d073ec76aa02889d63b04d2180cb66c3f58/src/navigation/checkout/Search.js#L81-L86

In the last code we're calling the action selectServer which makes all the work related to reconnect the user to a new server.

Paul-Eraman-CoopCycle commented 1 year ago

Sorry, but why is this a problem? Is this a real life use case?

Paul-Eraman-CoopCycle commented 1 year ago

@alexsegura unless there is a reason to prioritize this, I´m going to move to backlog in a few days

lucasferraro commented 1 year ago

Hi! For Ciudad de Mexico we need to support multiple instances in the app and the feature for multiple carts introduced with the custom app, at least in its original implementation, did not work for this case. There's an old PR open to fix that https://github.com/coopcycle/coopcycle-app/pull/1423 but I don't know if current state of the app works fine with that PR.

Paul-Eraman-CoopCycle commented 1 year ago

@lucasferraro But if i have different custom apps for different cooperatives, why do i need this feature? I would have just one cart on each app right? No longer linked?

r0xsh commented 1 year ago

Yeah but the main application should not crash in this case. And since custom apps are still not a thing, we need to support this. I'll take a look at this PR this Monday and I'll move it to backlog if it require time

Paul-Eraman-CoopCycle commented 1 year ago

I have a meeting with the ITDP people in México on monday, ill ask if they really want this, or if having custom apps solves this, because if custom apps solve this we should focus on that instead. At least i dont know of any other use case where this would be useful

Paul-Eraman-CoopCycle commented 1 year ago

ITDP said that they aren't sure if custom apps fixes this but it might. I'm going to put in suggested for future sprints category until we have more information.