Contribution import - tests & fixes on dates, amount #23688
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
eileenmcnaughton
force-pushed
the
import_cont_tests
branch
4 times, most recently
from
6990696 to
51c0df5
Compare
Fixes inconsistent amount handling - these are issues that I found through writing unit tests to cover them The money handling was permitting amounts where the decimal separator was before the thousand. On the dates there was a format being handled for some fields but not others
eileenmcnaughton
force-pushed
the
import_cont_tests
branch
from
51c0df5 to
4b58c5c
Compare
|
This seems fine to me and has unit tests to support it |
|
thanks @seamuslee001 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Contribution import - tests & fixes on dates, amount
Before
Testing on contributions covered the validate function - but not the full process of submitted the form
After
Testing is more complete & would have picked up previous breakage
Technical Details
In the process of writing a test I found the money validation is a bit alarming in that it was not rejecting dates where the thousand separator was AFTER the decimal separator - meaning that
12.000,45was being converted to 1200045 rather than rejected. I added opt in validation to the rule with some commentary about how that feels too conservative so we might re-address in futureComments