From d9c6e07f2c4e42aa20407c90119d667b1bc4611b Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 10 Oct 2012 10:18:23 -0700 Subject: [PATCH] If users are on the email to Phabricator, do not send them the Phabricator reply. Summary: When we receive an email, figure out if any of the other tos and ccs are users. If they are, pass their phids through the stach as "exclude phids" and exclude them from getting the email. Test Plan: used the various applications (audit, differential, maniphest) and noted emails were sent as expected. Reviewers: epriestley, vrana Reviewed By: vrana CC: aran, Korvin, vrana Maniphest Tasks: T1676 Differential Revision: https://secure.phabricator.com/D3645 --- src/__phutil_library_map__.php | 14 ++++ .../audit/PhabricatorAuditReplyHandler.php | 4 +- .../PhabricatorAuditAddCommentController.php | 2 +- .../editor/PhabricatorAuditCommentEditor.php | 73 +++++++++---------- .../ConduitAPI_differential_close_Method.php | 2 +- ...tAPI_differential_createcomment_Method.php | 2 +- ...API_differential_createrevision_Method.php | 2 +- ...tAPI_differential_markcommitted_Method.php | 2 +- ...API_differential_updaterevision_Method.php | 4 +- .../maniphest/ConduitAPI_maniphest_Method.php | 1 + .../ConduitAPI_phriction_edit_Method.php | 2 +- .../differential/DifferentialReplyHandler.php | 4 +- .../DifferentialCommentSaveController.php | 2 +- .../DifferentialRevisionEditController.php | 5 +- .../editor/DifferentialCommentEditor.php | 51 +++++++------ .../editor/DifferentialRevisionEditor.php | 22 ++---- ...DifferentialFreeformFieldSpecification.php | 2 +- ...entialManiphestTasksFieldSpecification.php | 2 +- .../differential/mail/DifferentialMail.php | 36 ++++----- .../DiffusionCommitEditController.php | 2 +- .../maniphest/ManiphestReplyHandler.php | 3 + .../ManiphestBatchEditController.php | 1 + .../ManiphestSubpriorityController.php | 1 + .../ManiphestTaskEditController.php | 1 + .../ManiphestTransactionSaveController.php | 1 + .../editor/ManiphestTransactionEditor.php | 14 +++- .../event/ManiphestEdgeEventListener.php | 1 + .../PhabricatorMailReplyHandler.php | 10 +++ .../storage/PhabricatorMetaMTAMail.php | 12 ++- .../PhabricatorMetaMTAReceivedMail.php | 23 ++++++ .../people/PhabricatorUserEditor.php | 36 ++------- .../blog/PhameBlogDeleteController.php | 4 +- .../blog/PhameBlogEditController.php | 2 +- .../post/PhamePostDeleteController.php | 4 +- .../post/PhamePostEditController.php | 2 +- .../controller/PhrictionDeleteController.php | 2 +- .../controller/PhrictionEditController.php | 2 +- .../editor/PhrictionDocumentEditor.php | 23 ++---- .../controller/PonderAnswerSaveController.php | 2 +- .../PonderCommentSaveController.php | 2 +- .../PonderQuestionAskController.php | 2 +- .../controller/PonderVoteSaveController.php | 2 +- .../ponder/editor/PonderAnswerEditor.php | 20 ++--- .../ponder/editor/PonderCommentEditor.php | 20 ++--- .../ponder/editor/PonderQuestionEditor.php | 18 +---- .../ponder/editor/PonderVoteEditor.php | 22 ++---- .../PhabricatorProjectCreateController.php | 2 +- ...habricatorProjectMembersEditController.php | 2 +- ...habricatorProjectProfileEditController.php | 2 +- .../editor/PhabricatorProjectEditor.php | 33 +++------ .../PhabricatorProjectEditorTestCase.php | 2 +- ...torRepositoryCommitMessageParserWorker.php | 4 +- .../PhabricatorSearchAttachController.php | 3 +- ...PhabricatorSubscriptionsEditController.php | 2 +- .../editor/PhabricatorSubscriptionsEditor.php | 15 +--- src/infrastructure/PhabricatorEditor.php | 47 ++++++++++++ .../__tests__/PhabricatorEdgeTestCase.php | 6 +- .../edges/editor/PhabricatorEdgeEditor.php | 12 +-- 58 files changed, 302 insertions(+), 292 deletions(-) create mode 100644 src/infrastructure/PhabricatorEditor.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a08a4d5f4b..cd07a4e727 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -668,6 +668,7 @@ phutil_register_library_map(array( 'PhabricatorEdgeGraph' => 'infrastructure/edges/util/PhabricatorEdgeGraph.php', 'PhabricatorEdgeQuery' => 'infrastructure/edges/query/PhabricatorEdgeQuery.php', 'PhabricatorEdgeTestCase' => 'infrastructure/edges/__tests__/PhabricatorEdgeTestCase.php', + 'PhabricatorEditor' => 'infrastructure/PhabricatorEditor.php', 'PhabricatorEmailLoginController' => 'applications/auth/controller/PhabricatorEmailLoginController.php', 'PhabricatorEmailTokenController' => 'applications/auth/controller/PhabricatorEmailTokenController.php', 'PhabricatorEmailVerificationController' => 'applications/people/controller/PhabricatorEmailVerificationController.php', @@ -1450,6 +1451,7 @@ phutil_register_library_map(array( 'DifferentialChangesetParserTestCase' => 'ArcanistPhutilTestCase', 'DifferentialChangesetViewController' => 'DifferentialController', 'DifferentialComment' => 'DifferentialDAO', + 'DifferentialCommentEditor' => 'PhabricatorEditor', 'DifferentialCommentMail' => 'DifferentialMail', 'DifferentialCommentPreviewController' => 'DifferentialController', 'DifferentialCommentSaveController' => 'DifferentialController', @@ -1507,6 +1509,7 @@ phutil_register_library_map(array( 'DifferentialRevisionCommentView' => 'AphrontView', 'DifferentialRevisionDetailView' => 'AphrontView', 'DifferentialRevisionEditController' => 'DifferentialController', + 'DifferentialRevisionEditor' => 'PhabricatorEditor', 'DifferentialRevisionIDFieldParserTestCase' => 'PhabricatorTestCase', 'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialRevisionListController' => 'DifferentialController', @@ -1714,6 +1717,7 @@ phutil_register_library_map(array( 1 => 'PhabricatorMarkupInterface', ), 'ManiphestTransactionDetailView' => 'ManiphestView', + 'ManiphestTransactionEditor' => 'PhabricatorEditor', 'ManiphestTransactionListView' => 'ManiphestView', 'ManiphestTransactionPreviewController' => 'ManiphestController', 'ManiphestTransactionSaveController' => 'ManiphestController', @@ -1765,6 +1769,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationsListController' => 'PhabricatorController', 'PhabricatorAuditAddCommentController' => 'PhabricatorAuditController', 'PhabricatorAuditComment' => 'PhabricatorAuditDAO', + 'PhabricatorAuditCommentEditor' => 'PhabricatorEditor', 'PhabricatorAuditCommitListView' => 'AphrontView', 'PhabricatorAuditController' => 'PhabricatorController', 'PhabricatorAuditDAO' => 'PhabricatorLiskDAO', @@ -1839,6 +1844,7 @@ phutil_register_library_map(array( 'PhabricatorDraftDAO' => 'PhabricatorLiskDAO', 'PhabricatorEdgeConfig' => 'PhabricatorEdgeConstants', 'PhabricatorEdgeCycleException' => 'Exception', + 'PhabricatorEdgeEditor' => 'PhabricatorEditor', 'PhabricatorEdgeGraph' => 'AbstractDirectedGraph', 'PhabricatorEdgeQuery' => 'PhabricatorQuery', 'PhabricatorEdgeTestCase' => 'PhabricatorTestCase', @@ -2083,6 +2089,7 @@ phutil_register_library_map(array( 'PhabricatorProjectController' => 'PhabricatorController', 'PhabricatorProjectCreateController' => 'PhabricatorProjectController', 'PhabricatorProjectDAO' => 'PhabricatorLiskDAO', + 'PhabricatorProjectEditor' => 'PhabricatorEditor', 'PhabricatorProjectEditorTestCase' => 'PhabricatorTestCase', 'PhabricatorProjectListController' => 'PhabricatorProjectController', 'PhabricatorProjectMembersEditController' => 'PhabricatorProjectController', @@ -2202,6 +2209,7 @@ phutil_register_library_map(array( 'PhabricatorStorageManagementWorkflow' => 'PhutilArgumentWorkflow', 'PhabricatorSubscribersQuery' => 'PhabricatorQuery', 'PhabricatorSubscriptionsEditController' => 'PhabricatorController', + 'PhabricatorSubscriptionsEditor' => 'PhabricatorEditor', 'PhabricatorSubscriptionsUIEventListener' => 'PhutilEventListener', 'PhabricatorSymbolNameLinter' => 'ArcanistXHPASTLintNamingHook', 'PhabricatorTaskmasterDaemon' => 'PhabricatorDaemon', @@ -2229,6 +2237,7 @@ phutil_register_library_map(array( 1 => 'PhutilPerson', ), 'PhabricatorUserDAO' => 'PhabricatorLiskDAO', + 'PhabricatorUserEditor' => 'PhabricatorEditor', 'PhabricatorUserEmail' => 'PhabricatorUserDAO', 'PhabricatorUserLDAPInfo' => 'PhabricatorUserDAO', 'PhabricatorUserLog' => 'PhabricatorUserDAO', @@ -2304,6 +2313,7 @@ phutil_register_library_map(array( 'PhrictionDiffController' => 'PhrictionController', 'PhrictionDocument' => 'PhrictionDAO', 'PhrictionDocumentController' => 'PhrictionController', + 'PhrictionDocumentEditor' => 'PhabricatorEditor', 'PhrictionDocumentPreviewController' => 'PhrictionController', 'PhrictionDocumentStatus' => 'PhrictionConstants', 'PhrictionDocumentTestCase' => 'PhabricatorTestCase', @@ -2318,6 +2328,7 @@ phutil_register_library_map(array( 1 => 'PhabricatorMarkupInterface', 2 => 'PonderVotableInterface', ), + 'PonderAnswerEditor' => 'PhabricatorEditor', 'PonderAnswerListView' => 'AphrontView', 'PonderAnswerPreviewController' => 'PonderController', 'PonderAnswerQuery' => 'PhabricatorOffsetPagedQuery', @@ -2329,6 +2340,7 @@ phutil_register_library_map(array( 0 => 'PonderDAO', 1 => 'PhabricatorMarkupInterface', ), + 'PonderCommentEditor' => 'PhabricatorEditor', 'PonderCommentListView' => 'AphrontView', 'PonderCommentMail' => 'PonderMail', 'PonderCommentQuery' => 'PhabricatorQuery', @@ -2347,6 +2359,7 @@ phutil_register_library_map(array( ), 'PonderQuestionAskController' => 'PonderController', 'PonderQuestionDetailView' => 'AphrontView', + 'PonderQuestionEditor' => 'PhabricatorEditor', 'PonderQuestionPreviewController' => 'PonderController', 'PonderQuestionQuery' => 'PhabricatorOffsetPagedQuery', 'PonderQuestionSummaryView' => 'AphrontView', @@ -2355,6 +2368,7 @@ phutil_register_library_map(array( 'PonderRuleQuestion' => 'PhabricatorRemarkupRuleObjectName', 'PonderUserProfileView' => 'AphrontView', 'PonderVotableView' => 'AphrontView', + 'PonderVoteEditor' => 'PhabricatorEditor', 'PonderVoteSaveController' => 'PonderController', 'QueryFormattingTestCase' => 'PhabricatorTestCase', ), diff --git a/src/applications/audit/PhabricatorAuditReplyHandler.php b/src/applications/audit/PhabricatorAuditReplyHandler.php index 32393c7e80..c9dd80ea3a 100644 --- a/src/applications/audit/PhabricatorAuditReplyHandler.php +++ b/src/applications/audit/PhabricatorAuditReplyHandler.php @@ -61,7 +61,9 @@ final class PhabricatorAuditReplyHandler extends PhabricatorMailReplyHandler { ->setContent($mail->getCleanTextBody()); $editor = new PhabricatorAuditCommentEditor($commit); - $editor->setUser($actor); + $editor->setActor($actor); + $editor->setExcludeMailRecipientPHIDs( + $this->getExcludeMailRecipientPHIDs()); $editor->addComment($comment); } diff --git a/src/applications/audit/controller/PhabricatorAuditAddCommentController.php b/src/applications/audit/controller/PhabricatorAuditAddCommentController.php index d0180d4785..3fdb9f4b94 100644 --- a/src/applications/audit/controller/PhabricatorAuditAddCommentController.php +++ b/src/applications/audit/controller/PhabricatorAuditAddCommentController.php @@ -61,7 +61,7 @@ final class PhabricatorAuditAddCommentController } id(new PhabricatorAuditCommentEditor($commit)) - ->setUser($user) + ->setActor($user) ->setAttachInlineComments(true) ->addAuditors($auditors) ->addCCs($ccs) diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php index 93d431219a..2cb45c1523 100644 --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -16,10 +16,9 @@ * limitations under the License. */ -final class PhabricatorAuditCommentEditor { +final class PhabricatorAuditCommentEditor extends PhabricatorEditor { private $commit; - private $user; private $attachInlineComments; private $auditors = array(); @@ -30,11 +29,6 @@ final class PhabricatorAuditCommentEditor { return $this; } - public function setUser(PhabricatorUser $user) { - $this->user = $user; - return $this; - } - public function addAuditors(array $auditor_phids) { $this->auditors = array_merge($this->auditors, $auditor_phids); return $this; @@ -53,7 +47,7 @@ final class PhabricatorAuditCommentEditor { public function addComment(PhabricatorAuditComment $comment) { $commit = $this->commit; - $user = $this->user; + $actor = $this->getActor(); $other_comments = id(new PhabricatorAuditComment())->loadAllWhere( 'targetPHID = %s', @@ -64,12 +58,12 @@ final class PhabricatorAuditCommentEditor { $inline_comments = id(new PhabricatorAuditInlineComment())->loadAllWhere( 'authorPHID = %s AND commitPHID = %s AND auditCommentID IS NULL', - $user->getPHID(), + $actor->getPHID(), $commit->getPHID()); } $comment - ->setActorPHID($user->getPHID()) + ->setActorPHID($actor->getPHID()) ->setTargetPHID($commit->getPHID()) ->save(); @@ -106,13 +100,13 @@ final class PhabricatorAuditCommentEditor { $ccs = array_merge($ccs, $metacc); } - // When a user submits an audit comment, we update all the audit requests + // When an actor submits an audit comment, we update all the audit requests // they have authority over to reflect the most recent status. The general // idea here is that if audit has triggered for, e.g., several packages, but // a user owns all of them, they can clear the audit requirement in one go // without auditing the commit for each trigger. - $audit_phids = self::loadAuditPHIDsForUser($this->user); + $audit_phids = self::loadAuditPHIDsForUser($actor); $audit_phids = array_fill_keys($audit_phids, true); $requests = id(new PhabricatorRepositoryAuditRequest()) @@ -128,7 +122,7 @@ final class PhabricatorAuditCommentEditor { // and handle the no-effect cases (e.g., closing and already-closed audit). - $user_is_author = ($user->getPHID() == $commit->getAuthorPHID()); + $actor_is_author = ($actor->getPHID() == $commit->getAuthorPHID()); if ($action == PhabricatorAuditActionConstants::CLOSE) { // "Close" means wipe out all the concerns. @@ -144,25 +138,25 @@ final class PhabricatorAuditCommentEditor { // user row (never package/project rows), and always affects the user // row (other actions don't, if they were able to affect a package/project // row). - $user_request = null; + $actor_request = null; foreach ($requests as $request) { - if ($request->getAuditorPHID() == $user->getPHID()) { - $user_request = $request; + if ($request->getAuditorPHID() == $actor->getPHID()) { + $actor_request = $request; break; } } - if (!$user_request) { - $user_request = id(new PhabricatorRepositoryAuditRequest()) + if (!$actor_request) { + $actor_request = id(new PhabricatorRepositoryAuditRequest()) ->setCommitPHID($commit->getPHID()) - ->setAuditorPHID($user->getPHID()) + ->setAuditorPHID($actor->getPHID()) ->setAuditReasons(array("Resigned")); } - $user_request + $actor_request ->setAuditStatus(PhabricatorAuditStatusConstants::RESIGNED) ->save(); - $requests[] = $user_request; + $requests[] = $actor_request; } else { $have_any_requests = false; foreach ($requests as $request) { @@ -170,7 +164,8 @@ final class PhabricatorAuditCommentEditor { continue; } - $request_is_for_user = ($request->getAuditorPHID() == $user->getPHID()); + $request_is_for_actor = + ($request->getAuditorPHID() == $actor->getPHID()); $have_any_requests = true; $new_status = null; @@ -181,7 +176,7 @@ final class PhabricatorAuditCommentEditor { // Commenting or adding cc's/auditors doesn't change status. break; case PhabricatorAuditActionConstants::ACCEPT: - if (!$user_is_author || $request_is_for_user) { + if (!$actor_is_author || $request_is_for_actor) { // When modifying your own commits, you act only on behalf of // yourself, not your packages/projects -- the idea being that // you can't accept your own commits. @@ -189,7 +184,7 @@ final class PhabricatorAuditCommentEditor { } break; case PhabricatorAuditActionConstants::CONCERN: - if (!$user_is_author || $request_is_for_user) { + if (!$actor_is_author || $request_is_for_actor) { // See above. $new_status = PhabricatorAuditStatusConstants::CONCERNED; } @@ -203,7 +198,7 @@ final class PhabricatorAuditCommentEditor { } } - // If the user has no current authority over any audit trigger, make a + // If the actor has no current authority over any audit trigger, make a // new one to represent their audit state. if (!$have_any_requests) { $new_status = null; @@ -227,7 +222,7 @@ final class PhabricatorAuditCommentEditor { $request = id(new PhabricatorRepositoryAuditRequest()) ->setCommitPHID($commit->getPHID()) - ->setAuditorPHID($user->getPHID()) + ->setAuditorPHID($actor->getPHID()) ->setAuditStatus($new_status) ->setAuditReasons(array("Voluntary Participant")) ->save(); @@ -270,7 +265,7 @@ final class PhabricatorAuditCommentEditor { ->setAuditorPHID($auditor_phid) ->setAuditStatus($audit_requested) ->setAuditReasons( - array('Added by ' . $user->getUsername())) + array('Added by ' . $actor->getUsername())) ->save(); } } @@ -283,7 +278,7 @@ final class PhabricatorAuditCommentEditor { ->setAuditorPHID($cc_phid) ->setAuditStatus($audit_cc) ->setAuditReasons( - array('Added by ' . $user->getUsername())) + array('Added by ' . $actor->getUsername())) ->save(); } } @@ -322,13 +317,10 @@ final class PhabricatorAuditCommentEditor { } // The user can audit on behalf of all projects they are a member of. - $query = new PhabricatorProjectQuery(); - - // TODO: As above. - $query->setViewer($user); - - $query->withMemberPHIDs(array($user->getPHID())); - $projects = $query->execute(); + $projects = id(new PhabricatorProjectQuery()) + ->setViewer($user) + ->withMemberPHIDs(array($user->getPHID())) + ->execute(); foreach ($projects as $project) { $phids[$project->getPHID()] = true; } @@ -341,18 +333,18 @@ final class PhabricatorAuditCommentEditor { array $more_phids) { $commit = $this->commit; - $user = $this->user; + $actor = $this->getActor(); $related_phids = array_merge( array( - $user->getPHID(), + $actor->getPHID(), $commit->getPHID(), ), $more_phids); id(new PhabricatorFeedStoryPublisher()) ->setRelatedPHIDs($related_phids) - ->setStoryAuthorPHID($user->getPHID()) + ->setStoryAuthorPHID($actor->getPHID()) ->setStoryTime(time()) ->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_AUDIT) ->setStoryData( @@ -445,6 +437,7 @@ final class PhabricatorAuditCommentEditor { ->setThreadID($thread_id, $is_new) ->addHeader('Thread-Topic', $thread_topic) ->setRelatedPHID($commit->getPHID()) + ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) ->setIsBulk(true) ->setBody($body); @@ -484,8 +477,8 @@ final class PhabricatorAuditCommentEditor { assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface'); $commit = $this->commit; - $user = $this->user; - $name = $user->getUsername(); + $actor = $this->getActor(); + $name = $actor->getUsername(); $verb = PhabricatorAuditActionConstants::getActionPastTenseVerb( $comment->getAction()); diff --git a/src/applications/conduit/method/differential/ConduitAPI_differential_close_Method.php b/src/applications/conduit/method/differential/ConduitAPI_differential_close_Method.php index bcfe5a67f0..894347ed45 100644 --- a/src/applications/conduit/method/differential/ConduitAPI_differential_close_Method.php +++ b/src/applications/conduit/method/differential/ConduitAPI_differential_close_Method.php @@ -63,8 +63,8 @@ final class ConduitAPI_differential_close_Method $editor = new DifferentialCommentEditor( $revision, - $request->getUser()->getPHID(), DifferentialAction::ACTION_CLOSE); + $editor->setActor($request->getUser()); $editor->save(); $revision->setStatus(ArcanistDifferentialRevisionStatus::CLOSED); diff --git a/src/applications/conduit/method/differential/ConduitAPI_differential_createcomment_Method.php b/src/applications/conduit/method/differential/ConduitAPI_differential_createcomment_Method.php index 0eacd2efc2..93b7f9a0c2 100644 --- a/src/applications/conduit/method/differential/ConduitAPI_differential_createcomment_Method.php +++ b/src/applications/conduit/method/differential/ConduitAPI_differential_createcomment_Method.php @@ -63,8 +63,8 @@ final class ConduitAPI_differential_createcomment_Method $editor = new DifferentialCommentEditor( $revision, - $request->getUser()->getPHID(), $action); + $editor->setActor($request->getUser()); $editor->setContentSource($content_source); $editor->setMessage($request->getValue('message')); $editor->setNoEmail($request->getValue('silent')); diff --git a/src/applications/conduit/method/differential/ConduitAPI_differential_createrevision_Method.php b/src/applications/conduit/method/differential/ConduitAPI_differential_createrevision_Method.php index a6ca59fbbd..dd3321f0c0 100644 --- a/src/applications/conduit/method/differential/ConduitAPI_differential_createrevision_Method.php +++ b/src/applications/conduit/method/differential/ConduitAPI_differential_createrevision_Method.php @@ -54,7 +54,7 @@ final class ConduitAPI_differential_createrevision_Method $revision = DifferentialRevisionEditor::newRevisionFromConduitWithDiff( $fields, $diff, - $request->getUser()->getPHID()); + $request->getUser()); return array( 'revisionid' => $revision->getID(), diff --git a/src/applications/conduit/method/differential/ConduitAPI_differential_markcommitted_Method.php b/src/applications/conduit/method/differential/ConduitAPI_differential_markcommitted_Method.php index 47163a3561..8c836421e1 100644 --- a/src/applications/conduit/method/differential/ConduitAPI_differential_markcommitted_Method.php +++ b/src/applications/conduit/method/differential/ConduitAPI_differential_markcommitted_Method.php @@ -67,8 +67,8 @@ final class ConduitAPI_differential_markcommitted_Method $editor = new DifferentialCommentEditor( $revision, - $request->getUser()->getPHID(), DifferentialAction::ACTION_CLOSE); + $editor->setActor($request->getUser()); $editor->save(); } diff --git a/src/applications/conduit/method/differential/ConduitAPI_differential_updaterevision_Method.php b/src/applications/conduit/method/differential/ConduitAPI_differential_updaterevision_Method.php index 19b9338af0..1a94a74063 100644 --- a/src/applications/conduit/method/differential/ConduitAPI_differential_updaterevision_Method.php +++ b/src/applications/conduit/method/differential/ConduitAPI_differential_updaterevision_Method.php @@ -72,8 +72,8 @@ final class ConduitAPI_differential_updaterevision_Method array()); $editor = new DifferentialRevisionEditor( - $revision, - $revision->getAuthorPHID()); + $revision); + $editor->setActor($request->getUser()); $editor->setContentSource($content_source); $fields = $request->getValue('fields'); $editor->copyFieldsFromConduit($fields); diff --git a/src/applications/conduit/method/maniphest/ConduitAPI_maniphest_Method.php b/src/applications/conduit/method/maniphest/ConduitAPI_maniphest_Method.php index 2855d221a6..be9e18c127 100644 --- a/src/applications/conduit/method/maniphest/ConduitAPI_maniphest_Method.php +++ b/src/applications/conduit/method/maniphest/ConduitAPI_maniphest_Method.php @@ -196,6 +196,7 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod { $transactions = $event->getValue('transactions'); $editor = new ManiphestTransactionEditor(); + $editor->setActor($request->getUser()); $editor->applyTransactions($task, $transactions); $event = new PhabricatorEvent( diff --git a/src/applications/conduit/method/phriction/ConduitAPI_phriction_edit_Method.php b/src/applications/conduit/method/phriction/ConduitAPI_phriction_edit_Method.php index 708822891e..8d22a2f9bb 100644 --- a/src/applications/conduit/method/phriction/ConduitAPI_phriction_edit_Method.php +++ b/src/applications/conduit/method/phriction/ConduitAPI_phriction_edit_Method.php @@ -48,7 +48,7 @@ final class ConduitAPI_phriction_edit_Method $slug = $request->getValue('slug'); $editor = id(PhrictionDocumentEditor::newForSlug($slug)) - ->setUser($request->getUser()) + ->setActor($request->getUser()) ->setTitle($request->getValue('title')) ->setContent($request->getValue('content')) ->setDescription($request->getvalue('description')) diff --git a/src/applications/differential/DifferentialReplyHandler.php b/src/applications/differential/DifferentialReplyHandler.php index fc935b7691..49e03155a1 100644 --- a/src/applications/differential/DifferentialReplyHandler.php +++ b/src/applications/differential/DifferentialReplyHandler.php @@ -138,8 +138,10 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler { try { $editor = new DifferentialCommentEditor( $this->getMailReceiver(), - $actor->getPHID(), $command); + $editor->setActor($actor); + $editor->setExcludeMailRecipientPHIDs( + $this->getExcludeMailRecipientPHIDs()); // NOTE: We have to be careful about this because Facebook's // implementation jumps straight into handleAction() and will not have diff --git a/src/applications/differential/controller/DifferentialCommentSaveController.php b/src/applications/differential/controller/DifferentialCommentSaveController.php index cff04d2259..f5af856df9 100644 --- a/src/applications/differential/controller/DifferentialCommentSaveController.php +++ b/src/applications/differential/controller/DifferentialCommentSaveController.php @@ -37,7 +37,6 @@ final class DifferentialCommentSaveController extends DifferentialController { $editor = new DifferentialCommentEditor( $revision, - $request->getUser()->getPHID(), $action); $content_source = PhabricatorContentSource::newForSource( @@ -48,6 +47,7 @@ final class DifferentialCommentSaveController extends DifferentialController { try { $editor + ->setActor($request->getUser()) ->setMessage($comment) ->setContentSource($content_source) ->setAttachInlineComments(true) diff --git a/src/applications/differential/controller/DifferentialRevisionEditController.php b/src/applications/differential/controller/DifferentialRevisionEditController.php index 3442bfeabf..c71c3f5082 100644 --- a/src/applications/differential/controller/DifferentialRevisionEditController.php +++ b/src/applications/differential/controller/DifferentialRevisionEditController.php @@ -62,8 +62,6 @@ final class DifferentialRevisionEditController extends DifferentialController { if ($request->isFormPost() && !$request->getStr('viaDiffView')) { - $user_phid = $request->getUser()->getPHID(); - foreach ($aux_fields as $aux_field) { $aux_field->setValueFromRequest($request); try { @@ -74,7 +72,8 @@ final class DifferentialRevisionEditController extends DifferentialController { } if (!$errors) { - $editor = new DifferentialRevisionEditor($revision, $user_phid); + $editor = new DifferentialRevisionEditor($revision); + $editor->setActor($request->getUser()); if ($diff) { $editor->addDiff($diff, $request->getStr('comments')); } diff --git a/src/applications/differential/editor/DifferentialCommentEditor.php b/src/applications/differential/editor/DifferentialCommentEditor.php index 82817cd730..db2e699410 100644 --- a/src/applications/differential/editor/DifferentialCommentEditor.php +++ b/src/applications/differential/editor/DifferentialCommentEditor.php @@ -16,10 +16,9 @@ * limitations under the License. */ -final class DifferentialCommentEditor { +final class DifferentialCommentEditor extends PhabricatorEditor { protected $revision; - protected $actorPHID; protected $action; protected $attachInlineComments; @@ -37,11 +36,9 @@ final class DifferentialCommentEditor { public function __construct( DifferentialRevision $revision, - $actor_phid, $action) { $this->revision = $revision; - $this->actorPHID = $actor_phid; $this->action = $action; } @@ -112,16 +109,16 @@ final class DifferentialCommentEditor { } public function save() { - $revision = $this->revision; - $action = $this->action; - $actor_phid = $this->actorPHID; - $actor = id(new PhabricatorUser())->loadOneWhere('PHID = %s', $actor_phid); - $actor_is_author = ($actor_phid == $revision->getAuthorPHID()); - $allow_self_accept = PhabricatorEnv::getEnvConfig( + $actor = $this->requireActor(); + $revision = $this->revision; + $action = $this->action; + $actor_phid = $actor->getPHID(); + $actor_is_author = ($actor_phid == $revision->getAuthorPHID()); + $allow_self_accept = PhabricatorEnv::getEnvConfig( 'differential.allow-self-accept', false); $always_allow_close = PhabricatorEnv::getEnvConfig( 'differential.always-allow-close', false); - $revision_status = $revision->getStatus(); + $revision_status = $revision->getStatus(); $revision->loadRelationships(); $reviewer_phids = $revision->getReviewers(); @@ -135,7 +132,7 @@ final class DifferentialCommentEditor { if ($this->attachInlineComments) { $inline_comments = id(new DifferentialInlineComment())->loadAllWhere( 'authorPHID = %s AND revisionID = %d AND commentID IS NULL', - $this->actorPHID, + $actor_phid, $revision->getID()); } @@ -414,7 +411,7 @@ final class DifferentialCommentEditor { DifferentialRevisionEditor::addCC( $revision, $cc, - $this->actorPHID); + $actor_phid); } $key = DifferentialComment::METADATA_ADDED_CCS; @@ -490,12 +487,12 @@ final class DifferentialCommentEditor { if ($action != DifferentialAction::ACTION_RESIGN) { DifferentialRevisionEditor::addCC( $revision, - $this->actorPHID, - $this->actorPHID); + $actor_phid, + $actor_phid); } $comment = id(new DifferentialComment()) - ->setAuthorPHID($this->actorPHID) + ->setAuthorPHID($actor_phid) ->setRevisionID($revision->getID()) ->setAction($action) ->setContent((string)$this->message) @@ -541,7 +538,7 @@ final class DifferentialCommentEditor { DifferentialRevisionEditor::addCC( $revision, $cc_phid, - $this->actorPHID); + $actor_phid); $metacc[] = $cc_phid; } $metadata[DifferentialComment::METADATA_ADDED_CCS] = $metacc; @@ -553,10 +550,10 @@ final class DifferentialCommentEditor { $revision->saveTransaction(); - $phids = array($this->actorPHID); + $phids = array($actor_phid); $handles = id(new PhabricatorObjectHandleData($phids)) ->loadHandles(); - $actor_handle = $handles[$this->actorPHID]; + $actor_handle = $handles[$actor_phid]; $xherald_header = HeraldTranscript::loadXHeraldRulesHeader( $revision->getPHID()); @@ -568,6 +565,7 @@ final class DifferentialCommentEditor { $comment, $changesets, $inline_comments)) + ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) ->setToPHIDs( array_merge( $revision->getReviewers(), @@ -586,7 +584,7 @@ final class DifferentialCommentEditor { 'revision_author_phid' => $revision->getAuthorPHID(), 'action' => $comment->getAction(), 'feedback_content' => $comment->getContent(), - 'actor_phid' => $this->actorPHID, + 'actor_phid' => $actor_phid, ); id(new PhabricatorTimelineEvent('difx', $event_data)) ->recordEvent(); @@ -596,11 +594,11 @@ final class DifferentialCommentEditor { ->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_DIFFERENTIAL) ->setStoryData($event_data) ->setStoryTime(time()) - ->setStoryAuthorPHID($this->actorPHID) + ->setStoryAuthorPHID($actor_phid) ->setRelatedPHIDs( array( $revision->getPHID(), - $this->actorPHID, + $actor_phid, $revision->getAuthorPHID(), )) ->setPrimaryObjectPHID($revision->getPHID()) @@ -642,10 +640,11 @@ final class DifferentialCommentEditor { } private function alterReviewers() { - $revision = $this->revision; - $added_reviewers = $this->getAddedReviewers(); + $actor_phid = $this->getActor()->getPHID(); + $revision = $this->revision; + $added_reviewers = $this->getAddedReviewers(); $removed_reviewers = $this->getRemovedReviewers(); - $reviewer_phids = $revision->getReviewers(); + $reviewer_phids = $revision->getReviewers(); $reviewer_phids_map = array_fill_keys($reviewer_phids, true); foreach ($added_reviewers as $k => $user_phid) { @@ -672,7 +671,7 @@ final class DifferentialCommentEditor { $reviewer_phids, $removed_reviewers, $added_reviewers, - $this->actorPHID); + $actor_phid); } return array($added_reviewers, $removed_reviewers); diff --git a/src/applications/differential/editor/DifferentialRevisionEditor.php b/src/applications/differential/editor/DifferentialRevisionEditor.php index 62c1c3b64e..5436d99fff 100644 --- a/src/applications/differential/editor/DifferentialRevisionEditor.php +++ b/src/applications/differential/editor/DifferentialRevisionEditor.php @@ -21,10 +21,9 @@ * reviewers, diffs, and CCs. Unlike simple edits, these changes trigger * complicated email workflows. */ -final class DifferentialRevisionEditor { +final class DifferentialRevisionEditor extends PhabricatorEditor { protected $revision; - protected $actorPHID; protected $cc = null; protected $reviewers = null; @@ -35,24 +34,22 @@ final class DifferentialRevisionEditor { private $auxiliaryFields = array(); private $contentSource; - public function __construct(DifferentialRevision $revision, $actor_phid) { + public function __construct(DifferentialRevision $revision) { $this->revision = $revision; - $this->actorPHID = $actor_phid; } public static function newRevisionFromConduitWithDiff( array $fields, DifferentialDiff $diff, - $user_phid) { + PhabricatorUser $actor) { $revision = new DifferentialRevision(); $revision->setPHID($revision->generatePHID()); - - $revision->setAuthorPHID($user_phid); + $revision->setAuthorPHID($actor->getPHID()); $revision->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); - $editor = new DifferentialRevisionEditor($revision, $user_phid); - + $editor = new DifferentialRevisionEditor($revision); + $editor->setActor($actor); $editor->copyFieldsFromConduit($fields); $editor->addDiff($diff, null); @@ -63,19 +60,16 @@ final class DifferentialRevisionEditor { public function copyFieldsFromConduit(array $fields) { + $actor = $this->getActor(); $revision = $this->revision; $revision->loadRelationships(); $aux_fields = DifferentialFieldSelector::newSelector() ->getFieldSpecifications(); - $user = id(new PhabricatorUser())->loadOneWhere( - 'phid = %s', - $this->actorPHID); - foreach ($aux_fields as $key => $aux_field) { $aux_field->setRevision($revision); - $aux_field->setUser($user); + $aux_field->setUser($actor); if (!$aux_field->shouldAppearOnCommitMessage()) { unset($aux_fields[$key]); } diff --git a/src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php b/src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php index 4001219fc8..6b34a9e5db 100644 --- a/src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php @@ -102,7 +102,7 @@ abstract class DifferentialFreeformFieldSpecification } id(new PhabricatorEdgeEditor()) - ->setUser($user) + ->setActor($user) ->addEdge( $task->getPHID(), PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT, diff --git a/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php b/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php index 136beceadc..b436b057e2 100644 --- a/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php @@ -74,7 +74,7 @@ final class DifferentialManiphestTasksFieldSpecification $rem_phids = array_diff($old_phids, $add_phids); $edge_editor = id(new PhabricatorEdgeEditor()) - ->setUser($this->getUser()); + ->setActor($this->getUser()); foreach ($add_phids as $phid) { $edge_editor->addEdge($revision_phid, $edge_type, $phid); diff --git a/src/applications/differential/mail/DifferentialMail.php b/src/applications/differential/mail/DifferentialMail.php index f232186222..e94867af05 100644 --- a/src/applications/differential/mail/DifferentialMail.php +++ b/src/applications/differential/mail/DifferentialMail.php @@ -20,6 +20,7 @@ abstract class DifferentialMail { protected $to = array(); protected $cc = array(); + protected $excludePHIDs = array(); protected $actorHandle; @@ -91,6 +92,7 @@ abstract class DifferentialMail { $template ->setIsHTML($this->shouldMarkMailAsHTML()) ->setParentMessageID($this->parentMessageID) + ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) ->addHeader('Thread-Topic', $this->getThreadTopic()); $template->setAttachments($attachments); @@ -315,37 +317,25 @@ abstract class DifferentialMail { return implode("\n", $text); } + public function setExcludeMailRecipientPHIDs(array $exclude) { + $this->excludePHIDs = $exclude; + return $this; + } + + public function getExcludeMailRecipientPHIDs() { + return $this->excludePHIDs; + } + public function setToPHIDs(array $to) { - $this->to = $this->filterContactPHIDs($to); + $this->to = $to; return $this; } public function setCCPHIDs(array $cc) { - $this->cc = $this->filterContactPHIDs($cc); + $this->cc = $cc; return $this; } - protected function filterContactPHIDs(array $phids) { - return $phids; - - // TODO: actually do this? - - // Differential revisions use Subscriptions for CCs, so any arbitrary - // PHID can end up CC'd to them. Only try to actually send email PHIDs - // which have ToolsHandle types that are marked emailable. If we don't - // filter here, sending the email will fail. -/* - $handles = array(); - prep(new ToolsHandleData($phids, $handles)); - foreach ($handles as $phid => $handle) { - if (!$handle->isEmailable()) { - unset($handles[$phid]); - } - } - return array_keys($handles); -*/ - } - protected function getToPHIDs() { return $this->to; } diff --git a/src/applications/diffusion/controller/DiffusionCommitEditController.php b/src/applications/diffusion/controller/DiffusionCommitEditController.php index a1d712ef4f..c927c79bf9 100644 --- a/src/applications/diffusion/controller/DiffusionCommitEditController.php +++ b/src/applications/diffusion/controller/DiffusionCommitEditController.php @@ -51,7 +51,7 @@ final class DiffusionCommitEditController extends DiffusionController { $rem_proj_phids = array_diff($current_proj_phids, $new_proj_phids); $editor = id(new PhabricatorEdgeEditor()); - $editor->setUser($user); + $editor->setActor($user); foreach ($rem_proj_phids as $phid) { $editor->removeEdge($commit_phid, $edge_type, $phid); } diff --git a/src/applications/maniphest/ManiphestReplyHandler.php b/src/applications/maniphest/ManiphestReplyHandler.php index 233845fb0b..423f36e1b4 100644 --- a/src/applications/maniphest/ManiphestReplyHandler.php +++ b/src/applications/maniphest/ManiphestReplyHandler.php @@ -167,7 +167,10 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler { $editor = new ManiphestTransactionEditor(); + $editor->setActor($user); $editor->setParentMessageID($mail->getMessageID()); + $editor->setExcludeMailRecipientPHIDs( + $this->getExcludeMailRecipientPHIDs()); $editor->applyTransactions($task, $xactions); $event = new PhabricatorEvent( diff --git a/src/applications/maniphest/controller/ManiphestBatchEditController.php b/src/applications/maniphest/controller/ManiphestBatchEditController.php index 32b0fde384..bf538e1185 100644 --- a/src/applications/maniphest/controller/ManiphestBatchEditController.php +++ b/src/applications/maniphest/controller/ManiphestBatchEditController.php @@ -41,6 +41,7 @@ final class ManiphestBatchEditController extends ManiphestController { $xactions = $this->buildTransactions($actions, $task); if ($xactions) { $editor = new ManiphestTransactionEditor(); + $editor->setActor($user); $editor->applyTransactions($task, $xactions); } } diff --git a/src/applications/maniphest/controller/ManiphestSubpriorityController.php b/src/applications/maniphest/controller/ManiphestSubpriorityController.php index 48fdf17b23..5793ec214e 100644 --- a/src/applications/maniphest/controller/ManiphestSubpriorityController.php +++ b/src/applications/maniphest/controller/ManiphestSubpriorityController.php @@ -59,6 +59,7 @@ final class ManiphestSubpriorityController extends ManiphestController { $xaction->setNewValue($after_pri); $editor = new ManiphestTransactionEditor(); + $editor->setActor($request->getUser()); $editor->applyTransactions($task, array($xaction)); } diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index b298e16232..96a1704757 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -231,6 +231,7 @@ final class ManiphestTaskEditController extends ManiphestController { $transactions = $event->getValue('transactions'); $editor = new ManiphestTransactionEditor(); + $editor->setActor($user); $editor->setAuxiliaryFields($aux_fields); $editor->applyTransactions($task, $transactions); diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php index eb90e74470..18b98c21f4 100644 --- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php +++ b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php @@ -244,6 +244,7 @@ final class ManiphestTransactionSaveController extends ManiphestController { $transactions = $event->getValue('transactions'); $editor = new ManiphestTransactionEditor(); + $editor->setActor($user); $editor->applyTransactions($task, $transactions); $draft = id(new PhabricatorDraft())->loadOneWhere( diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index 8ced7f62c3..0570a200e9 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -19,9 +19,10 @@ /** * @group maniphest */ -final class ManiphestTransactionEditor { +final class ManiphestTransactionEditor extends PhabricatorEditor { private $parentMessageID; + private $excludePHIDs = array(); private $auxiliaryFields = array(); public function setAuxiliaryFields(array $fields) { @@ -35,6 +36,15 @@ final class ManiphestTransactionEditor { return $this; } + public function setExcludePHIDs(array $exclude) { + $this->excludePHIDs = $exclude; + return $this; + } + + public function getExcludePHIDs() { + return $this->excludePHIDs; + } + public function applyTransactions(ManiphestTask $task, array $transactions) { assert_instances_of($transactions, 'ManiphestTransaction'); @@ -221,6 +231,7 @@ final class ManiphestTransactionEditor { } private function sendEmail($task, $transactions, $email_to, $email_cc) { + $exclude = $this->getExcludePHIDs(); $email_to = array_filter(array_unique($email_to)); $email_cc = array_filter(array_unique($email_cc)); @@ -276,6 +287,7 @@ final class ManiphestTransactionEditor { ->addHeader('Thread-Topic', "T{$task_id}: ".$task->getOriginalTitle()) ->setThreadID($thread_id, $is_create) ->setRelatedPHID($task->getPHID()) + ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) ->setIsBulk(true) ->setMailTags($mailtags) ->setBody($body->render()); diff --git a/src/applications/maniphest/event/ManiphestEdgeEventListener.php b/src/applications/maniphest/event/ManiphestEdgeEventListener.php index 9ba10311ad..73f0c56711 100644 --- a/src/applications/maniphest/event/ManiphestEdgeEventListener.php +++ b/src/applications/maniphest/event/ManiphestEdgeEventListener.php @@ -73,6 +73,7 @@ final class ManiphestEdgeEventListener extends PhutilEventListener { $new_edges = $this->loadAllEdges($event); $editor = new ManiphestTransactionEditor(); + $editor->setActor($event->getUser()); foreach ($tasks as $phid => $task) { $xactions = array(); diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php index 3c74108c3e..02429d9d36 100644 --- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php @@ -20,6 +20,7 @@ abstract class PhabricatorMailReplyHandler { private $mailReceiver; private $actor; + private $excludePHIDs = array(); final public function setMailReceiver($mail_receiver) { $this->validateMailReceiver($mail_receiver); @@ -40,6 +41,15 @@ abstract class PhabricatorMailReplyHandler { return $this->actor; } + final public function setExcludeMailRecipientPHIDs(array $exclude) { + $this->excludePHIDs = $exclude; + return $this; + } + + final public function getExcludeMailRecipientPHIDs() { + return $this->excludePHIDs; + } + abstract public function validateMailReceiver($mail_receiver); abstract public function getPrivateReplyHandlerEmailAddress( PhabricatorObjectHandle $handle); diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index da7d2ef889..78df0ef773 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -36,6 +36,8 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { protected $nextRetry; protected $relatedPHID; + private $excludePHIDs = array(); + public function __construct() { $this->status = self::STATUS_QUEUE; @@ -119,6 +121,14 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { return $this; } + public function setExcludeMailRecipientPHIDs($exclude) { + $this->excludePHIDs = $exclude; + return $this; + } + private function getExcludeMailRecipientPHIDs() { + return $this->excludePHIDs; + } + public function getTranslation(array $objects) { $default_translation = PhabricatorEnv::getEnvConfig('translation.provider'); $return = null; @@ -349,7 +359,7 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { $this->loadEmailAndNameDataFromPHIDs($phids); - $exclude = array(); + $exclude = array_fill_keys($this->getExcludeMailRecipientPHIDs(), true); $params = $this->parameters; $default = PhabricatorEnv::getEnvConfig('metamta.default-address'); diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php index 59736d0538..2ddd902644 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php @@ -62,6 +62,23 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return $this->getRawEmailAddresses(idx($this->headers, 'to')); } + private function loadExcludeMailRecipientPHIDs() { + $addresses = array_merge( + $this->getToAddresses(), + $this->getCCAddresses() + ); + + $users = id(new PhabricatorUserEmail()) + ->loadAllWhere('address IN (%Ls)', $addresses); + $user_phids = mpull($users, 'getUserPHID'); + + $mailing_lists = id(new PhabricatorMetaMTAMailingList()) + ->loadAllWhere('email in (%Ls)', $addresses); + $mailing_list_phids = mpull($mailing_lists, 'getPHID'); + + return array_merge($user_phids, $mailing_list_phids); + } + /** * Parses "to" addresses, looking for a public create email address * first and if not found parsing the "to" address for reply handler @@ -182,9 +199,12 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { $receiver->setPriority(ManiphestTaskPriority::PRIORITY_TRIAGE); $editor = new ManiphestTransactionEditor(); + $editor->setActor($user); $handler = $editor->buildReplyHandler($receiver); $handler->setActor($user); + $handler->setExcludeMailRecipientPHIDs( + $this->loadExcludeMailRecipientPHIDs()); $handler->processEmail($this); $this->setRelatedPHID($receiver->getPHID()); @@ -248,6 +268,7 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { if ($receiver instanceof ManiphestTask) { $editor = new ManiphestTransactionEditor(); + $editor->setActor($user); $handler = $editor->buildReplyHandler($receiver); } else if ($receiver instanceof DifferentialRevision) { $handler = DifferentialMail::newReplyHandlerForRevision($receiver); @@ -257,6 +278,8 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { } $handler->setActor($user); + $handler->setExcludeMailRecipientPHIDs( + $this->loadExcludeMailRecipientPHIDs()); $handler->processEmail($this); $this->setMessage('OK'); diff --git a/src/applications/people/PhabricatorUserEditor.php b/src/applications/people/PhabricatorUserEditor.php index 5e6fd70544..9f811e42c5 100644 --- a/src/applications/people/PhabricatorUserEditor.php +++ b/src/applications/people/PhabricatorUserEditor.php @@ -26,24 +26,11 @@ * @task email Adding, Removing and Changing Email * @task internal Internals */ -final class PhabricatorUserEditor { +final class PhabricatorUserEditor extends PhabricatorEditor { - private $actor; private $logs = array(); -/* -( Configuration )------------------------------------------------------ */ - - - /** - * @task config - */ - public function setActor(PhabricatorUser $actor) { - $this->actor = $actor; - return $this; - } - - /* -( Creating and Editing Users )----------------------------------------- */ @@ -88,7 +75,7 @@ final class PhabricatorUserEditor { } $log = PhabricatorUserLog::newLog( - $this->actor, + $this->getActor(), $user, PhabricatorUserLog::ACTION_CREATE); $log->setNewValue($email->getAddress()); @@ -147,7 +134,7 @@ final class PhabricatorUserEditor { $user->save(); $log = PhabricatorUserLog::newLog( - $this->actor, + $this->getActor(), $user, PhabricatorUserLog::ACTION_CHANGE_PASSWORD); $log->save(); @@ -186,7 +173,7 @@ final class PhabricatorUserEditor { } $log = PhabricatorUserLog::newLog( - $this->actor, + $actor, $user, PhabricatorUserLog::ACTION_CHANGE_USERNAME); $log->setOldValue($old_username); @@ -429,7 +416,7 @@ final class PhabricatorUserEditor { } $log = PhabricatorUserLog::newLog( - $this->actor, + $actor, $user, PhabricatorUserLog::ACTION_EMAIL_ADD); $log->setNewValue($email->getAddress()); @@ -474,7 +461,7 @@ final class PhabricatorUserEditor { $email->delete(); $log = PhabricatorUserLog::newLog( - $this->actor, + $actor, $user, PhabricatorUserLog::ACTION_EMAIL_REMOVE); $log->setOldValue($email->getAddress()); @@ -553,17 +540,6 @@ final class PhabricatorUserEditor { /* -( Internals )---------------------------------------------------------- */ - /** - * @task internal - */ - private function requireActor() { - if (!$this->actor) { - throw new Exception("User edit requires actor!"); - } - return $this->actor; - } - - /** * @task internal */ diff --git a/src/applications/phame/controller/blog/PhameBlogDeleteController.php b/src/applications/phame/controller/blog/PhameBlogDeleteController.php index 0eb2ef6512..2743cc4d06 100644 --- a/src/applications/phame/controller/blog/PhameBlogDeleteController.php +++ b/src/applications/phame/controller/blog/PhameBlogDeleteController.php @@ -88,8 +88,8 @@ extends PhameController { $blogger_phids = array_keys($blogger_edges); $post_edges = $edges[$blog_phid][$post_edge_type]; $post_phids = array_keys($post_edges); - $editor = id(new PhabricatorEdgeEditor()); - $editor->setUser($user); + $editor = id(new PhabricatorEdgeEditor()) + ->setActor($user); foreach ($blogger_phids as $phid) { $editor->removeEdge($blog_phid, $blogger_edge_type, $phid); } diff --git a/src/applications/phame/controller/blog/PhameBlogEditController.php b/src/applications/phame/controller/blog/PhameBlogEditController.php index 7aa6ba4328..9d55abc8be 100644 --- a/src/applications/phame/controller/blog/PhameBlogEditController.php +++ b/src/applications/phame/controller/blog/PhameBlogEditController.php @@ -163,7 +163,7 @@ final class PhameBlogEditController $rem_phids = array_diff($old_bloggers, $new_bloggers); $editor = new PhabricatorEdgeEditor(); $edge_type = PhabricatorEdgeConfig::TYPE_BLOG_HAS_BLOGGER; - $editor->setUser($user); + $editor->setActor($user); foreach ($add_phids as $phid) { $editor->addEdge($blog->getPHID(), $edge_type, $phid); } diff --git a/src/applications/phame/controller/post/PhamePostDeleteController.php b/src/applications/phame/controller/post/PhamePostDeleteController.php index ed94af410c..3e0db52cbf 100644 --- a/src/applications/phame/controller/post/PhamePostDeleteController.php +++ b/src/applications/phame/controller/post/PhamePostDeleteController.php @@ -62,8 +62,8 @@ extends PhameController { $blog_edges = $edges[$post_phid][$edge_type]; $blog_phids = array_keys($blog_edges); - $editor = id(new PhabricatorEdgeEditor()); - $editor->setUser($user); + $editor = id(new PhabricatorEdgeEditor()) + ->setActor($user); foreach ($blog_phids as $phid) { $editor->removeEdge($post_phid, $edge_type, $phid); } diff --git a/src/applications/phame/controller/post/PhamePostEditController.php b/src/applications/phame/controller/post/PhamePostEditController.php index 1f18b1a74d..f094c5779d 100644 --- a/src/applications/phame/controller/post/PhamePostEditController.php +++ b/src/applications/phame/controller/post/PhamePostEditController.php @@ -198,7 +198,7 @@ final class PhamePostEditController $editor = new PhabricatorEdgeEditor(); $edge_type = PhabricatorEdgeConfig::TYPE_POST_HAS_BLOG; - $editor->setUser($user); + $editor->setActor($user); foreach ($blogs_to_publish as $phid) { $editor->addEdge($post->getPHID(), $edge_type, $phid); } diff --git a/src/applications/phriction/controller/PhrictionDeleteController.php b/src/applications/phriction/controller/PhrictionDeleteController.php index ada62b50a5..7f2fdd1c75 100644 --- a/src/applications/phriction/controller/PhrictionDeleteController.php +++ b/src/applications/phriction/controller/PhrictionDeleteController.php @@ -41,7 +41,7 @@ final class PhrictionDeleteController extends PhrictionController { if ($request->isFormPost()) { $editor = id(PhrictionDocumentEditor::newForSlug($document->getSlug())) - ->setUser($user) + ->setActor($user) ->delete(); return id(new AphrontRedirectResponse())->setURI($document_uri); } diff --git a/src/applications/phriction/controller/PhrictionEditController.php b/src/applications/phriction/controller/PhrictionEditController.php index adfcbbd8f8..79b9188681 100644 --- a/src/applications/phriction/controller/PhrictionEditController.php +++ b/src/applications/phriction/controller/PhrictionEditController.php @@ -134,7 +134,7 @@ final class PhrictionEditController if (!count($errors)) { $editor = id(PhrictionDocumentEditor::newForSlug($document->getSlug())) - ->setUser($user) + ->setActor($user) ->setTitle($title) ->setContent($request->getStr('content')) ->setDescription($notes); diff --git a/src/applications/phriction/editor/PhrictionDocumentEditor.php b/src/applications/phriction/editor/PhrictionDocumentEditor.php index 7e0903efa0..adb9f52ac8 100644 --- a/src/applications/phriction/editor/PhrictionDocumentEditor.php +++ b/src/applications/phriction/editor/PhrictionDocumentEditor.php @@ -21,13 +21,11 @@ * * @group phriction */ -final class PhrictionDocumentEditor { +final class PhrictionDocumentEditor extends PhabricatorEditor { private $document; private $content; - private $user; - private $newTitle; private $newContent; private $description; @@ -65,11 +63,6 @@ final class PhrictionDocumentEditor { return $obj; } - public function setUser(PhabricatorUser $user) { - $this->user = $user; - return $this; - } - public function setTitle($title) { $this->newTitle = $title; return $this; @@ -90,9 +83,7 @@ final class PhrictionDocumentEditor { } public function delete() { - if (!$this->user) { - throw new Exception("Call setUser() before deleting a document!"); - } + $actor = $this->requireActor(); // TODO: Should we do anything about deleting an already-deleted document? // We currently allow it. @@ -109,9 +100,7 @@ final class PhrictionDocumentEditor { } public function save() { - if (!$this->user) { - throw new Exception("Call setUser() before updating a document!"); - } + $actor = $this->requireActor(); if ($this->newContent === '') { // If this is an edit which deletes all the content, just treat it as @@ -134,7 +123,7 @@ final class PhrictionDocumentEditor { $new_content = new PhrictionContent(); $new_content->setSlug($document->getSlug()); - $new_content->setAuthorPHID($this->user->getPHID()); + $new_content->setAuthorPHID($this->getActor()->getPHID()); $new_content->setChangeType(PhrictionChangeType::CHANGE_EDIT); $new_content->setTitle( @@ -214,7 +203,7 @@ final class PhrictionDocumentEditor { $related_phids = array( $document->getPHID(), - $this->user->getPHID(), + $this->getActor()->getPHID(), ); if ($project_phid) { @@ -223,7 +212,7 @@ final class PhrictionDocumentEditor { id(new PhabricatorFeedStoryPublisher()) ->setRelatedPHIDs($related_phids) - ->setStoryAuthorPHID($this->user->getPHID()) + ->setStoryAuthorPHID($this->getActor()->getPHID()) ->setStoryTime(time()) ->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_PHRICTION) ->setStoryData( diff --git a/src/applications/ponder/controller/PonderAnswerSaveController.php b/src/applications/ponder/controller/PonderAnswerSaveController.php index 00f0e61a4d..a62dfcf224 100644 --- a/src/applications/ponder/controller/PonderAnswerSaveController.php +++ b/src/applications/ponder/controller/PonderAnswerSaveController.php @@ -60,7 +60,7 @@ final class PonderAnswerSaveController extends PonderController { ->setContentSource($content_source); id(new PonderAnswerEditor()) - ->setUser($user) + ->setActor($user) ->setQuestion($question) ->setAnswer($res) ->saveAnswer(); diff --git a/src/applications/ponder/controller/PonderCommentSaveController.php b/src/applications/ponder/controller/PonderCommentSaveController.php index 5c1688e131..d852a99345 100644 --- a/src/applications/ponder/controller/PonderCommentSaveController.php +++ b/src/applications/ponder/controller/PonderCommentSaveController.php @@ -61,7 +61,7 @@ final class PonderCommentSaveController extends PonderController { ->setQuestion($question) ->setComment($res) ->setTargetPHID($target) - ->setUser($user) + ->setActor($user) ->save(); return id(new AphrontRedirectResponse()) diff --git a/src/applications/ponder/controller/PonderQuestionAskController.php b/src/applications/ponder/controller/PonderQuestionAskController.php index 5f1d59461a..e3b0d8282a 100644 --- a/src/applications/ponder/controller/PonderQuestionAskController.php +++ b/src/applications/ponder/controller/PonderQuestionAskController.php @@ -53,7 +53,7 @@ final class PonderQuestionAskController extends PonderController { id(new PonderQuestionEditor()) ->setQuestion($question) - ->setUser($user) + ->setActor($user) ->save(); return id(new AphrontRedirectResponse()) diff --git a/src/applications/ponder/controller/PonderVoteSaveController.php b/src/applications/ponder/controller/PonderVoteSaveController.php index 6d58ef946d..28174eb787 100644 --- a/src/applications/ponder/controller/PonderVoteSaveController.php +++ b/src/applications/ponder/controller/PonderVoteSaveController.php @@ -49,7 +49,7 @@ final class PonderVoteSaveController extends PonderController { $editor = id(new PonderVoteEditor()) ->setVotable($target) - ->setUser($user) + ->setActor($user) ->setVote($newvote) ->saveVote(); diff --git a/src/applications/ponder/editor/PonderAnswerEditor.php b/src/applications/ponder/editor/PonderAnswerEditor.php index 9225873da8..66fb8e3b89 100644 --- a/src/applications/ponder/editor/PonderAnswerEditor.php +++ b/src/applications/ponder/editor/PonderAnswerEditor.php @@ -16,10 +16,10 @@ * limitations under the License. */ -final class PonderAnswerEditor { +final class PonderAnswerEditor extends PhabricatorEditor { + private $question; private $answer; - private $viewer; private $shouldEmail = true; public function setQuestion($question) { @@ -32,15 +32,8 @@ final class PonderAnswerEditor { return $this; } - public function setUser(PhabricatorUser $user) { - $this->viewer = $user; - return $this; - } - public function saveAnswer() { - if (!$this->viewer) { - throw new Exception("Must set user before saving question"); - } + $actor = $this->requireActor(); if (!$this->question) { throw new Exception("Must set question before saving answer"); } @@ -50,7 +43,6 @@ final class PonderAnswerEditor { $question = $this->question; $answer = $this->answer; - $viewer = $this->viewer; $conn = $answer->establishConnection('w'); $trans = $conn->openTransaction(); $trans->beginReadLocking(); @@ -76,7 +68,7 @@ final class PonderAnswerEditor { // subscribe author and @mentions $subeditor = id(new PhabricatorSubscriptionsEditor()) ->setObject($question) - ->setUser($viewer); + ->setActor($actor); $subeditor->subscribeExplicit(array($answer->getAuthorPHID())); @@ -98,7 +90,7 @@ final class PonderAnswerEditor { id(new PonderMentionMail( $question, $answer, - $viewer)) + $actor)) ->setToPHIDs($at_mention_phids) ->send(); } @@ -115,7 +107,7 @@ final class PonderAnswerEditor { id(new PonderAnsweredMail( $question, $answer, - $viewer)) + $actor)) ->setToPHIDs($other_subs) ->send(); } diff --git a/src/applications/ponder/editor/PonderCommentEditor.php b/src/applications/ponder/editor/PonderCommentEditor.php index f51bcb9e0c..de28add69a 100644 --- a/src/applications/ponder/editor/PonderCommentEditor.php +++ b/src/applications/ponder/editor/PonderCommentEditor.php @@ -16,13 +16,11 @@ * limitations under the License. */ - -final class PonderCommentEditor { +final class PonderCommentEditor extends PhabricatorEditor { private $question; private $comment; private $targetPHID; - private $viewer; private $shouldEmail = true; public function setComment(PonderComment $comment) { @@ -40,12 +38,8 @@ final class PonderCommentEditor { return $this; } - public function setUser(PhabricatorUser $user) { - $this->viewer = $user; - return $this; - } - public function save() { + $actor = $this->requireActor(); if (!$this->comment) { throw new Exception("Must set comment before saving it"); } @@ -55,14 +49,10 @@ final class PonderCommentEditor { if (!$this->targetPHID) { throw new Exception("Must set target before saving comment"); } - if (!$this->viewer) { - throw new Exception("Must set viewer before saving comment"); - } $comment = $this->comment; $question = $this->question; $target = $this->targetPHID; - $viewer = $this->viewer; $comment->save(); $question->attachRelated(); @@ -71,7 +61,7 @@ final class PonderCommentEditor { // subscribe author and @mentions $subeditor = id(new PhabricatorSubscriptionsEditor()) ->setObject($question) - ->setUser($viewer); + ->setActor($actor); $subeditor->subscribeExplicit(array($comment->getAuthorPHID())); @@ -92,7 +82,7 @@ final class PonderCommentEditor { id(new PonderMentionMail( $question, $comment, - $viewer)) + $actor)) ->setToPHIDs($at_mention_phids) ->send(); } @@ -124,7 +114,7 @@ final class PonderCommentEditor { id(new PonderCommentMail( $question, $comment, - $viewer)) + $actor)) ->setToPHIDs($other_subs) ->send(); } diff --git a/src/applications/ponder/editor/PonderQuestionEditor.php b/src/applications/ponder/editor/PonderQuestionEditor.php index 08ec5e5236..b37a39a5ca 100644 --- a/src/applications/ponder/editor/PonderQuestionEditor.php +++ b/src/applications/ponder/editor/PonderQuestionEditor.php @@ -16,11 +16,9 @@ * limitations under the License. */ - -final class PonderQuestionEditor { +final class PonderQuestionEditor extends PhabricatorEditor { private $question; - private $viewer; private $shouldEmail = true; public function setQuestion(PonderQuestion $question) { @@ -28,25 +26,17 @@ final class PonderQuestionEditor { return $this; } - public function setUser(PhabricatorUser $user) { - $this->viewer = $user; - return $this; - } - public function setShouldEmail($se) { $this->shouldEmail = $se; return $this; } public function save() { - if (!$this->viewer) { - throw new Exception("Must set user before saving question"); - } + $actor = $this->requireActor(); if (!$this->question) { throw new Exception("Must set question before saving it"); } - $viewer = $this->viewer; $question = $this->question; $question->save(); @@ -57,7 +47,7 @@ final class PonderQuestionEditor { // subscribe author and @mentions $subeditor = id(new PhabricatorSubscriptionsEditor()) ->setObject($question) - ->setUser($viewer); + ->setActor($actor); $subeditor->subscribeExplicit(array($question->getAuthorPHID())); @@ -72,7 +62,7 @@ final class PonderQuestionEditor { id(new PonderMentionMail( $question, $question, - $viewer)) + $actor)) ->setToPHIDs($at_mention_phids) ->send(); } diff --git a/src/applications/ponder/editor/PonderVoteEditor.php b/src/applications/ponder/editor/PonderVoteEditor.php index 4a6090d497..35623f81d2 100644 --- a/src/applications/ponder/editor/PonderVoteEditor.php +++ b/src/applications/ponder/editor/PonderVoteEditor.php @@ -16,13 +16,11 @@ * limitations under the License. */ - -final class PonderVoteEditor { +final class PonderVoteEditor extends PhabricatorEditor { private $answer; private $votable; private $anwer; - private $user; private $vote; public function setAnswer($answer) { @@ -35,39 +33,31 @@ final class PonderVoteEditor { return $this; } - public function setUser($user) { - $this->user = $user; - return $this; - } - public function setVote($vote) { $this->vote = $vote; return $this; } public function saveVote() { + $actor = $this->requireActor(); if (!$this->votable) { throw new Exception("Must set votable before saving vote"); } - if (!$this->user) { - throw new Exception("Must set user before saving vote"); - } - $user = $this->user; $votable = $this->votable; $newvote = $this->vote; // prepare vote add, or update if this user is amending an // earlier vote $editor = id(new PhabricatorEdgeEditor()) - ->setUser($user) + ->setActor($actor) ->addEdge( - $user->getPHID(), + $actor->getPHID(), $votable->getUserVoteEdgeType(), $votable->getVotablePHID(), array('data' => $newvote)) ->removeEdge( - $user->getPHID(), + $actor->getPHID(), $votable->getUserVoteEdgeType(), $votable->getVotablePHID()); @@ -77,7 +67,7 @@ final class PonderVoteEditor { $votable->reload(); $curvote = (int)PhabricatorEdgeQuery::loadSingleEdgeData( - $user->getPHID(), + $actor->getPHID(), $votable->getUserVoteEdgeType(), $votable->getVotablePHID()); diff --git a/src/applications/project/controller/PhabricatorProjectCreateController.php b/src/applications/project/controller/PhabricatorProjectCreateController.php index c93ec5f6b4..4f184b061d 100644 --- a/src/applications/project/controller/PhabricatorProjectCreateController.php +++ b/src/applications/project/controller/PhabricatorProjectCreateController.php @@ -49,7 +49,7 @@ final class PhabricatorProjectCreateController $xactions[] = $xaction; $editor = new PhabricatorProjectEditor($project); - $editor->setUser($user); + $editor->setActor($user); $editor->applyTransactions($xactions); } catch (PhabricatorProjectNameCollisionException $ex) { $e_name = 'Not Unique'; diff --git a/src/applications/project/controller/PhabricatorProjectMembersEditController.php b/src/applications/project/controller/PhabricatorProjectMembersEditController.php index f2dcae0482..3942755b0e 100644 --- a/src/applications/project/controller/PhabricatorProjectMembersEditController.php +++ b/src/applications/project/controller/PhabricatorProjectMembersEditController.php @@ -80,7 +80,7 @@ final class PhabricatorProjectMembersEditController if ($xactions) { $editor = new PhabricatorProjectEditor($project); - $editor->setUser($user); + $editor->setActor($user); $editor->applyTransactions($xactions); } diff --git a/src/applications/project/controller/PhabricatorProjectProfileEditController.php b/src/applications/project/controller/PhabricatorProjectProfileEditController.php index a288cd735d..e782c94b9b 100644 --- a/src/applications/project/controller/PhabricatorProjectProfileEditController.php +++ b/src/applications/project/controller/PhabricatorProjectProfileEditController.php @@ -92,7 +92,7 @@ final class PhabricatorProjectProfileEditController $xactions[] = $xaction; $editor = new PhabricatorProjectEditor($project); - $editor->setUser($user); + $editor->setActor($user); $editor->applyTransactions($xactions); } catch (PhabricatorProjectNameCollisionException $ex) { $e_name = 'Not Unique'; diff --git a/src/applications/project/editor/PhabricatorProjectEditor.php b/src/applications/project/editor/PhabricatorProjectEditor.php index 3282520aa4..edce444584 100644 --- a/src/applications/project/editor/PhabricatorProjectEditor.php +++ b/src/applications/project/editor/PhabricatorProjectEditor.php @@ -16,10 +16,9 @@ * limitations under the License. */ -final class PhabricatorProjectEditor { +final class PhabricatorProjectEditor extends PhabricatorEditor { private $project; - private $user; private $projectName; private $addEdges = array(); @@ -65,7 +64,7 @@ final class PhabricatorProjectEditor { $xaction->setNewValue($new_value); $editor = new PhabricatorProjectEditor($project); - $editor->setUser($user); + $editor->setActor($user); $editor->applyTransactions(array($xaction)); } @@ -74,24 +73,16 @@ final class PhabricatorProjectEditor { $this->project = $project; } - public function setUser(PhabricatorUser $user) { - $this->user = $user; - return $this; - } - public function applyTransactions(array $transactions) { assert_instances_of($transactions, 'PhabricatorProjectTransaction'); - if (!$this->user) { - throw new Exception('Call setUser() before save()!'); - } - $user = $this->user; + $actor = $this->requireActor(); $project = $this->project; $is_new = !$project->getID(); if ($is_new) { - $project->setAuthorPHID($user->getPHID()); + $project->setAuthorPHID($actor->getPHID()); } foreach ($transactions as $key => $xaction) { @@ -105,7 +96,7 @@ final class PhabricatorProjectEditor { if (!$is_new) { // You must be able to view a project in order to edit it in any capacity. PhabricatorPolicyFilter::requireCapability( - $user, + $actor, $project, PhabricatorPolicyCapability::CAN_VIEW); @@ -122,14 +113,14 @@ final class PhabricatorProjectEditor { if ($need_edit) { PhabricatorPolicyFilter::requireCapability( - $user, + $actor, $project, PhabricatorPolicyCapability::CAN_EDIT); } if ($need_join) { PhabricatorPolicyFilter::requireCapability( - $user, + $actor, $project, PhabricatorPolicyCapability::CAN_JOIN); } @@ -149,7 +140,7 @@ final class PhabricatorProjectEditor { $edge_type = PhabricatorEdgeConfig::TYPE_PROJ_MEMBER; $editor = new PhabricatorEdgeEditor(); - $editor->setUser($this->user); + $editor->setActor($actor); foreach ($this->remEdges as $phid) { $editor->removeEdge($project->getPHID(), $edge_type, $phid); } @@ -159,7 +150,7 @@ final class PhabricatorProjectEditor { $editor->save(); foreach ($transactions as $xaction) { - $xaction->setAuthorPHID($user->getPHID()); + $xaction->setAuthorPHID($actor->getPHID()); $xaction->setProjectID($project->getID()); $xaction->save(); } @@ -278,7 +269,7 @@ final class PhabricatorProjectEditor { // You can't edit away your ability to edit the project. PhabricatorPolicyFilter::mustRetainCapability( - $this->user, + $this->getActor(), $project, PhabricatorPolicyCapability::CAN_EDIT); break; @@ -369,7 +360,7 @@ final class PhabricatorProjectEditor { if (count($add) > 1) { return null; } else if (count($add) == 1) { - if (reset($add) != $this->user->getPHID()) { + if (reset($add) != $this->getActor()->getPHID()) { return null; } else { return 'join'; @@ -379,7 +370,7 @@ final class PhabricatorProjectEditor { if (count($rem) > 1) { return null; } else if (count($rem) == 1) { - if (reset($rem) != $this->user->getPHID()) { + if (reset($rem) != $this->getActor()->getPHID()) { return null; } else { return 'leave'; diff --git a/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php b/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php index 649a7c8361..7f45383c4e 100644 --- a/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php +++ b/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php @@ -114,7 +114,7 @@ final class PhabricatorProjectEditorTestCase extends PhabricatorTestCase { $xaction->setNewValue($new_name); $editor = new PhabricatorProjectEditor($proj); - $editor->setUser($user); + $editor->setActor($user); $editor->applyTransactions(array($xaction)); return true; diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index 2a4083776e..5f8e21f455 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -122,14 +122,16 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $committer_phid, $author_phid, $revision->getAuthorPHID()); + $actor = id(new PhabricatorUser()) + ->loadOneWhere('phid = %s', $actor_phid); $diff = $this->attachToRevision($revision, $actor_phid); $revision->setDateCommitted($commit->getEpoch()); $editor = new DifferentialCommentEditor( $revision, - $actor_phid, DifferentialAction::ACTION_CLOSE); + $editor->setActor($actor); $editor->setIsDaemonWorkflow(true); $vs_diff = $this->loadChangedByCommit($diff); diff --git a/src/applications/search/controller/PhabricatorSearchAttachController.php b/src/applications/search/controller/PhabricatorSearchAttachController.php index 1237c0d898..32b5b87ec2 100644 --- a/src/applications/search/controller/PhabricatorSearchAttachController.php +++ b/src/applications/search/controller/PhabricatorSearchAttachController.php @@ -78,7 +78,7 @@ final class PhabricatorSearchAttachController $rem_phids = array_diff($old_phids, $add_phids); $editor = id(new PhabricatorEdgeEditor()); - $editor->setUser($user); + $editor->setActor($user); foreach ($add_phids as $phid) { $editor->addEdge($this->phid, $edge_type, $phid); } @@ -159,6 +159,7 @@ final class PhabricatorSearchAttachController } $editor = new ManiphestTransactionEditor(); + $editor->setActor($user); $task_names = array(); diff --git a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php index bcfe19a368..78078e2079 100644 --- a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php +++ b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php @@ -74,7 +74,7 @@ final class PhabricatorSubscriptionsEditController } $editor = id(new PhabricatorSubscriptionsEditor()) - ->setUser($user) + ->setActor($user) ->setObject($object); if ($is_add) { diff --git a/src/applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php b/src/applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php index 8b01034fbf..37329f7940 100644 --- a/src/applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php +++ b/src/applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php @@ -16,10 +16,9 @@ * limitations under the License. */ -final class PhabricatorSubscriptionsEditor { +final class PhabricatorSubscriptionsEditor extends PhabricatorEditor { private $object; - private $user; private $explicitSubscribePHIDs = array(); private $implicitSubscribePHIDs = array(); @@ -30,12 +29,6 @@ final class PhabricatorSubscriptionsEditor { return $this; } - public function setUser(PhabricatorUser $user) { - $this->user = $user; - return $this; - } - - /** * Add explicit subscribers. These subscribers have explicitly subscribed * (or been subscribed) to the object, and will be added even if they @@ -81,9 +74,7 @@ final class PhabricatorSubscriptionsEditor { if (!$this->object) { throw new Exception('Call setObject() before save()!'); } - if (!$this->user) { - throw new Exception('Call setUser() before save()!'); - } + $actor = $this->requireActor(); $src = $this->object->getPHID(); @@ -109,7 +100,7 @@ final class PhabricatorSubscriptionsEditor { $s_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER; $editor = id(new PhabricatorEdgeEditor()) - ->setUser($this->user); + ->setActor($actor); foreach ($add as $phid => $ignored) { $editor->removeEdge($src, $u_type, $phid); diff --git a/src/infrastructure/PhabricatorEditor.php b/src/infrastructure/PhabricatorEditor.php new file mode 100644 index 0000000000..df6e901541 --- /dev/null +++ b/src/infrastructure/PhabricatorEditor.php @@ -0,0 +1,47 @@ +user = $user; + return $this; + } + final protected function getActor() { + return $this->user; + } + final protected function requireActor() { + $actor = $this->getActor(); + if (!$actor) { + throw new Exception('You must setActor()!'); + } + return $actor; + } + + final public function setExcludeMailRecipientPHIDs($phids) { + $this->excludeMailRecipientPHIDs = $phids; + return $this; + } + final protected function getExcludeMailRecipientPHIDs() { + return $this->excludeMailRecipientPHIDs; + } + +} diff --git a/src/infrastructure/edges/__tests__/PhabricatorEdgeTestCase.php b/src/infrastructure/edges/__tests__/PhabricatorEdgeTestCase.php index ac07e3aaeb..0327b29337 100644 --- a/src/infrastructure/edges/__tests__/PhabricatorEdgeTestCase.php +++ b/src/infrastructure/edges/__tests__/PhabricatorEdgeTestCase.php @@ -37,7 +37,7 @@ final class PhabricatorEdgeTestCase extends PhabricatorTestCase { $phid2 = $obj2->getPHID(); $editor = id(new PhabricatorEdgeEditor()) - ->setUser($user) + ->setActor($user) ->addEdge($phid1, PhabricatorEdgeConfig::TYPE_TEST_NO_CYCLE, $phid2) ->addEdge($phid2, PhabricatorEdgeConfig::TYPE_TEST_NO_CYCLE, $phid1); @@ -57,12 +57,12 @@ final class PhabricatorEdgeTestCase extends PhabricatorTestCase { // fail (it introduces a cycle). $editor = id(new PhabricatorEdgeEditor()) - ->setUser($user) + ->setActor($user) ->addEdge($phid1, PhabricatorEdgeConfig::TYPE_TEST_NO_CYCLE, $phid2) ->save(); $editor = id(new PhabricatorEdgeEditor()) - ->setUser($user) + ->setActor($user) ->addEdge($phid2, PhabricatorEdgeConfig::TYPE_TEST_NO_CYCLE, $phid1); $caught = null; diff --git a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php index 2287ca7edc..0428b4091b 100644 --- a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php +++ b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php @@ -28,26 +28,20 @@ * * id(new PhabricatorEdgeEditor()) * ->addEdge($src, $type, $dst) - * ->setUser($user) + * ->setActor($user) * ->save(); * * @task edit Editing Edges * @task cycles Cycle Prevention * @task internal Internals */ -final class PhabricatorEdgeEditor { +final class PhabricatorEdgeEditor extends PhabricatorEditor { private $addEdges = array(); private $remEdges = array(); private $openTransactions = array(); - private $user; private $suppressEvents; - public function setUser(PhabricatorUser $user) { - $this->user = $user; - return $this; - } - /* -( Editing Edges )------------------------------------------------------ */ @@ -398,7 +392,7 @@ final class PhabricatorEdgeEditor { 'add' => $this->addEdges, 'rem' => $this->remEdges, )); - $event->setUser($this->user); + $event->setUser($this->getActor()); PhutilEventEngine::dispatchEvent($event); }