Commit 75382864 authored by epriestley's avatar epriestley
Browse files

Fix missing link targets for "View Object" header buttons in HTML email

Summary:
See <https://discourse.phabricator-community.org/t/view-task-from-maniphest-e-mail-doesnt-have-url/2827>.

I added "View Task" / "View Commit" buttons recently but the logic for generating URIs isn't quite right. Fix it up.

Test Plan:
  - Commented on a task.
  - Used `bin/mail show-outbound --id ... --dump-html > out.html` to dump the HTML.
  - Previewed the HTML in a browser.
  - This time, actually clicked the button to go to the task.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20586
parent 6219d30f
......@@ -485,7 +485,8 @@ final class PhabricatorAuditEditor
return $phids;
}
protected function getObjectLinkButtonLabelForMail() {
protected function getObjectLinkButtonLabelForMail(
PhabricatorLiskDAO $object) {
return pht('View Commit');
}
......
......@@ -601,7 +601,8 @@ final class DifferentialTransactionEditor
return $xactions;
}
protected function getObjectLinkButtonLabelForMail() {
protected function getObjectLinkButtonLabelForMail(
PhabricatorLiskDAO $object) {
return pht('View Revision');
}
......@@ -614,8 +615,7 @@ final class DifferentialTransactionEditor
$body = id(new PhabricatorMetaMTAMailBody())
->setViewer($viewer);
$revision_uri = $object->getURI();
$revision_uri = PhabricatorEnv::getProductionURI($revision_uri);
$revision_uri = $this->getObjectLinkButtonURIForMail($object);
$new_uri = $revision_uri.'/new/';
$this->addHeadersAndCommentsToMailBody(
......
......@@ -206,7 +206,8 @@ final class ManiphestTransactionEditor
->setSubject("T{$id}: {$title}");
}
protected function getObjectLinkButtonLabelForMail() {
protected function getObjectLinkButtonLabelForMail(
PhabricatorLiskDAO $object) {
return pht('View Task');
}
......
......@@ -3423,17 +3423,39 @@ abstract class PhabricatorApplicationTransactionEditor
->setContextObject($object);
$button_label = $this->getObjectLinkButtonLabelForMail($object);
$button_uri = $this->getObjectLinkButtonURIForMail($object);
$this->addHeadersAndCommentsToMailBody(
$body,
$xactions,
$button_label,
$button_uri);
$this->addHeadersAndCommentsToMailBody($body, $xactions, $button_label);
$this->addCustomFieldsToMailBody($body, $object, $xactions);
return $body;
}
protected function getObjectLinkButtonLabelForMail() {
protected function getObjectLinkButtonLabelForMail(
PhabricatorLiskDAO $object) {
return null;
}
protected function getObjectLinkButtonURIForMail(
PhabricatorLiskDAO $object) {
// Most objects define a "getURI()" method which does what we want, but
// this isn't formally part of an interface at time of writing. Try to
// call the method, expecting an exception if it does not exist.
try {
$uri = $object->getURI();
return PhabricatorEnv::getProductionURI($uri);
} catch (Exception $ex) {
return null;
}
}
/**
* @task mail
*/
......@@ -3455,7 +3477,7 @@ abstract class PhabricatorApplicationTransactionEditor
PhabricatorMetaMTAMailBody $body,
array $xactions,
$object_label = null,
$object_href = null) {
$object_uri = null) {
// First, remove transactions which shouldn't be rendered in mail.
foreach ($xactions as $key => $xaction) {
......@@ -3521,7 +3543,7 @@ abstract class PhabricatorApplicationTransactionEditor
$headers_html = phutil_implode_html(phutil_tag('br'), $headers_html);
$header_button = null;
if ($object_label !== null) {
if ($object_label !== null && $object_uri !== null) {
$button_style = array(
'text-decoration: none;',
'padding: 4px 8px;',
......@@ -3540,7 +3562,7 @@ abstract class PhabricatorApplicationTransactionEditor
'a',
array(
'style' => implode(' ', $button_style),
'href' => $object_href,
'href' => $object_uri,
),
$object_label);
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment