Closed
Bug 148687
Opened 22 years ago
Closed 22 years ago
queryhelp.cgi doesn't provide correct list of products if usebuggroups is on
Categories
(Bugzilla :: Bugzilla-General, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.16
People
(Reporter: stu, Assigned: bbaetz)
References
Details
(Whiteboard: [BLOCKER WILL FIX] applied to 2.14.2)
Attachments
(1 file)
4.27 KB,
patch
|
bbaetz
:
review-
|
Details | Diff | Splinter Review |
(it's 2.16rc1, but I don't see that in the Version list) queryhelp.cgi should be producing lists of products based on your bug group membership, but in the middle of it's listing it calls GroupExists(productname) which doesn't push/pop the SQL state, so the list gets messed up. possible fixes: - add PushGlobaleSQLState / PopGlobalSQLState to GroupExists - add them to queryhelp.cgi - Don't bother checking if the group exists in queryhelp.cgi - after all, it's listing products, not groups. All of these are barely worth patches - should I provide them? It might also be worth checking where else GroupExists is called from as it doesn't save state.
Assignee | ||
Comment 1•22 years ago
|
||
Option 1 is the correct one to do. We have another bug on a few functions not doing it, but lets do this for 2.16.
Target Milestone: --- → Bugzilla 2.16
Assignee | ||
Comment 2•22 years ago
|
||
Lets fix this properly. Stu, does the patch in bug 93167 fix this for you?
Assignee: justdave → bbaetz
Whiteboard: [BLOCKER WILL FIX] wanted for 2.14.2
Reporter | ||
Comment 3•22 years ago
|
||
Yes, that fixes it for me. I also noticed that BugInGroup, which seems similar to these, doesn't call ConnectToDatabase - should it? and should all things that do SendSQL be wrapped with Push/Pop?
Assignee | ||
Comment 4•22 years ago
|
||
BugInGroup already has push/pop arround it. In general, yes, stuff with SendSQL should have these wrappers arround it. I thikn we only care about the helper functions, though.
Reporter | ||
Comment 5•22 years ago
|
||
BugInGroup has Push/Pop, yes, but not the ConnectToDatabase before it - it probably doesn't matter, but it stands out from the other helpers :) coming soon is a patch to wrap all the SendSQL's in globals.pl with Push/Pop.
Reporter | ||
Comment 6•22 years ago
|
||
Assignee | ||
Comment 7•22 years ago
|
||
Comment on attachment 86010 [details] [diff] [review] wrap SendSQL's with Push/PopGlobalSQLState I don't follow about ConnectToDatabase. Thats a noop if we're already connected, and we have to be conneted to push a connection. The only extra sub in there which may need this is GetFieldID - the others aren't helper methods, they're subs in their own right. The entire Bugzilla<->DBI interface needs to be redone; I'd prefer to leave these as is for now
Attachment #86010 -
Flags: review-
Reporter | ||
Comment 8•22 years ago
|
||
The ConnectToDatabase observation is unrelated to the Push/Pop - I only mentioned it because I noticed it's in the other helpers (UserInGroup, GroupExists, GroupNameToBit,...) I wrapped all the SendSQLs based on your "In general, yes..." comment, not being too familiar with the code, I wasn't certain which subs were subs and which were helpers :)
Comment 9•22 years ago
|
||
How is this bug different from bug #126801?
Assignee | ||
Comment 10•22 years ago
|
||
Fixed by blocker. I think we'll leave the patch in this bug - we don't use GetFieldID in a situation where its needed, AFAICS
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: [BLOCKER WILL FIX] wanted for 2.14.2 → [BLOCKER WILL FIX] applied to 2.14.2
Updated•11 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•