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: update createdAt and updatedAt fields to store timestamps in UTC #155

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

HemalDesai
Copy link
Contributor

Feature Title[Follow conventional title. See: https://www.conventionalcommits.org/en/v1.0.0/]

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Fixes Bug (Fixes an existing functionality which was not working as expected)
  • Documentation change (Changes to Readme.md)

Describe your changes

[A brief overview of the feature being added. Explain the functionality and its intended purpose.]

Screenshots/Videos (if applicable)

[Attach screenshots or videos demonstrating the new feature in action.]
Loom Video: loom

Checklist

  • I have taken a (git pull origin main --rebase) from main branch and tested it.
  • I have conducted a self-review of my code, and the code contains only changes relevant to this feature, without including modifications to other files.
  • I have commented my code, particularly in complex areas.
  • I have checked for compatibility with other parts of the codebase by manually running the site and doing actions not related to my changes.

Comment on lines 51 to 52
createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(),
updatedAt: timestamp('updated_at', { withTimezone: true }).defaultNow().notNull(),
Copy link
Member

Choose a reason for hiding this comment

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

why this?

Copy link
Member

Choose a reason for hiding this comment

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

If you need timezone, convert it to client's time zone on client-side. Keep default timezone to UTC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

{withTimezone: true} takes care that the timestamp stored in DB is in UTC format

Copy link
Member

Choose a reason for hiding this comment

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

understood, did some research. give me some time to merge.
Question: what will happen to existing dates? which are already in database?
As per my research, data will still be the same, it's just postgres will assume they are in UTC. However, since server is in us-east, i.e. UTC−05:00 current timestamps are stored in that format. Now how do we deal with this? Will we have to write a seed which retrieves time, converts to UTC and saves it back?

Copy link
Member

@amandesai01 amandesai01 Sep 16, 2024

Choose a reason for hiding this comment

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

As per debugging, current timestamps are saved in UTC, so no need for seed.

server/db/schema.ts Outdated Show resolved Hide resolved
@amandesai01
Copy link
Member

Final resolution: timestamps are already stored in UTC. So we won't be storing timezone, rather, we will be converting them to client timezone in client side. We will use some date library to do this.

@HemalDesai

@amandesai01 amandesai01 merged commit 8b0fd75 into profilecity:main Sep 16, 2024
1 check passed
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