diff --git a/trytond/CHANGELOG b/trytond/CHANGELOG index c008cdf270..eefc5e00b1 100644 --- a/trytond/CHANGELOG +++ b/trytond/CHANGELOG @@ -1,3 +1,4 @@ +* Include row and column in error message when importing data * Remove support for ``:id`` suffix in import data * Add export translations option to trytond-admin * Update TinyMCE to version 7 diff --git a/trytond/trytond/ir/message.xml b/trytond/trytond/ir/message.xml index 861a82a6ee..d9a9e83671 100644 --- a/trytond/trytond/ir/message.xml +++ b/trytond/trytond/ir/message.xml @@ -152,22 +152,22 @@ this repository contains the full copyright notices and license terms. --> This record is part of the base configuration. - Missing values for last column. + Missing values for last column at row %(row)d. - Missing values for %(n)s last columns. + Missing values for %(n)s last columns at row %(row)d. - Relation not found: %(value)r in "%(model)s" (%(column)s). + Could not find relation for %(value)r in "%(model)s" at row %(row)d and column %(column)d. - Too many relations found: %(value)r in "%(model)s" (%(column)s). + Found too many relation for %(value)r in "%(model)s" at row %(row)d and column %(column)d. - Syntax error for value: %(value)r in "%(field)s" of "%(model)s" (%(column)s). + Syntax error for %(value)r in "%(field)s" of "%(model)s" at row %(row)d and column %(column)d. - Syntax error for reference: %(value)r in "%(field)s" of "%(model)s" (%(column)s). + Syntax error for %(value)r in "%(field)s" of "%(model)s" at row %(row)d and column %(column)d. The value "%(value)s" for field "%(field)s" in record "%(record)s" of "%(model)s" is not valid according to its domain. diff --git a/trytond/trytond/model/modelstorage.py b/trytond/trytond/model/modelstorage.py index 46a1040857..b85cacb9b9 100644 --- a/trytond/trytond/model/modelstorage.py +++ b/trytond/trytond/model/modelstorage.py @@ -838,8 +838,27 @@ class ModelStorage(Model): ''' pool = Pool() + row_index = data.index + + class _Error(Exception): + def __init__(self, msg_id, **variables): + self.msg_id = msg_id + self.variables = variables + + def error(func): + def wrapper(line, index, *args, **kwargs): + try: + return func(line[index], *args, **kwargs) + except _Error as e: + raise ImportDataError(gettext( + e.msg_id, **e.variables, + row=row_index(line) + 1, + column=index + 1)) from e + return wrapper + + @error @lru_cache(maxsize=1000) - def get_many2one(relation, value, column): + def get_many2one(value, relation): if not value: return None Relation = pool.get(relation) @@ -847,23 +866,22 @@ class ModelStorage(Model): ('rec_name', '=', value), ], limit=2) if len(res) < 1: - raise ImportDataError(gettext( - 'ir.msg_relation_not_found', - value=value, - column=column, - **Relation.__names__())) + raise _Error( + 'ir.msg_relation_not_found', + value=value, + **Relation.__names__()) elif len(res) > 1: - raise ImportDataError( - gettext('ir.msg_too_many_relations_found', - value=value, - column=column, - **Relation.__names__())) + raise _Error( + 'ir.msg_too_many_relations_found', + value=value, + **Relation.__names__()) else: res = res[0].id return res + @error @lru_cache(maxsize=1000) - def get_many2many(relation, value, column): + def get_many2many(value, relation): if not value: return None res = [] @@ -874,52 +892,48 @@ class ModelStorage(Model): ('rec_name', '=', word), ], limit=2) if len(res2) < 1: - raise ImportDataError( - gettext('ir.msg_relation_not_found', - value=word, - column=column, - **Relation.__names__())) + raise _Error( + 'ir.msg_relation_not_found', + value=word, + **Relation.__names__()) elif len(res2) > 1: - raise ImportDataError( - gettext('ir.msg_too_many_relations_found', - value=word, - column=column, - **Relation.__names__())) + raise _Error( + 'ir.msg_too_many_relations_found', + value=word, + **Relation.__names__()) else: res.append(res2[0].id) return res - def get_one2one(relation, value, column): - return ('add', get_many2one(relation, value, column)) + def get_one2one(*args, **kwargs): + return ('add', get_many2one(*args, **kwargs)) + @error @lru_cache(maxsize=1000) - def get_reference(value, field, klass, column): + def get_reference(value, field, klass): if not value: return None try: relation, value = value.split(',', 1) Relation = pool.get(relation) except (ValueError, KeyError) as e: - raise ImportDataError( - gettext('ir.msg_reference_syntax_error', - value=value, - column=column, - **klass.__names__(field))) from e + raise _Error( + 'ir.msg_reference_syntax_error', + value=value, + **klass.__names__(field)) from e res = Relation.search([ ('rec_name', '=', value), ], limit=2) if len(res) < 1: - raise ImportDataError(gettext( - 'ir.msg_relation_not_found', - value=value, - column=column, - **Relation.__names__())) + raise _Error( + 'ir.msg_relation_not_found', + value=value, + **Relation.__names__()) elif len(res) > 1: - raise ImportDataError( - gettext('ir.msg_too_many_relations_found', - value=value, - column=column, - **Relation.__names__())) + raise _Error( + 'ir.msg_too_many_relations_found', + value=value, + **Relation.__names__()) else: res = '%s,%s' % (relation, res[0].id) return res @@ -933,7 +947,8 @@ class ModelStorage(Model): create.append(row) return id_ - def convert(value, ftype, field, klass, column): + @error + def convert(value, ftype, field, klass): def convert_boolean(value): if value.lower() == 'true': return True @@ -995,15 +1010,16 @@ class ModelStorage(Model): return value try: - return locals()['convert_%s' % ftype](value) + func = locals()['convert_%s' % ftype] except KeyError: return value + try: + return func(value) except (ValueError, TypeError, decimal.InvalidOperation) as e: - raise ImportDataError( - gettext('ir.msg_value_syntax_error', - value=value, - column=column, - **klass.__names__(field))) from e + raise _Error( + 'ir.msg_value_syntax_error', + value=value, + **klass.__names__(field)) from e def process_lines(data, prefix, fields_def, position=0, klass=cls): line = data[position] @@ -1016,18 +1032,19 @@ class ModelStorage(Model): n = len(fields_names) - len(line) if n > 1: raise ImportDataError(gettext( - 'ir.msg_import_data_missing_columns', n=n)) + 'ir.msg_import_data_missing_columns', + n=n, + row=row_index(line) + 1)) else: raise ImportDataError(gettext( - 'ir.msg_import_data_missing_column')) + 'ir.msg_import_data_missing_column', + row=row_index(line) + 1)) # Import normal fields_names for i, field in enumerate(fields_names): is_prefix_len = (len(field) == (prefix_len + 1)) - value = line[i] - column = '/'.join(field) if is_prefix_len and ':lang=' in field[-1]: field_name, lang = field[-1].split(':lang=') - translate.setdefault(lang, {})[field_name] = value + translate.setdefault(lang, {})[field_name] = line[i] elif is_prefix_len and prefix == field[:-1]: field_name = field[-1] this_field_def = fields_def[field_name] @@ -1035,25 +1052,22 @@ class ModelStorage(Model): res = None if field_name == 'id': try: - res = int(value) + res = int(line[i]) except ValueError: - res = get_many2one(klass.__name__, value, column) + res = get_many2one(line, i, klass.__name__) elif field_type == 'many2one': - res = get_many2one( - this_field_def['relation'], value, column) + res = get_many2one(line, i, this_field_def['relation']) elif field_type == 'many2many': many2many.add(field_name) res = get_many2many( - this_field_def['relation'], value, column) + line, i, this_field_def['relation']) res = [('add', res)] if res else [] elif field_type == 'one2one': - res = get_one2one( - this_field_def['relation'], value, column) + res = get_one2one(line, i, this_field_def['relation']) elif field_type == 'reference': - res = get_reference(value, field_name, klass, column) + res = get_reference(line, i, field_name, klass) else: - res = convert( - value, field_type, field_name, klass, column) + res = convert(line, i, field_type, field_name, klass) row[field_name] = res elif prefix == field[0:prefix_len]: todo.add(field[prefix_len]) @@ -1117,7 +1131,8 @@ class ModelStorage(Model): to_create, to_create_translations = [], [] to_write, to_write_translations = [], [] languages = set() - while len(data): + data = data.copy() + while data: (row, _, translate) = \ process_lines(data, [], fields_def) if dispatch(to_create, to_write, row): diff --git a/trytond/trytond/tests/test_importdata.py b/trytond/trytond/tests/test_importdata.py index 8d2ba3f37d..cadd464495 100644 --- a/trytond/trytond/tests/test_importdata.py +++ b/trytond/trytond/tests/test_importdata.py @@ -24,10 +24,12 @@ class ImportDataTestCase(unittest.TestCase): pool = Pool() Model = pool.get('test.import_data') - with self.assertRaisesRegex(ImportDataError, "for last column"): + with self.assertRaisesRegex( + ImportDataError, "for last column at row 2"): Model.import_data(['name'], [["Test"], []]) - with self.assertRaisesRegex(ImportDataError, "for 2 last column"): + with self.assertRaisesRegex( + ImportDataError, "for 2 last columns at row 2"): Model.import_data(['name', 'value'], [["Test", 1], []]) @with_transaction() @@ -56,7 +58,8 @@ class ImportDataTestCase(unittest.TestCase): pool = Pool() Boolean = pool.get('test.import_data.boolean') - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "at row 1 and column 1"): Boolean.import_data(['boolean'], [['foo']]) @with_transaction() @@ -87,7 +90,8 @@ class ImportDataTestCase(unittest.TestCase): for value in ['1.1', '-1.1', 'foo']: with self.subTest(value=value): - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "at row 1 and column 1"): Integer.import_data(['integer'], [[value]]) @with_transaction() @@ -108,7 +112,8 @@ class ImportDataTestCase(unittest.TestCase): pool = Pool() Float = pool.get('test.import_data.float') - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "'foo' .* at row 1 and column 1"): Float.import_data(['float'], [['foo']]) @with_transaction() @@ -129,7 +134,8 @@ class ImportDataTestCase(unittest.TestCase): pool = Pool() Numeric = pool.get('test.import_data.numeric') - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "'foo' .* at row 1 and column 1"): Numeric.import_data(['numeric'], [['foo']]) @with_transaction() @@ -220,7 +226,8 @@ class ImportDataTestCase(unittest.TestCase): pool = Pool() Date = pool.get('test.import_data.date') - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "'foo' .* at row 1 and column 1"): Date.import_data(['date'], [['foo']]) @with_transaction() @@ -252,7 +259,8 @@ class ImportDataTestCase(unittest.TestCase): pool = Pool() Datetime = pool.get('test.import_data.datetime') - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "'foo' .* at row 1 and column 1"): Datetime.import_data(['datetime'], [['foo']]) @with_transaction() @@ -276,7 +284,8 @@ class ImportDataTestCase(unittest.TestCase): pool = Pool() Timedelta = pool.get('test.import_data.timedelta') - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "'foo' .* at row 1 and column 1"): Timedelta.import_data(['timedelta'], [['foo']]) @with_transaction() @@ -334,7 +343,8 @@ class ImportDataTestCase(unittest.TestCase): for value in ['foo', 'Duplicate']: with self.subTest(value=value): - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "at row 1 and column 1"): Many2one.import_data(['many2one'], [[value]]) @with_transaction() @@ -381,7 +391,8 @@ class ImportDataTestCase(unittest.TestCase): for value in ['foo', 'Test 1,foo', 'Duplicate', 'Test 1,Duplicate']: with self.subTest(value=value): - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "at row 1 and column 1"): Many2many.import_data(['many2many'], [[value]]) @with_transaction() @@ -518,7 +529,8 @@ class ImportDataTestCase(unittest.TestCase): 'test.import_data.reference.selection,Duplicate', 'test.import_data.reference.selection,test.foo']: with self.subTest(value=value): - with self.assertRaises(ImportDataError): + with self.assertRaisesRegex( + ImportDataError, "at row 1 and column 1"): Reference.import_data(['reference'], [[value]]) @with_transaction()