fix(#641): allow csvs with blank rows

[garethbowen]

Closes #641

I have verified this PR works in these browsers (latest versions):

• Chrome
• Firefox
• Safari (macOS)
• Safari (iOS)
• Chrome for Android
• Not applicable

What else has been done to verify that this works as intended?

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Do we need any specific form for testing your changes? If so, please attach one.

csv_blank_lines.csv
csv_blank_lines.xml

What's changed

[changeset-bot]

🦋 Changeset detected

Latest commit: 6b914f2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@getodk/xforms-engine	Patch
@getodk/web-forms	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[garethbowen]

@latin-panda I really wanted to implement a unit test for this (or at least, somewhat close to a unit test) because it's testing an error condition which is painful if you have to create a specific csv and integration test for each possible error. But because the CSVExternalSecondaryInstanceSource file uses a lot of types and inheritance it's very difficult to just test what you want because you end up having to mock a whole lot of properties which we don't actually use in the unit. So long story short, I refactored it a lot to pull out a utility class for just doing the parsing, leaving the type in place so the hierarchy still works. Please let me know if you like this pattern because I think it'll make the code much more testable so if you're on board I'll do it more in future!

To make the review a little easier the first commit in the PR is just the refactor and adding unit tests with a it.fails for the specific bug in the issue. The second commit is the change to actually fix the issue.

[latin-panda]

Yes, the extraction to a library-like structure makes sense to me. I prefer it for testability while still keeping type safety