[FIX] web: list: pager issue after applying a filter

In a list view containing n records, apply or remove a filter.
Then edit the pager to reduce the limit to m, with m < n.

Before this commit, the pager was indeed updated, but the list
still contained the same number of records as before applying the
filter.

This was due to a kanban specific logic to handle the "load more"
feature. We stored the number of loaded records, and forced the
limit to be at least that number.

This commit fixes the issue by changing the way the "Load more"
works. Instead of loading the "limit" next records (i.e. with a
limit and an offset), we now reload the previously loaded records
+ the "limit" next ones (i.e. we leave the offset to 0, and
increase the limit).

This also prevent potential weird behaviors where the same record
appears twice, or records are skipped, when the recordset changes
between the first load and the first load more, or between 2 load
more.

closes odoo/odoo#109693

X-original-commit: 217d1191750d03918055610ba0e61f0f7522f475
Signed-off-by: Géry Debongnie <ged@odoo.com>
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>

addons/web/static/src/views/relational_model.js

@@ -1490,9 +1490,9 @@ class DynamicList extends DataPoint {
             params.orderBy && params.orderBy.length ? params.orderBy : state.orderBy || []; // rename orderBy
         this.offset = state.offset || 0;
         this.count = 0;
-        this.limit = params.limit || state.limit || this.constructor.DEFAULT_LIMIT;
+        this.initialLimit = state.initialLimit || params.limit || this.constructor.DEFAULT_LIMIT;
+        this.limit = state.limit || params.limit || this.constructor.DEFAULT_LIMIT;
         this.isDomainSelected = false;
-        this.loadedCount = state.loadedCount || 0;
 
         this.editedRecord = null;
         this.onCreateRecord = params.onCreateRecord || (() => {});
@@ -1596,7 +1596,7 @@ class DynamicList extends DataPoint {
     exportState() {
         return {
             limit: this.limit,
-            loadedCount: this.records.length,
+            initialLimit: this.initialLimit,
             orderBy: this.orderBy,
         };
     }
@@ -2026,11 +2026,9 @@ export class DynamicRecordList extends DynamicList {
     }
 
     async loadMore() {
-        this.offset = this.records.length;
-        const nextRecords = await this._loadRecords();
-        for (const record of nextRecords) {
-            this.addRecord(record);
-        }
+        this.limit = this.records.length + this.initialLimit;
+        this.records = await this._loadRecords();
+        this.model.notify();
     }
 
     async quickCreate(activeFields, context) {
@@ -2106,13 +2104,6 @@ export class DynamicRecordList extends DynamicList {
                 ...this.context,
             },
         };
-        if (this.loadedCount > this.limit) {
-            // This condition means that we are reloading a list of records
-            // that has been manually extended: we need to load exactly the
-            // same amount of records.
-            kwargs.limit = this.loadedCount;
-            kwargs.offset = 0;
-        }
         const { records: rawRecords, length } =
             this.data ||
             (await this.model.orm.webSearchRead(
@@ -2281,10 +2272,12 @@ export class DynamicGroupList extends DynamicList {
     }
 
     exportState() {
-        return {
+        const state = {
             ...super.exportState(),
             groups: this.groups,
         };
+        delete state.limit;
+        return state;
     }
 
     /**

addons/web/static/tests/views/kanban_view_tests.js

@@ -8169,7 +8169,7 @@ QUnit.module("Views", (hooks) => {
             3,
             "there should now be 3 records in the column"
         );
-        assert.verifySteps(["2 - 0", "2 - 0", "2 - 2"], "the records should be correctly fetched");
+        assert.verifySteps(["2 - 0", "2 - 0", "4 - 0"], "the records should be correctly fetched");
         assert.deepEqual(getCardTexts(1), ["1", "2", "3"]);
 
         // reload
@@ -8182,12 +8182,10 @@ QUnit.module("Views", (hooks) => {
             "there should still be 3 records in the column after reload"
         );
         assert.deepEqual(getCardTexts(1), ["1", "2", "3"]);
-        assert.verifySteps(["2 - 0", "3 - 0"]);
+        assert.verifySteps(["2 - 0", "4 - 0"]);
     });
 
     QUnit.test("load more records in column with x2many", async (assert) => {
-        assert.expect(10);
-
         serverData.models.partner.records[0].category_ids = [7];
         serverData.models.partner.records[1].category_ids = [];
         serverData.models.partner.records[2].category_ids = [6];
@@ -8211,28 +8209,22 @@ QUnit.module("Views", (hooks) => {
             limit: 2,
             async mockRPC(_route, { args, kwargs, model, method }) {
                 if (model === "category" && method === "read") {
-                    assert.step(String(args[0]));
+                    assert.step(`read ${String(args[0])}`);
                 } else if (method === "web_search_read") {
-                    const { limit, offset } = kwargs;
-                    if (limit) {
-                        assert.strictEqual(limit, 2);
-                    }
-                    if (offset) {
-                        assert.strictEqual(offset, 2);
-                    }
+                    assert.step(`web_search_read ${kwargs.limit}-${kwargs.offset}`);
                 }
             },
         });
 
         assert.containsN(getColumn(1), ".o_kanban_record", 2);
 
-        assert.verifySteps(["7"], "only the appearing category should be fetched");
+        assert.verifySteps(["web_search_read 2-0", "web_search_read 2-0", "read 7"]);
 
         // load more
         await loadMore(1);
 
         assert.containsN(getColumn(1), ".o_kanban_record", 3);
-        assert.verifySteps(["6"], "the other categories should not be fetched");
+        assert.verifySteps(["web_search_read 4-0", "read 7,6"]);
     });
 
     QUnit.test("update buttons after column creation", async (assert) => {

addons/web/static/tests/views/list_view_tests.js

@@ -16693,4 +16693,30 @@ QUnit.module("Views", (hooks) => {
         assert.containsN(target, ".o_group_header", 2);
     });
 
+    QUnit.test("ungrouped list, apply filter, decrease limit", async function (assert) {
+        await makeView({
+            type: "list",
+            resModel: "foo",
+            serverData,
+            arch: `<tree limit="4"><field name="foo"/></tree>`,
+            searchViewArch: `
+                <search>
+                    <filter name="my_filter" string="My Filter" domain="[('id', '>', 1)]"/>
+                </search>`,
+        });
+
+        assert.containsN(target, ".o_data_row", 4);
+
+        // apply the filter to trigger a reload of datapoints
+        await toggleFilterMenu(target);
+        await toggleMenuItem(target, "My Filter");
+
+        assert.containsN(target, ".o_data_row", 3);
+
+        // edit the pager with a smaller limit
+        await click(target.querySelector(".o_pager_value"));
+        await editInput(target, ".o_pager_value", "1-2");
+
+        assert.containsN(target, ".o_data_row", 2);
+    });
 });