Closed
Bug 733586
Opened 12 years ago
Closed 12 years ago
Add cf_last_resolved column for metrics to use to determine when a bug was last closed
Categories
(bugzilla.mozilla.org :: Extensions, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dkl, Assigned: dkl)
References
Details
Attachments
(1 file, 3 obsolete files)
5.73 KB,
patch
|
dkl
:
review+
|
Details | Diff | Splinter Review |
When at Red Hat we wrote an extension that added a new custom date field called 'cf_last_closed' that recorded that timestamp when the bug was last resolved. The show_bug.cgi page would show a readonly value similar to reported time and modified. Using the bug_end_of_update hook, the extension would keep the value updated properly based on status change. Then anyone needing access to the database would be able to query that column to get the date. Otherwise if we abstracted the calculation of this value out using an extension it would only be available for query through the UI or a WebService call. I have a working extension patch for review. Also if it lands, we can use it in the BMO extension instead of the last_closed_date method that is there now. dkl
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #603496 -
Flags: review?(glob)
Comment on attachment 603496 [details] [diff] [review] Patch to add cf_last_closed for use by metrics (v1) Review of attachment 603496 [details] [diff] [review]: ----------------------------------------------------------------- mcoate's request: <mocates> is it correct that we have no "closed" date field for bugs? <LpSolit> mcoates: correct <mcoates> so there is no way to determine a metric on average time to close? <LpSolit> define close. resolved, verified? <mcoates> resolved this is different from "closed" because a closed bug according to the bug_status table is RESOLVED, VERIFIED or CLOSED. this extension and the field it adds should be called "last_resolved" not "last_closed", and only track when the bug was resolved, not when it was verified or closed. ::: extensions/LastClosed/Extension.pm @@ +66,5 @@ > +} > + > +sub _cf_last_closed { > + my ($self) = @_; > + return format_time($self->{'cf_last_closed'}); don't call format_time if cf_last_close is undefined: Use of uninitialized value $date in pattern match (m//) at Bugzilla/Util.pm line 421. at Bugzilla/Util.pm line 421 \tBugzilla::Util::format_time(...) called at ./extensions/LastClosed/Extension.pm line 70 \tBugzilla::Extension::LastClosed::_cf_last_closed(...) called at ./extensions/LastClosed/Extension.pm line 96 @@ +87,5 @@ > + @$args{qw(bug old_bug timestamp changes)}; > + if ($changes->{'bug_status'}) { > + # If the bug has been closed then update the cf_last_closed > + # value to the current timestamp if cf_last_closed exists > + if (!$bug->status->is_open you need to check for RESOLVED only, not is_open()
Attachment #603496 -
Flags: review?(glob) → review-
Assignee | ||
Comment 3•12 years ago
|
||
Thanks for the review and the clarification about RESOLVED. Here is a new patch which should address your suggested changes. dkl
Attachment #603496 -
Attachment is obsolete: true
Attachment #603725 -
Flags: review?(glob)
Summary: Add cf_last_closed column for metrics to use to determine when a bug was last closed → Add cf_last_resolved column for metrics to use to determine when a bug was last closed
Attachment #603725 -
Attachment description: Patch to add cf_last_closed for use by metrics (v2) → Patch to add cf_last_resolved for use by metrics (v2)
Assignee | ||
Comment 4•12 years ago
|
||
New patch that hides field completely from show_bug.cgi but still allows as a column of buglist.cgi Also fixed _cf_last_resolved to return undef. Note to self: Need to schedule outage when this is pushed live so will try to do so with other changes such as globs qa contact migration. dkl
Attachment #603725 -
Attachment is obsolete: true
Attachment #603752 -
Flags: review?(glob)
Attachment #603725 -
Flags: review?(glob)
Comment on attachment 603752 [details] [diff] [review] Patch to add cf_last_resolved for use by metrics (v3) Review of attachment 603752 [details] [diff] [review]: ----------------------------------------------------------------- r=glob review comments can be addressed at commit time. ::: extensions/LastResolved/Extension.pm @@ +20,5 @@ > +our $VERSION = '0.01'; > + > +BEGIN { > + *Bugzilla::Bug::cf_last_resolved = \&_cf_last_resolved; > +} bugzilla already knows now to return values for datatime custom fields, you don't need to monkeypatch this. @@ +88,5 @@ > + if ($changes->{'bug_status'}) { > + # If the bug has been resolved then update the cf_last_resolved > + # value to the current timestamp if cf_last_resolved exists > + if ($bug->bug_status eq 'RESOLVED' > + && Bugzilla::Field->new({ name => 'cf_last_resolved' })) you don't need to check for the field; if this ext is installed it will always be there.
Attachment #603752 -
Flags: review?(glob) → review+
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Byron Jones ‹:glob› from comment #5) > ::: extensions/LastResolved/Extension.pm > @@ +20,5 @@ > > +our $VERSION = '0.01'; > > + > > +BEGIN { > > + *Bugzilla::Bug::cf_last_resolved = \&_cf_last_resolved; > > +} > > bugzilla already knows now to return values for datatime custom fields, you > don't need to monkeypatch this. Originally I did when I was displaying the field on show_bug.cgi as I wanted to put it through format_time(). But now that we're not doing that, I can remove this. dkl
Assignee | ||
Comment 7•12 years ago
|
||
New patch with suggested fixes. Moving r+ forward. We will need to wait til a planned outage for this to be pushed to production as it modifies the DB schema and also requires a migration of values. We may have an outage coming soon to do some other tasks so we can roll it in with that one. dkl
Attachment #603752 -
Attachment is obsolete: true
Attachment #604100 -
Flags: review+
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.0/ added extensions/LastResolved added extensions/LastResolved/Config.pm added extensions/LastResolved/Extension.pm added extensions/LastResolved/template added extensions/LastResolved/template/en added extensions/LastResolved/template/en/default added extensions/LastResolved/template/en/default/hook added extensions/LastResolved/template/en/default/hook/global added extensions/LastResolved/template/en/default/hook/global/field-descs-end.none.tmpl Committed revision 8109.
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•5 years ago
|
Component: Extensions: Other → Extensions
You need to log in
before you can comment on or make changes to this bug.
Description
•