backdrop-contrib / metatag

Add structured metadata, aka "meta tags", for various pages on your site.
GNU General Public License v2.0
3 stars 15 forks source link

The configuration "metatag.instance.node.page" must have the "name" attribute specified. #76

Closed herbdool closed 2 years ago

herbdool commented 3 years ago

Error message when trying sync configuration:

The configuration "metatag.instance.node.page" must have the "name" attribute specified.

I don't see this option in the settings. The global metags have "name" attribute but not these instances. This prevents any configuration syncing. I may have to disable the module until this can be fixed.

yorkshire-pudding commented 2 years ago

I'm getting a similar issue with

 The configuration "metatag.instance.node.article" must have a "label" attribute specified.

If I manually create a setting for article it doesn't have a label attribute.

@jenlampton @quicksketch - I think not being able to sync config of metatag is quite a significant issue, please can you look at it.

I found something similar in this issue, so tried disabling, uninstalling and re-enabling metatag in case there was an issue with the original config, but still get the same.

quicksketch commented 2 years ago

Putting my thoughts here for reference... these errors come from metatag having specified name and label keys in hook_config_info():

function metatag_config_info() {
  // Metatag settings.
  $prefixes['metatag.settings'] = array(
    'label' => t('Metatag settings'),
    'group' => t('Configuration'),
  );

  // Metatag instances.
  $prefixes['metatag.instance'] = array(
    'name_key' => 'name', // <----- Name key missing will throw errors
    'label_key' => 'label', // <---- Label key missing will throw errors
    'group' => t('Metatag'),
  );

  return $prefixes;
}

None of my metatag configuration files have a label key in them. Other config types like image styles simply use the same label and name key. So perhaps that might work here?

@yorkshire-pudding Could you try setting 'label_key' => 'name' and see if that solves the problem on your site? That may be the correct solution for metatag module to make.

yorkshire-pudding commented 2 years ago

@quicksketch - thanks for getting back to me. Doing that, gets me the same issue as @herbdool but does clear up the label issue

 The configuration "metatag.instance.node.service" must have the "name" attribute specified.
quicksketch commented 2 years ago

Well, maybe half the battle then! We'll need to check where metatag is saving the config and make sure a name is specified. Is this a clean install or an upgrade? Either way an update hook to make sure names are specified is needed.

yorkshire-pudding commented 2 years ago

It was an upgrade, but I did completely uninstall and reinstall as per comment above to see if that was the issue.

yorkshire-pudding commented 2 years ago

@quicksketch - any joy on this? It's making syncing config changes for this module a bit of a nightmare. I'm having to just copy these into active rather than import, which is not ideal.

yorkshire-pudding commented 2 years ago

After quite a bit of investigation, I've found what the issue is and prepared a fix.

metatag_config_save is not adding a name and label attribute.

metatag_update_1003 in metatag.install gave default instances names and labels but there is no mechanism to add these for any new configs.

I have prepared a PR that:

I appreciate you'll all be a bit busy today, so no pressure.

@herbdool - please consider testing as the issue was originally raised by you @quicksketch / @jenlampton - please can you review at your earliest convenience

klonos commented 2 years ago

Thanks for providing a PR for this @yorkshire-pudding 🙏🏼 ...I've left comments in the PR with code suggestions (mostly fixing coding standards issues, like missing or extraneous spaces, wrong indentation etc.)

yorkshire-pudding commented 2 years ago

Thanks @klonos - no idea how that deletion happened. I've resolved the whitespace issues. Hopefully the commits can be squashed as git went a bit crazy on my local. I still struggle with dealing with merge conflicts.

yorkshire-pudding commented 2 years ago

While investigating #98 I found that metatag/metatag_views/config/metatag.instance.view.json does not have a name or label attribute so added these as part of this.

quicksketch commented 2 years ago

Great work @yorkshire-pudding! I've confirmed that your PR fixes the problems with metatag config import/export and it fixes it on my current site with existing config. This looks ready to go to me.

quicksketch commented 2 years ago

I merged https://github.com/backdrop-contrib/metatag/pull/97 and I'll make a new release. Thanks @yorkshire-pudding!