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

Support HIV on Results and PXP pages #7161

Merged
merged 2 commits into from
Jan 19, 2024
Merged

Support HIV on Results and PXP pages #7161

merged 2 commits into from
Jan 19, 2024

Conversation

emyl3
Copy link
Collaborator

@emyl3 emyl3 commented Jan 13, 2024

FRONTEND PULL REQUEST

Related Issue

Changes Proposed

  • Displays HIV in the Test Results page "Condition" filter when hivEnabled feature flag is on
Screenshot 2024-01-16 at 16 43 25
  • Display "HIV result" in CSV when clicking "Download results"
Screenshot 2024-01-16 at 16 45 43
  • Print modal displays HIV results
Screenshot 2024-01-16 at 16 47 40
  • View details includes HIV results and AOE questions (pregnant and gender of sexual partners)
Screenshot 2024-01-16 at 16 48 33
  • PXP page of a HIV test result
Screenshot 2024-01-16 at 17 21 46

Additional Information

Testing

  • available on dev2 for testing

Screenshots / Demos

  • Please see above

@@ -56,6 +56,20 @@ export function hasCovidResults(results: MultiplexResults): boolean {
);
}

export function hasDiseaseSpecificResults(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

⚠️ eventually we can refactor to use this instead of the covid specific version on line 51

@@ -40,6 +40,7 @@ export const es: LanguageConfig = {
FLUA: "Flu A",
FLUB: "Flu B",
RSV: "VRS",
HIV: "VIH",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

⚠️ got VIH from the CDC's usage of it

@emyl3 emyl3 force-pushed the elisa/7077-hiv-results-pxp branch from b324f9d to 2336d2e Compare January 13, 2024 01:22
@@ -42,40 +50,6 @@ type Result = {
};
};

export const testResultDetailsQuery = gql`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

⚠️ moved to separate operations.graphql file

removed={removed}
aria-describedby="result-detail-title"
/>
{!isHIVResult && (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

⚠️ We are currently not collecting this for HIV results so symptoms and symptom onset date will only be displayed if it is not a HIV result

@emyl3 emyl3 force-pushed the elisa/7077-hiv-results-pxp branch from 2336d2e to 2fdcfda Compare January 16, 2024 22:30
symptoms
symptomOnset
pregnancy
genderOfSexualPartners
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

⚠️ added genderOfSexualPartners so it can be displayed

@emyl3 emyl3 marked this pull request as ready for review January 16, 2024 23:45
@@ -0,0 +1,25 @@
import { UNKNOWN } from "../../lang/en";
Copy link
Contributor

@fzhao99 fzhao99 Jan 17, 2024

Choose a reason for hiding this comment

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

I think we might be able to use some TS language features to make this file more robust. I put together a playground link here

Let me know if you want to chat through anything since there are a couple TS nerdery things that I pulled together to make the above work

Copy link
Collaborator

Choose a reason for hiding this comment

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

@fzhao99 I like that idea of using a typed dictionary for the gender values (non-blocking if we wanna have that in a separate PR). Saw this comment too which makes me wonder if there's any particular reason the test result details are not currently translated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OOO thank you for nerding out! love that -- will take a look now! 👀

const MALE = "Male";
const TRANSWOMAN = "Trans femme or transwoman";
const TRANSMAN = "Trans masculine or transman";
const NONBINARY = "Nonbinary or gender non-conforming";
Copy link
Contributor

Choose a reason for hiding this comment

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

Extending on this comment, would love to couple these constants here with the value of the GenderIdentities dictionary we're defining so that there's one source of truth for these strings. Worry that we might have string drift if we redefine the constants here

Think we could replace these with GenderIdentityDisplay.female, GenderIdentityDisplay.male, etc. so that the dictionary in the format file is the source of truth for what the strings are.

mpbrown
mpbrown previously approved these changes Jan 17, 2024
Copy link
Collaborator

@mpbrown mpbrown left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for tidying up some of the graphql!

@@ -0,0 +1,25 @@
import { UNKNOWN } from "../../lang/en";
Copy link
Collaborator

Choose a reason for hiding this comment

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

@fzhao99 I like that idea of using a typed dictionary for the gender values (non-blocking if we wanna have that in a separate PR). Saw this comment too which makes me wonder if there's any particular reason the test result details are not currently translated?

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions

0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@emyl3
Copy link
Collaborator Author

emyl3 commented Jan 17, 2024

@fzhao99 and @mpbrown Ready for re-review! The checks are not all successful because the sonarcloud one is failing for no test coverage which is not true 😅

@emyl3 emyl3 added this pull request to the merge queue Jan 19, 2024
Merged via the queue into main with commit 37f25f7 Jan 19, 2024
40 of 41 checks passed
@emyl3 emyl3 deleted the elisa/7077-hiv-results-pxp branch January 19, 2024 18:46
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.

HIV on results page and PXP
3 participants