(core) TypeTransform race condition fix

Summary:
TypeTransformation was flaky. Probably after upgrading AceEditor we introduced a race condition between updating the revised formula and doing the transformation. Now we explicitly make sure that the formula is updated.

I also fixed some other flaky tests.

Test Plan: Updated

Reviewers: paulfitz

Reviewed By: paulfitz

Subscribers: paulfitz

Differential Revision: https://phab.getgrist.com/D3984

app/client/components/ColumnTransform.ts

@@ -4,6 +4,7 @@
  * and TypeTransform.
  */
 import * as commands from 'app/client/components/commands';
+import * as AceEditor from 'app/client/components/AceEditor';
 import {GristDoc} from 'app/client/components/GristDoc';
 import {ColumnRec} from 'app/client/models/entities/ColumnRec';
 import {ViewFieldRec} from 'app/client/models/entities/ViewFieldRec';
@@ -88,6 +89,13 @@ export class ColumnTransform extends Disposable {
    * @param {String} optInit - Optional initial value for the editor.
    */
   protected buildEditorDom(optInit?: string) {
+    if (!this.editor) {
+      this.editor = this.autoDispose(AceEditor.create({
+        gristDoc: this.gristDoc,
+        observable: this.transformColumn.formula,
+        saveValueOnBlurEvent: false,
+      }));
+    }
     return this.editor.buildDom((aceObj: any) => {
       this.editor.adjustContentToWidth();
       this.editor.attachSaveCommand();
@@ -238,6 +246,7 @@ export class ColumnTransform extends Disposable {
      {...this.origWidgetOptions as object, ...this._fieldBuilder.options.peek()} :
      this._fieldBuilder.options.peek();
     return [
+      ...this.previewActions(),
       [
         'CopyFromColumn',
         this._tableData.tableId,
@@ -268,4 +277,23 @@ export class ColumnTransform extends Disposable {
   protected isFinalizing(): boolean {
     return this._isFinalizing;
   }
+
+  protected preview() {
+    if (!this.editor) { return; }
+    return this.editor.writeObservable();
+  }
+
+  /**
+   * Generates final actions before executing the transform. Used only when the editor was created.
+   */
+  protected previewActions(): UserAction[] {
+    if (!this.editor) { return []; }
+    const formula = this.editor.getValue();
+    const oldFormula = this.transformColumn.formula();
+    if (formula === oldFormula) { return []; }
+    if (!formula && !oldFormula) { return []; }
+    return [
+      ['UpdateRecord', '_grist_Tables_column', this.transformColumn.getRowId(), {formula}]
+    ];
+  }
 }

app/client/components/FormulaTransform.ts

@@ -5,7 +5,6 @@
  */
 
 // Client libraries
-import * as AceEditor from 'app/client/components/AceEditor';
 import {ColumnTransform} from 'app/client/components/ColumnTransform';
 import {GristDoc} from 'app/client/components/GristDoc';
 import {cssButtonRow} from 'app/client/ui/RightPanelStyles';
@@ -26,10 +25,6 @@ export class FormulaTransform extends ColumnTransform {
    * Build the transform menu for a formula transform
    */
   public buildDom() {
-    this.editor = this.autoDispose(AceEditor.create({
-      gristDoc: this.gristDoc,
-      observable: this.transformColumn.formula,
-    }));
     return [
       dom('div.transform_menu',
         dom('div.transform_editor',
@@ -40,7 +35,7 @@ export class FormulaTransform extends ColumnTransform {
       cssButtonRow(
         basicButton(dom.on('click', () => this.cancel()),
           'Cancel', testId("formula-transform-cancel")),
-        basicButton(dom.on('click', () => this.editor.writeObservable()),
+        basicButton(dom.on('click', () => this.preview()),
           'Preview',
           dom.cls('disabled', this.formulaUpToDate),
           { title: 'Update formula (Shift+Enter)' },

app/client/components/TypeTransform.ts

@@ -5,7 +5,6 @@
  * to be pre-entered for certain transforms (to Reference / Date) which the user can modify via dropdown menus.
  */
 
-import * as AceEditor from 'app/client/components/AceEditor';
 import {ColumnTransform} from 'app/client/components/ColumnTransform';
 import {GristDoc} from 'app/client/components/GristDoc';
 import * as TypeConversion from 'app/client/components/TypeConversion';
@@ -50,12 +49,7 @@ export class TypeTransform extends ColumnTransform {
   public buildDom() {
     // An observable to disable all buttons before the dom get removed.
     const disableButtons = Observable.create(null, false);
-
     this._reviseTypeChange.set(false);
-    this.editor = this.autoDispose(AceEditor.create({
-      gristDoc: this.gristDoc,
-      observable: this.transformColumn.formula,
-    }));
     return dom('div',
       testId('type-transform-top'),
       dom.maybe(this._transformWidget, transformWidget => transformWidget.buildTransformConfigDom()),
@@ -71,7 +65,7 @@ export class TypeTransform extends ColumnTransform {
         ),
         dom.domComputed(this._reviseTypeChange, revising => {
           if (revising) {
-            return basicButton(dom.on('click', () => this.editor.writeObservable()),
+            return basicButton(dom.on('click', () => this.preview()),
               t('Preview'), testId("type-transform-update"),
               dom.cls('disabled', (use) => use(disableButtons) || use(this.formulaUpToDate)),
               { title: t('Update formula (Shift+Enter)') }

test/nbrowser/GridOptions.ntest.js

@@ -42,6 +42,7 @@ describe("GridOptions.ntest", function() {
     await gu.supportOldTimeyTestCode();
     await gu.useFixtureDoc(cleanup, "World-v10.grist", true);
     await $('.test-gristdoc').wait();
+    await gu.hideBanners();
   });
 
   beforeEach(async function() {
@@ -109,6 +110,7 @@ describe("GridOptions.ntest", function() {
     await driver.navigate().refresh();
     //await $.injectIntoPage();
     await gu.waitForDocToLoad();
+    await gu.hideBanners();
     await assertHVZ(0, true, true, true);     // all on
     await assertHVZ(1, false, false, false);  // all off
     await assertHVZ(2, false, true, true);    // -h +v +z

test/nbrowser/gristUtils.ts

@@ -976,6 +976,14 @@ export async function confirm(save = true, remember = false) {
   }
 }
 
+/** Hides all top banners by injecting css style */
+export async function hideBanners() {
+  const style = `.test-banner-element { display: none !important; }`;
+  await driver.executeScript(`const style = document.createElement('style');
+    style.innerHTML = ${JSON.stringify(style)};
+    document.head.appendChild(style);`);
+}
+
 /**
  * Returns the left-panel item for the given page, given by a full string name, or a RegExp.
  * You may simply click it to switch to that page.
@@ -1582,10 +1590,11 @@ export async function sendKeys(...keys: string[]) {
 }
 
 /**
- * Clears active input by sending HOME + SHIFT END + DELETE.
+ * Clears active input/textarea by sending CTRL HOME + CTRL + SHIFT END + DELETE.
  */
 export async function clearInput() {
-  return sendKeys(Key.HOME, Key.chord(Key.SHIFT, Key.END), Key.DELETE);
+  const ctrl = await modKey();
+  return sendKeys(Key.chord(ctrl, Key.HOME), Key.chord(ctrl, Key.SHIFT, Key.END), Key.DELETE);
 }
 
 /**