Problem/Motivation

User can use route:<nolink> to create no-link links (see #2693725: Add <nolink> to allow for non-link links ). However, D8 migration does not handle empty URI, and properly. They are skipped.

Proposed resolution

Add support for migrating empty URI, <nolink>, <none> into route:<nolink> to create no-link links.

As <front> is a special migration case, so should be these.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None

CommentFileSizeAuthor
#16 3110064-12.patch1.65 KBbenjifisher
#13 3110064-13.patch1.65 KBidebr
#13 3110064-13-test-only.patch768 bytesidebr
#12 interdiff-8-12.txt510 bytesVidushi Mehta
#12 3110064-12.patch1.65 KBVidushi Mehta
#8 interdiff_2-8.txt596 byteschandrashekhar_srijan
#8 3110064-8.patch1.66 KBchandrashekhar_srijan
#2 drupal-migrate_empty_nolink-3110064-2.patch929 bytesckng
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ckng created an issue. See original summary.

ckng’s picture

ckng
Primary language English
CreditAttribution: ckng at O8 for University of Minnesota commented
Status: Active » Needs review
FileSize
drupal-migrate_empty_nolink-3110064-2.patch929 bytes

See attached patch.

ckng’s picture

ckng
Primary language English
CreditAttribution: ckng at O8 for University of Minnesota commented
Version: 9.0.x-dev » 8.9.x-dev
rutiolma’s picture

rutiolma
Location 🇵🇹
CreditAttribution: rutiolma commented
Status: Needs review » Closed (duplicate)
Related issues: +#3067576: <nolink> menu items lost on migration

This is a duplicate of #3067576
Also, this patch doesn't work for me but the one on the related issue does.

ckng’s picture

ckng
Primary language English
CreditAttribution: ckng at O8 for University of Minnesota commented
Status: Closed (duplicate) » Needs review

#3067576: <nolink> menu items lost on migration is a patch for menu links, and it is for `link_uri` process plugin.

This patch is for `field_link` process plugin, to process link field, totally different thing.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer and at Four Kitchens commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Agreed, this is a different and valid issue. We will need to write tests for this.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

chandrashekhar_srijan’s picture

chandrashekhar_srijan CreditAttribution: chandrashekhar_srijan at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
FileSize
3110064-8.patch1.66 KB
interdiff_2-8.txt596 bytes

Worked on the test cases.

Status: Needs review » Needs work

The last submitted patch, 8: 3110064-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Kumar Kundan’s picture

Kumar Kundan CreditAttribution: Kumar Kundan as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
idebr’s picture

idebr CreditAttribution: idebr at iO commented
Status: Needs review » Needs work
Issue tags: -Needs tests

Test failure is unrelated:

Drupal\Tests\ckeditor\FunctionalJavascript\CKEditorIntegrationTest::testOffCanvasStyles
Behat\Mink\Exception\ElementNotFoundException: Element matching css ".cke_top" not found.

+++ b/core/modules/link/src/Plugin/migrate/process/FieldLink.php
@@ -60,10 +60,15 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
+    // Empty URI and non-links are allowed.
+    if (empty($uri) || in_array($uri, array('<nolink>', '<none>'))) {
+      return 'route:<nolink>';
+    }

array('<nolink>', '<none>')

should be a short array syntax: 

['<nolink>', '<none>']
Vidushi Mehta’s picture

Vidushi Mehta CreditAttribution: Vidushi Mehta at gai Technologies Pvt Ltd commented
Status: Needs work » Needs review
FileSize
3110064-12.patch1.65 KB
interdiff-8-12.txt510 bytes

Added a patch that pointed #11.

idebr’s picture

idebr CreditAttribution: idebr at iO commented
Status: Needs review » Reviewed & tested by the community
FileSize
3110064-13-test-only.patch768 bytes
3110064-13.patch1.65 KB

The patch in #12 works as expected.

Added a test-only patch to indicate the changed behaviour. The actual patch is identical to #12.

The last submitted patch, 13: 3110064-13-test-only.patch, failed testing. View results

The last submitted patch, 13: 3110064-13-test-only.patch, failed testing. View results

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
CreditAttribution: benjifisher commented
FileSize
3110064-12.patch1.65 KB

I am re-uploading the patch from #12, since the testbot automatically retests RTBC patches every two days ... and it has been re-testing the test-only patch.

I have not reviewed the patch myself.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 16: 3110064-12.patch, failed testing. View results

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
CreditAttribution: mikelutz as a volunteer and at Four Kitchens commented
Status: Needs work » Reviewed & tested by the community

Unrelated test failure.

  • catch committed 9ca47a6 on 9.1.x 
    Issue #3110064 by idebr, chandrashekhar_srijan, Vidushi Mehta, ckng,...

  • catch committed 8a851e1 on 9.0.x 
    Issue #3110064 by idebr, chandrashekhar_srijan, Vidushi Mehta, ckng,...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.1.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.1.x and cherry-picked back to 8.9.x, thanks!

  • catch committed d0ab3ca on 8.9.x 
    Issue #3110064 by idebr, chandrashekhar_srijan, Vidushi Mehta, ckng,...
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

The fact that migrating <nolink> is now supported means that 50% of the functionality of https://git.drupalcode.org/project/special_menu_items/-/blob/7.x-2.x/spe... is now migrated from D7 to D8|9!

If we'd also add support for <separator>, that could be 100%, and then we could update core/modules/migrate_drupal/migrations/state/migrate_drupal.migrate_drupal.yml accordingly!

Status: Fixed » Closed (fixed)

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