Currently target Link.php expected that uri for link file will be always absolute
UrlHelper::isValid($values['uri'], TRUE)

Next patch checks if link is external and set absolute argument only for external links.

CommentFileSizeAuthor
#12 interdiff-3034413-09-12.txt4.66 KBmegachriz
#12 feeds-do-not-empty-link-fields-3034413-12.patch6.44 KBmegachriz
#9 feeds-do-not-empty-link-fields-3034413-9.patch3.8 KBmegachriz
#9 feeds-do-not-empty-link-fields-3034413-9-tests-only.patch2.79 KBmegachriz
#4 feeds-target_link_related-3034413-4-D8.patch1.23 KBartematem
#2 feeds-target_link_related-3034413-2-D8.patch534 bytesartematem

Comments

artematem created an issue. See original summary.

artematem’s picture

artematem commented
StatusFileSize
new feeds-target_link_related-3034413-2-D8.patch534 bytes

Status: Needs review » Needs work

The last submitted patch, 2: feeds-target_link_related-3034413-2-D8.patch, failed testing. View results

artematem’s picture

artematem commented
StatusFileSize
new feeds-target_link_related-3034413-4-D8.patch1.23 KB

Update patch + test. Not sure if this is best decision, but for start ok.

artematem’s picture

artematem commented
Status: Needs work » Needs review
megachriz’s picture

megachriz
Primary language Dutch
commented

\Drupal::pathValidator() is not available in unit tests. If this service is needed, it needs to be injected.

I also found out that internal links were not accepted. Thus if the url source starts with "internal:/".

We might need a kernel test for importing links too.

artematem’s picture

artematem commented

I also found out that internal links were not accepted. Thus if the url source starts with "internal:/".

I added Tamper with str replace to change "http://example.com/" to "internal:/" before passing to feed target.

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Needs work because the current patch breaks tests, but also because additional tests are needed.

An unit test is needed to test that relative links are supported in Link::prepareValue(). The patch provides this, but it also removes testing that passing a non-url value results into an empty value.
A kernel test is needed to test that importing relative links is passing field validation.

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new feeds-do-not-empty-link-fields-3034413-9-tests-only.patch2.79 KB
new feeds-do-not-empty-link-fields-3034413-9.patch3.8 KB

Here is a patch with tests.

The last submitted patch, 9: feeds-do-not-empty-link-fields-3034413-9-tests-only.patch, failed testing. View results

megachriz’s picture

megachriz
Primary language Dutch
commented

It would be great if internal links are automatically prepended with "internal:/", so the source doesn't need to add that prefix.

megachriz’s picture

megachriz
Primary language Dutch
commented
StatusFileSize
new feeds-do-not-empty-link-fields-3034413-12.patch6.44 KB
new interdiff-3034413-09-12.txt4.66 KB

This adds the prefix "internal:" if the input url is something like "/node".

Feeds now also converts <front>, <nolink> and <none> the same as the link widget does.

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs review » Fixed

Committed #12.

  • MegaChriz committed 6d4c7e1 on 8.x-3.x 
    Issue #3034413 by MegaChriz, artematem: Link target: added support for...

Status: Fixed » Closed (fixed)

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