Problem/Motivation

On #3090659: Make a way for help topics to generate links only if they work and are accessible, we created two new functions for making links in help topics that degrade nicely when the link cannot be accessed or doesn't exist: help_route_link and help_topic_link.

On #3215784: [META] Fix up topics to use new help_route_link function and #3192585: Fix up topics to use new help_topic_link function, we fixed up the existing help topics to use these functions.

On this issue, we want to enforce the correct use of those functions in a test. The test should add code to core/modules/help_topics/tests/src/Functional/HelpTopicsSyntaxTest to make sure that:

  • Both help_topic_link and help_route_link functions are being used for links and not the base Twig url() function.
  • If help_route_link or help_topic_link is used, the text is translated.
  • The syntax test includes "bad" examples to verify the tests are working, so there should also be "bad" examples added that violate those two test cases, to make sure the test is working properly.

Steps to reproduce

Proposed resolution

Add code to HelpTopicsSyntaxTest for these tests.

Remaining tasks

  1. Patch, review, commit.
  2. Add a change record to note that use of these functions is required by this test. See also the change record for #3090659: Make a way for help topics to generate links only if they work and are accessible: https://www.drupal.org/node/3192582

User interface changes

No, test only

API changes

Sort of -- we'll be requiring use of these functions.

Data model changes

No.

Release notes snippet

No.

Comments

jhodgdon created an issue. See original summary.

jhodgdon’s picture

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

Status: Postponed » Active

Moving to active as ticket in #2 is closed.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

amber himes matz’s picture

Assigned: Unassigned » amber himes matz

Working on this. Looks like I can use Drupal\Tests\Core\Utility\LinkGeneratorTest for guidance.

andypost’s picture

Good idea 👍 looks that one is primary blocker

amber himes matz’s picture

Assigned: amber himes matz » Unassigned
Status: Active » Needs review
StatusFileSize
new2.25 KB

According to the IS, there are 3 objectives. The only thing missing from HelpTopicsSyntaxTest (as far as I could tell) was a check for use of url(). I happened to have a month-old pull of 10.1.x which had 2 core help topics that still used url() (i.e. {% set search_url = render_var(url('search.view')) %}) and my update to this test caught them. (It doesn't catch all of them at once, just the first one it finds. When that one is fixed, then it would catch the next failure.)

1. Both help_topic_link and help_route_link functions are being used for links and not the base Twig url() function.

- I added a string-match check in HelpTopicsSyntaxTest::verifyTopic() to see if url( was found. I didn't know how to explicitly check for use of a certain function in this testing framework. So I hope that string-match type of check will suffice here.
- I added a "bad topic" (core/modules/help_topics/tests/modules/help_topics_test/bad_help_topics/syntax/bad_help_topics.url_func_used.html.twig) that uses url()
- I added a case to HelpTopicsSyntaxTest::verifyBadTopic() that verifies that the new bad topic fails as expected.
- There is already a test topic (core/modules/help_topics/tests/modules/help_topics_test/help_topics/help_topics_test.test_urls.html.twig) that uses help_route_link() and help_topic_link() in various ways and tests them in HelpTopicsSyntaxTest::verifyTopic()

2. If help_route_link or help_topic_link is used, the text is translated.

- This appears to be covered already with HelpTopicsSyntaxTest::verifyTopic() as it checks core/modules/help_topics/tests/modules/help_topics_test/help_topics/help_topics_test.test_urls.html.twig

3. The syntax test includes "bad" examples to verify the tests are working, so there should also be "bad" examples added that violate those two test cases, to make sure the test is working properly.

- This appears to be covered already with HelpTopicsSyntaxTest::verifyTopic() as it checks core/modules/help_topics/tests/modules/help_topics_test/help_topics/help_topics_test.test_urls.html.twig

I'm also working on a unit test to cover HelpTwigExtension::getRouteLink() and HelpTwigExtension::getTopicLink() but I'm struggling. Maybe it's something we can add later and isn't necessary for this issue, as it isn't in the original scope. (I can open a follow-up issue if it seems like a good idea to others.)

Status: Needs review » Needs work

The last submitted patch, 9: 3219923-9.patch, failed testing. View results

amber himes matz’s picture

Status: Needs work » Needs review

Re-testing the patch as test failure looked like an infrastructure error and not an error from the patch.

andypost’s picture

It looks great and solves the issue but follow-up is required for twig functions, \Drupal\Tests\Core\Template\TwigExtensionTest::testFileUrl() could be used as example, I'm fine with follow-up

andypost’s picture

Status: Needs review » Reviewed & tested by the community
Related issues: +#3037230: Finalize the merge of Help Topics into Help

Re-reading #3090659-93: Make a way for help topics to generate links only if they work and are accessible I see that we have extensive functional testing coverage and the scope of the issue is to check all shipped topics has no usage of url(

Also I found no requirement to have unit test and moreover, this class will be moved to help module #3037230: Finalize the merge of Help Topics into Help

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 3219923-9.patch, failed testing. View results

smustgrave’s picture

Status: Needs work » Reviewed & tested by the community

Seems random

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

andypost’s picture

StatusFileSize
new1021 bytes
new1.38 KB

Fixing nit, dot at the end of links we trying to prevent to get full url, leaving RTBC

andypost’s picture

StatusFileSize
new2.25 KB

Last patch went without bad topic(

  • longwave committed c42d65e4 on 11.x
    Issue #3219923 by andypost, Amber Himes Matz, jhodgdon: Add tests to...
longwave’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed c42d65e4a7 to 11.x (10.2.x). Thanks!

Status: Fixed » Closed (fixed)

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