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

(refactor) O3-3806: Refactor Service queue entries api call using OpenMrs Fetch All #1394

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

kb019
Copy link
Contributor

@kb019 kb019 commented Dec 2, 2024

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

To use hooks already present in esm-core to fetch all the queue entries.

388370799-867ef32b-fb36-4a01-a49b-c5fbf7d3b0ff.webm

Related Issue

https://openmrs.atlassian.net/browse/O3-3806

Other

Copy link
Contributor

@brandones brandones left a comment

Choose a reason for hiding this comment

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

It's a great idea but your implementation doesn't make any sense (yet).

isEnded: false,
});

const { data } = useSWR<QueueEntryResponse>(
Copy link
Contributor

Choose a reason for hiding this comment

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

You are no longer using useQueueEntries at all. I can't imagine that this works in cases where the number of entries exceeds the page size. I did my development and testing of this feature by creating 11 service queue entries and setting the page size to 3. I recommend you do something similar. There are a lot of edge cases that we need to make sure will continue to work.

const stillWaitingForMutate = waitingForMutate && !isValidating;
if (waitingForMutate && isValidating) {
setWaitingForMutate(false);
if (initialMount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

First off, this initialMount is a ref, so it is always true. You want initialMount.current.

But moreover, if pageUrl changes, then useOpenmrsFetchAll will get called with the new value of pageUrl as a parameter. Why would you call mutate at the same time?

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