179293 - time tracking js should only appear if time tracking is enabled

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Bradley Baetz (:bbaetz)]

enter_bug sends the inline js all the time; it should only do so if the
appropriate param is set

Comment 1

[Jeff Hedlund]

Attachment: patch v.1

Fix...

Comment 2

[Jeff Hedlund]

Assigning to me.

I assume you meant the js in show_bug... 

Comment 3

[Bradley Baetz (:bbaetz)]

Comment on attachment 105793 [details] [diff] [review]
patch v.1

Assuming that a diff -w will only show the IF and the END, r=bbaetz

As a side note, I've noticed that a lot of stuff just checks if the user is in
the group, w/o checking if the param is set first. Whilst we don't allow empty
groups, thats still involving an sql query, so you may want to file a separate
bug to prefix those with a test for existence first

Comment 4

[Jeff Hedlund]

> Assuming that a diff -w will only show the IF and the END, r=bbaetz

The other drivel is indentions from the diff -u.  Are the indentions not necessary?

> As a side note, I've noticed that a lot of stuff just checks if the user is in
> the group, w/o checking if the param is set first. Whilst we don't allow empty
> groups, thats still involving an sql query, so you may want to file a separate
> bug to prefix those with a test for existence first

From bug 24789 comment 105, I took the "redundant" param checks out.  ?

Comment 5

[Bradley Baetz (:bbaetz)]

Yeah, you should move hte indent, but its hard to see whats going on that way ;)

Re the extra checks, you may as well leave them. Gerv's right in that the result
will be the same, but it will cause an extra db hit in theory.

In practice, its likly to be cached, and will (for the logged in user, which is
all you care about here) soon definatley be cached, once I get arround to making
Bugzilla::User a reality

Comment 6

[Bradley Baetz (:bbaetz)]

dave, a=?

jeff, do you have checkin access? I can't recall.

Comment 7

[Jeff Hedlund]

> jeff, do you have checkin access? I can't recall.

Yes, I do have checkin access now.

Comment 8

[Dave Miller [:justdave]]

a= justdave

Comment 9

[Bradley Baetz (:bbaetz)]

jeff's cvs access wasn't set up corrrectly, so I checked this in for him