Problem/Motivation

Multiple use cases will require to lookup for an entity with a default lookup value, for instance, importing content from other systems not always have an author to map in the importing system.

When entity_lookup don't found the entity NULL is given and no entity is set.

Actually, the best option is to use default_value plugin, but this requires them to fix the id in the configuration, but as long as the 'default entity' is content it can have different id in different site installations and also makes the configuration less readable.

Proposed resolution

Implement a default entity lookup plugin where instead of the fallback to fixed value fallbacks to a entity field value to lookup for the entity.

Comments

quiron created an issue. See original summary.

quiron’s picture

This patch is working for me.

quiron’s picture

Added checking of the user already exists on the system.

heddn’s picture

Status: Active » Needs work
Issue tags: +Needs tests
    Some thoughts. First great work here. Generally useful plugin. I wonder if we could make it even more useful? Also, we should add some tests. See other examples in this module. Since you are interacting with entities, it will probably need to be a kernel test.

    Setting to NW for tests and the suggestion to make this more generally useful.

  1. +++ b/src/Plugin/migrate/process/DefaultUser.php
    @@ -0,0 +1,50 @@
    +class DefaultUser extends ProcessPluginBase {
    

    Can we extend DefaultValue and pass things off to it after loading the uid?

    Actually, what if we made this a 'default_value_plus' plugin? And provided an entity type, lookup_value, lookup_key, etc so we can then use an entity query to look for a default value based on any query-able condition.

    Example:
    entity_type: user
    lookup_key: username
    lookup_value: "john smith"
    default_value_key: uid

    The above names are just examples. If you have better suggestions on naming, feel free to use them instead.

quiron’s picture

Status: Needs work » Needs review
StatusFileSize
new1.4 KB

After work on it IMHO the better approach to get the DefaultValuePlus is directly extend EntityLookup and keep it's behavior and configuration keys. This way the plugin implementation keeps simple and full-featured.

@heddn agree with this approach? If yes I will start with tests. There is an example of similar tests for it I can take as reference?

Status: Needs review » Needs work

The last submitted patch, 5: migrate_plus-2944437-default-user-name-5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quiron’s picture

This is a small syntax fix.

@heddn if you can point me how to work on the test I would start with it.

quiron’s picture

@heddn Added new version of the patch with the default_value_plus plugin + a proper Kernel test for it. It checks both the case a value is provided and no value is provided and the default is used. Test case with users, like the Kernel test for EntityLookup.

quiron’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 8: migrate_plus-2944437-default-user-name-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quiron’s picture

Status: Needs work » Needs review
StatusFileSize
new4.09 KB

Fixed the codesniffer issues.

heddn’s picture

Status: Needs review » Needs work
Issue tags: -Needs tests
  1. +++ b/src/Plugin/migrate/process/DefaultValuePlus.php
    @@ -0,0 +1,46 @@
    + *   id = "default_value_plus",
    ...
    +class DefaultValuePlus extends EntityLookup {
    

    Naming is hard. Would default_entity_value / DefaultEntityValue make sense more to convey a more direct meaning?

quiron’s picture

Status: Needs work » Needs review
StatusFileSize
new4.1 KB

I was also wondering about the naming. I guess that default_entity_reference is also a good candidate, but there is no similar name in the actual migration universe. So ok to go with default_entity_value. I kept the configuration option name to default_value as long IMHO makes sense to not add the 'entity' keyword because it stands to a field value, not the whole entity.

quiron’s picture

Title: Provide default_value plugin for user importing based on user_name » Provide an entity lookup plugin with support for default lookup value.
Issue summary: View changes

Updated title and issue description

  • heddn committed 0f1a598 on 8.x-5.x authored by quiron
    Issue #2944437 by quiron, heddn: Provide an entity lookup plugin with...
heddn’s picture

Status: Needs review » Fixed

Thanks for your contributions!

Status: Fixed » Closed (fixed)

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