- Notifications
You must be signed in to change notification settings - Fork0
Start work on API endpoints and API access used by frontend adapters#79
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
0fc6207 tobb1cdfaComparebb1cdfa to6a45b80Compare
sjaghori left a comment
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.
Looks good! 🚀
I got some comments...
| "kysely":"^0.27.3", | ||
| "pino":"^9.0.0", | ||
| "pino-pretty":"^11.0.0", | ||
| "uuid":"^9.0.1", |
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.
since node v20 the std. crypto library has builtin uuid v4 generator. Let's not add the dependency here.
| import{createTableMigration}from'../migration.util' | ||
| exportasyncfunctionup(db:Kysely<unknown>):Promise<void>{ | ||
| awaitcreateTableMigration(db,'apiaccess') |
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.
| awaitcreateTableMigration(db,'apiaccess') | |
| awaitcreateTableMigration(db,'api_access') |
| exportasyncfunctionup(db:Kysely<unknown>):Promise<void>{ | ||
| awaitcreateTableMigration(db,'apiaccess') | ||
| .addColumn('apikey','text',(col)=>col.unique().notNull()) |
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.
| .addColumn('apikey','text',(col)=>col.unique().notNull()) | |
| .addColumn('api_key','text',(col)=>col.unique().notNull()) |
what do you think?
| import{z}from'zod' | ||
| consttranslationKeySchema=z.string().brand('translation-key') | ||
| exporttypeTranslationKey=z.infer<typeoftranslationKeySchema> |
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.
Do we want to keep this in this folder?
| @@ -1 +1 @@ | |||
| pnpm exec lint-staged | |||
| npm test | |||
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.
we also want to run lint-staged no?
| } | ||
| returnvalidationResult.data | ||
| } |
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.
Great, we got something similar in production, here is the snippet.
Theoratically the request payload clould be a formdata as well, let's support both json and formdata.
importtype{Logger}from'pino'import{ZodError,typez}from'zod'typeParsedRequestData<Textendsz.ZodType>=|{status:'success'data:z.output<T>}|{status:'error'data:{validationErrorMessage:stringcode:number}}exportconstparseRequestData=async<Textendsz.ZodType>(schema:T,request:Request,logger:Logger,kind:'json'|'from-data'='json'):Promise<ParsedRequestData<T>>=>{letparsedPayload:z.output<typeofschema>try{letpayload:unknownif(kind==='json'){payload=awaitrequest.json()}else{payload=Object.fromEntries(awaitrequest.formData())}parsedPayload=schema.parse(payload)}catch(e:unknown){letvalidationErrorMessage:string='Error parsing request payload.'letcode=500if(einstanceofZodError){validationErrorMessage=`Error validating request payload against zod schema.${e.message}`code=400logger.error({message:validationErrorMessage,name:e.name,errors:e.issues})}else{logger.error(validationErrorMessage)}return{status:'error',data:{ validationErrorMessage, code}}}return{status:'success',data:parsedPayload}}
This PR starts adding some groundwork necessary for the endpoints that allow frontend adapters to sync.
Among other minor things, it: