Skip to content

Conversation

@dmendelowitz
Copy link
Contributor

Summary

The patient IDs CSV is parsed in app.js rather than in CSVModule.js like the other CSVs and we forgot to add the BOM flag to that instance of the parse function being called

New behavior

BOMs are now handled when parsing the Patient IDs CSV

Code changes

The call to the CSV parse function in app.js now has the flag to properly handle BOMs

Testing guidance

To test that this works, you can go and replace the fs.readFileSync() call on line 87 of app.js with the following '\uFEFFmrn\n123\n456\n789\n1011' (the same as the sample client data but with a BOM added at the start). The client should still run as expected.

Copy link
Contributor

@jafeltra jafeltra left a comment

Choose a reason for hiding this comment

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

Great catch!

Copy link
Contributor

@Dtphelan1 Dtphelan1 left a comment

Choose a reason for hiding this comment

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

We're so good at software development 🤦

@dmendelowitz dmendelowitz merged commit c866ea3 into develop May 19, 2021
@dmendelowitz dmendelowitz deleted the better-bom-parsing branch May 19, 2021 18:24
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.

4 participants