Skip to content

Rework RideType for new standardized schema#598

Open
benkoppe wants to merge 3 commits into
masterfrom
bek76/new_rides
Open

Rework RideType for new standardized schema#598
benkoppe wants to merge 3 commits into
masterfrom
bek76/new_rides

Conversation

@benkoppe

@benkoppe benkoppe commented Oct 6, 2025

Copy link
Copy Markdown
Contributor

Summary

This PR is meant to mark progress on a reworked schema for rides across the codebase. Right now, it only contains a few TODO comments outlining a basic plan for implementation, and should not be merged. It mostly serves to easily link to this design document and serve as an indicator for future progress. This PR will be updated as that progress materializes.

This pull request is the first step towards implementing the new RideType:

  • documented basic outline for implementation with TODO comments
  • implement minimum necessary data structure for basic functionality
  • implement conflicts in final schedule with students' requests
  • recurrent rides
  • ensuring all parts of Carriage still work

Test Plan

We will need rigorous testing to ensure that rides function correctly under this new schema.

Breaking Changes

  • Database schema change (anything that changes DynamoDB document structure)
  • Platform-wide functionality reimplementation needed

@benkoppe benkoppe requested a review from a team as a code owner October 6, 2025 02:28
@CLAassistant

CLAassistant commented Oct 6, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@dti-github-bot

Copy link
Copy Markdown
Member

[diff-counting] Significant lines: 22.

@benkoppe benkoppe mentioned this pull request Oct 6, 2025
2 tasks

@mjaydenkim mjaydenkim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this isn't done yet but my notes so far - this overall list of TODOs makes sense and I don't see any problems with it (except for what I noted about the name LOL)!

Comment thread server/src/models/ride.ts

Copy link
Copy Markdown
Contributor

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 RideType2 on top of the existing RideType will be confusing -- maybe we should name it RideTypeWithRecurrence or something b/c it might be poor documentation-wise to just name it RideType2

Copy link
Copy Markdown
Contributor Author

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 original RideType and rename RideType2. But this might be difficult for the external dynamoDB, so I'm not sure.

@anika-4444 anika-4444 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some notes on ride design doc:

  • The requested vs final fields is a good idea!
  • For bounds on how far into the future a ride can be requested, a per-semester basis might also be a good idea. Because students use CULift to get to their classes/on campus activities (which change between fall and spring), I think it makes sense to limit recurrences there. That way someone can't create a recurring ride in May that CULift will have to manually delete as it runs into June.
  • Will operational status be updated by the driver (after a ride has been completed) or using time? If a driver forgets to update status, what will happen?

Comment thread server/src/models/ride.ts
- driver?
- requestedPickupTime
- finalPickupTime?
- requestedDropoffTime

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

Base automatically changed from dka34/changes to master November 2, 2025 21:57
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.

5 participants