@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Update the "Notification Test" workflow to use more modern mechanisms
Summary:
Depends on D19860. Ref T13222. Ref T10743. See PHI996.
Long ago, there were different types of feed stories. Over time, there was less and less need for this, and nowadays basically everything is a "transaction" feed story. Each story renders differently, but they're fundamentally all about transactions.
The Notification test controller still uses a custom type of feed story to send notifications. Move away from this, and apply a transaction against the user instead. This has the same ultimate effect, but involves less weird custom code from ages long forgotten.
This doesn't fix the actual problem with these things showing up in feed. Currently, stories always use the same rendering for feed and notifications, and there need to be some additional changes to fix this. So no behavioral change yet, just slightly more reasonable code.
Test Plan: Clicked the button and got some test notifications, with Aphlict running.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13222, T10743
Differential Revision: https://secure.phabricator.com/D19861
+111
-39
+2
src/__phutil_library_map__.php
+2
src/__phutil_library_map__.php
···
4623
4623
'PhabricatorUserLogView' => 'applications/people/view/PhabricatorUserLogView.php',
4624
4624
'PhabricatorUserMessageCountCacheType' => 'applications/people/cache/PhabricatorUserMessageCountCacheType.php',
4625
4625
'PhabricatorUserNotificationCountCacheType' => 'applications/people/cache/PhabricatorUserNotificationCountCacheType.php',
4626
+
'PhabricatorUserNotifyTransaction' => 'applications/people/xaction/PhabricatorUserNotifyTransaction.php',
4626
4627
'PhabricatorUserPHIDResolver' => 'applications/phid/resolver/PhabricatorUserPHIDResolver.php',
4627
4628
'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php',
4628
4629
'PhabricatorUserPreferencesCacheType' => 'applications/people/cache/PhabricatorUserPreferencesCacheType.php',
···
10676
10677
'PhabricatorUserLogView' => 'AphrontView',
10677
10678
'PhabricatorUserMessageCountCacheType' => 'PhabricatorUserCacheType',
10678
10679
'PhabricatorUserNotificationCountCacheType' => 'PhabricatorUserCacheType',
10680
+
'PhabricatorUserNotifyTransaction' => 'PhabricatorUserTransactionType',
10679
10681
'PhabricatorUserPHIDResolver' => 'PhabricatorPHIDResolver',
10680
10682
'PhabricatorUserPreferences' => array(
10681
10683
'PhabricatorUserDAO',
+20
-23
src/applications/notification/controller/PhabricatorNotificationTestController.php
+20
-23
src/applications/notification/controller/PhabricatorNotificationTestController.php
···
6
6
public function handleRequest(AphrontRequest $request) {
7
7
$viewer = $request->getViewer();
8
8
9
-
$story_type = 'PhabricatorNotificationTestFeedStory';
10
-
$story_data = array(
11
-
'title' => pht(
9
+
if ($request->validateCSRF()) {
10
+
$message_text = pht(
12
11
'This is a test notification, sent at %s.',
13
-
phabricator_datetime(time(), $viewer)),
14
-
);
12
+
phabricator_datetime(time(), $viewer));
15
13
16
-
$viewer_phid = $viewer->getPHID();
14
+
// NOTE: Currently, the FeedStoryPublisher explicitly filters out
15
+
// notifications about your own actions. Send this notification from
16
+
// a different actor to get around this.
17
+
$application_phid = id(new PhabricatorNotificationsApplication())
18
+
->getPHID();
17
19
18
-
// NOTE: Because we don't currently show you your own notifications, make
19
-
// sure this comes from a different PHID.
20
-
$application_phid = id(new PhabricatorNotificationsApplication())
21
-
->getPHID();
20
+
$xactions = array();
21
+
22
+
$xactions[] = id(new PhabricatorUserTransaction())
23
+
->setTransactionType(
24
+
PhabricatorUserNotifyTransaction::TRANSACTIONTYPE)
25
+
->setNewValue($message_text)
26
+
->setForceNotifyPHIDs(array($viewer->getPHID()));
22
27
23
-
// TODO: When it's easier to get these buttons to render as forms, this
24
-
// would be slightly nicer as a more standard isFormPost() check.
28
+
$editor = id(new PhabricatorUserTransactionEditor())
29
+
->setActor($viewer)
30
+
->setActingAsPHID($application_phid)
31
+
->setContentSourceFromRequest($request);
25
32
26
-
if ($request->validateCSRF()) {
27
-
id(new PhabricatorFeedStoryPublisher())
28
-
->setStoryType($story_type)
29
-
->setStoryData($story_data)
30
-
->setStoryTime(time())
31
-
->setStoryAuthorPHID($application_phid)
32
-
->setRelatedPHIDs(array($viewer_phid))
33
-
->setPrimaryObjectPHID($viewer_phid)
34
-
->setSubscribedPHIDs(array($viewer_phid))
35
-
->setNotifyAuthor(true)
36
-
->publish();
33
+
$editor->applyTransactions($viewer, $xactions);
37
34
}
38
35
39
36
return id(new AphrontAjaxResponse());
+6
src/applications/people/controller/PhabricatorPeopleProfileManageController.php
+6
src/applications/people/controller/PhabricatorPeopleProfileManageController.php
···
43
43
->setCurtain($curtain)
44
44
->addPropertySection(pht('Details'), $properties);
45
45
46
+
$timeline = $this->buildTransactionTimeline(
47
+
$user,
48
+
new PhabricatorPeopleTransactionQuery());
49
+
$timeline->setShouldTerminate(true);
50
+
46
51
return $this->newPage()
47
52
->setTitle(
48
53
array(
···
54
59
->appendChild(
55
60
array(
56
61
$manage,
62
+
$timeline,
57
63
));
58
64
}
59
65
+14
src/applications/people/editor/PhabricatorUserTransactionEditor.php
+14
src/applications/people/editor/PhabricatorUserTransactionEditor.php
···
11
11
return pht('Users');
12
12
}
13
13
14
+
protected function shouldPublishFeedStory(
15
+
PhabricatorLiskDAO $object,
16
+
array $xactions) {
17
+
return true;
18
+
}
19
+
20
+
protected function getMailTo(PhabricatorLiskDAO $object) {
21
+
return array();
22
+
}
23
+
24
+
protected function getMailCC(PhabricatorLiskDAO $object) {
25
+
return array();
26
+
}
27
+
14
28
}
+4
-13
src/applications/people/xaction/PhabricatorUserDisableTransaction.php
+4
-13
src/applications/people/xaction/PhabricatorUserDisableTransaction.php
···
35
35
}
36
36
}
37
37
38
-
public function getTitleForFeed() {
39
-
$new = $this->getNewValue();
40
-
if ($new) {
41
-
return pht(
42
-
'%s disabled %s.',
43
-
$this->renderAuthor(),
44
-
$this->renderObject());
45
-
} else {
46
-
return pht(
47
-
'%s enabled %s.',
48
-
$this->renderAuthor(),
49
-
$this->renderObject());
50
-
}
38
+
public function shouldHideForFeed() {
39
+
// Don't publish feed stories about disabling users, since this can be
40
+
// a sensitive action.
41
+
return true;
51
42
}
52
43
53
44
public function validateTransactions($object, array $xactions) {
+34
src/applications/people/xaction/PhabricatorUserNotifyTransaction.php
+34
src/applications/people/xaction/PhabricatorUserNotifyTransaction.php
···
1
+
<?php
2
+
3
+
final class PhabricatorUserNotifyTransaction
4
+
extends PhabricatorUserTransactionType {
5
+
6
+
const TRANSACTIONTYPE = 'notify';
7
+
8
+
public function generateOldValue($object) {
9
+
return null;
10
+
}
11
+
12
+
public function generateNewValue($object, $value) {
13
+
return $value;
14
+
}
15
+
16
+
public function getTitle() {
17
+
return pht(
18
+
'%s sent this user a test notification.',
19
+
$this->renderAuthor());
20
+
}
21
+
22
+
public function getTitleForFeed() {
23
+
return $this->getNewValue();
24
+
}
25
+
26
+
public function shouldHideForFeed() {
27
+
return false;
28
+
}
29
+
30
+
public function shouldHideForMail() {
31
+
return true;
32
+
}
33
+
34
+
}
+22
-3
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+22
-3
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
···
3378
3378
PhabricatorLiskDAO $object,
3379
3379
array $xactions) {
3380
3380
3381
-
return array_unique(array_merge(
3382
-
$this->getMailTo($object),
3383
-
$this->getMailCC($object)));
3381
+
// If some transactions are forcing notification delivery, add the forced
3382
+
// recipients to the notify list.
3383
+
$force_list = array();
3384
+
foreach ($xactions as $xaction) {
3385
+
$force_phids = $xaction->getForceNotifyPHIDs();
3386
+
3387
+
if (!$force_phids) {
3388
+
continue;
3389
+
}
3390
+
3391
+
foreach ($force_phids as $force_phid) {
3392
+
$force_list[] = $force_phid;
3393
+
}
3394
+
}
3395
+
3396
+
$to_list = $this->getMailTo($object);
3397
+
$cc_list = $this->getMailCC($object);
3398
+
3399
+
$full_list = array_merge($force_list, $to_list, $cc_list);
3400
+
$full_list = array_fuse($full_list);
3401
+
3402
+
return array_keys($full_list);
3384
3403
}
3385
3404
3386
3405
+9
src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+9
src/applications/transactions/storage/PhabricatorApplicationTransaction.php
···
1662
1662
return null;
1663
1663
}
1664
1664
1665
+
public function setForceNotifyPHIDs(array $phids) {
1666
+
$this->setMetadataValue('notify.force', $phids);
1667
+
return $this;
1668
+
}
1669
+
1670
+
public function getForceNotifyPHIDs() {
1671
+
return $this->getMetadataValue('notify.force', array());
1672
+
}
1673
+
1665
1674
1666
1675
/* -( PhabricatorDestructibleInterface )----------------------------------- */
1667
1676