-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Conversation
Now, we’re associating to a visit always.
Checked in for collaboration purposes.
I HAVE MADE FIRE!!! Seriously though, the patient-notes feature is functional.
…e patient history. @jkleinsc I want to talk to you about this change.
…tes-feature-try-again # Conflicts: # app/patients/edit/template.hbs
Aligning the date of birth display with the implied standard from the procedure and diagnosis references.
@tangollama unless I am missing something, it looks like your acceptance test doesn't really test the functionality of notes. It should test crud operations on notes, not just test that the modal shows up. |
I needed it for a computed property in patient-note.
Added the translations to the patient note and a type-ahead search for the physician list.
deletePatientNote and showDeletePatientNote
deletePatientNote
authoredBy computed property
Added an insert, update, and delete assertions.
@jkleinsc I've expanded the test suite, addressed several translation issues (including injecting the i18n service into the model), and found and fixed a new bugs along the way. I think this is ready to go. The big question is how we can run the node script for updates. Do we need a release note process? |
An assertion failed in the travis build but passed in my environment. I’m guessing that’s an execution issue on my part. Hoping this resolves it.
The test passes locally. Hoping it will checked in as well.
Boom! @jkleinsc: MERGE. THAT. BRANCH! |
LOL well played gif @tangollama. 👏 😆 PS: to put your gifs inline, just use the markdown image syntax: |
@tangollama settle down... its not like I'm busy with something else right now! LOL! |
Cool. As always, I'm open to instruction and recommendations. Typed or dictated imperfectly on my iPhone (717.385.9970)
|
Per issues pointed out by @jkleinsc, now resolved the size of the On Behalf Of input as well as the “undefined” issue with the background display. Two-way data binding for the win! Now… let’s merge this sucker.
Oooo yeah, I didn't see that. @jkleinsc thanks for the screenshot (ALWAYS BE SCREENSHOTING 😉) That definitely needs some help, although we could take one of two approaches here:
I'm open to either. @tangollama which would you prefer? |
@tangollama awesome! I'd call that Good Enough™ for this branch. I'll address modal styling overall in a new PR. |
Looks good @tangollama. I am going to merge this PR in |
@jkleinsc FYI when you merge a request that way the contributor never gets credit for the contributions as they all become your commits. Maybe not a big deal (especially if you are just doing it in certain cases), but ongoing that will be frustrating to contributors if we use that as a pattern. |
@jglovier I do this for PRs that have lots of commits because I don't want to bring all those commit messages over. I guess we could ask contributors to squash the commits on their end and then we could just merge the PR. If you have other thoughts on how we can better do this, I'm all ears. Actually I would love a Github feature where I can merge a PR and squash the commits into one commit that credits the author! That would make me 😀😀😀😀😀! |
@jkleinsc ask and ye shall receive: https://github.com/blog/2141-squash-your-commits 😄 |
@jglovier I think I shall do a happy dance: |
Addresses issues #32 and #239