datocms / cli

Official CLI to interact with DatoCMS projects!
MIT License
7 stars 7 forks source link

Migration issue: menu order on newly created groupings #14

Closed lucasconstantino closed 1 year ago

lucasconstantino commented 1 year ago

Hello! First of all, thanks A LOT for the quick pace on developing tooling around DatoCMS. Much appreciated, and I'm very eager to help with some PRs when I get the time. Until then, I'll post some issues with minor problems I find in the way.

Wrong creation order for nested menu items Upon creating multiple menu items, the current order is set to the item's ID found on the source environment. This brings the following issue:

  log('Create menu item "Tags"')
  carry.newMenuItems['641313'] = await client.menuItems.create({
    label: 'Tags',
    item_type: carry.newItemTypes['1237046'],
    parent: carry.newMenuItems['697016'],
  })

  log('Create menu item "Articles"')
  carry.newMenuItems['641314'] = await client.menuItems.create({
    label: 'Articles',
    item_type: carry.newItemTypes['1237047'],
    parent: carry.newMenuItems['697016'],
  })

  log('Create menu item "Blog"')
  carry.newMenuItems['697016'] = await client.menuItems.create({ label: 'Blog' })

As you can see, item named "Blog" is the parent item for items "Tags" and "Articles" – but as it's being created last, the nesting doesn't happen as expected.

I suggest one of two solutions:

  1. Proper: ensure the diff system recognizes dependencies, and sort them accordingly;
  2. Easy: ensure "parent" field is always included when updating item's position, right after.
matjack1 commented 1 year ago

hey @lucasconstantino !

In your code example, it's difficult for us to do anything as you are passing references to objects that will change in the future, like the menu item "Blog".

When you are creating the first items there's no knowledge about the last one, so we cannot help there, or am I missing something?

One way to solve this would be to pass the full menu items tree so that we could build it all in one go, but this is not an existing feature. I guess you need to keep the tree structure in your end and reconstruct it by navigating the tree appropriately. Not sure if this helps :)

lucasconstantino commented 1 year ago

Sorry sorry @matjack1, I made it hard for you really: the above code is a slightly modified version from an auto-generated migration script – and that is the actual problem. This is I believe the original:

  console.log('Create menu item "Tags"')
  newMenuItems['641313'] = await client.menuItems.create({
    label: 'Tags',
    item_type: newItemTypes['1237046'],
    parent: newMenuItems['697016'],
  })

  console.log('Create menu item "Articles"')
  newMenuItems['641314'] = await client.menuItems.create({
    label: 'Articles',
    item_type: newItemTypes['1237047'],
    parent: newMenuItems['697016'],
  })

  console.log('Create menu item "Blog"')
  newMenuItems['697016'] = await client.menuItems.create({ label: 'Blog' })

So, I definitely understand the problem you pointed with the code, my problem and the reason to report is that this was autogenerated by @datocms/cli. I did fix it myself for my case, so no need for support; I'm just reporting the problem with the migration generation script on these kind of nested menu cases :)

stefanoverna commented 1 year ago

Thanks for clarifying @lucasconstantino! Fix shipped in v1.1.7

Before (v1.1.6):

module.exports = async function (client) {
  const newMenuItems = {};

  newMenuItems["874212"] = await client.menuItems.create({
    label: "Foo",
    parent: newMenuItems["874213"],
  });

  newMenuItems["874213"] = await client.menuItems.create({ label: "Bar" });

  newMenuItems["874214"] = await client.menuItems.create({
    label: "Qux",
    parent: newMenuItems["874213"],
  });
};

After (v1.1.7):

module.exports = async function (client) {
  const newMenuItems = {};

  newMenuItems["874213"] = await client.menuItems.create({ label: "Bar" });

  newMenuItems["874212"] = await client.menuItems.create({
    label: "Foo",
    parent: newMenuItems["874213"],
  });

  newMenuItems["874214"] = await client.menuItems.create({
    label: "Qux",
    parent: newMenuItems["874213"],
  });
};