Closed (duplicate)
Project:
Migrate Plus
Version:
8.x-4.x-dev
Component:
Plugins
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
23 Jun 2016 at 15:38 UTC
Updated:
5 Jun 2019 at 21:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
mikeryanNope, not yet.
Comment #3
mikeryanBump - this would be useful. And so would regular expression replacements.
Comment #4
mikeryanSetting to Novice - this is actually pretty simple, given http://cgit.drupalcode.org/drupal/tree/core/modules/migrate/src/Plugin/m... as a model.
Comment #5
Lowell commentedinitial code for string_replace plugin
Comment #6
Lowell commentedinitial code for the preg_replace plugin
Comment #7
heddnGreat start here. Much appreciated you stepping up and rolling some patches.
Nit: normally, we use NULL.
Can you just throw a MigrateException if it is missing?
If it isn't a string, should we fail more loudly? Or provide that as an option?
I think isset on 0 returns FALSE? Might try adding a phpunit test to confirm.
Should we fail more loudly if the required parameters aren't available?
#2820490: FormatDate process plugin has some examples on how to throw exceptions and write unit tests, if you are up for the challenge. On your next patches, can you either combine them together or (preferably) split out the regex into a separate issue.
Comment #8
Lowell commentedsure, thanks for the feedback.
My thought was to let them fail gracefully and return the input $value.
If we want exceptions thrown, I did see how those were done.
I will make regex a separate issue
Comment #9
Lowell commentedupdated str_replace plugin
I looked at the skip_on_value test as a reference, but haven't made the test for this plugin yet
Comment #10
Lowell commentedComment #11
shawngo commentedThis looks like it may be a duplicate ofSimilar things are happening here #2830512: String Replace Process pluginEdit: I didn't see the PregReplace plugin at first.
Comment #12
mikeryanGiven we've got a patch in the other issue for str_replace, let's narrow the scope of this to preg_replace. #6 is the current patch along those lines.
Comment #13
osopolarWould be nice if we can also manipulate vallues captured by subpatterns in the regular expression. Imagine for example a link to an old path like
<a href="/node/1234">should be rewritten to use the destination nid instead of the source nid. Using preg_replace_callback() instead preg_replace may allow manipulating the node-id captured with the subpattern in the regex#<a href=['"]/node/(\d+)['"]>#i.The following patch allows to use process plugins to alter the captured subpattern matches. That way the source node-id could be replaced with the destination node-id using the migrate_lookup process plugin. Or for example Wordpress Next Gen Gallery tags could be replaced with entity embed tags and the correct destination entity ids:
If there are no subpatterns the function will just replace the pattern with the replacement_pattern, as in preg_replace.
Tests are still missing, sorry.
Comment #14
pingwin4egThanks @osopolar ! Your solution is perfect!
Though one notice: the
$selfvariable is undefined.I suppose the patch from #9 can be hidden as it only contains the str_replace plugin.
Comment #15
osopolarThanks @pingwin4eg for bringing this up again. I did som changes, to make the process plugins (inside
preg_replace) optional, to be able to do simple replacementes with no additional processing. This version also allow source placeholder (likepreg_replace_match_1) in replace patterns.Yes, this $self variable is a leftover. Before I used
$self = &$this;to access $this inside the anonymous function. In newer php versions (> 5.3 I think) $this can be accessed from inside the anonymous function. I removed it from the patch (but didn't test again).Still no tests, sorry.
Comment #16
heddnAdding tests to this is a fairly trivial phpunit task. Tagging for that. See #2923891: Transliterate file names as a recent example on how to add this type of test.
Comment #17
heddnThis also seems very similar to #2830512: String Replace Process plugin, which has more progress right now. So I might suggest joining forces and closing this one as duplicate.
Comment #18
osopolarNot sure how you came to the conclusion that #2830512: String Replace Process plugin has more progress, as both issues do not have tests, but if it is about popularity you are right, the other issue has attracted more attention. So do you think it would be better to merge both solutions into one plugin or to have two separate plugins?
I'd like to provide tests and want to add them, but currently I have zero resources for that.
The functionality could be helpful for a lot of migrations, for example the following modules:
* Node Embed
* Entity Embed
* Media or Media (WYSIWYG) Embed
* Insert Views
* Many Wordpress shortcodes etc.
Comment #19
k_a_l commentedJust confirming that patch #15 worked for me. Thanks!
Comment #20
k_a_l commentedre-assigning back to 4.x-dev. Sorry
Comment #21
heddnLet's close this as duplicate.
Comment #22
osopolar@heddn: You didn't answer my question in #18, so I guess there is no interest in getting this code in. Otherwise let me know.
Comment #23
heddnre #22:sorry I missed responding. The issue that this is closed dup to has regex capabilities.
Comment #24
osopolarYes, I have see that, but is it able to process the regex matches? For example Replace a source id with a destination id (i.e. node id), can't see that. That seemed important to me and worth implementing, because it has so much power. Lets say you have a migration where sorce nid (i.e. 123) != destination nid (i.e. 987) and content with Links like node/123. Is there any plugin which can replace node/123 with node/987, this one can, see #13 (more examples for use-cases in #18). This works, while for each matched value
$migrate_executable->processRow($new_row, $this->configuration['process']);gets called, so it's no only about replacing IDs, it exposed the matched value to all process plugins.Maybe I have overlooked it in the other issue, if so we won't need this functionality.
See also #37 in the other issue.
Comment #25
heddnThere's regex capabilities that uses preg_replace in that other issue. I'm fine with re-opening this, but it seems like the features exist?
Comment #26
nathan.simmonds commentedI agree with @osopolar, the ability to expose and manipulate matched patterns as described would be really useful, and is in fact exactly what I need to migrate embedded entities. Basic regex search & replace can't look up the migrated ids, and there's no other contrib module that I can see which enables embedded media to be migrated correctly. So unless it's is already covered elsewhere definitely I support inclusion of this patch.
Comment #27
fls-pcate commentedI'm in the same position. This feature would make it easy to map migrated id's in body field urls. Has anyone found a solution which does this, or is it possible with the referenced str_replace plugin?
Comment #28
bcweaver commentedI've got to agree, the ability to sub-process matched subpatterns through other process plugins is a killer feature, and looks like exactly what I need for handling embedded media references.
The killer feature in @osopolar's patch isn't the use of regular expressions, it's the ability to pass what's matched through other process plugins, most notably
migration_lookupin my case. Match an embedded reference and then pass the ID portion of that match throughmigration_lookupto convert it to the destination ID of the previously-migrated content.The example YAML in #13 illustrates this usage.
If other replacement plugins already provide the ability to pass matches through sub-process-plugins, I'll certainly use those, but it seems like this functionality isn't provided elsewhere.
Comment #29
fls-pcate commentedJust a followup.
After finding: https://www.drupal.org/project/migrate_plus/issues/2958642 and https://www.drupal.org/project/migrate_plus/issues/2958672 I applied the patches and used it to replace entity id links in HTML body fields with relative ease.
I still link @osopolar additions would be useful, but the above solved my immediate use case, and may for others.
Comment #30
bcweaver commented@fls-pcate thanks so much for those links. I had not already come across these from my searching.
Comment #31
andyg5000Several people agree with @osopolar, including myself. I'm likely going to try the patches mentioned by @fls-pcate for my immediate needs, but I see a ton of value in the #15 patch. Especially, because the other issue only supports DOM objects.
Comment #32
andyg5000After further review, the patch isn't working as expected and throws "Undefined variable: self" error for me. If someone can provide an example of using the `str_replace` plugin and iterating/processing the result, I think we could close this out again. Otherwise, still feel it would be useful.
Comment #33
osopolar@andyg5000 the patch was working at time of publishing. Currently I don't have a need for this functionality and maintainers aren't interested to implement this functionality, therefore I do not maintain the plugin until I have a project requiring it. Be free to re-roll the patch to get it work with current state of migration API.
Comment #34
alisonLooooooove this functionality, and thank you for all the work on it.
And thanks for sharing those links, @fls-pcate -- in particular, I hadn't noticed #2958672: Use migration lookup on text fields before, which does seem to provide comparable functionality to @osopolar's work here, it just happens to be more current, because, #time.
If others agree, maybe close this out as outdated or duplicate (even though this one originally came first)?
Comment #35
andyg5000Hey @alisonjo2786,
Thanks for finding and posting the related issue. I reopened this a few weeks ago, so I'm closing it back out based on your feedback ;)