Fix venues reporting Closed while open past midnight - #581
Open
eastagiletracker wants to merge 1 commit into
Open
eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
parseWeeklyHours files each interval under the weekday it starts on and lets it run past midnight, but isOpen and nextOpenInterval() only read today's bucket. Between midnight and closing time a venue therefore reports Closed and its hours disappear from the detail card. sameDayThisWeek() compounded this by searching forward from the start of today, so an interval that began yesterday and is still running was projected a week into the future instead of staying in the current week. Search from the start of yesterday instead, and check the previous day's intervals alongside today's when resolving the current interval. Days from today onwards resolve exactly as before, and an interval that ended yesterday still reads as closed because the interval must contain the current time to match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR proposes a fix for venues reporting Closed while they are still open past midnight. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/229. You can sign in with your GitHub ID to claim ownership of the project.
The defect
parseWeeklyHoursfiles every interval under the weekday it starts on and deliberately lets it run past midnight —WeeklyHoursdocuments exactly that: "DateIntervals are adjusted to the current week, and may extend to the next day." ButisOpenandnextOpenInterval()only read the bucket for today, so once midnight passes, the interval that is currently running sits in yesterday's bucket and neither one finds it. This is the case the// TODO: Fixme, Dining halls may have open/close times that overlap multiple days.comment directly aboveisOpenrefers to.Date.sameDayThisWeek()compounds it. It searches forward from the start of today, so an interval that began yesterday and is still running is not merely in the wrong bucket — it gets projected a week into the future.The user-visible result: a dining hall open Friday 21:00 to Saturday 01:00 shows the red "Closed" badge from midnight until it actually closes, and its hours row disappears from the detail card — precisely during the late-night window someone would open the app to check.
Reproducing on current master
The open-hours logic is plain Foundation, so it can be exercised directly against the four real source files. Save this as
main.swift:then build it against the repo and run it with the clock frozen at Saturday 00:30, when the venue is open:
On master:
With this branch:
The fix
Date.sameDayThisWeek()now searches from the start of yesterday rather than the start of today, so an occurrence that began yesterday and may still be running stays in the current week instead of jumping forward a week. Anything from today onwards resolves exactly as it did before.isOpenandnextOpenInterval()now consult the previous day's intervals alongside today's, through a small private helper. An interval that ended yesterday still reads as closed, because matching still requires the interval to actually contain the current time — pulling in the extra bucket cannot turn a closed venue open.Nothing public changes shape:
sameDayThisWeek()'s only caller in the app isparseWeeklyHours, and the weekly schedule rows are unaffected because they render each interval throughtimeOnly(), which uses only the hour and minute.Verification
The project has no test target, so rather than add one blind to the Xcode project I ran the harness above across a scenario matrix, before and after, with the clock frozen for each case:
That is 7 passing / 4 failing on master, and 11 passing / 0 failing here — the four that flip are the overnight cases, and every previously-passing case still passes. The ninth row is the control that matters most for this change: a venue whose yesterday interval has already ended must keep reading Closed, and it does.
How this was managed
This work was tracked as Venues with hours past midnight report Closed while they are open on a board at https://eastagiletracker.com/projects/229 imported from this repository's own issues and pull requests (572 stories, 13 labels), where the story was picked up and moved through to done as the fix was written.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com