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

Schedule Review Table Data Fetch #259

Merged
merged 13 commits into from
Apr 22, 2022
Merged

Conversation

hujoseph99
Copy link
Contributor

@hujoseph99 hujoseph99 commented Apr 2, 2022

Ticket link

Closes #162

Implementation description

  • Created new endpoint shiftsWithSignupsAndVolunteersByPosting
  • This is a draft PR, still have to add the front end data-fetching

Steps to test

  1. Populate database as in this Built out shift signup endpoints #90
  2. Add skills and branches for the user with the following queries: `
  • insert into skills values (1, "test skill");
  • insert into branches values (1, "test branch");
  • insert into "_SkillToVolunteer" values (1, 1);
  • insert into "_BranchToVolunteer" values (1, 1);
  1. Test the query to see if it works as expected in :5000/graphql
  2. Go to frontend route /admin/scheduleTableDemo to see demo of query
  3. Input appropriate posting ID, user ID, and signup status for testing

Note that the queries createShifts, createShiftSignups, and updateShiftSignup will likely be useful for creating shifts and signing up to them.

What should reviewers focus on?

  • The reviewer should focus on checking to see if the table is populated as expected with the different filters for posting ID, user ID, and signup status.

Checklist

  • My PR name is descriptive and in imperative tense
  • My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
  • I have run the appropriate linter(s)
  • I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

@hujoseph99 hujoseph99 requested a review from a team April 2, 2022 20:50
@hujoseph99 hujoseph99 force-pushed the joseph/schedule-review-table-data branch from e010ec8 to 833ff65 Compare April 3, 2022 17:54
@github-actions
Copy link

github-actions bot commented Apr 3, 2022

Visit the preview URL for this PR (updated for commit 7c7b8e3):

https://sistering-dev--pr259-joseph-schedule-revi-rnqlxck9.web.app

(expires Fri, 29 Apr 2022 00:42:39 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@hujoseph99 hujoseph99 changed the title DRAFT: Schedule Review Table Data Fetch Schedule Review Table Data Fetch Apr 3, 2022
@hujoseph99 hujoseph99 marked this pull request as ready for review April 3, 2022 18:23
Copy link
Member

@LenaNguyen LenaNguyen left a comment

Choose a reason for hiding this comment

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

looking good, the main changes have to do with 1) removing skills + branches from the response and 2) add another where filter to the prisma query for userId and signupStatus

backend/services/implementations/shiftService.ts Outdated Show resolved Hide resolved
backend/services/implementations/shiftService.ts Outdated Show resolved Hide resolved
backend/services/implementations/shiftService.ts Outdated Show resolved Hide resolved
backend/services/implementations/shiftService.ts Outdated Show resolved Hide resolved
backend/services/implementations/shiftService.ts Outdated Show resolved Hide resolved
Comment on lines 323 to 334
const firebaseUser = await firebaseAdmin
.auth()
.getUser(signup.user.authId);

filteredSignups.push(
this.convertSignupResponeWithUserAndVolunteerToDTO(
signup,
shift.startTime,
shift.endTime,
firebaseUser.email ?? "",
),
);
Copy link
Member

Choose a reason for hiding this comment

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

No action needed on this pr but we should probably store user emails in the db so we don't have to make a call to firebase every time. We'll do this in the future!

backend/services/implementations/shiftService.ts Outdated Show resolved Hide resolved
backend/services/implementations/shiftService.ts Outdated Show resolved Hide resolved
Copy link
Member

@LenaNguyen LenaNguyen left a comment

Choose a reason for hiding this comment

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

remember to run yarn fix to address linter errors :)

@hujoseph99 hujoseph99 force-pushed the joseph/schedule-review-table-data branch from 222a876 to c517b89 Compare April 21, 2022 22:50
Copy link
Member

@LenaNguyen LenaNguyen left a comment

Choose a reason for hiding this comment

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

Thanks for incorporating all the changes! So glad to have this merged in :))

@hujoseph99 hujoseph99 merged commit 1d01881 into main Apr 22, 2022
@hujoseph99 hujoseph99 deleted the joseph/schedule-review-table-data branch April 22, 2022 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schedule Review Table Data Fetch
2 participants