fix: Use correct API parameter names for thread and inbox timestamp filtering#117
fix: Use correct API parameter names for thread and inbox timestamp filtering#117pauloslund wants to merge 4 commits intomainfrom
Conversation
…iltering Add newerThan/olderThan Date params to threads and inbox clients, mapping them to the correct snake_case API parameters. Deprecate the old since/until params on inbox client with backward-compatible fallback. Add tests for both new and deprecated parameter paths.
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR updates the threads and inbox clients to properly support timestamp filtering by introducing newerThan and olderThan parameters and mapping them to the correct API formats. These updates effectively align the SDK with the API's expected parameters and improve the clarity of the public contract. A few minor adjustments are noted to preserve backward compatibility for existing snake_case inputs in the threads client and to ensure a limit value of zero is not silently ignored.
src/clients/threads-client.ts
Outdated
| const method = 'GET' | ||
| const url = `${ENDPOINT_THREADS}/get` | ||
| const params = args | ||
| const params: Record<string, unknown> = { |
There was a problem hiding this comment.
[P1] Rebuilding params from scratch here drops the previously documented newer_than_ts / older_than_ts inputs for getThreads(). Before this change, those snake_case keys were forwarded through request() and worked for JS consumers; after this refactor they are silently ignored, so existing calls can start returning unfiltered threads. Please add a deprecated fallback for the old keys, similar to the inbox/comments migrations, before removing that runtime path.
There was a problem hiding this comment.
Fixed in 639ff5a. Switched back to the passthrough pattern — we now destructure only the timestamp fields and spread ...rest through. Added deprecated newerThanTs/olderThanTs number fields on GetThreadsArgs (which auto-convert to newer_than_ts/older_than_ts via snakeCaseKeys), with newerThan Date taking precedence when both are provided.
src/clients/threads-client.ts
Outdated
| if (args.archived != null) params.archived = args.archived | ||
| if (args.newerThan) params.newer_than_ts = Math.floor(args.newerThan.getTime() / 1000) | ||
| if (args.olderThan) params.older_than_ts = Math.floor(args.olderThan.getTime() / 1000) | ||
| if (args.limit) params.limit = args.limit |
There was a problem hiding this comment.
[P2] This truthy check means the new limit support silently ignores an explicit 0. GetThreadsArgsSchema now accepts any number, so callers can pass 0 and get a full result set instead of either limit=0 or a validation error. Using args.limit != null here, or tightening the schema to positive integers, would keep the implementation aligned with the public contract.
…werThanTs/olderThanTs Instead of rebuilding params from scratch, use the spread/passthrough pattern consistent with other methods. Add deprecated newerThanTs and olderThanTs number fields so existing JS consumers passing the old snake_case keys continue to work.
3 participants
Summary
newerThanandolderThanDate parameters toThreadsClient.getThreads()andInboxClient.getInbox()/archiveAll(), properly converting them to Unix timestamps for the APInewer_than_ts,older_than_ts,since_ts_or_obj_idx,until_ts_or_obj_idx)since/untilparams on inbox client with backward-compatible??fallbacklimitparam support togetThreadsinbox-client.test.tsfileTest plan
newerThan/olderThancorrectly map to API params in threads and inbox clientssince/untilstill work via fallback