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

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Vlad Dascalu]

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.

Comment 1

[Dave Miller [:justdave]]

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

[GavinS]

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?

Comment 3

[Dave Miller [:justdave]]

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.

Comment 4

[Frédéric Buclin]

Attachment: patch, v1

Comment 5

[Marc Schumann [:Wurblzap]]

Comment on attachment 187801 [details] [diff] [review]
patch, v1

Tested; works.

Comment 6

[Frédéric Buclin]

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