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

feat: RateMyProf integration #494

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

feat: RateMyProf integration #494

wants to merge 5 commits into from

Conversation

keithradford
Copy link
Collaborator

Description

Closes #436

Screenshots

Screen Shot 2023-07-21 at 11 25 29 AM

Checklist

  • The code follows all style guidelines.
  • The code passes all required tests.
  • The code is documented.
  • The code includes tests.
  • I have self-reviewed my changes and have done QA.

General Comments

@keithradford keithradford requested a review from a team as a code owner July 21, 2023 18:25
@vercel
Copy link

vercel bot commented Jul 21, 2023

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

Name Status Preview Comments Updated (UTC)
courseup ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 21, 2023 6:31pm


export const RMP_GRAPHQL_URL = 'https://www.ratemyprofessors.com/graphql';
export const RMP_UVIC_ID = 'U2Nob29sLTE0ODg=';
export const RMP_AUTH_TOKEN = 'dGVzdDp0ZXN0';

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical

The hard-coded value "dGVzdDp0ZXN0" is used as
authorization header
.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These aren't actually credentials, the auth token is public and not cycled on ratemyprof's end. Weird it's needed at all, but blame them.

Copy link
Contributor

@szeckirjr szeckirjr left a comment

Choose a reason for hiding this comment

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

this looks sweet 👀 def super useful!

where did you find info on the API? I can't seem to find any kind of schema or type on the kind of info we could request, was just wondering if you ran into anything

{meetingTimes.map(
(m, i) =>
m.instructors.length > 0 && (
<Td>
Copy link
Contributor

Choose a reason for hiding this comment

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

getting error for missing key here

@@ -77,7 +91,7 @@ export function Schedule({ meetingTimes }: ScheduleProps): JSX.Element {
{/* TODO: verify if we can safely exclude this for most cases */}
{/* <Th>Schedule Type</Th> */}
<Th>Location</Th>
<Th>Instructors</Th>
{professor.length > 0 && <Th>Instructors</Th>}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused on the professor variable here, since it's still an array will all profs from that course no? why not use m.instructors length directly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll clean this up

@keithradford
Copy link
Collaborator Author

this looks sweet 👀 def super useful!

where did you find info on the API? I can't seem to find any kind of schema or type on the kind of info we could request, was just wondering if you ran into anything

Since it's graphql, it has a schema built in. Nothing is public, but when you put ratemyprofessor.com/graphql into insomnia or Apollo playground it loads in the scheme for you to view

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.

feat: RateMyProf details for professors
2 participants