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 7d67d32.

Part-of: #2154
  • Loading branch information
rrahir committed Mar 2, 2023
1 parent 59bec2d commit 895adfa
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 422 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
2 changes: 1 addition & 1 deletion src/plugins/core/image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class ImagePlugin extends CorePlugin<ImageState> implements ImageState {
switch (cmd.type) {
case "CREATE_IMAGE":
if (this.getters.getFigure(cmd.sheetId, cmd.figureId)) {
return CommandResult.DuplicatedImageId;
return CommandResult.InvalidFigureId;
}
return CommandResult.Success;
default:
Expand Down
3 changes: 1 addition & 2 deletions src/types/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,7 @@ export const enum CommandResult {
InvalidRange,
InvalidZones,
InvalidSheetId,
DuplicatedImageId,
InvalidFigureId,
InputAlreadyFocused,
MaximumRangesReached,
InvalidChartDefinition,
Expand Down Expand Up @@ -1140,7 +1140,6 @@ export const enum CommandResult {
NonContinuousTargets,
DuplicatedFigureId,
InvalidSelectionStep,
DuplicatedChartId,
}

export interface CommandHandler<T> {
Expand Down
Loading

0 comments on commit 895adfa

Please sign in to comment.