Added LazyCustomerLoader for OrderType of SyliusAdminApiBundle #9050
Conversation
jdeveloper
commented
Dec 22, 2017
jdeveloper
commented
| Q | A |
|---|---|
| Branch? | 1.0.5 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Related tickets | fixes #8776, mentioned in #8815 |
| License | MIT |
| return $self->customerRepository->findOneBy(['email' => $email]); | ||
| }; | ||
|
|
||
| return array_map($mapEmailToCustomer, $values); |
There was a problem hiding this comment.
I was not sure if taking this approach or add a findCustomersWithEmails method in the CustomerRepository.
I finally opted this approach since it was created specifically to just load the customer by the email submited.
pamil
added
Enhancement
|
@pamil @jdeveloper this pull request will accepted in some time ? We have the same problem... |
|
Is planned solve this issue, or is fixed in v1.2? Thanks. |
|
I'm on it, sorry for the delay! |
|
Hello, I am asking once again for the state of this PR since I think it's a clear improve when there are a lot of users in the database. I have no problem to change implementation if you think it 's not a valid one. Thanks in advance. |
|
@pamil Sorry for asking this one more time Is this PR to be considered? I once again need to creat by REST a cart and it just creates a timeout with a large customer table. I have no regards to take some time to adapt, improve the code |
|
Hi @pamil Sorry for asking once again but we have a database with more then 200 k customers and we want to use the sylius API without deppending on SyliusShopApiPlugin. It is imposible to create a cart without this PR. It would be nice to review this PR, I think it is also a benefit for other developers. If we have to make changes on this PR just comment them and we will work on them. |
|
Hey @jdeveloper, It took too long to provide you some feedback here. We all are humans, and sometimes we tend to forget or miss something. When you asked for a review, we (as a core team) had too much on our heads. I will ping @CoderMaggie to add this issue to our sprints. Therefore someone should take a look over it in the nearest sprint (if it only will fit into backlog). Please, ping me one more time if it won't be resolved during the next month. Thank you for your patience. |
|
Hey @lchrusciel, Thanks for replying. Don't worry, we understand that we are all humans and with such a huge project like Sylius, things can quickly go out of control. We appreciate your reply to an old issue. As always, if we have to work in some issue around this PR, just ping me. Thank you for your attention. |
|
Hi @lchrusciel I just wonder about the state of this PR. As you said to ping you if I got sign on this PR in a month here I am just pinging you to look if we can solve this together. I think it can benefit a lot of people and shops that have a lot of customers and want to integrate through REST API. Have a nice time. |
lchrusciel
force-pushed
the
optimize_create_cart_api
branch
from
8efb3a1
to
de1f2bf
Compare
lchrusciel
force-pushed
the
optimize_create_cart_api
branch
3 times, most recently
from
57dfd94
to
48eb318
Compare
|
As a part of the work over this PR, I've decided to make some profiling. @jdeveloper proposition reduced the time of execution by 62%, but this change may be much greater for a bigger set of data. You can check the complete profile here. The current solution assumes that customers are not loaded to choice type, which makes sense because it is used only in the Admin API Context. To make this choice type usable for the twig based templates, we should introduce Autocomplete Form Type. On the other hand, this form type wouldn't be used anywhere in current Sylius implementation. It is tested already with cart creation endpoint in Admin API. Therefore, it could be improved in the future, but I think that the current solution is good to go. I'm also considering writing an ADR for this feature, not to lose track of it. (/cc @Sylius/core-team) Also, thank you @jdeveloper for the contribution and sorry for a long time for merging it. I've decided to append your work, I hope you don't mind. |
lchrusciel
force-pushed
the
optimize_create_cart_api
branch
from
48eb318
to
7e8d1fe
Compare
|
Thanks, @jdeveloper! 🥇 |
|
Congrats @jdeveloper ! Congrats Sylius Team ! |
|
|
|
WoW I'm so happy to see this merged. Thanks a lot. It's a great pleasure being able to contribute. |
|
Hope things will go more smoothly in the future, thanks for your patience 💯 |
