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

Opening hours across Midnight #88

Closed
rabauss opened this issue Feb 5, 2019 · 13 comments
Closed

Opening hours across Midnight #88

rabauss opened this issue Feb 5, 2019 · 13 comments
Labels
enhancement good first issue Good issue if you want to contribute
Milestone

Comments

@rabauss
Copy link

rabauss commented Feb 5, 2019

Is there a way to define Opening Hours accross Midnight??

For example:

$openingHours = OpeningHours::create([
    'tuesday'     => ['09:00-00:00'],
    'tuesday'     => ['09:00-03:00'],
    'exceptions' => [
        '2019-02-05' => ['09:00-04:00'],
    ],
]);

If we split into two slots 09:00-23-59 and 00:00-03:00, then at 16:00 the nextClose would be 23:59 instead of 03:00. Has someone a possible solution for that problem?

@kylekatarnls
Copy link
Collaborator

Yep, explained here: #22, you can use '09:00-24:00'

@rabauss
Copy link
Author

rabauss commented Feb 5, 2019

Unfortunately this won't bring the right solution for this:

$openingHours = OpeningHours::create([
    'monday'    => ['09:00-03:00'],
    'tuesday'   => ['09:00-03:00'],
]);

transformed to:

$openingHours = OpeningHours::create([
    'monday'     => ['09:00-24:00'],
    'tuesday'    => ['00:00-03:00','09:00-24:00'],
    'wednesday'  => ['00:00-03:00'],
]);

this won't say nextClose is 03:00 - it returns: 2019-02-05 00:00:00.000000

@kylekatarnls kylekatarnls reopened this Feb 5, 2019
@kylekatarnls
Copy link
Collaborator

I don't think we should handle ['09:00-03:00'] but consecutive 24:00 to next 00:00 seems fair. I will try to find a way.

@kylekatarnls kylekatarnls self-assigned this Feb 5, 2019
@kylekatarnls kylekatarnls added this to the v2.1 milestone Feb 5, 2019
kylekatarnls added a commit to kylekatarnls/opening-hours that referenced this issue Feb 13, 2019
@kylekatarnls
Copy link
Collaborator

Fixed in #90, please try to update to dev-master and see if it fixes your problem. Thanks.

@kylekatarnls
Copy link
Collaborator

@kylekatarnls kylekatarnls changed the title Opening hours accross Midnight Opening hours across Midnight Feb 18, 2019
@rabauss
Copy link
Author

rabauss commented Feb 26, 2019

Just, wanted to give short feedback: it's working!
thank you!!

The "24:00 Hack" should be added in the Readme ;-)

@kylekatarnls
Copy link
Collaborator

kylekatarnls commented Feb 26, 2019

Thanks for your feedback!

From the README:

'wednesday' => ['22:00-24:00'], // use the special "24:00" to reach midnight included

@gildebrand
Copy link

I'd like to express my interest as well for the possibility to have opening hours across midnight. Although it doesn't make sense technically, opening hours in the format of 12:00-02:00 is a format that's commonly used in real life (for restaurants, night clubs, etc.). I think this would be a great feature to have.

@kylekatarnls
Copy link
Collaborator

So specifications would be:

Instead of having an error if start > end, we would consider start-24:00 + 00:00-end (on the next day) but if I want to display those opening hours, I would still get start-end as in the definition.

Right?

This change may be considered as breaking because if an application rely on the verification to handle some user input opening hours validation, now it will no longer throw an exception and it may allow inputs the application itself would not be ready to handle.

So as long as we don't start a new major release, it must remain an option you first need to enable such as: 12:00-02:00+1day, or 12:00-24+02:00, those notations are ugly, but we need something to say explicitly we want the hour on the next day. Or else it can be a property at the root of the definition array: 'allow_overflow' => true maybe.

I'm open to pull-requests for this.

@kylekatarnls kylekatarnls reopened this Mar 29, 2019
@kylekatarnls kylekatarnls added the good first issue Good issue if you want to contribute label Mar 29, 2019
@gildebrand
Copy link

I think the best way would be to pass a flag when creating the OpeningHours instance. Maybe as second argument of OpeningHours::create. The 12:00-24+02:00 option would also be viable I think, but passing a flag would be preferred.

I might have a look at implementing this. If I find a good way to solve this, I'll submit a PR.

@kylekatarnls kylekatarnls removed their assignment Mar 30, 2019
kylekatarnls pushed a commit that referenced this issue May 7, 2019
* Add asStructuredData to README usage

* Allow on construction for setting a flag for overlapping times. A night club opens that opens till 3am on Friday and Saturday. And modify the API to search for yesterday

* Oops, change docs to use overflowing rather than overlapping

* Fix styleguide
@kylekatarnls
Copy link
Collaborator

Explanation about flags pitfalls: #104 (comment)

So final API is:

$openingHours = \Spatie\OpeningHours\OpeningHours::create([
    'overflow' => true,
    'friday'   => ['20:00-03:00'],
    'saturday' => ['20:00-03:00'],
], null);

It will be released on 2.2.0.

Thank you to @rlweb for the implementation, thank you all for your feedback on this feature.

@rabauss
Copy link
Author

rabauss commented May 9, 2019

Oh nice, that we can now directly create overflowing opening hours.

Recently we had some problems with exceptions and overflows. If you have an exception for closed on one day, then of course the overflow should also be closed. Or if you have special overflow opening then the overflow should not overwrite the regular opening hours on next day.

Here some example data:

$openingHours = \Spatie\OpeningHours\OpeningHours::create([
    'overflow' => true,
    'thursday'   => ['20:00-03:00'],
    'friday'   => ['20:00-03:00'],
    'saturday' => ['20:00-03:00'],
    'sunday' => [],

    'exceptions' => [
        '2019-05-10' => [
            'hours' => ['20:00-02:00'],
        ],
        '2019-05-11' => [
            'hours' => [],
        ],
    ],
]);

Tests should be:

$openingHours->isOpenAt(new DateTime('2019-05-10 02:00:00')); // true (from regular thursday)
$openingHours->isOpenAt(new DateTime('2019-05-11 01:00:00')); // true (from exception 2019-05-10)
$openingHours->isOpenAt(new DateTime('2019-05-12 02:00:00')); // false (from exception 2019-05-11 -- without exception this test should say true!)

Can the extension handle that? would be nice if it can ;-)
Maybe in the next few days I have some time to test it by myself.

@kylekatarnls kylekatarnls modified the milestones: v2.1, v2.2 May 9, 2019
@kylekatarnls
Copy link
Collaborator

Hello @rabauss I do not plan to add this myself right now and as 2.2.0 has been released, I encourage you to create a new issue for this new part of the feature. Maybe you could add your unit tests and submit them as a pull-request. I would merge them on a dedicated branch to the feature so it could help to implement it if someone is interested in proposing a solution for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue Good issue if you want to contribute
Projects
None yet
Development

No branches or pull requests

3 participants