-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
First step at allergy work #786
First step at allergy work #786
Conversation
…into feature/allergy_service
…into feature/allergy_service
…into feature/allergy_service
@donaldwasserman thanks for working on this PR. As far as UI feedback, can you provide screenshots in this PR so that we can review the changes visually? |
Patient Summary https://www.dropbox.com/s/yht7zckn0g1pgkp/Screenshot%202016-11-10%2020.21.01.png?dl=0 Generic medicationAllergy Component (with ops) This view is from Also on the visit view: https://www.dropbox.com/s/i8q1trf29svc5u7/Screenshot%202016-11-10%2020.23.59.png?dl=0 And the new Medication request view: https://www.dropbox.com/s/nzmtef1k6yxxk9m/Screenshot%202016-11-10%2020.24.57.png?dl=0 This may be too much on the medication request view, but my wife strongly suggested that be included on there. In terms of functionality, I was thinking that the medicationAllergy service could check for interactions/allergies between the existing prescribed drugs and proposed new medication with some sort of warning concept. Lots of rough edges including:
I think i'm starting to understand a bit of the lay of the land, and it's a joy to work in. I took the liberty of refactoring some computed properties and whatnot of a file to the more updated syntax when I was working nearby. Hope that's not problematic. |
@donaldwasserman @jglovier here are the screenshots directly in the PR for reference: Patient SummaryGeneric medicationAllergy Component (with ops)This view is from patient.editAlso on the visit view: |
@donaldwasserman thanks for your work on this PR. After looking at the screenshots, I have a couple comments:
Does all that make sense? Let me know if you have any questions or if I can further clarify. |
So basically:
Will take care of that (and tests and translations). What's the best practice for translations? Should I repurpose existing ones or create new, specific ones? |
@donaldwasserman yes that is exactly what I am asking. Thanks for following up. In regards to translations, for the most part translations shouldn't be reused unless they are for the same functional area. For example if you are working on one of the patient screens and you add new functionality you can use translations from the patients subsection of the translations. Also, components have their own section in the translations, so for components such as medication-allergy, the translation path would be |
…into feature/allergy_service
@donaldwasserman thanks for the update. I have been working on making diagnosis editable from the patient summary and I would like allergies to follow the same design direction. Basically the idea is that instead of showing the list of items to manage in a modal, the modal only shows one item at a time. I made a small change to the modal-dialog component (and checked it into master) to allow you to specify additional buttons so we can accomodate a delete button. Here's the computed property on use on the diagnosis edit controller to set the additionalButtons: additionalButtons: computed('model.isNew', function() {
let i8n = this.get('i18n');
let isNew = this.get('model.isNew');
if (!isNew) {
return [{
class: 'btn btn-default warning',
buttonAction: 'deleteDiagnosis',
buttonIcon: 'octicon octicon-x',
buttonText: i8n.t('buttons.delete')
}];
}
}),
additionalButtons: computed('model.isNew', function() {
let i8n = this.get('i18n');
let isNew = this.get('model.isNew');
if (!isNew) {
return [{
class: 'btn btn-default warning',
buttonAction: 'deleteDiagnosis',
buttonIcon: 'octicon octicon-x',
buttonText: i8n.t('buttons.delete')
}];
}
}), Another general comment: I think having the ICD9/ICD 10 codes in the allergy edit screen is going to be too much for clinical staff. I think for now, we should just record allergy name and then for phase 2 (eg allergy interaction), there should be an admin screen to maintain list of allergies with their mappings to ICD9/ICD10 codes. Does that make sense? |
@jkleinsc all of that makes sense. One question though: how should I handle the case of a lot of allergies? Just a show more to toggle all in the patient summary? |
@donaldwasserman I think a show more toggle would be fine if there are too many items to display. |
…into feature/allergy_service
@jkleinsc Made all those changes, wrote some tests/translations. |
@jkleinsc Ok, now this is good to go, I think. Let me know your thoughts. |
@donaldwasserman thanks for the update. I was hoping to look at this today, but didn't get to it. I plan on taking a look at it tomorrow. |
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.
@donaldwasserman thanks for your continued work on this PR!
A couple of code specific comments are below. Also, after playing with the feature, I'm not sure that the Show More functionality is needed. For most patients I don't think the list will be that large and the danger of not always showing all allergies is that a clinician doesn't realize there are more to display. Also, when you add more allergies than are displayed you don't see the newly added allergy unless you then click show more, so it "feels" like the update didn't take place.
@@ -0,0 +1,53 @@ | |||
<label for="">{{t 'allergies.patientAllergy'}}</label> |
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.
The markup here should match what is being used in the patient summary. Also, instead of a button, use a link for the createNewAllergy action
Here is what I am using for the work I am doing for patient diagnosis:
<div class="ps-info-group long-form">
<label class="ps-info-label wide">{{t 'patients.labels.primaryDiagnosis'}}</label>
{{#if canAddDiagnosis}}
<a class="clickable" {{action "showAddDiagnosis" bubbles=false }}><span class="octicon octicon-plus"></span> {{t 'visits.buttons.addDiagnosis' }}</a>
{{/if}}
<div class="ps-info-data-block">
{{#each primaryDiagnoses as |diagnosis index|}}
{{#unless (eq index 0)}}, {{/unless}}
<a class="clickable" {{action "editDiagnosis" diagnosis bubbles=false}}>
{{diagnosis.diagnosis}} (<strong>{{date-format diagnosis.date}}</strong>)
</a>
{{/each}}
</div>
</div>
name: this.get('name') | ||
}); | ||
model.get('allergies').pushObject(allergyModel); | ||
model.save().then(() => { |
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 to save the allergyModel as well. When I look in the database, it currently isn't saving new allergies, but it saves the relationship (to the non saved allergy) on the patient.
{{/if}} | ||
{{/if}} | ||
{{/each}} | ||
{{#if (gt patient.allergies.length 4)}} |
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.
This line should be
{{#if (gt patient.allergies.length 5)}}
because right now the showAll link displays even when there are no more allergies to display.
@donaldwasserman another observation -- typically modals/edit screens in HR change the title/buttons depending upon whether or not an item is new or being edited. It is not a big deal, but I would prefer it for the sake of consistency. |
…into feature/allergy_service
Made those updates you requested, let me know if you have any other changes. |
@donaldwasserman thanks for updating. It looks like Travis testing is failing because of formatting/code issues:
Also, the Add New Allergy button should be a link instead of button and it should be on the same line as the Patient Allergies label, eg: <div class="ps-info-group long-form">
<label class="ps-info-label wide">{{t 'allergies.patientAllergy'}}</label>
{{#if canAddDiagnosis}}
<a class="clickable" {{action "createNewAllergy" bubbles=false }}><span class="octicon octicon-plus"></span> {{t 'visits.buttons.addNewAllergy' }}</a>
{{/if}}
<div class="ps-info-data-block">
... Last thing, the button on Add New Diagnosis should read "Add" instead of "Update" |
…into feature/allergy_service
ok, now i think it's done. |
@donaldwasserman thanks for the updates. When I test clicking on an existing allergy to edit, the modal doesn't appear, it appears because line 28 should be Also, the Add New Allergy action is still showing up as a button, but it should be link. |
Should be set to review. Thanks for keeping me honest on this :) |
Thanks for the updates @donaldwasserman! I'm going to merge this in. |
Fixes #235 #679.
Changes proposed in this pull request:
Not sure about UI/workflow etc on where to go from here.
cc @HospitalRun/core-maintainers