#293 support limit query param in /txns.json#517
Conversation
The /txns.json endpoint returns the full wallet transaction history on every call. Wallets with hundreds of transactions make this slow when a caller only wants the most recent few. Accept an optional `limit` parameter and slice the list down to the first `limit` entries before serialising, mirroring the shape requested in the issue (`/txns?limit=10`). Reject non-positive values with a UserError so an empty or malformed parameter does not silently return everything.
kreinba
left a comment
kreinba
left a comment
There was a problem hiding this comment.
Request changes. The reused E207 error code clashes with the cash-out path in front/front_upwork.rb:26 and must be renumbered before merge; the new limit branch also ships without a test for either the happy path or the rejected-zero path. Two inline comments below name the two defects. The two failing CI checks (typos on objects/referrals.rb and yamllint on .github/workflows/actionlint.yml) point at files outside this diff and so are not caused by this change.
| list.reverse! if params[:sort] && params[:sort] == 'desc' | ||
| if params[:limit] | ||
| limit = params[:limit].to_i | ||
| raise WTS::UserError, "E207: The 'limit' parameter must be a positive integer" if limit <= 0 |
There was a problem hiding this comment.
Error code E207 is already taken by front/front_upwork.rb:26, which raises WTS::UserError on a different condition (You have to work in Zerocracy in order to cash out to UpWork). Reusing the same code on a new failure path breaks the contract that callers and operators rely on to identify which user-facing error fired. Pick the next free code in the sequence; E227 appears to be the next unused slot after E226 in objects/ops.rb.
| confirmed_user.wallet do |wallet| | ||
| list = wallet.txns | ||
| list.reverse! if params[:sort] && params[:sort] == 'desc' | ||
| if params[:limit] |
There was a problem hiding this comment.
The new limit branch is not exercised by any test. The smoke test in test/test_wts.rb only hits /txns.json without a parameter, so a regression in the parse, the bound check, or the first(limit) slice would land silently. Add a case that calls /txns.json?limit=1 and asserts the response is a JSON array of length at most one, and a case that asserts a 4xx response for limit=0 or a negative value so the new WTS::UserError is covered.
2 participants
Closes #293.
The
/txns.jsonroute serialised the full wallet transaction history on every call, which the linked issue measured at roughly 17 seconds for a 715-transaction wallet when only the most recent few rows were needed.The change accepts an optional
limitquery parameter, parses it as an integer after the optionalsort=descreverse, and slices the list down to the firstlimitentries before serialising. A non-positive value short-circuits withWTS::UserErrorE207 so a typo never silently returns the entire history.Locally
ruby -c wts.rbreportsSyntax OKandbundle exec rubocop wts.rbreports no offences against the project config. The full rake target was not executed in this environment because it requires a live PostgreSQL pool viaDATABASE_URL; the existing smoke test intest/test_wts.rbalready hits/txns.jsonwithout alimitparameter and the new branch leaves that no-parameter path unchanged.