Skip to content

Commit 3b5e3b8

Browse files
committed
[FIX] CF: Avoid getter crash on invalid sheet
The Evaluated CF getters would crash when provided an invalid sheet id. Providing an inexisting or invalid col/row would not throw though. This becomes an issue since #7634 because during the excel export, we fetch the style of all the cells of *all the sheets* in the data and some of them are "ghost" sheets that are injected only in the export data. As such, they are not part of the plugins lifecycle. This revision fixes the getters for CF so that they don't throw on invalid sheets for 2 reasons: - coherence: bad xc does not throw but a bad sheet id does - other style-related getters do not throw closes #8229 Task: 0 Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
1 parent 5aa16da commit 3b5e3b8

File tree

2 files changed

+25
-4
lines changed

2 files changed

+25
-4
lines changed

packages/o-spreadsheet-engine/src/plugins/ui_core_views/evaluation_conditional_format.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,17 @@ export class EvaluationConditionalFormatPlugin extends CoreViewPlugin {
7373

7474
getCellConditionalFormatStyle(position: CellPosition): Style | undefined {
7575
const { sheetId, col, row } = position;
76-
const styles = this.computedStyles[sheetId]();
76+
const styles = this.computedStyles[sheetId]?.();
7777
return styles && styles[col]?.[row];
7878
}
7979

8080
getConditionalIcon({ sheetId, col, row }: CellPosition): string | undefined {
81-
const icons = this.computedIcons[sheetId]();
81+
const icons = this.computedIcons[sheetId]?.();
8282
return icons && icons[col]?.[row];
8383
}
8484

8585
getConditionalDataBar({ sheetId, col, row }: CellPosition): DataBarFill | undefined {
86-
const dataBars = this.computedDataBars[sheetId]();
86+
const dataBars = this.computedDataBars[sheetId]?.();
8787
return dataBars && dataBars[col]?.[row];
8888
}
8989

tests/conditional_formatting/conditional_formatting_plugin.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@ import {
55
ConditionalFormatRule,
66
IconThreshold,
77
} from "@odoo/o-spreadsheet-engine/types/conditional_formatting";
8-
import { CommandResult, ConditionalFormattingOperatorValues, UID } from "../../src/types";
8+
import {
9+
CellPosition,
10+
CommandResult,
11+
ConditionalFormattingOperatorValues,
12+
UID,
13+
} from "../../src/types";
914
import {
1015
activateSheet,
1116
addCf,
@@ -2703,4 +2708,20 @@ describe("conditional formats types", () => {
27032708
);
27042709
expect(getStyle(model, "B1")).toEqual({ fillColor: undefined });
27052710
});
2711+
2712+
test("Evaluated CF getters do not throw on invalid position", () => {
2713+
const model = new Model();
2714+
const badSheetIdPosition = toCellPosition("fakeSSheet", "A1");
2715+
expect(() => model.getters.getCellConditionalFormatStyle(badSheetIdPosition)).not.toThrow();
2716+
expect(() => model.getters.getConditionalIcon(badSheetIdPosition)).not.toThrow();
2717+
expect(() => model.getters.getConditionalDataBar(badSheetIdPosition)).not.toThrow();
2718+
const badXCPosition: CellPosition = {
2719+
sheetId: model.getters.getActiveSheetId(),
2720+
col: -1,
2721+
row: -1,
2722+
};
2723+
expect(() => model.getters.getCellConditionalFormatStyle(badXCPosition)).not.toThrow();
2724+
expect(() => model.getters.getConditionalIcon(badXCPosition)).not.toThrow();
2725+
expect(() => model.getters.getConditionalDataBar(badXCPosition)).not.toThrow();
2726+
});
27062727
});

0 commit comments

Comments
 (0)