In addition to specifying a default owner for a component, we'd like to be able to specify a CC list too. Scenarios in which this would be useful include: - the default owner is not reading their email (on holiday, say), and a new bug is raised unnoticed. - the component is a place holder for sub-components which are owned by different people, but the person raising the bug is unlikely to be able to determine which sub-component is at fault. We'd want to put all sub-component owners on the default CC list. An alternative would be to specify a maining list as the default owner, but this would remove the element of personal responsibility.

alternatively, there could be an option to "e-mail me on NEW bugs in this component but don't add me to cc".

This feature would be great for modules relying on volunteer work, like XSLT. This way, contributors could easily grab bugs, but remove themselves from CCs if they wan't to reduce their spam. It is more public to the reporters, and better to maintain than watching the QA or default owner. Axel

One way would be bug #14137, but that is defined by the user rather than the sysadmin, which would probably be better for this. Jesse's slightly different version is over at bug #53717.

The previous patch adds a field to the database (using ./checksetup.pl) to hold an initial cc list for each component. It does not affect a persons ability to add someone to the cc list while reporting a bug. Also, if a person is both in the initial cc list and entered on the form, they will only be entered once. This initial cc list can not be overriden at reporting time (by design). Because of that, this patch is also a solution to making sure that the default owner gets an e-mail no matter what (of course, they'll be both the owner and on the cc list if the owner filed is left blank when the bug is reported). This patch also contains a minor but unrelated fix for "missing" not displaying correctly for the Milestone URL on the delete component page. (Also, please excuse the missing "ow" [should be "Allow"] in the attachment description.)

Being that 2.12 is getting close to being done I don't think this is gonna be included (it's a kinda bug patch). I'll update it again when the 2.12 tarball comes out so it will apply clean.

moving to real milestones...

See also bug 76794, "RFE: Implement the ability to watch components".

-> Bugzilla product, Administration component.

What's the status of this? I just came to Bugzilla to submit the very same RFE... One other scenario when this is useful not mentioned in the original report is when a group of people is working on a single component and they all want to be aware of what's going on with it. The big plus of this approach (compared to, say bug bug 76794) is that it allows people to remove themselves from the CC list once they realize they are not interested in a particular bug.

The patch attached to this bug doesn't allow users to be able to add/remove themselves from the default CC list. The schema it uses is quite hackish and really doesn't provide a good way to add that ability (I figured it was OK if only the admin was modifying it, but if each user is able to modify their own settings, collisions are bound to happen and this schema is not capabale of handling those). Removing patch and review keywords and reassigning to default owner (I may still work on it, but it'll be a ways off :)

Note that allowing users to add/remove them to/from the list is bug 76794 (that depends on this one).

Jake, I think these two are independent. I don't think the user should be able to access this list. Sometimes admins want total control over this stuff, both by forcing a user to receive mail, and by preventing them receiving mail if they're not on the list (if the bugs were confidential).

Matthew, I completely agree that the two should be kept independent. Can we get this patch (or something similar) in and then move the discussion on giving users ability to add/remove themselves to/from the list to bug 76794? P.S. I've built RedHat packages with bugzilla-2.14 + this patch, get them at http://nogin.org/RPM/bugzilla.html if you are interested.

Comment on attachment 32548 [details] [diff] [review] DIFF for 2.12 (should apply clean to the tarball) This patch uses comas as CC list separator, it should use "[ ,]" instead (to be consistent with the way post_bug.cgi parses the CC list).

I updated the previous patch to - cleanly apply to current CVS trunk - use [ ,] as CC list separator re instead of just , - display initial CC list in editcomponents.cgi more consistently I also moved a few lines of recurring code to globals

Does this bug depend on bug 73665, 'Need an "emailprefs" table'? It seems like that bug could cover what is wanted by this one. Athough, it seems like the management in that case would be done from the user's standpoint rather than from the component's.

> Does this bug depend on bug 73665, 'Need an "emailprefs" table'? Steve, I do not think so. This bug is specifically for admin-side setting and there is already a patch here that does not use emailprefs. For user-modifiable default CC lists, see bug 76794.

Comment on attachment 57397 [details] [diff] [review] Slightly updated version of the previous patch; against the current CVS This is interesting, but the patch needs an update to be valid against the current tree. That shouldn't be too much work, though; most of it was fine.

I've refreshed the patch to work with the current CVS trunk.

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

-> ayn2@cornell.edu who made the newest patch. Review coming up.

Comment on attachment 86893 [details] [diff] [review] A patch to add a default CC list to each component. >+# 2001-04-28 Adding the ability to have a default CC list for each Note: This date needs to be updated (should be done on checkin anyway). >- components.initialqacontact,components.description >+ components.initialqacontact, components.initialcclist,components.description Although adding the space is good for clarity in itself, let's preserve style here. No space between commas, then. >+ foreach my $name(split(/[ ,]+/,$initialcclist)) { >+ push(@cclist, DBNameToIdAndCheck($name)); >+ } The errors produced by this check have a double header. >+ if (($ccid ne "") && !$ccids{$ccid}) { >+ SendSQL("INSERT INTO cc (bug_id, who) VALUES ($id, $ccid)"); >+ $ccids{$ccid} = 1; >+ push(@cc, DBID_to_name($ccid)); You can't do this here; you don't have "$id" at this point when posting a new bug. Why insert at this point? ccids is being handled later on anyway. Also, editcomponents' index page has now a new column "initial cc list". That's fine, but you need to increase the colspan of the add link below the table, too. Now the add link is column too left.

I am afraid I would not have time to look into this for the next couple of months, so reassigning back to default owner. Jake, may be you could take it over?

This would be handy, but the list of userids probably needs to be in a seperate table rather than a text field.

Taking

I've updated the patch to apply cleanly to the current trunk and fixed the flaws found by Jouni Heikniemi (see comment #24). Anybody care to review? Thanks!

I think that the correct approach to the problem that you are trying to solve is component watching: bug 76794. So, I would say (and I'm sorry that you've done the work) that this shouldn't be checked in. :-| Gerv

> I think that the correct approach to the problem that you are trying to solve is > component watching: bug 76794. Well, the bug 76794 *depends* on this one! This bug is about a) Creating a default CC list b) Making such list editable by admins The bug 76794 is about c) Letting users add/delete del to/from such list. So obviously (c) depends on (a)!

I'm sure it's not intentional, but comment #30 has a little mis-information. Bug 76794 isn't blocked by this bug (at least it shouldn't be). Bug 76794 is dependent on bug 73665 which is re-doing part of the email prefs code. The new "emailprefs" table will contain the ability to watch more than just individual bugs. What you'll end up doing is actually watching the component, not being cc'd on a bunch of bugs.

To further confuse matters, I have been watching this with the intent of adding a capability to have a list of people automatically notified when a bug is created on a component, but not automatically CC'd.

> Well, the bug 76794 *depends* on this one! This bug is about > a) Creating a default CC list > b) Making such list editable by admins > > The bug 76794 is about > c) Letting users add/delete del to/from such list. So obviously (c) depends on > (a)! That's not right :-) This should be implemented so that users go to their prefs, and can select a number of components to watch. Once selected, they get all mail about changes to bugs in those components. There is no need for a "list for each component" - you'd have a table mapping userids to component IDs. There's no need for it to be admin-editable, either - people can choose which components they want to watch themselves. Again, I say that this is going to duplicate functionality we are going to implement another way, and so it shouldn't be checked in. Gerv

OK. Forgetting, for the moment, about which bug this should be under, there needs to be a component_user table that can express..... a) user being added to the CC list of a component by default (only when bugs are created or moved into the component) b) user being notified when a bug is created or moved into the component. c) user watching evrything in the component And this should be controllable by the user and the admin.

> a) user being added to the CC list of a component by default (only when bugs are > created or moved into the component) > b) user being notified when a bug is created or moved into the component. > c) user watching evrything in the component > > And this should be controllable by the user and the admin. Do we implement c) in terms of a)? I.e. do we implement Component Watching as "Added to CC list of all new bugs"? Advantage: You can un-watch bugs you aren't interested in Disadvantage: Watching a component is not retroactive Why do admins need to be able to force people to watch a component? Gerv

> Why do admins need to be able to force people to watch a component? Not force, but just set a default. If I create a new component and I know that there are 5 people working on it who should all be interested in seeing all the bugs, I want to be able to specify those 5 people right away when creating a component. Which is what this bug is really about. Another example - when a component is split up in two, it may be a good idea to duplicate the CC lists... Basically, when admin knows who are the right default CC people, admin should be able to specify that (as opposed to forcing each of those CC people to add themselves).

Bug 76794 will impliment the b) and c) from comment #34. It will do this by applying the current email prefs (such as the one for the cc list) to any type of watch. So when you decide to watch the Browser/General component, you can choose to only watch the add/remove event (by unchecking the rest of the prefs for that watch). But, it doesn't do anything for a).

WRT comment 37: Not exactly. Somewhere, I should be able to specify that I want to do a complete watch of some components and only be notified on creation of others. If we look at this group of requests with a common thread in mind, it is clear that there is a need for a user_component_map of some sort that permits all 3 types of relationship.

> Not exactly. Somewhere, I should be able to specify that I want to do > a complete watch of some components and only be notified on creation > of others. There are two alternative approaches and each has it own advantages. Yes, your "user_component_map" would allow users to specify preferences as you've described above. On the other hand, the "initial CC list" approach that this bug advocates would allow users (as soon as this patch is checked in) to decide on per-bug basis - e.g. the members of the default CC list are added to the CC list at bug creation, and afterwards they can remove and re-add themselves from/to CC list at any point in the life of the bug. In your approach, there does not seem to be a way to exclude certain bugs while watching the component.

I guess I should clarify... I'd like to be able to do much less than watching. I'd like to get a single email when the bugs if initially created or moved to the component, then never hear about it again unless I react by adding myself to the CC list.

joel: You could do that by only selecting a 'new bugs' checkbox for watching in user_prefs. That table should have one row per watched entity.

Removing approval request, I don't see anything here to approve yet (there's lots of arguing going on and no clear definition of what the intention is to check in), not to mention no patches with review yet. I can imaging an admin wanting to force default CCs in a corporate environment, so there may be some substance to that end of this. But there's obviously enough objection to it that in needs to be a configuration option... sorta like QA contact. Someone with tweakparams can decide whether the people with editcomponents can see it or not when they're editing components. Trying to think of other possible ways we might accomplish this... How about allowing an admin from editcomponents to edit the watch list for that component, and also allowing a per-bug "anti-CC" that would override your watching preferences...

Comment on attachment 102115 [details] [diff] [review] A patch to add a default CC list to each component, v 5 Looking at all the things that need to be assicated with various userids by component, this truly needs to be out of the comma-seperated list and into a table that can scale properly and can be queried reliably. Then, the various other items that have been mentioned in the comment thread become incremental additions rather than complete duplications. That part aside, this is on the right track.

Updated to work with recent Bugzilla releases (mainly fixed the bitrot caused by bug 43600 commit). This patch still uses a single string for the whole CC list as opposed to using a separate table. I do not think I'll have time to rewrite it to use the table, so feel free to take over.

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

Please free to take over.

.

Comment on attachment 109883 [details] [diff] [review] A patch to add a default CC list to each component, against 2.17.1 as mentioned before, it needs to use a table in case people change their email addresses. No reason for anyone to not use this if they want it for a local hack, but we won't check it in like this.

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

IMHO this is probably the most important thing we could do to make bugzilla.mozilla.org's Browser component more usable.

I'm using this feature in my installation of bugzilla now, and I've updated Aleksey's patch so that it applies cleanly to 2.17.4. I'd like to see this feature committed to the trunk. If I can find time to rework the patch to use a separate "defaultcc" table (with the appropriate glue in checksetup.pl, etc.) would you accept it? Should there also be a param to turn defaultcc off and on, such as for useqacontact?

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

yeah, should be a param for it. Enough people ask for this it'll probably be worth it. If you leave it off, nobody would notice the difference. :) But it would need to match with userids and store the userids in the database when you create the list in the admin interface for it to be accepted.

Why does there need to be a param? I assumed that the default CC list would be controlled by admins via the admin UI - is this not the case? Does this patch add UI to the standard bug display? Gerv

Comment on attachment 125924 [details] [diff] [review] updated defaultcc patch for 2.17.4 >diff -uNr bugzilla-2.17.4/checksetup.pl bugzilla-2.17.4.fixed/checksetup.pl >--- bugzilla-2.17.4/checksetup.pl Thu Apr 24 19:11:59 2003 >+++ bugzilla-2.17.4.fixed/checksetup.pl Wed Jun 18 13:31:18 2003 >@@ -1551,6 +1551,7 @@ > product_id smallint not null, > initialowner mediumint not null, > initialqacontact mediumint not null, >+ initialcclist mediumtext, > description mediumtext not null, > > Storing a list of userids as a text field is not good. This needs to be table. The right way to do this is to extend the watch table so that it permits more than one type of watch. A user should be able to be configured to watch another user OR watch a component by auto-CC (or, when component watching is done, watch a component directly) Dave, Gerv, Should we hold this to fix this or let it land and then add more code to checksetup later to convert it??

> Storing a list of userids as a text field is not good. This needs to be table. This is my fault... when I did the original patch it was the easiest way (it still is easy :). My thinking was that the initial cc list would only be on the admin screen where a limited number of people could change it. > The right way to do this is to extend the watch table so that it permits more > than one type of watch. A user should be able to be configured to watch > another user OR watch a component by auto-CC (or, when component watching is > done, watch a component directly) auto-CC is interesting, but doesn't really strike me as a function of watching (component watching, on the other hand, is). Bug 73665 is about providing that framework and bug 76794 is about component watching specifically.

As 2.16.3 is stable release, i had to downgrade precedent patches to allow using it with the running environment. Might be helpfull for some of yours...

As 2.16.3 is stable release, i had to downgrade precedent patches to allow using it with the running environment. Might be helpfull for some of yours...

As 2.16.3 is stable release, i had to downgrade precedent patches to allow using it with the running environment. Might be helpfull for some of yours...

As 2.16.3 is stable release, i had to downgrade precedent patches to allow using it with the running environment. Might be helpfull for some of yours...

Hum... Sorry for multiple posting... Error came from my proxy that send back an error 500, but in fact, data was sent... :=\

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

addendum to attachment #125924 [details] [diff] [review] minor cleanup to get rid of the "uninitialized value in split" error message.

This patch automatically displays the initial cclist on the enter new bug form, in the same way the "Assign To" field gets automatically set. Patch is based on 2.17.5

Apologies for the spam, previous patch didn't take into account non-javascript browsers as well as empty cclists.

Is this going to make it into a bmo install any time soon? Or is this going to be WONTFIXED in deferene to bug 76794? (The two aren't quite the same.)

Both this and bug 76794 have been implemented by me over Christmas. It's all held up on the emailprefs database changes, the action for which is currently with me. Gerv

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

Sadly, we're just not there yet on this, if we want 2.18 to happen any time soon. Gerv

I cobbled together a default CC list for our site, with no UI (just a text file in /data). One of the biggest requests was for automatic changes if a bug changed component. Is that in the plans for this bug?

Under the current plans, if a bug changes component, it acquires the default CC for the new component, but does not lose any of the default CCs from the old component. (That's what Component Watching is for.) Gerv

Just out of interest, is the plan to have this user-configurable, or administrator-configurable? (Translation: Will I be having to do a lot of updates to the config when this is done, or will each person be able to fix their own config without my help?)

Each person configures it for themselves. Fret not :-) Gerv

yay! check it in! check it in! r1=hixie r2=mozbot (i don't have to check if code exists to review it right?)

The current blocker is me doing more testing on some data provided by Myk, for the migration to a table for email profiles. The work for this builds on that change. Gerv

Applied patch in attachment #130043 [details] [diff] [review] to a heavily customized 2.16.5, and it works flawlessly. One cosmetic suggestion: change the line my $span = 4; in editcomponents.cgi to my $span = 5;

I've merged all the patches and updated them to make it work for the 2.19 tip. Most of the changes are due to the edit*.cgi scripts being templatized.

+ foreach my $id (split(/[ ,]/, $ids)) { I think you forgot a "+" here. :-) + <script type="text/javascript" language="JavaScript"> + document.write([...]); + </script> Maybe you should add a <noscript> section.

Is a maxlength of 64 for the new field OK - I would have thought this field could be bigger than that??

This needs some major changes. [ see my comments from 2002 :-) ] DefaultCC needs to be a table, not a a text string. The interface to edit the defaultcc needs to be similar to adding and dropping users from the CC-list in the first place (add the listed/delected/matched users to the list, or delete the selected users from the list) rather than a long text box. We also need to decide to what extent we will let users add themselves to an auto-CC list or delete themselves from one. Naturally, they should not be able to do so to a product that they could either not select or not edit. Should they be able to add themselves to an auto-CC list of a product where they cannot see bugs by default?

The attached patch contains changes to files in a directory template/en/default/admin/components/. However, the directory is not contained in the most recent tarball, nor in untagged CVS, nor in BUGZILLA-2_18-BRANCH. What am I missing?

(In reply to comment #81) > The attached patch contains changes to files in a directory > template/en/default/admin/components/. However, the directory is not contained > in the most recent tarball, nor in untagged CVS, nor in BUGZILLA-2_18-BRANCH. > What am I missing? Components have been templatised in 2.19, so it is in the latest untagged CVS (try a cvs update -d to create the dir maybe?)

Thanks loads. I thought I had done an untagged checkout, but update -dA worked. - Now let's see if I can fix up this patch :)

This is my VERY FIRST Bugzilla patch EVER, so please don't bite me! - This patch create a new table, 'componentautocc' - Currently only users who can administrate the components can change it - I call it "auto-CC" rather than "initial CC" because some time in the future someone might want to allow users to add or remove themselves from the auto-CC list, and if that is done, this feature should no longer add people to bugs they could normally not see. Instead, it should automatically add them when the bug becomes visible to them. - In lines 74 ff. of the patch you can see that, after inserting a row into 'components', I am retrieving the ID that was assigned to the new component. Is there a way to do this without an extra query?

Thanks Joel/Timwi. Just wanted to upgrade the current patch to the cvs head as is. It's not ideal, but works quite well for me. Still, looking forward to seeing a real defautcc/autocc table so that administrators don't have to maintain the list.

The table is as real as it can get :-) To alleviate the need for administrators to maintain the auto-CC list, all you need to do is create a new user interface in the user preferences or something like that. Normally I would do this, but since this requires quite advanced knowledge of Bugzilla's permissions and security mechanisms, I trust someone more experienced than I can do it much faster.

The feature requested in this bug has already been implemented as part of the emailprefs rewrite which also tidies up that part of the code and implements component watching. It's quite a large patch, and it's been in suspension while we finish 2.18, but it'll come out of deep freeze soon, I hope. The work is going on over in bug 76794, which this one is marked as blocking. Gerv

> The feature requested in this bug has already been implemented as part of the > emailprefs rewrite... in bug 76794 I take it back, this bug addresses forcing a default-cc list, managed by the administrator. Bug 76794 is driven by the individual user. Both seem to be useful?

I think it would be confusing to have two default-CC implementations. If users can do it for themselves, that's enough - if an admin wants a user to have it, either he has enough control of the user to ask them to do it, or they shouldn't be forcing it anyway :-) Gerv

(In reply to comment #89) > I think it would be confusing to have two default-CC implementations. Yes, but two interfaces (admin's and user's) for a single one is reasonable. > If users > can do it for themselves, that's enough - if an admin wants a user to have it, > either he has enough control of the user to ask them to do it, or they > shouldn't be forcing it anyway :-) Admins should be able to specify CC lists at least when creating new components! Imagine you decide to split an existing component into two. If you do not replicate the CC list for both, users are likely to be upset (and they would be right). Having to bother all the CC users just because you decided to reorganize your components is IMO bad as well. BTW, this "replication" case is a perfect example of why I would _prefer_ admins' interface to provide just a last coma-separated string of addresses that would allow me to copy the whole thing in one step as opposed to an add/remove interface which would make it harder.

> Having to bother all the CC users just because you decided to reorganize > your components is IMO bad as well. I don't agree. Presumably you are splitting it in two for a reason, which may well be because a subset of people are interested in one half of the bugs and not the other. Therefore getting people to choose which halves they are interested in makes perfect sense. Gerv

> this "replication" case is a perfect example of why I would _prefer_ admins' > interface to provide just a last coma-separated string of addresses I concur I don't want to go through a long-winded process to supply a list of userids. Even a multiple select-option is rather cumbersome.

(In reply to comment #91) > > Having to bother all the CC users just because you decided to reorganize > > your components is IMO bad as well. > > I don't agree. Presumably you are splitting it in two for a reason, which may > well be because a subset of people are interested in one half of the bugs and > not the other. Therefore getting people to choose which halves they are > interested in makes perfect sense. Well, in my case we are using Bugzilla to track issues in a research project. Normally, _all_ members of a group working on a specific project are interested in _all_ bugs against that project (in fact, I think each of our projects has identical CC list for all components, although if users had a chance to edit things, it might have been a bit different). The component field is mainly used to figure out whould should be a default assignee and to make the pile of bugs more organized. OTOH, people are busy and I would hate to have to bother everybody just because I wanted to reorganize the components.

I concur with Aleksey - this is a fairly useful feature - we had it hacked into our 2.8 version long ago. We had numerous people getting email / watching components and new bugs in this manner. "Getting people to choose" is not an great option or solution in a corporate environment, unfortunately. I guess we won't be seeing this in 2.18, but I wanted to add my desire for this feature as well. In our version, we just had another entry next to default owner, default qa contact, and default cc per component.

Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004. Anything flagged enhancement that hasn't already landed is being pushed out. If this bug is otherwise ready to land, we'll handle it on a case-by-case basis, please set the blocking2.20 flag to '?' if you think it qualifies.

You'll be lucky :-) We might get this for 2.20, though. It need the email patch in first. Gerv

Updated bug description - added Initial to word list

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

Please, include it into the distribution. I have seen similar feature built as custom extension in one bugzilla and I miss it a lot in other bugzilla. It is very usefull.

without looking at the patches, i'd like to vote against them. a proper implementation should let most users (for some definition of trusted) add themselves to the cc list for a given component w/o needing admin permission or approval.

> a proper implementation should let most users (for some definition of trusted) > add themselves to the cc list for a given component w/o needing admin > permission or approval. That is not a reason to vote against this patch, because it is still perfectly possible to add that later.

Now here's a really cool idea - to prevent users from having to create mailing lists for groups of people, can we add functionality to this patch that it would not only add individuals if requested, but if the ID in the initialcclist was preceeded by an @, then it would treat that ID as a GROUP ID instead, and add the entire group. This would help prevent email duplicates from going to others due to mail going to mailing lists *and* their users, plus it would make maintaining those groups a bunch easier. Maybe this should be added as a separate patch to help this one get landed sooner, but I think it's a better way to go.

Srike that - filing separate bug.

I updated the "Patch against current CVS" (attachment #155948 [details] [diff] [review]) to work with 2.20rc2. This is the auto CC list patch mentioned in Comment #84. Unfortunately, i then realized that i didn't want the auto CC list patch, i really wanted the initial cc list patch. Hopefully this patch will be useful to someone... It looks to me like the auto cc list feature is better handled by the "watch" functionality in bug 76794. Unfortunately, that patch apparently includes multiple features (including another implementation of default cc list!), some of which are now actually in the cvs code, making it a mess to actually apply to 2.20. So i guess i will try to bring the 2.19 initialcclist patch up to date next...

This is an update of the Initial CC patch (attachment #155893 [details] [diff] [review]) in Comment #77 applicable to 2.20rc2 I really wish this feature would make it to prime time... i had to apply a version of this patch four years ago to a 2.16 install!

Updated version of attachment 198073 [details] [diff] [review] applicable to 2.20 version. There is a small bug in Adam Morton's patch in adding a new field to a existing database (checksetup.pl).

The trunk is now frozen to prepare Bugzilla 2.22. Enhancement bugs are retargetted to 2.24.

This ticket has become fairly long winded. Seems like the main issue is defining what we want, an admin managed initial-cc or a user managed auto-cc. For this specific ticket, I think the initial discussions is for an admin managed initial-cc. I can also see an user-managed auto-cc feature, but perhaps that ought be to be a separate ticket? Besides, initial-cc is what I'm most interested in... :) The secondary issue is the database schema, using a table of id's instead of a string of id's. And then perhaps improve the UI for entering the usernames. The attachment is an updated patch that works with the 2.22 tip. It also updates the schema to use a table of id's. There's probably a fair amount of code clean up, but I hope there can be some agreement to the feature request and what it entails.

I agree completely. Moving away from a mediumtext entry and toward a separate table seems like the best way to handle this moving forward. I think it'll make it much easier for us to handle the ability to group assignments at a later date (when we choose). I envision handling user/group assignment by adding a new field to each field that can contain a user and/or group to include a flag specifying whether or not the id that follows is a user or group id, but that's for another bug discussion.

Comment on attachment 203835 [details] [diff] [review] 2.22 CVS tip, v1 Bitrotten: patching file template/en/default/admin/components/updated.html.tmpl Hunk #1 FAILED at 34. Hunk #2 FAILED at 69. 2 out of 2 hunks FAILED -- saving rejects to file template/en/default/admin/components/updated.html.tmpl.rej + Updated Initial CC List to '[% default_cclist_names FILTER html %]'. + [% ELSE %] + Removed initial CC List. The capitalization of the word 'initial' is inconsistent in those 2 examples above. + <script type="text/javascript" language="JavaScript"> + document.write('<td align="right" valign="top"><strong>Initial CC:</strong></td>'); + document.write("<td>"); + document.write('<input name="initialcclist" size="32" value="" readonly>'); + document.write("</td>"); + </script> Question: why are we using Javascript here? Shouldn't there be something like <noscript> for non-Javascript enabled browsers? + Updated Initial CC List to '[% default_cclist_names FILTER html %]'. If we're allowing to update the initial CC list, maybe "initial CC" is not a very good name for it. How about "implicit CC list", "default CC list" or any other alternative that doesn't contain the word "Initial"?

After doing some thinking, 'initial' sounds ok as well. We just have to make sure that admins understand that we're talking about the initial CC list for new bugs --updating the 'initial CC list' that they filled upon component creation (and getting 'initial CC list updated') is perfectly logical and correct (because it's an initial CC list per new bug and not per component).

I'm also mentioning a conclusion reached with Albert by mail: > One option could be to only display this field (and update the value) if > javascript support is available. But when submitting the form, ignore this > field altogether and access the initialcclist from the database itself. > This is a must. Otherwise I could submit bugs via a modified form (saved in a local .html on my hard-disk), and by-pass the initial CC list.

IMO, the initial CC list should never be displayed outside editcomponents.cgi and it should not be alterable from outside this admin page. The reason is that some people want to make *sure* they will always get notifications for some given component, such as project leaders or bots (e.g. ssdbot). So the big difference compared to the default assignee or QA contact is that you can never prevent people on the initial CC list from receiving notifications (unless they cannot see a bug because they haven't sufficient privileges), while you can easily do that for the default assignee and QA contact (you simply have to change the assignee and QA contact on a bug to prevent them to get notifications). Albert, any updated patch soon?

> Albert, any updated patch soon? As much as I have wanted to get this done soon, I'm doubtful I'll be able to repatch it until June/July timeframe. That is when we're slated to revamp my company's infrastructure and will be using the latest branch. If that is too late, perhaps somebody else can take over?

Okay. I have a client who wants their Default CC customization to be the same as what we do upstream, so I can take this. I'll probably get to it before Albert.

I'm working on code for this internally at my company. Once we're done and we get the copyright release on it, we can submit it here.

I'm currently trying attachment 203835 [details] [diff] [review] on a 2.20 installation and will report back how it does.

I've reworked/fixed attachment 203835 [details] [diff] [review] "2.22 CVS tip, v1" to work on our 2.20+ installation at pixar. Here's the current patch, I have fixed some of the admin error cases, I can delete/re-add components w/o duplicate warnings, this is working pretty well. I would be willing to help land this on 2.23 or the 2.20 branch, please let me know. I like the initialcclist table, it seems to be a good way to solve this problem.

I actually already have a patch for 2.22 that would require only a little re-working to port to HEAD. The patch is complete, I just have to wait for copyright release on it from my client. The advantage is that I've already gone through a review process on the patch internally, expecting to have to submit it to upstream.

max, is your solution based on the patch I've posted, namely the initialcclist table addition?

(In reply to comment #120) > max, is your solution based on the patch I've posted, namely the > initialcclist table addition? Basically. The table is called component_cc and I had to fix a few mistakes that the original patch had in the schema. I probably can't give too much more info than that until I get copyright release.

Chris/Max, thanks for moving this forward! One thing I should add is that the patch should not CC anybody in the initialcclist that has a disabled account. Or at least this is how I prefer to use this patch.

This feature should only be responsible for populating the CC entry for the bug, what happens after that is up to the normal bug behavior. The code to mail the cc recipients should check for disabled-ness and not-send mail accordingly. If that's not happening, we should probabaly file a separate bug for that.

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

Comment #122: Disabled users should probably not get mail, is that another bug? Pinging Max to see if his patch can see the light of day yet. This feature is working for us, maybe max & I can compare patches and come up with a good 2.23 dev patch for this.

(In reply to comment #125) > Comment #122: Disabled users should probably not get mail, is that another bug? Yeah, that's bug 100953. > Pinging Max to see if his patch can see the light of day yet. > This feature is working for us, maybe max & I can compare patches > and come up with a good 2.23 dev patch for this. Yeah, I'm just awaiting the darn copyright release. It's taking much longer than I expected.

This bug was entered in 2000. (6 years ago?) :-) I could really use the feature and it sounds like the patch is already done and working. Any way I can expedite this to be included in the main trunk?

Looks like Firefox did some strange selecting of the group radio boxes on the mass-change page when I clicked refresh in my browser before that last mass-change. This is not really a security bug. :-)

Any updates on the status of the 2.22 patch? I would really like to add this to our 2.22 installation in the next couple of days (we just upgraded a 2.18 unstallation that had the attachment 125924 [details] [diff] [review] version of the CC lists in it) and while I can probably just do something based on one of the existing attachments, I'd rather apply a patch that has a hope of getting actually checked in at some point in the future. Thanks!

I'm ready to do some work on my end, Max, any word?

(In reply to comment #130) > I'm ready to do some work on my end, Max, any word? No word. I just pinged them again, though.

(In reply to comment #130) > I'm ready to do some work on my end, Max, any word? > Chris - so am I. I've got it implemented here in an older version and would like to see it re-done with a cross-reference table rather than being stuck with what we have now (developed before I started working on Bugzilla here).

Note that we freeze in two weeks for Bugzilla 3.0. Unless Max can guarantee that he will be able to attach his patch on time, I suggest that one of you starts working on it. Else this feature will miss the boat for 3.0.

(In reply to comment #133) > Note that we freeze in two weeks for Bugzilla 3.0. Unless Max can guarantee > that he will be able to attach his patch on time, I suggest that one of you > starts working on it. Else this feature will miss the boat for 3.0. I should be ready tomorrow or the next day.

Max: great. I can help review & compare this with the patch we have.

(In reply to comment #134) > (In reply to comment #133) > > Note that we freeze in two weeks for Bugzilla 3.0. Unless Max can guarantee > > that he will be able to attach his patch on time, I suggest that one of you > > starts working on it. Else this feature will miss the boat for 3.0. > > I should be ready tomorrow or the next day. > Hey! Great! Less work for us :) Max - please feel free to r? to me if you'd like. I expect this will probably be one of the largest patches I've reviewed, but like I said, it'll save me work at work so it's a piece of cake to justify to mgt. :)

Okay, here's the code as it was released, for 2.22. I just have to update this code for the tip and then it will be ready for review!

Okay, here's the patch. Make sure you test all the different parts of it--I tested everything except component creation, but it would still be good to test things again. I think in a future bug we should have a parameter for what happens when you move something from one component to another--should the CC always be added, never be added, or only added if requested by reassignbycomponent? Right now it does #3 by default, as the most sensible (based on the Principle of Least Surprise).

That was the wrong patch. This is the right one.

Just from your comments, it would also be helpful if the patch would offer the option of removing existing default CC's to the user (in the case of an OOPS on creation of a bug), but it's up to you on whether or not you feel that should be incorporated in this patch or another bug (probably another bug).

Comment on attachment 236283 [details] [diff] [review] Actual v1 Great start! :) Three problems (*'ed) found below: Installed patch Ran checksetup.pl Updated localconfig Re-ran checksetup.pl - no problems Set permissions as follows: chgrp -R httpd . chmod -R g+r . chmod -R g+w data Connected to web interface successfully. Updated parameters - maintainer, urlbase, cookiepath, timezone, announcehtml. Added new user - nobody@ Added myself and nobody to initial cc list of the temporary product/component. Attempted to update the initial cc list with k.b@amd.com;nobody... * Directions at initial cc list maintenance page does not specify entry format. Was able to add myself, myself and nobody, nobody, each time without issues. Was able to myself, nobody, myself and nobody as well. * Attempting to delete only component while initial CC installed causes software error: DBD::mysql::db do failed: Table 'component_cc' was not locked with LOCK TABLES at .../editcomponents.cgi line 306 Added new bug that had both myself and nobody in the initialcclist but I selected only myself and it correctly added just me. Added new user bztest1 * Add CC displays users that are already CC'ed in the Add CC list when usemenuforusers is turned on. More to follow with updates :)

Okay, I fixed all your comments. I also made the Default CC not editable by people with editbugs. They can always remove them after they file the bug, if they want.

Comment on attachment 236325 [details] [diff] [review] v2 drop database initialcclist_bugs Re-installed code base Installed patch Ran checksetup.pl Ral checksetup.pl Updated localconfig Re-ran checksetup.pl - no problems Set permissions as follows: chgrp -R httpd . chmod -R g+r . chmod -R g+w data Connected to web interface successfully. Updated parameters - maintainer, urlbase, cookiepath. Added new user - nobody@a.c Added new user bztest1@a.c Entered Test Component Added k.b@a.c, nobody@a.c Intentionally failed with k.b@a.c; nobody@a.c Surprisingly, k.b@a.c nobody@a.c bztest1@a.c worked - is this a bug or should it have accepted them without ,'s ? Created new bug with initial cc list of all 3 users with usemenuforusers off Created new bug with initial cc list of all 3 users with usemenuforusers on Updated initial CC list to exclude bztest1@a.c Created new bug with initial cc list selection of just Test User - all three were automatically added to the CC list in the new bug. Created new bug with initial cc list selection of just nobody@a.c, still both CC's were included. Q: Should users be allowed to override the initial CC list by selecting some members of the list but not others? I don't know. Deleted only component from Bugzilla without errors. Checked cc table following deletion - CC's removed as expected. -- notes: Should probably have permissions set up to limit who can change the initalcclist (next version probably). More tomorrow when I get back to work.

re: "Q: Should users be allowed to override the initial CC list by selecting some members of the list but not others?" That is how my implementation works, the default cc basically just pre-populates the cc field at bug creation time w/o adding dups. The bug creator then would have the option of editing that list. Another Q that we brought up here at my work: Should any user be able to add him/herself to the default cc list for a component? My implementation attached this feature to the admin component page, and was only admin-accessible.

(In reply to comment #144) > "Q: Should users be allowed to override the initial CC list by selecting some > members of the list but not others?" We discussed this, sort of. That's how the original patch worked, but the problem is this: what if on enter_bug.cgi, somebody enters something in the CC field before he selects a component? Then the Default CC list will never appear. Also, any reporter can remove CCs after a bug is filed, so anybody can be removed from the CC list after the bug is initially filed. > Should any user be able to add him/herself to the default cc list > for a component? My implementation attached this feature to the admin > component page, and was only admin-accessible. Yeah, it should be only admin-accessible. We're going to have bug 278455 for people to watch specific field values.

Comment on attachment 236325 [details] [diff] [review] v2 >Index: editcomponents.cgi >@@ -171,6 +198,16 @@ > ($product->id, $comp_name, $description, > $default_assignee_id, $default_qa_contact_id)); > >+ my $component_id = $dbh->bz_last_key('components', 'id'); Nit: You could move |$component = new Bugzilla::Component(...)| at this place and use $component->id instead of $component_id. >@@ -292,8 +331,12 @@ > > if ($action eq 'edit') { > >+ $vars->{'initial_cc_names'} = >+ join(', ', map($_->login, @{$component->initial_cc})); Nit: I would prefer if you only pass the component object and let the template use it. But I guess you do it here because 1) it's easier than inside a template and 2) because you have to pass this variable to global/userselect.html.tmpl? >+ $vars->{'initial_cc_names'} = >+ join(', ', map($_->login, @{$component->initial_cc})); Same comment here. >Index: enter_bug.cgi >- $default{'component_'} = $cloned_bug->component; >+ $default{'component_'} = $cloned_bug->component->name; Wrong! $bug->component returns the component name, not a component object. The current code is correct. >Index: template/en/default/bug/create/create.html.tmpl >@@ -223,7 +235,7 @@ > [% sel = { description => 'Priority', name => 'priority' } %] > [% INCLUDE select %] > [% ELSE %] >- <td colspan="2"> >+ > <input type="hidden" name="priority" value="[% default.priority FILTER html %]"> > </td> > [% END %] Do not remove this line, because 1) you cannot have an <input> field within <tr></tr> even if this field is hidden, 2) you have a </td> which doesn't close anything (this page is no longer a valid HTML page), 3) we must keep the UI consistent where all rows of the table must have 4 columns, and 4) I don't want to see the severity field jumping to left or right depending on whether the priority field is displayed or not. I did some tests only, but this looks good so far. Except for the $vars->{'initial_cc_names'} nits, I would like to see other comments fixed.

I fixed the bitrot and most of your comments. I left the initial_cc_names stuff the same, because yes, it's much easier to do it in perl than in the template.

Comment on attachment 239062 [details] [diff] [review] v3 >Index: process_bug.cgi >+ # And add in the Default CC for the Component. >+ my $comp_obj = new Bugzilla::Component($new_comp_id); Nit: you could reuse the component object, if it already exists: $comp_obj = $component || new Bugzilla::Component($new_comp_id); Nit: also, it would be fine to see the current default CC list when viewing a list of components for a given product, see admin/components/list.html.tmpl. This comment could be fixed in a separate bug. Your patch is working fine. r=LpSolit

Comment on attachment 239062 [details] [diff] [review] v3 >Index: template/en/default/admin/components/create.html.tmpl >+ <em>Enter user names for the CC in a comma-separated list.</em> I forgot to mention this in my review: this comment doesn't make sense when the 'usemenuforusers' is turned on. Please fix that in an updated patch. >Index: template/en/default/admin/components/edit.html.tmpl >+ <em>Enter user names for the CC in a comma-separated list.</em> Same comment here.

Okay, here's the version with the fixes that LpSolit recommended.

Checking in editcomponents.cgi; /cvsroot/mozilla/webtools/bugzilla/editcomponents.cgi,v <-- editcomponents.cginew revision: 1.77; previous revision: 1.76 done Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.343; previous revision: 1.342 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.156; previous revision: 1.155 done Checking in Bugzilla/Component.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Component.pm,v <-- Component.pm new revision: 1.12; previous revision: 1.11 done Checking in Bugzilla/DB/Schema.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v <-- Schema.pm new revision: 1.74; previous revision: 1.73 done Checking in template/en/default/admin/components/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/create.html.tmpl,v <-- create.html.tmpl new revision: 1.8; previous revision: 1.7 done Checking in template/en/default/admin/components/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.9; previous revision: 1.8 done Checking in template/en/default/admin/components/updated.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/updated.html.tmpl,v <-- updated.html.tmpl new revision: 1.5; previous revision: 1.4 done Checking in template/en/default/bug/knob.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/knob.html.tmpl,v <-- knob.html.tmpl new revision: 1.23; previous revision: 1.22 done Checking in template/en/default/bug/create/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v <-- create.html.tmpl new revision: 1.66; previous revision: 1.65 done

Added to the release notes as part of bug 349423.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.0/ modified t/test_edit_products_properties.t Committed revision 171. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/3.6/ modified t/test_edit_products_properties.t Committed revision 143. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/3.4/ modified t/test_edit_products_properties.t Committed revision 117.