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

[jacquesbh]

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: 1e32bf24cf6f2486be6892392c625f49d3e7af51

[veloxy]

@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]

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]

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.