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

status:Filter lab orders by status #48

Merged
merged 4 commits into from
Feb 1, 2024
Merged

Conversation

gitcliff
Copy link
Contributor

@gitcliff gitcliff commented Jan 30, 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

This PR addresses the issue of filtering orders by status in the Tests ordered tab

Screenshots

https://www.loom.com/share/0a7d15e5f8384314b931aedb55c6d6c6?sid=7af4fda7-161f-4915-b03e-e7a94b93cf8b

Related Issue

Other

@jabahum jabahum requested review from pirupius and jabahum January 31, 2024 05:25
@pirupius
Copy link
Member

Hi @gitcliff please update the description with the summary and screenshots to speed up the review process

@gitcliff
Copy link
Contributor Author

Hi @gitcliff please update the description with the summary and screenshots to speed up the review process

thanks @pirupius ,,i have added them

src/queue-list/laboratory-patient-list.component.tsx Outdated Show resolved Hide resolved
translations/en.json Outdated Show resolved Hide resolved
Comment on lines 50 to 58
const OrderStatuses = [
t("all", "All"),
t("received", "RECEIVED"),
t("inProgressOrder", "IN_PROGRESS"),
t("completedOrder", "COMPLETED"),
t("exception", "EXCEPTION"),
t("on_hold", "ON_HOLD"),
t("decline", "DECLINED"),
];
Copy link
Member

Choose a reason for hiding this comment

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

Hi @gitcliff i think this might have been of a mixture of approach for this. The values here are referenced in the backend so the translations isn't something that applies here otherwise someone can fix a typo thinking these texts don't have an impact but in reality the values have to stay intact.
One approach we could use is to map plain text to the values passed but for now let's just remove the translations so that this doesn't drag.

Suggested change
const OrderStatuses = [
t("all", "All"),
t("received", "RECEIVED"),
t("inProgressOrder", "IN_PROGRESS"),
t("completedOrder", "COMPLETED"),
t("exception", "EXCEPTION"),
t("on_hold", "ON_HOLD"),
t("decline", "DECLINED"),
];
const OrderStatuses = [
"All",
"RECEIVED",
"IN_PROGRESS",
"COMPLETED",
"EXCEPTION",
"ON_HOLD",
"DECLINED",
];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @gitcliff i think this might have been of a mixture of approach for this. The values here are referenced in the backend so the translations isn't something that applies here otherwise someone can fix a typo thinking these texts don't have an impact but in reality the values have to stay intact. One approach we could use is to map plain text to the values passed but for now let's just remove the translations so that this doesn't drag.

thanks @pirupius ,,i have updated

@pirupius pirupius merged commit e0818a9 into openmrs:main Feb 1, 2024
3 checks passed
@ojwanganto
Copy link
Contributor

Thanks for this change. I know this comment might be coming in late but am wondering how the status' filter will be helpful specifically in the tests ordered tab. The risk is that the list will grow fast and long as it will contain tests that are no longer relevant in the lab. I expect tests with rejected or completed statuses to be removed from this list and made visible in the investigative results section of the patient chart.

@pirupius
Copy link
Member

pirupius commented Feb 4, 2024

@ojwanganto you have a valid point and i think this comes down to aligning how we want to default tab setup to be. There's been a little bit of back and forth about this topic with different organizations having different requirements.

Work was previously done to make the tabs configurable and implementors can remove the ones they don't want to use which validates having a filter for such scenarios where multiple statuses are within the same tab. But this brings up and issue of a redundant filter in a tab with orders of the same status.

Hopefully this is someone we can agree on during the Tuesday labs call with all stakeholders present.

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.

4 participants