diff --git a/resources/sql/autopatches/20140731.audit.1.subscribers.php b/resources/sql/autopatches/20140731.audit.1.subscribers.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140731.audit.1.subscribers.php @@ -0,0 +1,30 @@ +establishConnection('w'); + +echo "Migrating Audit subscribers to subscriptions...\n"; +foreach (new LiskMigrationIterator($table) as $request) { + $id = $request->getID(); + + echo "Migrating auditor {$id}...\n"; + + if ($request->getAuditStatus() != 'cc') { + // This isn't a "subscriber", so skip it. + continue; + } + + queryfx( + $conn_w, + 'INSERT IGNORE INTO %T (src, type, dst) VALUES (%s, %d, %s)', + PhabricatorEdgeConfig::TABLE_NAME_EDGE, + $request->getCommitPHID(), + PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER, + $request->getAuditorPHID()); + + + // Wipe the row. + $request->delete(); +} + +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4847,6 +4847,7 @@ 'PhabricatorPolicyInterface', 'PhabricatorFlaggableInterface', 'PhabricatorTokenReceiverInterface', + 'PhabricatorSubscribableInterface', 'HarbormasterBuildableInterface', 'PhabricatorCustomFieldInterface', ), diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -93,6 +93,8 @@ } } + $add_self_cc = false; + if ($action == PhabricatorAuditActionConstants::CLOSE) { if (!PhabricatorEnv::getEnvConfig('audit.can-author-close-audit')) { throw new Exception('Cannot Close Audit without enabling'. @@ -177,9 +179,9 @@ $new_status = null; switch ($action) { case PhabricatorAuditActionConstants::COMMENT: - case PhabricatorAuditActionConstants::ADD_CCS: case PhabricatorAuditActionConstants::ADD_AUDITORS: - $new_status = PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED; + case PhabricatorAuditActionConstants::ADD_CCS: + $add_self_cc = true; break; case PhabricatorAuditActionConstants::ACCEPT: $new_status = PhabricatorAuditStatusConstants::ACCEPTED; @@ -193,18 +195,25 @@ throw new Exception("Unknown or invalid action '{$action}'!"); } - $request = id(new PhabricatorRepositoryAuditRequest()) - ->setCommitPHID($commit->getPHID()) - ->setAuditorPHID($actor->getPHID()) - ->setAuditStatus($new_status) - ->setAuditReasons(array('Voluntary Participant')) - ->save(); - $requests[] = $request; + if ($new_status !== null) { + $request = id(new PhabricatorRepositoryAuditRequest()) + ->setCommitPHID($commit->getPHID()) + ->setAuditorPHID($actor->getPHID()) + ->setAuditStatus($new_status) + ->setAuditReasons(array('Voluntary Participant')) + ->save(); + $requests[] = $request; + } } } $auditors = array(); $ccs = array(); + + if ($add_self_cc) { + $ccs[] = $actor->getPHID(); + } + foreach ($comments as $comment) { $meta = $comment->getMetadata(); @@ -244,22 +253,17 @@ } } - if ($ccs) { - foreach ($ccs as $cc_phid) { - $audit_cc = PhabricatorAuditStatusConstants::CC; - $requests[] = id (new PhabricatorRepositoryAuditRequest()) - ->setCommitPHID($commit->getPHID()) - ->setAuditorPHID($cc_phid) - ->setAuditStatus($audit_cc) - ->setAuditReasons( - array('Added by '.$actor->getUsername())) - ->save(); - } - } - $commit->updateAuditStatus($requests); $commit->save(); + if ($ccs) { + id(new PhabricatorSubscriptionsEditor()) + ->setActor($actor) + ->setObject($commit) + ->subscribeExplicit($ccs) + ->save(); + } + $content_source = PhabricatorContentSource::newForSource( PhabricatorContentSource::SOURCE_LEGACY, array()); @@ -288,15 +292,14 @@ foreach ($requests as $request) { $status = $request->getAuditStatus(); switch ($status) { - case PhabricatorAuditStatusConstants::RESIGNED: - case PhabricatorAuditStatusConstants::NONE: - case PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED: - case PhabricatorAuditStatusConstants::CC: - $feed_dont_publish_phids[$request->getAuditorPHID()] = 1; - break; - default: - unset($feed_dont_publish_phids[$request->getAuditorPHID()]); - break; + case PhabricatorAuditStatusConstants::RESIGNED: + case PhabricatorAuditStatusConstants::NONE: + case PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED: + $feed_dont_publish_phids[$request->getAuditorPHID()] = 1; + break; + default: + unset($feed_dont_publish_phids[$request->getAuditorPHID()]); + break; } } $feed_dont_publish_phids = array_keys($feed_dont_publish_phids); @@ -452,9 +455,14 @@ $email_cc[$other_comment->getActorPHID()] = true; } + $subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID( + $commit->getPHID()); + foreach ($subscribers as $subscriber) { + $email_cc[$subscriber] = true; + } + foreach ($requests as $request) { switch ($request->getAuditStatus()) { - case PhabricatorAuditStatusConstants::CC: case PhabricatorAuditStatusConstants::AUDIT_REQUIRED: $email_cc[$request->getAuditorPHID()] = true; break; diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -1100,12 +1100,6 @@ 'blue', pht('Closed')); break; - case PhabricatorAuditStatusConstants::CC: - $item->setIcon( - PHUIStatusItemView::ICON_INFO, - 'dark', - pht('Subscribed')); - break; default: $item->setIcon( PHUIStatusItemView::ICON_QUESTION, diff --git a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php --- a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php +++ b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php @@ -69,10 +69,6 @@ foreach ($requests as $request) { $status = $request->getAuditStatus(); - if ($status == PhabricatorAuditStatusConstants::CC) { - // We handle these specially below. - continue; - } $object = idx($objects, $request->getAuditorPHID()); if (!$object) { @@ -133,13 +129,8 @@ } public function getCCUserPHIDs($object) { - $ccs = array(); - foreach ($this->getAuditRequests() as $request) { - if ($request->getAuditStatus() == PhabricatorAuditStatusConstants::CC) { - $ccs[] = $request->getAuditorPHID(); - } - } - return $ccs; + return PhabricatorSubscribersQuery::loadSubscribersForPHID( + $object->getPHID()); } public function getObjectTitle($object) { diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -6,6 +6,7 @@ PhabricatorPolicyInterface, PhabricatorFlaggableInterface, PhabricatorTokenReceiverInterface, + PhabricatorSubscribableInterface, HarbormasterBuildableInterface, PhabricatorCustomFieldInterface { @@ -330,4 +331,24 @@ return $this; } + +/* -( PhabricatorSubscribableInterface )----------------------------------- */ + + + public function isAutomaticallySubscribed($phid) { + + // TODO: This should also list auditors, but handling that is a bit messy + // right now because we are not guaranteed to have the data. + + return ($phid == $this->getAuthorPHID()); + } + + public function shouldShowSubscribersProperty() { + return true; + } + + public function shouldAllowSubscription($phid) { + return true; + } + } diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php --- a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php @@ -244,7 +244,6 @@ $maps = array( PhabricatorAuditStatusConstants::AUDIT_REQUIRED => $map, - PhabricatorAuditStatusConstants::CC => $ccmap, ); foreach ($maps as $status => $map) { @@ -281,6 +280,14 @@ $commit->updateAuditStatus($requests); $commit->save(); + + if ($ccmap) { + id(new PhabricatorSubscriptionsEditor()) + ->setActor(PhabricatorUser::getOmnipotentUser()) + ->setObject($commit) + ->subscribeExplicit(array_keys($ccmap)) + ->save(); + } }