🐕🦺 Feat/6 API Service Layer #40
Conversation
…/daco into feat/5-setup-docker-compose
…/daco into feat/6-api-service-layer
…Canadian-Genome-Library/daco into feat/6-api-service-layer
…/daco into feat/6-api-service-layer
…/daco into feat/6-api-service-layer
demariadaniel
force-pushed
the
feat/6-api-service-layer
branch
from
1c8d677 to
70cfdfb
Compare
|
|
||
| import { applications } from '../db/schemas/applications.js'; | ||
|
|
||
| export type ApplicationUpdates = Partial<typeof applications.$inferInsert>; |
| startWith: 1, | ||
| increment: 1, | ||
| minValue: 1, | ||
| maxValue: 9223372036854775807, |
There was a problem hiding this comment.
This was generated using drizzle-kit pull so it's the auto-generated upper limit for a BigInt
There was a problem hiding this comment.
I think this file and other files generated by drizzle-kit pull are not required, as we're already managing the database schemas using schema files located at src/db/schemas/.
There was a problem hiding this comment.
Leo and I chatted about this -- there is a disconnect between the DBML schema input (expects an object) and the Drizzle config schema definition (string glob path to schemas folder './schemas/**')
This should be refactored so that both point to the same schema definition, my only hesitation is to limit the scope of this PR. I will update this and the test files in the next branch.
| institutionCity: varchar('institution_city', { length: 255 }), | ||
| institutionStreetAddress: text('institution_street_address'), | ||
| institutionPostalCode: varchar('institution_postal_code', { length: 255 }), | ||
| institutionBuilding: varchar('institution_building', { length: 255 }), |
There was a problem hiding this comment.
out of curiosity, whats the default length if not specified?
There was a problem hiding this comment.
If character varying (or varchar) is used without length specifier, the type accepts strings of any length.
https://www.postgresql.org/docs/current/datatype-character.html
https://orm.drizzle.team/docs/column-types/pg#varchar
Drizzle says nothing so referring to PG docs
| @@ -6,14 +6,16 @@ | |||
| "scripts": { | |||
| "build": "rimraf ./dist && tsc", | |||
| "dev": "tsx watch --env-file=.env ./src/main.ts", | |||
| "test": "tsx --env-file=.env.test --test --test-reporter=spec --experimental-test-coverage ./src/tests/**/*", | |||
| "test": "npx tsx --test --test-reporter=spec --experimental-test-coverage ./src/tests/**/*", | |||
There was a problem hiding this comment.
shouldn't need npx here, tsx is installed in dev dependencies.
| DRAFT = 'DRAFT', | ||
| INSTITUTIONAL_REP_REVIEW = 'INSTITUTIONAL_REP_REVIEW', | ||
| DAC_REVIEW = 'DAC_REVIEW', | ||
| DAC_REVISIONS_REQUESTED = 'DAC_REVISIONS_REQUESTED', | ||
| REJECTED = 'REJECTED', | ||
| APPROVED = 'APPROVED', | ||
| CLOSED = 'CLOSED', | ||
| REVOKED = 'REVOKED', |
There was a problem hiding this comment.
Important to use these named enum values if we are using TS enums, so thank you for this update. We can take a moment to review our enum usage in a future change.
| }; | ||
|
|
||
| try { | ||
| const application = await db.transaction(async (transaction) => { |
…/daco into feat/6-api-service-layer
There was a problem hiding this comment.
just wondering, should test/ folder be at top level along the src/. also consider changing extension to differentiate the actual code like .test.ts or .spec.ts
There was a problem hiding this comment.
True, argo-clinical does have test at the same level as src and we have generally used the .spec convention
I need these changes in main to unblock other work so I can incorporate these suggestions into the next branch
#6
updatedAtfield to Applications