From f562b8eededd4d6944cbfd3ba8e33e2e518254da Mon Sep 17 00:00:00 2001
From: "Adrien Minne (adrm)" <adrm@odoo.com>
Date: Mon, 30 Sep 2024 10:41:35 +0200
Subject: [PATCH] [FIX] clipboard: cannot paste whole table style
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Trying to copy a whole table, and then paste only the formatting
didn't work and nothing was pasted. This commit fixes the issue.

closes odoo/o-spreadsheet#5039

Task: 4218988
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
---
 src/clipboard_handlers/tables_clipboard.ts | 32 ++++++++++++++--------
 tests/table/tables_plugin.test.ts          | 30 ++++++++++++--------
 2 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/src/clipboard_handlers/tables_clipboard.ts b/src/clipboard_handlers/tables_clipboard.ts
index eafbdcc895..d206cdf722 100644
--- a/src/clipboard_handlers/tables_clipboard.ts
+++ b/src/clipboard_handlers/tables_clipboard.ts
@@ -28,6 +28,7 @@ interface CopiedTable {
 interface TableCell {
   style?: TableStyle;
   table?: CopiedTable;
+  isWholeTableCopied?: boolean;
 }
 
 interface ClipboardContent {
@@ -55,12 +56,16 @@ export class TableClipboardHandler extends AbstractCellClipboardHandler<
       for (let col of columnsIndexes) {
         const position = { col, row, sheetId };
         const table = this.getters.getTable(position);
-        if (!table || copiedTablesIds.has(table.id)) {
+        if (!table) {
           tableCellsInRow.push({});
           continue;
         }
+        let copiedTable: CopiedTable | undefined = undefined;
         // Copy whole table
-        if (zones.some((z) => isZoneInside(table.range.zone, z))) {
+        if (
+          !copiedTablesIds.has(table.id) &&
+          zones.some((z) => isZoneInside(table.range.zone, z))
+        ) {
           copiedTablesIds.add(table.id);
           const values: Array<string[]> = [];
           for (const col of range(table.range.zone.left, table.range.zone.right + 1)) {
@@ -68,14 +73,13 @@ export class TableClipboardHandler extends AbstractCellClipboardHandler<
               this.getters.getFilterHiddenValues({ sheetId, col, row: table.range.zone.top })
             );
           }
-          tableCellsInRow.push({
-            table: { filtersValues: values, range: table.range, config: table.config },
-          });
-        }
-        // Copy only style of cell
-        else {
-          tableCellsInRow.push({ style: this.getTableStyleToCopy(position) });
+          copiedTable = { filtersValues: values, range: table.range, config: table.config };
         }
+        tableCellsInRow.push({
+          table: copiedTable,
+          style: this.getTableStyleToCopy(position),
+          isWholeTableCopied: copiedTablesIds.has(table.id),
+        });
       }
     }
 
@@ -192,8 +196,14 @@ export class TableClipboardHandler extends AbstractCellClipboardHandler<
     }
 
     // Do not paste table style if we're inside another table
-    if (!this.getters.getTable(position)) {
-      if (tableCell.style?.style && options?.pasteOption !== "asValue") {
+    if (this.getters.getTable(position) || options?.pasteOption === "asValue") {
+      return;
+    }
+    if (
+      (!options?.pasteOption && !tableCell.isWholeTableCopied) ||
+      options?.pasteOption === "onlyFormat"
+    ) {
+      if (tableCell.style?.style) {
         this.dispatch("UPDATE_CELL", { ...position, style: tableCell.style.style });
       }
       if (tableCell.style?.border) {
diff --git a/tests/table/tables_plugin.test.ts b/tests/table/tables_plugin.test.ts
index 198a0f8bed..28dff778da 100644
--- a/tests/table/tables_plugin.test.ts
+++ b/tests/table/tables_plugin.test.ts
@@ -2,7 +2,7 @@ import { CommandResult, Model } from "../../src";
 import { DEFAULT_BORDER_DESC } from "../../src/constants";
 import { toUnboundedZone, toZone, zoneToXc } from "../../src/helpers";
 import { TABLE_PRESETS } from "../../src/helpers/table_presets";
-import { ClipboardPasteOptions, UID } from "../../src/types";
+import { UID } from "../../src/types";
 import {
   addColumns,
   addRows,
@@ -803,17 +803,25 @@ describe("Table plugin", () => {
       expect(getCell(model, "B1")?.style?.fillColor).not.toEqual("#FF0000");
     });
 
-    test.each(["onlyFormat", "asValue"] as ClipboardPasteOptions[])(
-      "Special paste %s don't paste whole tables",
-      (pasteOption: ClipboardPasteOptions) => {
-        createTable(model, "A1:B4");
-        updateFilter(model, "A1", ["thisIsAValue"]);
+    test("Paste as value do not copy the table", () => {
+      createTable(model, "A1:B4", { styleId: "TestStyleAllRed" });
 
-        copy(model, "A1:B4");
-        paste(model, "A5", pasteOption);
-        expect(getTable(model, "A5")).toBeFalsy();
-      }
-    );
+      copy(model, "A1:B4");
+      paste(model, "A5", "asValue");
+      expect(getTable(model, "A5")).toBeFalsy();
+      expect(getCell(model, "A5")?.style).toBeUndefined();
+    });
+
+    test("Can copy/paste the whole table formatting", () => {
+      createTable(model, "A1:A2", { styleId: "TestStyleAllRed" });
+
+      copy(model, "A1:A2");
+      paste(model, "A5", "onlyFormat");
+      expect(getTable(model, "A5")).toBeFalsy();
+      expect(getCell(model, "A5")?.style).toEqual({ fillColor: "#FF0000", bold: true });
+      expect(getBorder(model, "A5")).toEqual({ top: DEFAULT_BORDER_DESC });
+      expect(getCell(model, "A6")?.style).toEqual({ fillColor: "#FF0000" });
+    });
 
     test("Pasting onlyFormat with a partial table copied paste the table style, not asValue", () => {
       createTable(model, "A1:B4");