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)

2.16
x86
Linux
defect
Not set
normal

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)

(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.
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
Depends on: 93167
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
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?
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.
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.
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-
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 :)
How is this bug different from bug #126801?
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
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: