Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#4280 closed defect (fixed)

Skip menus when privileges are missing

Reported by: izzy Owned by: cbalan
Priority: normal Component: MenusPlugin
Severity: normal Keywords:
Cc: Trac Release: 0.11

Description

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.

Attachments (0)

Change History (11)

comment:1 Changed 6 years ago by 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.

comment:2 Changed 6 years ago by cbalan

  • Status changed from new to assigned

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

comment:3 Changed 6 years ago by 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...

comment:4 Changed 6 years ago by 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.

comment:5 Changed 6 years ago by cbalan

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

comment:6 Changed 6 years ago by cbalan

  • Resolution set to fixed
  • Status changed from assigned to closed

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

comment:7 Changed 6 years ago by izzy

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 ;)

comment:8 follow-up: Changed 6 years ago by cbalan

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

comment:9 in reply to: ↑ 8 Changed 6 years ago by 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.

comment:10 Changed 6 years ago by cbalan

  • Resolution set to fixed
  • Status changed from reopened to closed

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

comment:11 Changed 6 years ago by 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.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.