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

Filter out drivers in Reassign drivers for scheduled rides based on availability. #514

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

namanhboi
Copy link
Contributor

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.

image

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

@namanhboi namanhboi requested a review from a team as a code owner May 5, 2024 18:41
@dti-github-bot
Copy link
Member

dti-github-bot commented May 5, 2024

[diff-counting] Significant lines: 78.

Atikpui007
Atikpui007 previously approved these changes May 6, 2024
Copy link
Collaborator

@Atikpui007 Atikpui007 left a 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';
Copy link
Collaborator

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

Copy link
Contributor Author

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.

Copy link
Contributor

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

@namanhboi
Copy link
Contributor Author

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.

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.

4 participants