From 63eb7fc0230382c038f7d021c8dcce27d3076732 Mon Sep 17 00:00:00 2001 From: Lucas Perais Date: Fri, 5 Aug 2022 06:48:49 +0000 Subject: [PATCH] [FIX] web: record's images correctly cached by the browser Before this commit, images' url where just built upon the record's id and model. Since the browser caches GET requests by url, the same record would always have the same image url. Hence it appeared that when changing a record's image in form view, and going back to the kanban, the image displayed was the old one. A good enough approximation to solve this (and was actually solved in legacy code), is to use the record's __last_update field, which basically tells the browser to re-fetch a record's image when then record go through any modification. After this commit, images are correctly updated in kanban and form views. closes odoo/odoo#97544 Signed-off-by: Lucas Perais (lpe) --- .../src/views/fields/image/image_field.js | 32 ++++- .../static/src/views/kanban/kanban_record.js | 9 +- .../tests/views/fields/image_field_tests.js | 110 +++++++++++++++++- .../static/tests/views/kanban_view_tests.js | 37 ++++-- 4 files changed, 170 insertions(+), 18 deletions(-) diff --git a/addons/web/static/src/views/fields/image/image_field.js b/addons/web/static/src/views/fields/image/image_field.js index f2898369b95..5e68da4201b 100644 --- a/addons/web/static/src/views/fields/image/image_field.js +++ b/addons/web/static/src/views/fields/image/image_field.js @@ -8,7 +8,8 @@ import { registry } from "@web/core/registry"; import { FileUploader } from "../file_handler"; import { standardFieldProps } from "../standard_field_props"; -const { Component, useState } = owl; +const { Component, useState, onWillUpdateProps } = owl; +const { DateTime } = luxon; export const fileTypeMagicWordMap = { "/": "jpg", @@ -22,6 +23,21 @@ function isBinarySize(value) { return /^\d+(\.\d*)? [^0-9]+$/.test(value); } +/** + * Formats a value to be injected in the image's url in order for that url + * to be correctly cached and discarded by the browser (the browser caches + * fetch requests with the url as key). + * + * For records, a not-so-bad approximation is to compute that key on the basis + * of the record's __last_update field. + */ +export function imageCacheKey(value) { + if (value instanceof DateTime) { + return value.ts; + } + return ""; +} + export class ImageField extends Component { setup() { this.notification = useService("notification"); @@ -29,6 +45,15 @@ export class ImageField extends Component { this.state = useState({ isValid: true, }); + + this.rawCacheKey = this.props.record.data.__last_update; + onWillUpdateProps((nextProps) => { + const { record } = this.props; + const { record: nextRecord } = nextProps; + if (record.resId !== nextRecord.resId || nextRecord.mode === "readonly") { + this.rawCacheKey = nextRecord.data.__last_update; + } + }); } get sizeStyle() { @@ -58,6 +83,7 @@ export class ImageField extends Component { model: this.props.record.resModel, id: this.props.record.resId, field: previewFieldName, + unique: imageCacheKey(this.rawCacheKey), }); } else { // Use magic-word technique for detecting image type @@ -103,6 +129,10 @@ ImageField.defaultProps = { ImageField.displayName = _lt("Image"); ImageField.supportedTypes = ["binary"]; +ImageField.fieldDependencies = { + __last_update: { type: "datetime" }, +}; + ImageField.extractProps = ({ attrs }) => { return { enableZoom: attrs.options.zoom, diff --git a/addons/web/static/src/views/kanban/kanban_record.js b/addons/web/static/src/views/kanban/kanban_record.js index 4428bcad96c..29c8fce5e5f 100644 --- a/addons/web/static/src/views/kanban/kanban_record.js +++ b/addons/web/static/src/views/kanban/kanban_record.js @@ -10,7 +10,7 @@ import { useService } from "@web/core/utils/hooks"; import { sprintf } from "@web/core/utils/strings"; import { url } from "@web/core/utils/urls"; import { Field } from "@web/views/fields/field"; -import { fileTypeMagicWordMap } from "@web/views/fields/image/image_field"; +import { fileTypeMagicWordMap, imageCacheKey } from "@web/views/fields/image/image_field"; import { ViewButton } from "@web/views/view_button/view_button"; import { useViewCompiler } from "@web/views/view_compiler"; import { Widget } from "@web/views/widgets/widget"; @@ -82,7 +82,12 @@ function getImageSrcFromRecordInfo(record, model, field, idOrIds, placeholder) { return placeholder; } else { // Else: fetches the image related to the given id. - return url("/web/image", { model, field, id }); + return url("/web/image", { + model, + field, + id, + unique: imageCacheKey(record.data.__last_update), + }); } } diff --git a/addons/web/static/tests/views/fields/image_field_tests.js b/addons/web/static/tests/views/fields/image_field_tests.js index 4e6b832811d..796e94f9281 100644 --- a/addons/web/static/tests/views/fields/image_field_tests.js +++ b/addons/web/static/tests/views/fields/image_field_tests.js @@ -1,7 +1,16 @@ /** @odoo-module **/ -import { click, getFixture, nextTick, triggerEvent } from "@web/../tests/helpers/utils"; +import { + click, + getFixture, + nextTick, + triggerEvent, + clickEdit, + clickSave, + editInput, +} from "@web/../tests/helpers/utils"; import { makeView, setupViewRegistries } from "@web/../tests/views/helpers"; +import { pagerNext } from "@web/../tests/search/helpers"; const MY_IMAGE = "iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg=="; @@ -25,6 +34,7 @@ QUnit.module("Fields", (hooks) => { relation: "partner_type", searchable: true, }, + foo: { type: "char" }, document: { string: "Binary", type: "binary" }, }, records: [ @@ -82,7 +92,7 @@ QUnit.module("Fields", (hooks) => { if (route === "/web/dataset/call_kw/partner/read") { assert.deepEqual( args[1], - ["document", /*"__last_update",*/ "display_name"], + ["__last_update", "document", "display_name"], "The fields document, display_name and __last_update should be present when reading an image" ); } @@ -315,6 +325,7 @@ QUnit.module("Fields", (hooks) => { "ImageField displays the right images with zoom and preview_image options", async function (assert) { serverData.models.partner.records[0].document = "3 kb"; + serverData.models.partner.records[0].__last_update = "2022-08-05 08:37:00"; await makeView({ type: "form", @@ -330,7 +341,7 @@ QUnit.module("Fields", (hooks) => { assert.ok( JSON.parse( target.querySelector(".o_field_image img").dataset["tooltipInfo"] - ).url.endsWith("/web/image?model=partner&id=1&field=document"), + ).url.endsWith("/web/image?model=partner&id=1&field=document&unique=1659688620000"), "tooltip show the full image from the field value" ); assert.strictEqual( @@ -341,7 +352,9 @@ QUnit.module("Fields", (hooks) => { assert.ok( target .querySelector(".o_field_image img") - .dataset.src.endsWith("/web/image?model=partner&id=1&field=document_preview"), + .dataset.src.endsWith( + "/web/image?model=partner&id=1&field=document_preview&unique=1659688620000" + ), "image src is the preview image given in option" ); } @@ -448,4 +461,93 @@ QUnit.module("Fields", (hooks) => { "image field should be displayed as invalid" ); }); + + QUnit.test("unique in url doesn't change on onchange", async (assert) => { + serverData.models.partner.onchanges = { + foo: () => {}, + }; + + const rec = serverData.models.partner.records.find((rec) => rec.id === 1); + rec.document = "3 kb"; + rec.__last_update = "2022-08-05 08:37:00"; + + await makeView({ + resId: 1, + type: "form", + resModel: "partner", + serverData, + arch: ` +
+ + + `, + mockRPC(route, { method, args }) { + assert.step(method); + if (method === "onchange") { + return { + value: { + __last_update: "", // actual return of the server + }, + }; + } + if (method === "write") { + args[1].__last_update = "2022-08-05 09:37:00"; + } + }, + }); + + function getUnique(target) { + const src = target.dataset.src; + return new URL(src).searchParams.get("unique"); + } + + assert.verifySteps(["get_views", "read"]); + assert.strictEqual(getUnique(target.querySelector(".o_field_image img")), "1659688620000"); + + await clickEdit(target); + assert.verifySteps([]); + // same unique as before + assert.strictEqual(getUnique(target.querySelector(".o_field_image img")), "1659688620000"); + + await editInput(target, ".o_field_widget[name='foo'] input", "grrr"); + assert.verifySteps(["onchange"]); + // also same unique + assert.strictEqual(getUnique(target.querySelector(".o_field_image img")), "1659688620000"); + + await clickSave(target); + assert.verifySteps(["write", "read"]); + // different unique: the record has been written + assert.strictEqual(getUnique(target.querySelector(".o_field_image img")), "1659692220000"); + }); + + QUnit.test("unique in url change on record change", async (assert) => { + const rec = serverData.models.partner.records.find((rec) => rec.id === 1); + rec.document = "3 kb"; + rec.__last_update = "2022-08-05 08:37:00"; + + const rec2 = serverData.models.partner.records.find((rec) => rec.id === 2); + rec2.document = "3 kb"; + rec2.__last_update = "2022-08-05 09:37:00"; + + await makeView({ + resIds: [1, 2], + resId: 1, + type: "form", + resModel: "partner", + serverData, + arch: ` +
+ + `, + }); + + function getUnique(target) { + const src = target.dataset.src; + return new URL(src).searchParams.get("unique"); + } + + assert.strictEqual(getUnique(target.querySelector(".o_field_image img")), "1659688620000"); + await pagerNext(target); + assert.strictEqual(getUnique(target.querySelector(".o_field_image img")), "1659692220000"); + }); }); diff --git a/addons/web/static/tests/views/kanban_view_tests.js b/addons/web/static/tests/views/kanban_view_tests.js index 712a1a4c096..532d58fc294 100644 --- a/addons/web/static/tests/views/kanban_view_tests.js +++ b/addons/web/static/tests/views/kanban_view_tests.js @@ -8403,7 +8403,10 @@ QUnit.module("Views", (hooks) => { QUnit.test("test displaying image (__last_update field)", async (assert) => { // the presence of __last_update field ensures that the image is reloaded when necessary - assert.expect(1); + assert.expect(2); + + const rec = serverData.models.partner.records.find((r) => r.id === 1); + rec.__last_update = "2022-08-05 08:37:00"; await makeView({ type: "kanban", @@ -8421,7 +8424,17 @@ QUnit.module("Views", (hooks) => { assert.deepEqual(kwargs.fields, ["id", "__last_update"]); } }, + domain: [["id", "in", [1]]], }); + + assert.ok( + target + .querySelector(".o_kanban_record:not(.o_kanban_ghost) img") + .dataset.src.endsWith( + "/web/image?model=partner&field=image&id=1&unique=1659688620000" + ), + "image src is the preview image given in option" + ); }); QUnit.test("test displaying image (binary & placeholder)", async (assert) => { @@ -8521,12 +8534,12 @@ QUnit.module("Views", (hooks) => { }); assert.containsOnce( target, - 'img[data-src*="/web/image"][data-src$="&id=1"]', + 'img[data-src*="/web/image"][data-src$="&id=1&unique="]', "image url should contain id of set partner_id" ); assert.containsOnce( target, - 'img[data-src*="/web/image"][data-src$="&id="]', + 'img[data-src*="/web/image"][data-src$="&id=&unique="]', "image url should contain an empty id if partner_id is not set" ); }); @@ -10897,14 +10910,16 @@ QUnit.module("Views", (hooks) => { type: "kanban", resModel: "partner", serverData, - arch: - "" + - '' + - "
" + - '' + - "
" + - "" + - "", + arch: ` + + + +
+ +
+ + + `, groupBy: ["product_id"], });