-
Notifications
You must be signed in to change notification settings - Fork 3
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
_patient_info CDT pagination breaks the client #55
Comments
@edcohen08 Can you illustrate the steps to reproduce this bug? |
|
Discovered another bug in relation to this. If you call Created #85 to track this. |
The data returned by this specific CDT is pretty different than others. Example: {
"name": "_patient_info",
"data": {
"content": [],
"pageable": "INSTANCE",
"last": true,
"totalPages": 1,
"totalElements": 1,
"first": true,
"number": 0,
"sort": {
"sorted": false,
"unsorted": true,
"empty": true
},
"numberOfElements": 1,
"size": 1,
"empty": false
}
} What we expect: {
"name": "allergies",
"data": {
"content": [],
"pageable": {
"sort": {
"sorted": false,
"unsorted": true,
"empty": true
},
"pageNumber": 0,
"pageSize": 20,
"offset": 0,
"unpaged": false,
"paged": true
},
"last": true,
"totalElements": 0,
"totalPages": 0,
"first": true,
"sort": {
"sorted": false,
"unsorted": true,
"empty": true
},
"number": 0,
"numberOfElements": 0,
"size": 20,
"empty": true
}
} @edcohen08 Is this a paging strategy you recognize? |
@samamorgan this specific call is the only time I've seen that where pageable is a string rather than a dict |
The code around pagination is pretty bloated, and we pepper a lot of logic to figure out pagination in We shouldn't have to determine what paginator class to use as we currently do (setting the class on the model). Instead, we should sort out the pagination strategy in a method based on the structure of the response, then assign that iterator to the underlying collection during the request. @edcohen08 I assume given the age of this ticket that you already have a workaround in place for this issue. How critical is this fix for your work? |
It turned out that we actually weren't using that CDT so I never fixed it. What I was working on was just checking whether the pageable was a string and handling it then. That said I definitely agree it's at the point it should be broken out |
The _patient_info CDT meta_key value is just a string 'INSTANCE', we expect a dictionary
The text was updated successfully, but these errors were encountered: