diff --git a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php index e0f561b97f..d67c3b1589 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php @@ -58,6 +58,9 @@ final class PhabricatorCalendarEventSearchEngine $min_range = $this->getDateFrom($saved)->getEpoch(); $max_range = $this->getDateTo($saved)->getEpoch(); + $user_datasource = id(new PhabricatorPeopleUserFunctionDatasource()) + ->setViewer($viewer); + if ($this->isMonthView($saved) || $this->isDayView($saved)) { list($start_year, $start_month, $start_day) = @@ -124,11 +127,13 @@ final class PhabricatorCalendarEventSearchEngine } $invited_phids = $saved->getParameter('invitedPHIDs'); + $invited_phids = $user_datasource->evaluateTokens($invited_phids); if ($invited_phids) { $query->withInvitedPHIDs($invited_phids); } $creator_phids = $saved->getParameter('creatorPHIDs'); + $creator_phids = $user_datasource->evaluateTokens($creator_phids); if ($creator_phids) { $query->withCreatorPHIDs($creator_phids); } @@ -196,13 +201,13 @@ final class PhabricatorCalendarEventSearchEngine $form ->appendControl( id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorPeopleDatasource()) + ->setDatasource(new PhabricatorPeopleUserFunctionDatasource()) ->setName('creators') ->setLabel(pht('Created By')) ->setValue($creator_phids)) ->appendControl( id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorPeopleDatasource()) + ->setDatasource(new PhabricatorPeopleUserFunctionDatasource()) ->setName('invited') ->setLabel(pht('Invited')) ->setValue($invited_phids)) -- 2.51.2 From 667897639fe27e21e0ed0e60e18b17e719fb3ccc Mon Sep 17 00:00:00 2001 From: lkassianik Date: Tue, 16 Jun 2015 13:38:48 -0700 Subject: [PATCH 02/17] First pass at event modal create options. Summary: Ref T8568, First pass at event modal create options. Test Plan: {nav Calendar > Create} should offer three options: Create Event, Create Public Event, Create Recurring Event. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Maniphest Tasks: T8568 Differential Revision: https://secure.phabricator.com/D13309 --- .../controller/PhabricatorCalendarController.php | 10 +++++++--- .../calendar/storage/PhabricatorCalendarEvent.php | 6 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/applications/calendar/controller/PhabricatorCalendarController.php b/src/applications/calendar/controller/PhabricatorCalendarController.php index a17a96784f..6cba6d8fa7 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarController.php @@ -9,12 +9,16 @@ abstract class PhabricatorCalendarController extends PhabricatorController { ->setUser($this->getViewer()) ->addAction( id(new PhabricatorActionView()) - ->setName(pht('Create Private Event')) - ->setHref('/calendar/event/create/?mode=private')) + ->setName(pht('Create Event')) + ->setHref('/calendar/event/create/')) ->addAction( id(new PhabricatorActionView()) ->setName(pht('Create Public Event')) - ->setHref('/calendar/event/create/?mode=public')); + ->setHref('/calendar/event/create/?mode=public')) + ->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Create Recurring Event')) + ->setHref('/calendar/event/create/?mode=recurring')); $crumbs->addAction( id(new PHUIListItemView()) diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index f12bf9a9c3..2b3fc4bdb8 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -50,10 +50,10 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO if ($mode == 'public') { $view_policy = PhabricatorPolicies::getMostOpenPolicy(); - } else if ($mode == 'recurring') { + } + + if ($mode == 'recurring') { $is_recurring = true; - } else { - $view_policy = $actor->getPHID(); } return id(new PhabricatorCalendarEvent()) -- 2.51.2 From 5240ae8bd1cae65999d0d40d95aa3a9f91a34cc0 Mon Sep 17 00:00:00 2001 From: Paul Kassianik Date: Tue, 16 Jun 2015 13:41:55 -0700 Subject: [PATCH 03/17] Added Instance Number to Ghost Events Summary: Closes T8460. Test Plan: In the Calendar application, create a recursive event. After editing a recursion of the event, verify that the "Recurrence of Event" property is of the form " of ". Reviewers: lpriestley, epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Maniphest Tasks: T8460 Differential Revision: https://secure.phabricator.com/D13307 --- resources/celerity/map.php | 38 +++++++++---------- ...PhabricatorCalendarEventViewController.php | 4 +- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 1d78bcbc69..80107b8d27 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -444,7 +444,7 @@ return array( 'rsrc/js/core/behavior-device.js' => 'a205cf28', 'rsrc/js/core/behavior-drag-and-drop-textarea.js' => '6d49590e', 'rsrc/js/core/behavior-error-log.js' => '6882e80a', - 'rsrc/js/core/behavior-fancy-datepicker.js' => '510b5809', + 'rsrc/js/core/behavior-fancy-datepicker.js' => 'ea5cec5d', 'rsrc/js/core/behavior-file-tree.js' => '88236f00', 'rsrc/js/core/behavior-form.js' => '5c54cbf3', 'rsrc/js/core/behavior-gesture.js' => '3ab51e2c', @@ -468,7 +468,7 @@ return array( 'rsrc/js/core/behavior-scrollbar.js' => '834a1173', 'rsrc/js/core/behavior-search-typeahead.js' => '048330fa', 'rsrc/js/core/behavior-select-on-click.js' => '4e3e79a6', - 'rsrc/js/core/behavior-time-typeahead.js' => '8bfbb401', + 'rsrc/js/core/behavior-time-typeahead.js' => 'f80d6bf0', 'rsrc/js/core/behavior-toggle-class.js' => '5d7c9f33', 'rsrc/js/core/behavior-tokenizer.js' => 'b3a4b884', 'rsrc/js/core/behavior-tooltip.js' => '3ee3408b', @@ -577,7 +577,7 @@ return array( 'javelin-behavior-durable-column' => 'c72aa091', 'javelin-behavior-error-log' => '6882e80a', 'javelin-behavior-event-all-day' => '38dcf3c8', - 'javelin-behavior-fancy-datepicker' => '510b5809', + 'javelin-behavior-fancy-datepicker' => 'ea5cec5d', 'javelin-behavior-global-drag-and-drop' => 'c8e57404', 'javelin-behavior-herald-rule-editor' => '7ebaeed3', 'javelin-behavior-high-security-warning' => 'a464fe03', @@ -645,7 +645,7 @@ return array( 'javelin-behavior-slowvote-embed' => '887ad43f', 'javelin-behavior-stripe-payment-form' => '3f5d6dbf', 'javelin-behavior-test-payment-form' => 'fc91ab6c', - 'javelin-behavior-time-typeahead' => '8bfbb401', + 'javelin-behavior-time-typeahead' => 'f80d6bf0', 'javelin-behavior-toggle-class' => '5d7c9f33', 'javelin-behavior-typeahead-browse' => '635de1ec', 'javelin-behavior-typeahead-search' => '93d0c9e3', @@ -1168,13 +1168,6 @@ return array( 'javelin-typeahead-source', 'javelin-util', ), - '510b5809' => array( - 'javelin-behavior', - 'javelin-util', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-vector', - ), '519705ea' => array( 'javelin-install', 'javelin-dom', @@ -1502,14 +1495,6 @@ return array( 'javelin-request', 'javelin-typeahead-source', ), - '8bfbb401' => array( - 'javelin-behavior', - 'javelin-util', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-vector', - 'javelin-typeahead-static-source', - ), '8ce821c5' => array( 'phabricator-notification', 'javelin-stratcom', @@ -1950,6 +1935,13 @@ return array( 'javelin-dom', 'phabricator-draggable-list', ), + 'ea5cec5d' => array( + 'javelin-behavior', + 'javelin-util', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-vector', + ), 'ea681761' => array( 'javelin-behavior', 'javelin-aphlict', @@ -1995,6 +1987,14 @@ return array( 'javelin-typeahead-ondemand-source', 'javelin-util', ), + 'f80d6bf0' => array( + 'javelin-behavior', + 'javelin-util', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-vector', + 'javelin-typeahead-static-source', + ), 'f829edb3' => array( 'javelin-view', 'javelin-install', diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php index 7409bc5586..b6eb901bb8 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php @@ -291,7 +291,9 @@ final class PhabricatorCalendarEventViewController if ($event->getInstanceOfEventPHID()) { $properties->addProperty( pht('Recurrence of Event'), - $viewer->renderHandle($event->getInstanceOfEventPHID())); + pht('%s of %s', + $event->getSequenceIndex(), + $viewer->renderHandle($event->getInstanceOfEventPHID())->render())); } } -- 2.51.2 From 9537f983f6c03a0b386332b3de71031c00d7d552 Mon Sep 17 00:00:00 2001 From: Paul Kassianik Date: Tue, 16 Jun 2015 13:44:51 -0700 Subject: [PATCH 04/17] Added a Note Credential Type for Passphrase Summary: Closes T8481. Test Plan: Verify that in Passphrase > Create an option to create a Note credential exists and credentials of type Note are createable. Reviewers: epriestley, #blessed_reviewers, eadler, lpriestley Reviewed By: eadler Subscribers: joshuaspence, epriestley, Korvin Maniphest Tasks: T8481 Differential Revision: https://secure.phabricator.com/D13261 --- src/__phutil_library_map__.php | 2 + .../PassphraseCredentialEditController.php | 16 +++++--- .../PassphraseCredentialViewController.php | 8 ++-- .../PassphraseCredentialType.php | 4 ++ .../PassphraseNoteCredentialType.php | 37 +++++++++++++++++++ .../PassphraseCredentialTransactionEditor.php | 4 ++ 6 files changed, 62 insertions(+), 9 deletions(-) create mode 100644 src/applications/passphrase/credentialtype/PassphraseNoteCredentialType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2b435ef161..6a45078f42 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1271,6 +1271,7 @@ phutil_register_library_map(array( 'PassphraseCredentialTypeTestCase' => 'applications/passphrase/credentialtype/__tests__/PassphraseCredentialTypeTestCase.php', 'PassphraseCredentialViewController' => 'applications/passphrase/controller/PassphraseCredentialViewController.php', 'PassphraseDAO' => 'applications/passphrase/storage/PassphraseDAO.php', + 'PassphraseNoteCredentialType' => 'applications/passphrase/credentialtype/PassphraseNoteCredentialType.php', 'PassphrasePasswordCredentialType' => 'applications/passphrase/credentialtype/PassphrasePasswordCredentialType.php', 'PassphrasePasswordKey' => 'applications/passphrase/keys/PassphrasePasswordKey.php', 'PassphraseQueryConduitAPIMethod' => 'applications/passphrase/conduit/PassphraseQueryConduitAPIMethod.php', @@ -4766,6 +4767,7 @@ phutil_register_library_map(array( 'PassphraseCredentialTypeTestCase' => 'PhabricatorTestCase', 'PassphraseCredentialViewController' => 'PassphraseController', 'PassphraseDAO' => 'PhabricatorLiskDAO', + 'PassphraseNoteCredentialType' => 'PassphraseCredentialType', 'PassphrasePasswordCredentialType' => 'PassphraseCredentialType', 'PassphrasePasswordKey' => 'PassphraseAbstractKey', 'PassphraseQueryConduitAPIMethod' => 'PassphraseConduitAPIMethod', diff --git a/src/applications/passphrase/controller/PassphraseCredentialEditController.php b/src/applications/passphrase/controller/PassphraseCredentialEditController.php index 9732808aa9..abde525382 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialEditController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialEditController.php @@ -47,7 +47,7 @@ final class PassphraseCredentialEditController extends PassphraseController { $is_new = true; // Prefill username if provided. - $credential->setUsername($request->getStr('username')); + $credential->setUsername((string)$request->getStr('username')); if (!$request->getStr('isInitialized')) { $type->didInitializeNewCredential($viewer, $credential); @@ -151,10 +151,11 @@ final class PassphraseCredentialEditController extends PassphraseController { $credential->openTransaction(); if (!$credential->getIsLocked()) { - $xactions[] = id(new PassphraseCredentialTransaction()) + if ($type->shouldRequireUsername()) { + $xactions[] = id(new PassphraseCredentialTransaction()) ->setTransactionType($type_username) ->setNewValue($v_username); - + } // If some value other than a sequence of bullets was provided for // the credential, update it. In particular, note that we are // explicitly allowing empty secrets: one use case is HTTP auth where @@ -263,15 +264,18 @@ final class PassphraseCredentialEditController extends PassphraseController { pht('This credential is permanently locked and can not be edited.')); } - $form + if ($type->shouldRequireUsername()) { + $form ->appendChild( id(new AphrontFormTextControl()) ->setName('username') ->setLabel(pht('Login/Username')) ->setValue($v_username) ->setDisabled($credential_is_locked) - ->setError($e_username)) - ->appendChild( + ->setError($e_username)); + } + $form + ->appendChild( $secret_control ->setName('secret') ->setLabel($type->getSecretLabel()) diff --git a/src/applications/passphrase/controller/PassphraseCredentialViewController.php b/src/applications/passphrase/controller/PassphraseCredentialViewController.php index ad39db96ba..f9cc193dbf 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialViewController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialViewController.php @@ -182,9 +182,11 @@ final class PassphraseCredentialViewController extends PassphraseController { pht('Editable By'), $descriptions[PhabricatorPolicyCapability::CAN_EDIT]); - $properties->addProperty( - pht('Username'), - $credential->getUsername()); + if ($type->shouldRequireUsername()) { + $properties->addProperty( + pht('Username'), + $credential->getUsername()); + } $used_by_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( $credential->getPHID(), diff --git a/src/applications/passphrase/credentialtype/PassphraseCredentialType.php b/src/applications/passphrase/credentialtype/PassphraseCredentialType.php index fa5f54edb6..2dd22a0aa4 100644 --- a/src/applications/passphrase/credentialtype/PassphraseCredentialType.php +++ b/src/applications/passphrase/credentialtype/PassphraseCredentialType.php @@ -131,4 +131,8 @@ abstract class PassphraseCredentialType extends Phobject { return $secret; } + public function shouldRequireUsername() { + return true; + } + } diff --git a/src/applications/passphrase/credentialtype/PassphraseNoteCredentialType.php b/src/applications/passphrase/credentialtype/PassphraseNoteCredentialType.php new file mode 100644 index 0000000000..ff835bd9d8 --- /dev/null +++ b/src/applications/passphrase/credentialtype/PassphraseNoteCredentialType.php @@ -0,0 +1,37 @@ +getCredentialTypeImplementation(); + if (!$credential_type->shouldRequireUsername()) { + break; + } $missing = $this->validateIsEmptyTextField( $object->getUsername(), $xactions); -- 2.51.2 From c5c5523c1ff837ed79f0ee1757978d9da64a162f Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 16 Jun 2015 13:55:17 -0700 Subject: [PATCH 05/17] Add missing $mailer property to PhabricatorMailImplementationPHPMailerAdapter Fixes T8541. Auditors: joshuaspence --- .../adapter/PhabricatorMailImplementationPHPMailerAdapter.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php index d9609b1a7b..8f7e809d6c 100644 --- a/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php @@ -3,6 +3,8 @@ final class PhabricatorMailImplementationPHPMailerAdapter extends PhabricatorMailImplementationAdapter { + private $mailer; + /** * @phutil-external-symbol class PHPMailer */ -- 2.51.2 From 6b7d7401ca6d23f5f93331d17a4182a8dd744ca5 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Wed, 17 Jun 2015 07:09:53 +1000 Subject: [PATCH 06/17] Modernize Diviner Summary: Ref T4558. This diff modernizes the #diviner application. Basically: - Add an edit controller, accessible at `/book/$BOOK/edit/`. - Add edit/view policies. - Added an action menu to the `DivinerBookController` to expose the edit interface. - Allows projects to be associated with books. - Implement edges and transactions. - Implemented `PhabricatorApplicationTransactionInterface` in `DivinerLiveBook`. Test Plan: - Generated a Diviner book with `./bin/diviner generate`. - Added projects to a book and ensured that they persisted. - Changed the view policy on a book and made sure it was effective. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T4558 Differential Revision: https://secure.phabricator.com/D13091 --- .../autopatches/20150605.diviner.edges.sql | 17 +++ .../20150605.diviner.editPolicy.sql | 6 + .../autopatches/20150605.diviner.xaction.sql | 19 +++ src/__phutil_library_map__.php | 16 +++ .../PhabricatorDivinerApplication.php | 13 ++ .../atomizer/DivinerArticleAtomizer.php | 2 +- .../diviner/atomizer/DivinerPHPAtomizer.php | 22 ++-- .../diviner/cache/DivinerAtomCache.php | 1 + .../diviner/cache/DivinerDiskCache.php | 4 +- .../diviner/cache/DivinerPublishCache.php | 1 + .../DivinerDefaultEditCapability.php | 11 ++ .../DivinerDefaultViewCapability.php | 15 +++ .../controller/DivinerAtomController.php | 41 +++--- .../controller/DivinerAtomListController.php | 11 +- .../controller/DivinerBookController.php | 51 ++++++-- .../controller/DivinerBookEditController.php | 117 ++++++++++++++++++ .../diviner/controller/DivinerController.php | 15 +-- .../controller/DivinerFindController.php | 12 +- .../controller/DivinerMainController.php | 33 +++-- .../diviner/editor/DivinerLiveBookEditor.php | 23 ++++ .../diviner/phid/DivinerAtomPHIDType.php | 4 + .../diviner/phid/DivinerBookPHIDType.php | 4 + .../publisher/DivinerLivePublisher.php | 7 +- .../diviner/publisher/DivinerPublisher.php | 14 ++- .../diviner/query/DivinerAtomQuery.php | 17 ++- .../diviner/query/DivinerBookQuery.php | 31 +++++ .../query/DivinerLiveBookTransactionQuery.php | 10 ++ .../renderer/DivinerDefaultRenderer.php | 5 +- .../search/DivinerBookSearchIndexer.php | 5 + .../diviner/storage/DivinerLiveBook.php | 53 +++++++- .../storage/DivinerLiveBookTransaction.php | 18 +++ .../diviner/storage/DivinerLiveSymbol.php | 25 ++-- .../diviner/storage/DivinerSchemaSpec.php | 9 ++ .../diviner/view/DivinerBookItemView.php | 18 +-- .../workflow/DivinerAtomizeWorkflow.php | 8 +- .../workflow/DivinerGenerateWorkflow.php | 48 +++++-- .../PhabricatorUSEnglishTranslation.php | 46 +++++++ 37 files changed, 601 insertions(+), 151 deletions(-) create mode 100644 resources/sql/autopatches/20150605.diviner.edges.sql create mode 100644 resources/sql/autopatches/20150605.diviner.editPolicy.sql create mode 100644 resources/sql/autopatches/20150605.diviner.xaction.sql create mode 100644 src/applications/diviner/capability/DivinerDefaultEditCapability.php create mode 100644 src/applications/diviner/capability/DivinerDefaultViewCapability.php create mode 100644 src/applications/diviner/controller/DivinerBookEditController.php create mode 100644 src/applications/diviner/editor/DivinerLiveBookEditor.php create mode 100644 src/applications/diviner/query/DivinerLiveBookTransactionQuery.php create mode 100644 src/applications/diviner/storage/DivinerLiveBookTransaction.php create mode 100644 src/applications/diviner/storage/DivinerSchemaSpec.php diff --git a/resources/sql/autopatches/20150605.diviner.edges.sql b/resources/sql/autopatches/20150605.diviner.edges.sql new file mode 100644 index 0000000000..b0e1d61705 --- /dev/null +++ b/resources/sql/autopatches/20150605.diviner.edges.sql @@ -0,0 +1,17 @@ +CREATE TABLE {$NAMESPACE}_diviner.edge ( + src VARBINARY(64) NOT NULL, + type INT UNSIGNED NOT NULL, + dst VARBINARY(64) NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + seq INT UNSIGNED NOT NULL, + dataID INT UNSIGNED, + + PRIMARY KEY (src, type, dst), + KEY src (src, type, dateCreated, seq), + UNIQUE KEY key_dst (dst, type, src) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; + +CREATE TABLE {$NAMESPACE}_diviner.edgedata ( + id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT, + data LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT} +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20150605.diviner.editPolicy.sql b/resources/sql/autopatches/20150605.diviner.editPolicy.sql new file mode 100644 index 0000000000..8c960a4f07 --- /dev/null +++ b/resources/sql/autopatches/20150605.diviner.editPolicy.sql @@ -0,0 +1,6 @@ +ALTER TABLE {$NAMESPACE}_diviner.diviner_livebook + ADD COLUMN editPolicy VARBINARY(64) NOT NULL AFTER viewPolicy; + +UPDATE {$NAMESPACE}_diviner.diviner_livebook + SET editPolicy = 'admin' + WHERE editPolicy = ''; diff --git a/resources/sql/autopatches/20150605.diviner.xaction.sql b/resources/sql/autopatches/20150605.diviner.xaction.sql new file mode 100644 index 0000000000..33f9b5d313 --- /dev/null +++ b/resources/sql/autopatches/20150605.diviner.xaction.sql @@ -0,0 +1,19 @@ +CREATE TABLE {$NAMESPACE}_diviner.diviner_livebooktransaction ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + phid VARBINARY(64) NOT NULL, + authorPHID VARBINARY(64) NOT NULL, + objectPHID VARBINARY(64) NOT NULL, + viewPolicy VARBINARY(64) NOT NULL, + editPolicy VARBINARY(64) NOT NULL, + commentPHID VARBINARY(64) DEFAULT NULL, + commentVersion INT UNSIGNED NOT NULL, + transactionType VARCHAR(32) COLLATE {$COLLATE_TEXT} NOT NULL, + oldValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + newValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + contentSource LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + metadata LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + UNIQUE KEY key_phid (phid), + KEY key_object (objectPHID) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6a45078f42..fc465aeea6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -649,19 +649,25 @@ phutil_register_library_map(array( 'DivinerAtomizeWorkflow' => 'applications/diviner/workflow/DivinerAtomizeWorkflow.php', 'DivinerAtomizer' => 'applications/diviner/atomizer/DivinerAtomizer.php', 'DivinerBookController' => 'applications/diviner/controller/DivinerBookController.php', + 'DivinerBookEditController' => 'applications/diviner/controller/DivinerBookEditController.php', 'DivinerBookItemView' => 'applications/diviner/view/DivinerBookItemView.php', 'DivinerBookPHIDType' => 'applications/diviner/phid/DivinerBookPHIDType.php', 'DivinerBookQuery' => 'applications/diviner/query/DivinerBookQuery.php', 'DivinerBookSearchIndexer' => 'applications/diviner/search/DivinerBookSearchIndexer.php', 'DivinerController' => 'applications/diviner/controller/DivinerController.php', 'DivinerDAO' => 'applications/diviner/storage/DivinerDAO.php', + 'DivinerDefaultEditCapability' => 'applications/diviner/capability/DivinerDefaultEditCapability.php', 'DivinerDefaultRenderer' => 'applications/diviner/renderer/DivinerDefaultRenderer.php', + 'DivinerDefaultViewCapability' => 'applications/diviner/capability/DivinerDefaultViewCapability.php', 'DivinerDiskCache' => 'applications/diviner/cache/DivinerDiskCache.php', 'DivinerFileAtomizer' => 'applications/diviner/atomizer/DivinerFileAtomizer.php', 'DivinerFindController' => 'applications/diviner/controller/DivinerFindController.php', 'DivinerGenerateWorkflow' => 'applications/diviner/workflow/DivinerGenerateWorkflow.php', 'DivinerLiveAtom' => 'applications/diviner/storage/DivinerLiveAtom.php', 'DivinerLiveBook' => 'applications/diviner/storage/DivinerLiveBook.php', + 'DivinerLiveBookEditor' => 'applications/diviner/editor/DivinerLiveBookEditor.php', + 'DivinerLiveBookTransaction' => 'applications/diviner/storage/DivinerLiveBookTransaction.php', + 'DivinerLiveBookTransactionQuery' => 'applications/diviner/query/DivinerLiveBookTransactionQuery.php', 'DivinerLivePublisher' => 'applications/diviner/publisher/DivinerLivePublisher.php', 'DivinerLiveSymbol' => 'applications/diviner/storage/DivinerLiveSymbol.php', 'DivinerMainController' => 'applications/diviner/controller/DivinerMainController.php', @@ -671,6 +677,7 @@ phutil_register_library_map(array( 'DivinerPublisher' => 'applications/diviner/publisher/DivinerPublisher.php', 'DivinerRenderer' => 'applications/diviner/renderer/DivinerRenderer.php', 'DivinerReturnTableView' => 'applications/diviner/view/DivinerReturnTableView.php', + 'DivinerSchemaSpec' => 'applications/diviner/storage/DivinerSchemaSpec.php', 'DivinerSectionView' => 'applications/diviner/view/DivinerSectionView.php', 'DivinerStaticPublisher' => 'applications/diviner/publisher/DivinerStaticPublisher.php', 'DivinerSymbolRemarkupRule' => 'applications/diviner/markup/DivinerSymbolRemarkupRule.php', @@ -4010,13 +4017,16 @@ phutil_register_library_map(array( 'DivinerAtomizeWorkflow' => 'DivinerWorkflow', 'DivinerAtomizer' => 'Phobject', 'DivinerBookController' => 'DivinerController', + 'DivinerBookEditController' => 'DivinerController', 'DivinerBookItemView' => 'AphrontTagView', 'DivinerBookPHIDType' => 'PhabricatorPHIDType', 'DivinerBookQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DivinerBookSearchIndexer' => 'PhabricatorSearchDocumentIndexer', 'DivinerController' => 'PhabricatorController', 'DivinerDAO' => 'PhabricatorLiskDAO', + 'DivinerDefaultEditCapability' => 'PhabricatorPolicyCapability', 'DivinerDefaultRenderer' => 'DivinerRenderer', + 'DivinerDefaultViewCapability' => 'PhabricatorPolicyCapability', 'DivinerDiskCache' => 'Phobject', 'DivinerFileAtomizer' => 'DivinerAtomizer', 'DivinerFindController' => 'DivinerController', @@ -4025,8 +4035,13 @@ phutil_register_library_map(array( 'DivinerLiveBook' => array( 'DivinerDAO', 'PhabricatorPolicyInterface', + 'PhabricatorProjectInterface', 'PhabricatorDestructibleInterface', + 'PhabricatorApplicationTransactionInterface', ), + 'DivinerLiveBookEditor' => 'PhabricatorApplicationTransactionEditor', + 'DivinerLiveBookTransaction' => 'PhabricatorApplicationTransaction', + 'DivinerLiveBookTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'DivinerLivePublisher' => 'DivinerPublisher', 'DivinerLiveSymbol' => array( 'DivinerDAO', @@ -4041,6 +4056,7 @@ phutil_register_library_map(array( 'DivinerPublisher' => 'Phobject', 'DivinerRenderer' => 'Phobject', 'DivinerReturnTableView' => 'AphrontTagView', + 'DivinerSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'DivinerSectionView' => 'AphrontTagView', 'DivinerStaticPublisher' => 'DivinerPublisher', 'DivinerSymbolRemarkupRule' => 'PhutilRemarkupRule', diff --git a/src/applications/diviner/application/PhabricatorDivinerApplication.php b/src/applications/diviner/application/PhabricatorDivinerApplication.php index 68b569213a..4d0691d990 100644 --- a/src/applications/diviner/application/PhabricatorDivinerApplication.php +++ b/src/applications/diviner/application/PhabricatorDivinerApplication.php @@ -39,6 +39,7 @@ final class PhabricatorDivinerApplication extends PhabricatorApplication { 'find/' => 'DivinerFindController', ), '/book/(?P[^/]+)/' => 'DivinerBookController', + '/book/(?P[^/]+)/edit/' => 'DivinerBookEditController', '/book/'. '(?P[^/]+)/'. '(?P[^/]+)/'. @@ -52,6 +53,18 @@ final class PhabricatorDivinerApplication extends PhabricatorApplication { return self::GROUP_UTILITIES; } + protected function getCustomCapabilities() { + return array( + DivinerDefaultViewCapability::CAPABILITY => array( + 'template' => DivinerBookPHIDType::TYPECONST, + ), + DivinerDefaultEditCapability::CAPABILITY => array( + 'default' => PhabricatorPolicies::POLICY_ADMIN, + 'template' => DivinerBookPHIDType::TYPECONST, + ), + ); + } + public function getRemarkupRules() { return array( new DivinerSymbolRemarkupRule(), diff --git a/src/applications/diviner/atomizer/DivinerArticleAtomizer.php b/src/applications/diviner/atomizer/DivinerArticleAtomizer.php index e973a49447..bf375640f6 100644 --- a/src/applications/diviner/atomizer/DivinerArticleAtomizer.php +++ b/src/applications/diviner/atomizer/DivinerArticleAtomizer.php @@ -20,7 +20,7 @@ final class DivinerArticleAtomizer extends DivinerAtomizer { $atom->setDocblockMetaValue('title', $title); } - // If the article has no @name, use the filename after stripping any + // If the article has no `@name`, use the filename after stripping any // extension. $name = idx($meta, 'name'); if (!$name) { diff --git a/src/applications/diviner/atomizer/DivinerPHPAtomizer.php b/src/applications/diviner/atomizer/DivinerPHPAtomizer.php index b964f7961f..c22cb4bcc3 100644 --- a/src/applications/diviner/atomizer/DivinerPHPAtomizer.php +++ b/src/applications/diviner/atomizer/DivinerPHPAtomizer.php @@ -151,9 +151,9 @@ final class DivinerPHPAtomizer extends DivinerAtomizer { if (count($docs) < count($params)) { $atom->addWarning( pht( - 'This call takes %d parameters, but only %d are documented.', - count($params), - count($docs))); + 'This call takes %s parameter(s), but only %s are documented.', + new PhutilNumber(count($params)), + new PhutilNumber(count($docs)))); } } @@ -212,13 +212,13 @@ final class DivinerPHPAtomizer extends DivinerAtomizer { if (preg_match('/@(return|param|task|author)/', $value, $matches)) { $atom->addWarning( pht( - 'Atom "%s" is preceded by a comment containing "@%s", but the '. - 'comment is not a documentation comment. Documentation '. - 'comments must begin with "%s", followed by a newline. Did '. + 'Atom "%s" is preceded by a comment containing `%s`, but '. + 'the comment is not a documentation comment. Documentation '. + 'comments must begin with `%s`, followed by a newline. Did '. 'you mean to use a documentation comment? (As the comment is '. 'not a documentation comment, it will be ignored.)', $atom->getName(), - $matches[1], + '@'.$matches[1], '/**')); } } @@ -248,8 +248,8 @@ final class DivinerPHPAtomizer extends DivinerAtomizer { if ($matches[1] !== $name) { $atom->addWarning( pht( - 'Parameter "%s" is named "%s" in the documentation. The '. - 'documentation may be out of date.', + 'Parameter "%s" is named "%s" in the documentation. '. + 'The documentation may be out of date.', $name, $matches[1])); } @@ -292,8 +292,8 @@ final class DivinerPHPAtomizer extends DivinerAtomizer { if ($return) { $atom->addWarning( pht( - 'Method %s has explicitly documented %s. The %s method always '. - 'returns %s. Diviner documents this implicitly.', + 'Method `%s` has explicitly documented `%s`. The `%s` method '. + 'always returns `%s`. Diviner documents this implicitly.', '__construct()', '@return', '__construct()', diff --git a/src/applications/diviner/cache/DivinerAtomCache.php b/src/applications/diviner/cache/DivinerAtomCache.php index 4b3ca32b79..b37bd71419 100644 --- a/src/applications/diviner/cache/DivinerAtomCache.php +++ b/src/applications/diviner/cache/DivinerAtomCache.php @@ -18,6 +18,7 @@ final class DivinerAtomCache extends DivinerDiskCache { public function delete() { parent::delete(); + $this->fileHashMap = null; $this->atomMap = null; $this->atoms = array(); diff --git a/src/applications/diviner/cache/DivinerDiskCache.php b/src/applications/diviner/cache/DivinerDiskCache.php index 9f8cc60c9d..f605796c0c 100644 --- a/src/applications/diviner/cache/DivinerDiskCache.php +++ b/src/applications/diviner/cache/DivinerDiskCache.php @@ -26,8 +26,8 @@ abstract class DivinerDiskCache extends Phobject { * Convert a long-form hash key like `ccbbaaaaaaaaaaaaaaaaaaaaaaaaaaaaN` into * a shortened directory form, like `cc/bb/aaaaaaaaN`. In conjunction with * @{class:PhutilDirectoryKeyValueCache}, this gives us nice directories - * inside .divinercache instead of a million hash files with huge names at - * top level. + * inside `.divinercache` instead of a million hash files with huge names at + * the top level. */ protected function getHashKey($hash) { return implode( diff --git a/src/applications/diviner/cache/DivinerPublishCache.php b/src/applications/diviner/cache/DivinerPublishCache.php index 38ee1aa3b5..e1bff354dd 100644 --- a/src/applications/diviner/cache/DivinerPublishCache.php +++ b/src/applications/diviner/cache/DivinerPublishCache.php @@ -45,6 +45,7 @@ final class DivinerPublishCache extends DivinerDiskCache { /* -( Index )-------------------------------------------------------------- */ + public function getIndex() { if ($this->index === null) { $this->index = $this->getCache()->getKey('index', array()); diff --git a/src/applications/diviner/capability/DivinerDefaultEditCapability.php b/src/applications/diviner/capability/DivinerDefaultEditCapability.php new file mode 100644 index 0000000000..fcb93c0323 --- /dev/null +++ b/src/applications/diviner/capability/DivinerDefaultEditCapability.php @@ -0,0 +1,11 @@ +bookName = $data['book']; - $this->atomType = $data['type']; - $this->atomName = $data['name']; - $this->atomContext = nonempty(idx($data, 'context'), null); - $this->atomIndex = nonempty(idx($data, 'index'), null); - } - - public function processRequest() { - $request = $this->getRequest(); + public function handleRequest(AphrontRequest $request) { $viewer = $request->getUser(); + $book_name = $request->getURIData('book'); + $atom_type = $request->getURIData('type'); + $atom_name = $request->getURIData('name'); + $atom_context = nonempty($request->getURIData('context'), null); + $atom_index = nonempty($request->getURIData('index'), null); + require_celerity_resource('diviner-shared-css'); $book = id(new DivinerBookQuery()) ->setViewer($viewer) - ->withNames(array($this->bookName)) + ->withNames(array($book_name)) ->executeOne(); if (!$book) { @@ -38,10 +29,10 @@ final class DivinerAtomController extends DivinerController { $symbol = id(new DivinerAtomQuery()) ->setViewer($viewer) ->withBookPHIDs(array($book->getPHID())) - ->withTypes(array($this->atomType)) - ->withNames(array($this->atomName)) - ->withContexts(array($this->atomContext)) - ->withIndexes(array($this->atomIndex)) + ->withTypes(array($atom_type)) + ->withNames(array($atom_name)) + ->withContexts(array($atom_context)) + ->withIndexes(array($atom_index)) ->withIsDocumentable(true) ->needAtoms(true) ->needExtends(true) @@ -75,7 +66,7 @@ final class DivinerAtomController extends DivinerController { ->setName(DivinerAtom::getAtomTypeNameString( $atom ? $atom->getType() : $symbol->getType()))); - $properties = id(new PHUIPropertyListView()); + $properties = new PHUIPropertyListView(); $group = $atom ? $atom->getProperty('group') : $symbol->getGroupName(); if ($group) { @@ -134,9 +125,7 @@ final class DivinerAtomController extends DivinerController { $document->appendChild( id(new PHUIInfoView()) ->setSeverity(PHUIInfoView::SEVERITY_NOTICE) - ->appendChild( - pht( - 'This atom no longer exists.'))); + ->appendChild(pht('This atom no longer exists.'))); } if ($atom) { @@ -304,7 +293,6 @@ final class DivinerAtomController extends DivinerController { pht('Implements'), phutil_implode_html(phutil_tag('br'), $items)); } - } private function renderAtomTag(DivinerLiveSymbol $symbol) { @@ -471,6 +459,7 @@ final class DivinerAtomController extends DivinerController { private function renderFullSignature( DivinerLiveSymbol $symbol, $is_link = false) { + switch ($symbol->getType()) { case DivinerAtom::TYPE_CLASS: case DivinerAtom::TYPE_INTERFACE: diff --git a/src/applications/diviner/controller/DivinerAtomListController.php b/src/applications/diviner/controller/DivinerAtomListController.php index 68d1f6b652..209fdfb9ea 100644 --- a/src/applications/diviner/controller/DivinerAtomListController.php +++ b/src/applications/diviner/controller/DivinerAtomListController.php @@ -2,20 +2,15 @@ final class DivinerAtomListController extends DivinerController { - private $key; - public function shouldAllowPublic() { return true; } - public function willProcessRequest(array $data) { - $this->key = idx($data, 'key', 'all'); - } + public function handleRequest(AphrontRequest $request) { + $query_key = $request->getURIData('key'); - public function processRequest() { - $request = $this->getRequest(); $controller = id(new PhabricatorApplicationSearchController()) - ->setQueryKey($this->key) + ->setQueryKey($query_key) ->setSearchEngine(new DivinerAtomSearchEngine()) ->setNavigation($this->buildSideNavView()); diff --git a/src/applications/diviner/controller/DivinerBookController.php b/src/applications/diviner/controller/DivinerBookController.php index 125edbb6ab..f8d222b116 100644 --- a/src/applications/diviner/controller/DivinerBookController.php +++ b/src/applications/diviner/controller/DivinerBookController.php @@ -2,41 +2,46 @@ final class DivinerBookController extends DivinerController { - private $bookName; - public function shouldAllowPublic() { return true; } - public function willProcessRequest(array $data) { - $this->bookName = $data['book']; - } + public function handleRequest(AphrontRequest $request) { + $viewer = $request->getViewer(); - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $book_name = $request->getURIData('book'); $book = id(new DivinerBookQuery()) ->setViewer($viewer) - ->withNames(array($this->bookName)) + ->withNames(array($book_name)) ->executeOne(); if (!$book) { return new Aphront404Response(); } + $actions = $this->buildActionView($viewer, $book); + $crumbs = $this->buildApplicationCrumbs(); $crumbs->setBorder(true); - $crumbs->addTextCrumb( $book->getShortTitle(), '/book/'.$book->getName().'/'); + $action_button = id(new PHUIButtonView()) + ->setTag('a') + ->setText(pht('Actions')) + ->setHref('#') + ->setIconFont('fa-bars') + ->addClass('phui-mobile-menu') + ->setDropdownMenu($actions); + $header = id(new PHUIHeaderView()) ->setHeader($book->getTitle()) ->setUser($viewer) ->setPolicyObject($book) - ->setEpoch($book->getDateModified()); + ->setEpoch($book->getDateModified()) + ->addActionLink($action_button); $document = new PHUIDocumentView(); $document->setHeader($header); @@ -100,4 +105,28 @@ final class DivinerBookController extends DivinerController { )); } + private function buildActionView( + PhabricatorUser $user, + DivinerLiveBook $book) { + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $user, + $book, + PhabricatorPolicyCapability::CAN_EDIT); + + $action_view = id(new PhabricatorActionListView()) + ->setUser($user) + ->setObject($book) + ->setObjectURI($this->getRequest()->getRequestURI()); + + $action_view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Edit Book')) + ->setIcon('fa-pencil') + ->setHref('/book/'.$book->getName().'/edit/') + ->setDisabled(!$can_edit)); + + return $action_view; + } + } diff --git a/src/applications/diviner/controller/DivinerBookEditController.php b/src/applications/diviner/controller/DivinerBookEditController.php new file mode 100644 index 0000000000..3ac8c4fe2e --- /dev/null +++ b/src/applications/diviner/controller/DivinerBookEditController.php @@ -0,0 +1,117 @@ +getViewer(); + + $book_name = $request->getURIData('book'); + + $book = id(new DivinerBookQuery()) + ->setViewer($viewer) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->needProjectPHIDs(true) + ->withNames(array($book_name)) + ->executeOne(); + + if (!$book) { + return new Aphront404Response(); + } + + $view_uri = '/book/'.$book->getName().'/'; + + if ($request->isFormPost()) { + $v_projects = $request->getArr('projectPHIDs'); + $v_view = $request->getStr('viewPolicy'); + $v_edit = $request->getStr('editPolicy'); + + $xactions = array(); + $xactions[] = id(new DivinerLiveBookTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) + ->setMetadataValue( + 'edge:type', + PhabricatorProjectObjectHasProjectEdgeType::EDGECONST) + ->setNewValue( + array( + '=' => array_fuse($v_projects), + )); + $xactions[] = id(new DivinerLiveBookTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY) + ->setNewValue($v_view); + $xactions[] = id(new DivinerLiveBookTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_EDIT_POLICY) + ->setNewValue($v_edit); + + id(new DivinerLiveBookEditor()) + ->setContinueOnNoEffect(true) + ->setContentSourceFromRequest($request) + ->setActor($viewer) + ->applyTransactions($book, $xactions); + + return id(new AphrontRedirectResponse())->setURI($view_uri); + } + + $crumbs = $this->buildApplicationCrumbs(); + $crumbs->addTextCrumb(pht('Edit Basics')); + + $title = pht('Edit %s', $book->getTitle()); + + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->setObject($book) + ->execute(); + $view_capability = PhabricatorPolicyCapability::CAN_VIEW; + $edit_capability = PhabricatorPolicyCapability::CAN_EDIT; + + $form = id(new AphrontFormView()) + ->setUser($viewer) + ->appendControl( + id(new AphrontFormTokenizerControl()) + ->setDatasource(new PhabricatorProjectDatasource()) + ->setName('projectPHIDs') + ->setLabel(pht('Projects')) + ->setValue($book->getProjectPHIDs())) + ->appendChild( + id(new AphrontFormPolicyControl()) + ->setName('viewPolicy') + ->setPolicyObject($book) + ->setCapability($view_capability) + ->setPolicies($policies) + ->setCaption($book->describeAutomaticCapability($view_capability))) + ->appendChild( + id(new AphrontFormPolicyControl()) + ->setName('editPolicy') + ->setPolicyObject($book) + ->setCapability($edit_capability) + ->setPolicies($policies) + ->setCaption($book->describeAutomaticCapability($edit_capability))) + ->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue(pht('Save')) + ->addCancelButton($view_uri)); + + $object_box = id(new PHUIObjectBoxView()) + ->setHeaderText($title) + ->setForm($form); + + $timeline = $this->buildTransactionTimeline( + $book, + new DivinerLiveBookTransactionQuery()); + $timeline->setShouldTerminate(true); + + return $this->buildApplicationPage( + array( + $crumbs, + $object_box, + $timeline, + ), + array( + 'title' => $title, + )); + } + +} diff --git a/src/applications/diviner/controller/DivinerController.php b/src/applications/diviner/controller/DivinerController.php index cf4c9e197c..e9678c1b4b 100644 --- a/src/applications/diviner/controller/DivinerController.php +++ b/src/applications/diviner/controller/DivinerController.php @@ -3,19 +3,15 @@ abstract class DivinerController extends PhabricatorController { protected function buildSideNavView() { - $menu = $this->buildMenu(); + $menu = $this->buildApplicationMenu(); return AphrontSideNavFilterView::newFromMenu($menu); } public function buildApplicationMenu() { - return $this->buildMenu(); - } - - private function buildMenu() { $menu = new PHUIListView(); id(new DivinerAtomSearchEngine()) - ->setViewer($this->getRequest()->getUser()) + ->setViewer($this->getRequest()->getViewer()) ->addNavigationItems($menu); return $menu; @@ -24,12 +20,8 @@ abstract class DivinerController extends PhabricatorController { protected function renderAtomList(array $symbols) { assert_instances_of($symbols, 'DivinerLiveSymbol'); - $request = $this->getRequest(); - $user = $request->getUser(); - $list = array(); foreach ($symbols as $symbol) { - switch ($symbol->getType()) { case DivinerAtom::TYPE_FUNCTION: $title = $symbol->getTitle().'()'; @@ -43,8 +35,7 @@ abstract class DivinerController extends PhabricatorController { ->setTitle($title) ->setHref($symbol->getURI()) ->setSubtitle($symbol->getSummary()) - ->setType(DivinerAtom::getAtomTypeNameString( - $symbol->getType())); + ->setType(DivinerAtom::getAtomTypeNameString($symbol->getType())); $list[] = $item; } diff --git a/src/applications/diviner/controller/DivinerFindController.php b/src/applications/diviner/controller/DivinerFindController.php index 061e357b9f..b9e165fc1d 100644 --- a/src/applications/diviner/controller/DivinerFindController.php +++ b/src/applications/diviner/controller/DivinerFindController.php @@ -6,11 +6,10 @@ final class DivinerFindController extends DivinerController { return true; } - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $request->getViewer(); - $book_name = $request->getStr('book'); + $book_name = $request->getStr('book'); $query_text = $request->getStr('name'); $book = null; @@ -19,6 +18,7 @@ final class DivinerFindController extends DivinerController { ->setViewer($viewer) ->withNames(array($book_name)) ->executeOne(); + if (!$book) { return new Aphront404Response(); } @@ -70,8 +70,8 @@ final class DivinerFindController extends DivinerController { ->setTitle(pht('Documentation Not Found')) ->appendChild( pht( - 'Unable to find the specified documentation. You may have '. - 'followed a bad or outdated link.')) + 'Unable to find the specified documentation. '. + 'You may have followed a bad or outdated link.')) ->addCancelButton($not_found_uri, pht('Read More Documentation')); return id(new AphrontDialogResponse())->setDialog($dialog); diff --git a/src/applications/diviner/controller/DivinerMainController.php b/src/applications/diviner/controller/DivinerMainController.php index 34647dc7cf..49fc323733 100644 --- a/src/applications/diviner/controller/DivinerMainController.php +++ b/src/applications/diviner/controller/DivinerMainController.php @@ -6,9 +6,8 @@ final class DivinerMainController extends DivinerController { return true; } - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $request->getViewer(); $books = id(new DivinerBookQuery()) ->setViewer($viewer) @@ -31,10 +30,10 @@ final class DivinerMainController extends DivinerController { ->setHeader(pht('Documentation Books')) ->addActionLink($query_button); - $document = new PHUIDocumentView(); - $document->setHeader($header); - $document->setFontKit(PHUIDocumentView::FONT_SOURCE_SANS); - $document->addClass('diviner-view'); + $document = id(new PHUIDocumentView()) + ->setHeader($header) + ->setFontKit(PHUIDocumentView::FONT_SOURCE_SANS) + ->addClass('diviner-view'); if ($books) { $books = msort($books, 'getTitle'); @@ -54,24 +53,20 @@ final class DivinerMainController extends DivinerController { ->appendChild($list); $document->appendChild($list); - } else { $text = pht( - "(NOTE) **Looking for Phabricator documentation?** If you're looking ". - "for help and information about Phabricator, you can ". - "[[ https://secure.phabricator.com/diviner/ | browse the public ". - "Phabricator documentation ]] on the live site.\n\n". - "Diviner is the documentation generator used to build the Phabricator ". - "documentation.\n\n". + "(NOTE) **Looking for Phabricator documentation?** ". + "If you're looking for help and information about Phabricator, ". + "you can [[https://secure.phabricator.com/diviner/ | ". + "browse the public Phabricator documentation]] on the live site.\n\n". + "Diviner is the documentation generator used to build the ". + "Phabricator documentation.\n\n". "You haven't generated any Diviner documentation books yet, so ". "there's nothing to show here. If you'd like to generate your own ". "local copy of the Phabricator documentation and have it appear ". "here, run this command:\n\n". - " phabricator/ $ ./bin/diviner generate\n\n". - "Right now, Diviner isn't very useful for generating documentation ". - "for projects other than Phabricator. If you're interested in using ". - "it in your own projects, leave feedback for us on ". - "[[ https://secure.phabricator.com/T4558 | T4558 ]]."); + " %s\n\n", + 'phabricator/ $ ./bin/diviner generate'); $text = PhabricatorMarkupEngine::renderOneObject( id(new PhabricatorMarkupOneOff())->setContent($text), diff --git a/src/applications/diviner/editor/DivinerLiveBookEditor.php b/src/applications/diviner/editor/DivinerLiveBookEditor.php new file mode 100644 index 0000000000..3f392391bb --- /dev/null +++ b/src/applications/diviner/editor/DivinerLiveBookEditor.php @@ -0,0 +1,23 @@ +book) { $book_name = $this->getConfig('name'); - $book = id(new DivinerLiveBook())->loadOneWhere('name = %s', $book_name); + $book = id(new DivinerLiveBook())->loadOneWhere( + 'name = %s', + $book_name); + if (!$book) { $book = id(new DivinerLiveBook()) ->setName($book_name) ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy()) + ->setEditPolicy(PhabricatorPolicies::POLICY_ADMIN) ->save(); } @@ -144,7 +148,6 @@ final class DivinerLivePublisher extends DivinerPublisher { ->setContent(null) ->save(); } - } } diff --git a/src/applications/diviner/publisher/DivinerPublisher.php b/src/applications/diviner/publisher/DivinerPublisher.php index 591897f0e5..4b6490639d 100644 --- a/src/applications/diviner/publisher/DivinerPublisher.php +++ b/src/applications/diviner/publisher/DivinerPublisher.php @@ -133,10 +133,20 @@ abstract class DivinerPublisher extends Phobject { $created = array_keys($created); } - echo pht('Deleting %d documents.', count($deleted))."\n"; + $console = PhutilConsole::getConsole(); + + $console->writeOut( + "%s\n", + pht( + 'Deleting %s document(s).', + new PhutilNumber(count($deleted)))); $this->deleteDocumentsByHash($deleted); - echo pht('Creating %d documents.', count($created))."\n"; + $console->writeOut( + "%s\n", + pht( + 'Creating %s document(s).', + new PhutilNumber(count($created)))); $this->createDocumentsByHash($created); } diff --git a/src/applications/diviner/query/DivinerAtomQuery.php b/src/applications/diviner/query/DivinerAtomQuery.php index 2daa9bc3a7..b8856141a5 100644 --- a/src/applications/diviner/query/DivinerAtomQuery.php +++ b/src/applications/diviner/query/DivinerAtomQuery.php @@ -79,7 +79,6 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $this; } - /** * Include or exclude "ghosts", which are symbols which used to exist but do * not exist currently (for example, a function which existed in an older @@ -137,6 +136,7 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { foreach ($atoms as $key => $atom) { $book = idx($books, $atom->getBookPHID()); if (!$book) { + $this->didRejectResult($atom); unset($atoms[$key]); continue; } @@ -158,12 +158,9 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { // Load all of the symbols this symbol extends, recursively. Commonly, // this means all the ancestor classes and interfaces it extends and // implements. - if ($this->needExtends) { - // First, load all the matching symbols by name. This does 99% of the // work in most cases, assuming things are named at all reasonably. - $names = array(); foreach ($atoms as $atom) { if (!$atom->getAtom()) { @@ -303,6 +300,7 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { if ($this->titles) { $hashes = array(); + foreach ($this->titles as $title) { $slug = DivinerAtomRef::normalizeTitleString($title); $hash = PhabricatorHash::digestForIndex($slug); @@ -318,6 +316,7 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { if ($this->contexts) { $with_null = false; $contexts = $this->contexts; + foreach ($contexts as $key => $value) { if ($value === null) { unset($contexts[$key]); @@ -373,10 +372,9 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { } if ($this->nameContains) { - // NOTE: This CONVERT() call makes queries case-insensitive, since the - // column has binary collation. Eventually, this should move into + // NOTE: This `CONVERT()` call makes queries case-insensitive, since + // the column has binary collation. Eventually, this should move into // fulltext. - $where[] = qsprintf( $conn_r, 'CONVERT(name USING utf8) LIKE %~', @@ -388,7 +386,6 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $this->formatWhereClause($where); } - /** * Walk a list of atoms and collect all the node hashes of the atoms' * children. When recursing, also walk up the tree and collect children of @@ -413,6 +410,7 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { foreach ($child_hashes as $hash) { $hashes[$hash] = $hash; } + if ($recurse_up) { $hashes += $this->getAllChildHashes($symbol->getExtends(), true); } @@ -421,7 +419,6 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $hashes; } - /** * Attach child atoms to existing atoms. In recursive mode, also attach child * atoms to atoms that these atoms extend. @@ -452,7 +449,9 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { $symbol_children[] = $children[$hash]; } } + $symbol->attachChildren($symbol_children); + if ($recurse_up) { $this->attachAllChildren($symbol->getExtends(), $children, true); } diff --git a/src/applications/diviner/query/DivinerBookQuery.php b/src/applications/diviner/query/DivinerBookQuery.php index 90fed52a23..b1f5f0a995 100644 --- a/src/applications/diviner/query/DivinerBookQuery.php +++ b/src/applications/diviner/query/DivinerBookQuery.php @@ -6,6 +6,8 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { private $phids; private $names; + private $needProjectPHIDs; + public function withIDs(array $ids) { $this->ids = $ids; return $this; @@ -21,6 +23,11 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $this; } + public function needProjectPHIDs($need_phids) { + $this->needProjectPHIDs = $need_phids; + return $this; + } + protected function loadPage() { $table = new DivinerLiveBook(); $conn_r = $table->establishConnection('r'); @@ -36,6 +43,30 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $table->loadAllFromArray($data); } + protected function didFilterPage(array $books) { + assert_instances_of($books, 'DivinerLiveBook'); + + if ($this->needProjectPHIDs) { + $edge_query = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs(mpull($books, 'getPHID')) + ->withEdgeTypes( + array( + PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, + )); + $edge_query->execute(); + + foreach ($books as $book) { + $project_phids = $edge_query->getDestinationPHIDs( + array( + $book->getPHID(), + )); + $book->attachProjectPHIDs($project_phids); + } + } + + return $books; + } + protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { $where = array(); diff --git a/src/applications/diviner/query/DivinerLiveBookTransactionQuery.php b/src/applications/diviner/query/DivinerLiveBookTransactionQuery.php new file mode 100644 index 0000000000..e3ce6d41d7 --- /dev/null +++ b/src/applications/diviner/query/DivinerLiveBookTransactionQuery.php @@ -0,0 +1,10 @@ +getBook() != $this->getConfig('name')) { - // If the ref is from a different book, we can't normalize it. Just return - // it as-is if it has enough information to resolve. + // If the ref is from a different book, we can't normalize it. + // Just return it as-is if it has enough information to resolve. if ($ref->getName() && $ref->getType()) { return $ref; } else { @@ -260,5 +260,4 @@ final class DivinerDefaultRenderer extends DivinerRenderer { $ref->getTitle()); } - } diff --git a/src/applications/diviner/search/DivinerBookSearchIndexer.php b/src/applications/diviner/search/DivinerBookSearchIndexer.php index b08042a4f8..106ae9e389 100644 --- a/src/applications/diviner/search/DivinerBookSearchIndexer.php +++ b/src/applications/diviner/search/DivinerBookSearchIndexer.php @@ -18,6 +18,11 @@ final class DivinerBookSearchIndexer extends PhabricatorSearchDocumentIndexer { PhabricatorSearchDocumentFieldType::FIELD_BODY, $book->getPreface()); + $this->indexTransactions( + $doc, + new DivinerLiveBookTransactionQuery(), + array($phid)); + return $doc; } diff --git a/src/applications/diviner/storage/DivinerLiveBook.php b/src/applications/diviner/storage/DivinerLiveBook.php index ddb03e9d14..e689955171 100644 --- a/src/applications/diviner/storage/DivinerLiveBook.php +++ b/src/applications/diviner/storage/DivinerLiveBook.php @@ -3,12 +3,17 @@ final class DivinerLiveBook extends DivinerDAO implements PhabricatorPolicyInterface, - PhabricatorDestructibleInterface { + PhabricatorProjectInterface, + PhabricatorDestructibleInterface, + PhabricatorApplicationTransactionInterface { protected $name; protected $viewPolicy; + protected $editPolicy; protected $configurationData = array(); + private $projectPHIDs = self::ATTACHABLE; + protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -63,28 +68,47 @@ final class DivinerLiveBook extends DivinerDAO return idx($spec, 'name', $group); } + public function attachProjectPHIDs(array $project_phids) { + $this->projectPHIDs = $project_phids; + return $this; + } + + public function getProjectPHIDs() { + return $this->assertAttached($this->projectPHIDs); + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ + public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, ); } public function getPolicy($capability) { - return PhabricatorPolicies::getMostOpenPolicy(); + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + return $this->getViewPolicy(); + case PhabricatorPolicyCapability::CAN_EDIT: + return $this->getEditPolicy(); + } } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - return false; + return false; } public function describeAutomaticCapability($capability) { return null; } + /* -( PhabricatorDestructibleInterface )----------------------------------- */ + public function destroyObjectPermanently( PhabricatorDestructionEngine $engine) { @@ -102,4 +126,27 @@ final class DivinerLiveBook extends DivinerDAO $this->saveTransaction(); } + +/* -( PhabricatorApplicationTransactionInterface )------------------------- */ + + + public function getApplicationTransactionEditor() { + return new DivinerLiveBookEditor(); + } + + public function getApplicationTransactionObject() { + return $this; + } + + public function getApplicationTransactionTemplate() { + return new DivinerLiveBookTransaction(); + } + + public function willRenderTimeline( + PhabricatorApplicationTransactionView $timeline, + AphrontRequest $request) { + + return $timeline; + } + } diff --git a/src/applications/diviner/storage/DivinerLiveBookTransaction.php b/src/applications/diviner/storage/DivinerLiveBookTransaction.php new file mode 100644 index 0000000000..ae461e751a --- /dev/null +++ b/src/applications/diviner/storage/DivinerLiveBookTransaction.php @@ -0,0 +1,18 @@ +identityHash) { $this->identityHash = PhabricatorHash::digestForIndex( @@ -159,14 +158,17 @@ final class DivinerLiveSymbol extends DivinerDAO public function getTitle() { $title = parent::getTitle(); + if (!strlen($title)) { $title = $this->getName(); } + return $title; } public function setTitle($value) { $this->writeField('title', $value); + if (strlen($value)) { $slug = DivinerAtomRef::normalizeTitleString($value); $hash = PhabricatorHash::digestForIndex($slug); @@ -174,6 +176,7 @@ final class DivinerLiveSymbol extends DivinerDAO } else { $this->titleSlugHash = null; } + return $this; } @@ -200,16 +203,15 @@ final class DivinerLiveSymbol extends DivinerDAO /* -( PhabricatorPolicyInterface )----------------------------------------- */ + public function getCapabilities() { return $this->getBook()->getCapabilities(); } - public function getPolicy($capability) { return $this->getBook()->getPolicy($capability); } - public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { return $this->getBook()->hasAutomaticCapability($capability, $viewer); } @@ -219,19 +221,17 @@ final class DivinerLiveSymbol extends DivinerDAO } -/* -( Markup Interface )--------------------------------------------------- */ +/* -( PhabricatorMarkupInterface )------------------------------------------ */ public function getMarkupFieldKey($field) { return $this->getPHID().':'.$field.':'.$this->getGraphHash(); } - public function newMarkupEngine($field) { return PhabricatorMarkupEngine::getEngine('diviner'); } - public function getMarkupText($field) { if (!$this->getAtom()) { return; @@ -240,21 +240,18 @@ final class DivinerLiveSymbol extends DivinerDAO return $this->getAtom()->getDocblockText(); } - - public function didMarkupText( - $field, - $output, - PhutilMarkupEngine $engine) { + public function didMarkupText($field, $output, PhutilMarkupEngine $engine) { return $output; } - public function shouldUseMarkupCache($field) { return true; } + /* -( PhabricatorDestructibleInterface )----------------------------------- */ + public function destroyObjectPermanently( PhabricatorDestructionEngine $engine) { diff --git a/src/applications/diviner/storage/DivinerSchemaSpec.php b/src/applications/diviner/storage/DivinerSchemaSpec.php new file mode 100644 index 0000000000..e57408b89f --- /dev/null +++ b/src/applications/diviner/storage/DivinerSchemaSpec.php @@ -0,0 +1,9 @@ +buildEdgeSchemata(new DivinerLiveBook()); + } + +} diff --git a/src/applications/diviner/view/DivinerBookItemView.php b/src/applications/diviner/view/DivinerBookItemView.php index 227a791308..5abf43638f 100644 --- a/src/applications/diviner/view/DivinerBookItemView.php +++ b/src/applications/diviner/view/DivinerBookItemView.php @@ -43,23 +43,23 @@ final class DivinerBookItemView extends AphrontTagView { $title = phutil_tag( 'span', - array( - 'class' => 'diviner-book-item-title', - ), + array( + 'class' => 'diviner-book-item-title', + ), $this->title); $subtitle = phutil_tag( 'span', - array( - 'class' => 'diviner-book-item-subtitle', - ), + array( + 'class' => 'diviner-book-item-subtitle', + ), $this->subtitle); $type = phutil_tag( 'span', - array( - 'class' => 'diviner-book-item-type', - ), + array( + 'class' => 'diviner-book-item-type', + ), $this->type); return array($title, $type, $subtitle); diff --git a/src/applications/diviner/workflow/DivinerAtomizeWorkflow.php b/src/applications/diviner/workflow/DivinerAtomizeWorkflow.php index c16cb24c30..7a1d5edfaf 100644 --- a/src/applications/diviner/workflow/DivinerAtomizeWorkflow.php +++ b/src/applications/diviner/workflow/DivinerAtomizeWorkflow.php @@ -36,8 +36,10 @@ final class DivinerAtomizeWorkflow extends DivinerWorkflow { $atomizer_class = $args->getArg('atomizer'); if (!$atomizer_class) { - throw new Exception( - pht('Specify an atomizer class with %s.', '--atomizer')); + throw new PhutilArgumentUsageException( + pht( + 'Specify an atomizer class with %s.', + '--atomizer')); } $symbols = id(new PhutilSymbolLoader()) @@ -46,7 +48,7 @@ final class DivinerAtomizeWorkflow extends DivinerWorkflow { ->setAncestorClass('DivinerAtomizer') ->selectAndLoadSymbols(); if (!$symbols) { - throw new Exception( + throw new PhutilArgumentUsageException( pht( "Atomizer class '%s' must be a concrete subclass of %s.", $atomizer_class, diff --git a/src/applications/diviner/workflow/DivinerGenerateWorkflow.php b/src/applications/diviner/workflow/DivinerGenerateWorkflow.php index 2b0e4ed3ae..a08180e4a1 100644 --- a/src/applications/diviner/workflow/DivinerGenerateWorkflow.php +++ b/src/applications/diviner/workflow/DivinerGenerateWorkflow.php @@ -50,6 +50,7 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { } else { $cwd = getcwd(); $this->log(pht('FINDING DOCUMENTATION BOOKS')); + $books = id(new FileFinder($cwd)) ->withType('f') ->withSuffix('book') @@ -92,7 +93,7 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { // amount of work we can, so that regenerating documentation after minor // changes is quick. // - // = ATOM CACHE = + // = Atom Cache = // // In the first stage, we find all the direct changes to source code since // the last run. This stage relies on two data structures: @@ -118,7 +119,7 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { // its methods). The File Hash Map contains an exhaustive list of all atoms // with type "file", but not child atoms of those top-level atoms.) // - // = GRAPH CACHE = + // = Graph Cache = // // We now know which atoms exist, and can compare the Atom Map to some // existing cache to figure out what has changed. However, this isn't @@ -176,8 +177,9 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { ->setConcreteOnly(true) ->setAncestorClass('DivinerPublisher') ->selectAndLoadSymbols(); + if (!$symbols) { - throw new Exception( + throw new PhutilArgumentUsageException( pht( "Publisher class '%s' must be a concrete subclass of %s.", $publisher_class, @@ -188,22 +190,37 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { $this->publishDocumentation($args->getArg('clean'), $publisher); } + /* -( Atom Cache )--------------------------------------------------------- */ + private function buildAtomCache() { $this->log(pht('BUILDING ATOM CACHE')); $file_hashes = $this->findFilesInProject(); - $this->log(pht('Found %d file(s) in project.', count($file_hashes))); + $this->log( + pht( + 'Found %s file(s) in project.', + new PhutilNumber(count($file_hashes)))); $this->deleteDeadAtoms($file_hashes); $atomize = $this->getFilesToAtomize($file_hashes); - $this->log(pht('Found %d unatomized, uncached file(s).', count($atomize))); + $this->log( + pht( + 'Found %s unatomized, uncached file(s).', + new PhutilNumber(count($atomize)))); $file_atomizers = $this->getAtomizersForFiles($atomize); - $this->log(pht('Found %d file(s) to atomize.', count($file_atomizers))); + $this->log( + pht( + 'Found %s file(s) to atomize.', + new PhutilNumber(count($file_atomizers)))); + $futures = $this->buildAtomizerFutures($file_atomizers); - $this->log(pht('Atomizing %d file(s).', count($file_atomizers))); + $this->log( + pht( + 'Atomizing %s file(s).', + new PhutilNumber(count($file_atomizers)))); if ($futures) { $this->resolveAtomizerFutures($futures, $file_hashes); @@ -344,6 +361,7 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { ->setTotal(count($futures)); $futures = id(new FutureIterator($futures)) ->limit(4); + foreach ($futures as $key => $future) { try { $atoms = $future->resolveJSON(); @@ -396,6 +414,7 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { /* -( Graph Cache )-------------------------------------------------------- */ + private function buildGraphCache() { $this->log(pht('BUILDING GRAPH CACHE')); @@ -407,7 +426,10 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { $dirty_nhashes = array(); $del_atoms = array_diff_key($symbol_map, $atoms); - $this->log(pht('Found %d obsolete atom(s) in graph.', count($del_atoms))); + $this->log( + pht( + 'Found %s obsolete atom(s) in graph.', + new PhutilNumber(count($del_atoms)))); foreach ($del_atoms as $nhash => $shash) { $atom_cache->deleteSymbol($nhash); @@ -418,7 +440,10 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { } $new_atoms = array_diff_key($atoms, $symbol_map); - $this->log(pht('Found %d new atom(s) in graph.', count($new_atoms))); + $this->log( + pht( + 'Found %s new atom(s) in graph.', + new PhutilNumber(count($new_atoms)))); foreach ($new_atoms as $nhash => $ignored) { $shash = $this->computeSymbolHash($nhash); @@ -454,7 +479,10 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { } } - $this->log(pht('Found %d affected atoms.', count($dirty_nhashes))); + $this->log( + pht( + 'Found %s affected atoms.', + new PhutilNumber(count($dirty_nhashes)))); foreach ($dirty_nhashes as $nhash => $ignored) { $atom_cache->addGraph($nhash, $this->computeGraphHash($nhash)); diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index 320665fd73..b13dfe1b50 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1124,6 +1124,52 @@ final class PhabricatorUSEnglishTranslation '%s changed package owners, added: %4$s; removed: %6$s.', ), + 'Found %s book(s).' => array( + 'Found %s book.', + 'Found %s books.', + ), + 'Found %s file(s) in project.' => array( + 'Found %s file in project.', + 'Found %s files in project.', + ), + 'Found %s unatomized, uncached file(s).' => array( + 'Found %s unatomized, uncached file.', + 'Found %s unatomized, uncached files.', + ), + 'Found %s file(s) to atomize.' => array( + 'Found %s file to atomize.', + 'Found %s files to atomize.', + ), + 'Atomizing %s file(s).' => array( + 'Atomizing %s file.', + 'Atomizing %s files.', + ), + 'Creating %s document(s).' => array( + 'Creating %s document.', + 'Creating %s documents.', + ), + 'Deleting %s document(s).' => array( + 'Deleting %s document.', + 'Deleting %s documents.', + ), + 'Found %s obsolete atom(s) in graph.' => array( + 'Found %s obsolete atom in graph.', + 'Found %s obsolete atoms in graph.', + ), + 'Found %s new atom(s) in graph.' => array( + 'Found %s new atom in graph.', + 'Found %s new atoms in graph.', + ), + 'This call takes %s parameter(s), but only %s are documented.' => array( + array( + 'This call takes %s parameter, but only %s is documented.', + 'This call takes %s parameter, but only %s are documented.', + ), + array( + 'This call takes %s parameters, but only %s is documented.', + 'This call takes %s parameters, but only %s are documented.', + ), + ), ); } -- 2.51.2 From 8965c7ec9b8935f453fce5ecd9ecb6bcceeb6fad Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 16 Jun 2015 14:25:13 -0700 Subject: [PATCH 07/17] Email Preferences - round out options for Projects and Phriction Summary: Fixes T8524, T8550. These were both missing "subscribers" and Phriction was also the only application with no "other". Test Plan: - Project - set user A to notify only for project subscriber changes - made a project with user A - subscribed user B to project - verified notification sent to user A - used ./bin/mail to make sure no mail was sent to user A because "This mail has tags which control which users receive it, and this recipient has not elected to receive mail with any of the tags on this message (Settings > Email Preferences)." - Phriction - set user A to notify only for phriction document subscriber changes - made a document with user A - subscribed user B to document - verified notification sent to user A - used ./bin/mail to make sure no mail was sent to user A because "This mail has tags which control which users receive it, and this recipient has not elected to receive mail with any of the tags on this message (Settings > Email Preferences)." - observed option for "other" in email preferences Reviewers: epriestley Reviewed By: epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T8524, T8550 Differential Revision: https://secure.phabricator.com/D13313 --- .../editor/PhrictionTransactionEditor.php | 4 ++++ .../phriction/storage/PhrictionTransaction.php | 15 +++++++++++---- .../PhabricatorProjectTransactionEditor.php | 2 ++ .../storage/PhabricatorProjectTransaction.php | 12 ++++++++---- 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/applications/phriction/editor/PhrictionTransactionEditor.php b/src/applications/phriction/editor/PhrictionTransactionEditor.php index b276aaf34a..5d1534df19 100644 --- a/src/applications/phriction/editor/PhrictionTransactionEditor.php +++ b/src/applications/phriction/editor/PhrictionTransactionEditor.php @@ -391,6 +391,10 @@ final class PhrictionTransactionEditor pht("A document's content changes."), PhrictionTransaction::MAILTAG_DELETE => pht('A document is deleted.'), + PhrictionTransaction::MAILTAG_SUBSCRIBERS => + pht('A document\'s subscribers change.'), + PhrictionTransaction::MAILTAG_OTHER => + pht('Other document activity not listed above occurs.'), ); } diff --git a/src/applications/phriction/storage/PhrictionTransaction.php b/src/applications/phriction/storage/PhrictionTransaction.php index 069e147017..a3ae6adb9d 100644 --- a/src/applications/phriction/storage/PhrictionTransaction.php +++ b/src/applications/phriction/storage/PhrictionTransaction.php @@ -9,9 +9,11 @@ final class PhrictionTransaction const TYPE_MOVE_TO = 'move-to'; const TYPE_MOVE_AWAY = 'move-away'; - const MAILTAG_TITLE = 'phriction-title'; - const MAILTAG_CONTENT = 'phriction-content'; - const MAILTAG_DELETE = 'phriction-delete'; + const MAILTAG_TITLE = 'phriction-title'; + const MAILTAG_CONTENT = 'phriction-content'; + const MAILTAG_DELETE = 'phriction-delete'; + const MAILTAG_SUBSCRIBERS = 'phriction-subscribers'; + const MAILTAG_OTHER = 'phriction-other'; public function getApplicationName() { return 'phriction'; @@ -280,7 +282,12 @@ final class PhrictionTransaction case self::TYPE_DELETE: $tags[] = self::MAILTAG_DELETE; break; - + case PhabricatorTransactions::TYPE_SUBSCRIBERS: + $tags[] = self::MAILTAG_SUBSCRIBERS; + break; + default: + $tags[] = self::MAILTAG_OTHER; + break; } return $tags; } diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php index 4193c2e4c4..c5253da956 100644 --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -433,6 +433,8 @@ final class PhabricatorProjectTransactionEditor pht('Project membership changes.'), PhabricatorProjectTransaction::MAILTAG_WATCHERS => pht('Project watcher list changes.'), + PhabricatorProjectTransaction::MAILTAG_SUBSCRIBERS => + pht('Project subscribers change.'), PhabricatorProjectTransaction::MAILTAG_OTHER => pht('Other project activity not listed above occurs.'), ); diff --git a/src/applications/project/storage/PhabricatorProjectTransaction.php b/src/applications/project/storage/PhabricatorProjectTransaction.php index c8698dbe6b..01a458dee2 100644 --- a/src/applications/project/storage/PhabricatorProjectTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectTransaction.php @@ -14,10 +14,11 @@ final class PhabricatorProjectTransaction // NOTE: This is deprecated, members are just a normal edge now. const TYPE_MEMBERS = 'project:members'; - const MAILTAG_METADATA = 'project-metadata'; - const MAILTAG_MEMBERS = 'project-members'; - const MAILTAG_WATCHERS = 'project-watchers'; - const MAILTAG_OTHER = 'project-other'; + const MAILTAG_METADATA = 'project-metadata'; + const MAILTAG_MEMBERS = 'project-members'; + const MAILTAG_SUBSCRIBERS = 'project-subscribers'; + const MAILTAG_WATCHERS = 'project-watchers'; + const MAILTAG_OTHER = 'project-other'; public function getApplicationName() { return 'project'; @@ -369,6 +370,9 @@ final class PhabricatorProjectTransaction case self::TYPE_COLOR: $tags[] = self::MAILTAG_METADATA; break; + case PhabricatorTransactions::TYPE_SUBSCRIBERS: + $tags[] = self::MAILTAG_SUBSCRIBERS; + break; case PhabricatorTransactions::TYPE_EDGE: $type = $this->getMetadata('edge:type'); $type = head($type); -- 2.51.2 From 751162d363e6b2434b4cfb131da3d6a688b0aba4 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Wed, 17 Jun 2015 07:20:34 +1000 Subject: [PATCH 08/17] Set icons for Diviner PHID types Summary: Ref T4558. Give `DivinerAtomPHIDType` and `DivinerBookPHIDType` a font icon. Test Plan: Saw icons in global search. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T4558 Differential Revision: https://secure.phabricator.com/D13304 --- src/applications/diviner/phid/DivinerAtomPHIDType.php | 4 ++++ src/applications/diviner/phid/DivinerBookPHIDType.php | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/applications/diviner/phid/DivinerAtomPHIDType.php b/src/applications/diviner/phid/DivinerAtomPHIDType.php index 3ee95c9ea1..7387b0cafe 100644 --- a/src/applications/diviner/phid/DivinerAtomPHIDType.php +++ b/src/applications/diviner/phid/DivinerAtomPHIDType.php @@ -12,6 +12,10 @@ final class DivinerAtomPHIDType extends PhabricatorPHIDType { return new DivinerLiveSymbol(); } + public function getTypeIcon() { + return 'fa-cube'; + } + public function getPHIDTypeApplicationClass() { return 'PhabricatorDivinerApplication'; } diff --git a/src/applications/diviner/phid/DivinerBookPHIDType.php b/src/applications/diviner/phid/DivinerBookPHIDType.php index 39420ec12c..da08ae3e87 100644 --- a/src/applications/diviner/phid/DivinerBookPHIDType.php +++ b/src/applications/diviner/phid/DivinerBookPHIDType.php @@ -12,6 +12,10 @@ final class DivinerBookPHIDType extends PhabricatorPHIDType { return new DivinerLiveBook(); } + public function getTypeIcon() { + return 'fa-book'; + } + public function getPHIDTypeApplicationClass() { return 'PhabricatorDivinerApplication'; } -- 2.51.2 From a3010148e24d0750fd3a279baeb9cac9632adc02 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Wed, 17 Jun 2015 07:28:16 +1000 Subject: [PATCH 09/17] Mark diviner atoms as closed in search index Summary: Ref T4558. If a Diviner atom is a ghost (i.e. the underlying source code has been removed), mark it as closed in the search index. Test Plan: Searched for a ghost atom in global searcn and saw the results show "Closed". Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Maniphest Tasks: T4558 Differential Revision: https://secure.phabricator.com/D13297 --- .../diviner/search/DivinerAtomSearchIndexer.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/applications/diviner/search/DivinerAtomSearchIndexer.php b/src/applications/diviner/search/DivinerAtomSearchIndexer.php index ea134058f6..51a3866641 100644 --- a/src/applications/diviner/search/DivinerAtomSearchIndexer.php +++ b/src/applications/diviner/search/DivinerAtomSearchIndexer.php @@ -27,7 +27,15 @@ final class DivinerAtomSearchIndexer extends PhabricatorSearchDocumentIndexer { PhabricatorSearchRelationship::RELATIONSHIP_BOOK, $atom->getBookPHID(), DivinerBookPHIDType::TYPECONST, - $book->getDateCreated()); + PhabricatorTime::getNow()); + + $doc->addRelationship( + $atom->getGraphHash() + ? PhabricatorSearchRelationship::RELATIONSHIP_CLOSED + : PhabricatorSearchRelationship::RELATIONSHIP_OPEN, + $atom->getBookPHID(), + DivinerBookPHIDType::TYPECONST, + PhabricatorTime::getNow()); return $doc; } -- 2.51.2 From 281082f4d73007de4190534fd8389f9fab14851f Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 16 Jun 2015 14:38:29 -0700 Subject: [PATCH 10/17] Conpherence - fix a bug where users have no events Summary: Fixes T8518. Defaulting $events to null creates carnage further down the stack. Instead, default it to array() so foreach, array_select_keys, etc. don't barf. Test Plan: logic and will ask the reporting user to verify since they can repro 100% Reviewers: epriestley Reviewed By: epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T8518 Differential Revision: https://secure.phabricator.com/D13314 --- src/applications/conpherence/query/ConpherenceThreadQuery.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index 1b311516f8..a04037cf6c 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -355,6 +355,7 @@ final class ConpherenceThreadQuery $start_epoch = $epochs['start_epoch']; $end_epoch = $epochs['end_epoch']; + $events = array(); if ($participant_phids) { $events = id(new PhabricatorCalendarEventQuery()) ->setViewer($this->getViewer()) @@ -363,8 +364,6 @@ final class ConpherenceThreadQuery ->withDateRange($start_epoch, $end_epoch) ->execute(); $events = mpull($events, null, 'getPHID'); - } else { - $events = null; } $invitees = array(); -- 2.51.2 From ca8adac1eb40758afaa06c9d59de660616870571 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 16 Jun 2015 14:49:52 -0700 Subject: [PATCH 11/17] Config - make "No auth providers configured" check use proper hyperlinking technology Summary: Fixes T8346. Also gets rid of the air quotes on Auth and uses the "Appname Application" style we use elsewhere. Test Plan: slapped an "|| true" in the checking logic and verified the link was rendered in a usable fashion just below the error message. Reviewers: epriestley Reviewed By: epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T8346 Differential Revision: https://secure.phabricator.com/D13315 --- src/applications/config/check/PhabricatorAuthSetupCheck.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/applications/config/check/PhabricatorAuthSetupCheck.php b/src/applications/config/check/PhabricatorAuthSetupCheck.php index 0295a85b2d..43fab77eb7 100644 --- a/src/applications/config/check/PhabricatorAuthSetupCheck.php +++ b/src/applications/config/check/PhabricatorAuthSetupCheck.php @@ -27,14 +27,14 @@ final class PhabricatorAuthSetupCheck extends PhabricatorSetupCheck { 'You have not configured any authentication providers yet. You '. 'should add a provider (like username/password, LDAP, or GitHub '. 'OAuth) so users can register and log in. You can add and configure '. - 'providers using the [[%s | "Auth" application]].', - '/auth/'); + 'providers using the Auth Application.'); $this ->newIssue('auth.noproviders') ->setShortName(pht('No Auth Providers')) ->setName(pht('No Authentication Providers Configured')) - ->setMessage($message); + ->setMessage($message) + ->addLink('/auth/', pht('Auth Application')); } } } -- 2.51.2 From db9fc369ca6230cc06ac9102d0d34066e6836cfa Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 16 Jun 2015 15:33:53 -0700 Subject: [PATCH 12/17] Phriction - improve workflow for when user entered slug needs to be normalized. Summary: Encountered this playing with T8402 on my test instance. I think warning the user about adding a trailing "/" is unnecessary so don't do it. I think its confusing to not call out spaces / to lump them in with special characters so call out the sapces. Test Plan: moved a page around and verified no warning if the slug is missing a "/" as user specified and that a change to spaces is called out Reviewers: epriestley Reviewed By: epriestley Subscribers: epriestley, Korvin Differential Revision: https://secure.phabricator.com/D13316 --- .../phriction/controller/PhrictionMoveController.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/applications/phriction/controller/PhrictionMoveController.php b/src/applications/phriction/controller/PhrictionMoveController.php index 21c65f0495..a5febbbaaa 100644 --- a/src/applications/phriction/controller/PhrictionMoveController.php +++ b/src/applications/phriction/controller/PhrictionMoveController.php @@ -36,13 +36,14 @@ final class PhrictionMoveController extends PhrictionController { // about it. if (strlen($v_slug)) { $normal_slug = PhabricatorSlug::normalize($v_slug); - if ($normal_slug !== $v_slug) { + $no_slash_slug = rtrim($normal_slug, '/'); + if ($normal_slug !== $v_slug && $no_slash_slug !== $v_slug) { return $this->newDialog() ->setTitle(pht('Adjust Path')) ->appendParagraph( pht( 'The path you entered (%s) is not a valid wiki document '. - 'path. Paths may not contain special characters.', + 'path. Paths may not contain spaces or special characters.', phutil_tag('strong', array(), $v_slug))) ->appendParagraph( pht( -- 2.51.2 From 42170ab151a9fcfc1d3f8333b4cbeb07c060b7e6 Mon Sep 17 00:00:00 2001 From: Eitan Adler Date: Tue, 16 Jun 2015 15:32:47 -0700 Subject: [PATCH 13/17] Change text ADD_CC and REMOVE_CC actions Summary: Fixes T8559 Test Plan: ran phab locally, saw text changes Reviewers: lpriestley, #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley Maniphest Tasks: T8559 Differential Revision: https://secure.phabricator.com/D13317 --- .../audit/constants/PhabricatorAuditActionConstants.php | 2 +- src/applications/herald/adapter/HeraldAdapter.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/applications/audit/constants/PhabricatorAuditActionConstants.php b/src/applications/audit/constants/PhabricatorAuditActionConstants.php index ca2a497aff..d323540795 100644 --- a/src/applications/audit/constants/PhabricatorAuditActionConstants.php +++ b/src/applications/audit/constants/PhabricatorAuditActionConstants.php @@ -19,7 +19,7 @@ final class PhabricatorAuditActionConstants extends Phobject { self::ACCEPT => pht("Accept Commit \xE2\x9C\x94"), self::RESIGN => pht('Resign from Audit'), self::CLOSE => pht('Close Audit'), - self::ADD_CCS => pht('Add CCs'), + self::ADD_CCS => pht('Add Subscribers'), self::ADD_AUDITORS => pht('Add Auditors'), ); diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 01014adedf..cfd026f518 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -850,8 +850,8 @@ abstract class HeraldAdapter extends Phobject { case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: $standard = array( self::ACTION_NOTHING => pht('Do nothing'), - self::ACTION_ADD_CC => pht('Add emails to CC'), - self::ACTION_REMOVE_CC => pht('Remove emails from CC'), + self::ACTION_ADD_CC => pht('Add Subscribers'), + self::ACTION_REMOVE_CC => pht('Remove Subscribers'), self::ACTION_EMAIL => pht('Send an email to'), self::ACTION_AUDIT => pht('Trigger an Audit by'), self::ACTION_FLAG => pht('Mark with flag'), @@ -868,8 +868,8 @@ abstract class HeraldAdapter extends Phobject { case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: $standard = array( self::ACTION_NOTHING => pht('Do nothing'), - self::ACTION_ADD_CC => pht('Add me to CC'), - self::ACTION_REMOVE_CC => pht('Remove me from CC'), + self::ACTION_ADD_CC => pht('Add me as a subscriber'), + self::ACTION_REMOVE_CC => pht('Remove me as a subscriber'), self::ACTION_EMAIL => pht('Send me an email'), self::ACTION_AUDIT => pht('Trigger an Audit by me'), self::ACTION_FLAG => pht('Mark with flag'), -- 2.51.2 From bb7f2ea9055cec04a81d67f56f026e3f548e1273 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 16 Jun 2015 16:06:42 -0700 Subject: [PATCH 14/17] Fail gracefully on bad API call when building commit mail Summary: Ref T8574. This could fail because the target is disabled (as here), or doesn't have access to the API, or the API request needs to be satisfied by a different host which isn't available. In any of these cases, just show the failure and continue generating the mail. This field isn't important enough to block the mail, and many of these errors are permanent. (I'll follow up on T8574 with some more permanent ideas to address this class of issue.) Test Plan: Faked API call failure, generated mail, saw clean generation of mail with a failure message. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8574 Differential Revision: https://secure.phabricator.com/D13319 --- .../PhabricatorCommitBranchesField.php | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/applications/repository/customfield/PhabricatorCommitBranchesField.php b/src/applications/repository/customfield/PhabricatorCommitBranchesField.php index 0d430f36a6..63a807ea55 100644 --- a/src/applications/repository/customfield/PhabricatorCommitBranchesField.php +++ b/src/applications/repository/customfield/PhabricatorCommitBranchesField.php @@ -29,18 +29,25 @@ final class PhabricatorCommitBranchesField 'callsign' => $this->getObject()->getRepository()->getCallsign(), ); - $branches_raw = id(new ConduitCall('diffusion.branchquery', $params)) - ->setUser($this->getViewer()) - ->execute(); - - $branches = DiffusionRepositoryRef::loadAllFromDictionaries($branches_raw); - if (!$branches) { - return; + try { + $branches_raw = id(new ConduitCall('diffusion.branchquery', $params)) + ->setUser($this->getViewer()) + ->execute(); + + $branches = DiffusionRepositoryRef::loadAllFromDictionaries( + $branches_raw); + if (!$branches) { + return; + } + + $branch_names = mpull($branches, 'getShortName'); + sort($branch_names); + $branch_text = implode(', ', $branch_names); + } catch (Exception $ex) { + $branch_text = pht('<%s: %s>', get_class($ex), $ex->getMessage()); } - $branch_names = mpull($branches, 'getShortName'); - sort($branch_names); - $body->addTextSection(pht('BRANCHES'), implode(', ', $branch_names)); + $body->addTextSection(pht('BRANCHES'), $branch_text); } } -- 2.51.2 From 30c4783c426f10d90801178fac2d61bff5b4747d Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 16 Jun 2015 16:43:24 -0700 Subject: [PATCH 15/17] Dramatically limit the range of failures which can cause duplicate mail Summary: Ref T8574. Currently, failures during mail body construction, feed publishing, or search indexing could cause us to retry the publishing task and potentially send duplicate mail. Instead, build (but do not send) the mail first, then send all the mail at the very end. This isn't completley perfect, but should make it enormously harder for duplicate mail to be generated. Test Plan: Sent some mail, ran the daemons, saw it show up normally in the outbound queue. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8574 Differential Revision: https://secure.phabricator.com/D13320 --- .../replyhandler/PhabricatorMailTarget.php | 4 +- .../ReleephRequestTransactionalEditor.php | 12 ++---- ...habricatorApplicationTransactionEditor.php | 40 +++++++++++-------- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/applications/metamta/replyhandler/PhabricatorMailTarget.php b/src/applications/metamta/replyhandler/PhabricatorMailTarget.php index fd2e7a0f7a..e7e79c5ef9 100644 --- a/src/applications/metamta/replyhandler/PhabricatorMailTarget.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailTarget.php @@ -55,7 +55,7 @@ final class PhabricatorMailTarget extends Phobject { return $this->viewer; } - public function sendMail(PhabricatorMetaMTAMail $mail) { + public function willSendMail(PhabricatorMetaMTAMail $mail) { $viewer = $this->getViewer(); $mail->addPHIDHeaders('X-Phabricator-To', $this->rawToPHIDs); @@ -92,7 +92,7 @@ final class PhabricatorMailTarget extends Phobject { $mail->addCCs($cc); } - return $mail->save(); + return $mail; } private function getRecipientsSummary( diff --git a/src/applications/releeph/editor/ReleephRequestTransactionalEditor.php b/src/applications/releeph/editor/ReleephRequestTransactionalEditor.php index 8f6805e661..17ae6fcc35 100644 --- a/src/applications/releeph/editor/ReleephRequestTransactionalEditor.php +++ b/src/applications/releeph/editor/ReleephRequestTransactionalEditor.php @@ -166,28 +166,22 @@ final class ReleephRequestTransactionalEditor protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { - return true; - } - - protected function sendMail( - PhabricatorLiskDAO $object, - array $xactions) { // Avoid sending emails that only talk about commit discovery. $types = array_unique(mpull($xactions, 'getTransactionType')); if ($types === array(ReleephRequestTransaction::TYPE_DISCOVERY)) { - return null; + return false; } // Don't email people when we discover that something picks or reverts OK. if ($types === array(ReleephRequestTransaction::TYPE_PICK_STATUS)) { if (!mfilter($xactions, 'isBoringPickStatus', true /* negate */)) { // If we effectively call "isInterestingPickStatus" and get nothing... - return null; + return false; } } - return parent::sendMail($object, $xactions); + return true; } protected function buildReplyHandler(PhabricatorLiskDAO $object) { diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 6bf88b4e25..fd547c4049 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -1040,10 +1040,10 @@ abstract class PhabricatorApplicationTransactionEditor // Hook for edges or other properties that may need (re-)loading $object = $this->willPublish($object, $xactions); - $mailed = array(); + $messages = array(); if (!$this->getDisableEmail()) { if ($this->shouldSendMail($object, $xactions)) { - $mailed = $this->sendMail($object, $xactions); + $messages = $this->buildMail($object, $xactions); } } @@ -1055,10 +1055,21 @@ abstract class PhabricatorApplicationTransactionEditor } if ($this->shouldPublishFeedStory($object, $xactions)) { - $this->publishFeedStory( - $object, - $xactions, - $mailed); + $mailed = array(); + foreach ($messages as $mail) { + foreach ($mail->buildRecipientList() as $phid) { + $mailed[$phid] = true; + } + } + + $this->publishFeedStory($object, $xactions, $mailed); + } + + // NOTE: This actually sends the mail. We do this last to reduce the chance + // that we send some mail, hit an exception, then send the mail again when + // retrying. + foreach ($messages as $mail) { + $mail->save(); } return $xactions; @@ -2241,7 +2252,7 @@ abstract class PhabricatorApplicationTransactionEditor /** * @task mail */ - protected function sendMail( + private function buildMail( PhabricatorLiskDAO $object, array $xactions) { @@ -2255,8 +2266,7 @@ abstract class PhabricatorApplicationTransactionEditor // Set this explicitly before we start swapping out the effective actor. $this->setActingAsPHID($this->getActingAsPHID()); - - $mailed = array(); + $messages = array(); foreach ($targets as $target) { $original_actor = $this->getActor(); @@ -2270,7 +2280,7 @@ abstract class PhabricatorApplicationTransactionEditor // Reload handles for the new viewer. $this->loadHandles($xactions); - $mail = $this->sendMailToTarget($object, $xactions, $target); + $mail = $this->buildMailForTarget($object, $xactions, $target); } catch (Exception $ex) { $caught = $ex; } @@ -2283,16 +2293,14 @@ abstract class PhabricatorApplicationTransactionEditor } if ($mail) { - foreach ($mail->buildRecipientList() as $phid) { - $mailed[$phid] = true; - } + $messages[] = $mail; } } - return array_keys($mailed); + return $messages; } - private function sendMailToTarget( + private function buildMailForTarget( PhabricatorLiskDAO $object, array $xactions, PhabricatorMailTarget $target) { @@ -2354,7 +2362,7 @@ abstract class PhabricatorApplicationTransactionEditor $mail->setParentMessageID($this->getParentMessageID()); } - return $target->sendMail($mail); + return $target->willSendMail($mail); } private function addMailProjectMetadata( -- 2.51.2 From 8484adcffdf61fc9088d86765524e5e5d60d9db9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 16 Jun 2015 19:32:37 -0700 Subject: [PATCH 16/17] Cache application visibility in the request cache Summary: Ref T8575. We check if users can see applications frequently, and caching on the Query isn't especially effective. Use the new Request cache instead. Test Plan: - Saw `/feed/` drop 7% (from ~830ms to ~770ms) on profiles. Reviewers: btrahan, avivey Reviewed By: avivey Subscribers: avivey, epriestley Maniphest Tasks: T8575 Differential Revision: https://secure.phabricator.com/D13321 --- .../base/PhabricatorApplication.php | 23 ++++++++++++++----- .../policy/PhabricatorPolicyAwareQuery.php | 19 ++++----------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index 03f14a241b..1f605476bc 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -449,14 +449,25 @@ abstract class PhabricatorApplication $class, PhabricatorUser $viewer) { - if (!self::isClassInstalled($class)) { - return false; + $cache = PhabricatorCaches::getRequestCache(); + $viewer_phid = $viewer->getPHID(); + $key = 'app.'.$class.'.installed.'.$viewer_phid; + + $result = $cache->getKey($key); + if ($result === null) { + if (!self::isClassInstalled($class)) { + $result = false; + } else { + $result = PhabricatorPolicyFilter::hasCapability( + $viewer, + self::getByClass($class), + PhabricatorPolicyCapability::CAN_VIEW); + } + + $cache->setKey($key, $result); } - return PhabricatorPolicyFilter::hasCapability( - $viewer, - self::getByClass($class), - PhabricatorPolicyCapability::CAN_VIEW); + return $result; } diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php index e7b9b35628..30b7b8dd0e 100644 --- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php @@ -35,7 +35,6 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { private $workspace = array(); private $inFlightPHIDs = array(); private $policyFilteredPHIDs = array(); - private $canUseApplication; /** * Should we continue or throw an exception when a query result is filtered @@ -679,21 +678,13 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * execute the query. */ public function canViewerUseQueryApplication() { - if ($this->canUseApplication === null) { - $class = $this->getQueryApplicationClass(); - if (!$class) { - $this->canUseApplication = true; - } else { - $result = id(new PhabricatorApplicationQuery()) - ->setViewer($this->getViewer()) - ->withClasses(array($class)) - ->execute(); - - $this->canUseApplication = (bool)$result; - } + $class = $this->getQueryApplicationClass(); + if (!$class) { + return true; } - return $this->canUseApplication; + $viewer = $this->getViewer(); + return PhabricatorApplication::isClassInstalledForViewer($class, $viewer); } } -- 2.51.2 From 6933ee5fb86dc4afd0c5e93e98f13f382e32a15e Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 16 Jun 2015 19:32:58 -0700 Subject: [PATCH 17/17] Don't spend ridiculous amounts of time rebuilding orderable columns Summary: Ref T8575. Because orderable columns need to build custom fields, they are relatively expensive to build. Use the request cache. (The request cache is technically more correct than `static`, because configuration may change between requests and we may eventually reuse interpreters.) Test Plan: Saw home page time drop 39% (from 462ms to 283ms). Reviewers: btrahan, avivey Reviewed By: avivey Subscribers: avivey, epriestley Maniphest Tasks: T8575 Differential Revision: https://secure.phabricator.com/D13322 --- .../policy/PhabricatorCursorPagedPolicyAwareQuery.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 4d0f96bde5..772be350a9 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -872,6 +872,15 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery * @task order */ public function getOrderableColumns() { + $cache = PhabricatorCaches::getRequestCache(); + $class = get_class($this); + $cache_key = 'query.orderablecolumns.'.$class; + + $columns = $cache->getKey($cache_key); + if ($columns !== null) { + return $columns; + } + $columns = array( 'id' => array( 'table' => $this->getPrimaryTableAlias(), @@ -909,6 +918,8 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } } + $cache->setKey($cache_key, $columns); + return $columns; }