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

In admin the fetch of collection times fails if collection time is outside the window #50

Open
jacquesbh opened this issue Nov 7, 2020 · 3 comments

Comments

@jacquesbh
Copy link
Contributor

Description

Let's say a customer saves a collection time for 7am UTC in 10 days.
Then later today the merchant changes the location hours and excludes the 7am UTC.

Then in admin you'll get an exception when you try to get all the collection times.

RuntimeException:
This collection time isn't part of the recurrence.

  at vendor/tilleuls/sylius-click-n-collect-plugin/src/CollectionTime/RecurrenceInstanceFinder.php:51
  at CoopTilleuls\SyliusClickNCollectPlugin\CollectionTime\RecurrenceInstanceFinder->__invoke(object(Shipment))
     (vendor/tilleuls/sylius-click-n-collect-plugin/src/Controller/Admin/CollectionsApiController.php:63)
  at CoopTilleuls\SyliusClickNCollectPlugin\Controller\Admin\CollectionsApiController->__invoke(object(Request), 'cancale')
     (vendor/symfony/http-kernel/HttpKernel.php:158)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:80)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:201)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (public/index.php:33)                

How to reproduce

Follow steps listed above.

Possible Solution

Solution is just not to validate the collection time in admin: if you have a collection time then the merchant MUST know.

→ quick fix here: 1e32bf2

@veloxy
Copy link
Contributor

veloxy commented Nov 24, 2020

@jacquesbh I'm also bumping into this issue, your quickfix assumes the duration didn't change, does it not? It seems better to also just save the collection end date and not have the admin use the computer at all?

@jacquesbh
Copy link
Contributor Author

My fix is a quick fix to bypass the issue. It's not a proper fix :(.
But if the duration changes, it changes at a certain moment, so from this moment the duration is changed… I mean, it works well this way, it's not perfect because all old slots will have a change as well, but we don't care, they're in the past.

@veloxy
Copy link
Contributor

veloxy commented Nov 26, 2020

Thanks for the response! I'm currently saving the start and end time and took out the checks so the admin area doesn't rely on the location settings. Seems more correct this way.

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

No branches or pull requests

2 participants