937180 - creation_ts of bugs and attachments always at 0 seconds

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Mark Côté [:mcote]]

Using the attachment API (REST at least), creation_time of attachments always ends in ":00Z", meaning it is rounded down to the exact minute.  From my dev box:

$ curljson 'http://localhost:8080/bmo/rest/bug/823122/attachment'
{"attachments":{},"bugs":{"823122":[{"is_private":0,"creator":"[e-mail]","bug_id":823122,"last_change_time":"2012-12-21T14:35:12Z","size":0,"file_name":"0001-Bug-823122-Added-settime-and-setutime-commands.patch","summary":"Add settime and setutime commands","creation_time":"2012-12-21T14:23:00Z","id":694998,"is_obsolete":0,"flags":[{"modification_date":"2012-12-21T14:35:12Z","creation_date":"2012-12-21T14:23:13Z","type_id":4,"status":"+","name":"review","id":584755,"setter":"[e-mail]"}],"data":"","description":"Add settime and setutime commands","content_type":"text/plain","attacher":"[e-mail]","is_patch":1}]}


mysql> select creation_ts from attachments where bug_id=823122;
+---------------------+
| creation_ts         |
+---------------------+
| 2012-12-21 14:23:13 |
+---------------------+
1 row in set (0.00 sec)

Comment 1

[:glob ✱]

this is due the column definition in the attachment object not returning the full timestamp:

sub DB_COLUMNS {
    my $dbh = Bugzilla->dbh;
    return qw(
        attach_id
        bug_id
        description
        filename
        isobsolete
        ispatch
        isprivate
        mimetype
        modification_time
        submitter_id),
        $dbh->sql_date_format('attachments.creation_ts', '%Y.%m.%d %H:%i') . ' AS creation_ts';
}

Comment 2

[Frédéric Buclin]

That's not a bug. That's intentional. We use %H:%i for attachments, which means hours + minutes only.

Comment 3

[:glob ✱]

this also applies to a bug's creation_ts.

we need to move the formatting of creation_ts for bugs and attachments from the object's column definition to the templates if we want to continue to hide the seconds value.  i think it would be preferable to always display the seconds value, making the creation timestamp for bugs and attachments consistent with all other timestamps in bugzilla.

Comment 4

[:glob ✱]

(In reply to Frédéric Buclin from comment #2)
> That's not a bug. That's intentional. We use %H:%i for attachments, which
> means hours + minutes only.

it's a bug that this formatting happens in the object instead of in the template.

Comment 5

[:glob ✱]

Attachment: 937180_1.patch

Comment 6

[Frédéric Buclin]

Comment on attachment 830278 [details] [diff] [review]
937180_1.patch

No, I'm not going to review this. We said several times that we had no guarantee on the output format used by a DB server to format dates + times, which was the reason why we always specified the output format to be sure we get consistent data. All DB servers are supposed to follow ISO 8601, but that's not the case for e.g. Oracle, which prefers a completely different default output. Bugzilla code alters NLS_DATE_FORMAT and NLS_TIMESTAMP_FORMAT to reformat dates and times for Oracle servers, but I think it's safe to format the output ourselves, always.

Comment 7

[:glob ✱]

(In reply to Frédéric Buclin from comment #6)
> No, I'm not going to review this. We said several times that we had no
> guarantee on the output format used by a DB server to format dates + times,
> which was the reason why we always specified the output format to be sure we
> get consistent data.

that's a different issue from this bug.  there are many datetime fields on objects where we do not currently specify formatting - eg. bug.delta_ts, bug.lastdiffed, attachment.modification_time.  this patch changes creation_ts to match.

Comment 8

[David Lawrence [:dkl]]

Comment on attachment 830278 [details] [diff] [review]
937180_1.patch

Review of attachment 830278 [details] [diff] [review]:
-----------------------------------------------------------------

Can be fixes on commit.

::: template/en/default/attachment/list.html.tmpl
@@ +88,4 @@
>              <br>
>              <a href="#attach_[% attachment.id %]"
>                 title="Go to the comment associated with the attachment">
> +              [%- attachment.attached FILTER time("%Y-%m-%d %H:%M %z") %]</a>,

s/%z/%Z/ to maintain consistency with current format in the UI.

::: template/en/default/bug/edit.html.tmpl
@@ +712,4 @@
>        Reported:
>      </th>
>      <td>
> +     [% bug.creation_ts FILTER time("%Y-%m-%d %H:%M %z") %] by [% INCLUDE global/user.html.tmpl who = bug.reporter %]

Same

@@ +721,4 @@
>        Modified:
>      </th>
>      <td>
> +      [% bug.delta_ts FILTER time("%Y-%m-%d %H:%M %z") %]

Same

Comment 9

[Simon Green]

Changed to Bugzilla 5.0 because it changes what is returned in Bug.get and Bug.search

Comment 10

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Attachment.pm
modified Bugzilla/Bug.pm
modified template/en/default/attachment/list.html.tmpl
modified template/en/default/bug/edit.html.tmpl
Committed revision 8814.