Ticket #8964 (closed enhancement: wontfix)

Opened 2 years ago

Last modified 11 months ago

Store the date as a timestamp

Reported by: rjollos Assigned to: doki_pen
Priority: normal Component: DateFieldPlugin
Severity: normal Keywords: date time storage value
Cc: ChrisNelson, hasienda, bof, matobaa Trac Release: 0.11

Description

I made the bad assumption that the date was being stored as an integer, as Trac does for Milestone's due and completed fields. A problem can occur if you change the separator or date format after some data has been entered; the database now has different formats for the date in its row. Here is an example:

This also makes it more difficult for other plugins to query and do something useful with the data because they need to get the format from trac.ini. Storing the value as an integer would also make comparisons possible, which would help with implementing features such as #6410.

I'm wondering what led to the design decision to store the field as text. Do you see any downsides to storing it as an integer?

Attachments

TracDbTable.png (86.7 kB) - added by rjollos on 07/08/11 23:27:31.

Change History

07/08/11 23:27:31 changed by rjollos

  • attachment TracDbTable.png added.

07/09/11 06:32:56 changed by rjollos

  • cc set to ChrisNelson, hasienda.

(follow-up: ↓ 3 ) 07/09/11 12:35:26 changed by bof

  • cc changed from ChrisNelson, hasienda to ChrisNelson, hasienda, bof.

I second this suggestion, even though changing the format would have consequences for both values stored in existing setups, and plugins using such fields.

Maybe a separate, second field type timespamp instead of date, would make that more explicit.

And maybe the plugin could provide importable accessor functions for other plugins to use, to access the fields, making the type choice transparent?

(in reply to: ↑ 2 ) 07/09/11 22:25:19 changed by rjollos

Replying to bof:

Maybe a separate, second field type timespamp instead of date, would make that more explicit.

That is an interesting idea. Two other options, not mutually exclusive would be:

  • Use the environment upgrade extensions in Trac to convert the format of existing databases. Like you mentioned, the major problem is breaking the interaction with plugins that expect the format to be a date string, which could be addressed by:
  • Having an option to specify whether the datefield's value is stored in a text string or a numeric timestamp. Your idea of having a second column might be better than this though.

I'm thinking this feature would go into a version 2.0 of the plugin. For my own purposes, I'd like to be able to convert the datafield data in my Trac instance to numeric timestamps, and then deal with the issue of fixing up existing plugins that would be broken by this change. But we must implement this in a way that this doesn't happen to everyone using the plugin.

(follow-up: ↓ 5 ) 07/09/11 22:35:15 changed by ChrisNelson

I'm a little confused why I'm cc'd here. Was this ticket originally about my Gantt? I don't see any indication of the component changing.

(in reply to: ↑ 4 ) 07/09/11 22:40:31 changed by rjollos

Replying to ChrisNelson:

I'm a little confused why I'm cc'd here. Was this ticket originally about my Gantt? I don't see any indication of the component changing.

It's not originally about TracJsGanttPlugin, but since the TracJsGanttPlugin is often used with the DateFieldPlugin and depends on how the DateFieldPlugin stores data, I thought you might have an interest in this.

After this ticket is implemented, we might need a date_format = timestamp option in the TracJsGanttPlugin for the cases in which the DateFieldPlugin stores its data as a timestamp.

(follow-ups: ↓ 7 ↓ 11 ) 08/10/11 16:11:47 changed by anonymous

Is this differerent from Trac core ticket 1942, scheduled for 0.13?

DateFieldPlugin does not store dates in the database at all - it draws a calendar with javascript. This calendar inserts text to a form field; and that's it. The date is stored to and retrieved from the database by other code, depending on the use case (canonical example: ticket custom fields system). By all means, I would to see this implemented, but does it make sense to spend lots of time writing and modifying plugins, when core support is planned for next major release?

(in reply to: ↑ 6 ; follow-up: ↓ 8 ) 08/10/11 16:22:51 changed by ChrisNelson

Replying to anonymous:

... but does it make sense to spend lots of time writing and modifying plugins, when core support is planned for next major release?

I'm on 0.11.6, 0.12 is going to be a lot of work. I may not be on 0.13 until 2013. If it's "lots" of time, maybe not but some accommodation would be nice.

(in reply to: ↑ 7 ) 08/10/11 19:40:41 changed by rjollos

Replying to ChrisNelson:

I'm on 0.11.6, 0.12 is going to be a lot of work. I may not be on 0.13 until 2013. If it's "lots" of time, maybe not but some accommodation would be nice.

Same for me. The problem I've since realized with storing the date as a timestamp is that it will be displayed as a timestamp on the report page when you run a query. However, AFAIK it's not possible to apply a filter against the existing text field for a datefield, so we aren't really losing any functionality by implementing this feature.

I'm definitely interested to know if anyone else comes up with other considerations with regard to this feature.

(follow-up: ↓ 10 ) 01/13/12 00:39:28 changed by debacle@debian.org

An interims solution might be to store the date always in a normalized, sortable form, such as ISO-8601 (yyyy-mm-dd) and only display it differently, if the user wishes so.

(in reply to: ↑ 9 ) 01/13/12 14:13:12 changed by hasienda

  • keywords set to date time storage value.

Replying to debacle@debian.org:

An interims solution might be to store the date always in a normalized, sortable form, such as ISO-8601 (yyyy-mm-dd) and only display it differently, if the user wishes so.

interim, viewing for this plugin, yes.

(in reply to: ↑ 6 ; follow-up: ↓ 12 ) 01/13/12 15:01:42 changed by hasienda

Replying to anonymous:

Is this differerent from Trac core ticket 1942, scheduled for 0.13? DateFieldPlugin does not store dates in the database at all ... The date is stored to and retrieved from the database by other code, depending on the use case ... By all means, I would to see this implemented, but does it make sense to spend lots of time writing and modifying plugins, when core support is planned for next major release?

Interesting question, indeed.

I guess, for existing Trac installations before 0.12 this plugin will remain as a viable option, because true date/time field support is a big deal to patch for a plugin.

OTOH and a bit off-topic, sad to say that interest has been really low to adapt the announced patches for Trac core. I'd welcome more testers, provided that I run my production environments with these patches since more than a year now. Works just great, including query for future time intervals, such as 'tomorrow', 'nextmonth' or '+90d'.

OT, I even rebased the changes by Remy and me on Trac trunk t:r10883 lately, to get the awesome work of Jun, that is configurable time, exactly what is discussed here: Imaging, no hard-coding into strings anymore. (It's still stings, but (microseconds) POSIX time stamps, and converted back and forth on-the-fly). So different users with browsers set to different time zones and different locales see the same date/time in their requested format, correct relative to UTC, respecting time savings settings too. Have it here since 6-Dec-2011, and will certainly never turn back again.

Strongly OT now, the current state is, it's still lacking some unit and functional tests to be finally pushed to review for inclusion. Would be terrible to miss this feature for 0.13 only due to my lack of knowledge about writing such good tests on my side. There's on more pending problem, but I expect to deal with this myself (turning the format parser TracError into a warning and fallback to ticket editor). Anyone up to help this effort to finally come home?

Sorry for abusing this issue a bit, but it's only too related, and I don't understand, why people invest a lot of time into the former, rather hopeless state. Sure upgrading is a bit hard, but who managed the switch from Trac 0.11 to 0.12 and even 0.10 to 0.11, he/she should be fit to jump to 0.13 too.

On-topic again, I'm quite sure, that any plugin dealing with date/time should get ready for 0.13 with true custom date/time ticket field support, really. I.e. WikiCalendarMacro recognizes 'ts' as used extensively in my applications as a powerful alternative to arbitrary other string formats. Tell me, if you need help to get started.

(in reply to: ↑ 11 ) 02/06/12 01:45:31 changed by rjollos

Replying to hasienda:

I guess, for existing Trac installations before 0.12 this plugin will remain as a viable option, because true date/time field support is a big deal to patch for a plugin.

I agree with throwing effort at getting true datetime field support for 0.13. As I've been looking at the source code and working with DateFieldPlugin, I've realized that it is actually not a Date-Field-Plugin at all. Rather, it is a DatePickerPlugin. I'm now of the opinion that DateFieldPlugin's goal should just be to provide a jQuery UI datepicker for Trac text fields. Later, when a true datetime field is supported by Trac (0.13?), we should make sure to support that as well.

I tend to think we should state the DateFieldPlugin's goal on the main wiki page, as that of adding a DatePicker. We should close #4729, #8964 and #6410 as outside the scope of the plugin and direct users to the t:#1942 if they are interested in true datefield support. I think that would support hasienda's goal (and mine too) of driving support for this in the Trac core. If someone ever comes up with a Trac 0.11/0.12 patch for adding a datetime field, that should exist outside of the DateFieldPlugin.

Keeping with the theme of OT ;) I'm considering, wouldn't it be nice to bundle AutocompleteUsersPlugin, DateFieldPlugin and KeywordSuggestPlugin as a single JqueryUiPlugin? The components could still be enabled individually, so no loss to the end user that only wants one. It would sure help with maintainance though, and I'm a (co-)maintainer for all 3, so no issues there. We'd have to make all the licenses compatible - maybe k0s would allow us to switch the AutocompleteUsersPlugin from GPL to BSD; KeywordSuggestPlugin and DateFieldPlugin are both BSD.

AutocompleteUsersPlugin currently conflicts with KeywordSuggestPlugin and probably DateFieldPlugin as well, because it is using an old jQuery plugin (#9599). I think this issue can be solved by upgrading to jQuery UI 1.8.17. My goal is to get all 3 plugins using jQuery UI 1.8.17. This will only be supported in Trac 0.12 or later because of the required jQuery version (>= 1.3.2) for jQuery UI 1.8.17, and Trac 0.11.x shipping with jQuery 1.2.6 (0.12 ships with a 1.4.x version). However, we can piece together a similar solution for a Trac 0.11 branch.

jQuery UI conflicts are a constant problem in my experience, see for example #9752. Putting all 3 of these plugins together and testing them together would make for a more robust solution, and we could provide better end-user support. In the long run, maybe we bundle more jQuery UI components into the plugin too? For example, DuplicateTicketSearchPlugin.

02/06/12 03:32:47 changed by rjollos

Opened #9768 to ask if k0s would be open to changing the AutocompleteUsersPlugin to a BSD license.

02/06/12 22:03:20 changed by rjollos

There were some reasonable arguments by k0s in #9768 against bundling the components together. I think this is something that would need to be presented to the trac community in a mailing list post, but I'd appreciate any feedback I get in this ticket before I take that step. My main goals here are to avoid jQuery UI conflicts and ease my maintenance burden, and maybe there are better ways to accomplish those goals.

04/03/12 22:58:50 changed by rjollos

  • cc changed from ChrisNelson, hasienda, bof to ChrisNelson, hasienda, bof, matobaa.

It looks like the EpochFieldPlugin may be adding the datetime field support that this ticket is requesting. If that is the case, it would be interesting to see how it works with the DateFieldPlugin, and if they don't immediately work well together, perhaps we can add EpochFieldPlugin support to the DateFieldPlugin.

07/09/12 22:09:45 changed by rjollos

  • status changed from new to closed.
  • resolution set to wontfix.

Closing this since date-type support is forthcoming in t:#1942. There has been a bit of activity on that ticket over the past several months, so I'm hopefully we will see the feature added in the next major release after 1.0.

Add/Change #8964 (Store the date as a timestamp)




Change Properties
Action