Skip to content

Add semester season property from semester type#593

Merged
benjamin-shen merged 1 commit into
masterfrom
ben/semester-season-migration-1
Nov 17, 2021
Merged

Add semester season property from semester type#593
benjamin-shen merged 1 commit into
masterfrom
ben/semester-season-migration-1

Conversation

@benjamin-shen

Copy link
Copy Markdown
Collaborator

Summary

This pull request is a script to migrate all old instances of semester type to semester season. After this, the schema will include identical values for type and season (instead of just including type). I ran it on dev, it just needs to be run on prod.

Depends on #546

Test Plan

npm run ts-node -- src/requirements/admin/semester-season-migration <user email> to dry-run on a single user.
npm run ts-node -- src/requirements/admin/semester-season-migration --run-on-db to run on the entire database.

Pasted Graphic

@benjamin-shen benjamin-shen requested a review from a team as a code owner November 16, 2021 22:26
@dti-github-bot

Copy link
Copy Markdown
Member

[diff-counting] Significant lines: 48.

@github-actions

Copy link
Copy Markdown
Contributor

Visit the preview URL for this PR (updated for commit 1d5ec49):

https://cornelldti-courseplan-dev--pr593-ben-semester-season-f322e1pe.web.app

(expires Tue, 23 Nov 2021 22:28:40 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@benjamin-shen benjamin-shen changed the title Semester type => semester season Add semester season property from semester type Nov 16, 2021
Comment on lines +19 to +20
// each semester should have type but not necessarily season
({ ...semester, season: semester.season || semester.type })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe also do some cleanup to remove the old type field?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There's currently still a dependency on type in the code. There was probably a better way to do it 😭 but I plan on doing the cleanup in a future script.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh, I remember why I did it this way. It was because I didn't want to update the database schema while prod and other devs were still using type, so I split up the migration into multiple parts

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.

3 participants