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.

Sign up for GitHub

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

Jump to bottom

Semesters singleton migration #723

Merged
merged 13 commits into from
Mar 5, 2023
Merged

Semesters singleton migration #723

merged 13 commits into from
Mar 5, 2023

Conversation

andxu282
Copy link
Collaborator

@andxu282 andxu282 commented Oct 18, 2022 •
edited
Loading

Summary

Begins the migration script to move semesters to a list of semesters by writing to a new field called plans that has a list of a list of semesters for each user.

  • Added field called plans and makes it so that it is written to whenever semesters is written to
  • Read from plans whenever reading from semesters

Remaining TODOs:

  • Run migration script in dev/prod
  • Remove semester field
  • Run remove script in dev/prod

Test Plan

@andxu282 andxu282 requested a review from a team as a code owner October 18, 2022 00:00
@dti-github-bot
Copy link
Member

dti-github-bot commented Oct 18, 2022 •
edited
Loading

[diff-counting] Significant lines: 66.

@andxu282 andxu282 marked this pull request as draft October 18, 2022 00:02
@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2022 •
edited
Loading

Visit the preview URL for this PR (updated for commit 57d3276):

https://cornelldti-courseplan-dev--pr723-semesters-singleton-vtj336ie.web.app

(expires Tue, 04 Apr 2023 00:03:41 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 6d4e0437c4559ed895272bbd63991394f1e0e933

@benjamin-shen benjamin-shen mentioned this pull request Oct 19, 2022
Merged
7 tasks
@andxu282 andxu282 marked this pull request as ready for review October 24, 2022 23:17
benjamin-shen
benjamin-shen reviewed Oct 28, 2022
View reviewed changes
Copy link
Collaborator

@benjamin-shen benjamin-shen left a comment

Choose a reason for hiding this comment

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

Nice job, this looks pretty good! In this PR can you also start reading from plans, if the field exists?

Could you also

  • flesh out the TODO steps
  • include some sample commands in the PR description

zachary-kent
zachary-kent reviewed Feb 22, 2023
View reviewed changes
src/utilities.ts Outdated Show resolved Hide resolved
@noschiff
Copy link
Member

noschiff commented Mar 4, 2023

@zachary0kent what's the status of this PR?

zachary-kent
zachary-kent approved these changes Mar 5, 2023
View reviewed changes
Copy link
Collaborator

@zachary-kent zachary-kent left a comment

Choose a reason for hiding this comment

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

Looks good, I ran the migration script on my dev account and everything works as expected.

@zachary-kent zachary-kent merged commit 51d8c92 into master Mar 5, 2023
@zachary-kent zachary-kent deleted the semesters-singleton-migration branch March 5, 2023 00:11
@zachary-kent
Copy link
Collaborator

@zachary0kent what's the status of this PR?

done

@noschiff noschiff mentioned this pull request Apr 26, 2023
Merged
22 tasks
@noschiff noschiff mentioned this pull request May 9, 2023
Merged
@noschiff noschiff mentioned this pull request May 9, 2023
Merged
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjamin-shen benjamin-shen benjamin-shen left review comments

@zachary-kent zachary-kent zachary-kent approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@andxu282 @dti-github-bot @noschiff @zachary-kent @benjamin-shen