From bba53205de4a568ba7a41a6194a5dc690debd558 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 13 Jun 2016 05:22:00 -0700 Subject: [PATCH 01/42] Remove all uses of PhutilGitURI in Phabricator Summary: Ref T11137. This class is removed in D16099. Depends on D16099. `PhutilURI` now attempts to "just work" with Git-style URIs, so at least in theory we can just delete all of this code and pretend it does not exist. (I've left "Display URI" and "Effective URI" as distinct, at least for now, because I think the distinction may be relevant in the future even though it isn't right now, and to keep this diff small, although I may go remove one after I think about this for a bit.) Test Plan: - Created a new Git repository with a Git URI. - Pulled/updated it, which now works correctly and should resolve the original issue in T11137. - Verified that daemons now align the origin to a Git-style URI with a relative path, which should resolve the original issue in T11004. - Grepped for `PhutilGitURI`. - Also grepped in `arcanist/`, but found no matches, so no patch for that. - Checked display/conduit URIs. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11137 Differential Revision: https://secure.phabricator.com/D16100 --- ...rmasterCircleCIBuildStepImplementation.php | 5 -- .../PhabricatorRepositoryURINormalizer.php | 20 +----- .../storage/PhabricatorRepository.php | 45 +++---------- .../storage/PhabricatorRepositoryURI.php | 63 ++++++------------- 4 files changed, 29 insertions(+), 104 deletions(-) diff --git a/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php index 365cf657de..e74e25a395 100644 --- a/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php @@ -69,11 +69,6 @@ EOTEXT $uri_object = new PhutilURI($uri); $domain = $uri_object->getDomain(); - if (!strlen($domain)) { - $uri_object = new PhutilGitURI($uri); - $domain = $uri_object->getDomain(); - } - $domain = phutil_utf8_strtolower($domain); switch ($domain) { case 'github.com': diff --git a/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php b/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php index 42224c9553..27e93535ab 100644 --- a/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php +++ b/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php @@ -78,16 +78,7 @@ final class PhabricatorRepositoryURINormalizer extends Phobject { switch ($this->type) { case self::TYPE_GIT: $uri = new PhutilURI($this->uri); - if ($uri->getProtocol()) { - return $uri->getPath(); - } - - $uri = new PhutilGitURI($this->uri); - if ($uri->getDomain()) { - return $uri->getPath(); - } - - return $this->uri; + return $uri->getPath(); case self::TYPE_SVN: case self::TYPE_MERCURIAL: $uri = new PhutilURI($this->uri); @@ -136,14 +127,7 @@ final class PhabricatorRepositoryURINormalizer extends Phobject { $domain = null; $uri = new PhutilURI($this->uri); - if ($uri->getProtocol()) { - $domain = $uri->getDomain(); - } - - if (!strlen($domain)) { - $uri = new PhutilGitURI($this->uri); - $domain = $uri->getDomain(); - } + $domain = $uri->getDomain(); if (!strlen($domain)) { $domain = ''; diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 84c7cfd055..b771066ed6 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -1201,27 +1201,19 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO */ public function getRemoteProtocol() { $uri = $this->getRemoteURIObject(); - - if ($uri instanceof PhutilGitURI) { - return 'ssh'; - } else { - return $uri->getProtocol(); - } + return $uri->getProtocol(); } /** - * Get a parsed object representation of the repository's remote URI. This - * may be a normal URI (returned as a @{class@libphutil:PhutilURI}) or a git - * URI (returned as a @{class@libphutil:PhutilGitURI}). + * Get a parsed object representation of the repository's remote URI.. * - * @return wild A @{class@libphutil:PhutilURI} or - * @{class@libphutil:PhutilGitURI}. + * @return wild A @{class@libphutil:PhutilURI}. * @task uri */ public function getRemoteURIObject() { $raw_uri = $this->getDetail('remote-uri'); - if (!$raw_uri) { + if (!strlen($raw_uri)) { return new PhutilURI(''); } @@ -1229,17 +1221,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return new PhutilURI('file://'.$raw_uri); } - $uri = new PhutilURI($raw_uri); - if ($uri->getProtocol()) { - return $uri; - } - - $uri = new PhutilGitURI($raw_uri); - if ($uri->getDomain()) { - return $uri; - } - - throw new Exception(pht("Remote URI '%s' could not be parsed!", $raw_uri)); + return new PhutilURI($raw_uri); } @@ -1666,27 +1648,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return $this; } - public static function getRemoteURIProtocol($raw_uri) { - $uri = new PhutilURI($raw_uri); - if ($uri->getProtocol()) { - return strtolower($uri->getProtocol()); - } - - $git_uri = new PhutilGitURI($raw_uri); - if (strlen($git_uri->getDomain()) && strlen($git_uri->getPath())) { - return 'ssh'; - } - - return null; - } - public static function assertValidRemoteURI($uri) { if (trim($uri) != $uri) { throw new Exception( pht('The remote URI has leading or trailing whitespace.')); } - $protocol = self::getRemoteURIProtocol($uri); + $uri_object = new PhutilURI($uri); + $protocol = $uri_object->getProtocol(); // Catch confusion between Git/SCP-style URIs and normal URIs. See T3619 // for discussion. This is usually a user adding "ssh://" to an implicit diff --git a/src/applications/repository/storage/PhabricatorRepositoryURI.php b/src/applications/repository/storage/PhabricatorRepositoryURI.php index 853dcfa1b3..15e25205c8 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryURI.php +++ b/src/applications/repository/storage/PhabricatorRepositoryURI.php @@ -191,11 +191,6 @@ final class PhabricatorRepositoryURI return self::IO_NONE; } - - public function getDisplayURI() { - return $this->getURIObject(false); - } - public function getNormalizedURI() { $vcs = $this->getRepository()->getVersionControlSystem(); @@ -216,8 +211,12 @@ final class PhabricatorRepositoryURI return $normal_uri->getNormalizedURI(); } + public function getDisplayURI() { + return $this->getURIObject(); + } + public function getEffectiveURI() { - return $this->getURIObject(true); + return $this->getURIObject(); } public function getURIEnvelope() { @@ -243,11 +242,10 @@ final class PhabricatorRepositoryURI return new PhutilOpaqueEnvelope((string)$uri); } - private function getURIObject($normalize) { + private function getURIObject() { // Users can provide Git/SCP-style URIs in the form "user@host:path". - // These are equivalent to "ssh://user@host/path". We use the more standard - // form internally, and convert to it if we need to specify a port number, - // but try to preserve what the user typed when displaying the URI. + // In the general case, these are not equivalent to any "ssh://..." form + // because the path is relative. if ($this->isBuiltin()) { $builtin_protocol = $this->getForcedProtocol(); @@ -271,43 +269,22 @@ final class PhabricatorRepositoryURI // with it; this should always be provided by the associated credential. $uri->setPass(null); - if (!$uri->getProtocol()) { - $git_uri = new PhutilGitURI($raw_uri); - - // We must normalize this Git-style URI into a normal URI - $must_normalize = ($port && ($port != $default_ports['ssh'])); - if ($must_normalize || $normalize) { - $domain = $git_uri->getDomain(); - - - $uri = id(new PhutilURI("ssh://{$domain}")) - ->setUser($git_uri->getUser()) - ->setPath($git_uri->getPath()); - } else { - $uri = $git_uri; - } + $protocol = $this->getForcedProtocol(); + if ($protocol) { + $uri->setProtocol($protocol); } - $is_normal = ($uri instanceof PhutilURI); + if ($port) { + $uri->setPort($port); + } - if ($is_normal) { - $protocol = $this->getForcedProtocol(); - if ($protocol) { - $uri->setProtocol($protocol); - } + // Remove any explicitly set default ports. + $uri_port = $uri->getPort(); + $uri_protocol = $uri->getProtocol(); - if ($port) { - $uri->setPort($port); - } - - // Remove any explicitly set default ports. - $uri_port = $uri->getPort(); - $uri_protocol = $uri->getProtocol(); - - $uri_default = idx($default_ports, $uri_protocol); - if ($uri_default && ($uri_default == $uri_port)) { - $uri->setPort(null); - } + $uri_default = idx($default_ports, $uri_protocol); + if ($uri_default && ($uri_default == $uri_port)) { + $uri->setPort(null); } $user = $this->getForcedUser(); From f56f1b05c0ddb492bdf3b3d885318351f4c0ed7a Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 13 Jun 2016 09:15:20 -0700 Subject: [PATCH 02/42] Install a SIGTERM handler in ssh-connect Summary: Ref T10547. This has been around for a while but I was never able to reproduce it. I caught a repro case in the cluster recently and I think this is the right fix. We tell Subversion to run `ssh-connect` instead of `ssh` so we can provide options and credentials, by using `SVN_SSH` in the environment. Subversion will sometimes kill the SSH tunnel subprocess aggressively with SIGTERM -- as of writing, you can search for `SIGTERM` in `make_tunnel()` here: http://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_ra_svn/client.c By default, when a PHP process gets SIGTERM it just exits immediately, without running destructors or shutdown functions. Since destructors/shutdown functions don't run, `TempFile` doesn't get a chance to remove the file. I don't have a clear picture of //when// Subversion sends SIGTERM to the child process. I can't really get this to trigger locally via `svn`, although I was able to get it to trigger explicitly. So I'm only about 95% sure this fixes it, but it seems likely. Test Plan: Locally, I couldn't get this to reproduce "normally" even knowing the cause (maybe Subversion doesn't do the SIGTERM stuff on OSX?) but I was able to get it to reproduce reliabily by adding `posix_kill(getmypid(), SIGTERM);` to the body of the script. With that added, running the script with `PHABRICATOR_CREDENTIAL=PHID-CDTL-...` in the environment reliably left straggler temporary files. Adding `declare()` and a signal handler fixed this: the script now runs the `TempFile` destructor and longer leaves the stragglers around. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10547 Differential Revision: https://secure.phabricator.com/D16102 --- scripts/ssh/ssh-connect.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/scripts/ssh/ssh-connect.php b/scripts/ssh/ssh-connect.php index fac8d19130..d42a542140 100755 --- a/scripts/ssh/ssh-connect.php +++ b/scripts/ssh/ssh-connect.php @@ -4,6 +4,11 @@ // This is a wrapper script for Git, Mercurial, and Subversion. It primarily // serves to inject "-o StrictHostKeyChecking=no" into the SSH arguments. +// In some cases, Subversion sends us SIGTERM. If we don't catch the signal and +// react to it, we won't run object destructors by default and thus won't clean +// up temporary files. Declare ticks so we can install a signal handler. +declare(ticks=1); + $root = dirname(dirname(dirname(__FILE__))); require_once $root.'/scripts/__init_script__.php'; @@ -21,6 +26,16 @@ $args->parsePartial( )); $unconsumed_argv = $args->getUnconsumedArgumentVector(); +if (function_exists('pcntl_signal')) { + pcntl_signal(SIGTERM, 'ssh_connect_signal'); +} + +function ssh_connect_signal($signo) { + // This is just letting destructors fire. In particular, we want to clean + // up any temporary files we wrote. See T10547. + exit(128 + $signo); +} + $pattern = array(); $arguments = array(); From a5e29f3ffa9aa7444da81d7ac705987bfd6062db Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 13 Jun 2016 10:10:58 -0700 Subject: [PATCH 03/42] Fix an ancient ad-hoc string truncation Summary: Fixes T11139. We missed this years ago when we moved to PhutilUTF8StringTruncator. Test Plan: {F1686072} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11139 Differential Revision: https://secure.phabricator.com/D16105 --- src/applications/diffusion/data/DiffusionPathChange.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/applications/diffusion/data/DiffusionPathChange.php b/src/applications/diffusion/data/DiffusionPathChange.php index be21125e6c..62dfdd6ace 100644 --- a/src/applications/diffusion/data/DiffusionPathChange.php +++ b/src/applications/diffusion/data/DiffusionPathChange.php @@ -113,9 +113,7 @@ final class DiffusionPathChange extends Phobject { if (!$this->getCommitData()) { return null; } - $message = $this->getCommitData()->getCommitMessage(); - $first = idx(explode("\n", $message), 0); - return substr($first, 0, 80); + return $this->getCommitData()->getSummary(); } public static function convertToArcanistChanges(array $changes) { From 72c57d36a30c6607192988e222309769910c44f3 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 13 Jun 2016 10:34:13 -0700 Subject: [PATCH 04/42] Ability to archive Phame Posts Summary: Ref T9897. Adds ability to Archive a Phame Post (only visible under ApplicationSearch). Test Plan: Archive a post, re-publish it, search for it, archive it again. View Home, Blog, Live pages. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T9897 Differential Revision: https://secure.phabricator.com/D16104 --- src/__phutil_library_map__.php | 2 + .../PhabricatorPhameApplication.php | 1 + .../phame/constants/PhameConstants.php | 5 +- .../phame/controller/PhameHomeController.php | 4 +- .../phame/controller/PhameLiveController.php | 3 +- .../blog/PhameBlogFeedController.php | 2 +- .../blog/PhameBlogViewController.php | 8 ++- .../post/PhamePostArchiveController.php | 56 +++++++++++++++++++ .../post/PhamePostViewController.php | 50 ++++++++++++++--- .../phame/editor/PhamePostEditor.php | 3 + .../phame/query/PhamePostQuery.php | 6 +- .../phame/query/PhamePostSearchEngine.php | 11 +++- src/applications/phame/storage/PhamePost.php | 11 +++- .../phame/storage/PhamePostTransaction.php | 11 ++++ 14 files changed, 153 insertions(+), 20 deletions(-) create mode 100644 src/applications/phame/controller/post/PhamePostArchiveController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index bd8c8c1476..6e29d196c5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3771,6 +3771,7 @@ phutil_register_library_map(array( 'PhameLiveController' => 'applications/phame/controller/PhameLiveController.php', 'PhameNextPostView' => 'applications/phame/view/PhameNextPostView.php', 'PhamePost' => 'applications/phame/storage/PhamePost.php', + 'PhamePostArchiveController' => 'applications/phame/controller/post/PhamePostArchiveController.php', 'PhamePostCommentController' => 'applications/phame/controller/post/PhamePostCommentController.php', 'PhamePostController' => 'applications/phame/controller/post/PhamePostController.php', 'PhamePostEditConduitAPIMethod' => 'applications/phame/conduit/PhamePostEditConduitAPIMethod.php', @@ -8627,6 +8628,7 @@ phutil_register_library_map(array( 'PhabricatorTokenReceiverInterface', 'PhabricatorConduitResultInterface', ), + 'PhamePostArchiveController' => 'PhamePostController', 'PhamePostCommentController' => 'PhamePostController', 'PhamePostController' => 'PhameController', 'PhamePostEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', diff --git a/src/applications/phame/application/PhabricatorPhameApplication.php b/src/applications/phame/application/PhabricatorPhameApplication.php index 9138961214..24d1a05b31 100644 --- a/src/applications/phame/application/PhabricatorPhameApplication.php +++ b/src/applications/phame/application/PhabricatorPhameApplication.php @@ -55,6 +55,7 @@ final class PhabricatorPhameApplication extends PhabricatorApplication { 'preview/' => 'PhabricatorMarkupPreviewController', 'framed/(?P\d+)/' => 'PhamePostFramedController', 'move/(?P\d+)/' => 'PhamePostMoveController', + 'archive/(?P\d+)/' => 'PhamePostArchiveController', 'comment/(?P[1-9]\d*)/' => 'PhamePostCommentController', ), 'blog/' => array( diff --git a/src/applications/phame/constants/PhameConstants.php b/src/applications/phame/constants/PhameConstants.php index c9444411f0..39f35f71ce 100644 --- a/src/applications/phame/constants/PhameConstants.php +++ b/src/applications/phame/constants/PhameConstants.php @@ -2,13 +2,15 @@ final class PhameConstants extends Phobject { - const VISIBILITY_DRAFT = 0; + const VISIBILITY_DRAFT = 0; const VISIBILITY_PUBLISHED = 1; + const VISIBILITY_ARCHIVED = 2; public static function getPhamePostStatusMap() { return array( self::VISIBILITY_PUBLISHED => pht('Published'), self::VISIBILITY_DRAFT => pht('Draft'), + self::VISIBILITY_ARCHIVED => pht('Archived'), ); } @@ -16,6 +18,7 @@ final class PhameConstants extends Phobject { $map = array( self::VISIBILITY_PUBLISHED => pht('Published'), self::VISIBILITY_DRAFT => pht('Draft'), + self::VISIBILITY_ARCHIVED => pht('Archived'), ); return idx($map, $status, pht('Unknown')); } diff --git a/src/applications/phame/controller/PhameHomeController.php b/src/applications/phame/controller/PhameHomeController.php index 95d1544abe..349ff563b1 100644 --- a/src/applications/phame/controller/PhameHomeController.php +++ b/src/applications/phame/controller/PhameHomeController.php @@ -35,7 +35,7 @@ final class PhameHomeController extends PhamePostController { $posts = id(new PhamePostQuery()) ->setViewer($viewer) ->withBlogPHIDs($blog_phids) - ->withVisibility(PhameConstants::VISIBILITY_PUBLISHED) + ->withVisibility(array(PhameConstants::VISIBILITY_PUBLISHED)) ->executeWithCursorPager($pager); if ($posts) { @@ -97,7 +97,7 @@ final class PhameHomeController extends PhamePostController { ->setViewer($viewer) ->withBloggerPHIDs(array($viewer->getPHID())) ->withBlogPHIDs(mpull($blogs, 'getPHID')) - ->withVisibility(PhameConstants::VISIBILITY_DRAFT) + ->withVisibility(array(PhameConstants::VISIBILITY_DRAFT)) ->setLimit(5) ->execute(); diff --git a/src/applications/phame/controller/PhameLiveController.php b/src/applications/phame/controller/PhameLiveController.php index 57ac875465..e2f1789a6c 100644 --- a/src/applications/phame/controller/PhameLiveController.php +++ b/src/applications/phame/controller/PhameLiveController.php @@ -97,7 +97,8 @@ abstract class PhameLiveController extends PhameController { // Only show published posts on external domains. if ($is_external) { - $post_query->withVisibility(PhameConstants::VISIBILITY_PUBLISHED); + $post_query->withVisibility( + array(PhameConstants::VISIBILITY_PUBLISHED)); } $post = $post_query->executeOne(); diff --git a/src/applications/phame/controller/blog/PhameBlogFeedController.php b/src/applications/phame/controller/blog/PhameBlogFeedController.php index b8b47c019f..c0c6e60cc4 100644 --- a/src/applications/phame/controller/blog/PhameBlogFeedController.php +++ b/src/applications/phame/controller/blog/PhameBlogFeedController.php @@ -21,7 +21,7 @@ final class PhameBlogFeedController extends PhameBlogController { $posts = id(new PhamePostQuery()) ->setViewer($viewer) ->withBlogPHIDs(array($blog->getPHID())) - ->withVisibility(PhameConstants::VISIBILITY_PUBLISHED) + ->withVisibility(array(PhameConstants::VISIBILITY_PUBLISHED)) ->execute(); $blog_uri = PhabricatorEnv::getProductionURI( diff --git a/src/applications/phame/controller/blog/PhameBlogViewController.php b/src/applications/phame/controller/blog/PhameBlogViewController.php index da027d584b..f2210b452b 100644 --- a/src/applications/phame/controller/blog/PhameBlogViewController.php +++ b/src/applications/phame/controller/blog/PhameBlogViewController.php @@ -19,10 +19,14 @@ final class PhameBlogViewController extends PhameLiveController { $post_query = id(new PhamePostQuery()) ->setViewer($viewer) - ->withBlogPHIDs(array($blog->getPHID())); + ->withBlogPHIDs(array($blog->getPHID())) + ->withVisibility(array( + PhameConstants::VISIBILITY_PUBLISHED, + PhameConstants::VISIBILITY_DRAFT, + )); if ($is_live) { - $post_query->withVisibility(PhameConstants::VISIBILITY_PUBLISHED); + $post_query->withVisibility(array(PhameConstants::VISIBILITY_PUBLISHED)); } $posts = $post_query->executeWithCursorPager($pager); diff --git a/src/applications/phame/controller/post/PhamePostArchiveController.php b/src/applications/phame/controller/post/PhamePostArchiveController.php new file mode 100644 index 0000000000..5a3b32944a --- /dev/null +++ b/src/applications/phame/controller/post/PhamePostArchiveController.php @@ -0,0 +1,56 @@ +getViewer(); + + $id = $request->getURIData('id'); + $post = id(new PhamePostQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + if (!$post) { + return new Aphront404Response(); + } + + $cancel_uri = $post->getViewURI(); + + if ($request->isFormPost()) { + $xactions = array(); + + $new_value = PhameConstants::VISIBILITY_ARCHIVED; + $xactions[] = id(new PhamePostTransaction()) + ->setTransactionType(PhamePostTransaction::TYPE_VISIBILITY) + ->setNewValue($new_value); + + id(new PhamePostEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->applyTransactions($post, $xactions); + + return id(new AphrontRedirectResponse()) + ->setURI($cancel_uri); + } + + $title = pht('Archive Post'); + $body = pht( + 'This post will revert to archived status and no longer be visible '. + 'to other users or members of this blog.'); + $button = pht('Archive Post'); + + return $this->newDialog() + ->setTitle($title) + ->appendParagraph($body) + ->addSubmitButton($button) + ->addCancelButton($cancel_uri); + } + +} diff --git a/src/applications/phame/controller/post/PhamePostViewController.php b/src/applications/phame/controller/post/PhamePostViewController.php index fdbaf139c2..d28f1b5150 100644 --- a/src/applications/phame/controller/post/PhamePostViewController.php +++ b/src/applications/phame/controller/post/PhamePostViewController.php @@ -48,6 +48,16 @@ final class PhamePostViewController 'Use "Publish" to publish this post.'))); } + if ($post->isArchived()) { + $document->appendChild( + id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_ERROR) + ->setTitle(pht('Archived Post')) + ->appendChild( + pht('Only you can see this archived post until you publish it. '. + 'Use "Publish" to publish this post.'))); + } + if (!$post->getBlog()) { $document->appendChild( id(new PHUIInfoView()) @@ -92,6 +102,8 @@ final class PhamePostViewController $date = phabricator_datetime($post->getDatePublished(), $viewer); if ($post->isDraft()) { $subtitle = pht('Unpublished draft by %s.', $author); + } else if ($post->isArchived()) { + $subtitle = pht('Archived post by %s.', $author); } else { $subtitle = pht('Written by %s on %s.', $author, $date); } @@ -207,6 +219,21 @@ final class PhamePostViewController ->setName(pht('Publish')) ->setDisabled(!$can_edit) ->setWorkflow(true)); + $actions->addAction( + id(new PhabricatorActionView()) + ->setIcon('fa-ban') + ->setHref($this->getApplicationURI('post/archive/'.$id.'/')) + ->setName(pht('Archive')) + ->setDisabled(!$can_edit) + ->setWorkflow(true)); + } else if ($post->isArchived()) { + $actions->addAction( + id(new PhabricatorActionView()) + ->setIcon('fa-eye') + ->setHref($this->getApplicationURI('post/publish/'.$id.'/')) + ->setName(pht('Publish')) + ->setDisabled(!$can_edit) + ->setWorkflow(true)); } else { $actions->addAction( id(new PhabricatorActionView()) @@ -215,6 +242,13 @@ final class PhamePostViewController ->setName(pht('Unpublish')) ->setDisabled(!$can_edit) ->setWorkflow(true)); + $actions->addAction( + id(new PhabricatorActionView()) + ->setIcon('fa-ban') + ->setHref($this->getApplicationURI('post/archive/'.$id.'/')) + ->setName(pht('Archive')) + ->setDisabled(!$can_edit) + ->setWorkflow(true)); } if ($post->isDraft()) { @@ -223,12 +257,14 @@ final class PhamePostViewController $live_name = pht('View Live'); } - $actions->addAction( - id(new PhabricatorActionView()) - ->setUser($viewer) - ->setIcon('fa-globe') - ->setHref($post->getLiveURI()) - ->setName($live_name)); + if (!$post->isArchived()) { + $actions->addAction( + id(new PhabricatorActionView()) + ->setUser($viewer) + ->setIcon('fa-globe') + ->setHref($post->getLiveURI()) + ->setName($live_name)); + } return $actions; } @@ -255,7 +291,7 @@ final class PhamePostViewController $query = id(new PhamePostQuery()) ->setViewer($viewer) - ->withVisibility(PhameConstants::VISIBILITY_PUBLISHED) + ->withVisibility(array(PhameConstants::VISIBILITY_PUBLISHED)) ->withBlogPHIDs(array($post->getBlog()->getPHID())) ->setLimit(1); diff --git a/src/applications/phame/editor/PhamePostEditor.php b/src/applications/phame/editor/PhamePostEditor.php index fa43a131ff..96f2c2016f 100644 --- a/src/applications/phame/editor/PhamePostEditor.php +++ b/src/applications/phame/editor/PhamePostEditor.php @@ -66,6 +66,9 @@ final class PhamePostEditor case PhamePostTransaction::TYPE_VISIBILITY: if ($xaction->getNewValue() == PhameConstants::VISIBILITY_DRAFT) { $object->setDatePublished(0); + } else if ($xaction->getNewValue() == + PhameConstants::VISIBILITY_ARCHIVED) { + $object->setDatePublished(0); } else { $object->setDatePublished(PhabricatorTime::getNow()); } diff --git a/src/applications/phame/query/PhamePostQuery.php b/src/applications/phame/query/PhamePostQuery.php index d2a1feb2c1..22b04bcd99 100644 --- a/src/applications/phame/query/PhamePostQuery.php +++ b/src/applications/phame/query/PhamePostQuery.php @@ -29,7 +29,7 @@ final class PhamePostQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $this; } - public function withVisibility($visibility) { + public function withVisibility(array $visibility) { $this->visibility = $visibility; return $this; } @@ -98,10 +98,10 @@ final class PhamePostQuery extends PhabricatorCursorPagedPolicyAwareQuery { $this->bloggerPHIDs); } - if ($this->visibility !== null) { + if ($this->visibility) { $where[] = qsprintf( $conn, - 'visibility = %d', + 'visibility IN (%Ld)', $this->visibility); } diff --git a/src/applications/phame/query/PhamePostSearchEngine.php b/src/applications/phame/query/PhamePostSearchEngine.php index a7a331fbdf..83f11436ea 100644 --- a/src/applications/phame/query/PhamePostSearchEngine.php +++ b/src/applications/phame/query/PhamePostSearchEngine.php @@ -19,7 +19,7 @@ final class PhamePostSearchEngine $query = $this->newQuery(); if (strlen($map['visibility'])) { - $query->withVisibility($map['visibility']); + $query->withVisibility(array($map['visibility'])); } return $query; @@ -35,6 +35,7 @@ final class PhamePostSearchEngine '' => pht('All'), PhameConstants::VISIBILITY_PUBLISHED => pht('Published'), PhameConstants::VISIBILITY_DRAFT => pht('Draft'), + PhameConstants::VISIBILITY_ARCHIVED => pht('Archived'), )), ); } @@ -48,6 +49,7 @@ final class PhamePostSearchEngine 'all' => pht('All Posts'), 'live' => pht('Published Posts'), 'draft' => pht('Draft Posts'), + 'archived' => pht('Archived Posts'), ); return $names; } @@ -65,6 +67,9 @@ final class PhamePostSearchEngine case 'draft': return $query->setParameter( 'visibility', PhameConstants::VISIBILITY_DRAFT); + case 'archived': + return $query->setParameter( + 'visibility', PhameConstants::VISIBILITY_ARCHIVED); } return parent::buildSavedQueryFromBuiltin($query_key); @@ -100,6 +105,10 @@ final class PhamePostSearchEngine $item->setStatusIcon('fa-star-o grey'); $item->setDisabled(true); $item->addIcon('none', pht('Draft Post')); + } else if ($post->isArchived()) { + $item->setStatusIcon('fa-ban grey'); + $item->setDisabled(true); + $item->addIcon('none', pht('Archived Post')); } else { $date = $post->getDatePublished(); $item->setEpoch($date); diff --git a/src/applications/phame/storage/PhamePost.php b/src/applications/phame/storage/PhamePost.php index fb2e8058dc..f77032ffab 100644 --- a/src/applications/phame/storage/PhamePost.php +++ b/src/applications/phame/storage/PhamePost.php @@ -53,7 +53,8 @@ final class PhamePost extends PhameDAO public function getLiveURI() { $blog = $this->getBlog(); $is_draft = $this->isDraft(); - if (strlen($blog->getDomain()) && !$is_draft) { + $is_archived = $this->isArchived(); + if (strlen($blog->getDomain()) && !$is_draft && !$is_archived) { return $this->getExternalLiveURI(); } else { return $this->getInternalLiveURI(); @@ -92,6 +93,10 @@ final class PhamePost extends PhameDAO return ($this->getVisibility() == PhameConstants::VISIBILITY_DRAFT); } + public function isArchived() { + return ($this->getVisibility() == PhameConstants::VISIBILITY_ARCHIVED); + } + protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -165,7 +170,7 @@ final class PhamePost extends PhameDAO switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: - if (!$this->isDraft() && $this->getBlog()) { + if (!$this->isDraft() && !$this->isArchived() && $this->getBlog()) { return $this->getBlog()->getViewPolicy(); } else if ($this->getBlog()) { return $this->getBlog()->getEditPolicy(); @@ -319,6 +324,8 @@ final class PhamePost extends PhameDAO public function getFieldValuesForConduit() { if ($this->isDraft()) { $date_published = null; + } else if ($this->isArchived()) { + $date_published = null; } else { $date_published = (int)$this->getDatePublished(); } diff --git a/src/applications/phame/storage/PhamePostTransaction.php b/src/applications/phame/storage/PhamePostTransaction.php index be5dbfd8b5..a1efb584b0 100644 --- a/src/applications/phame/storage/PhamePostTransaction.php +++ b/src/applications/phame/storage/PhamePostTransaction.php @@ -73,6 +73,8 @@ final class PhamePostTransaction case self::TYPE_VISIBILITY: if ($new == PhameConstants::VISIBILITY_PUBLISHED) { return 'fa-globe'; + } else if ($new == PhameConstants::VISIBILITY_ARCHIVED) { + return 'fa-ban'; } else { return 'fa-eye-slash'; } @@ -144,6 +146,10 @@ final class PhamePostTransaction return pht( '%s marked this post as a draft.', $this->renderHandleLink($author_phid)); + } else if ($new == PhameConstants::VISIBILITY_ARCHIVED) { + return pht( + '%s archived this post.', + $this->renderHandleLink($author_phid)); } else { return pht( '%s published this post.', @@ -201,6 +207,11 @@ final class PhamePostTransaction '%s marked %s as a draft.', $this->renderHandleLink($author_phid), $this->renderHandleLink($object_phid)); + } else if ($new == PhameConstants::VISIBILITY_ARCHIVED) { + return pht( + '%s marked %s as archived.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($object_phid)); } else { return pht( '%s published %s.', From e78488f6eb7c5d36c60405e6b2c841cac14dacb3 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 13 Jun 2016 10:52:14 -0700 Subject: [PATCH 05/42] Clean up some PhamePostEditor archive cases Summary: Forgot to save this file locally. Adds isArchived to same hidden features as isDraft Test Plan: test mail on archived posts Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16106 --- src/applications/phame/editor/PhamePostEditor.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/applications/phame/editor/PhamePostEditor.php b/src/applications/phame/editor/PhamePostEditor.php index 96f2c2016f..b10484723d 100644 --- a/src/applications/phame/editor/PhamePostEditor.php +++ b/src/applications/phame/editor/PhamePostEditor.php @@ -171,7 +171,7 @@ final class PhamePostEditor protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { - if ($object->isDraft()) { + if ($object->isDraft() || ($object->isArchived())) { return false; } return true; @@ -180,7 +180,7 @@ final class PhamePostEditor protected function shouldPublishFeedStory( PhabricatorLiskDAO $object, array $xactions) { - if ($object->isDraft()) { + if ($object->isDraft() || $object->isArchived()) { return false; } return true; @@ -231,7 +231,7 @@ final class PhamePostEditor foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case PhamePostTransaction::TYPE_VISIBILITY: - if (!$object->isDraft()) { + if (!$object->isDraft() && !$object->isArchived()) { $body->addRemarkupSection(null, $object->getBody()); } break; From aaf3698666a740f7a0ca5d874828e34ca3d4fca5 Mon Sep 17 00:00:00 2001 From: Shijie Feng Date: Mon, 13 Jun 2016 18:08:44 +0000 Subject: [PATCH 06/42] Add datasources to allow search revisions by project. Summary: When having lots of repos, seeing "all revisions in this project" is hard, and we ended up adding herald rules to basically copy project tags to the revisions on a per-project basis. Adding a "tagged: project" function to the Repositories search field allows users to find differentials within a project. Fix T10850. Test Plan: search differentials by tagging project and repository in the Repository field Reviewers: avivey, epriestley, #blessed_reviewers Reviewed By: avivey, epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Maniphest Tasks: T10850 Differential Revision: https://secure.phabricator.com/D16096 --- src/__phutil_library_map__.php | 4 + .../DifferentialRevisionSearchEngine.php | 2 +- .../DifferentialRepositoryDatasource.php | 25 +++++ ...onTaggedRepositoriesFunctionDatasource.php | 100 ++++++++++++++++++ 4 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 src/applications/differential/typeahead/DifferentialRepositoryDatasource.php create mode 100644 src/applications/diffusion/typeahead/DiffusionTaggedRepositoriesFunctionDatasource.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6e29d196c5..f68e463546 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -489,6 +489,7 @@ phutil_register_library_map(array( 'DifferentialReleephRequestFieldSpecification' => 'applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php', 'DifferentialRemarkupRule' => 'applications/differential/remarkup/DifferentialRemarkupRule.php', 'DifferentialReplyHandler' => 'applications/differential/mail/DifferentialReplyHandler.php', + 'DifferentialRepositoryDatasource' => 'applications/differential/typeahead/DifferentialRepositoryDatasource.php', 'DifferentialRepositoryField' => 'applications/differential/customfield/DifferentialRepositoryField.php', 'DifferentialRepositoryLookup' => 'applications/differential/query/DifferentialRepositoryLookup.php', 'DifferentialRequiredSignaturesField' => 'applications/differential/customfield/DifferentialRequiredSignaturesField.php', @@ -821,6 +822,7 @@ phutil_register_library_map(array( 'DiffusionSymbolQuery' => 'applications/diffusion/query/DiffusionSymbolQuery.php', 'DiffusionTagListController' => 'applications/diffusion/controller/DiffusionTagListController.php', 'DiffusionTagListView' => 'applications/diffusion/view/DiffusionTagListView.php', + 'DiffusionTaggedRepositoriesFunctionDatasource' => 'applications/diffusion/typeahead/DiffusionTaggedRepositoriesFunctionDatasource.php', 'DiffusionTagsQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionTagsQueryConduitAPIMethod.php', 'DiffusionURIEditConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionURIEditConduitAPIMethod.php', 'DiffusionURIEditEngine' => 'applications/diffusion/editor/DiffusionURIEditEngine.php', @@ -4788,6 +4790,7 @@ phutil_register_library_map(array( 'DifferentialReleephRequestFieldSpecification' => 'Phobject', 'DifferentialRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'DifferentialReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', + 'DifferentialRepositoryDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'DifferentialRepositoryField' => 'DifferentialCoreCustomField', 'DifferentialRepositoryLookup' => 'Phobject', 'DifferentialRequiredSignaturesField' => 'DifferentialCoreCustomField', @@ -5138,6 +5141,7 @@ phutil_register_library_map(array( 'DiffusionSymbolQuery' => 'PhabricatorOffsetPagedQuery', 'DiffusionTagListController' => 'DiffusionController', 'DiffusionTagListView' => 'DiffusionView', + 'DiffusionTaggedRepositoriesFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'DiffusionTagsQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', 'DiffusionURIEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', 'DiffusionURIEditEngine' => 'PhabricatorEditEngine', diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php index 5cf6f82198..5fac0708fc 100644 --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -75,7 +75,7 @@ final class DifferentialRevisionSearchEngine ->setLabel(pht('Repositories')) ->setKey('repositoryPHIDs') ->setAliases(array('repository', 'repositories', 'repositoryPHID')) - ->setDatasource(new DiffusionRepositoryDatasource()) + ->setDatasource(new DifferentialRepositoryDatasource()) ->setDescription( pht('Find revisions from specific repositories.')), id(new PhabricatorSearchSelectField()) diff --git a/src/applications/differential/typeahead/DifferentialRepositoryDatasource.php b/src/applications/differential/typeahead/DifferentialRepositoryDatasource.php new file mode 100644 index 0000000000..1decc9c1ca --- /dev/null +++ b/src/applications/differential/typeahead/DifferentialRepositoryDatasource.php @@ -0,0 +1,25 @@ +)...'); + } + + public function getDatasourceApplicationClass() { + return 'PhabricatorProjectApplication'; + } + + public function getComponentDatasources() { + return array( + new PhabricatorProjectDatasource(), + ); + } + + public function getDatasourceFunctions() { + return array( + 'tagged' => array( + 'name' => pht('Repositories: ...'), + 'arguments' => pht('project'), + 'summary' => pht('Find results for repositories of a project.'), + 'description' => pht( + 'This function allows you to find results for any of the `. + `repositories of a project:'. + "\n\n". + '> tagged(engineering)'), + ), + ); + } + + protected function didLoadResults(array $results) { + foreach ($results as $result) { + $result + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) + ->setColor(null) + ->setPHID('tagged('.$result->getPHID().')') + ->setDisplayName(pht('Tagged: %s', $result->getDisplayName())) + ->setName('tagged '.$result->getName()); + } + + return $results; + } + + protected function evaluateFunction($function, array $argv_list) { + $phids = array(); + foreach ($argv_list as $argv) { + $phids[] = head($argv); + } + + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getViewer()) + ->withEdgeLogicPHIDs( + PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, + PhabricatorQueryConstraint::OPERATOR_OR, + $phids) + ->execute(); + + $results = array(); + + foreach ($repositories as $repository) { + $results[] = $repository->getPHID(); + } + + return $results; + } + + public function renderFunctionTokens($function, array $argv_list) { + $phids = array(); + foreach ($argv_list as $argv) { + $phids[] = head($argv); + } + + $tokens = $this->renderTokens($phids); + foreach ($tokens as $token) { + // Remove any project color on this token. + $token->setColor(null); + + if ($token->isInvalid()) { + $token + ->setValue(pht('Repositories: Invalid Project')); + } else { + $token + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) + ->setKey('tagged('.$token->getKey().')') + ->setValue(pht('Tagged: %s', $token->getValue())); + } + } + + return $tokens; + } + +} From 63d413e20136df381899768a91aac06d6e12d4f3 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 13 Jun 2016 11:19:18 -0700 Subject: [PATCH 07/42] Don't truncate blogs or posts in Phame sidebar Summary: Removes the CSS truncation, so it's easier to track drafts. Ref T9897 Test Plan: Really long draft titles. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T9897 Differential Revision: https://secure.phabricator.com/D16107 --- resources/celerity/map.php | 4 ++-- webroot/rsrc/css/application/phame/phame.css | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 43fd7c5ea3..b0d278e6a0 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -81,7 +81,7 @@ return array( 'rsrc/css/application/owners/owners-path-editor.css' => '2f00933b', 'rsrc/css/application/paste/paste.css' => '1898e534', 'rsrc/css/application/people/people-profile.css' => '2473d929', - 'rsrc/css/application/phame/phame.css' => '7448a969', + 'rsrc/css/application/phame/phame.css' => 'b78f5f1e', 'rsrc/css/application/pholio/pholio-edit.css' => '07676f51', 'rsrc/css/application/pholio/pholio-inline-comments.css' => '8e545e49', 'rsrc/css/application/pholio/pholio.css' => 'ca89d380', @@ -808,7 +808,7 @@ return array( 'phabricator-uiexample-reactor-sendclass' => '1def2711', 'phabricator-uiexample-reactor-sendproperties' => 'b1f0ccee', 'phabricator-zindex-css' => '5b6fcf3f', - 'phame-css' => '7448a969', + 'phame-css' => 'b78f5f1e', 'pholio-css' => 'ca89d380', 'pholio-edit-css' => '07676f51', 'pholio-inline-comments-css' => '8e545e49', diff --git a/webroot/rsrc/css/application/phame/phame.css b/webroot/rsrc/css/application/phame/phame.css index 61c3ce4361..22d0317054 100644 --- a/webroot/rsrc/css/application/phame/phame.css +++ b/webroot/rsrc/css/application/phame/phame.css @@ -84,7 +84,6 @@ .phame-blog-list-item { display: block; color: {$darkgreytext}; - height: 24px; position: relative; margin-bottom: 8px; padding-right: 20px; @@ -108,14 +107,12 @@ .phame-blog-list-title { margin-left: 30px; - margin-top: 4px; + margin-top: 2px; display: inline-block; font-weight: bold; color: {$bluetext}; width: 190px; overflow: hidden; - white-space: nowrap; - text-overflow: ellipsis; } .phame-blog-list-new-post { From 74682d46ae425eb71258a9ce2b38558cc7e54f91 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 13 Jun 2016 12:00:14 -0700 Subject: [PATCH 08/42] Add edit-pencil to ApplicationSearch for PhamePosts Summary: Adds a quick edit link to PhamePosts in ApplicationSearch Test Plan: Review a few searches, click on pencil. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16109 --- src/applications/phame/query/PhamePostSearchEngine.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/applications/phame/query/PhamePostSearchEngine.php b/src/applications/phame/query/PhamePostSearchEngine.php index 83f11436ea..dbc248d8b2 100644 --- a/src/applications/phame/query/PhamePostSearchEngine.php +++ b/src/applications/phame/query/PhamePostSearchEngine.php @@ -104,15 +104,19 @@ final class PhamePostSearchEngine if ($post->isDraft()) { $item->setStatusIcon('fa-star-o grey'); $item->setDisabled(true); - $item->addIcon('none', pht('Draft Post')); + $item->addIcon('fa-star-o', pht('Draft Post')); } else if ($post->isArchived()) { $item->setStatusIcon('fa-ban grey'); $item->setDisabled(true); - $item->addIcon('none', pht('Archived Post')); + $item->addIcon('fa-ban', pht('Archived Post')); } else { $date = $post->getDatePublished(); $item->setEpoch($date); } + $item->addAction( + id(new PHUIListItemView()) + ->setIcon('fa-pencil') + ->setHref($post->getEditURI())); $list->addItem($item); } From 65634781b44fb6a5e182c896705d44e04490ba76 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 9 Jun 2016 05:25:14 -0700 Subject: [PATCH 09/42] Don't re-mention users for comment edits Summary: Ref T11035. This only fixes half of the issue: comment editing has been fixed, but normal transactions which edit things like descriptions haven't yet. The normal edits aren't fixed because the "oldValues" are populated too late. The code should start working once they get populated sooner, but I don't want to jump the gun on that since it'll probably have some spooky effects. I have some other transaction changes coming down the pipe which should provide a better context for testing "oldValue" population order. Test Plan: - Mentioned `@dog` in a comment. - Removed `@dog` as a subscriber. - Edited the comment, adding some unrelated text at the end (e.g., fixing a typo). - Before change: `@dog` re-added as subscriber. - After change: no re-add. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11035 Differential Revision: https://secure.phabricator.com/D16108 --- src/__phutil_library_map__.php | 4 + .../audit/editor/PhabricatorAuditEditor.php | 4 +- .../editor/DifferentialTransactionEditor.php | 4 +- .../storage/ManiphestTransaction.php | 10 ++- .../data/PhabricatorTransactionChange.php | 37 +++++++++ .../PhabricatorTransactionRemarkupChange.php | 4 + ...torApplicationTransactionCommentEditor.php | 3 + ...habricatorApplicationTransactionEditor.php | 79 ++++++++++++------- .../PhabricatorApplicationTransaction.php | 48 ++++++++++- ...abricatorApplicationTransactionComment.php | 16 ++++ 10 files changed, 167 insertions(+), 42 deletions(-) create mode 100644 src/applications/transactions/data/PhabricatorTransactionChange.php create mode 100644 src/applications/transactions/data/PhabricatorTransactionRemarkupChange.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f68e463546..cc59c09d15 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3591,6 +3591,8 @@ phutil_register_library_map(array( 'PhabricatorTokensCurtainExtension' => 'applications/tokens/engineextension/PhabricatorTokensCurtainExtension.php', 'PhabricatorTokensSettingsPanel' => 'applications/settings/panel/PhabricatorTokensSettingsPanel.php', 'PhabricatorTooltipUIExample' => 'applications/uiexample/examples/PhabricatorTooltipUIExample.php', + 'PhabricatorTransactionChange' => 'applications/transactions/data/PhabricatorTransactionChange.php', + 'PhabricatorTransactionRemarkupChange' => 'applications/transactions/data/PhabricatorTransactionRemarkupChange.php', 'PhabricatorTransactions' => 'applications/transactions/constants/PhabricatorTransactions.php', 'PhabricatorTransactionsApplication' => 'applications/transactions/application/PhabricatorTransactionsApplication.php', 'PhabricatorTransactionsDestructionEngineExtension' => 'applications/transactions/engineextension/PhabricatorTransactionsDestructionEngineExtension.php', @@ -8397,6 +8399,8 @@ phutil_register_library_map(array( 'PhabricatorTokensCurtainExtension' => 'PHUICurtainExtension', 'PhabricatorTokensSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorTooltipUIExample' => 'PhabricatorUIExample', + 'PhabricatorTransactionChange' => 'Phobject', + 'PhabricatorTransactionRemarkupChange' => 'PhabricatorTransactionChange', 'PhabricatorTransactions' => 'Phobject', 'PhabricatorTransactionsApplication' => 'PhabricatorApplication', 'PhabricatorTransactionsDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension', diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index d795d4b97d..f9524084a0 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -542,7 +542,7 @@ final class PhabricatorAuditEditor protected function expandCustomRemarkupBlockTransactions( PhabricatorLiskDAO $object, array $xactions, - $blocks, + array $changes, PhutilMarkupEngine $engine) { // we are only really trying to find unmentionable phids here... @@ -563,7 +563,7 @@ final class PhabricatorAuditEditor return $result; } - $flat_blocks = array_mergev($blocks); + $flat_blocks = mpull($changes, 'getNewValue'); $huge_block = implode("\n\n", $flat_blocks); $phid_map = array(); $phid_map[] = $this->getUnmentionablePHIDMap(); diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 2d2671227d..a87c12c926 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1303,10 +1303,10 @@ final class DifferentialTransactionEditor protected function expandCustomRemarkupBlockTransactions( PhabricatorLiskDAO $object, array $xactions, - $blocks, + array $changes, PhutilMarkupEngine $engine) { - $flat_blocks = array_mergev($blocks); + $flat_blocks = mpull($changes, 'getNewValue'); $huge_block = implode("\n\n", $flat_blocks); $task_map = array(); diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php index 79901d7ae6..1caae54d47 100644 --- a/src/applications/maniphest/storage/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/ManiphestTransaction.php @@ -54,16 +54,18 @@ final class ManiphestTransaction return parent::shouldGenerateOldValue(); } - public function getRemarkupBlocks() { - $blocks = parent::getRemarkupBlocks(); + protected function newRemarkupChanges() { + $changes = array(); switch ($this->getTransactionType()) { case self::TYPE_DESCRIPTION: - $blocks[] = $this->getNewValue(); + $changes[] = $this->newRemarkupChange() + ->setOldValue($this->getOldValue()) + ->setNewValue($this->getNewValue()); break; } - return $blocks; + return $changes; } public function getRequiredHandlePHIDs() { diff --git a/src/applications/transactions/data/PhabricatorTransactionChange.php b/src/applications/transactions/data/PhabricatorTransactionChange.php new file mode 100644 index 0000000000..2fc59ce5e5 --- /dev/null +++ b/src/applications/transactions/data/PhabricatorTransactionChange.php @@ -0,0 +1,37 @@ +transaction = $xaction; + return $this; + } + + final public function getTransaction() { + return $this->transaction; + } + + final public function setOldValue($old_value) { + $this->oldValue = $old_value; + return $this; + } + + final public function getOldValue() { + return $this->oldValue; + } + + final public function setNewValue($new_value) { + $this->newValue = $new_value; + return $this; + } + + final public function getNewValue() { + return $this->newValue; + } + +} diff --git a/src/applications/transactions/data/PhabricatorTransactionRemarkupChange.php b/src/applications/transactions/data/PhabricatorTransactionRemarkupChange.php new file mode 100644 index 0000000000..bf4a4a11b1 --- /dev/null +++ b/src/applications/transactions/data/PhabricatorTransactionRemarkupChange.php @@ -0,0 +1,4 @@ +setTransactionPHID($xaction->getPHID()); $comment->save(); + $old_comment = $xaction->getComment(); + $comment->attachOldComment($old_comment); + $xaction->setCommentVersion($new_version); $xaction->setCommentPHID($comment->getPHID()); $xaction->setViewPolicy($comment->getViewPolicy()); diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 1a4afa2c9f..098b45b9c4 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -1320,17 +1320,28 @@ abstract class PhabricatorApplicationTransactionEditor private function buildSubscribeTransaction( PhabricatorLiskDAO $object, array $xactions, - array $blocks) { + array $changes) { if (!($object instanceof PhabricatorSubscribableInterface)) { return null; } if ($this->shouldEnableMentions($object, $xactions)) { - $texts = array_mergev($blocks); - $phids = PhabricatorMarkupEngine::extractPHIDsFromMentions( + // Identify newly mentioned users. We ignore users who were previously + // mentioned so that we don't re-subscribe users after an edit of text + // which mentions them. + $old_texts = mpull($changes, 'getOldValue'); + $new_texts = mpull($changes, 'getNewValue'); + + $old_phids = PhabricatorMarkupEngine::extractPHIDsFromMentions( $this->getActor(), - $texts); + $old_texts); + + $new_phids = PhabricatorMarkupEngine::extractPHIDsFromMentions( + $this->getActor(), + $new_texts); + + $phids = array_diff($new_phids, $old_phids); } else { $phids = array(); } @@ -1381,11 +1392,6 @@ abstract class PhabricatorApplicationTransactionEditor return $xaction; } - protected function getRemarkupBlocksFromTransaction( - PhabricatorApplicationTransaction $transaction) { - return $transaction->getRemarkupBlocks(); - } - protected function mergeTransactions( PhabricatorApplicationTransaction $u, PhabricatorApplicationTransaction $v) { @@ -1464,15 +1470,12 @@ abstract class PhabricatorApplicationTransactionEditor $xactions = $this->applyImplicitCC($object, $xactions); - $blocks = array(); - foreach ($xactions as $key => $xaction) { - $blocks[$key] = $this->getRemarkupBlocksFromTransaction($xaction); - } + $changes = $this->getRemarkupChanges($xactions); $subscribe_xaction = $this->buildSubscribeTransaction( $object, $xactions, - $blocks); + $changes); if ($subscribe_xaction) { $xactions[] = $subscribe_xaction; } @@ -1484,7 +1487,7 @@ abstract class PhabricatorApplicationTransactionEditor $block_xactions = $this->expandRemarkupBlockTransactions( $object, $xactions, - $blocks, + $changes, $engine); foreach ($block_xactions as $xaction) { @@ -1494,27 +1497,46 @@ abstract class PhabricatorApplicationTransactionEditor return $xactions; } + private function getRemarkupChanges(array $xactions) { + $changes = array(); + + foreach ($xactions as $key => $xaction) { + foreach ($this->getRemarkupChangesFromTransaction($xaction) as $change) { + $changes[] = $change; + } + } + + return $changes; + } + + private function getRemarkupChangesFromTransaction( + PhabricatorApplicationTransaction $transaction) { + return $transaction->getRemarkupChanges(); + } + private function expandRemarkupBlockTransactions( PhabricatorLiskDAO $object, array $xactions, - $blocks, + array $changes, PhutilMarkupEngine $engine) { $block_xactions = $this->expandCustomRemarkupBlockTransactions( $object, $xactions, - $blocks, + $changes, $engine); $mentioned_phids = array(); if ($this->shouldEnableMentions($object, $xactions)) { - foreach ($blocks as $key => $xaction_blocks) { - foreach ($xaction_blocks as $block) { - $engine->markupText($block); - $mentioned_phids += $engine->getTextMetadata( - PhabricatorObjectRemarkupRule::KEY_MENTIONED_OBJECTS, - array()); - } + foreach ($changes as $change) { + // Here, we don't care about processing only new mentions after an edit + // because there is no way for an object to ever "unmention" itself on + // another object, so we can ignore the old value. + $engine->markupText($change->getNewValue()); + + $mentioned_phids += $engine->getTextMetadata( + PhabricatorObjectRemarkupRule::KEY_MENTIONED_OBJECTS, + array()); } } @@ -1559,7 +1581,7 @@ abstract class PhabricatorApplicationTransactionEditor protected function expandCustomRemarkupBlockTransactions( PhabricatorLiskDAO $object, array $xactions, - $blocks, + array $changes, PhutilMarkupEngine $engine) { return array(); } @@ -3096,11 +3118,8 @@ abstract class PhabricatorApplicationTransactionEditor PhabricatorLiskDAO $object, array $xactions) { - $blocks = array(); - foreach ($xactions as $xaction) { - $blocks[] = $this->getRemarkupBlocksFromTransaction($xaction); - } - $blocks = array_mergev($blocks); + $changes = $this->getRemarkupChanges($xactions); + $blocks = mpull($changes, 'getNewValue'); $phids = array(); if ($blocks) { diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 3a7e3841a3..a612be6769 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -179,6 +179,50 @@ abstract class PhabricatorApplicationTransaction return $this->assertAttached($this->object); } + public function getRemarkupChanges() { + $changes = $this->newRemarkupChanges(); + assert_instances_of($changes, 'PhabricatorTransactionRemarkupChange'); + + // Convert older-style remarkup blocks into newer-style remarkup changes. + // This builds changes that do not have the correct "old value", so rules + // that operate differently against edits (like @user mentions) won't work + // properly. + foreach ($this->getRemarkupBlocks() as $block) { + $changes[] = $this->newRemarkupChange() + ->setOldValue(null) + ->setNewValue($block); + } + + $comment = $this->getComment(); + if ($comment) { + if ($comment->hasOldComment()) { + $old_value = $comment->getOldComment()->getContent(); + } else { + $old_value = null; + } + + $new_value = $comment->getContent(); + + $changes[] = $this->newRemarkupChange() + ->setOldValue($old_value) + ->setNewValue($new_value); + } + + return $changes; + } + + protected function newRemarkupChanges() { + return array(); + } + + protected function newRemarkupChange() { + return id(new PhabricatorTransactionRemarkupChange()) + ->setTransaction($this); + } + + /** + * @deprecated + */ public function getRemarkupBlocks() { $blocks = array(); @@ -195,10 +239,6 @@ abstract class PhabricatorApplicationTransaction break; } - if ($this->getComment()) { - $blocks[] = $this->getComment()->getContent(); - } - return $blocks; } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php index 75964b218c..3239125249 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php @@ -18,6 +18,8 @@ abstract class PhabricatorApplicationTransactionComment protected $contentSource; protected $isDeleted = 0; + private $oldComment = self::ATTACHABLE; + abstract public function getApplicationTransactionObject(); public function generatePHID() { @@ -85,6 +87,20 @@ abstract class PhabricatorApplicationTransactionComment return $this; } + public function attachOldComment( + PhabricatorApplicationTransactionComment $old_comment) { + $this->oldComment = $old_comment; + return $this; + } + + public function getOldComment() { + return $this->assertAttached($this->oldComment); + } + + public function hasOldComment() { + return ($this->oldComment !== self::ATTACHABLE); + } + /* -( PhabricatorMarkupInterface )----------------------------------------- */ From d68b2cc0e4c446c1d67e8ebf2dbf3247af77919a Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 13 Jun 2016 15:19:51 -0700 Subject: [PATCH 10/42] Fix construction of default settings for users with no settings at all Summary: Ref T11098. Users with at least one setting set correctly fall back to the defaults, but users with no settings at all currently do not. Make them fall back to global defaults properly. Test Plan: - Set global defaults to some non-default setting. - Completely delete a user's settings. - `bin/cache purge --purge-all` or `--purge-user`. - View settings as the user. - Before change: showed hard-coded defaults instead of global defaults until you save anything. - After change: properly shows global defaults from the start. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11098 Differential Revision: https://secure.phabricator.com/D16112 --- .../PhabricatorUserPreferencesCacheType.php | 26 ++++++++++++++++--- .../storage/PhabricatorUserPreferences.php | 16 +++++++++++- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php index 1e8999837a..015a24cb0f 100644 --- a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php +++ b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php @@ -31,19 +31,39 @@ final class PhabricatorUserPreferencesCacheType ->setViewer($viewer) ->withUserPHIDs($user_phids) ->execute(); + $preferences = mpull($preferences, null, 'getUserPHID'); + + // If some users don't have settings of their own yet, we need to load + // the global default settings to generate caches for them. + if (count($preferences) < count($user_phids)) { + $global = id(new PhabricatorUserPreferencesQuery()) + ->setViewer($viewer) + ->withBuiltinKeys( + array( + PhabricatorUserPreferences::BUILTIN_GLOBAL_DEFAULT, + )) + ->executeOne(); + } else { + $global = null; + } $all_settings = PhabricatorSetting::getAllSettings(); $settings = array(); - foreach ($preferences as $preference) { - $user_phid = $preference->getUserPHID(); + foreach ($users as $user_phid => $user) { + $preference = idx($preferences, $user_phid, $global); + + if (!$preference) { + continue; + } + foreach ($all_settings as $key => $setting) { $value = $preference->getSettingValue($key); // As an optimization, we omit the value from the cache if it is // exactly the same as the hardcoded default. $default_value = id(clone $setting) - ->setViewer($users[$user_phid]) + ->setViewer($user) ->getSettingDefaultValue(); if ($value === $default_value) { continue; diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index 31719c8195..b03365bd32 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -130,9 +130,23 @@ final class PhabricatorUserPreferences return $preferences; } - return id(new self()) + $preferences = id(new self()) ->setUserPHID($user->getPHID()) ->attachUser($user); + + $global = id(new PhabricatorUserPreferencesQuery()) + ->setViewer($user) + ->withBuiltinKeys( + array( + self::BUILTIN_GLOBAL_DEFAULT, + )) + ->executeOne(); + + if ($global) { + $preferences->attachDefaultSettings($global); + } + + return $preferences; } public function newTransaction($key, $value) { From 33ec855449dde85b3053bb22e59b9c55d6b7f810 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 9 Jun 2016 16:00:06 -0700 Subject: [PATCH 11/42] Modularize application transactions in Paste, mostly Summary: Ref T9789. `Transaction` and `Editor` classes are the last major pieces of infrastructure that haven't been fully modularized. Some of the specific issues are: - `Editor` classes rely on a bunch of `instanceof` stuff in the base class to pick up transaction types like "subscribe", "projects", etc. Instead, applications should be adding these, and third-party applications should be able to add them. - Code is spread across `Transaction` and `Editor` classes somewhat oddly. For example, generating old/new values would probably make more sense at the `Transaction` level, but it currently exists at the `Editor` level. - Both types of classes have a lot of functions based on `switch()` statements, which require a ton of boilerplate and are just generally kind of hard to work with. This creates classes for each type of transaction, and moves almost all of the logic to them. These classes are simpler and more focused than the old stuff was, and can organize related code better. This starts inching toward defining `CoreTransactions` for features shared across applications. It only defines the "Create" transaction so far, but at some point I plan to move all the other shared transactions to Core and let them control which objects they're available for. Test Plan: - Created pastes with web UI and API. - Edited all paste properites. - Archived/activated. - Verified files got reasonable names. - Reviewed timeline and feed. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9789 Differential Revision: https://secure.phabricator.com/D16111 --- .../sql/patches/20130801.pastexactions.php | 48 +--- src/__phutil_library_map__.php | 22 +- .../conduit/PasteCreateConduitAPIMethod.php | 6 +- .../PhabricatorPasteArchiveController.php | 4 +- .../editor/PhabricatorPasteEditEngine.php | 11 +- .../paste/editor/PhabricatorPasteEditor.php | 185 +------------- .../PhabricatorPasteTestDataGenerator.php | 6 +- .../paste/mail/PasteCreateMailReceiver.php | 8 +- .../storage/PhabricatorPasteTransaction.php | 227 +----------------- .../PhabricatorPasteContentTransaction.php | 135 +++++++++++ .../PhabricatorPasteLanguageTransaction.php | 29 +++ .../PhabricatorPasteStatusTransaction.php | 62 +++++ .../PhabricatorPasteTitleTransaction.php | 33 +++ .../PhabricatorPasteTransactionType.php | 4 + ...habricatorApplicationTransactionEditor.php | 114 ++++++++- .../PhabricatorApplicationTransaction.php | 1 + .../storage/PhabricatorModularTransaction.php | 138 +++++++++++ .../PhabricatorModularTransactionType.php | 140 +++++++++++ .../PhabricatorCoreCreateTransaction.php | 30 +++ .../PhabricatorCoreTransactionType.php | 4 + .../PhabricatorCoreVoidTransaction.php | 8 + 21 files changed, 747 insertions(+), 468 deletions(-) create mode 100644 src/applications/paste/xaction/PhabricatorPasteContentTransaction.php create mode 100644 src/applications/paste/xaction/PhabricatorPasteLanguageTransaction.php create mode 100644 src/applications/paste/xaction/PhabricatorPasteStatusTransaction.php create mode 100644 src/applications/paste/xaction/PhabricatorPasteTitleTransaction.php create mode 100644 src/applications/paste/xaction/PhabricatorPasteTransactionType.php create mode 100644 src/applications/transactions/storage/PhabricatorModularTransaction.php create mode 100644 src/applications/transactions/storage/PhabricatorModularTransactionType.php create mode 100644 src/applications/transactions/xaction/PhabricatorCoreCreateTransaction.php create mode 100644 src/applications/transactions/xaction/PhabricatorCoreTransactionType.php create mode 100644 src/applications/transactions/xaction/PhabricatorCoreVoidTransaction.php diff --git a/resources/sql/patches/20130801.pastexactions.php b/resources/sql/patches/20130801.pastexactions.php index 75c2ece940..9d3d2c2853 100644 --- a/resources/sql/patches/20130801.pastexactions.php +++ b/resources/sql/patches/20130801.pastexactions.php @@ -1,47 +1,5 @@ establishConnection('w'); -$conn_w->openTransaction(); - -echo pht('Adding transactions for existing paste objects...')."\n"; - -$rows = new LiskRawMigrationIterator($conn_w, 'pastebin_paste'); -foreach ($rows as $row) { - - $id = $row['id']; - echo pht('Adding transactions for paste id %d...', $id)."\n"; - - $xaction_phid = PhabricatorPHID::generateNewPHID( - PhabricatorApplicationTransactionTransactionPHIDType::TYPECONST); - - queryfx( - $conn_w, - 'INSERT INTO %T (phid, authorPHID, objectPHID, viewPolicy, editPolicy, - transactionType, oldValue, newValue, - contentSource, metadata, dateCreated, dateModified, - commentVersion) - VALUES (%s, %s, %s, %s, %s, %s, %ns, %ns, %s, %s, %d, %d, %d)', - $x_table->getTableName(), - $xaction_phid, - $row['authorPHID'], - $row['phid'], - 'public', - $row['authorPHID'], - PhabricatorPasteTransaction::TYPE_CONTENT, - 'null', - $row['filePHID'], - PhabricatorContentSource::newForSource( - PhabricatorOldWorldContentSource::SOURCECONST)->serialize(), - '[]', - $row['dateCreated'], - $row['dateCreated'], - 0); - -} - -$conn_w->saveTransaction(); - -echo pht('Done.')."\n"; +// Long ago, this migration populated initial "create" transactions for old +// pastes from before transactions came into existence. It was removed after +// about three years. diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cc59c09d15..4f5eb4caa6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2160,6 +2160,9 @@ phutil_register_library_map(array( 'PhabricatorController' => 'applications/base/controller/PhabricatorController.php', 'PhabricatorCookies' => 'applications/auth/constants/PhabricatorCookies.php', 'PhabricatorCoreConfigOptions' => 'applications/config/option/PhabricatorCoreConfigOptions.php', + 'PhabricatorCoreCreateTransaction' => 'applications/transactions/xaction/PhabricatorCoreCreateTransaction.php', + 'PhabricatorCoreTransactionType' => 'applications/transactions/xaction/PhabricatorCoreTransactionType.php', + 'PhabricatorCoreVoidTransaction' => 'applications/transactions/xaction/PhabricatorCoreVoidTransaction.php', 'PhabricatorCountdown' => 'applications/countdown/storage/PhabricatorCountdown.php', 'PhabricatorCountdownApplication' => 'applications/countdown/application/PhabricatorCountdownApplication.php', 'PhabricatorCountdownController' => 'applications/countdown/controller/PhabricatorCountdownController.php', @@ -2757,6 +2760,8 @@ phutil_register_library_map(array( 'PhabricatorMetaMTASendGridReceiveController' => 'applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php', 'PhabricatorMetaMTAWorker' => 'applications/metamta/PhabricatorMetaMTAWorker.php', 'PhabricatorMetronomicTriggerClock' => 'infrastructure/daemon/workers/clock/PhabricatorMetronomicTriggerClock.php', + 'PhabricatorModularTransaction' => 'applications/transactions/storage/PhabricatorModularTransaction.php', + 'PhabricatorModularTransactionType' => 'applications/transactions/storage/PhabricatorModularTransactionType.php', 'PhabricatorMonospacedFontSetting' => 'applications/settings/setting/PhabricatorMonospacedFontSetting.php', 'PhabricatorMonospacedTextareasSetting' => 'applications/settings/setting/PhabricatorMonospacedTextareasSetting.php', 'PhabricatorMotivatorProfilePanel' => 'applications/search/profilepanel/PhabricatorMotivatorProfilePanel.php', @@ -2916,12 +2921,14 @@ phutil_register_library_map(array( 'PhabricatorPasteArchiveController' => 'applications/paste/controller/PhabricatorPasteArchiveController.php', 'PhabricatorPasteConfigOptions' => 'applications/paste/config/PhabricatorPasteConfigOptions.php', 'PhabricatorPasteContentSearchEngineAttachment' => 'applications/paste/engineextension/PhabricatorPasteContentSearchEngineAttachment.php', + 'PhabricatorPasteContentTransaction' => 'applications/paste/xaction/PhabricatorPasteContentTransaction.php', 'PhabricatorPasteController' => 'applications/paste/controller/PhabricatorPasteController.php', 'PhabricatorPasteDAO' => 'applications/paste/storage/PhabricatorPasteDAO.php', 'PhabricatorPasteEditController' => 'applications/paste/controller/PhabricatorPasteEditController.php', 'PhabricatorPasteEditEngine' => 'applications/paste/editor/PhabricatorPasteEditEngine.php', 'PhabricatorPasteEditor' => 'applications/paste/editor/PhabricatorPasteEditor.php', 'PhabricatorPasteFilenameContextFreeGrammar' => 'applications/paste/lipsum/PhabricatorPasteFilenameContextFreeGrammar.php', + 'PhabricatorPasteLanguageTransaction' => 'applications/paste/xaction/PhabricatorPasteLanguageTransaction.php', 'PhabricatorPasteListController' => 'applications/paste/controller/PhabricatorPasteListController.php', 'PhabricatorPastePastePHIDType' => 'applications/paste/phid/PhabricatorPastePastePHIDType.php', 'PhabricatorPasteQuery' => 'applications/paste/query/PhabricatorPasteQuery.php', @@ -2930,10 +2937,13 @@ phutil_register_library_map(array( 'PhabricatorPasteSchemaSpec' => 'applications/paste/storage/PhabricatorPasteSchemaSpec.php', 'PhabricatorPasteSearchEngine' => 'applications/paste/query/PhabricatorPasteSearchEngine.php', 'PhabricatorPasteSnippet' => 'applications/paste/snippet/PhabricatorPasteSnippet.php', + 'PhabricatorPasteStatusTransaction' => 'applications/paste/xaction/PhabricatorPasteStatusTransaction.php', 'PhabricatorPasteTestDataGenerator' => 'applications/paste/lipsum/PhabricatorPasteTestDataGenerator.php', + 'PhabricatorPasteTitleTransaction' => 'applications/paste/xaction/PhabricatorPasteTitleTransaction.php', 'PhabricatorPasteTransaction' => 'applications/paste/storage/PhabricatorPasteTransaction.php', 'PhabricatorPasteTransactionComment' => 'applications/paste/storage/PhabricatorPasteTransactionComment.php', 'PhabricatorPasteTransactionQuery' => 'applications/paste/query/PhabricatorPasteTransactionQuery.php', + 'PhabricatorPasteTransactionType' => 'applications/paste/xaction/PhabricatorPasteTransactionType.php', 'PhabricatorPasteViewController' => 'applications/paste/controller/PhabricatorPasteViewController.php', 'PhabricatorPathSetupCheck' => 'applications/config/check/PhabricatorPathSetupCheck.php', 'PhabricatorPeopleAnyOwnerDatasource' => 'applications/people/typeahead/PhabricatorPeopleAnyOwnerDatasource.php', @@ -6729,6 +6739,9 @@ phutil_register_library_map(array( 'PhabricatorController' => 'AphrontController', 'PhabricatorCookies' => 'Phobject', 'PhabricatorCoreConfigOptions' => 'PhabricatorApplicationConfigOptions', + 'PhabricatorCoreCreateTransaction' => 'PhabricatorCoreTransactionType', + 'PhabricatorCoreTransactionType' => 'PhabricatorModularTransactionType', + 'PhabricatorCoreVoidTransaction' => 'PhabricatorModularTransactionType', 'PhabricatorCountdown' => array( 'PhabricatorCountdownDAO', 'PhabricatorPolicyInterface', @@ -7406,6 +7419,8 @@ phutil_register_library_map(array( 'PhabricatorMetaMTASendGridReceiveController' => 'PhabricatorMetaMTAController', 'PhabricatorMetaMTAWorker' => 'PhabricatorWorker', 'PhabricatorMetronomicTriggerClock' => 'PhabricatorTriggerClock', + 'PhabricatorModularTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorModularTransactionType' => 'Phobject', 'PhabricatorMonospacedFontSetting' => 'PhabricatorStringSetting', 'PhabricatorMonospacedTextareasSetting' => 'PhabricatorSelectSetting', 'PhabricatorMotivatorProfilePanel' => 'PhabricatorProfilePanel', @@ -7601,12 +7616,14 @@ phutil_register_library_map(array( 'PhabricatorPasteArchiveController' => 'PhabricatorPasteController', 'PhabricatorPasteConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPasteContentSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment', + 'PhabricatorPasteContentTransaction' => 'PhabricatorPasteTransactionType', 'PhabricatorPasteController' => 'PhabricatorController', 'PhabricatorPasteDAO' => 'PhabricatorLiskDAO', 'PhabricatorPasteEditController' => 'PhabricatorPasteController', 'PhabricatorPasteEditEngine' => 'PhabricatorEditEngine', 'PhabricatorPasteEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorPasteFilenameContextFreeGrammar' => 'PhutilContextFreeGrammar', + 'PhabricatorPasteLanguageTransaction' => 'PhabricatorPasteTransactionType', 'PhabricatorPasteListController' => 'PhabricatorPasteController', 'PhabricatorPastePastePHIDType' => 'PhabricatorPHIDType', 'PhabricatorPasteQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', @@ -7615,10 +7632,13 @@ phutil_register_library_map(array( 'PhabricatorPasteSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorPasteSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorPasteSnippet' => 'Phobject', + 'PhabricatorPasteStatusTransaction' => 'PhabricatorPasteTransactionType', 'PhabricatorPasteTestDataGenerator' => 'PhabricatorTestDataGenerator', - 'PhabricatorPasteTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorPasteTitleTransaction' => 'PhabricatorPasteTransactionType', + 'PhabricatorPasteTransaction' => 'PhabricatorModularTransaction', 'PhabricatorPasteTransactionComment' => 'PhabricatorApplicationTransactionComment', 'PhabricatorPasteTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorPasteTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorPasteViewController' => 'PhabricatorPasteController', 'PhabricatorPathSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorPeopleAnyOwnerDatasource' => 'PhabricatorTypeaheadDatasource', diff --git a/src/applications/paste/conduit/PasteCreateConduitAPIMethod.php b/src/applications/paste/conduit/PasteCreateConduitAPIMethod.php index 5cdf6afdab..b6ccd151ba 100644 --- a/src/applications/paste/conduit/PasteCreateConduitAPIMethod.php +++ b/src/applications/paste/conduit/PasteCreateConduitAPIMethod.php @@ -47,15 +47,15 @@ final class PasteCreateConduitAPIMethod extends PasteConduitAPIMethod { $xactions = array(); $xactions[] = id(new PhabricatorPasteTransaction()) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_CONTENT) + ->setTransactionType(PhabricatorPasteContentTransaction::TRANSACTIONTYPE) ->setNewValue($content); $xactions[] = id(new PhabricatorPasteTransaction()) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_TITLE) + ->setTransactionType(PhabricatorPasteTitleTransaction::TRANSACTIONTYPE) ->setNewValue($title); $xactions[] = id(new PhabricatorPasteTransaction()) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_LANGUAGE) + ->setTransactionType(PhabricatorPasteLanguageTransaction::TRANSACTIONTYPE) ->setNewValue($language); $editor = id(new PhabricatorPasteEditor()) diff --git a/src/applications/paste/controller/PhabricatorPasteArchiveController.php b/src/applications/paste/controller/PhabricatorPasteArchiveController.php index c51b522085..143584e426 100644 --- a/src/applications/paste/controller/PhabricatorPasteArchiveController.php +++ b/src/applications/paste/controller/PhabricatorPasteArchiveController.php @@ -20,7 +20,7 @@ final class PhabricatorPasteArchiveController return new Aphront404Response(); } - $view_uri = '/P'.$paste->getID(); + $view_uri = $paste->getURI(); if ($request->isFormPost()) { if ($paste->isArchived()) { @@ -32,7 +32,7 @@ final class PhabricatorPasteArchiveController $xactions = array(); $xactions[] = id(new PhabricatorPasteTransaction()) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_STATUS) + ->setTransactionType(PhabricatorPasteStatusTransaction::TRANSACTIONTYPE) ->setNewValue($new_status); id(new PhabricatorPasteEditor()) diff --git a/src/applications/paste/editor/PhabricatorPasteEditEngine.php b/src/applications/paste/editor/PhabricatorPasteEditEngine.php index f88ab76697..1bad86da15 100644 --- a/src/applications/paste/editor/PhabricatorPasteEditEngine.php +++ b/src/applications/paste/editor/PhabricatorPasteEditEngine.php @@ -71,7 +71,7 @@ final class PhabricatorPasteEditEngine id(new PhabricatorTextEditField()) ->setKey('title') ->setLabel(pht('Title')) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_TITLE) + ->setTransactionType(PhabricatorPasteTitleTransaction::TRANSACTIONTYPE) ->setDescription(pht('The title of the paste.')) ->setConduitDescription(pht('Retitle the paste.')) ->setConduitTypeDescription(pht('New paste title.')) @@ -79,7 +79,8 @@ final class PhabricatorPasteEditEngine id(new PhabricatorSelectEditField()) ->setKey('language') ->setLabel(pht('Language')) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_LANGUAGE) + ->setTransactionType( + PhabricatorPasteLanguageTransaction::TRANSACTIONTYPE) ->setAliases(array('lang')) ->setIsCopyable(true) ->setOptions($langs) @@ -94,7 +95,8 @@ final class PhabricatorPasteEditEngine id(new PhabricatorTextAreaEditField()) ->setKey('text') ->setLabel(pht('Text')) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_CONTENT) + ->setTransactionType( + PhabricatorPasteContentTransaction::TRANSACTIONTYPE) ->setMonospaced(true) ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL) ->setDescription(pht('The main body text of the paste.')) @@ -104,7 +106,8 @@ final class PhabricatorPasteEditEngine id(new PhabricatorSelectEditField()) ->setKey('status') ->setLabel(pht('Status')) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_STATUS) + ->setTransactionType( + PhabricatorPasteStatusTransaction::TRANSACTIONTYPE) ->setIsConduitOnly(true) ->setOptions(PhabricatorPaste::getStatusNameMap()) ->setDescription(pht('Active or archived status.')) diff --git a/src/applications/paste/editor/PhabricatorPasteEditor.php b/src/applications/paste/editor/PhabricatorPasteEditor.php index cd7a0f271a..063b72cfc0 100644 --- a/src/applications/paste/editor/PhabricatorPasteEditor.php +++ b/src/applications/paste/editor/PhabricatorPasteEditor.php @@ -3,8 +3,6 @@ final class PhabricatorPasteEditor extends PhabricatorApplicationTransactionEditor { - private $fileName; - public function getEditorApplicationClass() { return 'PhabricatorPasteApplication'; } @@ -13,29 +11,17 @@ final class PhabricatorPasteEditor return pht('Pastes'); } - public static function initializeFileForPaste( - PhabricatorUser $actor, - $name, - $data) { + public function getCreateObjectTitle($author, $object) { + return pht('%s created this paste.', $author); + } - return PhabricatorFile::newFromFileData( - $data, - array( - 'name' => $name, - 'mime-type' => 'text/plain; charset=utf-8', - 'authorPHID' => $actor->getPHID(), - 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, - 'editPolicy' => PhabricatorPolicies::POLICY_NOONE, - )); + public function getCreateObjectTitleForFeed($author, $object) { + return pht('%s created %s.', $author, $object); } public function getTransactionTypes() { $types = parent::getTransactionTypes(); - $types[] = PhabricatorPasteTransaction::TYPE_CONTENT; - $types[] = PhabricatorPasteTransaction::TYPE_TITLE; - $types[] = PhabricatorPasteTransaction::TYPE_LANGUAGE; - $types[] = PhabricatorPasteTransaction::TYPE_STATUS; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; $types[] = PhabricatorTransactions::TYPE_COMMENT; @@ -43,163 +29,14 @@ final class PhabricatorPasteEditor return $types; } - protected function shouldApplyInitialEffects( - PhabricatorLiskDAO $object, - array $xactions) { - return true; - } - - protected function applyInitialEffects( - PhabricatorLiskDAO $object, - array $xactions) { - - // Find the most user-friendly filename we can by examining the title of - // the paste and the pending transactions. We'll use this if we create a - // new file to store raw content later. - - $name = $object->getTitle(); - if (!strlen($name)) { - $name = 'paste.raw'; - } - - $type_title = PhabricatorPasteTransaction::TYPE_TITLE; - foreach ($xactions as $xaction) { - if ($xaction->getTransactionType() == $type_title) { - $name = $xaction->getNewValue(); - } - } - - $this->fileName = $name; - } - - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - switch ($type) { - case PhabricatorPasteTransaction::TYPE_CONTENT: - if (!$object->getFilePHID() && !$xactions) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('You must provide content to create a paste.'), - null); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - break; - } - - return $errors; - } - - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorPasteTransaction::TYPE_CONTENT: - return $object->getFilePHID(); - case PhabricatorPasteTransaction::TYPE_TITLE: - return $object->getTitle(); - case PhabricatorPasteTransaction::TYPE_LANGUAGE: - return $object->getLanguage(); - case PhabricatorPasteTransaction::TYPE_STATUS: - return $object->getStatus(); - } - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorPasteTransaction::TYPE_TITLE: - case PhabricatorPasteTransaction::TYPE_LANGUAGE: - case PhabricatorPasteTransaction::TYPE_STATUS: - return $xaction->getNewValue(); - case PhabricatorPasteTransaction::TYPE_CONTENT: - // If this transaction does not really change the paste content, return - // the current file PHID so this transaction no-ops. - $new_content = $xaction->getNewValue(); - $old_content = $object->getRawContent(); - $file_phid = $object->getFilePHID(); - if (($new_content === $old_content) && $file_phid) { - return $file_phid; - } - - $file = self::initializeFileForPaste( - $this->getActor(), - $this->fileName, - $xaction->getNewValue()); - - return $file->getPHID(); - } - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorPasteTransaction::TYPE_CONTENT: - $object->setFilePHID($xaction->getNewValue()); - return; - case PhabricatorPasteTransaction::TYPE_TITLE: - $object->setTitle($xaction->getNewValue()); - return; - case PhabricatorPasteTransaction::TYPE_LANGUAGE: - $object->setLanguage($xaction->getNewValue()); - return; - case PhabricatorPasteTransaction::TYPE_STATUS: - $object->setStatus($xaction->getNewValue()); - return; - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorPasteTransaction::TYPE_CONTENT: - case PhabricatorPasteTransaction::TYPE_TITLE: - case PhabricatorPasteTransaction::TYPE_LANGUAGE: - case PhabricatorPasteTransaction::TYPE_STATUS: - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - - protected function extractFilePHIDsFromCustomTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorPasteTransaction::TYPE_CONTENT: - return array($xaction->getNewValue()); - } - - return parent::extractFilePHIDsFromCustomTransaction($object, $xaction); - } - protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { - foreach ($xactions as $xaction) { - switch ($xaction->getTransactionType()) { - case PhabricatorPasteTransaction::TYPE_CONTENT: - return false; - default: - break; - } + + if ($this->getIsNewObject()) { + return false; } + return true; } @@ -258,8 +95,4 @@ final class PhabricatorPasteEditor return true; } - protected function supportsSearch() { - return false; - } - } diff --git a/src/applications/paste/lipsum/PhabricatorPasteTestDataGenerator.php b/src/applications/paste/lipsum/PhabricatorPasteTestDataGenerator.php index 2a4fa9da5d..43be2d0eba 100644 --- a/src/applications/paste/lipsum/PhabricatorPasteTestDataGenerator.php +++ b/src/applications/paste/lipsum/PhabricatorPasteTestDataGenerator.php @@ -17,15 +17,15 @@ final class PhabricatorPasteTestDataGenerator $xactions = array(); $xactions[] = $this->newTransaction( - PhabricatorPasteTransaction::TYPE_TITLE, + PhabricatorPasteTitleTransaction::TRANSACTIONTYPE, $name); $xactions[] = $this->newTransaction( - PhabricatorPasteTransaction::TYPE_LANGUAGE, + PhabricatorPasteLanguageTransaction::TRANSACTIONTYPE, $language); $xactions[] = $this->newTransaction( - PhabricatorPasteTransaction::TYPE_CONTENT, + PhabricatorPasteContentTransaction::TRANSACTIONTYPE, $content); $editor = id(new PhabricatorPasteEditor()) diff --git a/src/applications/paste/mail/PasteCreateMailReceiver.php b/src/applications/paste/mail/PasteCreateMailReceiver.php index 85acf7f45d..a8f7693d1c 100644 --- a/src/applications/paste/mail/PasteCreateMailReceiver.php +++ b/src/applications/paste/mail/PasteCreateMailReceiver.php @@ -24,17 +24,13 @@ final class PasteCreateMailReceiver extends PhabricatorMailReceiver { $xactions = array(); $xactions[] = id(new PhabricatorPasteTransaction()) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_CONTENT) + ->setTransactionType(PhabricatorPasteContentTransaction::TRANSACTIONTYPE) ->setNewValue($mail->getCleanTextBody()); $xactions[] = id(new PhabricatorPasteTransaction()) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_TITLE) + ->setTransactionType(PhabricatorPasteTitleTransaction::TRANSACTIONTYPE) ->setNewValue($title); - $xactions[] = id(new PhabricatorPasteTransaction()) - ->setTransactionType(PhabricatorPasteTransaction::TYPE_LANGUAGE) - ->setNewValue(''); // auto-detect - $paste = PhabricatorPaste::initializeNewPaste($sender); $content_source = $mail->newContentSource(); diff --git a/src/applications/paste/storage/PhabricatorPasteTransaction.php b/src/applications/paste/storage/PhabricatorPasteTransaction.php index 1402f4c14c..1cd77a7048 100644 --- a/src/applications/paste/storage/PhabricatorPasteTransaction.php +++ b/src/applications/paste/storage/PhabricatorPasteTransaction.php @@ -1,12 +1,7 @@ getTransactionType()) { - case self::TYPE_CONTENT: - $phids[] = $this->getObjectPHID(); - break; - } - - return $phids; - } - - public function shouldHide() { - $old = $this->getOldValue(); - switch ($this->getTransactionType()) { - case self::TYPE_TITLE: - case self::TYPE_LANGUAGE: - if ($old === null) { - return true; - } - break; - } - return parent::shouldHide(); - } - - public function getIcon() { - switch ($this->getTransactionType()) { - case self::TYPE_CONTENT: - return 'fa-plus'; - break; - case self::TYPE_TITLE: - case self::TYPE_LANGUAGE: - return 'fa-pencil'; - break; - case self::TYPE_STATUS: - $new = $this->getNewValue(); - switch ($new) { - case PhabricatorPaste::STATUS_ACTIVE: - return 'fa-check'; - case PhabricatorPaste::STATUS_ARCHIVED: - return 'fa-ban'; - } - break; - } - return parent::getIcon(); - } - - public function getTitle() { - $author_phid = $this->getAuthorPHID(); - $object_phid = $this->getObjectPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $type = $this->getTransactionType(); - switch ($type) { - case PhabricatorTransactions::TYPE_CREATE: - return pht( - '%s created this paste.', - $this->renderHandleLink($author_phid)); - case self::TYPE_CONTENT: - if ($old === null) { - return pht( - '%s created this paste.', - $this->renderHandleLink($author_phid)); - } else { - return pht( - '%s edited the content of this paste.', - $this->renderHandleLink($author_phid)); - } - break; - case self::TYPE_TITLE: - return pht( - '%s updated the paste\'s title to "%s".', - $this->renderHandleLink($author_phid), - $new); - break; - case self::TYPE_LANGUAGE: - return pht( - "%s updated the paste's language.", - $this->renderHandleLink($author_phid)); - break; - case self::TYPE_STATUS: - switch ($new) { - case PhabricatorPaste::STATUS_ACTIVE: - return pht( - '%s activated this paste.', - $this->renderHandleLink($author_phid)); - case PhabricatorPaste::STATUS_ARCHIVED: - return pht( - '%s archived this paste.', - $this->renderHandleLink($author_phid)); - } - break; - - } - - return parent::getTitle(); - } - - public function getTitleForFeed() { - $author_phid = $this->getAuthorPHID(); - $object_phid = $this->getObjectPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $type = $this->getTransactionType(); - switch ($type) { - case self::TYPE_CONTENT: - if ($old === null) { - return pht( - '%s created %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - } else { - return pht( - '%s edited %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - } - break; - case self::TYPE_TITLE: - return pht( - '%s updated the title for %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - break; - case self::TYPE_LANGUAGE: - return pht( - '%s updated the language for %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - break; - case self::TYPE_STATUS: - switch ($new) { - case PhabricatorPaste::STATUS_ACTIVE: - return pht( - '%s activated %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - case PhabricatorPaste::STATUS_ARCHIVED: - return pht( - '%s archived %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - } - break; - } - - return parent::getTitleForFeed(); - } - - public function getColor() { - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_CONTENT: - return PhabricatorTransactions::COLOR_GREEN; - case self::TYPE_STATUS: - switch ($new) { - case PhabricatorPaste::STATUS_ACTIVE: - return 'green'; - case PhabricatorPaste::STATUS_ARCHIVED: - return 'indigo'; - } - break; - } - - return parent::getColor(); - } - - - public function hasChangeDetails() { - switch ($this->getTransactionType()) { - case self::TYPE_CONTENT: - return ($this->getOldValue() !== null); - } - - return parent::hasChangeDetails(); - } - - public function renderChangeDetails(PhabricatorUser $viewer) { - switch ($this->getTransactionType()) { - case self::TYPE_CONTENT: - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $files = id(new PhabricatorFileQuery()) - ->setViewer($viewer) - ->withPHIDs(array_filter(array($old, $new))) - ->execute(); - $files = mpull($files, null, 'getPHID'); - - $old_text = ''; - if (idx($files, $old)) { - $old_text = $files[$old]->loadFileData(); - } - - $new_text = ''; - if (idx($files, $new)) { - $new_text = $files[$new]->loadFileData(); - } - - return $this->renderTextCorpusChangeDetails( - $viewer, - $old_text, - $new_text); - } - - return parent::renderChangeDetails($viewer); + public function getBaseTransactionClass() { + return 'PhabricatorPasteTransactionType'; } public function getMailTags() { $tags = array(); switch ($this->getTransactionType()) { - case self::TYPE_TITLE: - case self::TYPE_CONTENT: - case self::TYPE_LANGUAGE: + case PhabricatorPasteTitleTransaction::TRANSACTIONTYPE: + case PhabricatorPasteContentTransaction::TRANSACTIONTYPE: + case PhabricatorPasteLanguageTransaction::TRANSACTIONTYPE: $tags[] = self::MAILTAG_CONTENT; break; case PhabricatorTransactions::TYPE_COMMENT: diff --git a/src/applications/paste/xaction/PhabricatorPasteContentTransaction.php b/src/applications/paste/xaction/PhabricatorPasteContentTransaction.php new file mode 100644 index 0000000000..b04ed61642 --- /dev/null +++ b/src/applications/paste/xaction/PhabricatorPasteContentTransaction.php @@ -0,0 +1,135 @@ +getFilePHID(); + } + + public function applyInternalEffects($object, $value) { + $object->setFilePHID($value); + } + + public function extractFilePHIDs($object, $value) { + return array($value); + } + + public function validateTransactions($object, array $xactions) { + if ($object->getFilePHID() || $xactions) { + return array(); + } + + $error = $this->newError( + pht('Required'), + pht('You must provide content to create a paste.')); + $error->setIsMissingFieldError(true); + + return array($error); + } + + public function willApplyTransactions($object, array $xactions) { + // Find the most user-friendly filename we can by examining the title of + // the paste and the pending transactions. We'll use this if we create a + // new file to store raw content later. + + $name = $object->getTitle(); + if (!strlen($name)) { + $name = 'paste.raw'; + } + + $type_title = PhabricatorPasteTitleTransaction::TRANSACTIONTYPE; + foreach ($xactions as $xaction) { + if ($xaction->getTransactionType() == $type_title) { + $name = $xaction->getNewValue(); + } + } + + $this->fileName = $name; + } + + public function generateNewValue($object, $value) { + // If this transaction does not really change the paste content, return + // the current file PHID so this transaction no-ops. + $old_content = $object->getRawContent(); + if ($value === $old_content) { + $file_phid = $object->getFilePHID(); + if ($file_phid) { + return $file_phid; + } + } + + $editor = $this->getEditor(); + $actor = $editor->getActor(); + + $file = $this->newFileForPaste($actor, $this->fileName, $value); + + return $file->getPHID(); + } + + private function newFileForPaste(PhabricatorUser $actor, $name, $data) { + return PhabricatorFile::newFromFileData( + $data, + array( + 'name' => $name, + 'mime-type' => 'text/plain; charset=utf-8', + 'authorPHID' => $actor->getPHID(), + 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, + 'editPolicy' => PhabricatorPolicies::POLICY_NOONE, + )); + } + + public function getIcon() { + return 'fa-plus'; + } + + public function getTitle() { + return pht( + '%s edited the content of this paste.', + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s edited %s.', + $this->renderAuthor(), + $this->renderObject()); + } + + public function hasChangeDetailView() { + return true; + } + + public function newChangeDetailView() { + $viewer = $this->getViewer(); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $files = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(array($old, $new)) + ->execute(); + $files = mpull($files, null, 'getPHID'); + + $old_text = ''; + if (idx($files, $old)) { + $old_text = $files[$old]->loadFileData(); + } + + $new_text = ''; + if (idx($files, $new)) { + $new_text = $files[$new]->loadFileData(); + } + + return id(new PhabricatorApplicationTransactionTextDiffDetailView()) + ->setViewer($viewer) + ->setOldText($old_text) + ->setNewText($new_text); + } + +} diff --git a/src/applications/paste/xaction/PhabricatorPasteLanguageTransaction.php b/src/applications/paste/xaction/PhabricatorPasteLanguageTransaction.php new file mode 100644 index 0000000000..2dccc57385 --- /dev/null +++ b/src/applications/paste/xaction/PhabricatorPasteLanguageTransaction.php @@ -0,0 +1,29 @@ +getLanguage(); + } + + public function applyInternalEffects($object, $value) { + $object->setLanguage($value); + } + + public function getTitle() { + return pht( + "%s updated the paste's language.", + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s updated the language for %s.', + $this->renderAuthor(), + $this->renderObject()); + } + +} diff --git a/src/applications/paste/xaction/PhabricatorPasteStatusTransaction.php b/src/applications/paste/xaction/PhabricatorPasteStatusTransaction.php new file mode 100644 index 0000000000..a313e45eab --- /dev/null +++ b/src/applications/paste/xaction/PhabricatorPasteStatusTransaction.php @@ -0,0 +1,62 @@ +getStatus(); + } + + public function applyInternalEffects($object, $value) { + $object->setStatus($value); + } + + private function isActivate() { + return ($this->getNewValue() == PhabricatorPaste::STATUS_ARCHIVED); + } + + public function getIcon() { + if ($this->isActivate()) { + return 'fa-check'; + } else { + return 'fa-ban'; + } + } + + public function getColor() { + if ($this->isActivate()) { + return 'green'; + } else { + return 'indigo'; + } + } + + public function getTitle() { + if ($this->isActivate()) { + return pht( + '%s activated this paste.', + $this->renderAuthor()); + } else { + return pht( + '%s archived this paste.', + $this->renderAuthor()); + } + } + + public function getTitleForFeed() { + if ($this->isActivate()) { + return pht( + '%s activated %s.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s archived %s.', + $this->renderAuthor(), + $this->renderObject()); + } + } + +} diff --git a/src/applications/paste/xaction/PhabricatorPasteTitleTransaction.php b/src/applications/paste/xaction/PhabricatorPasteTitleTransaction.php new file mode 100644 index 0000000000..0fc86b3d96 --- /dev/null +++ b/src/applications/paste/xaction/PhabricatorPasteTitleTransaction.php @@ -0,0 +1,33 @@ +getTitle(); + } + + public function applyInternalEffects($object, $value) { + $object->setTitle($value); + } + + public function getTitle() { + return pht( + '%s updated the paste\'s title from "%s" to "%s".', + $this->renderAuthor(), + $this->getOldValue(), + $this->getNewValue()); + } + + public function getTitleForFeed() { + return pht( + '%s updated the title for %s from "%s" to "%s".', + $this->renderAuthor(), + $this->renderObject(), + $this->getOldValue(), + $this->getNewValue()); + } + +} diff --git a/src/applications/paste/xaction/PhabricatorPasteTransactionType.php b/src/applications/paste/xaction/PhabricatorPasteTransactionType.php new file mode 100644 index 0000000000..bed6af0a67 --- /dev/null +++ b/src/applications/paste/xaction/PhabricatorPasteTransactionType.php @@ -0,0 +1,4 @@ +object->getApplicationTransactionTemplate(); + if ($template instanceof PhabricatorModularTransaction) { + $xtypes = $template->newModularTransactionTypes(); + foreach ($xtypes as $xtype) { + $types[] = $xtype->getTransactionTypeConstant(); + } + } + return $types; } @@ -304,7 +313,15 @@ abstract class PhabricatorApplicationTransactionEditor private function getTransactionOldValue( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { + + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + return $xtype->generateOldValue($object); + } + + switch ($type) { case PhabricatorTransactions::TYPE_CREATE: return null; case PhabricatorTransactions::TYPE_SUBSCRIBERS: @@ -374,7 +391,15 @@ abstract class PhabricatorApplicationTransactionEditor private function getTransactionNewValue( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { + + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + return $xtype->generateNewValue($object, $xaction->getNewValue()); + } + + switch ($type) { case PhabricatorTransactions::TYPE_CREATE: return null; case PhabricatorTransactions::TYPE_SUBSCRIBERS: @@ -496,7 +521,14 @@ abstract class PhabricatorApplicationTransactionEditor PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + return $xtype->applyInternalEffects($object, $xaction->getNewValue()); + } + + switch ($type) { case PhabricatorTransactions::TYPE_CUSTOMFIELD: $field = $this->getCustomFieldForTransaction($object, $xaction); return $field->applyApplicationTransactionInternalEffects($xaction); @@ -520,7 +552,15 @@ abstract class PhabricatorApplicationTransactionEditor private function applyExternalEffects( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { + + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + return $xtype->applyExternalEffects($object, $xaction->getNewValue()); + } + + switch ($type) { case PhabricatorTransactions::TYPE_SUBSCRIBERS: $subeditor = id(new PhabricatorSubscriptionsEditor()) ->setObject($object) @@ -802,6 +842,8 @@ abstract class PhabricatorApplicationTransactionEditor throw new PhabricatorApplicationTransactionValidationException($errors); } + $this->willApplyTransactions($object, $xactions); + if ($object->getID()) { foreach ($xactions as $xaction) { @@ -2006,6 +2048,12 @@ abstract class PhabricatorApplicationTransactionEditor array $xactions) { $errors = array(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + $errors[] = $xtype->validateTransactions($object, $xactions); + } + switch ($type) { case PhabricatorTransactions::TYPE_VIEW_POLICY: $errors[] = $this->validatePolicyTransaction( @@ -3129,9 +3177,16 @@ abstract class PhabricatorApplicationTransactionEditor } foreach ($xactions as $xaction) { - $phids[] = $this->extractFilePHIDsFromCustomTransaction( - $object, - $xaction); + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + $phids[] = $xtype->extractFilePHIDs($object, $xaction->getNewValue()); + } else { + $phids[] = $this->extractFilePHIDsFromCustomTransaction( + $object, + $xaction); + } } $phids = array_unique(array_filter(array_mergev($phids))); @@ -3692,5 +3747,50 @@ abstract class PhabricatorApplicationTransactionEditor $proxy_phids); } + private function getModularTransactionTypes() { + if ($this->modularTypes === null) { + $template = $this->object->getApplicationTransactionTemplate(); + if ($template instanceof PhabricatorModularTransaction) { + $xtypes = $template->newModularTransactionTypes(); + foreach ($xtypes as $key => $xtype) { + $xtype = clone $xtype; + $xtype->setEditor($this); + $xtypes[$key] = $xtype; + } + } else { + $xtypes = array(); + } + + $this->modularTypes = $xtypes; + } + + return $this->modularTypes; + } + + private function getModularTransactionType($type) { + $types = $this->getModularTransactionTypes(); + return idx($types, $type); + } + + private function willApplyTransactions($object, array $xactions) { + foreach ($xactions as $xaction) { + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if (!$xtype) { + continue; + } + + $xtype->willApplyTransactions($object, $xactions); + } + } + + public function getCreateObjectTitle($author, $object) { + return pht('%s created this object.', $author); + } + + public function getCreateObjectTitleForFeed($author, $object) { + return pht('%s created an object: %s.', $author, $object); + } } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index a612be6769..53259984b4 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -280,6 +280,7 @@ abstract class PhabricatorApplicationTransaction $new = $this->getNewValue(); $phids[] = array($this->getAuthorPHID()); + $phids[] = array($this->getObjectPHID()); switch ($this->getTransactionType()) { case PhabricatorTransactions::TYPE_CUSTOMFIELD: $field = $this->getTransactionCustomField(); diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php new file mode 100644 index 0000000000..9700252454 --- /dev/null +++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php @@ -0,0 +1,138 @@ +implementation) { + $this->implementation = $this->newTransactionImplementation(); + } + + return $this->implementation; + } + + public function newModularTransactionTypes() { + $base_class = $this->getBaseTransactionClass(); + + $types = id(new PhutilClassMapQuery()) + ->setAncestorClass($base_class) + ->setUniqueMethod('getTransactionTypeConstant') + ->execute(); + + // Add core transaction types. + $types += id(new PhutilClassMapQuery()) + ->setAncestorClass('PhabricatorCoreTransactionType') + ->setUniqueMethod('getTransactionTypeConstant') + ->execute(); + + return $types; + } + + private function newTransactionImplementation() { + $types = $this->newModularTransactionTypes(); + + $key = $this->getTransactionType(); + + if (empty($types[$key])) { + $type = new PhabricatorCoreVoidTransaction(); + } else { + $type = clone $types[$key]; + } + + $type->setStorage($this); + + return $type; + } + + final public function generateOldValue($object) { + return $this->getTransactionImplementation()->generateOldValue($object); + } + + final public function generateNewValue($object) { + return $this->getTransactionImplementation() + ->generateNewValue($object, $this->getNewValue()); + } + + final public function willApplyTransactions($object, array $xactions) { + return $this->getTransactionImplementation() + ->willApplyTransactions($object, $xactions); + } + + final public function applyInternalEffects($object) { + return $this->getTransactionImplementation() + ->applyInternalEffects($object); + } + + final public function applyExternalEffects($object) { + return $this->getTransactionImplementation() + ->applyExternalEffects($object); + } + + final public function shouldHide() { + if ($this->getTransactionImplementation()->shouldHide()) { + return true; + } + + return parent::shouldHide(); + } + + final public function getIcon() { + $icon = $this->getTransactionImplementation()->getIcon(); + if ($icon !== null) { + return $icon; + } + + return parent::getIcon(); + } + + final public function getTitle() { + $title = $this->getTransactionImplementation()->getTitle(); + if ($title !== null) { + return $title; + } + + return parent::getTitle(); + } + + final public function getTitleForFeed() { + $title = $this->getTransactionImplementation()->getTitleForFeed(); + if ($title !== null) { + return $title; + } + + return $this->getTitle(); + } + + final public function getColor() { + $color = $this->getTransactionImplementation()->getColor(); + if ($color !== null) { + return $color; + } + + return parent::getColor(); + } + + final public function hasChangeDetails() { + if ($this->getTransactionImplementation()->hasChangeDetailView()) { + return true; + } + + return parent::hasChangeDetails(); + } + + final public function renderChangeDetails(PhabricatorUser $viewer) { + $impl = $this->getTransactionImplementation(); + $impl->setViewer($viewer); + $view = $impl->newChangeDetailView(); + if ($view !== null) { + return $view; + } + + return parent::renderChangeDetails($viewer); + } + +} diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php new file mode 100644 index 0000000000..b37bf0d61d --- /dev/null +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -0,0 +1,140 @@ +getPhobjectClassConstant('TRANSACTIONTYPE'); + } + + public function generateOldValue($object) { + throw new PhutilMethodNotImplementedException(); + } + + public function generateNewValue($object, $value) { + return $value; + } + + public function validateTransactions($object, array $xactions) { + return array(); + } + + public function willApplyTransactions($object, array $xactions) { + return; + } + + public function applyInternalEffects($object, $value) { + return; + } + + public function applyExternalEffects($object, $value) { + return; + } + + public function extractFilePHIDs($object, $value) { + return array(); + } + + public function shouldHide() { + return false; + } + + public function getIcon() { + return null; + } + + public function getTitle() { + return null; + } + + public function getTitleForFeed() { + return null; + } + + public function getColor() { + return null; + } + + public function hasChangeDetailView() { + return false; + } + + public function newChangeDetailView() { + throw new PhutilMethodNotImplementedException(); + } + + final public function setStorage( + PhabricatorApplicationTransaction $xaction) { + $this->storage = $xaction; + return $this; + } + + private function getStorage() { + return $this->storage; + } + + final public function setViewer(PhabricatorUser $viewer) { + $this->viewer = $viewer; + return $this; + } + + final protected function getViewer() { + return $this->viewer; + } + + final public function setEditor( + PhabricatorApplicationTransactionEditor $editor) { + $this->editor = $editor; + return $this; + } + + final protected function getEditor() { + if (!$this->editor) { + throw new PhutilInvalidStateException('setEditor'); + } + return $this->editor; + } + + final protected function getAuthorPHID() { + return $this->getStorage()->getAuthorPHID(); + } + + final protected function getObjectPHID() { + return $this->getStorage()->getObjectPHID(); + } + + final protected function getObject() { + return $this->getStorage()->getObject(); + } + + final protected function getOldValue() { + return $this->getStorage()->getOldValue(); + } + + final protected function getNewValue() { + return $this->getStorage()->getNewValue(); + } + + final protected function renderAuthor() { + $author_phid = $this->getAuthorPHID(); + return $this->getStorage()->renderHandleLink($author_phid); + } + + final protected function renderObject() { + $object_phid = $this->getObjectPHID(); + return $this->getStorage()->renderHandleLink($object_phid); + } + + final protected function newError($title, $message, $xaction = null) { + return new PhabricatorApplicationTransactionValidationError( + $this->getTransactionTypeConstant(), + $title, + $message, + $xaction); + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorCoreCreateTransaction.php b/src/applications/transactions/xaction/PhabricatorCoreCreateTransaction.php new file mode 100644 index 0000000000..0c8c337193 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorCoreCreateTransaction.php @@ -0,0 +1,30 @@ +getObject()->getApplicationTransactionEditor(); + + $author = $this->renderAuthor(); + $object = $this->renderObject(); + + return $editor->getCreateObjectTitle($author, $object); + } + + public function getTitleForFeed() { + $editor = $this->getObject()->getApplicationTransactionEditor(); + + $author = $this->renderAuthor(); + $object = $this->renderObject(); + + return $editor->getCreateObjectTitleForFeed($author, $object); + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorCoreTransactionType.php b/src/applications/transactions/xaction/PhabricatorCoreTransactionType.php new file mode 100644 index 0000000000..f4020b1eb9 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorCoreTransactionType.php @@ -0,0 +1,4 @@ + Date: Tue, 14 Jun 2016 17:13:12 +0000 Subject: [PATCH 12/42] fix Vary Subjects option names Summary: The option names for `Vary Subjects` are copypasta from the `Add "Re:" Prefix` option. Fix their names to refer to `Vary Subjects` instead. Fixes T11148 Test Plan: Verify option names for `Vary Subjects` refer to `Add "Re:" Prefix` before apply. Verify they no longer do after apply. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin Maniphest Tasks: T11148 Differential Revision: https://secure.phabricator.com/D16113 --- .../settings/setting/PhabricatorEmailVarySubjectsSetting.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php b/src/applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php index b8e6474155..0c6b73907b 100644 --- a/src/applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php +++ b/src/applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php @@ -48,8 +48,8 @@ final class PhabricatorEmailVarySubjectsSetting protected function getSelectOptions() { return array( - self::VALUE_VARY_SUBJECTS => pht('Enable "Re:" Prefix'), - self::VALUE_STATIC_SUBJECTS => pht('Disable "Re:" Prefix'), + self::VALUE_VARY_SUBJECTS => pht('Enable Vary Subjects'), + self::VALUE_STATIC_SUBJECTS => pht('Disable Vary Subjects'), ); } From e44d92babccac24df39594a3956a10be83d4e69c Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 14 Jun 2016 10:56:53 -0700 Subject: [PATCH 13/42] Have modular transactions fall back correctly when selecting feed titles Summary: Ref T9789. Falling back to `parent::` is better, and fixes older-style feed stories for Pastes, like "added a comment". Test Plan: Viewed a comment feed story about a paste. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9789 Differential Revision: https://secure.phabricator.com/D16114 --- .../transactions/storage/PhabricatorModularTransaction.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php index 9700252454..5dc6229a77 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransaction.php +++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php @@ -104,7 +104,7 @@ abstract class PhabricatorModularTransaction return $title; } - return $this->getTitle(); + return parent::getTitleForFeed(); } final public function getColor() { From bce44c8b02b312fa3cb5cc9e144dda7593ef4734 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Tue, 14 Jun 2016 12:17:09 -0700 Subject: [PATCH 14/42] Add PhamePost to full text search Summary: Adds PhamePost object to fulltextsearch index. Some issue searching just "Open" though? Also "closed" objects search fine but don't display as disabled. Test Plan: bin/search index --type POST {F1687043} Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T9897 Differential Revision: https://secure.phabricator.com/D16116 --- src/__phutil_library_map__.php | 3 ++ .../phid/PhabricatorPhamePostPHIDType.php | 6 ++++ .../phame/search/PhamePostFulltextEngine.php | 34 +++++++++++++++++++ src/applications/phame/storage/PhamePost.php | 10 +++++- 4 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 src/applications/phame/search/PhamePostFulltextEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4f5eb4caa6..971b9be04c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3792,6 +3792,7 @@ phutil_register_library_map(array( 'PhamePostEditController' => 'applications/phame/controller/post/PhamePostEditController.php', 'PhamePostEditEngine' => 'applications/phame/editor/PhamePostEditEngine.php', 'PhamePostEditor' => 'applications/phame/editor/PhamePostEditor.php', + 'PhamePostFulltextEngine' => 'applications/phame/search/PhamePostFulltextEngine.php', 'PhamePostHistoryController' => 'applications/phame/controller/post/PhamePostHistoryController.php', 'PhamePostListController' => 'applications/phame/controller/post/PhamePostListController.php', 'PhamePostListView' => 'applications/phame/view/PhamePostListView.php', @@ -8655,6 +8656,7 @@ phutil_register_library_map(array( 'PhabricatorDestructibleInterface', 'PhabricatorTokenReceiverInterface', 'PhabricatorConduitResultInterface', + 'PhabricatorFulltextInterface', ), 'PhamePostArchiveController' => 'PhamePostController', 'PhamePostCommentController' => 'PhamePostController', @@ -8663,6 +8665,7 @@ phutil_register_library_map(array( 'PhamePostEditController' => 'PhamePostController', 'PhamePostEditEngine' => 'PhabricatorEditEngine', 'PhamePostEditor' => 'PhabricatorApplicationTransactionEditor', + 'PhamePostFulltextEngine' => 'PhabricatorFulltextEngine', 'PhamePostHistoryController' => 'PhamePostController', 'PhamePostListController' => 'PhamePostController', 'PhamePostListView' => 'AphrontTagView', diff --git a/src/applications/phame/phid/PhabricatorPhamePostPHIDType.php b/src/applications/phame/phid/PhabricatorPhamePostPHIDType.php index 5573f2db9f..4250c91949 100644 --- a/src/applications/phame/phid/PhabricatorPhamePostPHIDType.php +++ b/src/applications/phame/phid/PhabricatorPhamePostPHIDType.php @@ -34,7 +34,13 @@ final class PhabricatorPhamePostPHIDType extends PhabricatorPHIDType { $handle->setName($post->getTitle()); $handle->setFullName($post->getTitle()); $handle->setURI('/phame/post/view/'.$post->getID().'/'); + + if ($post->isArchived()) { + $handle->setStatus(PhabricatorObjectHandle::STATUS_CLOSED); + } + } + } } diff --git a/src/applications/phame/search/PhamePostFulltextEngine.php b/src/applications/phame/search/PhamePostFulltextEngine.php new file mode 100644 index 0000000000..27a97ae4ba --- /dev/null +++ b/src/applications/phame/search/PhamePostFulltextEngine.php @@ -0,0 +1,34 @@ +setDocumentTitle($post->getTitle()); + + $document->addField( + PhabricatorSearchDocumentFieldType::FIELD_BODY, + $post->getBody()); + + $document->addRelationship( + PhabricatorSearchRelationship::RELATIONSHIP_AUTHOR, + $post->getBloggerPHID(), + PhabricatorPeopleUserPHIDType::TYPECONST, + $post->getDateCreated()); + + $document->addRelationship( + $post->isArchived() + ? PhabricatorSearchRelationship::RELATIONSHIP_CLOSED + : PhabricatorSearchRelationship::RELATIONSHIP_OPEN, + $post->getPHID(), + PhabricatorPhamePostPHIDType::TYPECONST, + PhabricatorTime::getNow()); + + } + +} diff --git a/src/applications/phame/storage/PhamePost.php b/src/applications/phame/storage/PhamePost.php index f77032ffab..2d8fc887c5 100644 --- a/src/applications/phame/storage/PhamePost.php +++ b/src/applications/phame/storage/PhamePost.php @@ -10,7 +10,8 @@ final class PhamePost extends PhameDAO PhabricatorSubscribableInterface, PhabricatorDestructibleInterface, PhabricatorTokenReceiverInterface, - PhabricatorConduitResultInterface { + PhabricatorConduitResultInterface, + PhabricatorFulltextInterface { const MARKUP_FIELD_BODY = 'markup:body'; const MARKUP_FIELD_SUMMARY = 'markup:summary'; @@ -344,4 +345,11 @@ final class PhamePost extends PhameDAO return array(); } + +/* -( PhabricatorFulltextInterface )--------------------------------------- */ + + public function newFulltextEngine() { + return new PhamePostFulltextEngine(); + } + } From cb7560d301724bdb83ebda8fb7f7d7d35aa506cc Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 14 Jun 2016 11:27:18 -0700 Subject: [PATCH 15/42] Remove "re prefix" and "vary subjects" config Summary: Ref T11098. There is no reason to maintain these as separate values now that they can be configured in global settings. Test Plan: - Hit and read setup issue. - Fiddled with settings. - I'll vet this more throughly in the next diff since I need to fix an issue with global defaults in mail and can explicitly test this at the same time. Reviewers: chad Reviewed By: chad Subscribers: eadler Maniphest Tasks: T11098 Differential Revision: https://secure.phabricator.com/D16117 --- .../check/PhabricatorExtraConfigSetupCheck.php | 7 +++++++ .../option/PhabricatorMetaMTAConfigOptions.php | 16 ---------------- .../metamta/storage/PhabricatorMetaMTAMail.php | 15 ++------------- .../setting/PhabricatorEmailRePrefixSetting.php | 2 +- 4 files changed, 10 insertions(+), 30 deletions(-) diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index 99d3f9962b..1e2eaa5680 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -190,6 +190,10 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'The Differential revision list view age UI elements have been removed '. 'to simplify the interface.'); + $global_settings_reason = pht( + 'The "Re: Prefix" and "Vary Subjects" settings are now configured '. + 'in global settings.'); + $ancient_config += array( 'phid.external-loaders' => pht( @@ -321,6 +325,9 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'differential.days-fresh' => $stale_reason, 'differential.days-stale' => $stale_reason, + + 'metamta.re-prefix' => $global_settings_reason, + 'metamta.vary-subjects' => $global_settings_reason, ); return $ancient_config; diff --git a/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php b/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php index 16bdb7e520..45604f6872 100644 --- a/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php +++ b/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php @@ -276,22 +276,6 @@ EODOC )) ->setSummary(pht('Show email preferences link in email.')) ->setDescription($email_preferences_description), - $this->newOption('metamta.re-prefix', 'bool', false) - ->setBoolOptions( - array( - pht('Force "Re:" Subject Prefix'), - pht('No "Re:" Subject Prefix'), - )) - ->setSummary(pht('Control "Re:" subject prefix, for Mail.app.')) - ->setDescription($re_prefix_description), - $this->newOption('metamta.vary-subjects', 'bool', true) - ->setBoolOptions( - array( - pht('Allow Varied Subjects'), - pht('Always Use the Same Thread Subject'), - )) - ->setSummary(pht('Control subject variance, for some mail clients.')) - ->setDescription($vary_subjects_description), $this->newOption('metamta.insecure-auth-with-reply-to', 'bool', false) ->setBoolOptions( array( diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index dcc14f0cc1..ceea3f0429 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -1131,27 +1131,16 @@ final class PhabricatorMetaMTAMail } private function shouldAddRePrefix(PhabricatorUserPreferences $preferences) { - $default_value = PhabricatorEnv::getEnvConfig('metamta.re-prefix'); - - $value = $preferences->getPreference( + $value = $preferences->getSettingValue( PhabricatorEmailRePrefixSetting::SETTINGKEY); - if ($value === null) { - return $default_value; - } return ($value == PhabricatorEmailRePrefixSetting::VALUE_RE_PREFIX); } private function shouldVarySubject(PhabricatorUserPreferences $preferences) { - $default_value = PhabricatorEnv::getEnvConfig('metamta.vary-subjects'); - - $value = $preferences->getPreference( + $value = $preferences->getSettingValue( PhabricatorEmailVarySubjectsSetting::SETTINGKEY); - if ($value === null) { - return $default_value; - } - return ($value == PhabricatorEmailVarySubjectsSetting::VALUE_VARY_SUBJECTS); } diff --git a/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php b/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php index 2a2f63b461..7b04ef80c3 100644 --- a/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php +++ b/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php @@ -39,7 +39,7 @@ final class PhabricatorEmailRePrefixSetting } public function getSettingDefaultValue() { - return self::VALUE_RE_PREFIX; + return self::VALUE_NO_PREFIX; } protected function getSelectOptions() { From 2e450212503e80e07c55821d5ba68d6370e98d04 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 14 Jun 2016 11:49:59 -0700 Subject: [PATCH 16/42] Fix several issues with email-related global preferences Summary: Ref T11098. Mixture of issues here: - Similar problem to D16112, where users with no settings at all could fail to fall back to the global defaults. - I made `UserPreferencesQuery` responsible for building defaults instead to simplify this, since we have 4 or 5 callsites which need to do it and they aren't easily reducible. - Handle cases where `metamta.one-mail-per-recipient` is off (and thus users can not have any custom settings) more explicitly. - When `metamta.one-mail-per-recipient` is off, remove the "Email Format" panel for users only -- administrators can still access it in global preferences. Test Plan: - Deleted a user's preferences, changed globals, purged cache, made sure defaults reflected global defaults. - Changed global mail tags, sent mail to the user, verified it was dropped in accordinace with global settings. - Changed user's settings to get the mail instead, verified mail was sent. - Toggled user's Re / Vary settings, verified mail subject lines reflected user settings. - Disabled `metamta.one-mail-per-recipient`, verified user "Email Format" panel vanished. - Edited "Email Format" in single-mail-mode in global prefs as an administrator. - Sent more mail, verified mail respected new global settings. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11098 Differential Revision: https://secure.phabricator.com/D16118 --- .../feed/PhabricatorFeedStoryPublisher.php | 1 + .../storage/PhabricatorMetaMTAMail.php | 26 ++++++----------- .../PhabricatorUserPreferencesCacheType.php | 19 ++---------- .../PhabricatorSettingsMainController.php | 4 +++ .../PhabricatorEmailFormatSettingsPanel.php | 8 +++++ .../panel/PhabricatorSettingsPanel.php | 12 ++++++++ .../query/PhabricatorUserPreferencesQuery.php | 29 ++++++++++++++++++- .../setting/PhabricatorEmailFormatSetting.php | 4 --- .../PhabricatorEmailRePrefixSetting.php | 4 --- .../PhabricatorEmailVarySubjectsSetting.php | 4 --- .../storage/PhabricatorUserPreferences.php | 28 ++++++++++-------- 11 files changed, 81 insertions(+), 58 deletions(-) diff --git a/src/applications/feed/PhabricatorFeedStoryPublisher.php b/src/applications/feed/PhabricatorFeedStoryPublisher.php index 3b59edc37b..8d018c61b3 100644 --- a/src/applications/feed/PhabricatorFeedStoryPublisher.php +++ b/src/applications/feed/PhabricatorFeedStoryPublisher.php @@ -215,6 +215,7 @@ final class PhabricatorFeedStoryPublisher extends Phobject { $all_prefs = id(new PhabricatorUserPreferencesQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withUserPHIDs($phids) + ->needSyntheticPreferences(true) ->execute(); $all_prefs = mpull($all_prefs, null, 'getUserPHID'); } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index ceea3f0429..0c90e43832 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -877,6 +877,7 @@ final class PhabricatorMetaMTAMail $all_prefs = id(new PhabricatorUserPreferencesQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withUserPHIDs($actor_phids) + ->needSyntheticPreferences(true) ->execute(); $all_prefs = mpull($all_prefs, null, 'getUserPHID'); @@ -1105,29 +1106,20 @@ final class PhabricatorMetaMTAMail private function loadPreferences($target_phid) { - if (!self::shouldMultiplexAllMail()) { - $target_phid = null; - } + $viewer = PhabricatorUser::getOmnipotentUser(); - if ($target_phid) { + if (self::shouldMultiplexAllMail()) { $preferences = id(new PhabricatorUserPreferencesQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($viewer) ->withUserPHIDs(array($target_phid)) + ->needSyntheticPreferences(true) ->executeOne(); - } else { - $preferences = null; + if ($preferences) { + return $preferences; + } } - // TODO: Here, we would load global preferences once they exist. - - if (!$preferences) { - // If we haven't found suitable preferences yet, return an empty object - // which implicitly has all the default values. - $preferences = id(new PhabricatorUserPreferences()) - ->attachUser(new PhabricatorUser()); - } - - return $preferences; + return PhabricatorUserPreferences::loadGlobalPreferences($viewer); } private function shouldAddRePrefix(PhabricatorUserPreferences $preferences) { diff --git a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php index 015a24cb0f..7fee680def 100644 --- a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php +++ b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php @@ -29,29 +29,16 @@ final class PhabricatorUserPreferencesCacheType $preferences = id(new PhabricatorUserPreferencesQuery()) ->setViewer($viewer) - ->withUserPHIDs($user_phids) + ->withUsers($users) + ->needSyntheticPreferences(true) ->execute(); $preferences = mpull($preferences, null, 'getUserPHID'); - // If some users don't have settings of their own yet, we need to load - // the global default settings to generate caches for them. - if (count($preferences) < count($user_phids)) { - $global = id(new PhabricatorUserPreferencesQuery()) - ->setViewer($viewer) - ->withBuiltinKeys( - array( - PhabricatorUserPreferences::BUILTIN_GLOBAL_DEFAULT, - )) - ->executeOne(); - } else { - $global = null; - } - $all_settings = PhabricatorSetting::getAllSettings(); $settings = array(); foreach ($users as $user_phid => $user) { - $preference = idx($preferences, $user_phid, $global); + $preference = idx($preferences, $user_phid); if (!$preference) { continue; diff --git a/src/applications/settings/controller/PhabricatorSettingsMainController.php b/src/applications/settings/controller/PhabricatorSettingsMainController.php index 2a368d8650..fada4a0937 100644 --- a/src/applications/settings/controller/PhabricatorSettingsMainController.php +++ b/src/applications/settings/controller/PhabricatorSettingsMainController.php @@ -152,6 +152,10 @@ final class PhabricatorSettingsMainController if (!$this->isSelf() && !$panel->isManagementPanel()) { continue; } + + if ($this->isSelf() && !$panel->isUserPanel()) { + continue; + } } if (!empty($result[$key])) { diff --git a/src/applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php b/src/applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php index 5cab5dea41..107816f2eb 100644 --- a/src/applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php @@ -13,7 +13,15 @@ final class PhabricatorEmailFormatSettingsPanel return PhabricatorSettingsEmailPanelGroup::PANELGROUPKEY; } + public function isUserPanel() { + return PhabricatorMetaMTAMail::shouldMultiplexAllMail(); + } + public function isManagementPanel() { + if (!$this->isUserPanel()) { + return false; + } + if ($this->getUser()->getIsMailingList()) { return true; } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanel.php b/src/applications/settings/panel/PhabricatorSettingsPanel.php index b66b03f9c8..7d86eaf243 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanel.php @@ -154,6 +154,18 @@ abstract class PhabricatorSettingsPanel extends Phobject { } + /** + * Return true if this panel is available to users while editing their own + * settings. + * + * @return bool True to enable management on behalf of a user. + * @task config + */ + public function isUserPanel() { + return true; + } + + /** * Return true if this panel is available to administrators while managing * bot and mailing list accounts. diff --git a/src/applications/settings/query/PhabricatorUserPreferencesQuery.php b/src/applications/settings/query/PhabricatorUserPreferencesQuery.php index 280ca3eea6..de4887cbb8 100644 --- a/src/applications/settings/query/PhabricatorUserPreferencesQuery.php +++ b/src/applications/settings/query/PhabricatorUserPreferencesQuery.php @@ -9,6 +9,7 @@ final class PhabricatorUserPreferencesQuery private $builtinKeys; private $hasUserPHID; private $users = array(); + private $synthetic; public function withIDs(array $ids) { $this->ids = $ids; @@ -42,12 +43,38 @@ final class PhabricatorUserPreferencesQuery return $this; } + /** + * Always return preferences for every queried user. + * + * If no settings exist for a user, a new empty settings object with + * appropriate defaults is returned. + * + * @param bool True to generat synthetic preferences for missing users. + */ + public function needSyntheticPreferences($synthetic) { + $this->synthetic = $synthetic; + return $this; + } + public function newResultObject() { return new PhabricatorUserPreferences(); } protected function loadPage() { - return $this->loadStandardPage($this->newResultObject()); + $preferences = $this->loadStandardPage($this->newResultObject()); + + if ($this->synthetic) { + $user_map = mpull($preferences, null, 'getUserPHID'); + foreach ($this->userPHIDs as $user_phid) { + if (isset($user_map[$user_phid])) { + continue; + } + $preferences[] = $this->newResultObject() + ->setUserPHID($user_phid); + } + } + + return $preferences; } protected function willFilterPage(array $prefs) { diff --git a/src/applications/settings/setting/PhabricatorEmailFormatSetting.php b/src/applications/settings/setting/PhabricatorEmailFormatSetting.php index 0cf0db5d74..333d85c6f4 100644 --- a/src/applications/settings/setting/PhabricatorEmailFormatSetting.php +++ b/src/applications/settings/setting/PhabricatorEmailFormatSetting.php @@ -20,10 +20,6 @@ final class PhabricatorEmailFormatSetting return 100; } - protected function isEnabledForViewer(PhabricatorUser $viewer) { - return PhabricatorMetaMTAMail::shouldMultiplexAllMail(); - } - protected function getControlInstructions() { return pht( 'You can opt to receive plain text email from Phabricator instead '. diff --git a/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php b/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php index 7b04ef80c3..5e70b731cd 100644 --- a/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php +++ b/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php @@ -20,10 +20,6 @@ final class PhabricatorEmailRePrefixSetting return 200; } - protected function isEnabledForViewer(PhabricatorUser $viewer) { - return PhabricatorMetaMTAMail::shouldMultiplexAllMail(); - } - protected function getControlInstructions() { return pht( 'The **Add "Re:" Prefix** setting adds "Re:" in front of all messages, '. diff --git a/src/applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php b/src/applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php index 0c6b73907b..1c088d9411 100644 --- a/src/applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php +++ b/src/applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php @@ -20,10 +20,6 @@ final class PhabricatorEmailVarySubjectsSetting return 300; } - protected function isEnabledForViewer(PhabricatorUser $viewer) { - return PhabricatorMetaMTAMail::shouldMultiplexAllMail(); - } - protected function getControlInstructions() { return pht( 'With **Vary Subjects** enabled, most mail subject lines will include '. diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index b03365bd32..5ed360ca2c 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -122,31 +122,35 @@ final class PhabricatorUserPreferences * @param PhabricatorUser User to load or create preferences for. */ public static function loadUserPreferences(PhabricatorUser $user) { - $preferences = id(new PhabricatorUserPreferencesQuery()) + return id(new PhabricatorUserPreferencesQuery()) ->setViewer($user) ->withUsers(array($user)) + ->needSyntheticPreferences(true) ->executeOne(); - if ($preferences) { - return $preferences; - } - - $preferences = id(new self()) - ->setUserPHID($user->getPHID()) - ->attachUser($user); + } + /** + * Load or create a global preferences object. + * + * If no global preferences exist, an empty preferences object is returned. + * + * @param PhabricatorUser Viewing user. + */ + public static function loadGlobalPreferences(PhabricatorUser $viewer) { $global = id(new PhabricatorUserPreferencesQuery()) - ->setViewer($user) + ->setViewer($viewer) ->withBuiltinKeys( array( self::BUILTIN_GLOBAL_DEFAULT, )) ->executeOne(); - if ($global) { - $preferences->attachDefaultSettings($global); + if (!$global) { + $global = id(new self()) + ->attachUser(new PhabricatorUser()); } - return $preferences; + return $global; } public function newTransaction($key, $value) { From cfa73eb5440895908d49080e53c73ddaa00f25b9 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Tue, 14 Jun 2016 12:58:21 -0700 Subject: [PATCH 17/42] Make PhameBlog full text searchable Summary: Ref T9897, makes blogs searchable Test Plan: Make a blog, index it, search for it. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T9897 Differential Revision: https://secure.phabricator.com/D16119 --- src/__phutil_library_map__.php | 3 ++ .../phid/PhabricatorPhameBlogPHIDType.php | 5 ++++ .../phame/search/PhameBlogFulltextEngine.php | 28 +++++++++++++++++++ src/applications/phame/storage/PhameBlog.php | 9 +++++- 4 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 src/applications/phame/search/PhameBlogFulltextEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 971b9be04c..32425de0bb 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3764,6 +3764,7 @@ phutil_register_library_map(array( 'PhameBlogEditEngine' => 'applications/phame/editor/PhameBlogEditEngine.php', 'PhameBlogEditor' => 'applications/phame/editor/PhameBlogEditor.php', 'PhameBlogFeedController' => 'applications/phame/controller/blog/PhameBlogFeedController.php', + 'PhameBlogFulltextEngine' => 'applications/phame/search/PhameBlogFulltextEngine.php', 'PhameBlogListController' => 'applications/phame/controller/blog/PhameBlogListController.php', 'PhameBlogListView' => 'applications/phame/view/PhameBlogListView.php', 'PhameBlogManageController' => 'applications/phame/controller/blog/PhameBlogManageController.php', @@ -8615,6 +8616,7 @@ phutil_register_library_map(array( 'PhabricatorDestructibleInterface', 'PhabricatorApplicationTransactionInterface', 'PhabricatorConduitResultInterface', + 'PhabricatorFulltextInterface', ), 'PhameBlog404Controller' => 'PhameLiveController', 'PhameBlogArchiveController' => 'PhameBlogController', @@ -8625,6 +8627,7 @@ phutil_register_library_map(array( 'PhameBlogEditEngine' => 'PhabricatorEditEngine', 'PhameBlogEditor' => 'PhabricatorApplicationTransactionEditor', 'PhameBlogFeedController' => 'PhameBlogController', + 'PhameBlogFulltextEngine' => 'PhabricatorFulltextEngine', 'PhameBlogListController' => 'PhameBlogController', 'PhameBlogListView' => 'AphrontTagView', 'PhameBlogManageController' => 'PhameBlogController', diff --git a/src/applications/phame/phid/PhabricatorPhameBlogPHIDType.php b/src/applications/phame/phid/PhabricatorPhameBlogPHIDType.php index 18ea30aea8..905f4b893b 100644 --- a/src/applications/phame/phid/PhabricatorPhameBlogPHIDType.php +++ b/src/applications/phame/phid/PhabricatorPhameBlogPHIDType.php @@ -34,6 +34,11 @@ final class PhabricatorPhameBlogPHIDType extends PhabricatorPHIDType { $handle->setName($blog->getName()); $handle->setFullName($blog->getName()); $handle->setURI('/phame/blog/view/'.$blog->getID().'/'); + + if ($blog->isArchived()) { + $handle->setStatus(PhabricatorObjectHandle::STATUS_CLOSED); + } + } } diff --git a/src/applications/phame/search/PhameBlogFulltextEngine.php b/src/applications/phame/search/PhameBlogFulltextEngine.php new file mode 100644 index 0000000000..c25550c655 --- /dev/null +++ b/src/applications/phame/search/PhameBlogFulltextEngine.php @@ -0,0 +1,28 @@ +setDocumentTitle($blog->getName()); + + $document->addField( + PhabricatorSearchDocumentFieldType::FIELD_BODY, + $blog->getDescription()); + + $document->addRelationship( + $blog->isArchived() + ? PhabricatorSearchRelationship::RELATIONSHIP_CLOSED + : PhabricatorSearchRelationship::RELATIONSHIP_OPEN, + $blog->getPHID(), + PhabricatorPhameBlogPHIDType::TYPECONST, + PhabricatorTime::getNow()); + + } + +} diff --git a/src/applications/phame/storage/PhameBlog.php b/src/applications/phame/storage/PhameBlog.php index 861def127b..d4d12df6c8 100644 --- a/src/applications/phame/storage/PhameBlog.php +++ b/src/applications/phame/storage/PhameBlog.php @@ -9,7 +9,8 @@ final class PhameBlog extends PhameDAO PhabricatorProjectInterface, PhabricatorDestructibleInterface, PhabricatorApplicationTransactionInterface, - PhabricatorConduitResultInterface { + PhabricatorConduitResultInterface, + PhabricatorFulltextInterface { const MARKUP_FIELD_DESCRIPTION = 'markup:description'; @@ -370,4 +371,10 @@ final class PhameBlog extends PhameDAO } +/* -( PhabricatorFulltextInterface )--------------------------------------- */ + + public function newFulltextEngine() { + return new PhameBlogFulltextEngine(); + } + } From 695f0b09b23761c813b015c2b62075cbdee1de59 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Tue, 14 Jun 2016 13:31:17 -0700 Subject: [PATCH 18/42] Add supportsSearch to Phame Blog/Post Summary: Flips the bits from true to false in transaction editor. Test Plan: update a post, search for new term Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T9897 Differential Revision: https://secure.phabricator.com/D16120 --- src/applications/phame/editor/PhameBlogEditor.php | 2 +- src/applications/phame/editor/PhamePostEditor.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/applications/phame/editor/PhameBlogEditor.php b/src/applications/phame/editor/PhameBlogEditor.php index a59ede32be..c4ffb7c0f9 100644 --- a/src/applications/phame/editor/PhameBlogEditor.php +++ b/src/applications/phame/editor/PhameBlogEditor.php @@ -227,7 +227,7 @@ final class PhameBlogEditor protected function supportsSearch() { - return false; + return true; } protected function shouldApplyHeraldRules( diff --git a/src/applications/phame/editor/PhamePostEditor.php b/src/applications/phame/editor/PhamePostEditor.php index b10484723d..363f39fb46 100644 --- a/src/applications/phame/editor/PhamePostEditor.php +++ b/src/applications/phame/editor/PhamePostEditor.php @@ -264,7 +264,7 @@ final class PhamePostEditor } protected function supportsSearch() { - return false; + return true; } protected function shouldApplyHeraldRules( From f9a58fafba0d15e043f20e410bbe782357130183 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 14 Jun 2016 13:50:21 -0700 Subject: [PATCH 19/42] Add "video/quicktime" as a default Video MIME type Summary: Ref T11142. H264 video in a Quicktime container works in Safari and Firefox for me (although not Chrome), so include it in the default video mime types. Test Plan: Uploaded video file from T11142 locally, saw it render with `