-
Notifications
You must be signed in to change notification settings - Fork 477
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
PRN prescriptions table set upto latest administration #6345
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
✅ Deploy Preview for care-egov-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
-
You need not compute the latest administration date manually by fetching the administrations. The prescription record itself has last_administered_on field that will give u the latest date without fetching administrations.
-
We are now querying administrations list api two times now for each prescription.
N+1 queries are bad to begin with, we now have 2N+1 queries :) which is even more bad 😅
You need not create additional state either to store last administered. The problem is within how the slots are generated by the useRangePagination hook alone. |
The last_administered_on is storing the time at which the administration was made by the user instead of storing the latest administered time given by the user. If a medicine is administered right now(current time: 8:50 AM) to a previous time, let's say to 4:30 AM then 5-8 slots will also be displayed even though latest administration was at. 4:30 AM |
So the issue is to remove only 1 additional column that is ahead of the last_administered_on time. At 8:50 AM if I administer a medicine and set the administration time as 4:30 AM then currently we can see columns upto 9-10 slot as latest_administered_on is 8:50 AM. |
I will remove that N additional queries part now and also the additional state as we can use the last_administered_on due to above fix on backend |
@rithviknishad I fixed it as you suggested by modifying in useRangePagination hook but "last modified" part on the top of the table is shown as 4:30 AM as per the above example. But it should be shown as 8:50AM. There is another attribute Previously before the above fix on backend we used |
That can be a separate issue |
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.
Nice work! LGTM
LGTM |
👋 Hi, @print-Sathvik, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
I am closing this PR as the issue is fixed by this commit: b7fc53c |
WHAT
🤖 Generated by Copilot at 07a6035
This pull request improves the functionality and appearance of the
PrescriptionAdministrationsTable
component by using the latest administration dates and avoiding data conflicts. It also modifies thelistAdministrations
action insrc/Redux/actions.tsx
to support multiple keys for concurrent requests.Proposed Changes
@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers
HOW
🤖 Generated by Copilot at 07a6035
latestAdministration
to store the latest administration date of all the prescriptions in the table (link)latestAdministration
as a fallback value for the end date of the time bounds for the table (link)listAdministrations
action creator to avoid conflicts with other requests (link, link, link, link)