diff --git a/src/components/error_tooltip/error_tooltip.ts b/src/components/error_tooltip/error_tooltip.ts index d870640817..d742f6a95a 100644 --- a/src/components/error_tooltip/error_tooltip.ts +++ b/src/components/error_tooltip/error_tooltip.ts @@ -1,6 +1,7 @@ import { Component } from "@odoo/owl"; +import { deepEquals, positionToZone } from "../../helpers"; import { _t } from "../../translation"; -import { CellValueType } from "../../types"; +import { CellPosition, CellValueType, EvaluatedCell, SpreadsheetChildEnv } from "../../types"; import { CellPopoverComponent, PopoverBuilders } from "../../types/cell_popovers"; import { css } from "../helpers/css"; @@ -29,28 +30,64 @@ export interface ErrorToolTipMessage { } interface ErrorToolTipProps { + cellPosition: CellPosition; errors: ErrorToolTipMessage[]; onClosed?: () => void; } -export class ErrorToolTip extends Component { +export class ErrorToolTip extends Component { static maxSize = { maxHeight: ERROR_TOOLTIP_MAX_HEIGHT }; static template = "o-spreadsheet-ErrorToolTip"; static props = { + cellPosition: Object, errors: Array, onClosed: { type: Function, optional: true }, }; + + get evaluationError() { + const cell = this.env.model.getters.getEvaluatedCell(this.props.cellPosition); + if (cell.message) { + return cell; + } + return undefined; + } + + get errorOriginPositionString() { + const evaluationError = this.evaluationError; + const position = evaluationError?.errorOriginPosition; + if (!position || deepEquals(position, this.props.cellPosition)) { + return ""; + } + const sheetId = position.sheetId; + return this.env.model.getters.getRangeString( + this.env.model.getters.getRangeFromZone(sheetId, positionToZone(position)), + this.env.model.getters.getActiveSheetId() + ); + } + + selectCell() { + const position = this.evaluationError?.errorOriginPosition; + if (!position) { + return; + } + const activeSheetId = this.env.model.getters.getActiveSheetId(); + if (position.sheetId !== activeSheetId) { + this.env.model.dispatch("ACTIVATE_SHEET", { + sheetIdFrom: activeSheetId, + sheetIdTo: position.sheetId, + }); + } + this.env.model.selection.selectCell(position.col, position.row); + } } export const ErrorToolTipPopoverBuilder: PopoverBuilders = { onHover: (position, getters): CellPopoverComponent => { const cell = getters.getEvaluatedCell(position); const errors: ErrorToolTipMessage[] = []; + let evaluationError: EvaluatedCell | undefined; if (cell.type === CellValueType.error && !!cell.message) { - errors.push({ - title: _t("Error"), - message: cell.message, - }); + evaluationError = cell; } const validationErrorMessage = getters.getInvalidDataValidationMessage(position); @@ -61,13 +98,16 @@ export const ErrorToolTipPopoverBuilder: PopoverBuilders = { }); } - if (!errors.length) { + if (!errors.length && !evaluationError) { return { isOpen: false }; } return { isOpen: true, - props: { errors: errors }, + props: { + cellPosition: position, + errors, + }, Component: ErrorToolTip, cellCorner: "TopRight", }; diff --git a/src/components/error_tooltip/error_tooltip.xml b/src/components/error_tooltip/error_tooltip.xml index 40dbd1a0d1..b2f2760579 100644 --- a/src/components/error_tooltip/error_tooltip.xml +++ b/src/components/error_tooltip/error_tooltip.xml @@ -1,6 +1,21 @@
+ +
Error
+
+ +
+ Caused by + +
+
+
diff --git a/src/plugins/ui_core_views/cell_evaluation/evaluator.ts b/src/plugins/ui_core_views/cell_evaluation/evaluator.ts index 0989a1fcad..ac682cf360 100644 --- a/src/plugins/ui_core_views/cell_evaluation/evaluator.ts +++ b/src/plugins/ui_core_views/cell_evaluation/evaluator.ts @@ -366,11 +366,15 @@ export class Evaluator { ); if (!isMatrix(formulaReturn)) { - return createEvaluatedCell( + const evaluatedCell = createEvaluatedCell( nullValueToZeroValue(formulaReturn), this.getters.getLocale(), cellData ); + if (evaluatedCell.type === CellValueType.error) { + evaluatedCell.errorOriginPosition = formulaReturn.errorOriginPosition ?? formulaPosition; + } + return evaluatedCell; } this.assertSheetHasEnoughSpaceToSpreadFormulaResult(formulaPosition, formulaReturn); @@ -494,6 +498,9 @@ export class Evaluator { this.getters.getLocale(), cell ); + if (evaluatedCell.type === CellValueType.error) { + evaluatedCell.errorOriginPosition = matrixResult[i][j].errorOriginPosition ?? position; + } this.evaluatedCells.set(position, evaluatedCell); }; return spreadValues; diff --git a/src/types/misc.ts b/src/types/misc.ts index df4214c252..a182809e42 100644 --- a/src/types/misc.ts +++ b/src/types/misc.ts @@ -198,7 +198,14 @@ export interface RangeCompiledFormula extends Omit = T[][]; -export type FunctionResultObject = { value: CellValue; format?: Format; message?: string }; + +export type FunctionResultObject = { + value: CellValue; + format?: Format; + errorOriginPosition?: CellPosition; + message?: string; +}; + export type FunctionResultNumber = { value: number; format?: string }; // FORMULA FUNCTION VALUE AND FORMAT INPUT diff --git a/tests/evaluation/evaluation.test.ts b/tests/evaluation/evaluation.test.ts index e47033ce76..79ff6293d9 100644 --- a/tests/evaluation/evaluation.test.ts +++ b/tests/evaluation/evaluation.test.ts @@ -22,7 +22,12 @@ import { getCellError, getEvaluatedCell, } from "../test_helpers/getters_helpers"; -import { evaluateCell, evaluateGrid, restoreDefaultFunctions } from "../test_helpers/helpers"; +import { + evaluateCell, + evaluateGrid, + restoreDefaultFunctions, + toCellPosition, +} from "../test_helpers/helpers"; import resetAllMocks = jest.resetAllMocks; describe("evaluateCells", () => { @@ -1086,8 +1091,51 @@ describe("evaluateCells", () => { expect(getCellError(model, "C1")).toBe("Invalid expression"); }); - // TO DO: add tests for exp format (ex: 4E10) - // RO DO: add tests for DATE string format (ex match: "28 02 2020") + test("error original position from the cell itself", () => { + const model = new Model(); + const sheetId = model.getters.getActiveSheetId(); + setCellContent(model, "A1", "=0/0"); + expect(getEvaluatedCell(model, "A1").errorOriginPosition).toEqual( + toCellPosition(sheetId, "A1") + ); + }); + + test("error original position from simple references", () => { + const model = new Model(); + const sheetId = model.getters.getActiveSheetId(); + setCellContent(model, "A1", "=0/0"); + setCellContent(model, "A2", "=A1"); + setCellContent(model, "A3", "=A2"); + const A1 = toCellPosition(sheetId, "A1"); + expect(getEvaluatedCell(model, "A2").errorOriginPosition).toEqual(A1); + expect(getEvaluatedCell(model, "A3").errorOriginPosition).toEqual(A1); + }); + + test("error original position from a range reference", () => { + const model = new Model(); + const sheetId = model.getters.getActiveSheetId(); + setCellContent(model, "A1", "1"); + setCellContent(model, "A2", "=0/0"); + setCellContent(model, "A3", "=MAX(A1:A2)"); + expect(getEvaluatedCell(model, "A3").errorOriginPosition).toEqual( + toCellPosition(sheetId, "A2") + ); + }); + + test("error original position in a spilled result", () => { + const model = new Model(); + const sheetId = model.getters.getActiveSheetId(); + setCellContent(model, "A1", "1"); + setCellContent(model, "A2", "=0/0"); + setCellContent(model, "A3", "=TRANSPOSE(A1:A2)"); + setCellContent(model, "A4", "=TRANSPOSE(A3:B3)"); + expect(getEvaluatedCell(model, "B3").errorOriginPosition).toEqual( + toCellPosition(sheetId, "A2") + ); + expect(getEvaluatedCell(model, "A5").errorOriginPosition).toEqual( + toCellPosition(sheetId, "A2") + ); + }); }); describe("evaluate formula getter", () => { diff --git a/tests/popover/error_tooltip_component.test.ts b/tests/popover/error_tooltip_component.test.ts index 513f356f23..e547af3b5d 100644 --- a/tests/popover/error_tooltip_component.test.ts +++ b/tests/popover/error_tooltip_component.test.ts @@ -1,17 +1,16 @@ -import { Model } from "../../src"; -import { - ErrorToolTip, - ErrorToolTipMessage, -} from "../../src/components/error_tooltip/error_tooltip"; +import { Model, PropsOf } from "../../src"; +import { ErrorToolTip } from "../../src/components/error_tooltip/error_tooltip"; import { DEFAULT_CELL_HEIGHT, DEFAULT_CELL_WIDTH } from "../../src/constants"; import { addDataValidation, createChart, + createSheet, merge, setCellContent, } from "../test_helpers/commands_helpers"; import { TEST_CHART_DATA } from "../test_helpers/constants"; import { + click, clickCell, gridMouseEvent, hoverCell, @@ -24,6 +23,7 @@ import { mountComponent, mountSpreadsheet, nextTick, + toCellPosition, } from "../test_helpers/helpers"; mockChart(); @@ -31,21 +31,30 @@ mockChart(); describe("Error tooltip component", () => { let fixture: HTMLElement; - async function mountErrorTooltip(errors: ErrorToolTipMessage[]) { - ({ fixture } = await mountComponent(ErrorToolTip, { props: { errors } })); + async function mountErrorTooltip(model: Model, props: PropsOf) { + ({ fixture } = await mountComponent(ErrorToolTip, { + props, + model, + })); } test("Can display an error message", async () => { - await mountErrorTooltip([{ message: "This is an error", title: "Error" }]); + await mountErrorTooltip(new Model(), { + errors: [{ message: "This is an error", title: "Error" }], + cellPosition: toCellPosition("sheet1", "A1"), + }); expect(fixture.querySelector(".o-error-tooltip-title")?.textContent).toBe("Error"); expect(fixture.querySelector(".o-error-tooltip-message")?.textContent).toBe("This is an error"); }); test("Can display multiple error messages", async () => { - await mountErrorTooltip([ - { message: "This is an error", title: "Error" }, - { message: "Invalid data", title: "Invalid" }, - ]); + await mountErrorTooltip(new Model(), { + errors: [ + { message: "This is an error", title: "Error" }, + { message: "Invalid data", title: "Invalid" }, + ], + cellPosition: toCellPosition("sheet1", "A1"), + }); const titles = fixture.querySelectorAll(".o-error-tooltip-title"); const messages = fixture.querySelectorAll(".o-error-tooltip-message"); expect(titles).toHaveLength(2); @@ -57,6 +66,56 @@ describe("Error tooltip component", () => { expect(titles[1].textContent).toBe("Invalid"); expect(messages[1].textContent).toBe("Invalid data"); }); + + test("can display error origin position", async () => { + const model = new Model(); + const sheetId = model.getters.getActiveSheetId(); + setCellContent(model, "A1", "=1/0"); + setCellContent(model, "A2", "=A1"); + await mountErrorTooltip(model, { + errors: [], + cellPosition: toCellPosition(sheetId, "A2"), + }); + expect(".fst-italic").toHaveText(" Caused by A1"); + }); + + test("can display error position from another sheet", async () => { + const model = new Model(); + const sheetId = model.getters.getActiveSheetId(); + createSheet(model, { sheetId: "sheet2" }); + setCellContent(model, "A1", "=1/0", "sheet2"); + setCellContent(model, "A2", "=Sheet2!A1"); + await mountErrorTooltip(model, { + errors: [], + cellPosition: toCellPosition(sheetId, "A2"), + }); + expect(".fst-italic").toHaveText(" Caused by Sheet2!A1"); + }); + + test("clicking on error position selects the position", async () => { + const model = new Model(); + const sheetId = model.getters.getActiveSheetId(); + createSheet(model, { sheetId: "sheet2" }); + setCellContent(model, "J10", "=1/0", "sheet2"); + setCellContent(model, "A2", "=Sheet2!J10"); + await mountErrorTooltip(model, { + errors: [], + cellPosition: toCellPosition(sheetId, "A2"), + }); + click(fixture, ".o-button-link"); + expect(model.getters.getActivePosition()).toEqual(toCellPosition("sheet2", "J10")); + }); + + test("does not display error origin position if it is the same cell", async () => { + const model = new Model(); + const sheetId = model.getters.getActiveSheetId(); + setCellContent(model, "A1", "=1/0"); + await mountErrorTooltip(model, { + errors: [], + cellPosition: toCellPosition(sheetId, "A1"), + }); + expect(".fst-italic").toHaveCount(0); + }); }); describe("Grid integration", () => {