Trac Hacks - Plugins Macros etc.: Ticket #4280: Skip menus when privileges are missinghttps://trac-hacks.org/ticket/4280
<p>
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 <code>hide_if_disabled</code> - 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.
</p>
en-usTrac Hacks - Plugins Macros etc.https://trac-hacks.org/chrome/hacks/png/trachacks_banner.pnghttps://trac-hacks.org/ticket/4280
Trac 1.2.2izzyMon, 15 Dec 2008 09:47:02 GMThttps://trac-hacks.org/ticket/4280#comment:1
https://trac-hacks.org/ticket/4280#comment:1
<p>
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. <code>roadmap.parent = timeline</code>) but doesn't find the permissions anymore. A work-around is to additionally define the permissions explicitly (in the given example: <code>roadmap.perm = ROADMAP_VIEW</code> - 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:
</p>
<div class="wiki-code"><div class="code"><pre> <span class="ow">or</span> config_menu<span class="o">.</span>get<span class="p">(</span>name<span class="p">,</span> <span class="p">{})</span><span class="o">.</span>get<span class="p">(</span><span class="s1">'href'</span><span class="p">,</span> <span class="s1">''</span><span class="p">)</span><span class="o">==</span><span class="s1">''</span> \
</pre></div></div><p>
But this would hit the same problem which causes <a class="closed ticket" href="https://trac-hacks.org/ticket/4267" title="#4267: defect: class=active only appended to manually added entries when ... (closed: duplicate)">#4267</a>: The original Trac menus don't seem to have the "href" attribute, and would be skipped by that as well (for <a class="closed ticket" href="https://trac-hacks.org/ticket/4267" title="#4267: defect: class=active only appended to manually added entries when ... (closed: duplicate)">#4267</a> 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.
</p>
TicketCatalin BALANMon, 15 Dec 2008 10:39:20 GMTstatus changedhttps://trac-hacks.org/ticket/4280#comment:2
https://trac-hacks.org/ticket/4280#comment:2
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>assigned</em>
</li>
</ul>
<p>
Hey <a class="wiki" href="https://trac-hacks.org/wiki/izzy">izzy</a>,
</p>
<p>
Thank you very much for your feedback.
</p>
<p>
<code>hide_if_disabled</code> option should be used in order to obtain the required functionality.
</p>
<p>
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, <code>hide_if_disabled</code> actually means, "don't add a new item, only update existing item, if any"
</p>
<p>
I'll keep this ticket open, since I'm not happy either with this approach.
Waiting for suggestions.
</p>
<p>
Thank you,
Catalin Balan
</p>
TicketizzyMon, 15 Dec 2008 11:14:39 GMThttps://trac-hacks.org/ticket/4280#comment:3
https://trac-hacks.org/ticket/4280#comment:3
<p>
Hi Catalin,
</p>
<p>
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").
</p>
<p>
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, <code>tree_node.get('href')</code> seems to return <code>None</code> for them. Investigating the code in <code>trac/web/chrome.py</code> it seems that <code>chrome['nav']</code> 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...
</p>
TicketizzyTue, 16 Dec 2008 11:28:06 GMThttps://trac-hacks.org/ticket/4280#comment:4
https://trac-hacks.org/ticket/4280#comment:4
<p>
Other suggestion: Why not make <code>hide_if_disabled = 1</code> the default? Guess in more than 80% of the cases this makes most sense, and the additional setting as described in <a class="ticket" href="https://trac-hacks.org/ticket/4280#comment:3" title="Comment 3">comment:3</a> is probably not required.
</p>
TicketCatalin BALANSat, 20 Dec 2008 17:45:53 GMThttps://trac-hacks.org/ticket/4280#comment:5
https://trac-hacks.org/ticket/4280#comment:5
<p>
(In <a class="changeset" href="https://trac-hacks.org/changeset/5035" title="MenusPlugin: - Replaced 'hide_if_disabled' with 'enabled'. Checkout ...">[5035]</a>) <a class="wiki" href="https://trac-hacks.org/wiki/MenusPlugin">MenusPlugin</a>: - Replaced 'hide_if_disabled' with 'enabled'. Checkout documentation for more details. refs <a class="closed ticket" href="https://trac-hacks.org/ticket/4280" title="#4280: defect: Skip menus when privileges are missing (closed: fixed)">#4280</a>
</p>
TicketCatalin BALANSat, 20 Dec 2008 18:14:32 GMTstatus changed; resolution sethttps://trac-hacks.org/ticket/4280#comment:6
https://trac-hacks.org/ticket/4280#comment:6
<ul>
<li><strong>status</strong>
changed from <em>assigned</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
Now, the old <code>hide_if_disabled</code> option is enabled by default. <code>item = enabled</code> is equivalent with Izzy's force_display suggestion, but reusing trac core functionality. Closing this ticket.
</p>
<p>
Thank you,
Catalin Balan
</p>
TicketizzyMon, 22 Dec 2008 05:45:19 GMTstatus changed; resolution deletedhttps://trac-hacks.org/ticket/4280#comment:7
https://trac-hacks.org/ticket/4280#comment:7
<ul>
<li><strong>status</strong>
changed from <em>closed</em> to <em>reopened</em>
</li>
<li><strong>resolution</strong>
<em>fixed</em> deleted
</li>
</ul>
<p>
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 <strong>which are not trac components</strong> - 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 (<code>xxx.perm=TICKET_CREATE</code>) (they cannot be controlled by enabled/disabled as they are no components, as you explained), which worked with the previous code version.
</p>
<p>
So this is not really an improvement: Rather to always have to specify <code>hide_if_disabled</code>, I now had to specify <code>enabled</code> - 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 ;)
</p>
TicketCatalin BALANMon, 22 Dec 2008 06:21:47 GMThttps://trac-hacks.org/ticket/4280#comment:8
https://trac-hacks.org/ticket/4280#comment:8
<p>
Hey Izzy,
</p>
<p>
Since <a class="changeset" href="https://trac-hacks.org/changeset/5035" title="MenusPlugin: - Replaced 'hide_if_disabled' with 'enabled'. Checkout ...">[5035]</a> <code>hide_if_disabled</code> is droped and replaced by <code>enabled</code>(<a class="ticket" href="https://trac-hacks.org/ticket/4280#comment:6" title="Comment 6">comment:6</a>), which is mandatory for "non-components" items.
</p>
<p>
So, just to make sure that everything is clear,
the following config(before <a class="changeset" href="https://trac-hacks.org/changeset/5035" title="MenusPlugin: - Replaced 'hide_if_disabled' with 'enabled'. Checkout ...">[5035]</a>):
</p>
<pre class="wiki">[mainnav]
tickets.parent = some_item
tickets.hide_if_disabled = True
new_item.label = New item
new_item.perm = SOME_PERMISSION
new_item...
</pre><p>
should look like (after <a class="changeset" href="https://trac-hacks.org/changeset/5035" title="MenusPlugin: - Replaced 'hide_if_disabled' with 'enabled'. Checkout ...">[5035]</a>):
</p>
<pre class="wiki">[mainnav]
tickets.parent = some_item
new_item = enabled
new_item.label = New item
new_item.perm = SOME_PERMISSION
new_item...
</pre><p>
Please let me know if more details are needed(if not, feel free to close this ticket).
</p>
<p>
Thank you,
Catalin Balan
</p>
TicketizzyMon, 22 Dec 2008 08:45:39 GMThttps://trac-hacks.org/ticket/4280#comment:9
https://trac-hacks.org/ticket/4280#comment:9
<p>
Replying to <a class="ticket" href="https://trac-hacks.org/ticket/4280#comment:8" title="Comment 8">cbalan</a>:
</p>
<blockquote class="citation">
<p>
Since <a class="changeset" href="https://trac-hacks.org/changeset/5035" title="MenusPlugin: - Replaced 'hide_if_disabled' with 'enabled'. Checkout ...">[5035]</a> <code>hide_if_disabled</code> is droped and replaced by <code>enabled</code>(<a class="ticket" href="https://trac-hacks.org/ticket/4280#comment:6" title="Comment 6">comment:6</a>), which is mandatory for "non-components" items.
</p>
</blockquote>
<p>
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.
</p>
<blockquote class="citation">
<p>
should look like (after <a class="changeset" href="https://trac-hacks.org/changeset/5035" title="MenusPlugin: - Replaced 'hide_if_disabled' with 'enabled'. Checkout ...">[5035]</a>):
</p>
</blockquote>
<pre class="wiki">[mainnav]
tickets.parent = some_item
new_item = enabled
</pre><p>
Oops? Sure you did not mean <code>new_item.enabled = 1</code>? Hm - both seem to work the same way, so never mind.
</p>
<blockquote class="citation">
<p>
Please let me know if more details are needed(if not, feel free to close this ticket).
</p>
</blockquote>
<p>
After all, it looks like this makes the setup a bit complex. And I always mistake <code>enable</code> for <code>enabled</code>. 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 <code>trac.ini</code>, so I could outsource the menu config to make the <code>trac.ini</code> readable again ;)
</p>
<p>
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 <code>inherit</code> and <code>path_info</code> which I did not understand completely. As I see it now, <code>path_info</code> probably is for ctxnav entries to be displayed only on pages matching that regular expression, and thus <code>path_info</code> should contain a regex according to which standard - Perl (PReg)?).
</p>
<p>
Best regards,
Izzy.
</p>
TicketCatalin BALANMon, 22 Dec 2008 14:10:28 GMTstatus changed; resolution sethttps://trac-hacks.org/ticket/4280#comment:10
https://trac-hacks.org/ticket/4280#comment:10
<ul>
<li><strong>status</strong>
changed from <em>reopened</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
Izzy,
</p>
<p>
Thank you for updating <a class="wiki" href="https://trac-hacks.org/wiki/MenusPlugin">MenusPlugin</a> page.
</p>
<p>
<code>Enabled</code> accepts boolean values, so the following values are available:
</p>
<pre class="wiki">new_item = 1
new_item = true
new_item = enabled
new_item.enabled = 1
</pre><p>
For ini include, check out <a class="ext-link" href="http://trac.edgewall.org/intertrac/wiki%3ATracIni%23GlobalConfiguration" title="wiki:TracIni#GlobalConfiguration in Edgewall's Trac for Trac"><span class="icon">​</span>wiki:TracIni#GlobalConfiguration</a>
</p>
<p>
Closing this ticket.
</p>
<p>
Thank you,
Catalin Balan
</p>
TicketizzyMon, 22 Dec 2008 15:48:14 GMThttps://trac-hacks.org/ticket/4280#comment:11
https://trac-hacks.org/ticket/4280#comment:11
<p>
Thank you, Catalin. One last comment: Since <code>enabled = True</code> 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 <code>force_display</code> suggestion (as you write in <a class="ticket" href="https://trac-hacks.org/ticket/4280#comment:6" title="Comment 6">comment:6</a>), 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 <code>force_display</code> is not really needed). Making its defaults equivalent for trac-defined and user-defined items would make things more clear.
</p>
Ticket