[IMP] base_import: reintroduce encoding selector & override guess

* if an encoding is explicitly specified, use it and don't guess
* otherwise guess and return the guessed encoding so it can be
  displayed in the configuration UI
* fix less-than-stellar configuration & behaviour of select2 inputs
  to properly reflect underlying values as they get modified, to
  correctly handle future configuration guesses

addons/base_import/models/base_import.py

@@ -2,6 +2,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 import base64
+import codecs
 import collections
 import unicodedata
 
@@ -311,8 +312,12 @@ class Import(models.TransientModel):
             :throws csv.Error: if an error is detected during CSV parsing
         """
         csv_data = self.file
-        encoding = chardet.detect(csv_data)['encoding']
-        csv_data = csv_data.decode(encoding).encode('utf-8')
+        encoding = options.get('encoding')
+        if not encoding:
+            encoding = options['encoding'] = chardet.detect(csv_data)['encoding'].lower()
+
+        if encoding != 'utf-8':
+            csv_data = csv_data.decode(encoding).encode('utf-8')
 
         csv_iterator = pycompat.csv_reader(
             io.BytesIO(csv_data),

addons/base_import/models/test_models.py

@@ -81,3 +81,13 @@ class FloatModel(models.Model):
     value = fields.Float()
     value2 = fields.Monetary()
     currency_id = fields.Many2one('res.currency')
+
+class ComplexModel(models.Model):
+    _name = name('complex')
+
+    f = fields.Float()
+    m = fields.Monetary()
+    c = fields.Char()
+    currency_id = fields.Many2one('res.currency')
+    d = fields.Date()
+    dt = fields.Datetime()

addons/base_import/static/src/js/import_action.js

@@ -43,10 +43,39 @@ function jsonp(form, attributes, callback) {
     });
     $(form).ajaxSubmit(attributes);
 }
+function _make_option(term) { return {id: term, text: term }; }
+function _from_data(data, term) {
+    return _.findWhere(data, {id: term}) || _make_option(term);
+}
+
+/**
+ * query returns a list of suggestion select2 objects, this function:
+ *
+ * * returns data exactly matching query by either id or text if those exist
+ * * otherwise it returns a select2 option matching the term and any data
+ *   option whose id or text matches (by substring)
+ */
+function dataFilteredQuery(q) {
+    var suggestions = _.clone(this.data);
+    if (q.term) {
+        var exact = _.filter(suggestions, function (s) {
+            return s.id === q.term || s.text === q.term;
+        });
+        if (exact.length) {
+            suggestions = exact;
+        } else {
+            suggestions = [_make_option(q.term)].concat(_.filter(suggestions, function (s) {
+                return s.id.indexOf(q.term) !== -1 || s.text.indexOf(q.term) !== -1
+            }));
+        }
+    }
+    q.callback({results: suggestions});
+}
 
 var DataImport = AbstractAction.extend(ControlPanelMixin, {
     template: 'ImportView',
     opts: [
+        {name: 'encoding', label: _lt("Encoding:"), value: ''},
         {name: 'separator', label: _lt("Separator:"), value: ','},
         {name: 'quoting', label: _lt("Text Delimiter:"), value: '"'}
     ],
@@ -124,6 +153,7 @@ var DataImport = AbstractAction.extend(ControlPanelMixin, {
     },
     start: function () {
         var self = this;
+        this.setup_encoding_picker();
         this.setup_separator_picker();
         this.setup_float_format_picker();
 
@@ -163,57 +193,54 @@ var DataImport = AbstractAction.extend(ControlPanelMixin, {
             self.exit();
         });
     },
+    setup_encoding_picker: function () {
+        this.$('input.oe_import_encoding').select2({
+            width: '160px',
+            data: _.map(('utf-8 utf-16 windows-1252 latin1 latin2 big5 gb18030 shift_jis windows-1251 koir8_r').split(/\s+/), _make_option),
+            query: dataFilteredQuery,
+            initSelection: function ($e, c) {
+                return c(_make_option($e.val()));
+            }
+        });
+    },
     setup_separator_picker: function () {
+        var data = [
+            {id: ',', text: _t("Comma")},
+            {id: ';', text: _t("Semicolon")},
+            {id: '\t', text: _t("Tab")},
+            {id: ' ', text: _t("Space")}
+        ];
         this.$('input.oe_import_separator').select2({
             width: '160px',
-            query: function (q) {
-                var suggestions = [
-                    {id: ',', text: _t("Comma")},
-                    {id: ';', text: _t("Semicolon")},
-                    {id: '\t', text: _t("Tab")},
-                    {id: ' ', text: _t("Space")}
-                ];
-                if (q.term) {
-                    suggestions.unshift({id: q.term, text: q.term});
-                }
-                q.callback({results: suggestions});
-            },
-            initSelection: function (e, c) {
-                return c({id: ',', text: _t("Comma")});
-            },
+            data: data,
+            query: dataFilteredQuery,
+            // 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()))
+            }
         });
     },
     setup_float_format_picker: function () {
-        var sub_query = function (q, options) {
-            var suggestions = [
-                {id: ',', text: _t("Comma")},
-                {id: '.', text: _t("Dot")},
-            ];
-            if (options && options.no_separator) {
-                suggestions.push({id: '', text: _t("No Separator")});
-            }
-            if (q.term) {
-                suggestions.unshift({id: q.term, text: q.term});
-            }
-            q.callback({results: suggestions});
-        };
+        var data_decimal = [
+            {id: ',', text: _t("Comma")},
+            {id: '.', text: _t("Dot")},
+        ];
+        var data_digits = data_decimal.concat([{id: '', text: _t("No Separator")}]);
         this.$('input.oe_import_float_thousand_separator').select2({
             width: '160px',
-            query: function (q) {
-                sub_query(q, {no_separator: true})
-            },
-            initSelection: function (e, c) {
-                return c({id: ',', text: _t("Comma")});
-            },
+            data: data_digits,
+            query: dataFilteredQuery,
+            initSelection: function ($e, c) {
+                c(_from_data(data_digits, $e.val()) || _make_option($e.val()))
+            }
         });
         this.$('input.oe_import_float_decimal_separator').select2({
             width: '160px',
-            query: function (q) {
-                sub_query(q);
-            },
-            initSelection: function (e, c) {
-                return c({id: ',', text: _t("Dot")});
-            },
+            data: data_decimal,
+            query: dataFilteredQuery,
+            initSelection: function ($e, c) {
+                c(_from_data(data_decimal, $e.val()) || _make_option($e.val()))
+            }
         });
     },
 
@@ -248,7 +275,6 @@ var DataImport = AbstractAction.extend(ControlPanelMixin, {
 
     //- File & settings change section
     onfile_loaded: function () {
-        var file = this.$('.oe_import_file')[0].files[0];
         this.$buttons.filter('.o_import_import, .o_import_validate, .o_import_file_reload').addClass('d-none');
         if (!this.$('input.oe_import_file').val()) { return this['settings_changed'](); }
         this.$('.oe_import_date_format').val('');
@@ -315,10 +341,12 @@ var DataImport = AbstractAction.extend(ControlPanelMixin, {
             }]});
         }
 
-        this.$('.oe_import_date_format').val(time.strftime_to_moment_format(result.options.date_format));
+        // merge option values back in case they were updated/guessed
+        _.each(['encoding', 'separator', 'float_thousand_separator', 'float_decimal_separator'], function (id) {
+            self.$('.oe_import_' + id).select2('val', result.options[id])
+        });
+        this.$('.oe_import_date_format').val(time.strftime_to_moment_format(result.options.date_format)).change();
         this.$('.oe_import_datetime_format').val(time.strftime_to_moment_format(result.options.datetime_format));
-        this.$('.oe_import_float_thousand_separator').val(result.options.float_thousand_separator).change();
-        this.$('.oe_import_float_decimal_separator').val(result.options.float_decimal_separator).change();
         if (result.debug === false){
             this.$('.oe_import_tracking').hide();
             this.$('.oe_import_deferparentstore').hide();

addons/base_import/tests/__init__.py

@@ -2,3 +2,4 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from . import test_base_import
+from . import test_csv_magic

addons/base_import/tests/test_base_import.py

@@ -452,23 +452,6 @@ class test_convert_import_data(TransactionCase):
         self.assertEqual(float(result[0][-1]), 5.69)
         self.assertEqual(str(result[0][-2]), '2017-10-12')
 
-    def test_parse_float(self):
-        w = self.env['base_import.import'].create({
-            'res_model': 'base_import.tests.models.float',
-        })
-        data = w._parse_import_data(
-            [
-                ['1.62'], ['-1.62'], ['+1.62'], ['  +1.62  '], ['(1.62)'],
-                ["1'234'567,89"], ["1.234.567'89"]
-            ],
-            ['value'], {}
-        )
-        self.assertEqual(
-            [d[0] for d in data],
-            ['1.62', '-1.62', '+1.62', '+1.62', '-1.62',
-             '1234567.89', '1234567.89']
-        )
-
     def test_filtered(self):
         """ If ``False`` is provided as field mapping for a column,
         that column should be removed from importable data

addons/base_import/tests/test_csv_magic.py

@@ -0,0 +1,74 @@
+# -*- coding: utf-8 -*-
+"""
+Tests for various autodetection magics for CSV imports
+"""
+import codecs
+
+from odoo.tests import common
+
+class TestEncoding(common.TransactionCase):
+    """
+    create + parse_preview -> check result options
+    """
+
+    def _check_text(self, text, encodings, **options):
+        options.setdefault('quoting', '"')
+        options.setdefault('separator', '\t')
+        test_text = "text\tnumber\tdate\tdatetime\n%s\t1.23.45,67\t\t\n" % text
+        for encoding in ['utf-8', 'utf-16', 'utf-32', *encodings]:
+            preview = self._make_import(
+                test_text.encode(encoding)).parse_preview(dict(options))
+
+            guessed = preview['options']['encoding']
+            self.assertIsNotNone(guessed, encoding)
+            self.assertEqual(codecs.lookup(guessed).name, codecs.lookup(encoding).name)
+
+    def test_autodetect_encoding(self):
+        """ Check that import preview can detect & return encoding
+        """
+        self._check_text("Iñtërnâtiônàlizætiøn", ['iso-8859-1'])
+
+        self._check_text("やぶら小路の藪柑子。海砂利水魚の、食う寝る処に住む処、パイポパイポ パイポのシューリンガン。", ['eucjp', 'shift_jis', 'iso2022_jp'])
+
+        self._check_text("대통령은 제4항과 제5항의 규정에 의하여 확정된 법률을 지체없이 공포하여야 한다, 탄핵의 결정.", ['euc_kr', 'iso2022_kr'])
+
+    # + control in widget
+    def test_override_detection(self):
+        """ ensure an explicitly specified encoding is not overridden by the
+        auto-detection
+        """
+        s = "Iñtërnâtiônàlizætiøn".encode('utf-8')
+        r = self._make_import(b'text\n' + s)\
+            .parse_preview({
+            'quoting': '"',
+            'separator': '\t',
+            'encoding': 'iso-8859-1',
+        })
+        self.assertEqual(r['options']['encoding'], 'iso-8859-1')
+        self.assertEqual(r['preview'], [['text'], [s.decode('iso-8859-1')]])
+
+    def _make_import(self, contents):
+        return self.env['base_import.import'].create({
+            'res_model': 'base_import.tests.models.complex',
+            'file_name': 'f',
+            'file_type': 'text/csv',
+            'file': contents,
+        })
+
+class TestNumberSeparators(common.TransactionCase):
+    def test_parse_float(self):
+        w = self.env['base_import.import'].create({
+            'res_model': 'base_import.tests.models.float',
+        })
+        data = w._parse_import_data(
+            [
+                ['1.62'], ['-1.62'], ['+1.62'], ['  +1.62  '], ['(1.62)'],
+                ["1'234'567,89"], ["1.234.567'89"]
+            ],
+            ['value'], {}
+        )
+        self.assertEqual(
+            [d[0] for d in data],
+            ['1.62', '-1.62', '+1.62', '+1.62', '-1.62',
+             '1234567.89', '1234567.89']
+        )