Skip to content

Commit

Permalink
[REV] figure,chart: Prevent destructive creation
Browse files Browse the repository at this point in the history
Since we did not verify the coherence of the commands payload in the
past, we cannot trust them and cannot rely on it now.

This reverts commit c1c901a.

Part-of: #2153
  • Loading branch information
rrahir committed Mar 2, 2023
1 parent 2ac9223 commit 0c6f8d9
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 400 deletions.
19 changes: 3 additions & 16 deletions src/plugins/core/chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ import {
Command,
CommandResult,
CoreCommand,
CreateChartCommand,
ExcelWorkbookData,
Figure,
FigureData,
Pixel,
UID,
UpdateChartCommand,
WorkbookData,
} from "../../types/index";
import { CorePlugin } from "../core_plugin";
Expand Down Expand Up @@ -46,8 +44,8 @@ export class ChartPlugin extends CorePlugin<ChartState> implements ChartState {
readonly charts: ChartState["charts"] = {};

private createChart = chartFactory(this.getters);
private validateChartDefinition = (cmd: CreateChartCommand | UpdateChartCommand) =>
validateChartDefinition(this, cmd.definition);
private validateChartDefinition = (definition: ChartDefinition) =>
validateChartDefinition(this, definition);

adaptRanges(applyChange: ApplyRangeChange) {
for (const sheetId of Object.keys(this.charts)) {
Expand All @@ -64,12 +62,8 @@ export class ChartPlugin extends CorePlugin<ChartState> implements ChartState {
allowDispatch(cmd: Command) {
switch (cmd.type) {
case "CREATE_CHART":
return this.checkValidations(
cmd,
this.chainValidations(this.validateChartDefinition, this.checkChartDuplicate)
);
case "UPDATE_CHART":
return this.validateChartDefinition(cmd);
return this.validateChartDefinition(cmd.definition);
default:
return CommandResult.Success;
}
Expand Down Expand Up @@ -243,11 +237,4 @@ export class ChartPlugin extends CorePlugin<ChartState> implements ChartState {
private addChart(id: UID, sheetId: UID, definition: ChartDefinition) {
this.history.update("charts", sheetId, id, this.createChart(id, definition, sheetId));
}

private checkChartDuplicate(cmd: CreateChartCommand): CommandResult {
if (this.charts[cmd.sheetId]?.[cmd.id]) {
return CommandResult.DuplicatedChartId;
}
return CommandResult.Success;
}
}
6 changes: 3 additions & 3 deletions src/plugins/core/figures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export class FigurePlugin extends CorePlugin<FigureState> implements FigureState
allowDispatch(cmd: CoreCommand) {
switch (cmd.type) {
case "CREATE_FIGURE":
return this.checkFigureDuplicate(cmd.sheetId, cmd.figure.id);
return this.checkFigureDuplicate(cmd.figure.id);
case "UPDATE_FIGURE":
case "DELETE_FIGURE":
return this.checkFigureExists(cmd.sheetId, cmd.id);
Expand Down Expand Up @@ -95,8 +95,8 @@ export class FigurePlugin extends CorePlugin<FigureState> implements FigureState
return CommandResult.Success;
}

private checkFigureDuplicate(sheetId: UID, figureId: UID): CommandResult {
if (this.figures[sheetId]?.[figureId]) {
private checkFigureDuplicate(figureId: UID): CommandResult {
if (Object.values(this.figures).find((sheet) => sheet?.[figureId])) {
return CommandResult.DuplicatedFigureId;
}
return CommandResult.Success;
Expand Down
1 change: 0 additions & 1 deletion src/types/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,6 @@ export const enum CommandResult {
NonContinuousTargets,
DuplicatedFigureId,
InvalidSelectionStep,
DuplicatedChartId,
}

export interface CommandHandler<T> {
Expand Down
Loading

0 comments on commit 0c6f8d9

Please sign in to comment.