diff --git a/src/helpers/pivot/pivot_registry.ts b/src/helpers/pivot/pivot_registry.ts index dae9b8a343..0adeb9918d 100644 --- a/src/helpers/pivot/pivot_registry.ts +++ b/src/helpers/pivot/pivot_registry.ts @@ -1,6 +1,6 @@ import { ModelConfig } from "../../model"; import { Registry } from "../../registries/registry"; -import { CoreGetters, Getters } from "../../types"; +import { ApplyRangeChange, CoreGetters, Getters, Range } from "../../types"; import { PivotCoreDefinition, PivotField, PivotFields } from "../../types/pivot"; import { Pivot } from "../../types/pivot_runtime"; import { PivotRuntimeDefinition } from "./pivot_runtime_definition"; @@ -32,6 +32,11 @@ export interface PivotRegistryItem { datetimeGranularities: string[]; isMeasureCandidate: (field: PivotField) => boolean; isGroupable: (field: PivotField) => boolean; + adaptRanges?: ( + getters: CoreGetters, + definition: PivotCoreDefinition, + applyChange: ApplyRangeChange + ) => PivotCoreDefinition; } export const pivotRegistry = new Registry(); @@ -54,4 +59,40 @@ pivotRegistry.add("SPREADSHEET", { datetimeGranularities: [...dateGranularities, "hour_number", "minute_number", "second_number"], isMeasureCandidate: (field: PivotField) => !["datetime", "boolean"].includes(field.type), isGroupable: () => 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/src/plugins/core/pivot.ts b/src/plugins/core/pivot.ts index eee1a109eb..511c35bd9e 100644 --- a/src/plugins/core/pivot.ts +++ b/src/plugins/core/pivot.ts @@ -1,6 +1,7 @@ import { compile } from "../../formulas"; import { deepCopy, deepEquals } from "../../helpers"; import { createPivotFormula, getMaxObjectId } from "../../helpers/pivot/pivot_helpers"; +import { pivotRegistry } from "../../helpers/pivot/pivot_registry"; import { SpreadsheetPivotTable } from "../../helpers/pivot/table_spreadsheet_pivot"; import { ApplyRangeChange, @@ -132,6 +133,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]; @@ -314,19 +327,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/src/plugins/core/range.ts b/src/plugins/core/range.ts index ceda12b806..5fdbc4d29f 100644 --- a/src/plugins/core/range.ts +++ b/src/plugins/core/range.ts @@ -42,6 +42,7 @@ interface RangeStringOptions { export class RangeAdapter implements CommandHandler { private getters: CoreGetters; private providers: Array = []; + private isAdaptingRanges: boolean = false; constructor(getters: CoreGetters) { this.getters = getters; } @@ -73,6 +74,9 @@ export class RangeAdapter implements CommandHandler { beforeHandle(command: Command) {} handle(cmd: Command) { + if (this.isAdaptingRanges) { + throw new Error("Plugins cannot dispatch commands during adaptRanges phase"); + } switch (cmd.type) { case "REMOVE_COLUMNS_ROWS": { let start: "left" | "top" = cmd.dimension === "COL" ? "left" : "top"; @@ -243,10 +247,12 @@ export class RangeAdapter implements CommandHandler { } private executeOnAllRanges(adaptRange: ApplyRangeChange, sheetId?: UID) { + this.isAdaptingRanges = true; const func = this.verifyRangeRemoved(adaptRange); for (const provider of this.providers) { provider(func, sheetId); } + this.isAdaptingRanges = false; } /** diff --git a/src/plugins/core/spreadsheet_pivot.ts b/src/plugins/core/spreadsheet_pivot.ts index 1e0973f228..daee552519 100644 --- a/src/plugins/core/spreadsheet_pivot.ts +++ b/src/plugins/core/spreadsheet_pivot.ts @@ -1,31 +1,7 @@ import { isZoneValid } from "../../helpers"; -import { - ApplyRangeChange, - CommandResult, - CoreCommand, - PivotCoreDefinition, - Range, -} from "../../types"; +import { CommandResult, CoreCommand, PivotCoreDefinition } from "../../types"; 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) { @@ -37,30 +13,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 dd09fe4e96..c475dd4c15 100644 --- a/tests/range_plugin.test.ts +++ b/tests/range_plugin.test.ts @@ -745,3 +745,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" }); + } + } + addTestPlugin(corePluginRegistry, PluginDispatchInAdaptRanges); + + const model = new Model({}); + expect(() => { + addColumns(model, "before", "A", 1); + }).toThrowError("Plugins cannot dispatch commands during adaptRanges phase"); +});