diff --git a/addons/base_import/__manifest__.py b/addons/base_import/__manifest__.py index 3e67f593910..c27516aaac3 100644 --- a/addons/base_import/__manifest__.py +++ b/addons/base_import/__manifest__.py @@ -22,6 +22,7 @@ Re-implement Odoo's file import system: need or want an online import can avoid it being available to users. """, 'depends': ['web'], + 'version': '2.0', 'category': 'Hidden/Tools', 'installable': True, 'auto_install': True, @@ -34,6 +35,7 @@ Re-implement Odoo's file import system: ], 'web.assets_backend': [ 'base_import/static/src/scss/base_import.scss', + 'base_import/static/src/scss/icons.scss', 'base_import/static/lib/javascript-state-machine/state-machine.js', 'base_import/static/src/js/import_action.js', 'base_import/static/src/js/import_menu.js', diff --git a/addons/base_import/models/base_import.py b/addons/base_import/models/base_import.py index 733c3060a97..2b7ed221fe0 100644 --- a/addons/base_import/models/base_import.py +++ b/addons/base_import/models/base_import.py @@ -5,6 +5,7 @@ import base64 import binascii import codecs import collections +import difflib import unicodedata import chardet @@ -21,7 +22,6 @@ import requests from PIL import Image from odoo import api, fields, models -from odoo.exceptions import AccessError from odoo.tools.translate import _ from odoo.tools.mimetypes import guess_mimetype from odoo.tools import config, DEFAULT_SERVER_DATE_FORMAT, DEFAULT_SERVER_DATETIME_FORMAT, pycompat @@ -66,6 +66,29 @@ EXTENSIONS = { for mime, (ext, handler, req) in FILE_TYPE_DICT.items() } + +class ImportValidationError(Exception): + """ + This class is made to correctly format all the different error types that + can occur during the pre-validation of the import that is made before + calling the data loading itself. The Error data structure is meant to copy + the one of the errors raised during the data loading. It simplifies the + error management at client side as all errors can be treated the same way. + + This exception is typically raised when there is an error during data + parsing (image, int, dates, etc..) or if the user did not select at least + one field to map with a column. + """ + def __init__(self, message, **kwargs): + super().__init__(message) + self.type = kwargs.get('error_type', 'error') + self.message = message + self.record = False + self.not_matching_error = True + self.field_path = [kwargs['field']] if kwargs.get('field') else False + self.field_type = kwargs.get('field_type') + + class Base(models.AbstractModel): _inherit = 'base' @@ -113,12 +136,61 @@ class ResUsers(models.Model): return self._is_admin() class Import(models.TransientModel): + """ + This model is used to prepare the loading of data coming from a user file. + + Here is the process that is followed: + + #. The user selects a file to import. + #. File parsing and mapping suggestion (see "parse_preview" method) + + #. Extract the current model's importable fields tree (see :meth:`get_fields_tree`). + #. Read the file (see :meth:`_read_file`) and extract header names and file + length (used for batch import). + #. Extract headers types from the data preview (10 first line of the file) + (see :meth:`_extract_headers_types`). + #. Try to find for each header a field to map with (see :meth:`_get_mapping_suggestions`) + + - First check the previously saved mappings between the header name + and one of the model's fields. + - If no mapping found, try an exact match comparison using fields + technical names, labels and user language translated labels. + - If nothing found, try a fuzzy match using word distance between + header name and fields tachnical names, labels and user language + translated labels. Keep only the closest match. + + #. Prepare examples for each columns using the first non null value from each column. + #. Send the info back to the UI where the user can modify the suggested mapping. + #. Execute the import: There are two import mode with uses the same process. (see :meth:`execute_import`) + + #. Test import: Try to import but rollback the transaction. This allows + the check errors during the import process and allow the user to + choose import options for the different encountered errors. + #. Real import: Try to import the file using the configured mapping and + the eventual "error mapping options". If import encounters blocking + errors, the transaction is rollbacked and the user is allowed to + choose import options for the different errors. + + - Get file data and fields to import into (see :meth:`_convert_import_data`). + - Parse date, float and binary data (see :meth:`_parse_import_data`). + - Handle multiple mapping -> concatenate char/text/many2many columns + mapped on the same field (see :meth:`_handle_multi_mapping`). + - Handle fallback values for boolean and selection fields, in case + input data does not match any allowed values (see :meth:`_handle_fallback_values`). + - Load data (see ir.model "load" method). + - Rollback transaction if test mode or if encountered error. + - Save mapping if any import is successful to ease later mapping suggestions. + - Return import result to the UI (success or errors if any). + """ _name = 'base_import.import' _description = 'Base Import' # allow imports to survive for 12h in case user is slow _transient_max_hours = 12.0 + # we consider that if the difference is more than 0.2, then the two compared strings are "too different" to propose + # any match between them. (see '_get_mapping_suggestion' for more details) + FUZZY_MATCH_DISTANCE = 0.2 res_model = fields.Char('Model') file = fields.Binary('File', help="File to check and/or import, raw binary (not base64)", attachment=False) @@ -126,38 +198,37 @@ class Import(models.TransientModel): file_type = fields.Char('File Type') @api.model - def get_fields(self, model, depth=FIELDS_RECURSION_LIMIT): + def get_fields_tree(self, model, depth=FIELDS_RECURSION_LIMIT): """ Recursively get fields for the provided model (through fields_get) and filter them according to importability - The output format is a list of ``Field``, with ``Field`` - defined as: + The output format is a list of :class:`Field`: .. class:: Field - .. attribute:: id (str) + .. attribute:: id: str A non-unique identifier for the field, used to compute the span of the ``required`` attribute: if multiple ``required`` fields have the same id, only one of them is necessary. - .. attribute:: name (str) + .. attribute:: name: str The field's logical (Odoo) name within the scope of its parent. - .. attribute:: string (str) + .. attribute:: string: str The field's human-readable name (``@string``) - .. attribute:: required (bool) + .. attribute:: required: bool Whether the field is marked as required in the model. Clients must provide non-empty import values for all required fields or the import will error out. - .. attribute:: fields (list(Field)) + .. attribute:: fields: list[Field] The current field's subfields. The database and external identifiers for m2o and m2m fields; a @@ -167,6 +238,28 @@ class Import(models.TransientModel): Fields with no sub-fields will have an empty list of sub-fields. + .. attribute:: model_name: str + + Used in the Odoo Field Tooltip on the import view + and to get the model of the field of the related field(s). + Name of the current field's model. + + .. attribute:: comodel_name: str + + Used in the Odoo Field Tooltip on the import view + and to get the model of the field of the related field(s). + Name of the current field's comodel, i.e. if the field is a relation field. + + Structure example for 'crm.team' model for returned importable_fields:: + + [ + {'name': 'message_ids', 'string': 'Messages', 'model_name': 'crm.team', 'comodel_name': 'mail.message', 'fields': [ + {'name': 'moderation_status', 'string': 'Moderation Status', 'model_name': 'mail.message', 'fields': []}, + {'name': 'body', 'string': 'Contents', 'model_name': 'mail.message', 'fields' : []} + ]}, + {{'name': 'name', 'string': 'Sales Team', 'model_name': 'crm.team', 'fields' : []} + ] + :param str model: name of the model to get fields form :param int depth: depth of recursion into o2m fields """ @@ -207,6 +300,7 @@ class Import(models.TransientModel): 'required': bool(field.get('required')), 'fields': [], 'type': field['type'], + 'model_name': model } if field['type'] in ('many2many', 'many2one'): @@ -214,19 +308,42 @@ class Import(models.TransientModel): dict(field_value, name='id', string=_("External ID"), type='id'), dict(field_value, name='.id', string=_("Database ID"), type='id'), ] + field_value['comodel_name'] = field['relation'] elif field['type'] == 'one2many': - field_value['fields'] = self.get_fields(field['relation'], depth=depth-1) + field_value['fields'] = self.get_fields_tree(field['relation'], depth=depth-1) if self.user_has_groups('base.group_no_one'): field_value['fields'].append({'id': '.id', 'name': '.id', 'string': _("Database ID"), 'required': False, 'fields': [], 'type': 'id'}) + field_value['comodel_name'] = field['relation'] importable_fields.append(field_value) # TODO: cache on model? return importable_fields + def _filter_fields_by_types(self, model_fields_tree, header_types): + """ Remove from model_fields_tree param all the fields and subfields + that do not match the types in header_types + + :param: list[dict] model_fields_tree: Contains recursively all the importable fields of the target model. + Generated in "get_fields_tree" method. + :param: list header_types: Contains the extracted fields types of the current header. + Generated in :meth:`_extract_header_types`. + """ + most_likely_fields_tree = [] + for field in model_fields_tree: + subfields = field.get('fields') + if subfields: + filtered_field = dict(field) # Avoid modifying fields. + filtered_field['fields'] = self._filter_fields_by_types(subfields, header_types) + most_likely_fields_tree.append(filtered_field) + elif field.get('type') in header_types: + most_likely_fields_tree.append(field) + return most_likely_fields_tree + def _read_file(self, options): """ Dispatch to specific method to read file content, according to its mimetype or file type - :param options : dict of reading options (quoting, separator, ...) + + :param dict options: reading options (quoting, separator, ...) """ self.ensure_one() # guess mimetype from file content @@ -262,7 +379,6 @@ class Import(models.TransientModel): raise ValueError(_("Unsupported file format \"{}\", import only supports CSV, ODS, XLS and XLSX").format(self.file_type)) def _read_xls(self, options): - """ Read file content, using xlrd lib """ book = xlrd.open_workbook(file_contents=self.file or b'') sheets = options['sheets'] = book.sheet_names() sheet = options['sheet'] = options.get('sheet') or sheets[0] @@ -270,6 +386,7 @@ class Import(models.TransientModel): def _read_xls_book(self, book, sheet_name): sheet = book.sheet_by_name(sheet_name) + rows = [] # emulate Sheet.get_rows for pre-0.9.4 for rowx, row in enumerate(map(sheet.row, range(sheet.nrows)), 1): values = [] @@ -303,30 +420,36 @@ class Import(models.TransientModel): else: values.append(cell.value) if any(x for x in values if x.strip()): - yield values + rows.append(values) + + # return the file length as first value + return sheet.nrows, rows # use the same method for xlsx and xls files _read_xlsx = _read_xls def _read_ods(self, options): - """ Read file content using ODSReader custom lib """ doc = odf_ods_reader.ODSReader(file=io.BytesIO(self.file or b'')) sheets = options['sheets'] = list(doc.SHEETS.keys()) sheet = options['sheet'] = options.get('sheet') or sheets[0] - return ( + content = [ row for row in doc.getSheet(sheet) if any(x for x in row if x.strip()) - ) + ] + + # return the file length as first value + return len(content), content def _read_csv(self, options): - """ Returns a CSV-parsed iterator of all non-empty lines in the file - :throws csv.Error: if an error is detected during CSV parsing + """ Returns file length and a CSV-parsed list of all non-empty lines in the file. + + :raises csv.Error: if an error is detected during CSV parsing """ csv_data = self.file or b'' if not csv_data: - return iter([]) + return () encoding = options.get('encoding') if not encoding: @@ -367,16 +490,36 @@ class Import(models.TransientModel): quotechar=options['quoting'], delimiter=separator) - return ( + content = [ row for row in csv_iterator if any(x for x in row if x.strip()) - ) + ] + + # return the file length as first value + return len(content), content @api.model - def _try_match_column(self, preview_values, options): - """ Returns the potential field types, based on the preview values, using heuristics - :param preview_values : list of value for the column to determine - :param options : parsing options + def _extract_header_types(self, preview_values, options): + """ Returns the potential field types, based on the preview values, using heuristics. + + This methods is only used for suggested mapping at 2 levels: + + 1. for fuzzy mapping at file load -> Execute the fuzzy mapping only + on "most likely field types" + 2. For "Suggested fields" section in the fields mapping dropdown list at UI side. + + The following heuristic is used: If all preview values + + - Start with ``__export__``: return id + relational field types + - Can be cast into integer: return id + relational field types, integer, float and monetary + - Can be cast into Boolean: return boolean + - Can be cast into float: return float, monetary + - Can be cast into date/datetime: return date / datetime + - Cannot be cast into any of the previous types: return only text based fields + + :param preview_values: list of value for the column to determine + see :meth:`parse_preview` for more details. + :param options: parsing options """ values = set(preview_values) # If all values are empty in preview than can be any field @@ -390,7 +533,7 @@ class Import(models.TransientModel): # If all values can be cast to int type is either id, float or monetary # Exception: if we only have 1 and 0, it can also be a boolean if all(v.isdigit() for v in values if v): - field_type = ['id', 'integer', 'char', 'float', 'monetary', 'many2one', 'many2many', 'one2many'] + field_type = ['integer', 'float', 'monetary'] if {'0', '1', ''}.issuperset(values): field_type.append('boolean') return field_type @@ -400,7 +543,6 @@ class Import(models.TransientModel): return ['boolean'] # If all values can be cast to float, type is either float or monetary - results = [] try: thousand_separator = decimal_separator = False for val in preview_values: @@ -433,16 +575,16 @@ class Import(models.TransientModel): if thousand_separator and not options.get('float_decimal_separator'): options['float_thousand_separator'] = thousand_separator options['float_decimal_separator'] = decimal_separator - results = ['float', 'monetary'] + return ['float', 'monetary'] # Allow float to be mapped on a text field. except ValueError: pass - results += self._try_match_date_time(preview_values, options) + results = self._try_match_date_time(preview_values, options) if results: return results - return ['id', 'text', 'boolean', 'char', 'datetime', 'selection', 'many2one', 'one2many', 'many2many', 'html'] - + # If not boolean, date/datetime, float or integer, only suggest text based fields. + return ['text', 'char', 'binary', 'selection', 'html'] def _try_match_date_time(self, preview_values, options): # Or a date/datetime if it matches the pattern @@ -476,131 +618,329 @@ class Import(models.TransientModel): return [] @api.model - def _find_type_from_preview(self, options, preview): - type_fields = [] - if preview: - for column in range(0, len(preview[0])): - preview_values = [value[column].strip() for value in preview] - type_field = self._try_match_column(preview_values, options) - type_fields.append(type_field) - return type_fields - - def _match_header(self, header, fields, options): - """ Attempts to match a given header to a field of the - imported model. - - :param str header: header name from the CSV file - :param fields: - :param dict options: - :returns: an empty list if the header couldn't be matched, or - all the fields to traverse - :rtype: list(Field) + def _extract_headers_types(self, headers, preview, options): """ - string_match = None - IrTranslation = self.env['ir.translation'] - for field in fields: - # FIXME: should match all translations & original - # TODO: use string distance (levenshtein? hamming?) - if header.lower() == field['name'].lower(): - return [field] - if header.lower() == field['string'].lower(): - # matching string are not reliable way because - # strings have no unique constraint - string_match = field - translated_header = IrTranslation._get_source('ir.model.fields,field_description', 'model', self.env.lang, header).lower() - if translated_header == field['string'].lower(): - string_match = field - if string_match: - # this behavior is only applied if there is no matching field['name'] - return [string_match] + For each column, this method will extract the potential data types based on the preview values + + :param list headers: list of headers names. Used as part of key for + returned headers_types to ease understanding of its usage + :param list preview: list of the first file records (see "parse_preview" for more detail) e.g.:: + + [ ["lead_name1", "1", "partner_id1"], ["lead_name2", "2", "partner_id2"], ... ] + + :param options: parsing options + :returns: dict headers_types: + + contains all the extracted header types for each header e.g.:: + + { + (header_index, header_name): ["char", "text", ...], + ... + } + """ + headers_types = {} + for column_index, header_name in enumerate(headers): + preview_values = [record[column_index].strip() for record in preview] + type_field = self._extract_header_types(preview_values, options) + headers_types[(column_index, header_name)] = type_field + return headers_types + + def _get_mapping_suggestion(self, header, fields_tree, header_types, mapping_fields): + """ Attempts to match a given header to a field of the imported model. + + We can distinguish 2 types of header format: + + - simple header string that aim to directly match a field of the target model + e.g.: "lead_id" or "Opportunities" or "description". + - composed '/' joined header string that aim to match a field of a + relation field of the target model (= subfield) e.g.: + 'lead_id/description' aim to match the field ``description`` of the field lead_id. + + When returning result, to ease further treatments, the result is + returned as a list, where each element of the list is a field or + a sub-field of the preceding field. + + - ``["lead_id"]`` for simple case = simple matching + - ``["lead_id", "description"]`` for composed case = hierarchy matching + + Mapping suggestion is found using the following heuristic: + + - first we check if there was a saved mapping by the user + - then try to make an exact match on the field technical name / + english label / translated label + - finally, try the "fuzzy match": word distance between the header + title and the field technical name / english label / translated + label, using the lowest result. The field used for the fuzzy match + are based on the field types we extracted from the header data + (see :meth:`_extract_header_types`). + + For subfields, use the same logic. + + Word distance is a score between 0 and 1 to express the distance + between two char strings where ``0`` denotes an exact match and + ``1`` indicates completely different strings + + In order to keep only one column matched per field, we return the + distance. That distance will be used during the deduplicate process + (see :meth:`_deduplicate_mapping_suggestions`) and only the + mapping with the smallest distance will be kept in case of multiple + mapping on the same field. Note that we don't need to return the + distance in case of hierachy mapping as we consider that as an + advanced behaviour. The deduplicate process will ignore hierarchy + mapping. The user will have to manually select on which field he + wants to map what in case of mapping duplicates for sub-fields. + + :param str header: header name from the file + :param list fields_tree: list of all the field of the target model + Coming from :meth:`get_fields_tree` + e.g: ``[ { 'name': 'fieldName', 'string': 'fieldLabel', fields: [ { 'name': 'subfieldName', ...} ]} , ... ]`` + :param list header_types: Extracted field types for each column in the parsed file, based on its data content. + Coming from :meth:`_extract_header_types` + e.g.: ``['int', 'float', 'char', 'many2one', ...]`` + :param dict mapping_fields: contains the previously saved mapping between header and field for the current model. + E.g.: ``{ header_name: field_name }`` + :returns: if the header couldn't be matched: an empty dict + else: a dict with the field path and the distance between header and the matched field. + :rtype: ``dict(field_path + Word distance)`` + + In case of simple matching: ``{'field_path': [field_name], distance: word_distance}`` + e.g.: ``{'field_path': ['lead_id'], distance: 0.23254}`` + In case of hierarchy matching: ``{'field_path': [parent_field_name, child_field_name, subchild_field_name]}`` + e.g.: ``{'field_path': ['lead_id', 'description']}`` + """ + if not fields_tree: + return {} + + # First, check in saved mapped fields + mapping_field_name = mapping_fields.get(header.lower()) + if mapping_field_name and mapping_field_name: + return { + 'field_path': [name for name in mapping_field_name.split('/')], + 'distance': -1 # Trick to force to keep that match during mapping deduplication. + } if '/' not in header: - return [] + # Then, try exact match + IrTranslation = self.env['ir.translation'] + translated_header = IrTranslation._get_source('ir.model.fields,field_description', 'model', self.env.lang, header).lower() + for field in fields_tree: + # exact match found based on the field technical name + if header.casefold() == field['name'].casefold(): + break + + field_string = field.get('string', '').casefold() + # match found using either user translation, either model defined field label + if translated_header == field_string or header.casefold() == field_string: + break + else: + field = None + + if field: # found an exact match, no need to go further + return { + 'field_path': [field['name']], + 'distance': 0 + } + + # If no match found, try fuzzy match on fields filtered based on extracted header types + # Filter out fields with types that does not match corresponding header types. + filtered_fields = self._filter_fields_by_types(fields_tree, header_types) + if not filtered_fields: + return {} + + min_dist = 1 + min_dist_field = False + for field in filtered_fields: + field_string = field.get('string', '').casefold() + + # use string distance for fuzzy match only on most likely field types + name_field_dist = self._get_distance(header.casefold(), field['name'].casefold()) + string_field_dist = self._get_distance(header.casefold(), field_string) + translated_string_field_dist = self._get_distance(translated_header.casefold(), field_string) + + # Keep only the closest mapping suggestion. Note that in case of multiple mapping on the same field, + # a mapping suggestion could be canceled by another one that has a smaller distance on the same field. + # See 'deduplicate_mapping_suggestions' method for more info. + current_field_dist = min([name_field_dist, string_field_dist, translated_string_field_dist]) + if current_field_dist < min_dist: + min_dist_field = field['name'] + min_dist = current_field_dist + + if min_dist < self.FUZZY_MATCH_DISTANCE: + return { + 'field_path': [min_dist_field], + 'distance': min_dist + } + + return {} # relational field path - traversal = [] - subfields = fields + field_path = [] + subfields_tree = fields_tree # Iteratively dive into fields tree - for section in header.split('/'): - # Strip section in case spaces are added around '/' for + for sub_header in header.split('/'): + # Strip sub_header in case spaces are added around '/' for # readability of paths - match = self._match_header(section.strip(), subfields, options) + # Skip Saved mapping (mapping_field = {}) + match = self._get_mapping_suggestion(sub_header.strip(), subfields_tree, header_types, {}) # Any match failure, exit if not match: - return [] - # prep subfields for next iteration within match[0] - field = match[0] - subfields = field['fields'] - traversal.append(field) - return traversal + return {} + # prep subfields for next iteration within match['name'][0] + field_name = match['field_path'][0] + subfields_tree = next(item['fields'] for item in subfields_tree if item['name'] == field_name) + field_path.append(field_name) + # No need to return distance for hierarchy mapping + return {'field_path': field_path} - def _match_headers(self, rows, fields, options): + def _get_distance(self, a, b): + """ This method return an index that reflects the distance between the + two given string a and b. + + This index is a score between 0 and 1 where ``0`` indicates an exact + match and ``1`` indicates completely different strings. + """ + return 1 - difflib.SequenceMatcher(None, a, b).ratio() + + def _get_mapping_suggestions(self, headers, header_types, fields_tree): """ Attempts to match the imported model's fields to the titles of the parsed CSV file, if the file is supposed to have headers. - Will consume the first line of the ``rows`` iterator. + Returns a dict mapping cell indices to key paths in the ``fields`` tree. - Returns the list of headers and a dict mapping cell indices - to key paths in the ``fields`` tree. If headers were not - requested, both collections are empty. + :param list headers: titles of the parsed file + :param dict header_types: - :param Iterator rows: - :param dict fields: - :param dict options: - :rtype: (list(str), dict(int: list(str))) + extracted types for each column in the parsed file e.g.:: + + { + (header_index, header_name): ['int', 'float', 'char', 'many2one',...], + ... + } + + :param list fields_tree: + + list of the target model's fields e.g.:: + + [ + { + 'name': 'fieldName', + 'string': 'fieldLabel', + 'fields': [{ 'name': 'subfieldName', ...}] + }, + ... + ] + + :rtype: dict[(int, str), {'field_path': list[str], 'distance': int}] + :returns: mapping_suggestions e.g.: + + .. code-block:: python + + { + (header_index, header_name): { + 'field_path': ['child_id','name'], + 'distance': 0 + }, + ... + } """ - if not options.get('headers'): - return [], {} - - headers = next(rows, None) - if not headers: - return [], {} - - matches = {} + mapping_suggestions = {} mapping_records = self.env['base_import.mapping'].search_read([('res_model', '=', self.res_model)], ['column_name', 'field_name']) mapping_fields = {rec['column_name']: rec['field_name'] for rec in mapping_records} for index, header in enumerate(headers): - match_field = [] - mapping_field_name = mapping_fields.get(header.lower()) - if mapping_field_name: - match_field = mapping_field_name.split('/') - if not match_field: - match_field = [field['name'] for field in self._match_header(header, fields, options)] - matches[index] = match_field or None - return headers, matches + match_field = self._get_mapping_suggestion(header, fields_tree, header_types[(index, header)], mapping_fields) + mapping_suggestions[(index, header)] = match_field or None + + self._deduplicate_mapping_suggestions(mapping_suggestions) + return mapping_suggestions + + def _deduplicate_mapping_suggestions(self, mapping_suggestions): + """ This method is meant to avoid multiple columns to be matched on the same field. + + Taking ``mapping_suggestions`` as input, it will check if multiple + columns are mapped to the same field and will only keep the mapping + that has the smallest distance. The other columns that were matched + to the same field are removed from the mapping suggestions. + + Hierarchy mapping is considered as advanced and is skipped during this + deduplication process. We consider that multiple mapping on hierarchy + mapping will not occur often and due to the fact that this won't lead + to any particular issues when a non 'char/text' field is selected more + than once in the UI, we keep only the last selected mapping. The + objective is to lighten the mapping suggestion process as much as we can. + + :param dict mapping_suggestions: ``{ (column_index, header_name) : { 'field_path': [header_name], 'distance': word_distance }}`` + """ + min_dist_per_field = {} + headers_to_keep = [] + for header, suggestion in mapping_suggestions.items(): + if suggestion is None or len(suggestion['field_path']) > 1: + headers_to_keep.append(header) + continue + + field_name = suggestion['field_path'][0] + field_distance = suggestion['distance'] + + best_distance, _best_header = min_dist_per_field.get(field_name, (1, None)) + if field_distance < best_distance: + min_dist_per_field[field_name] = (field_distance, header) + + headers_to_keep = headers_to_keep + [value[1] for value in min_dist_per_field.values()] + for header in mapping_suggestions.keys() - headers_to_keep: + del mapping_suggestions[header] def parse_preview(self, options, count=10): """ Generates a preview of the uploaded files, and performs fields-matching between the import's file data and the model's columns. - If the headers are not requested (not options.headers), - ``matches`` and ``headers`` are both ``False``. + If the headers are not requested (not options.has_headers), + returned ``matches`` and ``headers`` are both ``False``. :param int count: number of preview lines to generate :param options: format-specific options. CSV: {quoting, separator, headers} :type options: {str, str, str, bool} - :returns: {fields, matches, headers, preview} | {error, preview} + :returns: ``{fields, matches, headers, preview} | {error, preview}`` :rtype: {dict(str: dict(...)), dict(int, list(str)), list(str), list(list(str))} | {str, str} """ self.ensure_one() - fields = self.get_fields(self.res_model) + fields_tree = self.get_fields_tree(self.res_model) try: - rows = self._read_file(options) - headers, matches = self._match_headers(rows, fields, options) - # Match should have consumed the first row (iif headers), get - # the ``count`` next rows for preview - preview = list(itertools.islice(rows, count)) - assert preview, "file seems to have no content" - header_types = self._find_type_from_preview(options, preview) - if options.get('keep_matches') and len(options.get('fields', [])): - matches = {} - for index, match in enumerate(options.get('fields')): + file_length, rows = self._read_file(options) + if file_length <= 0: + raise ImportValidationError(_("Import file has no content or is corrupt")) + + preview = rows[:count] + + # Get file headers + if options.get('has_headers') and preview: + # We need the header types before matching columns to fields + headers = preview.pop(0) + header_types = self._extract_headers_types(headers, preview, options) + else: + header_types, headers = {}, [] + + # Get matches: the ones already selected by the user or propose a new matching. + matches = {} + # If user checked to the advanced mode, we re-parse the file but we keep the mapping "as is". + # No need to make another mapping proposal + if options.get('keep_matches') and options.get('fields'): + for index, match in enumerate(options.get('fields', [])): if match: matches[index] = match.split('/') + elif options.get('has_headers'): + matches = self._get_mapping_suggestions(headers, header_types, fields_tree) + # remove header_name for matches keys as tuples are no supported in json. + # and remove distance from suggestion (keep only the field path) as not used at client side. + matches = { + header_key[0]: suggestion['field_path'] + for header_key, suggestion in matches.items() + if suggestion + } + # compute if we should activate advanced mode or not: + # if was already activated of if file contains "relational fields". if options.get('keep_matches'): advanced_mode = options.get('advanced') else: @@ -610,6 +950,18 @@ class Import(models.TransientModel): has_relational_match = any(len(match) > 1 for field, match in matches.items() if match) advanced_mode = has_relational_header or has_relational_match + # Take the first non null value for each column to display example to user + column_example = [] + for column_index, _unused in enumerate(preview[0]): + for record in preview: + if record[column_index]: + column_example.append("%s%s" % (record[column_index][:50], "..." if len(record[column_index]) > 50 else "")) + break + # add a blank value if no example have been found at all for the current column + else: + column_example.append("") + + # Batch management batch = False batch_cutoff = options.get('limit') if batch_cutoff: @@ -622,15 +974,16 @@ class Import(models.TransientModel): )) return { - 'fields': fields, + 'fields': fields_tree, 'matches': matches or False, 'headers': headers or False, - 'headers_type': header_types or False, - 'preview': preview, + 'header_types': list(header_types.values()) or False, + 'preview': column_example, 'options': options, 'advanced_mode': advanced_mode, 'debug': self.user_has_groups('base.group_no_one'), 'batch': batch, + 'file_length': file_length } except Exception as error: # Due to lazy generators, UnicodeDecodeError (for @@ -664,7 +1017,7 @@ class Import(models.TransientModel): # Get indices for non-empty fields indices = [index for index, field in enumerate(fields) if field] if not indices: - raise ValueError(_("You must configure at least one field to import")) + raise ImportValidationError(_("You must configure at least one field to import")) # If only one index, itemgetter will return an atom rather # than a 1-tuple if len(indices) == 1: @@ -674,9 +1027,10 @@ class Import(models.TransientModel): # Get only list of actually imported fields import_fields = [f for f in fields if f] - rows_to_import = self._read_file(options) - if options.get('headers'): - rows_to_import = itertools.islice(rows_to_import, 1, None) + _file_length, rows_to_import = self._read_file(options) + + if options.get('has_headers'): + rows_to_import = rows_to_import[1:] data = [ list(row) for row in map(mapper, rows_to_import) # don't try inserting completely empty rows (e.g. from @@ -738,7 +1092,7 @@ class Import(models.TransientModel): old_value = line[index] line[index] = self._remove_currency_symbol(line[index]) if line[index] is False: - raise ValueError(_("Column %s contains incorrect values (value: %s)", name, old_value)) + raise ImportValidationError(_("Column %s contains incorrect values (value: %s)", name, old_value), field=name) def _infer_separators(self, value, options): """ Try to infer the shape of the separators: if there are two @@ -770,8 +1124,8 @@ class Import(models.TransientModel): return thousand_separator, decimal_separator def _parse_import_data(self, data, import_fields, options): - """ Lauch first call to _parse_import_data_recursive with an - empty prefix. _parse_import_data_recursive will be run + """ Lauch first call to :meth:`_parse_import_data_recursive` with an + empty prefix. :meth:`_parse_import_data_recursive` will be run recursively for each relational field. """ return self._parse_import_data_recursive(self.res_model, '', data, import_fields, options) @@ -803,14 +1157,20 @@ class Import(models.TransientModel): for num, line in enumerate(data): if re.match(config.get("import_image_regex", DEFAULT_IMAGE_REGEX), line[index]): if not self.env.user._can_import_remote_urls(): - raise AccessError(_("You can not import images via URL, check with your administrator or support for the reason.")) + raise ImportValidationError( + _("You can not import images via URL, check with your administrator or support for the reason."), + field=name, field_type=field['type'] + ) line[index] = self._import_image_by_url(line[index], session, name, num) else: try: base64.b64decode(line[index], validate=True) except binascii.Error: - raise ValueError(_("Found invalid image data, images should be imported as either URLs or base64-encoded data.")) + raise ImportValidationError( + _("Found invalid image data, images should be imported as either URLs or base64-encoded data."), + field=name, field_type=field['type'] + ) return data @@ -836,9 +1196,15 @@ class Import(models.TransientModel): # or datetime line[index] = fmt(dt.strptime(v, d_fmt)) except ValueError as e: - raise ValueError(_("Column %s contains incorrect values. Error in line %d: %s") % (name, num + 1, e)) + raise ImportValidationError( + _("Column %s contains incorrect values. Error in line %d: %s") % (name, num + 1, e), + field=name, field_type=field_type + ) except Exception as e: - raise ValueError(_("Error Parsing Date [%s:L%d]: %s") % (name, num + 1, e)) + raise ImportValidationError( + _("Error Parsing Date [%s:L%d]: %s") % (name, num + 1, e), + field=name, field_type=field_type + ) def _import_image_by_url(self, url, session, field, line_number): """ Imports an image by URL @@ -857,32 +1223,40 @@ class Import(models.TransientModel): response.raise_for_status() if response.headers.get('Content-Length') and int(response.headers['Content-Length']) > maxsize: - raise ValueError(_("File size exceeds configured maximum (%s bytes)", maxsize)) + raise ImportValidationError( + _("File size exceeds configured maximum (%s bytes)", maxsize), + field=field + ) content = bytearray() for chunk in response.iter_content(DEFAULT_IMAGE_CHUNK_SIZE): content += chunk if len(content) > maxsize: - raise ValueError(_("File size exceeds configured maximum (%s bytes)", maxsize)) + raise ImportValidationError( + _("File size exceeds configured maximum (%s bytes)", maxsize), + field=field + ) image = Image.open(io.BytesIO(content)) w, h = image.size if w * h > 42e6: # Nokia Lumia 1020 photo resolution - raise ValueError( - u"Image size excessive, imported images must be smaller " - u"than 42 million pixel") + raise ImportValidationError( + _("Image size excessive, imported images must be smaller than 42 million pixel"), + field=field + ) return base64.b64encode(content) except Exception as e: _logger.exception(e) - raise ValueError(_("Could not retrieve URL: %(url)s [%(field_name)s: L%(line_number)d]: %(error)s") % { - 'url': url, - 'field_name': field, - 'line_number': line_number + 1, - 'error': e - }) + raise ImportValidationError( + _( + "Could not retrieve URL: %(url)s [%(field_name)s: L%(line_number)d]: %(error)s", + url=url, field_name=field, line_number=line_number + 1, error=e + ), + field=field + ) - def do(self, fields, columns, options, dryrun=False): + def execute_import(self, fields, columns, options, dryrun=False): """ Actual execution of the import :param fields: import mapping: maps each column to a field, @@ -897,35 +1271,42 @@ class Import(models.TransientModel): the risk of clobbering the database. :returns: A list of errors. If the list is empty the import executed fully and correctly. If the list is - non-empty it contains dicts with 3 keys ``type`` the - type of error (``error|warning``); ``message`` the - error message associated with the error (a string) - and ``record`` the data which failed to import (or - ``false`` if that data isn't available or provided) + non-empty it contains dicts with 3 keys: + + ``type`` + the type of error (``error|warning``) + ``message`` + the error message associated with the error (a string) + ``record`` + the data which failed to import (or ``false`` if that data + isn't available or provided) :rtype: dict(ids: list(int), messages: list({type, message, record})) """ self.ensure_one() self._cr.execute('SAVEPOINT import') try: - data, import_fields = self._convert_import_data(fields, options) + input_file_data, import_fields = self._convert_import_data(fields, options) # Parse date and float field - data = self._parse_import_data(data, import_fields, options) - except ValueError as error: - return { - 'messages': [{ - 'type': 'error', - 'message': str(error), - 'record': False, - }] - } + input_file_data = self._parse_import_data(input_file_data, import_fields, options) + except ImportValidationError as error: + return {'messages': [error.__dict__]} - _logger.info('importing %d rows...', len(data)) + _logger.info('importing %d rows...', len(input_file_data)) + + import_fields, merged_data = self._handle_multi_mapping(import_fields, input_file_data) + + if options.get('fallback_values'): + merged_data = self._handle_fallback_values(import_fields, merged_data, options['fallback_values']) name_create_enabled_fields = options.pop('name_create_enabled_fields', {}) import_limit = options.pop('limit', None) - model = self.env[self.res_model].with_context(import_file=True, name_create_enabled_fields=name_create_enabled_fields, _import_limit=import_limit) - import_result = model.load(import_fields, data) + model = self.env[self.res_model].with_context( + import_file=True, + name_create_enabled_fields=name_create_enabled_fields, + import_skip_fields=options.get('import_skip_fields'), + _import_limit=import_limit) + import_result = model.load(import_fields, merged_data) _logger.info('done') # If transaction aborted, RELEASE SAVEPOINT is going to raise @@ -947,7 +1328,7 @@ class Import(models.TransientModel): pass # Insert/Update mapping columns when import complete successfully - if import_result['ids'] and options.get('headers'): + if import_result['ids'] and options.get('has_headers'): BaseImportMapping = self.env['base_import.mapping'] for index, column_name in enumerate(columns): if column_name: @@ -969,9 +1350,9 @@ class Import(models.TransientModel): # pad front as data doesn't contain anythig for skipped lines r = import_result['name'] = [''] * skipped # only add names for the window being imported - r.extend(x[index_of_name] for x in data[:import_limit]) + r.extend(x[index_of_name] for x in input_file_data[:import_limit]) # pad back (though that's probably not useful) - r.extend([''] * (len(data) - (import_limit or 0))) + r.extend([''] * (len(input_file_data) - (import_limit or 0))) else: import_result['name'] = [] @@ -982,6 +1363,145 @@ class Import(models.TransientModel): return import_result + def _handle_multi_mapping(self, import_fields, input_file_data): + """ This method handles multiple mapping on the same field. + + It will return the list of the mapped fields and the concatenated data for each field: + + - If two column are mapped on the same text or char field, they will end up + in only one column, concatenated via space (char) or new line (text). + - The same logic is used for many2many fields. Multiple values can be + imported if they are separated by ``,``. + + Input/output Example: + + input data + .. code-block:: python + + [ + ["Value part 1", "1", "res.partner_id1", "Value part 2"], + ["I am", "1", "res.partner_id1", "Batman"], + ] + + import_fields + ``[desc, some_number, partner, desc]`` + + output merged_data + .. code-block:: python + + [ + ["Value part 1 Value part 2", "1", "res.partner_id1"], + ["I am Batman", "1", "res.partner_id1"], + ] + fields + ``[desc, some_number, partner]`` + """ + # Get fields and their occurrences indexes + # Among the fields that have been mapped, we get their corresponding mapped column indexes + # as multiple fields could have been mapped to multiple columns. + mapped_field_indexes = {} + for idx, field in enumerate(field for field in import_fields if field): + mapped_field_indexes.setdefault(field, list()).append(idx) + import_fields = list(mapped_field_indexes.keys()) + + # recreate data and merge duplicates (applies only on text or char fields) + # Also handles multi-mapping on "field of relation fields". + merged_data = [] + for record in input_file_data: + new_record = [] + for fields, indexes in mapped_field_indexes.items(): + split_fields = fields.split('/') + target_field = split_fields[-1] + + # get target_field type (on target model) + target_model = self.res_model + for field in split_fields: + if field != target_field: # if not on the last hierarchy level, retarget the model + target_model = self.env[target_model][field]._name + field_type = self.env[target_model].fields_get().get(target_field, {}).get('type', '') + + # merge data if necessary + if field_type == 'char': + new_record.append(' '.join(record[idx] for idx in indexes if record[idx])) + elif field_type == 'text': + new_record.append('\n'.join(record[idx] for idx in indexes if record[idx])) + elif field_type == 'many2many': + new_record.append(','.join(record[idx] for idx in indexes if record[idx])) + else: + new_record.append(record[indexes[0]]) + + merged_data.append(new_record) + + return import_fields, merged_data + + def _handle_fallback_values(self, import_field, input_file_data, fallback_values): + """ + If there are fallback values, this method will replace the input file + data value if it does not match the possible values for the given field. + This is only valid for boolean and selection fields. + + .. note:: + + We can consider that we need to retrieve the selection values for + all the fields in fallback_values, as if they are present, it's because + there was already a conflict during first import run and user had to + select a fallback value for the field. + + :param: list import_field: ordered list of field that have been matched to import data + :param: list input_file_data: ordered list of values (list) that need to be imported in the given import_fields + :param: dict fallback_values: + + contains all the fields that have been tagged by the user to use a + specific fallback value in case the value to import does not match + values accepted by the field (selection or boolean) e.g.:: + + { + 'fieldName': { + 'fallback_value': fallback_value, + 'field_model': field_model, + 'field_type': field_type + }, + 'state': { + 'fallback_value': 'draft', + 'field_model': field_model, + 'field_type': 'selection' + }, + 'active': { + 'fallback_value': 'true', + 'field_model': field_model, + 'field_type': 'boolean' + } + } + """ + # add possible selection values into our fallback dictionary for fields of type "selection" + for field_string in fallback_values: + if fallback_values[field_string]['field_type'] != "selection": + continue + field_path = field_string.split('/') + target_field = field_path[-1] + target_model = self.env[fallback_values[field_string]['field_model']] + + selection_values = [value.lower() for (key, value) in target_model.fields_get([target_field])[target_field]['selection']] + fallback_values[field_string]['selection_values'] = selection_values + + # check fallback values + for record_index, records in enumerate(input_file_data): + for column_index, value in enumerate(records): + field = import_field[column_index] + + if field in fallback_values: + fallback_value = fallback_values[field]['fallback_value'] + # Boolean + if fallback_values[field]['field_type'] == "boolean": + value = value if value.lower() in ('0', '1', 'true', 'false') else fallback_value + # Selection + elif value.lower() not in fallback_values[field]["selection_values"]: + value = fallback_value if fallback_value != 'skip' else None # don't set any value if we skip + + input_file_data[record_index][column_index] = value + + return input_file_data + _SEPARATORS = [' ', '/', '-', ''] _PATTERN_BASELINE = [ ('%m', '%d', '%Y'), diff --git a/addons/base_import/static/src/img/ui/dropdown_arrow_small.png b/addons/base_import/static/src/img/ui/dropdown_arrow_small.png new file mode 100644 index 00000000000..70b5d50a577 Binary files /dev/null and b/addons/base_import/static/src/img/ui/dropdown_arrow_small.png differ diff --git a/addons/base_import/static/src/img/ui/studio_icons.png b/addons/base_import/static/src/img/ui/studio_icons.png new file mode 100644 index 00000000000..28d294e70ee Binary files /dev/null and b/addons/base_import/static/src/img/ui/studio_icons.png differ diff --git a/addons/base_import/static/src/js/import_action.js b/addons/base_import/static/src/js/import_action.js index 0b608953978..b80c28fc274 100644 --- a/addons/base_import/static/src/js/import_action.js +++ b/addons/base_import/static/src/js/import_action.js @@ -94,19 +94,15 @@ var DataImport = AbstractAction.extend({ {name: 'float_decimal_separator', label: _lt("Decimal Separator:"), value: '.'} ], events: { - // 'change .oe_import_grid input': 'import_dryrun', 'change .oe_import_file': 'loaded_file', 'change input.oe_import_has_header, .js_import_options input': 'settings_changed', 'change input.oe_import_advanced_mode': function (e) { this.do_not_change_match = true; this['settings_changed'](); }, - 'click a.oe_import_toggle': function (e) { - e.preventDefault(); - this.$('.oe_import_options').toggle(); - }, 'click .oe_import_report a.oe_import_report_count': function (e) { e.preventDefault(); + $(e.target).parent().find('i.arrow').toggleClass('up down'); $(e.target).parent().parent().toggleClass('oe_import_report_showmore'); }, 'click .oe_import_report_see_possible_value': function (e) { @@ -150,6 +146,7 @@ var DataImport = AbstractAction.extend({ this._title = _t('Import a File'); // Displayed in the breadcrumbs this.do_not_change_match = false; this.sheets = []; + this.selectionFields = {}; // Used to compute fallback values in backend. }, /** * @override @@ -214,6 +211,7 @@ var DataImport = AbstractAction.extend({ width: '50%', data: _.map(('utf-8 utf-16 windows-1252 latin1 latin2 big5 gb18030 shift_jis windows-1251 koir8_r').split(/\s+/), _make_option), query: dataFilteredQuery, + minimumResultsForSearch: -1, initSelection: function ($e, c) { return c(_make_option($e.val())); } @@ -230,6 +228,7 @@ var DataImport = AbstractAction.extend({ width: '50%', data: data, query: dataFilteredQuery, + minimumResultsForSearch: -1, // this is not provided to initSelection so can't use this.data initSelection: function ($e, c) { c(_from_data(data, $e.val()) || _make_option($e.val())) @@ -246,6 +245,7 @@ var DataImport = AbstractAction.extend({ width: '50%', data: data_digits, query: dataFilteredQuery, + minimumResultsForSearch: -1, initSelection: function ($e, c) { c(_from_data(data_digits, $e.val()) || _make_option($e.val())) } @@ -254,6 +254,7 @@ var DataImport = AbstractAction.extend({ width: '50%', data: data_decimal, query: dataFilteredQuery, + minimumResultsForSearch: -1, initSelection: function ($e, c) { c(_from_data(data_decimal, $e.val()) || _make_option($e.val())) } @@ -284,6 +285,7 @@ var DataImport = AbstractAction.extend({ width: '50%', data: data, query: dataFilteredQuery, + minimumResultsForSearch: -1, initSelection: function ($e, c) { c(_from_data(data, $e.val()) || _make_option($e.val())); } @@ -292,23 +294,25 @@ var DataImport = AbstractAction.extend({ setup_sheets_picker: function () { var data = this.sheets.map(_make_option); this.$('input.oe_import_sheet').select2({ - width: '50%', + width: '100%', data: data, query: dataFilteredQuery, + minimumResultsForSearch: -1, initSelection: function ($e, c) { c(_from_data(data, $e.val()) || _make_option($e.val())) }, - minimumResultsForSearch: 10, }); }, import_options: function () { var self = this; var options = { - headers: this.$('input.oe_import_has_header').prop('checked'), + has_headers: this.$('input.oe_import_has_header').prop('checked'), advanced: this.$('input.oe_import_advanced_mode').prop('checked'), keep_matches: this.do_not_change_match, name_create_enabled_fields: {}, + import_skip_fields: [], + fallback_values: {}, // start at row 1 = skip 0 lines skip: Number(this.$('#oe_import_row_start').val()) - 1 || 0, limit: Number(this.$('#oe_import_batch_limit').val()) || null, @@ -323,17 +327,32 @@ var DataImport = AbstractAction.extend({ _(this.parse_opts_separators).each(function (opt) { options[opt.name] = self.$('input.oe_import_' + opt.name).val(); }); + options['fields'] = []; if (this.do_not_change_match) { - options['fields'] = this.$('.oe_import_fields input.oe_import_match_field').map(function (index, el) { + options['fields'] = this.$('input.oe_import_match_field').map(function (index, el) { return $(el).select2('val') || false; }).get(); } this.do_not_change_match = false; - this.$('input.o_import_create_option').each(function () { + this.$('select.o_import_create_option').each(function () { var field = this.getAttribute('field'); + var type = this.getAttribute('type'); if (field) { - options.name_create_enabled_fields[field] = this.checked; + if (['many2one', "many2many"].includes(type)) { + if (this.value === 'skip') { + options.import_skip_fields.push(field); + } else { + options.name_create_enabled_fields[field] = this.value === 'create'; + } + // for selection, include also 'skip' that will be interpreted as 'None' in backend. + } else if (['boolean','selection'].includes(type) && this.value !== 'prevent') { + options.fallback_values[field] = { + fallback_value: this.value, + field_model: this.getAttribute('model'), + field_type: type + }; + } } }); return options; @@ -356,10 +375,18 @@ var DataImport = AbstractAction.extend({ var import_toggle = false; var file = this.$('input.oe_import_file')[0].files[0]; // some platforms send text/csv, application/csv, or other things if Excel is prevent - if ((file.type && _.last(file.type.split('/')) === "csv") || ( _.last(file.name.split('.')) === "csv")) { - import_toggle = true; - } - this.$form.find('.oe_import_box').toggle(import_toggle); + var isCSV = ((file.type && _.last(file.type.split('/')) === "csv") || ( _.last(file.name.split('.')) === "csv")) + this.$form.find('.o_import_formatting').toggleClass('d-none', !isCSV); + + // get file name and extension separately, to apply ellipsis to the name and still keep the extension visible. + // E.g. : superLongFileName.csv -> "superLongFi....csv" instead of "superLongFileNa..." + var fileName = file.name.split('.'); + var fileExtension = fileName.pop(); + fileName = fileName.join('.'); + this.$('#oe_imported_file').text(fileName); + this.$('#oe_imported_file_extension').text('.' + fileExtension); + + this.$form.find('.oe_import_box').toggle(true); jsonp(this.$form, { url: '/base_import/set_file' }, this.proxy('settings_changed')); @@ -371,12 +398,20 @@ var DataImport = AbstractAction.extend({ // TODO: test that write // succeeded? this.$form.removeClass('oe_import_preview_error oe_import_error'); this.$form.toggleClass( - 'oe_import_noheaders text-muted', + 'oe_import_noheaders', !this.$('input.oe_import_has_header').prop('checked')); // Clear the input value to allow onchange to be triggered // if the file is the same (for all browsers) - self.$('input.oe_import_file').val(''); + this.$('input.oe_import_file').val(''); + this.$('.oe_import_options_cell,.oe_import_options_header').addClass('d-none'); + + this._cleanComments(); + + // Block UI during loading file. + $.blockUI({message: QWeb.render('Throbber')}); + $(document.body).addClass('o_ui_blocked'); + $('.oe_throbber_message').text(_t("Loading file...")); this._rpc({ model: 'base_import.import', @@ -386,6 +421,8 @@ var DataImport = AbstractAction.extend({ }).then(function (result) { var signal = result.error ? 'preview_failed' : 'preview_succeeded'; self[signal](result); + $(document.body).removeClass('o_ui_blocked'); + $.unblockUI(); }); }, onpreview_error: function (event, from, to, result) { @@ -408,7 +445,9 @@ var DataImport = AbstractAction.extend({ this.$form.addClass('oe_import_preview'); this.$('input.oe_import_advanced_mode').prop('checked', result.advanced_mode); this.$('.oe_import_grid').html(QWeb.render('ImportView.preview', result)); - + // Activate the batch configuration panel only of the file length > 100. (In order to let the user choose + // the batch size even for medium size file. Could be useful to reduce the batch size for complex models). + this.$('.o_import_batch').toggleClass('d-none', !(result.file_length > 100)); this.$('.o_import_batch_alert').toggleClass('d-none', !result.batch); var messages = []; @@ -438,9 +477,9 @@ var DataImport = AbstractAction.extend({ // hide all "true debug" options when not in debug mode this.$('.oe_import_debug_option').toggleClass('d-none', !result.debug); - var $fields = this.$('.oe_import_fields input'); + var $fields = this.$('.oe_import_match_field'); this.render_fields_matches(result, $fields); - var data = this.generate_fields_completion(result); + var data = this._generate_fields_completion(result); var item_finder = function (id, items) { items = items || data; for (var i=0; i < items.length; ++i) { @@ -456,81 +495,235 @@ var DataImport = AbstractAction.extend({ return ''; }; $fields.each(function (k,v) { - var filtered_data = self.generate_fields_completion(result, k); + var $fieldInput = $(v); + var filteredData = self._generate_fields_completion(result, k); - var $thing = $(); - var bind = function (d) {}; - if (config.isDebug()) { - $thing = $(QWeb.render('ImportView.create_record_option')).insertAfter(v).hide(); - bind = function (data) { - switch (data.type) { - case 'many2one': case 'many2many': - $thing.find('input').attr('field', data.id); - $thing.show(); - break; - default: - $thing.find('input').attr('field', '').prop('checked', false); - $thing.hide(); - } + var updateFieldInformation = function (fieldInfo) { + // Get the comment cell on the same row in the table. + var $commentCell = $fieldInput.closest('tr.oe_import_grid-row').find('.oe_import_comment_cell'); + var $optionsDiv = $commentCell.find('.oe_import_options_div'); + var isRelational = fieldInfo.type === 'many2many' || fieldInfo.type === 'many2one'; + var showSkip = !fieldInfo.required && (isRelational || fieldInfo.type === 'selection'); + + // get options cell related to that field + if (isRelational || fieldInfo.type === 'boolean') { + $optionsDiv.empty(); + $optionsDiv.append( + QWeb.render('ImportView.create_record_option', { + data: fieldInfo, + is_relational: isRelational, + show_skip: showSkip, + }) + ); + } else if (fieldInfo.type === 'selection') { + self._rpc({ + model: fieldInfo.comodel_name || fieldInfo.model_name, + method: 'fields_get', + }).then(function (values) { + var selectionField = fieldInfo.id.split('/').pop(); + var selectionLabels = values[selectionField]["selection"].map(value => value[1]); + + $optionsDiv.empty(); + $optionsDiv.append( + QWeb.render('ImportView.create_record_option', { + data: fieldInfo, + values: selectionLabels, + show_skip: showSkip, + }) + ); + self.selectionFields[data.id] = values; + }); + } else if ($optionsDiv.find('.o_import_create_option').length == 1) { + $optionsDiv.empty(); } + + // re-attribute comment cell to the new selected field + $commentCell.attr('field', fieldInfo.id || ""); + $commentCell.attr('model', fieldInfo.comodel_name || fieldInfo.model_name || ""); + + // assign class for field icon + var $fieldDropdown = $fieldInput.closest('.oe_import_match_cell').find('.select2-choice'); + var oldType = $fieldDropdown.getAttributes()['type']; + $fieldDropdown.removeClass(`o_import_field_${oldType}`).addClass(`o_import_field_icon o_import_field_${fieldInfo.type}`); + $fieldDropdown.attr('type', fieldInfo.type); + }; + + var default_value = $fieldInput.val(); + var fieldInfo = item_finder(default_value); + if (!fieldInfo) { + $fieldInput.val(''); } - $(v).select2({ + $fieldInput.select2({ allowClear: true, minimumInputLength: 0, - data: filtered_data, + data: filteredData, initSelection: function (element, callback) { - var default_value = element.val(); if (!default_value) { callback(''); return; } - var data = item_finder(default_value); - bind(data); - callback(data); + updateFieldInformation(fieldInfo); + callback(fieldInfo); + self._handleMappingComments(v, fieldInfo); }, - placeholder: _t('Don\'t import'), + // Format the tooltip. + formatSelection: function (object, container) { + var fieldTooltipString = `%(label)s: %(labelValue)s +%(name)s: %(nameValue)s +%(type)s: %(typeValue)s +%(model)s: %(modelValue)s`; + var tooltip = _.str.sprintf(fieldTooltipString, { + 'label': _t("Label"), 'labelValue': object.text, + 'name': _t("Name"), 'nameValue': object.id, + 'type': _t("Type"), 'typeValue': object.type, + 'model': _t("Model"), 'modelValue': object.comodel_name || object.model_name, + }); + $(container[0]).closest('a').attr('title', tooltip); + return object.text; + }, + // For each choice, if field is required, make it bold in the list + formatResultCssClass: function (object) { + if (object.required) { return "font-weight-bold text-decoration-underline"; } + return ""; + }, + placeholder: _t('To import, select a field...'), width: 'resolve', dropdownCssClass: 'oe_import_selector' - }).on('change', function (e) { - bind(item_finder(e.currentTarget.value)); + }).on('change', function (event) { + var changedField = event.currentTarget; + var fieldRemovedId = event.removed ? event.removed.id : false; + self._cleanFieldComments(changedField, fieldRemovedId); + + var fieldInfo = item_finder(changedField.value); + updateFieldInformation(fieldInfo); + self._handleMappingComments(changedField, fieldInfo); + + if (!event.val) { + var $matchingCell = $(changedField).closest('.oe_import_match_cell') + $matchingCell.find('.o_import_field_icon').removeClass('o_import_field_icon'); + $matchingCell.find('a.select2-choice').removeAttr("title"); + } + }); + + $fieldInput.closest('.oe_import_match_cell').find('.select2-input').attr('placeholder', _t('Search for a field...')); + }); + }, + + /** + * This method is called when changing the field to map with a file column, or when removing it. + * It will clean the eventual comments, errors and the mapping options related to the previously selected field. + * + * @private + */ + _cleanFieldComments: function (changedField, fieldRemovedId) { + // Check that the column was not mapped to same field than another column + if (fieldRemovedId) { + var $sameMappedFields = self.$(`.oe_import_comment_cell[field=\"${fieldRemovedId}\"]`).find('.oe_import_same_mapped_field'); + if ($sameMappedFields.length == 2) { + // remove all same mapped field comments + $sameMappedFields.remove(); + } + } + // remove all comments for this header-field mapping row + var $fieldRow = $(changedField).closest('tr.oe_import_grid-row'); + $fieldRow.find('.oe_import_comments_div').empty(); + $fieldRow.find('.oe_import_options_div').addClass("d-none"); + }, + + /** + * This method is called when selecting a sheet or a new file to import, but also when running to import or the + * import test. It will remove all the general comments and/or field specific warnings and errors + * + * @private + */ + _cleanComments: function () { + this.$('.oe_import_error_report').empty(); + this.$('.oe_import_comments_div').find('.alert-error,.alert-warning').remove(); + this.$form.removeClass('oe_import_error'); + }, + + /** + * This method is called when selecting a new mapping field, or when changing/removing an already selected mapping + * field. It will check if multiple field columns are/were mapped on the same field. + * 1. In case the field type is char or text: + * This method will display / remove the comments informing the user that two columns content will be + * concatenated on the field they have chosen. + * 2. Else: + * This method will remove the field mapping from the old column, as each field, except for the case 1., can only + * be mapped once. + * + * @private + */ + _handleMappingComments: function (changedField, fieldInfo) { + // check if two columns are mapped on the same fields (for char/text fields) + var commentsToAdd = []; + var $sameMappedFields = self.$('.oe_import_comment_cell[field="' + fieldInfo.id + '"]'); + + if (fieldInfo.type == 'many2many') { + commentsToAdd.push(QWeb.render('ImportView.comment_m2m_field')); + } + if ($sameMappedFields.length >= 2) { + if (['char', 'text', "many2many"].includes(fieldInfo.type)) { + commentsToAdd.push(QWeb.render('ImportView.comment_same_mapped_field', { + field: fieldInfo.text, + })); + } else { // if column is mapped on an already mapped field, remove that field from the old column. + var $targetMappedFieldId = $(changedField).parent().find('div.oe_import_match_field').getAttributes()['id']; + _.each($sameMappedFields, function(fieldComment) { + var $mappingCell = $(fieldComment).parent().find('div.oe_import_match_field'); + if ($mappingCell.getAttributes()['id'] !== $targetMappedFieldId) { + $mappingCell.find('.select2-search-choice-close').trigger('mousedown').trigger('click'); + } + }); + } + } + + var $commentDiv = $sameMappedFields.find(".oe_import_comments_div"); + $commentDiv.empty(); + _.each($commentDiv, function(fieldComment) { + _.each(commentsToAdd, function(comment) { + $(fieldComment).append(comment); }); }); }, - generate_fields_completion: function (root, index) { + + _generate_fields_completion: function (root, index) { var self = this; var basic = []; var regulars = []; var o2m = []; - var headers_type = root.headers_type; + var suggested = []; + var header_types = root.header_types; function traverse(field, ancestors, collection, type) { var subfields = field.fields; var advanced_mode = self.$('input.oe_import_advanced_mode').prop('checked'); var field_path = ancestors.concat(field); var label = _(field_path).pluck('string').join(' / '); var id = _(field_path).pluck('name').join('/'); - if (type === undefined || (type !== undefined && (type.indexOf('all') !== -1 || type.indexOf(field['type']) !== -1))){ - // If non-relational, m2o or m2m, collection is regulars - if (!collection) { - if (field.name === 'id') { - collection = basic; - } else if (_.isEmpty(subfields) - || _.isEqual(_.pluck(subfields, 'name'), ['id', '.id'])) { - collection = regulars; - } else { - collection = o2m; - } + // If non-relational, m2o or m2m, collection is either suggested if field type is in header_types, either regulars if type does not match + if (!collection) { + if (field.name === 'id') { + collection = basic; + } else if (_.isEmpty(subfields) + || _.isEqual(_.pluck(subfields, 'name'), ['id', '.id'])) { + collection = (type && type.indexOf(field['type']) !== -1) ? suggested : regulars; + } else { + collection = o2m; } - - collection.push({ - id: id, - text: label, - required: field.required, - type: field.type - }); - } + + collection.push({ + id: id, + text: label, + required: field.required, + type: field.type, + default_value: field.default_value, + comodel_name: field.comodel_name, + model_name: field.model_name, + }); + if (advanced_mode){ for(var i=0, end=subfields.length; i partial import this.toggle_partial(results); @@ -773,9 +973,10 @@ var DataImport = AbstractAction.extend({ // display 1-indexed. var offset = 1; // offset more if header - if (this.import_options().headers) { offset += 1; } + if (this.import_options().has_headers) { offset += 1; } - var messagesSorted = _.sortBy(_(message).groupBy('message'), function (group) { + // Sort errors by error gravity then by field + var errorsSorted = _.sortBy(_(errorMessages).groupBy('message'), function (group) { if (group[0].priority){ return -2; } @@ -783,7 +984,7 @@ var DataImport = AbstractAction.extend({ // sort by gravity, then, order of field in list var order = 0; switch (group[0].type) { - case 'error': order = 0; break; + case 'error': order = 0; error_type = 'error'; break; case 'warning': order = fields.length + 1; break; case 'info': order = 2 * (fields.length + 1); break; default: order = 3 * (fields.length + 1); break; @@ -791,72 +992,145 @@ var DataImport = AbstractAction.extend({ return order + _.indexOf(fields, group[0].field); }); - this.$form.addClass('oe_import_error'); - this.$('.oe_import_error_report').html( - QWeb.render('ImportView.error', { - errors: messagesSorted, - at: function (rows) { - var from = rows.from + offset; - var to = rows.to + offset; - var rowName = ''; - if (results.name.length > rows.from && results.name[rows.from] !== '') { - rowName = _.str.sprintf(' (%s)', results.name[rows.from]); - } - if (from === to) { - return _.str.sprintf(_t("at row %d%s"), from, rowName); - } - return _.str.sprintf(_t("between rows %d and %d"), - from, to); - }, - at_multi: function (rows) { - var from = rows.from + offset; - var to = rows.to + offset; - var rowName = ''; - if (results.name.length > rows.from && results.name[rows.from] !== '') { - rowName = _.str.sprintf(' (%s)', results.name[rows.from]); - } - if (from === to) { - return _.str.sprintf(_t("Row %d%s"), from, rowName); - } - return _.str.sprintf(_t("Between rows %d and %d"), - from, to); - }, - at_multi_header: function (numberLines) { - return _.str.sprintf(_t("at %d different rows:"), - numberLines); - }, - more: function (n) { - return _.str.sprintf(_t("(%d more)"), n); - }, - info: function (msg) { - if (typeof msg === 'string') { - return _.str.sprintf( - '
%s
', - _.str.escapeHTML(msg)); - } - if (msg instanceof Array) { - return _.str.sprintf( - '
%s
', - _.str.escapeHTML(_t("See possible values")), - _(msg).map(function (msg) { - return '
  • ' - + _.str.escapeHTML(msg) - + '
    • '; - }).join('')); - } - // Final should be object, action descriptor - return [ - '
    ', - _.str.sprintf(' ', - _.str.escapeHTML(JSON.stringify(msg))), - _.str.escapeHTML( - _t("See possible values")), - '', - '
    ' - ].join(''); - }, - })); + // regroup errors by field + var errorsByFields = this._regroupErrorsByFields(errorsSorted); + + // If no general comments (index "0"), add general warning that there are some errors/warnings + if (!errorsByFields[0] && error_type) { + var message = error_type === "warning" ? + _t("The file contains non-blocking warnings (see below)") : + _t("The file contains blocking errors (see below)"); + errorsByFields[0] = [{ + 'type': error_type, + 'message': message, + }]; + } + + // Clean old comments + self._cleanComments(); + + // Add new comments + _.each(errorsByFields, function (errors, field) { + var $errorCell = self.$('.oe_import_comment_cell[field="'+ field +'"]'); + var $errorDiv = $errorCell.find(".oe_import_comments_div"); + var errorTemplate = 'ImportView.fieldError'; + // If error not linked to a targeted Odoo field, show in global error div + if ($errorDiv.length === 0) { + $errorDiv = self.$('.oe_import_error_report'); + self.$form.addClass('oe_import_error'); + field = 0; + errorTemplate = 'ImportView.error'; + } + $errorDiv.append( + QWeb.render(errorTemplate, { + errors: errors, + field: field, + result_names: results.name, + offset: offset, + }) + ); + var mainError = errors[0]; + if (!mainError.not_matching_error && mainError.type === "error") { + $errorCell.find(".oe_import_options_div").removeClass('d-none'); + } + }); }, + + /** + * This method regroups the errors by their 'field' key. + * Errors of same nature encountered on multiple rows for the same field are regrouped into the same unique error + * and rows_from and rows_to are adapted accordingly. + * The goal is to ease the error attribution by looping through each mapped field and add their corresponding errors + * to the comment cell of that mapped field. + * Some errors are not linked to a mapped field and will be displayed in the general error container. + * Those general errors will be identified with key "0". + * + * @private + * @param {Array} errorsSorted: list of errors sorted by error gravity, then by field. + * Each item of the list is a list of error of the same nature and can be therefore composed of 1 or more errors. + * E.g.: [ + * [ + * { + * 'type': 'error', + * 'field_path': 'product_id/price', + * 'rows': {'from': 1, 'to': 1}, // This error have been encountered between row 1 and row 1 (at row 1) + * 'message': {"Missing Value"} + * }, + * { + * 'type': 'error', + * 'field_path': 'product_id/price', + * 'rows': {'from': 2, 'to': 2}, // This error have been encountered between row 2 and row 2 (at row 2) + * 'message': {"Missing Value"} + * } + * ], + * [ // this one is a general error, not linked specifically to a mapped field. + * { + * 'type': 'error', + * 'message': {"Found more than 10 errors and more than one error per 10 records, interrupted to avoid showing too many errors."} + * } + * ], + * [ + * { + * 'type': 'warning', + * 'field': 'quantity', + * 'rows': {'from': 2, 'to': 4}, // This error have been encountered between row 2 and row 4 (at 3 different rows) + * 'message': {} + * } + * ], + * ] + * @returns {Dict} errorsByFields: {fieldName: [error1, error2, ...]} + * E.g.: { + * 0: [ + * { + * 'type': 'error', + * 'message': {"Found more than 10 errors and more than one error per 10 records, interrupted to avoid showing too many errors."} + * } + * ], + * 'product_id/price': [ + * { + * 'type': 'error', + * 'field_path': 'product_id/price', + * 'rows': {'from': 1, 'to': 2}, // This error have been encountered between row 1 and row 2 + * 'message': {"Missing Value"} + * } + * ], + * 'quantity': [ + * { + * 'type': 'warning', + * 'field': 'quantity', + * 'rows': {'from': 2, 'to': 4}, // This error have been encountered between row 2 and row 4 + * 'message': {} + * } + * ], + * } + */ + _regroupErrorsByFields: function (errorsSorted) { + var errorsByFields = {} + _.each(errorsSorted, function (errors) { + var mainError = errors[0]; + var field = mainError.field_path ? mainError.field_path.join('/') : mainError.field || 0; + + // regroup errors for same value at different rows in the same error. + if (errors.length > 1 && mainError.rows) { + var rowFrom = mainError.rows.from; + var rowTo = errors[errors.length - 1].rows.to; + errors = mainError; + mainError.rows.from = rowFrom; + mainError.rows.to = rowTo; + } else { + errors = mainError; + } + + if (!errorsByFields[field]) { + errorsByFields[field] = [errors]; + } else { + errorsByFields[field].push(errors); + } + }); + + return errorsByFields; + }, + toggle_partial: function (result) { var $form = this.$('.oe_import'); var $partial_warning = this.$('.o_import_partial_alert'); diff --git a/addons/base_import/static/src/scss/base_import.scss b/addons/base_import/static/src/scss/base_import.scss index 955c8f66048..c108fcabe77 100644 --- a/addons/base_import/static/src/scss/base_import.scss +++ b/addons/base_import/static/src/scss/base_import.scss @@ -1,6 +1,6 @@ .oe_import { - @include o-webclient-padding($top: 8px); - overflow: auto; + padding: 0px; + overflow: hidden; position: absolute; // Needed for chrome top: 0; right: 0; @@ -22,10 +22,10 @@ } .oe_import_box { - padding: 8px; - background: #F0EEEE; + flex: 0 0 250px; + padding: 10px 14px; border-radius: $border-radius; - border: solid 1px #dddddd; + border-right: solid 1px #dddddd; label { font-weight: normal; } @@ -34,26 +34,8 @@ } } - a.oe_import_toggle { - display: block; - &:before { - content: '+' - } - } - .oe_import_options { - margin-top: 8px; - p { - margin: 0; - } - label { - width: 48%; - line-height: 32px; - text-align: right; - } - } /* ----------- INITIAL SETUP ------------ */ dd, - .oe_import_toggled, .oe_import_grid, .oe_import_error_report, .oe_import_noheaders, @@ -61,20 +43,30 @@ display: none; } - .oe_import_with_file label { - font-weight: normal; - } - .oe_import_debug_options { - max-width: 800px; - columns: 1; - @include media-breakpoint-up(md) { - columns: 2; + .oe_import_with_file { + flex: 1 1 100%; + background-color: white; + overflow: auto; + label { + font-weight: normal; + &.custom-control-label { + left: 20px; + } } - // try to keep the batch fields together, doesn't work on firefox & - // not sure how to do that (except by adding intermediate dom - // elements) - .oe_import_batch_limit { - break-before: column; + h4 { + margin: 10px 0px; + font-size: 16px; + font-weight: 500; + } + input:checked + label:after { + background-image: url('/web_studio/static/src/img/ui/checkbox_active.svg'); + background-size: cover; + } + .truncate { + max-width: 178px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; } } @@ -87,11 +79,39 @@ } .oe_import_report_showmore .oe_import_report_more { display: list-item; + list-style-type: none; + li { + list-style-type: disc; + } + } + .oe_import_report_count, .oe_import_report_count_less { + .arrow { + border: solid black; + border-width: 0 2px 2px 0; + display: inline-block; + padding: 2px; + margin-right: 2px; + pointer-events: none; + } + + .down { + transform: rotate(45deg); + -webkit-transform: rotate(45deg); + margin-bottom: 2px; + } + + .up { + transform: rotate(-135deg); + -webkit-transform: rotate(-135deg); + } } /* ------------- ERRORS AND WARNINGS REPORT ------------ */ - .oe_import_error_report > ul { - padding: 0; + .oe_import_error_report { + border-bottom: 1px solid lightgrey; + > ul { + padding: 0; + } } .oe_import_report { list-style: none; @@ -117,16 +137,20 @@ /* ------------- THE CSV TABLE ------------ */ $cell-max-width: 350px; - $cell-padding: 4px; + $cell-padding: 8px; .oe_import_grid { tr { + border-bottom: 1px solid lightgrey; &.oe_import_grid-header:first-child { line-height: 24px; font-weight: normal; } .oe_import_grid-cell { + cursor:default; + border-left: 0px; + border-right: 0px; max-width: $cell-max-width; - padding: $cell-padding; + padding: $cell-padding 12px; vertical-align: top; .o_multi_line_text { word-break: break-word; @@ -134,13 +158,60 @@ .o_single_line_text { @include o-text-overflow($display: table-cell, $max-width: $cell-max-width - $cell-padding); } + .alert { + // alias -error to -danger + &.alert-error { + background-color: transparent; + } + &.alert-warning { + background-color: transparent; + } + &.alert-info { + background-color: transparent; + } + } } } } /* Default Color for placeholder on import fields*/ .select2-default{ - color: #F00 !important; + color: #F0AD4E !important; + font-weight: 500; + } + + /* Common dropdown style */ + .oe_import_dropdown { + width: 100% !important; + + select { + border-radius: 4px; + padding-left: 8px; + } + + select, .select2-choice { + border: 1px solid #aaa; + height: 26px; + padding-right: 24px; + border-bottom-right-radius: 4px; + border-top-right-radius: 4px; + background-image: url('/base_import/static/src/img/ui/dropdown_arrow_small.png'); + background-position: center right; + background-repeat: no-repeat; + appearance: none; + width: auto; + } + + .select2-arrow { + display: none !important; + } + } + + input.oe_import_dropdown { + border-radius: 4px; + border: 1px solid darkgrey; + height: 26px; + padding-left: 8px; } } /* ------------- PARTIAL MODE buttons ------------ */ @@ -156,5 +227,10 @@ ul, li { margin: 0; padding: 0; } - width: 250px !important; + .select2-results { + max-height: 192px; + .text-decoration-underline { + text-decoration: underline; + } + } } diff --git a/addons/base_import/static/src/scss/icons.scss b/addons/base_import/static/src/scss/icons.scss new file mode 100644 index 00000000000..09e40c6b3cd --- /dev/null +++ b/addons/base_import/static/src/scss/icons.scss @@ -0,0 +1,100 @@ +@mixin o-import-sprite-icon($x: 0, $y: 0) { + background-position: $x $y; +} + +/* + * Override select style + */ + +/* Field Type Dropdown */ +.oe_import_dropdown.oe_import_match_field { + .o_import_field_icon { + padding-left: 34px; + + abbr { + top: 6px; + } + + /* Show icon */ + &:before { + width: 30px; + height: 26px; + display: block; + float: left; + margin: 0px 5px 0 0; + border-right: 1px solid #a0abb9; + background-color: white; + content: ""; + position: absolute; + left: -1px; + top: -1px; + border-bottom-left-radius: 4px; + border-top-left-radius: 4px; + border: 1px solid darkgrey; + } + &:after{ + background-image: url('/base_import/static/src/img/ui/studio_icons.png'); + background-repeat: no-repeat; + content: ''; + width: 20px; + height: 20px; + position: absolute; + top: 2px; + left: 4px; + bottom: auto; + right: auto; + } + } +} + +.o_import_field_binary:after { + @include o-import-sprite-icon(0, 0); +} + +.o_import_field_boolean:after { + @include o-import-sprite-icon(0, -20px); +} + +.o_import_field_char:after { + @include o-import-sprite-icon(0, -40px); +} + +.o_import_field_date:after { + @include o-import-sprite-icon(0, -80px); +} + +.o_import_field_datetime:after { + @include o-import-sprite-icon(0, -100px); +} + +.o_import_field_float:after { + @include o-import-sprite-icon(0, -120px); +} + +.o_import_field_html:after { + @include o-import-sprite-icon(0, -140px); +} + +.o_import_field_integer:after { + @include o-import-sprite-icon(0, -180px); +} + +.o_import_field_many2many:after { + @include o-import-sprite-icon(0, -200px); +} + +.o_import_field_many2one:after { + @include o-import-sprite-icon(0, -220px); +} + +.o_import_field_one2many:after { + @include o-import-sprite-icon(0, -260px); +} + +.o_import_field_selection:after { + @include o-import-sprite-icon(0, -280px); +} + +.o_import_field_text:after { + @include o-import-sprite-icon(0, -340px); +} diff --git a/addons/base_import/static/src/xml/base_import.xml b/addons/base_import/static/src/xml/base_import.xml index d3bc01f1acb..109614b3ed3 100644 --- a/addons/base_import/static/src/xml/base_import.xml +++ b/addons/base_import/static/src/xml/base_import.xml @@ -12,101 +12,25 @@
    -
    - -
    - - Formatting Options… -
    -
    -

    - - - -

    -
    -
    -

    - - - -

    -
    -
    -
    -
    -
    - Due to its large size, the file will be imported by batches. + +
    + +
    +
    Click 'Resume' to proceed with the import, resuming at line 0.
    You can test or reload your file before resuming the import.
    -
    -

    Map your columns to import

    -
    -
    - - -
    - -
    - - -
    - -
    - - -
    -
    - - -
    -
    - - -
    -
    - - -
    -
    -

    If the file contains - the column names, Odoo can try auto-detecting the - field corresponding to the column. This makes imports - simpler especially when the file has many columns.

    - -
    -
    - - -
    This is a preview of the first 10 rows of your file
    -

    - Select a CSV or Excel file to import. + Upload an Excel or CSV file to import

    - Excel files are recommended as fields formatting is automatic. + Excel files are recommended as formatting is automatic.

    Need Help?
    @@ -124,47 +48,73 @@ - + -
    - +
    + When a value cannot be matched: +
    - - - - - - + + + + - - + + + + + +
    + This column will be concatenated in field +
    +     + + +
    + To import multiple values, separate them by a comma +
    +     +

    Import preview failed due to: .

    @@ -173,55 +123,196 @@
    -
      -
    • - - - - - -
        - -
      • - - - - -
        • -
      • - - - -
        • -         -
      - - - - - + +
      + + + + + + + -
      • -
    - - - - - + - + +
    + + + + +
    + - + + No matching records found for '' in field + + + + + + + + - - - - - - + + +
    + + No matching records found for the following + Multiple errors occured + in field : + +
    + + + + + + + + + + + + at row () + + + at multiple rows + + + + +
    + +
    +
    + + See possible values +
      +
      • +
    +
    +
    + + See possible values +
    +     + + +
    +
    +

    If the file contains + the column names, Odoo can try auto-detecting the + field corresponding to the column. This makes imports + simpler especially when the file has many columns.

    +
    +
    +
    +
    - -
    + File Column + Odoo Field + Comments
    - - - - - - - + +
    + + + Untitled + + + + +
    +
    + + + + + +
    + +
    +

    Imported file

    +
    + +
    + +
    + +
    + + +
    + + + + +
    +

    Formatting

    +
    +
    + + + +
    +
    +
    + +
    +

    Batch Import

    +
    + The file will be imported by batches. +
    +
    +
    + + +
    +
    + + +
    +
    +
    + +

    Help

    + + + Go to Import FAQ + + +
    +

    Advanced

    +
    + + +
    + +
    + + +
    +
    + +
    +
    diff --git a/addons/base_import/tests/test_base_import.py b/addons/base_import/tests/test_base_import.py index d4cbbbd866c..4b5224f0db0 100644 --- a/addons/base_import/tests/test_base_import.py +++ b/addons/base_import/tests/test_base_import.py @@ -9,6 +9,7 @@ import unittest from odoo.tests.common import TransactionCase, can_import from odoo.modules.module import get_module_resource from odoo.tools import mute_logger, pycompat +from odoo.addons.base_import.models.base_import import ImportValidationError ID_FIELD = { 'id': 'id', @@ -21,10 +22,17 @@ ID_FIELD = { } -def make_field(name='value', string='Value', required=False, fields=[], field_type='id'): +def make_field(name='value', string='Value', required=False, fields=None, field_type='id', model_name=None, comodel_name=None): + if fields is None: + fields = [] + field = {'id': name, 'name': name, 'string': string, 'required': required, 'fields': fields, 'type': field_type} + if model_name: + field['model_name'] = model_name + if comodel_name: + field['comodel_name'] = comodel_name return [ ID_FIELD, - {'id': name, 'name': name, 'string': string, 'required': required, 'fields': fields, 'type': field_type}, + field, ] @@ -44,18 +52,19 @@ class BaseImportCase(TransactionCase): pprint.pformat(f2).splitlines() )) + class TestBasicFields(BaseImportCase): def get_fields(self, field): - return self.env['base_import.import'].get_fields('base_import.tests.models.' + field) + return self.env['base_import.import'].get_fields_tree('base_import.tests.models.' + field) def test_base(self): """ A basic field is not required """ - self.assertEqualFields(self.get_fields('char'), make_field(field_type='char')) + self.assertEqualFields(self.get_fields('char'), make_field(field_type='char', model_name='base_import.tests.models.char')) def test_required(self): """ Required fields should be flagged (so they can be fill-required) """ - self.assertEqualFields(self.get_fields('char.required'), make_field(required=True, field_type='char')) + self.assertEqualFields(self.get_fields('char.required'), make_field(required=True, field_type='char', model_name='base_import.tests.models.char.required')) def test_readonly(self): """ Readonly fields should be filtered out""" @@ -63,7 +72,7 @@ class TestBasicFields(BaseImportCase): def test_readonly_states(self): """ Readonly fields with states should not be filtered out""" - self.assertEqualFields(self.get_fields('char.states'), make_field(field_type='char')) + self.assertEqualFields(self.get_fields('char.states'), make_field(field_type='char', model_name='base_import.tests.models.char.states')) def test_readonly_states_noreadonly(self): """ Readonly fields with states having nothing to do with @@ -78,9 +87,11 @@ class TestBasicFields(BaseImportCase): def test_m2o(self): """ M2O fields should allow import of themselves (name_get), their id and their xid""" - self.assertEqualFields(self.get_fields('m2o'), make_field(field_type='many2one', fields=[ - {'id': 'value', 'name': 'id', 'string': 'External ID', 'required': False, 'fields': [], 'type': 'id'}, - {'id': 'value', 'name': '.id', 'string': 'Database ID', 'required': False, 'fields': [], 'type': 'id'}, + self.assertEqualFields(self.get_fields('m2o'), make_field( + field_type='many2one', comodel_name='base_import.tests.models.m2o.related', model_name='base_import.tests.models.m2o', + fields=[ + {'id': 'value', 'name': 'id', 'string': 'External ID', 'required': False, 'fields': [], 'type': 'id', 'model_name': 'base_import.tests.models.m2o'}, + {'id': 'value', 'name': '.id', 'string': 'Database ID', 'required': False, 'fields': [], 'type': 'id', 'model_name': 'base_import.tests.models.m2o'}, ])) def test_m2o_required(self): @@ -88,41 +99,43 @@ class TestBasicFields(BaseImportCase): required as well (the client has to handle that: requiredness is id-based) """ - self.assertEqualFields(self.get_fields('m2o.required'), make_field(field_type='many2one', required=True, fields=[ - {'id': 'value', 'name': 'id', 'string': 'External ID', 'required': True, 'fields': [], 'type': 'id'}, - {'id': 'value', 'name': '.id', 'string': 'Database ID', 'required': True, 'fields': [], 'type': 'id'}, + self.assertEqualFields(self.get_fields('m2o.required'), make_field( + field_type='many2one', required=True, comodel_name='base_import.tests.models.m2o.required.related', model_name='base_import.tests.models.m2o.required', + fields=[ + {'id': 'value', 'name': 'id', 'string': 'External ID', 'required': True, 'fields': [], 'type': 'id', 'model_name': 'base_import.tests.models.m2o.required'}, + {'id': 'value', 'name': '.id', 'string': 'Database ID', 'required': True, 'fields': [], 'type': 'id', 'model_name': 'base_import.tests.models.m2o.required'}, ])) class TestO2M(BaseImportCase): def get_fields(self, field): - return self.env['base_import.import'].get_fields('base_import.tests.models.' + field) + return self.env['base_import.import'].get_fields_tree('base_import.tests.models.' + field) def test_shallow(self): self.assertEqualFields( self.get_fields('o2m'), [ ID_FIELD, - {'id': 'name', 'name': 'name', 'string': "Name", 'required': False, 'fields': [], 'type': 'char',}, + {'id': 'name', 'name': 'name', 'string': "Name", 'required': False, 'fields': [], 'type': 'char', 'model_name': 'base_import.tests.models.o2m'}, { - 'id': 'value', 'name': 'value', 'string': 'Value', - 'required': False, 'type': 'one2many', + 'id': 'value', 'name': 'value', 'string': 'Value', 'model_name': 'base_import.tests.models.o2m', + 'required': False, 'type': 'one2many', 'comodel_name': 'base_import.tests.models.o2m.child', 'fields': [ ID_FIELD, { - 'id': 'parent_id', 'name': 'parent_id', - 'string': 'Parent', 'type': 'many2one', + 'id': 'parent_id', 'name': 'parent_id', 'model_name': 'base_import.tests.models.o2m.child', + 'string': 'Parent', 'type': 'many2one', 'comodel_name': 'base_import.tests.models.o2m', 'required': False, 'fields': [ - {'id': 'parent_id', 'name': 'id', + {'id': 'parent_id', 'name': 'id', 'model_name': 'base_import.tests.models.o2m.child', 'string': 'External ID', 'required': False, 'fields': [], 'type': 'id'}, - {'id': 'parent_id', 'name': '.id', + {'id': 'parent_id', 'name': '.id', 'model_name': 'base_import.tests.models.o2m.child', 'string': 'Database ID', 'required': False, 'fields': [], 'type': 'id'}, ] }, {'id': 'value', 'name': 'value', 'string': 'Value', - 'required': False, 'fields': [], 'type': 'integer' + 'required': False, 'fields': [], 'type': 'integer', 'model_name': 'base_import.tests.models.o2m.child', }, ] } @@ -133,16 +146,28 @@ class TestO2M(BaseImportCase): class TestMatchHeadersSingle(TransactionCase): def test_match_by_name(self): - match = self.env['base_import.import']._match_header('f0', [{'name': 'f0'}], {}) - self.assertEqual(match, [{'name': 'f0'}]) + match = self.env['base_import.import']._get_mapping_suggestion('f0', [{'name': 'f0'}], [], {}) + self.assertEqual(match, {'field_path': ['f0'], 'distance': 0}) def test_match_by_string(self): - match = self.env['base_import.import']._match_header('some field', [{'name': 'bob', 'string': "Some Field"}], {}) - self.assertEqual(match, [{'name': 'bob', 'string': "Some Field"}]) + match = self.env['base_import.import']._get_mapping_suggestion('some field', [{'name': 'bob', 'string': "Some Field"}], [], {}) + self.assertEqual(match, {'field_path': ['bob'], 'distance': 0}) def test_nomatch(self): - match = self.env['base_import.import']._match_header('should not be', [{'name': 'bob', 'string': "wheee"}], {}) - self.assertEqual(match, []) + match = self.env['base_import.import']._get_mapping_suggestion('should not be', [{'name': 'bob', 'string': "wheee"}], [], {}) + self.assertEqual(match, {}) + + def test_close_match(self): + match = self.env['base_import.import']._get_mapping_suggestion('bobe', [{'name': 'bob', 'type': 'char'}], ['char'], {}) + self.assertEqual(match, {'field_path': ['bob'], 'distance': 0.1428571428571429}) + + def test_distant_match(self): + Import = self.env['base_import.import'] + header, field_string = 'same Folding', 'Some Field' + match = Import._get_mapping_suggestion(header, [{'name': 'bob', 'string': field_string, 'type': 'char'}], ['char'], {}) + string_field_dist = Import._get_distance(header.lower(), field_string.lower()) + self.assertEqual(string_field_dist, 0.36363636363636365) + self.assertEqual(match, {}) # if distance >= 0.2, no match returned def test_recursive_match(self): f = { @@ -153,8 +178,8 @@ class TestMatchHeadersSingle(TransactionCase): {'name': 'f1', 'string': "Sub field 2", 'fields': []}, ] } - match = self.env['base_import.import']._match_header('f0/f1', [f], {}) - self.assertEqual(match, [f, f['fields'][1]]) + match = self.env['base_import.import']._get_mapping_suggestion('f0/f1', [f], [], {}) + self.assertEqual(match, {'field_path': [f['name'], f['fields'][1]['name']]}) def test_recursive_nomatch(self): """ Match first level, fail to match second level @@ -167,50 +192,59 @@ class TestMatchHeadersSingle(TransactionCase): {'name': 'f1', 'string': "Sub field 2", 'fields': []}, ] } - match = self.env['base_import.import']._match_header('f0/f2', [f], {}) - self.assertEqual(match, []) + match = self.env['base_import.import']._get_mapping_suggestion('f0/f2', [f], [], {}) + self.assertEqual(match, {}) class TestMatchHeadersMultiple(TransactionCase): def test_noheaders(self): self.assertEqual( - self.env['base_import.import']._match_headers([], [], {}), ([], {}) + self.env['base_import.import']._get_mapping_suggestions([], {}, []), {} ) def test_nomatch(self): self.assertEqual( - self.env['base_import.import']._match_headers( - iter([ - ['foo', 'bar', 'baz', 'qux'], - ['v1', 'v2', 'v3', 'v4'], - ]), - [], - {'headers': True}), - ( + self.env['base_import.import']._get_mapping_suggestions( ['foo', 'bar', 'baz', 'qux'], - dict.fromkeys(range(4)) - ) + { + (0, 'foo'): ['int'], + (1, 'bar'): ['char'], + (2, 'baz'): ['text'], + (3, 'qux'): ['many2one'] + }, + {}), + { + (0, 'foo'): None, + (1, 'bar'): None, + (2, 'baz'): None, + (3, 'qux'): None + } ) def test_mixed(self): self.assertEqual( - self.env['base_import.import']._match_headers( - iter(['foo bar baz qux/corge'.split()]), + self.env['base_import.import']._get_mapping_suggestions( + 'foo bar baz qux/corge'.split(), + { + (0, 'foo'): ['int'], + (1, 'bar'): ['char'], + (2, 'baz'): ['text'], + (3, 'qux/corge'): ['text'] + }, [ - {'name': 'bar', 'string': 'Bar'}, - {'name': 'bob', 'string': 'Baz'}, - {'name': 'qux', 'string': 'Qux', 'fields': [ - {'name': 'corge', 'fields': []}, + {'name': 'bar', 'string': 'Bar', 'type': 'char'}, + {'name': 'bob', 'string': 'Baz', 'type': 'text'}, + {'name': 'qux', 'string': 'Qux', 'type': 'many2one', 'fields': [ + {'name': 'corge', 'type': 'text', 'fields': []}, ]} - ], - {'headers': True}), - (['foo', 'bar', 'baz', 'qux/corge'], { - 0: None, - 1: ['bar'], - 2: ['bob'], - 3: ['qux', 'corge'], - }) + ]), + { + (0, 'foo'): None, + (1, 'bar'): {'field_path': ['bar'], 'distance': 0}, + (2, 'baz'): {'field_path': ['bob'], 'distance': 0}, + (3, 'qux/corge'): {'field_path': ['qux', 'corge']} + } ) @@ -225,10 +259,10 @@ class TestColumnMapping(TransactionCase): 'file_type': 'text/csv', 'file_name': 'data.csv', }) - import_record.do( + import_record.execute_import( ['name', 'somevalue', 'othervalue'], ['Name', 'Some Value', 'value'], - {'quoting': '"', 'separator': ',', 'headers': True}, + {'quoting': '"', 'separator': ',', 'has_headers': True}, True ) fields = self.env['base_import.mapping'].search_read( @@ -238,6 +272,29 @@ class TestColumnMapping(TransactionCase): self.assertItemsEqual([f['column_name'] for f in fields], ['Name', 'Some Value', 'value']) self.assertItemsEqual([f['field_name'] for f in fields], ['somevalue', 'name', 'othervalue']) + def test_fuzzy_match_distance(self): + values_to_test = [ + ('opportunities', 'opportinuties'), + ('opportunities', 'opportunate'), + ('opportunities', 'operable'), + ('opportunities', 'purchasing'), + ('lead_id', 'laed_id'), + ('lead_id', 'leen_id'), + ('lead_id', 'let_id_be'), + ('lead_id', 'not related'), + ] + + Import = self.env['base_import.import'] + max_distance = 0.2 # see FUZZY_MATCH_DISTANCE. We don't use it here to avoid making test work after modifying this constant. + for value in values_to_test: + distance = Import._get_distance(value[0].lower(), value[1].lower()) + model_fields_info = [{'name': value[0], 'string': value[0], 'type': 'char'}] + match = self.env['base_import.import']._get_mapping_suggestion(value[1], model_fields_info, ['char'], {}) + + self.assertEqual( + bool(match), distance < max_distance + ) + class TestPreview(TransactionCase): @@ -279,8 +336,8 @@ class TestPreview(TransactionCase): import_wizard = self.env['base_import.import'].create({ 'res_model': 'base_import.tests.models.preview', 'file': b'name,Some Value,Counter\n' - b'foo,1,2\n' - b'bar,3,4\n' + b'foo,,\n' + b'bar,,4\n' b'qux,5,6\n', 'file_type': 'text/csv' }) @@ -288,23 +345,19 @@ class TestPreview(TransactionCase): result = import_wizard.parse_preview({ 'quoting': '"', 'separator': ',', - 'headers': True, + 'has_headers': True, }) self.assertIsNone(result.get('error')) - self.assertEqual(result['matches'], {0: ['name'], 1: ['somevalue'], 2: None}) + self.assertEqual(result['matches'], {0: ['name'], 1: ['somevalue']}) self.assertEqual(result['headers'], ['name', 'Some Value', 'Counter']) # Order depends on iteration order of fields_get self.assertItemsEqual(result['fields'], [ ID_FIELD, - {'id': 'name', 'name': 'name', 'string': 'Name', 'required': False, 'fields': [], 'type': 'char'}, - {'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required': True, 'fields': [], 'type': 'integer'}, - {'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required': False, 'fields': [], 'type': 'integer'}, - ]) - self.assertEqual(result['preview'], [ - ['foo', '1', '2'], - ['bar', '3', '4'], - ['qux', '5', '6'], + {'id': 'name', 'name': 'name', 'string': 'Name', 'required': False, 'fields': [], 'type': 'char', 'model_name': 'base_import.tests.models.preview'}, + {'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required': True, 'fields': [], 'type': 'integer', 'model_name': 'base_import.tests.models.preview'}, + {'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required': False, 'fields': [], 'type': 'integer', 'model_name': 'base_import.tests.models.preview'}, ]) + self.assertEqual(result['preview'], ['foo', '5', '4']) @unittest.skipUnless(can_import('xlrd'), "XLRD module not available") def test_xls_success(self): @@ -317,22 +370,18 @@ class TestPreview(TransactionCase): }) result = import_wizard.parse_preview({ - 'headers': True, + 'has_headers': True, }) self.assertIsNone(result.get('error')) - self.assertEqual(result['matches'], {0: ['name'], 1: ['somevalue'], 2: None}) + self.assertEqual(result['matches'], {0: ['name'], 1: ['somevalue']}) self.assertEqual(result['headers'], ['name', 'Some Value', 'Counter']) self.assertItemsEqual(result['fields'], [ ID_FIELD, - {'id': 'name', 'name': 'name', 'string': 'Name', 'required': False, 'fields': [], 'type': 'char'}, - {'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required': True, 'fields': [], 'type': 'integer'}, - {'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required': False, 'fields': [], 'type': 'integer'}, - ]) - self.assertEqual(result['preview'], [ - ['foo', '1', '2'], - ['bar', '3', '4'], - ['qux', '5', '6'], + {'id': 'name', 'name': 'name', 'string': 'Name', 'required': False, 'fields': [], 'type': 'char', 'model_name': 'base_import.tests.models.preview'}, + {'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required': True, 'fields': [], 'type': 'integer', 'model_name': 'base_import.tests.models.preview'}, + {'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required': False, 'fields': [], 'type': 'integer', 'model_name': 'base_import.tests.models.preview'}, ]) + self.assertEqual(result['preview'], ['foo', '1', '2']) @unittest.skipUnless(can_import('xlrd.xlsx'), "XLRD/XLSX not available") def test_xlsx_success(self): @@ -345,22 +394,18 @@ class TestPreview(TransactionCase): }) result = import_wizard.parse_preview({ - 'headers': True, + 'has_headers': True, }) self.assertIsNone(result.get('error')) - self.assertEqual(result['matches'], {0: ['name'], 1: ['somevalue'], 2: None}) + self.assertEqual(result['matches'], {0: ['name'], 1: ['somevalue']}) self.assertEqual(result['headers'], ['name', 'Some Value', 'Counter']) self.assertItemsEqual(result['fields'], [ ID_FIELD, - {'id': 'name', 'name': 'name', 'string': 'Name', 'required': False, 'fields': [], 'type': 'char'}, - {'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required': True, 'fields': [], 'type': 'integer'}, - {'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required': False, 'fields': [], 'type': 'integer'}, - ]) - self.assertEqual(result['preview'], [ - ['foo', '1', '2'], - ['bar', '3', '4'], - ['qux', '5', '6'], + {'id': 'name', 'name': 'name', 'string': 'Name', 'required': False, 'fields': [], 'type': 'char', 'model_name': 'base_import.tests.models.preview'}, + {'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required': True, 'fields': [], 'type': 'integer', 'model_name': 'base_import.tests.models.preview'}, + {'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required': False, 'fields': [], 'type': 'integer', 'model_name': 'base_import.tests.models.preview'}, ]) + self.assertEqual(result['preview'], ['foo', '1', '2']) @unittest.skipUnless(can_import('odf'), "ODFPY not available") def test_ods_success(self): @@ -373,22 +418,19 @@ class TestPreview(TransactionCase): }) result = import_wizard.parse_preview({ - 'headers': True, + 'has_headers': True, }) self.assertIsNone(result.get('error')) - self.assertEqual(result['matches'], {0: ['name'], 1: ['somevalue'], 2: None}) + self.assertEqual(result['matches'], {0: ['name'], 1: ['somevalue']}) self.assertEqual(result['headers'], ['name', 'Some Value', 'Counter']) self.assertItemsEqual(result['fields'], [ ID_FIELD, - {'id': 'name', 'name': 'name', 'string': 'Name', 'required': False, 'fields': [], 'type': 'char'}, - {'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required': True, 'fields': [], 'type': 'integer'}, - {'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required': False, 'fields': [], 'type': 'integer'}, - ]) - self.assertEqual(result['preview'], [ - ['foo', '1', '2'], - ['bar', '3', '4'], - ['aux', '5', '6'], + {'id': 'name', 'name': 'name', 'string': 'Name', 'required': False, 'fields': [], 'type': 'char', 'model_name': 'base_import.tests.models.preview'}, + {'id': 'somevalue', 'name': 'somevalue', 'string': 'Some Value', 'required': True, 'fields': [], 'type': 'integer', 'model_name': 'base_import.tests.models.preview'}, + {'id': 'othervalue', 'name': 'othervalue', 'string': 'Other Variable', 'required': False, 'fields': [], 'type': 'integer', 'model_name': 'base_import.tests.models.preview'}, ]) + self.assertEqual(result['preview'], ['foo', '1', '2']) + class test_convert_import_data(TransactionCase): """ Tests conversion of base_import.import input into data which @@ -406,7 +448,7 @@ class test_convert_import_data(TransactionCase): }) data, fields = import_wizard._convert_import_data( ['name', 'somevalue', 'othervalue'], - {'quoting': '"', 'separator': ',', 'headers': True} + {'quoting': '"', 'separator': ',', 'has_headers': True} ) self.assertItemsEqual(fields, ['name', 'somevalue', 'othervalue']) @@ -425,7 +467,7 @@ class test_convert_import_data(TransactionCase): }) - results = import_wizard.do( + results = import_wizard.execute_import( ['name', 'date', 'create_date'], [], { @@ -433,7 +475,7 @@ class test_convert_import_data(TransactionCase): 'datetime_format': '%Y-%m-%d %H:%M', 'quoting': '"', 'separator': ',', - 'headers': True + 'has_headers': True } ) @@ -457,7 +499,7 @@ class test_convert_import_data(TransactionCase): 'separator': ',', 'float_decimal_separator': ',', 'float_thousand_separator': '.', - 'headers': True + 'has_headers': True } data, import_fields = import_wizard._convert_import_data( ['name', 'parent_id/.id', 'parent_id/date', 'parent_id/credit_limit'], @@ -503,7 +545,7 @@ class test_convert_import_data(TransactionCase): }) data, fields = import_wizard._convert_import_data( ['name', False, 'othervalue'], - {'quoting': '"', 'separator': ',', 'headers': True} + {'quoting': '"', 'separator': ',', 'has_headers': True} ) self.assertItemsEqual(fields, ['name', 'othervalue']) @@ -527,7 +569,7 @@ class test_convert_import_data(TransactionCase): }) data, fields = import_wizard._convert_import_data( ['name', False, 'othervalue'], - {'quoting': '"', 'separator': ',', 'headers': True} + {'quoting': '"', 'separator': ',', 'has_headers': True} ) self.assertItemsEqual(fields, ['name', 'othervalue']) @@ -549,7 +591,7 @@ class test_convert_import_data(TransactionCase): }) data, fields = import_wizard._convert_import_data( ['name', 'somevalue'], - {'quoting': '"', 'separator': ',', 'headers': True} + {'quoting': '"', 'separator': ',', 'has_headers': True} ) self.assertItemsEqual(fields, ['name', 'somevalue']) @@ -566,7 +608,7 @@ class test_convert_import_data(TransactionCase): 'file_type': 'text/csv' }) - self.assertRaises(ValueError, import_wizard._convert_import_data, [], {'quoting': '"', 'separator': ',', 'headers': True}) + self.assertRaises(ImportValidationError, import_wizard._convert_import_data, [], {'quoting': '"', 'separator': ',', 'has_headers': True}) def test_falsefields(self): import_wizard = self.env['base_import.import'].create({ @@ -577,10 +619,10 @@ class test_convert_import_data(TransactionCase): }) self.assertRaises( - ValueError, + ImportValidationError, import_wizard._convert_import_data, [False, False, False], - {'quoting': '"', 'separator': ',', 'headers': True}) + {'quoting': '"', 'separator': ',', 'has_headers': True}) def test_newline_import(self): """ @@ -601,11 +643,12 @@ class test_convert_import_data(TransactionCase): }) data, _ = import_wizard._convert_import_data( ['name', 'somevalue'], - {'quoting': '"', 'separator': ',', 'headers': True} + {'quoting': '"', 'separator': ',', 'has_headers': True} ) self.assertItemsEqual(data, [data_row]) + class TestBatching(TransactionCase): def _makefile(self, rows): f = io.BytesIO() @@ -625,7 +668,7 @@ class TestBatching(TransactionCase): result = import_wizard.parse_preview({ 'quoting': '"', 'separator': ',', - 'headers': True, + 'has_headers': True, 'limit': 100, }) self.assertIsNone(result.get('error')) @@ -634,7 +677,7 @@ class TestBatching(TransactionCase): result = import_wizard.parse_preview({ 'quoting': '"', 'separator': ',', - 'headers': True, + 'has_headers': True, 'limit': 5, }) self.assertIsNone(result.get('error')) @@ -669,11 +712,11 @@ class TestBatching(TransactionCase): 'file_name': 'things.csv', 'file': f.getvalue(), }) - opts = {'quoting': '"', 'separator': ',', 'headers': True} + opts = {'quoting': '"', 'separator': ',', 'has_headers': True} preview = import_wizard.parse_preview({**opts, 'limit': 15}) self.assertIs(preview['batch'], True) - results = import_wizard.do( + results = import_wizard.execute_import( ['name', 'value/value'], [], {**opts, 'limit': 5} ) @@ -681,7 +724,7 @@ class TestBatching(TransactionCase): self.assertEqual(len(results['ids']), 1, "should have imported the first record in full, got %s" % results['ids']) self.assertEqual(results['nextrow'], 10) - results = import_wizard.do( + results = import_wizard.execute_import( ['name', 'value/value'], [], {**opts, 'limit': 15} ) @@ -692,7 +735,7 @@ class TestBatching(TransactionCase): def test_batches(self): partners_before = self.env['res.partner'].search([]) - opts = {'headers': True, 'separator': ',', 'quoting': '"'} + opts = {'has_headers': True, 'separator': ',', 'quoting': '"'} import_wizard = self.env['base_import.import'].create({ 'res_model': 'res.partner', @@ -710,7 +753,7 @@ g,g@example.com """ }) - results = import_wizard.do(['name', 'email'], [], {**opts, 'limit': 1}) + results = import_wizard.execute_import(['name', 'email'], [], {**opts, 'limit': 1}) self.assertFalse(results['messages']) self.assertEqual(len(results['ids']), 1) # titlerow is ignored by lastrow's counter @@ -718,7 +761,7 @@ g,g@example.com partners_1 = self.env['res.partner'].search([]) - partners_before self.assertEqual(partners_1.name, 'a') - results = import_wizard.do(['name', 'email'], [], {**opts, 'limit': 2, 'skip': 1}) + results = import_wizard.execute_import(['name', 'email'], [], {**opts, 'limit': 2, 'skip': 1}) self.assertFalse(results['messages']) self.assertEqual(len(results['ids']), 2) # empty row should also be ignored @@ -726,13 +769,14 @@ g,g@example.com partners_2 = self.env['res.partner'].search([]) - (partners_before | partners_1) self.assertEqual(partners_2.mapped('name'), ['b', 'c']) - results = import_wizard.do(['name', 'email'], [], {**opts, 'limit': 10, 'skip': 3}) + results = import_wizard.execute_import(['name', 'email'], [], {**opts, 'limit': 10, 'skip': 3}) self.assertFalse(results['messages']) self.assertEqual(len(results['ids']), 4) self.assertEqual(results['nextrow'], 0) partners_3 = self.env['res.partner'].search([]) - (partners_before | partners_1 | partners_2) self.assertEqual(partners_3.mapped('name'), ['d', 'e', 'f', 'g']) + class test_failures(TransactionCase): def test_big_attachments(self): """ @@ -755,8 +799,8 @@ class test_failures(TransactionCase): 'file': fout.getvalue(), 'file_type': 'text/csv' }) - results = import_wizard.do( + results = import_wizard.execute_import( ['name', 'db_datas'], [], - {'headers': True, 'separator': ',', 'quoting': '"'}) + {'has_headers': True, 'separator': ',', 'quoting': '"'}) self.assertFalse(results['messages'], "results should be empty on successful import") diff --git a/addons/base_import/tests/test_csv_magic.py b/addons/base_import/tests/test_csv_magic.py index b7975337361..4e693ed32a1 100644 --- a/addons/base_import/tests/test_csv_magic.py +++ b/addons/base_import/tests/test_csv_magic.py @@ -6,6 +6,7 @@ import codecs from odoo.tests import common + class ImportCase(common.TransactionCase): def _make_import(self, contents): return self.env['base_import.import'].create({ @@ -58,7 +59,7 @@ class TestEncoding(ImportCase): auto-detection """ s = "Iñtërnâtiônàlizætiøn".encode('utf-8') - r = self._make_import(b'text\n' + s)\ + r = self._make_import(s + b'\ntext')\ .parse_preview({ 'quoting': '"', 'separator': '\t', @@ -66,7 +67,8 @@ class TestEncoding(ImportCase): }) self.assertIsNone(r.get('error')) self.assertEqual(r['options']['encoding'], 'iso-8859-1') - self.assertEqual(r['preview'], [['text'], [s.decode('iso-8859-1')]]) + self.assertEqual(r['preview'], [s.decode('iso-8859-1')]) + class TestFileSeparator(ImportCase): @@ -83,17 +85,12 @@ d|4 def test_explicit_success(self): r = self.imp.parse_preview({ 'separator': '|', - 'headers': True, + 'has_headers': True, 'quoting': '"', }) self.assertIsNone(r.get('error')) self.assertEqual(r['headers'], ['c', 'f']) - self.assertEqual(r['preview'], [ - ['a', '1'], - ['b', '2'], - ['c', '3'], - ['d', '4'], - ]) + self.assertEqual(r['preview'], ['a', '1']) self.assertEqual(r['options']['separator'], '|') def test_explicit_fail(self): @@ -101,33 +98,23 @@ d|4 """ r = self.imp.parse_preview({ 'separator': ',', - 'headers': True, + 'has_headers': True, 'quoting': '"', }) self.assertIsNone(r.get('error')) self.assertEqual(r['headers'], ['c|f']) - self.assertEqual(r['preview'], [ - ['a|1'], - ['b|2'], - ['c|3'], - ['d|4'], - ]) + self.assertEqual(r['preview'], ['a|1']) self.assertEqual(r['options']['separator'], ',') def test_guess_ok(self): r = self.imp.parse_preview({ 'separator': '', - 'headers': True, + 'has_headers': True, 'quoting': '"', }) self.assertIsNone(r.get('error')) self.assertEqual(r['headers'], ['c', 'f']) - self.assertEqual(r['preview'], [ - ['a', '1'], - ['b', '2'], - ['c', '3'], - ['d', '4'], - ]) + self.assertEqual(r['preview'], ['a', '1']) self.assertEqual(r['options']['separator'], '|') def test_noguess(self): @@ -137,19 +124,15 @@ d|4 imp = self._make_import('c\na\nb\nc\nd') r = imp.parse_preview({ 'separator': '', - 'headers': True, + 'has_headers': True, 'quoting': '"', }) self.assertIsNone(r.get('error')) self.assertEqual(r['headers'], ['c']) - self.assertEqual(r['preview'], [ - ['a'], - ['b'], - ['c'], - ['d'], - ]) + self.assertEqual(r['preview'], ['a']) self.assertEqual(r['options']['separator'], '') + class TestNumberSeparators(common.TransactionCase): def test_parse_float(self): w = self.env['base_import.import'].create({ diff --git a/odoo/addons/base/models/ir_fields.py b/odoo/addons/base/models/ir_fields.py index 037a1491f77..80f3ce31513 100644 --- a/odoo/addons/base/models/ir_fields.py +++ b/odoo/addons/base/models/ir_fields.py @@ -42,6 +42,41 @@ class IrFieldsConverter(models.AbstractModel): error_params = tuple(sanitize(v) for v in error_params) return error_type(error_msg % error_params, error_args) + def _get_import_field_path(self, field, value): + """ Rebuild field path for import error attribution to the right field. + This method uses the 'parent_fields_hierarchy' context key built during treatment of one2many fields + (_str_to_one2many). As the field to import is the last of the chain (child_id/child_id2/field_to_import), + we need to retrieve the complete hierarchy in case of error in order to assign the error to the correct + column in the import UI. + + :param (str) field: field in which the value will be imported. + :param (str or list) value: + - str: in most of the case the value we want to import into a field is a string (or a number). + - list: when importing into a one2may field, all the records to import are regrouped into a list of dict. + E.g.: creating multiple partners: [{None: 'ChildA_1', 'type': 'Private address'}, {None: 'ChildA_2', 'type': 'Private address'}] + where 'None' is the name. (because we can find a partner by his name, we don't need to specify the field.) + + The field_path value is computed based on the last field in the chain. + for example, + - path_field for 'Private address' at childA_1 is ['partner_id', 'type'] + - path_field for 'childA_1' is ['partner_id'] + + So, by retrieving the correct field_path for each value to import, if errors are raised for those fields, + we can the link the errors to the correct header-field couple in the import UI. + """ + field_path = [field] + parent_fields_hierarchy = self._context.get('parent_fields_hierarchy') + if parent_fields_hierarchy: + field_path = parent_fields_hierarchy + field_path + + field_path_value = value + while isinstance(field_path_value, list): + key = list(field_path_value[0].keys())[0] + if key: + field_path.append(key) + field_path_value = field_path_value[0][key] + return field_path + @api.model def for_model(self, model, fromtype=str): """ Returns a converter object for the model. A converter is a @@ -63,6 +98,7 @@ class IrFieldsConverter(models.AbstractModel): def fn(record, log): converted = {} + import_file_context = self.env.context.get('import_file') for field, value in record.items(): if field in REFERENCING_FIELDS: continue @@ -78,6 +114,21 @@ class IrFieldsConverter(models.AbstractModel): w = ImportWarning(w) log(field, w) except ValueError as e: + if import_file_context: + # if the error is linked to a matching error, the error is a tuple + # E.g.:("Value X cannot be found for field Y at row 1", { + # 'more_info': {}, + # 'value': 'X', + # 'field': 'Y', + # 'field_path': child_id/Y, + # }) + # In order to link the error to the correct header-field couple in the import UI, we need to add + # the field path to the additional error info. + # As we raise the deepest child in error, we need to add the field path only for the deepest + # error in the import recursion. (if field_path is given, don't overwrite it) + error_info = len(e.args) > 1 and e.args[1] + if error_info and not error_info.get('field_path'): # only raise the deepest child in error + error_info['field_path'] = self._get_import_field_path(field, value) log(field, e) return converted @@ -134,7 +185,6 @@ class IrFieldsConverter(models.AbstractModel): @api.model def _str_to_boolean(self, model, field, value): # all translatables used for booleans - true, yes, false, no = _(u"true"), _(u"yes"), _(u"false"), _(u"no") # potentially broken casefolding? What about locales? trues = set(word.lower() for word in itertools.chain( [u'1', u"true", u"yes"], # don't use potentially translated values @@ -154,9 +204,9 @@ class IrFieldsConverter(models.AbstractModel): return False, [] return True, [self._format_import_error( - ImportWarning, - _(u"Unknown value '%s' for boolean field '%%(field)s', assuming '%s'"), - (value, yes), + ValueError, + _(u"Unknown value '%s' for boolean field '%%(field)s'"), + value, {'moreinfo': _(u"Use '1' for yes and '0' for no")} )] @@ -263,7 +313,9 @@ class IrFieldsConverter(models.AbstractModel): for item, label in selection: label = ustr(label) labels = [label] + self._get_translations(('selection', 'model', 'code'), label) - if value == str(item) or value in labels: + # case insensitive comparaison of string to allow to set the value even if the given 'value' param is not + # exactly (case sensitive) the same as one of the selection item. + if value.lower() == str(item).lower() or any(value.lower() == label.lower() for label in labels): return item, [] raise self._format_import_error( @@ -347,8 +399,8 @@ class IrFieldsConverter(models.AbstractModel): if ids: if len(ids) > 1: warnings.append(ImportWarning( - _(u"Found multiple matches for field '%%(field)s' (%d matches)") - % (len(ids)))) + _(u"Found multiple matches for value '%s' in field '%%(field)s' (%d matches)") + % (value, len(ids)))) id, _name = ids[0] else: name_create_enabled_fields = self.env.context.get('name_create_enabled_fields') or {} @@ -364,16 +416,29 @@ class IrFieldsConverter(models.AbstractModel): subfield ) - if id is None: + import_skip = False + if self.env.context.get('import_file'): + import_skip_fields = self.env.context.get('import_skip_fields') or [] + field_path = "/".join((self.env.context.get('parent_fields_hierarchy', []) + [field.name])) + import_skip = field_path in import_skip_fields + if id is None and not import_skip: if error_msg: message = _("No matching record found for %(field_type)s '%(value)s' in field '%%(field)s' and the following error was encountered when we attempted to create one: %(error_message)s") else: message = _("No matching record found for %(field_type)s '%(value)s' in field '%%(field)s'") + + error_info_dict = {'moreinfo': action} + if self.env.context.get('import_file'): + # limit to 50 char to avoid too long error messages. + value = value[:50] if isinstance(value, str) else value + error_info_dict.update({'value': value, 'field_type': field_type}) + if error_msg: + error_info_dict['error_message'] = error_msg raise self._format_import_error( ValueError, message, {'field_type': field_type, 'value': value, 'error_message': error_msg}, - {'moreinfo': action}) + error_info_dict) return id, field_type, warnings def _xmlid_to_record_id(self, xmlid, model): @@ -485,7 +550,14 @@ class IrFieldsConverter(models.AbstractModel): raise exception warnings.append(exception) - convert = self.with_context(name_create_enabled_fields=relative_name_create_enabled_fields).for_model(self.env[field.comodel_name]) + # Complete the field hierarchy path + # E.g. For "parent/child/subchild", field hierarchy path for "subchild" is ['parent', 'child'] + parent_fields_hierarchy = self._context.get('parent_fields_hierarchy', []) + [field.name] + + convert = self.with_context( + name_create_enabled_fields=relative_name_create_enabled_fields, + parent_fields_hierarchy=parent_fields_hierarchy + ).for_model(self.env[field.comodel_name]) for record in records: id = None diff --git a/odoo/addons/test_impex/tests/test_load.py b/odoo/addons/test_impex/tests/test_load.py index c3dfba07534..bc989799532 100644 --- a/odoo/addons/test_impex/tests/test_load.py +++ b/odoo/addons/test_impex/tests/test_load.py @@ -43,6 +43,10 @@ class ImporterCase(common.TransactionCase): self.cr.cache.clear() def import_(self, fields, rows, context=None): + context = context or {} + context.update({ + 'import_file': True + }) return self.model.with_context(context or {}).load(fields, rows) def read(self, fields=('value',), domain=(), context=None): @@ -169,20 +173,23 @@ class test_boolean_field(ImporterCase): self.assertEqual([False] * len(falses), values(self.read())) def test_trues(self): + # Since importing wrong boolean values is now returning error, import should not return any ids if an error is raised. trues = [['None'], ['nil'], ['()'], ['f'], ['#f'], # Problem: OpenOffice (and probably excel) output localized booleans ['VRAI'], ['ok'], ['true'], ['yes'], ['1'], ] result = self.import_(['value'], trues) - self.assertEqual(len(result['ids']), 10) - self.assertEqual(result['messages'], [ - message(u"Unknown value '%s' for boolean field 'Value', assuming 'yes'" % v[0], - moreinfo=u"Use '1' for yes and '0' for no", - type='warning', from_=i, to_=i, record=i) - for i, v in enumerate(trues) - if v[0] not in ('true', 'yes', '1') - ]) + self.assertEqual(result['ids'], False) + self.assertEqual(result['messages'], [{ + 'rows': {'from': i, 'to': i}, 'type': 'error', 'record': i, 'field': 'value', + 'message': "Unknown value '%s' for boolean field 'Value'" % v[0], + 'moreinfo': "Use '1' for yes and '0' for no", 'field_name': 'Value' + } for i, v in enumerate(trues) if v[0] not in ('true', 'yes', '1')]) + + # Only correct boolean values are accepted. + result = self.import_(['value'], [['1'], ['yes'], ['true']]) + self.assertEqual(len(result['ids']), 3) self.assertEqual( - [True] * 10, + [True] * 3, values(self.read())) @@ -265,6 +272,7 @@ class test_integer_field(ImporterCase): result = self.import_(['value'], [['zorglub']]) self.assertIs(result['ids'], False) self.assertEqual(result['messages'], [{ + 'field_name': 'Value', 'type': 'error', 'rows': {'from': 0, 'to': 0}, 'record': 0, @@ -336,7 +344,7 @@ class test_float_field(ImporterCase): result = self.import_(['value'], [['foobar']]) self.assertIs(result['ids'], False) self.assertEqual(result['messages'], [ - message(u"'foobar' does not seem to be a number for field 'Value'")]) + message(u"'foobar' does not seem to be a number for field 'Value'", field_name='Value')]) class test_string_field(ImporterCase): model_name = 'export.string.bounded' @@ -480,13 +488,14 @@ class test_selection(ImporterCase): self.assertIs(result['ids'], False) self.assertEqual(result['messages'], [message( u"Value 'Baz' not found in selection field 'Value'", - moreinfo="Foo Bar Qux 4".split())]) + moreinfo="Foo Bar Qux 4".split(), field_name='Value', field_path=['value'])]) result = self.import_(['value'], [['42']]) self.assertIs(result['ids'], False) self.assertEqual(result['messages'], [message( u"Value '42' not found in selection field 'Value'", - moreinfo="Foo Bar Qux 4".split())]) + moreinfo="Foo Bar Qux 4".split(), + field_name='Value', field_path=['value'])]) class test_selection_with_default(ImporterCase): @@ -627,7 +636,7 @@ class test_m2o(ImporterCase): result = self.import_(['value'], [[name2]]) self.assertEqual( result['messages'], - [message(u"Found multiple matches for field 'Value' (2 matches)", + [message(u"Found multiple matches for value 'export.integer:42' in field 'Value' (2 matches)", type='warning')]) self.assertEqual(len(result['ids']), 1) self.assertEqual([ @@ -651,7 +660,8 @@ class test_m2o(ImporterCase): self.assertEqual(result['messages'], [ message(u"No matching record found for name '%s' in field 'Value'" % id, from_=index, to_=index, record=index, - moreinfo=moreaction(res_model='export.integer')) + moreinfo=moreaction(res_model='export.integer'), field_name='Value', field_path=['value'], + field_type='name', value=id) for index, id in enumerate([record1.id, record2.id, record1.id])]) self.assertIs(result['ids'], False) @@ -662,7 +672,8 @@ class test_m2o(ImporterCase): self.assertEqual(result['messages'], [ message(u"Invalid database id 'foo' for the field 'Value'", moreinfo=moreaction(res_model='ir.model.data', - domain=[('model','=','export.integer')])) + domain=[('model', '=', 'export.integer')]), + field_name='Value', field_path=['value', '.id']) ]) self.assertIs(result['ids'], False) @@ -681,21 +692,25 @@ class test_m2o(ImporterCase): self.assertEqual(result['messages'], [message( u"No matching record found for name 'nameisnoexist:3' " u"in field 'Value'", moreinfo=moreaction( - res_model='export.integer'))]) + res_model='export.integer'), + field_name='Value', field_path=['value'], field_type='name', value='nameisnoexist:3' + )]) self.assertIs(result['ids'], False) result = self.import_(['value/id'], [['noxidhere']]) self.assertEqual(result['messages'], [message( u"No matching record found for external id 'noxidhere' " u"in field 'Value'", moreinfo=moreaction( - res_model='ir.model.data', domain=[('model','=','export.integer')]))]) + res_model='ir.model.data', domain=[('model', '=', 'export.integer')]), + field_name='Value', field_path=['value', 'id'], field_type="external id", value="noxidhere")]) self.assertIs(result['ids'], False) result = self.import_(['value/.id'], [['66']]) self.assertEqual(result['messages'], [message( u"No matching record found for database id '66' " u"in field 'Value'", moreinfo=moreaction( - res_model='ir.model.data', domain=[('model','=','export.integer')]))]) + res_model='ir.model.data', domain=[('model', '=', 'export.integer')]), + field_name='Value', field_path=['value', '.id'], field_type="database id", value="66")]) self.assertIs(result['ids'], False) def test_fail_multiple(self): @@ -712,7 +727,8 @@ class test_m2o(ImporterCase): self.assertEqual(result['messages'], [message( u"No matching record found for name '101' " u"in field 'Value'", moreinfo=moreaction( - res_model='export.integer'))]) + res_model='export.integer'), + field_name='Value', field_path=['value'], field_type='name', value=101)]) self.assertIs(result['ids'], False) context = { 'name_create_enabled_fields': {'value': True}, @@ -758,7 +774,8 @@ class test_m2m(ImporterCase): self.assertEqual(result['messages'], [message( u"No matching record found for database id '42' in field " u"'Value'", moreinfo=moreaction( - res_model='ir.model.data', domain=[('model','=','export.many2many.other')]))]) + res_model='ir.model.data', domain=[('model', '=', 'export.many2many.other')]), + field_name='Value', field_path=['value', '.id'], field_type="database id", value='42')]) self.assertIs(result['ids'], False) def test_xids(self): @@ -784,7 +801,8 @@ class test_m2m(ImporterCase): self.assertEqual(result['messages'], [message( u"No matching record found for external id 'noxidforthat' in field" u" 'Value'", moreinfo=moreaction( - res_model='ir.model.data', domain=[('model','=','export.many2many.other')]))]) + res_model='ir.model.data', domain=[('model', '=', 'export.many2many.other')]), + field_name='Value', field_path=['value', 'id'], field_type='external id', value='noxidforthat')]) self.assertIs(result['ids'], False) def test_names(self): @@ -812,7 +830,8 @@ class test_m2m(ImporterCase): self.assertEqual(result['messages'], [message( u"No matching record found for name 'wherethem2mhavenonames' in " u"field 'Value'", moreinfo=moreaction( - res_model='export.many2many.other'))]) + res_model='export.many2many.other'), + field_name='Value', field_path=['value'], field_type="name", value='wherethem2mhavenonames')]) self.assertIs(result['ids'], False) def test_import_to_existing(self): @@ -845,8 +864,9 @@ class test_o2m(ImporterCase): ['const', 'value'], [['5', s]]) self.assertEqual(result['messages'], [message( - u"No matching record found for name '%s' in field 'Value'" % s, - moreinfo=moreaction(res_model='export.one2many.child'))]) + u"No matching record found for name '%s' in field 'Value'" % s[:50], + moreinfo=moreaction(res_model='export.one2many.child'), + field_name='Value', field_path=['value'], field_type='name', value=s[:50])]) self.assertIs(result['ids'], False) def test_single(self): @@ -907,8 +927,8 @@ class test_o2m(ImporterCase): result = self.import_(['value/parent_id'], [['noxidforthat']]) self.assertEqual(result['messages'], [message( u"No matching record found for name 'noxidforthat' in field 'Value/Parent'", - moreinfo=moreaction(res_model='export.one2many') - )]) + moreinfo=moreaction(res_model='export.one2many'), + field_name='Value', field_path=['value', 'parent_id'], field_type='name', value='noxidforthat')]) self.assertIs(result['ids'], False) def test_link_inline(self): @@ -999,7 +1019,8 @@ class test_o2m(ImporterCase): self.assertEqual(result['messages'], [message( u"No matching record found for name '101' " u"in field 'Value/M2O'", moreinfo=moreaction( - res_model='export.integer'))]) + res_model='export.integer'), + field_name='Value', field_path=['value', 'm2o'], field_type='name', value=101)]) self.assertEqual(result['ids'], False) context = { 'name_create_enabled_fields': {'value/m2o': True}, @@ -1117,13 +1138,14 @@ class test_realworld(SavepointCaseWithUserDemo): def test_o2m_subfields_fail_by_implicit_id(self): self.env['ir.model.data'].clear_caches() Model = self.env['export.one2many.recursive'] - result = Model.load( + result = Model.with_context(import_file=True).load( ['child/child1/parent_id'], [['5'],], ) self.assertEqual(result['messages'], [message( u"No matching record found for name '5' in field 'Child/Child1/Parent'", field='child', - moreinfo=moreaction(res_model='export.one2many.multiple'))]) + moreinfo=moreaction(res_model='export.one2many.multiple'), + field_name='Child', field_path=['child', 'child1', 'parent_id'], field_type='name', value='5')]) self.assertIs(result['ids'], False) @@ -1145,7 +1167,7 @@ class test_date(ImporterCase): self.assertEqual(result['messages'], [ message(u"'not really a date' does not seem to be a valid date " u"for field 'Value'", - moreinfo=u"Use the format '2012-12-31'")]) + moreinfo=u"Use the format '2012-12-31'", field_name='Value', field_path=['value'])]) self.assertIs(result['ids'], False) @@ -1167,7 +1189,7 @@ class test_datetime(ImporterCase): self.assertEqual(result['messages'], [ message(u"'not really a datetime' does not seem to be a valid " u"datetime for field 'Value'", - moreinfo=u"Use the format '2012-12-31 23:59:59'")]) + moreinfo=u"Use the format '2012-12-31 23:59:59'", field_name='Value', field_path=['value'])]) self.assertIs(result['ids'], False) def test_checktz1(self): diff --git a/odoo/models.py b/odoo/models.py index 5487606f1c3..1f78c213b75 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -1230,10 +1230,16 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})): type = 'warning' if isinstance(exception, Warning) else 'error' # logs the logical (not human-readable) field name for automated # processing of response, but injects human readable in message - exc_vals = dict(base, record=record, field=field_names[field]) + field_name = field_names[field] + exc_vals = dict(base, record=record, field=field_name) record = dict(base, type=type, record=record, field=field, message=str(exception.args[0]) % exc_vals) - if len(exception.args) > 1 and exception.args[1]: + # ensure to add field_name to the exception. Used in import to concatenate multiple errors in the same block + if len(exception.args) > 1: + if not exception.args[1]: + exception.args = (exception.args[0], {'field_name': field_name}) + else: + exception.args[1]['field_name'] = field_name record.update(exception.args[1]) log(record)