- 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_config
is outdated. As part of this issue, do we want the new plugin ID to beconfig
ordrupal_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,
DrupalSqlBase
implementsDrupal\Component\Plugin\DependentPluginInterface
anduse
sDrupal\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
DrupalSqlBase
sets the$entityTypeManager
property, which is not used in theConfig
class.The
getSystemData()
andvariableGet()
methods will not work with a D8+ source because they need thesystem
andvariable
database tables. Most of the other methods in the class depend ongetSystemData()
.Also,
DrupalSqlBase
extendsSqlBase
in themigrate
module. The base class definesgetDatabase()
, which is the basis for connecting to a secondary database. So the new class can extendSqlBase
directly 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_entity
as a counterpart to the existingcontent_entity
source 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 legacy
and copy them to themigrate
module. - πΊπΈ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
migrate
module. I think the updates are straightforward, but someone should review them.The hardest part is that
phpcs
checks for the first@see
comment after a@deprecated
comment and complains if it references a class instead of a URL (for the change record). At first, when I added the@deprecated
comment, I did not know this and I did not add an@see
comment for the change record.