Help link always appears in navigation

Apologies if this is a duplicate, I couldn't find it reported elsewhere.

maneesha binish → made their first commit to this issue’s fork.

I attempted to reproduce this issue by installing Drupal 11 with the Minimal profile, but I was unable to reproduce the exact behavior described in the issue. Here's what I observed:

-The help link in the navigation module appears only if:
-The Help module is installed and placed in the left sidebar region.
-The cache is cleared afterward to ensure the link becomes visible.
-When the Help module is uninstalled, the link disappears as expected.

Based on these observations, I could not confirm the issue as reported. However, I identified a potential enhancement to address this scenario if it arises. I have added the following temporary change to the navigation.block_layout.yml file:

additional: { }
dependencies:
module:
- help

This change ensures that the Help module dependency is explicitly defined, potentially resolving any unexpected behavior related to the navigation link.

Hi
I have tested MR !10363 on Drupal version 11.x with minimal Profile
MR was applied successfully,
but Help still appears in Navigation instead of the module that was not installed, and the Help Page displays a 404 error.

Attaching screenshots

moving to "Needs work"

Tested locally the fix 35b58e49 - Fix: Prevent Help link from being displayed when Help module is not enabled works
Changing status to needs review.

Thank you for jumping into this!

Some comments added in the MR that might need to be addressed.

Work done on the MR to resolve comments.

I am not sure that the new access checking code is needed. Seems like this could be a one-liner fix. Just check whether the help module is enabled..

Thank you for stepping up into this @oily.

The NavigationLinkBlock can support any link. In this very specific case is a link to the help page, but it could be a link to anywhere.

That's the reason why we cannot ad any one-liner just to check whether the help module exists, because the bug would be present for other links. We need a generic way to solve it.

Agree that references to help in the yml file are not needed.

Would be amazing to include basic automated tests for this as well. They'll be necessary to approve the MR.

Hi @plopesc, I have noticed that the MR varies a lot from the current Drupal 11.x versions of the same 2x files.

I suggest we create a new branch from 11.x and hide the existing branch. The code in it may have made sense for Drupal 8 or 9 but it seems just a source of confusion now. I think it best to start from a clean slate.

We need to confirm that this issue has not been fixed already in Drupal 11.x. Up-to-date screenshots would help.

If it is still a valid issue, we need to work out how and where and in which file to implement the logic that checks whether the help module is enabled and if it is not to not display the link in the navigation block.

oily → changed the visibility of the branch 3488293-help-link-always to hidden.

Created fresh branch forked from 11.x. Hid the old branch.

Thank you @oily.

Even if the old branch was not actually outdated, agree that we had some noise there. On the other hand, access to the history of the discussions is a bit more complex.

Took the generic bits from the original MR and added test coverage. This should be ready for review now.

Could you please remove the "Draft" state from the MR? It seems I don't have permissions for that since you created it.

Hi
I have applied MR! 10662 on Drupal 11.x with a minimal Profile
The MR is applied successfully...

Test Result: PASS
If the Help module is not installed, the Help link will not appear in the navigation menu.
If the Help module is installed, the Help link will appear in the navigation menu and redirect to the appropriate page.

Attaching Screenshot

RTBC+1

But MR is in Draft state so keeping the issue in "needs review"

Removed draft status of the new 11.x branch.

@plopesc Thank you for the code. Great that you created a test, too! I confused this with another issue that dates back to Drupal 8. We seem to be making progress on the new branch.

@kanchan bhogade Thank you for the review. I think you can change the status to RTBTC now?

The 'test-only' test fails as it should. Changing to RTBTC.

Hi @oily, noticed that you reverted back the issue from RTBC to Needs Review with no comment.

Would be great if you could explain the reasons behind that change, so we would be able to work on them.

Thank you in advance.

@plopesc I am not sure about this but I have noticed several of tickets I am following set to RTBTC which have not automatically fixed within 2 weeks as I thought they should. So I moved 2 such tickets back to NR as I thought perhaps if a core maintainer reviews and sets to RTBTC then it will auto-fix in 2 weeks. I also thought that perhaps NR would get the attention of a maintainer whereas RTBTC might be a status that will be ignored for a longer period.

@oily I want to encourage you to review the documentation on the issue Status → . It might clear up some confusion.

For this issue, and in general, you'd ideally have someone who did not contribute to the code to do a technical review and testing. That person, given they found no issues, could then move the issue to RTBC. RTBC issues remain in that state until a core committor has time for a final review before commiting. Once committed, it would move to Fixed. Then if there are no further comments or activity, it woul automatically move to Closed (Fixed). Again, review the issue Status → docs for more details.

I'll go through the MR to review and test. It looks like there is a test failure to handle. I'll see if I can work out why that is.

Test failures we're fixed by merging the latest 11.x.

Bug fix works as advertised and the test is super comprehensive. Nice work on that @plopesc!

Marking as RTBC.

@m4olivei WRT #32 I had a core maintainer quibble with me over this issue when another person reviewed an MR. I really dont wish to review all these documents/ issues right now. I am pleased you have stepped in to progress this issue. Hopefully a maintainer will step in soon. But that maintainer might not hurry if he does not see something more than 'code looks great!' but at least one line of feedback on the code, some small improvement. It is not an exact science. In this issue we have a coder with more experience and a reviewer with more experience than in the other issue so all-in-all more likely the code IS 100% good. Such matters as experience level is not factored into the documents but maybe it should be.

Yep fair points. I'm only becoming more involved in core development recently. I'm still learning. There definitely is a human element at play that can be the difference between how fast issues move. This issue will also get more eyes being tied to Navigation, which is tied to Drupal CMS, which has a lot of resources focused on it ATM. Unfortunately, these types of things are not always well articulated in documentation and otherwise.

In any case, thank you for your contribution and thoughtful comments. Hopefully we see this one through to completion soon!

nod_ → closed merge request !10662

It's not an exact science how to get a committer to look at an issue, you're always welcome to ping us on slack. we have the #core-development channel for that :)

Committed and pushed 9b3b1c6b552 to 11.x and 8e61da94f96 to 11.1.x. Thanks!

Thank you @nod_ I will take a look at the #core-development channel.

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