Closed Bug 52557 Opened 24 years ago Closed 20 years ago

reassigning a bug via a drop-down list (popup menu)

Categories

(Bugzilla :: Creating/Changing Bugs, enhancement, P3)

Product:
Bugzilla
Component:
Creating/Changing Bugs
Type:
enhancement

Tracking

()

Status:
RESOLVED DUPLICATE of bug 251669

People

(Reporter: seth, Assigned: nn4l)

References

Details

Attachments

(5 files, 41 obsolete files)

Drop-down patch for 2.16, 2.16.1, and 2.16.2
30.83 KB, patch
Details | Diff | Splinter Review
Drop-down patch for July 10 tip of CVS
29.07 KB, patch
Details | Diff | Splinter Review
Modified dropdown patch for 2.16.4, including some bugfixes
29.94 KB, patch
Details | Diff | Splinter Review
Patch against 2004-03-10 cvs
31.39 KB, patch
Details | Diff | Splinter Review
Drop-Down Patch for 2.17.7
28.81 KB, patch
Details | Diff | Splinter Review 
So I hacked up this patch this afternoon for my installation of bugzilla. 
Essentially, both installations have bugzilla I run have less than ten users. 
It would be much, much easier if I had a drop down list of all users in bugzilla
when i wanted to reassign a bug.

So the attached patch puts in a new parameter, assign-to-list and makes that
functionality happen.

This patch should be clean, I moved the code to come up with the $assigned_to
variable into a subroutine in bug_form.pl and I banged on it for a bit.

    
    

    
  
*** Bug 30843 has been marked as a duplicate of this bug. ***

    
    

    
  
Bug 30843 has a lot of discussion that is worth a read if you're reviewing this 
bug report.  Resolved as a duplicate of this one only because this one has a 
patch on it.


    
    

    
  
In the previous path I used the original (9/13) patch as a base and expanded it
to include the qa_contact.  I also changed the wording a bit in defparams.pl to
be a little more discriptive (and also to say that it should only be used on
small installations).
Keywords: patch, 
          
            review

    
    

    
  
Should/Could the SQL be changed to the following, to exclude disabled users??

SendSQL("SELECT login_name FROM profiles WHERE disabledtext = '' ORDER BY
login_name ");

And if so, is this the correct/best way to do it?

(It seems to work on our installation, anyway.)


    
    

    
  
Looks valid to me... different than the way I would've done it, but as long as
it works, that's the important part...

    
    

    
  
*** Bug 65011 has been marked as a duplicate of this bug. ***

    
    

    
  
There was a patch submitted with bug 65011 when it was filed that does things 
just a little bit differently than this one (but with the same intention).  
Anyone want to take a look at it?


    
    

    
  
*** Bug 66191 has been marked as a duplicate of this bug. ***

    
    

    
  
filed bug 68531 for a xul system.

i've seen some html based 'application's that allow you to look up people in an 
address book and then use javascript to return their addresses.  I have bug 
63689 where i'm working on a findusers.cgi bug that I could probably finish and 
then adapt to work here.

    
    

    
  
*** Bug 69811 has been marked as a duplicate of this bug. ***

    
    

    
  
1)Would love to see this as a feature that can be controlled at the component level.
2)Would be great to see user lists that are filtered to members in the relevant group.
Target Milestone: --- → Bugzilla 2.16

    
    

    
  
-> Bugzilla product, Changing Bugs component, reassigning.
Assignee: tara → myk
Component: Bugzilla → Creating/Changing Bugs
Product: Webtools → Bugzilla
Version: other → unspecified

    
    

    
  
*** Bug 97601 has been marked as a duplicate of this bug. ***
Summary: reassigning a bug via a drop-down list → reassigning a bug via a drop-down list (popup menu)

    
    

    
  
*** Bug 98640 has been marked as a duplicate of this bug. ***

    
    

    
  
I wanted to see the realname as well...


+++ CGI.pl.woodie       Thu Sep 20 14:42:57 2001
@@ -47,21 +47,30 @@
 
 sub GeneratePersonInput {
     my ($field, $required, $def_value, $extraJavaScript) = (@_);
     $extraJavaScript ||= "";
     if ($extraJavaScript ne "") {
         $extraJavaScript = "onChange=\"$extraJavaScript\"";
     }
-    return "<INPUT NAME=\"$field\" SIZE=32 $extraJavaScript 
VALUE=\"$def_value\">";
+    my $html = "<SELECT NAME=\"$field\" $extraJavaScript>\n";
+    $html .= "<OPTION value=\"\"></OPTION>\n";
+    SendSQL("SELECT login_name, realname FROM profiles;");
+    while (MoreSQLData()) {
+        my ($user, $name) = (FetchSQLData());
+        $html .= "<OPTION value=\"$user\"";
+        $html .= " SELECTED" if ($user eq $def_value);
+        $html .= ">$user ($name)</option>\n";
+    }
+    $html .= "</SELECT>";
+    return $html;
 }
 

    
    

    
  

      
      
      
      

        Attached patch
          
          
        The drop down menu jumbo patch (obsolete)
        — 
          — Splinter Review
      

    


  

    
    

    
  
I added "The drop down menu jumbo patch": a merge of all the available patches 
plus some ideas of mine.
Features: drop down menus for assigned to, qa contact and initial owner of 
component. The drop down menus show real names instead of email addresses, 
sorted in ascending order. Also, disabled users are left out.

This patch is for bugzilla-2.14.


    
    

    
  
It would be great for BuildUserElement() to filter users that have an access to
the current project if Param("usebuggroups") is on.



    
    

    
  
I believe a better way to do this would be to generate the list based on the
distinct results from looking up all component owners for the particular product
that the bug report is against. For example use the following SQL statement instead.

SELECT
    distinct profiles.login_name,
    profiles.realname
FROM
    components,
    profiles
WHERE
    components.initialowner = profiles.userid
    AND components.program = 'some product';
You should also have a text box nearby for assigned_to which would override
whatever is selected in the list box. This could be used on either the
bug_form.pl page or enter_bug.cgi. If both unused on the enter_bug.cgi form it
should just pick the right one for you.

    
    

    
  
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18

    
    

    
  
Will make a new patch for 2.16 if it can't be included -- I can't live without 
the drop down lists anymore...

    
    

    
  
yeah, fairly popular feature request so I'm sure it'll make 2.18.  A patch
against 2.16 when it's out certainly wouldn't hurt.

    
    

    
  
I installed the latest patch (attachment#50270 [details] [diff] [review]) on my company's bugzilla
installation. Looks like everything is still working properly. I will see what
user feedback I get from everyone to see if they like it. 

One problem with the patch though is that if a user hasn't filled in the Real
Name box on their user page, their name DOES NOT show up on the list. Instead
there are blank spaces on the list. The feature should account for this by
displaying the email address instead of the real name is blank.

    
    

    
  
Reassigning to patch author.
Assignee: myk → nn4l

    
    

    
  
>One problem with the patch though is that if a user hasn't filled in the Real
>Name box on their user page, their name DOES NOT show up on the list. Instead
>there are blank spaces on the list. The feature should account for this by
>displaying the email address instead of the real name is blank.

I usually do this by checking for the presence of a real name and using the
following syntax if there is one:

Real Name <email@address>

... and the following syntax if there isn't one:

email@address



    
    

    
  
perhaps there needs to be a reuseable function somewhere in the bugzilla code
base that given a user_id or something gives the "name <email@domain>" or
"email@domain" strings appropriately. That way there won't be redundant
instances of code doing the same thing.

    
    

    
  
I tried the jumbo patch with today's 2.15 CVS snapshot. One line had to be
patched manually, but it still works as far as I can tell. 

How should the entries in the dropdown lists look like? Currently, the real name
is displayed. I could change it to "mail@company.com (real name)" to make it
consistent with other occurrences of the real name, such as the existing
"Assigned To:" field. But it uses more space. The list would then be sorted
according to mail address instead of name, which makes it more difficult to find
an entry. I could also change it to "real name <mail@company.com>", as Myk
suggested.

The list currently displays all registered users. Alexey suggested restricting
the list to those users which have the proper access rights to the particular
record. This clearly makes sense. David suggested filling the list with
component  owners. I have patched our Bugzilla installation to have an "Is
Developer" privilege flag, so only users with that privilege set would be listed
in the dropdown list.

Which features do you want for the 2.16 patch? Please comment.
Status: NEW → ASSIGNED

    
    

    
  


  
Jumbo patch plus added drop downs for 'Cc:' and 'Add CC:'

    
    

    
  
Does the patch for the drop-down list on CC: allow you to add more than one user
to the cc list at the same time? Otherwise we should keep the drop-down list out
of the CC fields.

    
    

    
  
I ran this patch on a win32 install of Bugzilla 2.14.1.  Everything works 
except on the enter_bug.cgi page(after I choose a product), there is a series 
of perl errors at the top of the page.

It starts with:
Use of uninitialized value in string eq at CGI.pl line 65
Here are the two lines of CGI.pl it is concerned with:
65: if ($null_entry ne "") {
74: if ($login_name eq $defaultvalue) {

I haven't figured out a way to deal with it yet.

    
    

    
  
Responding to myself - changing my appmapping from perl %s %s to perl -X %s %s 
gets rid of the warnings.   Good enough for me.  By the way, 'this patch' in my 
previous comment refers to the drop down menu jumbo patch(not ddmjppluscc).

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Allow multiple select for cc (obsolete)
        — 
          — Splinter Review
      

    


  
Allow multiple select for cc

    
    

    
  


  
This is a jumbo patch that I'm going to maintain so that it's ready to be
merged into the tree come 2.17.  This encorporates #71491 and #71685 but
obsoletes neither of them.  The enter_bug.cgi patch failed because it has been
"Template'ized" since the patch was written, this is fixed.

    
    

    
  
I applied your patch/attachment "Drop down menu jumbo patch plus CC" (02/26/02
08:04) to my Bugzilla 2.14.1 installation and I think I detected the following
problems. That patch was for 2.14.1 right? I'm not sure if I should upgrade to
2.15 because I am hoping to wait to upgrade until the stable 2.16 version comes
out.

Problems with the patch against vanilla 2.14.1:
- I have the ccdropdown param set to off, but in the new bug entry page,
"(Multiple selections allowed)." still appears next to the CC: text entry box.
- Initial QA Contact has completely dissapeared from the new bug entry page. My
params for that are set to: initialqacontactdropdown ON, qa_contactdropdown ON,
useqacontact ON
- On the bug page, CC drop-down is there, but it is multi-selectable. Relevant
settings in my params: newccdropdown ON, ccdropdown OFF, (? relevant?)
preferlists ON.

Otherwise, the patch is working great. Thanks!!!

    
    

    
  


  
Obsoletes 72333 but I don't have permission to do that... This patch brings
72333 up to date as of today.  I've mildly tested the functionality of this
patch.	

Two items of significance in this patch:
- what used to be enter.html.tmpl has been moved to
template/en/default/bug/create/create.html.tmpl
- what used to be show_bug.html.tmpl has been moved to
template/en/default/bug/edit.html.tmpl

It's possible these could be used in the templatized v2.16rc1 but I haven't
looked at it at all.

    
    

    
  
Nice patch.

I tried the patch from attachment 84137 [details] [diff] [review] and found that the entry form got rather
cluttered if BuildUserElement is allowed to generate 
<select NAME="assigned_to" >
and <select NAME="cc" multiple>

By adding a "size=10" argument to the BuildUserElement calls in enet_bug.cgi,
these get verical scrollbars and the assigned to and cc boxes no longer run into
each other.




    
    

    
  


  
Applied  patch 84137 to 2.16rc1 and made the following changes
Handles new template locations as Paul suggests
Permit mutiple select of new cc users on bugs being edited
Supress disabled accounts from cc user list
Make enter_bug cc list a scrolling list

    
  
Blocks: 145499

    
    

    
  
Comment on attachment 72333 [details] [diff] [review]
"Jumbo" Patch against CVS (2.15) on March 3, 2002.

Marking obsolete per comment 38.

        Attachment #72333 -
        Attachment is obsolete: true

    
    

    
  
The show_bug.cgi page is messed up after applying the patch in attachment 84165 [details] [diff] [review].
I tested this with Netscape 6.2 and MSIE 5.5.

The dropdown menu for reassign shows email addresses, all other dropdown menus
show  real names. I would like to have real names if they exist.

    
    

    
  
Paul,

   Any reason that the code in the bug_form template shouldn't use a variable
created by BuildUserElement and just spit out the HTML from the variable instead
of passing a list of email addresses and calling select?



    
    

    
  
Not sure if this is relevant to this discussion, but I think the main idea
behind the templatization is to separate presentation from the backend logic
completely, thus no html should be created in the backend any more.

    
    

    
  
Andreas:

  So, for the purposes of this type of function, would the correct migration be
to pass a list containing  ( firstlogname, firstrealname, secondlogname,
secondrealname,.....) to the template and have a function in the template build
the HTML for the box?



    
    

    
  


  
This latest cut on the patch fixes the <td> merge problem noted in comment 42
as well as properly processing multiple selections in the cc box on the new bug
form.

Assuming this merges in and works for most interested parties, this will do it
for me for a while.  *someone* should look at moving the html-aware portions of
BuildUserElement into the templates and address both the issue from comment 44
as well as using realname in some of the boxes that currently show the login
name.

        Attachment #84165 -
        Attachment is obsolete: true

    
    

    
  
All - right now I'm just maintaining compatibility (functionality?) of the
various patches submitted thus far.  I haven't done much with the templates
except to make things work, that'll come a little later for me.

Presenting real names v.s. email addresses shouldn't be difficult to change.  I
may try that tomorrow.

-Paul

    
    

    
  


  
Obsoletes 84137 and 84759.

Updated to use templates properly for all affected cgi programs except
editcomponents.cgi as it has not been templatized in CVS yet.

All functionality should be maintained from previous patches.  One notable
change is that the individual's realname is used instead of login_name for
display on HTML pages.	Behind the scenes the unique userid is used instead of
another identifier to ensure uniqueness and privacy.  Specifically, email
addresses (login_name) are not used in the source HTML that this patch changes.
 

One known bug - The javascript that automatically selects the reassign radio
button when the assignee is changed is not sent to the browser.  I couldn't
think clearly about how it everything should be escaped so it can be passed
around the template so I left it commented out.  This is on line 433 of
template/en/default/bug/edit.html.tmpl if anyone has time to figure this out.

Once again, minor testing done.  Please beat on it and let me know what breaks!


-Paul

    
    

    
  
I have applied the patch in attachment 84939 [details] [diff] [review] to 2.16rc1 (with some manual help).

Problems: 

1. I have selected only the assigned_todropdown flag. The new bug entry form now
has the dropdown menu, but the edit bug entry form still has the text box for
reassign. I think this field should be dependend on assignedto_dropdown too.

2. When selecting both assigned_todropdown and initialownerdropdown, then the
edit bug entry form shows a dropdown menu for reassign.

3. When selecting all *dropdown flags, then the reassign element in edit bug
entry form changes into a listbox, allowing multiple entries.

    
  

        Attachment #84759 -
        Attachment is obsolete: true

    
    

    
  
Arne,

  The patch is meant for the CVS tip at 2.17.  Having said that I'm curious to
know what help you needed to give the patch to make it apply.

  As you've found, there are actually two parameters that control the
assigned_to fields, one for new bugs (initialownerdropdown) and one for editing
bugs (assigned_todropdown).  If this isn't correct behavior, that's fine - just
let me know what the *correct* behavior is or should be and I'll fix it.  That
sort of thing is trivial.

  I think the third problem (reassign being a listbox) is related to something
that needed help.  Do all the pull-downs render this way, or just the reassign?

  I use Mozilla and it presents it as a drop-down.  The source that's generated
in my test installation shows as follows:

<a href="bug_status.html#assigned_to">Reassign</a> bug to &nbsp;
<select NAME="assigned_to" >
        <option VALUE="0">---
        </option>
        <option VALUE="2" SELECTED>Paul 2 Kronenwetter
        </option>
        <option VALUE="1">Paul Kronenwetter
        </option>
</select>

This should be rendered as the drop down.  Please check the source of the page
with the multiple-select list.  If you see <select multiple ... or something
similar, there's a problem.  If so, I'd ask you to check the source of
template/en/default/bug/edit.html.tmpl.  If there's a multiple at line 570 the
patch (or the help) is wrong.

Please let me know how you do on this!
-Paul

    
    

    
  
Arne,

  I should have said around line 570, since I'm not sure how closely 2.16r1
relates to 2.17...

Sorry!
-Paul

    
    

    
  
My html source is in fact: <select NAME="assigned_to" MULTIPLE SIZE=3>

The "MULTIPLE SIZE=3" string comes from line 88 of edit.html.tmpl. After
double-checking by modifying to lower-case I noticed that there are now several
occurrences of this particular string.

My guess is that the xtra variable gets filled in line 88, but never reset for
other [% PROCESS %] invocations. So it looks like a general template problem.

Note that I tested against 2.16rc1 and I don't understand why you don't see the
problem. Maybe the cgi template handling code has changed, the templates
themselves seem to be pretty much unchanged between 2.16rc1 and current cvs.

Should I file a new bug against 2.16rc1?



    
    

    
  


  
Apply this patch to 2.16rc1 before applying patch 84939.

    
    

    
  
My understanding of the initialowner field: This is the person who is
responsible for a bug until it is assigned to a person. The only occurrence of
this field is in the edit components page.

The assigned_to dropdown flag should control both the Assign to field in the New
Bug page and the Reassign to field in the Edit bug page. I don't think anybody
would turn on the dropdown in one page and leave the text field in the other, so
one flag controlling both pages should be sufficient.

    
    

    
  
Arne (Re: #52) - I'd try putting an xtra => "" in the offending parts of
create/edit.html.tmpl.  If that works, I'll encorporate that into my patch and
we can at least have some evidence for filing a bug report against 2.16r1.



    
    

    
  
Arne - PS: This is the output of ./checksetup.pl:

Checking perl modules ...
Checking for       AppConfig (v1.52)   ok: found v1.52
Checking for       CGI::Carp (any)     ok: found v1.20
Checking for    Data::Dumper (any)     ok: found v2.102
Checking for     Date::Parse (any)     ok: found v2.20
Checking for             DBI (v1.13)   ok: found v1.21
Checking for      DBD::mysql (v1.2209) ok: found v2.0416
Checking for      File::Spec (v0.82)   ok: found v0.82
Checking for        Template (v2.07)   ok: found v2.07
Checking for      Text::Wrap (v2001.0131) ok: found v2001.0131
Checking for       CGI::Carp (any)     ok: found v1.20

The following Perl modules are optional:
Checking for              GD (v1.19)   ok: found v1.29
Checking for     Chart::Base (v0.99)   ok: found v0.99
Checking for     XML::Parser (any)     ok: found v2.30

Checking user setup ...
Checking for    MySQL Server (v3.22.5) ok: found v3.23.47

Populating duplicates table...
Reminder: Bugzilla now requires version 8.7 or later of sendmail.


    
    

    
  
In line 434 of edit.html.tmpl there is a commented out assignment to "xtra",
leaving the variable with whatever had been assigned before. I added 

xtra => ""

which fixed the broken Reassign to control.

I did not even notice that all other dropdowns (priority, version etc.) had
changed to lists too, allowing multiple selections. The cause for this is similar:

In line 555 of edit.html.tmpl ([%BLOCK select %] macro definition) the xtra
variable is used but it is never defined in any of the [% PROCESS select ... %]
macros. I removed the [% xtra %] variable, which fixed that problem too. As an
alternative one could add the xtra => "" assignment to all [% PROCESS select ...
%] macros.

I think this is in fact worth filing a new bug, and code inspection of the
existing templates. If a variable is used in the macro definition then it must
be set in all invocations of that variable.

Paul, could you still investigate why you do not experience this problem?

    
    

    
  
sorry, that should actually read 

"If a variable is used in the macro definition then it must be set in all
invocations of that macro."



    
    

    
  
Arne,

  I suspect the reason that some people see this and others don't is because of
a goof I made in attachment 84759 [details] [diff] [review].  I left BOTH the select without the "xtra"
parameter AND the select with the "xtra" parameter in the select process
definition.  This makes the HTML below which, I suspect, is highly subject to
interpretation by the browser.


 <select name="version" multiple size=3>
<select name="version">
<option value="unspecified" selected>unspecified
        </option>
</select>


Paul, Arne,

   I don't want to step on each other's patches, so I'll presume you will roll
the fix into your next patch. 

-Joel




    
    

    
  
Joel,

  I actually found that and had fixed it before Arne's last attempt.  That's not
the problem here.

-Paul

    
    

    
  
Comment on attachment 85218 [details] [diff] [review]
changes to 2.16rc1 before applying attachment 84939 [details] [diff] [review]

obsolete because Paul's attachment 84939 [details] [diff] [review] works with 2.16rc2 just fine.

        Attachment #85218 -
        Attachment is obsolete: true

    
    

    
  


  
This patch catches up with the CVS tree as of June 27 (or 28th, depending on
time zone :-).

This probably will not work with 2.16rX.  But try it anyway and let me know.

There's no new functionality here, just keeping things clean as best I can.

-Paul

    
    

    
  
Is there a patch yet that works with official 2.16 release?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Drop-down patch for 2.16 (obsolete)
        — 
          — Splinter Review
      

    


  
This one works with the release 2.16. Some code labeled "Add the CC list" has
been deleted in 2.16, so the corresponding patch code did not apply. It has
also a fix for the problem described in comment #57.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Drop-down patch for 2.16 (obsolete)
        — 
          — Splinter Review
      

    


  
forgot a closing curly brace somewhere in post_bug.cgi

        Attachment #93266 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 93269 [details] [diff] [review]
Drop-down patch for 2.16

>diff -ur bugzilla-2.16-orig/bug_form.pl bugzilla-2.16-patched/bug_form.pl
>--- bugzilla-2.16-orig/bug_form.pl	Wed Jul 10 02:05:51 2002
>+++ bugzilla-2.16-patched/bug_form.pl	Tue Jul 30 10:19:03 2002

> 
>+my $loginok = quietly_check_login();

Why do you need this? The caller should have done this.

> sub show_bug {    
>     # Shut up misguided -w warnings about "used only once".  For some reason,
>     # "use vars" chokes on me when I try it here.
>@@ -180,6 +186,24 @@
>     # Attachments
>     $bug{'attachments'} = Attachment::query($id);
> 
>+    # Profile information for everyone so it can be used in selections
>+    # and menus.
>+    my @profiles;
>+    my %profile;
>+    SendSQL("SELECT userid,login_name,realname FROM profiles where disabledtext = '' ORDER BY realname ");

You should only do this if the relevent params are selected - bmo doesn't want
30000 rows being retruned every time someone looks at a bug.

>+    push(@profiles, "0");
>+    $profile{"realname"}{"0"}="---";
>+    $profile{"login_name"}{"0"}="---";
>+    while (MoreSQLData()) {
>+        my ($userid,$login_name,$realname) = FetchSQLData();
>+        push(@profiles, $userid);
>+        $profile{"realname"}{$userid}=$realname;
>+        $profile{"login_name"}{$userid}=$login_name;
>+    }
>+
>+    $vars->{'profiles'} = \@profiles;
>+    $vars->{'profile'} = \%profile;

Why do you need this - surely you can just iterate over the kys of the hash,
rahter than using a separate list.

>     # People involved with the bug
>     $bug{'assigned_to_email'} = DBID_to_name($bug{'assigned_to'});
>-    $bug{'assigned_to'} = DBID_to_real_or_loginname($bug{'assigned_to'});
>+    $bug{'assigned_to'} = $bug{'assigned_to'};
>     $bug{'reporter'} = DBID_to_real_or_loginname($bug{'reporter'});
>-    $bug{'qa_contact'} = $bug{'qa_contact'} > 0 ? 
>-                                          DBID_to_name($bug{'qa_contact'}) : "";
> 

Whats all this for?


>diff -ur bugzilla-2.16-orig/enter_bug.cgi bugzilla-2.16-patched/enter_bug.cgi
>--- bugzilla-2.16-orig/enter_bug.cgi	Tue Jul 30 10:34:00 2002
>+++ bugzilla-2.16-patched/enter_bug.cgi	Tue Jul 30 10:19:03 2002

>@@ -378,6 +376,24 @@
> 
>     $vars->{'group'} = \@groups;
> }
>+
>+# Profile information for everyone so it can be used in selections
>+# and menus.

Again, don't build this unless you need to. You probably want to put this in
its own sub, to avoid duplicating code.

> 
> $vars->{'default'} = \%default;
> 
>diff -ur bugzilla-2.16-orig/globals.pl bugzilla-2.16-patched/globals.pl
>--- bugzilla-2.16-orig/globals.pl	Sun Jul 28 02:54:28 2002
>+++ bugzilla-2.16-patched/globals.pl	Tue Jul 30 10:19:03 2002
>@@ -934,6 +934,19 @@
>     }
> }
> 
>+sub DBIdCheck {
>+    my ($userid) = (@_);
>+    PushGlobalSQLState();
>+    SendSQL("select userid,realname from profiles where userid = @{[SqlQuote($userid)]}");
>+    my $r = FetchOneColumn();
>+    PopGlobalSQLState();
>+    # $r should be a positive integer, this makes Taint mode happy

Why does taint mode care? processmail?

>+    if (defined $r && $r =~ m/^([1-9][0-9]*)$/) {
>+        return $1;
>+    } else {
>+        return 0;
>+    }
>+}
> 

>+[%############################################################################%]
>+[%# Block for SELECT fields for profiles.                                    #%]
>+[%############################################################################%]
>+
>+[% BLOCK selectprofile %]
>+    <SELECT NAME="[% selname %]" [% xtra %]>
>+      [% FOREACH x = selvalues %]
>+        <OPTION VALUE="[% x FILTER html %]"
>+          [% " SELECTED" IF x == bug.${selname} %]>
>+	    [% selitem.${x} FILTER html %]
>+        </OPTION>
>+      [% END %]
>+    </SELECT>

lowercase attributes, selected="selected", etc.

>+[%############################################################################%]
>+[%# Block for SELECT fields for profiles.                                    #%]
>+[%############################################################################%]
>+
>+[% BLOCK selectprofile %]
>+    <SELECT NAME="[% selname %]" [% xtra %]>
>+      [% FOREACH x = selvalues %]
>+        <OPTION VALUE="[% x FILTER html %]"
>+          [% " SELECTED" IF x == bug.${selname} %]>
>+	    [% selitem.${x} FILTER html %]
>+        </OPTION>
>+      [% END %]
>+    </SELECT>
> [% END %]

Ditto. We may want to require TT2.08 and put this into a common global routine
file somewhere, but you can leave that for the moment

OK, thats a first run through...

        Attachment #93269 -
        Flags: review-

    
    

    
  
I was only merging existing patches, but I'll see whether I can work through the
issues.

    
    

    
  
After applying the latest patch to 2.16 I'm getting an odd error message from 
the CC field on the New Bugs page.  It is:  "The name 6 is not a valid 
username. Either you misspelled it, or the person has not registered for a 
Bugzilla account."

And I've confirmed that 6 is the id of the person I selected in the CC box.

Any ideas where I should look?

   -Scott

    
    

    
  
Scott - I just applied Arne's latest patch to the 2.16 distribution and it works
fine...  What's happening is that the CGI validation code wasn't prepared to
have a '6' handed to it when it was expecting an email address.  This is (and
has been for a while) implemented in the patch so I'm not sure why yours doesn't
work.

    
    

    
  
When I tried the patch, I had the same problem. I was trying to create a new bug with an explicit assignee (the first one in the list) and I got the message saying that "name 1" wasn't a valid username. Since it was on a running system, I quickly removed the patch.

    
    

    
  


  
Obsoletes 89487 and 84939.

This patch *DOES NOT* apply to 2.16.  I will see what I can do about that and
post another patch.

All functionality should be maintained from previous patches. However, it does
appear as though I've broken the real name display in the CC list.  I'll work
on fixing that but I don't see the obvious problem (which is why it's not fixed
here :-).

One known bug - The javascript that automatically selects the reassign radio
button when the assignee is changed is not sent to the browser.  I couldn't
think clearly about how it everything should be escaped so it can be passed
around the template so I left it commented out.  This is on line 433 of
template/en/default/bug/edit.html.tmpl if anyone has time to figure this out.

Once again, minor testing done.  Please beat on it and let me know what breaks!



-Paul

        Attachment #84939 -
        Attachment is obsolete: true

        Attachment #89487 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Drop-down patch for 2.16 (obsolete)
        — 
          — Splinter Review
      

    


  
This applies cleanly to 2.16 so I'm obsoleting 93269.  It also has all the
updates that work from the next patch in the list (the one that obsoletes 93808
that I just fixed:-)

Beat on it, and let me know what problems occur.  This includes those last two
folks who had problems.  I don't see the problems so any additional information
will be most helpful!

        Attachment #93269 -
        Attachment is obsolete: true

    
    

    
  


  
Obsoletes 93808.  (Stupid operator error)

This patch *DOES NOT* apply to 2.16.  See 93822 for the 2.16 patch.

All functionality should be maintained from previous patches. However, the
loginname of the user in the current CC list still appears.  It's going to take
a bit of coding to get rid of that one and I don't think I'll try.  That stomps
on the other bug that asks to take care of that thru the whole application.

One known and very pesky bug - The javascript that automatically selects the
reassign radio
button when the assignee is changed is not sent to the browser.  I couldn't
think clearly about how it everything should be escaped so it can be passed
around the template so I left it commented out.  This is on line 442 of
template/en/default/bug/edit.html.tmpl if anyone has time to figure this out.

Once again, minor testing done.  Please beat on it and let me know what breaks!


-Paul

        Attachment #93808 -
        Attachment is obsolete: true

    
    

    
  
Oh, and 93822 and 93833 take care of a few items that Bradley Baetz pointed out
but not all of them...  Some will take quite a bit of thought to fix.

Bradley - I'm not familiar enough with what gets passed where and how to fix the
profile/profiles part that you suggested nuking.  Perhaps you could fiddle with
that part of the code and show me what's going on?

I also don't know what's actually being done to make taint happy in the section
below.  I haven't had any problems with taint checks so I haven't been tempted
to write 'make taint happy' code.

Any help you can provide is appreciated!

-Paul

    
    

    
  
re profiles, you have a list of userids, and then what basically ammounts to a
hash for the realnamename and a hash for the login_name. Ignoring the fact that
the way the hash is done isn't really useful, its duplicating a whole lot of
usless data.

Why not just have a list of a realname/login_name pair (or a two element hash)
and have the template iterate through that?

For taint mode, I was referring to the comment.

    
    

    
  
Bradley - Yes, I agree.  I was hoping for a push in the right direction since
you appear to have an idea as to how to accomplish this efficiency.  Perhaps a
short example or reference another part of the code/template where it's done.

-Paul

    
    

    
  
I applied 93822 to my new 2.16 bugzilla installation. The patch doesn't seem to
work as well as older versions of the patch did for 2.14. In particular, the
drop down lists seem to work on the New bug entry page, but when I do into an
old bug, the "Assigned To" and "QA Contact" fields contain only a small number,
and not the name of the person. The number does corollate to the proper name if
you count down in the drop-down list.

Any idea why this broke between 2.14 and 2.16?

    
    

    
  
That's strange.  I'll try to take a look at it.  It sounds like a quick fix but
I need to find it :-).

-Paul

    
    

    
  
I have been using the drop-down patch for a few months on a growing site and the
drop-downs rapidly became too large to be practical, especially for CC lists. 
Bug 162990 is intended to address the needs of larger sites.  It would be a big
help if people with similar needs were to comment on bug 162990 before
implementation begins.



    
    

    
  

      
      
      
      

        Attached patch
          
          
        Drop-down patch for 2.16 (obsolete)
        — 
          — Splinter Review
      

    


  
Fixed problem identified where the assigned to field was a number instead of
the text of real name or login name.

Please use and post problems with it.

-Paul

        Attachment #93822 -
        Attachment is obsolete: true

    
    

    
  


  
Obsoletes 93823.

This is just an update to bring the patch current with the tip of CVS as of
August 16th.  No new functionality or features.  Probably new bugs, but I'm not
sure.

Please try this and post if you find problems!
-Paul

        Attachment #93823 -
        Attachment is obsolete: true

    
    

    
  
Re comment #79: I made a modification of the drop down menu suitable for sites 
with lots of users. I introduced a new user privilege bit: "is a developer". 
Only users with this attribute set are displayed in the drop down menus. This 
worked for me because usually you have a lot more bug reporters than 
developers. The disadvantage was that it uses an additional bit of the user 
privilege bits, which are limited. 

    
    

    
  
The privilege bit stuff should become a lot easier during 2.17 as the new group
system lands.

    
    

    
  
Thanks for the updated patch for 2.16!

Regaring comment #80, I have tested out the patch and it is working much better
than the previous version. BUT, there is still one place I noticed where a
number is given in a text box rather than a drop down list:

* On the show_bug.cgi (full bug report) page, QA Contact still lists a number in
a text field rather than a name in a drop down list. Also, it looks like the
number may not correspond to the right QA Contact person/user... but I could be
wrong about that.

As soon as another patch is available, I will be happy to test it. Thanks again!

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Drop-down patch for 2.16 (obsolete)
        — 
          — Splinter Review
      

    


  
Probable fix for #84.  I've tested this one a little more than the rest and it
appears to work well.

This patch is only for 2.16 release.

Share & Enjoy!

        Attachment #95675 -
        Attachment is obsolete: true

    
    

    
  


  
No particular updates except one:  I've removed the 'initialqacontactdropdown'
parameter.  I couldn't figure out how/why it was different from
'qa_contactdropdown' and it was confusing me :-).

Please try this and let me know how things go!

        Attachment #95677 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 96590 [details] [diff] [review]
Drop-down patch for 2.16

I also removed initialqacontactdropdown from this patch.

    
    

    
  
Paul, thanks for the new patch! I think I still found a problem with it, I am
guessing the templatization and other changes from 2.14 to 2.16 must have been
very significant wrt the number of changes required in your patch!

I was trying to edit a component today, after applying the latest dropdown patch
for 2.16, and I got the following error in
http://hsp.nws.noaa.gov/bugzilla/editcomponents.cgi:

Can't find param named initialqacontactdropdown at globals.pl line 1466.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Drop-down patch for 2.16 (obsolete)
        — 
          — Splinter Review
      

    


  
Ok, not so bright of an idea to rid ourselves of initialqacontactdropdown
parameter.  That's what broke.	So this again has that parameter and that part
works better now :-)...

Again, this only works with 2.16.

        Attachment #96590 -
        Attachment is obsolete: true

    
    

    
  


  
Along with patch #96771 initialqacontactdropdown parameter is back.  Duh.  No
additional changes made.

This doesn't work with 2.16 released, only the tip of CVS as of August 25th,
2002.

    
    

    
  


  
Along with patch #96771 initialqacontactdropdown parameter is back.  Duh.  No
additional changes made.

This doesn't work with 2.16 released, only the tip of CVS as of August 25th,
2002.

        Attachment #96591 -
        Attachment is obsolete: true

    
  

        Attachment #96772 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 96773 [details] [diff] [review]
Drop-down patch for August 25 tip of CVS.

The CGI.pl bits don't appear to be needed.

I don't like the copying of code to extract the user list. Put it somewhere
else, and have the function test the params, returning undef if the list didn't
need to be built. Then the template can test for insertion. Alternately, have
the template call the function.

You can drop the eidt* changes too - that doens't happen that often, and then
this can be fixed once templatiasation happens for those (that may make the
above easier)

What is the point of DBIdCheck? You appear to be using it to verify that the
results we get from the db somewhere else are valid. You can assume that the db
isn't lying to you. The detainting of out vals shouln't be needed, anyway, nor
is getting the realname.

/me looks further

Oh, I see.

Rather than needing to add Param checks everywhere, just have the dropdown code
comain the email addresses as the values. That removes a whole lot of the if
statements, and makes the patch very localised

        Attachment #96773 -
        Flags: review-

    
    

    
  
Ah! That did it! The patch created and referenced in comment #89 did it for our
site. Now our Bugzilla 2.16 is working great with the drop down patch. Thanks!!

    
    

    
  
Bradley - I see where you're going about the CGI.pl stuff.  That's gone.

What file would you suggest for the common routines?  I see two possibilities
CGI.pl and globals.pl.  From the larger picture it appears that they are about
equally popular places to put things.  What are your thoughts on this?  Since
bug_form includes neither I suppose it might just as well be CGI.pl.

I don't have much problem removing the stuff from edit*.  It was there when I
started maintaining the code.  Although I don't see the harm in leaving it there.

I'll need a little more guidance on the implementation of what you're thinking
about for replacing DBIdCheck.

    
    

    
  
Problem with attachment #96590 [details] [diff] [review] and v. 2.16:

Do a query, click on "Change several bugs at once". The "Reassign bugs to" item
does not have a dropdown menu, it is still a text field. When entering the
user's email address the standard way, I get the "You cannot reassign to a bug
to nobody" error message after committing the dialog.

The unpatched version works as expected. I did not retry this with a more recent
patch.

    
    

    
  
Stick it in CGI.pl, and someone will move it to User.pm once such a file exists.

(Bugzilla::User::GetAll, iow)

    
    

    
  
ACK comment #95.  Completely forgot about that one.  It's also broken in the
"other" patch set...  Working on it.

    
    

    
  


  
Updates 96771 and adds dropdown support to one of the multiple bug edit
screens.  If there are other things that I've missed, please point them out.

This patch only works with Bugzilla 2.16 release.

        Attachment #96771 -
        Attachment is obsolete: true

    
    

    
  


  
This updates 96773 and adds dropdown support for one of the multiple bug pages.


I will now go and combine the various parts of code that are duplicated around
the  system into CGI.pl as suggested by a previous post.

As usual, please let me know of things that are broken or should be better!

This patch will *not* work with Bugzilla 2.16, only the current tip of CVS.

        Attachment #96773 -
        Attachment is obsolete: true

    
    

    
  


  
This patch attempts to address the concern voiced in several comments and last
in #96 about dropdown variable population code being duplicated in many places.
 I've played a little with it but not fully.

Please beat on this one and let me know what breaks.  This was *really* easy to
do so something must have gone wrong... :)  I'm not going to obsolete 98381
yet.

Again this patch doesn't work with 2.16.

    
    

    
  
Another problem with attachment #96590 [details] [diff] [review] and v. 2.16:

In the show bug dialog, select two or more entries from the "Add CC:" list
(using shift-click or ctrl-click). After committing the change the "CC:" list
has an entry "__UNKNOWN__", it is also attempted to send email to that address.

    
    

    
  
Comment on attachment 98385 [details] [diff] [review]
Drop-down patch for the September 8 tip of CVS w/Consolidation.

>Index: CGI.pl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/CGI.pl,v
>retrieving revision 1.175
>diff -u -u -r1.175 CGI.pl
>--- CGI.pl	5 Sep 2002 07:28:28 -0000	1.175
>+++ CGI.pl	9 Sep 2002 01:40:08 -0000
>@@ -1021,6 +1021,30 @@
>     return(\@operations, $incomplete_data);
> }
> 
>+sub PopulateDropdownVars {

Maybe call this GetAllUsers, or something?

>+    # Extract profile information for everyone so it can be used in selections
>+    # and menus, only if something needs it.
>+    if (Param('assigned_todropdown') || Param('qa_contactdropdown') ||
>+        Param('initialownerdropdown') || 
>+        Param('newccdropdown') || Param('ccdropdown')) {

This function should take an argument, and you should then look up
Param("$type_dropdown"). Alternately, since the callers all have to check
anyway, just drop this extra check.

Hmm, although that may not be possible...

>+        my @profiles;
>+        my %profile;
>+        SendSQL("SELECT userid,login_name,realname FROM profiles where disabledtext = '' ORDER BY realname ");
>+        push(@profiles, "0");
>+        $profile{"realname"}{"0"}="---";
>+        $profile{"login_name"}{"0"}="---";

Thats an evil ugly hack. Have the template prepend that to the results instead.

>+        while (MoreSQLData()) {
>+            my ($userid,$login_name,$realname) = FetchSQLData();
>+            push(@profiles, $userid);
>+            $profile{"realname"}{$userid}=$realname;
>+            $profile{"login_name"}{$userid}=$login_name;
>+        }

This should be $profile->{$userid}->{<whatever>}, logically speaking. That
notation also allows for teh dropin replacement of a User object, once we have
one.

>+
>+        $vars->{'profiles'} = \@profiles;
>+        $vars->{'profile'} = \%profile;

Don't push stuff into $vars from a global function - return the pair of values,
and ahve the calling code do it.

Global vars are bad...

buglist.cgi should only call this function for multiple edit mode (ie
$dotweak=1). It should also be protected by an if statement for the only types
of 'stuff' you're interested in. (ie buglist isn';t interested in initial*,
etc) BTW, are the spearate params worth it?

Don't you want the cc select in teh template to be MUTLTIPLE?

As well, you need to think about how this interacts with bug 162990. Its
probably not an issue (the two are probably just mutually exclusive), but...

        Attachment #98385 -
        Flags: review-

    
    

    
  
Having looked at both the drop-down patch and thw wildcards (bug 162990),  I see
no case where the 2 would conflict.  Drop-down changes the construction of the
forms heavily and only changes the bug processing slightly.  Wildcards goes only
into the bug processing and drop-down's changes to bug processing would view it
exactly as if the user had manually entered the final addresses.

I am removing the dependency from this to bug 145499.  Bug 145499 will actualy
wind up depending on bug 162990.

The only other item of note is that Paul should probably watch the development
of User.pm from myk's request tracker. 

No longer blocks: 145499

    
  

        Attachment #98380 -
        Attachment description: Drop-down patch for 2.16 → Drop-down patch for 2.16 and 2.16.1

    
    

    
  
Hi -- I've got the patch in attachment #98385 [details] [diff] [review] working on 2.16 just fine.

I wonder if it's possible to also change the "Reassign to:" field to use the 
drop-down.  That's actually the one I want to change the most.

I'm going to try to hack it myself but I wonder if it's been left out for good 
reason? :)

    
    

    
  
*** Bug 174089 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 174621 has been marked as a duplicate of this bug. ***

    
    

    
  
Hi Paul,

Just want to report a problem on the change multiple bugs page with
patch 98380 on 2.16.

If you add one person as Cc: to several bugs, the add Cc: operation
works, but the QA Contact (on each bug) is replaced with "__UNKNOWN__"
(which is person id 0 in the database).

Let me know if you want more information and thanks for the patch.

Yours,

Tim

-----------------------------------------------------------------------
Tim Miller                Zero G Software, Inc.   +1 415 512-7771 voice
Director of Engineering   http://www.ZeroG.com/   +1 415 723-7244 fax



    
    

    
  


  
Drop-down patch for 2.16.1 that handles (very badly) the report cited in
comment #107.  It is otherwise unchanged.

        Attachment #98380 -
        Attachment is obsolete: true

    
    

    
  


  
This patch doesn't do anything different from 98381 and 98385 except fix the
problem noted in #107.	The hack is stupid and nasty but it works.  This needs
to be done much better when done for real.

It does encorporate the consolidation from 98385.

This patch will not work with the 2.16.1 release of bugzilla.

        Attachment #98381 -
        Attachment is obsolete: true

        Attachment #98385 -
        Attachment is obsolete: true

    
    

    
  
The problem described in comment #101 still persists with attachment #104095 [details] [diff] [review].

    
    

    
  


  
This patch is for release 2.16.1 and fixes the problem described in comment
#101.
Fortunately this wasn't a nasty hack :)

I say that the patches work for 2.16 but they're not tested with it.  I just
remember that the patch for 2.16 to 2.16.1 didn't touch the code I'm mangling.

As always, share & enjoy!

        Attachment #104095 -
        Attachment is obsolete: true

    
    

    
  


  
This patch fixes the problem described in comment #101.  Fortunately this
wasn't a nasty hack :)

This patch will not work with the 2.16 series releases.

As always, share & enjoy!

    
  

        Attachment #104096 -
        Attachment is obsolete: true

    
    

    
  


  
This is the actual patch mentioned in attachment #104216 [details] [diff] [review], not the script I use
to place stuff in my HTTP server's doc root. :)

This patch is for release 2.16.1 and fixes the problem described in comment
#101.
Fortunately this wasn't a nasty hack :)

I say that the patches work for 2.16 but they're not tested with it.  I just
remember that the patch for 2.16 to 2.16.1 didn't touch the code I'm mangling.

As always, share & enjoy!

        Attachment #104216 -
        Attachment is obsolete: true

    
    

    
  
attachment #104218 [details] [diff] [review] fixes the problem described in comment #101.

One minor discrepancy though: The "Add CC:" list in the show_bug.cgi page allows
multiple selections. The "CC:" list in the enter_bug.cgi page doesn't.

Unfortunately, it is not sufficient to just add the "multiple" attribute to line
161 of file create.html.tmpl. One can select multiple cc's then, but none of
them are stored in the db.

It would be nice to have identical behavior in both lists. It is not a bug
though, just different behavior.

    
    

    
  
After applying the patch (2002-10-25 17:39) to a clean 2.16.1 install, email
seems to be broken. Has anyone else noticed this? 

    
    

    
  
(You're gonna love this one)

I've gone through the patch twice and I don't see anything that modifies
anything relating to sending email. :)

Sorry.  What specificically do you see when you submit a change that (normally)
causes bugzilla to send an email?  Can you send an email from the machine normally?

Thanks!

    
    

    
  
i applied attachment #104218 [details] [diff] [review] (oct 25 17.39) to my install of 2.16.1 and it works
just fine (except for comment #114).  however something forgotten from the
patch: the "watchers list" inside of user email preference.  that is still a box
which says to enter in all the emails and separate with a comma.  no drop box is
present.

    
    

    
  
removing obsolete keywords (we have flags on the attachments now)

OK, now tha the product groups rearchitecturing is in (bug 147275), how about a
checkbox on the group control map for "can be assignee", which if any group is
checked, creates a drop-down menu with the members of that(those) group(s) in it
for the assigned-to field, and if no boxes are checked, it remains a freetext
field like it is now?
Keywords: patch, 
          
            review

    
    

    
  
WRT comment 118

This issue should be addressed in a way that covers both drop-down and wildcard
alike and scales well for large sites.  In the case of wildcard, it should limit
the users who the wildcard would be permitted to match.

A very closely related item is bug 145499.  These need to be thought about
together and I suspect that there will be a common approach that can be used for
all of them.

While we are at it, this should apply to flags as well so that it is possible to
indicate that only reviewers can be matched by a review request.



    
    

    
  
WRT Comments 118 and 119

Let me re-state my goals for this set of patches.  I am maintaining them so that
they will cleanly apply to the tip of CVS and (now) 2.16.1.  While I appreciate
the input for additional features and see the benefit they will provide, I don
t think it's a good idea for them to be applied to this code.  

In other words, I am not comfortable with the implementation as it stands now. 
I'd like someone from the bugzilla team to "properly" implement this patch, or
fix the existing implementation.  Then let's build improvements into it.

Oh, yeah.  Isn't it about time someone from the team took this it's been over
two years... :)

    
    

    
  
If I may make a suggestion, I think a better option would be to have the
drop down built by product (meaning that each drop down list would be
include only the users that belong to a product group).
One other way would be to include in the product definition a list of
developers associated with that product. You fill that at the beginning and
than the drop down is built from that list.


    
    

    
  
Is there any work happening to move this patch to 2.17.1 or 2.18. series. Am 
interested in trying this patch on a test installation of BZ 2.17.1.
Any help would be appreciated.



    
    

    
  
Unfortunately recent updates (past 3 months or so) have seen quite a number of
changes in the files I was updating.  I haven't had the chance to find where the
changes should go.

So, nothing has been done for a while, like since November...  If anyone wants
to pick this up who has time to find where everything goes, please feel free.

    
    

    
  
There appears to be a problem with the CC-list processing in the patch against
2.16.1.  (I'm actually running 2.16.2, but I don't think that should make any
difference.)  When I try to remove one user from the CC-list (nothing selected
in the add cc list), I get the following:

Software error:

Can't use an undefined value as an ARRAY reference at
/var/www/html/bugzilla/process_bug.cgi line 561.

Turning off the "newccdropdown" param fixes it.

    
    

    
  
I'm going to work on this tonight.  Hopefully I can make some progress, and do
it properly to boot...  Wish me luck!

    
    

    
  


  
This patch is only moderately tested and replaces patch #104217 for the current
tip of CVS.  This is the first workable patch since the file bug_form.pl was
removed from CVS and the Bugzilla::User class was used.

Please provide comments as usual.  Please make the comments fairly specific,
especially if you have an idea as to how to fix the problem you find.  If you
know how, provide a solution or point to an example on how to do it.  If you
don't know how, that's ok.

        Attachment #104217 -
        Attachment is obsolete: true

    
    

    
  


  
This patch is for release 2.16/2.16.1/2.16.2 and fixes the problem described in
comment #124.  I've verified with Matt that the patch actually does fix the
problem he described.

I've moved to 2.16.2 for testing this patch.  I'm assuming that moving to
2.16.2 is a good thing(tm) anyway.

        Attachment #104218 -
        Attachment is obsolete: true

    
    

    
  
Have these patches been updated for the 2.16 version of Bugzilla_Stable?  I'm having trouble patching it with the cvs updated version of Bugzilla.  The error log is as follows: patching file post_bug.cgi Hunk #1 succeeded at 118 (offset -3 lines). Hunk #2 FAILED at 268. 1 out of 2 hunks FAILED -- saving rejects to file post_bug.cgi.rej  and in the Assigned To and CC fields, I get HTML code (rather than a blank field)  Regards Hrishikesh  

    
    

    
  
Hrishikesh,
Please view attachment #112985 [details] [diff] [review] for the 2.16 series version of this patch.

That is, it should work.  The Bugzilla folks recommend that you upgrade to
2.16.2 anyway to fix some rather serious security bugs.  The 2.16 series patch
was generated from 2.16.2.

    
  

        Attachment #112985 -
        Flags: review?

    
  

        Attachment #112593 -
        Flags: review?

    
  

        Attachment #84137 -
        Attachment is obsolete: true

    
  

        Attachment #71685 -
        Attachment is obsolete: true

        Attachment #71685 -
        Attachment is patch: true

    
  

        Attachment #71491 -
        Attachment is obsolete: true

        Attachment #71491 -
        Attachment is patch: true

    
  

        Attachment #50270 -
        Attachment is obsolete: true

    
  

        Attachment #34300 -
        Attachment is obsolete: true

    
  

        Attachment #21731 -
        Attachment is obsolete: true

    
  

        Attachment #14638 -
        Attachment is obsolete: true

        Attachment #112985 -
        Flags: review?

    
    

    
  
OK, after reading this bug and the patch, here's a design for how I would
implement this feature.

The first thing to note is that aliases go some way to mitigating the underlying
problem of having to remember and type email addresses. So I would provide a
dropdown for the single-select fields - assignee and QA contact, on both the
enter_bug and show_bug pages, only.

The feature would be controlled by two params - assigneedropdowngroup and
qacontactdropdowngroup - which you set to the name of the group whose people you
want in the dropdowns for assignee and QA contact. A group is used to give you
maximum flexibility over who appears in each dropdown, so your engineers can
appear in the assignee dropdown, and your QA team in the QA dropdown. Bugzilla
would not permit values for these fields for people not in the relevant group.

Turning on assigneedropdowngroup would turn on the assignee dropdown for both
enter_bug and show_bug; similarly for the QA one.

There would be a new method on Bugzilla::User which would return an array of all
users in a particular group. This would get called, and the result passed into
the templates, if either param was enabled. The templates would then use a
single generic block to produce the dropdown.

I would not do anything for CC, for a couple of reasons of varying applicability
to a given situation:
- You might want to CC anyone (as opposed to assignee/QA contact, where it's
  more likely you want to select from a subset)
- You might want to CC multiple people at once, and the multiselect UI for that 
  would be problematic
- aliases mean that you don't have to type a load of long email addresses 
  anymore anyway.

I would not do anything for the admin interface because, compared to the main
interface, it gets very little use, and those CGIs are a mess anyway and need
rewriting rather than adding to.

I'd estimate this as four or five hours work.

How does this strike people?

Gerv


    
    

    
  
sounds good to me.  There'd probably be a lot of benefit to having the group
selection be per-product instead of global though.

    
    

    
  
That makes it much harder to configure, though, because you have to define an
arbitrary number of groups. You could attach it to the products table, but
that's mixing configuration information with data. 

IMO, if and when we have a proper mechanism for configuring things per-product,
we can switch this over to use it. The migration would be trivial :-)

Gerv

    
    

    
  
Has comment #114 been addressed yet?  It would be great to CC many people at
once when submitting a bug.  Currently, I have to submit a bug and then go back
and Add CCs.  Because of this, the "new" CCs only get an email stating that I
added them....and the actual bug report doesn't get any real exposure.

    
    

    
  
H Z - I believe comment #114 has been fixed, however I'd suggest trying it. 
I've looked at the patch and the code looks like it handles this case (circa
line 337).  Please let me know if you disagree.

PS: The current patches still apply to the tip of CVS.

    
    

    
  
I just installed the patch on my system yesterday and the bug entry form does 
not allow multiple selections for the CC list.

    
    

    
  
Ok, I (HZ) see where the multiple CC's are addressed on line 337 of the
"Drop-down patch for January 24 tip of CVS" attachment.  My problem is that I
installed the "Drop-down patch for 2.16, 2.16.1, and 2.16.2" attachment.  I did
this because I have 2.16.2 installed and also because it is the latest patch
listed (chronologically).  Should I use the CVS tip version?  Sorry for the
question, but I'm in no way an expert on this!

    
    

    
  
Line 418 of the "Drop-down patch for 2.16, 2.16.1, and 2.16.2" corresponds to 
line 337 of the "Drop-down patch for January 24 tip of CVS."  The multiple CC
selection does not work...at least not with the 2.16.2 patch on my installation. 



    
    

    
  
Ok, I'll look at the new-bug CC problem.  I maintain the patches for the tip of
CVS and for the 2.16.x series at the same time so both should function the same way.

    
    

    
  
I've fixed the new bug multiple-CC recipients problem.  Unfortunately I'm not in
a place where I can generate the diffs.  I'll do that tonight.

    
    

    
  


  
This patch was created against the current tip of CVS.	The patch should apply
to CVS from before January 24, 2003 as the bugzilla code around these areas
hasn't changed in a while.

This patch fixes various comments above concerning adding CC people from the
new bug form from a multiple-input list.  Please test and provide feedback!

        Attachment #112593 -
        Attachment is obsolete: true

    
    

    
  


  
This patch was created against the 2.16.2 release.  The patch should apply to
2.16.1 as the bugzilla code around these areas didn't change between these two
releases.

This patch fixes various comments above concerning adding CC people from the
new bug form from a multiple-input list.  Please test and provide feedback!

    
  

        Attachment #112985 -
        Attachment is obsolete: true

    
    

    
  
I applied the latest patch for 2.16.2, and I am still not able to select
multiple CC's from the New Bug form.  However, the "Add  CC" multiple selection
still works fine on the Show Bug form.  I tested it on Mozilla 1.3 and IE 6.0. 

    
    

    
  
I don't understand, it should work.  Wah.  Anyone else been able to try it?

    
    

    
  
The "MULTIPLE" attribute for the CC: list is missing in the template.

After modifying line 158-161 of file
template/en/default/bug/create/create.html.tmpl to

        [% PROCESS selectprofile selname => "cc"
                                 selitem => profile.realname
                                 selvalues => profiles
                                 xtra => "MULTIPLE size=3" %]

it seems to work in 2.16.2. I did a total of 1 (one) tests though.

    
    

    
  


  
Re: comment #144: If that wasn't *the* problem it was one of them.  Fixed in
this patch.

        Attachment #119988 -
        Attachment is obsolete: true

    
    

    
  
Almost!  The latest patch (4-11-2003) allowed me to pick multiple CC's in the
New Bug form.....but no-one was actually CC'd.  My selection was ignored.  I
tried it a few times with no success.

    
    

    
  


  
I don't see that much changed between this morning's patch and this one but
this one I tested (finally) and it does work.  I can provide screen shots to
prove it ;).

Seriously, the only changes made in this patch were to put quotes around some
FORM{'literal'} and MFORM{'literal'} and add a 'my $ckperson;'	Nothing serious
but please double-check the parameters and be sure to ./checksetup.pl

Thanks again!

        Attachment #120196 -
        Attachment is obsolete: true

    
    

    
  
attachment #120268 [details] [diff] [review] works for me (2.16.2). Thanks!

There is still a problem for users which do not have a real name, only a login
name. They show up as an empty row in the drop down list (see comment #26).

An inconsistency: in the show_bug.cgi window, the "Add CC:" list displays the
real names, but in the "CC:" list the login names (email addresses) are shown.
This could be confusing, as email addresses might look completely different from
the real names.

    
    

    
  
The problem described in comment #50 and comment #54 still exist:

The "reassign bug to" dropdown is currently tied to the "initial owner dropdown"
parameter flag. I believe both "Assigned to " and "Reassign bug to" fields in
the enter_bug.cgi and show_bug.cgi dialogs should be tied to the
"assigned_todropdown" parameter flag.

The "initial owner dropdown" parameter flag should only be used to control the
dropdown in the editcomponents.cgi dialog, just as the "initial qa contact"
parameter flag.

    
    

    
  
Re: 2nd half of comment #148... I don't know enough about how the templates work
to reference an outside variable indexed by a counter, c in this case.  Anyone
with suggestions on this?

    
    

    
  
Attachment #120268 [details] [diff] works for me also.  Thanks.

    
    

    
  


  
Fix for comment #148 (first half) and comment #149.  I believe there was a
discrepancy between the parameters throughout this code.  This situation should
be better now.

        Attachment #119987 -
        Attachment is obsolete: true

    
    

    
  


  
Fix for comment #148 (first half) and comment #149.  I believe there was a
discrepancy between the parameters throughout this code.  This situation should
be better now.

        Attachment #120268 -
        Attachment is obsolete: true

    
    

    
  
In the show_bug.cgi dialog, when using the "Resolve bug, changing resolution to
FIXED etc." dropdown menu, the corresponding radio button is automatically
selected when using the menu.

This is not the case with the "Reassign user" dropdown menu. I would like to
have the same behavior for both dropdown menus: using the menu selects the
corresponding radio button.

    
    

    
  
Arne, I agree and it's been broken for some time now.  However, the javascript
to do that broke when it was put in a parameter to the function that generates
the HTML.  I tried a while ago to fix it but I was unsuccessful.  This is
another place where someone with more template knowledge would be helpful.

    
    

    
  
This line in edit.html.tmpl works (sort of):

xtra => "onchange=\"if ((this.value != '$bug.assigned_to_email') && (this.value
!= '')) { document.changeform.knob[$knum].checked=true; }\""

I could not get FILTERing working, so this is probably unsafe. 

Also, $bug.assigned_to_email has the email address, whereas this.value has the
real name, so it does not work as intended.

    
    

    
  
The whole if() clause belongs to the non-dropdown case, where the user must
enter the email address. It is not needed when using a dropdown menu.

Maybe we should get rid of this and only use the
document.changeform.knob[$knum].checked=true statement?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch for 2.17.4 (obsolete)
        — 
          — Splinter Review
      

    


  
Patch for 2.17.4, with the following fixes:

1. auto-select of the corresponding radio button when selecting the dropdown. I
changed the javascript code as explained in comment #157.

2. fixed problem in comment #148 for editcomponents.cgi too.

3. dropdown menus are incompatible with the user match function. I added a
warning text to defparams.pl, but did not fix the real problem. This needs some
extra work.

    
    

    
  
The array of user names for the dropdown menu contains the string "---" for "no
selection". This array is reused for the CC: lists too, but the "---" string is
not needed here. If this entry is selected, nothing happens so this issue is
somewhat cosmetic.

    
    

    
  
The '---' entry is a remnant from the original patch.  It's actually used in
template/en/default/bug/create/create.html.tmpl to accept the default component
assignee for the new bug.  A special case could be used there I suppose.  I'll
leave the decision on that one up to you.

    
    

    
  
I think it would be very useful if the default component owner was selected in
the dropdown box when creating new bugs, instead of having the special '---' entry.

It makes it easier to see if you need to change it or not.

    
    

    
  
*** Bug 206892 has been marked as a duplicate of this bug. ***

    
    

    
  
Jens - The identification of a default owner for a bug is determined by the
product component.  Unfortunately the component isn't known to bugzills until
the bug is submitted.  At that point it can be queried from the database and the
assignment made.

If the drop-down list were to be automatically selected for the proper owner,
javascript would need to be written to monitor the selection of the component
and alter the default assignee field.  While possible, I'm not sure this is
really the direction we want to go.

Other thoughts?

    
    

    
  
Ah yes of course.

You're probably right that the javascript way is too heavy. The interface
already does a lot (making is slowish on older computers).

    
    

    
  
Hmmm, so I've downloaded patch 120525 and applied it to 2.16.3 (maybe that's the
problem), but I'm seeing problems now.  Some of the drop-down menus are showing
correctly, such as initial owner.  The rest aren't showing up, most noteable the
reassing field, and yes, I've checked them in the parameter page, and now, when
I try to reassign a bug, I get the nice bright red banner saying:
You cannot reassign to a bug to nobody. Unless you intentionally cleared out the
"Reassign bug to" field, this may indicate a bug in your browser.

So, any thougts on why this is screwing up?

thanks

    
    

    
  
Re: comment 163 - on the cvs-tip (and I think in 2.17.4 though I'm not positive)
the javascript containing the default owners for each component is already
loaded with the bug form.  If you change the component, it automatically fills
the textbox with the default owner for that component.  If you change the
textbox to a list box and the values of the options in that listbox are the
email addresses, then it should be a piece of cake to change that to select it
from the popup instead of filling a text filed :)

    
    

    
  
Re: comment #165 - Robert, I'm sure it's something I've messed up somewhere. 
Can you tell me if the previous 2.6.2 patch, 120268, behaves correctly?  If it
does, I can figure out what's up pretty quickly.

Re: comment #166 - Dave, I haven't had much luck with javascript (see comment
#155 :).  Is this something where I can get some aid in fixing?   Or at least an
example?

    
    

    
  
Re: comment 167. Okay, I tried patch 120268, and I still get the same
behavior--ie. when I try to reassign a bug, I get the bright red banner.

    
    

    
  
Bug in attachment #121919 [details] [diff] [review]:

If the assigned to dropdown option is not selected, then entering a new bug
fails with a large red error message.

Fix:

In template/en/default/bug/edit.html.tmpl, change all occurrences of
assigned_to_email into assigned_to.email .

This bug is probably in the 2.16.x patch too, didn't check that.

    
    

    
  


  
Re: Comment #169.  This makes the change recommended for assigned_to_email ->
assigned_to.email.  I don't currently have an active 2.16.x installation so
I'll need to install one before I can verify the change in that environment.

This also includes the additions/changes from attachment #121919 [details] [diff] [review].

        Attachment #120523 -
        Attachment is obsolete: true

        Attachment #121919 -
        Attachment is obsolete: true

    
    

    
  
Re comment 168 and comment 169:  The problems I was having had to do with the 
fact that I had templates in /en/custom that weren't getting the patch applied 
to them.  After renaming them, so as to use the /en/default templates instead, I 
get the correct drop-down behavior.  Thanks for the help.

-bob

    
    

    
  
Sorry if this comment is long or re: a stupid problem, but I get a bunch of
patch errors when I attempt to apply patches attachment#120525 [details] [diff] [review] or
attachment#127445 [details] [diff] [review] to Bugzilla 2.16.3.  What am I doing wrong?  Any specific
patch options?  Should I just revert to 2.16.2?  I have no other patches applied.  

 > patch < ../archive/bugzilla-52557.120525.patch
patching file CGI.pl
patching file bug_form.pl
patching file buglist.cgi
patching file defparams.pl
Hunk #2 succeeded at 386 (offset 1 line).
patching file editcomponents.cgi
patching file enter_bug.cgi
patching file globals.pl
Hunk #2 succeeded at 944 (offset 1 line).
patching file post_bug.cgi
patching file process_bug.cgi
can't find file to patch at input line 571
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|Index: template/en/default/bug/edit.html.tmpl
|===================================================================
|RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v
|retrieving revision 1.7.2.5
|diff -u -u -b -B -r1.7.2.5 edit.html.tmpl
|--- template/en/default/bug/edit.html.tmpl     9 Jul 2002 01:17:59 -0000      
1.7.2.5
|+++ template/en/default/bug/edit.html.tmpl     15 Apr 2003 01:30:40 -0000
--------------------------
File to patch: ^C

 > patch < ../archive/bugzilla-52557.127445.patch
patching file CGI.pl
Hunk #1 succeeded at 1091 with fuzz 2 (offset 616 lines).
patching file buglist.cgi
Hunk #1 succeeded at 1554 (offset 751 lines).
patching file defparams.pl
Hunk #1 FAILED at 24.
Hunk #2 FAILED at 633.
Hunk #3 FAILED at 1080.
3 out of 3 hunks FAILED -- saving rejects to file defparams.pl.rej
patching file editcomponents.cgi
Hunk #2 succeeded at 136 (offset -2 lines).
patching file enter_bug.cgi
Hunk #2 succeeded at 379 with fuzz 2 (offset 21 lines).
patching file globals.pl
Hunk #1 FAILED at 23.
Hunk #2 succeeded at 944 (offset 156 lines).
1 out of 2 hunks FAILED -- saving rejects to file globals.pl.rej
patching file post_bug.cgi
Hunk #1 succeeded at 22 with fuzz 2.
Hunk #2 FAILED at 127.
Hunk #3 FAILED at 223.
Hunk #4 FAILED at 524.
3 out of 4 hunks FAILED -- saving rejects to file post_bug.cgi.rej
patching file process_bug.cgi
Hunk #1 succeeded at 23 with fuzz 2.
Hunk #2 FAILED at 746.
Hunk #3 succeeded at 544 (offset -280 lines).
Hunk #4 FAILED at 643.
2 out of 4 hunks FAILED -- saving rejects to file process_bug.cgi.rej
patching file show_bug.cgi
Hunk #1 FAILED at 104.
1 out of 1 hunk FAILED -- saving rejects to file show_bug.cgi.rej
can't find file to patch at input line 523
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|Index: template/en/default/bug/edit.html.tmpl
|===================================================================
|RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v
|retrieving revision 1.33
|diff -u -u -b -B -r1.33 edit.html.tmpl
|--- template/en/default/bug/edit.html.tmpl     3 Jul 2003 21:31:37 -0000       1.33
|+++ template/en/default/bug/edit.html.tmpl     10 Jul 2003 12:56:31 -0000
--------------------------
File to patch: ^C



    
    

    
  
The attachment #120525 [details] [diff] [review] works just fine. Do not forget the "-p0" option. 

/home/arne/tmp/bugzilla-2.16.3 $ patch -p0 < ../../rel2.16.2.patch 
patching file CGI.pl
patching file bug_form.pl
patching file buglist.cgi
patching file defparams.pl
Hunk #2 succeeded at 386 (offset 1 line).
patching file editcomponents.cgi
patching file enter_bug.cgi
patching file globals.pl
Hunk #2 succeeded at 944 (offset 1 line).
patching file post_bug.cgi
patching file process_bug.cgi
patching file template/en/default/bug/edit.html.tmpl
patching file template/en/default/bug/create/create.html.tmpl
Hunk #2 succeeded at 139 (offset 1 line).
patching file template/en/default/list/edit-multiple.html.tmpl

    
    

    
  
Doh!  Thanks Arne!


    
    

    
  
I'm using the CVS tip as of today (Sep 17 2003). There seem to be a bunch of
things wrong with the July 10 patch above:

1. In template/en/default/bug/edit.html.tmpl in the patch, the line
      [% PROCESS select selname => "version" accesskey => "v" xtra -> "" %]
   causes an error in the template toolkit (2.08). The -> should be =>, no?

2. After I do this, any attempt to change any user causes a "Match Failed"
error, saying things like
      0  did not match anything
      3  did not match anything.

  For any actual users that I select in drop-down lists, it gives the userid. If
the field is "--" (e.g. for the reassign field), I get the 0 message.

3. I patched Bugzilla/User.pm as follows:
Index: Bugzilla/User.pm
===================================================================
RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v
retrieving revision 1.16
diff -u -r1.16 User.pm
--- Bugzilla/User.pm    14 Sep 2003 06:05:10 -0000      1.16
+++ Bugzilla/User.pm    17 Sep 2003 23:12:29 -0000
@@ -383,7 +383,8 @@
         my $sqlstr = &::SqlQuote($str);
         my $query  = "SELECT userid, realname, login_name " .
                      "FROM profiles " .
-                     "WHERE login_name = $sqlstr ";
+                     "WHERE login_name = $sqlstr " .
+                     "OR userid = $sqlstr ";
         # Exact matches don't care if a user is disabled.

         &::PushGlobalSQLState();


This gets rid of the "nnn did not match anything" message for non-zero nnn's,
but still moans about "0 did not match anything" when any of the fields is "--"
(e.g. when not reassigning the bug).

4. Even if I give values to all of them to get past this "0 did not match
anything" message (effectively forcing a reassign), I then get sent to a screen
that asks me to confirm each match (though each matches exactly 1 entry, as it
should).

At this point, I'm starting to get lost in the deep waters. Has anyone else
tried this patch on a recent tip of CVS?  What am I missing?

    
    

    
  
Shankar - You haven't missed much.  I haven't updated the patch and the source
has moved out from under it.  I'll see what I can do, but I must admit that I'm
getting frustrated with it. :)

    
    

    
  
Shankar - I spoke too soon.  The patch should have applied cleanly, but I see
now that you're talking about problems *using* the code...  Stupid me :)

The code should catch the '--' user case before it even gets to the SQL code
because that's supposed to be a marker saying that the user didn't change this item.

From the messages you've seen my impression is that they've added features to
bugzilla itself and the templates that my code doesn't use properly.

I can't say that I'm really good at the template stuff or the changes to the
system since July (or before).  So, it'll take me longer to figure out what's
gone wrong and fix it.

    
    

    
  
I think the problem is there because the Value of the dropdown is 0... 0 does
not match any ids, and returns an error.

    
    

    
  
Let me just clarify... The -- on the dropdown actually translates to a 0, value
(just do view source on your browser when you bring up a bug). then the
DBIdCheck method gets the email address for 0 userid and assigns it to the
assign_to field on the dropdown... 0 userid does not correspond to a user, so an
error is returned...

I cant figure out the template which creates the dropdown list... Where exactly
is the --,0 added? You just need to put --,'' instead....

    
    

    
  
Ok, found the bugger.

there is a match method thats being called at the begging of execution in
process_bug.cgi
&Bugzilla::User::match_field({
    'qa_contact'                => { 'type' => 'single' },
    'newcc'                     => { 'type' => 'multi'  },
    'assigned_to'               => { 'type' => 'single' },
    '^requestee(_type)?-(\d+)$' => { 'type' => 'single' },
})

That match method will check if the ids match a regular expression.

the ids then get converted to email addresses automatically....

change the match in that to the following:


if (! Param('assigned_todropdown')) {
  &Bugzilla::User::match_field({
      'qa_contact'                => { 'type' => 'single' },
      'newcc'                     => { 'type' => 'multi'  },
      'assigned_to'               => { 'type' => 'single' },
      '^requestee(_type)?-(\d+)$' => { 'type' => 'single' },
  });
} else {
  &Bugzilla::User::match_field({
      'qa_contact'                => { 'type' => 'single' },
      'newcc'                     => { 'type' => 'multi'  },
      '^requestee(_type)?-(\d+)$' => { 'type' => 'single' }
  });
}


and it will work (for the assign_to at least)

The whole drop down can be rewritten to use the match method instead of the
DBIdCheck instead of the various ifs in process_bug.cgi.

Perhaps i will try and do it if i get a chance.

    
    

    
  
I've discovered a problem with this patch on my 2.16.3 installation with the
edit-multiple.html.tmpl form. Actually, my original 2.16.3 installation had a
problem where adding CC: addresses to multiple bugs did not work at all, 
removing CC: addresses worked fine, however would only allow removing one CC: at
a time from multiple bugs. After I applied the dropdown patch for 2.16.3, I
turned on the parameters for ccdropdown and newccdropdown, and then noticed that
there was no dropdown on the edit-multiple.html.tmpl form and that neither
adding or removing CC: addresses from multiple bugs worked any longer. So, I
took it upon myself to try and fix this problem and I think the patch shown
below does the trick (YMMV). I'm rather new at this so if this isn't the right
format or procedure for submitting a proposed patch, please let me know. Thanks.
/dave

---
/mnt/av148/local/bugzilla-2.16.3/template/en/custom/list/edit-multiple.html.tmpl
   2003-10-06 12:27:28.000000000 -0700
+++ template/en/custom/list/edit-multiple.html.tmpl     2003-10-13
11:25:59.000000000 -0700
@@ -134,9 +134,18 @@

   <tr>

+    </td>
+  </tr>
     <th><label for="masscc">CC List:</label></th>
     <td colspan="3">
-      <input id="masscc" name="masscc" size="32">
+      [% IF Param("ccdropdown") %]
+        [% PROCESS selectprofile selname => "masscc"
+                                 selitem => profile.realname
+                                 selvalues => profiles
+                                xtra => "MULTIPLE size=3" %]
+      [% ELSE %]
+        <input id="masscc" name="masscc" size="32">
+      [% END %]
       <select name="ccaction">
         <option value="add">Add these to the CC List</option>
         <option value="remove">Remove these from the CC List</option>
--- /mnt/av148/local/bugzilla-2.16.3/process_bug.cgi    2003-10-06
12:17:06.000000000 -0700
+++ process_bug.cgi     2003-10-13 13:30:40.000000000 -0700
@@ -550,9 +550,9 @@
     my ($cc_add, $cc_remove) = "";
     if (defined $::FORM{'masscc'}) {
         if ($::FORM{'ccaction'} eq 'add') {
-            $cc_add = $::FORM{'masscc'};
+            $cc_add = join(',',@{$::MFORM{'masscc'}});
         } elsif ($::FORM{'ccaction'} eq 'remove') {
-            $cc_remove = $::FORM{'masscc'};
+            $cc_remove = join(',',@{$::MFORM{'masscc'}});
         }
     } else {
         if ( ! Param('newccdropdown') ) {
@@ -590,8 +590,18 @@
     if ($cc_remove) {
         $cc_remove =~ s/[\s,]+/ /g; # Change all delimiters to a single space
         foreach my $person ( split(" ", $cc_remove) ) {
+            if (! Param('newccdropdown')) {
             my $pid = DBNameToIdAndCheck($person);
             $cc_remove{$pid} = $person;
+            } else {
+                my $pid = DBIdCheck($person);
+                my $person = DBID_to_name($pid);
+                # Since there's no use in sending mail to __UNKNOWN__,
+                # make sure the person exists and the pid isn't 0.
+                if ($person ne "" && $person ne "__UNKNOWN__" && $pid > 0) {
+                  $cc_remove{$pid} = $person;
+                }
+            }
         }
     }
 }




    
    

    
  
I've got similar problems as discribed in #172. Can anyone tell me what is 
wrong? I thought it's permissions but i tried it on a fresh installation 
(version 2.17.4) and still i get the errors.

        Attachment #112593 -
        Flags: review?

    
    

    
  
attachment #127445 [details] [diff] [review] does not work with 2.17.5. The patch fails with one file, and
the drop down menus in the other files do not work anymore. 

    
    

    
  
Does anyone have a jumbo patch for this against 2.17.6?

    
    

    
  
Bug 102852 has the right idea. All of the picker code is external with no need
to patch CGI.pl.

    
    

    
  


  
I have modified the patch against 2.16.2, the new version works with 2.16.4.
I had some problems with the old patch, some operations did not work correctly
(e.g. QA contact was always displayed as "---", changinge several bugs at once
did not work), all known problems are fixed.

    
    

    
  


  
This patch also includes a patch for bug 225221. I was too lazy to remove that
patch from this one
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20

    
    

    
  


  
Cleaned up patch for application to 2.17.7

    
    

    
  
Comment on attachment 147665 [details] [diff] [review]
Drop-Down Patch for 2.17.7

Couldn't you pass the name of the param itself to selectprofilend block and not
have to place conditionals all over the place?

    
    

    
  
Just wanted to make a note here about bug 250380, which seems to be caused by a
bug in some version of the 2.16 patch for this bug.  However, I'm pretty sure I
don't have the most up-to-date patch (attachment 120525 [details] [diff] [review]) on my installation.  In
diagnosing the bug w/ justdave on IRC, it sounds like the most recent patch
would only cause the bug I saw if the "ccdropdown" param was turned off.

    
    

    
  
/me mumbles something about people checking for duplicates before filing new bugs

This is about to land, but the person who did it filed a new bug and managed to
push the patch through review before the dupe was discovered.


But the good news is: 2.19.1 and up will include this feature built-in :)

*** This bug has been marked as a duplicate of 251669 ***
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → DUPLICATE
Target Milestone: Bugzilla 2.20 → ---
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: