- Issue created by @benjifisher
- πΊπΈUnited States benjifisher Boston area
There are some differences between this issue and π Move content_entity source plugin to migrate module Active .
The plugin ID
d8_configis outdated. As part of this issue, do we want the new plugin ID to beconfigordrupal_config? If so, do we still want to remove the plugin annotation (or attribute after π Convert MigrateSource plugin discovery to attributes Active ) from the existing class?This class extends
DrupalSqlBase, and we probably do not want to keep that class. Should we migrate some parts of that class (properties, methods) into the new plugin class? Should we create traits?My initial thought is that we want to leave as much as possible behind. That means that the new plugin is more of a re-implementation than a straight move, which also argues in favor of choosing a new plugin ID. For example,
DrupalSqlBaseimplementsDrupal\Component\Plugin\DependentPluginInterfaceandusesDrupal\Core\Entity\DependencyTrait:public function calculateDependencies() { // Generic handling for Drupal source plugin constants. if (isset($this->configuration['constants']['entity_type'])) { $this->addDependency('module', $this->entityTypeManager->getDefinition($this->configuration['constants']['entity_type'])->getProvider()); } if (isset($this->configuration['constants']['module'])) { $this->addDependency('module', $this->configuration['constants']['module']); } return $this->dependencies; }I think the new plugin does not need that.
- πΊπΈUnited States benjifisher Boston area
More on the same point: the constructor for
DrupalSqlBasesets the$entityTypeManagerproperty, which is not used in theConfigclass.The
getSystemData()andvariableGet()methods will not work with a D8+ source because they need thesystemandvariabledatabase tables. Most of the other methods in the class depend ongetSystemData().Also,
DrupalSqlBaseextendsSqlBasein themigratemodule. The base class definesgetDatabase(), which is the basis for connecting to a secondary database. So the new class can extendSqlBasedirectly and still generate config items from the current database or another one. - πΊπΈUnited States benjifisher Boston area
I suggested a couple of different plugin IDs, but I think I prefer
config_entityas a counterpart to the existingcontent_entitysource plugin. - πΊπΈUnited States nicxvan
I vote for config_entity i think the parallel makes sense.
- πΊπΈUnited States benjifisher Boston area
I am going to start work on this issue.
- πΊπΈUnited States benjifisher Boston area
I think that https://git.drupalcode.org/project/drupal/-/merge_requests/11271 does most of the work: it creates the new source plugin and deprecates the old one.
I created a draft change record and used it in the deprecation notice.
I still have to update the tests: mark the old ones as
@group legacyand copy them to themigratemodule. - πΊπΈUnited States benjifisher Boston area
I think the MR is ready for review.
I am updating the issue summary with some of the points we discussed in the comments.
I had to update one existing test, which checks all the source plugins provided by the
migratemodule. I think the updates are straightforward, but someone should review them.The hardest part is that
phpcschecks for the first@seecomment after a@deprecatedcomment and complains if it references a class instead of a URL (for the change record). At first, when I added the@deprecatedcomment, I did not know this and I did not add an@seecomment for the change record. - πΊπΈUnited States smustgrave
Giving it a shot.
Looking at the deprecation and seems correct format, and move seems to be pretty good 1 to 1, minus name change.
CR is great, well detailed and exactly what I need to do to update my custom code.
Going to go on a limb and say this is good.
- π¬π§United Kingdom catch
This looks good to me too. Committed/pushed to 11.x, thanks!
- Status changed to Fixed
6 months ago 11:39pm 27 April 2025 Automatically closed - issue fixed for 2 weeks with no activity.