Follow-up to #1010850: Update MigrateFieldHandler to work with migrate-7.x-2.4 and later

I have a link field where I always want to set a title but not always I have a link. In the link field settings is an option for that:

Optional URL: If checked, the URL field is optional and submitting a title alone will be acceptable. If the URL is omitted, the title will be displayed as plain text.

But on migration this is not allowed, because csandanov added in #53 the code $values = array_filter($values); to the patch with the comment:

Patch #45 doesn't consider if URL is NULL and we always have empty record in link field.

Empty fields should be detected somehow else. Is there a validation hook for that?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

osopolar’s picture

Status: Active » Needs review
FileSize
1 KB

Patch removes $values = array_filter($values) and checks if url or title is set before adding the value. I am still not sure if there isn't a more appropriate function to validate the value, is it?

Status: Needs review » Needs work

The last submitted patch, 1: link-migration-allow-title-without-url-2537248-1.patch, failed testing.

osopolar’s picture

Status: Needs work » Needs review
FileSize
1.02 KB
osopolar’s picture

FIX: Check 'required' != $instance['settings']['title'] not 'required' != $item['title'].

Ronino’s picture

I can confirm that empty URL's are not migrated. But it seems like some things are different now either in the link or the migrate module as patch #4 doesn't change anything for me. I use link 7.x-1.4 and migrate 7.x-2.8.

In my patch I removed the line with array_filter(), too, but only because it does nothing. Arguments are removed from $values before and there's nothing to do for array_filter() as there are only array elements for non-empty URL's.

This patch adds elements to $values for those empty URL's based on the maximum delta it finds in $values. But it then also keeps all records (without checking URL and title for being optional like #4 does) as I think it should be up to the developer what data to import. For example, there is no automatic validation when importing nodes and I don't think this should be different here.

Ronino’s picture

Status: Needs review » Needs work

The last submitted patch, 5: link-migration-allow-title-without-url-2537248-5.patch, failed testing.

Ronino’s picture

Status: Needs work » Needs review

Tests are fully passed.

DamienMcKenna’s picture

This is good, thank you.

I made some refinements to the comments.

DamienMcKenna’s picture

Committed.

Status: Fixed » Closed (fixed)

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