Closed Bug 179293 Opened 19 years ago Closed 18 years ago

time tracking js should only appear if time tracking is enabled

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

2.17
x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: bbaetz, Assigned: jeff.hedlund)

Details

Attachments

(1 file)

enter_bug sends the inline js all the time; it should only do so if the
appropriate param is set
Attached patch patch v.1Splinter Review
Fix...
Assigning to me.

I assume you meant the js in show_bug... 
Assignee: myk → jeff.hedlund
Status: NEW → ASSIGNED
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
Attachment #105793 - Flags: review+
> 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.  ?
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
dave, a=?

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

Yes, I do have checkin access now.
jeff's cvs access wasn't set up corrrectly, so I checked this in for him
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Target Milestone: --- → Bugzilla 2.18
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.