When adding a background image to an entity (without inline form) then the target for the background image entity contains the id of the entity instead of the uuid. I'll be submitting a patch in a minute.

CommentFileSizeAuthor
#6 3032110-7.patch5.87 KBmarkhalliwell
#2 3032110-2.background_image.Mixed-usage-of-entity-uuid-and-id.patch891 bytesjurgenhaas

Comments

jurgenhaas created an issue. See original summary.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented
Status: Active » Needs review
StatusFileSize
new 3032110-2.background_image.Mixed-usage-of-entity-uuid-and-id.patch891 bytes
markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs review » Postponed (maintainer needs more info)

I'm confused. The patch in #2 is replacing an entity type identifier (bundles), not an entity identifier.

Entity type identifiers (bundles) are static since they are provided by whatever created the entities.

I don't see the purpose behind complicating this with a UUID, which are intended for actual user created content/entities.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented
Status: Postponed (maintainer needs more info) » Needs review

Well, the bundle is the next case right below the one I patched.

else if ($this->type === BackgroundImageInterface::TYPE_ENTITY && ($entity_type = $this->getSubformValue('entity_type'))) {

This is true when I want to define a background image for an entity, e.g. for a node, and the value in $entity_type_id is the node id. But later, when getEntityBackgroundImage(EntityInterface $entity) is called, it searches for a background image by using the entity uuid - and simply doesn't find anything.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs review » Needs work

Ahhhhh, I missed when you said "(without inline form)".

So I've been going over the code and I believe you're right.

It's unfortunate that the form name is entity_type and the variable name is $entity_type_id. I think this was some legacy code prior to my initial commit and prior to when I switch to UUIDs.

Looking at the form above, it is definitely an autocomplete for entities, so yes the patch in #2 makes sense.

This actually may explain why some people have randomly said that "it doesn't work".

I think what was tripping me up is that I was thinking of the following which detects either: http://cgit.drupalcode.org/background_image/tree/src/Entity/BackgroundIm...

That being said, there should probably be an update hook so any background images saved with the entity ID can be resaved using the proper UUID and then that method can just use UUIDs; so setting to CNW.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs work » Fixed
StatusFileSize
new 3032110-7.patch5.87 KB

@therobyouknow confirmed/tested this patch in #3125096-7: Background image does not display for Type: "Entity".

Here is a little more comprehensive patch that renames the variables (to avoid further confusion), removes an outdated if/else block and adds an update hook to convert any existing entity targets saved using a normal ID instead of a UUID.

  • markcarver committed fba1127 on 8.x-1.x 
    Issue #3032110 by markcarver, jurgenhaas: Mixed usage of entity uuid and...
therobyouknow’s picture

therobyouknow commented

Thank you Mark for the new patch! (in comment 6, above, https://www.drupal.org/project/background_image/issues/3032110#comment-1... - https://www.drupal.org/files/issues/2020-04-05/3032110-7.patch )

I will try to apply this in composer, expecting it to apply ok. Also - I note your latest commit of the same fix - into the dev version of this module. Thank you again.

I had manually, by hand, applied the previous version of this fix in earlier patch https://www.drupal.org/files/issues/2019-02-11/3032110-2.background_imag... and found it to solve the issue.

Thank you again for your time and work in our discussion via issue comments about this issue!

Status: Fixed » Closed (fixed)

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