Skip to content

Commit 489f9e7

Browse files
AnhNhanepriestley
authored and
epriestley
committedMar 21, 2013
Added subscriptions to Phriction documents
Summary: Fixes T2694 added edge infrastructure for Phriction added mail subject prefix option for Phriction added messy mail support for subscribers adds edges to the phriction db, along with the subscriber interface which gives us subscriptions for free. simple display of subscribers, adequate to the current design and sufficient fallbacks for exceptional cases. @chad may be mailed about that one more UI element may be added to his redesign mail support is messy. not generic at all. only sends to subscribed non-authors. Test Plan: tried out all kinds of stuff. applied patch, subscribed, unsubscribed with multiple accs. verified proper edited documents, verified that mail was sent in MetaMTA. Verified contents, tos and stuff by looking into the db, comparing PHIDs etc. functional testing per serious MTA (that is, AWS SES) worked wonderfully. Here's how the subscription list looks like: {F36320, layout=link} Reviewers: epriestley, chad, btrahan Reviewed By: epriestley CC: hfcorriez, aran, Korvin Maniphest Tasks: T2686, T2694 Differential Revision: https://secure.phabricator.com/D5372 Conflicts: src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
1 parent 7260641 commit 489f9e7

File tree

8 files changed

+121
-4
lines changed

8 files changed

+121
-4
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
CREATE TABLE {$NAMESPACE}_phriction.edge (
2+
src VARCHAR(64) NOT NULL COLLATE utf8_bin,
3+
type INT UNSIGNED NOT NULL COLLATE utf8_bin,
4+
dst VARCHAR(64) NOT NULL COLLATE utf8_bin,
5+
dateCreated INT UNSIGNED NOT NULL,
6+
seq INT UNSIGNED NOT NULL,
7+
dataID INT UNSIGNED,
8+
PRIMARY KEY (src, type, dst),
9+
KEY (src, type, dateCreated, seq)
10+
) ENGINE=InnoDB, COLLATE utf8_general_ci;
11+
12+
CREATE TABLE {$NAMESPACE}_phriction.edgedata (
13+
id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT,
14+
data LONGTEXT NOT NULL COLLATE utf8_bin
15+
) ENGINE=InnoDB, COLLATE utf8_general_ci;
16+

‎src/__phutil_library_map__.php

+1
Original file line numberDiff line numberDiff line change
@@ -3166,6 +3166,7 @@
31663166
array(
31673167
0 => 'PhrictionDAO',
31683168
1 => 'PhabricatorPolicyInterface',
3169+
2 => 'PhabricatorSubscribableInterface',
31693170
),
31703171
'PhrictionDocumentController' => 'PhrictionController',
31713172
'PhrictionDocumentEditor' => 'PhabricatorEditor',

‎src/applications/phriction/config/PhabricatorPhrictionConfigOptions.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ public function getOptions() {
1919
pht("Enable Phriction"),
2020
pht("Disable Phriction"),
2121
))
22-
->setDescription(pht("Enable or disable Phriction."))
22+
->setDescription(pht("Enable or disable Phriction.")),
23+
$this->newOption(
24+
'metamta.phriction.subject-prefix', 'string', '[Phriction]')
25+
->setDescription(pht("Subject prefix for Phriction email.")),
2326
);
2427
}
2528

‎src/applications/phriction/controller/PhrictionDocumentController.php

+27-2
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,19 @@ public function processRequest() {
9797
}
9898
}
9999

100+
$subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID(
101+
$document->getPHID());
102+
100103
$phids = array_filter(
101104
array(
102105
$content->getAuthorPHID(),
103106
$project_phid,
104107
));
108+
109+
if ($subscribers) {
110+
$phids = array_merge($phids, $subscribers);
111+
}
112+
105113
$handles = $this->loadViewerHandles($phids);
106114

107115
$age = time() - $content->getDateCreated();
@@ -131,12 +139,29 @@ public function processRequest() {
131139
),
132140
pht('Document Index'));
133141

142+
$subscriber_view = null;
143+
if ($subscribers) {
144+
$subcriber_list = array();
145+
foreach ($subscribers as $subscriber) {
146+
$subcriber_list[] = $handles[$subscriber];
147+
}
148+
149+
$subcriber_list = phutil_implode_html(', ',
150+
mpull($subcriber_list, 'renderLink'));
151+
152+
$subscriber_view = array(
153+
hsprintf('<br />Subscribers: '),
154+
$subcriber_list,
155+
);
156+
}
157+
134158
$byline = hsprintf(
135-
'<div class="phriction-byline">%s%s</div>',
159+
'<div class="phriction-byline">%s%s%s</div>',
136160
pht('Last updated %s by %s.',
137161
$when,
138162
$handles[$content->getAuthorPHID()]->renderLink()),
139-
$project_info);
163+
$project_info,
164+
$subscriber_view);
140165

141166

142167
$doc_status = $document->getStatus();

‎src/applications/phriction/editor/PhrictionDocumentEditor.php

+63
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,70 @@ private function updateDocument($document, $content, $new_content) {
278278
->publish();
279279
}
280280

281+
// TODO: Migrate to ApplicationTransactions fast, so we get rid of this code
282+
$subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID(
283+
$document->getPHID());
284+
$this->sendMailToSubscribers($subscribers, $content);
285+
281286
return $this;
282287
}
283288

289+
private function sendMailToSubscribers(array $subscribers, $old_content) {
290+
if (!$subscribers) {
291+
return;
292+
}
293+
294+
$author_phid = $this->getActor()->getPHID();
295+
$document = $this->document;
296+
$content = $document->getContent();
297+
298+
$old_title = $old_content->getTitle();
299+
$title = $content->getTitle();
300+
301+
// TODO: Currently, this produces something like
302+
// Phrictioh Document Xyz was Edit
303+
// I'm too lazy to build my own action string everywhere
304+
// Plus, it does not have pht() anyway
305+
$action = PhrictionChangeType::getChangeTypeLabel(
306+
$content->getChangeType());
307+
$name = pht("Phriction Document %s was %s", $title, $action);
308+
309+
$body = array($name);
310+
// Content may have changed, you never know
311+
if ($content->getChangeType() == PhrictionChangeType::CHANGE_EDIT) {
312+
313+
if ($old_title != $title) {
314+
$body[] = pht('Title was changed from "%s" to "%s"',
315+
$old_title, $title);
316+
}
317+
318+
// The Remarkup text renderer comes in handy
319+
// TODO: Consider sending a diff instead?
320+
$body[] = pht("Content was changed to \n%s", $content->getContent());
321+
}
322+
323+
$body = implode("\n\n", $body);
324+
325+
$subject_prefix = $this->getMailSubjectPrefix();
326+
327+
$mail = new PhabricatorMetaMTAMail();
328+
$mail->setSubject($name)
329+
->setSubjectPrefix($subject_prefix)
330+
->setVarySubjectPrefix('['.$action.']')
331+
->addHeader('Thread-Topic', $name)
332+
->setFrom($author_phid)
333+
->addTos($subscribers)
334+
->setBody($body)
335+
->setRelatedPHID($document->getPHID())
336+
->setIsBulk(true);
337+
338+
$mail->saveAndSend();
339+
}
340+
341+
/* --( For less copy-pasting when switching to ApplicationTransactions )--- */
342+
343+
protected function getMailSubjectPrefix() {
344+
return PhabricatorEnv::getEnvConfig('metamta.phriction.subject-prefix');
345+
}
346+
284347
}

‎src/applications/phriction/storage/PhrictionDocument.php

+5-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* @group phriction
55
*/
66
final class PhrictionDocument extends PhrictionDAO
7-
implements PhabricatorPolicyInterface {
7+
implements PhabricatorPolicyInterface, PhabricatorSubscribableInterface {
88

99
protected $id;
1010
protected $phid;
@@ -125,4 +125,8 @@ public function hasAutomaticCapability($capability, PhabricatorUser $user) {
125125
}
126126
return false;
127127
}
128+
129+
public function isAutomaticallySubscribed($phid) {
130+
return false;
131+
}
128132
}

‎src/infrastructure/edges/constants/PhabricatorEdgeConfig.php

+1
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ public static function establishConnection($phid_type, $conn_type) {
116116
PhabricatorPHIDConstants::PHID_TYPE_MOCK => 'PholioMock',
117117
PhabricatorPHIDConstants::PHID_TYPE_MCRO => 'PhabricatorFileImageMacro',
118118
PhabricatorPHIDConstants::PHID_TYPE_CONP => 'ConpherenceThread',
119+
PhabricatorPHIDConstants::PHID_TYPE_WIKI => 'PhrictionDocument',
119120

120121
);
121122

‎src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

+4
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,10 @@ public function getPatches() {
11861186
'type' => 'sql',
11871187
'name' => $this->getPatchPath('20130320.phlux.sql'),
11881188
),
1189+
'20130317.phrictionedge.sql' => array(
1190+
'type' => 'sql',
1191+
'name' => $this->getPatchPath('20130317.phrictionedge.sql'),
1192+
),
11891193
);
11901194
}
11911195

0 commit comments

Comments
 (0)
Failed to load comments.