283139 - Zero out 'hours remaining' field on certain state transitions r.t. throwing an error saying it's not zeroed out.

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement)

Description

[Onno Garms]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624 Netscape/7.1 (ax)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624 Netscape/7.1 (ax)

It should be possible to set a bug to RESOLVED [WONTFIX|LATER|REMIND] without
settings "hours remaining" to zero.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Shane H. W. Travis]

Attachment: Code patch for tip

This patch sets automatically the remaining_time field to 0 whenever a user
commits changes to a bug and the knob-resolve, knob-duplicate, or knob-close
radio buttons are active -- basically, when a bug is RESOLVED in any manner
(including DUPLICATE) or CLOSED. The now-extraneous resolving_remaining_time
error has also been removed.

Rationale: when a bug hits either one of these two states, it has finished that
part of its workflow, and all remaining_time should disappear. The code
currently enforces this, but poorly, and only partially; if you try and commit
a bug with knob-resolve active and time in the remaining_hours field, it
flashes an error message at you and forces you to go back and zero it out
yourself before you can proceed. 

IMHO, that's ridiculous; a user is much, much, MUCH more likely to try and
RESOLVE a bug and forget to set the "Hours Remaining" value 0 than they are to
click the radio button for 'resolve' by accident, then commit, then say, "Phew!
Sure am glad I had hours left in that field! That saved me from resolving the
bug by mistake!" All that is accomplished by the current restriction is to
frustrate the user, and potentially to lose all the user's data (cf: bug 36943)
when they hit the 'Back' button.

This change should definitely be relnoted, and the behaviour should be
described explicitly in the (as-yet-unwritten) documentation for time tracking.

Comment 2

[Shane H. W. Travis]

Previous comment should have said, (cf: bug 36843)

Comment 3

[Onno Garms]

(In reply to comment #1)

> This patch sets automatically the remaining_time field to 0 whenever a user
> commits changes to a bug and the knob-resolve, knob-duplicate, or knob-close
> radio buttons are active -- basically, when a bug is RESOLVED in any manner
> (including DUPLICATE) or CLOSED. The now-extraneous resolving_remaining_time
> error has also been removed.

That is an improvement over the status quo.

> Rationale: when a bug hits either one of these two states, it has finished that
> part of its workflow, and all remaining_time should disappear. 

I'd like it to remain hours on LATER and REMIND, maybe also on WONTFIX.
If you prefer to set to zero, an option in preferences would be great.
For now, a hint how to modify the patch for my intentions, will do.

Sorry, I don't have any idea about the bugzilla implementation.

Comment 4

[Stephen Lee]

Hmmm.... seems to me you completely changed the nature of this bug.

Original request was to be able to have a possibility of bug that was
RESOLVED WONTFIX - XX hours remaining

i.e. AIUI "We don't intend to fix this bug, but if we were to change our minds 
or be persuaded otherwise then our investigations to date suggest it would take 
XX hours to fix".


The fix in the patch is for a different bug (as also referred to in bug 238678 
comment 3).


Furthermore, I can see the possibility that in some instances or usage patterns 
there could be a use for hours remaining when it is RESOLVED FIXED too (or are 
QA always expected to take zero hours to VERIFY a bugfix?). That would 
certainly be another bug though, with ensuing discussion about whether QA time 
should be an entirely separate field etc.

Comment 5

[Onno Garms]

(In reply to comment #4)
> Hmmm.... seems to me you completely changed the nature of this bug.

ACK

> Original request was to be able to have a possibility of bug that was
> RESOLVED WONTFIX - XX hours remaining
> i.e. AIUI "We don't intend to fix this bug, but if we were to change our minds 
> or be persuaded otherwise then our investigations to date suggest it would take 
> XX hours to fix".

or simpler RESOLVED LATER -- XX hours remaining
i.e. we will fix the bug in a later version and that will take XX hours.

Comment 6

[Stephen Lee]

(In reply to comment #5)
> (In reply to comment #4)
> > Hmmm.... seems to me you completely changed the nature of this bug.
> ACK

Incidentally, for avoidance of doubt, the "you" was referring to Shane with his 
patch and change to bug summary - I "midaired" with your comment 3 as I'd had 
the page open while doing other stuff.


> or simpler RESOLVED LATER -- XX hours remaining

Using LATER/REMIND to justify something is unlikely to get far, as these are 
deprecated in favour of the "target milestone" field - see bug 13534.

Comment 7

[Shane H. W. Travis]

(In reply to comment #3)

> I'd like it to remain hours on LATER and REMIND, maybe also on WONTFIX.
LATER and REMIND are deprecated; there is a bug open to get rid of them 
entirely, as something isn't 'RESOLVED' if you are going to work on it later or 
you want a reminder of it. I believe when the ENUMS patch lands, these two will 
not be part of the stock database for Bugzilla any longer.

If you WONTFIX a bug, again, there's no effort required on it.

Regardless, what you're asking for is not possible under the current state, and 
will not be possible under the new state, so there is no change in 
functionality there.

> If you prefer to set to zero, an option in preferences would be great.
I'm trying to avoid (further) parameter bloat.

> For now, a hint how to modify the patch for my intentions, will do.
It's not trivial to do something on one type of RESOLVE but not on another 
(unless the other is DUPLICATE, which has its own action from the editbugs.cgi 
screen).


(In reply to comment #4)
> Hmmm.... seems to me you completely changed the nature of this bug.
I suppose, that's true, yes. The original bug pointed out a flaw in how the 
remaining_hours field, in that it threw an unnecessary and extraneous error in 
the user's face, and forced the user to go back and make an action that could 
just as easily be done automatically. This patch fixes that.

You're correct in that it doesn't give the original author what he's looking 
for, but you've already explained (in Comment #6) why that's unlikely to happen 
(LATER and REMIND deprecated in favour of target milestones). I believe 
strongly that his request is a non-starter, for the reasons listed in this 
comment and by you... but you're correct in that I probably should have opened 
a completely new bug. I felt it was better to do it here because then at least 
the original poster would see the discussion and the rationale behind it. If I 
hear from him that he wants his bug put back the way it was, I will indeed open 
a new bug and move my patch there.

> i.e. AIUI "We don't intend to fix this bug, but if we were to change our 
minds 
> or be persuaded otherwise then our investigations to date suggest it would 
take 
> XX hours to fix".

That is, IMHO, a local customization, and not a trunk feature. WONTFIX is a 
resolution -- i.e. the bug is resolved. If it is resolved, then no further 
developer time need be spent on it. (If one wanted to add time to the 
bug's 'remaining hours' field after resolving it to signify this situation, 
neither the current state of the trunk nor this patch stops you from doing 
that... but for reasons explained below I don't think that's wise.)

> Furthermore, I can see the possibility that in some instances or usage 
patterns 
> there could be a use for hours remaining when it is RESOLVED FIXED too (or 
are 
> QA always expected to take zero hours to VERIFY a bugfix?).

QA takes time, of course, but QA hours-remaining are a completely different 
issue than developer hours-remaining. Nothing stops someone from adding hours 
to the remaining_time field on the bug after resolution; this would be the best 
indication of how much QA time it is expected to take. It also allows for easy 
querying: "select sum(remaining_hours) from bugs where bug_status = 'RESOLVED'" 
shows how much QA time is left. Of course, if you add hours to WONTFIXed bugs, 
then you'll mess this up...

> That would certainly be another bug though, with ensuing discussion
> about whether QA time should be an entirely separate field etc.

No need, IMHO. You can't do QA until a bug is RESOLVED/FIXED, and you can't 
RESOLVE a bug until there is no developer time left needed on it. This workflow 
allows for overloading of the same field with different meanings; as such, I'd 
probably be against an attempt to bring in something like a remaining_qa_hours 
field.

(Heck, with cloning, you could open up an entirely new bug for QA if you wanted 
to, and go through the whole workflow again.)

(In reply to comment #6)
> (In reply to comment #5)
> > or simpler RESOLVED LATER -- XX hours remaining
> Using LATER/REMIND to justify something is unlikely to get far, as these are 
> deprecated in favour of the "target milestone" field - see bug 13534.

This was one reason I 'hijacked' the bug. My apologies to the original poster; 
please make your wishes known re: continuing this here/opening a new bug, and I 
will abide by them.

Comment 8

[Onno Garms]

OK, Shane, you convinced me. I agree with you that instead of my original
request the problem should be fixed as suggested in comment #1.

Comment 9

[Shane H. W. Travis]

Thanks, Onno. Full speed ahead! :)

Also, thanks Stephen for calling me to task on what was, from external 
perception, some very rude behaviour... even though it had honorable 
motivations behind it.

Comment 10

[Onno Garms]

Btw:

In our system, REMIND or LATER is sort of exceptional state for a bug. We use
both a bit different from the inteded use (which indeed is not needed if
milestones are used).

LATER could be used for example if the reporter seems to be no longer intersted
in the bug. For example, if he does not provide requested information. We are
aware that we need to fix the bug if he does (or complains again about the bug),
but currently we won't fix it. This state is independent of the milestone for
which the bug must be fixed.

Comment 11

[Shane H. W. Travis]

Attachment: Code patch for tip, take 2

Changes in this version:
* The remaining_time field is now zeroed out whether or not the person
    resolving/closing the bug is in the timetrackinggroup
* If the user resolving/closing the bug is in the timetrackinggroup, a message
    is given to indicate that the field was changed as side-effect of changing
    the state of the bug.

Comment 12

[Stephen Lee]

right... seeing as I stuck my oar in already, here goes with some more comments!

(In reply to comment #7)
> QA takes time, of course, but QA hours-remaining are a completely different 
> issue than developer hours-remaining. Nothing stops someone from adding hours 
> to the remaining_time field on the bug after resolution;

Indeed... in which case these would be QA hours rather than developer hours, 
and should be re-zeroed if the bug is VERIFIED or REOPENED, right? But then 
what if QA want to reopen and simultaneously allocate more hours for the 
developers (or even vice-versa when resolving...)


It was also occurring to me to think "what was this error message actually 
trying to prevent?", and (although I've not hunted down the bug where it WAS 
added) the logical explanation is that it was trying to prevent someone 
forgetting to update the E/A/R with the hours they'd worked when resolving the 
bug.

Additional Hours Worked = 0  AND  Hours Left != 0 would indicate this.


The patch as it stands just destroys all the remaining time, when in some cases 
a proportion of it should have been converted into hours worked.

After seeing the "remaining_time_zeroed" message, you would then have to go 
back to the bug, update the time fields, and then have bugzilla swear at you 
for not making a comment, etc...

If you resolve a bug and have not updated ANY time tracking info, then I think 
it is quicker for bugzilla to whine at you outright, but it should probably 
only automatically zero the hours remaining when the developer has actually put 
some additional hours worked, otherwise the developer needs to determine 
whether they worked any of these "hours left".


The real solution here is to keep the error but implement Bug 36843 for this 
form, but for a partial fix of that, I wondered whether it could work in a 
similar mechanism to the midair check, and just throw a form at you with the 
timetracking controls on?



(In reply to comment #9)
> Also, thanks Stephen for calling me to task
...

Just seemed odd reading a "here is a patch to do X" in a bug requesting Y with 
no explanation about why X was preferable to Y, or even that the patch author 
realised X was different to Y!
Such explanation has now been extracted! :-)


(In reply to comment #10)
> LATER could be used for example if the reporter seems to be no longer
> intersted in the bug. For example, if he does not provide requested
> information.

You might want to keep an eye on Bug 136107...

Comment 13

[Shane H. W. Travis]

(In reply to comment #12)
> Indeed... in which case these would be QA hours rather than developer hours, 
> and should be re-zeroed if the bug is VERIFIED or REOPENED, right?

Once a bug is VERIFIED, next step is to CLOSE it. time_remaining are, as of 
this patch, automatically zeroed on closure. If you wanted to argue that it 
should be done on VERIFIED in addition to CLOSED, I would definitely listen... 
but for now, ISTM that the latter is 'enough'.

Removing hours on REOPEN would be a logical extension of the above argument. 
I'll give it some serious thought.

> But then what if QA want to reopen and simultaneously allocate more 
> hours for the developers (or even vice-versa when resolving...)

"What if a developer wants to go straight from ASSIGNED to VERIFIED? Why 
shouldn't he be able to?" Answer: Just Because. It's a two-step process, and 
not everything *has* to be a one-step process.


> It was also occurring to me to think "what was this error message actually 
> trying to prevent?", and (although I've not hunted down the bug where it WAS 
> added)

There is no 'bug where it was added'; timetracking was all done as a piece in 
bug 24789, and now that it's in more widespread use with the release of 2.18 
some issues with its implementation are cropping up. This is one of them.

> the logical explanation is that it was trying to prevent someone 
> forgetting to update the E/A/R with the hours they'd worked when resolving 
the 
> bug.

That's your interpretation. Mine is different, and explained above. Jeff 
Hedlund (the original patch author) seems to be inactive w.r.t Bugzilla (or at 
least, it has appeared that way in that he hasn't answered any other 
timetracking questions I've raised) so it's not possible to find out for sure.


> The patch as it stands just destroys all the remaining time...

That is correct. The actual information is not destroyed, however; one can 
always retrieve it from the Bug Activity if it is needed.

> After seeing the "remaining_time_zeroed" message, you would then have to go 
> back to the bug, update the time fields, and then have bugzilla swear at you 
> for not making a comment, etc...

Inability to create a 'commentontimetracking': bug 271913, opened by me. Not an 
easy fix, because this was not implemented in an RDB-friendly way. (The hours 
worked for a given bug should have been stored in a separate table, not as part 
of the longdesc table. Because it was done like that, one MUST enter a comment 
when entering hours since you can't make an entry in longdescs without an 
actual comment.) Another implementation mistake, as far as I'm concerned.

> If you resolve a bug and have not updated ANY time tracking info, then I 
think 
> it is quicker for bugzilla to whine at you outright...

... only then you can run into the scenarios of 'but I added all my time 
tracking info on comment #foo, and asked my boss if he wanted me to go any 
further and he said no so I tried to close it in comment#foo+1 but it won't let 
me because I'm not entering any time tracking info!' 

Whatever is implemented, one can conceive of scenarios that would void it. The 
goal is to have a design philosophy, and a consistent implementation. I don't 
know whether Jeff had the former when he created this error, but I can say 
unequivocally that the latter is not true. Deducing the philosophy, I'm trying 
to *make* everything consistent across the board.

> I wondered whether it could work in a similar mechanism to the midair 
> check, and just throw a form at you with the timetracking controls on?

This would be nice... but definitely in conflict with bug 36843 as far as 
implementation goes... and a full metric boatload more work. :) 

Comment 14

[Frédéric Buclin]

Comment on attachment 175381 [details] [diff] [review]
Code patch for tip, take 2

debug stuff here

Comment 15

[Shane H. W. Travis]

Attachment: Code patch for tip, take 3

Removed debug, fixed most nits pointed out in IRC.

Comment 16

[Stephen Lee]

(In reply to comment #13)
> not everything *has* to be a one-step process.

True, but one-step processes are usually nicer if two steps are not needed...
In this case, two steps *are* needed to avoid confusing Developer hours with QA 
hours remaining.


> That's your interpretation. Mine is different, and explained above.

I personally find it inconceivable that it was *intended* to save you "from 
resolving the bug by mistake", or to "frustrate the user". YMMV.


> > it is quicker for bugzilla to whine at you outright...
> ... only then you can run into the scenarios of 'but I added all my time 
> tracking info on comment #foo, and asked my boss if he wanted me to go any 
> further and he said no so I tried to close it in comment#foo+1 but it won't
> let me because I'm not entering any time tracking info!' 

In this scenario the user should still set the hours remaining to zero.

I had been thinking along the lines of:

if (remaining_time > 0)
  if (work_time == 0)
    ThrowUserError("resolving_remaining_time");
  else
    _remove_remaining_time();

With the resolving_remaining_time error refactored...

... however I can see your point about consistency, and will concede that it is 
better to act in the same way in both instances.


Either way is non-perfect, but since the current method can potentially lead to 
unnecessary and irretrievable data loss until bug 36843 is fixed, your method 
is better in the short/medium term. I'd suggest a further bug be filed to 
restore the error message, made dependent on both this and bug 36843.

Incidentally, you still seem to have this line in the current patch:
+        print "query == $::query";

Comment 17

[Marc Schumann [:Wurblzap]]

Adding my 2 cents' worth...

Isn't it easy to add a zerooutremaingtime param, which is a multi-value-select
list of Bugzilla states, allowing 0 or more states to be selected? Whenever a
bug moves into one of the selected states, the remaining time is zeroed out.
Default setting of that param: "RESOLVED".

Without making it too complicated, this would allow all scenarios proposed here,
wouldn't it?

Comment 18

[Frédéric Buclin]

Please stop adding parameters everywhere; a param here does not seem required.

I haven't time to fully review and test this patch today, but what I have seen
looks good, except the print "" line which has already been mentionned in IRC
yesterday. As 2.18.1 and 2.19.3 releases seem to be delayed a bit, I think it
won't hurt if I review it on Monday.

Comment 19

[Shane H. W. Travis]

(In reply to comment #16)
> (In reply to comment #13)
> > not everything *has* to be a one-step process.
> True, but one-step processes are usually nicer if two steps are not needed...
> In this case, two steps *are* needed to avoid confusing Developer hours with 
QA 
> hours remaining.

... So are you agreeing with me (that the scenario of 'reopening/resolving, and 
then adding hours' should be two steps) or disagreeing? Sorry Stephen, but I 
really can't tell.


> > That's your interpretation. Mine is different, and explained above.
> I personally find it inconceivable that it was *intended* to save you "from 
> resolving the bug by mistake", or to "frustrate the user". YMMV.

You read the wrong interpretation. (not surprising with this much verbiage in 
play. :) 

My belief is that the intent was to stop people from resolving bugs with time 
remaining, because RESOLVED bugs (by definition) require no further effort by 
the developer. The annoying error message is a side-effect; the inconsistent 
behaviour on RESOLVE by a ttg/non-ttg user is consistent with how the code is 
done elsewhere (which, nonetheless, does not make it 'right').

In light of that belief, this patch does what I think should have been done at 
the start; removes the annoying error message, and acts consistently for all 
users. If you don't share that belief, then of course you will also not share 
the analysis of the effectiveness/'good'ness of this patch.

> I'd suggest a further bug be filed to 
> restore the error message, made dependent on both this and bug 36843.

I would still disagree with that, due to my intent-belief above... but we can 
argue that one if/when it comes to it. :)  


> Incidentally, you still seem to have this line in the current patch:
> +        print "query == $::query";

/me mutters... yeah, LpSolit mentioned that to me already. I have a new patch 
that will fix that on checkin. If there's more that needs fixing, I'll add the 
new stuff to the fixed patch, but if not then there's no point putting up YET 
another nearly-identical patch just to remove one line of debug.

Comment 20

[Shane H. W. Travis]

(In reply to comment #17)
> Isn't it easy to add a zerooutremaingtime param
...
> this would allow all scenarios proposed here, wouldn't it?

You misconstrue my intent with this bug/patch. I am perfectly happy that the 
time must be 0 when a bug is resolved. I have absolutely no problem with that 
design decision, and no desire to change it. 

This patch is not about allowing more choices on when the field should or 
should not be set to 0; this patch is about implementing the initial design 
(which is that it *must* be zero) in a more effective way, across all RESOLVED 
scenarios (the original author missed RESO/DUPLICATE), and consistently for all 
users. 

This patch is, by my way of thinking, a *fix*; what you are talking about is -- 
also by my way of thinking -- an *enhancement*. I am not interested in making 
enhancement because I don't need one, nor do I see the need for one (although I 
would not oppose someone else making one, so long as I could continue to get 
this functionality out of it).

This is the underlying point behind most of this friction and conversation, I 
think. I should never have 'hijacked' this bug, because it was, in the 
beginning, about an enhancement to the existing functionality, whereas I have 
no issues with how it currently works, nor do I think that it is 'wrong'. Once 
again, Onno, I apologize.

Comment 21

[Stephen Lee]

(In reply to comment #19)
> ... So are you agreeing with me (that the scenario of 'reopening/resolving,
> and then adding hours' should be two steps)

Yes, zeroing development "hours left", and adding QA "hours left" (or vice-
versa) should be two steps given the current schema and bug form.

Ideologically it would be better if it was possible to unambiguously do this in 
one step, but it does not allow for that. Any enhancement that allowed this 
would not be part of this bug.


> My belief is that the intent was to stop people from resolving bugs with time 
> remaining, because RESOLVED bugs (by definition) require no further effort by 
> the developer.

Agreed.

> The annoying error message is a side-effect;

Also agreed.

However, it is extremely likely that the actions taken to resolve a bug took 
time. IMHO it is reasonably likely that a developer who failed to set 
the "Hours Left" to zero would also have failed to mark any "Hours Worked", 
since one would normally update both fields together.

I would also, as a general rule, consider it preferable for validation errors 
either to be corrected by the user, or to be presented to the user for approval 
when corrected automatically.

As you say though, let's argue about that elsewhere if/when the time comes.


After reading your ttg / non-ttg comments:

+    else {
+        DoComma();
+        $::query .= "remaining_time = 0";
+    }

Should a non-ttg user be allowed to do this? Are there other instances in the 
code where a non-ttg user can (indirectly) modify any time tracking info?

Previous behaviour appeared to be to leave the time tracking info as-is, 
presumably to be fixed up later by a ttg group member.

Comment 22

[Shane H. W. Travis]

> However, it is extremely likely that the actions taken to resolve a bug took 
> time. IMHO it is reasonably likely that a developer who failed to set 
> the "Hours Left" to zero would also have failed to mark any "Hours Worked", 
> since one would normally update both fields together.

I disagree. You want developers to get into the habit of adding hours to 
the 'Hours Worked' field as they progress on a bug. Now, for this one step 
only, they have to add hours *and* zero out another field. Non-routine 
expectations lead to non-routine results.

Anyway, even if we WERE going to demand that work_time be non-zero before 
resolving a bug, we still need to decide:
1) Do they have to enter the work_time at the same time as RESOLVE, or are
    we just looking at the historical record to ensure that some work time
    has been entered on this bug at some point in the past?
2) If we're looking at history, how do we keep track of things like 
    REOPENED->RESOLVED? Do we need to calculate (work_time_at_reopen - 
    work_time_at_resolve)>0? Because that's a lot of SQL, and it depends
    on a table (bugs_activity) with notoriously slow performance...

For the record, I'm not just talking out of my hat here. We use the 
timetracking fields a lot locally; we had a lesser version (with only discrete 
ranges; 0-2 hours, 2-5 hours, etc.) implemented as far back as 2.14, so 
everyone locally is used to entering time, and one of the major reasons we had 
to upgrade to 2.18 was to get this new, better, developer-supported 
functionality. I already have a local patch that won't let people RESOLVE a bug 
without entering some work_time (at the time of resolution; no history 
examined) but I had no confidence that it was something desired on the trunk. 
If you honestly think otherwise, open a bug about it and assign it to me, and 
I'll clean up my patch for general use. (IMHO, something like that *would* 
definitely need a parameter controlling it.)


> I would also, as a general rule, consider it preferable for validation
> errors either to be corrected by the user, or to be presented to the
> user for approval when corrected automatically.

As a general rule, I agree. In this specific case, however, there is exactly 
one correct value that the user can enter -- zero -- so forcing him to go back 
and do that is idiotic.


> Should a non-ttg user be allowed to do this? Are there other instances
> in the code where a non-ttg user can (indirectly) modify any time
> tracking info?

No; everywhere else that tt is modified, it is protected by the ttg 
restriction, so this is a fairly significant departure. Knowing that, I ran it 
past Dave and asked it if was more important to keep the non-ttg users from 
changing tt info, or to keep the actions consistent regardless of who takes the 
action; the answer was that the latter takes priority, because otherwise you 
end up with things in inconsistent states.

Comment 23

[Stephen Lee]

(In reply to comment #22)

[I had written a full reply, but it got lost when windows just crashed :-(]

Essentially, everything you say makes perfect sense for a site such as yours 
which already has a customisation to enforce that there must be 
hours_worked_on_resolve. You seem to disagree that developers should be 
checking/adjusting "Hours Left" as well as "Hours Worked" on each update, and I 
guess this is just dependent on the particular work-flow - I would 
interpret "Hours Left" as "estimated time remaining" which the developer re-
evaluates each time he marks hours worked, in which case setting to zero when 
resolving is just a special case of this. Thinking about it further, I can see 
that it could also be used as a "remaining time budget" (perhaps set by a 
manager), in which case the developer would not normally expect to change it.


The issue is for sites which do not have your customisation, and it is possible 
to get the existing error simply from forgetting to update timetracking fields 
at all (and this patch has the effect of destroying time, some of which should 
have been marked as "worked"). In such cases (as with existing problems such 
as "forgetting to select 'Resolve bug'") it can be fixed by a later bug update, 
and nothing has been permanently lost.

After bug 36843 the risk of "back button dataloss" will have gone, so Bugzilla 
can afford to throw data back at the user for confirmation of automatic changes 
and for relatively trivial data validation problems.


For avoidance of doubt, and as stated earlier:

*** I agree that the current patch is the correct way to proceed right now. ***

Let's discuss what happens afterwards on a different bug.

Comment 24

[Frédéric Buclin]

Comment on attachment 175479 [details] [diff] [review]
Code patch for tip, take 3

>+    else {
>+        DoComma();
>+        $::query .= "remaining_time = 0";
>+        print "query == $::query";
>+    }

Do not forget to *remove* the print "..." line when checking this patch in.
Else, it's good!

r=LpSolit

Comment 25

[Shane H. W. Travis]

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.239; previous revision: 1.238
done
Checking in template/en/default/global/messages.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl
,v  <--  messages.html.tmpl
new revision: 1.24; previous revision: 1.23
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-
error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.97; previous revision: 1.96
done

Comment 26

[Max Kanat-Alexander]

Added to the release notes in bug 286274.

Comment 27

[Frédéric Buclin]

Documentation updated as part of bug 250410.