(2931/1) Model a financial value from a csv #2284
Merged
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.
To try and simplify our approach to importing from csv files this
Import::Csv::Financial class is a basic wrapper for a financial value
that is to be imported from csv.
Our philosophy when importing from csv is that a financial value is to expect:
Here, invalid means
This implementation treats a
0
and an empty cell the same,which makes the upstream import code more approachable as we can
assume everything will be
0
.We resue the existing
ConvertFinancialValue
service object to do theactual conversion, dealing with an errors as described above - we chose
this approach over changing the
ConvertFinancialValue
to keep thechanges isolated to the current work.
When we use this class during import, we can expect a financial value to
be one of three states:
Where nil means it could not be converted.
We also retain the original string value from the csv so it can be
presented back to the user in any error messages.
This work is the first in a series which I hope will:
Next step: model the rest of the attributes in a row along with the financial values using this new model.
If you want more context about the longer plan - just ask! :)
https://trello.com/c/lBZkYq5c