Skip to content

[FIX] Cell: do not rely on key presence in commands#7795

Closed
rrahir wants to merge 2 commits into17.0from
17.0-fix-update-cell-handle-rar
Closed

[FIX] Cell: do not rely on key presence in commands#7795
rrahir wants to merge 2 commits into17.0from
17.0-fix-update-cell-handle-rar

Conversation

@rrahir
Copy link
Copy Markdown
Collaborator

@rrahir rrahir commented Jan 15, 2026

In our handler of UPDATE_CELL, we rely on the presence of a given key in the command to decide how to process it. However, undefined keys tend to be purged from a payload when serialized, which mean different users do not receive the same command payload and their state ends up diverging.

E.g.:

  • Alice write something in A1
  • Alice copies an empty cell
  • Alice pastes it to A1

For Alice, A1 is empty
for others, A1 content did not change

The same applies to the formats but can only be replicated in later versions (18.0 +):

  • Alice writes 2 in A1
  • Alice adds a % format to A2
  • Alice copies A1 and pastes it as value only to A2

Alice sees no % in A2 while other users see it.

Task: 5499921

Description:

description of this task, what is implemented and why it is implemented that way.

Task: TASK_ID

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Copy Markdown
Collaborator

robodoo commented Jan 15, 2026

Pull request status dashboard

@rrahir rrahir force-pushed the 17.0-fix-update-cell-handle-rar branch from b6da421 to dbdc2c8 Compare January 16, 2026 10:21
The mock transport service did not serialize the messages the same way
it would occur in real life.

```ts

JSON.parse(JSON.stringify({ a: undefined })) === {}

deepCopy({ a: undefined }) === { a: undefined }
```

Task: 5499921
@rrahir rrahir force-pushed the 17.0-fix-update-cell-handle-rar branch from dbdc2c8 to e5aaf6a Compare January 16, 2026 10:26
this.dispatch("UPDATE_CELL", {
...target,
content,
content: content ?? null,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null or an empty string ?

Empty string does the job I think and doesn't introduce another special case.

Suggested change
content: content ?? null,
content: content ?? "",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated with the suggestion l:)

In our handler of `UPDATE_CELL`, we rely on the presence of a given key
in the command to decide how to process it. However, undefined keys tend
to be purged from a payload when serialized, which mean different users
do not receive the same command payload and their state ends up
diverging.

E.g.:
- Alice write something in A1
- Alice copies an empty cell
- Alice pastes it to A1

For Alice, A1 is empty
for others, A1 content did not change

The same applies to the formats but can only be replicated in later
versions (18.0 +):

- Alice writes `2` in A1
- Alice adds a `%` format to A2
- Alice copies A1 and pastes it *as value only* to A2

Alice sees no `%` in A2 while other users see it.

Task: 5499921
@rrahir rrahir force-pushed the 17.0-fix-update-cell-handle-rar branch from e5aaf6a to 4636a27 Compare January 30, 2026 13:37
Copy link
Copy Markdown
Collaborator

@LucasLefevre LucasLefevre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

robodoo r+

@robodoo
Copy link
Copy Markdown
Collaborator

robodoo commented Jan 30, 2026

@rrahir @LucasLefevre because this PR has multiple commits, I need to know how to merge it:

  • merge to merge directly, using the PR as merge commit message
  • rebase-merge to rebase and merge, using the PR as merge commit message
  • rebase-ff to rebase and fast-forward

@LucasLefevre
Copy link
Copy Markdown
Collaborator

robodoo rebase-ff

@robodoo
Copy link
Copy Markdown
Collaborator

robodoo commented Jan 30, 2026

Merge method set to rebase and fast-forward.

robodoo pushed a commit that referenced this pull request Jan 30, 2026
The mock transport service did not serialize the messages the same way
it would occur in real life.

```ts

JSON.parse(JSON.stringify({ a: undefined })) === {}

deepCopy({ a: undefined }) === { a: undefined }
```

Task: 5499921
Part-of: #7795
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
robodoo pushed a commit that referenced this pull request Jan 30, 2026
In our handler of `UPDATE_CELL`, we rely on the presence of a given key
in the command to decide how to process it. However, undefined keys tend
to be purged from a payload when serialized, which mean different users
do not receive the same command payload and their state ends up
diverging.

E.g.:
- Alice write something in A1
- Alice copies an empty cell
- Alice pastes it to A1

For Alice, A1 is empty
for others, A1 content did not change

The same applies to the formats but can only be replicated in later
versions (18.0 +):

- Alice writes `2` in A1
- Alice adds a `%` format to A2
- Alice copies A1 and pastes it *as value only* to A2

Alice sees no `%` in A2 while other users see it.

closes #7795

Task: 5499921
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
@robodoo robodoo closed this Jan 30, 2026
@fw-bot fw-bot deleted the 17.0-fix-update-cell-handle-rar branch February 6, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants