From 952829714b22a5020afabb02b6bab6ce0c8572fa Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Wed, 8 Jul 2015 15:34:04 +0800 Subject: Bug 1143132: emails about redirected attachment urls link to attachment-edit instead of the actual url (mozreview, github, etc) --- extensions/BMO/Extension.pm | 28 ++++++++++++++-------- extensions/BMO/lib/Data.pm | 3 +++ .../en/default/hook/request/email-start.txt.tmpl | 15 ++++++++++++ template/en/default/request/email.txt.tmpl | 12 +++++++--- 4 files changed, 45 insertions(+), 13 deletions(-) create mode 100644 extensions/BMO/template/en/default/hook/request/email-start.txt.tmpl 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 %] -- cgit v1.2.3-24-g4f1b