Problem/Motivation

\Drupal\ban\Plugin\migrate\destination\BlockedIp migrate destination plugin's import method violates \Drupal\migrate\Plugin\MigrateDestinationInterface::import, because it doesn't return anything (so its return value is NULL. This triggers a migration message in MigrateExecutable.

Steps to reproduce

Execute d7_blocked_ips migration and then check the migration messages.

Proposed resolution

Since the IPs are unique, and since the destination plugin already declares that it identifies the migrated rows with the blocked IP address, add a return statement:
return ['ip' => $row->getDestinationProperty('ip')];

Remaining tasks

Patch + test.

User interface changes

Nothing.

API changes

Nothing.

Data model changes

Nothing.

Release notes snippet

Not needed.

Issue fork drupal-3260391

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

huzooka created an issue. See original summary.

quietone’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -migrate, -migrate-d6-d7 +migrate-d7-d8

Yes, this does bring the BlockIp import destination in line with the interface.

I applied the patch to prove to myself that before this patch the migrate_map_d7_blocked_ips was empty was empty and filled after the patch was applied. And it was.

Thanks @huzooka

Status: Reviewed & tested by the community » Needs work

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

arunkumark made their first commit to this issue’s fork.

arunkumark’s picture

Status: Needs work » Needs review
Issue tags: +Needs reroll, +migration
FileSize
1.44 KB

Re-rolled the patch to work with 10.0.x version.

Spokje’s picture

Status: Needs review » Needs work
Issue tags: +PHPStan-1
Spokje’s picture

Status: Needs work » Needs review
FileSize
560 bytes
2.08 KB
Spokje’s picture

Issue tags: -Needs reroll
longwave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @Spokje. This is RTBC as per #4 with the additional removal of the baseline entry for PHPStan.

alexpott’s picture

Version: 10.1.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 4936c9bd07 to 10.1.x and 8d6284210f to 10.0.x. Thanks!

Committed 90e026f and pushed to 9.5.x. Thanks!

I backported this to D9.5 because only the phpstan changes are not relevant.

alexpott’s picture

  • alexpott committed 4936c9bd on 10.1.x
    Issue #3260391 by huzooka, arunkumark, Spokje, quietone: BlockedIp::...

  • alexpott committed 8d628421 on 10.0.x
    Issue #3260391 by huzooka, arunkumark, Spokje, quietone: BlockedIp::...

  • alexpott committed 90e026fc on 9.5.x
    Issue #3260391 by huzooka, arunkumark, Spokje, quietone: BlockedIp::...

Status: Fixed » Closed (fixed)

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