Bug::AppendComment() should receive the user ID as a 2nd parameter

RESOLVED FIXED in Bugzilla 2.20

Status

()

Bugzilla
Bugzilla-General
--
minor
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: Vlad Dascalu, Assigned: Frédéric Buclin)

Tracking

unspecified
Bugzilla 2.20
Bug Flags:
approval +

Details

Attachments

(1 attachment)

(Reporter)

Description

14 years ago
In globals.pl we have an AppendComment sub defined, which takes as a 2rd 
parameter the name of the user, then uses DBNameToIdAndCheck in order to find 
the ID based on the name. That's the only place where it's used.

What's worst is that most .cgi which call AppendComment have handy the ID. So 
they call DBID_to_name to get the name and be able to call AppendComment 
correctly, only to find out that in AppendComment the conversion is done the 
other way around.

We should modify AppendComment to take the ID as a 2nd parameter and get ride 
of the conversion inside it, then get ride of the conversions done in the 
callers of this sub.

Doing a search shows only 3 places which call AppendComment:

-> CGI.pl
-> attachment.cgi
-> process_bug.cgi

and where I looked (that is, in most places), the ID of the user is available 
already without any sort of conversion.
Don't forget bugzilla_email_append.pl, which probably doesn't have the userid
handy, but doing the conversion in the other direction before calling makes
sense there.

Comment 2

14 years ago
If we did this, we would lose the validity check on the user parameter - is this OK?

Or would you want to validate the (new) userid parameter in AppendComment() anyway?
Reassigning bugs that I'm not actively working on to the default component owner
in order to try to make some sanity out of my personal buglist.  This doesn't
mean the bug isn't being dealt with, just that I'm not the one doing it.  If you
are dealing with this bug, please assign it to yourself.
Assignee: justdave → general
QA Contact: mattyt-bugzilla → default-qa
(Assignee)

Comment 4

13 years ago
Created attachment 187801 [details] [diff] [review]
patch, v1
Assignee: general → LpSolit
Status: NEW → ASSIGNED
Attachment #187801 - Flags: review?(wurblzap)
(Assignee)

Updated

13 years ago
Summary: AppendComment from globals.pl should receive the userid as a 2rd param → Bug::AppendComment() should receive the user ID as a 2nd parameter
Comment on attachment 187801 [details] [diff] [review]
patch, v1

Tested; works.
Attachment #187801 - Flags: review?(wurblzap) → review+
(Assignee)

Updated

13 years ago
Flags: approval?
Flags: approval? → approval+
Target Milestone: --- → Bugzilla 2.20
(Assignee)

Comment 6

13 years ago
Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.87; previous revision: 1.86
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.262; previous revision: 1.261
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.81; previous revision: 1.80
done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.