From ac1e187eea33266095f49a9e1e1ea6b0698be865 Mon Sep 17 00:00:00 2001
From: Nicolas Lempereur <nle@odoo.com>
Date: Tue, 7 Aug 2018 17:19:57 +0200
Subject: [PATCH] [FIX] document: no query attachment out of form view

Since cce70fd4 the attachment menu is only displayed for form view.

But if in another view there is active_ids (eg. selecting record in a
list view), we will still make a query to ir.attachment without using
the result.

Without the code change, the added test would fail when doing a
search_read RPC call to ir.attachment model.

related to #26213
---
 addons/document/static/src/js/document.js     | 33 ++++++++++++-------
 .../document/static/tests/document_tests.js   | 33 +++++++++++++++++++
 2 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/addons/document/static/src/js/document.js b/addons/document/static/src/js/document.js
index 0af85179b49..98bfe4e5ef3 100644
--- a/addons/document/static/src/js/document.js
+++ b/addons/document/static/src/js/document.js
@@ -15,12 +15,13 @@ Sidebar.include({
      */
     init : function (parent, options) {
         this._super.apply(this, arguments);
-        if (options.viewType === "form") {
+        this.hasAttachments = options.viewType === "form";
+        if (this.hasAttachments) {
             this.sections.splice(1, 0, { 'name' : 'files', 'label' : _t('Attachment(s)'), });
             this.items.files = [];
+            this.fileuploadId = _.uniqueId('oe_fileupload');
+            $(window).on(this.fileuploadId, this._onFileUploaded.bind(this));
         }
-        this.fileuploadId = _.uniqueId('oe_fileupload');
-        $(window).on(this.fileuploadId, this._onFileUploaded.bind(this));
     },
     /**
      * Get the attachment linked to the record when the toolbar started
@@ -29,13 +30,16 @@ Sidebar.include({
      */
     start: function () {
         var _super = this._super.bind(this);
-        this._updateAttachments().then(_super);
+        var def = this.hasAttachments ? this._updateAttachments() : $.when();
+        return def.then(_super);
     },
     /**
      * @override
      */
     destroy: function () {
-        $(window).off(this.fileuploadId);
+        if (this.hasAttachments) {
+            $(window).off(this.fileuploadId);
+        }
         this._super.apply(this, arguments);
     },
 
@@ -46,8 +50,10 @@ Sidebar.include({
      * @override
      */
     updateEnv: function (env) {
-        this.env = env;
-        this._updateAttachments().then(this._redraw.bind(this));
+        if (this.hasAttachments) {
+            this.env = env;
+            this._updateAttachments().then(this._redraw.bind(this));
+        }
     },
 
     //--------------------------------------------------------------------------
@@ -90,10 +96,12 @@ Sidebar.include({
      */
     _redraw: function () {
         this._super.apply(this, arguments);
-        this.$('.o_sidebar_add_attachment .o_form_binary_form')
-            .change(this._onAddAttachment.bind(this));
-        this.$('.o_sidebar_delete_attachment')
-            .click(this._onDeleteAttachment.bind(this));
+        if (this.hasAttachments) {
+            this.$('.o_sidebar_add_attachment .o_form_binary_form')
+                .change(this._onAddAttachment.bind(this));
+            this.$('.o_sidebar_delete_attachment')
+                .click(this._onDeleteAttachment.bind(this));
+        }
     },
     /**
      * Update the attachments to be displayed in the attachment section
@@ -102,6 +110,9 @@ Sidebar.include({
      * @private
      */
     _updateAttachments: function () {
+        if (this.items.files === undefined) {
+            return $.when();
+        }
         var activeId = this.env.activeIds[0];
         if (!activeId) {
             this.items.files = [];
diff --git a/addons/document/static/tests/document_tests.js b/addons/document/static/tests/document_tests.js
index 9fd1e7d25ae..cc6f2668fc5 100644
--- a/addons/document/static/tests/document_tests.js
+++ b/addons/document/static/tests/document_tests.js
@@ -3,6 +3,7 @@ odoo.define('document.tests', function (require) {
 
     var testUtils = require('web.test_utils');
     var FormView = require('web.FormView');
+    var ListView = require('web.ListView');
 
     var createView = testUtils.createView;
 
@@ -16,6 +17,9 @@ odoo.define('document.tests', function (require) {
                     records: [{
                         id: 1,
                         display_name: "first record",
+                    }, {
+                        id: 2,
+                        display_name: "second record",
                     }]
                 },
                 'ir.attachment': {
@@ -74,5 +78,34 @@ odoo.define('document.tests', function (require) {
             assert.strictEqual(form.sidebar.$('.o_sidebar_delete_attachment').length, 1, "there should be only one attachment");
             form.destroy();
         });
+
+        QUnit.test('no attachment on list view', function (assert) {
+            assert.expect(4);
+
+            var list = createView({
+                View: ListView,
+                model: 'partner',
+                data: this.data,
+                groupBy: ['display_name'],
+                viewOptions: {sidebar: true},
+                arch: '<tree string="Partners">' +
+                        '<field name="display_name"/>' +
+                      '</tree>',
+                mockRPC: function (route, args) {
+                    assert.step(args.model);
+                    return this._super.apply(this, arguments);
+                }
+            });
+
+            // select record then trigger render
+            list.$('.o_group_header:last').click();
+            list.$('.o_data_row input').click();
+            list.$('.o_group_header:first').click();
+
+            assert.verifySteps(['partner', 'partner', 'partner'],
+                "ir.attachment not called when selecting record in list view");
+
+            list.destroy();
+        });
     });
 });