-
Notifications
You must be signed in to change notification settings - Fork 4
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
Filter out drivers in Reassign drivers for scheduled rides based on availability. #514
base: master
Are you sure you want to change the base?
Conversation
[diff-counting] Significant lines: 78. |
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.
The PR looks good and I like how you used the reassignDriversFilters to allow for more filters when necessary. This was tested and it works as expected
Good job
// You can add additional filters to filter our drivers to reassign here. | ||
const reassignDriverFilters: ((driver: DriverType) => boolean)[] = [ | ||
// (driver: DriverType) => { | ||
// return driver.firstName != 'Bin laden'; |
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.
Please dont add this lol
Specifically line 42. Change the first name example
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.
Yup, forgot to removed it when I was messing around. Changed it now tho.
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.
Don't forget to push your changes 🙏 The test driver name is still there lol
I refactored the code to make some functions like isAvaiable (which might be useful in other contexts) exportable and added some needed documentation. Tested everything informatlly and it works. Also resolved any merge conflicts with the master branch at the time of writing this comment. |
Summary
When you reassign driver for scheduled rides, it will only show the drivers available. Additionally, you add can more filters into an array called reassignDriverFilters.
Test Plan
I have tested this very informally using console.log but plan to do more rigrous testing with unit tests in the future (after finals).