From 20efd483ad81517ee664e0ab2386d1adac8c2950 Mon Sep 17 00:00:00 2001 From: Janko Marklein <43542437+ruderngespra@users.noreply.github.com> Date: Mon, 14 Nov 2022 15:32:28 +0100 Subject: [PATCH] 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 <816004+Silhoue@users.noreply.github.com> --- ...-on-editor-interface-with-editor-layout.js | 12 ++++ ...ield-on-content-type-with-editor-layout.js | 12 ++++ src/lib/entities/content-type.ts | 19 ++++++ test/integration/migration.spec.js | 32 ++++++++++ .../lib/entities/editor-interface.spec.ts | 61 +++++++++++++++++++ 5 files changed, 136 insertions(+) create mode 100644 examples/50-change-field-control-on-editor-interface-with-editor-layout.js create mode 100644 examples/51-move-field-on-content-type-with-editor-layout.js diff --git a/examples/50-change-field-control-on-editor-interface-with-editor-layout.js b/examples/50-change-field-control-on-editor-interface-with-editor-layout.js new file mode 100644 index 000000000..8cda25e03 --- /dev/null +++ b/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') +} diff --git a/examples/51-move-field-on-content-type-with-editor-layout.js b/examples/51-move-field-on-content-type-with-editor-layout.js new file mode 100644 index 000000000..fc4e9e3fb --- /dev/null +++ b/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') +} diff --git a/src/lib/entities/content-type.ts b/src/lib/entities/content-type.ts index e155849de..2c199ea45 100644 --- a/src/lib/entities/content-type.ts +++ b/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) { diff --git a/test/integration/migration.spec.js b/test/integration/migration.spec.js index ffca9abe7..a13deb11f 100644 --- a/test/integration/migration.spec.js +++ b/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) diff --git a/test/unit/lib/entities/editor-interface.spec.ts b/test/unit/lib/entities/editor-interface.spec.ts index f0ad08d0a..67ddcb6cf 100644 --- a/test/unit/lib/entities/editor-interface.spec.ts +++ b/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', () => {