mirror of
https://we.phorge.it/source/phorge.git
synced 2024-09-20 01:08:50 +02:00
Make maniphest add CCs when users are @mentioned
Summary: We don't currently add CCs, but should (similar to how Differential works). This also fixes some problems where you can get no-op CC transactions, and makes mentions a little less aggressively colored. Test Plan: Applied a bunch of CC/mention transactions to tasks and observed the behavior. Reviewed By: jungejason Reviewers: jungejason, tuomaspelkonen, aran, codeblock CC: aran, jungejason Differential Revision: 634
This commit is contained in:
epriestley
parent
30733ca575
commit
4452239d61
5 changed files with 101 additions and 43 deletions
|
|
@ -1026,7 +1026,7 @@ celerity_register_resource_map(array(
|
|||
),
|
||||
'phabricator-feed-css' =>
|
||||
array(
|
||||
'uri' => '/res/80952af2/rsrc/css/application/feed/feed.css',
|
||||
'uri' => '/res/617811ab/rsrc/css/application/feed/feed.css',
|
||||
'type' => 'css',
|
||||
'requires' =>
|
||||
array(
|
||||
|
|
@ -1080,7 +1080,7 @@ celerity_register_resource_map(array(
|
|||
),
|
||||
'phabricator-remarkup-css' =>
|
||||
array(
|
||||
'uri' => '/res/e0c44a00/rsrc/css/core/remarkup.css',
|
||||
'uri' => '/res/7ca89111/rsrc/css/core/remarkup.css',
|
||||
'type' => 'css',
|
||||
'requires' =>
|
||||
array(
|
||||
|
|
@ -1192,7 +1192,24 @@ celerity_register_resource_map(array(
|
|||
'uri' => '/res/pkg/307df223/javelin.pkg.js',
|
||||
'type' => 'js',
|
||||
),
|
||||
'8e9024dc' =>
|
||||
'95b66c1a' =>
|
||||
array (
|
||||
'name' => 'differential.pkg.css',
|
||||
'symbols' =>
|
||||
array (
|
||||
0 => 'differential-core-view-css',
|
||||
1 => 'differential-changeset-view-css',
|
||||
2 => 'differential-revision-detail-css',
|
||||
3 => 'differential-revision-history-css',
|
||||
4 => 'differential-table-of-contents-css',
|
||||
5 => 'differential-revision-comment-css',
|
||||
6 => 'differential-revision-add-comment-css',
|
||||
7 => 'differential-revision-comment-list-css',
|
||||
),
|
||||
'uri' => '/res/pkg/95b66c1a/differential.pkg.css',
|
||||
'type' => 'css',
|
||||
),
|
||||
'a06761eb' =>
|
||||
array (
|
||||
'name' => 'core.pkg.css',
|
||||
'symbols' =>
|
||||
|
|
@ -1213,24 +1230,7 @@ celerity_register_resource_map(array(
|
|||
13 => 'phabricator-remarkup-css',
|
||||
14 => 'syntax-highlighting-css',
|
||||
),
|
||||
'uri' => '/res/pkg/8e9024dc/core.pkg.css',
|
||||
'type' => 'css',
|
||||
),
|
||||
'95b66c1a' =>
|
||||
array (
|
||||
'name' => 'differential.pkg.css',
|
||||
'symbols' =>
|
||||
array (
|
||||
0 => 'differential-core-view-css',
|
||||
1 => 'differential-changeset-view-css',
|
||||
2 => 'differential-revision-detail-css',
|
||||
3 => 'differential-revision-history-css',
|
||||
4 => 'differential-table-of-contents-css',
|
||||
5 => 'differential-revision-comment-css',
|
||||
6 => 'differential-revision-add-comment-css',
|
||||
7 => 'differential-revision-comment-list-css',
|
||||
),
|
||||
'uri' => '/res/pkg/95b66c1a/differential.pkg.css',
|
||||
'uri' => '/res/pkg/a06761eb/core.pkg.css',
|
||||
'type' => 'css',
|
||||
),
|
||||
'd0713563' =>
|
||||
|
|
@ -1266,15 +1266,15 @@ celerity_register_resource_map(array(
|
|||
),
|
||||
'reverse' =>
|
||||
array (
|
||||
'aphront-crumbs-view-css' => '8e9024dc',
|
||||
'aphront-dialog-view-css' => '8e9024dc',
|
||||
'aphront-form-view-css' => '8e9024dc',
|
||||
'aphront-list-filter-view-css' => '8e9024dc',
|
||||
'aphront-panel-view-css' => '8e9024dc',
|
||||
'aphront-side-nav-view-css' => '8e9024dc',
|
||||
'aphront-table-view-css' => '8e9024dc',
|
||||
'aphront-tokenizer-control-css' => '8e9024dc',
|
||||
'aphront-typeahead-control-css' => '8e9024dc',
|
||||
'aphront-crumbs-view-css' => 'a06761eb',
|
||||
'aphront-dialog-view-css' => 'a06761eb',
|
||||
'aphront-form-view-css' => 'a06761eb',
|
||||
'aphront-list-filter-view-css' => 'a06761eb',
|
||||
'aphront-panel-view-css' => 'a06761eb',
|
||||
'aphront-side-nav-view-css' => 'a06761eb',
|
||||
'aphront-table-view-css' => 'a06761eb',
|
||||
'aphront-tokenizer-control-css' => 'a06761eb',
|
||||
'aphront-typeahead-control-css' => 'a06761eb',
|
||||
'differential-changeset-view-css' => '95b66c1a',
|
||||
'differential-core-view-css' => '95b66c1a',
|
||||
'differential-revision-add-comment-css' => '95b66c1a',
|
||||
|
|
@ -1311,13 +1311,13 @@ celerity_register_resource_map(array(
|
|||
'javelin-util' => '307df223',
|
||||
'javelin-vector' => '307df223',
|
||||
'javelin-workflow' => 'd0713563',
|
||||
'phabricator-core-buttons-css' => '8e9024dc',
|
||||
'phabricator-core-css' => '8e9024dc',
|
||||
'phabricator-directory-css' => '8e9024dc',
|
||||
'phabricator-core-buttons-css' => 'a06761eb',
|
||||
'phabricator-core-css' => 'a06761eb',
|
||||
'phabricator-directory-css' => 'a06761eb',
|
||||
'phabricator-keyboard-shortcut' => 'd0713563',
|
||||
'phabricator-keyboard-shortcut-manager' => 'd0713563',
|
||||
'phabricator-remarkup-css' => '8e9024dc',
|
||||
'phabricator-standard-page-view' => '8e9024dc',
|
||||
'syntax-highlighting-css' => '8e9024dc',
|
||||
'phabricator-remarkup-css' => 'a06761eb',
|
||||
'phabricator-standard-page-view' => 'a06761eb',
|
||||
'syntax-highlighting-css' => 'a06761eb',
|
||||
),
|
||||
));
|
||||
|
|
|
|||
|
|
@ -84,6 +84,14 @@ class ManiphestTransactionSaveController extends ManiphestController {
|
|||
$file_transaction = $transaction;
|
||||
}
|
||||
|
||||
// Compute new CCs added by @mentions. We're going to try to add them to
|
||||
// another CC transaction if we can. If there aren't any CC transactions,
|
||||
// we'll create a new CC transaction after we handle everything else.
|
||||
$mention_ccs = DifferentialMarkupEngineFactory::extractPHIDsFromMentions(
|
||||
array(
|
||||
$request->getStr('comments'),
|
||||
));
|
||||
|
||||
$transaction = new ManiphestTransaction();
|
||||
$transaction
|
||||
->setAuthorPHID($user->getPHID())
|
||||
|
|
@ -109,6 +117,10 @@ class ManiphestTransactionSaveController extends ManiphestController {
|
|||
case ManiphestTransactionType::TYPE_CCS:
|
||||
$ccs = $request->getArr('ccs');
|
||||
$ccs = array_merge($ccs, $task->getCCPHIDs());
|
||||
if ($mention_ccs) {
|
||||
$ccs = array_merge($ccs, $mention_ccs);
|
||||
$mention_ccs = array();
|
||||
}
|
||||
$ccs = array_filter($ccs);
|
||||
$ccs = array_unique($ccs);
|
||||
$transaction->setNewValue($ccs);
|
||||
|
|
@ -143,9 +155,11 @@ class ManiphestTransactionSaveController extends ManiphestController {
|
|||
$old = $task->getCCPHIDs();
|
||||
$new = array_merge(
|
||||
$old,
|
||||
array($task->getOwnerPHID()));
|
||||
array($task->getOwnerPHID()),
|
||||
$mention_ccs);
|
||||
$new = array_unique(array_filter($new));
|
||||
if ($old != $new) {
|
||||
$mention_ccs = null;
|
||||
$cc = new ManiphestTransaction();
|
||||
$cc->setAuthorPHID($user->getPHID());
|
||||
$cc->setTransactionType(ManiphestTransactionType::TYPE_CCS);
|
||||
|
|
@ -165,7 +179,7 @@ class ManiphestTransactionSaveController extends ManiphestController {
|
|||
$transactions[] = $assign;
|
||||
}
|
||||
break;
|
||||
case ManiphestTransactionType::TYPE_NONE:
|
||||
default:
|
||||
$ccs = $task->getCCPHIDs();
|
||||
$owner = $task->getOwnerPHID();
|
||||
|
||||
|
|
@ -173,17 +187,31 @@ class ManiphestTransactionSaveController extends ManiphestController {
|
|||
// Current user, who is commenting, is not the owner or in ccs.
|
||||
// Add him to ccs
|
||||
$ccs[] = $user->getPHID();
|
||||
if ($mention_ccs) {
|
||||
$ccs = array_merge($ccs, $mention_ccs);
|
||||
$mention_ccs = null;
|
||||
}
|
||||
$cc = new ManiphestTransaction();
|
||||
$cc->setAuthorPHID($user->getPHID());
|
||||
$cc->setTransactionType(ManiphestTransactionType::TYPE_CCS);
|
||||
$cc->setNewValue($ccs);
|
||||
$transactions[] = $cc;
|
||||
}
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
if ($mention_ccs) {
|
||||
// We have mention CCs and didn't pick up a CC transaction anywhere that
|
||||
// we could shove them into, create a new CC transaction.
|
||||
$cc = new ManiphestTransaction();
|
||||
$cc->setAuthorPHID($user->getPHID());
|
||||
$cc->setTransactionType(ManiphestTransactionType::TYPE_CCS);
|
||||
|
||||
$ccs = array_merge($task->getCCPHIDs(), $mention_ccs);
|
||||
$cc->setNewValue($ccs);
|
||||
|
||||
$transactions[] = $cc;
|
||||
}
|
||||
|
||||
$editor = new ManiphestTransactionEditor();
|
||||
$editor->applyTransactions($task, $transactions);
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@
|
|||
|
||||
phutil_require_module('phabricator', 'aphront/response/404');
|
||||
phutil_require_module('phabricator', 'aphront/response/redirect');
|
||||
phutil_require_module('phabricator', 'applications/differential/parser/markup');
|
||||
phutil_require_module('phabricator', 'applications/draft/storage/draft');
|
||||
phutil_require_module('phabricator', 'applications/files/storage/file');
|
||||
phutil_require_module('phabricator', 'applications/maniphest/constants/status');
|
||||
|
|
|
|||
|
|
@ -40,6 +40,8 @@ class ManiphestTransactionEditor {
|
|||
$new = $transaction->getNewValue();
|
||||
$email_to[] = $transaction->getAuthorPHID();
|
||||
|
||||
$value_is_phid_set = false;
|
||||
|
||||
switch ($type) {
|
||||
case ManiphestTransactionType::TYPE_NONE:
|
||||
$old = null;
|
||||
|
|
@ -52,6 +54,7 @@ class ManiphestTransactionEditor {
|
|||
break;
|
||||
case ManiphestTransactionType::TYPE_CCS:
|
||||
$old = $task->getCCPHIDs();
|
||||
$value_is_phid_set = true;
|
||||
break;
|
||||
case ManiphestTransactionType::TYPE_PRIORITY:
|
||||
$old = $task->getPriority();
|
||||
|
|
@ -67,12 +70,40 @@ class ManiphestTransactionEditor {
|
|||
break;
|
||||
case ManiphestTransactionType::TYPE_PROJECTS:
|
||||
$old = $task->getProjectPHIDs();
|
||||
$value_is_phid_set = true;
|
||||
break;
|
||||
default:
|
||||
throw new Exception('Unknown action type.');
|
||||
}
|
||||
|
||||
if (($old !== null) && ($old == $new)) {
|
||||
$old_cmp = $old;
|
||||
$new_cmp = $new;
|
||||
if ($value_is_phid_set) {
|
||||
|
||||
// Normalize the old and new values if they are PHID sets so we don't
|
||||
// get any no-op transactions where the values differ only by keys,
|
||||
// order, duplicates, etc.
|
||||
|
||||
if (is_array($old)) {
|
||||
$old = array_filter($old);
|
||||
$old = array_unique($old);
|
||||
sort($old);
|
||||
$old = array_values($old);
|
||||
$old_cmp = $old;
|
||||
}
|
||||
|
||||
if (is_array($new)) {
|
||||
$new = array_filter($new);
|
||||
$new = array_unique($new);
|
||||
$transaction->setNewValue($new);
|
||||
|
||||
$new_cmp = $new;
|
||||
sort($new_cmp);
|
||||
$new_cmp = array_values($new_cmp);
|
||||
}
|
||||
}
|
||||
|
||||
if (($old !== null) && ($old_cmp == $new_cmp)) {
|
||||
if (count($transactions) > 1 && !$transaction->hasComments()) {
|
||||
// If we have at least one other transaction and this one isn't
|
||||
// doing anything and doesn't have any comments, just throw it
|
||||
|
|
|
|||
|
|
@ -62,12 +62,10 @@
|
|||
|
||||
.phabricator-remarkup-mention-exists {
|
||||
font-weight: bold;
|
||||
padding: 0 .25em;
|
||||
background: #aaffaa;
|
||||
background: #e6f3ff;
|
||||
}
|
||||
|
||||
.aphront-panel-preview .phabricator-remarkup-mention-unknown {
|
||||
font-weight: bold;
|
||||
padding: 0 .25em;
|
||||
background: #ffaaaa;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue