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

Update pending courses in prof view #769

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

Conversation

Sydney-tran
Copy link
Contributor

@Sydney-tran Sydney-tran commented Nov 2, 2023

Summary

This PR finishes the frontend for creating a new pending course as a professor/ta. On submit, the request sent popup is shown if the pending course is successfully added to the collection. Otherwise if the inputted course info is invalid, red error messages are shown. If the user is a professor or ta for any current courses or current pending courses, then the create a class button is gray instead of blue and disabled. If the user is a professor or ta for any current pending course, then a message that explains their pending course is in review is shown when the user hovers over the create a class button.

Invalid course info:

  • course code is not of form: group of chars, space, group of numbers
  • course name is empty string
  • course code and semester combination already exists in courses or pendingCourses collection
Screen Shot 2023-11-02 at 12 05 33 AM Screen Shot 2023-11-02 at 12 04 58 AM Screen Shot 2023-11-02 at 12 05 16 AM Screen Shot 2023-11-02 at 12 04 16 AM

Also, PR updates the createPendingCourse function to allow for creating pending courses in future semesters (dates for fall sems: aug 1st to dec 31st, dates for spring sems: jan 1st to may 31st)

Note: Need to finish notifications in another PR

Documentation:

Test Plan

  • Test request sent popup appears when valid course submitted
  • Test error message appears when course with invalid code submitted
  • Test error message appears when course with invalid name submitted
  • Test error message appears when course that already exists in courses collection submitted
  • Test error message appears when course that already exists in pendingCourses collection submitted
  • Test create a class button is gray when user is already prof/ta of current course
  • Test create a class button is gray when user is already prof/ta of current pendingCourse
  • Test hover message appears when user is prof/ta of current pendingCourse

Notes

  • Firebase must be updated to include pendingCourses
  • PR must be merged in at same time as other PRs for creating pending courses

Breaking Changes

None

  • I have updated the documentation accordingly.
  • My PR adds a @ts-ignore

@Sydney-tran Sydney-tran requested a review from a team as a code owner November 2, 2023 04:08
@dti-github-bot
Copy link
Member

[diff-counting] Significant lines: 846.

Copy link
Contributor

@burninglilies burninglilies left a comment

Choose a reason for hiding this comment

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

Awesome work!! I looked through everything and all components work as intended when I'm running this myself. I tested for random edge cases as well like having a pending course then being added as a TA for another course and the hover message still displays. The code looks great, I left a note about the CourseCreatePopup and am not sure what you mean in the PR note about merging in the same time as other PR's for creating pending courses.
Hopefully we can get another review or two but aside from that I'm approving this PR for now so that you can merge this into master whenever you feel is ready.

Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot to do this for my original commit can you add .CourseCreatePopup { ... } around everything in here and move it into /styles

@swang235
Copy link
Contributor

Hi Sydney, good work! The feature looks very straightforward for users, and I saw that everything was functional during the demo at standup :). Your code also is pretty readable, it's very clean and organized! I see that there are two messages that confirm the submission of a course request (the 1st and last image). It seems that the 'create a class' button is disabled after a request is submitted- does that mean that the user can only create one class at a time?

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