[Node] Document created/modified time search params on search() - #786
[Node] Document created/modified time search params on search()#786dishanthirpara-maker wants to merge 1 commit into
Conversation
#9538 What changed: - Added a JSDoc block above the search() function in lib/management/user.ts documenting fromCreatedTime, toCreatedTime, fromModifiedTime, and toModifiedTime, matching the JSDoc style already used on the deprecated searchAll() function above it. - Updated the README "Search all users" example to show fromCreatedTime/toModifiedTime usage. - No functional/type code was changed: these 4 fields already existed on the SearchRequest type (with only inline `//` comments) and were already wired through to the request body via the `...searchReq` spread in search() — they just lacked a proper JSDoc block on the function itself. - Per explicit instruction, the deprecated searchAll() function (which does not support these params) was left untouched. Verified: - Manual diff review only: the change is a JSDoc comment block plus a README edit, added directly above an existing, unmodified arrow function — no syntax risk to the surrounding code. Not verified: - No TypeScript compile/typecheck was run (`tsc --noEmit` or similar) because node_modules were not installed in this session and doing so was out of scope for this check. No integration/functional testing against a live API was performed.
|
🐕 Review complete — View session on Shuni Portal 🐾 |
🐕 Suggested ReviewersThe review assignment considers breadth of file coverage and expertise, selecting contributors who have worked on both the core logic and documentation to ensure high-quality review.
Suggested by Shuni based on git history and PR context. Names are not @-mentioned to avoid notifying anyone — request a review from whoever fits best. |
There was a problem hiding this comment.
🐕 Shuni's Review
Docs-only PR: adds a JSDoc block for user.search() covering the four time-range filters and updates the README example. No functional code touched — the fields were already on SearchRequest and already forwarded via the ...searchReq spread, and user.test.ts:1300-1320 already asserts they reach the request body.
Sniffed out 1 issue:
- 1 🟡 MEDIUM: new docs assert inclusive time bounds, contradicting the existing field comments in the same file
Good bones otherwise — JSDoc placement and style match the neighbouring searchAll() block. Woof!
Declared coverage: FULL — 2/2 changed files reviewed.
| * @param searchReq.fromCreatedTime only include users created on or after this time (epoch in milliseconds) | ||
| * @param searchReq.toCreatedTime only include users created on or before this time (epoch in milliseconds) | ||
| * @param searchReq.fromModifiedTime only include users modified on or after this time (epoch in milliseconds) | ||
| * @param searchReq.toModifiedTime only include users modified on or before this time (epoch in milliseconds) |
There was a problem hiding this comment.
🟡 MEDIUM — New docs claim inclusive bounds ("on or after"/"on or before"), but the existing field comments at lib/management/user.ts:60-63 say "after"/"before" (exclusive). Same inclusive wording is repeated in README.md:913-914.
Since this PR's whole purpose is documenting these params, the boundary semantics need to be right: anyone chunking a time window (feeding the previous page's max created time into the next fromCreatedTime) will either double-count or skip users at the boundary depending on which is true.
Fix: confirm the actual server-side comparison, then use one consistent phrasing across the JSDoc, the type comments, and the README.
Fixes descope/etc#9538
What
Documents the time-based user search filters on the Node SDK's
search()function, closing the last gap identified in #9538 for this SDK — the fields already existed and worked, they just weren't documented with proper JSDoc.The
search()function's documentation now includes:fromCreatedTime/toCreatedTime— filter users by creation time range (epoch milliseconds)fromModifiedTime/toModifiedTime— filter users by last-modified time range (epoch milliseconds)Implementation Details
search()function inlib/management/user.tsdocumenting all 4 time-filter fields, matching the JSDoc style already used on the deprecatedsearchAll()function above it.fromCreatedTime/toModifiedTimeusage.SearchRequesttype (with only inline//comments) and were already wired through to the request body via the...searchReqspread insearch().searchAll()function (which does not support these params) was intentionally left untouched.Verification
Verified that:
npm install(1008 packages) and ran a real TypeScript typecheck:npx tsc --noEmit -p tsconfig.json→ "No errors found." This confirms the new JSDoc block and its@param searchReq.xxxtags don't break compilation and are syntactically valid against the project's actualtsconfig.json.Not verified:
Notes
searchAll()function.