fix(sheets-data-validation): data validation valid error on custom fo…

…rmula (#4776)

packages/core/src/services/undoredo/undoredo.service.ts

@@ -49,6 +49,8 @@ export interface IUndoRedoService {
     popUndoToRedo(): void;
     popRedoToUndo(): void;
 
+    cancelLastRedo(unitId?: string): Nullable<IUndoRedoItem>;
+
     clearUndoRedo(unitId: string): void;
 
     /**
@@ -271,6 +273,15 @@ export class LocalUndoRedoService extends Disposable implements IUndoRedoService
         }
     }
 
+    cancelLastRedo(unitID?: string): void {
+        const unitId = unitID || this._getFocusedUnitId();
+        const stack = this._getUndoStack(unitId);
+        const item = stack?.pop();
+        if (item) {
+            sequenceExecute(item.undoMutations, this._commandService);
+        }
+    }
+
     __tempBatchingUndoRedo(unitId: string): IDisposable {
         if (this._batchingStatus.has(unitId)) {
             throw new Error('[LocalUndoRedoService]: cannot batching undo redo twice at the same time!');

packages/core/src/shared/rectangle.ts

@@ -14,8 +14,10 @@
  * limitations under the License.
  */
 
+import type { BBox } from 'rbush';
 import type { IRange, IRectLTRB } from '../sheets/typedef';
 import type { Nullable } from './types';
+import RBush from 'rbush';
 import { AbsoluteRefType, RANGE_TYPE } from '../sheets/typedef';
 import { mergeRanges, multiSubtractSingleRange, splitIntoGrid } from './range';
 
@@ -167,6 +169,30 @@ export class Rectangle {
         return zx <= x && zy <= y;
     }
 
+    /**
+     * Checks if any of the ranges in the target array intersect with any of the ranges in the source array.
+     * Attention! Please make sure there is no NaN in the ranges.
+     * @param src
+     * @param target
+     * @example
+     * ```typescript
+     * const ranges1 = [
+     *   { startRow: 0, startColumn: 0, endRow: 2, endColumn: 2 },
+     *   { startRow: 3, startColumn: 3, endRow: 5, endColumn: 5 }
+     * ];
+     * const ranges2 = [
+     *   { startRow: 1, startColumn: 1, endRow: 4, endColumn: 4 },
+     *   { startRow: 6, startColumn: 6, endRow: 8, endColumn: 8 }
+     * ];
+     * const doIntersect = Rectangle.doAnyRangesIntersect(ranges1, ranges2); // true
+     * ```
+     */
+    static doAnyRangesIntersect(src: IRange[], target: IRange[]): boolean {
+        const rbush = new RBush<BBox>();
+        rbush.load(src.map((r) => ({ minX: r.startColumn, minY: r.startRow, maxX: r.endColumn, maxY: r.endRow })));
+        return target.some((r) => rbush.search({ minX: r.startColumn, minY: r.startRow, maxX: r.endColumn, maxY: r.endRow }).length > 0);
+    }
+
     /**
      * Gets the intersection range between two ranges
      * @param src

packages/sheets-data-validation-ui/src/controllers/dv-reject-input.controller.ts

@@ -14,11 +14,11 @@
  * limitations under the License.
  */
 
-import { DataValidationErrorStyle, Disposable, Inject, LocaleService } from '@univerjs/core';
+import { DataValidationErrorStyle, DataValidationStatus, Disposable, Inject, LocaleService } from '@univerjs/core';
 import { DataValidatorRegistryService } from '@univerjs/data-validation';
 import { Button } from '@univerjs/design';
-import { AFTER_CELL_EDIT_ASYNC, SheetInterceptorService } from '@univerjs/sheets';
-import { getCellValueOrigin, SheetDataValidationModel } from '@univerjs/sheets-data-validation';
+import { SheetInterceptorService, VALIDATE_CELL } from '@univerjs/sheets';
+import { SheetDataValidationModel, SheetsDataValidationValidatorService } from '@univerjs/sheets-data-validation';
 import { IDialogService } from '@univerjs/ui';
 import React from 'react';
 
@@ -28,50 +28,39 @@ export class DataValidationRejectInputController extends Disposable {
         @Inject(SheetDataValidationModel) private readonly _dataValidationModel: SheetDataValidationModel,
         @Inject(DataValidatorRegistryService) private readonly _dataValidatorRegistryService: DataValidatorRegistryService,
         @IDialogService private readonly _dialogService: IDialogService,
-        @Inject(LocaleService) private readonly _localeService: LocaleService
+        @Inject(LocaleService) private readonly _localeService: LocaleService,
+        @Inject(SheetsDataValidationValidatorService) private readonly _sheetsDataValidationValidatorService: SheetsDataValidationValidatorService
     ) {
         super();
         this._initEditorBridgeInterceptor();
     }
 
     private _initEditorBridgeInterceptor() {
         this._sheetInterceptorService.writeCellInterceptor.intercept(
-            AFTER_CELL_EDIT_ASYNC,
+            VALIDATE_CELL,
             {
-                handler: async (cellPromise, context, next) => {
-                    const cell = await cellPromise;
-                    const { worksheet, row, col, unitId, subUnitId, workbook } = context;
+                handler: async (lastResult, context, next) => {
+                    const cell = await lastResult;
+                    const { row, col, unitId, subUnitId } = context;
                     const ruleId = this._dataValidationModel.getRuleIdByLocation(unitId, subUnitId, row, col);
                     const rule = ruleId ? this._dataValidationModel.getRuleById(unitId, subUnitId, ruleId) : undefined;
-                    if (!rule || rule.errorStyle !== DataValidationErrorStyle.STOP) {
-                        return next(Promise.resolve(cell));
+                    if (cell === false) {
+                        return next(Promise.resolve(false))!;
                     }
 
-                    const validator = await this._dataValidatorRegistryService.getValidatorItem(rule.type);
+                    if (!rule || rule.errorStyle !== DataValidationErrorStyle.STOP) {
+                        return next(Promise.resolve(true))!;
+                    }
+                    const validator = this._dataValidatorRegistryService.getValidatorItem(rule.type);
                     if (!validator) {
-                        return next(Promise.resolve(cell));
+                        return next(Promise.resolve(true))!;
                     }
 
-                    const success = await validator.validator(
-                        {
-                            value: getCellValueOrigin(cell),
-                            interceptValue: getCellValueOrigin(context?.origin ?? cell),
-                            row,
-                            column: col,
-                            unitId,
-                            subUnitId,
-                            worksheet,
-                            workbook,
-                            t: cell?.t,
-                        },
-                        rule
-                    );
-
-                    if (success) {
-                        return next(Promise.resolve(cell));
+                    const res = await this._sheetsDataValidationValidatorService.validatorCell(unitId, subUnitId, row, col);
+                    if (res === DataValidationStatus.VALID) {
+                        return next(Promise.resolve(true))!;
                     }
 
-                    const oldCell = worksheet.getCellRaw(row, col);
                     this._dialogService.open({
                         width: 368,
                         title: {
@@ -95,7 +84,8 @@ export class DataValidationRejectInputController extends Disposable {
                             this._dialogService.close('reject-input-dialog');
                         },
                     });
-                    return next(Promise.resolve(oldCell));
+
+                    return next(Promise.resolve(false))!; // Add explicit return for invalid data
                 },
             }
         );

packages/sheets-data-validation-ui/src/views/components/detail/index.tsx

@@ -303,25 +303,27 @@ export function DataValidationDetail() {
                 : null}
             {FormulaInput
                 ? (
-                    <FormulaInput
-                        key={key + localRule.type}
-                        isTwoFormula={isTwoFormula}
-                        value={{
-                            formula1: localRule.formula1,
-                            formula2: localRule.formula2,
-                        }}
-                        onChange={(value: any) => {
-                            handleUpdateRuleSetting({
-                                ...baseRule,
-                                ...value,
-                            });
-                        }}
-                        showError={showError}
-                        validResult={validator.validatorFormula(localRule, unitId, subUnitId)}
-                        unitId={unitId}
-                        subUnitId={subUnitId}
-                        ruleId={ruleId}
-                    />
+                    <FormLayout>
+                        <FormulaInput
+                            key={key + localRule.type}
+                            isTwoFormula={isTwoFormula}
+                            value={{
+                                formula1: localRule.formula1,
+                                formula2: localRule.formula2,
+                            }}
+                            onChange={(value: any) => {
+                                handleUpdateRuleSetting({
+                                    ...baseRule,
+                                    ...value,
+                                });
+                            }}
+                            showError={showError}
+                            validResult={validator.validatorFormula(localRule, unitId, subUnitId)}
+                            unitId={unitId}
+                            subUnitId={subUnitId}
+                            ruleId={ruleId}
+                        />
+                    </FormLayout>
                 )
                 : null}
             <FormLayout>

packages/sheets-data-validation/src/services/dv-cache.service.ts

@@ -22,7 +22,7 @@ import { Subject } from 'rxjs';
 
 export class DataValidationCacheService extends Disposable {
     private _cacheMatrix: Map<string, Map<string, ObjectMatrix<Nullable<DataValidationStatus>>>> = new Map();
-    private _dirtyRanges$ = new Subject<{ unitId: string; subUnitId: string; ranges: IRange[] }>();
+    private _dirtyRanges$ = new Subject<{ unitId: string; subUnitId: string; ranges: IRange[]; isSetRange?: boolean }>();
 
     readonly dirtyRanges$ = this._dirtyRanges$.asObservable();
 
@@ -42,7 +42,7 @@ export class DataValidationCacheService extends Disposable {
                 if (cellValue) {
                     const range = new ObjectMatrix(cellValue).getDataRange();
                     if (range.endRow === -1) return;
-                    this.markRangeDirty(unitId, subUnitId, [range]);
+                    this.markRangeDirty(unitId, subUnitId, [range], true);
                 }
             }
         }));
@@ -93,15 +93,15 @@ export class DataValidationCacheService extends Disposable {
         this._deleteRange(unitId, subUnitId, rule.ranges);
     }
 
-    markRangeDirty(unitId: string, subUnitId: string, ranges: IRange[]) {
+    markRangeDirty(unitId: string, subUnitId: string, ranges: IRange[], isSetRange?: boolean) {
         const cache = this._ensureCache(unitId, subUnitId);
         ranges.forEach((range) => {
             Range.foreach(range, (row, col) => {
                 cache.setValue(row, col, undefined);
             });
         });
 
-        this._dirtyRanges$.next({ unitId, subUnitId, ranges });
+        this._dirtyRanges$.next({ unitId, subUnitId, ranges, isSetRange });
     }
 
     private _deleteRange(unitId: string, subUnitId: string, ranges: IRange[]) {

packages/sheets-data-validation/src/services/dv-custom-formula.service.ts

@@ -15,7 +15,7 @@
  */
 
 import type { IRange, ISheetDataValidationRule } from '@univerjs/core';
-import { Disposable, Inject, isFormulaString, IUniverInstanceService, UniverInstanceType } from '@univerjs/core';
+import { Disposable, Inject, isFormulaString, IUniverInstanceService, Rectangle, UniverInstanceType } from '@univerjs/core';
 import { DataValidationModel, DataValidatorRegistryService } from '@univerjs/data-validation';
 import { RegisterOtherFormulaService } from '@univerjs/sheets-formula';
 import { getFormulaCellData, shouldOffsetFormulaByRange } from '../utils/formula';
@@ -49,6 +49,7 @@ export class DataValidationCustomFormulaService extends Disposable {
         super();
 
         this._initFormulaResultHandler();
+        this._initDirtyRanges();
     }
 
     private _initFormulaResultHandler() {
@@ -107,6 +108,25 @@ export class DataValidationCustomFormulaService extends Disposable {
         return this._registerOtherFormulaService.registerFormulaWithRange(unitId, subUnitId, formulaString, ranges, { ruleId });
     };
 
+    private _handleDirtyRanges(unitId: string, subUnitId: string, ranges: IRange[]) {
+        const rules = this._dataValidationModel.getRules(unitId, subUnitId);
+        rules.forEach((rule) => {
+            const ruleRanges = rule.ranges as IRange[];
+            const hasOverLap = Rectangle.doAnyRangesIntersect(ruleRanges, ranges);
+            if (hasOverLap) {
+                this.makeRuleDirty(unitId, subUnitId, rule.uid);
+            }
+        });
+    }
+
+    private _initDirtyRanges() {
+        this._dataValidationCacheService.dirtyRanges$.subscribe((data) => {
+            if (data.isSetRange) {
+                this._handleDirtyRanges(data.unitId, data.subUnitId, data.ranges);
+            }
+        });
+    }
+
     deleteByRuleId(unitId: string, subUnitId: string, ruleId: string) {
         const { ruleFormulaMap, ruleFormulaMap2 } = this._ensureMaps(unitId, subUnitId);
         const rule = this._dataValidationModel.getRuleById(unitId, subUnitId, ruleId) as ISheetDataValidationRule;
@@ -224,4 +244,15 @@ export class DataValidationCustomFormulaService extends Disposable {
 
         return ruleFormulaMap.get(ruleId);
     }
+
+    makeRuleDirty(unitId: string, subUnitId: string, ruleId: string) {
+        const formula1 = this._ruleFormulaMap.get(unitId)?.get(subUnitId)?.get(ruleId);
+        const formula2 = this._ruleFormulaMap2.get(unitId)?.get(subUnitId)?.get(ruleId);
+        if (formula1) {
+            this._registerOtherFormulaService.markFormulaDirty(unitId, subUnitId, formula1.formulaId);
+        }
+        if (formula2) {
+            this._registerOtherFormulaService.markFormulaDirty(unitId, subUnitId, formula2.formulaId);
+        }
+    }
 }

packages/sheets-formula/src/services/register-other-formula.service.ts

@@ -31,7 +31,6 @@ export class RegisterOtherFormulaService extends Disposable {
     public formulaChangeWithRange$ = this._formulaChangeWithRange$.asObservable();
 
     // FIXME: this design could be improved.
-
     private _formulaResult$ = new Subject<Record<string, Record<string, IOtherFormulaResult[]>>>();
     public formulaResult$ = this._formulaResult$.asObservable();
 
@@ -238,4 +237,14 @@ export class RegisterOtherFormulaService extends Disposable {
         const cacheMap = this._ensureCacheMap(unitId, subUnitId);
         return cacheMap.get(formulaId);
     }
+
+    markFormulaDirty(unitId: string, subUnitId: string, formulaId: string) {
+        const cache = this.getFormulaValueSync(unitId, subUnitId, formulaId);
+        if (!cache) return;
+        cache.status = FormulaResultStatus.WAIT;
+        this._commandService.executeCommand(
+            OtherFormulaMarkDirty.id,
+            { [unitId]: { [subUnitId]: { [formulaId]: true } } }
+        );
+    }
 }

packages/sheets-hyper-link/src/commands/commands/add-hyper-link.command.ts

@@ -17,7 +17,7 @@
 import type { ICellData, ICommand, IDocumentData, IMutationInfo } from '@univerjs/core';
 import type { ISetRangeValuesMutationParams } from '@univerjs/sheets';
 import type { ISheetHyperLink } from '../../types/interfaces/i-hyper-link';
-import { BuildTextUtils, CellValueType, CommandType, CustomRangeType, generateRandomId, ICommandService, IUndoRedoService, IUniverInstanceService, sequenceExecuteAsync, TextX, Tools } from '@univerjs/core';
+import { BuildTextUtils, CellValueType, CommandType, CustomRangeType, generateRandomId, ICommandService, IUndoRedoService, IUniverInstanceService, sequenceExecute, TextX, Tools } from '@univerjs/core';
 import { addCustomRangeBySelectionFactory } from '@univerjs/docs';
 import { getSheetCommandTarget, SetRangeValuesMutation, SetRangeValuesUndoMutationFactory, SheetInterceptorService } from '@univerjs/sheets';
 import { HyperLinkModel } from '../../models/hyper-link.model';
@@ -108,7 +108,7 @@ export const AddHyperLinkCommand: ICommand<IAddHyperLinkCommandParams> = {
             t: CellValueType.STRING,
         };
 
-        const finalCellData = await sheetInterceptorService.onWriteCell(workbook, worksheet, row, column, newCellData);
+        const finalCellData = sheetInterceptorService.onWriteCell(workbook, worksheet, row, column, newCellData);
         const redoParams: ISetRangeValuesMutationParams = {
             unitId,
             subUnitId,
@@ -151,8 +151,13 @@ export const AddHyperLinkCommand: ICommand<IAddHyperLinkCommandParams> = {
             });
         }
 
-        const res = await sequenceExecuteAsync(redos, commandService);
+        const res = await sequenceExecute(redos, commandService);
         if (res) {
+            const isValid = await sheetInterceptorService.onValidateCell(workbook, worksheet, row, column);
+            if (isValid === false) {
+                sequenceExecute(undos, commandService);
+                return false;
+            }
             undoRedoService.pushUndoRedo({
                 redoMutations: redos,
                 undoMutations: undos,

packages/sheets-hyper-link/src/commands/commands/update-hyper-link.command.ts

@@ -16,7 +16,7 @@
 
 import type { DocumentDataModel, ICellData, ICommand, IMutationInfo } from '@univerjs/core';
 import type { ICellLinkContent } from '../../types/interfaces/i-hyper-link';
-import { CellValueType, CommandType, CustomRangeType, generateRandomId, getBodySlice, ICommandService, IUndoRedoService, IUniverInstanceService, sequenceExecuteAsync, TextX, Tools, UniverInstanceType } from '@univerjs/core';
+import { CellValueType, CommandType, CustomRangeType, generateRandomId, getBodySlice, ICommandService, IUndoRedoService, IUniverInstanceService, sequenceExecute, TextX, Tools, UniverInstanceType } from '@univerjs/core';
 import { replaceSelectionFactory } from '@univerjs/docs';
 import { getSheetCommandTarget, SetRangeValuesMutation, SetRangeValuesUndoMutationFactory, SheetInterceptorService } from '@univerjs/sheets';
 import { HyperLinkModel } from '../../models/hyper-link.model';
@@ -107,7 +107,7 @@ export const UpdateHyperLinkCommand: ICommand<IUpdateHyperLinkCommandParams> = {
             t: CellValueType.STRING,
         };
 
-        const finalCellData = await interceptorService.onWriteCell(workbook, worksheet, row, column, newCellData);
+        const finalCellData = interceptorService.onWriteCell(workbook, worksheet, row, column, newCellData);
 
         const redo = {
             id: SetRangeValuesMutation.id,
@@ -149,8 +149,13 @@ export const UpdateHyperLinkCommand: ICommand<IUpdateHyperLinkCommandParams> = {
             });
         }
 
-        const res = await sequenceExecuteAsync(redos, commandService);
+        const res = sequenceExecute(redos, commandService);
         if (res) {
+            const isValid = await interceptorService.onValidateCell(workbook, worksheet, row, column);
+            if (isValid === false) {
+                sequenceExecute(undos, commandService);
+                return false;
+            }
             undoRedoService.pushUndoRedo({
                 redoMutations: redos,
                 undoMutations: undos,