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

DevEx #811 Postgres migration to timestamptz #824

Open
wants to merge 70 commits into
base: main
Choose a base branch
from

Conversation

Haviles04
Copy link
Contributor

@Haviles04 Haviles04 commented Dec 1, 2023

Issue number

Relevant issue number

Please check the following

  • Do the tests still pass? (see Run the Tests)
  • Is the code formatted properly? (see Linting (Formatting))
  • For New Features:
    • Have tests been added to cover any new features or fixes?
    • Has the documentation been updated accordingly?

Please describe additional details for testing this change

@Haviles04 Haviles04 requested a review from a team December 1, 2023 20:54
@Haviles04
Copy link
Contributor Author

Haviles04 commented Dec 1, 2023

This still needs a bunch of testing and a database migration, but I got a good start on the backend changes.

@Haviles04
Copy link
Contributor Author

Haviles04 commented Dec 2, 2023

alter table "match"
add column "new_startTime" timestamptz,
add column "new_endTime" timestamptz;

update "match"
set "new_startTime" = to_timestamp("startTime"/1000.0),
	"new_endTime" = to_timestamp("endTime"/1000.0);

alter table "match"
drop column "startTime",
drop column "endTime";

alter table "match"
rename column "new_startTime" to "startTime",
rename column "new_endTime" to "endTime";

The DB migration will probably look like this. Imo it makes sense to create the new columns and check a bunch of them before deleting and renaming.

@Haviles04 Haviles04 marked this pull request as ready for review December 3, 2023 17:11
@Haviles04
Copy link
Contributor Author

Haviles04 commented Dec 7, 2023

apparently you can't use one ALTER for multiple renames

alter table "season"
add column "newStartTime" timestamptz,
add column "newEndTime" timestamptz;

update "season"
set "newStartTime" = to_timestamp("startTime"/1000.0),
	"newEndTime" = to_timestamp("endTime" /1000.0);
	
alter table "season"
drop column "startTime",
drop column "endTime";

alter table "season"
rename column "newStartTime" to "startTime";

alter table "season"
rename column "newEndTime" to "endTime";

The SQL is the same for the season and match table, just with the table name changed respectively, the game will be almost the same thing just lockedAt

@Haviles04
Copy link
Contributor Author

I tested this by staging the database on main locally and ran the first test in stats.spec.js to populate the database, I then ran the above migration on the match and season tables. Then switched branches to this PR, restarted the server with migrate: 'safe' in staging.js and was able to view all the stats from the stats fixture on the stats page.

This should probably be tested by at least one other person.

@Haviles04 Haviles04 added the version-major A large update that warrants changing the MAJOR version of the app e.g. (4.0.0 => 5.0.0) label Dec 9, 2023
Comment on lines 148 to 149
startTime: { '>': dayjs(currentSeason.startTime).format('YYYY-MM-DD HH:mm:ss.SSS Z') },
endTime: { '<': dayjs(currentSeason.endTime).format('YYYY-MM-DD HH:mm:ss.SSS Z') },
Copy link
Contributor

Choose a reason for hiding this comment

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

This string is used all over the place. Let's pull it into a utility file so that it can be imported and standardized. Since it's used in test code and backend, maybe it makes the most sense to make it json?

Something like utils/time.json with

{
    "formatString": "YYYY-MM-DD HH:mm:ss.SSS Z"
}

Haviles04 and others added 9 commits May 27, 2024 09:39
* test(videoPlayground): initialize queen video test

* test(videoPlayground): add queen test

* test(playground.config): update playground config to include all specs in and out of playground dir

* test(videoPlayground): add test for queen video

* test(videoPlayground): add spec for playing 2 to scrap royal

* test(videoPlayground): add spec for countering

* test(videoPlayground): add spec for threes

* test(videoPlayground): add spec for fours

* test(videoPlayground): add fives spec

* test(videoPlayground): add specs for eight and nine

* chore(videoPlayground): remove .only()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version-major A large update that warrants changing the MAJOR version of the app e.g. (4.0.0 => 5.0.0)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DevEx]: Update the timestamp columns across the codebase to use postgres’ timestampz data type
5 participants