Fixes 572: validating uniqueness of a patient's friendly ID #706
Conversation
|
If two users are adding patients at the same time, both records would get the same friendly ID. To prevent duplicates, I added a new commit to force revalidating before saving a patient. So, if the ID has been taken, we'll see an error message in a modal dialog.
Some rare race conditions might still occur because the enforcement happens in the application level, not the database. But considering the friendly ID is not something we use to uniquely identify the records, perhaps the solution is good enough now. What do you guys think? |
gnowoel
changed the title
gnowoel
changed the title
gnowoel
changed the title
|
@gnowoel thanks for the PR. I have been thinking about this a bit and I think we need to go in a slightly different direction. Here is what I am thinking:
|
|
Sounds good to me :) |
| @@ -547,6 +548,14 @@ export default AbstractEditController.extend(BloodTypes, ReturnTo, UserSession, | |||
| }); | |||
| }, | |||
|
|
|||
| beforeUpdate: function() { | |||
There was a problem hiding this comment.
@gnowoel instead of validating through the model, you should just move the uniqueness code here. Also, it should only run when the model is new (eg if (this.get('model.isNew')
The problem with the code the way it is right now if the model is invalid for other reasons (say for example the user didn't enter a first name, it would change the id).
| @@ -90,7 +90,8 @@ export default AbstractModel.extend(DOBDays, PatientName, { | |||
| } | |||
| }, | |||
| friendlyId: { | |||
| presence: true | |||
| presence: true, | |||
| uniqueness: true | |||
There was a problem hiding this comment.
Remove the validation from the model and just do it in the controller instead.
| }; | ||
|
|
||
| this.set('isValidating', true); | ||
| database.queryMainDB(query, 'patient_by_display_id') |
There was a problem hiding this comment.
Move this code to the controller.
|
@jkleinsc Thanks for your guidance. I updated the code again. Please correct me. |
|
@gnowoel I think this PR is complete if you make one more change. I mentioned disabling the friendly id from editing, but I realized all we need to do is remove the field from the form completely because it is already displayed in the header. So, to complete this PR can you remove this line? |
|
@jkleinsc Thanks for your precise instruction. I have removed the ID field completely from the patient form in this follow-up commit. |
|
@gnowoel thanks for finishing up this PR! Looks good to me, so I'll merge it in. |
I added a custom validator for dynamically showing if the display ID of a patient has already been taken. This should work when either adding or editing a patient. For the former case, we make sure the display ID is not used, and for the latter, we instead check if the display ID is only used by the current record.
This is aiming to solve issue #572 partially. The remaining problem would be also validating the display ID on save, so that two people adding records simultaneously won't result in the same ID. I'll try to tackle that later as a separate task.
I added a custom validator because it seems
ember-validationsdoes not allow promises in an inline one.Please correct me!
cc @HospitalRun/core-maintainers