Move list of available tokens from Help to Reports

Done changes as per suggested.

Thanks,
Rajeshwari.

I have rerolled the patch to remove whitespace & new line warnings.

Please review.

@ifrik

I have done changes as per the proposed solution.

Please review.

1. +++ b/token.module
   @@ -29,13 +30,13 @@ function token_help($route_name, RouteMatchInterface $route_match) {
   +    $output .= '<dt>' . t('The list of the currently available tokens on this site are shown ' . Link::createFromRoute(t('here.'), 'token.reports')->toString()) . '</dt>';
   

   That's not the right way to create a link in text. The 'here' needs to be within the text, so the whole sentence can be translated, and the URL added in as a t() placeholder.

2. +++ b/token.routing.yml
   @@ -20,3 +20,10 @@ token.flush_cache:
   +    _permission: 'access content'
   

   Is this the right permission? That means pretty much any visitor to the site can see this, including anon users who typically will have this permission.

   IIRC, generating a token list is reasonably performance-heavy, so that would be a way to DDOS a site...

PS. Also, the link text should be descriptive, so 'here' shouldn't be the link. Something like 'You can also see [LINK]the list of all currently available tokens[LINK] on the site.

I guess this should be the correct way

+++ b/token.module
@@ -31,13 +32,13 @@ function token_help($route_name, RouteMatchInterface $route_match) {
+    $output .= '<dt>' . t('You can also see ' . Link::createFromRoute('Tokens', 'token.reports')->toString().' ,The list of all currently available tokens on the site') . '</dt>';

Nope, that's not right either I'm afraid. That will also break translation!

You need to use a placeholder for the url, so the string passed to t() is completely hardcoded, eg

 t('This is a <href=":url">link</a>.')

See the docs for https://api.drupal.org/api/drupal/core%21includes%21bootstrap.inc/functi... for more details. (I can't find anything about this in the developer docs on d.o, which is either a big omission, or a google fail...)

+++ b/token.module
@@ -31,13 +32,13 @@ function token_help($route_name, RouteMatchInterface $route_match) {
+    $output .= '<dt>' . t('You can also see :url ,The list of all currently available tokens on the site',array(':url' => Link::createFromRoute('Tokens', 'token.reports'))) . '</dt>';

Sorry, but we're now back to the problem with the first patch!

The *complete sentence* needs to be within the call to t().

To make a link, put the actual A element inside the t(), like in my example. Use a placeholder for the value of the href property.

This means also that the placeholder value should be a URL, not a link, so use the Url rather than Link class.

Finally, make sure the sentence is correctly punctuated and capitalized, and also to put a space after a parameter separating comma.

Thanks for persevering! :)
That's now the right approach for the use of the url in the translated string. A few things still need cleaning up though:

1. +++ b/token.module
   @@ -5,6 +5,7 @@
   +use Drupal\Core\Link;
   

   You don't need this import any more.

2. +++ b/token.module
   @@ -31,13 +32,13 @@ function token_help($route_name, RouteMatchInterface $route_match) {
   +
   

   Stray space.

3. +++ b/token.module
   @@ -31,13 +32,13 @@ function token_help($route_name, RouteMatchInterface $route_match) {
   +    $output .= '<dt>' . t('You can also see :<a href=":url">Tokens</a> ,The list of all currently available tokens on the site', array(':url' => url('token.reports'))) . '</dt>';
   

   url() doesn't exist on D8. You want the Url class, and in particular https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Url.php/function/....

   Also, the text should form a complete sentence. Currently, it will read:

   > You can also see :Tokens ,The list of all currently available tokens on the site

   What's wrong with this:

   - there is a stray ':' before the word Tokens
   - Tokens should be lower case
   - there is a stray space after the word Tokens
   - there is no full stop at the end
   - the sentence doesn't make sense really. It should read "You can also see the list of all currently available tokens on the site."

I have added the required controller.Just need the Tokens info which should be displayed in the link.

Getting there! :)

1. +++ b/src/Controller/TokenAvailableTokensController.php
   @@ -0,0 +1,23 @@
   +      '#type' => 'markup',
   +      '#markup' => t('List of Tokens'),
   

   The list of tokens isn't here! See patch #3, which has it.

2. +++ b/src/Controller/TokenAvailableTokensController.php
   @@ -0,0 +1,23 @@
   \ No newline at end of file
   

   Missing newline.

3. +++ b/token.module
   @@ -36,8 +37,7 @@ function token_help($route_name, RouteMatchInterface $route_match) {
   +    $output .= '<dt>' . t('You can also see the list of currently available <a href=":url">tokens</a> on the site.', array(':url' => Url::fromRoute('token.reports'))) . '</dt>';
   

   This looks good!
   Though this is causing a test failure, I think. The problem is that Url::fromRoute() returns an object, and the placeholder must be a string.

   There should be a method on the Url class that outputs the url as a string.

Looking good!

The test failures the latest patch is getting are all because the tests are expecting the help page to have the tokens list:

> fail: [Other] Line 40 of modules/contrib/token/src/Tests/Tree/HelpPageTest.php:
"The list of the currently available tokens on this site are shown below." found

In other words, it looks like the test needs to be updated too. The testing of the token list needs to be moved out of HelpPageTest, and into a new test case for the reports page.

Thanks @joachim. But i am not able to understand what should be the exact logic to be used in new test. We have to modify the previous tests or create a new file with code for test case of report page??

The existing, failing test is called HelpPageTest. That makes it sound like it is testing just the help page. So adding a test for the new reports page here would be wrong, as the name of the test class would no longer be accurate.

So I would say:

- remove the parts of HelpPageTest that test the tokens list
- add a new test (ReportsPageTest?)
- adapt the removed parts and put them into the new test

1. +++ b/src/Controller/TokenAvailableTokensController.php
   @@ -0,0 +1,27 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   

   This is not inheriting from anything, so it needs documentation.

2. +++ b/src/Controller/TokenAvailableTokensController.php
   @@ -0,0 +1,27 @@
   +    $token_tree = \Drupal::service('token.tree_builder')->buildAllRenderable([
   

   this should be injected as a service and assigned to $build['token_tree'] or so.

3. +++ b/src/Controller/TokenAvailableTokensController.php
   @@ -0,0 +1,27 @@
   +    $output .= t('The list of the currently available tokens on this site are shown below');
   +    $output .= \Drupal::service('renderer')->render($token_tree);
   +    return array(
   +      '#markup' => $output,
   +    );
   

   Then you can make the text as $build['info']['#markup'] or anything like that and drop the render call.

4. +++ b/token.routing.yml
   @@ -20,3 +20,10 @@ token.flush_cache:
   +
   +token.reports:
   +  path: '/admin/reports/tokens'
   +  defaults:
   +    _controller: '\Drupal\token\Controller\TokenAvailableTokensController::content'
   

   this also a needs a menu link so it shows up on /admin/reports.

I have no problem with just changing the help test, we can easily rename it to TokenHelpAndReportsTest or something.

I find it inappropriate to move the token list into Reports. The reports page shows usage and statistics, and not all available functionality in the site. In your field list example, the Reports page shows which fields have actually been added in the entities, and it does not show all available field types which you can add.
Help is a very appropriate section for the token list.

If you're concerned that people may not find the help page, we might add a link to it from /token/tree pages.

Adding a patch covering the changes suggested by #40.

Adding a patch covering the changes suggested by #40.

Adding patch with test.