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

Exception based scheduling #385

Merged
merged 22 commits into from
Nov 14, 2023
Merged

Conversation

br648
Copy link
Collaborator

@br648 br648 commented Jun 22, 2023

Checklist

  • Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
  • Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented
  • The description lists all applicable issues this PR seeks to resolve
  • The description lists any configuration setting(s) that differ from the default settings
  • All tests and CI builds passing

Description

The crux of this feature is to allow calendar dates (which are not associated to a calendar, as this is already imlemented) to be loaded, snapshotted and exported without the need to be joined to a calendar. This then allows calendar date services to be defined in their own right and not tied to a calendar.

From the spec (https://developers.google.com/transit/gtfs/reference#calendar_datestxt) this will allow the alternative approach to using calendar dates.

It should be possible under the UI to define a calendar date service by providing the following:

  • Exception name. This will be used as the service id, so will need to be unique.
  • New schedule type of "CALENDAR_DATE_SERVICE" with a value of 10.
  • A single date.

It is assumed that:

  • Only active service dates are provided (exception_type: 1).
  • That the existing process for submitting this data to the backend will not need altering (testing will confirm either way).

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to play with this code more, but see my comments so far.

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm able to replicate a correct export of a schedule exception not linked to a calendar (with some cheating), so I will go ahead and approve subject to the refactorings I mentioned in my previous comment.

@br648 br648 assigned philip-cline and unassigned br648 Aug 8, 2023
Copy link
Collaborator

@philip-cline philip-cline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, but right now I'm a bit confused about the use of the ScheduleException name as the service_id for a calendar_date based service. For example, calendar_dates may be used to add service on 2023-05-29 and 2023-07-04 for a service ID "4" which does not exist in calendar.txt. These would be represented as two separate calendar_dates but right now result in two separate ScheduleExceptions but with the same name. How should this use case be handled if we requiring name to be unique?
image

@philip-cline philip-cline assigned br648 and unassigned philip-cline Aug 23, 2023
Co-authored-by: Philip Cline  <63798641+philip-cline@users.noreply.github.com>
@br648
Copy link
Collaborator Author

br648 commented Aug 29, 2023

Code looks good, but right now I'm a bit confused about the use of the ScheduleException name as the service_id for a calendar_date based service. For example, calendar_dates may be used to add service on 2023-05-29 and 2023-07-04 for a service ID "4" which does not exist in calendar.txt. These would be represented as two separate calendar_dates but right now result in two separate ScheduleExceptions but with the same name. How should this use case be handled if we requiring name to be unique? image

@philip-cline How does this look? If the calendar date is not assoicated with a calendar the name is made up of service id and the first date. For only-in-calendar-dates-txt-20170916 there is only one entry so strictly speaking the appended date is not needed, but for consistency I've kept it.

image

@br648 br648 assigned philip-cline and unassigned br648 Aug 29, 2023
@philip-cline
Copy link
Collaborator

Thanks yeah this is looking good. I'll start on a front end PR for this.

Copy link
Collaborator

@philip-cline philip-cline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM to me now

@philip-cline philip-cline assigned br648 and unassigned philip-cline Nov 14, 2023
@philip-cline philip-cline merged commit 9bc752d into dev Nov 14, 2023
2 checks passed
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

Successfully merging this pull request may close these issues.

None yet

3 participants