Mi idea is to implement a destination handler for every entity that allow to specify a entityqueue machine name to make that entity part of the entityqueue.

Comments

mercepedraza’s picture

Status: Active » Needs review
StatusFileSize
new2.89 KB

This is an approach that adds a new migrate destination field to any entity where to specify the entityqueue/s where the entity has to be added.

mercepedraza’s picture

StatusFileSize
new2.91 KB
new863 bytes

This patch corrects the previous way to get the values of the entityqueues names from $entity instead of from $row.
In this way it guarantees that when multiples values are received they arrive in an array without any separator.

mercepedraza’s picture

StatusFileSize
new2.91 KB

The previous patch contains only the interdiff, this one is the right one.

rodrigoaguilera’s picture

Status: Needs review » Reviewed & tested by the community

I can confirm this patch is working as expected. Even with no rollback functionality I think it can be commited.

amateescu’s picture

Status: Reviewed & tested by the community » Needs work

The patch looks ok to me as well. Here's a few things that could be improved:

  1. +++ b/entityqueue.migrate.inc
    @@ -0,0 +1,87 @@
    + * DestinationHandler.
    

    This description could be expanded with a few more words :)

  2. +++ b/entityqueue.migrate.inc
    @@ -0,0 +1,87 @@
    +   * Overrides fields().
    ...
    +   * Overrides complete().
    +   *
    +   * @param object $entity
    +   *   The Drupal entity.
    +   * @param object $row
    +   *   The row being migrated.
    

    These can be simply {@inheritdoc}.

  3. +++ b/entityqueue.migrate.inc
    @@ -0,0 +1,87 @@
    +      $migrate_entityqueue = $entity->entityqueue;
    

    We can change this to:

    $migrate_entityqueue = (array) $entity->entityqueue;
    

    and drop the conversion below.

  4. +++ b/entityqueue.migrate.inc
    @@ -0,0 +1,87 @@
    +
    ...
    +
    

    Unneeded empty lines in the complete() method.

  5. +++ b/entityqueue.migrate.inc
    @@ -0,0 +1,87 @@
    +    $migration = Migration::currentMigration();
    +    $destination = $migration->getDestination();
    +    $entity_type = $destination->getEntityType();
    

    It looks like we're not using $migration or $destination below, so we can inline everything to:

    $entity_type = Migration::currentMigration()->getDestination()->getEntityType();
    
rodrigoaguilera’s picture

Status: Needs work » Needs review
StatusFileSize
new2.72 KB

Addressed all the points.

amateescu’s picture

Thanks, looks much better :)

+++ b/entityqueue.migrate.inc
@@ -0,0 +1,77 @@
+ * Doesn't support rollback, when you rollback the entities with this field the
+ * number of items in the entityqueue is not updated.

I wonder why this is the case. We can not support rollback at all or it's just not part of this implementation because you didn't need it?

rodrigoaguilera’s picture

StatusFileSize
new2.23 KB

AFAIK We cannot support rollback. I removed the wording about support, now is just a phrase telling what happens when you rollback.

Removed the files[] declaration because migrate already detects the file. See the first lines of
http://cgit.drupalcode.org/migrate/tree/migrate_example/migrate_example....

amateescu’s picture

Removed the files[] declaration because migrate already detects the file.

Are you sure it detects the file even when the hook_migrate_api() implementation is in that file? I kind of doubt that.. :) It would make more sense to move the hook at the bottom of entityqueue.module.

rodrigoaguilera’s picture

Yes, I'm 100% sure. If you look at the migrate_example module there's no other reference to the *.migrate.inc file in the .info file.

Yo can also test it very quickly in https://simplytest.me/

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Oh, I overlooked the fact that our filename ends in .migrate.inc, I thought it was named after the class name like we do in Drupal 8. So, yep, I think we're good here.

Setting back to RTBC to give some time to @jojonaloha if he wants to review this as well.

jojonaloha’s picture

Status: Reviewed & tested by the community » Needs work

I haven't tested this, but the only thing I see is I would change $key = 'eq_' . $entity_type; to $key = _entityqueue_get_target_field_name($entity_type);

amateescu’s picture

Status: Needs work » Closed (outdated)

Closing issues for the 7.x version, which is not supported anymore.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.