Closed
Bug 153578
Opened 22 years ago
Closed 22 years ago
Attachment modified date is meant to be attachment creation date
Categories
(Bugzilla :: Creating/Changing Bugs, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: MatsPalmgren_bugz, Assigned: bbaetz)
References
()
Details
(Whiteboard: [schema])
Attachments
(1 file, 2 obsolete files)
5.88 KB,
patch
|
preed
:
review+
bugreport
:
review+
|
Details | Diff | Splinter Review |
Attachment 'Modified' date does not match 'View Activity' date for change. In the given URL, I added an attchment that I later obsoleted. In the 'Modified' column it says: 2002-06-22 05:56 In the 'View Bug Activity' list it says: 2002-06-22 05:58:36 The comment that was created when I first attached it says: 2002-06-22 05:56 My guess is that 'Modified' really is 'Created' ?
Assignee | ||
Comment 1•22 years ago
|
||
Its the created time in the source. Except that setting is_obsolete when creating a new bug will update it. Oops If its meant to be the creation time, we should fix that by having the field not be a timestamp, and using now() on the INSERT. (We should also retroactivly fix this my searching for the initial 'created an attachment' comment for all obsolete attachments) If its meant to be the modification time, then we should remove the explicit creation_ts=creation_ts from attachment.cgi I'm not sure which makes more sense to me. Creation is probably what we want, though, I think. Its definately what was originally intended, and theres alwasy teh activity log if someone wants more. myk?
Comment 2•22 years ago
|
||
I think the value should be the creation time, and the obsoletion code is buggy, which is easy for timestamps because they update automatically unless you explicitly tell them not to. We should fix this not only by fixing the obsoletion code and the corrupt data but also by making that field not be automatically updated (i.e. make it a different field type like a datetime field).
Assignee | ||
Comment 3•22 years ago
|
||
Yeah, it should be a datetime type. OK.
Assignee | ||
Comment 4•22 years ago
|
||
I have a patch for this.
Assignee | ||
Comment 5•22 years ago
|
||
This recreates all of the creation times. I don't think I need a progress meter thingy, although I would like to know how long this does take on a large db. Since the now() for the comment time and the now() for the attachment time may be different with mysql, I guess this could change stuff by a second or two. I don't care about that, though - the number isn't meant to be that exact, anyway. I tested this on landfill, and no numbers were changed. I removed the formatting stuff since mysql's datetime uses that format anyway. Once we support multiple dbs, we'll have to pull that code out somewhere and use Date::Format on it, or something.
Comment 6•22 years ago
|
||
Comment on attachment 94920 [details] [diff] [review] v1 Looks good; r=preed, with the following nits: >Index: checksetup.pl >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v >+ # Restrict this as much as possible in order to avoid false positives, and >+ # keep the db search time down >+ my $sth2 = $dbh->prepare("SELECT bug_when, thetext FROM longdescs WHERE bug_id=? AND who=? AND thetext LIKE ? ORDER BY bug_when LIMIT 1"); Why do you select thetext? I don't see you using it anywhere below. >+ while (my ($bug_id, $attach_id, $submitter_id) = $sth->fetchrow_array()) { >+ $sth2->execute($bug_id, $submitter_id, "Created an attachment (id=$attach_id)%"); >+ my ($when, $text) = $sth2->fetchrow_array(); >+ if ($when) { >+ $dbh->do("UPDATE attachments SET creation_ts='$when' WHERE attach_id=$attach_id"); As you note in your comment, I think the status meter thing is relatively important, especially on larger Bugzillas. To make it simpler, maybe you could just put a print statement in here like "attachment $attach_id updated" or something, maybe? Granted, that'd be a lot of text and it might obscure the warning... but I still think it's important. If we can find a simple way to do that, it would be nice, but if not... >+ } else { >+ print "Warning - could not determine correct creation time for attachment $attach_id on bug $bug_id\n"; >+ } >+ }
Attachment #94920 -
Flags: review+
Assignee | ||
Comment 7•22 years ago
|
||
Getting thetext was a side effect from when I handled the seaching in perl. I'll print updates every 1000, but I don't kno wif thats too much/too little/whatever.
Attachment #94920 -
Attachment is obsolete: true
Comment 8•22 years ago
|
||
Comment on attachment 95063 [details] [diff] [review] v2 - add counter My r= will carry forward, but considering that v2 is the exact same patch as v1, I'm going to needs-work v2. :-) I'd say every 500... but you're right... it's a hard call to make. Bleh... whatever you think works...
Attachment #95063 -
Flags: review-
Comment 10•22 years ago
|
||
Comment on attachment 95243 [details] [diff] [review] v2.1 Much better! :-) r=preed
Attachment #95243 -
Flags: review+
Comment 11•22 years ago
|
||
Comment on attachment 95243 [details] [diff] [review] v2.1 2xr from joel
Attachment #95243 -
Flags: review+
Assignee | ||
Comment 12•22 years ago
|
||
Fixed, and changing summary to match what ended up happening.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Summary: Attachment 'Modified' date does not match 'View Activity' date for change → Attachment modified date is meant to be attachment creation date
Updated•11 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•