Postponed on #3039240: Create a way to declare a plugin as deprecated.
#2500535: Upgrade path for Tracker 7.x introduced a migration for the tracker module. It should be a very simple migration since it's one of the very few that's pretty much just copying a DB table.
While this migration is not needed running it does not cause problems on the destination. See #5 and #10.
Leaving as postponed because it can be done when this is moved to the contrib project.
Fix all this:
+++ b/core/modules/tracker/migration_templates/d7_tracker_node.yml
@@ -0,0 +1,15 @@
+ nid: nid
No migration lookup, which means that changed node IDs will fail to have their tracker data correctly migrated.
+++ b/core/modules/tracker/migration_templates/d7_tracker_node.yml
@@ -0,0 +1,15 @@
+migration_dependencies:
+ required:
+ - d7_user
This is only depending on d7_user but it also needs d7_node.
+++ b/core/modules/tracker/migration_templates/d7_tracker_user.yml
@@ -0,0 +1,16 @@
+process:
+ nid: nid
+ uid: uid
+ published: published
+ changed: changed
+destination:
+ plugin: entity:user
+migration_dependencies:
+ required:
+ - d7_user
… same here.
+++ b/core/modules/tracker/migration_templates/d7_tracker_node.yml
@@ -0,0 +1,15 @@
+destination:
+ plugin: entity:node
This sends the tracker source data to … not the tracker_node table in D8|9 … but to the node entity.
+++ b/core/modules/tracker/migration_templates/d7_tracker_user.yml
@@ -0,0 +1,16 @@
+destination:
+ plugin: entity:user
This sends the tracker source data to … not the tracker_user table in D8|9 … but to the user entity.
+++ b/core/modules/tracker/src/Tests/Migrate/d7/MigrateTrackerNodeTest.php
@@ -0,0 +1,74 @@
+ public function testMigrateTrackerNode() {
+ $connection = Database::getConnection('default', 'migrate');
This is a test.
But it tests not the migration.
It tests the _source DB_ 😳
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 3204511-2.patch | 786 bytes | wim leers |
Comments
Comment #2
wim leersLess offensive sounding title, this is what I actually meant — sorry! 😞
Comment #3
wim leersThis fixes the first problem: now these migration plugins can only run when it actually makes sense to run them.
But much more importantly, we need to fix the tests and fix the migration definitions.
Comment #5
quietone commentedThe title makes me laugh, is the tracker migration correct for every site not trying to use it? Still, as we know, someone worked really hard on that migration. Changing title to focus on fixing.
From the IS.
1. This is not wrong. It is an established practice for core migrations to use
nid: nidbecause core preserves IDs. See #2748609: [meta] Preserving auto-increment IDs on migration is fragile for some background.2, 3. Agree
4, 5. Hmm.
6. Yes, the test is wrong. Removing the tag because there are tests, they just need to be fixed.
As far as I can tell, when tracker is enable and the node migration is run the tracker tables are populated. There doesn't seem to be a need to run these migrations at all. Are d7_tracker_node.yml and d7_tracker_user.yml needed at all?
For me, this does not fit the criteria for Major.
Comment #7
catchtracker_cron() can rebuild this table from scratch, and entries are also added when nodes are inserted or updated, so this migration just isn't needed at all.
Comment #8
quietone commented@catch, thank you for the confirmation.
That means that the tracker migration and associated tests can be removed. Except that there isn't a way to deprecate a migration yet, #3039240: Create a way to declare a plugin as deprecated.
Also changing to a task.
Comment #9
quietone commentedComment #10
catchI don't think we need to wait for #3039240: Create a way to declare a plugin as deprecated - that's more for plugins that are exposed in UIs (fields, formatters, widgets etc.)
Can we just deprecate the classes and delete the migration template?
As it stands, it's impossible for anyone to actually use this, so we could probably just delete it too?
Just realised this can be handled in the contrib module, as long as we leave core migration fixtures as-is.
Comment #11
quietone commentedComment #12
quietone commentedNot sure about that. The IS for #3039240: Create a way to declare a plugin as deprecated specifically addressed migration plugins and the reasoning for why they need deprecation.
A reason to not delete it is if someone is modifying them in hook_migration_plugins_alter. They would expect the key to exist in the array of plugins IDs.
I agree this can be dealt with later either in core or in the contrib version of tracker. Changing the component to tracker so this will get moved and changing back to postponed. It is listed in the Meta for tracking the removal process as well. What does need to happen is an issue to deal with the functional tests.
Comment #13
catchEven if they're deprecated, we won't have a way to indicate to an alter hook that they're about to be removed. This is mostly covered under https://www.drupal.org/about/core/policies/core-change-policies/drupal-d...
Comment #16
quietone commentedComment #17
quietone commentedThis extension is deprecated and scheduled for removal in Drupal 11.
This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.
It will be moved to the contributed extension once the Drupal 11 branch is open.
Comment #19
andypostThe module already deprecated, contrib will polish it on adoption