Update hook_help for Node module

ifrik: this looks like you were attempting to set up a task rather than an issue? Normally we don't put steps, goals, skill level, etc. on issues.

Patch to change tokens from @ to !.
Also url changed to \Drupal::url

Thanks! Sorry this took so long to review -- I'm still behind from the Prague sprint.

This looks like it is probably OK. We need someone to do a manual test to make sure all the links still work. We also need someone to manually test whether all the text and instructions in the help still match what you see in the Drupal 8 user interface.

#4: 2091343-hook-help-node-3.patch queued for re-testing.

#4: 2091343-hook-help-node-3.patch queued for re-testing.

That's odd. The PHP syntax in the patched file node.module is fine.

Problem with routing was that I`ve tried to generate URL for path admin/content using \Drupal::url('view.content.page_1') which seems to be incorrect.
If someone could advise how to generate URL by using routing name for Views path 'admin/content' would be nice to know.
For now using \Drupal::urlGenerator()->generateFromPath('admin/content') for that.

RE #12 - look at node.routing.yml - you want node.content_overview for /admin/content.

Create documentation patch

patch:

- http > https
- reference to online docs according to standard
- fix url() mentioned in #12

I don't see any problems... looks pretty straightforward and I don't think anything really needs to be changed.

So I think this one is ready for a quick manual test:
- Verify that all the links work
- Verify that all mentions of pages/text within the UI match what is seen in the UI
- Verify that the formatting is OK.

Based on the manual test this still needs a bit of work.

user_access() is marked as depreciated so it should probably be replaced

A number of the cases didn't render any help text

• admin/structure/types/manage/%/form-display
• admin/structure/types/manage/%/display
• node/%/revisions
• node/%/edit

I am assuming the % isn't being converted to the content type or nid depending. An article was created to conduct type and node testing of the pages that failed.

All links worked and text reads well.

Most of #18 is probably due to:
#2180343: hook_help's $path doesn't contain '%' wildcards.
and would probably be fixed by
#2183113: Update hook_help signature to use route_name instead of path
so I think we should just leave those for the moment.

This issue is purely about fixing up the help *text* anyway, so we should probably file a follow-up issue to address coding issues. Are there any text issues we need to fix? I wasn't sure what "user_access() is marked as depreciated so it should probably be replaced" referred to as it is not in the patch.

the user_access actually occurs earlier in the code than the patch, around line 87 of node.module as of my pull this morning.

The text itself is fine in the patch as can be read but can not be fully manually tested until the cases are fixed. I am just wary of breaking things by adding a patch that fails to do what it is supposed to do. Even though this is probably the case now for most of the other hook helps as well.

Good point about the manual testing... I don't think this applies to most help text in Core, because actually very few of the paths have % in them, and other paths are working fine.

So I see what you're saying about the top part of hook_help(). It is calling user_access() as well as url(), both of which are deprecated... Let's have a follow-up issue to fix that part.

Regarding the main module help text (case 'admin/help#node'), which is all that this patch actually addresses, that has been reviewed and tested, so let's go ahead and get this patch in.

And then the follow-up issue should also review the help fragments with paths containing %.

Here's the follow-up issue:
#2191133: Fix up/review additional parts of node module help

Excellent I agree RTBC

interdiff.diff no longer applies.

error: patch failed: core/modules/node/node.module:97
error: core/modules/node/node.module: patch does not apply

Sorry an automated tool thought that interdiff.diff was the correct patch for the issue. Interdiffs should be posted as .txt files to avoid testbot wasting time on them.

sorry for that

yeah due to a brainlapse i posted a .diff file

attached is the same file w .txt extension

Committed 1ec781d and pushed to 8.x. Thanks!

@batigolix I've done the same thing more than once! I've fixed the automated tool to detect this :)