I am not sure if this is a support or a feature request.

I am working on a phpbb2drupal migration at the moment. The install if phpbb2drupal I am using has some custom fields.

I declared a class like this.

  class my_PhpbbUserMigration extends PhpbbUserMigration {

which worked as expected, but when I tried to alter the query I was unable to. I would have thought that: 

  public function __construct($settings) {
    parent::__construct($settings);
    $this->source->addField('pfd','my_custom_field');

Would work, but source is a MigrateSourceSQL object not a normal Drupal query object and won't allow it to be extended.

I ended up having to copy and paste the query from PhpbbUserMigration and then set that as the source. This works, but it dosn't quite seem in keeping with the flexibility of the migrate framework.

Have I missed a trick in being able to extend migrate queries? Or is this something that is worth considering for a feature?

CommentFileSizeAuthor
#3 query_reference-1799964-3.patch680 bytesmikeryan

Comments

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Component: Miscellaneous » Code
Category: support » feature

Well, PHP doesn't support multiple inheritance, so the source can't be both a MigrateSource and a query. Today, this can be dealt with as follows (which is how migrate_d2d does it):

class PhpbbUserMigration extends Migration {
  public function __construct() {
    ...
    $this->source = new MigrateSourceSQL($this->query());
    ...
  }
  protected function query() {
    return db_select('foo', 'f')...
  }
}

class my_PhpbbUserMigration extends PhpbbUserMigration {
  protected function query() {
    $query = parent::query();
    $query->addField('pfd','my_custom_field');
    return $query;
  }
}

We could support an alternative by having a query getter in MigrateSourceSQL returning a reference to the query and doing this in the constructor:

  $this->source->query()->addField('pfd','my_custom_field');
JeremyFrench’s picture

JeremyFrench commented

I Like your technique of having the query as a method for the migrate object. That will work in my case as I can change the source of the parent migration.

However this may not always be possible (if people don't want to hack contrib modules)

The Query object is protected in MigrateSource, as in the constructor it is copied and altered a bit. So modifying it after construction is risky. So I would propose that we either:

  • Have a getQuery and setQuery method on MigrateSourceSQL so that when a query is altered it is set and the migrateSource object changes its' other internal variables. Or
  • Have a getQueryCopy method on MigrateSourceSQL which provides a copy of the orignal query, it will allow it to be altered and a new MigrateSourceSQL created, but should be obvious that it is not allowing you to change an already created MigrateSourceSQL object.

I'm leaning towards the second option, I'm happy to code if you think it is worthwhile?

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Active » Needs review
StatusFileSize
new query_reference-1799964-3.patch680 bytes

Give this a shot, per #1:

  $this->source->query()->addField('pfd','my_custom_field');
mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Title: Allow multiple levels of inheritance for migrations » Allow manipulation of source queries
Status: Needs review » Fixed

Well, seems like a simple and useful extension, I've gone ahead and committed it.

Status: Fixed » Closed (fixed)

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

neruda001’s picture

neruda001 commented
Version: 7.x-2.4 » 7.x-2.5
Category: feature » bug
Status: Closed (fixed) » Active

hi,

it seems to be a bug in version 2.5:
in the MigrateSourceSQL object, which resides in plugins/sources/sql.inc, on line 26 we have a method &query() which calls a non-existing method originalQuery().
To get the originalQuery value, not the reference, I implemented a new method in the same object: getQueryCopy() which returns the clone of $originalQuery protected variable.
Is this the best way to do it? Why there's not the implementation of originalQuery() method?

thanks

Pablo

dropfen’s picture

dropfen commented
Status: Active » Fixed

As I can see, this was already fixed here https://drupal.org/node/1896042

So, you can use a newer version of migrate, 7.x-2.6-rc1 instead of the 2.5

Status: Fixed » Closed (fixed)

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