MigrateDestinationOGMembership should extend MigrateDestinationEntity

I just had a brief play with it before and noticed its not a matter of a simple
$this->complete($entity, $row); The class needs to extend MigrateDestinationEntity and not MigrateDestination. I will try write up and add a patch tomorrow morning.

So i have attached a patch which now makes MigrateDestinationOGMembership extend MigrateDestinationEntity rather than MigrateDestination. Note: MigrateDestinationEntity extends MigrateDestination.

I noticed that some other destinations in Migrate such as menu and menu_links etc extend MigrateDestination directly and have their own implementation of prepare() and complete() very similar to MigrateDestinationEntity. Rather than re-using code i thought it would make more sense just to extend MigrateDestinationEntity and in this way it will use this classes prepare() and complete().

If anyone thinks it would be better to leave the class extend MigrateDestination and include prepare() and complete() functions like menu.inc (MigrateDestinationMenu) and menu_links.inc (MigrateDestinationMenuLinks) please let me know or write a patch. Other wise i think this way is fine.

I was not entirely sure on the bundle to be honest and as I believe the parent class MigrateDestinationEntity requires a bundle to be specified in the constructor. I also thought that if anyone else was instantiating the class in their code with just new MigrateDestinationOGMembership() then atleast it will still hopefully continue to work. I must admit though I'm not entirely sure on what the default bundle is or if it should have one. Perhaps someone such as yourself would have a better understanding of the working internals of og and be able to help out.

Sorry about the trailing space.

Attached is the MigrateDestinationEntity class. As you can see it already has the complete() and prepare() functions aswell as other helper functions implemented. You can also see here how i was mentioning the constructor requires a bundle.

The other way to do this and avoid specifying any bundle is to leave MigrateDestinationOGMembership extend MigrateDestination, (and like some other destination plugins such as menu and menu_links in the Migrate module) is to just implement the code directly into the MigrateDestinationOGMembership class ourselves. This code was taken from menu.inc and menu_links.inc destinations

  /**
   * Implementation of MigrateDestination::prepare().
   */
  public function prepare($menu, stdClass $row) {
    // We do nothing here but allow child classes to act.
    $migration = Migration::currentMigration();
    $menu->migrate = array(
      'machineName' => $migration->getMachineName(),
    );

    // Call any general handlers.
    migrate_handler_invoke_all('menu', 'prepare', $menu, $row);
    // Then call any prepare handler for this specific Migration.
    if (method_exists($migration, 'prepare')) {
      $migration->prepare($menu, $row);
    }
  }

  public function complete($menu, stdClass $row) {
    // We do nothing here but allow child classes to act.
    $migration = Migration::currentMigration();
    $menu->migrate = array(
      'machineName' => $migration->getMachineName(),
    );
    // Call any general handlers.
    migrate_handler_invoke_all('menu', 'complete', $menu, $row);
    // Then call any complete handler for this specific Migration.
    if (method_exists($migration, 'complete')) {
      $migration->complete($menu, $row);
    }
  }

As you can see the code is very similar to that of MigrateDestinationEntity and such we could avoid code re-use by inheriting from that class. Im happy either way. If you think its best we dont have a bundle then ill leave MigrateDestinationOGMembership extend MigrateDestination and write a patch including these two functions. Otherwise we could use the patch i uploaded above.

Please let me know your thoughts.

This is a patch that implements the second way I was mentioning and still lets MigrateDestinationOGMembership extend MigrateDestination without having to specify a bundle. The class now has its own prepare() and complete() functions which prepare handlers before calling any user implemented prepare() and complete() functions. I base this on other destination migrations implementations see attached.