BFD-3728: Refactor NPI Module to encompass all NPI requests #2539
Conversation
dondevun
requested review from
mjburling,
aschey-forpeople and
malessi
as code owners
dondevun
changed the title
| * @return true if the stream is deflated | ||
| * @throws IOException on read error. | ||
| */ | ||
| public boolean isStreamDeflated(InputStream inputStream) throws IOException { |
There was a problem hiding this comment.
This is necessary because the test data isn't deflated, while the production data is; so the easiest way to determine if it is a compressed stream is to look for the zlib signature.
| * | ||
| * @param orgLookup The instance of NPIOrgLookup. | ||
| */ | ||
| public static void setNpiOrgLookup(NPIOrgLookup orgLookup) { |
There was a problem hiding this comment.
CommonTransformerUtils is not a managed bean (and isn't instantiated, anyway), so the NPIOrgLookup instance is set here from the ServerInitializer (which has access to the NPIOrgLookup managed bean).
| .providerCredential(providerCredential) | ||
| .build(); | ||
| String json = objectMapper.writeValueAsString(npiData); | ||
| dos.write((npi + "\t" + json).getBytes()); |
There was a problem hiding this comment.
Do we still need to do this tab-separated format when the npi value is already part of the json object that's being written?
There was a problem hiding this comment.
No, we don't. Updated.
| @@ -3133,8 +3134,7 @@ | |||
| }, | |||
| "system" : "http://hl7.org/fhir/sid/us-npi", | |||
| "value" : "1023011079" | |||
| }, | |||
| "display" : "ADVANTAGE HOME HEALTH CARE, INC." | |||
There was a problem hiding this comment.
Why are these display values missing now?
There was a problem hiding this comment.
Let me look into that.
There was a problem hiding this comment.
Thanks for catching that, turned out to be a bug in the new code.
| String npi; | ||
|
|
||
| /** Entity Type. Will be 1 or 2. */ | ||
| String entityTypeCode; |
There was a problem hiding this comment.
Maybe it would be cleaner to have an enum for this? Or at least a constant. Because it's not clear what entity type code 1 or 2 means.
There was a problem hiding this comment.
I think an enum is overkill, but I think you're right about the constant.
JIRA Ticket:
BFD-3728
What Does This PR Do?
Refactors the NPI Org module to return Practitioner NPI information, and replaces
retrieveNpiCodeDisplaymethod inCommonTransformerUtils.javawith code to use this new lookup (instead of the flat file, which was previously used).What Should Reviewers Watch For?
If you're reviewing this PR, please check for these things in particular:
What Security Implications Does This PR Have?
Please indicate if this PR does any of the following:
Adds any new software dependencies
Modifies any security controls
Adds new transmission or storage of data
Any other changes that could possibly affect security?
I have considered the above security implications as it relates to this PR. (If one or more of the above apply, it cannot be merged without the ISSO or team security engineer's (
@sb-benohe) approval.)Validation
Have you fully verified and tested these changes? Is the acceptance criteria met? Please provide reproducible testing instructions, code snippets, or screenshots as applicable.
Verified that Unit tests and integration tests properly use the new lookup, and additionally verified on a local server against the golden bene in the test database.