diff --git a/src/plugins/core/range.ts b/src/plugins/core/range.ts index 5948fdd4f..829ce469e 100644 --- a/src/plugins/core/range.ts +++ b/src/plugins/core/range.ts @@ -81,6 +81,7 @@ export class RangeAdapter implements CommandHandler { newRange = this.createAdaptedRange(newRange, dimension, changeType, -toRemove); } else if (range.zone[start] >= min && range.zone[end] <= max) { changeType = "REMOVE"; + newRange = range.clone({ ...this.getInvalidRange() }); } else if (range.zone[start] <= max && range.zone[end] >= max) { const toRemove = max - range.zone[start] + 1; changeType = "RESIZE"; @@ -150,9 +151,10 @@ export class RangeAdapter implements CommandHandler { return { changeType: "NONE" }; } const invalidSheetName = this.getters.getSheetName(cmd.sheetId); - const sheetId = ""; - range = range.clone({ sheetId, invalidSheetName }); - + range = range.clone({ + ...this.getInvalidRange(), + invalidSheetName, + }); return { changeType: "REMOVE", range }; }, cmd.sheetId); @@ -446,4 +448,14 @@ export class RangeAdapter implements CommandHandler { return str; } + + private getInvalidRange() { + return { + parts: [], + prefixSheet: false, + zone: { left: -1, top: -1, right: -1, bottom: -1 }, + sheetId: "", + invalidXc: INCORRECT_RANGE_STRING, + }; + } } diff --git a/tests/collaborative/clipboard.test.ts b/tests/collaborative/clipboard.test.ts index dff6394e3..0f8dd5936 100644 --- a/tests/collaborative/clipboard.test.ts +++ b/tests/collaborative/clipboard.test.ts @@ -73,11 +73,11 @@ describe("Collaborative range manipulation", () => { ); expect([alice, bob, charlie]).toHaveSynchronizedValue( (user) => getCell(user, "A1", "Sheet2")?.content, - "=Sheet1!A2" + "=#REF" ); }); - test("cut and paste and delete target sheet concurrentlkjhgkjhg khg y", () => { + test("cut and paste and delete target sheet concurrently (delete first)", () => { setCellContent(alice, "A2", "=A1"); cut(alice, "A1"); createSheet(alice, { sheetId: "Sheet2", activate: true }); @@ -85,13 +85,13 @@ describe("Collaborative range manipulation", () => { deleteSheet(bob, "Sheet2"); paste(alice, "D4"); }); - expect([alice]).toHaveSynchronizedValue( + expect([alice, bob, charlie]).toHaveSynchronizedValue( (user) => getCell(user, "A2", "Sheet1")?.content, "=A1" ); }); - test("cut and paste and delete target sheet concurrently", () => { + test("cut and paste and delete target sheet concurrently (paste first)", () => { setCellContent(alice, "A2", "=A1"); cut(alice, "A1"); createSheet(alice, { sheetId: "Sheet2", activate: true }); @@ -101,7 +101,7 @@ describe("Collaborative range manipulation", () => { }); expect([alice, bob, charlie]).toHaveSynchronizedValue( (user) => getCell(user, "A2", "Sheet1")?.content, - "=Sheet2!D4" + "=#REF" ); }); diff --git a/tests/formulas/formulas.test.ts b/tests/formulas/formulas.test.ts index aea6b108c..c53c763f1 100644 --- a/tests/formulas/formulas.test.ts +++ b/tests/formulas/formulas.test.ts @@ -1,8 +1,13 @@ import { Model } from "../../src"; import { INCORRECT_RANGE_STRING } from "../../src/constants"; import { FormulaCell } from "../../src/types"; -import { createSheetWithName, setCellContent } from "../test_helpers/commands_helpers"; -import { getCell } from "../test_helpers/getters_helpers"; +import { + createSheetWithName, + deleteColumns, + deleteRows, + setCellContent, +} from "../test_helpers/commands_helpers"; +import { getCell, getCellContent, getCellText } from "../test_helpers/getters_helpers"; function moveFormula(model: Model, formula: string, offsetX: number, offsetY: number): string { const sheetId = model.getters.getActiveSheetId(); @@ -101,3 +106,38 @@ describe("createAdaptedRanges", () => { expect(moveFormula(model, "='Sheet 2'!B2", 1, 10)).toEqual("='Sheet 2'!C12"); }); }); + +describe("Remove columns/rows that are references of formula", () => { + let model: Model; + beforeEach(() => { + model = new Model(); + }); + + test("delete multiple columns, including the one in formula and the one before it", () => { + setCellContent(model, "A1", "=SUM(C1,D1)"); + deleteColumns(model, ["B", "C"]); + expect(getCellContent(model, "A1")).toEqual("#REF"); + expect(getCellText(model, "A1")).toEqual("=SUM(#REF,B1)"); + }); + + test("delete multiple columns, including the one in formula and the one after it", () => { + setCellContent(model, "A1", "=SUM(C1,D1)"); + deleteColumns(model, ["C", "D"]); + expect(getCellContent(model, "A1")).toEqual("#REF"); + expect(getCellText(model, "A1")).toEqual("=SUM(#REF,#REF)"); + }); + + test("delete multiple rows, including the one in formula and the one before it", () => { + setCellContent(model, "A1", "=SUM(C3,C4)"); + deleteRows(model, [1, 2]); + expect(getCellContent(model, "A1")).toEqual("#REF"); + expect(getCellText(model, "A1")).toEqual("=SUM(#REF,C2)"); + }); + + test("delete multiple rows, including the one in formula and the one after it", () => { + setCellContent(model, "A1", "=SUM(C3,C4)"); + deleteRows(model, [2, 3]); + expect(getCellContent(model, "A1")).toEqual("#REF"); + expect(getCellText(model, "A1")).toEqual("=SUM(#REF,#REF)"); + }); +}); diff --git a/tests/plugins/range.test.ts b/tests/plugins/range.test.ts index 29c5175a3..b3f0c027d 100644 --- a/tests/plugins/range.test.ts +++ b/tests/plugins/range.test.ts @@ -41,7 +41,7 @@ class PluginTestRange extends CorePlugin { const change = applyChange(range); switch (change.changeType) { case "REMOVE": - this.ranges.splice(i, 1); + this.ranges[i] = change.range; break; case "RESIZE": case "MOVE": @@ -87,9 +87,9 @@ describe("range plugin", () => { beforeEach(() => { m = new Model({ sheets: [ - { id: "s1", name: "s1", rows: 10, cols: 10 }, - { id: "s2", name: "s 2", rows: 10, cols: 10 }, - { id: "s1!!", name: "s1!!", rows: 10, cols: 10 }, + { id: "s1", name: "s1" }, + { id: "s2", name: "s 2" }, + { id: "s1!!", name: "s1!!" }, ], }); m.dispatch("USE_RANGE", { sheetId: m.getters.getActiveSheetId(), rangesXC: ["B2:D4"] }); @@ -132,6 +132,59 @@ describe("range plugin", () => { }); }); + describe("create a range and remove multiple columns", () => { + beforeEach(() => { + m = new Model({ + sheets: [ + { id: "s1", name: "s1" }, + { id: "s2", name: "s 2" }, + ], + }); + m.dispatch("USE_RANGE", { sheetId: m.getters.getActiveSheetId(), rangesXC: ["C2:F5"] }); + }); + + test("in the middle", () => { + deleteColumns(m, ["D", "E"]); + expect(m.getters.getUsedRanges()).toEqual(["C2:D5"]); + }); + + test("in the start", () => { + deleteColumns(m, ["B", "C"]); + expect(m.getters.getUsedRanges()).toEqual(["B2:D5"]); + }); + + test("in the end", () => { + deleteColumns(m, ["E", "F"]); + expect(m.getters.getUsedRanges()).toEqual(["C2:D5"]); + }); + + test("before the start", () => { + deleteColumns(m, ["A", "B"]); + expect(m.getters.getUsedRanges()).toEqual(["A2:D5"]); + }); + + test("after the end", () => { + deleteColumns(m, ["G", "H"]); + expect(m.getters.getUsedRanges()).toEqual(["C2:F5"]); + }); + + test("including one column before the start and the first column", () => { + deleteColumns(m, ["C", "B"]); + expect(m.getters.getUsedRanges()).toEqual(["B2:D5"]); + }); + + test("including one column after the end and the last column", () => { + deleteColumns(m, ["G", "F"]); + expect(m.getters.getUsedRanges()).toEqual(["C2:E5"]); + }); + + test("delete columns causing invalid reference will be marked as #REF", () => { + m.dispatch("USE_RANGE", { sheetId: m.getters.getActiveSheetId(), rangesXC: ["C1"] }); + deleteColumns(m, ["B", "C"]); + expect(m.getters.getUsedRanges()[1]).toEqual("#REF"); + }); + }); + describe("create a range and remove a row", () => { test("in the middle", () => { deleteRows(m, [2]); @@ -165,6 +218,61 @@ describe("range plugin", () => { }); }); + describe("create a range and remove multiple rows", () => { + beforeEach(() => { + m = new Model({ + sheets: [ + { id: "s1", name: "s1" }, + { id: "s2", name: "s 2" }, + ], + }); + m.dispatch("USE_RANGE", { sheetId: m.getters.getActiveSheetId(), rangesXC: ["C3:F7"] }); + }); + + test("in the middle", () => { + deleteRows(m, [3, 4]); + expect(m.getters.getUsedRanges()).toEqual(["C3:F5"]); + }); + + test("in the start", () => { + deleteRows(m, [2, 3]); + expect(m.getters.getUsedRanges()).toEqual(["C3:F5"]); + }); + + test("in the end", () => { + deleteRows(m, [5, 6]); + expect(m.getters.getUsedRanges()).toEqual(["C3:F5"]); + }); + + test("including one row before start and the first row", () => { + deleteRows(m, [1, 2]); + expect(m.getters.getUsedRanges()).toEqual(["C2:F5"]); + }); + + test("including one row after end and the last row", () => { + deleteRows(m, [6, 7]); + expect(m.getters.getUsedRanges()).toEqual(["C3:F6"]); + }); + + test("before the start", () => { + deleteRows(m, [0, 1]); + expect(m.getters.getUsedRanges()).toEqual(["C1:F5"]); + }); + + test("after the end", () => { + deleteRows(m, [7, 8]); + expect(m.getters.getUsedRanges()).toEqual(["C3:F7"]); + }); + + test("delete rows causing invalid reference will be marked as #REF", () => { + m.dispatch("USE_RANGE", { sheetId: m.getters.getActiveSheetId(), rangesXC: ["C3"] }); + deleteRows(m, [1, 2]); + expect(m.getters.getUsedRanges().length).toEqual(2); + expect(m.getters.getUsedRanges()[0]).toEqual("C2:F5"); + expect(m.getters.getUsedRanges()[1]).toEqual("#REF"); + }); + }); + describe("create a range and add a column", () => { test("after, in the middle", () => { addColumns(m, "after", "C", 1); @@ -291,7 +399,7 @@ describe("range plugin", () => { m.dispatch("USE_RANGE", { rangesXC: ["A1"], sheetId: "s2" }); expect(m.getters.getUsedRanges()).toEqual(["B2:D4", "'s 2'!A1"]); deleteSheet(m, "s2"); - expect(m.getters.getUsedRanges()).toEqual(["B2:D4"]); + expect(m.getters.getUsedRanges()).toEqual(["B2:D4", "#REF"]); }); }); @@ -305,7 +413,7 @@ describe("range plugin", () => { m.dispatch("USE_RANGE", { rangesXC: ["A1"], sheetId: "s2" }); expect(m.getters.getUsedRanges()).toEqual(["B2:D4", "'s 2'!A1"]); deleteSheet(m, "s2"); - expect(m.getters.getUsedRanges()).toEqual(["B2:D4"]); + expect(m.getters.getUsedRanges()).toEqual(["B2:D4", "#REF"]); }); }); }); diff --git a/tests/plugins/sheets.test.ts b/tests/plugins/sheets.test.ts index b042203e5..2381c5d45 100644 --- a/tests/plugins/sheets.test.ts +++ b/tests/plugins/sheets.test.ts @@ -862,8 +862,8 @@ describe("sheets", () => { const sheet2 = model.getters.getActiveSheetId(); setCellContent(model, "A1", "42"); model.dispatch("DELETE_SHEET", { sheetId: sheet2 }); - expect(getCellText(model, "A1")).toBe("=NEW_NAME!A1"); - expect(getEvaluatedCell(model, "A1").value).toBe("#ERROR"); + expect(getCellText(model, "A1")).toBe("=#REF"); + expect(getEvaluatedCell(model, "A1").value).toBe("#REF"); undo(model); activateSheet(model, sheet1); expect(getCellText(model, "A1")).toBe("=NEW_NAME!A1");