Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add patientIdentifierType resources #10

Merged
merged 8 commits into from
Oct 30, 2019
Merged

Conversation

batbrain7
Copy link
Collaborator

@batbrain7 batbrain7 commented Oct 13, 2019

Description

Added the patientIdentifierType resource.

patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
@batbrain7 batbrain7 changed the title [WIP] chore: add patientIdentifierType resources chore: add patientIdentifierType resources Oct 14, 2019
Copy link
Member

@bmamlin bmamlin left a comment

Choose a reason for hiding this comment

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

Close. Small changes requested.

patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
patientIdentifierType.md Outdated Show resolved Hide resolved
@batbrain7
Copy link
Collaborator Author

@bmamlin updated the PR, please review

Comment on lines 66 to 67
"name": "Wilson Hospital Medical Record Number",
"description": "Wilson Hospital Medical Record Number is used as a patientIdentifier type.",
Copy link
Member

Choose a reason for hiding this comment

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

Names are used as labels, so better to keep short. Description is user-facing information to clarify a likely abbreviated name. Users would not be expected to know or care about an internal Java class like PatientIdentifierType. I would suggest something like:

"name": "Wilson Hosp MRN",
"description": "Wilson Hospital Medical Record Number",

{
"name": "Wilson Hospital Medical Record Number",
"description": "Wilson Hospital Medical Record Number is used as a patientIdentifier type.",
"format": "234-7",
Copy link
Member

Choose a reason for hiding this comment

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

As your documentation explains, format is "a regular expression defining what the identifier text should contain." The regular expression "234-7" only matches a single string: the literal value "234-7". To make matters worse, not even the exact value would be accepted, since it has an invalid check digit.

I would suggest:

"format": "\d{1,10}-\d",

The regular expression \d{1,10}-\d matches 1-10 digit(s) (\d{1,10}) followed by a literal hyphen (-) and a single digit. If you want to limit the medical record system to 1000 patients by using the pattern NNN-N, the corresponding regular expression would be \d{3}-\d.

Comment on lines 81 to 85
#### Properties

Parameter | Type | Description
--- | --- | ---
*Object* | `PatientIdentifierType` | Patient resource with updated properties (same body as while creating the resource body)
Copy link
Member

Choose a reason for hiding this comment

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

The body of the POST isn't a property. I would eliminate this section.


```console
POST /patientidentifertype/:target_patientidentifiertype_uuid
-d modified_patientidentifiertype_object
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what this means. I assume the -d is a reference to a cURL command, suggesting the updates would be in JSON format in a file named "modified_patientidentifiertype_object". Please omit the -d option and include an example JSON object after the POST command. See encounter.md for an example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure, will update

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bmamlin for this particular object I'll use this one :
` {
"name": "Wilson Hosp MRN",

    "description": "Wilson Hospital Medical Record Number",

    "format": "\d{1,10}-\d",

    "formatDescription": "Up to ten digts followed by a hyphen and another digit",

    "required": true,

    "validator": "org.openmrs.patient.impl.LuhnIdentifierValidator",

    "checkDigit": true,

    "locationBehavior": "REQUIRED",

    "uniquenessBehavior": "Unique"

}`

&q=**searchqQuery**
```

* #### List patientIdentifierType by UUID.
Copy link
Member

Choose a reason for hiding this comment

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

This is "Get", not "List", since you are fetching a single resource.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure, will update


```console
GET /patientidentifiertype?
&q=**searchqQuery**
Copy link
Member

Choose a reason for hiding this comment

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

?q={searchQuery} doesn't make sense here, since you are describing getting all patient identifier types. If searching for patient identifier types was supported (I don't think it is), you would want to describe it separately.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, to list the patientIdentiferTypes nothing is required, will update

Copy link
Member

@bmamlin bmamlin left a comment

Choose a reason for hiding this comment

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

Thanks for the changes!

@bmamlin bmamlin merged commit 873294b into master Oct 30, 2019
@Akayeshmantha Akayeshmantha deleted the patientIdentifierBranch branch November 6, 2019 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants