Hi,
I'm currently using this module for the first time on a Drupal 8 project, and I like its integration with the metatag module. For my project I'm using the Book sub-module to markup a node type and I encountered a problem within my use case with the ISBN field.
The books on my site have two separate ISBN fields. One for the old ISBN-10 and one for current ISBN-13. On the metatag configuration page, in the ISBN field under WorkExample with Pivot enabled, I have entered the tokens as follows:
[node:field_isbn_10],[node:field_isbn_13]
In the following cases the ISBN field is correctly filled in the LD+JSON output:
- When ISBN-10 has a value, but ISBN-13 has not.
- When ISBN-10 and ISBN-13 have a value. In this case WorkExample has 2 separate objects with each a different ISBN value.
But when ISBN-10 has no value, but ISBN-13 does, the LD+JSON output gives ISBN a null value.
Is this by design or a bug?
Kind regards,
Suraj
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | schema_metatag-pivot-empty-multiple-values-3043425-3.patch | 614 bytes | edwardchiapet |
Comments
Comment #2
SurajHo commentedComment #3
edwardchiapetWe are also noticing the same issue when pivot is enabled on a tag with multiple tokens and if any, but the last, token returns no value, this error is thrown:
Notice: Undefined offset: 0 in Drupal\schema_metatag\SchemaMetatagManager::pivot() (line 140 of /var/www/web/modules/contrib/schema_metatag/src/SchemaMetatagManager.php)In
SchemaNameBase::processItem(), the value is exploded into an array, and any tokens with no value will be included in that position of the array as an empty element:$value = array_filter($value);does remove all the empty elements from the array, but it does not reindex the array to fill in the indices of the removed empty elements. This would need to be updated to$value = array_values(array_filter($value));for the reindexing.For example, if a tag was mapped with
[node:field_foo],[node:field_bar]where[node:field_foo]returns no value, the resulting value array:would become:
instead of the expected:
The attached patch should handle this appropriately.
Comment #4
edwardchiapetComment #5
damienmckennaThis looks reasonable.
Comment #8
damienmckennaCommitted. Thank you.