Fix API documentation and client types for various endpoints - #945
Merged
Merged
Conversation
Three endpoints returned a list but declared a single object, so the generated client typed them as one item and consumers could not iterate the response: GET /analytics/events count per event GET /scans/analytics/events scans per event GET /hackathons every hackathon GET /hackathons is the awkward one. It returns an array when active is omitted or false, and a single object when active=true, which duplicates GET /hackathons/active. Nothing in adminv2, finance-dashboard, inventory, check-in, sponsor, auth or emails passes active=true, so the array is what every real caller receives. It is declared as an array and the quirk is described on the operation rather than papered over. Worth collapsing into /hackathons/active separately, which would be a behaviour change. ApiDoc gains an optional description for exactly this: prose a summary cannot carry. No runtime change; only the document and the types derived from it.
* fix(api): describe three response and request shapes accurately All three were found by migrating adminv2 onto the generated client, and all three made the client wrong rather than merely incomplete. RegistrationWithScoreDto omitted firstName and lastName. Both score endpoints join the user table and select them, so every response carried fields the schema denied existed, and consumers could not read the applicant's name without casting. PatchFlagsBody.flags used a bare @ApiProperty on an object array. TypeScript types are erased, so Swagger cannot see the element type and emitted string[]. The endpoint takes FlagEntity objects, so the generated client described a body the API would reject. Declared as @ApiProperty({ type: [FlagEntity] }). GET /sponsors declared hackathonId as required while the handler has it optional, so the client demanded an argument that was never needed. Same class of mistake as the active parameter fixed earlier; an audit of every optional @query against its ApiDoc entry found this was the only one left. No runtime change: handlers, guards and validation are untouched. * chore: mark the OpenAPI document and generated client as generated Collapses them in pull request diffs and signals that they are never hand-edited. CI already fails when either is stale; this makes the intent visible in review. * fix(api): declare boolean columns that were described as objects Four boolean properties are written with an initializer and no type annotation: travelReimbursement = false; TypeScript infers the type for the compiler but emitDecoratorMetadata has nothing to reflect, so it records Object and the document described each one as an untyped object. The generated client typed them `{ [key: string]: unknown }`, which made `registration.travelReimbursement === true` a compile error in adminv2 rather than a boolean comparison. Declaring type: Boolean on @ApiProperty fixes all four, across every schema that includes them: RegistrationEntity, CreateUserRegistrationEntity, RegistrationWithScoreDto and the three Finance shapes, so twelve schema properties in total. Found by auditing the document for properties emitted as bare objects. The six remaining are correct: UserProfileResponse.registration carries an allOf $ref, and the metadata and mail data fields really are free-form Record<string, any>. No runtime change.
POST /finances takes a receipt and POST /organizer-applications takes a
resume, both through a FileInterceptor, but neither body schema declared
the file. The document described those endpoints as multipart bodies with
no binary field, so the generated client had no parameter to pass the
upload through and the feature was unreachable from a typed client.
Declare each as { type: "string", format: "binary" }, matching how
EventCreateEntity already declares its icon.
Found by auditing every multipart route in the document against the
binary fields in its body schema; these two were the only ones missing.
No runtime change: the interceptors and handlers are untouched.
AuthGuard read minimumRole only from the provider, so an app could gate everything or nothing. frontend-template leaves most routes public and gates only its (protected) group, which the SDK could not express: raising the provider's role would have put the marketing pages behind a login. Accept an optional minimumRole prop that overrides the provider default for that subtree. Provider-level config remains the default when the prop is absent, so existing usage is unchanged.
Five responses were documented with hand-written inline schemas. Being unnamed, the generated client produced throwaway types like PhotoGetAllPhotos200Item with every field optional, so consumers could not read photo.name without a null check even though the handler always returns it. Worse, the inline pagination schema had drifted from the service. It described page, limit and total; PhotoService returns currentPage, totalItems and totalPages. Anything typed against the document was reading fields that are never sent. Declare them as classes in photo.types.ts instead. The interfaces there could not carry @ApiProperty, which is why the inline schemas existed in the first place. POST /photos/upload also had no request body at all in the document despite taking a photo file and a fileType, so the endpoint was unreachable from a typed client. Same failure as the finance receipt and the application resume, in a route my earlier multipart audit missed because it only inspected routes that already declared a body. GET /users/export/data becomes UserRegistrationExportRow. No runtime change.
Fourteen properties carry @IsOptional() or a default initializer, so the API accepts a request without them, but their @ApiProperty omitted required: false and the document therefore declared them required. The generated client demanded fields the server never needed: registration travelReimbursement, driving, firstHackathon, hackathonId, applicationStatus sponsor lightLogo, darkLogo event description, locationId reservation teamId flag isEnabled notification scheduleTime, metadata, topic frontend-template's registration form has always omitted driving and applicationStatus and works in production; against the generated types it failed to compile. applicationStatus is optional through its property initializer rather than @IsOptional(): the model supplies PENDING, so a client never sends it. Found by auditing every @IsOptional() property against its @ApiProperty. This only widens the client's input types, so no existing caller breaks. No runtime change: validation decorators are untouched.
…941) Six persisted columns carried @column and class-validator decorators but no @ApiProperty, so they were absent from the document entirely and the generated client could not send or read them: scan organizerId, which the event check-in endpoint requires, so check-in was impossible from a typed client registration shareAddressMlh, shareAddressSponsors, shareEmailMlh finance amount inventory reason The two resume downloads declared StreamableFile as their response type. That is a Nest transport wrapper rather than a response shape, and it leaked into the document as an empty object, so the generated client typed a zip archive as an object with no properties. Both routes already set a Content-Type header; the document now matches, and StreamableFile is gone from components.schemas. GET /users/resumes application/zip GET /users/{id}/resume application/pdf Found by auditing every @column against its @ApiProperty, and by tracing why the sponsor scanner could not build a check-in payload. No runtime change.
* fix(sdk): re-export the client by name instead of with export * The barrel did `export * from "@hackpsu/api-client"` inside a module that declares "use client" for its providers. Next rejects that combination: It's currently unsupported to use "export *" in a client boundary. adminv2, finance-dashboard and frontend-template happened not to trip it; sponsor did, and failed to compile. Generate the re-export list from the client's own emitted export statement instead: 861 values and 663 types, named explicitly. Consumers keep a single import, and the list cannot drift because it is derived rather than written. Bundling the client into this package was the other option. It removed the star export from the JavaScript but left it in the declarations, since tsup's dts bundler still treated the workspace package as external, so the types disappeared instead. src/generated-reexports.ts is gitignored and rebuilt by yarn build. * fix(sdk): regenerate re-exports before build and typecheck The build script change was lost when package.json was reverted to restore a version stamp, so CI ran tsup without generating the re-export file and failed on a module that does not exist in a clean checkout. Typecheck needs it too, for the same reason.
…#944) The auth service already returns the session as a token, and its sessionUser route already accepts Authorization: Bearer, but nothing on the client ever consumed either. Neither the hand-written providers nor this package read the authToken the redirect carries, so a developer on localhost had no way to hold a session: the cookie is scoped to .hackpsu.org and unreadable there. Capture the token from the redirect, strip it from the URL so it does not sit in the address bar, history or referrer, keep it in sessionStorage for later navigations, and attach it to sessionUser and sessionLogout. On hackpsu.org none of this runs. No token is issued, so the header is absent and the cookie continues to carry the session exactly as before. Requires the matching auth service change: until then the token is never issued, because that server decided cookie versus token auth from the request's Origin rather than from where the session was going.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.