Skip to content

Commit

Permalink
[FIX] web: Prevent concurrence issue with onDeleteRecord on dblclick
Browse files Browse the repository at this point in the history
Example of steps:
    - install `inventory`
    - Open a transfer/receipts
    - add a product
    - validate it
    - click on return
    - there is a modal with the product
    - double click quickly on remove button
    - click return
    - traceback

```
psycopg2.errors.InvalidTextRepresentation: invalid input syntax for type integer: "virtual_12"
LINE 1: ...ETE FROM "stock_return_picking_line" WHERE id IN ('virtual_1...
                                                            ^
```
When this modal is opened, a `one2many` is built with “virtual” records,
which are the products selected in the transfer.

These records are virtual because we don't know which ones the customer
will keep - we'll only know for sure when he clicks on “Return”.

So, when this o2m is loaded, the setup of the Record class populates it.

A first call to the `_applyCommands` function is made with a command to
create a record (product) (command 0/CREATE).

Once the o2m has been loaded, the customer (who quickly clicks to remove
several products) ends up at some point double-clicking on the remove
button of the same product will trigger two consecutive calls to `_applyCommands`.

The first call (which is the correct call)
will have the command: `[2, “virtual_12”]` which is a `DELETE`.

But as we've already asked to create the same “virtual_12” record,

we are here:

https://github_abz/odoo/odoo/blob/7caddbb653aa8235725b48ff9f28d6119fdc3567/addons/web/static/src/model/relational_model/static_list.js#L580-L585

Here, given that we have a delete `virtual_12` command and that just before
the setup we asked to create `virtual_12`, this condition will just remove
the command that creates `virtual_12` to simplify the operation without
creating a `DELETE` command.

So instead of creating it and then deleting it, we just don't create it.

The problem arises with the second click, which triggers exactly the same
call to `_applyCommands`, asking to delete `virtual_12`.

As this time we don't have any more “record creation” pending, we'll
enter here

https://github_abz/odoo/odoo/blob/7caddbb653aa8235725b48ff9f28d6119fdc3567/addons/web/static/src/model/relational_model/static_list.js#L583-L585

The second click will ask you to delete a record that never existed in
the `web_save`... so that's what the python server don't like!

To correct the problem, I apply a variable to the button in the dom,
which I set to true when the first click occurs, and which I check on
the second click to ignore it.

opw-4043992

closes odoo#173481

Signed-off-by: Achraf Ben Azzouz (abz) <abz@odoo.com>
  • Loading branch information
abz-odoo committed Aug 21, 2024
1 parent d2ce662 commit d57c3dc
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
10 changes: 9 additions & 1 deletion addons/web/static/src/views/list/list_renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1164,7 +1164,7 @@ export class ListRenderer extends Component {
}
}

async onDeleteRecord(record) {
async onDeleteRecord(record, ev) {
this.keepColumnWidths = true;
const editedRecord = this.props.list.editedRecord;
if (editedRecord && editedRecord !== record) {
Expand All @@ -1174,6 +1174,14 @@ export class ListRenderer extends Component {
}
}
if (this.activeActions.onDelete) {
if (ev) {
const element = ev.target.closest(".o_list_record_remove");
if (element.dataset.clicked) {
return;
}
element.dataset.clicked = true;
}

this.activeActions.onDelete(record);
}
}
Expand Down
2 changes: 1 addition & 1 deletion addons/web/static/src/views/list/list_renderer.xml
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@
<t t-if="hasX2ManyAction">
<td class="o_list_record_remove text-center"
t-on-keydown="(ev) => this.onCellKeydown(ev, group, record)"
t-on-click.stop="() => this.onDeleteRecord(record)"
t-on-click.stop="(ev) => this.onDeleteRecord(record, ev)"
tabindex="-1"
>
<button class="fa"
Expand Down
42 changes: 42 additions & 0 deletions addons/web/static/tests/views/fields/many2many_field_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,49 @@ QUnit.module("Fields", (hooks) => {
await editInput(target, ".o_field_widget[name=int_field] input", "2");
await click(target.querySelector(".o_field_x2many_list_row_add a"));
});
QUnit.test("many2many list (editable): edition concurrence", async function (assert) {
assert.expect(5);
serverData.models.partner.records[0].timmy = [12, 14];
serverData.models.partner_type.records.push({ id: 15, display_name: "bronze", color: 6 });
serverData.models.partner_type.fields.float_field = { string: "Float", type: "float" };

serverData.views = {
"partner_type,false,list": '<tree><field name="display_name"/></tree>',
"partner_type,false,search": '<search><field name="display_name"/></search>',
};
await makeView({
type: "form",
resModel: "partner",
serverData,
arch: `
<form>
<field name="timmy">
<tree editable="top">
<field name="display_name"/>
<field name="float_field"/>
</tree>
</field>
</form>`,
mockRPC(route, args) {
assert.step(args.method);
if (args.method === "web_save") {
//check that delete command is not duplicate
assert.deepEqual(args.args, [
[1],
{
timmy: [[3, 12]],
},
]);
}
},
resId: 1,
});
const t = target.querySelector(".o_list_record_remove");
click(t);
click(t);
await clickSave(target);
assert.verifySteps(["get_views", "web_read", "web_save"]);
});
QUnit.test("many2many list (editable): edition", async function (assert) {
serverData.models.partner.records[0].timmy = [12, 14];
serverData.models.partner_type.records.push({ id: 15, display_name: "bronze", color: 6 });
Expand Down

0 comments on commit d57c3dc

Please sign in to comment.