[BI-641] - Validate lower and upper limits for correctness#73
[BI-641] - Validate lower and upper limits for correctness#73
Conversation
ctucker3
force-pushed
the
feature/BI-641
branch
2 times, most recently
from
a5e478a to
59da9ac
Compare
| .max(trait.getScale().getValidValueMax()) | ||
| .min(trait.getScale().getValidValueMin()); | ||
| .categories(trait.getScale().getCategories()); | ||
| if (trait.getScale().getValidValueMax() != null) { |
There was a problem hiding this comment.
My suggestion would be to have the BigDecimal#setScale things done directly in the Scale class. This would mean overriding the setValidValueMax and setValidValueMin methods that are currently handled by Lombok
There was a problem hiding this comment.
Then the max is less than valid value min check would be after rounding which I guess is what you'd want since that's what's being sent out.
There was a problem hiding this comment.
Wouldn't that be the same as if we had kept the scale valid value max and min as an integer?
I like the idea of having it in the class though. Maybe I could add a getBrAPIValidMax and min?
There was a problem hiding this comment.
Yeah, good question. Maybe I missed why that was changed to BigDecimal from Integer, @ctucker3 do you know?
There was a problem hiding this comment.
Tim and I talked about this. The BigDecimal doesn't have a use since it will never be persisted. I think this card was initially created when we were going to bring all of the trait data into our schema.
I reverted all of those related changes and just implemented the min/max check.
DebWeigand
left a comment
DebWeigand
left a comment
There was a problem hiding this comment.
The row numbers in the error messages are off by 2
ctucker3
force-pushed
the
feature/BI-641
branch
2 times, most recently
from
74a3d6b to
f3dbd61
Compare
I checked your file with the new changes for this card and the errors started off at row 2. Let me know if it is working for you now as well. |
ctucker3
force-pushed
the
feature/BI-641
branch
from
7890358 to
738f859
Compare
4 participants
Looks like a big PR, but is mostly changes to test import files. The change to BigDecimal in the scale class required a few extra changes.
This is based off on BI-645, so that should be reviewed and merged before the review of this.