[FIX] OT: fix missing transformations

With the OT registry condition, there are some Specific range
transformations that are bypassed, specifically for changing the content
of a cell after the renaming of a sheet.

This revision adds a check for specific range transformation to ensure
that we correctly update the content of a cell after a transformation.

More widely, this also allows to support the range conversion upon sheet
renaming for charts, cf, data validations and pivot computed
measures.

While this is an improvement of behaviour, it is a necessary refactoring
to fix the update of ranges inside the gauge charts.

Task: 4868971
X-original-commit: f14a229
Part-of: #7030
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>

Author: rrahir

src/collaborative/ot/ot.ts

@@ -5,6 +5,7 @@ import {
   isInside,
   moveHeaderIndexesOnHeaderAddition,
   moveHeaderIndexesOnHeaderDeletion,
+  rangeAdapterRegistry,
 } from "../../helpers/index";
 import { otRegistry } from "../../registries/ot_registry";
 import { specificRangeTransformRegistry } from "../../registries/srt_registry";
@@ -102,7 +103,10 @@ export function transformAll(
     // If the executed command is not in the registry, we skip it
     // because we know there won't be any transformation impacting the
     // commands to transform.
-    if (possibleTransformations.has(executedCommand.type)) {
+    if (
+      possibleTransformations.has(executedCommand.type) ||
+      rangeAdapterRegistry.contains(executedCommand.type)
+    ) {
       transformedCommands = transformedCommands.reduce<CoreCommand[]>((acc, cmd) => {
         const transformed = transform(cmd, executedCommand);
         if (transformed) {

src/helpers/formulas.ts

@@ -39,7 +39,7 @@ export function adaptStringRange(
   const sheetName = splitReference(sheetXC).sheetName;
   if (
     sheetName
-      ? !isSheetNameEqual(sheetName, applyChange.sheetName)
+      ? !isSheetNameEqual(sheetName, applyChange.sheetName.old)
       : defaultSheetId !== applyChange.sheetId
   ) {
     return sheetXC;
@@ -61,7 +61,7 @@ export function adaptStringRange(
 
 function getSheetNameGetter(applyChange: RangeAdapter) {
   return (sheetId: UID): string => {
-    return sheetId === applyChange.sheetId ? applyChange.sheetName : "";
+    return sheetId === applyChange.sheetId ? applyChange.sheetName.current : "";
   };
 }
 

src/helpers/range.ts

@@ -1,9 +1,11 @@
+import { Registry } from "../registries/registry";
 import {
   AddColumnsRowsCommand,
   ApplyRangeChange,
   CellPosition,
   ChangeType,
-  Command,
+  CoreCommand,
+  CoreCommandTypes,
   CoreGetters,
   DeleteSheetCommand,
   MoveRangeCommand,
@@ -284,42 +286,54 @@ export function orderRange(range: Range): Range {
   };
 }
 
-export function getRangeAdapter(cmd: Command): RangeAdapter | undefined {
-  switch (cmd.type) {
-    case "REMOVE_COLUMNS_ROWS":
-      return {
-        applyChange: getApplyRangeChangeRemoveColRow(cmd),
-        sheetId: cmd.sheetId,
-        sheetName: cmd.sheetName,
-      };
-    case "ADD_COLUMNS_ROWS":
-      return {
-        applyChange: getApplyRangeChangeAddColRow(cmd),
-        sheetId: cmd.sheetId,
-        sheetName: cmd.sheetName,
-      };
-    case "DELETE_SHEET":
-      return {
-        applyChange: getApplyRangeChangeDeleteSheet(cmd),
-        sheetId: cmd.sheetId,
-        sheetName: cmd.sheetName,
-      };
-    case "RENAME_SHEET":
-      return {
-        applyChange: getApplyRangeChangeRenameSheet(cmd),
-        sheetId: cmd.sheetId,
-        sheetName: cmd.oldName,
-      };
-    case "MOVE_RANGES":
-      return {
-        applyChange: getApplyRangeChangeMoveRange(cmd),
-        sheetId: cmd.sheetId,
-        sheetName: cmd.sheetName,
-      };
+export function getRangeAdapter(cmd: CoreCommand): RangeAdapter | undefined {
+  return rangeAdapterRegistry.get(cmd.type)?.(cmd);
+}
+
+type GetRangeAdapter<C extends CoreCommand> = (cmd: C) => RangeAdapter;
+
+class RangeAdapterRegistry extends Registry<GetRangeAdapter<CoreCommand>> {
+  add<C extends CoreCommandTypes>(
+    cmdType: C,
+    fn: GetRangeAdapter<Extract<CoreCommand, { type: C }>>
+  ): this {
+    super.add(cmdType, fn);
+    return this;
+  }
+  get<C extends CoreCommandTypes>(cmdType: C): GetRangeAdapter<Extract<CoreCommand, { type: C }>> {
+    return this.content[cmdType];
   }
-  return undefined;
 }
 
+export const rangeAdapterRegistry = new RangeAdapterRegistry();
+
+rangeAdapterRegistry
+  .add("REMOVE_COLUMNS_ROWS", (cmd) => ({
+    applyChange: getApplyRangeChangeRemoveColRow(cmd),
+    sheetId: cmd.sheetId,
+    sheetName: { old: cmd.sheetName, current: cmd.sheetName },
+  }))
+  .add("ADD_COLUMNS_ROWS", (cmd) => ({
+    applyChange: getApplyRangeChangeAddColRow(cmd),
+    sheetId: cmd.sheetId,
+    sheetName: { old: cmd.sheetName, current: cmd.sheetName },
+  }))
+  .add("DELETE_SHEET", (cmd) => ({
+    applyChange: getApplyRangeChangeDeleteSheet(cmd),
+    sheetId: cmd.sheetId,
+    sheetName: { old: cmd.sheetName, current: cmd.sheetName },
+  }))
+  .add("RENAME_SHEET", (cmd) => ({
+    applyChange: getApplyRangeChangeRenameSheet(cmd),
+    sheetId: cmd.sheetId,
+    sheetName: { old: cmd.oldName, current: cmd.newName },
+  }))
+  .add("MOVE_RANGES", (cmd) => ({
+    applyChange: getApplyRangeChangeMoveRange(cmd),
+    sheetId: cmd.sheetId,
+    sheetName: { old: cmd.sheetName, current: cmd.sheetName },
+  }));
+
 function getApplyRangeChangeRemoveColRow(cmd: RemoveColumnsRowsCommand): ApplyRangeChange {
   const start: "left" | "top" = cmd.dimension === "COL" ? "left" : "top";
   const end: "right" | "bottom" = cmd.dimension === "COL" ? "right" : "bottom";

src/plugins/core/cell.ts

@@ -20,6 +20,7 @@ import {
   toXC,
 } from "../../helpers/index";
 import {
+  AdaptSheetName,
   AddColumnsRowsCommand,
   ApplyRangeChange,
   Cell,
@@ -72,12 +73,12 @@ export class CellPlugin extends CorePlugin<CoreState> implements CoreState {
   readonly nextId = 1;
   public readonly cells: { [sheetId: string]: { [id: string]: Cell } } = {};
 
-  adaptRanges(applyChange: ApplyRangeChange, sheetId: UID, sheetName: string) {
+  adaptRanges(applyChange: ApplyRangeChange, sheetId: UID, sheetName: AdaptSheetName) {
     for (const sheet of Object.keys(this.cells)) {
       for (const cell of Object.values(this.cells[sheet] || {})) {
         if (cell.isFormula) {
           for (const range of cell.compiledFormula.dependencies) {
-            if (range.sheetId === sheetId || range.invalidSheetName === sheetName) {
+            if (range.sheetId === sheetId || range.invalidSheetName === sheetName.old) {
               const change = applyChange(range);
               if (change.changeType !== "NONE") {
                 this.history.update(

src/plugins/core/conditional_format.ts

@@ -180,7 +180,7 @@ export class ConditionalFormatPlugin
     }
   }
 
-  adaptRanges(applyChange: ApplyRangeChange, sheetId?: UID, sheetName?: string) {
+  adaptRanges(applyChange: ApplyRangeChange, sheetId: UID) {
     const sheetIds = sheetId ? [sheetId] : Object.keys(this.cfRules);
     for (const sheetId of sheetIds) {
       this.adaptCFRanges(sheetId, applyChange);

src/plugins/core/range.ts

@@ -17,6 +17,7 @@ import {
 } from "../../helpers/index";
 import { CellErrorType } from "../../types/errors";
 import {
+  AdaptSheetName,
   ApplyRangeChange,
   ApplyRangeChangeResult,
   Command,
@@ -62,15 +63,15 @@ export class RangeAdapter implements CommandHandler<CoreCommand> {
   // ---------------------------------------------------------------------------
   // Command Handling
   // ---------------------------------------------------------------------------
-  allowDispatch(cmd: Command): CommandResult {
+  allowDispatch(cmd: CoreCommand): CommandResult {
     if (cmd.type === "MOVE_RANGES") {
       return cmd.target.length === 1 ? CommandResult.Success : CommandResult.InvalidZones;
     }
     return CommandResult.Success;
   }
   beforeHandle(command: Command) {}
 
-  handle(cmd: Command) {
+  handle(cmd: CoreCommand) {
     const rangeAdapter = getRangeAdapter(cmd);
     if (rangeAdapter?.applyChange) {
       this.executeOnAllRanges(
@@ -99,7 +100,11 @@ export class RangeAdapter implements CommandHandler<CoreCommand> {
     };
   }
 
-  private executeOnAllRanges(adaptRange: ApplyRangeChange, sheetId: UID, sheetName: string) {
+  private executeOnAllRanges(
+    adaptRange: ApplyRangeChange,
+    sheetId: UID,
+    sheetName: AdaptSheetName
+  ) {
     const func = this.verifyRangeRemoved(adaptRange);
     for (const provider of this.providers) {
       provider(func, sheetId, sheetName);

src/plugins/core_plugin.ts

@@ -1,6 +1,7 @@
 import { ModelConfig } from "../model";
 import { StateObserver } from "../state_observer";
 import {
+  AdaptSheetName,
   ApplyRangeChange,
   CoreCommand,
   CoreCommandDispatcher,
@@ -64,9 +65,10 @@ export class CorePlugin<State = any>
    * the type of change that occurred.
    *
    * @param applyChange a function that, when called, will adapt the range according to the change on the grid
-   * @param sheetId an optional sheetId to adapt either range of that sheet specifically, or ranges pointing to that sheet
+   * @param sheetId an sheetId to adapt either range of that sheet specifically, or ranges pointing to that sheet
+   * @param sheetName couple of old and new sheet names to adapt ranges pointing to that sheet
    */
-  adaptRanges(applyChange: ApplyRangeChange, sheetId: UID, sheetName: string): void {}
+  adaptRanges(applyChange: ApplyRangeChange, sheetId: UID, sheetName: AdaptSheetName): void {}
 
   /**
    * Implement this method to clean unused external resources, such as images

src/registries/inverse_command_registry.ts

@@ -16,6 +16,7 @@ import {
   RemoveMergeCommand,
   RemovePivotCommand,
   RemoveTableStyleCommand,
+  RenameSheetCommand,
   UnhideColumnsRowsCommand,
   coreTypes,
 } from "../types/commands";
@@ -37,7 +38,8 @@ export const inverseCommandRegistry = new Registry<InverseFunction>()
   .add("HIDE_COLUMNS_ROWS", inverseHideColumnsRows)
   .add("UNHIDE_COLUMNS_ROWS", inverseUnhideColumnsRows)
   .add("CREATE_TABLE_STYLE", inverseCreateTableStyle)
-  .add("ADD_PIVOT", inverseAddPivot);
+  .add("ADD_PIVOT", inverseAddPivot)
+  .add("RENAME_SHEET", inverseRenameSheet);
 
 for (const cmd of coreTypes.values()) {
   if (!inverseCommandRegistry.contains(cmd)) {
@@ -150,3 +152,14 @@ function inverseUnhideColumnsRows(cmd: UnhideColumnsRowsCommand): HideColumnsRow
 function inverseCreateTableStyle(cmd: CreateTableStyleCommand): RemoveTableStyleCommand[] {
   return [{ type: "REMOVE_TABLE_STYLE", tableStyleId: cmd.tableStyleId }];
 }
+
+function inverseRenameSheet(cmd: RenameSheetCommand): RenameSheetCommand[] {
+  return [
+    {
+      type: "RENAME_SHEET",
+      sheetId: cmd.sheetId,
+      oldName: cmd.newName,
+      newName: cmd.oldName,
+    },
+  ];
+}

src/registries/srt_registry.ts

@@ -11,14 +11,9 @@ import { Registry } from "./registry";
  *
  */
 
-export type CommandAdaptRangeFunction<C extends CoreCommand> = (
-  cmd: C,
-  applyChange: RangeAdapter
-) => C;
+type CommandAdaptRangeFunction<C extends CoreCommand> = (cmd: C, applyChange: RangeAdapter) => C;
 
-export class SpecificRangeTransformRegistry extends Registry<
-  CommandAdaptRangeFunction<CoreCommand>
-> {
+class SpecificRangeTransformRegistry extends Registry<CommandAdaptRangeFunction<CoreCommand>> {
   add<C extends CoreCommand>(cmdType: C["type"], fn: CommandAdaptRangeFunction<C>): this {
     super.add(cmdType, fn);
     return this;

src/types/misc.ts

@@ -293,9 +293,11 @@ export type ApplyRangeChangeResult =
   | { changeType: "NONE" };
 export type ApplyRangeChange = (range: Range) => ApplyRangeChangeResult;
 
+export type AdaptSheetName = { old: string; current: string };
+
 export type RangeAdapter = {
   sheetId: UID;
-  sheetName: string;
+  sheetName: AdaptSheetName;
   applyChange: ApplyRangeChange;
 };
 
@@ -304,7 +306,7 @@ export type Dimension = "COL" | "ROW";
 export type ConsecutiveIndexes = HeaderIndex[];
 
 export interface RangeProvider {
-  adaptRanges: (applyChange: ApplyRangeChange, sheetId: UID, sheetName: string) => void;
+  adaptRanges: (applyChange: ApplyRangeChange, sheetId: UID, sheetName: AdaptSheetName) => void;
 }
 
 export type Validation<T> = (toValidate: T) => CommandResult | CommandResult[];