diff --git a/packages/o-spreadsheet-engine/src/helpers/pivot/pivot_registry.ts b/packages/o-spreadsheet-engine/src/helpers/pivot/pivot_registry.ts index 0fda75059b..5d82c44960 100644 --- a/packages/o-spreadsheet-engine/src/helpers/pivot/pivot_registry.ts +++ b/packages/o-spreadsheet-engine/src/helpers/pivot/pivot_registry.ts @@ -1,10 +1,11 @@ import { Registry } from "../../registry"; import { CoreGetters } from "../../types/core_getters"; import { Getters } from "../../types/getters"; -import { UID } from "../../types/misc"; +import { ApplyRangeChange, UID } from "../../types/misc"; import { ModelConfig } from "../../types/model"; import { PivotCoreDefinition, PivotField, PivotFields } from "../../types/pivot"; import { Pivot } from "../../types/pivot_runtime"; +import { Range } from "../../types/range"; import { PivotRuntimeDefinition } from "./pivot_runtime_definition"; import { SpreadsheetPivotRuntimeDefinition } from "./spreadsheet_pivot/runtime_definition_spreadsheet_pivot"; import { SpreadsheetPivot } from "./spreadsheet_pivot/spreadsheet_pivot"; @@ -36,6 +37,11 @@ export interface PivotRegistryItem { isGroupable: (field: PivotField) => boolean; canHaveCustomGroup: (field: PivotField) => boolean; isPivotUnused: (getters: Getters, pivotId: UID) => boolean; + adaptRanges?: ( + getters: CoreGetters, + definition: PivotCoreDefinition, + applyChange: ApplyRangeChange + ) => PivotCoreDefinition; } export const pivotRegistry = new Registry(); @@ -60,4 +66,40 @@ pivotRegistry.add("SPREADSHEET", { isGroupable: () => true, canHaveCustomGroup: (field: PivotField) => field.type === "char" && !field.isCustomField, isPivotUnused: () => true, + adaptRanges: (getters, definition, applyChange) => { + if (definition.type !== "SPREADSHEET" || !definition.dataSet) { + return definition; + } + const { sheetId, zone } = definition.dataSet; + const range = getters.getRangeFromZone(sheetId, zone); + const adaptedRange = adaptPivotRange(range, applyChange); + + if (adaptedRange === range) { + return definition; + } + + const dataSet = adaptedRange && { + sheetId: adaptedRange.sheetId, + zone: adaptedRange.zone, + }; + return { ...definition, dataSet }; + }, }); + +function adaptPivotRange( + range: Range | undefined, + applyChange: ApplyRangeChange +): Range | undefined { + if (!range) { + return undefined; + } + const change = applyChange(range); + switch (change.changeType) { + case "NONE": + return range; + case "REMOVE": + return undefined; + default: + return change.range; + } +} diff --git a/packages/o-spreadsheet-engine/src/plugins/core/pivot.ts b/packages/o-spreadsheet-engine/src/plugins/core/pivot.ts index 2cb0f78843..a2ed0630a6 100644 --- a/packages/o-spreadsheet-engine/src/plugins/core/pivot.ts +++ b/packages/o-spreadsheet-engine/src/plugins/core/pivot.ts @@ -1,6 +1,7 @@ import { compile } from "../../formulas/compiler"; import { deepCopy, deepEquals } from "../../helpers/misc"; import { createPivotFormula, getMaxObjectId } from "../../helpers/pivot/pivot_helpers"; +import { pivotRegistry } from "../../helpers/pivot/pivot_registry"; import { SpreadsheetPivotTable } from "../../helpers/pivot/table_spreadsheet_pivot"; import { CommandResult, CoreCommand } from "../../types/commands"; import { CellPosition, UID } from "../../types/misc"; @@ -141,6 +142,18 @@ export class PivotCorePlugin extends CorePlugin implements CoreState } adaptRanges(applyChange: ApplyRangeChange) { + for (const pivotId in this.pivots) { + const definition = deepCopy(this.pivots[pivotId]?.definition); + if (!definition) { + continue; + } + const newDefinition = pivotRegistry + .get(definition.type) + ?.adaptRanges?.(this.getters, definition, applyChange); + if (newDefinition && !deepEquals(definition, newDefinition)) { + this.history.update("pivots", pivotId, "definition", newDefinition); + } + } for (const sheetId in this.compiledMeasureFormulas) { for (const formulaString in this.compiledMeasureFormulas[sheetId]) { const compiledFormula = this.compiledMeasureFormulas[sheetId][formulaString]; @@ -325,19 +338,19 @@ export class PivotCorePlugin extends CorePlugin implements CoreState if (!pivot) { continue; } - const def = deepCopy(pivot.definition); - for (const measure of def.measures) { + for (const measure of pivot.definition.measures) { if (measure.computedBy?.formula === formulaString) { - const measureIndex = def.measures.indexOf(measure); - if (measureIndex !== -1) { - def.measures[measureIndex].computedBy = { - formula: newFormulaString, - sheetId, - }; - } - - this.dispatch("UPDATE_PIVOT", { pivotId, pivot: def }); + const measureIndex = pivot.definition.measures.indexOf(measure); + this.history.update( + "pivots", + pivotId, + "definition", + "measures", + measureIndex, + "computedBy", + { formula: newFormulaString, sheetId } + ); } } } diff --git a/packages/o-spreadsheet-engine/src/plugins/core/range.ts b/packages/o-spreadsheet-engine/src/plugins/core/range.ts index 8684f3c989..ebba6ef998 100644 --- a/packages/o-spreadsheet-engine/src/plugins/core/range.ts +++ b/packages/o-spreadsheet-engine/src/plugins/core/range.ts @@ -32,6 +32,7 @@ import { Range, RangeData, RangeStringOptions } from "../../types/range"; export class RangeAdapter implements CommandHandler { private getters: CoreGetters; private providers: Array = []; + private isAdaptingRanges: boolean = false; constructor(getters: CoreGetters) { this.getters = getters; } @@ -66,6 +67,9 @@ export class RangeAdapter implements CommandHandler { beforeHandle(command: Command) {} handle(cmd: CoreCommand) { + if (this.isAdaptingRanges) { + throw new Error("Plugins cannot dispatch commands during adaptRanges phase"); + } const rangeAdapter = getRangeAdapter(cmd); if (rangeAdapter?.applyChange) { this.executeOnAllRanges( @@ -99,10 +103,12 @@ export class RangeAdapter implements CommandHandler { sheetId: UID, sheetName: AdaptSheetName ) { + this.isAdaptingRanges = true; const func = this.verifyRangeRemoved(adaptRange); for (const provider of this.providers) { provider(func, sheetId, sheetName); } + this.isAdaptingRanges = false; } /** diff --git a/packages/o-spreadsheet-engine/src/plugins/core/spreadsheet_pivot.ts b/packages/o-spreadsheet-engine/src/plugins/core/spreadsheet_pivot.ts index 8332684a01..cef31d76d2 100644 --- a/packages/o-spreadsheet-engine/src/plugins/core/spreadsheet_pivot.ts +++ b/packages/o-spreadsheet-engine/src/plugins/core/spreadsheet_pivot.ts @@ -1,28 +1,8 @@ import { isZoneValid } from "../../helpers/zones"; import { CommandResult, CoreCommand } from "../../types/commands"; -import { ApplyRangeChange } from "../../types/misc"; import { PivotCoreDefinition } from "../../types/pivot"; -import { Range } from "../../types/range"; import { CorePlugin } from "../core_plugin"; -function adaptPivotRange( - range: Range | undefined, - applyChange: ApplyRangeChange -): Range | undefined { - if (!range) { - return undefined; - } - const change = applyChange(range); - switch (change.changeType) { - case "NONE": - return range; - case "REMOVE": - return undefined; - default: - return change.range; - } -} - export class SpreadsheetPivotCorePlugin extends CorePlugin { allowDispatch(cmd: CoreCommand) { switch (cmd.type) { @@ -34,30 +14,6 @@ export class SpreadsheetPivotCorePlugin extends CorePlugin { return CommandResult.Success; } - adaptRanges(applyChange: ApplyRangeChange) { - for (const pivotId of this.getters.getPivotIds()) { - const definition = this.getters.getPivotCoreDefinition(pivotId); - if (definition.type !== "SPREADSHEET") { - continue; - } - if (definition.dataSet) { - const { sheetId, zone } = definition.dataSet; - const range = this.getters.getRangeFromZone(sheetId, zone); - const adaptedRange = adaptPivotRange(range, applyChange); - - if (adaptedRange === range) { - return; - } - - const dataSet = adaptedRange && { - sheetId: adaptedRange.sheetId, - zone: adaptedRange.zone, - }; - this.dispatch("UPDATE_PIVOT", { pivotId, pivot: { ...definition, dataSet } }); - } - } - } - private checkDataSetValidity(definition: PivotCoreDefinition) { if (definition.type === "SPREADSHEET" && definition.dataSet) { const { zone, sheetId } = definition.dataSet; diff --git a/tests/range_plugin.test.ts b/tests/range_plugin.test.ts index 54b0e05654..0d1ed0f9eb 100644 --- a/tests/range_plugin.test.ts +++ b/tests/range_plugin.test.ts @@ -744,3 +744,17 @@ test.each([ const adaptedRange = model.getters.createAdaptedRanges([range], 1, 1, sheetId); expect(model.getters.getRangeString(adaptedRange[0], sheetId)).toBe(expected); }); + +test("Plugins cannot dispatch a command during adaptRanges", () => { + class PluginDispatchInAdaptRanges extends CorePlugin { + adaptRanges() { + this.dispatch("DELETE_SHEET", { sheetId: "s1", sheetName: "coucou" }); + } + } + addTestPlugin(corePluginRegistry, PluginDispatchInAdaptRanges); + + const model = new Model({}); + expect(() => { + addColumns(model, "before", "A", 1); + }).toThrowError("Plugins cannot dispatch commands during adaptRanges phase"); +});