Edgewall Software
Modify ↓

Opened 4 years ago

Closed 4 years ago

#13309 closed defect (fixed)

TicketQuery shows time for time custom field of format date

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.4.2
Component: ticket system Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:
  • Removed time component from query results for custom field of format date.
  • Only the date/datetime is shown in query results, rather than the decorated text on/at %(datetime).
API Changes:
Internal Changes:

Description (last modified by Ryan J Ollos) 

time1 = time
time1.format = date

The datepicker on the ticket form shows only dates.

However, the following ticket query shows 12:00:00 AM. 

[[TicketQuery(id=102, format=table, col=time1)]]

Tested with trunk and 1.4-stable.

Attachments (1)

Screen Shot 2020-06-02 at 00.50.11.jpg​ (12.8 KB ) - added by Ryan J Ollos 4 years ago.

Download all attachments as: .zip

Change History (17)

by Ryan J Ollos, 4 years ago

Attachment: Screen Shot 2020-06-02 at 00.50.11.jpg​ added

comment:1 by Ryan J Ollos, 4 years ago

Description: modified (diff)

comment:2 by Ryan J Ollos, 4 years ago

  • trac/web/chrome.py 

    diff --git a/trac/web/chrome.py b/trac/web/chrome.py
index 9d009c1da..34ef59b70 100644
    a b class Chrome(Component):  
    12571257                    label = absolute
    12581258                elif req.lc_time == 'iso8601':
    12591259                    label = _("at %(iso8601)s", iso8601=absolute)
    1260                 else:
     1260                elif format == 'date':
     1261                    label = _("on %(date)s",
     1262                              date=user_time(req, format_date, date))
     1263                else:  # format == 'datetime'
    12611264                    label = _("on %(date)s at %(time)s",
    12621265                              date=user_time(req, format_date, date),
    12631266                              time=user_time(req, format_time, date))

comment:3 by Ryan J Ollos, 4 years ago

Also, avoid text decoration in query results:

  • trac/ticket/templates/query_results.html 

    diff --git a/trac/ticket/templates/query_results.html b/trac/ticket/templates/query_results.html
index b8d58ecbf..7b4efd253 100644
    a b  
    129129          #       if name == 'summary':
    130130          <a href="${result.href}" title="${_('View ticket')}">${value}</a>
    131131          #       elif header.field.type == 'time':
    132           ${pretty_dateinfo(value, header.field.format)}
     132          ${pretty_dateinfo(value, header.field.format, dateonly=True)}
    133133          #       elif header.field.type == 'checkbox':
    134134          ${_("yes") if value else _("no")}
    135135          #       elif name == 'reporter':

comment:4 by Ryan J Ollos, 4 years ago

Milestone: next-stable-1.4.x → 1.4.2
Owner: set to Ryan J Ollos
Status: new → assigned

comment:5 by Ryan J Ollos, 4 years ago

Release Notes: modified (diff)

comment:6 by Ryan J Ollos, 4 years ago

Proposed changes in [b1da5506a/rjollos.git] (fixed test failure in [2b0c6418b/rjollos.git]).

Once the Python 3 work is integrated I'll try adding more test coverage of pretty_dateinfo and [[TicketQuery(..., format=table, col=time1)]] using ​unittest.mock.patch to replace datetime_now. Setting a fixed date now (rather than using the actual system date) will make it easier to deal with the relative datetimes: in x years, x years ago.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:7 by Jun Omae, 4 years ago

I tried the proposed changes. test_pretty_dateinfo fails when Babel is unavailable. 

FAIL: test_pretty_dateinfo (trac.web.tests.chrome.ChromeTemplateRenderingTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac/web/tests/chrome.py", line 1115, in test_pretty_dateinfo
    </html>"""), content)
AssertionError: <!DOCTYPE html>
<html>
<body>
<ul>
<li></li>
<li><span title="07/01/07 12:34:56">13 years ago</span></li>
<li><span title="07/01/07 12:34:56">13 years ago</span></li>
<li><span title="13 years ago">on 07/01/07</span></li>
<li><span title="13 years ago">on 07/01/07 at 12:34:56</span></li>
<li><span title="13 years ago">07/01/07</span></li>
<li><span title="13 years ago">07/01/07 12:34:56</span></li>
</ul>
</body>
</html>: '<!DOCTYPE html>\n<html>\n<body>\n<ul>\n<li></li>\n<li><span title="Jul 1, 2007, 12:34:56 PM">\\d+ years ago</span></li>\n<li><span title="Jul 1, 2007, 12:34:56 PM">\\d+ years ago</span></li>\n<li><span title="\\d+ years ago">on Jul 1, 2007</span></li>\n<li><span title="\\d+ years ago">on Jul 1, 2007 at 12:34:56 PM</span></li>\n<li><span title="\\d+ years ago">Jul 1, 2007</span></li>\n<li><span title="\\d+ years ago">Jul 1, 2007, 12:34:56 PM</span></li>\n</ul>\n</body>\n</html>' not found in '<!DOCTYPE html>\n<html>\n<body>\n<ul>\n<li></li>\n<li><span title="07/01/07 12:34:56">13 years ago</span></li>\n<li><span title="07/01/07 12:34:56">13 years ago</span></li>\n<li><span title="13 years ago">on 07/01/07</span></li>\n<li><span title="13 years ago">on 07/01/07 at 12:34:56</span></li>\n<li><span title="13 years ago">07/01/07</span></li>\n<li><span title="13 years ago">07/01/07 12:34:56</span></li>\n</ul>\n</body>\n</html>'

in reply to:  2 ; comment:8 by Jun Omae, 4 years ago

Replying to Ryan J Ollos: 

-                else:
+                elif format == 'date':
+                    label = _("on %(date)s",
+                              date=user_time(req, format_date, date))
+                else:  # format == 'datetime'

I consider the changes would be nice when the all users are using same timezone, but not good for a user which is using other timezone.

in reply to:  8 comment:9 by Jun Omae, 4 years ago

Replying to Jun Omae:

Replying to Ryan J Ollos: 

-                else:
+                elif format == 'date':
+                    label = _("on %(date)s",
+                              date=user_time(req, format_date, date))
+                else:  # format == 'datetime'

I consider the changes would be nice when the all users are using same timezone, but not good for a user which is using other timezone.

I missed pretty_dateinfo() in TimelineModule already has format == 'date' at tags/trac-1.2/trac/timeline/web_ui.py@:307-308#L277​.

in reply to:  3 comment:10 by Jun Omae, 4 years ago

Replying to Ryan J Ollos:

Also, avoid text decoration in query results: 

-          ${pretty_dateinfo(value, header.field.format)}
+          ${pretty_dateinfo(value, header.field.format, dateonly=True)}

This issue is introduced in [11331] (1.1.1dev, #1942).

in reply to:  7 comment:11 by Ryan J Ollos, 4 years ago

Replying to Jun Omae:

I tried the proposed changes. test_pretty_dateinfo fails when Babel is unavailable.

Yeah, I noticed and amended comment:6. Still waiting for ​tests on Travis CI to finish but I think the issue is fixed. Thanks for testing.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:12 by Ryan J Ollos, 4 years ago

Thanks for pointing to similar code in TimelineModule. I added a refactoring [ed49cb8b4/rjollos.git].

I don't understand the timezone issue in comment:8. I considered format=date should be rendered similar to format=datetime. Is there another change you are suggesting?

comment:13 by Jun Omae, 4 years ago

Considering the field has 2020-06-10 which is updated by a user with UTC timezone, the field would be shown for each user:

UTC 2020-06-10
UTC +09:00 2020-06-10
UTC -07:00 2020-06-09

I think this behavior is not nice.

How about formatting date only when time part is zero for user's timezone?

  • trac/timeline/web_ui.py 

    diff --git a/trac/timeline/web_ui.py b/trac/timeline/web_ui.py
index 2ef06b465..8b632a0b0 100644
    a b class TimelineModule(Component):  
    276276            def pretty_dateinfo(date, format=None, dateonly=False):
    277277                if not date:
    278278                    return ''
     279                date = to_datetime(date, tzinfo=req.tz)
    279280                if format == 'date':
    280281                    absolute = user_time(req, format_date, date)
    281282                else:
    … … class TimelineModule(Component):  
    303304                        label = absolute
    304305                    elif req.lc_time == 'iso8601':
    305306                        label = _("at %(iso8601)s", iso8601=absolute)
    306                     elif format == 'date':
     307                    elif format == 'date' and date.hour == 0 and \
     308                            date.minute == 0 and date.second == 0 and \
     309                            date.microsecond == 0:
    307310                        label = _("on %(date)s", date=absolute)
    308311                    else:
    309312                        label = _("on %(date)s at %(time)s",

in reply to:  13 comment:14 by Ryan J Ollos, 4 years ago

Replying to Jun Omae:

How about formatting date only when time part is zero for user's timezone?

I've only considered format=date which doesn't have a time component, and format=datetime. Is there a use-case, such as a plugin, for requesting format=date with a time component and expecting to see a formatted datetime? Shouldn't the plugin just request format=datetime?

I agree about the poor behavior showing different dates in different timezones, but isn't it just a fundamental characteristic to format=date?

comment:15 by Ryan J Ollos, 4 years ago

Another consideration regarding proposed comment:13 changes:

They don't seem to have an effect on query results for time fields with format=date, which is primarily what I was trying to address in this ticket. The proposed changes make pretty_dateinfo for Chrome behave like pretty_dateinfo in TimelineModule.

If there are other considerations maybe we could do them in another ticket, assuming I'm correct about proposed changes having no impact on query results for time fields. Since I don't understand the impacts entirely it would be better that I didn't make those changes.

comment:16 by Ryan J Ollos, 4 years ago

Release Notes: modified (diff)
Resolution: → fixed
Status: assigned → closed

Committed to 1.4-stable in r17417, merged to trunk in r17418.

If you want to push additional changes such as comment:13, we can hold on the release, or do in 1.4.3. Just let me know.

Modify Ticket

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

Add Comment
E-mail address and name can be saved in the Preferences .
Attachments ↑ Description ↑
 
Note: See TracTickets for help on using tickets.