Bug/hubtel gateway#12
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Node.js SDK release workflow ergonomics and fixes Hubtel transaction verification to query by clientReference.
Changes:
- Fix Hubtel
verifyTransactionto includeclientReferencewhen querying transaction status. - Add npm scripts to automate patch/minor/major releases (version bump, commit, tag, push).
- Bump Node SDK version from
0.2.0to0.2.1and update release instructions in the root README.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/node/src/providers/hubtel/hubtel.adapter.ts | Adjusts Hubtel transaction status lookup to query by clientReference. |
| packages/node/package.json | Bumps SDK version and adds automated release scripts. |
| README.md | Updates release documentation to use the new npm release scripts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { data } = await axios.get<HubtelAPIResponse<HubtelTransaction>>( | ||
| `https://api-txnstatus.hubtel.com/transactions/${this.merchantAccount}/status`, | ||
| `https://api-txnstatus.hubtel.com/transactions/${this.merchantAccount}/status?clientReference=${reference}`, | ||
| { |
There was a problem hiding this comment.
Building the URL by string-interpolating reference into the query string can break when the reference contains reserved characters (e.g. &, ?, #, spaces) and can also allow unintended extra query params. Prefer passing clientReference via axios params (or at least encodeURIComponent(reference)) so it’s correctly encoded.
| const { data } = await axios.get<HubtelAPIResponse<HubtelTransaction>>( | ||
| `https://api-txnstatus.hubtel.com/transactions/${this.merchantAccount}/status`, | ||
| `https://api-txnstatus.hubtel.com/transactions/${this.merchantAccount}/status?clientReference=${reference}`, | ||
| { |
There was a problem hiding this comment.
There are existing HubtelAdapter tests, but none assert the verifyTransaction request URL/params. Since this change fixes which transaction is queried, please add/adjust a test to assert axios.get is called with clientReference (ideally via params) so regressions are caught.
| "release:patch": "npm version patch --no-git-tag-version && git add package.json && git commit -m \"chore: release v$(node -p \"require('./package.json').version\")\" && git tag node-v$(node -p \"require('./package.json').version\") && git push && git push origin node-v$(node -p \"require('./package.json').version\")", | ||
| "release:minor": "npm version minor --no-git-tag-version && git add package.json && git commit -m \"chore: release v$(node -p \"require('./package.json').version\")\" && git tag node-v$(node -p \"require('./package.json').version\") && git push && git push origin node-v$(node -p \"require('./package.json').version\")", | ||
| "release:major": "npm version major --no-git-tag-version && git add package.json && git commit -m \"chore: release v$(node -p \"require('./package.json').version\")\" && git tag node-v$(node -p \"require('./package.json').version\") && git push && git push origin node-v$(node -p \"require('./package.json').version\")" |
There was a problem hiding this comment.
npm version ... will also update package-lock.json when it exists (it does in this package), but this script only stages package.json. That leaves the lockfile modified but uncommitted after running the release script. Consider staging/committing package-lock.json too, or configuring the command to avoid touching the lockfile.
| "release:patch": "npm version patch --no-git-tag-version && git add package.json && git commit -m \"chore: release v$(node -p \"require('./package.json').version\")\" && git tag node-v$(node -p \"require('./package.json').version\") && git push && git push origin node-v$(node -p \"require('./package.json').version\")", | ||
| "release:minor": "npm version minor --no-git-tag-version && git add package.json && git commit -m \"chore: release v$(node -p \"require('./package.json').version\")\" && git tag node-v$(node -p \"require('./package.json').version\") && git push && git push origin node-v$(node -p \"require('./package.json').version\")", | ||
| "release:major": "npm version major --no-git-tag-version && git add package.json && git commit -m \"chore: release v$(node -p \"require('./package.json').version\")\" && git tag node-v$(node -p \"require('./package.json').version\") && git push && git push origin node-v$(node -p \"require('./package.json').version\")" |
There was a problem hiding this comment.
These release scripts rely on POSIX shell features ($(...) command substitution and && chaining), so they won’t work in Windows’ default npm script shell. If maintainers release from different OSes, consider moving the logic into a small Node.js script (or a cross-platform tool) and reusing it for patch/minor/major to avoid triplicated long commands.
This pull request introduces improvements to the Node.js SDK release process and updates the Hubtel provider to correctly query transaction status by client reference. The most significant changes are the addition of automated release scripts for patch, minor, and major versions, a version bump, and a bug fix for the Hubtel transaction status API call.
Release process improvements:
release:patch,release:minor, andrelease:majornpm scripts to automate versioning, tagging, and pushing releases for the Node.js SDK inpackage.json. This streamlines and standardizes the release workflow.README.mdto reflect the new automated release scripts, replacing the manual tagging process.Versioning:
0.2.0to0.2.1inpackage.json.Bug fix:
clientReferencequery parameter, ensuring the correct transaction is retrieved.