From 00e76a7da0306b7808aadb7637af8912304be41b Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Mon, 30 Jan 2023 07:44:24 +0000 Subject: [PATCH] [FIX] web: pager: count_limit: fetch count on previous For performance reasons, we added a feature that allows to limit the search_count done when fetching records in list and kanban views, to display on the right side of the pager [1][2]. However, the page previous case wasn't correctly handled when we had a limited count. When doing page, we need to know the total number of records to compute the offset. Before this commit, we went to the last page, which wasn't really the last page (because we sent a wrong offset), which can be misleading for users. With the commit, we force a search_count when doing page previous on the first page. [1] 6acfad26513298f57a2edcf40d0f7a2e38a97295 [2] 699215ec9a71b8c9b6a53fe96e924a400d216729 Task 3162026 closes odoo/odoo#113134 X-original-commit: cefff0dcd5b3b1120b215e6af28d678c7ec34d52 Signed-off-by: Pierre Paridans (app) --- addons/web/static/src/core/pager/pager.js | 4 ++ addons/web/static/tests/core/pager_tests.js | 26 ++++++++++++ .../static/tests/views/kanban_view_tests.js | 41 +++++++++++++++++++ .../web/static/tests/views/list_view_tests.js | 31 ++++++++++++++ 4 files changed, 102 insertions(+) diff --git a/addons/web/static/src/core/pager/pager.js b/addons/web/static/src/core/pager/pager.js index 25f83d903ba..c311574fc1f 100644 --- a/addons/web/static/src/core/pager/pager.js +++ b/addons/web/static/src/core/pager/pager.js @@ -64,6 +64,10 @@ export class Pager extends Component { async navigate(direction) { let minimum = this.props.offset + this.props.limit * direction; let total = this.props.total; + if (this.props.updateTotal && minimum < 0) { + // we must know the real total to be able to loop by doing "previous" + total = await this.props.updateTotal(); + } if (minimum >= total) { if (!this.props.updateTotal) { // only loop forward if we know the real total, otherwise let the minimum diff --git a/addons/web/static/tests/core/pager_tests.js b/addons/web/static/tests/core/pager_tests.js index 154f57729c4..1e327678ac9 100644 --- a/addons/web/static/tests/core/pager_tests.js +++ b/addons/web/static/tests/core/pager_tests.js @@ -382,4 +382,30 @@ QUnit.module("Components", ({ beforeEach }) => { assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "10+"); assert.hasClass(target.querySelector(".o_pager_limit"), "o_pager_limit_fetch"); }); + + QUnit.test("updateTotal props: click previous", async function (assert) { + const pager = await makePager({ + offset: 0, + limit: 5, + total: 10, + onUpdate(data) { + pager.updateProps(data); + }, + async updateTotal() { + const total = 23; + pager.updateProps({ total, updateTotal: undefined }); + return total; + }, + }); + + assert.strictEqual(target.querySelector(".o_pager_value").innerText, "1-5"); + assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "10+"); + assert.hasClass(target.querySelector(".o_pager_limit"), "o_pager_limit_fetch"); + + await click(target, ".o_pager_previous"); + + assert.strictEqual(target.querySelector(".o_pager_value").innerText, "21-23"); + assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "23"); + assert.doesNotHaveClass(target.querySelector(".o_pager_limit"), "o_pager_limit_fetch"); + }); }); diff --git a/addons/web/static/tests/views/kanban_view_tests.js b/addons/web/static/tests/views/kanban_view_tests.js index 125d9506d70..2c43614b27d 100644 --- a/addons/web/static/tests/views/kanban_view_tests.js +++ b/addons/web/static/tests/views/kanban_view_tests.js @@ -1342,6 +1342,47 @@ QUnit.module("Views", (hooks) => { assert.verifySteps(["search_count"]); }); + QUnit.test( + "pager, ungrouped, deleting all records from last page should move to previous page", + async (assert) => { + patchDialog((_cls, props) => { + assert.step("open-dialog"); + props.confirm(); + }); + + await makeView({ + type: "kanban", + resModel: "partner", + serverData, + arch: ` + + +
+ + +
+ + + `, + }); + + assert.deepEqual(getPagerValue(target), [1, 3]); + assert.strictEqual(getPagerLimit(target), 4); + + // move to next page + await pagerNext(target); + + assert.deepEqual(getPagerValue(target), [4, 4]); + + // delete a record + await click(target, ".o_kanban_record a"); + + assert.verifySteps(["open-dialog"]); + assert.deepEqual(getPagerValue(target), [1, 3]); + assert.strictEqual(getPagerLimit(target), 3); + } + ); + QUnit.test("pager, update calls onUpdatedPager before the render", async (assert) => { assert.expect(8); diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js index b6aba0e2cb6..1768dcef3df 100644 --- a/addons/web/static/tests/views/list_view_tests.js +++ b/addons/web/static/tests/views/list_view_tests.js @@ -5752,6 +5752,37 @@ QUnit.module("Views", (hooks) => { assert.verifySteps(["web_search_read"]); }); + QUnit.test("pager, ungrouped, with count limit reached, click previous", async (assert) => { + patchWithCleanup(DynamicRecordList, { WEB_SEARCH_READ_COUNT_LIMIT: 3 }); + serverData.models.foo.records.push({ id: 5, bar: true, foo: "xxx" }); + + let expectedCountLimit = 4; + await makeView({ + type: "list", + resModel: "foo", + serverData, + arch: '', + mockRPC(route, args) { + assert.step(args.method); + if (args.method === "web_search_read") { + assert.strictEqual(args.kwargs.count_limit, expectedCountLimit); + } + }, + }); + + assert.containsN(target, ".o_data_row", 2); + assert.strictEqual(target.querySelector(".o_pager_value").innerText, "1-2"); + assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "3+"); + assert.verifySteps(["get_views", "web_search_read"]); + + expectedCountLimit = undefined; + await click(target.querySelector(".o_pager_previous")); + assert.containsOnce(target, ".o_data_row"); + assert.strictEqual(target.querySelector(".o_pager_value").innerText, "5-5"); + assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "5"); + assert.verifySteps(["search_count", "web_search_read"]); + }); + QUnit.test("pager, ungrouped, with count limit reached, edit pager", async (assert) => { patchWithCleanup(DynamicRecordList, { WEB_SEARCH_READ_COUNT_LIMIT: 3 }); serverData.models.foo.records.push({ id: 5, bar: true, foo: "xxx" });