Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the tokenFields query in the legacy storage module to wrap the last_used_at field in a to_json call. Feedback suggests reverting this and the existing to_json for created_at, recommending that timestamp parsing be handled via Zod coercion in the model layer instead. This aligns with the architectural goal of moving logic out of the legacy storage package and ensuring robust data handling.
| , to_json("created_at") AS "date" | ||
| , "last_used_at" AS "lastUsedAt" | ||
| , to_json("last_used_at") AS "lastUsedAt" |
There was a problem hiding this comment.
New database interaction logic should be encapsulated in smaller classes within the corresponding GraphQL modules, not added to the legacy /packages/services/storage module. Additionally, using z.coerce.date() in the TokenModel is a more robust approach for handling timestamps. Note that the database driver returns timestamps as Unix timestamp numbers in milliseconds, so Zod coercion is appropriate to ensure they are treated as Date objects.
| , to_json("created_at") AS "date" | |
| , "last_used_at" AS "lastUsedAt" | |
| , to_json("last_used_at") AS "lastUsedAt" | |
| , "created_at" AS "date" | |
| , "last_used_at" AS "lastUsedAt" |
References
- New database interaction logic should be encapsulated in smaller classes within the corresponding GraphQL modules, not added to the legacy /packages/services/storage module.
- The database driver returns timestamps as Unix timestamp numbers in milliseconds, not seconds.
|
🐋 This PR was built and pushed to the following Docker images: Targets: Platforms: Image Tag: |
Background
This one slipped QA on #7887
It did not get caught by a integration test as the token is cached in redis after creation. This test case creates the database record using the database directly, to simulate the read of a non-cached access token.