tastyigniter / TastyIgniter

:fire: Powerful, yet easy to use, open-source online ordering, table reservation and management system for restaurants
https://tastyigniter.com
MIT License
2.98k stars 976 forks source link

[Bug]: Lead time NOT work #1004

Open BreakSecurity opened 2 years ago

BreakSecurity commented 2 years ago

What happened?

Lead time stops working if the option to "add lead time to opening hours" is disabled.

1) The following setting must be deactivated image 2) Set a Lead Time image 3) The Lead time won't be respected (It was 06:03 pm when the screenshot was taken) image 4) Once the option is enabled, everything works as expected image

What did you expect to happen?

Lead time should always work.

Version

3.5.x

What browser are you seeing the problem on?

Firefox, Chrome

Relevant log output

No response

ARTdrakon commented 2 years ago

Isn't this what supposed to happen? If you disable lead time, then it's obviously shouldn't be added and just follows schedule time.

sampoyigi commented 1 year ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider the Dedicated Support Service where a Service Level Agreement is offered.

krozendaal commented 1 year ago

I also noticed this issue, which didn't happen on earlier versions.

The issue misses one step I think: it's important that you enable: Allow Pre-order for Delivery.

Now the issue is: the lead time is not added anymore, only when you turn on the "Add lead time to Delivery start time".

Why do you not want to enable this feature, but still have lead times? Well, you could start delivering from your delivery start time, e.g. 20:00, but orders can come in from 15:00 for example (or even the day before). Now where does this go wrong? If you turn on the setting ("Add lead time to Delivery start time") the first time someone can order (also at 15:00) is 20:00 + lead time, however opening times is set to 20:00. So you might think to turn off this feature, which leads to the actual unexpected behavior: now anyone can order from 20:00 (which is good if it's 15:00, or in general 20:00 - lead time), but people can also order for 20:00 if it's 19:59 - which is not how it should work. It seems like the lead time is not used anymore in calculating which delivery/pickup schedules to show. So even one minute before the time happens, you can order for that time.

In short The setting Add lead time to Delivery start time influences the start time of delivery or pick-up, however when the option is turned off the lead time isn't added to the delivery or pick-up options.

I think the issue can be found in the Location.php class in the ti-ext-local extension, see: https://github.com/tastyigniter/ti-ext-local/blob/3137bf9e5863409d6d18aa66a3ac183f67d96afb/classes/Location.php#L306

Essentially LeadTime should be 0 for all days in the future, and for today when the current time + lead time is smaller than the first delivery / pick-up time. However, it should be enabled once today starts getting into first delivery / pickup time - lead time.

Workaround There is a 'dirty' workaround available, which involves setting your opening hours to your opening hours - lead time and enabling this feature. However, this will show the opening hours wrong in different locations.

BreakSecurity commented 1 year ago

@sampoyigi I tested https://github.com/tastyigniter/ti-ext-local/pull/87 and it fixed the issue. Thanks @krozendaal

sampoyigi commented 1 year ago

@krozendaal So, just to be clear, when the current time is less than (20:00 - lead time), the first delivery time is 20:00 + lead time, but when the current time is exactly 20:00 - 1 minute, that is 19:59, the first delivery time is 20:00 instead of 20:00 + lead time? Is this only true when the current time is one minute before the delivery start time (19:59) or when the current time is between (20:00 - lead time) and 20:00?

krozendaal commented 1 year ago

Hi @sampoyigi,

Not exactly. When you turn on "Allow Pre-order for Delivery", but turn off "Add lead time to Delivery start time", the lead time is disregarded in general (which makes sense looking at the code for ti-ext-local). Also after 20:00 in my example. So let's say it's 20:14, than the first option in the list is 20:15 (if the block time is set to 15 minutes for example).

This is logicial if the minimum days to order in pre-order is set to > 0 (e.g. 1). In this case, it's never possible that the lead time is necessary. However, then the minimum days to order in pre-order is set to 0, it's possible that an order comes in on the current day, for which the lead time should always be added for the first option.

Just to make it a little bit more visual here are some examples: Assumptions: lead time 60 minutes, order blocks 15 minutes, opening set to 20:00 Current time 15:00 - this goes perfectly fine, the first option is 20:00. Current time 19:00 - this still goes perfectly fine, as the first option should still be 20:00 Current time 19:01 - here it starts to go wrong, first option should be 20:15 now, however it's still 20:00 Current time 19:30 - going wrong, first option is still 20:00 Current time 19:59 - going wrong, first option is still 20:00 Current time 20:01 - going wrong, first option is now 20:15

I hope this makes it a little bit more clear.

My pull request in the ti-ext-local repo removes all the options that are within the current time + lead time time frame. It's probably not the best way of doing this, but I couldn't think of another way of doing it, without needing extra functions. This way the lead time is always respected, without the option of "Add lead time to Delivery start time" turned on.

sampoyigi commented 1 year ago

Got it, thank you. But, you can enable Add lead time to Delivery start time? This way the first option will be opening time + lead time.

krozendaal commented 1 year ago

Yes, that’s correct. If you enable “Add lead time to delivery start time”, the first option is indeed opening time + lead time.

The workaround I provided in an earlier post shows this works as it should, however than the delivery times/pick up times don’t correspond to the real values.

sampoyigi commented 1 year ago

Ok, i'll review the PR once i get the chance to.