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

Add updated at column #66

Merged
merged 14 commits into from
Nov 1, 2024
Merged

Add updated at column #66

merged 14 commits into from
Nov 1, 2024

Conversation

koji-m
Copy link
Contributor

@koji-m koji-m commented Oct 31, 2024

Summary

ref. #50

In this PR, I will add columns that represent the update datetime (timestamp) of records to several tables in the DB.
Currently, add a column that records record update timestamps only to tables that have record creation timestamps.
(Exclude the files and requests tables as they are not used.)

Related Issue

Changes

  • Added the updatedAt column to the tables below. (drizzle/schema.ts)
    • agents
    • organizations
    • teams
  • Added migration to apply schema changes to DB (migrations/0005_overrated_scarlet_spider.sql)

Testing

  • Create an agent.
    • Verify that new records are created in agents and updated_at == created_at is set.
  • Modify agent structure. (ex. add some nodes)
    • Verify that updated_at of the modified agent's record has been updated.
  • Create an account.
    • Verify that new records are created in organizations and teams and updated_at == created_at is set.

Other Information

  • In the column definition of schema.ts, defaultNow() is specified, so the initial value of update_at for records that already exist are set to the current timestamp. So that, it does not accurately reflect the actual last update time. The alternatives considered are:
    • Same value as created_at
      • Like the current timestamp, it does not accurately reflect the actual last update time.
      • After migration, I will need to manually set the updated_at value to the created_at value.
      • Therefore, there is no reason to update to the same value as created_at after changing the schema.
    • NULL
      • This column does not allow nulls.

Copy link

vercel bot commented Oct 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
giselle ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 1, 2024 2:56am

Copy link
Contributor

@toyamarinyon toyamarinyon left a comment

Choose a reason for hiding this comment

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

Thanks, I will check it out! I think it's generally good, but I did comment on one point, so please check it out.

@@ -1,6 +1,6 @@
# giselle

As of October 29, 2024 10:23am. 926 total
As of October 31, 2024 8:07am. 926 total
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this change is unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were no changes to the dependencies, but since bun.lockb was included in the early commit, that file was automatically updated. I reverted it because it is unnecessary. thank you. cb4dad8

Copy link
Member

@shige shige left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@toyamarinyon toyamarinyon left a comment

Choose a reason for hiding this comment

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

👍

@koji-m
Copy link
Contributor Author

koji-m commented Nov 1, 2024

Thank you!

@koji-m koji-m merged commit a019f60 into main Nov 1, 2024
9 checks passed
@koji-m koji-m deleted the add-updated-at-column branch November 1, 2024 03:57
@shige
Copy link
Member

shige commented Nov 1, 2024

FYI: @koji-m @toyamarinyon
I executed the following command!

bun drizzle-kit migrate

@koji-m
Copy link
Contributor Author

koji-m commented Nov 1, 2024

Thank you 🙏

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