From 7f67bd8aff022c6e3dc52555409510281eeef688 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 1 Jul 2015 14:54:43 -0700 Subject: [PATCH 1/2] Fix some pht-strings Summary: This correct some bad strings for translations Test Plan: merge similar stings removes some word wraps Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T8700 Differential Revision: https://secure.phabricator.com/D13464 --- ...abricatorCalendarEventCancelController.php | 20 +++++++++---------- .../PhabricatorCalendarEventTransaction.php | 6 +++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php index c29541bcfd..cb87cd7373 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php @@ -81,14 +81,14 @@ final class PhabricatorCalendarEventCancelController if ($is_cancelled) { if ($sequence || $is_parent_cancelled) { $title = pht('Cannot Reinstate Instance'); - $paragraph = pht('Cannot reinstate an instance of a - cancelled recurring event.'); + $paragraph = pht('Cannot reinstate an instance of a ' . + 'cancelled recurring event.'); $cancel = pht('Cancel'); $submit = null; } else if ($is_parent) { $title = pht('Reinstate Recurrence'); - $paragraph = pht('Reinstate the entire series - of recurring events?'); + $paragraph = pht('Reinstate the entire series ' . + 'of recurring events?'); $cancel = pht('Don\'t Reinstate Recurrence'); $submit = pht('Reinstate Recurrence'); } else { @@ -100,20 +100,20 @@ final class PhabricatorCalendarEventCancelController } else { if ($sequence) { $title = pht('Cancel Instance'); - $paragraph = pht('Cancel just this instance - of a recurring event.'); + $paragraph = pht('Cancel just this instance ' . + 'of a recurring event.'); $cancel = pht('Don\'t Cancel Instance'); $submit = pht('Cancel Instance'); } else if ($is_parent) { $title = pht('Cancel Recurrence'); - $paragraph = pht('Cancel the entire series - of recurring events?'); + $paragraph = pht('Cancel the entire series ' . + 'of recurring events?'); $cancel = pht('Don\'t Cancel Recurrence'); $submit = pht('Cancel Recurrence'); } else { $title = pht('Cancel Event'); - $paragraph = pht('You can always reinstate - the event later.'); + $paragraph = pht('You can always reinstate ' . + 'the event later.'); $cancel = pht('Don\'t Cancel Event'); $submit = pht('Cancel Event'); } diff --git a/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php b/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php index 65ef68e573..c6ee1c4c37 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php @@ -238,7 +238,7 @@ final class PhabricatorCalendarEventTransaction } else if ($count_added > 0 && $count_uninvited > 0) { $added_text = $this->renderHandleList($added); $uninvited_text = $this->renderHandleList($uninvited); - $text = pht('%s invited %s and uninvited: %s', + $text = pht('%s invited %s and uninvited %s.', $this->renderHandleLink($author_phid), $added_text, $uninvited_text); @@ -303,7 +303,7 @@ final class PhabricatorCalendarEventTransaction case self::TYPE_NAME: if ($old === null) { return pht( - '%s created %s', + '%s created %s.', $this->renderHandleLink($author_phid), $this->renderHandleLink($object_phid)); } else { @@ -445,7 +445,7 @@ final class PhabricatorCalendarEventTransaction } else if ($count_added > 0 && $count_uninvited > 0) { $added_text = $this->renderHandleList($added); $uninvited_text = $this->renderHandleList($uninvited); - $text = pht('%s invited %s and uninvited %s to %s', + $text = pht('%s invited %s and uninvited %s to %s.', $this->renderHandleLink($author_phid), $added_text, $uninvited_text, From 6a996d76fd1a1a144177b1fb9e10c816f0e8e98e Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 1 Jul 2015 15:06:55 -0700 Subject: [PATCH 2/2] Remove "feed.public" and sort out feed policies Summary: Ref T6817. Fixes T8731. On the old `secure` host, `feed.public` was set to `true`. I didn't bring the option over, which caused the secondary issue in T8731. Specifically, when `feed.public` is off, a logged-out user looking at feed can't see //any// stories, so they query all of feed until they hit the time limit. To fix this immediately, just use the most open policy, which is basically equivalent but always correct. To fix this more thoroughly: - Remove `feed.public`, which violates policies and has been slated for removal for a while (see T6817). - Clean up policy handling. Test Plan: - As a logged-out user, viewed feed on a public install with `feed.public` off; no longer saw all stories get queried + no feed shown. - Grepped for `feed.public`. Reviewers: btrahan Reviewed By: btrahan Subscribers: chad, epriestley Maniphest Tasks: T6817, T8731 Differential Revision: https://secure.phabricator.com/D13518 --- src/__phutil_library_map__.php | 2 - .../PhabricatorExtraConfigSetupCheck.php | 2 + .../PhabricatorFeedApplication.php | 1 - .../feed/builder/PhabricatorFeedBuilder.php | 7 -- .../config/PhabricatorFeedConfigOptions.php | 18 ----- .../PhabricatorFeedPublicStreamController.php | 39 ----------- .../feed/story/PhabricatorFeedStory.php | 65 ++----------------- 7 files changed, 8 insertions(+), 126 deletions(-) delete mode 100644 src/applications/feed/controller/PhabricatorFeedPublicStreamController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 05716a9e93..e4c1d337e6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1885,7 +1885,6 @@ phutil_register_library_map(array( 'PhabricatorFeedListController' => 'applications/feed/controller/PhabricatorFeedListController.php', 'PhabricatorFeedManagementRepublishWorkflow' => 'applications/feed/management/PhabricatorFeedManagementRepublishWorkflow.php', 'PhabricatorFeedManagementWorkflow' => 'applications/feed/management/PhabricatorFeedManagementWorkflow.php', - 'PhabricatorFeedPublicStreamController' => 'applications/feed/controller/PhabricatorFeedPublicStreamController.php', 'PhabricatorFeedQuery' => 'applications/feed/query/PhabricatorFeedQuery.php', 'PhabricatorFeedSearchEngine' => 'applications/feed/query/PhabricatorFeedSearchEngine.php', 'PhabricatorFeedStory' => 'applications/feed/story/PhabricatorFeedStory.php', @@ -5520,7 +5519,6 @@ phutil_register_library_map(array( 'PhabricatorFeedListController' => 'PhabricatorFeedController', 'PhabricatorFeedManagementRepublishWorkflow' => 'PhabricatorFeedManagementWorkflow', 'PhabricatorFeedManagementWorkflow' => 'PhabricatorManagementWorkflow', - 'PhabricatorFeedPublicStreamController' => 'PhabricatorFeedController', 'PhabricatorFeedQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorFeedSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorFeedStory' => array( diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index 9301329394..a482f4d073 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -274,6 +274,8 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'security.allow-conduit-act-as-user' => pht( 'Impersonating users over the API is no longer supported.'), + + 'feed.public' => pht('The framable public feed is no longer supported.'), ); return $ancient_config; diff --git a/src/applications/feed/application/PhabricatorFeedApplication.php b/src/applications/feed/application/PhabricatorFeedApplication.php index 6f2dbea50a..ac8c21cda2 100644 --- a/src/applications/feed/application/PhabricatorFeedApplication.php +++ b/src/applications/feed/application/PhabricatorFeedApplication.php @@ -25,7 +25,6 @@ final class PhabricatorFeedApplication extends PhabricatorApplication { public function getRoutes() { return array( '/feed/' => array( - 'public/' => 'PhabricatorFeedPublicStreamController', '(?P\d+)/' => 'PhabricatorFeedDetailController', '(?:query/(?P[^/]+)/)?' => 'PhabricatorFeedListController', ), diff --git a/src/applications/feed/builder/PhabricatorFeedBuilder.php b/src/applications/feed/builder/PhabricatorFeedBuilder.php index 898f2aa6f7..7e5b65786a 100644 --- a/src/applications/feed/builder/PhabricatorFeedBuilder.php +++ b/src/applications/feed/builder/PhabricatorFeedBuilder.php @@ -4,7 +4,6 @@ final class PhabricatorFeedBuilder extends Phobject { private $user; private $stories; - private $framed; private $hovercards = false; private $noDataString; @@ -13,11 +12,6 @@ final class PhabricatorFeedBuilder extends Phobject { $this->stories = $stories; } - public function setFramed($framed) { - $this->framed = $framed; - return $this; - } - public function setUser(PhabricatorUser $user) { $this->user = $user; return $this; @@ -47,7 +41,6 @@ final class PhabricatorFeedBuilder extends Phobject { $last_date = null; foreach ($stories as $story) { - $story->setFramed($this->framed); $story->setHovercard($this->hovercards); $date = ucfirst(phabricator_relative_date($story->getEpoch(), $user)); diff --git a/src/applications/feed/config/PhabricatorFeedConfigOptions.php b/src/applications/feed/config/PhabricatorFeedConfigOptions.php index a3f7bf522f..5e6926f357 100644 --- a/src/applications/feed/config/PhabricatorFeedConfigOptions.php +++ b/src/applications/feed/config/PhabricatorFeedConfigOptions.php @@ -21,24 +21,6 @@ final class PhabricatorFeedConfigOptions public function getOptions() { return array( - $this->newOption('feed.public', 'bool', false) - ->setLocked(true) - ->setBoolOptions( - array( - pht('Allow anyone to view the feed'), - pht('Require authentication'), - )) - ->setSummary(pht('Should the feed be public?')) - ->setDescription( - pht( - "If you set this to true, you can embed Phabricator activity ". - "feeds in other pages using iframes. These feeds are completely ". - "public, and a login is not required to view them! This is ". - "intended for things like open source projects that want to ". - "expose an activity feed on the project homepage.\n\n". - "NOTE: You must also set `%s` to true for this ". - "setting to work properly.", - 'policy.allow-public')), $this->newOption('feed.http-hooks', 'list', array()) ->setLocked(true) ->setSummary(pht('POST notifications of feed events.')) diff --git a/src/applications/feed/controller/PhabricatorFeedPublicStreamController.php b/src/applications/feed/controller/PhabricatorFeedPublicStreamController.php deleted file mode 100644 index b43e79450f..0000000000 --- a/src/applications/feed/controller/PhabricatorFeedPublicStreamController.php +++ /dev/null @@ -1,39 +0,0 @@ -getRequest(); - $viewer = PhabricatorUser::getOmnipotentUser(); - - $query = new PhabricatorFeedQuery(); - $query->setViewer($viewer); - $query->setLimit(100); - $stories = $query->execute(); - - $builder = new PhabricatorFeedBuilder($stories); - $builder - ->setFramed(true) - ->setUser($viewer); - - $view = phutil_tag_div( - 'phabricator-public-feed-frame', - $builder->buildView()); - - return $this->buildStandardPageResponse( - $view, - array( - 'title' => pht('Public Feed'), - 'public' => true, - )); - } -} diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index d91690a570..d586104e70 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -16,7 +16,6 @@ abstract class PhabricatorFeedStory private $data; private $hasViewed; - private $framed; private $hovercard = false; private $renderingTarget = PhabricatorApplicationTransaction::TARGET_HTML; @@ -289,11 +288,6 @@ abstract class PhabricatorFeedStory return $this->hasViewed; } - final public function setFramed($framed) { - $this->framed = $framed; - return $this; - } - final public function setHandles(array $handles) { assert_instances_of($handles, 'PhabricatorObjectHandle'); $this->handles = $handles; @@ -367,24 +361,7 @@ abstract class PhabricatorFeedStory return $handle->getLinkName(); } - // NOTE: We render our own link here to customize the styling and add - // the '_top' target for framed feeds. - - $class = null; - if ($handle->getType() == PhabricatorPeopleUserPHIDType::TYPECONST) { - $class = 'phui-link-person'; - } - - return javelin_tag( - 'a', - array( - 'href' => $handle->getURI(), - 'target' => $this->framed ? '_top' : null, - 'sigil' => $this->hovercard ? 'hovercard' : null, - 'meta' => $this->hovercard ? array('hoverPHID' => $phid) : null, - 'class' => $class, - ), - $handle->getLinkName()); + return $handle->renderLink(); } final protected function renderString($str) { @@ -462,16 +439,10 @@ abstract class PhabricatorFeedStory * @task policy */ public function getPolicy($capability) { - $policy_object = $this->getPrimaryPolicyObject(); - if ($policy_object) { - return $policy_object->getPolicy($capability); - } - - // TODO: Remove this once all objects are policy-aware. For now, keep - // respecting the `feed.public` setting. - return PhabricatorEnv::getEnvConfig('feed.public') - ? PhabricatorPolicies::POLICY_PUBLIC - : PhabricatorPolicies::POLICY_USER; + // NOTE: We enforce that a user can see all the objects a story is about + // when loading it, so we don't need to perform a equivalent secondary + // policy check later. + return PhabricatorPolicies::getMostOpenPolicy(); } @@ -479,39 +450,15 @@ abstract class PhabricatorFeedStory * @task policy */ public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - $policy_object = $this->getPrimaryPolicyObject(); - if ($policy_object) { - return $policy_object->hasAutomaticCapability($capability, $viewer); - } - return false; } + public function describeAutomaticCapability($capability) { return null; } - /** - * Get the policy object this story is about, if such a policy object - * exists. - * - * @return PhabricatorPolicyInterface|null Policy object, if available. - * @task policy - */ - private function getPrimaryPolicyObject() { - $primary_phid = $this->getPrimaryObjectPHID(); - if (empty($this->objects[$primary_phid])) { - $object = $this->objects[$primary_phid]; - if ($object instanceof PhabricatorPolicyInterface) { - return $object; - } - } - - return null; - } - - /* -( PhabricatorMarkupInterface Implementation )--------------------------- */