r92833 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r92832‎ | r92833 | r92834 >
Date:00:09, 22 July 2011
Author:mah
Status:resolved (Comments)
Tags:
Comment:
Followup Bug #18831: Apply MZMcBride's patch do make the regex a
little better (according to my own testing).
Modified paths:
  • /trunk/tools/wikibugs/wikibugs (modified) (history)

Diff [purge]

Index: trunk/tools/wikibugs/wikibugs
@@ -15,23 +15,23 @@
1616 my $output;
1717
1818 my $shash = {
19 - 'enhancement' => "\00315enhancement\003",
20 - 'trivial' => 'trivial',
21 - 'minor' => "minor",
22 - 'normal' => "normal",
23 - 'major' => "major",
24 - 'critical' => "\00304CRIT\003",
25 - 'blocker' => "\00304\002BLOCKER\002\003"
 19+ 'enhancement' => "\00315enhancement\003",
 20+ 'trivial' => 'trivial',
 21+ 'minor' => "minor",
 22+ 'normal' => "normal",
 23+ 'major' => "major",
 24+ 'critical' => "\00304CRIT\003",
 25+ 'blocker' => "\00304\002BLOCKER\002\003"
2626 };
2727
2828 my $rhash = {
29 - 'WORKSFORME' => "\00314(WFM)\003",
30 - 'INVALID' => "\00314(INVALID)\003",
31 - 'DUPLICATE' => "\00314(DUP)\003",
32 - 'FIXED' => "\00303(FIXED)\003",
33 - 'WONTFIX' => "\00303(WONTFIX)\003",
34 - 'LATER' => "\00306(LATER)\003",
35 - 'REMIND' => "\00306(REMIND)\003"
 29+ 'WORKSFORME' => "\00314(WFM)\003",
 30+ 'INVALID' => "\00314(INVALID)\003",
 31+ 'DUPLICATE' => "\00314(DUP)\003",
 32+ 'FIXED' => "\00303(FIXED)\003",
 33+ 'WONTFIX' => "\00303(WONTFIX)\003",
 34+ 'LATER' => "\00306(LATER)\003",
 35+ 'REMIND' => "\00306(REMIND)\003"
3636 };
3737
3838 use Email::MIME;
@@ -42,128 +42,121 @@
4343 my $body = $mail->body;
4444 my $subject = $mail->header( 'Subject' );
4545
46 -if ($from =~ /^bugzilla-daemon/)
47 -{
48 - # E-mail is from MediaZilla
 46+# Check that the e-mail is from MediaZilla.
 47+if ($from =~ /^bugzilla-daemon/) {
4948
50 - $/ = "";
51 - # extract and remove the comment section
52 - my ($haschanges, $user);
53 - my $comment = undef;
54 - for ($body) {
55 - # Use fancy lookahead magic to match comments in the form of
56 - # "Foo <bar@baz.com>" or "bing@boop.com".
57 - if ( m{^--- Comment #\d+ from (.*) ?<?\S+\@.*}m ) {
58 - $user = $1;
59 - $haschanges = 1;
60 - $comment = 1;
61 - }
62 - if ( m{^(.*) ?<?(\S+)\@\S+>? changed:$}m ) {
63 - # Try to grab the "real name" again.
64 - $haschanges = 1;
65 - $user = $1 if !$user;
66 - } else {
67 - # If "real name" isn't available and we didn't get a user from the
68 - # comment header, just use the "X-Bugzilla-Who" header.
69 - my @who = split '@', $mail->header( 'X-Bugzilla-Who' );
70 - $user = $who[0];
71 - }
72 - }
 49+ $/ = "";
 50+ # Extract and remove the comment section.
 51+ my ($haschanges, $user);
 52+ my $comment = undef;
 53+ for ($body) {
 54+ # Attempt to grab the "real name".
 55+ if ( m{^--- Comment #\d+ from (.*) <\S+\@.*}m ) {
 56+ $haschanges = 1;
 57+ $user = $1;
 58+ $comment = 1;
 59+ }
 60+ # Re-attempt to grab the "real name".
 61+ if ( m{^(.*) <\S+\@\S+> changed:$}m ) {
 62+ $haschanges = 1;
 63+ $user = $1 if !$user;
 64+ } else {
 65+ # If "real name" isn't available and we didn't get a user from the
 66+ # comment header, just use the "X-Bugzilla-Who" header.
 67+ my @who = split '@', $mail->header( 'X-Bugzilla-Who' );
 68+ $user = $who[0];
 69+ }
 70+ }
 71+ die $user;
7372
74 - my @changed_fields = split /\s+/, $mail->header( 'X-Bugzilla-Changed-Fields' );
 73+ my @changed_fields = split /\s+/, $mail->header( 'X-Bugzilla-Changed-Fields' );
7574
7675
77 - # Check if this is a dependency e-mail. If so, ignore it.
78 - # We have removed the comment section to prevent people from using
79 - # this by adding the right text to a comment.
80 - if ($body !~ /^Bug \d+ depends on bug \d+, which changed state/m)
81 - {
 76+ # Check if this is a dependency e-mail. If so, ignore it.
 77+ # We have removed the comment section to prevent people from using
 78+ # this by adding the right text to a comment.
 79+ if ($body !~ /^Bug \d+ depends on bug \d+, which changed state/m) {
8280
83 - my ($bug, $summary, $st);
84 - if ($subject =~ /\[Bug (\d+)\]\s+New:\s+(.*)/s) {
85 - ($bug, $summary, $st) = ($1, $2, "\00303(NEW)\003");
86 - } elsif ($subject =~ /\[Bug (\d+)\]\s(.*)/s) {
87 - ($bug, $summary, $st) = ($1, $2, "\00303(mod)\003");
88 - }
 81+ my ($bug, $summary, $st);
 82+ if ($subject =~ /\[Bug (\d+)\]\s+New:\s+(.*)/s) {
 83+ ($bug, $summary, $st) = ($1, $2, "\00303(NEW)\003");
 84+ } elsif ($subject =~ /\[Bug (\d+)\]\s(.*)/s) {
 85+ ($bug, $summary, $st) = ($1, $2, "\00303(mod)\003");
 86+ }
8987
90 - ## Set the URL to the URL found in the message body if available, else construct our own URL
91 - my $url =
92 - $body =~ /^(http.*\/)show_bug\.cgi\?id=(.*)$/m
93 - ? "$1$2" # short URL!
94 - : "http://bugzilla.wikimedia.org/show_bug.cgi?id=$bug";
 88+ ## Set the URL to the URL found in the message body if available, else construct our own URL
 89+ my $url =
 90+ $body =~ /^(http.*\/)show_bug\.cgi\?id=(.*)$/m
 91+ ? "$1$2" # short URL!
 92+ : "http://bugzilla.wikimedia.org/show_bug.cgi?id=$bug";
9593
96 - $summary =~ s/\s+/ /g;
 94+ $summary =~ s/\s+/ /g;
9795
98 - # We are going to append stuff to the beginning of $output later.
99 - # This stuff is going to contain $st. But we want a chance of changing it first.
100 - $output = "";
 96+ # We are going to append stuff to the beginning of $output later.
 97+ # This stuff is going to contain $st. But we want a chance of changing it first.
 98+ $output = "";
10199
102 - if ($st eq "\00303(NEW)\003")
103 - {
104 - my $product = $mail->header( 'X-Bugzilla-Product' );
105 - my $component = $mail->header( 'X-Bugzilla-Component' );
106 - my $severity = $mail->header( 'X-Bugzilla-Severity' );
107 - ## Doesn't seem to be sent as a header.
108 - my $reporter = $1 if $body =~ /ReportedBy: (.*)\@.*$/m;
 100+ if ($st eq "\00303(NEW)\003") {
 101+ my $product = $mail->header( 'X-Bugzilla-Product' );
 102+ my $component = $mail->header( 'X-Bugzilla-Component' );
 103+ my $severity = $mail->header( 'X-Bugzilla-Severity' );
 104+ ## Doesn't seem to be sent as a header.
 105+ my $reporter = $1 if $body =~ /ReportedBy: (.*)\@.*$/m;
109106
110 - $output .= "$severity; \002$product\002\: $component; (\002$reporter\002)\n";
111 - }
112 - else
113 - {
114 - if ($haschanges) {
115 - my @outputs;
116 - my $status = $mail->header( 'X-Bugzilla-Status' );
117 - if ($status eq 'NEW') {
118 - $st = "\00303(mod)\003";
119 - } elsif ($status eq 'REOPENED' && grep {$_ eq 'Status'} @changed_fields) {
120 - $st = "\00304(REOPENED)\003";
121 - } elsif ( grep {$_ eq 'Status'} @changed_fields ) {
122 - $st = "\00303($status)\003";
123 - } else {
124 - $st = "\00303(mod)\003";
125 - }
 107+ $output .= "$severity; \002$product\002\: $component; (\002$reporter\002)\n";
 108+ } else {
 109+ if ($haschanges) {
 110+ my @outputs;
 111+ my $status = $mail->header( 'X-Bugzilla-Status' );
 112+ if ($status eq 'NEW') {
 113+ $st = "\00303(mod)\003";
 114+ } elsif ($status eq 'REOPENED' && grep {$_ eq 'Status'} @changed_fields) {
 115+ $st = "\00304(REOPENED)\003";
 116+ } elsif ( grep {$_ eq 'Status'} @changed_fields ) {
 117+ $st = "\00303($status)\003";
 118+ } else {
 119+ $st = "\00303(mod)\003";
 120+ }
126121
127 - if ($st eq "\00303(RESOLVED)\003" && $body =~ /Resolution\|\s+\|(\w+)/m) {
128 - $st = $rhash->{$1};
129 - }
130 - if ($body =~ /Severity\|(\w+)\s+\|(\w+)/m) {
131 - push @outputs, "$shash->{$1}\->$shash->{$2}";
132 - }
133 - if ($body =~ /Keywords\|.*$/s) {
134 - my @lines = split (/\n/, $&);
135 - my $added = '';
136 - my $removed = '';
137 - foreach my $a ( @lines )
138 - {
139 - last unless $a =~ /^(Keywords|\s+)\|(.*?)\s*\|(.*?)\s*$/;
140 - $removed .= $2;
141 - $added .= $3;
142 - }
143 - push @outputs, join ' ', (
144 - ($removed =~ /\S/ ? join (' ', map { "-$_" } split (/\s*,\s*/, $removed)) : ''),
145 - ($added =~ /\S/ ? join (' ', map { "+$_" } split (/\s*,\s*/, $added )) : '')
146 - );
147 - }
 122+ if ($st eq "\00303(RESOLVED)\003" && $body =~ /Resolution\|\s+\|(\w+)/m) {
 123+ $st = $rhash->{$1};
 124+ }
 125+ if ($body =~ /Severity\|(\w+)\s+\|(\w+)/m) {
 126+ push @outputs, "$shash->{$1}\->$shash->{$2}";
 127+ }
 128+ if ($body =~ /Keywords\|.*$/s) {
 129+ my @lines = split (/\n/, $&);
 130+ my $added = '';
 131+ my $removed = '';
 132+ foreach my $a ( @lines ) {
 133+ last unless $a =~ /^(Keywords|\s+)\|(.*?)\s*\|(.*?)\s*$/;
 134+ $removed .= $2;
 135+ $added .= $3;
 136+ }
 137+ push @outputs, join ' ', (
 138+ ($removed =~ /\S/ ? join (' ', map { "-$_" } split (/\s*,\s*/, $removed)) : ''),
 139+ ($added =~ /\S/ ? join (' ', map { "+$_" } split (/\s*,\s*/, $added )) : '')
 140+ );
 141+ }
148142
149 - push @outputs, 'summary' if $body =~ /Summary\|.*?\|.*?/;
 143+ push @outputs, 'summary' if $body =~ /Summary\|.*?\|.*?/;
150144
151 - push @outputs, 'deps' if $body =~ /OtherBugs\w+\|.*?\|.*?$/m;
 145+ push @outputs, 'deps' if $body =~ /OtherBugs\w+\|.*?\|.*?$/m;
152146
153 - push @outputs, "+comment" if $comment;
 147+ push @outputs, "+comment" if $comment;
154148
155 - $output .= " " . join ('; ', @outputs) if @outputs;
 149+ $output .= " " . join ('; ', @outputs) if @outputs;
156150
157 - }
158 - $output .= " (\002\00310$user\003\002)\n";
159 - }
160 - $output = "$st $summary - \00310$url\003 " . $output;
161 - }
 151+ }
 152+ $output .= " (\002\00310$user\003\002)\n";
 153+ }
 154+ $output = "$st $summary - \00310$url\003 " . $output;
 155+ }
162156 }
163157
164 -if ($output)
165 -{
166 - open (OUT, ">>/var/wikibugs/wikibugs.log");
167 - #print $output;
168 - print OUT $output;
169 - close OUT;
 158+if ($output) {
 159+ open (OUT, ">>/var/wikibugs/wikibugs.log");
 160+ #print $output;
 161+ print OUT $output;
 162+ close OUT;
170163 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r93776No dieing. Kthx r92833reedy18:44, 2 August 2011
r113013Bug 18831 always use real name when available...hashar10:39, 5 March 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r89717Fix Bug #18831 (“Wikibugs should use real name instead of e-mail...mah04:08, 8 June 2011

Comments

#Comment by MZMcBride (talk | contribs)   01:41, 22 July 2011

Thanks!

#Comment by Reedy (talk | contribs)   18:28, 2 August 2011

This seems to have brokened wikibugs

#Comment by ST47 (talk | contribs)   18:42, 2 August 2011

I'm guessing that the "die $user;" line wasn't supposed to make it in. Please delete that line and it will all be OK. It's line 70.

#Comment by MarkAHershberger (talk | contribs)   13:59, 3 August 2011

reedy fixed in r93776

Status & tagging log