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: remove department from professor #131

Open
wants to merge 1 commit into
base: beta
Choose a base branch
from

Conversation

cjlawson02
Copy link
Collaborator

Removes the department tag from the professor

The data still exists in KV but the dependence on it in backend/frontend is removed

Copy link

Deploying polyratings with  Cloudflare Pages  Cloudflare Pages

Latest commit: f1c71eb
Status: ✅  Deploy successful!
Preview URL: https://1e3d05ef.polyratings.pages.dev
Branch Preview URL: https://remove-prof-department.polyratings.pages.dev

View logs

Copy link

nx-cloud bot commented Oct 14, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f1c71eb. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 4 targets

Sent with 💌 from NxCloud.

@cjlawson02
Copy link
Collaborator Author

I found a case where there's multiple professors with the same name at cal poly:

https://polyratings.dev/professor/2e90246e-ef43-474b-9ea7-31d9b7182b71
https://polyratings.dev/professor/0073fed6-e0f6-48f0-bdf4-e5e0eccd445c

@mfish33
Copy link
Collaborator

mfish33 commented Nov 13, 2024

The duplicate professors no longer teach at Cal Poly so it isn't a big deal. However, I could see this case being a little confusing without the department listed. I know we talked about it previously but idk if this is still the best course of action. Once I added a dropdown on the admin page I found it not to annoying when people mess up and add the wrong department initially

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.

2 participants