#1116408: Support migrate module: Destination handler class is about adding a migration destination handler for redirect.

This issue is about adding one destination class, so we can create migration classes that only import redirects. I have a use case where I am not importing other entities at the same time, so this is the only clean way I could think to do it.

Patch following, naturally it started with the code on the other issue, but it's a different migration destination class.

PS: I though it will be clear to have it in a separate issue, but I could merge this into the other issue if one of the maintainers think it's better.

Files: 
CommentFileSizeAuthor
#12 1607038-12.redirect.migrate-destination-entity.patch8.69 KBjoachim
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1607038-12.redirect.migrate-destination-entity.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#8 redirect-migrateDestinationEntity-1607038-7.patch9.61 KBheddn
PASSED: [[SimpleTest]]: [MySQL] 101 pass(es).
[ View ]
#8 interdiff.txt11.05 KBheddn
#5 redirect-migrate-1607038-5.patch8.19 KBhussainweb
PASSED: [[SimpleTest]]: [MySQL] 101 pass(es).
[ View ]

Comments

marvil07’s picture

Status:Active» Needs review
StatusFileSize
new7.62 KB
PASSED: [[SimpleTest]]: [MySQL] 102 pass(es).
[ View ]
marvil07’s picture

StatusFileSize
new8.09 KB
PASSED: [[SimpleTest]]: [MySQL] 102 pass(es).
[ View ]

Minor fixes: add inc to info file, change pk schema to avoid serial.

Pedro Lozano’s picture

This worked pretty well for me.

Note that Migrate destination classes usually have the name starting with MigrateDestination, so instead of naming this class MigrateRedirectDestination you should name it MigrateDestinationRedirect.

benjifisher’s picture

I agree with #3.

I thought that redirect_migrate_api() should declare the destination class, but it looks like I was wrong.

Redirects are entities. (At least, the Redirect module implements hook_entity_info().) So why not extend MigrateDestinationEntity instead of MigrateDestination?

Assuming that both this issue and #1116408: Support migrate module: Destination handler class are adopted, there will be duplicated code: both define redirectValidate() methods. What is the best way to handle this? My inclination is to make it public here and then call this version in the destination-handler class. Currently it is protected there and private (why?) here.

hussainweb’s picture

Issue summary:View changes
StatusFileSize
new8.19 KB
PASSED: [[SimpleTest]]: [MySQL] 101 pass(es).
[ View ]

I have made some changes as per comments in #3 and #4. I have renamed the class, extended it from MigrateDestinationEntity and made redirectValidate method protected instead of private.

mallezie’s picture

I tested this on a site i'm migrating, and this works as advertised. Code also looks good.
Not sure if its possible to add a destination for is_new, which is used to generate the hashes.
Otherwise it looks good.

benjifisher’s picture

I marked #2238039: Support migrate module: MigrateDestinationEntity class as a duplicate. Based on the comment there, the proposed patch might have some improvements over the one here. Sorry, I do not have time to review it myself.

heddn’s picture

StatusFileSize
new11.05 KB
new9.61 KB
PASSED: [[SimpleTest]]: [MySQL] 101 pass(es).
[ View ]

Not finding this issue, I recently went down the same path as this in #2238039: Support migrate module: MigrateDestinationEntity class. The attached patch has some of the goodness from my efforts. Some key things to point out from it. It supports parsing out redirects that might include query parameters and targets (#) that were omitted from this. Other things are really just clean-up. Since this extends from MigrateDestinationEntity, there's no need for implementations of prepare, complete, prepareRollback and completeRollback.

heddn’s picture

re: #1607038-4: Support migrate module: Individual destination class
We can't reuse the validate function between the two implementations until PHP 5.4 and traits. OO inheritance won't allow.

benjifisher’s picture

How does parsing the query string and fragment interact with the default setting for the redirect module, Retain query string through redirect.? If you describe what the difference between the patches in #5 and #8 is supposed to be, then I will run a test.

heddn’s picture

re: #1607038-10: Support migrate module: Individual destination class
From all I can tell from working with redirect, query string and targets aren't stored in the redirect or source column in the DB, but rather in the options row.

So, I think this works the same as if someone manually created a redirect. If a destination query string is set, it would take precedence over the source query string. All the migrate does is parse the query string into the source_options/destination_options. This saves someone the work of creating their own processRow() to properly split the values out of the redirect / source and place them into the options.

joachim’s picture

StatusFileSize
new8.69 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1607038-12.redirect.migrate-destination-entity.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

The patches here and at #1116408: Support migrate module: Destination handler class are going to crash, because they both add the same file and the same hook.

#1116408: Support migrate module: Destination handler class is RTBC, so we should assume that one will get in first. Therefore, here's a reroll of the patch at #8, on top of the RTBC patch at #1116408: Support migrate module: Destination handler class.

Status:Needs review» Needs work

The last submitted patch, 12: 1607038-12.redirect.migrate-destination-entity.patch, failed testing.