watermarkchurch/contentful-schema-diff

field add / remove improperly generate update sometimes

jpowell opened this issue · 7 comments

I removed a field and added a field, that just so happened to look like a modification to the diff algorithm. See generated migration below:

/*
   {
-    id: "tag"
+    id: "internalTitle"
-    name: "Tag"
+    name: "Internal Title"
+    required: true
   }
   ...
 ]
 */

sectionBlockText.editField('internalTitle')
  .id('internalTitle')
  .name('Internal Title')
  .required(true)

Justin and I are now of the opinion that instead of writing out a .newId( modification, cases like this should be handled by writing out a createField('newFieldId') and then a deleteField('oldFieldId'), potentially with a no-op transformEntries in-between.

I have a similar issue, where the field type of the removed and inserted are both are references. The error is:

Error: Diff produced a "~" with a non-diff obj:
["~",{"items__added":{"type":"Link","validations":[{"linkContentType":["tag"]}],"linkType":"Entry"},"id":
{"__old":"section","__new":"tags"},"name":{"__old":"Section","__new":"Tags"},
"type":{"__old":"Symbol","__new":"Array"},"omitted": {"__old":false,"__new":true}}]
    at .\node_modules\contentful-schema-diff\dist\modify.js:151:43
    at Array.forEach (<anonymous>)
    at Object.<anonymous> (.\node_modules\contentful-schema-diff\dist\modify.js:125:32)
    at step (.\node_modules\contentful-schema-diff\dist\modify.js:33:23)
    at Object.next (.\node_modules\contentful-schema-diff\dist\modify.js:14:53)
    at fulfilled (.\npm\node_modules\contentful-schema-diff\dist\modify.js:5:58)

I made a dirty fix by exporting a version where the old field is deleted and another where the new field is added. This yields two migrations but at least it works this way.

Thanks @Jgfrausing ! That error is actually pretty helpful. Currently the tool doesn't have good support for deleting fields (it has basic support but complex scenarios will trip it up.)

If you'd like, you're welcome to write a pull request with a new test for this scenario, and to add handling for it!

Hi @gburgett, I will see how often the issue occurs. If it becomes too much a hassle to deal with, I will look into creating a PR. Thank you for an otherwise super great tool. Strange that Contentful did not create this themselves. 🤔

If I had the time to look into it, I'd attack the problem this way: If the diff produces a ~ on a field, and the id or type of the field has changed, then write a delete followed by a create instead of writing an editField.

One thing to consider is whether that would require multiple migrations - we'd need to test this! I vaguely remember that Contentful requires the field to be marked as "not included in response" and to save that first before you can delete it.

@gburgett have we seen this be an issue lately?

nope