Edgewall Software
Modify

Opened 16 years ago

Closed 14 years ago

Last modified 14 years ago

#6369 closed enhancement (fixed)

Allow milestones to be set to a specific time

Reported by: brons_tractrac@… Owned by: Ryan J Ollos <ryano@…>
Priority: normal Milestone: 0.12.1
Component: roadmap Version: devel
Severity: normal Keywords: milestone timezone
Cc: aaron@…, hoff.st@…, ryano@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Right now Trac uses 0:00 GMT for its internal date calculations. This should be changed to 12:00 GMT.

Picture two workers: Sam and Ralph. Sam is in Sao Paulo at GMT-2, Ralph is in Madrid at GMT+1. When Ralph enters a milestone due on 11/14/07, right now Trac displays it to Sam as 11/13/07. They are separated by only 3 hours, yet Trac is telling them it's due on different days!

If Trac used 12:00 GMT, both Sam and Ralph would see 11/14/07, and they would spend less time on email trying to figure out why they never quite agree on what day anything is due.

Attachments (1)

t6369-r10023.patch (2.5 KB ) - added by Ryan J Ollos <ryano@…> 14 years ago.
Patch that adds checkbox for Due field and default due datetime of today at 18:00

Download all attachments as: .zip

Change History (29)

comment:1 by anonymous, 16 years ago

Version: devel

comment:2 by Christian Boos, 16 years ago

Component: generalroadmap
Keywords: timezone added
Milestone: 0.11.1
Owner: changed from Jonas Borgström to Christopher Lenz
Priority: normalhigh

Hm, strange nobody made that proposal before (in particular in the googlegroups:trac-dev:4187c28512541cf0 discussion).

While this won't help for the most exotic time zones (-12 and +12/+13), that change will probably be an improvement for 99.99% of the cases.

in reply to:  2 comment:3 by salzer@…, 16 years ago

Replying to cboos:

Hm, strange nobody made that proposal before (in particular in the googlegroups:trac-dev:4187c28512541cf0 discussion).

While this won't help for the most exotic time zones (-12 and +12/+13), that change will probably be an improvement for 99.99% of the cases.

I'd rather like to have a milestone deadline as date and time (not just a day, as this is the source of this problem), and have this time displayed as the milestone due deadline. Next, I'd like to have a default time configurable, so there is no hardcoding involved, and it is still convenient enough for most people. If possible, configurable via webadmin.

Even if you don't have project participiants (not only developers!) from different timezones, a "milestone is due on tuesday" is pretty vague in an environment involving e.g. a paying customer and a project manager. And it is not always possible to tell the customer a deadline is always meant as 0:00 GMT, or 12:00 GMT, or 8:45 whatever timezone, and keeping a straight face. :-)

Otherwise, if you stick to days, I'd oppose the 12:00 calculation base, since the 0:00 base has the benefit of lesser complexity: A milestone switches to overdue at a time easy to explain (to yourself, if you have to). If it magically switches in the middle of the day, it adds to overall complexity, and nobody knows why the due status changed right now — which will generate tickets here, add to the required documentation and, for the users, the stuff (quirks?) to learn. In addition, a transition to a time-based milestone due calculation would be even harder in future, given you want to provide upgrade paths from several Trac versions.

Regards, /Till/

comment:4 by wangchun <wangchun@…>, 16 years ago

per /Till/ I oppose the 12:00 proposal. It is confusing.I think adding a new due time field would be a more reasonable solution.

comment:5 by Christian Boos, 16 years ago

Ok, so what about specifying date and time (like we do for versions), and we preset the time to, say, 8:00 in the localtime of the user creating the milestone (the time will be stored as GMT, as always).

comment:6 by Christian Boos, 15 years ago

Milestone: 0.11-retriage0.11.5

Move high prio tickets from 0.11-retriage to next maintenance release.

+ I noticed some timezone issue when setting the completed date (the user timezone is not taken into account leading to possible "Can't set completed date to a future date" warning).

in reply to:  6 comment:7 by Christian Boos, 15 years ago

+ I noticed some timezone issue when setting the completed date (the user timezone is not taken into account leading to possible "Can't set completed date to a future date" warning).

This has now been reported separately and in more details as #8240.

in reply to:  5 comment:8 by jevans, 15 years ago

Replying to cboos:

Ok, so what about specifying date and time (like we do for versions), and we preset the time to, say, 8:00 in the localtime of the user creating the milestone (the time will be stored as GMT, as always).

Seems like a good way although a little hard to pick one time over another. We would probably always go with 5pm. Perhaps the original 12noon suggestion would be as good a default as any.

comment:9 by Christian Boos, 15 years ago

Milestone: 0.11.60.12.1
Owner: Christopher Lenz removed
Priority: highnormal

comment:10 by aaron@…, 14 years ago

Keywords: milestone added
Summary: Should use 12:00 for due time instead of 0:00Allow milestones to be set to a specific time
Type: defectenhancement

A great big huge +1 here for setting milestone times, especially since we work with outside developers who have 5:00 PM deadlines.

Right now Trac tells them exactly how many hours they have until a milestone is due (via the Roadmap). This is incredibly misleading! Remember in science class, when they teach you the concept of significant digits? Well, if you're only allowing me to input a milestone time to the nearest day, don't use this data to calculate all the way down to the hour!

I would strongly prefer Trac stick with 12:00:00 AM on milestones that don't specify it, for consistency/migration/simplicity. Besides, the OP's point is somewhat invalid; Ralph's milestone at 2007-11-14 12:00:00 AM GMT +1 really is on the 13th in Sam's timezone! With milestone times, they could agree on a time of day that works for both of them.

comment:11 by aaron@…, 14 years ago

Cc: aaron@… added

comment:12 by Christian Boos, 14 years ago

Well, PatchWelcome, otherwise this has strong chances to not make it for 0.12-stable…

comment:13 by aaron@…, 14 years ago

I'd be happy to if I were a Python developer. :)

That said, good news: it turns out the ability IS there already! It's just significantly less than obvious: you use trac-admin to do this:

milestone due phase2-beta2 2010-08-05T17:00

Which seems to be totally undocumented. Granted, it was just a matter of using a properly formatted ISO 8601 date and time, so it was most certainly intuitive.

But now I've got my 5 PM milestone, and nothing appears to have broken! Which is awesome.

Therefore I'm pretty sure the Web interface just needs to be updated to make the Milestones interface consistent with the Versions interface (dates + times).

in reply to:  13 comment:14 by hasienda <hoff.st@…>, 14 years ago

Replying to aaron@…:

[…] That said, good news: it turns out the ability IS there already! […] But now I've got my 5 PM milestone, and nothing appears to have broken! Which is awesome.

Therefore I'm pretty sure the Web interface just needs to be updated to make the Milestones interface consistent with the Versions interface (dates + times).

No wonder, since this is not dates + times but just a POSIX mircosecond timestamp internally, just as ticket (create)time and ticket changetime. This is the good thing about consistency and modularity.

comment:15 by hasienda <hoff.st@…>, 14 years ago

Cc: hoff.st@… added

comment:16 by Ryan J Ollos <ryano@…>, 14 years ago

Cc: ryano@… added

comment:17 by Ryan J Ollos <ryano@…>, 14 years ago

See also #9582.

comment:18 by Remy Blank, 14 years ago

Milestone: next-minor-0.12.x0.12.1
Owner: set to Remy Blank

As of [10022], the milestone due date is a date + time. What remains to be done is defaulting to 12:00 (or possibly 18:00, the end of the day) in the user's local timezone, which probably requires adding a checkbox like for the "Completed" date.

comment:19 by Ryan J Ollos <ryano@…>, 14 years ago

Just to let you know this is high on my priority list and I should have a first cut at a patch ready by Wednesday or so (and for #9581 as well).

by Ryan J Ollos <ryano@…>, 14 years ago

Attachment: t6369-r10023.patch added

Patch that adds checkbox for Due field and default due datetime of today at 18:00

comment:20 by Ryan J Ollos <ryano@…>, 14 years ago

Here is my first cut at a patch. I set the default duetime to 18:00, but that is easy to change. After your comment:18, I started thinking that this would be a more common choice, and should be the default, but as comments in this ticket show, its probably one of those things that could be debated for all time.

comment:21 by Ryan J Ollos <ryano@…>, 14 years ago

This will be obvious to the devs, but to document the behavior for others in this ticket who may not want to dig into the code, the patch provides the following behavior:

  • If you don't select the due checkbox, you'll get a duedate of None (just as you would previously by not entering anything in the due field).
  • If you only enter a date, you get a duetime of 00:00 on that date (just as you would previously if you only entered a date).
  • If you select the due checkbox, and don't change the default date and time, it will be 18:00 on the current date. Note: it might make more sense to set the due datetime to the next day at 18:00 in the case that 18:00 on the current day in is in the past (I'll refer to rblank for a decision on this).
  • If you select the due checkbox, you are also free to enter any datetime in the format MM/DD/YYYY HH:MM PM, which is consistent with other datetime entry fields (e.g. Version).

comment:22 by Remy Blank, 14 years ago

Slightly corrected patch applied in [10034]:

  • Simplified the default_due calculation, and suggest tomorrow at 18:00 if the current time is already past 18:00.
  • Actually use the checkbox to decide whether to use the due date field or not. The patch worked when JavaScript was enabled, as un-checking the checkbox would disable the due date field and hence its content wouldn't be sent on submit, but would break with JavaScript disabled.

So the only remaining question is, should we add the following to set the time to 18:00 on submit if only a date is given?

  • trac/ticket/roadmap.py

    diff --git a/trac/ticket/roadmap.py b/trac/ticket/roadmap.py
    a b  
    1616# Author: Christopher Lenz <cmlenz@gmx.de>
    1717
    1818from StringIO import StringIO
    19 from datetime import datetime, timedelta
     19from datetime import datetime, time, timedelta
    2020import re
    2121
    2222from genshi.builder import tag
     
    627627
    628628        if 'due' in req.args:
    629629            due = req.args.get('duedate', '')
    630             milestone.due = due and parse_date(due, req.tz, 'datetime') or None
     630            due = due and parse_date(due, req.tz, 'datetime') or None
     631            if due and due.time() == time():
     632                due = due.replace(hour=18, minute=0, second=0, microsecond=0)
     633            milestone.due = due
    631634        else:
    632635            milestone.due = None
    633636

The minor drawback is that a time of 00:00 would also be replaced by 18:00. Fixing this would be quite tricky.

in reply to:  22 comment:23 by Ryan Ollos <ryano@…>, 14 years ago

Replying to rblank:

So the only remaining question is, should we add the following to set the time to 18:00 on submit if only a date is given?

My vote would probably be to set the time to 00:00 if only a date is given for the following reasons (much of my opinions being based on comments earlier in this thread);

  • It preserves existing behavior for setting the milestone due date that Trac users are accustomed to.
  • The behavior of over-writing 00:00 with 18:00 could be confusing even if documented, and is bound to be reported as a defect by some users. I also wonder if pushing a change in behavior such as this might be more appropriate for a major version release.
  • 00:00 is a more intuitive default, particularly for programmers who would expect that when creating a datetime object with a date only.

comment:24 by Christian Boos, 14 years ago

Same feeling here, plus due.time() == time() looking scary…

in reply to:  24 ; comment:25 by Remy Blank, 14 years ago

Resolution: fixed
Status: newclosed

Replying to cboos:

Same feeling here, plus due.time() == time() looking scary…

Why is that? time() is the same as time(0, 0, 0, 0), so this checks if the time part of a datetime is all zeros.

But yeah, I'm not convinced either, so let's leave it at that.

comment:26 by Remy Blank, 14 years ago

Owner: changed from Remy Blank to Ryan J Ollos <ryano@…>

in reply to:  25 comment:27 by Christian Boos, 14 years ago

Replying to rblank:

Replying to cboos:

Same feeling here, plus due.time() == time() looking scary…

Why is that? time() is the same as time(0, 0, 0, 0), so this checks if the time part of a datetime is all zeros.

Ah right, it was datetime.time() not time.time()

comment:28 by aaron@…, 14 years ago

I just wanted to re-affirm my opinion that a default time of 18:00 would cause confusion. 00:00 feels like the right move to me.

(SpamBayes, you're a piece of junk!)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos <ryano@…>.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos <ryano@…> 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.