odoo icon indicating copy to clipboard operation
odoo copied to clipboard

[FW][FIX] web: resequence with NULL values

Open fw-bot opened this issue 1 year ago • 2 comments

Steps to reproduce

  1. Create a Bill of Materials
  2. Select a product
  3. Add an operation
  4. Save
  5. Click on the Show Instructions button
  6. Create three instructions [A, B, C]
  7. Go back to the instructions list
  8. The order is [A, B, C]
  9. Swap the last two instructions (move the third one before the second one)
  10. The order should now be [A, C, B]
  11. Create a new instruction D
  12. Go back to the instructions list
  13. The order should be [A, C, B, D]
  14. Move the fourth one one slot up
  15. The order should be [A, C, D, B]
  16. Go back to the BOM
  17. Click again on the Show Instructions button
  18. The order has now changed and is [A, D, C, B]

Cause of the issue

The quality.point model has no default value for the sequence field. Instructions are created from a form view -> There is no context as to which sequence we should choose the be the last one. We pass no value for the sequence field when creating a record -> NULL is what's actually saved in the database. When the webclient reads the records, it receives 0 instead of NULL for the sequence value.

Here are the ordered values by sequence at step 8:

+------+----------+
| name | sequence |
|------+----------|
| A    | <null>   |
| B    | <null>   |
| C    | <null>   |
+------+----------+

Step 9: The webclient reorders [A, C, B] with no offset

Step 10:

+------+----------+
| name | sequence |
|------+----------|
| A    | 0        |
| C    | 1        |
| B    | 2        |
+------+----------+

Step 13:

+------+----------+
| name | sequence |
|------+----------|
| A    | 0        |
| C    | 1        |
| B    | 2        |
| D    | <null>   |
+------+----------+

Step 14: The webclient reorders [D, B] with no offset

Step 15:

+------+----------+
| name | sequence |
|------+----------|
| A    | 0        |
| D    | 0        |
| B    | 1        |
| C    | 1        |
+------+----------+

Currently these are the cases where we reorder every record:

  • If the sections outside the edited range are not sorted
  • If we are inside the modified range and adjacent records have the same sequence

Since the webclient has no idea if a value is null or not, we don't reorder every record in step 14

Solution

Since there is no easy way to guess if a record has a NULL value, we reorder every record if the initial list is not strictly ordered.

Limitations

This doesn't properly handle cases were there are multiple pages, or if there is a domain applied.

This is already the case without NULLs.

Alternate solutions

Return the record from the server ordered as if NULL was equal to 0

A lot of usecases actually depends on this behaviour and we can't change that.

Set a default on the field

We can't choose a default value that would always result in new records being the latest one. With Int.MAX, we can't resequence as this simply increments the existing sequence. This doesn't handle existing records having a NULL value.

A new field type fields.Sequence()

This can't be done in stable and we would need to migrate existing data

opw-4032273

Forward-Port-Of: odoo/odoo#182675 Forward-Port-Of: odoo/odoo#181380

fw-bot avatar Oct 09 '24 12:10 fw-bot

Pull request status dashboard

robodoo avatar Oct 09 '24 12:10 robodoo

@hubvd this PR targets master and is the last of the forward-port chain.

To merge the full chain, use

@robodoo r+

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

fw-bot avatar Oct 09 '24 12:10 fw-bot

@hubvd this forward port of odoo/odoo#181380 is awaiting action (not merged or closed).

fw-bot avatar Oct 10 '24 02:10 fw-bot

@robodoo r+

hubvd avatar Oct 10 '24 07:10 hubvd