You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bloodhound.apache.org by Apache Bloodhound <bl...@incubator.apache.org> on 2012/10/31 16:27:09 UTC
[Apache Bloodhound] #249: Provide ticket page with breadcrumb
#249: Provide ticket page with breadcrumb
-------------------------+-----------------------
Reporter: gjm | Owner: nobody
Type: enhancement | Status: new
Priority: major | Milestone: Release 3
Component: dashboard | Version:
Keywords: |
-------------------------+-----------------------
Separates out the work on breadcrumbs in #223
attachment:t223_r1394017_ticket_breadcrumbs.diff:ticket:223 is the work on
this so far from olemis.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: review
Priority: major | Milestone: Release 4
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Comment (by gjm):
Replying to [comment:8 gjm]:
> This would then work for any context that has data['ticket'] by calling
this request modifier or creating a new one to combine it with other
request modifiers.
where "data['ticket']" above should have read {{{data['ticket']}}}
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:9>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: accepted
Priority: major | Milestone: Release 3
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Changes (by gjm):
* owner: nobody => gjm
* status: new => accepted
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:1>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: review
Priority: major | Milestone: Release 4
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Comment (by gjm):
r1415116 factors out the relevant code.
I also added the check to make sure the ticket exists and that the
modification is only valid where there is a ticket resource for now. I
expect that to be generalised again and further enhancements in due course
but I suspect this is enough for this ticket.
Thanks for the input olemis.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:11>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: accepted
Priority: major | Milestone: Release 3
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Comment (by gjm):
r1404963 commits a similar patch to the suggested but with a change to
look at resource specific permissions and includes the product/[product
prefix] link.
r1404964 adjusts the permissions checking on product views so that per-
product PRODUCT_VIEW is taken into account - and reflected in the
breadcrumb too.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:3>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: review
Priority: major | Milestone: Release 3
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Comment (by olemis):
I have reviewed changesets related to this ticket . I'd like to suggest
these modifications .
{{{
#!diff
diff -r 91f12ca0f720 bhtheme/templates/bh_path_ticket.html
--- a/bhtheme/templates/bh_path_ticket.html Mon Nov 05 01:25:57 2012
-0500
+++ b/bhtheme/templates/bh_path_ticket.html Mon Nov 05 03:30:40 2012
-0500
@@ -21,21 +21,35 @@
xmlns:py="http://genshi.edgewall.org/"
xmlns:i18n="http://genshi.edgewall.org/i18n"
xmlns:xi="http://www.w3.org/2001/XInclude"
- py:strip="">
+ py:strip="" py:if="ticket.exists"
+ py:with="
+ path_res = dict([realm, Resource(realm, ticket[realm])]
+ for realm in ['milestone', 'product']);
+ path_show = dict([realm, permname in req.perm(path_res[realm])]
+ for realm, permname in [('milestone', 'MILESTONE_VIEW'),
+ ('product', 'PRODUCT_VIEW')])">
<li py:if="bhdb">
<a title="View the dashboard"
href="${href.dashboard()}">Dashboard</a>
<span class="divider">/</span>
</li>
- <li py:if="ticket.product" py:with="prodlink = path_show_product and
href.products(ticket.product) or href.query(product=ticket['product'],
active='!closed')">
- <a title="Active tickets for product ${ticket.product}"
- href="${prodlink}">
+ <li py:if="ticket.product">
+ <a href="${url_of(path_res.product)}" py:if="path_show.product">
+ ${shortname_of(path_res.product)}
+ </a>
+ <a py:if="not path_show.product"
+ href="${href.query(product=ticket['product'], active='!closed')}"
+ title="Active tickets for product ${ticket.product}">
${ticket.product}
</a>
<span class="divider">/</span>
</li>
- <li py:if="ticket.milestone" py:with="mslink = path_show_milestone and
href.milestone(ticket.milestone) or
href.query(milestone=ticket['milestone'], active='!closed')">
- <a title="Active tickets in milestone ${ticket.milestone}"
- href="${mslink}">
+ <li py:if="ticket.milestone">
+ <a href="${url_of(path_res.milestone)}" py:if="path_show.milestone">
+ ${shortname_of(path_res.milestone)}
+ </a>
+ <a py:if="not path_show.milestone"
+ title="Active tickets in milestone ${ticket.milestone}"
+ href="${href.query(milestone=ticket['milestone'],
active='!closed')}">
${ticket.milestone}
</a>
<span class="divider">/</span>
diff -r 91f12ca0f720 bhtheme/theme.py
--- a/bhtheme/theme.py Mon Nov 05 01:25:57 2012 -0500
+++ b/bhtheme/theme.py Mon Nov 05 03:30:40 2012 -0500
@@ -21,7 +21,6 @@
from trac.core import *
from trac.mimeview.api import get_mimetype
-from trac.resource import Resource
from trac.ticket.api import TicketSystem
from trac.ticket.model import Ticket
from trac.ticket.notification import TicketNotifyEmail
@@ -229,13 +228,8 @@
"""Insert Bootstrap scroll spy files.
"""
self._modify_scrollspy(req, template, data, content_type,
is_active)
- if data:
+ if data and data['ticket'].exists:
data['resourcepath_template'] = 'bh_path_ticket.html'
- # determine path permissions
- for resname, permname in [('milestone', 'MILESTONE_VIEW'),
- ('product', 'PRODUCT_VIEW')]:
- res = Resource(resname, data['ticket'][resname])
- data['path_show_' + resname] = permname in req.perm(res)
def _modify_scrollspy(self, req, template, data, content_type,
is_active):
"""Insert Bootstrap scroll spy files.
}}}
I was actually trying to achieve something slightly different but it turn
outs that there's no possible way to render resource description in
context by calling `shortname_of` function . But mainly the goal is to
move permission checks out of source code and include it in the template .
Anyway , if approved it'd be nice to commit this together with
[comment:2:ticket:254 patch] fixing #254 .
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:5>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Olemis Lang <ol...@gmail.com>.
Something similar for product link and product view might be needed .
OTOH , link to ticket view might be useful , but in any case we need
an indicator in there highlighting current page . Maybe <span>Ticket
#111</span> or something like that .
On 10/31/12, Gary Martin <ga...@wandisco.com> wrote:
> Hi,
>
> To get things moving with the breadcrumb, at least on the ticket page, I
> separated the work out from #223 and I adjusted the patch that Olemis
> suggested a little with a very simple check for the MILESTONE_VIEW
> permission. Suggestions for improvements would be great.
>
> Cheers,
> Gary
>
> On 31/10/12 16:20, Apache Bloodhound wrote:
>> #249: Provide ticket page with breadcrumb
>> --------------------------+-----------------------
>> Reporter: gjm | Owner: gjm
>> Type: enhancement | Status: accepted
>> Priority: major | Milestone: Release 3
>> Component: dashboard | Version:
>> Resolution: | Keywords:
>> --------------------------+-----------------------
>>
>> Comment (by gjm):
>>
>> attachment:t249_updated_ticket_breadcrumb.diff updates the patch
>> suggested
>> by olemis with a very basic permission check to decide whether to link
>> to
>> a {{{milestone/[milestone name]}}} view or a query of the milestone
>> depending on the the availability of {{{MILESTONE_VIEW}}}.
>>
>> There is also a small correction for the ticket link in the breadcrumb
>> but
>> I am tempted to remove the link and just leave the text instead.
>>
>> -- Ticket URL:
>> <https://issues.apache.org/bloodhound/ticket/249#comment:2> Apache
>> Bloodhound <https://issues.apache.org/bloodhound/> The Apache
>> Bloodhound (incubating) issue tracker
>
>
--
Regards,
Olemis.
Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/
Featured article:
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Gary Martin <ga...@wandisco.com>.
Hi,
To get things moving with the breadcrumb, at least on the ticket page, I
separated the work out from #223 and I adjusted the patch that Olemis
suggested a little with a very simple check for the MILESTONE_VIEW
permission. Suggestions for improvements would be great.
Cheers,
Gary
On 31/10/12 16:20, Apache Bloodhound wrote:
> #249: Provide ticket page with breadcrumb
> --------------------------+-----------------------
> Reporter: gjm | Owner: gjm
> Type: enhancement | Status: accepted
> Priority: major | Milestone: Release 3
> Component: dashboard | Version:
> Resolution: | Keywords:
> --------------------------+-----------------------
>
> Comment (by gjm):
>
> attachment:t249_updated_ticket_breadcrumb.diff updates the patch suggested
> by olemis with a very basic permission check to decide whether to link to
> a {{{milestone/[milestone name]}}} view or a query of the milestone
> depending on the the availability of {{{MILESTONE_VIEW}}}.
>
> There is also a small correction for the ticket link in the breadcrumb but
> I am tempted to remove the link and just leave the text instead.
>
> -- Ticket URL:
> <https://issues.apache.org/bloodhound/ticket/249#comment:2> Apache
> Bloodhound <https://issues.apache.org/bloodhound/> The Apache
> Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: accepted
Priority: major | Milestone: Release 3
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Comment (by gjm):
attachment:t249_updated_ticket_breadcrumb.diff updates the patch suggested
by olemis with a very basic permission check to decide whether to link to
a {{{milestone/[milestone name]}}} view or a query of the milestone
depending on the the availability of {{{MILESTONE_VIEW}}}.
There is also a small correction for the ticket link in the breadcrumb but
I am tempted to remove the link and just leave the text instead.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:2>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: review
Priority: major | Milestone: Release 4
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Comment (by rjollos):
Replying to [comment:5 olemis]:
> [..] But mainly the goal is to move permission checks out of source code
and include it in the template .
What are the motivations / advantages to performing the permissions checks
in the template rather than the source code? Are there some guidelines
that are generally followed in the Trac / Genshi community that motivated
your suggested changes?
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:6>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: review
Priority: major | Milestone: Release 3
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Changes (by gjm):
* status: accepted => review
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:4>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: closed
Priority: major | Milestone: Release 4
Component: dashboard | Version:
Resolution: fixed | Keywords:
--------------------------+-----------------------
Changes (by gjm):
* status: review => closed
* resolution: => fixed
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:12>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: review
Priority: major | Milestone: Release 4
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Comment (by gjm):
Replying to [comment:7 olemis]:
> Replying to [comment:6 rjollos]:
> > Replying to [comment:5 olemis]:
> > > [..] But mainly the goal is to move permission checks out of source
code and include it in the template .
> >
> > What are the motivations / advantages to performing the permissions
checks in the template rather than the source code?
>
> Well in this case such change will allow us to reuse the same template
(i.e. bhtheme/templates/bh_path_ticket.html ) in another context if
necessary without the need for adding the same data in a different (...
completely unrelated ...) request handler .
I am not sure that I entirely follow that logic as we are already
effectively modifying the request to do the template swapping. For
generalisation we could separate out the relevant code to give a
{{{_modify_ticket_breadcrumb}}} request modifier method that is called by
the {{{_modify_ticket}}} method in the same way that it calls the
{{{_modify_scrollspy}}} method.
This would then work for any context that has data['ticket'] by calling
this request modifier or creating a new one to combine it with other
request modifiers.
Of course, we are going to have to work a fair bit harder to generalise
for milestone and version resources, unless we put that off until
multiproduct solves these issues by indicating ownership of these
resources.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:8>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: review
Priority: major | Milestone: Release 4
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Comment (by olemis):
Replying to [comment:8 gjm]:
> Replying to [comment:7 olemis]:
> > Replying to [comment:6 rjollos]:
> > > Replying to [comment:5 olemis]:
> > > > [..] But mainly the goal is to move permission checks out of
source code and include it in the template .
> > >
> > > What are the motivations / advantages to performing the permissions
checks in the template rather than the source code?
> >
> > Well in this case such change will allow us to reuse the same template
(i.e. bhtheme/templates/bh_path_ticket.html ) in another context if
necessary without the need for adding the same data in a different (...
completely unrelated ...) request handler .
>
> I am not sure that I entirely follow that logic as we are already
effectively modifying the request to do the template swapping. For
generalisation we could separate out the relevant code to give a
{{{_modify_ticket_breadcrumb}}} request modifier method that is called by
the {{{_modify_ticket}}} method in the same way that it calls the
{{{_modify_scrollspy}}} method.
>
> This would then work for any context that has data['ticket'] by calling
this request modifier or creating a new one to combine it with other
request modifiers.
>
looks interesting ... even if I still prefer to check for such permissions
in the template , this is definitely better than before .
[...]
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:10>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #249: Provide ticket page with breadcrumb
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#249: Provide ticket page with breadcrumb
--------------------------+-----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: review
Priority: major | Milestone: Release 4
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------------
Comment (by olemis):
Replying to [comment:6 rjollos]:
> Replying to [comment:5 olemis]:
> > [..] But mainly the goal is to move permission checks out of source
code and include it in the template .
>
> What are the motivations / advantages to performing the permissions
checks in the template rather than the source code?
Well in this case such change will allow us to reuse the same template
(i.e. bhtheme/templates/bh_path_ticket.html ) in another context if
necessary without the need for adding the same data in a different (...
completely unrelated ...) request handler .
That's something happening quite often in ''Trac'' , and is one of the
reasons that make widgets so useful (i.e. ubiquitous access to view + data
for web UI snippets ;)
> Are there some guidelines that are generally followed in the Trac /
Genshi community that motivated your suggested changes?
Some minutes of reasoning ?
;)
Beyond that I'm not aware of any such guidelines ... and if they exist
I've never used any , but I'd like to know of them . D'u have any
reference ?
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/249#comment:7>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker