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.

Sign up for GitHub

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

Jump to bottom

teacher profile unstyled #20

Merged

135ze merged 12 commits into david/gcal-export from evelina/teacher-profile-unstyled

Oct 23, 2024

Collaborator

135ze commented Jul 11, 2024 •

edited by ChinemeremChigbo

Loading

Notion ticket link

Implement Unstyled Teacher Profile Page

Implementation description

add profile.tsx page with user info, absences, sign out
update Dockerfile for prisma schema
add endpoint to fetch user info based on email from gauth (userid and num of absences)
add endpoint to fetch user absences given user id
update .env to replace localhost with db (??) so db:5432 instead

Steps to test

run docker
ensure db is seeded (see PR Seeding script #12 and David/gcal export #19 for details)
log in with google on localhost:3000
go to localhost:3000/profile and validate info is correct
go to console to ensure the email logged is correct (code has a fake email from the seeded db)

What should reviewers focus on?

Dockerfile changes
whether profile.tsx has React best practices

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

135ze requested review from ChinemeremChigbo and ludavidca

July 11, 2024 18:08

135ze self-assigned this

135ze force-pushed the evelina/teacher-profile-unstyled branch 2 times, most recently from f1f9ee0 to a254945 Compare

July 11, 2024 20:31

Member

ChinemeremChigbo commented Jul 11, 2024

Please address the failing build issue and warning:

https://github.com/uwblueprint/sistema/actions/runs/9898109170/job/27344151094?pr=20

For the failing build I know what needs to be fixed so feel free to contact me if you need some help with that!

ChinemeremChigbo force-pushed the evelina/teacher-profile-unstyled branch from 00a4962 to 1b74398 Compare

July 18, 2024 22:29

135ze force-pushed the evelina/teacher-profile-unstyled branch from 4077550 to 1b74398 Compare

July 18, 2024 23:36

135ze marked this pull request as draft

July 18, 2024 23:42

135ze force-pushed the evelina/teacher-profile-unstyled branch from bec1d1f to fd3b9cb Compare

July 25, 2024 23:09

vercel bot commented Jul 25, 2024 •

edited

Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sistema	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 23, 2024 0:03am

vercel bot had a problem deploying to Preview

July 25, 2024 23:09

Failure

135ze force-pushed the evelina/teacher-profile-unstyled branch from fd3b9cb to c1616ea Compare

July 26, 2024 00:25

vercel bot had a problem deploying to Preview

July 26, 2024 00:25

Failure

135ze marked this pull request as ready for review

July 26, 2024 00:27

135ze force-pushed the evelina/teacher-profile-unstyled branch from c1616ea to 040860f Compare

July 26, 2024 00:31

vercel bot had a problem deploying to Preview

July 26, 2024 00:31

Failure

vercel bot had a problem deploying to Preview

July 27, 2024 01:28

Failure

135ze force-pushed the evelina/teacher-profile-unstyled branch from 001136e to 065d8f8 Compare

July 27, 2024 16:28

vercel bot had a problem deploying to Preview

July 27, 2024 16:28

Failure

ChinemeremChigbo reviewed

View reviewed changes

Member

ChinemeremChigbo left a comment

Please fix the deployment and re-request a review

135ze force-pushed the evelina/teacher-profile-unstyled branch from 065d8f8 to 3196ba3 Compare

July 27, 2024 16:36

vercel bot had a problem deploying to Preview

July 27, 2024 16:37

Failure

vercel bot had a problem deploying to Preview

July 27, 2024 16:46

Failure

135ze force-pushed the evelina/teacher-profile-unstyled branch from 5ef981b to 8e5384e Compare

July 27, 2024 17:39

vercel bot had a problem deploying to Preview

July 27, 2024 17:39

Failure

135ze force-pushed the evelina/teacher-profile-unstyled branch from 8e5384e to 6c09b39 Compare

July 27, 2024 17:42

vercel bot had a problem deploying to Preview

July 27, 2024 17:42

Failure

135ze force-pushed the evelina/teacher-profile-unstyled branch from 6c09b39 to 8e4947c Compare

July 27, 2024 17:44

vercel bot deployed to Preview

July 27, 2024 17:45

View deployment


          review comments and formatting

b7446bc

135ze force-pushed the evelina/teacher-profile-unstyled branch from 3424cbe to b7446bc Compare

October 9, 2024 00:35

vercel bot deployed to Preview

October 9, 2024 00:36

View deployment

Collaborator Author

135ze commented Oct 9, 2024

Looks good to me :p. I would just make these small changes. But the PR looks great overall and works very well locally :)

Thanks! Updated to comments

135ze requested a review from ludavidca

October 9, 2024 00:37


          move api to pages/api

2e3e233

vercel bot deployed to Preview

October 9, 2024 01:23

View deployment

ChinemeremChigbo requested changes

View reviewed changes

src/pages/api/users/email/[email].ts Outdated

+                if (encodedAbsences) {
+                  getAbsences = Boolean(encodedAbsences);
+                }
+                // TODO: rename realEmail to email to be used in the findUnique later

Member

ChinemeremChigbo Oct 9, 2024

I don't like having these TODOs in the code. Please make a Notion Ticket for the remaining work 🥺🙏

Member

ChinemeremChigbo Oct 12, 2024

I think Rahul volunteered to make that Notion ticket. Please coordinate with him to ensure that it is accurate.

Member

ChinemeremChigbo Oct 12, 2024

He said that he would add it to this ticket: https://www.notion.so/uwblueprintexecs/Teacher-Profile-Popup-Frontend-0e847ff1bef04037afd2b4d104875afa?pvs=4

src/pages/api/users/email/[email].ts Outdated

+                try {
+                  // TODO: remove block once emails are set up
+                  // fake stuff begins

Member

ChinemeremChigbo Oct 9, 2024

Don't love these comments. I think you should just get rid of them lol.

Collaborator Author

135ze Oct 16, 2024

Removed in latest

src/pages/api/users/email/[email].ts Outdated

+              import { PrismaClient } from '@prisma/client';
+              import type { NextApiRequest, NextApiResponse } from 'next';
+              const prisma = new PrismaClient();

Member

ChinemeremChigbo Oct 9, 2024

We shouldn't be remaking the PrismaClient each time. We should use the singleton pattern as documented here: https://www.prisma.io/docs/orm/more/help-and-troubleshooting/help-articles/nextjs-prisma-client-dev-practices. I mentioned this in David's PR here: https://github.com/uwblueprint/sistema/pull/19/files#r1789262220, so please have centralized Prisma access like he does in his PR.

Collaborator Author

135ze Oct 19, 2024

Built on top of his branch to share the same utils/prisma here:
bdf4ec0

src/pages/api/users/email/[email].ts Outdated

+                  } else {
+                    email = useFake ? fakeEmailReq.email : realEmail;
+                  }
+                  // fake stuff ends

Member

ChinemeremChigbo Oct 12, 2024

Don't like this comment.

src/pages/api/users/email/[email].ts Outdated

+                  });
+                  if (!user) {
+                    res.status(400).json({ error: 'User not found' });

Member

ChinemeremChigbo Oct 12, 2024

should this be a return?

Collaborator Author

135ze Oct 16, 2024 •

edited

Loading

No need - existing api code doesn't use return either:

sistema/src/pages/api/sendEmail.ts

Line 42 in 6800776

res.status(200).json({ message: 'Email sent successfully' });

src/pages/api/users/email/[email].ts Outdated

+                  if (!user) {
+                    res.status(400).json({ error: 'User not found' });
+                  } else {
+                    res.status(200).json(user);

Member

ChinemeremChigbo Oct 12, 2024

should this be a return?

src/pages/api/users/email/[email].ts Outdated

+                    res.status(200).json(user);
+                  }
+                } catch {
+                  res.status(500).json({ error: 'Internal server error' });

Member

ChinemeremChigbo Oct 12, 2024

should this be a return?

src/pages/api/users/email/[email].ts Outdated

+                  });
+                  let email;
+                  if (!fakeEmailReq) {
+                    res.status(400).json({ error: 'Database is empty' });

Member

ChinemeremChigbo Oct 12, 2024

should there be a return instead?

src/pages/profile.tsx Outdated Show resolved Hide resolved

src/pages/profile.tsx Show resolved Hide resolved


          Refactor useFake logic.

d4313b9

vercel bot deployed to Preview

October 16, 2024 01:32

View deployment


          add UserData interface to api response

vercel bot deployed to Preview

October 19, 2024 21:56

View deployment

135ze added 2 commits

October 19, 2024 17:59


          Merge branch 'david/gcal-export' into evelina/teacher-profile-unstyled

717785e


          update prisma to use singleton pattern

bdf4ec0

135ze changed the base branch from main to david/gcal-export

October 19, 2024 22:04

135ze requested a review from ChinemeremChigbo

October 19, 2024 22:05

vercel bot deployed to Preview

October 19, 2024 22:06

View deployment

ChinemeremChigbo reviewed

View reviewed changes

src/pages/api/users/email/[email].ts Outdated Show resolved Hide resolved

ChinemeremChigbo approved these changes

View reviewed changes

Member

ChinemeremChigbo left a comment

A few minor changes I would appreciate if you made. Otherwise LGTM.

src/pages/profile.tsx Outdated Show resolved Hide resolved

135ze and others added 2 commits

October 22, 2024 19:52


          Update src/pages/api/users/email/[email].ts

8dc0cb0

Co-authored-by: Chinemerem <[email protected]>


          Update src/pages/profile.tsx

Co-authored-by: Chinemerem <[email protected]>

vercel bot had a problem deploying to Preview

October 22, 2024 23:53

Failure


          fix formatting

bddecc0

vercel bot deployed to Preview

October 23, 2024 00:03

View deployment

135ze merged commit a81f4e1 into david/gcal-export

2 checks passed

135ze added a commit that referenced this pull request


          Revert "teacher profile unstyled (#20)"

e9a3a05

This reverts commit a81f4e1.

135ze mentioned this pull request

Revert "teacher profile unstyled" #37

Merged

135ze added a commit that referenced this pull request


          Revert "teacher profile unstyled (#20)" (#37)

84457c5

This reverts commit a81f4e1. sorry

135ze mentioned this pull request

Evelina/teacher profile unstyled #38

Merged

ludavidca added a commit that referenced this pull request


          David/gcal export (#19)

2976c44

* Add Prisma and create Prisma models

* Add MailingList table

* Add seeding script commmand

* Add seeding script

* Add seeding script command in package.json

* Fix code formatting with prettier

* Fix prettier issues

* First Commit

* Added preliminary files

* Fixed Querying Prisma issue via troubleshooting docker

* Implemented ICS hosting Feature

* Tidied up code, ran lint and prettier

* Created ICS Branch.

Squashing :)

* Added necessary files

* Remove uneeded changes

* Remove uneeded changes

* Remove uneeded changes

* Update schema.prisma

* Remove unused variables

* Move seed.config.ts back

* Added ics

* Fixed Docker issues

* Testing commit

* Testing commit

* Testing commit

* Testing commit

* Fixed map issues

* Fixed map issues

* Testing

* Testing

* fixed lock file

* Update src/pages/ics.tsx

Co-authored-by: Chinemerem <[email protected]>

* Commited some changes

* Update app/api/getAbsences/route.ts

Co-authored-by: Chinemerem <[email protected]>

* Update app/api/getAbsences/route.ts

Co-authored-by: Chinemerem <[email protected]>

* test

* Made changes based on CEO's instructions

* fixed merge issues

* Made Changes based on CEO's guidance

* teacher profile unstyled (#20)

* add unstyled teacher profile page with gauth info and x/10 absences

---------

Co-authored-by: Rahul Tandon <[email protected]>
Co-authored-by: David Lu <[email protected]>
Co-authored-by: Chinemerem <[email protected]>

* Revert "teacher profile unstyled (#20)" (#37)

This reverts commit a81f4e1. sorry

* Update utils/prisma.ts

Co-authored-by: Chinemerem <[email protected]>

* Update src/pages/ics.tsx

Co-authored-by: Chinemerem <[email protected]>

* Update app/api/getAbsences/route.ts

Co-authored-by: Chinemerem <[email protected]>

* Revert Original Files

* Fix ICS as per Chinemerem's request

* Yeehaw

* Update app/api/getAbsences/route.ts

Co-authored-by: Chinemerem <[email protected]>

* Update app/api/getAbsences/route.ts

Co-authored-by: Chinemerem <[email protected]>

* Update app/api/getAbsences/route.ts

Co-authored-by: Chinemerem <[email protected]>

---------

Co-authored-by: Kelly Pham <[email protected]>
Co-authored-by: Vihaan Jagiasi <[email protected]>
Co-authored-by: evelina <[email protected]>
Co-authored-by: Chinemerem <[email protected]>
Co-authored-by: evelina zheng <[email protected]>
Co-authored-by: Rahul Tandon <[email protected]>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet