Bug 1143132: emails about redirected attachment urls link to attachment-edit instead of the actual url (mozreview, github, etc)

4 files changed, 45 insertions, 13 deletions

diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm
index 1a254abab..db3385e77 100644
--- a/extensions/BMO/Extension.pm
+++ b/extensions/BMO/Extension.pm
@@ -46,7 +46,7 @@ use DateTime;
 use Email::MIME::ContentType qw(parse_content_type);
 use Encode qw(find_encoding encode_utf8);
 use File::MimeInfo::Magic;
-use List::MoreUtils qw(natatime);
+use List::MoreUtils qw(natatime any);
 use List::Util qw(first);
 use Scalar::Util qw(blessed);
 use Sys::Syslog qw(:DEFAULT setlogsock);
@@ -77,6 +77,7 @@ BEGIN {
     *Bugzilla::check_default_product_security_group = \&_check_default_product_security_group;
     *Bugzilla::Attachment::is_bounty_attachment     = \&_attachment_is_bounty_attachment;
     *Bugzilla::Attachment::bounty_details           = \&_attachment_bounty_details;
+    *Bugzilla::Attachment::external_redirect        = \&_attachment_external_redirect;
 }
 
 sub template_before_process {
@@ -926,7 +927,7 @@ sub attachment_process_data {
         $url = $data;
     }
 
-    if (my $content_type = _detect_attached_url($url)) {
+    if (my $content_type = _detect_attached_url($url)->{content_type}) {
         $attributes->{mimetype} = $content_type;
         $attributes->{ispatch}  = 0;
     }
@@ -943,13 +944,25 @@ sub _detect_attached_url {
 
     foreach my $key (keys %autodetect_attach_urls) {
         if ($url =~ $autodetect_attach_urls{$key}->{regex}) {
-            return $autodetect_attach_urls{$key}->{content_type};
+            return $autodetect_attach_urls{$key};
         }
     }
 
     return undef;
 }
 
+sub _attachment_external_redirect {
+    my ($self) = @_;
+
+    # must be our supported content-type
+    return undef unless
+        any { $self->contenttype eq $autodetect_attach_urls{$_}->{content_type} }
+        keys %autodetect_attach_urls;
+
+    # must still be a valid url
+    return _detect_attached_url($self->data)
+}
+
 # redirect automatically to github urls
 sub attachment_view {
     my ($self, $args) = @_;
@@ -959,13 +972,8 @@ sub attachment_view {
     # don't redirect if the content-type is specified explicitly
     return if defined $cgi->param('content_type');
 
-    # must be our supported content-type
-    return unless
-        grep { $attachment->contenttype eq $autodetect_attach_urls{$_}->{content_type} }
-        keys %autodetect_attach_urls;
-
-    # must still be a valid url
-    return unless _detect_attached_url($attachment->data);
+    # must be a valid redirection url
+    return unless defined $attachment->external_redirect;
 
     # redirect
     print $cgi->redirect(trim($attachment->data));
diff --git a/extensions/BMO/lib/Data.pm b/extensions/BMO/lib/Data.pm
index 2e6f7e020..dc2a289a4 100644
--- a/extensions/BMO/lib/Data.pm
+++ b/extensions/BMO/lib/Data.pm
@@ -26,14 +26,17 @@ our @EXPORT = qw( $cf_visible_in_products
 # attachment.
 our %autodetect_attach_urls = (
     github_pr => {
+        title        => 'GitHub Pull Request',
         regex        => qr#^https://github\.com/[^/]+/[^/]+/pull/\d+/?$#i,
         content_type => 'text/x-github-pull-request',
     },
     reviewboard => {
+        title        => 'MozReview',
         regex        => qr#^https?://reviewboard(?:-dev)?\.(?:allizom|mozilla)\.org/r/\d+/?#i,
         content_type => 'text/x-review-board-request',
     },
     google_docs => {
+        title        => 'Google Doc',
         regex        => qr#^https://docs\.google\.com/(?:document|spreadsheets|presentation)/d/#i,
         content_type => 'text/x-google-doc',
     },
diff --git a/extensions/BMO/template/en/default/hook/request/email-start.txt.tmpl b/extensions/BMO/template/en/default/hook/request/email-start.txt.tmpl
new file mode 100644
index 000000000..bd2af3d6f
--- /dev/null
+++ b/extensions/BMO/template/en/default/hook/request/email-start.txt.tmpl
@@ -0,0 +1,15 @@
+[%# This Source Code Form is subject to the terms of the Mozilla Public
+  # License, v. 2.0. If a copy of the MPL was not distributed with this
+  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
+  #
+  # This Source Code Form is "Incompatible With Secondary Licenses", as
+  # defined by the Mozilla Public License, v. 2.0.
+  #%]
+
+[%
+  RETURN UNLESS attachment;
+  external = attachment.external_redirect;
+  RETURN UNLESS external;
+  attach_url.title = external.title;
+  attach_url.href  = attachment.data;
+%]
diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl
index 01d72260f..2b9ea3782 100644
--- a/template/en/default/request/email.txt.tmpl
+++ b/template/en/default/request/email.txt.tmpl
@@ -25,14 +25,17 @@
 [% bugidsummary = bug.bug_id _ ': ' _ bug.short_desc %]
 [% attidsummary = attachment.id _ ': ' _ attachment.description %]
 [% flagtype_name = flag ? flag.type.name : old_flag.type.name %]
-[%# Upstreaming: denied (bug 621883) %]
-[% statuses = { '+' => "granted" , '-' => 'not granted' , 'X' => "canceled" ,
-                '?' => "asked" } %]
+[% statuses = { '+' => "granted" , '-' => 'not granted' , 'X' => "canceled" , '?' => "asked" } %]
 
 [% to_identity = "" %]
 [% on_behalf_of = 0 %]
 [% action = flag.status || 'X' %]
 
+[%
+  attach_url = { title => "", href => "" };
+  Hook.process("start");
+%]
+
 [% IF flag && flag.status == '?' %]
   [% subject_status = "requested" %]
   [% IF flag.setter_id == user.id %]
@@ -78,6 +81,9 @@ X-Bugzilla-Flag-Requestee: [% flag.requestee.email %]
 [% FILTER bullet = wrap(80) %]
 Attachment [% attidsummary %]
 [%- END %]
+[% IF attach_url.href %]
+[%+ attach_url.title _ ": " _ attach_url.href %]
+[%- END %]
 [%+ urlbase %]attachment.cgi?id=[% attachment.id %]&action=edit
 [%- END %]