1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 14:52:41 +01:00

Track content sources (email, web, conduit, mobile) for replies

Summary:
When an object is updated, record the content source for the update. This mostly
isn't terribly useful but one concrete thing I want to do with it is let admins
audit via-email replies more easily since there are a bunch of options which let
you do hyjinx if you intentionally configure them insecurely. I think having a
little more auditability around this feature is generally good. At some point
I'm going to turn this into a link admins can click to see details.

It also allows us to see how frequently different mechanisms are used, and lets
you see if someone is at their desk or on a mobile or whatever, at least
indirectly.

The "tablet" and "mobile" sources are currently unused but I figured I'd throw
them in anyway. SMS support should definitely happen at some point.

Not 100% sure about the design for this, I might change it to plain text at some
point.

Test Plan: Updated objects and saw update sources rendered.

Reviewers: jungejason, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, epriestley, jungejason

Differential Revision: 844
This commit is contained in:
epriestley 2011-08-22 10:25:45 -07:00
parent 764d3d1c65
commit 69445222f7
39 changed files with 358 additions and 18 deletions

View file

@ -0,0 +1,5 @@
ALTER TABLE phabricator_differential.differential_comment
ADD contentSource VARCHAR(255);
ALTER TABLE phabricator_maniphest.maniphest_transaction
ADD contentSource VARCHAR(255);

View file

@ -643,6 +643,17 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/js/application/maniphest/behavior-transaction-preview.js',
),
0 =>
array(
'uri' => '/res/1da00bfe/rsrc/js/javelin/lib/__tests__/URI.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-uri',
1 => 'javelin-php-serializer',
),
'disk' => '/rsrc/js/javelin/lib/__tests__/URI.js',
),
'javelin-behavior-owners-path-editor' =>
array(
'uri' => '/res/9cf78ffc/rsrc/js/application/owners/owners-path-editor.js',
@ -725,17 +736,6 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/js/application/projects/projects-resource-editor.js',
),
0 =>
array(
'uri' => '/res/1da00bfe/rsrc/js/javelin/lib/__tests__/URI.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-uri',
1 => 'javelin-php-serializer',
),
'disk' => '/rsrc/js/javelin/lib/__tests__/URI.js',
),
'javelin-behavior-refresh-csrf' =>
array(
'uri' => '/res/88beba4c/rsrc/js/application/core/behavior-refresh-csrf.js',
@ -1169,6 +1169,15 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/js/application/herald/PathTypeahead.js',
),
'phabricator-content-source-view-css' =>
array(
'uri' => '/res/7147f14c/rsrc/css/application/contentsource/content-source-view.css',
'type' => 'css',
'requires' =>
array(
),
'disk' => '/rsrc/css/application/contentsource/content-source-view.css',
),
'phabricator-core-buttons-css' =>
array(
'uri' => '/res/3059cf79/rsrc/css/core/buttons.css',

View file

@ -344,6 +344,8 @@ phutil_register_library_map(array(
'PhabricatorConduitLogController' => 'applications/conduit/controller/log',
'PhabricatorConduitMethodCallLog' => 'applications/conduit/storage/methodcalllog',
'PhabricatorConduitTokenController' => 'applications/conduit/controller/token',
'PhabricatorContentSource' => 'applications/metamta/contentsource/source',
'PhabricatorContentSourceView' => 'applications/metamta/contentsource/view',
'PhabricatorController' => 'applications/base/controller/base',
'PhabricatorCountdownController' => 'applications/countdown/controller/base',
'PhabricatorCountdownDAO' => 'applications/countdown/storage/base',
@ -951,6 +953,7 @@ phutil_register_library_map(array(
'PhabricatorConduitLogController' => 'PhabricatorConduitController',
'PhabricatorConduitMethodCallLog' => 'PhabricatorConduitDAO',
'PhabricatorConduitTokenController' => 'PhabricatorConduitController',
'PhabricatorContentSourceView' => 'AphrontView',
'PhabricatorController' => 'AphrontController',
'PhabricatorCountdownController' => 'PhabricatorController',
'PhabricatorCountdownDAO' => 'PhabricatorLiskDAO',

View file

@ -252,4 +252,8 @@ class AphrontRequest {
return $this->isFormPost() && $this->getStr('__dialog__');
}
final public function getRemoteAddr() {
return $_SERVER['REMOTE_ADDR'];
}
}

View file

@ -66,9 +66,14 @@ class ConduitAPI_differential_updaterevision_Method extends ConduitAPIMethod {
throw new ConduitException('ERR_COMMITTED');
}
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_CONDUIT,
array());
$editor = new DifferentialRevisionEditor(
$revision,
$revision->getAuthorPHID());
$editor->setContentSource($content_source);
$fields = $request->getValue('fields');
$editor->copyFieldsFromConduit($fields);

View file

@ -12,6 +12,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/revisi
phutil_require_module('phabricator', 'applications/differential/editor/revision');
phutil_require_module('phabricator', 'applications/differential/storage/diff');
phutil_require_module('phabricator', 'applications/differential/storage/revision');
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_module('phabricator', 'infrastructure/env');
phutil_require_module('phutil', 'utils');

View file

@ -87,7 +87,12 @@ final class ConduitAPI_maniphest_createtask_Method
);
}
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_CONDUIT,
array());
$template = new ManiphestTransaction();
$template->setContentSource($content_source);
$template->setAuthorPHID($request->getUser()->getPHID());
$transactions = array();

View file

@ -13,6 +13,7 @@ phutil_require_module('phabricator', 'applications/maniphest/constants/transacti
phutil_require_module('phabricator', 'applications/maniphest/editor/transaction');
phutil_require_module('phabricator', 'applications/maniphest/storage/task');
phutil_require_module('phabricator', 'applications/maniphest/storage/transaction');
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_module('phabricator', 'applications/phid/constants');

View file

@ -40,8 +40,15 @@ class DifferentialCommentSaveController extends DifferentialController {
$request->getUser()->getPHID(),
$action);
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_WEB,
array(
'ip' => $request->getRemoteAddr(),
));
$editor
->setMessage($comment)
->setContentSource($content_source)
->setAttachInlineComments(true)
->setAddCC($action != DifferentialAction::ACTION_RESIGN)
->setAddedReviewers($reviewers)

View file

@ -13,6 +13,7 @@ phutil_require_module('phabricator', 'applications/differential/controller/base'
phutil_require_module('phabricator', 'applications/differential/editor/comment');
phutil_require_module('phabricator', 'applications/differential/storage/revision');
phutil_require_module('phabricator', 'applications/draft/storage/draft');
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_module('phutil', 'utils');

View file

@ -30,6 +30,7 @@ class DifferentialCommentEditor {
private $addedCCs = array();
private $parentMessageID;
private $contentSource;
public function __construct(
DifferentialRevision $revision,
@ -88,6 +89,11 @@ class DifferentialCommentEditor {
return $this->addedCCs;
}
public function setContentSource(PhabricatorContentSource $content_source) {
$this->contentSource = $content_source;
return $this;
}
public function save() {
$revision = $this->revision;
$action = $this->action;
@ -320,8 +326,13 @@ class DifferentialCommentEditor {
->setRevisionID($revision->getID())
->setAction($action)
->setContent((string)$this->message)
->setMetadata($metadata)
->save();
->setMetadata($metadata);
if ($this->contentSource) {
$comment->setContentSource($this->contentSource);
}
$comment->save();
$changesets = array();
if ($inline_comments) {

View file

@ -33,6 +33,7 @@ class DifferentialRevisionEditor {
protected $silentUpdate;
private $auxiliaryFields = array();
private $contentSource;
public function __construct(DifferentialRevision $revision, $actor_phid) {
$this->revision = $revision;
@ -108,6 +109,11 @@ class DifferentialRevisionEditor {
return $this;
}
public function setContentSource(PhabricatorContentSource $content_source) {
$this->contentSource = $content_source;
return $this;
}
public function addDiff(DifferentialDiff $diff, $comments) {
if ($diff->getRevisionID() &&
$diff->getRevisionID() != $this->getRevision()->getID()) {
@ -652,6 +658,11 @@ class DifferentialRevisionEditor {
->setRevisionID($revision_id)
->setContent($this->getComments())
->setAction('update');
if ($this->contentSource) {
$comment->setContentSource($this->contentSource);
}
$comment->save();
return $comment;

View file

@ -139,6 +139,12 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
// implementation jumps straight into handleAction() and will not have
// a PhabricatorMetaMTAReceivedMail object.
if ($this->receivedMail) {
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_EMAIL,
array(
'id' => $this->receivedMail->getID(),
));
$editor->setContentSource($content_source);
$editor->setParentMessageID($this->receivedMail->getMessageID());
}
$editor->setMessage($body);

View file

@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/action
phutil_require_module('phabricator', 'applications/differential/editor/comment');
phutil_require_module('phabricator', 'applications/differential/editor/revision');
phutil_require_module('phabricator', 'applications/differential/mail/exception');
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_module('phabricator', 'applications/metamta/replyhandler/base');
phutil_require_module('phabricator', 'infrastructure/env');

View file

@ -27,6 +27,7 @@ class DifferentialComment extends DifferentialDAO {
protected $content;
protected $cache;
protected $metadata = array();
protected $contentSource;
public function getConfiguration() {
return array(
@ -36,4 +37,13 @@ class DifferentialComment extends DifferentialDAO {
) + parent::getConfiguration();
}
public function setContentSource(PhabricatorContentSource $content_source) {
$this->contentSource = $content_source->serialize();
return $this;
}
public function getContentSource() {
return PhabricatorContentSource::newFromSerialized($this->contentSource);
}
}

View file

@ -7,6 +7,7 @@
phutil_require_module('phabricator', 'applications/differential/storage/base');
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_source('DifferentialComment.php');

View file

@ -94,7 +94,14 @@ final class DifferentialRevisionCommentView extends AphrontView {
$date = phabricator_datetime($comment->getDateCreated(), $this->user);
}
$info = array($date);
$info = array();
$content_source = new PhabricatorContentSourceView();
$content_source->setContentSource($comment->getContentSource());
$content_source->setUser($this->user);
$info[] = $content_source->render();
$info[] = $date;
$comment_anchor = null;
$num = $this->commentNumber;
@ -110,7 +117,7 @@ final class DifferentialRevisionCommentView extends AphrontView {
$comment_anchor = 'anchor-comment-'.$num;
}
$info = implode(' · ', $info);
$info = implode(' · ', array_filter($info));
$author = $this->handles[$comment->getAuthorPHID()];
$author_link = $author->renderLink();
@ -278,6 +285,7 @@ final class DifferentialRevisionCommentView extends AphrontView {
'</div>'.
$inline_render.
'</div>'.
$content_source->render().
'</div>');
}

View file

@ -9,6 +9,7 @@
phutil_require_module('phabricator', 'aphront/writeguard');
phutil_require_module('phabricator', 'applications/differential/constants/action');
phutil_require_module('phabricator', 'applications/differential/storage/comment');
phutil_require_module('phabricator', 'applications/metamta/contentsource/view');
phutil_require_module('phabricator', 'infrastructure/celerity/api');
phutil_require_module('phabricator', 'infrastructure/javelin/api');
phutil_require_module('phabricator', 'view/base');

View file

@ -182,8 +182,15 @@ class ManiphestTaskEditController extends ManiphestController {
);
}
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_WEB,
array(
'ip' => $request->getRemoteAddr(),
));
$template = new ManiphestTransaction();
$template->setAuthorPHID($user->getPHID());
$template->setContentSource($content_source);
$transactions = array();
foreach ($changes as $type => $value) {

View file

@ -17,6 +17,7 @@ phutil_require_module('phabricator', 'applications/maniphest/editor/transaction'
phutil_require_module('phabricator', 'applications/maniphest/extensions/base');
phutil_require_module('phabricator', 'applications/maniphest/storage/task');
phutil_require_module('phabricator', 'applications/maniphest/storage/transaction');
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_module('phabricator', 'applications/phid/constants');
phutil_require_module('phabricator', 'applications/phid/handle/data');
phutil_require_module('phabricator', 'infrastructure/celerity/api');

View file

@ -219,6 +219,16 @@ class ManiphestTransactionSaveController extends ManiphestController {
$transactions[] = $cc_transaction;
}
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_WEB,
array(
'ip' => $request->getRemoteAddr(),
));
foreach ($transactions as $transaction) {
$transaction->setContentSource($content_source);
}
$editor = new ManiphestTransactionEditor();
$editor->applyTransactions($task, $transactions);

View file

@ -17,6 +17,7 @@ phutil_require_module('phabricator', 'applications/maniphest/editor/transaction'
phutil_require_module('phabricator', 'applications/maniphest/storage/task');
phutil_require_module('phabricator', 'applications/maniphest/storage/transaction');
phutil_require_module('phabricator', 'applications/markup/engine');
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_module('phabricator', 'applications/phid/constants');
phutil_require_module('phutil', 'utils');

View file

@ -66,7 +66,14 @@ class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
$xactions = array();
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_EMAIL,
array(
'id' => $mail->getID(),
));
$template = new ManiphestTransaction();
$template->setContentSource($content_source);
$template->setAuthorPHID($user->getPHID());
if ($is_new_task) {

View file

@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/maniphest/constants/status');
phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
phutil_require_module('phabricator', 'applications/maniphest/editor/transaction');
phutil_require_module('phabricator', 'applications/maniphest/storage/transaction');
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_module('phabricator', 'applications/metamta/replyhandler/base');
phutil_require_module('phabricator', 'applications/phid/constants');
phutil_require_module('phabricator', 'infrastructure/env');

View file

@ -29,6 +29,7 @@ class ManiphestTransaction extends ManiphestDAO {
protected $comments;
protected $cache;
protected $metadata = array();
protected $contentSource;
public function getConfiguration() {
return array(
@ -104,5 +105,13 @@ class ManiphestTransaction extends ManiphestDAO {
return (bool)strlen(trim($this->getComments()));
}
public function setContentSource(PhabricatorContentSource $content_source) {
$this->contentSource = $content_source->serialize();
return $this;
}
public function getContentSource() {
return PhabricatorContentSource::newFromSerialized($this->contentSource);
}
}

View file

@ -8,6 +8,7 @@
phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
phutil_require_module('phabricator', 'applications/maniphest/storage/base');
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_source('ManiphestTransaction.php');

View file

@ -202,6 +202,13 @@ class ManiphestTransactionDetailView extends ManiphestView {
}
$info = array();
$source_transaction = nonempty($comment_transaction, $any_transaction);
$content_source = new PhabricatorContentSourceView();
$content_source->setContentSource($source_transaction->getContentSource());
$content_source->setUser($this->user);
$info[] = $content_source->render();
$info[] = $timestamp;
$comment_anchor = null;
@ -218,7 +225,8 @@ class ManiphestTransactionDetailView extends ManiphestView {
$comment_anchor = 'anchor-comment-'.$num;
}
$info = implode(' &middot; ', $info);
$info = implode(' &middot; ', array_filter($info));
return phutil_render_tag(
'div',

View file

@ -11,6 +11,7 @@ phutil_require_module('phabricator', 'applications/maniphest/constants/priority'
phutil_require_module('phabricator', 'applications/maniphest/constants/status');
phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
phutil_require_module('phabricator', 'applications/maniphest/view/base');
phutil_require_module('phabricator', 'applications/metamta/contentsource/view');
phutil_require_module('phabricator', 'applications/phid/constants');
phutil_require_module('phabricator', 'infrastructure/celerity/api');
phutil_require_module('phabricator', 'infrastructure/env');

View file

@ -0,0 +1,75 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
final class PhabricatorContentSource {
const SOURCE_UNKNOWN = 'unknown';
const SOURCE_WEB = 'web';
const SOURCE_EMAIL = 'email';
const SOURCE_CONDUIT = 'conduit';
const SOURCE_MOBILE = 'mobile';
const SOURCE_TABLET = 'tablet';
private $source;
private $params = array();
private function __construct() {
// <empty>
}
public static function newForSource($source, array $params) {
$obj = new PhabricatorContentSource();
$obj->source = $source;
$obj->params = $params;
return $obj;
}
public static function newFromSerialized($serialized) {
$dict = json_decode($serialized, true);
if (!is_array($dict)) {
$dict = array();
}
$obj = new PhabricatorContentSource();
$obj->source = idx($dict, 'source', self::SOURCE_UNKNOWN);
$obj->params = idx($dict, 'params', array());
return $obj;
}
public function serialize() {
return json_encode(array(
'source' => $this->getSource(),
'params' => $this->getParams(),
));
}
public function getSource() {
return $this->source;
}
public function getParams() {
return $this->params;
}
public function getParam($key, $default = null) {
return idx($this->params, $key, $default);
}
}

View file

@ -0,0 +1,12 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phutil', 'utils');
phutil_require_source('PhabricatorContentSource.php');

View file

@ -0,0 +1,64 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
final class PhabricatorContentSourceView extends AphrontView {
private $contentSource;
private $user;
public function setContentSource(PhabricatorContentSource $content_source) {
$this->contentSource = $content_source;
return $this;
}
public function setUser(PhabricatorUser $user) {
$this->user = $user;
return $this;
}
public function render() {
require_celerity_resource('phabricator-content-source-view-css');
$type = null;
$map = array(
PhabricatorContentSource::SOURCE_WEB => 'web',
PhabricatorContentSource::SOURCE_CONDUIT => 'conduit',
PhabricatorContentSource::SOURCE_EMAIL => 'email',
PhabricatorContentSource::SOURCE_MOBILE => 'mobile',
PhabricatorContentSource::SOURCE_TABLET => 'tablet',
);
$source = $this->contentSource->getSource();
$type = idx($map, $source, null);
if (!$type) {
return;
}
$type_class = 'phabricator-content-source-'.$type;
return phutil_render_tag(
'span',
array(
'class' => "phabricator-content-source-view {$type_class}",
),
'Via');
}
}

View file

@ -0,0 +1,17 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
phutil_require_module('phabricator', 'infrastructure/celerity/api');
phutil_require_module('phabricator', 'view/base');
phutil_require_module('phutil', 'markup');
phutil_require_module('phutil', 'utils');
phutil_require_source('PhabricatorContentSourceView.php');

View file

@ -0,0 +1,32 @@
/**
* @provides phabricator-content-source-view-css
*/
.phabricator-content-source-view {
padding: 2px 20px 2px 0;
font-size: 11px;
color: #888888;
font-weight: normal;
background: no-repeat right center;
position: relative;
}
.phabricator-content-source-web {
background-image: url(/rsrc/image/icon/fatcow/source/web.png);
}
.phabricator-content-source-email {
background-image: url(/rsrc/image/icon/fatcow/source/email.png);
}
.phabricator-content-source-conduit {
background-image: url(/rsrc/image/icon/fatcow/source/conduit.png);
}
.phabricator-content-source-mobile {
background-image: url(/rsrc/image/icon/fatcow/source/mobile.png);
}
.phabricator-content-source-tablet {
background-image: url(/rsrc/image/icon/fatcow/source/tablet.png);
}

View file

@ -8,5 +8,9 @@ They are available under the Creative Commons Attribution 3.0 License:
Some icons have been adapted from the FatCow set for use in Phabricator:
key_question.png
key_question.png (from key_*.png)
source/web.png (from world.png)
source/email.png (from email.png)
source/conduit.png (from satellite_dish.png)
source/mobile.png (from phone.png)
source/tablet.png (from tablet.png)

Binary file not shown.

After

Width:  |  Height:  |  Size: 770 B

Binary file not shown.

After

Width:  |  Height:  |  Size: 505 B

Binary file not shown.

After

Width:  |  Height:  |  Size: 527 B

Binary file not shown.

After

Width:  |  Height:  |  Size: 523 B

Binary file not shown.

After

Width:  |  Height:  |  Size: 822 B