Edgewall Software
Modify

Opened 7 years ago

Closed 4 years ago

Last modified 4 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@…
Release Notes:
API 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@…> 4 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 Changed 7 years ago by anonymous

  • Version set to devel

comment:2 follow-up: Changed 7 years ago by cboos

  • Component changed from general to roadmap
  • Keywords timezone added
  • Milestone set to 0.11.1
  • Owner changed from jonas to cmlenz
  • Priority changed from normal to high

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.

comment:3 in reply to: ↑ 2 Changed 7 years ago by salzer@…

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 Changed 7 years ago by wangchun <wangchun@…>

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 follow-up: Changed 7 years ago by 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).

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

  • Milestone changed from 0.11-retriage to 0.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).

comment:7 in reply to: ↑ 6 Changed 5 years ago by cboos

+ 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.

comment:8 in reply to: ↑ 5 Changed 5 years ago by jevans

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 Changed 5 years ago by cboos

  • Milestone changed from 0.11.6 to 0.12.1
  • Owner cmlenz deleted
  • Priority changed from high to normal

comment:10 Changed 4 years ago by aaron@…

  • Keywords milestone added
  • Summary changed from Should use 12:00 for due time instead of 0:00 to Allow milestones to be set to a specific time
  • Type changed from defect to enhancement

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 Changed 4 years ago by aaron@…

  • Cc aaron@… added

comment:12 Changed 4 years ago by cboos

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

comment:13 follow-up: Changed 4 years ago by aaron@…

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).

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

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 Changed 4 years ago by hasienda <hoff.st@…>

  • Cc hoff.st@… added

comment:16 Changed 4 years ago by Ryan J Ollos <ryano@…>

  • Cc ryano@… added

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

See also #9582.

comment:18 Changed 4 years ago by rblank

  • Milestone changed from next-minor-0.12.x to 0.12.1
  • Owner set to rblank

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 Changed 4 years ago by Ryan J Ollos <ryano@…>

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).

Changed 4 years ago by Ryan J Ollos <ryano@…>

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

comment:20 Changed 4 years ago by Ryan J Ollos <ryano@…>

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 Changed 4 years ago by Ryan J Ollos <ryano@…>

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 follow-up: Changed 4 years ago by rblank

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.

comment:23 in reply to: ↑ 22 Changed 4 years ago by Ryan Ollos <ryano@…>

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 follow-up: Changed 4 years ago by cboos

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

comment:25 in reply to: ↑ 24 ; follow-up: Changed 4 years ago by rblank

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

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 Changed 4 years ago by rblank

  • Owner changed from rblank to Ryan J Ollos <ryano@…>

comment:27 in reply to: ↑ 25 Changed 4 years ago by cboos

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 Changed 4 years ago by aaron@…

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!)

Add Comment

Modify Ticket

Change Properties
<Author field>
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.
Author


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

 
Note: See TracTickets for help on using tickets.