Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support all options for getSubscriptionPayments #82

Merged
merged 2 commits into from
Apr 26, 2023

Conversation

GusRuss89
Copy link
Contributor

This PR adds support for all available parameters for the list payments API to getSubscriptionPayments. It maintains backward compatibility. Tests are included for both the legacy and new parameters.

It also adds some missing properties to the SubscriptionUser interface.

@GusRuss89 GusRuss89 changed the title Support all options for getSubscriptionPayments feat: support all options for getSubscriptionPayments Apr 22, 2023
Copy link
Owner

@avaly avaly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Just a few small change requests first.

src/sdk.ts Outdated Show resolved Hide resolved
src/sdk.ts Outdated Show resolved Hide resolved
@avaly
Copy link
Owner

avaly commented Apr 24, 2023

@GusRuss89 There's a lint error remaining.

@GusRuss89
Copy link
Contributor Author

GusRuss89 commented Apr 25, 2023

@GusRuss89 There's a lint error remaining.

@avaly I think you might need to decide how to deal with this. The mixed spaces and tabs are created by prettier, and forbidden by eslint.

You could fix it by:

  • Switching prettier to use spaces instead of tabs
  • Adding an eslint ignore comment
  • Turning off the mixed-spaces-and-tabs eslint rule (it's arguably redundant with prettier anyway)
  • Abstracting the arguments type so that it's options: number | GetSubscriptionPaymentsOptions

@avaly
Copy link
Owner

avaly commented Apr 25, 2023

@GusRuss89 Please rebase on main. It should be solved by dd3d6ea

@GusRuss89 GusRuss89 force-pushed the feat/support-list-payments-params branch from a99cac4 to de20bf2 Compare April 26, 2023 01:48
@GusRuss89
Copy link
Contributor Author

@GusRuss89 Please rebase on main. It should be solved by dd3d6ea

Done :)

@avaly avaly changed the title feat: support all options for getSubscriptionPayments feat: Support all options for getSubscriptionPayments Apr 26, 2023
@avaly avaly merged commit 7ec2c6d into avaly:main Apr 26, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants