Edgewall Software
Modify

Opened 9 years ago

Closed 2 years ago

Last modified 11 months ago

#1942 closed enhancement (fixed)

[patch] Add support for date type in custom ticket fields

Reported by: anonymous Owned by: psuter
Priority: high Milestone: 1.1.1
Component: ticket system Version: 0.8.4
Severity: normal Keywords: datetime jquery datepicker
Cc: jhn@…, felix.schwarz@…, mpotter@…, hoff.st@…, 4glitch@…, osimons, itamarost@…, franz.mayer@…, leho@…, rjollos
Release Notes:

Added support for custom ticket fields of type time.

API Changes:

new TicketFieldList class for efficiently accessing fields by name (r11330).

Description

This supports the rudimentary project managment for the next ticket.

Attachments (9)

EditMilestone.png (12.9 KB) - added by Ryan Ollos <ryano@…> 5 years ago.
ErrorInvalidDate.png (12.5 KB) - added by Ryan Ollos <ryano@…> 5 years ago.
query_future_exp.patch (2.3 KB) - added by shoffmann 4 years ago.
add future expressions to TracQuery
customtimefields.patch (31.9 KB) - added by shoffmann 3 years ago.
rebase cumulative changes from branch "ticket-1942" at r10880 (beware: one chunk)
ct-restrict-owner-fix.patch (2.9 KB) - added by shoffmann 3 years ago.
small fix and configurable datetime formatting and parsing added by user_time
fx-msgids.patch (1.7 KB) - added by shoffmann 3 years ago.
proposal to improve rather bad msgids (similar to #9159)
fx-tkt-test-6879b.patch (663 bytes) - added by shoffmann 3 years ago.
occasionally corrected typo for 2nd regression test related to #6879
ct-tests.patch (2.3 KB) - added by shoffmann 3 years ago.
1st attempt on implementation-specific unit tests
datefmt-err-to-warn.patch (2.5 KB) - added by shoffmann 3 years ago.
degrade TracError on invalid time stamp string to warning, now works for /newticket too

Download all attachments as: .zip

Change History (100)

comment:1 Changed 9 years ago by cmlenz

  • Milestone 0.9 deleted
  • Priority changed from high to normal

Please be more specific.

comment:2 Changed 9 years ago by anonymous

Custom tickets currently have five data types: text, checkbox, select, radio, and text box. I would like a date type: 23-Aug-05, for instance.

comment:3 Changed 9 years ago by eblot

As a side note:

Available data types are currently only "plain old" HTML data types, i.e. the ones you can get with the XHTML input set: text (single-line or multiline), checkbox, radiobox, drop-down list.

Date types are not POD: they requires some combination of POD fields, or some kind of validation (i.e. either Javascript on client side or Python code on server-side); same for "calendar" widgets, heavily based on Javascript code.

comment:4 Changed 9 years ago by anonymous

Well now that starts to sound like serious work. <sigh>

Still would be nice.

comment:5 Changed 9 years ago by cboos

#2832 was closed as duplicate of this one (look at the third comment).

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

Is there any plan to add date support as a custom ticket field? If so what version? Is there a work around?

comment:7 in reply to: ↑ 6 Changed 8 years ago by mgood

  • Milestone set to 0.11

Replying to anonymous:

Is there any plan to add date support as a custom ticket field? If so what version? Is there a work around?

I believe the WorkFlow changes scheduled for 0.11 should make this possible. It will allow for more flexible field types and validation.

comment:8 Changed 8 years ago by eblot

#3822 has been marked as a duplicate

comment:9 Changed 8 years ago by sid

With the addition of jQuery in 0.11, this makes it easier to use something like: http://kelvinluck.com/assets/jquery/datePicker/

As that above link points out though, no validation is performed on the text input when the form is submitted. The trac backend will still need to process the entry to make sure the format is okay.

comment:10 Changed 8 years ago by eblot

#4705 marked as a duplicate

comment:11 Changed 8 years ago by cboos

  • Keywords datetime jquery datepicker added
  • Milestone changed from 0.11 to 0.12

#2151 can be considered as a duplicate as well (i.e. no javascript datepicker without support for date type, and probably support for date type won't be nice without some javascript helpers).

Doesn't seem like it could be done for 0.11, though.

comment:12 Changed 8 years ago by nkantrowitz

This is implemented by the DateField plugin.

comment:13 follow-up: Changed 6 years ago by cboos

Now that #2288 is done, how far are we from this?

comment:14 in reply to: ↑ 13 Changed 6 years ago by rblank

If you mean having a real date type, I would say still pretty far. Custom field data is still stored as a string, and most of the date processing for #2288 is hardcoded for the time and changetime fields. This will really require FieldRefactoring IMO, which I would love to look into, but probably not before 0.13.

comment:15 Changed 5 years ago by jhn@…

  • Cc jhn@… added
  • Summary changed from Add support for date type in custom tickets to Add support for date type in custom ticket fields

adding my vote. there are so many requests for date-related functionality, of various complexity (time tracking, gantt charts, etc). if we could only get dates into and out of trac in a reliable way, users could come up with their own solutions for a lot of stuff, even if it meant exporting reports/queries from trac into excel.

i almost feel like commenting in all those other tickets with a link to this one to build consensus and support for something that could help all of them..

comment:16 Changed 5 years ago by Felix Schwarz <felix.schwarz@…>

  • Cc felix.schwarz@… added

Changed 5 years ago by Ryan Ollos <ryano@…>

comment:17 Changed 5 years ago by Ryan Ollos <ryano@…>

I'd like to see a datepicker implemented for the Date field on the Edit Milestone page. I was going to open a ticket for that feature, but it sounds like it would be a trivial change once this ticket is complete.

Changed 5 years ago by Ryan Ollos <ryano@…>

comment:18 follow-up: Changed 5 years ago by Ryan Ollos <ryano@…>

Actually, on second read through this ticket, I find that I am a little confused. All of the tickets that suggest adding the jQuery datepicker function were closed as duplicates of this ticket. comment:11 says that date type support is necessary for datepicker functionality. However, in the case of the Date field on the Edit Milestone page, there is already some validation done on the date, as shown below.

Would it be possible to add a datepicker for the Due field without having the Date type support discussed in this ticket?

Sorry if this is a bit off-topic … I'd open another ticket to discuss, but history suggests it would be closed as a duplicate of this ticket ;)

comment:19 in reply to: ↑ 18 ; follow-up: Changed 5 years ago by rblank

Replying to Ryan Ollos <ryano@…>:

Would it be possible to add a datepicker for the Due field without having the Date type support discussed in this ticket?

Yes, it's certainly possible, and probably for the "Completed" field, too. I'd find this a nice addition.

comment:20 Changed 5 years ago by rblank

  • Owner changed from jonas to rblank

That would be a good reason to include jQueryUI.

comment:21 in reply to: ↑ 19 Changed 5 years ago by hoff.st@…

Replying to rblank:

Replying to Ryan Ollos <ryano@…>:

Would it be possible to add a datepicker for the Due field without having the Date type support discussed in this ticket?

Yes, it's certainly possible, and probably for the "Completed" field, too. I'd find this a nice addition.

It's already there as a TracHack: CalendarPopUpPlugin. At least worksforme.

comment:22 follow-up: Changed 5 years ago by hoff.st@…

Still working hard for the last few days to provide code, that could be patched into trac trunk for new custom time field support. See TracTicketsCustomTimeFields for details on development status.

Seeking advice where/how to release the code, and how to start collaboration on it.

Even more I'll need help with testing, code review and pushing this to someone both, willing and able, to commit it into SVN. So, if you can't wait another 5 years, please join now.

comment:23 in reply to: ↑ 22 ; follow-up: Changed 5 years ago by Felix Schwarz <felix.schwarz@…>

Replying to hoff.st@…:

Seeking advice where/how to release the code, and how to start collaboration on it.

Maybe it's just me but I did not find any code (also the sourceforge repo seems to be empty). So start out by just importing the trac code in your repo and start working. I usually use Mercurial patch queues for that but it's not necessary. If you have code to show, put it in your repo and ask for feedback on trac-devel.

From an engineering point of view, usually the best way is the following:

  • write code + unit tests (the latter will dramatically improve your chance of being merged)
  • in the end there should not be one big patch but a patch series which can be reviewed and applied one by one.

comment:24 in reply to: ↑ 23 Changed 5 years ago by hoff.st@…

Replying to Felix Schwarz <felix.schwarz@…>:

Replying to hoff.st@…:

Seeking advice where/how to release the code, and how to start collaboration on it.

Maybe it's just me but I did not find any code (also the sourceforge repo seems to be empty). So start out by just importing the trac code in your repo and start working. I usually use Mercurial patch queues for that but it's not necessary. If you have code to show, put it in your repo and ask for feedback on trac-devel.

From an engineering point of view, usually the best way is the following:

  • write code + unit tests (the latter will dramatically improve your chance of being merged)
  • in the end there should not be one big patch but a patch series which can be reviewed and applied one by one.

Thanks for your professional advice, seeking exactly that. The repository is no longer empty, sorry for advertising a bit too early.
I knew SVN before, Mercurial is another new territory. Choose after own research, so nice you mention it too. However, I get the impression, I need to learn about this queues to unleash it's full power.
TracTicketsCustomTimeFields will continue to be the homepage for this development. It seems like messing up this comment trail, if I'd put in here too much details. However, I'm still open to hear from you and others, hopefully encouraging. And unit tests seems to be another unknown but required territory to explore. Any newbie-friendly references? Once I made a little personal page on Trac-Hacks, maybe see you in #trac or via e-mail.

comment:25 Changed 5 years ago by hoff.st@…

Patch files published, summarizing last three weeks work

for known/outstanding issues see TracTicketsCustomTimeFields

comment:26 Changed 5 years ago by Steffen Hoffmann <hoff.st@…>

  • Milestone changed from next-major-0.1X to next-minor-0.12.x
  • Summary changed from Add support for date type in custom ticket fields to [patch] Add support for date type in custom ticket fields

There is no decision on inclusion of this feature into Trac trunk. I just guess it'll be ready to be merged into a 0.12.x release.

comment:27 Changed 5 years ago by hoff.st@…

I announced start of beta-testing this morning in #trac. Your response with your own experience would be very welcome. Have fun and thanks in advance for taking care.

And I'd love to see some more discussion on various of the subjects I put up to the development wiki page for custom time fields support.

comment:28 Changed 5 years ago by hoff.st@…

Look at http://bitbucket.org/hasienda/custom-time/src/tip/series for information on the patches that you can get there. This way you'll even find a preliminary patch for CustomFieldAdmin plugin.

Thanks to osimons for pointing me at that quite clever kind of patch documentation embedded into Mercurial Queues infrastructure.

comment:29 Changed 5 years ago by hoff.st@…

If you like to see what you'll get, you can have a look at one screenshot showing Trac with a ticket list from Trac's custom ticket query based on a new style custom time field now. This was done with Trac 0.12dev-r9478 and patch set from my Mercurial Queue r8:78290d8b26fb applied. If you don't use MQ the file 'series' tells you what patch files to use, why and in which order. Enjoy.

comment:30 Changed 5 years ago by hasienda <hoff.st@…>

  • Milestone changed from next-minor-0.12.x to 0.13

set according to conversation with cboos in #trac today

comment:31 Changed 5 years ago by mpotter@…

  • Cc mpotter@… added

comment:32 Changed 4 years ago by rblank

  • Priority changed from normal to high

comment:33 Changed 4 years ago by rblank

Just a note that I have started working on this. I have rebased your mq patches onto current trunk, and applied them to the branch "ticket-1942" in my work repository. If you have any changes to do, please use that as a base.

comment:34 Changed 4 years ago by rblank

I have done a first round of simplifications (pushed to BB). A few issues still to work on:

  • An invalid value in a time field currently causes a TracError to be displayed. It would be nicer to show a warning.
  • In notifications, the time fields are formatted with the local timezone of the server. This may not be adequate, and we should at least use the default timezone if defined.
  • A few unit tests and functional tests would be nice.

comment:35 follow-up: Changed 4 years ago by cboos

Quick review of 8482b53eb2c0: looks quite good now!

About Ticket._to_db_types in , as it's meant to be used that way:

  values = self._to_db_types(self.values.copy())

i.e. in a functional way, as usual with the "to_xx" functions, it's not intuitive to see it modify the argument in place, and hence have to pass a copy in order to avoid it. Seems better to do the copy in the _to_db_types itself or to have a non-functional _convert_for_db.

For the default timezone in notifications, maybe we could add an Environment.default_timezone helper method which would return the timezone for the [trac] default_timezone value or the localtz if nothing is defined (a bit like RequestDispatcher._get_timezone but not caring about the tz in the session).

Regarding the BitBucket interface, is there a way to do a diff from the base of the branch to its tip so that I could see the changes as whole? If not, that would be a good reason to make a local copy visible here.

comment:36 in reply to: ↑ 35 Changed 4 years ago by rblank

Replying to cboos:

i.e. in a functional way, as usual with the "to_xx" functions, it's not intuitive to see it modify the argument in place, and hence have to pass a copy in order to avoid it. Seems better to do the copy in the _to_db_types itself or to have a non-functional _convert_for_db.

I remember thinking about exactly that, and deciding to do the copy outside of the function, but I can't remember why. I think I expected one call not to need a copy, but it turned out not to be the case. So yes, will fix.

Regarding the BitBucket interface, is there a way to do a diff from the base of the branch to its tip so that I could see the changes as whole? If not, that would be a good reason to make a local copy visible here.

No idea, I'm actually not using BitBucket that much, except as a storage location. So yes, it might be a good idea to set up a clone here.

Thanks for the review!

comment:37 follow-up: Changed 4 years ago by rblank

I have set up a clone of my work repository here (clonable here), but getting a good diff is still pretty tricky. We absolutely need a "branchy" revision log, so that it's easy to find the branch point. Here's the diff for the first round for reference.

comment:38 Changed 4 years ago by cboos

About the following in query_results.html:

FIXME: ugly hack for handling custom time fields before standard ones

What about a .format specifier for the fields of time type?

The 'time' and 'changetime' ones could have the 'age' format (i.e. use dateinfo()), and the custom fields could also have it or alternatively 'datetime' (i.e. use format_datetime()) or even 'date' (use only format_date()).

We'll also need to document the feature in 0.13/TracTicketsCustomFields.

comment:39 in reply to: ↑ 37 Changed 4 years ago by hasienda <hoff.st@…>

  • Cc hoff.st@… added

Replying to rblank:

I have set up a clone of my work repository here (clonable here), but getting a good diff is still pretty tricky. We absolutely need a "branchy" revision log, so that it's easy to find the branch point. Here's the diff for the first round for reference.

Great. You already mentioned your start to work on this on IRC, but I did not follow here, so was unable to react in a timely manner. Since I was finally annoyed by my bug in insert module of model.py I started to rework patches tonight as well. But I guess you'll have fixed this meanwhile on your own. Well, I have look into your code before commenting anything more here.

comment:40 Changed 4 years ago by rblank

Second round done, adds a .format attribute to custom time fields as suggested in comment:38, and uses the default timezone (if defined) for notifications (full diff against trunk).

Remaining issues:

  • Entering an invalid date / time currently shows a TracError with the format hint. This is not too nice, and should probably be replaced by a warning (and returning to the ticket form if the user wanted to save).
  • There are currently no unit nor functional tests for the new functionality.

Steffen, would you like to pick up from here and fix the remaining issues? Also, more testing "by hand" would certainly be welcome.

On a related note, working on this feature shows the limits of our current ticket field implementation, and the strong need for the FieldRefactoring. This is probably the last significant change that the current implementation will manage.

comment:41 Changed 4 years ago by rblank

One other thing to check is that the [[TicketQuery()]] macro works with custom time fields (see comment:4:ticket:9588).

comment:42 Changed 4 years ago by 4glitch@…

  • Cc 4glitch@… added

comment:43 Changed 4 years ago by osimons

  • Cc osimons added

comment:44 Changed 4 years ago by itamaro

  • Cc itamarost@… added

comment:45 Changed 4 years ago by rblank

The latest changes on trunk (in particular, the disruptive DB changes) have been merged into the ticket-1942 branch, so it would be ready for the last batch of issues from comment:40 and comment:41.

Steffen, ready for a last round?

Last edited 4 years ago by rblank (previous) (diff)

comment:46 follow-up: Changed 4 years ago by rblank

mxDateTime support was dropped completely in the current state. That was a bit accidental (I'm always a bit too enthusiastic about removing code), so I should re-add optional support as suggested by cboos on TracTicketsCustomTimeFields.

comment:47 in reply to: ↑ 46 Changed 4 years ago by hasienda <hoff.st@…>

Replying to rblank:

![…] so I should re-add optional support as suggested by cboos on TracTicketsCustomTimeFields.

As I've written to TracTicketsCustomTimeFields «I went for the included mxDatetime.Datetime.ticks() function to get good unix time seconds. So I may still need an own function to calculate unix time microseconds from unix time seconds.»

Now with POSIX microseconds being the new standard resolving this issue will decide, if the option could be done or not.

comment:48 Changed 4 years ago by cboos

Well, I'm not really encouraging you to add a dependency on mxDateTime, be it optional, as I'd rather not have it at all ;-) An optional dependency would be tolerable if the added value is worth it, but I'm not sure you've made it clear enough yet what would be that added value. "Performance" (as in speed) is certainly not a valid reason given this will never have a measurable impact, but maybe you meant robustness regarding imperfect user input… A few examples would make your point clearer. But I'm also not fully convinced by that, as we're likely going to offer the use of the date picker (we should still continue support relative dates though).

comment:49 Changed 4 years ago by rblank

The only part I would use is the date parser. If it is indeed sophisticated enough, it may be worth supporting. I'll test it.

The date picker is nice for date fields. For datetime fields, it should probably set the date in the field, and leave the time empty. This may be tricky to achieve, given the variance in date / time representations…

comment:50 follow-up: Changed 4 years ago by Carsten Klein <carsten.klein@…>

chatting with hasienda today, he pointed me to this ticket, and, after having read through most of the comments i cannot help my self adding some more input :)

supporting different input formats than just 'MM/DD/YY' is definitely a must.

with rblank adding field.format input format specifications to the custom time field plugin configuration, a parser capable of parsing these input formats will definitely be required.

however, i think that this is counter productive, as we definitely need a user preferences based setting for entering/selecting the preferred time/date formats.

so, every time/date/datetime field specified, including also milestone due dates and milestone / version accomplishment/release dates should be based on that time format set in the user preferences, or, if no such format is given, default to the standard input format for time and other calendarial types such as date and datetime, or even duration. and the application should adjust itself to support that notation, within reasonable bounds, of course.

the latter. however, should be based on the common standard, using a notation of for example P1d.

see wikipedia for more information on the xml datatypes, or the w3c specifications thereof.

+1 for getting rid of the standard MM/DD/YY notation from me for that.

comment:51 in reply to: ↑ 50 Changed 4 years ago by Carsten Klein <carsten.klein@…>

Replying to Carsten Klein <carsten.klein@…>:

however, i think that this is counter productive, as we definitely need a user preferences based setting for entering/selecting the preferred time/date formats.

i would go even further and say, that based on the locale setting chosen by the user (moving the locale setting in the preferences tab way to the left), the date/time formats available for that locale should be presented to the user as a fixed list of selectable formats, say, if the user chose en_US as the locale, then, on the date/time tab she would be able to select from the following date, time, and datetime formats, say

Date Format

MM/DD/YY - Short Format MM/DD/YYYY - Long Format YYYY-MM-DD - international format (which would be available for all locales)

Time Format

HH24:MM:SS - international format HH12:MM:SS.AM/PM - "middle ages format"

Datetime Format

MM/DD/YYTHH12:MM:SS - short format MM/DD/YYYYTHH12:MM:SS.AM/PM - long format YYYY-MM-DDTHH24:MM:SS - international format

etc.

How about it?

And, implementing a custom parser suite based upon the fixed input formats given, would be a blaze…

comment:52 Changed 4 years ago by hasienda <hoff.st@…>

Testing Trac build from a repo clone now. I use an older test environment, that has been setup prior to development of the original custom time field patches.

For TicketQuery functionality I've initially created following tickets (and later some more for verification of my findings):

ticketNo due_date value
#46 31.12.2010 23:00:00
#47 01.01.2011 11:00:00

TicketQuery and custom Query right now don't work as expected:

[[TicketQuery(due_date=..02.02.2012)]] (ok) and [[TicketQuery(due_date=..30.12.2009)]] (wrong!) show the tickets, while [[TicketQuery(due_date=01.01.2010..)]] (wrong!) and [[TicketQuery(due_date=01.01.2010..02.02.2012)]] (wrong!) do not. Results for a custom query are identical.

Entering "01.01. 2011 01:15" results in writing "06.11.2010 00:15:00" into ticket_custom db table. Next update, even just adding a comment, changes the string to corresponding POSIX microsecond time stamp, and Query/TicketQuery starts working as expected.

I conclude that we have some issues here:

  • parsing of initial input does respects local timezone setting, but does no string → timestamp conversion, while later ticket changes get converted correctly
  • ? no way to look for empty value yet ?
  • parser is not at all error resistant to common mistakes (additional space here)

While the last has been expected, as I've led discussion about more "intelligent" mxDateTime parser, I still have to research more about the other issues. Interestingly enough flaw pre-existing in my patches has survived in modified form in rblanks version. The different handling of changes coming from /ticket vs. input from /newticket seems to be confusing not only to me.

I'll provide patches against the hg repo soon for further discussion.

comment:53 follow-up: Changed 4 years ago by shoffmann

tested/patched: [43dbc50442fe4c662692297376eb9f5f2278b255/rblank]

  • trac/ticket/model.py

    diff --git a/trac/ticket/model.py b/trac/ticket/model.py
    a b  
    2828from trac.ticket.api import TicketSystem
    2929from trac.util import embedded_numbers, partition
    3030from trac.util.text import empty
    31 from trac.util.datefmt import from_utimestamp, to_utimestamp, utc, utcmax
     31from trac.util.datefmt import from_utimestamp, parse_date, to_utimestamp, \
     32                              utc, utcmax
    3233from trac.util.translation import _
    3334
    3435__all__ = ['Ticket', 'Type', 'Status', 'Resolution', 'Priority', 'Severity',
     
    211212
    212213        # Perform type conversions
    213214        values = dict(self.values)
     215        self.env.log.debug('insert: == type conversions ==')
     216        self.env.log.debug('insert: self.time_fields %s' % str(self.time_fields))
    214217        for field in self.time_fields:
     218            self.env.log.debug('insert: field %s' % field)
    215219            if field in values:
     220                old_ = str(values[field])
     221                if not isinstance(values[field], datetime):
     222                    try:
     223                        values[field] = parse_date(values[field])
     224                        self.env.log.debug(
     225            'insert: convert value %s to %s' % (old_, str(values[field])))
     226                        old_ = str(values[field])
     227                    except:
     228                        self.env.log.debug(
     229            'insert: value %s not parseable' % (old_))
     230                        pass
    216231                values[field] = to_utimestamp(values[field])
     232                self.env.log.debug(
     233            'insert: convert value %s to %s' % (old_, str(values[field])))
    217234
    218235        # Insert ticket record
    219236        std_fields = []
     
    223240            if fname in self.values:
    224241                if f.get('custom'):
    225242                    custom_fields.append(fname)
     243                    self.env.log.debug(
     244            'insert: %s in %s ' % (str(fname), str(values[fname])))
    226245                else:
    227246                    std_fields.append(fname)
    228247        with self.env.db_transaction as db:
     
    237256            if custom_fields:
    238257                db("""INSERT INTO ticket_custom (ticket, name, value)
    239258                      VALUES (%s, %s, %s)
    240                       """, [(tkt_id, c, self[c]) for c in custom_fields])
     259                      """, [(tkt_id, c, values[c]) for c in custom_fields])
    241260
    242261        self.id = tkt_id
    243262        self.resource = self.resource(id=tkt_id)

fixed: custom date/time saved as correct timestamp to db hint: disregard all the debug log statements, done to better see inside open: visual presentation (still integer on read-back from db)

But maybe I'm missing something. Where has the whole date sting-to-long-and-back conversion gone?

Last edited 4 years ago by shoffmann (previous) (diff)

Changed 4 years ago by shoffmann

add future expressions to TracQuery

comment:54 in reply to: ↑ 53 Changed 4 years ago by shoffmann

Replying to shoffmann:

tested/patched: [43dbc50442fe4c662692297376eb9f5f2278b255/rblank] […] But maybe I'm missing something. Where has the whole date sting-to-long-and-back conversion gone?

Obviously, must have done something very wrong, please disregard last comment, sorry for the noise.

Testing same revision goes well this time. As stated in comment:5:ticket:9588 I recognized the need for some future expressions in TracQuery lately.

See first version of a working implementation, that evaluates the following additional expressions:

  • tomorrow
  • next(day|week|month|year)
  • <number> (d|w|m|y) ahead
  • + <number> (d|w|m|y)

I'm still undecided, if I should really use current time limits behavior of last* with next* too (always use start of each period) or better "mirror" on current time (now) an use end of period for that future expressions. Example:

currently:

tomorrow ⇒ next day at 0:00 nextweek ⇒ start of first day of following week

while most of us would find end of day for all future time limits more intuitively right, so maybe better:

tomorrow ⇒ day after tomorrow at 0:00 nextweek ⇒ start of next day after following week

Last edited 4 years ago by shoffmann (previous) (diff)

comment:55 Changed 4 years ago by rblank

Replying to shoffmann:

Obviously, must have done something very wrong, please disregard last comment, sorry for the noise.

Did you mean to disregard all of comment:52 and comment:53, or is one of them still valid?

query_future_exp.patch applied and slightly simplified:

  • Removed the "<number> (d|w|m|y) ahead" form, as the "in|+" form is sufficient.
  • Allow an optional "-" for dates in the past.

About the "next *" form, I would keep the start of the period, as you already implemented.

Here's the full diff against trunk. Remaining issues before we can do a formal review:

  • Change the error on invalid date entry into a warning.
  • Allow queries for unset date fields. In the ticket query, this could be done by leaving both "between / and" fields empty, and in the [[TicketQuery]] macro, by using an empty field= argument.
  • Add a few unit and functional tests.
Last edited 4 years ago by rblank (previous) (diff)

comment:56 Changed 4 years ago by rblank

There's another, more philosophical issue with date fields, as opposed to date/time fields. The latter specify a point in time, so if I set a date/time field to e.g. 2011-01-20 18:00:00 in my time zone, it's quite logical that the time will appear different for a user in a different time zone.

On the other hand, date fields specify a day. If I set a date field to 2011-01-20 in my time zone, it shouldn't appear as the previous or next day for a user in a different time zone, which is what would happen with the current implementation.

So in principle, we should store a timestamp for date/time fields, but a day for date fields. Also, while the former is represented internally by a datetime object, the latter should probably be a date object.

I'm not sure if it's worth the extra complexity, though. Opinions welcome.

comment:57 follow-up: Changed 4 years ago by hvr@…

One could argue, that a datetime like 2011-01-20 18:00:00 describes the second starting at 18:00:00 and ending right before 18:00:01, thus one has the granularity of a second.

When using something with a reduced precision such as 2011-01-20 18:00 one has a granularity of a minute. Following this logic, when saying 2011-01-20 that would comprise a 24h-window (except for DST-changes and other anomalies), which would be no different than a datetime with seconds precision, i.e. subject to timezone transformations.

Just a thought…

comment:58 in reply to: ↑ 57 Changed 4 years ago by rblank

Replying to hvr@…:

Following this logic, when saying 2011-01-20 that would comprise a 24h-window (except for DST-changes and other anomalies), which would be no different than a datetime with seconds precision, i.e. subject to timezone transformations.

Interesting. So when using 2011-01-20 in a time interval in a query, we should use the start of the day if it's in the "from" field, and the end of the day if it's in the "to" field. That would require enhancing the date parser to provide an indication of the precision specified by the user.

Or do the reverse: if it's a "date" field, any tickets whose day overlaps the queried range is included. That wouldn't require "parsing" the precision, and may even be quite simple to implement.

It wouldn't solve the display issue, namely that a date entered in one timezone would appear as a different day in another. But maybe that's not really an issue.

comment:59 Changed 4 years ago by anonymous

  • Owner changed from rblank to anonymous
  • Status changed from new to assigned

comment:60 Changed 4 years ago by osimons

  • Owner changed from anonymous to rblank
  • Status changed from assigned to new

Please don't change ticket data without explanation.

comment:61 Changed 3 years ago by Ryan J Ollos <ryano@…>

  • Cc ryano@… added

Changed 3 years ago by shoffmann

rebase cumulative changes from branch "ticket-1942" at r10880 (beware: one chunk)

Changed 3 years ago by shoffmann

small fix and configurable datetime formatting and parsing added by user_time

Changed 3 years ago by shoffmann

proposal to improve rather bad msgids (similar to #9159)

Changed 3 years ago by shoffmann

occasionally corrected typo for 2nd regression test related to #6879

Changed 3 years ago by shoffmann

1st attempt on implementation-specific unit tests

comment:62 Changed 3 years ago by shoffmann

I've a Trac instance with implementation of custom time fields support almost identical to branch "ticket-1942" in production with a couple of environments for a year now. Moving the db's tainted by my preliminary code wasn't painless, but since then it works without problems.

But having it in trunk would still be a lot better. To catch up with upstream development I've rebased the changes on top of r10880 lately. I've attached corresponding patch files here. This includes

The patch files holding only updated message catalogs are omitted here, and will be contributed later as required, i.e. to #5475. Of course TracTicketsCustomTimeFields will need a major update too, but I'll concentrate on resolving the requirements for review first.

comment:63 Changed 3 years ago by shoffmann

another patch to help with graceful handling (preview) of invalid time stamps on ticket changes, but no changes can be saved as long as such values are present, that fail to be validated

Changed 3 years ago by shoffmann

degrade TracError on invalid time stamp string to warning, now works for /newticket too

comment:64 Changed 3 years ago by framay <franz.mayer@…>

  • Cc franz.mayer@… added

+1, since this would be a nice feature

comment:65 Changed 3 years ago by lkraav <leho@…>

  • Cc leho@… added

comment:66 follow-up: Changed 3 years ago by psuter

From my limited testing the code seemed to work quite well already. One problem was that the format hints did not yet use the user's locale information.

I forked your patches to make it easier for me to read and experiment with them. This ended in a bit of a reorganization of the patches. To get a better overview I collapsed all the really custom-time-field specific bits into one patch. I tried to summarize all the individual steps in the commit message. The not-so custom-time-field specific things are separated out into their own patches.

I thought it might be useful to merge the various bits that render the time fields. I missed that there might be a quite important change from #9777 in [10629] that seems very relevant here. The Chrome.pretty_date_info(...) introduced there should maybe be used here too?

Another remaining problem seems to be with relative age formatting. Both past and future times appear as e.g. 9 hours either way. (Not in 9 hours or 9 hours ago)

Also one of the unit tests fails here.

I'm not sure about this line in datefmt-err-to-warn.patch:

return u'' if 'None' else unicode(value)

Is this not equivalent to return u''?

I also tried to simplify and clarify some other small things. The rest of the code looks ok to me.

comment:67 in reply to: ↑ 66 ; follow-up: Changed 3 years ago by shoffmann

Replying to psuter:

From my limited testing the code seemed to work quite well already. One problem was that the format hints did not yet use the user's locale information.

I forked your patches to make it easier for me to read and experiment with them. This ended in a bit of a reorganization of the patches. To get a better overview I collapsed all the really custom-time-field specific bits into one patch. I tried to summarize all the individual steps in the commit message. The not-so custom-time-field specific things are separated out into their own patches.

Feel free to push your version back, or I could pull. Either way, you're the first to continue with this code, so your opinion will win, if it makes sense.

I thought it might be useful to merge the various bits that render the time fields. I missed that there might be a quite important change from #9777 in [10629] that seems very relevant here. The Chrome.pretty_date_info(...) introduced there should maybe be used here too?

Sure. I switched to user_time, but used pretty_dateinfo as well (complete-usertime.patch, dunno, why this is missing here). Where did you feel I still missed to change something?

Another remaining problem seems to be with relative age formatting. Both past and future times appear as e.g. 9 hours either way. (Not in 9 hours or 9 hours ago)

Ah, the nasty relative ages. Not trivial. Fixing this will significantly extend the patch set, but you're right, that it's relevant.

Also one of the unit tests fails here.

I had even a couple of them failing sometimes, what was just strange to me, but don't remember. Will have to check again.

I'm not sure about this line in datefmt-err-to-warn.patch:

return u'' if 'None' else unicode(value)

Is this not equivalent to return u''?

We want the logically corresponding Unicode string for None, the empty string u''.

>>> val = None
>>> unicode(val)
u'None'
>>> val = 'None'
>>> unicode(val)
u'None'

One may argue, if especially the first case is good Python behavior, but this is out of scope here. Nevertheless, you're right, that the current line doesn't make sense.

return u'' if value == 'None' else unicode(value)

Better?

I also tried to simplify and clarify some other small things. The rest of the code looks ok to me.

Will have a look and pull back, if you agree. Thanks for your effort so far.

comment:68 in reply to: ↑ 67 ; follow-up: Changed 3 years ago by psuter

Replying to shoffmann:

Sure. I switched to user_time, but used pretty_dateinfo as well (complete-usertime.patch, dunno, why this is missing here). Where did you feel I still missed to change something?

Oh, you are right, I missed that. I'm not sure yet if something or what exactly is missing.

return u'' if value == 'None' else unicode(value)

Better?

Much better. :) But I'm afraid I still don't quite get it. Is it really possible for value to be the string 'None' there?

Will have a look and pull back, if you agree.

Sure. Let me know if anything doesn't make sense.

comment:69 in reply to: ↑ 68 Changed 3 years ago by shoffmann

Replying to psuter:

Replying to shoffmann:

Sure. I switched to user_time, but used pretty_dateinfo as well (complete-usertime.patch, dunno, why this is missing here). Where did you feel I still missed to change something?

Oh, you are right, I missed that. I'm not sure yet if something or what exactly is missing.

Ok, just let me know, if you find something.

return u'' if value == 'None' else unicode(value)

Better?

Much better. :) But I'm afraid I still don't quite get it. Is it really possible for value to be the string 'None' there?

_datetime_to_str does such evil. And when taking care for custom field value display, we are already "after" that conversation.

Will have a look and pull back, if you agree.

Sure. Let me know if anything doesn't make sense.

Will do.

comment:70 Changed 3 years ago by rblank

  • Owner changed from rblank to psuter

Thanks Peter for looking into this.

comment:71 follow-up: Changed 3 years ago by jomae

I tried https://bitbucket.org/psuter/trac-1942. The value of custom field has variable widths depended on the timestamp. It's not easy to sort the field for a user.

Time to_utimestamp
0001-01-01T00:00:00Z -62135596800000000L
1901-12-13T20:45:52Z -2147483648000000L (time_t) -(2 ** 31)
1938-04-24T22:15:00Z -999999900000000L
1970-01-01T00:00:00Z 0L Unix epoch
2001-09-09T01:46:39Z 999999999000000L
2038-01-19T03:14:07Z 2147483647000000L (time_t) (2 ** 31 - 1)
9999-12-31T23:59:59Z 253402300799000000L

I think we should pad with 0 for the time custom fields.

  • trac/ticket/model.py

    diff -r cec614bd897c trac/ticket/model.py
    a b  
    6161def _datetime_to_db_str(dt, is_custom_field):
    6262    if not dt:
    6363        return ''
     64    ts = to_utimestamp(dt)
    6465    if is_custom_field:
    65         return str(to_utimestamp(dt))
     66        # Padding with '0' would be easy to sort in report page for a user
     67        fmt = '%018d' if ts >= 0 else '%+017d'
     68        return fmt % ts
    6669    else:
    67         return to_utimestamp(dt)
     70        return ts
    6871
    6972
    7073class Ticket(object):
Last edited 3 years ago by jomae (previous) (diff)

comment:72 Changed 3 years ago by psuter

I updated the patches some more:

  • pretty_dateinfo now differentiates between future and past relative dates ('… ago' and 'in …'), and I now try to pretty print relative dates only through that method. (Which is only available in templates due to the timeline links.)
  • I added a patch enabling date pickers from #10245 for custom date fields on the query and ticket pages.
  • I added one improvement for old dates (pre-1900), although apparently it's impractical to fix these 100%.
  • I also added the 0 padding proposed in comment:71 above. Sorting by date still doesn't seem to work quite right though.

comment:73 in reply to: ↑ 71 Changed 3 years ago by psuter

Or I just missed a date that was still unpadded.

Replying to jomae:

+        fmt = '%018d' if ts >= 0 else '%+017d'

Should that maybe be:

+        fmt = '%018d' if ts >= 0 else '%+018d'

? (Probably not a big difference.)

Tickets created before a date field was introduced have no entry in the ticket_custom table. Tickets created later have such an entry, even if it contains the empty string. These get sorted differently (the former appear "after", the latter "before" tickets with a specified date).

comment:74 Changed 2 years ago by rblank

  • Milestone changed from 1.0 to 1.0-triage

Preparing for 1.0.

comment:75 Changed 2 years ago by cboos

Move feature requests to next-dev.

comment:76 Changed 2 years ago by cboos

  • Milestone changed from next-stable-1.0.x to next-dev-1.1.x

well, once again… next-dev

comment:77 Changed 2 years ago by psuter

  • Milestone changed from next-dev-1.1.x to 1.1.1

As discussed these patches could now get some more exposure on trunk.

comment:78 Changed 2 years ago by rblank

Fine from my side.

comment:79 follow-up: Changed 2 years ago by psuter

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

Applied in [11330-11333].

comment:80 follow-ups: Changed 2 years ago by psuter

  • Resolution fixed deleted
  • Status changed from closed to reopened

Maybe this should remain open for a bit.

  • Documentation: I guess TracTicketsCustomFields gets copied to wiki:1.1.1/TracTicketsCustomFields?(?) and extended with something like this:
    • time: Time / date picker.
      • label: Descriptive label.
      • value: Default time.
      • order: Sort order placement.
      • format: Either relative for relative dates, date for absolute dates or datetime for absolute date and time values. (since 1.1.1)
  • The default time value doesn't work.
  • Time fields don't work with the batch modification UI.
  • Should time fields with relative format (modified and created and custom ones) get a date picker? (On the query page modified and created got one before. Not anymore.)

comment:81 Changed 2 years ago by rblank

Or, considering the length of this ticket, open a new one for follow-up work?

comment:82 Changed 2 years ago by psuter

Yes, that's probably a good idea. For now I created #10853 and #10854.

comment:83 in reply to: ↑ 80 Changed 2 years ago by psuter

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

comment:84 in reply to: ↑ 79 Changed 2 years ago by shoffmann

Replying to psuter:

Applied in [11330-11333].

Thank you very much for finally pushing this into mainline code. And what a great relief, that I'll be able to use Trac without custom patches for my environments, that heavily rely on "true" custom time fields, requiring tedious source changes in ticket code since 2009.

I've just installed trac-r11333 and will follow-up with test results later on. As I do respect the notion to keep this ticket closed, I'll comment to the resume-tickets or create another one and just reference it here.

comment:85 Changed 2 years ago by cboos

  • API Changes modified (diff)
  • Release Notes modified (diff)

comment:86 Changed 22 months ago by Ryan J Ollos <ryan.j.ollos@…>

  • API Changes modified (diff)
  • Release Notes modified (diff)

comment:87 Changed 21 months ago by lkraav <leho@…>

Before I go filing a new ticket, can someone in the know here confirm:

getting an ICS calendar feed based on source data from this date field implementation

is not already on the radar in some other ticket? Ticket search seems to support this claim, just making sure.

comment:88 in reply to: ↑ 80 ; follow-up: Changed 11 months ago by rjollos

Replying to psuter:

  • Should time fields with relative format (modified and created and custom ones) get a date picker? (On the query page modified and created got one before. Not anymore.)

Was there any additional discussion of this item? I tend to think they should get datepickers (related to #10245 and #10756).

I'll create a ticket for 1.1.2 or 1.1.3 if we decide to commit a change.

  • trac/htdocs/js/query.js

    diff --git a/trac/htdocs/js/query.js b/trac/htdocs/js/query.js
    index d147acb..cabd83c 100644
    a b  
    189189          if (property.format == "datetime") {
    190190            focusElement.datetimepicker();
    191191            endElement.datetimepicker();
    192           } else if (property.format == "date") {
     192          } else if (property.format == "date" ||
     193                     property.format == "relative") {
    193194            focusElement.datepicker();
    194195            endElement.datepicker();
    195196          }
     
    332333          focusElement = createText(inputName, 42).addClass("time");
    333334          if (property.format == "datetime") {
    334335            focusElement.datetimepicker();
    335           } else if (property.format == "date") {
     336          } else if (property.format == "date" ||
     337                     property.format == "relative") {
    336338            focusElement.datepicker();
    337339          }
    338340          td.append(focusElement);
  • trac/ticket/templates/query.html

    diff --git a/trac/ticket/templates/query.html b/trac/ticket/templates/query.html
    index 10cce79..2a5b441 100644
    a b  
    124124                        <py:when test="field.type == 'time'"
    125125                                 py:with="(start, end) = '..' in constraint_value and constraint_value.split('..', 1)
    126126                                                         or (constraint_value, '');
    127                                           date_or_datetime = field.get('format', 'datetime')">
     127                                          format = field.get('format', 'datetime');
     128                                          date_or_datetime = 'date' if format == 'relative' else format;">
    128129                          <i18n:msg>
    129130                            <label>between</label>
    130131                            <input type="text" name="${n_field_name}" value="$start" size="14" class="trac-${date_or_datetime}picker" />

Another minor change is to make the field on the batch modification form the same width (4214) as on the query filter form: branches/1.0-stable/trac/htdocs/js/query.js@12332:324#L323 & branches/1.0-stable/trac/htdocs/js/query.js@12332:187,191#L186.

Last edited 11 months ago by rjollos (previous) (diff)

comment:89 Changed 11 months ago by rjollos

  • Cc rjollos added

comment:90 Changed 11 months ago by rjollos

  • Cc ryano@… removed

comment:91 in reply to: ↑ 88 Changed 11 months ago by rjollos

Replying to rjollos:

Replying to psuter:

  • Should time fields with relative format (modified and created and custom ones) get a date picker? (On the query page modified and created got one before. Not anymore.)

Was there any additional discussion of this item? I tend to think they should get datepickers (related to #10245 and #10756).

I'll create a ticket for 1.1.2 or 1.1.3 if we decide to commit a change.

#11436

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain psuter.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from psuter to the specified user.

Add Comment


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

 
Note: See TracTickets for help on using tickets.