Skip to content

Conversation

@JamesbZhu
Copy link
Contributor

Related JIRA tickets:

Please enter the whole URL so we can just click/copy it
-https://jira.amida.com/projects/HL7/issues/HL7-38

What exactly does this PR do?

i.e. Added logic to do the thing because the thing didn't exist before. Please provide as much detail as possible.

  • adding the ability to verify that a uploaded text file is an hl7 file by asserting that it starts with a message header (MSH|)

What else was added outside of the scope of the ask?

i.e. Simple bugfix, corrected typos, cleaned up code, etc

Do ANY of these changes introduce a breaking change? (in-scope or otherwise)

i.e. Users need to update their .env files with the following... etc

Manual test cases?

Steps to manually test introduced changes can go here.
Remember the prerequisites!
Remember edge-cases you've caught in your code, etc..
i.e. ignoring the button and clicking on the "NEXT" button should trigger a warning event because the thing that clicking the new button does didn't happen

  • run yarn start
  • upload a text file that does not start with "MSH|" on postman
  • assert that the returned error reads "Please upload a valid HL7 file"

Any additional context/background?

N/A if this is straight-forward

  • N/A

Screenshots (if appropriate):

@JamesbZhu JamesbZhu requested a review from Perry5 September 5, 2019 21:25
Copy link
Contributor

@Perry5 Perry5 left a comment

Choose a reason for hiding this comment

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

Good work!

Left a small comment. Maybe we can also add a unit test for this case?

I will also leave this PR opened until we make the change to prevent saving the file to the disk. @amathew8 is creating the ticket for this.

}).catch(err =>
next(new APIError(err, httpStatus.BAD_REQUEST))
).then((data) => {
if (!data.startsWith('MSH|')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This approach works since (I believe) all HL7 messages have to start with the message header (MSH)

next(new APIError(err, httpStatus.BAD_REQUEST))
).then((data) => {
if (!data.startsWith('MSH|')) {
throw new Error('Please upload a valid HL7 file');
Copy link
Contributor

Choose a reason for hiding this comment

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

Throwing an error here like this will cause the sever to return a 500. We can instead do this:
next(new APIError('Please upload a valid HL7 file', httpStatus.BAD_REQUEST)); - this will return a 400 Bad Request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants