Problem/Motivation

TermTranslation and TermLocalizedTranslation are not caring of the return value of their parent's prepareRow() method which means that it is impossible to skip rows by returning FALSE in hook_migrate_prepare_row() implementations (note that SourcePluginBase::prepareRow() invokes these hooks).

See SourcePluginBase::prepareRow().

Proposed resolution

Return FALSE whenever parent::prepareRow() returns FALSE.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 core-term_translation_ignored_preparerow_return_value-3187426-7.patch1.31 KBhuzooka

Comments

huzooka created an issue. See original summary.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Title: Ignored prepareRow() return value in TermTranslation and in TermLocalizedTranslation and in » Ignored prepareRow() return value in TermTranslation and in TermLocalizedTranslation
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Title: Ignored prepareRow() return value in TermTranslation and in TermLocalizedTranslation » Ignored parent::prepareRow() return value in TermTranslation and in TermLocalizedTranslation
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Issue summary: View changes
quietone’s picture

quietone commented

It might be prudent to check all source plugins for this problem.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Parent issue: » #3187433: Eliminate parent::prepareRow() calls with ignored return value from all migrate source plugins

Added a new meta: #3187433: Eliminate parent::prepareRow() calls with ignored return value from all migrate source plugins.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: Unassigned » huzooka
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: huzooka » Unassigned
Status: Active » Needs review
StatusFileSize
new core-term_translation_ignored_preparerow_return_value-3187426-7.patch1.31 KB
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Trivial patch of the month

This clearly ignores the MigrateSkipRowException that is caught in \Drupal\migrate\Plugin\migrate\source\SourcePluginBase::prepareRow(), which is the parent implementation that is called eventually in both cases here.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

Is there any existing test coverage where a line or two could be added for this? I'm not sure it's worth adding new test coverage from scratch given the bug is in the specific implementation.

quietone’s picture

quietone commented

@catch, alexpott has done just that, in the meta for this #3187433: Eliminate parent::prepareRow() calls with ignored return value from all migrate source plugins. Since that will find all the source plugins that have this problem I think this can be closed as a duplicate and all the necessary changes made in the meta. And patch over there is a manageable size, ~10K, so I don't see a reason to keep this issue.

quietone’s picture

quietone commented
Status: Needs review » Closed (duplicate)

Yes, closing this as a duplicate.