From 0eccd3973e056b4ac6fcd6754a8b6af2818aac4f Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Fri, 1 Oct 2021 10:25:08 +0000
Subject: [PATCH] [FIX] web: more reliably avoid downloads on new file

519555054a3a0e2e0b8a139518da081c4d5e80aa mitigated an issue of being
able to try and download files which don't exist yet, make the fix
more reliable by clearing out the field completely and hiding the
content if the (readonly) field has no value *or the record is not
saved yet*.

Also clean up the code:

* an old-style forward port created a duplicate fixprovement
  (a8d01cbf4ea88f894ff927a6a5424bbb58e630d0) which seems less correct
  as it applies conditionally
* and the code is branchier than necessary, we can make it simpler by
  judiciously leveraging jquery's API

closes odoo/odoo#77603

Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
---
 .../web/static/src/js/fields/basic_fields.js  | 23 ++++++-------------
 .../static/tests/fields/basic_fields_tests.js |  9 +++++---
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js
index 97b1ee36a9e7..1469ae514bac 100644
--- a/addons/web/static/src/js/fields/basic_fields.js
+++ b/addons/web/static/src/js/fields/basic_fields.js
@@ -1295,22 +1295,13 @@ var FieldBinaryFile = AbstractFieldBinary.extend({
         this.filename_value = this.recordData[this.attrs.filename];
     },
     _renderReadonly: function () {
-        this.do_toggle(!!this.value);
-        if (this.value) {
-            this.$el.empty().append($("<span/>").addClass('fa fa-download'));
-            if (this.recordData.id) {
-                this.$el.css('cursor', 'pointer');
-            } else {
-                this.$el.css('cursor', 'not-allowed');
-            }
-            if (this.filename_value) {
-                this.$el.append(" " + this.filename_value);
-            }
-        }
-        if (!this.res_id) {
-            this.$el.css('cursor', 'not-allowed');
-        } else {
-            this.$el.css('cursor', 'pointer');
+        var visible = !!(this.value && this.res_id);
+        this.$el.empty().css('cursor', 'not-allowed');
+        this.do_toggle(visible);
+        if (visible) {
+            this.$el.css('cursor', 'pointer')
+                    .text(this.filename_value || '')
+                    .prepend($('<span class="fa fa-download"/>'), ' ');
         }
     },
     _renderEdit: function () {
diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js
index c0ba0353c491..40186334db5b 100644
--- a/addons/web/static/tests/fields/basic_fields_tests.js
+++ b/addons/web/static/tests/fields/basic_fields_tests.js
@@ -1512,7 +1512,7 @@ QUnit.module('basic_fields', {
     });
 
     QUnit.test('binary fields that are readonly in create mode do not download', function (assert) {
-        assert.expect(2);
+        assert.expect(3);
 
         // save the session function
         var oldGetFile = session.get_file;
@@ -1546,10 +1546,13 @@ QUnit.module('basic_fields', {
         form.$('.o_field_many2one input').click();
         $dropdown.find('li:not(.o_m2o_dropdown_option):contains(xphone)').click();
 
-        assert.strictEqual(form.$('a.o_field_widget[name="document"] > .fa-download').length, 1,
+        var $field = form.$('a.o_field_widget[name="document"]');
+        assert.strictEqual($field.length, 1,
             'The link to download the binary should be present');
+        assert.strictEqual($field.find('.fa-download').length, 0,
+                           'the download icon should not be present');
 
-        form.$('a.o_field_widget[name="document"]').click();
+        $field.click();
 
         assert.verifySteps([]); // We shoudln't have passed through steps
 
-- 
GitLab