Fix for clearballot parsing: ignore quoted text in non-header rows

[artoonie]

Fix for clearballot parsing: ignore quoted text in non-header

[artoonie]

@ryu-seg two requests:

1. Can you split out the regex string into a private static member so it can be reused instead of copy/pasted?
2. Can you bump the version number from RC2 to RC3?

[ryu-seg]

@artoonie done! let me know if that looks good.

[artoonie]

LGTM, thank you! @yezr shall we merge then create a Release for this?

[yezr]

Malachi is confirming locally everything works as expected and when that is done we'll rip a release

[nurse-the-code]

Hey, do you want me to replicate the error on https://github.com/BrightSpots/rcv/tree/develop or https://github.com/BrightSpots/rcv/tree/release/1.3.1-release-candidates?

And which do you want me to test the fix based off of?

Alternatively, I can test the error and the fix off of both trees.

[nurse-the-code]

Hey FYI, this is my plan for testing this.

• Replicate the bug observed in Ignore quoted commas when parsing non-header-row CSV values in ClearBallotCvrReader #880 on develop (canceled)
• Test the fix of the bug (i.e. f27f843 and a8dcb1a) against develop (canceled)
• Replicate the bug observed in Ignore quoted commas when parsing non-header-row CSV values in ClearBallotCvrReader #880 on 1.3.1-release-candidates
• Test the fix of the bug (i.e. f27f843 and a8dcb1a) against 1.3.1-release-candidates

Let me know what you think.

[artoonie]

Good call re: testing. Perhaps it would be useful to create a test case and replicate that test across repos.

I don't think we need an entirely separate test here; perhaps we can modify an existing test and ensure that it fails without this fix, and passes with this fix.

If that sounds good, I can go ahead and make such a test case.

[yezr]

Hey, this is my plan for testing this.

1. Replicate the bug observed in Ignore quoted commas when parsing non-header-row CSV values in ClearBallotCvrReader #880 on develop
2. Test the fix of the bug (i.e. f27f843 and a8dcb1a) against develop
3. Replicate the bug observed in Ignore quoted commas when parsing non-header-row CSV values in ClearBallotCvrReader #880 on 1.3.1-release-candidates
4. Test the fix of the bug (i.e. f27f843 and a8dcb1a) against 1.3.1-release-candidates

We just need the last two. 1.3.1-RC2 fixes haven't made it back to develop yet. Testing develop we would expect to see the parsing issue and it is a little superfluous. We just need to confirm that we can recreate the issue in the 1.3.1-RC2 version Ryu is using and that the new commits fix it there.

[artoonie]

I suspect the merge conflict is just that develop has updated the version number too?

[nurse-the-code]

I suspect the merge conflict is just that develop has updated the version number too?

Maybe, but I don't remember seeing that. It was more that the ClearBallotCvrReader class had changed a bit.

It is not a blocker since we are just testing the fix against release/1.3.1-release-candidates.

I mentioned the merge conflict as an FYI to make sure we know for future reference that when we merge release/1.3.1-release-candidates back into develop that there will be some merge conflicts to fix.

[nurse-the-code]

I tested the PR. The fix works and will resolve #880 once merged. Code looks good.

Only question: should we add a test case demonstrating this fix?

In the meantime, we should probably get this merged for Clear Ballot.