Uh oh!
There was an error while loading. Please reload this page.
chore: node sdk fixes - #134
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Node SDK’s resource typings and integration tests to reflect recent API shape changes across Payments, Entities (Connect/Attributes/Subscriptions), and Accounts (liability + card brand details).
Changes:
- Payments: add
destination_posted_dateand assertdestination_payment_methodin core payment tests. - Entities: update Attributes request shape to
requested_attributes, extend Connect responses withfiles, and add Connectartifactsrequest option + expanded attribute type/bundle definitions. - Accounts: extend liability details (
network) and expand card brand info (network_tier,details) while removing deprecated top-level brand fields.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/resources/Payment.tests.ts | Updates expected Payment response shape to include destination_posted_date and destination_payment_method. |
| test/resources/Event.tests.ts | Updates entity attributes create call to use requested_attributes and current attribute name. |
| test/resources/Entity.tests.ts | Aligns Connect expectations with new files field and updates attribute request shape/name. |
| test/resources/Account.tests.ts | Updates liability expectations (network) and card brand assertions (network_tier, details), and adjusts product subscribability expectation. |
| src/resources/Payment/types.ts | Adds destination_posted_date to IPayment. |
| src/resources/Entity/types.ts | Adds Connect artifact/file typing, modernizes attribute naming/request typing (bundles, legacy names), and expands attributes model. |
| src/resources/Entity/Connect.ts | Adds artifacts to Connect create options. |
| src/resources/Entity/Attributes.ts | Makes create() opts optional (defaults to requesting full attribute set). |
| src/resources/Account/types.ts | Adds network to liability and expands card brand info/details typing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/resources/Account/Subscriptions.ts:50
- The
createmethod now supports passing either a subscription name or an options object, but the JSDoc no longer documents the parameter and the@returnsdescription says it returns a map. The implementation returns a singleIAccountSubscriptionresponse, so the doc is currently misleading for SDK consumers.
async create(opts: IAccountSubscriptionCreateOpts | TAccountSubscriptionTypes) {
const data = (typeof opts === 'string' ? { enroll: opts } : opts) as IAccountSubscriptionCreateOpts;
return super._create<IResponse<IAccountSubscription>, IAccountSubscriptionCreateOpts>(data);
ossiggy
left a comment
There was a problem hiding this comment.
Please bump the version in package.json and package-lock.json, then we are GTG
srikar-methodfi
commented
Aug 26, 2026
@ossiggy fixed |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/resources/Account/Subscriptions.ts:51
createuses a type assertion to coercedataintoIAccountSubscriptionCreateOpts, which weakens type-safety and can mask invalid shapes. You can let TypeScript infer the correct union branch without anascast by explicitly typingdata.
async create(opts: IAccountSubscriptionCreateOpts | TAccountSubscriptionTypes) {
const data = (typeof opts === 'string' ? { enroll: opts } : opts) as IAccountSubscriptionCreateOpts;
return super._create<IResponse<IAccountSubscription>, IAccountSubscriptionCreateOpts>(data);
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/resources/Account/Subscriptions.ts:51
createuses a forced cast toIAccountSubscriptionCreateOpts, which can mask typing mistakes and makes the function harder to reason about. You can keep full type safety here by letting TypeScript infer the union from the conditional withoutas.
async create(opts: IAccountSubscriptionCreateOpts | TAccountSubscriptionTypes) {
const data = (typeof opts === 'string' ? { enroll: opts } : opts) as IAccountSubscriptionCreateOpts;
return super._create<IResponse<IAccountSubscription>, IAccountSubscriptionCreateOpts>(data);
update files, attributes, and other gaps and pass mocha tests