-
Notifications
You must be signed in to change notification settings - Fork 59
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) O3-3503: Fix pagination of problem datasource fetch results #333
Conversation
Size Change: +2 B (0%) Total Size: 1.14 MB ℹ️ View Unchanged
|
Loading all available concepts appears to be very resource-intensive. I suggest implementing a "background search" that uses user input as the search term. |
This will be a bit hard given that |
This would work perfectly for a single concept class: const searchTerm = "WHO";
/openmrs/ws/rest/v1/concept?searchType=fuzzy&v=custom:(uuid,display,conceptClass:(uuid,display))&class=8d4918b0-c2cc-11de-8d13-0010c6dffd0f&q=${searchTerm} To support multiple classes I think we can filter the returned results by the classes: const classes = [
'8d4918b0-c2cc-11de-8d13-0010c6dffd0f',
'8d492954-c2cc-11de-8d13-0010c6dffd0f',
'8d492b2a-c2cc-11de-8d13-0010c6dffd0f'
];
const url = `${restBaseUrl}/concept?searchType=fuzzy&v=custom:(uuid,display,conceptClass:(uuid,display))&q=${searchTerm}`;
openmrsFetch(url).then(({ data }) => {
return data.results.filter(
(concept) => classes.includes(concept.conceptClass.uuid)
);
}); |
The issue with searching through the results is that the data returned contains 50 results, and if you filter, you will only get one result, unless the limit is increased, which will still not return the expected results |
Ping @samuelmale @ibacher ? |
Since we lack the ideal backend support, I think @CynthiaKamau's solution can work for now ie. fetching concepts of all the relevant classes in parallel. Question: Does this problem exist in AFE? |
In AFE, diagnosis/problems are handled as a datasource, so
In AFE, the input is searchable, in RFE, there instances where its a select box |
IIRC the ui-select-ext supports searching, can we instead switch to doing a search? cc: @kajambiya |
ping @kajambiya |
Filed ticket: https://openmrs.atlassian.net/browse/O3-3770 |
@samuelmale Thanks for the ticket. What should we do about this PR? Close in favor of doing O3-3770? Merge until we have a better solution? |
Actually @brandones @samuelmale ... @chibongho just recently merged the following PR, worth taking a look at: openmrs/openmrs-esm-core@45bdc9b also, linked ticket - https://openmrs.atlassian.net/browse/O3-3189 |
Thanks for sharing, @mogoodrich. I think this is particularly useful when dealing with paginable components like data tables.
@brandones I’m inclined to implement or fix the search capability rather than eagerly loading all concepts from different classes in one shot. |
Ok, thanks @samuelmale . @CynthiaKamau , will you do that, do you have the time? |
Yes, working on updating it |
@samuelmale would switching to this format suffice , i wouldn't really need to make any changes ?
|
@CynthiaKamau The question's configuration looks okay; does it work as expected? |
Is it related to this ticket ? |
ed1a935
to
3836126
Compare
3836126
to
8c39210
Compare
Affirmative! I also noticed that you're working on the talked about ticket. |
Requirements
Summary
Currently, the implementation of the problem data source fetches only 50 results due to pagination constraints. This limitation affects the visibility of data across three concept classes. This pull request aims to enhance the data fetching process to retrieve all results from the endpoint, ensuring comprehensive data coverage.
Screenshots
Screen.Recording.2024-06-28.at.11.17.26.mov
Related Issue
https://openmrs.atlassian.net/browse/O3-3503
Other