@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
Simplify notification code
Summary:
Currently we have two different feed story classes, one for notifications and one for feed stories. However, we never actually do anything different with them -- the notification is always the same as the feed story, just shown differently. Delete the notification special case to reduce the amount of code we have supporting feed and notifications.
This is a precursor to @chad's notification designs.
Test Plan: Viewed notifications and feed, saw exactly the same result before and after the patch (but less, simpler code).
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D4114
+46
-151
-6
src/__phutil_library_map__.php
-6
src/__phutil_library_map__.php
···
745
745
'PhabricatorFeedStoryReference' => 'applications/feed/storage/PhabricatorFeedStoryReference.php',
746
746
'PhabricatorFeedStoryStatus' => 'applications/feed/story/PhabricatorFeedStoryStatus.php',
747
747
'PhabricatorFeedStoryTypeConstants' => 'applications/feed/constants/PhabricatorFeedStoryTypeConstants.php',
748
-
'PhabricatorFeedStoryUnknown' => 'applications/feed/story/PhabricatorFeedStoryUnknown.php',
749
748
'PhabricatorFeedStoryView' => 'applications/feed/view/PhabricatorFeedStoryView.php',
750
749
'PhabricatorFeedView' => 'applications/feed/view/PhabricatorFeedView.php',
751
750
'PhabricatorFile' => 'applications/files/storage/PhabricatorFile.php',
···
872
871
'PhabricatorNotificationPanelController' => 'applications/notification/controller/PhabricatorNotificationPanelController.php',
873
872
'PhabricatorNotificationQuery' => 'applications/notification/PhabricatorNotificationQuery.php',
874
873
'PhabricatorNotificationStatusController' => 'applications/notification/controller/PhabricatorNotificationStatusController.php',
875
-
'PhabricatorNotificationStoryView' => 'applications/notification/view/PhabricatorNotificationStoryView.php',
876
-
'PhabricatorNotificationView' => 'applications/notification/view/PhabricatorNotificationView.php',
877
874
'PhabricatorOAuthClientAuthorization' => 'applications/oauthserver/storage/PhabricatorOAuthClientAuthorization.php',
878
875
'PhabricatorOAuthClientAuthorizationBaseController' => 'applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationBaseController.php',
879
876
'PhabricatorOAuthClientAuthorizationDeleteController' => 'applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationDeleteController.php',
···
1986
1983
'PhabricatorFeedStoryReference' => 'PhabricatorFeedDAO',
1987
1984
'PhabricatorFeedStoryStatus' => 'PhabricatorFeedStory',
1988
1985
'PhabricatorFeedStoryTypeConstants' => 'PhabricatorFeedConstants',
1989
-
'PhabricatorFeedStoryUnknown' => 'PhabricatorFeedStory',
1990
1986
'PhabricatorFeedStoryView' => 'PhabricatorFeedView',
1991
1987
'PhabricatorFeedView' => 'AphrontView',
1992
1988
'PhabricatorFile' =>
···
2096
2092
'PhabricatorNotificationPanelController' => 'PhabricatorNotificationController',
2097
2093
'PhabricatorNotificationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
2098
2094
'PhabricatorNotificationStatusController' => 'PhabricatorNotificationController',
2099
-
'PhabricatorNotificationStoryView' => 'PhabricatorNotificationView',
2100
-
'PhabricatorNotificationView' => 'AphrontView',
2101
2095
'PhabricatorOAuthClientAuthorization' => 'PhabricatorOAuthServerDAO',
2102
2096
'PhabricatorOAuthClientAuthorizationBaseController' => 'PhabricatorOAuthServerController',
2103
2097
'PhabricatorOAuthClientAuthorizationDeleteController' => 'PhabricatorOAuthClientAuthorizationBaseController',
-6
src/applications/feed/story/PhabricatorFeedStory.php
-6
src/applications/feed/story/PhabricatorFeedStory.php
···
149
149
150
150
abstract public function renderView();
151
151
152
-
// TODO: Make abstract once all subclasses implement it.
153
-
public function renderNotificationView() {
154
-
return id(new PhabricatorFeedStoryUnknown($this->data))
155
-
->renderNotificationView();
156
-
}
157
-
158
152
public function getRequiredHandlePHIDs() {
159
153
return array();
160
154
}
+1
-12
src/applications/feed/story/PhabricatorFeedStoryDifferential.php
+1
-12
src/applications/feed/story/PhabricatorFeedStoryDifferential.php
···
10
10
$data = $this->getStoryData();
11
11
12
12
$view = new PhabricatorFeedStoryView();
13
+
$view->setViewed($this->getHasViewed());
13
14
14
15
$line = $this->getLineForData($data);
15
16
$view->setTitle($line);
···
33
34
} else {
34
35
$view->setOneLineStory(true);
35
36
}
36
-
37
-
return $view;
38
-
}
39
-
40
-
public function renderNotificationView() {
41
-
$data = $this->getStoryData();
42
-
43
-
$view = new PhabricatorNotificationStoryView();
44
-
45
-
$view->setTitle($this->getLineForData($data));
46
-
$view->setEpoch($data->getEpoch());
47
-
$view->setViewed($this->getHasViewed());
48
37
49
38
return $view;
50
39
}
+1
-6
src/applications/feed/story/PhabricatorFeedStoryDifferentialAggregate.php
+1
-6
src/applications/feed/story/PhabricatorFeedStoryDifferentialAggregate.php
···
4
4
extends PhabricatorFeedStoryAggregate {
5
5
6
6
public function renderView() {
7
-
return null;
8
-
}
9
-
10
-
11
-
public function renderNotificationView() {
12
7
$data = $this->getStoryData();
13
8
14
9
$task_link = $this->linkTo($data->getValue('revision_phid'));
···
59
54
break;
60
55
}
61
56
62
-
$view = new PhabricatorNotificationStoryView();
57
+
$view = new PhabricatorFeedStoryView();
63
58
$view->setEpoch($this->getEpoch());
64
59
$view->setViewed($this->getHasViewed());
65
60
$view->setTitle($title);
+1
-12
src/applications/feed/story/PhabricatorFeedStoryManiphest.php
+1
-12
src/applications/feed/story/PhabricatorFeedStoryManiphest.php
···
17
17
$data = $this->getStoryData();
18
18
19
19
$view = new PhabricatorFeedStoryView();
20
+
$view->setViewed($this->getHasViewed());
20
21
21
22
$line = $this->getLineForData($data);
22
23
$view->setTitle($line);
···
39
40
} else {
40
41
$view->setOneLineStory(true);
41
42
}
42
-
43
-
return $view;
44
-
}
45
-
46
-
public function renderNotificationView() {
47
-
$data = $this->getStoryData();
48
-
49
-
$view = new PhabricatorNotificationStoryView();
50
-
51
-
$view->setTitle($this->getLineForData($data));
52
-
$view->setEpoch($data->getEpoch());
53
-
$view->setViewed($this->getHasViewed());
54
43
55
44
return $view;
56
45
}
+1
-6
src/applications/feed/story/PhabricatorFeedStoryManiphestAggregate.php
+1
-6
src/applications/feed/story/PhabricatorFeedStoryManiphestAggregate.php
···
4
4
extends PhabricatorFeedStoryAggregate {
5
5
6
6
public function renderView() {
7
-
return null;
8
-
}
9
-
10
-
11
-
public function renderNotificationView() {
12
7
$data = $this->getStoryData();
13
8
14
9
$task_link = $this->linkTo($data->getValue('taskPHID'));
···
59
54
break;
60
55
}
61
56
62
-
$view = new PhabricatorNotificationStoryView();
57
+
$view = new PhabricatorFeedStoryView();
63
58
$view->setEpoch($this->getEpoch());
64
59
$view->setViewed($this->getHasViewed());
65
60
$view->setTitle($title);
-38
src/applications/feed/story/PhabricatorFeedStoryUnknown.php
-38
src/applications/feed/story/PhabricatorFeedStoryUnknown.php
···
1
-
<?php
2
-
3
-
final class PhabricatorFeedStoryUnknown extends PhabricatorFeedStory {
4
-
5
-
public function renderView() {
6
-
$data = $this->getStoryData();
7
-
8
-
$view = new PhabricatorFeedStoryView();
9
-
10
-
$view->setTitle('Unknown Story');
11
-
$view->setEpoch($data->getEpoch());
12
-
13
-
$view->appendChild(
14
-
'This is an unrenderable feed story of type '.
15
-
'"'.phutil_escape_html($data->getStoryType()).'".');
16
-
17
-
18
-
return $view;
19
-
}
20
-
21
-
public function renderNotificationView() {
22
-
$data = $this->getStoryData();
23
-
24
-
$view = new PhabricatorNotificationStoryView();
25
-
26
-
$view->setTitle('A wild notifcation appeared!');
27
-
$view->setEpoch($data->getEpoch());
28
-
29
-
$view->appendChild(
30
-
'This is an unrenderable feed story of type '.
31
-
'"'.phutil_escape_html($data->getStoryType()).'".');
32
-
33
-
34
-
return $view;
35
-
36
-
}
37
-
38
-
}
+27
src/applications/feed/view/PhabricatorFeedStoryView.php
+27
src/applications/feed/view/PhabricatorFeedStoryView.php
···
7
7
private $phid;
8
8
private $epoch;
9
9
private $viewer;
10
+
private $viewed;
10
11
11
12
private $oneLine;
12
13
···
33
34
public function setOneLineStory($one_line) {
34
35
$this->oneLine = $one_line;
35
36
return $this;
37
+
}
38
+
39
+
public function setViewed($viewed) {
40
+
$this->viewed = $viewed;
41
+
return $this;
42
+
}
43
+
44
+
public function getViewed() {
45
+
return $this->viewed;
46
+
}
47
+
48
+
public function renderNotification() {
49
+
$classes = array(
50
+
'phabricator-notification',
51
+
);
52
+
53
+
if (!$this->viewed) {
54
+
$classes[] = 'phabricator-notification-unread';
55
+
}
56
+
57
+
return phutil_render_tag(
58
+
'div',
59
+
array(
60
+
'class' => implode(' ', $classes),
61
+
),
62
+
$this->title);
36
63
}
37
64
38
65
public function render() {
+3
-2
src/applications/notification/builder/PhabricatorNotificationBuilder.php
+3
-2
src/applications/notification/builder/PhabricatorNotificationBuilder.php
···
123
123
$null_view = new AphrontNullView();
124
124
125
125
foreach ($stories as $story) {
126
-
$view = $story->renderNotificationView();
127
-
$null_view->appendChild($view);
126
+
$view = $story->renderView();
127
+
128
+
$null_view->appendChild($view->renderNotification());
128
129
}
129
130
130
131
return $null_view;
+12
-15
src/applications/notification/storage/PhabricatorFeedStoryNotification.php
+12
-15
src/applications/notification/storage/PhabricatorFeedStoryNotification.php
···
14
14
) + parent::getConfiguration();
15
15
}
16
16
17
-
static public function updateObjectNotificationViews(PhabricatorUser $user,
17
+
static public function updateObjectNotificationViews(
18
+
PhabricatorUser $user,
18
19
$object_phid) {
19
20
20
21
if (PhabricatorEnv::getEnvConfig('notification.enabled')) {
···
38
39
}
39
40
}
40
41
41
-
/* should only be called when notifications are enabled */
42
-
public function countUnread(
43
-
PhabricatorUser $user) {
44
-
45
-
$conn = $this->establishConnection('r');
42
+
public function countUnread(PhabricatorUser $user) {
43
+
$conn = $this->establishConnection('r');
46
44
47
-
$data = queryfx_one(
48
-
$conn,
49
-
"SELECT COUNT(*) as count
50
-
FROM %T
51
-
WHERE userPHID = %s
52
-
AND hasViewed=0",
53
-
$this->getTableName(),
54
-
$user->getPHID());
45
+
$data = queryfx_one(
46
+
$conn,
47
+
'SELECT COUNT(*) as count
48
+
FROM %T
49
+
WHERE userPHID = %s AND hasViewed = 0',
50
+
$this->getTableName(),
51
+
$user->getPHID());
55
52
56
-
return $data['count'];
53
+
return $data['count'];
57
54
}
58
55
59
56
}
-43
src/applications/notification/view/PhabricatorNotificationStoryView.php
-43
src/applications/notification/view/PhabricatorNotificationStoryView.php
···
1
-
<?php
2
-
3
-
final class PhabricatorNotificationStoryView
4
-
extends PhabricatorNotificationView {
5
-
6
-
private $title;
7
-
private $phid;
8
-
private $epoch;
9
-
private $viewed;
10
-
11
-
public function setTitle($title) {
12
-
$this->title = $title;
13
-
return $this;
14
-
}
15
-
16
-
public function setEpoch($epoch) {
17
-
$this->epoch = $epoch;
18
-
return $this;
19
-
}
20
-
21
-
public function setViewed($viewed) {
22
-
$this->viewed = $viewed;
23
-
}
24
-
25
-
public function render() {
26
-
27
-
$classes = array(
28
-
'phabricator-notification',
29
-
);
30
-
31
-
if (!$this->viewed) {
32
-
$classes[] = 'phabricator-notification-unread';
33
-
}
34
-
35
-
return phutil_render_tag(
36
-
'div',
37
-
array(
38
-
'class' => implode(' ', $classes),
39
-
),
40
-
$this->title);
41
-
}
42
-
43
-
}