Skip menus when privileges are missing

[izzy]

Would it be possible to skip (sub-) menus automatically if the user lacks the required permissions? Right now, the items name (e.g. "worklog" for "Work Log") is displayed link-less. I know there is some (undocumented) switch hide_if_disabled - but that would not be fitting here. If the user has no permission on an item, it should not appear. Makes no sense to explicitly state this for each item.

[izzy]

P.S.: This problem comes up only when simply assigning an existing trac menu item as sub-menu. Trac itself will not display it any longer than - so the menu plugin just sees the sub-menu definition (e.g. roadmap.parent = timeline) but doesn't find the permissions anymore. A work-around is to additionally define the permissions explicitly (in the given example: roadmap.perm = ROADMAP_VIEW - so the plugin nows to not display it. The real fix, however, would be to skip all entries not having a href (they make no sense either, since they'd link nowhere). Best place I guess would be around line 68 in web_ui.py:

  or config_menu.get(name, {}).get('href', '')=='' \

But this would hit the same problem which causes #4267: The original Trac menus don't seem to have the "href" attribute, and would be skipped by that as well (for #4267 the missing "href" causes the "class=active" to be skipped). I'm not familar enough with the Trac internals to solve this - but hopefully this at least points out where the problem lies.

[Catalin BALAN]

Hey izzy,

Thank you very much for your feedback.

hide_if_disabled option should be used in order to obtain the required functionality.

Currently there is no option to specify whether a specified option defined in config file should add a new item, or update an existent item. So, hide_if_disabled actually means, "don't add a new item, only update existing item, if any"

I'll keep this ticket open, since I'm not happy either with this approach. Waiting for suggestions.

Thank you, Catalin Balan

[izzy]

Hi Catalin,

as I suggested, you could check if the item has some target href - if not, it makes no sense (for people who think it makes sense, i.e. to add some "divisors", you could add a new property like e.g. "force_display").

What I still do not understand (and that's what makes my suggestion a bit difficult) is: Why is there no href property available for already existing menus? Obviously, tree_node.get('href') seems to return None for them. Investigating the code in trac/web/chrome.py it seems that chrome['nav'] is an array of elements only having the properties name, label and active (to be found around line 500 there). So how to get to its href? It must be there somewhere - otherwise it would not be rendered into the resulting menu. I'm quite confused...

[izzy]

Other suggestion: Why not make hide_if_disabled = 1 the default? Guess in more than 80% of the cases this makes most sense, and the additional setting as described in comment:3 is probably not required.

[Catalin BALAN]

(In [5035]) MenusPlugin: - Replaced 'hide_if_disabled' with 'enabled'. Checkout documentation for more details. refs #4280

[Catalin BALAN]

Now, the old hide_if_disabled option is enabled by default. item = enabled is equivalent with Izzy's force_display suggestion, but reusing trac core functionality. Closing this ticket.

Thank you, Catalin Balan

[izzy]

Yo man - now half of my menus are gone. Going back to the previous version, they are back again. Looks like that fix broke menus which are not trac components - since all of my manual "jumppoints" (like "create new task", "create new enhancement", etc.) are gone - though they should be controlled by the specified permission (xxx.perm=TICKET_CREATE) (they cannot be controlled by enabled/disabled as they are no components, as you explained), which worked with the previous code version.

So this is not really an improvement: Rather to always have to specify hide_if_disabled, I now had to specify enabled - and I'm not sure what would happen with those menus then. If they'd appear though to missing permission, that would be even worse than before. Hence I reopen the ticket - this one needs some more work, sorry ;)

[Catalin BALAN]

Hey Izzy,

Since [5035] hide_if_disabled is droped and replaced by enabled(comment:6), which is mandatory for "non-components" items.

So, just to make sure that everything is clear, the following config(before [5035]):

[mainnav]
tickets.parent = some_item
tickets.hide_if_disabled = True

new_item.label = New item
new_item.perm = SOME_PERMISSION
new_item...

should look like (after [5035]):

[mainnav]
tickets.parent = some_item

new_item = enabled
new_item.label = New item
new_item.perm = SOME_PERMISSION
new_item...

Please let me know if more details are needed(if not, feel free to close this ticket).

Thank you, Catalin Balan

[izzy]

Replying to cbalan:

Since [5035] hide_if_disabled is droped and replaced by enabled(comment:6), which is mandatory for "non-components" items.

Ahhh! Could it be you missed to stress the second part of that sentence? You simply pointed to the docu - but if you meant the wiki, you forgot to update this important detail there ;) Seems to work with that, yepp.

should look like (after [5035]):

[mainnav]
tickets.parent = some_item
new_item = enabled

Oops? Sure you did not mean new_item.enabled = 1? Hm - both seem to work the same way, so never mind.

Please let me know if more details are needed(if not, feel free to close this ticket).

After all, it looks like this makes the setup a bit complex. And I always mistake enable for enabled. But as long as it works, who cares. Only thing I dislike is that my config gets even longer, since I now have to specify a log of "enabled" lines. Need to check whether there is a possibility to include another file with trac.ini, so I could outsource the menu config to make the trac.ini readable again ;)

So I guess these changes shall go as they are now? If so, please confirm (while closing the ticket), so I will update the wiki page accordingly (besides: the wiki page still misses the description for inherit and path_info which I did not understand completely. As I see it now, path_info probably is for ctxnav entries to be displayed only on pages matching that regular expression, and thus path_info should contain a regex according to which standard - Perl (PReg)?).

Best regards, Izzy.

[Catalin BALAN]

Izzy,

Thank you for updating MenusPlugin page.

Enabled accepts boolean values, so the following values are available:

new_item = 1
new_item = true
new_item = enabled
new_item.enabled = 1

For ini include, check out ​wiki:TracIni#GlobalConfiguration

Closing this ticket.

Thank you, Catalin Balan

[izzy]

Thank you, Catalin. One last comment: Since enabled = True is the default for trac-provided items, wouldn't it be consequent to let this be the default for user-defined items as well? It is either not equivalent to my force_display suggestion (as you write in comment:6), as this does not really force the display (item will still not be displayed at all when e.g. permissions don't match - but I guess force_display is not really needed). Making its defaults equivalent for trac-defined and user-defined items would make things more clear.