-
Notifications
You must be signed in to change notification settings - Fork 0
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
[#357] Refactor endpoints with search #687
Conversation
e121536
to
ba30cea
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #687 +/- ##
==========================================
+ Coverage 90.26% 90.32% +0.05%
==========================================
Files 211 211
Lines 6248 6286 +38
Branches 675 675
==========================================
+ Hits 5640 5678 +38
Misses 608 608
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Do we know where the issue originates from, and can't we fix it with a setting? This fix feels a little bit intrusive to me and introduces inconsistency. I guess that's ok if its really needed but I'd like to see if we can find a simpeler solution. |
This was the problem that the different browsers were giving different errors. I don't think we can fix this problem with a setting, if the frontend makes a request with a humongous URL, things might break before it gets to the backend. This SO answer is quite informative: https://stackoverflow.com/a/417184/7146757 |
bed01bc
to
f77ce45
Compare
frontend/src/lib/api/zaken.ts
Outdated
@@ -8,7 +8,19 @@ export type PaginatedZaken = PaginatedResults<Zaak>; | |||
* Retrieve zaken using the configured ZRC service. For information over the query parameters accepted and the schema of | |||
* the response, look at the '/zaken/api/v1/zaken' list endpoint of Open Zaak. | |||
*/ | |||
export async function listZaken(params?: Record<string, string>) { | |||
export async function listZaken( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still used or can we safely remove it?
Since the filters now can come through the request body, so can the ordering and the pagination params. So these need to be supported
f77ce45
to
ec4d6bd
Compare
Fixes #357
The fix was a bit more invasive than expected.
I created a new search endpoint for the:
These endpoints do the same as the GET, but with a POST. The filter params then are passed into the request body instead of query params in the URL.
Since the Zaken list endpoint supports ordering and pagination, I had to support
passing these query params also through the request body.