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

Zot4plan import button #555

Merged
merged 22 commits into from
Jan 28, 2025
Merged

Zot4plan import button #555

merged 22 commits into from
Jan 28, 2025

Conversation

CadenLee2
Copy link
Contributor

There is now a button on the roadmap page for users to import a schedule from Zot4Plan.
This opens a modal prompting them for their Zot4Plan schedule name and their year (to determine the starting year for their roadmap).
They then can import the schedule from Zot4Plan, which is formatted and loaded locally as a new roadmap in PeterPortal with the same name; the roadmap is not saved to their account/browser (yet).

Description

  • Added the button and modal
    • The modal requires the user to input a schedule name that is 8 characters or longer because no Zot4Plan schedule is less than 8 characters
    • There is a helpful screenshot to clarify to users which schedule name is being asked for
  • On the backend, in a new route, PeterPortal now uses Zot4Plan's API to retrieve the schedule by name, then formats it into the correct roadmap format (the same format that PeterPortal roadmaps are saved as)
    • PeterPortal course names are Zot4Plan course names without the spaces. No exceptions have been found yet, but there are safeguards in place in case an invalid course name is encountered (see below)
    • Zot4Plan by default stores all 4 years, each with 4 quarters. Unnecessary (empty) additional years are trimmed off during conversion, and if the summer quarter is blank, it is not included
  • On the frontend, the "saved roadmap" format is expanded into the full roadmap format with course details and is loaded into Redux using a dispatch
    • The new roadmap is automatically selected
    • Fixed an issue with the roadmap selection dropdown where the name displayed did not properly update from state
  • Error handling/checking on the client side:
    • If the schedule was not found/imported successfully, the user is notified via a toast and it is not loaded
    • If during the expanding of the roadmap, courses with unrecognized names were found, they are removed and the user is notified how many problematic courses there were. This was never actually seen during testing with real Zot4Plan roadmaps, but artificially making the route return a fake course name caused the screen to go blank due to errors, so it is good to handle this just in case (especially if the roadmap is automatically saved in the future)
    • PeterPortal does not allow multiple roadmaps of the same name. If the imported roadmap has the same name as a current one, a "+" is appended to the end until there are no more conflicts (a simple solution because users can easily edit their roadmap names)

Screenshots

The import modal:
Screenshot 2025-01-23 204800
A Zot4Plan schedule and the imported roadmap side by side:
Screenshot 2025-01-23 204948

Test Plan

  • Test loading various schedules from Zot4Plan. You can make schedules there by clicking "save" then typing in the new schedule name.
    • Existing schedules I used for testing include test_migration, test_migration2, and test_migration_complex
    • Try loading a schedule name that does not exist
    • Try loading a schedule name that is the same as an existing roadmap name in PeterPortal (notice the "+"'s appended to prevent name collision)
    • Try having/not having summer classes, having empty years in various places
  • Code: the schedule conversion/roadmap verification code specifically is quite bulky because of all the various steps involved, but I tried to make it readable with comments and naming. This is a somewhat temporary feature so it isn't the most elegant, but if there are any glaring issues they can be fixed. Ideally it can be easily extended for the upcoming APs/auto saving features.

Issues

Closes #547

@CadenLee2 CadenLee2 added this to the Zot4Plan Migration milestone Jan 24, 2025
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 24, 2025 05:06 — with GitHub Actions Inactive
api/src/controllers/zot4planimport.ts Outdated Show resolved Hide resolved
api/src/controllers/zot4planimport.ts Outdated Show resolved Hide resolved
api/src/controllers/zot4planimport.ts Outdated Show resolved Hide resolved
api/src/controllers/zot4planimport.ts Outdated Show resolved Hide resolved
api/src/controllers/zot4planimport.ts Outdated Show resolved Hide resolved
site/src/pages/RoadmapPage/ImportZot4PlanPopup.tsx Outdated Show resolved Hide resolved
site/src/pages/RoadmapPage/ImportZot4PlanPopup.tsx Outdated Show resolved Hide resolved
site/src/pages/RoadmapPage/ImportZot4PlanPopup.tsx Outdated Show resolved Hide resolved
site/src/pages/RoadmapPage/ImportZot4PlanPopup.tsx Outdated Show resolved Hide resolved
site/src/pages/RoadmapPage/ImportZot4PlanPopup.tsx Outdated Show resolved Hide resolved
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 27, 2025 18:21 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 27, 2025 18:32 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 27, 2025 18:59 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 27, 2025 19:21 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 27, 2025 19:30 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 27, 2025 19:59 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 27, 2025 20:13 — with GitHub Actions Inactive
Copy link
Member

@Awesome-E Awesome-E left a comment

Choose a reason for hiding this comment

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

It looks like you might still be working on stuff so I'll leave this comment here for now. Re-request review once you're ready for me to take another look

api/src/controllers/zot4planimport.ts Show resolved Hide resolved
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 28, 2025 00:23 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 28, 2025 00:25 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 28, 2025 00:27 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 temporarily deployed to staging-555 January 28, 2025 00:30 — with GitHub Actions Inactive
@CadenLee2 CadenLee2 requested a review from Awesome-E January 28, 2025 00:32
Copy link
Member

@Awesome-E Awesome-E left a comment

Choose a reason for hiding this comment

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

Will raise this as a new issue, but since Zot4Plan only offers 1 summer, the best assumption to make would probably be to do it as a 10 week summer

@Awesome-E
Copy link
Member

You're good to merge now

@CadenLee2 CadenLee2 merged commit ae97888 into main Jan 28, 2025
2 checks passed
@CadenLee2 CadenLee2 deleted the zot4plan-import-button branch January 28, 2025 00:54
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.

There should be a button to import 4-year plans from Zot4Plan
2 participants