Bug 486554 - When "Blocks" is changed and the bugzillas blocked have aliases, the email should show the alias
Summary: When "Blocks" is changed and the bugzillas blocked have aliases, the email sh...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Bugzilla
Classification: Community
Component: Email Notifications
Version: 3.2
Hardware: All
OS: Linux
high
low
Target Milestone: ---
Assignee: Noura El hawary
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-02-20 09:48 UTC by Jan Pazdziora
Modified: 2012-06-04 00:39 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-02-25 09:28:51 UTC
Embargoed:


Attachments (Terms of Use)
v1 for adding aliases to dependencies changes email notifications (2.01 KB, patch)
2009-02-24 10:15 UTC, Noura El hawary
dkl: review+
Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 823399 0 high CLOSED Bug mail should show alias for depends on and blocked fields 2021-02-22 00:41:40 UTC

Internal Links: 823399

Description Jan Pazdziora 2009-02-20 09:48:41 UTC
Description of problem:

When "Blocks" is changed for a bugzilla, bugzilla sends email informing interested parties about that change. The email looks like this:

https://bugzilla.redhat.com/show_bug.cgi?id=245860

Brandon Perkins <bperkins> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bperkins
             Blocks|249459                      |456985
         AssignedTo|msuchy           |jpazdziora

If those "Blocks" bugzillas have aliases, it would be really great if the email could show those aliases.

Justification: We use trackers (tracking bugs) a lot for Satellite triaging and development and those trackers have aliases. If I get an email which says that bugzilla 245860 moved from 249459 to 456985, I need to open both 249459 and 456985 to see what really hapened because those numbers do not mean anything. If the email was

https://bugzilla.redhat.com/show_bug.cgi?id=245860

Brandon Perkins <bperkins> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bperkins
             Blocks|249459 (sat530-triage)      |456985 (sat530-blockers)
         AssignedTo|msuchy           |jpazdziora

I'd immediately, from that email, know what happened, so would need to hit bugzilla with two GETs.

Version-Release number of selected component (if applicable):

3.2

How reproducible:

Deterministic.

Steps to Reproduce:
1. Have email notifications setup.
2. Have bugzilla blocking another bugzilla. Have that another bugzilla have alias.
3. Change the "Blocks" to different bugzilla which also has alias set.
  
Actual results:

Email comes, showing that Blocks has changed, and shows bugzilla numbers.

Expected results:

Email comes, showing that Blocks has changed, and shows bugzilla numbers and next to them, aliases of those bugzillas.

Additional info:

If the bugzilla does not have any alias set, just the number should be shown (just like now). The aliases should be shown not just when "Blocks" changes from one bugzilla to another one but also when it is being set (from previous empty value), removed, or when there are multiple original or new bugzillas that are being blocked. In all cases, if any of those bugzillas have alias set, the alias should be shown next to the bugzilla number.

Comment 1 Jan Pazdziora 2009-02-20 09:50:22 UTC
> I'd immediately, from that email, know what happened, so would need to hit
> bugzilla with two GETs.

... so would *not* need to hit bugzilla with two GETs.

Comment 2 David Lawrence 2009-02-23 19:44:41 UTC
Just to clarify do you mean show the aliases in the three column list of field changes? For example:

Brandon Perkins <bperkins> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bperkins
             Blocks|249459                      |SomeBugAlias
         AssignedTo|msuchy           |jpazdziora

Where the bug id is replaced by the bug's alias if one exists? Or do you mean the following

Brandon Perkins <bperkins> changed:

Summary: SomeBugSummary
Alias: SomeBugAlias

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bperkins
             Blocks|249459                      |456985
         AssignedTo|msuchy           |jpazdziora

Dave

Comment 3 Jan Pazdziora 2009-02-24 08:46:01 UTC
I actually meant

Brandon Perkins <bperkins> changed:

Summary: SomeBugSummary
Alias: SomeBugAlias

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bperkins
             Blocks|249459 (SomeAlias)          |456985 (SomeOtherAlias)
         AssignedTo|msuchy           |jpazdziora

I don't think we want to remove the bugzilla numbers from those columns. I think it would be good to have both the number *and* the alias next to it.

Comment 4 Noura El hawary 2009-02-24 10:15:36 UTC
Created attachment 333019 [details]
v1 for adding aliases to dependencies changes email notifications

hey Dave,

I think from Jan's first comment, he wanted to see the bug aliases next to bug ids of blocks/dependson in the removed/added columns of mail change notifications, as the example he put there. I have worked a patch to do this, applied to bz-web2 for testing please let me know what you think.

Noura

Comment 5 David Lawrence 2009-02-24 16:53:22 UTC
Comment on attachment 333019 [details]
v1 for adding aliases to dependencies changes email notifications

>Index: Bugzilla/BugMail.pm
>+        # REDHAT EXTENSIONS START 486554
>+        } elsif (exists($diff->{'fieldname'}) &&
>+            ($diff->{'fieldname'} eq 'dependson' || $diff->{'fieldname'} eq 'blocked')) {
>+             my $col_name;
>+             my @old_dependencies = split(/, ?/, $diff->{'old'});
>+             my @new_dependencies = split(/, ?/, $diff->{'new'});
>+             my @aliased_old = ();
>+             my @aliased_new = ();
>+             foreach my $dependency_bug (@new_dependencies) {
>+                 my $bug_obj = new Bugzilla::Bug($dependency_bug);
>+                 my $alias_string = $dependency_bug;
>+                 $alias_string .= "(" . join(", ", @{$bug_obj->alias}) . ")" if scalar @{$bug_obj->alias};
>+                 push @aliased_new, $alias_string;

Nit-pick: Please remove space in the @{$bug_obj->alias} join so that the text does not wrap there. 

$alias_string .= "(" . join(",", @{$bug_obj->alias}) . ")" if scalar @{$bug_obj->alias};

For example, one test email I viewed has the line break inside the alias list which looked awkward.

David Lawrence <dkl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |218983(TestAlias2,
                   |                            |TestAlias3)


Or do you think the line could be too long if we have many aliases for a single bug and maybe wrapping
at least will not allow the line to grow too much. 

>+             }
>+             foreach my $dependency_bug (@old_dependencies) {
>+                 my $bug_obj = new Bugzilla::Bug($dependency_bug);
>+                 my $alias_string = $dependency_bug;
>+                 $alias_string .= "(" . join(", ", @{$bug_obj->alias}) . ")" if scalar @{$bug_obj->alias};
>+                 push @aliased_old, $alias_string;
>+             }
>+             if (@aliased_old || @aliased_new) {
>+                 $col_name = "Depends on" if $diff->{'fieldname'} eq 'dependson';
>+                 $col_name = "Blocks" if $diff->{'fieldname'} eq 'blocked';
>+                 $diff->{'text'} = three_columns($col_name, join(", ", @aliased_old), join(", ", @aliased_new));

Leaving the extra space in the join here is ok though as it will put each bug id on it's own line with it's aliases.

For example:

David Lawrence <dkl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |218983(TestAlias2,TestAlias3),
                   |                            |435433(TestAlias4)


Otherwise looks good and works as expected. If you feel that leaving the extra space in the aliases themselves
so as to not let the line become too long then disregard my previous nit-pick.

This is ready to go in either case.
Dave

Comment 6 Jan Pazdziora 2009-02-24 18:43:18 UTC
You guys rock.

I think that having the spaces there does not hurt -- if will wrap in the worst case. The information will be there and if someone is interested in that information, they should be able to parse wrapped lines. ;-)

Thanks,

Jan

Comment 7 Noura El hawary 2009-02-25 09:28:51 UTC
Thanks for the review Dave, I think it is tidier not to let it wrap for single bug aliases, and make it wrap for different bugs. so i changed it as you suggested. all committed now to cvs.

Noura


Note You need to log in before you can comment on or make changes to this bug.