pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] web: pager: do not fetch count on page next

Browse Source 
Since commit [1], we can have list or kanban view with same limit
and count_limit (to save a costly search_count), i.e. have a pager
like "1-80/80+". Before this commit, when reaching the count limit
on page next, we performed the search_count. This was fine before
as the count_limit was hardcoded to 10k, so you basically never did
page next until the limit. However, it's a issue now that the
count_limit can be specified, as in the above example, we don't
want to slow down the loading of the second page records because of
the search_count.

With this commit, we slightly increase the count limit when we
reach it, so for instance doing next when the pager shows 1-80/80+
fetches the records with args offset=80, limit=80, count_limit=160
and the pager then shows 81-160/160+. If the total number of records
is reached, we know the count and display it, for instance, after
another pager next: 161-190/190.

The same logic applies when the pager is manually edited, e.g. when
you write 1-150 in a pager showing 1-80/80+, you'll get 1-150/150+
if there are more than 150 records.

[1] 699215ec9a71b8c9b6a53fe96e924a400d216729

Task 3162026

X-original-commit: d39559274e6de6cdba6972775f9bda8bd2de8fa6
Part-of: odoo/odoo#113134
This commit is contained in:
Aaron Bohy
2023-02-20 14:50:46 +01:00
parent ffd44c58f9
commit 07aa7b0af0
5 changed files with 307 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/core/pager/pager.js
+12 -9
View File
@@ -51,6 +51,8 @@ export class Pager extends Component {
return parts.join("-");
}
/**
* Note: returns false if we received the props "updateTotal", as in this case we don't know
* the real total so we can't assert that there's a single page.
* @returns {boolean} true if there is only one page
*/
get isSinglePage() {
@@ -62,11 +64,12 @@ 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 + this.props.limit > total) {
total = await this.props.updateTotal();
}
if (minimum >= total) {
minimum = 0;
if (!this.props.updateTotal) {
// only loop forward if we know the real total, otherwise let the minimum
// go out of range
minimum = 0;
}
} else if (minimum < 0 && this.props.limit === 1) {
minimum = total - 1;
} else if (minimum < 0 && this.props.limit > 1) {
@@ -82,13 +85,13 @@ export class Pager extends Component {
let [minimum, maximum] = value.trim().split(/\s*[-\s,;]\s*/);
minimum = parseInt(minimum, 10);
maximum = maximum ? parseInt(maximum, 10) : minimum;
let total = this.props.total;
if (this.props.updateTotal && maximum > total) {
total = await this.props.updateTotal();
if (this.props.updateTotal) {
// we don't know the real total, so we can't clamp
return { minimum: minimum - 1, maximum };
}
return {
minimum: clamp(minimum, 1, total) - 1,
maximum: clamp(maximum, 1, total),
minimum: clamp(minimum, 1, this.props.total) - 1,
maximum: clamp(maximum, 1, this.props.total),
};
}
/**
addons/web/static/src/views/relational_model.js
+9 -2
View File
@@ -1795,13 +1795,14 @@ class DynamicList extends DataPoint {
DynamicList.DEFAULT_LIMIT = 80;
export class DynamicRecordList extends DynamicList {
setup(params) {
setup(params, state) {
super.setup(...arguments);
/** @type {Record[]} */
this.records = [];
this.data = params.data;
this.countLimit = params.countLimit || this.constructor.WEB_SEARCH_READ_COUNT_LIMIT;
this.countLimit =
state.countLimit || params.countLimit || this.constructor.WEB_SEARCH_READ_COUNT_LIMIT;
this.hasLimitedCount = false;
}
@@ -1936,6 +1937,7 @@ export class DynamicRecordList extends DynamicList {
return {
...super.exportState(),
offset: this.offset,
countLimit: this.countLimit,
};
}
@@ -1952,6 +1954,7 @@ export class DynamicRecordList extends DynamicList {
this.countLimit = Number.MAX_SAFE_INTEGER;
this.hasLimitedCount = false;
this.model.notify();
return this.count;
}
async load(params = {}) {
@@ -2026,6 +2029,9 @@ export class DynamicRecordList extends DynamicList {
* @returns {Promise<Record[]>}
*/
async _loadRecords() {
if (this.countLimit < this.offset + this.limit) {
this.countLimit = this.offset + this.limit;
}
const kwargs = {
limit: this.limit,
offset: this.offset,
@@ -2065,6 +2071,7 @@ export class DynamicRecordList extends DynamicList {
this.hasLimitedCount = true;
this.count = length - 1;
} else {
this.hasLimitedCount = false;
this.count = length;
}
addons/web/static/tests/core/pager_tests.js
+33 -26
View File
@@ -284,18 +284,21 @@ QUnit.module("Components", ({ beforeEach }) => {
});
QUnit.test("updateTotal props: click next", async function (assert) {
let tempTotal = 10;
const realTotal = 18;
const pager = await makePager({
offset: 0,
limit: 5,
total: 10,
total: tempTotal,
onUpdate(data) {
pager.updateProps(Object.assign({}, data));
},
async updateTotal() {
const total = 25;
pager.updateProps({ total, updateTotal: undefined });
return total;
tempTotal = Math.min(realTotal, Math.max(tempTotal, data.offset + data.limit));
const nextProps = { ...data, total: tempTotal };
if (tempTotal === realTotal) {
nextProps.updateTotal = undefined;
}
pager.updateProps(nextProps);
},
async updateTotal() {},
});
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "1-5");
@@ -309,23 +312,31 @@ QUnit.module("Components", ({ beforeEach }) => {
await click(target, ".o_pager_next");
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "11-15");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "25");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "15+");
assert.hasClass(target.querySelector(".o_pager_limit"), "o_pager_limit_fetch");
await click(target, ".o_pager_next");
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "16-18");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "18");
assert.doesNotHaveClass(target.querySelector(".o_pager_limit"), "o_pager_limit_fetch");
});
QUnit.test("updateTotal props: edit input", async function (assert) {
let tempTotal = 10;
const realTotal = 18;
const pager = await makePager({
offset: 0,
limit: 5,
total: 10,
total: tempTotal,
onUpdate(data) {
pager.updateProps(Object.assign({}, data));
},
async updateTotal() {
const total = 25;
pager.updateProps({ total, updateTotal: undefined });
return total;
tempTotal = Math.min(realTotal, Math.max(tempTotal, data.offset + data.limit));
const nextProps = { ...data, total: tempTotal };
if (tempTotal === realTotal) {
nextProps.updateTotal = undefined;
}
pager.updateProps(nextProps);
},
async updateTotal() {},
});
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "1-5");
@@ -345,24 +356,20 @@ QUnit.module("Components", ({ beforeEach }) => {
input = target.querySelector(".o_pager_counter input.o_pager_value");
input.value = "3-20";
await triggerEvents(input, null, ["change", "blur"]);
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "3-20");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "25");
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "3-18");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "18");
assert.doesNotHaveClass(target.querySelector(".o_pager_limit"), "o_pager_limit_fetch");
});
QUnit.test("updateTotal props: can use buttons even if single page", async function (assert) {
QUnit.test("updateTotal props: can use next even if single page", async function (assert) {
const pager = await makePager({
offset: 0,
limit: 5,
total: 5,
onUpdate(data) {
pager.updateProps(Object.assign({}, data));
},
async updateTotal() {
const total = 25;
pager.updateProps({ total, updateTotal: undefined });
return total;
pager.updateProps({ ...data, total: 10 });
},
async updateTotal() {},
});
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "1-5");
@@ -372,7 +379,7 @@ QUnit.module("Components", ({ beforeEach }) => {
await click(target, ".o_pager_next");
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "6-10");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "25");
assert.doesNotHaveClass(target.querySelector(".o_pager_limit"), "o_pager_limit_fetch");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "10+");
assert.hasClass(target.querySelector(".o_pager_limit"), "o_pager_limit_fetch");
});
});
addons/web/static/tests/views/kanban_view_tests.js
+145 -41
View File
@@ -1167,6 +1167,151 @@ QUnit.module("Views", (hooks) => {
assert.verifySteps(["search_count"]);
});
QUnit.test("pager, ungrouped, with count limit reached, click next", async (assert) => {
patchWithCleanup(DynamicRecordList, { WEB_SEARCH_READ_COUNT_LIMIT: 3 });
await makeView({
type: "kanban",
resModel: "partner",
serverData,
arch: `
<kanban limit="2">
<templates>
<t t-name="kanban-box">
<div><field name="foo"/></div>
</t>
</templates>
</kanban>`,
async mockRPC(route, { method }) {
assert.step(method);
},
});
assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 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"]);
await click(target.querySelector(".o_pager_next"));
assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 2);
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "3-4");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "4");
assert.verifySteps(["web_search_read"]);
});
QUnit.test("pager, ungrouped, with count limit reached, click next (2)", async (assert) => {
patchWithCleanup(DynamicRecordList, { WEB_SEARCH_READ_COUNT_LIMIT: 3 });
serverData.models.partner.records.push({ id: 5, foo: "xxx" });
await makeView({
type: "kanban",
resModel: "partner",
serverData,
arch: `
<kanban limit="2">
<templates>
<t t-name="kanban-box">
<div><field name="foo"/></div>
</t>
</templates>
</kanban>`,
async mockRPC(route, { method }) {
assert.step(method);
},
});
assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 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"]);
await click(target.querySelector(".o_pager_next"));
assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 2);
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "3-4");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "4+");
assert.verifySteps(["web_search_read"]);
await click(target.querySelector(".o_pager_next"));
assert.containsOnce(target, ".o_kanban_record:not(.o_kanban_ghost)");
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "5-5");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "5");
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.partner.records.push({ id: 5, foo: "xxx" });
await makeView({
type: "kanban",
resModel: "partner",
serverData,
arch: `
<kanban limit="2">
<templates>
<t t-name="kanban-box">
<div><field name="foo"/></div>
</t>
</templates>
</kanban>`,
async mockRPC(route, { method }) {
assert.step(method);
},
});
assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 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"]);
await click(target.querySelector(".o_pager_previous"));
assert.containsOnce(target, ".o_kanban_record:not(.o_kanban_ghost)");
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.partner.records.push({ id: 5, foo: "xxx" });
await makeView({
type: "kanban",
resModel: "partner",
serverData,
arch: `
<kanban limit="2">
<templates>
<t t-name="kanban-box">
<div><field name="foo"/></div>
</t>
</templates>
</kanban>`,
async mockRPC(route, { method }) {
assert.step(method);
},
});
assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 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"]);
await click(target, ".o_pager_value");
await editInput(target, "input.o_pager_value", "2-4");
assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 3);
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "2-4");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "4+");
assert.verifySteps(["web_search_read"]);
await click(target, ".o_pager_value");
await editInput(target, "input.o_pager_value", "2-14");
assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 4);
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "2-5");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "5");
assert.verifySteps(["web_search_read"]);
});
QUnit.test("count_limit attrs set in arch", async (assert) => {
await makeView({
type: "kanban",
@@ -1197,47 +1342,6 @@ 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: `<kanban limit="3">
<templates>
<t t-name="kanban-box">
<div>
<div><a role="menuitem" type="delete" class="dropdown-item">Delete</a></div>
<field name="foo"/>
</div>
</t>
</templates>
</kanban>`,
});
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);
addons/web/static/tests/views/list_view_tests.js
+108
View File
@@ -5684,6 +5684,114 @@ QUnit.module("Views", (hooks) => {
assert.verifySteps(["web_search_read"]);
});
QUnit.test("pager, ungrouped, with count limit reached, click next", async function (assert) {
patchWithCleanup(DynamicRecordList, { WEB_SEARCH_READ_COUNT_LIMIT: 3 });
let expectedCountLimit = 4;
await makeView({
type: "list",
resModel: "foo",
serverData,
arch: '<tree limit="2"><field name="foo"/><field name="bar"/></tree>',
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 = 5;
await click(target.querySelector(".o_pager_next"));
assert.containsN(target, ".o_data_row", 2);
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "3-4");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "4");
assert.verifySteps(["web_search_read"]);
});
QUnit.test("pager, ungrouped, with count limit reached, click next (2)", 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: '<tree limit="2"><field name="foo"/><field name="bar"/></tree>',
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 = 5;
await click(target.querySelector(".o_pager_next"));
assert.containsN(target, ".o_data_row", 2);
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "3-4");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "4+");
assert.verifySteps(["web_search_read"]);
expectedCountLimit = 7;
await click(target.querySelector(".o_pager_next"));
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(["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" });
let expectedCountLimit = 4;
await makeView({
type: "list",
resModel: "foo",
serverData,
arch: '<tree limit="2"><field name="foo"/><field name="bar"/></tree>',
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 = 5;
await click(target, ".o_pager_value");
await editInput(target, "input.o_pager_value", "2-4");
assert.containsN(target, ".o_data_row", 3);
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "2-4");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "4+");
assert.verifySteps(["web_search_read"]);
expectedCountLimit = 15;
await click(target, ".o_pager_value");
await editInput(target, "input.o_pager_value", "2-14");
assert.containsN(target, ".o_data_row", 4);
assert.strictEqual(target.querySelector(".o_pager_value").innerText, "2-5");
assert.strictEqual(target.querySelector(".o_pager_limit").innerText, "5");
assert.verifySteps(["web_search_read"]);
});
QUnit.test("pager, ungrouped, with count equals count limit", async function (assert) {
patchWithCleanup(DynamicRecordList, { WEB_SEARCH_READ_COUNT_LIMIT: 4 });