diff --git a/src/applications/badges/storage/PhabricatorBadgesBadge.php b/src/applications/badges/storage/PhabricatorBadgesBadge.php --- a/src/applications/badges/storage/PhabricatorBadgesBadge.php +++ b/src/applications/badges/storage/PhabricatorBadgesBadge.php @@ -185,10 +185,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorTokenReceiverInterface )---------------------------------- */ diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -535,10 +535,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorTokenReceiverInterface )---------------------------------- */ diff --git a/src/applications/countdown/storage/PhabricatorCountdown.php b/src/applications/countdown/storage/PhabricatorCountdown.php --- a/src/applications/countdown/storage/PhabricatorCountdown.php +++ b/src/applications/countdown/storage/PhabricatorCountdown.php @@ -74,10 +74,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -485,10 +485,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorCustomFieldInterface )------------------------------------ */ diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -1352,10 +1352,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorTokenReceiverInterface )---------------------------------- */ diff --git a/src/applications/fund/storage/FundInitiative.php b/src/applications/fund/storage/FundInitiative.php --- a/src/applications/fund/storage/FundInitiative.php +++ b/src/applications/fund/storage/FundInitiative.php @@ -182,10 +182,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorTokenRecevierInterface )---------------------------------- */ diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php @@ -129,10 +129,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php --- a/src/applications/herald/storage/HeraldRule.php +++ b/src/applications/herald/storage/HeraldRule.php @@ -332,10 +332,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/legalpad/storage/LegalpadDocument.php b/src/applications/legalpad/storage/LegalpadDocument.php --- a/src/applications/legalpad/storage/LegalpadDocument.php +++ b/src/applications/legalpad/storage/LegalpadDocument.php @@ -167,10 +167,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/macro/storage/PhabricatorFileImageMacro.php b/src/applications/macro/storage/PhabricatorFileImageMacro.php --- a/src/applications/macro/storage/PhabricatorFileImageMacro.php +++ b/src/applications/macro/storage/PhabricatorFileImageMacro.php @@ -115,10 +115,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorTokenRecevierInterface )---------------------------------- */ diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -227,10 +227,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( Markup Interface )--------------------------------------------------- */ diff --git a/src/applications/passphrase/storage/PassphraseCredential.php b/src/applications/passphrase/storage/PassphraseCredential.php --- a/src/applications/passphrase/storage/PassphraseCredential.php +++ b/src/applications/passphrase/storage/PassphraseCredential.php @@ -161,10 +161,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/paste/storage/PhabricatorPaste.php b/src/applications/paste/storage/PhabricatorPaste.php --- a/src/applications/paste/storage/PhabricatorPaste.php +++ b/src/applications/paste/storage/PhabricatorPaste.php @@ -159,10 +159,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorTokenReceiverInterface )---------------------------------- */ diff --git a/src/applications/phame/storage/PhameBlog.php b/src/applications/phame/storage/PhameBlog.php --- a/src/applications/phame/storage/PhameBlog.php +++ b/src/applications/phame/storage/PhameBlog.php @@ -340,10 +340,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorConduitResultInterface )---------------------------------- */ diff --git a/src/applications/phame/storage/PhamePost.php b/src/applications/phame/storage/PhamePost.php --- a/src/applications/phame/storage/PhamePost.php +++ b/src/applications/phame/storage/PhamePost.php @@ -286,10 +286,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorConduitResultInterface )---------------------------------- */ diff --git a/src/applications/pholio/storage/PholioMock.php b/src/applications/pholio/storage/PholioMock.php --- a/src/applications/pholio/storage/PholioMock.php +++ b/src/applications/pholio/storage/PholioMock.php @@ -188,10 +188,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorPolicyInterface Implementation )-------------------------- */ diff --git a/src/applications/phriction/storage/PhrictionDocument.php b/src/applications/phriction/storage/PhrictionDocument.php --- a/src/applications/phriction/storage/PhrictionDocument.php +++ b/src/applications/phriction/storage/PhrictionDocument.php @@ -198,10 +198,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/phurl/storage/PhabricatorPhurlURL.php b/src/applications/phurl/storage/PhabricatorPhurlURL.php --- a/src/applications/phurl/storage/PhabricatorPhurlURL.php +++ b/src/applications/phurl/storage/PhabricatorPhurlURL.php @@ -173,10 +173,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorTokenReceiverInterface )---------------------------------- */ diff --git a/src/applications/ponder/storage/PonderAnswer.php b/src/applications/ponder/storage/PonderAnswer.php --- a/src/applications/ponder/storage/PonderAnswer.php +++ b/src/applications/ponder/storage/PonderAnswer.php @@ -253,10 +253,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/ponder/storage/PonderQuestion.php b/src/applications/ponder/storage/PonderQuestion.php --- a/src/applications/ponder/storage/PonderQuestion.php +++ b/src/applications/ponder/storage/PonderQuestion.php @@ -252,10 +252,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorTokenReceiverInterface )---------------------------------- */ diff --git a/src/applications/project/controller/PhabricatorProjectMembersViewController.php b/src/applications/project/controller/PhabricatorProjectMembersViewController.php --- a/src/applications/project/controller/PhabricatorProjectMembersViewController.php +++ b/src/applications/project/controller/PhabricatorProjectMembersViewController.php @@ -152,22 +152,22 @@ ->setDisabled(!$can_leave) ->setWorkflow(true) ->setName(pht('Leave Project'))); + } - if (!$project->isUserWatcher($viewer->getPHID())) { - $view->addAction( - id(new PhabricatorActionView()) - ->setWorkflow(true) - ->setHref('/project/watch/'.$project->getID().'/') - ->setIcon('fa-eye') - ->setName(pht('Watch Project'))); - } else { - $view->addAction( - id(new PhabricatorActionView()) - ->setWorkflow(true) - ->setHref('/project/unwatch/'.$project->getID().'/') - ->setIcon('fa-eye-slash') - ->setName(pht('Unwatch Project'))); - } + if (!$project->isUserWatcher($viewer->getPHID())) { + $view->addAction( + id(new PhabricatorActionView()) + ->setWorkflow(true) + ->setHref('/project/watch/'.$project->getID().'/') + ->setIcon('fa-eye') + ->setName(pht('Watch Project'))); + } else { + $view->addAction( + id(new PhabricatorActionView()) + ->setWorkflow(true) + ->setHref('/project/unwatch/'.$project->getID().'/') + ->setIcon('fa-eye-slash') + ->setName(pht('Unwatch Project'))); } $can_add = $can_edit && $supports_edit; diff --git a/src/applications/project/controller/PhabricatorProjectProfileController.php b/src/applications/project/controller/PhabricatorProjectProfileController.php --- a/src/applications/project/controller/PhabricatorProjectProfileController.php +++ b/src/applications/project/controller/PhabricatorProjectProfileController.php @@ -52,6 +52,7 @@ $nav = $this->getProfileMenu(); $nav->selectFilter(PhabricatorProject::PANEL_PROFILE); + $watch_action = $this->renderWatchAction($project); $stories = id(new PhabricatorFeedQuery()) ->setViewer($viewer) @@ -62,10 +63,15 @@ ->setLimit(50) ->execute(); + $feed = $this->renderStories($stories); + $feed_header = id(new PHUIHeaderView()) + ->setHeader(pht('Recent Activity')) + ->addActionLink($watch_action); + $feed = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Recent Activity')) + ->setHeader($feed_header) ->appendChild($feed); $columns = id(new AphrontMultiColumnView()) @@ -144,4 +150,33 @@ return phutil_tag_div('profile-feed', $view->render()); } + private function renderWatchAction(PhabricatorProject $project) { + $viewer = $this->getViewer(); + $viewer_phid = $viewer->getPHID(); + $id = $project->getID(); + + $is_watcher = ($viewer_phid && $project->isUserWatcher($viewer_phid)); + + if (!$is_watcher) { + $watch_icon = 'fa-eye'; + $watch_text = pht('Watch Project'); + $watch_href = "/project/watch/{$id}/?via=profile"; + } else { + $watch_icon = 'fa-eye-slash'; + $watch_text = pht('Unwatch Project'); + $watch_href = "/project/unwatch/{$id}/?via=profile"; + } + + $watch_icon = id(new PHUIIconView()) + ->setIconFont($watch_icon); + + return id(new PHUIButtonView()) + ->setTag('a') + ->setWorkflow(true) + ->setIcon($watch_icon) + ->setText($watch_text) + ->setHref($watch_href); + } + + } diff --git a/src/applications/project/controller/PhabricatorProjectWatchController.php b/src/applications/project/controller/PhabricatorProjectWatchController.php --- a/src/applications/project/controller/PhabricatorProjectWatchController.php +++ b/src/applications/project/controller/PhabricatorProjectWatchController.php @@ -18,11 +18,11 @@ return new Aphront404Response(); } - $done_uri = "/project/members/{$id}/"; - - // You must be a member of a project to watch it. - if (!$project->isUserMember($viewer->getPHID())) { - return new Aphront400Response(); + $via = $request->getStr('via'); + if ($via == 'profile') { + $done_uri = $project->getURI(); + } else { + $done_uri = "/project/members/{$id}/"; } if ($request->isDialogFormPost()) { @@ -38,7 +38,7 @@ break; } - $type_member = PhabricatorObjectHasWatcherEdgeType::EDGECONST; + $type_watcher = PhabricatorObjectHasWatcherEdgeType::EDGECONST; $member_spec = array( $edge_action => array($viewer->getPHID() => $viewer->getPHID()), ); @@ -46,7 +46,7 @@ $xactions = array(); $xactions[] = id(new PhabricatorProjectTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) - ->setMetadataValue('edge:type', $type_member) + ->setMetadataValue('edge:type', $type_watcher) ->setNewValue($member_spec); $editor = id(new PhabricatorProjectTransactionEditor($project)) @@ -82,6 +82,7 @@ return $this->newDialog() ->setTitle($title) + ->addHiddenInput('via', $via) ->appendParagraph($body) ->addCancelButton($done_uri) ->addSubmitButton($submit); diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -188,21 +188,18 @@ switch ($edge_type) { case PhabricatorProjectProjectHasMemberEdgeType::EDGECONST: case PhabricatorObjectHasWatcherEdgeType::EDGECONST: + $edge_const = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST; + if ($edge_type != $edge_const) { + break; + } + $old = $xaction->getOldValue(); $new = $xaction->getNewValue(); - // When adding members or watchers, we add subscriptions. + // When adding members, we add subscriptions. When removing + // members, we remove subscriptions. $add = array_keys(array_diff_key($new, $old)); - - // When removing members, we remove their subscription too. - // When unwatching, we leave subscriptions, since it's fine to be - // subscribed to a project but not be a member of it. - $edge_const = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST; - if ($edge_type == $edge_const) { - $rem = array_keys(array_diff_key($old, $new)); - } else { - $rem = array(); - } + $rem = array_keys(array_diff_key($old, $new)); // NOTE: The subscribe is "explicit" because there's no implicit // unsubscribe, so Join -> Leave -> Join doesn't resubscribe you @@ -212,27 +209,12 @@ // this, which is a fairly weird edge case and pretty arguable both // ways. - // Subscriptions caused by watches should also clearly be explicit, - // and that case is unambiguous. - id(new PhabricatorSubscriptionsEditor()) ->setActor($this->requireActor()) ->setObject($object) ->subscribeExplicit($add) ->unsubscribe($rem) ->save(); - - if ($rem) { - // When removing members, also remove any watches on the project. - $edge_editor = new PhabricatorEdgeEditor(); - foreach ($rem as $rem_phid) { - $edge_editor->removeEdge( - $object->getPHID(), - PhabricatorObjectHasWatcherEdgeType::EDGECONST, - $rem_phid); - } - $edge_editor->save(); - } break; } break; diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php --- a/src/applications/project/storage/PhabricatorProject.php +++ b/src/applications/project/storage/PhabricatorProject.php @@ -548,11 +548,6 @@ return false; } - public function shouldAllowSubscription($phid) { - return $this->isUserMember($phid) && - !$this->isUserWatcher($phid); - } - /* -( PhabricatorCustomFieldInterface )------------------------------------ */ diff --git a/src/applications/project/view/PhabricatorProjectUserListView.php b/src/applications/project/view/PhabricatorProjectUserListView.php --- a/src/applications/project/view/PhabricatorProjectUserListView.php +++ b/src/applications/project/view/PhabricatorProjectUserListView.php @@ -79,7 +79,7 @@ ->setHref($handle->getURI()) ->setImageURI($handle->getImageURI()); - if ($can_edit) { + if ($can_edit && !$limit) { $remove_uri = $this->getRemoveURI($user_phid); $item->addAction( @@ -94,16 +94,32 @@ } if ($user_phids) { - $header = pht( + $header_text = pht( '%s (%s)', $this->getHeaderText(), phutil_count($user_phids)); } else { - $header = $this->getHeaderText(); + $header_text = $this->getHeaderText(); + } + + $id = $project->getID(); + + $header = id(new PHUIHeaderView()) + ->setHeader($header_text); + + if ($limit) { + $header->addActionLink( + id(new PHUIButtonView()) + ->setTag('a') + ->setIcon( + id(new PHUIIconView()) + ->setIconFont('fa-list-ul')) + ->setText(pht('View All')) + ->setHref("/project/members/{$id}/")); } return id(new PHUIObjectBoxView()) - ->setHeaderText($header) + ->setHeader($header) ->setObjectList($list); } 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 @@ -443,10 +443,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/slowvote/storage/PhabricatorSlowvotePoll.php b/src/applications/slowvote/storage/PhabricatorSlowvotePoll.php --- a/src/applications/slowvote/storage/PhabricatorSlowvotePoll.php +++ b/src/applications/slowvote/storage/PhabricatorSlowvotePoll.php @@ -183,10 +183,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorTokenReceiverInterface )---------------------------------- */ diff --git a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php --- a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php +++ b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php @@ -61,13 +61,6 @@ $handle->getURI()); } - if (!$object->shouldAllowSubscription($viewer->getPHID())) { - return $this->buildErrorResponse( - pht('You Can Not Subscribe'), - pht('You can not subscribe to this object.'), - $handle->getURI()); - } - if ($object instanceof PhabricatorApplicationTransactionInterface) { if ($is_add) { $xaction_value = array( diff --git a/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php b/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php --- a/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php +++ b/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php @@ -34,11 +34,6 @@ return; } - if (!$object->shouldAllowSubscription($user_phid)) { - // This object doesn't allow the viewer to subscribe. - return; - } - if ($user_phid && $object->isAutomaticallySubscribed($user_phid)) { $sub_action = id(new PhabricatorActionView()) ->setWorkflow(true) diff --git a/src/applications/subscriptions/interface/PhabricatorSubscribableInterface.php b/src/applications/subscriptions/interface/PhabricatorSubscribableInterface.php --- a/src/applications/subscriptions/interface/PhabricatorSubscribableInterface.php +++ b/src/applications/subscriptions/interface/PhabricatorSubscribableInterface.php @@ -22,17 +22,6 @@ */ public function shouldShowSubscribersProperty(); - - /** - * Return `true` to indicate that the "Subscribe" action should be shown and - * enabled when rendering action lists for this object, or `false` to omit - * the action. - * - * @param phid Viewing or acting user PHID. - * @return bool True to allow the user to subscribe. - */ - public function shouldAllowSubscription($phid); - } // TEMPLATE IMPLEMENTATION ///////////////////////////////////////////////////// @@ -48,8 +37,4 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - */ diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJob.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJob.php --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJob.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJob.php @@ -222,10 +222,6 @@ return true; } - public function shouldAllowSubscription($phid) { - return true; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */