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: cursor-based pagination for payments and tasks #81

Merged
merged 8 commits into from
Jan 11, 2023

Conversation

darkmatterpool
Copy link
Contributor

Signed-off-by: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Jan 5, 2023

Codecov Report

Merging #81 (7ddc4a0) into main (b1b2287) will increase coverage by 0.35%.
The diff coverage is 31.00%.

@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
+ Coverage   19.41%   19.76%   +0.35%     
==========================================
  Files         104      103       -1     
  Lines        4600     4674      +74     
==========================================
+ Hits          893      924      +31     
- Misses       3607     3646      +39     
- Partials      100      104       +4     
Impacted Files Coverage Δ
internal/app/api/accounts.go 0.00% <0.00%> (ø)
internal/app/api/connector.go 0.00% <0.00%> (ø)
internal/app/api/connectorconfigs.go 0.00% <ø> (ø)
internal/app/api/module.go 0.00% <0.00%> (ø)
internal/app/api/payments.go 0.00% <0.00%> (ø)
internal/app/connectors/bankingcircle/config.go 0.00% <ø> (ø)
internal/app/connectors/configtemplate/template.go 0.00% <ø> (ø)
internal/app/connectors/currencycloud/config.go 0.00% <ø> (ø)
internal/app/connectors/dummypay/config.go 62.96% <ø> (ø)
internal/app/connectors/modulr/config.go 0.00% <ø> (ø)
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

HasMore: paginationDetails.HasMore,
Previous: paginationDetails.PreviousPage,
Next: paginationDetails.NextPage,
Data: nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

When using cursor, data must be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Data: &data,
Cursor: &sharedapi.Cursor[[]listTasksResponseElement]{
PageSize: paginationDetails.PageSize,
Total: sharedapi.Total{
Copy link
Contributor

@gfyrag gfyrag Jan 5, 2023

Choose a reason for hiding this comment

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

Total is optional. Can be here if not expensive to compute.
It's a legacy field which is filled by search service has elasticsearch always returns this value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

swagger.yml Outdated
description: How many payments to skip, pagination can be achieved in conjunction with 'limit' parameter.
example: 100
type: string
description: Cursor for pagination.
Copy link
Contributor

@antoinegelloz antoinegelloz Jan 6, 2023

Choose a reason for hiding this comment

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

Can you please add details on how to use it?
For instance in the ledger I wrote this description:
"Parameter used in pagination requests. Maximum page size is set to 15. Set to the value of next for the next page of results. Set to the value of previous for the previous page of results. No other parameters can be set when the pagination token is set."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Signed-off-by: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com>
Signed-off-by: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com>
Antoine Gelloz and others added 4 commits January 9, 2023 10:18
Signed-off-by: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com>
Signed-off-by: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com>
@darkmatterpool darkmatterpool merged commit 580c619 into main Jan 11, 2023
@darkmatterpool darkmatterpool deleted the feat/switch-to-cursor-pagination branch January 11, 2023 12:58
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.

3 participants