With the support of status key in #2987787: Status of migration config file is not respected, the list of migrations page throws a 500 error if there is an inactive migration.

I'll provide a patch to fix that

CommentFileSizeAuthor
#11 migrate_tools-support_migration_status-3003080-11.patch2.23 KBsylvainm
#5 migrate_tools-support_migration_status-3003080-5.patch2.23 KBsylvainm
#5 migrate_tools-support_migration_status-3003080-5_test_only.patch1.58 KBsylvainm
#2 migrate_tools-support_migration_status-3003080-2.patch667 bytessylvainm

Comments

SylvainM created an issue. See original summary.

sylvainm’s picture

sylvainm commented
Title: Support status o fmigration config file (#2987787) » Support status of migration config file (#2987787)
Assigned: sylvainm » Unassigned
Status: Active » Needs review
StatusFileSize
new migrate_tools-support_migration_status-3003080-2.patch667 bytes

Here is a quick patch.

Without patched migrate_plus, we can't add a failing test, so I wonder if a test is needed, or if it should wait for Status of migration accepted.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Category: Feature request » Bug report
Status: Needs review » Needs work
Issue tags: +Needs tests

Can we now add a test? Now we have upstream committed?

sylvainm’s picture

sylvainm commented

I will try to work on that on Friday

sylvainm’s picture

sylvainm commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new migrate_tools-support_migration_status-3003080-5_test_only.patch1.58 KB
new migrate_tools-support_migration_status-3003080-5.patch2.23 KB

Test added

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work

The test only patch didn't fail? Could we assert the migration that is set to disabled doesn't show-up on the page? And maybe re-enable it and assert it then does?

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented
Issue summary: View changes
sylvainm’s picture

sylvainm commented

Well, this is strange: locally the test fails

$ vendor/bin/phpunit -c www/core www/modules/contrib/migrate_tools/tests/src/Functional/MigrationListPageTest.php 
PHPUnit 6.5.13 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\migrate_tools\Functional\MigrationListPageTest
.E                                                                  2 / 2 (100%)

Time: 41.61 seconds, Memory: 6.00MB

There was 1 error:

1) Drupal\Tests\migrate_tools\Functional\MigrationListPageTest::testMigrationListPageDisabledMigration
Exception: Error: Call to a member function get() on boolean
Drupal\migrate_tools\Controller\MigrationListBuilder->buildRow()() (Line: 145)

ERRORS!
Tests: 2, Assertions: 15, Errors: 1.
sylvainm’s picture

sylvainm commented
Assigned: Unassigned » sylvainm

ha, i get the same result in a fresh install of drupal

sylvainm’s picture

sylvainm commented
Assigned: sylvainm » Unassigned

Ok, I finally understood why: the migrate_plus version used by the dispatcher is the stable and in this version, there is not yet the commit https://cgit.drupalcode.org/migrate_plus/commit/?id=6cfbb3e91147dd137ad8...

Once a new release of migrate_plus will be released, we will be able to test again, and this time, the test_only will fail (I tested it locally)

sylvainm’s picture

sylvainm commented
Status: Needs work » Needs review
StatusFileSize
new migrate_tools-support_migration_status-3003080-11.patch2.23 KB

Test only now fails.

Here is a new patch which should apply

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Version: 8.x-4.x-dev » 6.1.x-dev
Status: Needs review » Needs work

This needs a rebase and to be rolled into a PR.

sylvainm’s picture

sylvainm commented

IMHO, it seems that the patch is more or less integrated in current version: https://git.drupalcode.org/project/migrate_tools/-/blob/6.1.x/src/Contro... and that we can close this issue, don't you think?

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs work » Closed (outdated)

If that isn't the case, we can re-open with steps to reproduce the issue on the current HEAD. Thanks for the confirmation this doesn't seem to be an issue.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.