Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(editor-interface): control changes of newly created fields should not trigger errors for editor interfaces with editorLayout property #1142

Conversation

ruderngespra
Copy link
Contributor

@ruderngespra ruderngespra commented Nov 10, 2022

We are seeing a few customer complaints for migration scripts which create new fields on a content type and then immediately change field controls for the newly created field in the same migration script. Customers run into a validation error and then are unable to do the field control changes. The reason for this is whenever field updates happen on content types which have corresponding editorInterfaces with editorLayout properties, those editorInterfaces are silently updated as well by our API. Contentful-migration however does not know about this and therefore sends an outdated payload to the editor_interface endpoint.

This PR aims to hotfix the issue for creating fields by adding a custom check during editorInterface updates that detects whether for a given editorInterface there is a fieldId missing in editorLayout and if so silently adds the id to the first group. This way, it mimics the implicit update to the editorInterface which silently happened in the backend and thereby avoids validation errors on the update.

what this is not yet solving

  • Similar cases for deleting fields or changing field ids. For that, we would need different patching logic.

On the long run, we should make sure that the migration tool always has access to the updated editorInterfaces it applies migrations to, so we might think about re-fetching editorInterfaces after content type updates which involve field updates.

todos

  • double check with team that owns editorLayout if two layers of nesting is enough ✅
  • For now we are just adding the fieldId to the first group in the list. Is there a better way to chose to which group the fieldId should be added? (e.g. a marked default?) --> The first item is used as default everywhere, so we are fine ✅
  • Create follow up tickets for the actual fix ✅

@ruderngespra ruderngespra force-pushed the fix/zend-2771-add-new-field-to-controls-with-editor-interface-that-uses-editor-layout branch from 098ba6d to ade4f7c Compare November 10, 2022 11:54
…rLayout present

fix(editor-interface): add more unit tests for field control updates
…ontrol updates

fix(editor-interface): use available util functions for id check
@ruderngespra ruderngespra force-pushed the fix/zend-2771-add-new-field-to-controls-with-editor-interface-that-uses-editor-layout branch from 5970bbb to 779784b Compare November 10, 2022 14:28
@ruderngespra ruderngespra requested a review from Vinz93 November 10, 2022 15:21
@ruderngespra ruderngespra marked this pull request as ready for review November 10, 2022 15:21
@ruderngespra ruderngespra requested a review from a team as a code owner November 10, 2022 15:21
@ruderngespra ruderngespra requested review from veu and Bikappa November 10, 2022 15:21
Copy link
Contributor

@andreascful andreascful left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good concise change. Great that you figured out this hard issue!

@ruderngespra ruderngespra changed the title fix(editor-interface-controls): control changes of newly created fields should not trigger errors for editor interfaces with editorLayout property fix(editor-interface): control changes of newly created fields should not trigger errors for editor interfaces with editorLayout property Nov 11, 2022
@damienxy damienxy merged commit 20efd48 into master Nov 14, 2022
@damienxy damienxy deleted the fix/zend-2771-add-new-field-to-controls-with-editor-interface-that-uses-editor-layout branch November 14, 2022 14:32
@contentful-automation
Copy link

🎉 This PR is included in version 4.12.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants