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

Comments

marcel66 created an issue. See original summary.

marcel66’s picture

Status: Active » Needs review
StatusFileSize
new3.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

Status: Needs work » Needs review
StatusFileSize
new860 bytes
new3.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

Status: Needs work » Needs review
StatusFileSize
new2.95 KB
new589 bytes

Fixed IteratorText.php

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Nice work!

alexpott’s picture

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

Issue tags: -DCTransylvania2018 +DCTransylvania

Cleaning up tags.

marcel66’s picture

Status: Needs work » Needs review
StatusFileSize
new1.32 KB
new1.63 KB

Added back deprecation message for Iterator which should remain.

marcel66’s picture

StatusFileSize
new3.29 KB
new2.53 KB

Thanks for support.

amateescu’s picture

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

Status: Needs work » Needs review
StatusFileSize
new3.16 KB
new1.04 KB

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

amateescu’s picture

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

Status: Needs work » Needs review
StatusFileSize
new3.1 KB
new1.18 KB

Thank you

marcel66’s picture

Status: Needs review » Needs work

I have seen a mistake

marcel66’s picture

StatusFileSize
new570 bytes
new3.37 KB

Thanks.

marcel66’s picture

Status: Needs work » Needs review
amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Looks great now :)

alexpott’s picture

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.