Paging seems to be broken

jsattler / go-comdirect

Go module and CLI tool to interact with the comdirect REST API

Apache License 2.0

42 stars 9 forks source link

Paging seems to be broken #24

Open eblechschmidt opened 1 year ago

eblechschmidt commented 1 year ago

Paging seems to be broken since the last release.

I was wondering why I got different results with my implementation of #22. I still had the older version isntalled to paging was working fine.

jsattler commented 1 year ago

@eblechschmidt thanks for reporting. I will have a look and provide a fix. You are right, it was probably introduced with the latest release.

jsattler commented 1 year ago

Could you please provide some more details on what is currently not working for you? I just installed the latest version and tried the comdirect document --index <index> --count <count> which worked as expected.

eblechschmidt commented 1 year ago

Ok, I was not precise enough. I did not check comdirect document. It does indeed work as expected.

But it does not work on transactions:

If I run comdirect account transaction ID the last line is 103 out of 623.
If I then run comdirect account transaction ID --index 3 I get the exact same output as above (same transactions shown as without flag)
If i run comdirect account transaction ID --count 20 I stil get 103 out of 623.
If I run comdirect account transaction ID --count 3 I get an error Failed to parse date from command line: parsing time "" as "2006-01-02: cannot parse "" as "2006"

onjen commented 1 year ago

Seems like I broke the behavior of the --count option in 515c113. I will propose a PR to fix this.

The --index option wasn't working also before 515c113 on transactions. Just curious, why are you manually specifying the page instead of using the new --since flag or the --count flag for transactions?

eblechschmidt commented 1 year ago

I don‘t use both. I was just wondering why I got different results on my working copy for #22 when comparing it to an installed version (which was an older version)

jsattler commented 1 year ago

@eblechschmidt and @onjen thanks for looking into this. I just figured out that the index (paging-first) parameter only works when the parameter transactionState=BOOKED. If not provided, the comdirect API will return a 422 response status. I have anyhow merged it, since I think leaving the --since flag empty is a better default.

Anyhow, in order to solve this we could

set the transactionState=BOOKED as default
or provide an option to set this as a command flag. If the transactionState=BOOKED is not provided when using --index flag, I propose provide a hint/info message to the user.

Any preference?

onjen commented 1 year ago

The NOTBOOKED transactions are already excluded from the since filter, so we can close this? https://github.com/jsattler/go-comdirect/blob/main/pkg/comdirect/account.go#L145-L147