Ticket Api does not contain 'time' and 'changetime' fields
Ilias Lazaridis
discussion here:
http://trac.edgewall.org/ticket/4019#comment:4
Replying to [comment:4 mgood]:
> I'm -1 on this. The `get_ticket_fields` function is intended for fields that should be
> user-editable.
a) if so, than it should be renamed to "get_editable_ticket_fields"
b) the function is used in _directly_ in context of the query, as
stated above:
http://trac.edgewall.org/browser/trunk/trac/ticket/query.py?rev=3935#L51
> The `Ticket` model class has attributes `time_created` and `time_changed`
> for accessing that information.
The ticket model is not relevant in this context (defect of the query
which '''uses''' get_ticket_fields and the original field names).
>I don't have problem with the query supporting filtering or sorting by the creation
>or changed date/time, but I don't think it requires adding those to `get_ticket_fields`.
get_ticket_fields(all_fields=False) seems to be the most non-intrusive
way to solve this.
-
This patch, together with a few further non-intrusive patches, solves
this task:
http://dev.lazaridis.com/base/wiki/PlanTicketQueryMacro
the result:
this allows e.g. usage of the code below within a wiki page
(latest-changed ticket is displayed on top of the result table, showing
a correct datetime):
#within a wiki page
testing order=changetime&desc=1
[[TicketQuery(status=assigned|new|reopened&reporter=$USER&order=changetime&desc=1,
format=table )]]
#
All patches can be refined during a further rework (e.g. in
collaboration with cboos during introduction of the datetime changes to
the query system).
.
Christian Boos
> ...
> All patches can be refined during a further rework (e.g. in
> collaboration with cboos during introduction of the datetime changes to
> the query system).
>
Those changes are in trunk now
(http://trac.edgewall.org/changeset/4045), you may want to refresh your
patches against them, and I'll take a look
-- Christian
Ilias Lazaridis
ok, very nice!
Ilias Lazaridis
Only one patch has to be changed (the UI one to use http_date).
The summary of patches:
http://trac.edgewall.org/attachment/ticket/4019/TicketApiAddMissingFieldsOptional.diff
http://dev.lazaridis.com/base/attachment/ticket/44/TicketQueryEnableTimeChangetimeOrdering.diff
http://dev.lazaridis.com/base/attachment/ticket/45/QueryDispalyDateTimeHttpDate.diff
http://dev.lazaridis.com/base/attachment/ticket/46/TicketQueryDefaultFieldsOrderOption.diff
-
some test-code for the wiki can be found here:
http://dev.lazaridis.com/base/wiki/PlanTicketQueryMacro#TestCode
.
Matt Good
> Christian Boos wrote:
> > Ilias Lazaridis wrote:
> > > ...
> > > All patches can be refined during a further rework (e.g. in
> > > collaboration with cboos during introduction of the datetime changes to
> > > the query system).
> >
> > Those changes are in trunk now
> > (http://trac.edgewall.org/changeset/4045), you may want to refresh your
> > patches against them, and I'll take a look
>
> Only one patch has to be changed (the UI one to use http_date).
An initial observation on your #45: it overwrites the "cols" list which
has been previously created, which is probably not what you want to do.
I checked the patches to add the "Created" and "Modified" fields, but a
side effect is that "Created" and "Modified" become available in the
"Add filter" list, since it now thinks they're "text" fields of the
ticket. I believe that the "workflow" branch contained at least the
basis for supporting date fields, though I don't know what the status
was, and it looks like it's going to take some work to get that branch
back in sync with the trunk.
This patch will allow the use of "changetime" and "time" fields for
sorting without putting them in "get_ticket_fields", so it avoids
unwanted side effects:
Index: trac/ticket/query.py
===================================================================
--- trac/ticket/query.py (revision 4062)
+++ trac/ticket/query.py (working copy)
@@ -25,7 +25,7 @@
from trac.perm import IPermissionRequestor
from trac.ticket.api import TicketSystem
from trac.ticket.model import Ticket
-from trac.util.datefmt import to_timestamp, utc
+from trac.util.datefmt import to_timestamp, utc, format_datetime
from trac.util.html import escape, html, unescape
from trac.util.text import shorten_line, CRLF
from trac.web import IRequestHandler
@@ -54,7 +54,7 @@
self.fields = TicketSystem(self.env).get_ticket_fields()
self.cols = [] # lazily initialized
- if self.order != 'id' \
+ if self.order not in ('id', 'changetime', 'time') \
and self.order not in [f['name'] for f in
self.fields]:
# order by priority by default
self.order = 'priority'
@@ -370,6 +370,8 @@
cols = self.get_columns()
labels = dict([(f['name'], f['label']) for f in self.fields])
+ labels['changetime'] = 'Modified'
+ labels['time'] = 'Created'
headers = [{
'name': col, 'label': labels.get(col, 'Ticket'),
'href': self.get_href(req, order=col, desc=(col ==
self.order and
@@ -414,6 +416,8 @@
groups.setdefault(group_key, []).append(ticket)
if not groupsequence or groupsequence[-1] !=
group_key:
groupsequence.append(group_key)
+ ticket['time'] = format_datetime(ticket['time'])
+ ticket['changetime'] =
format_datetime(ticket['changetime'])
description = ticket.get('description')
if description:
ticket['description'] = wiki_to_html(description,
self.env,
Christian Boos
> ...
> I checked the patches to add the "Created" and "Modified" fields, but a
> side effect is that "Created" and "Modified" become available in the
> "Add filter" list, since it now thinks they're "text" fields of the
> ticket.
And that's what we should have in the end, but as you said, not without
proper support for date fields.
> I believe that the "workflow" branch contained at least the
> basis for supporting date fields, though I don't know what the status
> was, and it looks like it's going to take some work to get that branch
> back in sync with the trunk.
>
Any news on the alect front?
Would it be ok if I have a try at updating the branch myself or is there
any work pending, in which case I don't want to interfere?
> This patch will allow the use of "changetime" and "time" fields for
> sorting without putting them in "get_ticket_fields", so it avoids
> unwanted side effects:
>
Yes, I like this way better for now.
> ...
> + ticket['time'] = format_datetime(ticket['time'])
> + ticket['changetime'] =
> format_datetime(ticket['changetime'])
>
except for this bit, which I think is better done in the template.
> description = ticket.get('description')
> if description:
> ticket['description'] = wiki_to_html(description,
> self.env,
>
(that will also go in the template, hopefully today if I find the time).
-- Christian
Alec Thomas
> Any news on the alect front?
> Would it be ok if I have a try at updating the branch myself or is there
> any work pending, in which case I don't want to interfere?
I've started the merge, don't touch it :P. I'm also going to refactor
the trac.ticket.field module while I'm at it, as it's not very elegant.
--
Evolution: Taking care of those too stupid to take care of themselves.
Ilias Lazaridis
> Ilias Lazaridis wrote:
> > Christian Boos wrote:
> > > Ilias Lazaridis wrote:
> > > > ...
> > > > All patches can be refined during a further rework (e.g. in
> > > > collaboration with cboos during introduction of the datetime changes to
> > > > the query system).
> > >
> > > Those changes are in trunk now
> > > (http://trac.edgewall.org/changeset/4045), you may want to refresh your
> > > patches against them, and I'll take a look
> >
> > Only one patch has to be changed (the UI one to use http_date).
>
> An initial observation on your #45: it overwrites the "cols" list which
> has been previously created, which is probably not what you want to do.
(I wanted to override the default created cols-list by a user defined
cols-list. Possibly I was to 'brutal'.)
> I checked the patches to add the "Created" and "Modified" fields, but a
> side effect is that "Created" and "Modified" become available in the
> "Add filter" list, since it now thinks they're "text" fields of the
> ticket.
the "get_ticket_fields(get_all=True) should return "all ticket fields"
provided.
Subsystems should then autonomously disable fields they are not able
(or 'willing') to handle.
E.g. the function which fills the "add filter" drop-down-list should
disable the fields which are unsupported. For now, just disabling
'time' and 'datetime' (and enable them later when support is
implemented).
for 'non-breakage' reasons, "get_ticket_fields" returs the UI editable
fields of the ticket.
> I believe that the "workflow" branch contained at least the
> basis for supporting date fields, though I don't know what the status
> was, and it looks like it's going to take some work to get that branch
> back in sync with the trunk.
I assume the "type" there will be "datetime", too.
An additional change could be to introduce immediately the field-type
"datetime":
# DateTime fields
if (all_fields):
fields.append( {'name': 'time', 'type': 'datetime',
'label': 'Created'} )
fields.append( {'name': 'changetime', 'type': 'datetime',
'label': 'Modified'} )
> This patch will allow the use of "changetime" and "time" fields for
> sorting without putting them in "get_ticket_fields", so it avoids
> unwanted side effects:
[...] - (patch)
I've provided a similar patch initially, but it just 'patched' the code
to solve my problem:
http://trac.edgewall.org/attachment/ticket/3990/TicketQueryMacroTimeOrdering.diff
http://trac.edgewall.org/attachment/ticket/3990/TicketQueryMacroAddTimestampFormattingInt.diff
The 2nd patch (get_ticket_fields) is 'more correct' and is a
development patch.
It triggers a few followup issues (e.g. missing model/view separation)
which allow to add further functionality and cleanup the code in _tiny_
incremental steps.
As said in another thread, I am full-time available to continue work on
this and deal with upcoming issues.
-
(Of course, either patch-style 1 or patch-style 2 has the desired
effect (enabling query-ordering on time/datetime), thus both are ok.
But 99%, get_ticket_fields is the way to go.)
.
Ilias Lazaridis
Alec Thomas wrote:
> On Fri, Oct 27, 2006 at 07:40:13AM +0200, Christian Boos wrote:
> > Any news on the alect front?
> > Would it be ok if I have a try at updating the branch myself or is there
> > any work pending, in which case I don't want to interfere?
>
> I've started the merge, don't touch it :P. I'm also going to refactor
> the trac.ticket.field module while I'm at it, as it's not very elegant.
It would be possibly better to just fulfill the main task and to
postpone the refactoring.
Refactoring (changing structure but not functionality) should better
happen isolated and in one chunk (possibly during a refactoring-party
where everyone is invited... thus no one produces new code in that
time-frame).
.
Ilias Lazaridis
Christian Boos wrote:
> Matt Good wrote:
> > ...
> > I checked the patches to add the "Created" and "Modified" fields, but a
> > side effect is that "Created" and "Modified" become available in the
> > "Add filter" list, since it now thinks they're "text" fields of the
> > ticket.
>
> And that's what we should have in the end, but as you said, not without
> proper support for date fields.
thus "add filter" must become aware of it's limitations.
[...]
> > This patch will allow the use of "changetime" and "time" fields for
> > sorting without putting them in "get_ticket_fields", so it avoids
> > unwanted side effects:
>
> Yes, I like this way better for now.
the 'side-effects' are not caused by the patch, but by an inproper use
of the "fields" list within existent code:
http://trac.edgewall.org/browser/trunk/templates/query.html?rev=4045#L117
"fields.iteritems()" should return the _full_ field list.
the filerable fields should be returned by e.g.:
fields.iteritems('for_filter')
fields.iteritemsForFilter()
filterfields.iteritems()
This constuct would correct the behaviour:
<option py:for="field_name, field in <USE_NEW_CONSTRUCT>"
and would additionally provide a flexible way to select relevant fields
out of the full-field-list.
> > ...
> > + ticket['time'] = format_datetime(ticket['time'])
> > + ticket['changetime'] =
> > format_datetime(ticket['changetime'])
[...]
.
Alec Thomas
> It would be possibly better to just fulfill the main task and to
> postpone the refactoring.
>
> Refactoring (changing structure but not functionality) should better
> happen isolated and in one chunk (possibly during a refactoring-party
> where everyone is invited... thus no one produces new code in that
> time-frame).
Thanks for the tip.
Brad Anderson
> On Fri, Oct 27, 2006 at 08:51:15AM -0700, Ilias Lazaridis wrote:
>> It would be possibly better to just fulfill the main task and to
>> postpone the refactoring.
>>
>> Refactoring (changing structure but not functionality) should better
>> happen isolated and in one chunk (possibly during a refactoring-party
>> where everyone is invited... thus no one produces new code in that
>> time-frame).
>
> Thanks for the tip.
>
Ilias Lazaridis
Mr. Anderson, I think the "Thanks for the tip" was sincere.
.
Brad Anderson
I either replied to the wrong message, or lost my faculties. Both are possible.
BA
Ilias Lazaridis
...
> This patch, together with a few further non-intrusive patches, solves
> this task:
>
> http://dev.lazaridis.com/base/wiki/PlanTicketQueryMacro
>
> the result:
>
> this allows e.g. usage of the code below within a wiki page
> (latest-changed ticket is displayed on top of the result table, showing
> a correct datetime):
followup:
CODE - Flexible TicketQuery Macro with extended Query functionality
http://groups.google.com/group/trac-dev/browse_frm/thread/1773e2c2891d9d57
.