illinois / queue

A microservice queue for holding open office hours
University of Illinois/NCSA Open Source License
82 stars 36 forks source link

Course Settings Page #293

Closed rittikaadhikari closed 4 years ago

rittikaadhikari commented 4 years ago

This PR creates a course settings page, which allows each course to configure the following:

image

image

vercel[bot] commented 4 years ago

This pull request is being automatically deployed with ZEIT Now (learn more). To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/illinois/queue/ewyqttpzpc 🌍 Preview: In Progress

rittikaadhikari commented 4 years ago

Thanks @nwalters512 for all of the comments! I think I've addressed pretty much all of the comments.

rittikaadhikari commented 4 years ago

Alright, I think I've gotten everything now (barring whether we should allow courses to change their name or shortcode). Lmk if there's anything else!

rittikaadhikari commented 4 years ago

@nwalters512 I've addressed everything in the comments -- I tried making a darkmode CSS for the table, but it could probably be improved on, so I'll open an issue for it. As for the icons, I could only find a reasonable icon for deleting a queue on the queue settings page - not sure about an icon for general or for admission control. Lmk if there's anything else!

nwalters512 commented 4 years ago

Regarding icons: maybe https://fontawesome.com/icons/hand-paper?style=solid for admission control and https://fontawesome.com/icons/sliders-h?style=solid for both general settings panels?

nwalters512 commented 4 years ago

The tests are failing due to different orders in columns between the Sequelize models and the migrations. Take a look at https://github.com/illinois/queue/blob/master/src/migrations/20190308215151-queue-admission-control.js#L10 for an example of how to control where the new column is inserted.

That linked migration is also a good example of wrapping multiple table alterations inside a single transaction - I'd still prefer that we do that here.

rittikaadhikari commented 4 years ago

Okay, I think I've covered everything! :) Lmk if I'm missing something else

rittikaadhikari commented 4 years ago

@nwalters512 I think we should be good! Is there anything else to add?