From 96c8dd3ca8b5e1d72e1600c426ff4f82f8cd08dd Mon Sep 17 00:00:00 2001 From: Renaud Thiry Date: Mon, 10 Oct 2022 12:33:27 +0000 Subject: [PATCH] [FIX] web_editor: onAttachmentChange only composer Due to a previous fix, attachments uploaded through media dialog would appear in the attachments of mail marketting. That fix prevents attachments from 'dangling' and being garbage collected later. That fix is now limited to the mail composer in this commit as attachments are only garbage collected for that model, for now. related commit: c112361bf9e2f5e7b087c5e5b9a31879856b1da4 task 3003939 closes odoo/odoo#103038 X-original-commit: 69fdca133b06eb66443635bc52d380233580ef51 Signed-off-by: Thibault Delavallee (tde) --- .../static/tests/field_html_file_upload.js | 35 +++++++++++-------- .../src/legacy/js/fields/relational_fields.js | 2 -- addons/web/static/src/legacy/xml/base.xml | 2 +- .../many2many_binary_field.js | 2 -- .../many2many_binary_field.xml | 2 +- .../legacy/fields/relational_fields_tests.js | 5 +-- .../fields/many2many_binary_field_tests.js | 2 +- .../static/src/js/backend/field_html.js | 5 +-- .../static/src/js/backend/html_field.js | 3 +- 9 files changed, 30 insertions(+), 28 deletions(-) diff --git a/addons/test_website/static/tests/field_html_file_upload.js b/addons/test_website/static/tests/field_html_file_upload.js index ab12178ff6c..a0bb612cbad 100644 --- a/addons/test_website/static/tests/field_html_file_upload.js +++ b/addons/test_website/static/tests/field_html_file_upload.js @@ -16,37 +16,38 @@ const { useEffect } = owl; QUnit.module('field html file upload', { beforeEach: function () { this.data = weTestUtils.wysiwygData({ - 'note.note': { + 'mail.compose.message': { fields: { display_name: { string: "Displayed name", type: "char" }, - header: { - string: "Header", - type: "html", - required: true, - }, body: { - string: "Message", + string: "Message Body inline (to send)", type: "html" }, + attachment_ids: { + string: "Attachments", + type: "many2many", + relation: "ir.attachment", + } }, records: [{ id: 1, - display_name: "first record", - header: "

  

", - body: "

toto toto toto

tata

", + display_name: "Some Composer", + body: "Hello", + attachment_ids: [], }], }, }); }, }, function () { QUnit.test('media dialog: upload', async function (assert) { - assert.expect(3); + assert.expect(4); const onAttachmentChangeTriggered = testUtils.makeTestPromise(); patchWithCleanup(HtmlField.prototype, { '_onAttachmentChange': function (event) { + this._super(event); onAttachmentChangeTriggered.resolve(true); } }); @@ -75,16 +76,17 @@ QUnit.module('field html file upload', { 1: { id: 1, name: "test", - res_model: "note.note", + res_model: "mail.compose.message", type: "ir.actions.act_window", views: [[false, "form"]], }, }; serverData.views = { - "note.note,false,search": "", - "note.note,false,form": ` + "mail.compose.message,false,search": "", + "mail.compose.message,false,form": `
+ `, }; const mockRPC = (route, args) => { @@ -122,9 +124,14 @@ QUnit.module('field html file upload', { fileInputs.forEach(input => { input.dispatchEvent(new Event('change', {})); }); + assert.ok(await Promise.race([onChangeTriggered, new Promise((res, _) => setTimeout(() => res(false), 400))]), "File change event was not triggered"); assert.ok(await Promise.race([onAttachmentChangeTriggered, new Promise((res, _) => setTimeout(() => res(false), 400))]), "_onAttachmentChange was not called with the new attachment, necessary for unsused upload cleanup on backend"); + + // wait to check that dom is properly updated + await new Promise((res, _) => setTimeout(() => res(false), 400)); + assert.ok(fixture.querySelector('.o_attachment[title="test.jpg"]')); }); }); diff --git a/addons/web/static/src/legacy/js/fields/relational_fields.js b/addons/web/static/src/legacy/js/fields/relational_fields.js index f985f187f61..0328f650938 100644 --- a/addons/web/static/src/legacy/js/fields/relational_fields.js +++ b/addons/web/static/src/legacy/js/fields/relational_fields.js @@ -2370,8 +2370,6 @@ var FieldMany2ManyBinaryMultiFiles = AbstractField.extend({ fieldsToFetch: { name: {type: 'char'}, mimetype: {type: 'char'}, - res_id: {type: 'number'}, - access_token: {type: 'char'}, }, events: { 'click .o_attach': '_onAttach', diff --git a/addons/web/static/src/legacy/xml/base.xml b/addons/web/static/src/legacy/xml/base.xml index f5c64a26020..25fd9c373a0 100644 --- a/addons/web/static/src/legacy/xml/base.xml +++ b/addons/web/static/src/legacy/xml/base.xml @@ -1338,7 +1338,7 @@ - +
diff --git a/addons/web/static/src/views/fields/many2many_binary/many2many_binary_field.js b/addons/web/static/src/views/fields/many2many_binary/many2many_binary_field.js index b7a5512b169..c7be72fe879 100644 --- a/addons/web/static/src/views/fields/many2many_binary/many2many_binary_field.js +++ b/addons/web/static/src/views/fields/many2many_binary/many2many_binary_field.js @@ -59,8 +59,6 @@ Many2ManyBinaryField.supportedTypes = ["many2many"]; Many2ManyBinaryField.fieldsToFetch = { name: { type: "char" }, mimetype: { type: "char" }, - res_id: { type: "number" }, - access_token: { type: "char" }, }; Many2ManyBinaryField.isEmpty = () => false; diff --git a/addons/web/static/src/views/fields/many2many_binary/many2many_binary_field.xml b/addons/web/static/src/views/fields/many2many_binary/many2many_binary_field.xml index ed18b060e35..53aa56ec88d 100644 --- a/addons/web/static/src/views/fields/many2many_binary/many2many_binary_field.xml +++ b/addons/web/static/src/views/fields/many2many_binary/many2many_binary_field.xml @@ -25,7 +25,7 @@ - +
diff --git a/addons/web/static/tests/legacy/fields/relational_fields_tests.js b/addons/web/static/tests/legacy/fields/relational_fields_tests.js index c98ea14e015..2ae838787dc 100644 --- a/addons/web/static/tests/legacy/fields/relational_fields_tests.js +++ b/addons/web/static/tests/legacy/fields/relational_fields_tests.js @@ -2977,14 +2977,11 @@ QUnit.module('Legacy relational_fields', { fields: { name: {string:"Name", type: "char"}, mimetype: {string: "Mimetype", type: "char"}, - res_id: {type: "number"}, - access_token: {type: "char"} }, records: [{ id: 17, name: 'Marley&Me.jpg', mimetype: 'jpg', - res_id: 1, //non-zero to avoid transiant model editor attachment protection }], }; this.data.turtle.fields.picture_ids = { @@ -3008,7 +3005,7 @@ QUnit.module('Legacy relational_fields', { mockRPC: function (route, args) { assert.step(route); if (route === '/web/dataset/call_kw/ir.attachment/read') { - assert.deepEqual(args.args[1], ['name', 'mimetype', 'res_id', 'access_token']); + assert.deepEqual(args.args[1], ['name', 'mimetype']); } return this._super.apply(this, arguments); }, diff --git a/addons/web/static/tests/views/fields/many2many_binary_field_tests.js b/addons/web/static/tests/views/fields/many2many_binary_field_tests.js index cf80b3fcd33..796327e478b 100644 --- a/addons/web/static/tests/views/fields/many2many_binary_field_tests.js +++ b/addons/web/static/tests/views/fields/many2many_binary_field_tests.js @@ -96,7 +96,7 @@ QUnit.module("Fields", (hooks) => { assert.step(route); } if (route === "/web/dataset/call_kw/ir.attachment/read") { - assert.deepEqual(args.args[1], ["name", "mimetype", "res_id", "access_token"]); + assert.deepEqual(args.args[1], ["name", "mimetype"]); } }, }); diff --git a/addons/web_editor/static/src/js/backend/field_html.js b/addons/web_editor/static/src/js/backend/field_html.js index 0a4db994d95..8a9b043dfd6 100644 --- a/addons/web_editor/static/src/js/backend/field_html.js +++ b/addons/web_editor/static/src/js/backend/field_html.js @@ -334,10 +334,11 @@ var FieldHtml = basic_fields.DebouncedField.extend(DynamicPlaceholderFieldMixin) * @param {Object} event the event containing attachment data */ _onAttachmentChange: function (event) { - const attachments = event.data; - if (!this.fieldNameAttachment) { + // This only needs to happen for the composer for now + if (!this.fieldNameAttachment || this.model !== 'mail.compose.message') { return; } + const attachments = event.data; this.trigger_up('field_changed', { dataPointID: this.dataPointID, changes: _.object([this.fieldNameAttachment], [{ diff --git a/addons/web_editor/static/src/js/backend/html_field.js b/addons/web_editor/static/src/js/backend/html_field.js index a8f76e08830..6e3b7b75608 100644 --- a/addons/web_editor/static/src/js/backend/html_field.js +++ b/addons/web_editor/static/src/js/backend/html_field.js @@ -497,7 +497,8 @@ export class HtmlField extends Component { return getWysiwygClass(); } _onAttachmentChange(attachment) { - if (!this.props.record.fieldNames.includes('attachment_ids')) { + // This only needs to happen for the composer for now + if (!(this.props.record.fieldNames.includes('attachment_ids') && this.props.record.resModel === 'mail.compose.message')) { return; } this.props.record.update(_.object(['attachment_ids'], [{