From ad1539b20bbf2b79e6aca975b6fbe58406c3c06d Mon Sep 17 00:00:00 2001 From: FrancoisGe Date: Tue, 20 Jun 2023 11:11:06 +0000 Subject: [PATCH] [FIX] web: sort x2m multi page Before this commit, x2m in list mode with several pages did not work correctly when sorted. The order was lost when the page was changed and any values not used for sorting were missing. How to reproduce: - Go to a form view with an x2m in list mode with at least 2 pages - Sort on a column - At least one record not present before sorting is displayed Before this commit: This record only contains the values used for sorting After this commit: This record contains all the values - Click on next page Before this commit: The records displayed on the page are not ordered and contain only the values used for sorting. After this commit: The records displayed are the correct ones for the sort and contain all the values. Part-of: odoo/odoo#114024 --- .../src/model/relational_model/static_list.js | 57 +++++++++++-------- .../views/fields/one2many_field_tests.js | 42 ++++++++++++++ 2 files changed, 75 insertions(+), 24 deletions(-) diff --git a/addons/web/static/src/model/relational_model/static_list.js b/addons/web/static/src/model/relational_model/static_list.js index 62616d0f7ff..1edf9042c60 100644 --- a/addons/web/static/src/model/relational_model/static_list.js +++ b/addons/web/static/src/model/relational_model/static_list.js @@ -10,8 +10,8 @@ import { markRaw } from "@odoo/owl"; function compareFieldValues(v1, v2, fieldType) { if (fieldType === "many2one") { - v1 = v1 ? v1[1] : false; - v2 = v2 ? v2[1] : false; + v1 = v1 ? v1[1] : ""; + v2 = v2 ? v2[1] : ""; } return v1 < v2; } @@ -317,7 +317,7 @@ export class StaticList extends DataPoint { this._onUpdate(); } - load({ limit, offset, orderBy }) { + load({ limit, offset, orderBy } = {}) { return this.model.mutex.exec(async () => { if (this.editedRecord && !(await this.editedRecord.checkValidity())) { return; @@ -750,10 +750,30 @@ export class StaticList extends DataPoint { return commands; } - async _load({ limit, offset, orderBy }) { - const currentIds = this._currentIds.slice(offset, offset + limit); - // FIXME: record in cache could be not yet loaded - const resIds = currentIds.filter((id) => typeof id === "number" && !this._cache[id]); + _getResIdsToLoad(resIds, fieldNames = this.fieldNames) { + return resIds.filter((resId) => { + if (typeof resId === "string") { + // this is a virtual id, we don't want to read it + return false; + } + const record = this._cache[resId]; + if (!record) { + // record hasn't been loaded yet + return true; + } + // record has already been loaded -> check if we already read all orderBy fields + return intersection(fieldNames, record.fieldNames).length !== fieldNames.length; + }); + } + + async _load({ + limit = this.limit, + offset = this.offset, + orderBy = this.orderBy, + nextCurrentIds = this._currentIds, + } = {}) { + const currentIds = nextCurrentIds.slice(offset, offset + limit); + const resIds = this._getResIdsToLoad(currentIds); if (resIds.length) { const records = await this.model._loadRecords({ ...this.config, resIds }); for (const record of records) { @@ -761,6 +781,7 @@ export class StaticList extends DataPoint { } } this.records = currentIds.map((id) => this._cache[id]); + this._currentIds = nextCurrentIds; await this.model._updateConfig(this.config, { limit, offset, orderBy }, { noReload: true }); } @@ -834,19 +855,7 @@ export class StaticList extends DataPoint { async _sort(currentIds = this.currentIds, orderBy = this.orderBy) { const fieldNames = orderBy.map((o) => o.name); - const resIds = currentIds.filter((id) => { - if (typeof id === "string") { - // this is a virtual id, we don't want to read it - return false; - } - const record = this._cache[id]; - if (!record) { - // record hasn't been loaded yet - return true; - } - // record has already been loaded -> check if we already read all orderBy fields - return intersection(record.fieldNames, fieldNames).length !== fieldNames.length; - }); + const resIds = this._getResIdsToLoad(currentIds, fieldNames); if (resIds.length) { const activeFields = pick(this.activeFields, fieldNames); const config = { ...this.config, resIds, activeFields }; @@ -859,10 +868,10 @@ export class StaticList extends DataPoint { const sortedRecords = allRecords.sort((r1, r2) => { return compareRecords(r1, r2, orderBy, this.fields); }); - const currentPageRecords = sortedRecords.slice(this.offset, this.offset + this.limit); - //TODO: read records that haven't been fully read yet - this.model._updateConfig(this.config, { orderBy }, { noReload: true }); - this.records = currentPageRecords; + await this._load({ + orderBy, + nextCurrentIds: sortedRecords.map((r) => r.resId || r._virtualId), + }); this._needsReordering = false; } diff --git a/addons/web/static/tests/views/fields/one2many_field_tests.js b/addons/web/static/tests/views/fields/one2many_field_tests.js index a64b413c2fc..6da415e82e6 100644 --- a/addons/web/static/tests/views/fields/one2many_field_tests.js +++ b/addons/web/static/tests/views/fields/one2many_field_tests.js @@ -2942,6 +2942,48 @@ QUnit.module("Fields", (hooks) => { ); }); + QUnit.test("sorting one2many fields with multi page", async function (assert) { + serverData.models.partner.fields.foo.sortable = true; + serverData.models.partner.records.push({ id: 23, foo: "abc", int_field: 1 }); + serverData.models.partner.records.push({ id: 24, foo: "xyz", int_field: 1 }); + serverData.models.partner.records.push({ id: 25, foo: "def", int_field: 2 }); + serverData.models.partner.records.push({ id: 26, foo: "otc", int_field: 2 }); + serverData.models.partner.records[0].p = [23, 24, 25, 26]; + + await makeView({ + type: "form", + resModel: "partner", + serverData, + arch: ` +
+ + + + + + +
`, + resId: 1, + }); + + assert.deepEqual( + [...target.querySelectorAll(".o_data_row")].map((c) => c.textContent), + ["abc1", "xyz1"] + ); + + await click(target.querySelector("table thead [data-name='foo'].o_column_sortable")); + assert.deepEqual( + [...target.querySelectorAll(".o_data_row")].map((c) => c.textContent), + ["abc1", "def2"] + ); + + await click(target.querySelector(".o_field_widget[name='p'] .o_pager_next")); + assert.deepEqual( + [...target.querySelectorAll(".o_data_row")].map((c) => c.textContent), + ["otc2", "xyz1"] + ); + }); + QUnit.test("one2many list field edition", async function (assert) { serverData.models.partner.records.push({ id: 3,