Add allergies to mobile app Health section #10339
Conversation
…location API, and update error handling for allergy screens
…ent-of-veterans-affairs/va-mobile-app into feature/addAllergiesList
VAMobile/src/screens/HealthScreen/Allergies/AllergyList/AllergyListScreen.tsx
Show resolved
Hide resolved
VAMobile/src/screens/HealthScreen/Allergies/AllergyList/AllergyListScreen.test.tsx
Show resolved
Hide resolved
VAMobile/src/screens/HealthScreen/Allergies/AllergyList/AllergyListScreen.tsx
Show resolved
Hide resolved
VAMobile/src/screens/HealthScreen/Allergies/AllergyList/AllergyListScreen.tsx
Show resolved
Hide resolved
VAMobile/src/screens/HealthScreen/Allergies/AllergyDetails/AllergyDetailsScreen.test.tsx
Outdated
Show resolved
Hide resolved
|
Does anyone know of a staging user that has allergy data? I'd prefer to review/test this PR in this manner since there are backend changes. |
VAMobile/e2e/tests/Allergies.e2e.ts
Outdated
|
|
||
| export const AllergiesE2eIdConstants = { | ||
| ALLERGY_1_ID: 'Sulfonamides allergy March 12, 2019', | ||
| ALLERGY_2_ID: 'penicillins allergy January 10, 2023', |
There was a problem hiding this comment.
Is the word penicillins not capitalized on purpose? Doesn't match the other allergy IDs.
VAMobile/e2e/tests/Navigation.e2e.ts
Outdated
| ['VaccineRecords.e2e', ['Medical records', 'V\ufeffA vaccine records'], 'VA vaccines'], | ||
| ['VaccineRecords.e2e', ['Medical records', 'V\ufeffA vaccine records', 'January 14, 2021'], 'COVID-19 vaccine'], | ||
| ['Allergies.e2e', ['Medical records', 'V\ufeffA allergy records'], 'Allergies'], | ||
| ['Allergies.e2e', ['Medical records', 'V\ufeffA allergy records', 'January 10, 2023'], 'penicillins allergy'], |
There was a problem hiding this comment.
Capitalize penicillins?
|
|
||
| it('verify no disclaimer is displayed when all fields are populated', async () => { | ||
| await element(by.id(AllergiesE2eIdConstants.ALLERGY_3_ID)).tap() | ||
| await expect(element(by.text('None noted '))).not.toExist() |
There was a problem hiding this comment.
Remove trailing space from 'None noted '?
There was a problem hiding this comment.
Also is this something we can use the i18n translation?
There was a problem hiding this comment.
Well I see in context with the other notes that aren't linked to text from common.json, I could go either way. But at least for the label on ln 59 and 75
There was a problem hiding this comment.
For e2e testing we'll want to check the displayed value so it would fail in the event someone deleted the key from i18n or introduced a typo or anything like that. Using the translation function to check it could lead to a situation where the text becomes incorrect and the e2e test still passes.
There was a problem hiding this comment.
Thanks for the clarification!
| await element(by.id(AllergiesE2eIdConstants.ALLERGIES_DETAILS_BACK_ID)).tap() | ||
| }) | ||
|
|
||
| it('multi-line note for dust allergy', async () => { |
There was a problem hiding this comment.
This test fails on iOS. Haven't tried it on Android. The reason for failure is that allergy 7 is off the bottom of the screen and not visible. I suggest looking at some of the other tests to implement scrolling down to this allergy or to the bottom of the screen. You'll likely have to add a testID to the FeatureLandingTemplate component in AllergyListScreen.tsx.
| @@ -7,7 +7,14 @@ This script should be updated whenever new things are added/changed in vaccines | |||
| import { combineReducers } from '@reduxjs/toolkit' | |||
…date translations for better clarity
Description of Change
This PR does the following:
Screenshots/Video
Before
After:
Testing
Tested locally in demo mode and against staging.va.gov.
added unit tests
added e2e tests
Tested on iOS
Tested on Android
Reviewer Validations
Looking for any and all feedback since this is our first major mobile PR :-)
PR Checklist
Reviewer: Confirm the items below as you review
For QA
Run a build for this branch