1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-27 01:02:42 +01:00

Improve voicing in text published to JIRA issues

Summary:
Ref T3687. JIRA is able to piggyback on a fair amount of Asana infrastructure, but the voicing we use on Asana tasks (which are always about one object) isn't very good for JIRA issues (which may have many linked objects). Specifically, we publish stories like this to Asana:

  alincoln accepted this revision.

This is meaningless in JIRA since you have no idea what it's talking about. Instead, publish like this:

  alincoln accepted D999: Put a bird on it

Additionally, supplement it with a URI, so the total story text we publish is:

  alincoln accepted D999: Put a bird on it

  https://phabricator.whitehouse.gov/D999

Signifcantly less useless!

Test Plan: {F57523} {F57524}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3687

Differential Revision: https://secure.phabricator.com/D6907
This commit is contained in:
epriestley 2013-09-10 15:22:24 -07:00
parent 3a28f86a6e
commit 8e45b466da
7 changed files with 86 additions and 8 deletions

View file

@ -93,9 +93,11 @@ final class DifferentialDoorkeeperRevisionFeedStoryPublisher
}
public function getStoryText($object) {
$implied_context = $this->getRenderWithImpliedContext();
$story = $this->getFeedStory();
if ($story instanceof PhabricatorFeedStoryDifferential) {
$text = $story->renderForAsanaBridge();
$text = $story->renderForAsanaBridge($implied_context);
} else {
$text = $story->renderText();
}

View file

@ -176,9 +176,11 @@ final class DiffusionDoorkeeperCommitFeedStoryPublisher
}
public function getStoryText($object) {
$implied_context = $this->getRenderWithImpliedContext();
$story = $this->getFeedStory();
if ($story instanceof PhabricatorFeedStoryAudit) {
$text = $story->renderForAsanaBridge();
$text = $story->renderForAsanaBridge($implied_context);
} else {
$text = $story->renderText();
}

View file

@ -1,9 +1,55 @@
<?php
/**
* @task config Configuration
*/
abstract class DoorkeeperFeedStoryPublisher {
private $feedStory;
private $viewer;
private $renderWithImpliedContext;
/* -( Configuration )------------------------------------------------------ */
/**
* Render story text using contextual langauge to identify the object the
* story is about, instead of the full object name. For example, without
* contextual language a story might render like this:
*
* alincoln created D123: Chop Wood for Log Cabin v2.0
*
* With contextual langauge, it will render like this instead:
*
* alincoln created this revision.
*
* If the interface where the text will be displayed is specific to an
* individual object (like Asana tasks that represent one review or commit
* are), it's generally more natural to use language that assumes context.
* If the target context may show information about several objects (like
* JIRA issues which can have several linked revisions), it's generally
* more useful not to assume context.
*
* @param bool True to assume object context when rendering.
* @return this
* @task config
*/
public function setRenderWithImpliedContext($render_with_implied_context) {
$this->renderWithImpliedContext = $render_with_implied_context;
return $this;
}
/**
* Determine if rendering should assume object context. For discussion, see
* @{method:setRenderWithImpliedContext}.
*
* @return bool True if rendering should assume object context is implied.
* @task config
*/
public function getRenderWithImpliedContext() {
return $this->renderWithImpliedContext;
}
public function setFeedStory(PhabricatorFeedStory $feed_story) {
$this->feedStory = $feed_story;

View file

@ -414,7 +414,9 @@ final class DoorkeeperFeedWorkerAsana extends DoorkeeperFeedWorker {
// because everything else is idempotent, so this is the only effect we
// can't safely run more than once.
$text = $publisher->getStoryText($object);
$text = $publisher
->setRenderWithImpliedContext(true)
->getStoryText($object);
$this->makeAsanaAPICall(
$oauth_token,

View file

@ -54,7 +54,7 @@ final class DoorkeeperFeedWorkerJIRA extends DoorkeeperFeedWorker {
return;
}
$story_text = $publisher->getStoryText($object);
$story_text = $this->renderStoryText();
$xobjs = mgroup($xobjs, 'getApplicationDomain');
foreach ($xobjs as $domain => $xobj_list) {
@ -156,4 +156,14 @@ final class DoorkeeperFeedWorkerJIRA extends DoorkeeperFeedWorker {
return $try_users;
}
private function renderStoryText() {
$object = $this->getStoryObject();
$publisher = $this->getPublisher();
$text = $publisher->getStoryText($object);
$uri = $publisher->getObjectURI($object);
return $text."\n\n".$uri;
}
}

View file

@ -50,7 +50,7 @@ final class PhabricatorFeedStoryAudit extends PhabricatorFeedStory {
// TODO: At some point, make feed rendering not terrible and remove this
// hacky mess.
public function renderForAsanaBridge() {
public function renderForAsanaBridge($implied_context = false) {
$data = $this->getStoryData();
$comment = $data->getValue('content');
@ -58,7 +58,15 @@ final class PhabricatorFeedStoryAudit extends PhabricatorFeedStory {
$action = $this->getValue('action');
$verb = PhabricatorAuditActionConstants::getActionPastTenseVerb($action);
$title = "{$author_name} {$verb} this commit.";
$commit_phid = $this->getPrimaryObjectPHID();
$commit_name = $this->getHandle($commit_phid)->getFullName();
if ($implied_context) {
$title = "{$author_name} {$verb} this commit.";
} else {
$title = "{$author_name} {$verb} commit {$commit_name}.";
}
if (strlen($comment)) {
$engine = PhabricatorMarkupEngine::newMarkupEngine(array())
->setConfig('viewer', new PhabricatorUser())

View file

@ -90,7 +90,7 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory {
// TODO: At some point, make feed rendering not terrible and remove this
// hacky mess.
public function renderForAsanaBridge() {
public function renderForAsanaBridge($implied_context = false) {
$data = $this->getStoryData();
$comment = $data->getValue('feedback_content');
@ -102,7 +102,15 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory {
->setConfig('viewer', new PhabricatorUser())
->setMode(PhutilRemarkupEngine::MODE_TEXT);
$title = "{$author_name} {$verb} this revision.";
$revision_phid = $this->getPrimaryObjectPHID();
$revision_name = $this->getHandle($revision_phid)->getFullName();
if ($implied_context) {
$title = "{$author_name} {$verb} this revision.";
} else {
$title = "{$author_name} {$verb} revision {$revision_name}.";
}
if (strlen($comment)) {
$comment = $engine->markupText($comment);