Skip to content

Conversation

@ctasada
Copy link
Contributor

@ctasada ctasada commented Jun 19, 2025

Added support for the POST /pa/allperiods operation.

This endpoint was missing from the implementation. See official documentation at https://www.interactivebrokers.com/campus/ibkr-api-page/cpapi-v1/#pa-all-periods

Added support for the `POST /pa/allperiods` operation.

This endpoint was missing from the implementation. See official documentation at https://www.interactivebrokers.com/campus/ibkr-api-page/cpapi-v1/#pa-all-periods
@Voyz
Copy link
Owner

Voyz commented Jun 23, 2025

hey @ctasada many thanks for the contribution! Looks all good, except that the name of the method should correspond with its endpoint - so all_periods instead of account_allperiods. Once that's done, I'd say it's ready to merge, thanks!

@ctasada
Copy link
Contributor Author

ctasada commented Jun 24, 2025

HI @Voyz I just pushed the change as requested.

I basically followed the correlated method design, where account_performance is mapped to pa/performance. I'm mentioning only to validate that the new method is introduced with the "correct" name 😄

@Voyz
Copy link
Owner

Voyz commented Jul 3, 2025

Hey @ctasada thanks for the update and for the explanation. It totally makes sense you'd made that decision. I try to stick to the names they give to these functions in the IBKR documentation where indeed they're called 'Account Performance' and 'All Periods', but your methodology is not incorrect either. I appreciate you correcting it for consistency 🙌 Thanks for the contribution, LGTM!

@Voyz Voyz merged commit 1439f75 into Voyz:master Jul 3, 2025
8 checks passed
@Voyz
Copy link
Owner

Voyz commented Jul 3, 2025

Released with 0.1.16 👍 Thanks once again!

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.

2 participants