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

🐛 fix storage record pagination with post-filter query params #3082

Conversation

ff137
Copy link
Contributor

@ff137 ff137 commented Jul 3, 2024

Related to: #3001 (comment)

Marking as draft to first validate things work as expected (testing here: didx-xyz/aries-cloudapi-python#850)

@ff137 ff137 marked this pull request as ready for review July 4, 2024 15:09
@ff137 ff137 force-pushed the fix/paginated-queries-with-post-filter branch from 3ebce10 to d824998 Compare July 4, 2024 15:10
@jamshale jamshale requested review from ianco and dbluhm July 4, 2024 16:52
Copy link
Contributor

@jamshale jamshale left a comment

Choose a reason for hiding this comment

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

Looks good 👍

Sonarcloud is saying it's a bit complicated and could be refactored but it looks well tested so I'm not worried about that right now.

Copy link

sonarqubecloud bot commented Jul 4, 2024

@jamshale jamshale merged commit d8490b9 into openwallet-foundation:main Jul 4, 2024
8 checks passed
@ff137
Copy link
Contributor Author

ff137 commented Jul 4, 2024

Oof, I was just about to commit a change to make it more readable 😅 I agree some parts were a bit overly complex

ff137 added a commit to ff137/acapy that referenced this pull request Jul 4, 2024
…llet-foundation#3082)

* 🐛 fix storage record pagination with post-filter query params

Signed-off-by: ff137 <[email protected]>

* ✅ add test coverage for querying storage record with pagination and post_filter

Signed-off-by: ff137 <[email protected]>

---------

Signed-off-by: ff137 <[email protected]>
Co-authored-by: jamshale <[email protected]>
@ff137
Copy link
Contributor Author

ff137 commented Jul 4, 2024

No prob, I'll make another PR with some minor amendments, also improving the error handling which is prob worth it

@jamshale
Copy link
Contributor

jamshale commented Jul 4, 2024

Ok. Sorry about that. Another PR will be good.

@ff137
Copy link
Contributor Author

ff137 commented Jul 4, 2024

All good, I was slow 😄 included the changes in #3083

@ff137 ff137 deleted the fix/paginated-queries-with-post-filter branch July 4, 2024 22:02
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