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

Emma/volunteer assigning #24

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Emma/volunteer assigning #24

wants to merge 17 commits into from

Conversation

emmaz12
Copy link

@emmaz12 emmaz12 commented Mar 4, 2025

Tracking Info

Resolves #15

Changes

Added dialog to assign a volunteer to a veteran. Created model and routing for active volunteers. Added additional endpoints to user model. Implemented and styled the UI of the dialog. Full functionality of dialog is completed.

Note: Dialog requires the program, and veteran being assigned as the User object as props.

Testing

Verified on both the frontend and backend that the dialog is working as expected and that the information is properly being fetched and saved to/from the backend.

##Confirmation of Changes

Screen.Recording.2025-03-03.at.7.31.04.PM.mov
Screen.Recording.2025-03-14.at.12.48.40.AM.mov

@emmaz12 emmaz12 requested a review from Anthonyp0329 as a code owner March 4, 2025 03:45
</p>
</div>
<a className={styles.profileLink} href="google.com">
View Profile
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this go to the veteran's profile?

Copy link
Author

Choose a reason for hiding this comment

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

I believe its supposed to go to the volunteer's profile. However, its not currently developed yet. I just linked it to google for now until the volunteer profile page is made. Will add a todo above it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's make the href /profile/${selectedVolunteerOption.value._id}

const data = (await response.json()) as ActiveVolunteer;

// Second fetch to update assigned veterans on User schema
const requestBodyVeteran = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the program also be passed into requestBodyVeteran, since the updateUser controller accesses both program and veteranEmail?:
const { program, veteranEmail } = req.body;

I seem to also be running into a validation error whenever I try to assign a volunteer to a veteran - it seems the phone number and zip code fields are required but not being passed into the body, preventing me from saving the user. Not sure if this is just due to differing versions; are the phone number and zip code fields marked as required in the mongo schema in this branch?

Copy link
Author

Choose a reason for hiding this comment

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

In theory, the user object should already contain the program anyways cause the options to select a volunteer is filtered by their program. So the only field that would be changed would be the assigned veteran field in the user object.

Yes, I realized that they changed the user model to require zip and phone now. We should probably update all the current test users in the db to reflect this change. Many the users that are being fetched into the dialog, don't have a zip/phone when they were created so its causing errors. Just tested it again with a valid user object containing both zip and phone, seems to be working.

Copy link
Contributor

@srikar-eranky srikar-eranky left a comment

Choose a reason for hiding this comment

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

Looks great overall! Just one main thing - I was getting a validation error when trying to save a volunteer to a veteran, might have to check if the User Schema matches up as it seems like the zip code and phone number fields are required. I left some other miscellaneous comments on the PR. Other than that, looks good to me.

emmaz12 added 3 commits March 7, 2025 16:20
… having assignedUsers vs userController addUser having assignedVeteran/volunteer instead of assignedUsers
…troller addUser having assignedVeteran/volunteer instead of assignedUsers
Copy link
Collaborator

@Anthonyp0329 Anthonyp0329 left a comment

Choose a reason for hiding this comment

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

Looks great Emma, thanks for the awesome work! Just some small refactoring fixes to do that I noted in the comments. Also, yep we need to clean up the database to make sure every user is updated with the latest schema. I'll try handling that this weekend. Also, once the profile page pr is merged, can you link the volunteer assigning dialogue to pop up when the appropriate button is pressed, with the appropriate volunteer/program? Thank you!!

</p>
</div>
<a className={styles.profileLink} href="google.com">
View Profile
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's make the href /profile/${selectedVolunteerOption.value._id}

Copy link
Contributor

@navyaa31 navyaa31 left a comment

Choose a reason for hiding this comment

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

Great work! Just left one comment.

};

//add a volunteer using their user email, program, and assigned veteran email
export const addVolunteer = async (req, res) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to validate userEmail and veteranEmail before assigning?

Copy link
Contributor

@srikar-eranky srikar-eranky left a comment

Choose a reason for hiding this comment

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

LGTM!

@navyaa31 navyaa31 self-requested a review March 10, 2025 06:26
Copy link
Contributor

@navyaa31 navyaa31 left a comment

Choose a reason for hiding this comment

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

looks good to me, just left one 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.

Assigning Volunteer to Veteran Flow
4 participants