[IMP] base_import: Improve import general UX (back2basics)

Purpose ======= Improve error management UX on import. TaskID: 2049992 closes odoo/odoo#35804 Signed-off-by: Yannick Tivisse (yti) <yti@odoo.com>
2019-08-20 12:23:08 +00:00
parent e8544f2f0f
commit 58b3557d6d
9 changed files with 123 additions and 44 deletions
@@ -1,9 +1,9 @@
-Name,Is a company,Related company,Address type,Customer,Supplier,Street,ZIP,City,State,Country
-Aurora Shelves,1,,,1,0,25 Pacific Road,95101,San José,California,United States
-Roger Martins,0,Aurora Shelves,Invoice address,1,0,27 Pacific Road,95102,San José,California,United States
-House Sales Direct,1,,,1,0,104 Saint Mary Avenue,94059,Redwood,California,United States
-Yvan Holiday,0,House Sales Direct,Contact,1,0,104 Saint Mary Avenue,94060,Redwood,California,United States
-Jack Unsworth,0,House Sales Direct,Invoice address,1,0,227 Jackson Road,94061,Redwood,California,United States
-Michael Mason,0,,,1,0,16 5th Avenue,94104,San Francisco,California,United States
-International Wood,1,,,1,0,748 White House Boulevard,20004,Washington,D.C.,United States
-Sharon Pecker,0,International Wood,Invoice address,1,0,755 White House Boulevard,20005,Washington,D.C.,United States
+Name,Is a company,Related company,Address type,Street,ZIP,City,State,Country
+Aurora Shelves,1,,,25 Pacific Road,95101,San José,California,United States
+Roger Martins,0,Aurora Shelves,Invoice address,27 Pacific Road,95102,San José,California,United States
+House Sales Direct,1,,,104 Saint Mary Avenue,94059,Redwood,California,United States
+Yvan Holiday,0,House Sales Direct,Contact,104 Saint Mary Avenue,94060,Redwood,California,United States
+Jack Unsworth,0,House Sales Direct,Invoice address,227 Jackson Road,94061,Redwood,California,United States
+Michael Mason,0,,,16 5th Avenue,94104,San Francisco,California,United States
+International Wood,1,,,748 White House Boulevard,20004,Washington,D.C.,United States
+Sharon Pecker,0,International Wood,Invoice address,755 White House Boulevard,20005,Washington,D.C.,United States
@@ -102,7 +102,7 @@ var DataImport = AbstractAction.extend({
        },
        'click .oe_import_report a.oe_import_report_count': function (e) {
            e.preventDefault();
-            $(e.target).parent().toggleClass('oe_import_report_showmore');
+            $(e.target).parent().parent().toggleClass('oe_import_report_showmore');
        },
        'click .oe_import_moreinfo_action a': function (e) {
            e.preventDefault();
@@ -140,6 +140,7 @@ var DataImport = AbstractAction.extend({
        this.session = session;
        this._title = _t('Import a File'); // Displayed in the breadcrumbs
        this.do_not_change_match = false;
+        this.preview = [[]];
    },
    /**
     * @override
@@ -374,6 +375,8 @@ var DataImport = AbstractAction.extend({
        this.$('input.oe_import_advanced_mode').prop('checked', result.advanced_mode);
        this.$('.oe_import_grid').html(QWeb.render('ImportView.preview', result));

+        this.preview = result.preview;
+
        if (result.headers.length === 1) {
            this.$('.oe_import_options').show();
            this.onresults(null, null, null, {'messages': [{
@@ -625,6 +628,12 @@ var DataImport = AbstractAction.extend({
        this.trigger_up('history_back');
    },
    onresults: function (event, from, to, results) {
+        var self = this;
+        var fields = this.$('.oe_import_fields input.oe_import_match_field').map(function (index, el) {
+            return $(el).select2('val') || false;
+        }).get();
+        var nameIndex = _.indexOf(fields, 'name');
+
        var message = results.messages;
        var no_messages = _.isEmpty(message);
        if (no_messages) {
@@ -639,19 +648,48 @@ var DataImport = AbstractAction.extend({
        // offset more if header
        if (this.import_options().headers) { offset += 1; }

+        var messagesSorted = _.sortBy(_(message).groupBy('message'), function (messageGroupped) {
+            var order = 0;
+            if (messageGroupped[0].type === 'warning') {
+                order = fields.length + 1;
+            }
+            return order + _.indexOf(fields, messageGroupped[0].field);
+        });
+
        this.$form.addClass('oe_import_error');
        this.$('.oe_import_error_report').html(
            QWeb.render('ImportView.error', {
-                errors: _(message).groupBy('message'),
+                errors: messagesSorted,
                at: function (rows) {
                    var from = rows.from + offset;
                    var to = rows.to + offset;
+                    var rowName = '';
+                    if (nameIndex !== -1) {
+                        rowName = _.str.sprintf(' (%s)', self.preview[rows.from][nameIndex]);
+                    }
                    if (from === to) {
-                        return _.str.sprintf(_t("at row %d"), from);
+                        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 (nameIndex !== -1) {
+                        rowName = _.str.sprintf(' (%s)', self.preview[rows.from][nameIndex]);
+                    }
+                    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);
                },
@@ -663,8 +701,8 @@ var DataImport = AbstractAction.extend({
                    }
                    if (msg instanceof Array) {
                        return _.str.sprintf(
-                            '<div class="oe_import_moreinfo oe_import_moreinfo_choices">%s <ul>%s</ul></div>',
-                            _.str.escapeHTML(_t("Here are the possible values:")),
+                            '<div class="oe_import_moreinfo oe_import_moreinfo_choices"><a href="#" class="oe_import_report_count oe_import_see_all"><i class="fa fa-arrow-right"/> %s </a><ul class="oe_import_report_more">%s</ul></div>',
+                            _.str.escapeHTML(_t("See possible values")),
                            _(msg).map(function (msg) {
                                return '<li>'
                                    + _.str.escapeHTML(msg)
@@ -674,10 +712,10 @@ var DataImport = AbstractAction.extend({
                    // Final should be object, action descriptor
                    return [
                        '<div class="oe_import_moreinfo oe_import_moreinfo_action">',
-                            _.str.sprintf('<a href="#" data-action="%s">',
+                            _.str.sprintf('<a href="#" data-action="%s" class="oe_import_see_all"><i class="fa fa-arrow-right"/> ',
                                    _.str.escapeHTML(JSON.stringify(msg))),
                                _.str.escapeHTML(
-                                    _t("Get all possible values")),
+                                    _t("See possible values")),
                            '</a>',
                        '</div>'
                    ].join('');
@@ -75,7 +75,7 @@
    &.oe_import_error .oe_import_error_report,
    &.oe_import_noheaders .oe_import_noheaders,
    .oe_import_report_showmore .oe_import_report_more {
-        display: block;
+        display: list-item;
    }

    /* ------------- ERRORS AND WARNINGS REPORT ------------ */
@@ -95,6 +95,12 @@
                @extend .text-danger;
            }
        }
+        .oe_import_report_count, .oe_import_see_all {
+            color: white;
+            &:hover {
+                color: lightgrey;
+            }
+        }
    }

    /* ------------- THE CSV TABLE ------------ */
@@ -18,6 +18,7 @@
                                        <t t-esc="option.label"/></label>
                                    <input t-attf-id="#{option.name}_#{_id}"
                                           t-attf-class="oe_import_#{option.name}"
+                                           style="max-width: 50%;"
                                           t-att-value="option.value"/>
                                </p>
                            </div>
@@ -28,6 +29,7 @@
                                        <t t-esc="option.label"/></label>
                                    <input t-attf-id="#{option.name}_#{_id}"
                                           t-attf-class="oe_import_#{option.name}"
+                                           style="max-width: 50%;"
                                           t-att-value="option.value"/>
                                </p>
                            </div>
@@ -140,23 +142,48 @@
    <ul t-name="ImportView.error">
        <li t-foreach="errors" t-as="error"
            t-attf-class="oe_import_report bg-#{error_value[0].type}">
-            <t t-call="ImportView.error.each">
-                <t t-set="error" t-value="error_value[0]"/>
-            </t>
-
-            <a href="#" class="oe_import_report_count" t-if="error_value.length gt 1">
-                <t t-esc="more(error_value.length - 1)"/>
-            </a>
-            <ul class="oe_import_report_more" t-if="error_value.length gt 1">
-                <li t-foreach="error_value.length - 1" t-as="index">
-                    <t t-call="ImportView.error.each">
-                        <t t-set="error" t-value="error_value[index + 1]"/>
+            <t t-if="error_value.length gt 1">
+                <t t-call="ImportView.error.multi.header">
+                    <t t-set="error" t-value="error_value[0]"/>
+                    <t t-set="error_length" t-value="error_value.length"/>
+                </t>
+                <ul>
+                    <t t-foreach="error_value.length" t-as="index">
+                        <li t-att-class="index gt 4 ? 'oe_import_report_more':''">
+                            <t t-call="ImportView.error.multi.body">
+                                <t t-set="error" t-value="error_value[index]"/>
+                                <t t-set="index" t-value="index"/>
+                            </t>
+                        </li>
+                        <li t-if="error_value.length gt 5 and index == 4" style="display: block;">
+                            <a href="#" class="oe_import_report_count">
+                                <t t-esc="more(error_value.length - 5)"/>
+                            </a>
+                        </li>
                    </t>
-                </li>
-            </ul>
+                </ul>
+                <t t-if="error_value[0].moreinfo" t-raw="info(error_value[0].moreinfo)"/>
+
+            </t>
+            <t t-else="">
+                <t t-call="ImportView.error.single">
+                    <t t-set="error" t-value="error_value[0]"/>
+                </t>
+            </t>
        </li>
    </ul>
-    <t t-name="ImportView.error.each">
+    <t t-name="ImportView.error.multi.header">
+        <span class="oe_import_report_message">
+            <t t-esc="error.message"/>
+            <t t-esc="at_multi_header(error_length)"/>
+        </span>
+    </t>
+    <t t-name="ImportView.error.multi.body">
+        <span class="oe_import_report_message">
+            <t t-esc="at_multi(error.rows)"/>
+        </span>
+    </t>
+    <t t-name="ImportView.error.single">
        <span class="oe_import_report_message">
            <t t-esc="error.message"/>
        </span>
@@ -305,10 +305,13 @@ class IrFieldsConverter(models.AbstractModel):
        id = None
        warnings = []
        error_msg = ''
-        action = {'type': 'ir.actions.act_window', 'target': 'new',
-                  'view_mode': 'tree,form',
-                  'views': [(False, 'tree'), (False, 'form')],
-                  'help': _(u"See all possible values")}
+        action = {
+            'name': 'Possible Values',
+            'type': 'ir.actions.act_window', 'target': 'new',
+            'view_mode': 'tree,form',
+            'views': [(False, 'list'), (False, 'form')],
+            'context': {'create': False},
+            'help': _(u"See all possible values")}
        if subfield is None:
            action['res_model'] = field.comodel_name
        elif subfield in ('id', '.id'):
@@ -353,8 +356,8 @@ class IrFieldsConverter(models.AbstractModel):
                if name_create_enabled_fields.get(field.name):
                    try:
                        id, _name = RelatedModel.name_create(name=value)
-                    except Exception as e:
-                        error_msg = repr(e)
+                    except (Exception, psycopg2.IntegrityError):
+                        error_msg = _(u"Cannot create new '%s' records from their name alone. Please create those records manually and try importing again.") % RelatedModel._description
        else:
            raise self._format_import_error(
                Exception,
@@ -16,11 +16,7 @@ AAAAAAAAAAAAA/9oACAEBAAEFAn//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDAQE/AX//xAAUEQE
 AA/9oACAECAQE/AX//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBAAY/An//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBA
 AE/IX//2gAMAwEAAgADAAAAEB//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDAQE/EH//xAAUEQEAAAAAAAAAAAAAAAAAAAAA
 /9oACAECAQE/EH//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBAAE/EH//2Q=="""
-SVG = b"""PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iaXNvLTg4NTktMSI/Pgo8IURPQ1RZUEUgc3ZnIFBVQkxJQyAiL
-S8vVzNDLy9EVEQgU1ZHIDIwMDAxMTAyLy9FTiIKICJodHRwOi8vd3d3LnczLm9yZy9UUi8yMDAwL0NSLVNWRy0yMDAwMTEwMi9E
-VEQvc3ZnLTIwMDAxMTAyLmR0ZCI+Cgo8c3ZnIHdpZHRoPSIxMDAlIiBoZWlnaHQ9IjEwMCUiPgogIDxnIHRyYW5zZm9ybT0idHJ
-hbnNsYXRlKDUwLDUwKSI+CiAgICA8cmVjdCB4PSIwIiB5PSIwIiB3aWR0aD0iMTUwIiBoZWlnaHQ9IjUwIiBzdHlsZT0iZmlsbD
-pyZWQ7IiAvPgogIDwvZz4KCjwvc3ZnPgo="""
+SVG = b"""PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iaXNvLTg4NTktMSI/PjwhRE9DVFlQRSBzdmcgUFVCTElDICItLy9XM0MvL0RURCBTVkcgMjAwMDExMDIvL0VOIlxuICJodHRwOi8vd3d3LnczLm9yZy9UUi8yMDAwL0NSLVNWRy0yMDAwMTEwMi9EVEQvc3ZnLTIwMDAxMTAyLmR0ZCI+PHN2ZyB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIj48ZyB0cmFuc2Zvcm09InRyYW5zbGF0ZSg1MCw1MCkiPjxyZWN0IHg9IjAiIHk9IjAiIHdpZHRoPSIxNTAiIGhlaWdodD0iNTAiIHN0eWxlPSJmaWxsOnJlZDsiIC8+PC9nPjwvc3ZnPg=="""


 class test_guess_mimetype(BaseCase):
@@ -18,8 +18,10 @@ def moreaction(**kwargs):
    return dict(kwargs,
        type='ir.actions.act_window',
        target='new',
+        context={'create': False},
+        name='Possible Values',
        view_mode='tree,form',
-        views=[(False, 'tree'), (False, 'form')],
+        views=[(False, 'list'), (False, 'form')],
        help=u"See all possible values")

 def values(seq, field='value'):
@@ -5916,7 +5916,7 @@ def itemgetter_tuple(items):

 def convert_pgerror_not_null(model, fields, info, e):
    if e.diag.table_name != model._table:
-        return {'message': tools.ustr(e)}
+        return {'message': _(u"Missing required value for the field '%s'") % (e.diag.column_name)}

    field_name = e.diag.column_name
    field = fields[field_name]
@@ -5964,11 +5964,18 @@ def convert_pgerror_unique(model, fields, info, e):
        # no field, unclear which one we should pick and they could be in any order
    }

+def convert_pgerror_constraint(model, fields, info, e):
+    sql_constraints = dict([(('%s_%s') % (e.diag.table_name, x[0]), x) for x in model._sql_constraints])
+    if e.diag.constraint_name in sql_constraints.keys():
+        return {'message': sql_constraints[e.diag.constraint_name][2]}
+    return {'message': tools.ustr(e)}
+
 PGERROR_TO_OE = defaultdict(
    # shape of mapped converters
    lambda: (lambda model, fvg, info, pgerror: {'message': tools.ustr(pgerror)}), {
    '23502': convert_pgerror_not_null,
    '23505': convert_pgerror_unique,
+    '23514': convert_pgerror_constraint,
 })