It seems that something is broken when generating media name for pinterest media entities. They all have only 'h' a media name. I have also

CommentFileSizeAuthor
#15 2837977_media-name-always-h.patch1.03 KBextect
#14 Warnings_MediaEntityPinterest.txt15.66 KBextect
#8 media_name_always_h-2837977-8.patch1.1 KBchr.fritsch
#5 media_entity_pinterest-media_name_always_h-2837977-5.patch620 bytesIT-Cru
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

IT-Cru created an issue. See original summary.

gg4’s picture

gg4 CreditAttribution: gg4 commented

@IT-Cru, can you provide some quick steps to reproduce so I can look into this?

IT-Cru’s picture

IT-Cru
Primary language English
Location Munich
CreditAttribution: IT-Cru commented

It seems, that in getDefaultName all 3 variables have value 'h'. So I think here is a good starting point to debug. I think it's 'h' from http:// of pinterest URL.

Reproduce:

* Create a pinterest media entity
* Add link URL only field
* Configure URL field for media entity provider
* No API & skip metadata fields

Add following URL https://www.pinterest.com/pin/99360735500167749/ without a media name and safe media entity.

IT-Cru’s picture

IT-Cru
Primary language English
Location Munich
CreditAttribution: IT-Cru commented

In Plugin/MediaEntity/Type/Pinterest.php $matches seems to be incorrect for getting media name after reset($matches) which works for pin URLs but not for other requirements. Index 'id' isn't found and so first element of string array 'h' of http is returned (see added hint from xdebug).

  public function getField(MediaInterface $media, $name) {
    $matches = $this->matchRegexp($media);

    if (!empty($matches)) {
      $matches = reset($matches);
    }
    else {
      return FALSE;
    }

    // First we return the fields that are available from regex.
    switch ($name) {
      case 'id':
        if ($matches['id']) {
          return $matches['id']; // HINT from xdebug::$matches = https://www.pinterest.com/pin/99360735500167749/
        }
        return FALSE;
IT-Cru’s picture

IT-Cru
Primary language English
Location Munich
CreditAttribution: IT-Cru commented
Status: Active » Needs review
FileSize
media_entity_pinterest-media_name_always_h-2837977-5.patch620 bytes
gg4’s picture

gg4 CreditAttribution: gg4 commented

@IT-Cru Thanks for the patch. This looks reasonable, but was not able to reproduce the issue with the steps you provided, specifically the final step of saving the media entity without the _Media name_ field completed. Let me know if I am misunderstanding, but you should not be able to same the entity with this field as it is required.

gg4’s picture

gg4 CreditAttribution: gg4 commented
Version: » 8.x-1.x-dev
Status: Needs review » Postponed (maintainer needs more info)
chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented
Status: Postponed (maintainer needs more info) » Needs review
FileSize
media_name_always_h-2837977-8.patch1.1 KB

I can confirm the bug. I could reproduce it with the steps IT-Cru described in #3.

I extended the patch a bit, with more safe field checks.

daniel.bosen’s picture

daniel.bosen
he/him
CreditAttribution: daniel.bosen at Thunder commented
Status: Needs review » Reviewed & tested by the community

The patch in #8 works for me.

gg4’s picture

gg4 CreditAttribution: gg4 commented

@chr.fritsch @daniel.bosen -- This patch seems sane, but I have still not been able to repro this issue. Would you mind providing more detailed steps you were taking to see this issue?

IT-Cru’s picture

IT-Cru
Primary language English
Location Munich
CreditAttribution: IT-Cru commented

@chr.fritsch and @daniel.bosen: Perhaps this is only an issue with the "media name could be empty" patch from thunder distribution?

gg4’s picture

gg4 CreditAttribution: gg4 commented

@chr.fritsch @daniel.bosen -- Let me know if you are able to reproduce this on a clean Drupal install.

gg4’s picture

gg4 CreditAttribution: gg4 commented
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)
extect’s picture

extect CreditAttribution: extect commented
Version: 8.x-1.x-dev » 8.x-2.x-dev
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Needs review
FileSize
Warnings_MediaEntityPinterest.txt15.66 KB

I can confirm the issue on 8.x-2.x.-dev with Drupal core media module. Once I create pinterest media content, I get several warnings that hint to the problem (see attachment).

extect’s picture

extect CreditAttribution: extect commented
FileSize
2837977_media-name-always-h.patch1.03 KB

The patch in #8 solves the issue. I have adapted it to 8.x-2.x-dev.

gg4’s picture

gg4 CreditAttribution: gg4 commented

@extect Can you provide the steps taken to reproduce this issue on a clean Drupal install, not Thunder distro, if that is what you are using as other have mentioned in this thread?

gg4’s picture

gg4 CreditAttribution: gg4 commented
Status: Needs review » Postponed (maintainer needs more info)
chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented
Status: Postponed (maintainer needs more info) » Needs review

Ok, I reproduced that in plain Drupal.

  1. Create pinterest type
  2. Remove the name field from the form display
  3. Create a media item

After that, I have the same notices in my log.

Patch from #15 solves the problem.

gg4’s picture

gg4 CreditAttribution: gg4 commented

Thanks @chr.fritsch -- confirmed this bug with your steps.

gg4’s picture

gg4 CreditAttribution: gg4 commented
Status: Needs review » Reviewed & tested by the community

Patches work for me. Will commit shortly.

  • bonus committed 5d0ef73 on 8.x-1.x authored by chr.fritsch 
    Issue #2837977 by extect, IT-Cru, chr.fritsch, bonus: Media name always...

  • bonus committed f5b2ebf on 8.x-2.x authored by chr.fritsch 
    Issue #2837977 by extect, IT-Cru, chr.fritsch, bonus: Media name always...
gg4’s picture

gg4 CreditAttribution: gg4 commented
Status: Reviewed & tested by the community » Fixed

Committed to both 8.x-1.x and 8.x-2.x

Status: Fixed » Closed (fixed)

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