-
Notifications
You must be signed in to change notification settings - Fork 3
Rework RideType for new standardized schema
#598
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,23 @@ export enum Status { | |
|
|
||
| // Use the proper LocationType instead of custom RideLocation | ||
|
|
||
| /* TODO: Create new type `RideType2` that contains | ||
| - id | ||
| - riders | ||
| - driver? | ||
| - requestedPickupTime | ||
| - finalPickupTime? | ||
| - requestedDropoffTime | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the user does not know exactly how long their ride will take, I'm not sure requested drop off time is a necessary field. Currently, the modal for student and admin ride creation should not allow for the user to pick their drop off time according to the bug bash notes. However, I still think storing a general end time is a good idea, especially for the day calendar that displays rides. I'm currently working on using Google Maps data to add a ride duration estimate (with added buffer minutes) for the student and admin ride view. That data might be better here than requested drop off time. |
||
| - finalDropoffTime? | ||
| - requestedPickupLocation | ||
| - finalPickupLocation? | ||
| - requestedDropoffLocation | ||
| - finalDropoffLocation? | ||
| - schedulingState: create type | ||
| - operationalStatus: create type | ||
| - recurrenceId? | ||
| - breaksRecurrence? | ||
| */ | ||
| export type RideType = { | ||
| id: string; | ||
| type: Type; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very small note b/c this schema looks great to me, but I feel like creating a
RideType2on top of the existingRideTypewill be confusing -- maybe we should name itRideTypeWithRecurrenceor something b/c it might be poor documentation-wise to just name itRideType2There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's definitely only meant to be a temporary switch from
RideType. I thought it might make sense to have a separate type that avoids deleting/modifying the original, so we don't break the entire codebase at once, and then at the end we can delete the originalRideTypeand renameRideType2. But this might be difficult for the external dynamoDB, so I'm not sure.