-
Notifications
You must be signed in to change notification settings - Fork 65
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 automatic row timestamps for Accounts and Groups tables #1674
Changes from all commits
a22412f
e0e8031
fbdd512
ef083d9
b20a89d
a872017
b1a9b0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,11 @@ | ||
import { GroupSchema } from '@/datasources/accounts/entities/group.entity'; | ||
import { RowSchema } from '@/datasources/db/entities/row.entity'; | ||
import { AddressSchema } from '@/validation/entities/schemas/address.schema'; | ||
import { z } from 'zod'; | ||
|
||
export type Account = z.infer<typeof AccountSchema>; | ||
|
||
export const AccountSchema = z.object({ | ||
id: z.number().int(), | ||
export const AccountSchema = RowSchema.extend({ | ||
group_id: GroupSchema.shape.id, | ||
address: AddressSchema, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
import { RowSchema } from '@/datasources/db/entities/row.entity'; | ||
import { z } from 'zod'; | ||
|
||
export type Group = z.infer<typeof GroupSchema>; | ||
|
||
export const GroupSchema = z.object({ | ||
id: z.number().int(), | ||
}); | ||
export const GroupSchema = RowSchema; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { z } from 'zod'; | ||
|
||
export type Row = z.infer<typeof RowSchema>; | ||
|
||
/** | ||
* Note: this is a base schema for all entities that are meant to be persisted to the database. | ||
* The 'id' field is a primary key, and the 'created_at' and 'updated_at' fields are timestamps. | ||
* These fields shouldn't be modified by the application, and should be managed by the database. | ||
*/ | ||
export const RowSchema = z.object({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: as we will be extending this for all tables, I would add some comments here as I've seen issues when using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added a comment in b1a9b0f Please let me know if you want to rephrase it or add something else! |
||
id: z.number().int(), | ||
created_at: z.date(), | ||
updated_at: z.date(), | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,11 @@ type Migration = { | |
name: string; | ||
}; | ||
|
||
type TestResult<BeforeType, AfterType> = { | ||
before: BeforeType | undefined; | ||
after: AfterType; | ||
}; | ||
|
||
/** | ||
* Migrates a Postgres database using SQL and JavaScript files. | ||
* | ||
|
@@ -51,7 +56,8 @@ export class PostgresDatabaseMigrator { | |
} | ||
|
||
/** | ||
* @private migrates up to/allows for querying before/after migration to test it. | ||
* Migrates up to/allows for querying before/after migration to test it. | ||
* Uses generics to allow the caller to specify the return type of the before/after functions. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Love this! |
||
* | ||
* Note: each migration is ran in separate transaction to allow queries in between. | ||
* | ||
|
@@ -72,15 +78,12 @@ export class PostgresDatabaseMigrator { | |
* expect(result.after).toStrictEqual(expected); | ||
* ``` | ||
*/ | ||
async test(args: { | ||
async test<BeforeType, AfterType>(args: { | ||
migration: string; | ||
before?: (sql: Sql) => Promise<unknown>; | ||
after: (sql: Sql) => Promise<unknown>; | ||
before?: (sql: Sql) => Promise<BeforeType>; | ||
after: (sql: Sql) => Promise<AfterType>; | ||
folder?: string; | ||
}): Promise<{ | ||
before: unknown; | ||
after: unknown; | ||
}> { | ||
}): Promise<TestResult<BeforeType, AfterType>> { | ||
const migrations = this.getMigrations( | ||
args.folder ?? PostgresDatabaseMigrator.MIGRATIONS_FOLDER, | ||
); | ||
|
@@ -97,21 +100,25 @@ export class PostgresDatabaseMigrator { | |
// Get migrations up to the specified migration | ||
const migrationsToTest = migrations.slice(0, migrationIndex + 1); | ||
|
||
let before: unknown; | ||
let before: BeforeType | undefined; | ||
|
||
for await (const migration of migrationsToTest) { | ||
const isMigrationBeingTested = migration.path.includes(args.migration); | ||
|
||
if (isMigrationBeingTested && args.before) { | ||
before = await args.before(this.sql).catch(() => undefined); | ||
before = await args.before(this.sql).catch((err) => { | ||
throw Error(`Error running before function: ${err}`); | ||
}); | ||
} | ||
|
||
await this.sql.begin((transaction) => { | ||
return this.run({ transaction, migration }); | ||
}); | ||
} | ||
|
||
const after = await args.after(this.sql).catch(() => undefined); | ||
const after = await args.after(this.sql).catch((err) => { | ||
throw Error(`Error running after function: ${err}`); | ||
}); | ||
|
||
return { before, after }; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: let's add another test that then ensures the
created_at
doesn't change butupdated_at
does after as theafter
callback combines all queries in one transaction.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a test in b1a9b0f