Buildqueryparams#343
Merged
Merged
Conversation
…mentsApi.ts's duplicated URLSearchParams logic
# Conflicts: # src/components/AnchorsPanel.test.tsx # src/components/SettlementsPanel.test.tsx # src/lib/api.test.ts
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
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.
Close: #180
Summary of the Issue
fetchSettlementsandexportSettlementsCsvinsrc/lib/settlementsApi.tseach independently build an identicalURLSearchParamsfrom the same{ anchor, page, pageSize }shape, duplicating the same conditional.set(...)calls twice in the same file. This pattern is also a natural fit foranchorsApi.tsfunctions if they ever grow filter parameters.Root Cause
There was no shared query-string builder utility. Each function that needed URL query parameters re-implemented the same pattern:
Solution Implemented
1.
buildQueryParamshelper (src/lib/api.ts)A reusable, tested utility function:
undefined(so callers can spread optional params freely)"?key=value&key2=value2"or""when no params are presentapiRequestinsrc/lib/api.tsso bothsettlementsApi.tsandanchorsApi.tscan reuse it2.
fetchSettlementsupdated (src/lib/settlementsApi.ts)Replaced the inline
URLSearchParamsconstruction with a single call tobuildQueryParams:3. Unit tests (
src/lib/api.test.ts)Added 11 test cases covering:
"""""?anchor=a""?anchor=a&page=1&pageSize=20""?page=5""?name=hello+world"FetchSettlementsOptionsshape""for all-undefinedKey Changes Made
src/lib/api.tsbuildQueryParams()helper alongsideapiRequestsrc/lib/settlementsApi.tsfetchSettlementsto usebuildQueryParamssrc/lib/api.test.tsbuildQueryParamsTrade-offs & Considerations
URLSearchParams(available in Node.js, browsers, and jsdom test environment)Record<string, string \| number \| undefined>type catches accidentalnullvalues while allowing optional params to be omitted naturallyanchorsApi.tscan adopt the same helper when its endpoints grow filter parametersTesting Steps (How to Verify the Fix)
npx vitest runsettlementsApi.test.tstests pass with identical request URLsapi.test.tstests pass including 11 newbuildQueryParamstestsbuildQueryParamstests include equivalence checks that confirm the output matches the original inlineURLSearchParamslogicPlease kindly review this task. If there are any corrections, improvements, adjustments, or merge conflicts that you notice regarding my implementation, I'd really appreciate your feedback. I'd also love to hear your overall review of my work on this branch. Thank you!