Support idlist option on migrate:messages #4728
Conversation
The current approach is just walking the full set. This can be improved by using a custom \FilterIterator class to filter the id map, as in migrate_tools implementation. For now, only using a simple approach to avoid introducing a new class.
|
@claudiu-cristea, thanks for the feedback!
During my testing of the feature, I think I discovered a bug, I opened #4730 about it. I made the changes related to the closure, and the documentation. I have also added test coverage. Next steps: use the existing filter class. |
Do not try to continue processing if there is nothing to process.
Move message item preprocessing into its own method to be able to reuse it easily. Use the provided set of values at idlist option iterating over them, to limit the processed rows. More details on the related inline comment.
|
That last item was less trivial than I was expecting 😅 Long story short:
More details on the inline comment. In the process I also fixed one related error on mainline: prevent error output if the id map is empty, e.g. before migrations has been executed; and moved the pre-processing of the message item into its own method for easier re-use. @claudiu-cristea, this is now ready for a second review. |
claudiu-cristea
dismissed
their stale review
Issues were fixed. Creating a new review.
Not entirely needed but easier to read.
Convert the following:
- id{s} into ID{s},
- sql into SQL,
- Drupal Core into Drupal core,
Fix preprocessMessageRow() docblock subject.
|
@claudiu-cristea, thanks for the feedback and the attention to detail. I have added most of the requested changes, details in context in previous review thread, went I took a slightly different path. |
|
About the in-ability to filter messages on an ID map by multiple items, I created a new Drupal core issue at #3210257: Improve migrate messages retrieval. |
In the context of trying to make
migrate_toolscompatible with drush again, this change is bringing--idlistoption for themigrate:messagescommand.More context at drupalspoons/migrate_tools#118: Migrate commands are now part of Drush core.
The current approach is just walking the full set.
This can be improved by using a custom \FilterIterator class to filter
the id map, as in migrate_tools implementation.
For now, only using a simple approach to avoid introducing a new class.