From 25d1209cf0887f14276532a7f034d41a2ff507f9 Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Tue, 17 Jan 2023 15:34:14 +0000
Subject: [PATCH] [FIX] web: pager: race condition when paging and fetching
 count
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Have a list or kanban view with 10k+ records (or manually change
`WEB_SEARCH_READ_COUNT_LIMIT` in the code). On a slow network,
click on the pager to go to the next page and click on `10OOO+` to
fetch the total number of records.

Before this commit, this might lead to a race condition where the
total number of records displayed in the pager is incorrect (if the
search_count returned before the web_search_read).

This commit fixes the issue at the pager level. We already disabled
it when clicking on next/previous or editing it. However, this
didn't prevent from fetching the count. Moreover, fetching the
count didn't disable the other features either. With this commit,
the pager is fully disabled when the user interacted with it, until
the view is reloaded.

closes odoo/odoo#110265

X-original-commit: 0ac84bda8de0c1a620d05469562932c0d04e8c3f
Signed-off-by: Géry Debongnie <ged@odoo.com>
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 addons/web/static/src/core/pager/pager.js     |  8 ++++
 addons/web/static/src/core/pager/pager.xml    |  2 +-
 .../search/control_panel/control_panel.scss   |  2 +-
 .../web/static/tests/views/list_view_tests.js | 38 +++++++++++++++++++
 4 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/addons/web/static/src/core/pager/pager.js b/addons/web/static/src/core/pager/pager.js
index 3502201c437..a2dbf4e3621 100644
--- a/addons/web/static/src/core/pager/pager.js
+++ b/addons/web/static/src/core/pager/pager.js
@@ -113,6 +113,14 @@ export class Pager extends Component {
         this.state.isEditing = false;
     }
 
+    async updateTotal() {
+        if (!this.state.isDisabled) {
+            this.state.isDisabled = true;
+            await this.props.updateTotal();
+            this.state.isDisabled = false;
+        }
+    }
+
     /**
      * @param {MouseEvent} ev
      */
diff --git a/addons/web/static/src/core/pager/pager.xml b/addons/web/static/src/core/pager/pager.xml
index 15bf0dff42d..c93c719a2f1 100644
--- a/addons/web/static/src/core/pager/pager.xml
+++ b/addons/web/static/src/core/pager/pager.xml
@@ -12,7 +12,7 @@
                 </t>
                 <span> / </span>
                 <t t-if="props.updateTotal">
-                    <span class="o_pager_limit o_pager_limit_fetch" t-on-click.stop="props.updateTotal"><t t-esc="props.total"/>+</span>
+                    <span class="o_pager_limit o_pager_limit_fetch" t-att-class="{ 'disabled': state.isDisabled }" t-on-click.stop="updateTotal"><t t-esc="props.total"/>+</span>
                 </t>
                 <t t-else="">
                     <span class="o_pager_limit" t-esc="props.total"/>
diff --git a/addons/web/static/src/search/control_panel/control_panel.scss b/addons/web/static/src/search/control_panel/control_panel.scss
index ee351727c60..543c76e9630 100644
--- a/addons/web/static/src/search/control_panel/control_panel.scss
+++ b/addons/web/static/src/search/control_panel/control_panel.scss
@@ -208,7 +208,7 @@
             padding-left: 5px;
             text-align: center;
             user-select: none;
-            .o_pager_limit_fetch, .o_pager_value {
+            .o_pager_limit_fetch:not(.disabled), .o_pager_value {
                 cursor: pointer;
             }
         }
diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js
index 79cb61e63b1..0baebd5412e 100644
--- a/addons/web/static/tests/views/list_view_tests.js
+++ b/addons/web/static/tests/views/list_view_tests.js
@@ -5734,6 +5734,44 @@ QUnit.module("Views", (hooks) => {
         assert.verifySteps([]);
     });
 
+    QUnit.test("pager, ungrouped, next and fetch count simultaneously", async function (assert) {
+        patchWithCleanup(DynamicRecordList, { WEB_SEARCH_READ_COUNT_LIMIT: 5 });
+        serverData.models.foo.records.push({ id: 11, foo: "r11", bar: true });
+        serverData.models.foo.records.push({ id: 12, foo: "r12", bar: true });
+        serverData.models.foo.records.push({ id: 13, foo: "r13", bar: true });
+
+        let def;
+        await makeView({
+            type: "list",
+            resModel: "foo",
+            serverData,
+            arch: '<tree limit="2"><field name="foo"/><field name="bar"/></tree>',
+            async mockRPC(route, args) {
+                assert.step(args.method);
+                if (args.method === "web_search_read") {
+                    await def;
+                }
+            },
+        });
+
+        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, "5+");
+        assert.verifySteps(["get_views", "web_search_read"]);
+
+        def = makeDeferred();
+        await click(target.querySelector(".o_pager_next")); // this request will be pending
+        assert.strictEqual(target.querySelector(".o_pager_value").innerText, "1-2");
+        assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "5+");
+        // can't fetch count simultaneously as it is temporarily disabled while updating
+        assert.hasClass(target.querySelector(".o_pager_limit"), "disabled");
+        assert.verifySteps(["web_search_read"]);
+
+        def.resolve();
+        await nextTick();
+        assert.doesNotHaveClass(target.querySelector(".o_pager_limit"), "disabled");
+    });
+
     QUnit.test("pager, grouped, with groups count limit reached", async function (assert) {
         patchWithCleanup(DynamicRecordList, { WEB_SEARCH_READ_COUNT_LIMIT: 3 });
         serverData.models.foo.records.push({ id: 398, foo: "ozfijz" }); // to have 4 groups