feat: add file attachment (s3File) support#14
feat: add file attachment (s3File) support#14dseeker wants to merge 1 commit intokarelklima:mainfrom
Conversation
dseeker
commented
Feb 7, 2026
dseeker
commented
- Add getFileUrl() method to Client for downloading file attachments
- Add s3File getter to List class with full metadata (filename, type, dimensions)
- Add s3File to JSON export output
- Update schema validation for s3File metadata
- Fix LoginResultSchema errors type to z.any() for flexibility
- Add comprehensive tests for s3File feature
- Update all export mock files to include file attachment examples
- Update README with usage examples
- Add getFileUrl() method to Client for downloading file attachments - Add s3File getter to List class with full metadata (filename, type, dimensions) - Add s3File to JSON export output - Update schema validation for s3File metadata - Fix LoginResultSchema errors type to z.any() for flexibility - Add comprehensive tests for s3File feature - Update all export mock files to include file attachment examples - Update README with usage examples
karelklima
left a comment
karelklima
left a comment
There was a problem hiding this comment.
Thanks for the PR, there are a bunch of good things there. I appreciate the effort, especially regarding tests and documentation.
Major points:
- There are two distinct endpoints for getting singed URL - one is for previews, the other is for downloading the original file.
- The
getFileUrl()method would be better placed inListto keep the API of the library consistent; combined with the point above, we might needgetFileUrl()to get the URL to the original andgetPreviewUrl(maxWidth, maxHeight)to get the preview.
I think this will be a nice contribution to the library once the review issues are addressed.
| nodeId: string, | ||
| maxWidth: number = 800, | ||
| maxHeight: number = 800, | ||
| ): Promise<{ url: string }> { |
There was a problem hiding this comment.
This method would be better placed in the List class so that the file URL can be retrieved directly from a WF item. That way the userId and nodeId would be provided for free from context.
Second, the return type should be a string (the actual URL, as the name of the function suggests), no need to return an object here.
| maxHeight: number = 800, | ||
| ): Promise<{ url: string }> { | ||
| const dimensions = `${maxWidth}x${maxHeight}`; | ||
| const url = `${WORKFLOWY_URL}/file-proxy/signed-preview/${userId}/${nodeId}/${dimensions}/?attempt=1`; |
There was a problem hiding this comment.
Based on my research, there are two distinct endpoints - signed-preview and signed-original. I think we need to distinguish between the two. Only the signed-preview accepts dimensions.
| imageOriginalWidth?: number; | ||
| imageOriginalHeight?: number; | ||
| imageOriginalPixels?: number; | ||
| } | undefined { |
There was a problem hiding this comment.
The complex type here should be inferred from the zod schema, similar to e.g. TreeItem or TreeItemShareInfo, so that it is defined in a single place only.
| imageOriginalWidth?: number; | ||
| imageOriginalHeight?: number; | ||
| imageOriginalPixels?: number; | ||
| } | undefined { |
There was a problem hiding this comment.
The complex type here should be inferred from the zod schema, similar to e.g. TreeItem or TreeItemShareInfo, so that it is defined in a single place only.
| } | ||
|
|
||
| /** S3 file attachment metadata, or undefined if no file attached */ | ||
| public get s3File(): { |
There was a problem hiding this comment.
Why not just file()? The S3 part is an implementation detail that should not leak to the interface of this library.
| export const LoginResultSchema = z.object({ | ||
| success: z.boolean().optional(), | ||
| errors: z.object().optional(), | ||
| errors: z.any().optional(), |
| isMirrorRoot: z.boolean().optional(), | ||
| }).optional(), | ||
| s3File: z.object({ | ||
| isFile: z.boolean(), |
There was a problem hiding this comment.
This field feels redundant and should not be included in the interface of the library.
| isFile: z.boolean(), | ||
| fileName: z.string(), | ||
| fileType: z.string(), | ||
| objectFolder: z.string(), |
There was a problem hiding this comment.
Do we need this field? If not let's remove it.
|
|
||
| ```typescript | ||
| // Check if a list has a file attachment | ||
| if (myList.s3File) { |
There was a problem hiding this comment.
Maybe some hasFile() helper would be beneficial in the List class.
|
|
||
| // Get initialization data to obtain userId | ||
| const initData = await client.getInitializationData(); | ||
| const userId = initData.mainProjectTreeInfo.ownerId; |
There was a problem hiding this comment.
This step will not be necessary if the getFileUrl() method is moved to the List.
2 participants
