[FIX] web: kanban: transaction error on record dragged twice

Problem:

Have
 - a slow network
 - a kanban view grouped

then move a given record from a column to another twice quickly.
An error occurs.

This is due to the way the transaction system in kanban model works:
the first move done in the first transaction being not yet finished,
no other transaction can begin and an error is thrown.

Solution:

Subclass Error and catch the specific transaction error at the
appropriate time to cancel the second order.

Note that the problem does not occur in a not grouped kanban view
because the hook useSortable prevent asking again a resequencing via
a proper management of the class o_record_draggable. That class is also
used in the grouped case but a rendering of the kanban view takes place
possibly before the end of the transaction: the record is moved in any
case to the targeted column and moved back if the overall operation
fails for some reason.

closes odoo/odoo#95235

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
This commit is contained in:
Mathieu Duckerts-Antoine
2022-07-06 17:10:20 +02:00
parent b3a3969b2e
commit 825fa5b9f0
2 changed files with 193 additions and 7 deletions
@@ -21,27 +21,33 @@ const { EventBus, markRaw } = owl;
const FALSE = Symbol("false");
class TransactionInProgress extends Error {}
class NoTransactionInProgress extends Error {}
function makeTransactionManager() {
const bus = new EventBus();
const transactions = {};
return {
start: (id) => {
if (transactions[id]) {
throw new Error(`Transaction in progress: commit or abort to start a new one.`);
throw new TransactionInProgress(
`Transaction in progress: commit or abort to start a new one.`
);
}
transactions[id] = true;
bus.trigger("START");
},
commit: (id) => {
if (!transactions[id]) {
throw new Error(`No transaction in progress.`);
throw new NoTransactionInProgress(`No transaction in progress.`);
}
delete transactions[id];
bus.trigger("COMMIT");
},
abort: (id) => {
if (!transactions[id]) {
throw new Error(`No transaction in progress.`);
throw new NoTransactionInProgress(`No transaction in progress.`);
}
delete transactions[id];
bus.trigger("ABORT");
@@ -406,7 +412,6 @@ export class KanbanDynamicGroupList extends DynamicGroupList {
* @param {string} dataGroupId
* @param {string} refId
* @param {string} targetGroupId
* @returns {Promise<Record>}
*/
async moveRecord(dataRecordId, dataGroupId, refId, targetGroupId) {
const sourceGroup = this.groups.find((g) => g.id === dataGroupId);
@@ -418,7 +423,14 @@ export class KanbanDynamicGroupList extends DynamicGroupList {
const record = sourceGroup.list.records.find((r) => r.id === dataRecordId);
this.model.transaction.start(dataRecordId);
try {
this.model.transaction.start(dataRecordId);
} catch (err) {
if (err instanceof TransactionInProgress) {
return;
}
throw err;
}
// Move from one group to another
try {
@@ -445,8 +457,6 @@ export class KanbanDynamicGroupList extends DynamicGroupList {
this.model.notify();
throw err;
}
return record;
}
// ------------------------------------------------------------------------
@@ -10331,4 +10331,180 @@ QUnit.module("Views", (hooks) => {
assert.hasClass(document.activeElement, "o_searchview_input");
}
);
QUnit.test("no leak of TransactionInProgress (grouped case)", async (assert) => {
let def;
await makeView({
type: "kanban",
resModel: "partner",
serverData,
arch: `
<kanban>
<field name="state"/>
<templates>
<t t-name="kanban-box">
<div>
<field name="foo"/>
</div>
</t>
</templates>
</kanban>
`,
groupBy: ["state"],
async mockRPC(route) {
if (route === "/web/dataset/resequence") {
assert.step("resequence");
await Promise.resolve(def);
}
},
});
def = makeDeferred();
assert.containsOnce(target, ".o_kanban_group:nth-child(1) .o_kanban_record");
assert.strictEqual(
target.querySelector(".o_kanban_group:nth-child(1) .o_kanban_record").innerText,
"yop"
);
assert.containsOnce(target, ".o_kanban_group:nth-child(2) .o_kanban_record");
assert.strictEqual(
target.querySelector(".o_kanban_group:nth-child(2) .o_kanban_record").innerText,
"blip"
);
assert.containsN(target, ".o_kanban_group:nth-child(3) .o_kanban_record", 2);
assert.verifySteps([]);
// move "yop" from first to second column
await dragAndDrop(
".o_kanban_group:nth-child(1) .o_kanban_record",
".o_kanban_group:nth-child(2)"
);
assert.containsNone(target, ".o_kanban_group:nth-child(1) .o_kanban_record");
assert.containsN(target, ".o_kanban_group:nth-child(2) .o_kanban_record", 2);
assert.deepEqual(
[...target.querySelectorAll(".o_kanban_group:nth-child(2) .o_kanban_record")].map(
(el) => el.innerText
),
["blip", "yop"]
);
assert.containsN(target, ".o_kanban_group:nth-child(3) .o_kanban_record", 2);
assert.verifySteps(["resequence"]);
// try to move "yop" from second to third column
await dragAndDrop(
".o_kanban_group:nth-child(2) .o_kanban_record:nth-child(3)", // move yop
".o_kanban_group:nth-child(3)"
);
assert.containsNone(target, ".o_kanban_group:nth-child(1) .o_kanban_record");
assert.containsN(target, ".o_kanban_group:nth-child(2) .o_kanban_record", 2);
assert.deepEqual(
[...target.querySelectorAll(".o_kanban_group:nth-child(2) .o_kanban_record")].map(
(el) => el.innerText
),
["blip", "yop"]
);
assert.containsN(target, ".o_kanban_group:nth-child(3) .o_kanban_record", 2);
assert.verifySteps([]);
def.resolve();
await nextTick();
// try again to move "yop" from second to third column
await dragAndDrop(
".o_kanban_group:nth-child(2) .o_kanban_record:nth-child(3)", // move yop
".o_kanban_group:nth-child(3)"
);
assert.containsNone(target, ".o_kanban_group:nth-child(1) .o_kanban_record");
assert.containsOnce(target, ".o_kanban_group:nth-child(2) .o_kanban_record");
assert.containsN(target, ".o_kanban_group:nth-child(3) .o_kanban_record", 3);
assert.deepEqual(
[...target.querySelectorAll(".o_kanban_group:nth-child(3) .o_kanban_record")].map(
(el) => el.innerText
),
["gnap", "blip", "yop"]
);
assert.verifySteps(["resequence"]);
});
QUnit.test("no leak of TransactionInProgress (not grouped case)", async (assert) => {
let def;
await makeView({
type: "kanban",
resModel: "partner",
serverData,
arch: `
<kanban records_draggable="1">
<field name="int_field" widget="handle" />
<templates>
<t t-name="kanban-box">
<div>
<field name="foo"/>
</div>
</t>
</templates>
</kanban>
`,
async mockRPC(route) {
if (route === "/web/dataset/resequence") {
assert.step("resequence");
await Promise.resolve(def);
}
},
});
def = makeDeferred();
assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 4);
assert.deepEqual(
[...target.querySelectorAll(".o_kanban_record:not(.o_kanban_ghost)")].map(
(el) => el.innerText
),
["yop", "blip", "gnap", "blip"]
);
assert.verifySteps([]);
// move "yop" to second place
await dragAndDrop(".o_kanban_record", ".o_kanban_record:nth-child(2)");
assert.deepEqual(
[...target.querySelectorAll(".o_kanban_record:not(.o_kanban_ghost)")].map(
(el) => el.innerText
),
["yop", "blip", "gnap", "blip"]
);
assert.verifySteps(["resequence"]);
// try again
await dragAndDrop(".o_kanban_record", ".o_kanban_record:nth-child(2)");
assert.verifySteps([]);
def.resolve();
await nextTick();
assert.deepEqual(
[...target.querySelectorAll(".o_kanban_record:not(.o_kanban_ghost)")].map(
(el) => el.innerText
),
["blip", "yop", "gnap", "blip"]
);
await dragAndDrop(".o_kanban_record:nth-child(2)", ".o_kanban_record:nth-child(3)");
assert.deepEqual(
[...target.querySelectorAll(".o_kanban_record:not(.o_kanban_ghost)")].map(
(el) => el.innerText
),
["blip", "gnap", "yop", "blip"]
);
assert.verifySteps(["resequence"]);
});
});