Background:
This issue is part of the task to update the hook_help texts of the Drupal 8 modules:
#1908570: [meta] Update or create hook_help() texts for D8 core modules

Tasks:
- review / write the hook_help text according to help guidelines

Files: 
CommentFileSizeAuthor
#7 update-hook-help-for-statistics-2091419-7.patch3.53 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 59,647 pass(es).
[ View ]
#5 update-hook-help-for-statistics-2091419-5.patch4 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 59,420 pass(es).
[ View ]

Comments

berkas1’s picture

Taking it... Drupalcon Prague

jhodgdon’s picture

Looks like it didn't get done.... berkas1 -- if you still want to make a patch, please assign this issue to yourself. Otherwise, it's open for anyone. Thanks!

TR’s picture

Title:Create hook_help for Statistics module» Update hook_help for Statistics module

hook_help() exists for the Statistics modules, just needs to be updated.

batigolix’s picture

Assigned:Unassigned» batigolix
Issue summary:View changes
Status:Active» Needs work

Givin it a shot

batigolix’s picture

Assigned:batigolix» Unassigned
Status:Needs work» Needs review
StatusFileSize
new4 KB
PASSED: [[SimpleTest]]: [MySQL] 59,420 pass(es).
[ View ]

Patch:

- changes refernce to online docs
- changes url token
- changes ulr() -> \Drupal::\\url()

The help text was already reviewed for D8 as part of this issue: #1446956: Remove the accesslog from statistics

To do:
The text contains a link to anchor on the permissions page.

\Drupal::url('user.admin_permissions', array('fragment' => 'module-statistics'))

The 'fragment' doe not seem to work.

jhodgdon’s picture

Status:Needs review» Needs work

It looks like the fragment needs to be in the 3rd parameter:

\Drupal::url('user.admin_permissions', array(), array('fragment' => 'module-statistics'))

Other than that, looks good!... Oh, there is still one use of url() -- the last line of the patch.

batigolix’s picture

StatusFileSize
new3.53 KB
PASSED: [[SimpleTest]]: [MySQL] 59,647 pass(es).
[ View ]

This patch fixes the fragment.

I removed this from the patch:

-      return '<p>' . t('Settings for the statistical information that Drupal will keep about the site. See <a href="@statistics">site statistics</a> for the actual information.', array('@statistics' => url('admin/reports/hits'))) . '</p>';
+      return '<p>' . t('Settings for the statistical information that Drupal will keep about the site. See <a href="!statistics">site statistics</a> for the actual information.', array('!statistics' => url('admin/reports/hits'))) . '</p>';

So that should be dealt with in another issue. (e.g. a UI text review)

batigolix’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 7: update-hook-help-for-statistics-2091419-7.patch, failed testing.

jhodgdon’s picture

Status:Needs work» Needs review
jhodgdon’s picture

Issue tags:+Needs manual testing

OK. This is looking good. Ready for a manual test that all the links work (which is all that is changing in this patch). (The test failures seemed to be a general failure of the test bot, so I expect they'll pass this time.)

Oh, and someone needs to give it one more check to make sure all the page names, etc. appearing in the help text still match what you actually see when administering the site.

batigolix’s picture

Issue tags:+Novice
jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

I gave this a manual test today and it is all working fine.

jhodgdon’s picture

There's an "avoid commit conflicts" issue touching statistics.module, so I'm going to be extra-careful and wait to commit this until it's taken care of:
#1996238: Replace hook_library_info() by *.libraries.yml file

jhodgdon’s picture

Status:Reviewed & tested by the community» Fixed
Issue tags:-Needs manual testing

I got leave from the other issue people to commit this, so it's in -- committed to 8.x. Thanks again!

Status:Fixed» Closed (fixed)

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