Problem/Motivation

We should remove usages of Drupal\migrate\Plugin\migrate\process\Iterator and replace with Drupal\migrate\Plugin\migrate\process\SubProcess.

Proposed resolution

Replace all the usages of Drupal\migrate\Plugin\migrate\process\Iterator with Drupal\migrate\Plugin\migrate\process\SubProcess.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#18 2969757-18.patch3.37 KBmarcel66
#18 interdiff-18.txt570 bytesmarcel66
#16 interdiff-16.txt1.18 KBmarcel66
#16 2969757-16.patch3.1 KBmarcel66
#14 interdiff-14.txt1.04 KBmarcel66
#14 2969757-14.patch3.16 KBmarcel66
#11 interdiff-11.txt2.53 KBmarcel66
#11 2969757-11.patch3.29 KBmarcel66
#10 interdiff-9.txt1.63 KBmarcel66
#10 2969757-9.patch1.32 KBmarcel66
#6 interdiff-6.txt589 bytesmarcel66
#6 2969757-6.patch2.95 KBmarcel66
#4 2969757-4.patch3.23 KBmarcel66
#4 interdiff-4.txt860 bytesmarcel66
#2 2969757-2.patch3.23 KBmarcel66
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marcel66 created an issue. See original summary.

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
Status: Active » Needs review
FileSize
2969757-2.patch3.23 KB

A simple patch replacing Drupal\migrate\Plugin\migrate\process\Iterator with Drupal\migrate\Plugin\migrate\process\SubProcess.

Status: Needs review » Needs work

The last submitted patch, 2: 2969757-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
Status: Needs work » Needs review
FileSize
interdiff-4.txt860 bytes
2969757-4.patch3.23 KB

Unchanges to IteratorTest.php.

Status: Needs review » Needs work

The last submitted patch, 4: 2969757-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
Status: Needs work » Needs review
FileSize
2969757-6.patch2.95 KB
interdiff-6.txt589 bytes

Fixed IteratorText.php

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs review » Reviewed & tested by the community

Nice work!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/migrate/tests/src/Unit/process/IteratorTest.php
@@ -18,7 +18,7 @@ class IteratorTest extends MigrateTestCase {
-   * @var \Drupal\migrate\Plugin\migrate\process\TestIterator
+   * @var \Drupal\migrate\Plugin\migrate\process\SubProcess

This is actually a Drupal\migrate\Plugin\migrate\process\Iterator - it is currently incorrect and the patch change is still not right. This is a legacy test - we should be adding an @expectedDeprecation annotation to the test and fixing this to point to the deprecated class.

The other changes look good.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Issue tags: -DCTransylvania2018 +DCTransylvania

Cleaning up tags.

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
Status: Needs work » Needs review
FileSize
2969757-9.patch1.32 KB
interdiff-9.txt1.63 KB

Added back deprecation message for Iterator which should remain.

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
FileSize
2969757-11.patch3.29 KB
interdiff-11.txt2.53 KB

Thanks for support.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs review » Reviewed & tested by the community

Looks great now :)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2969757-11.patch, failed testing. View results

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
Status: Needs work » Needs review
FileSize
2969757-14.patch3.16 KB
interdiff-14.txt1.04 KB

Thank you all to allow me make mistakes and correct them. I'm still learning.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs review » Needs work 
+++ b/core/modules/migrate/tests/src/Unit/process/IteratorTest.php
@@ -21,6 +19,7 @@ class IteratorTest extends MigrateTestCase {
+   * @expectedDeprecation The Drupal\migrate\Plugin\migrate\process\Iterator is deprecated in Drupal 8.4.x and will be removed before Drupal 9.0.0. Instead, use Drupal\migrate\Plugin\migrate\process\SubProcess

The @expectedDeprecation annotation needs to be placed in the documentation block of a test method. In this case, it needs to be in the testIterator() method of this test class.

You can find an example of how this works in the \Drupal\Tests\link\Unit\Plugin\migrate\cckfield\LinkCckTest test class, and the testProcessCckFieldValues() test method.

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
Status: Needs work » Needs review
FileSize
2969757-16.patch3.1 KB
interdiff-16.txt1.18 KB

Thank you

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
Status: Needs review » Needs work

I have seen a mistake

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
FileSize
interdiff-18.txt570 bytes
2969757-18.patch3.37 KB

Thanks.

marcel66’s picture

marcel66 CreditAttribution: marcel66 as a volunteer commented
Status: Needs work » Needs review
amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs review » Reviewed & tested by the community

Looks great now :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 282eb36c14 to 8.6.x and 5781652137 to 8.5.x. Thanks!

Backported to 8.5.x since all the changes are either docs or tests.

  • alexpott committed 282eb36 on 8.6.x 
    Issue #2969757 by marcel66, amateescu, alexpott: Fix "The Drupal\migrate...

  • alexpott committed 5781652 on 8.5.x 
    Issue #2969757 by marcel66, amateescu, alexpott: Fix "The Drupal\migrate...

Status: Fixed » Closed (fixed)

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