From 07c48aa0df6c7b587a44019985813385236502f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9mi=20Rahir=20=28rar=29?= <rar@odoo.com>
Date: Wed, 13 Nov 2024 16:33:14 +0100
Subject: [PATCH] [FIX] evaluation: Ensure dependency invalidation for array
 formulas
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

How to reproduce:
-> see the attached test

What is happening?
During the evaluation of SUMIFS, it will fetch its ranges.
First it will first fetch an empty range (E4:E7)  since MUNIT hasn't
been evaluated yet.
Afterwards, it will fetch H4, which in turn will request C4
and therefore force the evaluation of the spreaded function MUNIT.

AT this point, there is a disparity of values because the values
of the range (E4:E7) have changed.
To address those situations, the evaluator invalidates the dependencies
of all the spreaded zone. Which means marking A1 to be recomputed.

Unfortunately, A1 is still being evaluated and since #4589 (specifically
commit 8d22e86) we remove the cell from the queue after we evaluted it.

This revision changes the approach taken in 8d22e86 to allow
legitimate invalidations take place while avoiding useless
reevaluations.

Task: 4252800
Part-of: odoo/o-spreadsheet#5214
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
---
 .../cell_evaluation/evaluator.ts              |  5 ++++-
 .../evaluation_formula_array.test.ts          | 19 ++++++++++++++++---
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/plugins/ui_core_views/cell_evaluation/evaluator.ts b/src/plugins/ui_core_views/cell_evaluation/evaluator.ts
index 03cbe38fa6..9ad782b07e 100644
--- a/src/plugins/ui_core_views/cell_evaluation/evaluator.ts
+++ b/src/plugins/ui_core_views/cell_evaluation/evaluator.ts
@@ -294,6 +294,9 @@ export class Evaluator {
       }
       for (let i = 0; i < positions.length; ++i) {
         const position = positions[i];
+        if (this.nextPositionsToUpdate.has(position)) {
+          continue;
+        }
         const evaluatedCell = this.computeCell(position);
         if (evaluatedCell !== EMPTY_CELL) {
           this.evaluatedCells.set(position, evaluatedCell);
@@ -339,7 +342,6 @@ export class Evaluator {
       return createEvaluatedCell(e);
     } finally {
       this.cellsBeingComputed.delete(cellId);
-      this.nextPositionsToUpdate.delete(position);
     }
   }
 
@@ -401,6 +403,7 @@ export class Evaluator {
     const invalidatedPositions = this.formulaDependencies().getCellsDependingOn(
       excludeTopLeft(resultZone).map((zone) => ({ sheetId, zone }))
     );
+    invalidatedPositions.delete({ sheetId, col: resultZone.left, row: resultZone.top });
     this.nextPositionsToUpdate.addMany(invalidatedPositions);
   }
 
diff --git a/tests/evaluation/evaluation_formula_array.test.ts b/tests/evaluation/evaluation_formula_array.test.ts
index 3ffdb618c6..f4e3b9b414 100644
--- a/tests/evaluation/evaluation_formula_array.test.ts
+++ b/tests/evaluation/evaluation_formula_array.test.ts
@@ -654,6 +654,19 @@ describe("evaluate formulas that return an array", () => {
       expect(mockCompute).toHaveBeenCalledTimes(3);
     });
 
+    test("Formula depending on array formula is reevaluated when the array formula result changes", () => {
+      const model = new Model();
+      setCellContent(model, "A1", "=sumifs(E4:E7,H4:H7,1)");
+      setCellContent(model, "C4", "=MUNIT(4)");
+      setCellContent(model, "H4", "=C4");
+      setCellContent(model, "H6", "=E6");
+      expect(getEvaluatedCell(model, "A1").value).toBe(1);
+
+      // Force a reevaluation to avoid the incremental evaluation following each update_cell
+      model.dispatch("EVALUATE_CELLS");
+      expect(getEvaluatedCell(model, "A1").value).toBe(1);
+    });
+
     test("Spreaded formulas with range deps invalidate only once the dependencies of themselves", () => {
       let c = 0;
       functionRegistry.add("INCREMENTONEVAL", {
@@ -707,9 +720,9 @@ describe("evaluate formulas that return an array", () => {
 
       setCellContent(model, "A1", "2");
 
-      expect(getEvaluatedCell(model, "B1").value).toBe("#SPILL!");
-      expect(getEvaluatedCell(model, "B2").value).toBe(null);
-      expect(getEvaluatedCell(model, "B3").value).toBe(0);
+      expect(getEvaluatedCell(model, "B1").value).toBe(42);
+      expect(getEvaluatedCell(model, "B2").value).toBe(42);
+      expect(getEvaluatedCell(model, "B3").value).toBe(42);
 
       expect(getEvaluatedCell(model, "A3").value).toBe("#SPILL!");
     });