Skip to content

Conversation

@douglas-xt
Copy link
Contributor

@douglas-xt douglas-xt commented Feb 9, 2026

What does this PR do?

  • Ensures each file version displays the actual file modification timestamp instead of the version creation date
  • Preserves the exact moment when users modified their files in the version history

Why are we doing this?

  • Users cannot track when they actually modified a file because all versions currently show the date they became versions, not when the file was modified
  • This prevents users from properly identifying when changes were made and restoring files to the correct point in time

How are we doing this?

  • Use file.updatedAt when creating file versions
  • Each version now maintains its own unique modification timestamp reflecting the actual file change date

Should this be manually tested & how?

Already tested via HTTP requests by creating a file, replacing it multiple times with different timestamps, and verifying each version displays the correct modification time when listing versions.

Any background context you want to provide?

  • ⚠️ Includes migration

@douglas-xt douglas-xt requested a review from jzunigax2 as a code owner February 9, 2026 12:16
@douglas-xt douglas-xt self-assigned this Feb 9, 2026
@douglas-xt douglas-xt changed the title ix(file-versions): use file updatedAt as modification time for versions [PB-5856] fix(file-versions): use file updatedAt as modification time for versions Feb 9, 2026
@douglas-xt douglas-xt requested a review from sg-gs February 9, 2026 12:22
@douglas-xt douglas-xt changed the title [PB-5856] fix(file-versions): use file updatedAt as modification time for versions [PB-5856] fix(file-versions): use file updated time as modification time for versions Feb 9, 2026
jzunigax2
jzunigax2 previously approved these changes Feb 9, 2026
@douglas-xt douglas-xt requested review from sg-gs and removed request for sg-gs February 10, 2026 09:44
@sonarqubecloud
Copy link

Copy link
Member

@sg-gs sg-gs left a comment

Choose a reason for hiding this comment

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

My bad about contradicting you @jzunigax2, I have just realised about that. The modificationTime is the same field we use in files and in Unix-based OSes, it refers to precisely what @douglas-xt is explaining what is needed for in the description. If you want to rollback for the previous name, I will just approve it so we do not give more work to @douglas-xt.

Whatever you prefer @jzunigax2

@sg-gs
Copy link
Member

sg-gs commented Feb 10, 2026

@jzunigax2 confirmed it is okay as is, so, running migration @douglas-xt

@douglas-xt douglas-xt requested a review from sg-gs February 10, 2026 14:30
@sg-gs
Copy link
Member

sg-gs commented Feb 10, 2026

Sequelize CLI [Node: 22.16.0, CLI: 6.6.3, ORM: 6.37.7]

Loaded configuration file "src/config/sequelize.js".
Using environment "production".
== 20260207135909-add-modification-time-to-file-versions: migrating =======

ERROR: column "modification_time" of relation "file_versions" contains null values

I do not recall adding this column, did you @jzunigax2 ?
cc @douglas-xt

@jzunigax2
Copy link
Contributor

Sequelize CLI [Node: 22.16.0, CLI: 6.6.3, ORM: 6.37.7]

Loaded configuration file "src/config/sequelize.js".
Using environment "production".
== 20260207135909-add-modification-time-to-file-versions: migrating =======

ERROR: column "modification_time" of relation "file_versions" contains null values

I do not recall adding this column, did you @jzunigax2 ? cc @douglas-xt

hey @douglas-xt I believe this is due to introducing a new column with allow null false but without a default value

@douglas-xt
Copy link
Contributor Author

@sg-gs I think we need to truncate the entire file_version table to run this migration, similar to when we added the non-nullable user_id column. My bad, I didn't include this in the PR description. @jzunigax2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants