-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix handling of the Postgres money field. #14931
Conversation
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Just wondering what would happen if you tried to update a money field with three decimal places, e.g. `"200.123" ?
I'm uncertain if this is the correct behaviour or not. |
That looks correct to me. Yep! https://www.postgresql.org/docs/current/datatype-money.html Basically assumes dollars or similar two decimal currency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💸
Description
In #14861 I introduced a bug that broke our handling of the Postgres money field. We had no tests around this fields, so on top of fixing the bug I have also introduced new tests to prevent this happening again.