252789 - Empty timetrackinggroup causes error "hours worked needs to be positive" when changing bug.

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect, P2)

Description

[Jason Pyeron]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0; .NET CLR 1.0.3705; .NET CLR 1.1.4322)
Build Identifier: 

I just update to the tip
timetracking group is empty
new bug may be created but modifications result in a positive value required 
'hours worked' must be positive error.

Reproducible: Always
Steps to Reproduce:

Comment 1

[Jason Pyeron]

updating timetrackinggroup to editbugs resolved this issue


should sanitycheck or checksetup check/fix this?

Comment 2

[Christian Reis]

Hmm, can you be more specific about what was broken and what fixed the issue? Is
this reproducible with an empty timetracking group?

Comment 3

[Jason Pyeron]

yes this is reproducable with an empty time tracking group

process_bug.cgi is broken.

result:

Positive Number Required      
The Hours Worked field requires a positive number.  
Please press Back and try again. 

Comment 4

[Christian Reis]

Oops. Yeah, you're right.

Comment 5

[Christian Reis]

Attachment: kiko_v1: fix and cleanup while we're at it

This was caused as a trivial regression from bug 252159.

The reason this is so easy to get wrong is that the code is @#!# confusing.
I've done some reorganizing and it's now clearer and hopefully safer. Can you
please test, Jason?

Comment 6

[Christian Reis]

Comment on attachment 154128 [details] [diff] [review]
kiko_v1: fix and cleanup while we're at it

Whoever gets this first wins an easter egg for 2005.

Comment 7

[Jason Pyeron]

great that pathc fixes the multiple bugs at once issues too.

(there was no workaround for multiple bugs changed at once)

Comment 8

[Jouni Heikniemi]

Comment on attachment 154128 [details] [diff] [review]
kiko_v1: fix and cleanup while we're at it

>Index: process_bug.cgi
>===================================================================

>-            SendSQL("UNLOCK TABLES");

Note that this line never got added back. Intentional or not? 

>+            # AppendComment can in theory raise an error, but we've
>+            # already validated the work_time here and therefore it's
>+            # safe to log the entry before adding the comment.
>+            LogActivityEntry($id, "work_time", "", $::FORM{'work_time'},
>+                             $whoid, $timestamp);

Add "(called later in the code)" after "AppendComment" to make it clear what
we're talking about.

r=jouni, but check out that unlock issue and explain on this bug why it's ok
(or fix it). 

You'll be receiving my address for the easter egg soon.

Comment 9

[Christian Reis]

(In reply to comment #8)
> 
> >-            SendSQL("UNLOCK TABLES");
> 
> Note that this line never got added back. Intentional or not? 

Well, it "lives on implicitly":

   ThrowUserError('comment_required', undef, "abort");

I've made the comment change locally, awaiting approval.

Comment 10

[Joel Peshkin]

Comment on attachment 154128 [details] [diff] [review]
kiko_v1: fix and cleanup while we're at it


I'm not sure that all sites will want to prevent negative numbers
(corrections), but it is an easy site hack and this just makes it cleaner.

Comment 11

[Christian Reis]

/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.209; previous revision: 1.208

Thanks.