Should Patient Identifier Location Be Required?

Development
1

Currently, in PatientIdentifierResource1_8.java, identifier location is optional:

However, when I try to create a patient like this:

curl -X POST -H "Authorization: Basic YWRtaW46QWRtaW4xMjM=" -H "Content-Type: application/json" -d '{
    "person": {
        "names": [{
            "givenName": "John",
            "familyName": "Doe"
        }],
        "gender": "M"
    },
    "identifiers": [{
        "identifierType": "05a29f94-c0ed-11e2-94be-8c13b969e334",
        "identifier": "123456"
    }]
}' "http://localhost:8082/openmrs-standalone/ws/rest/v1/patient"

I get:

{
  "error": {
    "message": "Invalid Submission",
    "code": "webservices.rest.error.invalid.submission",
    "globalErrors": [{
      "code": "Identifier Location cannot be null for 123456",
      "message": "Identifier Location cannot be null for 123456"
    }],
    "fieldErrors": {}
  }
}

If I specify a location, it works. Am I doing something wrong or should I send a PR?

2

Most probably you set the location behavior as required for this patient identifier type. Here is an example screen to look at: http://demo.openmrs.org/openmrs/admin/patients/patientIdentifierType.form?patientIdentifierTypeId=3

3

@dkayiwa, thanks for pointing this out. However, my location behaviour is actually blank/unset:

Screen Shot 2016-04-06 at 9.02.11 AM.png1482Ă—1338 156 KB

When I change it to Not used, then it works, but when I change it back blank/unset, then I get the error again.

It seems like the blank/unset option has the same effect as Required (at least in this case), so what is reason for the blank/unset option?

4

According to this:

the default behaviour is to require it. So blank is the same as required.

1 Like
5

Ah okay, I see. So then I guess there is no value in having a blank/unset option?

7

This probably just dates back to the fact that “location behavior” was a later addition to patient identifiers–it used to that the locations were always required, and we added “location behavior” to support making it not required. Assumedly when the “location_behavior” column was added to patient_identiifer_type, for backwards compatibility it was decided that the simplest option was to make it nullable but have null = required. Other option would have been to make it “not null” but when adding the column setting all all existing rows to “required”.

1 Like
8

I would think we should have had a database migration set it to required for everything, and disallow null/blank. I don’t know what actually happened though. :slight_smile:

2 Likes
9

Okay, thanks everyone. I’ll create a ticket just so we don’t lose track of this.

EDIT: Ticket:

10

That’s the best solution, another one is use a Hibernate user type that converts NULL to FALSE (in case it is forbidden to change the database).

1 Like
11

Hi @pascal

I am working on the TRUNK-4847. As a part of this ticket, I guess we need to add NOT NULL constraint to the location_behavior column of patient_identifier_type table.

But I should make sure this change should not disturb the work done on TRUNK-427.

I request some senior dev go through this commit and confirm :slight_smile:

12

Based on TRUNK-427, it looks like we initially wanted three types of location behaviors (REQUIRED, NOT USED & OPTIONAL). I guess that during implementation it became clear that we didn’t really need OPTIONAL.

We should probably ask @mseaton for input and maybe to do the review.

1 Like
13

FYR The thread, where senior devs discussed regarding