From d3eff197a597671a5b0f41238d8cd8b2fc32cd4e Mon Sep 17 00:00:00 2001
From: Julien Mougenot <jum@odoo.com>
Date: Thu, 4 Aug 2022 15:17:03 +0000
Subject: [PATCH] [FIX] web: Fix binary and pdf_viewer fields

This commit aims to make the BinaryField and PdfViewerField appear and
act the same way as they did before their conversion to Owl.

This mostly consists of a few tweaks in the conditional rendering of
certain elements in the template, and the rest of the changes are meant
to clean and simplify the values the component is working with.

closes odoo/odoo#98251

Signed-off-by: Bruno Boi (boi) <boi@odoo.com>
---
 .../src/views/fields/binary/binary_field.js   | 47 +++--------
 .../src/views/fields/binary/binary_field.xml  | 52 ++++++------
 .../static/src/views/fields/file_handler.js   |  1 +
 .../static/src/views/fields/file_handler.xml  | 24 +++---
 .../fields/pdf_viewer/pdf_viewer_field.js     | 66 ++++++++--------
 .../fields/pdf_viewer/pdf_viewer_field.xml    | 57 ++++++-------
 .../tests/views/fields/binary_field_tests.js  | 79 +++++++++++--------
 .../views/fields/pdf_viewer_field_tests.js    | 52 ++++++------
 8 files changed, 186 insertions(+), 192 deletions(-)

diff --git a/addons/web/static/src/views/fields/binary/binary_field.js b/addons/web/static/src/views/fields/binary/binary_field.js
index aaa34ff561e..fe537bc72c5 100644
--- a/addons/web/static/src/views/fields/binary/binary_field.js
+++ b/addons/web/static/src/views/fields/binary/binary_field.js
@@ -17,35 +17,25 @@ export class BinaryField extends Component {
     setup() {
         this.notification = useService("notification");
         this.state = useState({
-            fileName: this.fileName || "",
-            isValid: true,
+            fileName: this.props.record.data[this.props.fileNameField] || "",
         });
         onWillUpdateProps((nextProps) => {
             if (nextProps.readonly) {
-                this.state.fileName = "";
+                this.state.fileName = nextProps.record.data[nextProps.fileNameField] || "";
             }
         });
     }
 
     get fileName() {
-        return this.props.record.data[this.props.fileNameField];
-    }
-    get file() {
-        return {
-            data: this.props.value,
-            name: this.state.fileName || this.props.value || null,
-        };
-    }
-    get isDownloadable() {
-        return !(
-            this.props.record.isReadonly(this.props.name) && this.props.record.mode === "edit"
-        );
+        return this.state.fileName || this.props.value || "";
     }
 
-    update(file) {
-        const changes = { [this.props.name]: file.data || false };
-        if (this.props.fileNameField && this.props.fileNameField !== this.props.name) {
-            changes[this.props.fileNameField] = file.name || false;
+    update({ data, name }) {
+        this.state.fileName = name || "";
+        const { fileNameField, record } = this.props;
+        const changes = { [this.props.name]: data || false };
+        if (fileNameField in record.fields && record.data[fileNameField] !== name) {
+            changes[fileNameField] = name || false;
         }
         return this.props.record.update(changes);
     }
@@ -56,29 +46,14 @@ export class BinaryField extends Component {
                 model: this.props.record.resModel,
                 id: this.props.record.resId,
                 field: this.props.name,
-                filename_field: this.file.name,
-                filename: this.file.name || "",
+                filename_field: this.fileName,
+                filename: this.fileName || "",
                 download: true,
                 data: isBinarySize(this.props.value) ? null : this.props.value,
             },
             url: "/web/content",
         });
     }
-    onFileRemove() {
-        this.state.isValid = true;
-        this.update(false);
-    }
-    onFileUploaded(file) {
-        this.state.fileName = file.name;
-        this.state.isValid = true;
-        this.update(file);
-    }
-    onLoadFailed() {
-        this.state.isValid = false;
-        this.notification.add(this.env._t("Could not display the selected binary"), {
-            type: "danger",
-        });
-    }
 }
 
 BinaryField.template = "web.BinaryField";
diff --git a/addons/web/static/src/views/fields/binary/binary_field.xml b/addons/web/static/src/views/fields/binary/binary_field.xml
index 8c9a9475a7c..0dc046d9de6 100644
--- a/addons/web/static/src/views/fields/binary/binary_field.xml
+++ b/addons/web/static/src/views/fields/binary/binary_field.xml
@@ -3,45 +3,47 @@
 
     <t t-name="web.BinaryField" owl="1">
         <t t-if="!props.readonly">
-            <t t-if="file.data">
-                <FileUploader
-                    acceptedFileExtensions="props.acceptedFileExtensions"
-                    file="file"
-                    onUploaded.bind="onFileUploaded"
-                >
-                    <t t-set-slot="toggler">
-                        <span class="mx-2" t-esc="file.name"/>
+            <t t-if="props.value">
+                <div class="w-100 d-inline-flex">
+                    <FileUploader
+                        acceptedFileExtensions="props.acceptedFileExtensions"
+                        file="{ data: props.value, name: fileName }"
+                        onUploaded.bind="update"
+                    >
+                        <t t-set-slot="toggler">
+                            <input type="text" class="o_input" t-att-value="fileName" readonly="readonly" />
+                            <button
+                                class="btn btn-secondary fa fa-pencil o_select_file_button"
+                                data-tooltip="Edit"
+                                aria-label="Edit"
+                            />
+                        </t>
                         <button
-                            class="btn btn-secondary fa fa-pencil o_select_file_button"
-                            data-tooltip="Edit"
-                            aria-label="Edit"
+                            class="btn btn-secondary fa fa-trash o_clear_file_button"
+                            data-tooltip="Clear"
+                            aria-label="Clear"
+                            t-on-click="() => this.update({})"
                         />
-                    </t>
-                    <button
-                        class="btn btn-secondary fa fa-trash o_clear_file_button"
-                        data-tooltip="Clear"
-                        aria-label="Clear"
-                        t-on-click="onFileRemove"
-                    />
-                </FileUploader>
+                    </FileUploader>
+                </div>
             </t>
             <t t-else="">
                 <label class="o_select_file_button btn btn-primary">
                     <FileUploader
                         acceptedFileExtensions="props.acceptedFileExtensions"
-                        onUploaded.bind="onFileUploaded"
+                        onUploaded.bind="update"
                     >
                         <t t-set-slot="toggler">
-                            <span t-esc="'Upload your file'"/>
+                            Upload your file
                         </t>
                     </FileUploader>
                 </label>
             </t>
         </t>
-        <t t-elif="file.data and isDownloadable">
-            <span t-esc="file.name"/>
-            <a class="o_form_uri mx-2" href="javascript:void(0)" t-on-click.prevent="onFileDownload">
-                <span class="fa fa-download"/>
+        <t t-elif="props.record.resId and props.value">
+            <a class="o_form_uri" href="#" t-on-click.prevent="onFileDownload">
+                <span class="fa fa-download me-2" />
+                <t t-if="state.fileName" t-esc="state.fileName" />
             </a>
         </t>
     </t>
diff --git a/addons/web/static/src/views/fields/file_handler.js b/addons/web/static/src/views/fields/file_handler.js
index d193a5b9b3f..e02a4711f85 100644
--- a/addons/web/static/src/views/fields/file_handler.js
+++ b/addons/web/static/src/views/fields/file_handler.js
@@ -92,5 +92,6 @@ export class FileUploader extends Component {
         this.fileInputRef.el.click();
     }
 }
+
 FileUploader.template = "web.FileUploader";
 FileUploader.nextId = 0;
diff --git a/addons/web/static/src/views/fields/file_handler.xml b/addons/web/static/src/views/fields/file_handler.xml
index fe9ee318b71..24686bf6a3f 100644
--- a/addons/web/static/src/views/fields/file_handler.xml
+++ b/addons/web/static/src/views/fields/file_handler.xml
@@ -2,20 +2,18 @@
 <templates xml:space="preserve">
 
     <t t-name="web.FileUploader" owl="1">
-        <span>
-            <span t-if="state.isUploading" class="btn btn-primary" t-esc="'Uploading...'"/>
-            <span t-else="" t-on-click.prevent="onSelectFileButtonClick">
-                <t t-slot="toggler"/>
-            </span>
-            <t t-slot="default"/>
-            <input
-                type="file"
-                t-ref="fileInput"
-                t-attf-class="o_input_file o_hidden {{ props.fileUploadClass or '' }}"
-                t-att-multiple="props.multiUpload ? 'multiple' : false" t-att-accept="props.acceptedFileExtensions or '*'"
-                t-on-change="onFileChange"
-            />
+        <t t-if="state.isUploading">Uploading...</t>
+        <span t-else="" t-on-click.prevent="onSelectFileButtonClick" style="display:contents">
+            <t t-slot="toggler"/>
         </span>
+        <t t-slot="default"/>
+        <input
+            type="file"
+            t-ref="fileInput"
+            t-attf-class="o_input_file o_hidden {{ props.fileUploadClass or '' }}"
+            t-att-multiple="props.multiUpload ? 'multiple' : false" t-att-accept="props.acceptedFileExtensions or '*'"
+            t-on-change="onFileChange"
+        />
     </t>
 
 </templates>
diff --git a/addons/web/static/src/views/fields/pdf_viewer/pdf_viewer_field.js b/addons/web/static/src/views/fields/pdf_viewer/pdf_viewer_field.js
index 7c57903dd8c..de5d405b575 100644
--- a/addons/web/static/src/views/fields/pdf_viewer/pdf_viewer_field.js
+++ b/addons/web/static/src/views/fields/pdf_viewer/pdf_viewer_field.js
@@ -13,56 +13,60 @@ export class PdfViewerField extends Component {
     setup() {
         this.notification = useService("notification");
         this.state = useState({
-            fileName: this.fileName || "",
+            fileName: this.props.record.data[this.props.fileNameField] || "",
             isValid: true,
             objectUrl: "",
         });
         onWillUpdateProps((nextProps) => {
             if (nextProps.readonly) {
-                this.state.fileName = "";
+                this.state.fileName = nextProps.record.data[nextProps.fileNameField] || "";
                 this.state.objectUrl = "";
             }
         });
     }
-    get defaultPage() {
-        return this.props.record.data[`${this.props.name}_page`];
-    }
+
     get fileName() {
-        return this.props.record.data[this.props.fileNameField];
-    }
-    get file() {
-        return {
-            data: this.props.value || "",
-            name: this.state.fileName || this.props.value || null,
-        };
+        return this.state.fileName || this.props.value || "";
     }
+
     get url() {
-        if (this.state.isValid && this.props.value) {
-            return (
-                "/web/static/lib/pdfjs/web/viewer.html?file=" +
-                encodeURIComponent(
-                    this.state.objectUrl ||
-                        url("/web/content", {
-                            model: this.props.record.resModel,
-                            id: this.props.record.resId,
-                            field: this.props.previewImage || this.props.name,
-                        })
-                ) +
-                `#page=${this.props.defaultPage || 1}`
-            );
+        if (!this.state.isValid || !this.props.value) {
+            return null;
         }
-        return null;
+        const page = this.props.record.data[`${this.props.name}_page`] || 1;
+        const file = encodeURIComponent(
+            this.state.objectUrl ||
+                url("/web/content", {
+                    model: this.props.record.resModel,
+                    field: this.props.previewImage || this.props.name,
+                    id: this.props.record.resId,
+                })
+        );
+        return `/web/static/lib/pdfjs/web/viewer.html?file=${file}#page=${page}`;
     }
+
+    update({ data, name }) {
+        this.state.fileName = name || "";
+        const { fileNameField, record } = this.props;
+        const changes = { [this.props.name]: data || false };
+        if (fileNameField in record.fields && record.data[fileNameField] !== name) {
+            changes[fileNameField] = name || false;
+        }
+        return this.props.record.update(changes);
+    }
+
     onFileRemove() {
         this.state.isValid = true;
-        this.props.update(false);
+        this.update({});
     }
-    onFileUploaded(file) {
-        this.state.fileName = file.name;
+
+    onFileUploaded({ data, name, objectUrl }) {
+        this.state.fileName = name;
         this.state.isValid = true;
-        this.props.update(file.data);
-        this.state.objectUrl = file.objectUrl;
+        this.state.objectUrl = objectUrl;
+        this.update({ data, name });
     }
+
     onLoadFailed() {
         this.state.isValid = false;
         this.notification.add(this.env._t("Could not display the selected pdf"), {
diff --git a/addons/web/static/src/views/fields/pdf_viewer/pdf_viewer_field.xml b/addons/web/static/src/views/fields/pdf_viewer/pdf_viewer_field.xml
index db85ef2aef9..e9d3170a3c3 100644
--- a/addons/web/static/src/views/fields/pdf_viewer/pdf_viewer_field.xml
+++ b/addons/web/static/src/views/fields/pdf_viewer/pdf_viewer_field.xml
@@ -3,42 +3,43 @@
 
     <t t-name="web.PdfViewerField" owl="1">
         <t t-if="!props.readonly">
-            <t t-if="file.data">
-                <FileUploader
-                    acceptedFileExtensions="'.pdf'"
-                    file="file"
-                    onUploaded.bind="onFileUploaded"
-                >
-                    <t t-set-slot="toggler">
-                        <span class="mx-2" t-esc="file.name"/>
-                        <button
-                            class="btn btn-secondary fa fa-pencil o_select_file_button"
-                            data-tooltip="Edit"
-                            aria-label="Edit"
-                        />
-                    </t>
-                    <button
-                        class="btn btn-secondary fa fa-trash o_clear_file_button"
-                        data-tooltip="Clear"
-                        aria-label="Clear"
-                        t-on-click="onFileRemove"
-                    />
-                </FileUploader>
-            </t>
-            <t t-else="">
-                <label class="o_select_file_button btn btn-primary">
+            <div  class="o_form_pdf_controls">
+                <t t-if="props.value">
                     <FileUploader
                         acceptedFileExtensions="'.pdf'"
+                        file="{ data: props.value, name: fileName }"
                         onUploaded.bind="onFileUploaded"
                     >
                         <t t-set-slot="toggler">
-                            <span t-esc="'Upload your file'"/>
+                            <button
+                                class="btn btn-secondary fa fa-pencil o_select_file_button"
+                                data-tooltip="Edit"
+                                aria-label="Edit"
+                            />
                         </t>
+                        <button
+                            class="btn btn-secondary fa fa-trash o_clear_file_button"
+                            data-tooltip="Clear"
+                            aria-label="Clear"
+                            t-on-click="onFileRemove"
+                        />
                     </FileUploader>
-                </label>
-            </t>
+                </t>
+                <t t-else="">
+                    <label class="o_select_file_button btn btn-primary">
+                        <FileUploader
+                            acceptedFileExtensions="'.pdf'"
+                            onUploaded.bind="onFileUploaded"
+                        >
+                            <t t-set-slot="toggler">
+                                Upload your file
+                            </t>
+                        </FileUploader>
+                    </label>
+                </t>
+            </div>
         </t>
-        <t t-if="file.data">
+        <t t-if="props.value">
             <iframe class="o_pdfview_iframe"
                 alt="PDF file"
                 t-att-src="url"
diff --git a/addons/web/static/tests/views/fields/binary_field_tests.js b/addons/web/static/tests/views/fields/binary_field_tests.js
index da4ae98c18a..e680e657b46 100644
--- a/addons/web/static/tests/views/fields/binary_field_tests.js
+++ b/addons/web/static/tests/views/fields/binary_field_tests.js
@@ -1,18 +1,16 @@
 /** @odoo-module **/
 
-import { browser } from "@web/core/browser/browser";
-import { patch, unpatch } from "@web/core/utils/patch";
-import { FileUploader } from "@web/views/fields/file_handler";
 import { registerCleanup } from "@web/../tests/helpers/cleanup";
 import { makeMockXHR } from "@web/../tests/helpers/mock_services";
 import {
     click,
+    editInput,
     getFixture,
     makeDeferred,
-    nextTick,
     patchWithCleanup,
 } from "@web/../tests/helpers/utils";
 import { makeView, setupViewRegistries } from "@web/../tests/views/helpers";
+import { browser } from "@web/core/browser/browser";
 
 let serverData;
 let target;
@@ -114,7 +112,7 @@ QUnit.module("Fields", (hooks) => {
             "the binary field should be rendered as a downloadable link in readonly"
         );
         assert.strictEqual(
-            target.querySelector('.o_field_widget[name="document"] span').textContent.trim(),
+            target.querySelector('.o_field_widget[name="document"]').textContent,
             "coucou.txt",
             "the binary field should display the name of the file in the link"
         );
@@ -148,8 +146,7 @@ QUnit.module("Fields", (hooks) => {
             "the binary field should not be rendered as a downloadable link in edit"
         );
         assert.strictEqual(
-            target.querySelector('.o_field_widget[name="document"].o_field_binary span')
-                .textContent,
+            target.querySelector('.o_field_widget[name="document"].o_field_binary .o_input').value,
             "coucou.txt",
             "the binary field should display the file name in the input edit mode"
         );
@@ -194,20 +191,33 @@ QUnit.module("Fields", (hooks) => {
         );
     });
 
+    QUnit.test("file name field is not defined", async (assert) => {
+        await makeView({
+            serverData,
+            type: "form",
+            resModel: "partner",
+            arch: /* xml */ `
+                <form>
+                    <field name="document" filename="foo"/>
+                </form>`,
+            resId: 1,
+        });
+
+        assert.strictEqual(
+            target.querySelector(".o_field_binary").textContent,
+            "",
+            "there should be no text since the name field is not in the view"
+        );
+        assert.isVisible(
+            target,
+            ".o_field_binary .o_form_uri fa-download",
+            "download icon should be visible"
+        );
+    });
+
     QUnit.test(
         "binary fields input value is empty when clearing after uploading",
         async function (assert) {
-            patch(FileUploader.prototype, "test.FileUploader", {
-                async onFileChange(ev) {
-                    const file = new File([ev.target.value], ev.target.value + ".txt", {
-                        type: "text/plain",
-                    });
-                    await this._super({
-                        target: { files: [file] },
-                    });
-                },
-            });
-
             await makeView({
                 type: "form",
                 resModel: "partner",
@@ -222,32 +232,35 @@ QUnit.module("Fields", (hooks) => {
 
             await click(target, ".o_form_button_edit");
 
-            const input = target.querySelector(".o_field_binary input");
-            // We need to convert the input type since we can't programmatically set
-            // the value of a file input. The patch of the onFileChange will create
-            // a file object to be used by the component.
-            input.setAttribute("type", "text");
-            input.value = "fake_file";
-            input.dispatchEvent(new InputEvent("input", { bubbles: true }));
-            input.dispatchEvent(new Event("change", { bubbles: true }));
-            await nextTick();
-            await nextTick();
+            const file = new File(["test"], "fake_file.txt", { type: "text/plain" });
+            await editInput(target, ".o_field_binary .o_input_file", file);
 
+            assert.ok(
+                target.querySelector(".o_field_binary input[type=text]").hasAttribute("readonly")
+            );
             assert.strictEqual(
-                target.querySelector(".o_field_binary span").textContent,
+                target.querySelector(".o_field_binary input[type=text]").value,
                 "fake_file.txt",
                 'displayed value should be changed to "fake_file.txt"'
             );
+            assert.strictEqual(
+                target.querySelector(".o_field_char input[type=text]").value,
+                "fake_file.txt",
+                'related value should be changed to "fake_file.txt"'
+            );
 
             await click(target.querySelector(".o_clear_file_button"));
 
             assert.strictEqual(
-                target.querySelector(".o_input_file").value,
+                target.querySelector(".o_field_binary .o_input_file").value,
                 "",
-                "input value should be empty"
+                "file input value should be empty"
+            );
+            assert.strictEqual(
+                target.querySelector(".o_field_char input").value,
+                "",
+                "related value should be empty"
             );
-
-            unpatch(FileUploader.prototype, "test.FileUploader");
         }
     );
 
diff --git a/addons/web/static/tests/views/fields/pdf_viewer_field_tests.js b/addons/web/static/tests/views/fields/pdf_viewer_field_tests.js
index 4efd0aea4bf..35895c53402 100644
--- a/addons/web/static/tests/views/fields/pdf_viewer_field_tests.js
+++ b/addons/web/static/tests/views/fields/pdf_viewer_field_tests.js
@@ -1,11 +1,18 @@
 /** @odoo-module **/
 
-import { getFixture, nextTick } from "@web/../tests/helpers/utils";
+import { click, editInput, getFixture } from "@web/../tests/helpers/utils";
 import { makeView, setupViewRegistries } from "@web/../tests/views/helpers";
 
 let serverData;
 let target;
 
+const getIframe = () => target.querySelector(".o_field_widget iframe.o_pdfview_iframe");
+
+const getIframeProtocol = () => getIframe().dataset.src.match(/\?file=(\w+)%3A/)[1];
+
+const getIframeViewerParams = () =>
+    decodeURIComponent(getIframe().dataset.src.match(/%2Fweb%2Fcontent%3F(.*)#page/)[1]);
+
 QUnit.module("Fields", (hooks) => {
     hooks.beforeEach(() => {
         target = getFixture();
@@ -54,12 +61,8 @@ QUnit.module("Fields", (hooks) => {
             serverData,
             resId: 1,
             arch: '<form><field name="document" widget="pdf_viewer"/></form>',
-            mockRPC: function (route) {
-                if (route.indexOf("/web/static/lib/pdfjs/web/viewer.html") !== -1) {
-                    return Promise.resolve();
-                }
-            },
         });
+
         assert.hasClass(target.querySelector(".o_field_widget"), "o_field_pdf_viewer");
         assert.containsNone(target, ".o_select_file_button", "there should be no 'Upload' button");
         assert.containsOnce(
@@ -67,39 +70,36 @@ QUnit.module("Fields", (hooks) => {
             ".o_field_widget iframe.o_pdfview_iframe",
             "there should be an iframe"
         );
-        const iframeFile = target
-            .querySelector(".o_field_widget iframe.o_pdfview_iframe")
-            .dataset.src.split("%2Fweb%2Fcontent")[1];
-        assert.strictEqual(
-            iframeFile,
-            "%3Fmodel%3Dpartner%26id%3D1%26field%3Ddocument#page=1",
-            "the src attribute should be correctly set on the iframe"
-        );
+        assert.strictEqual(getIframeProtocol(), "http");
+        assert.strictEqual(getIframeViewerParams(), "model=partner&field=document&id=1");
     });
 
     QUnit.test("PdfViewerField: upload rendering", async function (assert) {
+        assert.expect(6);
+
         await makeView({
             type: "form",
             resModel: "partner",
             serverData,
             arch: '<form><field name="document" widget="pdf_viewer"/></form>',
+            async mockRPC(_route, { method, args }) {
+                if (method === "create") {
+                    assert.deepEqual(args[0], { document: btoa("test") });
+                }
+            },
         });
 
         assert.containsNone(target, ".o_pdfview_iframe", "there is no PDF Viewer");
 
-        // Set and trigger the change of a pdf file for the input
-        const fileInput = target.querySelector('input[type="file"]');
-        const dataTransfer = new DataTransfer();
-        dataTransfer.items.add(new File(["test"], "test.pdf", { type: "application/pdf" }));
-        fileInput.files = dataTransfer.files;
-        fileInput.dispatchEvent(new Event("change", { bubbles: true }));
-        await nextTick();
-        await nextTick();
+        const file = new File(["test"], "test.pdf", { type: "application/pdf" });
+        await editInput(target, ".o_field_pdf_viewer input[type=file]", file);
 
         assert.containsOnce(target, ".o_pdfview_iframe", "there is a PDF Viewer");
-        const iframeFile = target
-            .querySelector(".o_field_widget iframe.o_pdfview_iframe")
-            .dataset.src.split("?file=")[1];
-        assert.ok(/^blob%3/.test(iframeFile), "the file starts with 'blob:'");
+        assert.strictEqual(getIframeProtocol(), "blob");
+
+        await click(target, ".o_form_button_save");
+
+        assert.strictEqual(getIframeProtocol(), "http");
+        assert.strictEqual(getIframeViewerParams(), "model=partner&field=document&id=2");
     });
 });