Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

feat(allergies): ability to click on allergy and view an allergy #2224

Merged
merged 7 commits into from
Jul 12, 2020

Conversation

gdemu13
Copy link
Contributor

@gdemu13 gdemu13 commented Jul 10, 2020

fixes #2215

Changes proposed in this pull request:

  • added ability to click and view detail of allergy on different route
  • modified test of Allergies component and added new one for ViewAllergy.

@jsf-clabot
Copy link

jsf-clabot commented Jul 10, 2020

CLA assistant check
All committers have signed the CLA.

@gitpod-io
Copy link

gitpod-io bot commented Jul 10, 2020

@jackcmeyer jackcmeyer changed the title feat(allergies, viewallergy): ability to click on allergy and view de… feat(allergies): ability to click on allergy and view an allergy Jul 11, 2020
@vercel
Copy link

vercel bot commented Jul 11, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hospitalrun/hospitalrun-frontend/df3zxkmb3
✅ Preview: https://hospitalrun-frontend-git-fork-gdemu13-feat-view-allergy.hospitalrun.vercel.app

@@ -0,0 +1,41 @@
import findLast from 'lodash/findLast'
Copy link
Member

Choose a reason for hiding this comment

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

convention for file naming is PascalCase.

So for this file, src/patients/allergies/ViewAllergy.tsx

Comment on lines 54 to 63
<List>
{patient.allergies?.map((a: Allergy) => (
<ListItem
key={a.id}
onClick={() => history.push(`/patients/${patient.id}/allergies/${a.id}`)}
>
{a.name}
</ListItem>
))}
</List>
Copy link
Member

Choose a reason for hiding this comment

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

I think we should extract this to its own component


useEffect(() => {
if (patient && allergyId) {
const currentAllergy = findLast(patient.allergies, (a: Allergy) => a.id === allergyId)
Copy link
Member

Choose a reason for hiding this comment

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

@@ -29,9 +29,14 @@ const expectedPatient = {
let user: any
let store: any

const setup = (patient = expectedPatient, permissions = [Permissions.AddAllergy]) => {
const setup = (
Copy link
Member

Choose a reason for hiding this comment

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

there should be a test that ensures the functionality of clicking on a list element navigates to the allergy.

<Route exact path="/patients/:id/allergies">
<List>
{patient.allergies?.map((a: Allergy) => (
<ListItem
Copy link
Member

Choose a reason for hiding this comment

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

We should indicate that the list item is clickable. We can do this by adding action as a prop to ListItem

@gdemu13
Copy link
Contributor Author

gdemu13 commented Jul 11, 2020

@jackcmeyer PR is ready for review

Comment on lines 12 to 14
// console.log('===========================>')
// console.log(patient)
return (
Copy link
Member

Choose a reason for hiding this comment

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

remove commented out code

@jackcmeyer jackcmeyer merged commit 071508c into HospitalRun:master Jul 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to view allergy
4 participants