fix(editor-interface): control changes of newly created fields should…

… not trigger errors for editor interfaces with editorLayout property (#1142)

* fix(editor-interface): add tests for field control updates with editorLayout present
* fix(editor-interface): add more unit tests for field control updates
* fix(editor-interface): if editorLayout exists add field id on field control updates
* fix(editor-interface): use available util functions for id check
* fix(editor-interface): test moving new field on ct with editorLayout
* Update src/lib/entities/content-type.ts

Co-authored-by: Joanna Zakrzewska <[email protected]>

examples/50-change-field-control-on-editor-interface-with-editor-layout.js

@@ -0,0 +1,12 @@
+module.exports = function (migration) {
+  const contentTypeWithEditorLayout = migration.editContentType('page')
+
+  contentTypeWithEditorLayout
+    .createField('additionalField')
+    .type('Text')
+    .name('additionalFieldName')
+    .localized(true)
+    .validations([{ size: { max: 3 } }])
+
+  contentTypeWithEditorLayout.changeFieldControl('additionalField', 'builtin', 'singleLine')
+}

examples/51-move-field-on-content-type-with-editor-layout.js

@@ -0,0 +1,12 @@
+module.exports = function (migration) {
+  const contentTypeWithEditorLayout = migration.editContentType('page')
+
+  contentTypeWithEditorLayout
+    .createField('anotherAdditionalField')
+    .type('Text')
+    .name('anotherAdditionalFieldName')
+    .localized(true)
+    .validations([{ size: { max: 3 } }])
+
+  contentTypeWithEditorLayout.moveField('anotherAdditionalField').beforeField('additionalField')
+}

src/lib/entities/content-type.ts

@@ -228,6 +228,25 @@ class EditorInterfaces {
       this._controls.push(control)
     }
 
+    // For existing editorInterfaces which use the editorLayout property, we need to check if the
+    // field is referenced, as the API requires every fieldId present in the content type to also be
+    // referenced in editorLayout.
+    if (this._editorLayout?.length > 0) {
+      const fieldIdIsNotPresentInExistingEditorLayout = !findEditorLayoutItem(
+        this._editorLayout,
+        (editorLayoutItem) => isFieldItem(editorLayoutItem) && editorLayoutItem.fieldId === fieldId
+      )
+
+      if (fieldIdIsNotPresentInExistingEditorLayout) {
+        // Add field id to the first group as default
+        // This is not ideal, as it is implicit unexpected behavior, but will prevent migrations
+        // from failing completely
+        this._editorLayout[0].items.push({
+          fieldId
+        })
+      }
+    }
+
     control.widgetId = widgetId
 
     if (settings) {

test/integration/migration.spec.js

@@ -37,6 +37,8 @@ const clearFieldAnnotations = require('../../examples/44-clear-field-annotations
 const clearContentTypeAnnotations = require('../../examples/45-clear-content-type-annotations')
 const canSetDisplayFieldBeforeAnnotations = require('../../examples/46-can-set-display-field-before-annotations')
 const createResourceLinkFields = require('../../examples/47-create-resource-link-fields')
+const changeFieldControlOnEditorInterfaceWithEditorLayout = require('../../examples/50-change-field-control-on-editor-interface-with-editor-layout')
+const moveFieldOnContentTypeWithEditorLayout = require('../../examples/51-move-field-on-content-type-with-editor-layout')
 
 const { createMigrationParser } = require('../../built/lib/migration-parser')
 const { DEFAULT_SIDEBAR_LIST } = require('../../built/lib/action/sidebarwidget')
@@ -1005,6 +1007,36 @@ describe('the migration', function () {
     ])
   })
 
+  it('adds new field and immediately change field control', async function () {
+    await migrator(changeFieldControlOnEditorInterfaceWithEditorLayout)
+
+    const editorInterface = await request({
+      method: 'GET',
+      url: '/content_types/page/editor_interface'
+    })
+
+    // We expect the newly created field to be present in the controls group
+    expect(
+      editorInterface.controls.some(({ fieldId }) => {
+        return fieldId === 'additionalField'
+      })
+    ).to.eql(true)
+  })
+
+  it('adds new field and immediately moves it on contentType with editorLayout', async function () {
+    await migrator(moveFieldOnContentTypeWithEditorLayout)
+
+    const contentType = await request({
+      method: 'GET',
+      url: '/content_types/page'
+    })
+
+    // anotherAdditionalField should be second last in the list, right before 'additionalField'
+    // Note that this is not testing moving of fields on editorLayout which currently still breaks
+    // for newly created fields and needs a fix.
+    expect(contentType.fields[2].id).to.eql('anotherAdditionalField')
+  })
+
   it('deletes editor layout and group controls', async function () {
     await migrator(deleteEditorLayout)
 

test/unit/lib/entities/editor-interface.spec.ts

@@ -159,6 +159,67 @@ describe('EditorInterfaces', () => {
         widgetNamespace: 'extension'
       }
     ])
+
+    expect(editorInterface.getEditorLayout()).to.eql(undefined)
+  })
+
+  it('adds field id to editorLayout on field control namespace update for interfaces with editorLayout', () => {
+    const control = {
+      fieldId: 'name',
+      widgetNamespace: 'builtin',
+      widgetId: 'singleLine',
+      settings: {
+        my: 'setting'
+      }
+    }
+
+    const editorInterface = makeEditorInterface({
+      controls: [control as APIEditorInterfaceControl],
+      editorLayout: [
+        {
+          groupId: 'tab',
+          items: [
+            { fieldId: 'a' },
+            { groupId: 'fieldSet', items: [{ fieldId: 'b' }, { fieldId: 'c' }] },
+            { fieldId: 'd' }
+          ]
+        }
+      ]
+    })
+
+    editorInterface.update(control.fieldId, control.widgetId, null, 'extension')
+
+    expect(editorInterface.getEditorLayout()[0].items.length).to.eql(4)
+    expect(editorInterface.getEditorLayout()[0].items[3]).to.eql({ fieldId: control.fieldId })
+  })
+
+  it('will not add duplicate field id to editorLayout if id already exists', () => {
+    const control = {
+      fieldId: 'c',
+      widgetNamespace: 'builtin',
+      widgetId: 'singleLine',
+      settings: {
+        my: 'setting'
+      }
+    }
+
+    const editorInterface = makeEditorInterface({
+      controls: [control as APIEditorInterfaceControl],
+      editorLayout: [
+        {
+          groupId: 'tab',
+          items: [
+            { fieldId: 'a' },
+            { groupId: 'fieldSet', items: [{ fieldId: 'b' }, { fieldId: 'c' }] },
+            { fieldId: 'd' }
+          ]
+        }
+      ]
+    })
+
+    editorInterface.update(control.fieldId, control.widgetId, null, 'extension')
+
+    expect(editorInterface.getEditorLayout()[0].items.length).to.eql(3)
   })
 
   it('configures editor', () => {