1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-01-19 03:01:11 +01:00

Remove reply handler instructions from email

Summary:
Ref T7199. Although this is useful for discovery, it's un-useful enough that we already have an option to disable it, and most applications do not provide any meaningful instructions.

Throwing it away makes it easier to move forward and lets us get rid of a config option.

This is becoming a more advanced/power-user feature anyway, and the new syntax will be significantly more complex and hard to explain with a one-liner. I'm currently thinking that I'll maybe make the "help" menu a dropdown and give it some options like:

  +---+
  | O |
  +---+---------------------+
  | Maniphest Documentation |
  | Maniphest Email Actions |
  +-------------------------+

Then you click the "Email Actions" thing and get a runtime-derived list of available options. Not sure if I'll actually build that, but I think we can fairly throw the in-mail instructions away even if we don't go in that specific direction.

Test Plan: Grepped for `replyHandlerInstructions`, got no hits.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12229
This commit is contained in:
epriestley 2015-03-31 16:48:17 -07:00
parent 7cf726c7f7
commit 030e05aa4c
22 changed files with 6 additions and 225 deletions

View file

@ -22,14 +22,6 @@ final class PhabricatorAuditReplyHandler extends PhabricatorMailReplyHandler {
'metamta.diffusion.reply-handler-domain');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
return pht('Reply to comment.');
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
$commit = $this->getMailReceiver();
$actor = $this->getActor();

View file

@ -220,6 +220,8 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck {
'security.allow-outbound-http' => pht(
'This option has been replaced with the more granular option '.
'`security.outbound-blacklist`.'),
'metamta.reply.show-hints' => pht(
'Phabricator no longer shows reply hints in mail.'),
);
return $ancient_config;

View file

@ -249,14 +249,6 @@ EODOC
'Domain used for reply email addresses. Some applications can '.
'override this configuration with a different domain.'))
->addExample('phabricator.example.com', ''),
$this->newOption('metamta.reply.show-hints', 'bool', true)
->setBoolOptions(
array(
pht('Show Reply Handler Hints'),
pht('No Reply Handler Hints'),
))
->setSummary(pht('Show hints about reply handler actions in email.'))
->setDescription($reply_hints_description),
$this->newOption('metamta.herald.show-hints', 'bool', true)
->setBoolOptions(
array(

View file

@ -27,14 +27,6 @@ final class ConpherenceReplyHandler extends PhabricatorMailReplyHandler {
return $this->getDefaultPublicReplyHandlerEmailAddress('Z');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
return pht('Reply to comment and attach files.');
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
$conpherence = $this->getMailReceiver();
$user = $this->getActor();

View file

@ -29,53 +29,6 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
'metamta.differential.reply-handler-domain');
}
/*
* Generate text like the following from the supported commands.
* "
*
* ACTIONS
* Reply to comment, or !accept, !reject, !abandon, !resign, !reclaim.
*
* "
*/
public function getReplyHandlerInstructions() {
if (!$this->supportsReplies()) {
return null;
}
$supported_commands = $this->getSupportedCommands();
$text = '';
if (empty($supported_commands)) {
return $text;
}
$comment_command_printed = false;
if (in_array(DifferentialAction::ACTION_COMMENT, $supported_commands)) {
$text .= pht('Reply to comment');
$comment_command_printed = true;
$supported_commands = array_diff(
$supported_commands, array(DifferentialAction::ACTION_COMMENT));
}
if (!empty($supported_commands)) {
if ($comment_command_printed) {
$text .= ', or ';
}
$modified_commands = array();
foreach ($supported_commands as $command) {
$modified_commands[] = '!'.$command;
}
$text .= implode(', ', $modified_commands);
}
$text .= '.';
return $text;
}
public function getSupportedCommands() {
$actions = array(
DifferentialAction::ACTION_COMMENT,

View file

@ -17,14 +17,6 @@ final class FileReplyHandler extends PhabricatorMailReplyHandler {
return $this->getDefaultPublicReplyHandlerEmailAddress('F');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
return pht('Reply to comment or !unsubscribe.');
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
$actor = $this->getActor();
$file = $this->getMailReceiver();

View file

@ -17,15 +17,6 @@ final class FundInitiativeReplyHandler extends PhabricatorMailReplyHandler {
return $this->getDefaultPublicReplyHandlerEmailAddress('I');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
// TODO: Implement.
return null;
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
// TODO: Implement.
return null;

View file

@ -17,14 +17,6 @@ final class LegalpadReplyHandler extends PhabricatorMailReplyHandler {
return $this->getDefaultPublicReplyHandlerEmailAddress('L');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
return pht('Reply to comment or !unsubscribe.');
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
$actor = $this->getActor();
$document = $this->getMailReceiver();

View file

@ -22,16 +22,6 @@ final class PhabricatorMacroReplyHandler extends PhabricatorMailReplyHandler {
'metamta.macro.reply-handler-domain');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
// TODO: Implement.
return null;
return pht('Reply to comment.');
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
// TODO: Implement this.
return null;

View file

@ -22,16 +22,6 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
'metamta.maniphest.reply-handler-domain');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
return pht(
'Reply to comment or attach files, or !close, !claim, '.
'!unsubscribe or !assign <username>.');
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
// NOTE: We'll drop in here on both the "reply to a task" and "create a
// new task" workflows! Make sure you test both if you make changes!

View file

@ -63,7 +63,6 @@ abstract class PhabricatorMailReplyHandler {
'metamta.reply-handler-domain');
}
abstract public function getReplyHandlerInstructions();
abstract protected function receiveEmail(
PhabricatorMetaMTAReceivedMail $mail);

View file

@ -159,26 +159,6 @@ final class PhabricatorMetaMTAMailBody {
}
/**
* Add a section with reply handler instructions.
*
* @param string Reply handler instructions.
* @return this
* @task compose
*/
public function addReplySection($instructions) {
if (!PhabricatorEnv::getEnvConfig('metamta.reply.show-hints')) {
return $this;
}
if (!strlen($instructions)) {
return $this;
}
$this->addTextSection(pht('REPLY HANDLER ACTIONS'), $instructions);
return $this;
}
/**
* Add a section with a link to email preferences.
*
@ -197,6 +177,7 @@ final class PhabricatorMetaMTAMailBody {
return $this;
}
/**
* Add an attachment.
*

View file

@ -13,12 +13,9 @@ HEADER
WHY DID I GET THIS EMAIL?
http://test.com/xscript/
REPLY HANDLER ACTIONS
pike
EOTEXT;
$this->assertEmail($expect, true, true);
$this->assertEmail($expect, true);
}
public function testBodyRenderNoHerald() {
@ -29,42 +26,20 @@ HEADER
bass
trout
REPLY HANDLER ACTIONS
pike
EOTEXT;
$this->assertEmail($expect, false, true);
$this->assertEmail($expect, false);
}
public function testBodyRenderNoReply() {
$expect = <<<EOTEXT
salmon
HEADER
bass
trout
WHY DID I GET THIS EMAIL?
http://test.com/xscript/
EOTEXT;
$this->assertEmail($expect, true, false);
}
private function assertEmail($expect, $herald_hints, $reply_hints) {
private function assertEmail($expect, $herald_hints) {
$env = PhabricatorEnv::beginScopedEnv();
$env->overrideEnvConfig('phabricator.production-uri', 'http://test.com/');
$env->overrideEnvConfig('metamta.herald.show-hints', $herald_hints);
$env->overrideEnvConfig('metamta.reply.show-hints', $reply_hints);
$body = new PhabricatorMetaMTAMailBody();
$body->addRawSection('salmon');
$body->addTextSection('HEADER', "bass\ntrout\n");
$body->addHeraldSection('/xscript/');
$body->addReplySection('pike');
$this->assertEqual($expect, $body->render());
}

View file

@ -20,10 +20,6 @@ final class OwnersPackageReplyHandler extends PhabricatorMailReplyHandler {
return null;
}
public function getReplyHandlerInstructions() {
return null;
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
return;
}

View file

@ -17,14 +17,6 @@ final class PasteReplyHandler extends PhabricatorMailReplyHandler {
return $this->getDefaultPublicReplyHandlerEmailAddress('P');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
return pht('Reply to comment or !unsubscribe.');
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
$actor = $this->getActor();
$paste = $this->getMailReceiver();

View file

@ -22,16 +22,6 @@ final class PholioReplyHandler extends PhabricatorMailReplyHandler {
'metamta.pholio.reply-handler-domain');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
// TODO: Implement.
return null;
return pht('Reply to comment.');
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
// TODO: Implement this.
return null;

View file

@ -17,15 +17,6 @@ final class PhortuneCartReplyHandler extends PhabricatorMailReplyHandler {
return $this->getDefaultPublicReplyHandlerEmailAddress('CART');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
// TODO: Implement.
return null;
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
// TODO: Implement.
return null;

View file

@ -20,15 +20,6 @@ final class PhrictionReplyHandler extends PhabricatorMailReplyHandler {
PhrictionDocumentPHIDType::TYPECONST);
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
// TODO: Implement.
return null;
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
// TODO: Implement.
return null;

View file

@ -17,10 +17,6 @@ final class PonderQuestionReplyHandler extends PhabricatorMailReplyHandler {
return $this->getDefaultPublicReplyHandlerEmailAddress('Q');
}
public function getReplyHandlerInstructions() {
return null;
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
// ignore this entirely for now
}

View file

@ -17,14 +17,6 @@ final class ReleephRequestReplyHandler extends PhabricatorMailReplyHandler {
return $this->getDefaultPublicReplyHandlerEmailAddress('RERQ');
}
public function getReplyHandlerInstructions() {
if ($this->supportsReplies()) {
return pht('Reply to comment.');
} else {
return null;
}
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
$rq = $this->getMailReceiver();
$user = $this->getActor();

View file

@ -16,10 +16,6 @@ final class PhabricatorRepositoryPushReplyHandler
return null;
}
public function getReplyHandlerInstructions() {
return null;
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
return;
}

View file

@ -1983,10 +1983,6 @@ abstract class PhabricatorApplicationTransactionEditor
$action = $this->getMailAction($object, $xactions);
$reply_handler = $this->buildReplyHandler($object);
$reply_section = $reply_handler->getReplyHandlerInstructions();
if ($reply_section !== null) {
$body->addReplySection($reply_section);
}
$body->addEmailPreferenceSection();