Closed
Bug 38922
Opened 25 years ago
Closed 18 years ago
Default (Initial) CC list for each component
Categories
(Bugzilla :: Administration, task, P3)
Bugzilla
Administration
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: matt.wallis, Assigned: mkanat)
References
Details
(Keywords: selenium)
Attachments
(2 files, 26 obsolete files)
19.71 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
3.38 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•25 years ago
|
||
alternatively, there could be an option to "e-mail me on NEW bugs in this
component but don't add me to cc".
Keywords: helpwanted
OS: Windows NT → All
Hardware: PC → All
Summary: Wanted: Default CC list for each component → [RFE] Wanted: Default CC list for each component
Comment 2•24 years ago
|
||
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
Comment 3•24 years ago
|
||
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.
Comment 4•24 years ago
|
||
Comment 5•24 years ago
|
||
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.)
Comment 6•24 years ago
|
||
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.
Assignee: tara → jake
Whiteboard: 2.14
Updated•24 years ago
|
Whiteboard: 2.14 → 2.16
Comment 8•24 years ago
|
||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Updated•23 years ago
|
Whiteboard: 2.16 → 2.16 [people:cc]
Comment 10•23 years ago
|
||
-> Bugzilla product, Administration component.
Component: Bugzilla → Administration
Product: Webtools → Bugzilla
Whiteboard: 2.16 [people:cc] → [people:cc]
Version: other → unspecified
Comment 11•23 years ago
|
||
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.
Updated•23 years ago
|
Blocks: bz-component-watch
Comment 12•23 years ago
|
||
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 :)
Updated•23 years ago
|
Attachment #21664 -
Attachment is obsolete: true
Updated•23 years ago
|
Attachment #32548 -
Flags: review-
Comment 13•23 years ago
|
||
Note that allowing users to add/remove them to/from the list is bug 76794 (that
depends on this one).
Comment 14•23 years ago
|
||
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).
Comment 15•23 years ago
|
||
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 16•23 years ago
|
||
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).
Comment 17•23 years ago
|
||
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
Attachment #32548 -
Attachment is obsolete: true
Updated•23 years ago
|
Comment 18•23 years ago
|
||
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.
Comment 19•23 years ago
|
||
Updated•22 years ago
|
Attachment #57397 -
Flags: review-
Comment 20•22 years ago
|
||
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.
Comment 21•22 years ago
|
||
I've refreshed the patch to work with the current CVS trunk.
Attachment #57397 -
Attachment is obsolete: true
Comment 22•22 years ago
|
||
*** Bug 158263 has been marked as a duplicate of this bug. ***
Comment 23•22 years ago
|
||
-> ayn2@cornell.edu who made the newest patch. Review coming up.
Assignee: justdave → ayn2
Comment 24•22 years ago
|
||
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.
Attachment #86893 -
Flags: review-
Comment 25•22 years ago
|
||
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?
Assignee: ayn2 → justdave
Comment 26•22 years ago
|
||
This would be handy, but the list of userids probably needs to be in a seperate
table rather than a text field.
Updated•22 years ago
|
Status: NEW → ASSIGNED
Comment 28•22 years ago
|
||
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!
Attachment #86893 -
Attachment is obsolete: true
Updated•22 years ago
|
Comment 29•22 years ago
|
||
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
Comment 30•22 years ago
|
||
> 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)!
Comment 31•22 years ago
|
||
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.
Comment 32•22 years ago
|
||
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.
Comment 33•22 years ago
|
||
> 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
Comment 34•22 years ago
|
||
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.
Comment 35•22 years ago
|
||
> 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
Comment 36•22 years ago
|
||
> 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).
Comment 37•22 years ago
|
||
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).
Comment 38•22 years ago
|
||
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.
Comment 39•22 years ago
|
||
> 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.
Comment 40•22 years ago
|
||
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.
Updated•22 years ago
|
Attachment #102115 -
Flags: review?
Comment 41•22 years ago
|
||
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.
Updated•22 years ago
|
Flags: approval?
Comment 42•22 years ago
|
||
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...
Flags: approval?
Comment 43•22 years ago
|
||
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.
Attachment #102115 -
Flags: review? → review-
Comment 44•22 years ago
|
||
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.
Attachment #102115 -
Attachment is obsolete: true
Comment 45•22 years ago
|
||
*** Bug 187963 has been marked as a duplicate of this bug. ***
Comment 46•22 years ago
|
||
Please free to take over.
Assignee: mozilla-bugs → nobody
Status: ASSIGNED → NEW
Updated•22 years ago
|
Attachment #109883 -
Flags: review?
Comment 48•22 years ago
|
||
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.
Attachment #109883 -
Attachment description: A patch to add a default CC list to each component, against 1.17.1 → A patch to add a default CC list to each component, against 2.17.1
Attachment #109883 -
Flags: review? → review-
Comment 49•22 years ago
|
||
*** Bug 53717 has been marked as a duplicate of this bug. ***
Comment 50•22 years ago
|
||
IMHO this is probably the most important thing we could do to make
bugzilla.mozilla.org's Browser component more usable.
Comment 51•21 years ago
|
||
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?
Comment 52•21 years ago
|
||
*** Bug 210733 has been marked as a duplicate of this bug. ***
Comment 53•21 years ago
|
||
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.
Comment 54•21 years ago
|
||
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 55•21 years ago
|
||
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??
Comment 56•21 years ago
|
||
> 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.
Updated•21 years ago
|
Summary: [RFE] Wanted: Default CC list for each component → Default CC list for each component
Comment 57•21 years ago
|
||
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...
Comment 58•21 years ago
|
||
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...
Comment 59•21 years ago
|
||
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...
Comment 60•21 years ago
|
||
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...
Comment 61•21 years ago
|
||
Hum... Sorry for multiple posting...
Error came from my proxy that send back an error 500, but in fact, data was
sent... :=\
Comment 62•21 years ago
|
||
*** Bug 219594 has been marked as a duplicate of this bug. ***
Comment 63•21 years ago
|
||
addendum to attachment #125924 [details] [diff] [review]
minor cleanup to get rid of the "uninitialized value in split" error message.
Comment 64•21 years ago
|
||
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
Comment 65•21 years ago
|
||
Apologies for the spam, previous patch didn't take into account non-javascript
browsers as well as empty cclists.
Attachment #134944 -
Attachment is obsolete: true
Comment 66•21 years ago
|
||
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.)
Comment 67•21 years ago
|
||
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
Comment 68•21 years ago
|
||
*** Bug 234239 has been marked as a duplicate of this bug. ***
Updated•21 years ago
|
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
Updated•21 years ago
|
Assignee: nobody → gerv
Target Milestone: Bugzilla 2.20 → Bugzilla 2.18
Comment 69•21 years ago
|
||
Sadly, we're just not there yet on this, if we want 2.18 to happen any time soon.
Gerv
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
Comment 70•21 years ago
|
||
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?
Comment 71•21 years ago
|
||
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
Comment 72•21 years ago
|
||
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?)
Comment 73•21 years ago
|
||
Each person configures it for themselves. Fret not :-)
Gerv
Comment 74•21 years ago
|
||
yay!
check it in! check it in! r1=hixie r2=mozbot
(i don't have to check if code exists to review it right?)
Comment 75•21 years ago
|
||
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
Comment 76•20 years ago
|
||
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;
Comment 77•20 years ago
|
||
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.
Comment 78•20 years ago
|
||
+ 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.
Comment 79•20 years ago
|
||
Is a maxlength of 64 for the new field OK - I would have thought this field
could be bigger than that??
Comment 80•20 years ago
|
||
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?
Comment 81•20 years ago
|
||
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?
Comment 82•20 years ago
|
||
(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?)
Comment 83•20 years ago
|
||
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 :)
Comment 84•20 years ago
|
||
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?
Comment 85•20 years ago
|
||
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.
Comment 86•20 years ago
|
||
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.
Comment 87•20 years ago
|
||
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
Comment 88•20 years ago
|
||
Comment 89•20 years ago
|
||
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
Comment 90•20 years ago
|
||
(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.
Comment 91•20 years ago
|
||
> 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
Comment 92•20 years ago
|
||
> 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.
Comment 93•20 years ago
|
||
(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.
Comment 94•20 years ago
|
||
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.
Comment 95•20 years ago
|
||
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.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Comment 96•20 years ago
|
||
You'll be lucky :-)
We might get this for 2.20, though. It need the email patch in first.
Gerv
Flags: blocking2.18.1? → blocking2.18.1-
Comment 97•20 years ago
|
||
Updated bug description - added Initial to word list
Summary: Default CC list for each component → Default (Initial) CC list for each component
Updated•20 years ago
|
Flags: blocking2.20?
QA Contact: mattyt-bugzilla → default-qa
Assignee | ||
Comment 98•20 years ago
|
||
*** Bug 270230 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
No longer blocks: bz-component-watch
Comment 99•19 years ago
|
||
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.
Comment 100•19 years ago
|
||
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.
Comment 101•19 years ago
|
||
> 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.
Comment 102•19 years ago
|
||
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.
Comment 103•19 years ago
|
||
Srike that - filing separate bug.
Comment 104•19 years ago
|
||
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...
Comment 105•19 years ago
|
||
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!
Comment 106•19 years ago
|
||
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).
Comment 107•19 years ago
|
||
The trunk is now frozen to prepare Bugzilla 2.22. Enhancement bugs are retargetted to 2.24.
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24
Comment 108•19 years ago
|
||
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.
Attachment #134376 -
Attachment is obsolete: true
Attachment #135014 -
Attachment is obsolete: true
Attachment #155893 -
Attachment is obsolete: true
Attachment #203835 -
Flags: review?(bugreport)
Comment 109•19 years ago
|
||
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.
Assignee | ||
Updated•19 years ago
|
Assignee: gerv → altlst
Comment 110•19 years ago
|
||
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"?
Attachment #203835 -
Flags: review?(bugreport) → review-
Comment 111•19 years ago
|
||
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).
Comment 112•19 years ago
|
||
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.
Comment 113•19 years ago
|
||
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?
Comment 114•19 years ago
|
||
> 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?
Assignee | ||
Comment 115•19 years ago
|
||
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.
Assignee: altlst → mkanat
Target Milestone: Bugzilla 2.24 → Bugzilla 3.0
Assignee | ||
Comment 116•18 years ago
|
||
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.
Status: NEW → ASSIGNED
Comment 117•18 years ago
|
||
I'm currently trying attachment 203835 [details] [diff] [review] on a 2.20 installation
and will report back how it does.
Comment 118•18 years ago
|
||
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.
Assignee | ||
Comment 119•18 years ago
|
||
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.
Comment 120•18 years ago
|
||
max, is your solution based on the patch I've posted, namely the
initialcclist table addition?
Assignee | ||
Comment 121•18 years ago
|
||
(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.
Comment 122•18 years ago
|
||
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.
Comment 123•18 years ago
|
||
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.
Comment 124•18 years ago
|
||
*** Bug 345548 has been marked as a duplicate of this bug. ***
Comment 125•18 years ago
|
||
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.
Assignee | ||
Comment 126•18 years ago
|
||
(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.
Comment 127•18 years ago
|
||
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?
Assignee | ||
Updated•18 years ago
|
Group: webtools-security
Target Milestone: Bugzilla 3.0 → Bugzilla 2.24
Assignee | ||
Comment 128•18 years ago
|
||
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. :-)
Group: webtools-security
Comment 129•18 years ago
|
||
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!
Comment 130•18 years ago
|
||
I'm ready to do some work on my end, Max, any word?
Assignee | ||
Comment 131•18 years ago
|
||
(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.
Comment 132•18 years ago
|
||
(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).
Comment 133•18 years ago
|
||
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.
Assignee | ||
Comment 134•18 years ago
|
||
(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.
Comment 135•18 years ago
|
||
Max: great. I can help review & compare this with the
patch we have.
Comment 136•18 years ago
|
||
(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. :)
Assignee | ||
Comment 137•18 years ago
|
||
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!
Attachment #109883 -
Attachment is obsolete: true
Attachment #125924 -
Attachment is obsolete: true
Attachment #129989 -
Attachment is obsolete: true
Attachment #129990 -
Attachment is obsolete: true
Attachment #129991 -
Attachment is obsolete: true
Attachment #130043 -
Attachment is obsolete: true
Attachment #155948 -
Attachment is obsolete: true
Attachment #197929 -
Attachment is obsolete: true
Attachment #198073 -
Attachment is obsolete: true
Attachment #200987 -
Attachment is obsolete: true
Attachment #203835 -
Attachment is obsolete: true
Attachment #225779 -
Attachment is obsolete: true
Assignee | ||
Comment 138•18 years ago
|
||
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).
Attachment #236248 -
Attachment is obsolete: true
Attachment #236281 -
Flags: review?(kevin.benton)
Assignee | ||
Comment 139•18 years ago
|
||
That was the wrong patch. This is the right one.
Attachment #236281 -
Attachment is obsolete: true
Attachment #236283 -
Flags: review?(kevin.benton)
Attachment #236281 -
Flags: review?(kevin.benton)
Comment 140•18 years ago
|
||
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 141•18 years ago
|
||
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 :)
Attachment #236283 -
Flags: review?(kevin.benton) → review-
Assignee | ||
Comment 142•18 years ago
|
||
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.
Attachment #236283 -
Attachment is obsolete: true
Attachment #236325 -
Flags: review?
Assignee | ||
Updated•18 years ago
|
Attachment #236325 -
Flags: review? → review?(kevin.benton)
Comment 143•18 years ago
|
||
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.
Comment 144•18 years ago
|
||
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.
Updated•18 years ago
|
Attachment #236325 -
Flags: review?(LpSolit)
Assignee | ||
Comment 145•18 years ago
|
||
(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 146•18 years ago
|
||
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.
Attachment #236325 -
Flags: review?(kevin.benton)
Attachment #236325 -
Flags: review?(LpSolit)
Attachment #236325 -
Flags: review-
Assignee | ||
Comment 147•18 years ago
|
||
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.
Attachment #236325 -
Attachment is obsolete: true
Attachment #239062 -
Flags: review?(LpSolit)
Comment 148•18 years ago
|
||
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
Attachment #239062 -
Flags: review?(LpSolit) → review+
Comment 149•18 years ago
|
||
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.
Updated•18 years ago
|
Flags: blocking2.18.1- → approval?
Updated•18 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 150•18 years ago
|
||
Okay, here's the version with the fixes that LpSolit recommended.
Attachment #239062 -
Attachment is obsolete: true
Attachment #239425 -
Flags: review+
Assignee | ||
Comment 151•18 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: relnote
Resolution: --- → FIXED
Whiteboard: [people:cc]
Updated•18 years ago
|
Flags: testcase?
Assignee | ||
Comment 152•18 years ago
|
||
Added to the release notes as part of bug 349423.
Keywords: relnote
Comment 153•14 years ago
|
||
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.
Attachment #510039 -
Flags: review+
You need to log in
before you can comment on or make changes to this bug.
Description
•