[FIX] Always round monetary values in database (#17010)

Let `create` and `write` round monetary field values before sending them to the
database. Pass the values to be written to `field.convert_to_column`, so that
the currency can be retrieved from the values, and the value be rounded.
This commit is contained in:
Raphael Collet
2017-05-18 14:15:07 +02:00
committed by GitHub
parent 440177f2fa
commit afef71d6b9
5 changed files with 94 additions and 58 deletions
+1 -1
View File
@@ -81,7 +81,7 @@ class Serialized(fields.Field):
} }
column_type = ('text', 'text') column_type = ('text', 'text')
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
return json.dumps(value) return json.dumps(value)
def convert_to_cache(self, value, record, validate=True): def convert_to_cache(self, value, record, validate=True):
@@ -5,7 +5,7 @@ from datetime import date, datetime
from odoo.exceptions import AccessError, except_orm from odoo.exceptions import AccessError, except_orm
from odoo.tests import common from odoo.tests import common
from odoo.tools import mute_logger from odoo.tools import mute_logger, float_repr
class TestFields(common.TransactionCase): class TestFields(common.TransactionCase):
@@ -305,21 +305,65 @@ class TestFields(common.TransactionCase):
record.number = 2.4999999999999996 record.number = 2.4999999999999996
self.assertEqual(record.number, 2.50) self.assertEqual(record.number, 2.50)
def check_monetary(self, record, amount, currency, msg=None):
# determine the possible roundings of amount
if currency:
ramount = currency.round(amount)
samount = float(float_repr(ramount, currency.decimal_places))
else:
ramount = samount = amount
# check the currency on record
self.assertEqual(record.currency_id, currency)
# check the value on the record
self.assertIn(record.amount, [ramount, samount], msg)
# check the value in the database
self.cr.execute('SELECT amount FROM test_new_api_mixed WHERE id=%s', [record.id])
value = self.cr.fetchone()[0]
self.assertEqual(value, samount, msg)
def test_20_monetary(self): def test_20_monetary(self):
""" test monetary fields """ """ test monetary fields """
record = self.env['test_new_api.mixed'].create({}) model = self.env['test_new_api.mixed']
self.assertTrue(record.currency_id) currency = self.env['res.currency'].with_context(active_test=False)
self.assertEqual(record.currency_id.rounding, 0.01) amount = 14.70126
# the conversion to cache should round the value to 14.700000000000001 for rounding in [0.01, 0.0001, 1.0, 0]:
record.amount = 14.7 # first retrieve a currency corresponding to rounding
self.assertNotEqual(record.amount, 14.7) if rounding:
self.assertEqual(record.amount, 14.700000000000001) currency = currency.search([('rounding', '=', rounding)], limit=1)
self.assertTrue(currency, "No currency found for rounding %s" % rounding)
else:
# rounding=0 corresponds to currency=False
currency = currency.browse()
# however when stored to database, it should be serialized as 14.70 # case 1: create with amount and currency
self.cr.execute('SELECT amount FROM test_new_api_mixed WHERE id=%s', (record.id,)) record = model.create({'amount': amount, 'currency_id': currency.id})
(amount,) = self.cr.fetchone() self.check_monetary(record, amount, currency, 'create(amount, currency)')
self.assertEqual(amount, 14.7)
# case 2: assign amount
record.amount = 0
record.amount = amount
self.check_monetary(record, amount, currency, 'assign(amount)')
# case 3: write with amount and currency
record.write({'amount': 0, 'currency_id': False})
record.write({'amount': amount, 'currency_id': currency.id})
self.check_monetary(record, amount, currency, 'write(amount, currency)')
# case 4: write with amount only
record.write({'amount': 0})
record.write({'amount': amount})
self.check_monetary(record, amount, currency, 'write(amount)')
# case 5: write with amount on several records
records = record + model.create({'currency_id': currency.id})
records.write({'amount': 0})
records.write({'amount': amount})
for record in records:
self.check_monetary(record, amount, currency, 'multi write(amount)')
def test_21_date(self): def test_21_date(self):
""" test date fields """ """ test date fields """
+31 -23
View File
@@ -18,7 +18,7 @@ except ImportError:
import psycopg2 import psycopg2
from odoo.sql_db import LazyCursor from odoo.sql_db import LazyCursor
from odoo.tools import float_precision, float_repr, float_round, frozendict, html_sanitize, human_size, pg_varchar, ustr, OrderedSet, pycompat from odoo.tools import float_repr, float_round, frozendict, html_sanitize, human_size, pg_varchar, ustr, OrderedSet, pycompat
from odoo.tools import DEFAULT_SERVER_DATE_FORMAT as DATE_FORMAT from odoo.tools import DEFAULT_SERVER_DATE_FORMAT as DATE_FORMAT
from odoo.tools import DEFAULT_SERVER_DATETIME_FORMAT as DATETIME_FORMAT from odoo.tools import DEFAULT_SERVER_DATETIME_FORMAT as DATETIME_FORMAT
from odoo.tools.translate import html_translate, _ from odoo.tools.translate import html_translate, _
@@ -737,7 +737,7 @@ class Field(MetaField('DummyField', (object,), {})):
""" Return the null value for this field in the record format. """ """ Return the null value for this field in the record format. """
return False return False
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
""" Convert ``value`` from the ``write`` format to the SQL format. """ """ Convert ``value`` from the ``write`` format to the SQL format. """
if value is None or value is False: if value is None or value is False:
return None return None
@@ -1140,7 +1140,7 @@ class Boolean(Field):
type = 'boolean' type = 'boolean'
column_type = ('bool', 'bool') column_type = ('bool', 'bool')
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
return bool(value) return bool(value)
def convert_to_cache(self, value, record, validate=True): def convert_to_cache(self, value, record, validate=True):
@@ -1161,7 +1161,7 @@ class Integer(Field):
_description_group_operator = property(attrgetter('group_operator')) _description_group_operator = property(attrgetter('group_operator'))
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
return int(value or 0) return int(value or 0)
def convert_to_cache(self, value, record, validate=True): def convert_to_cache(self, value, record, validate=True):
@@ -1225,7 +1225,7 @@ class Float(Field):
_description_digits = property(attrgetter('digits')) _description_digits = property(attrgetter('digits'))
_description_group_operator = property(attrgetter('group_operator')) _description_group_operator = property(attrgetter('group_operator'))
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
result = float(value or 0.0) result = float(value or 0.0)
digits = self.digits digits = self.digits
if digits: if digits:
@@ -1279,25 +1279,33 @@ class Monetary(Field):
assert self.currency_field in model._fields, \ assert self.currency_field in model._fields, \
"Field %s with unknown currency_field %r" % (self, self.currency_field) "Field %s with unknown currency_field %r" % (self, self.currency_field)
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
try: # retrieve currency from values or record
return value.float_repr() # see float_precision.float_repr() if values and self.currency_field in values:
except Exception: field = record._fields[self.currency_field]
return float(value or 0.0) currency = field.convert_to_cache(values[self.currency_field], record)
currency = field.convert_to_record(currency, record)
else:
# Note: this is wrong if 'record' is several records with different
# currencies, which is functional nonsense and should not happen
currency = record[:1][self.currency_field]
value = float(value or 0.0)
if currency:
return float_repr(currency.round(value), currency.decimal_places)
return value
def convert_to_cache(self, value, record, validate=True): def convert_to_cache(self, value, record, validate=True):
if validate: # cache format: float
currency = record[self.currency_field] value = float(value or 0.0)
if validate and record[self.currency_field]:
# FIXME @rco-odoo: currency may not be already initialized if it is # FIXME @rco-odoo: currency may not be already initialized if it is
# a function or related field! # a function or related field!
if currency: value = record[self.currency_field].round(value)
value = currency.round(float(value or 0.0)) return value
return float_precision(value, currency.decimal_places)
return float(value or 0.0)
def convert_to_read(self, value, record, use_name_get=True): def convert_to_read(self, value, record, use_name_get=True):
# float_precision values are not supported in pure XMLRPC return value
return float(value)
def convert_to_write(self, value, record): def convert_to_write(self, value, record):
return value return value
@@ -1398,7 +1406,7 @@ class Char(_String):
assert self.size is None or isinstance(self.size, int), \ assert self.size is None or isinstance(self.size, int), \
"Char field %s with non-integer size %r" % (self, self.size) "Char field %s with non-integer size %r" % (self, self.size)
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
if value is None or value is False: if value is None or value is False:
return None return None
# we need to convert the string to a unicode object to be able # we need to convert the string to a unicode object to be able
@@ -1463,7 +1471,7 @@ class Html(_String):
_description_strip_style = property(attrgetter('strip_style')) _description_strip_style = property(attrgetter('strip_style'))
_description_strip_classes = property(attrgetter('strip_classes')) _description_strip_classes = property(attrgetter('strip_classes'))
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
if value is None or value is False: if value is None or value is False:
return None return None
if self.sanitize: if self.sanitize:
@@ -1644,7 +1652,7 @@ class Binary(Field):
_description_attachment = property(attrgetter('attachment')) _description_attachment = property(attrgetter('attachment'))
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
# Binary values may be byte strings (python 2.6 byte array), but # Binary values may be byte strings (python 2.6 byte array), but
# the legacy OpenERP convention is to transfer and store binaries # the legacy OpenERP convention is to transfer and store binaries
# as base64-encoded strings. The base64 string may be provided as a # as base64-encoded strings. The base64 string may be provided as a
@@ -1804,7 +1812,7 @@ class Selection(Field):
return item[1] return item[1]
return False return False
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
""" Convert ``value`` from the ``write`` format to the SQL format. """ """ Convert ``value`` from the ``write`` format to the SQL format. """
if value is None or value is False: if value is None or value is False:
return None return None
@@ -1971,7 +1979,7 @@ class Many2one(_Relational):
""" """
records._cache[self] = self.convert_to_cache(value, records, validate=False) records._cache[self] = self.convert_to_cache(value, records, validate=False)
def convert_to_column(self, value, record): def convert_to_column(self, value, record, values=None):
return value or None return value or None
def convert_to_cache(self, value, record, validate=True): def convert_to_cache(self, value, record, validate=True):
+6 -4
View File
@@ -3066,7 +3066,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
if field.column_type: if field.column_type:
if single_lang or not (has_trans and field.translate is True): if single_lang or not (has_trans and field.translate is True):
# val is not a translation: update the table # val is not a translation: update the table
val = field.convert_to_column(val, self) val = field.convert_to_column(val, self, vals)
updates.append((name, field.column_format, val)) updates.append((name, field.column_format, val))
direct.append(name) direct.append(name)
else: else:
@@ -3106,7 +3106,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
# Insert value to DB # Insert value to DB
src_trans = vals[name] src_trans = vals[name]
self.with_context(lang=None).write({name: src_trans}) self.with_context(lang=None).write({name: src_trans})
val = field.convert_to_column(vals[name], self) val = field.convert_to_column(vals[name], self, vals)
tname = "%s,%s" % (self._name, name) tname = "%s,%s" % (self._name, name)
self.env['ir.translation']._set_ids( self.env['ir.translation']._set_ids(
tname, 'model', self.env.lang, self.ids, val, src_trans) tname, 'model', self.env.lang, self.ids, val, src_trans)
@@ -3330,7 +3330,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
parent_id = self.env[parent_model].create(parent_vals).id parent_id = self.env[parent_model].create(parent_vals).id
else: else:
self.env[parent_model].browse(parent_id).write(parent_vals) self.env[parent_model].browse(parent_id).write(parent_vals)
updates.append((self._inherits[parent_model], '%s', parent_id)) vals[self._inherits[parent_model]] = parent_id
# set boolean fields to False by default (to make search more powerful) # set boolean fields to False by default (to make search more powerful)
for name, field in pycompat.items(self._fields): for name, field in pycompat.items(self._fields):
@@ -3338,10 +3338,12 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
vals[name] = False vals[name] = False
# determine SQL values # determine SQL values
self = self.browse()
for name, val in pycompat.items(vals): for name, val in pycompat.items(vals):
field = self._fields[name] field = self._fields[name]
if field.store and field.column_type: if field.store and field.column_type:
updates.append((name, field.column_format, field.convert_to_column(val, self))) column_val = field.convert_to_column(val, self, vals)
updates.append((name, field.column_format, column_val))
else: else:
upd_todo.append(name) upd_todo.append(name)
-18
View File
@@ -180,24 +180,6 @@ def float_split(value, precision_digits):
return int(units), int(cents) return int(units), int(cents)
class float_precision(float):
""" A class for float values that carry precision digits. This is a thin
layer on top of ``float``, and the precision digits are not propagated
to the result of arithmetic operations. This class is used when
converting monetary values to cache, and for serializing them to the
database.
"""
__slots__ = ['precision_digits']
def __new__(cls, value, precision_digits):
obj = super(float_precision, cls).__new__(cls, value)
obj.precision_digits = precision_digits
return obj
def float_repr(self):
return _float_repr(self, self.precision_digits)
if __name__ == "__main__": if __name__ == "__main__":
import time import time