Right now, if you try to list the migrations for a group, you get all migrations listed, not just those for the desired group.

CommentFileSizeAuthor
#6 interdiff-2719775-4-6.txt1.26 KBbalazswmann
#6 filter-migrations-2719775-6.patch1.54 KBbalazswmann
#4 interdiff.txt412 bytesmallezie
#4 filter-migrations-2719775-4.patch1.08 KBmallezie
#2 filter-migrations-2719775-1.patch1.07 KBmallezie

Comments

mikeryan created an issue. See original summary.

mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented
Status: Active » Needs review
StatusFileSize
new filter-migrations-2719775-1.patch1.07 KB

Think i found it, this is also related to #2719777: Migrations not sorted by dependency, but split them up. Could be they conflict when one of them is commited.

draenen’s picture

draenen commented

There's an extra empty line above the new method but otherwise the patch applies and works for me.

mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented
StatusFileSize
new filter-migrations-2719775-4.patch1.08 KB
new interdiff.txt412 bytes

Removed the extra whiteline

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs review » Needs work 
+++ b/src/Controller/MigrationListBuilder.php
@@ -67,6 +67,31 @@ class MigrationListBuilder extends ConfigEntityListBuilder implements EntityHand
+    if ($migration_group == 'default') {
+      $query->notExists('migration_group');

Looks good, and works when I test in semi-normal normal circumstances - but it looks to me like this will fail to list migrations which explicitly reference the default group?

Thanks!

balazswmann’s picture

balazswmann commented
StatusFileSize
new filter-migrations-2719775-6.patch1.54 KB
new interdiff-2719775-4-6.txt1.26 KB

I modified the patch according to #5. I think the default group should exist by default after the module installation. Otherwise the following problems are possible:

#1 - If a new migration is added through the UI which group is "- None -" and the group with "default" machine name is not exist yet, then none of the groups will list the newly added migration because it will be part of the non-existent "Default" group.

#2 - Users can create a group with the "default" machine name which is not a problem in itself, but the purpose (and the description) of this group can be arbitrary which wouldn't be good. This also implies that the default group shouldn't be editable or deletable.

balazswmann’s picture

balazswmann commented
Status: Needs work » Needs review

  • mikeryan committed 0a4bb05 on 8.x-2.x authored by balazswmann 
    Issue #2719775 by mallezie, balazswmann: Migration listing not filtered...
mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs review » Fixed 
+++ b/src/Controller/MigrationListBuilder.php
@@ -67,6 +68,33 @@ class MigrationListBuilder extends ConfigEntityListBuilder implements EntityHand
+    else {
+      $query->notExists('migration_group');
+    }

Regarding the default group, every migration lacking a migration_group has 'default' added in migrate_plus_migration_plugins_alter(), and the default group is created at that time if necessary. Thus, the list should never see a migration without a group. But, it doesn't hurt to handle the else case, so leaving this in.

Committed, thanks!

Status: Fixed » Closed (fixed)

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