I started using this module on beta 16 and found it a bit confusing that there was a cardinality setting for this field.

This may be OOTB for all field types, but would there ever be a case in which there is more than one meta tag field per node/entity?

Comments

nerdstein created an issue. See original summary.

damienmckenna’s picture

Title: [meta] Should metatag field restrict cardinality? » Restrict Metatag field restrict cardinality
Category: Feature request » Bug report
Parent issue: » #2563605: Plan for Metatag 8.x-1.0-beta1 release

There's code in it that used to work, but it must have stopped working recently.

larowlan’s picture

There are tests in comment module for this, will adapt

michelle’s picture

This is the code in question. It did work last Spring. :) But that was a number of D8 Betas ago.

/**
 * Implements hook_form_alter().
 */
function metatag_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  // Disable the cardinality option on the field storage settings form for
  // metatag fields.
  if ($form_id == 'field_ui_field_storage_edit_form') {
    if ($form['#field']->getType() == 'metatag') {
      $form['field_storage']['cardinality_container']['#prefix'] = t("Metatag fields must be singular.");
      $form['field_storage']['cardinality_container']['#disabled'] = TRUE;
      $form['field_storage']['cardinality_container']['cardinality']['#disabled'] = TRUE;
      $form['field_storage']['cardinality_container']['cardinality_number']['#disabled'] = TRUE;
      unset($form['field_storage']['cardinality_container']['cardinality_number']['#states']);
    }
  }
}
michelle’s picture

Status: Active » Needs review
StatusFileSize
new1.7 KB

Looks like a few things have changed. Here's a patch that works with Beta 16. (Sorry, haven't gone to RC 1 just yet. :) )

damienmckenna’s picture

Status: Needs review » Fixed

Committed. Thanks Michelle!

larowlan’s picture

without a test :( - should I add a new issue for a follow up? Or add test here?

damienmckenna’s picture

Please add new issue for tests for existing changes, or just roll it into the main "add tests" issue.

michelle’s picture

@larowlan - Sorry. :( Learning to write tests is on my "to learn" list but I haven't gotten to it, yet.

larowlan’s picture

Added test coverage in #2563637: Write tests for the 8.x-1.x functionality for this bug.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.