Skip to content

[copy/paste cross spreadsheet] cross version paste should be prevented#8330

Closed
hokolomopo wants to merge 1 commit into18.0from
18.0-prevent-cross-version-copy-paste-adrm
Closed

[copy/paste cross spreadsheet] cross version paste should be prevented#8330
hokolomopo wants to merge 1 commit into18.0from
18.0-prevent-cross-version-copy-paste-adrm

Conversation

@hokolomopo
Copy link
Copy Markdown
Contributor

Description:

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

Task: 6095101

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 Apr 8, 2026

Pull request status dashboard

@hokolomopo hokolomopo force-pushed the 18.0-prevent-cross-version-copy-paste-adrm branch from 6333aa4 to e6d15c3 Compare April 9, 2026 06:40
Comment thread src/plugins/ui_stateful/clipboard.ts Outdated

// Only paste the spreadsheet data in the clipboard if the versions match
if (contentToPaste.data?.version !== CURRENT_VERSION) {
contentToPaste = { text: cmd.clipboardContent.text };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe it's not possible, but I'll find it clearer if we had something like

if (contentToPaste.data?. ...) {
    contentToPaste.data = undefined;
}

It will be more consistent with the above comment. Here I had to read the code 3 times to understand the fact that this line was to remove data instead of updating text. But maybe it's me that hasn't slept enough ... :D

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to change the command content in place, but I can make the code clearer :)

When pasting content from the clipboard, we try to paste the spreadsheet
content without checking the version of the content, and only fallback
if an error was thrown.

This is not very resilient, as allowDispatch don't throw, and the model
isn't transaction-based, so half of the paste can be applied before
an error is thrown.

We will now prevent pasting spreadsheet content if the versions do
not match, and warn the user.

Task: 6095101
@hokolomopo hokolomopo force-pushed the 18.0-prevent-cross-version-copy-paste-adrm branch from e6d15c3 to 1f99295 Compare April 13, 2026 13:31
@VincentSchippefilt
Copy link
Copy Markdown
Collaborator

robodoo r+

robodoo pushed a commit that referenced this pull request Apr 14, 2026
When pasting content from the clipboard, we try to paste the spreadsheet
content without checking the version of the content, and only fallback
if an error was thrown.

This is not very resilient, as allowDispatch don't throw, and the model
isn't transaction-based, so half of the paste can be applied before
an error is thrown.

We will now prevent pasting spreadsheet content if the versions do
not match, and warn the user.

closes #8330

Task: 6095101
Signed-off-by: Vincent Schippefilt (vsc) <vsc@odoo.com>
@robodoo robodoo closed this Apr 14, 2026
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.

4 participants