diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 3e622a32b9..c920068298 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,10 +7,10 @@ */ return array( 'names' => array( - 'core.pkg.css' => 'b7b8d101', + 'core.pkg.css' => '204cabae', 'core.pkg.js' => '6972d365', 'darkconsole.pkg.js' => 'e7393ebb', - 'differential.pkg.css' => '7ba78475', + 'differential.pkg.css' => '33da0633', 'differential.pkg.js' => 'd0cd0df6', 'diffusion.pkg.css' => '91c5d3a6', 'diffusion.pkg.js' => '3a9a8bfa', @@ -57,7 +57,7 @@ return array( 'rsrc/css/application/dashboard/dashboard.css' => 'bc6f2127', 'rsrc/css/application/diff/inline-comment-summary.css' => '51efda3a', 'rsrc/css/application/differential/add-comment.css' => 'c47f8c40', - 'rsrc/css/application/differential/changeset-view.css' => '3e3b0b76', + 'rsrc/css/application/differential/changeset-view.css' => '7bcbe615', 'rsrc/css/application/differential/core.css' => '5b7b8ff4', 'rsrc/css/application/differential/phui-inline-comment.css' => '5953c28e', 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', @@ -82,7 +82,7 @@ return array( 'rsrc/css/application/paste/paste.css' => '1898e534', 'rsrc/css/application/people/people-profile.css' => '2473d929', 'rsrc/css/application/phame/phame.css' => '737792d6', - 'rsrc/css/application/pholio/pholio-edit.css' => '3ad9d1ee', + 'rsrc/css/application/pholio/pholio-edit.css' => 'b15fec4a', 'rsrc/css/application/pholio/pholio-inline-comments.css' => '8e545e49', 'rsrc/css/application/pholio/pholio.css' => 'ca89d380', 'rsrc/css/application/phortune/phortune-credit-card-form.css' => '8391eb02', @@ -105,7 +105,7 @@ return array( 'rsrc/css/application/uiexample/example.css' => '528b19de', 'rsrc/css/core/core.css' => 'd0801452', 'rsrc/css/core/remarkup.css' => '787105d6', - 'rsrc/css/core/syntax.css' => '5101175d', + 'rsrc/css/core/syntax.css' => '9fc496d5', 'rsrc/css/core/z-index.css' => '5b6fcf3f', 'rsrc/css/diviner/diviner-shared.css' => 'aa3656aa', 'rsrc/css/font/font-aleo.css' => '8bdb2835', @@ -159,7 +159,7 @@ return array( 'rsrc/css/phui/phui-two-column-view.css' => 'b9538af1', 'rsrc/css/phui/workboards/phui-workboard-color.css' => 'ac6fe6a7', 'rsrc/css/phui/workboards/phui-workboard.css' => 'e6d89647', - 'rsrc/css/phui/workboards/phui-workcard.css' => '3646fb96', + 'rsrc/css/phui/workboards/phui-workcard.css' => '0c62d7c5', 'rsrc/css/phui/workboards/phui-workpanel.css' => '92197373', 'rsrc/css/sprite-login.css' => '60e8560e', 'rsrc/css/sprite-menu.css' => '9dd65b92', @@ -550,7 +550,7 @@ return array( 'conpherence-update-css' => 'faf6be09', 'conpherence-widget-pane-css' => '775eaaba', 'd3' => 'a11a5ff2', - 'differential-changeset-view-css' => '3e3b0b76', + 'differential-changeset-view-css' => '7bcbe615', 'differential-core-view-css' => '5b7b8ff4', 'differential-inline-comment-editor' => '64a5550f', 'differential-revision-add-comment-css' => 'c47f8c40', @@ -804,7 +804,7 @@ return array( 'phabricator-zindex-css' => '5b6fcf3f', 'phame-css' => '737792d6', 'pholio-css' => 'ca89d380', - 'pholio-edit-css' => '3ad9d1ee', + 'pholio-edit-css' => 'b15fec4a', 'pholio-inline-comments-css' => '8e545e49', 'phortune-credit-card-form' => '2290aeef', 'phortune-credit-card-form-css' => '8391eb02', @@ -858,7 +858,7 @@ return array( 'phui-two-column-view-css' => 'b9538af1', 'phui-workboard-color-css' => 'ac6fe6a7', 'phui-workboard-view-css' => 'e6d89647', - 'phui-workcard-view-css' => '3646fb96', + 'phui-workcard-view-css' => '0c62d7c5', 'phui-workpanel-view-css' => '92197373', 'phuix-action-list-view' => 'b5c256b8', 'phuix-action-view' => '8cf6d262', @@ -881,7 +881,7 @@ return array( 'sprite-menu-css' => '9dd65b92', 'sprite-tokens-css' => '4f399012', 'syntax-default-css' => '9923583c', - 'syntax-highlighting-css' => '5101175d', + 'syntax-highlighting-css' => '9fc496d5', 'tokens-css' => '3d0f239e', 'typeahead-browse-css' => 'd8581d2c', 'unhandled-exception-css' => '4c96257a', @@ -1148,9 +1148,6 @@ return array( 'javelin-util', 'javelin-uri', ), - '3e3b0b76' => array( - 'phui-inline-comment-view-css', - ), '3f5d6dbf' => array( 'javelin-behavior', 'javelin-dom', @@ -1243,9 +1240,6 @@ return array( 'javelin-typeahead-source', 'javelin-util', ), - '5101175d' => array( - 'syntax-default-css', - ), '519705ea' => array( 'javelin-install', 'javelin-dom', @@ -1495,6 +1489,9 @@ return array( 'javelin-stratcom', 'javelin-util', ), + '7bcbe615' => array( + 'phui-inline-comment-view-css', + ), '7cbe244b' => array( 'javelin-install', 'javelin-util', @@ -1660,6 +1657,9 @@ return array( 'javelin-dom', 'javelin-vector', ), + '9fc496d5' => array( + 'syntax-default-css', + ), 'a0b57eb8' => array( 'javelin-behavior', 'javelin-dom', diff --git a/resources/sql/autopatches/20160513.owners.01.autoreview.sql b/resources/sql/autopatches/20160513.owners.01.autoreview.sql new file mode 100644 index 0000000000..8b3d6e5819 --- /dev/null +++ b/resources/sql/autopatches/20160513.owners.01.autoreview.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_owners.owners_package + ADD autoReview VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20160513.owners.02.autoreviewnone.sql b/resources/sql/autopatches/20160513.owners.02.autoreviewnone.sql new file mode 100644 index 0000000000..d5c8a184e5 --- /dev/null +++ b/resources/sql/autopatches/20160513.owners.02.autoreviewnone.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_owners.owners_package + SET autoReview = 'none' WHERE autoReview = ''; diff --git a/resources/sql/autopatches/20160516.owners.01.dominion.sql b/resources/sql/autopatches/20160516.owners.01.dominion.sql new file mode 100644 index 0000000000..2fa4b0cae3 --- /dev/null +++ b/resources/sql/autopatches/20160516.owners.01.dominion.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_owners.owners_package + ADD dominion VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20160516.owners.02.dominionstrong.sql b/resources/sql/autopatches/20160516.owners.02.dominionstrong.sql new file mode 100644 index 0000000000..60177c554e --- /dev/null +++ b/resources/sql/autopatches/20160516.owners.02.dominionstrong.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_owners.owners_package + SET dominion = 'strong' WHERE dominion = ''; diff --git a/resources/sql/autopatches/20160517.oauth.01.edge.sql b/resources/sql/autopatches/20160517.oauth.01.edge.sql new file mode 100644 index 0000000000..7881d89251 --- /dev/null +++ b/resources/sql/autopatches/20160517.oauth.01.edge.sql @@ -0,0 +1,16 @@ +CREATE TABLE {$NAMESPACE}_oauth_server.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}_oauth_server.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/20160518.ssh.01.activecol.sql b/resources/sql/autopatches/20160518.ssh.01.activecol.sql new file mode 100644 index 0000000000..09c3e16df1 --- /dev/null +++ b/resources/sql/autopatches/20160518.ssh.01.activecol.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_auth.auth_sshkey + ADD isActive BOOL; diff --git a/resources/sql/autopatches/20160518.ssh.02.activeval.sql b/resources/sql/autopatches/20160518.ssh.02.activeval.sql new file mode 100644 index 0000000000..c70f91492c --- /dev/null +++ b/resources/sql/autopatches/20160518.ssh.02.activeval.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_auth.auth_sshkey + SET isActive = 1; diff --git a/resources/sql/autopatches/20160518.ssh.03.activekey.sql b/resources/sql/autopatches/20160518.ssh.03.activekey.sql new file mode 100644 index 0000000000..a6775edf92 --- /dev/null +++ b/resources/sql/autopatches/20160518.ssh.03.activekey.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_auth.auth_sshkey + ADD UNIQUE KEY `key_activeunique` (keyIndex, isActive); diff --git a/resources/sql/autopatches/20160519.ssh.01.xaction.sql b/resources/sql/autopatches/20160519.ssh.01.xaction.sql new file mode 100644 index 0000000000..8b6ddc62cd --- /dev/null +++ b/resources/sql/autopatches/20160519.ssh.01.xaction.sql @@ -0,0 +1,19 @@ +CREATE TABLE {$NAMESPACE}_auth.auth_sshkeytransaction ( + 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/scripts/ssh/ssh-auth-key.php b/scripts/ssh/ssh-auth-key.php index 80c553e563..0c23a20edf 100755 --- a/scripts/ssh/ssh-auth-key.php +++ b/scripts/ssh/ssh-auth-key.php @@ -14,6 +14,7 @@ try { $key = id(new PhabricatorAuthSSHKeyQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withKeys(array($public_key)) + ->withIsActive(true) ->executeOne(); if (!$key) { exit(1); diff --git a/scripts/ssh/ssh-auth.php b/scripts/ssh/ssh-auth.php index 5fa5891f49..af6f7f7f43 100755 --- a/scripts/ssh/ssh-auth.php +++ b/scripts/ssh/ssh-auth.php @@ -6,6 +6,7 @@ require_once $root.'/scripts/__init_script__.php'; $keys = id(new PhabricatorAuthSSHKeyQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withIsActive(true) ->execute(); if (!$keys) { diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e11c20a33c..7df48b8426 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -361,6 +361,7 @@ phutil_register_library_map(array( 'DifferentialAuthorField' => 'applications/differential/customfield/DifferentialAuthorField.php', 'DifferentialBlameRevisionField' => 'applications/differential/customfield/DifferentialBlameRevisionField.php', 'DifferentialBlockHeraldAction' => 'applications/differential/herald/DifferentialBlockHeraldAction.php', + 'DifferentialBlockingReviewerDatasource' => 'applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php', 'DifferentialBranchField' => 'applications/differential/customfield/DifferentialBranchField.php', 'DifferentialChangeDetailMailView' => 'applications/differential/mail/DifferentialChangeDetailMailView.php', 'DifferentialChangeHeraldFieldGroup' => 'applications/differential/herald/DifferentialChangeHeraldFieldGroup.php', @@ -435,6 +436,7 @@ phutil_register_library_map(array( 'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php', 'DifferentialDraft' => 'applications/differential/storage/DifferentialDraft.php', 'DifferentialEditPolicyField' => 'applications/differential/customfield/DifferentialEditPolicyField.php', + 'DifferentialExactUserFunctionDatasource' => 'applications/differential/typeahead/DifferentialExactUserFunctionDatasource.php', 'DifferentialFieldParseException' => 'applications/differential/exception/DifferentialFieldParseException.php', 'DifferentialFieldValidationException' => 'applications/differential/exception/DifferentialFieldValidationException.php', 'DifferentialFindConduitAPIMethod' => 'applications/differential/conduit/DifferentialFindConduitAPIMethod.php', @@ -491,9 +493,13 @@ phutil_register_library_map(array( 'DifferentialRepositoryField' => 'applications/differential/customfield/DifferentialRepositoryField.php', 'DifferentialRepositoryLookup' => 'applications/differential/query/DifferentialRepositoryLookup.php', 'DifferentialRequiredSignaturesField' => 'applications/differential/customfield/DifferentialRequiredSignaturesField.php', + 'DifferentialResponsibleDatasource' => 'applications/differential/typeahead/DifferentialResponsibleDatasource.php', + 'DifferentialResponsibleUserDatasource' => 'applications/differential/typeahead/DifferentialResponsibleUserDatasource.php', + 'DifferentialResponsibleViewerFunctionDatasource' => 'applications/differential/typeahead/DifferentialResponsibleViewerFunctionDatasource.php', 'DifferentialRevertPlanField' => 'applications/differential/customfield/DifferentialRevertPlanField.php', 'DifferentialReviewedByField' => 'applications/differential/customfield/DifferentialReviewedByField.php', 'DifferentialReviewer' => 'applications/differential/storage/DifferentialReviewer.php', + 'DifferentialReviewerDatasource' => 'applications/differential/typeahead/DifferentialReviewerDatasource.php', 'DifferentialReviewerForRevisionEdgeType' => 'applications/differential/edge/DifferentialReviewerForRevisionEdgeType.php', 'DifferentialReviewerStatus' => 'applications/differential/constants/DifferentialReviewerStatus.php', 'DifferentialReviewersAddBlockingReviewersHeraldAction' => 'applications/differential/herald/DifferentialReviewersAddBlockingReviewersHeraldAction.php', @@ -533,6 +539,8 @@ phutil_register_library_map(array( 'DifferentialRevisionQuery' => 'applications/differential/query/DifferentialRevisionQuery.php', 'DifferentialRevisionRepositoryHeraldField' => 'applications/differential/herald/DifferentialRevisionRepositoryHeraldField.php', 'DifferentialRevisionRepositoryProjectsHeraldField' => 'applications/differential/herald/DifferentialRevisionRepositoryProjectsHeraldField.php', + 'DifferentialRevisionRequiredActionResultBucket' => 'applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php', + 'DifferentialRevisionResultBucket' => 'applications/differential/query/DifferentialRevisionResultBucket.php', 'DifferentialRevisionReviewersHeraldField' => 'applications/differential/herald/DifferentialRevisionReviewersHeraldField.php', 'DifferentialRevisionSearchEngine' => 'applications/differential/query/DifferentialRevisionSearchEngine.php', 'DifferentialRevisionStatus' => 'applications/differential/constants/DifferentialRevisionStatus.php', @@ -1869,12 +1877,19 @@ phutil_register_library_map(array( 'PhabricatorAuthRevokeTokenController' => 'applications/auth/controller/PhabricatorAuthRevokeTokenController.php', 'PhabricatorAuthSSHKey' => 'applications/auth/storage/PhabricatorAuthSSHKey.php', 'PhabricatorAuthSSHKeyController' => 'applications/auth/controller/PhabricatorAuthSSHKeyController.php', - 'PhabricatorAuthSSHKeyDeleteController' => 'applications/auth/controller/PhabricatorAuthSSHKeyDeleteController.php', + 'PhabricatorAuthSSHKeyDeactivateController' => 'applications/auth/controller/PhabricatorAuthSSHKeyDeactivateController.php', 'PhabricatorAuthSSHKeyEditController' => 'applications/auth/controller/PhabricatorAuthSSHKeyEditController.php', + 'PhabricatorAuthSSHKeyEditor' => 'applications/auth/editor/PhabricatorAuthSSHKeyEditor.php', 'PhabricatorAuthSSHKeyGenerateController' => 'applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php', + 'PhabricatorAuthSSHKeyListController' => 'applications/auth/controller/PhabricatorAuthSSHKeyListController.php', 'PhabricatorAuthSSHKeyPHIDType' => 'applications/auth/phid/PhabricatorAuthSSHKeyPHIDType.php', 'PhabricatorAuthSSHKeyQuery' => 'applications/auth/query/PhabricatorAuthSSHKeyQuery.php', + 'PhabricatorAuthSSHKeyReplyHandler' => 'applications/auth/mail/PhabricatorAuthSSHKeyReplyHandler.php', + 'PhabricatorAuthSSHKeySearchEngine' => 'applications/auth/query/PhabricatorAuthSSHKeySearchEngine.php', 'PhabricatorAuthSSHKeyTableView' => 'applications/auth/view/PhabricatorAuthSSHKeyTableView.php', + 'PhabricatorAuthSSHKeyTransaction' => 'applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php', + 'PhabricatorAuthSSHKeyTransactionQuery' => 'applications/auth/query/PhabricatorAuthSSHKeyTransactionQuery.php', + 'PhabricatorAuthSSHKeyViewController' => 'applications/auth/controller/PhabricatorAuthSSHKeyViewController.php', 'PhabricatorAuthSSHPublicKey' => 'applications/auth/sshkey/PhabricatorAuthSSHPublicKey.php', 'PhabricatorAuthSession' => 'applications/auth/storage/PhabricatorAuthSession.php', 'PhabricatorAuthSessionEngine' => 'applications/auth/engine/PhabricatorAuthSessionEngine.php', @@ -2161,7 +2176,6 @@ phutil_register_library_map(array( 'PhabricatorCountdownTransactionQuery' => 'applications/countdown/query/PhabricatorCountdownTransactionQuery.php', 'PhabricatorCountdownView' => 'applications/countdown/view/PhabricatorCountdownView.php', 'PhabricatorCountdownViewController' => 'applications/countdown/controller/PhabricatorCountdownViewController.php', - 'PhabricatorCredentialsUsedByObjectEdgeType' => 'applications/passphrase/edge/PhabricatorCredentialsUsedByObjectEdgeType.php', 'PhabricatorCursorPagedPolicyAwareQuery' => 'infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php', 'PhabricatorCustomField' => 'infrastructure/customfield/field/PhabricatorCustomField.php', 'PhabricatorCustomFieldAttachment' => 'infrastructure/customfield/field/PhabricatorCustomFieldAttachment.php', @@ -2779,6 +2793,7 @@ phutil_register_library_map(array( 'PhabricatorOAuthServerDAO' => 'applications/oauthserver/storage/PhabricatorOAuthServerDAO.php', 'PhabricatorOAuthServerEditEngine' => 'applications/oauthserver/editor/PhabricatorOAuthServerEditEngine.php', 'PhabricatorOAuthServerEditor' => 'applications/oauthserver/editor/PhabricatorOAuthServerEditor.php', + 'PhabricatorOAuthServerSchemaSpec' => 'applications/oauthserver/query/PhabricatorOAuthServerSchemaSpec.php', 'PhabricatorOAuthServerScope' => 'applications/oauthserver/PhabricatorOAuthServerScope.php', 'PhabricatorOAuthServerTestCase' => 'applications/oauthserver/__tests__/PhabricatorOAuthServerTestCase.php', 'PhabricatorOAuthServerTokenController' => 'applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php', @@ -2802,7 +2817,6 @@ phutil_register_library_map(array( 'PhabricatorObjectQuery' => 'applications/phid/query/PhabricatorObjectQuery.php', 'PhabricatorObjectRemarkupRule' => 'infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php', 'PhabricatorObjectSelectorDialog' => 'view/control/PhabricatorObjectSelectorDialog.php', - 'PhabricatorObjectUsesCredentialsEdgeType' => 'applications/transactions/edges/PhabricatorObjectUsesCredentialsEdgeType.php', 'PhabricatorOffsetPagedQuery' => 'infrastructure/query/PhabricatorOffsetPagedQuery.php', 'PhabricatorOldWorldContentSource' => 'infrastructure/contentsource/PhabricatorOldWorldContentSource.php', 'PhabricatorOneTimeTriggerClock' => 'infrastructure/daemon/workers/clock/PhabricatorOneTimeTriggerClock.php', @@ -2833,6 +2847,7 @@ phutil_register_library_map(array( 'PhabricatorOwnersPackageOwnerDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageOwnerDatasource.php', 'PhabricatorOwnersPackagePHIDType' => 'applications/owners/phid/PhabricatorOwnersPackagePHIDType.php', 'PhabricatorOwnersPackageQuery' => 'applications/owners/query/PhabricatorOwnersPackageQuery.php', + 'PhabricatorOwnersPackageRemarkupRule' => 'applications/owners/remarkup/PhabricatorOwnersPackageRemarkupRule.php', 'PhabricatorOwnersPackageSearchEngine' => 'applications/owners/query/PhabricatorOwnersPackageSearchEngine.php', 'PhabricatorOwnersPackageTestCase' => 'applications/owners/storage/__tests__/PhabricatorOwnersPackageTestCase.php', 'PhabricatorOwnersPackageTransaction' => 'applications/owners/storage/PhabricatorOwnersPackageTransaction.php', @@ -3316,6 +3331,8 @@ phutil_register_library_map(array( 'PhabricatorSearchOrderField' => 'applications/search/field/PhabricatorSearchOrderField.php', 'PhabricatorSearchPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorSearchPreferencesSettingsPanel.php', 'PhabricatorSearchRelationship' => 'applications/search/constants/PhabricatorSearchRelationship.php', + 'PhabricatorSearchResultBucket' => 'applications/search/buckets/PhabricatorSearchResultBucket.php', + 'PhabricatorSearchResultBucketGroup' => 'applications/search/buckets/PhabricatorSearchResultBucketGroup.php', 'PhabricatorSearchResultView' => 'applications/search/view/PhabricatorSearchResultView.php', 'PhabricatorSearchSchemaSpec' => 'applications/search/storage/PhabricatorSearchSchemaSpec.php', 'PhabricatorSearchSelectController' => 'applications/search/controller/PhabricatorSearchSelectController.php', @@ -4554,6 +4571,7 @@ phutil_register_library_map(array( 'DifferentialAuthorField' => 'DifferentialCustomField', 'DifferentialBlameRevisionField' => 'DifferentialStoredCustomField', 'DifferentialBlockHeraldAction' => 'HeraldAction', + 'DifferentialBlockingReviewerDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'DifferentialBranchField' => 'DifferentialCustomField', 'DifferentialChangeDetailMailView' => 'DifferentialMailView', 'DifferentialChangeHeraldFieldGroup' => 'HeraldFieldGroup', @@ -4638,6 +4656,7 @@ phutil_register_library_map(array( 'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'DoorkeeperFeedStoryPublisher', 'DifferentialDraft' => 'DifferentialDAO', 'DifferentialEditPolicyField' => 'DifferentialCoreCustomField', + 'DifferentialExactUserFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'DifferentialFieldParseException' => 'Exception', 'DifferentialFieldValidationException' => 'Exception', 'DifferentialFindConduitAPIMethod' => 'DifferentialConduitAPIMethod', @@ -4700,9 +4719,13 @@ phutil_register_library_map(array( 'DifferentialRepositoryField' => 'DifferentialCoreCustomField', 'DifferentialRepositoryLookup' => 'Phobject', 'DifferentialRequiredSignaturesField' => 'DifferentialCoreCustomField', + 'DifferentialResponsibleDatasource' => 'PhabricatorTypeaheadCompositeDatasource', + 'DifferentialResponsibleUserDatasource' => 'PhabricatorTypeaheadCompositeDatasource', + 'DifferentialResponsibleViewerFunctionDatasource' => 'PhabricatorTypeaheadDatasource', 'DifferentialRevertPlanField' => 'DifferentialStoredCustomField', 'DifferentialReviewedByField' => 'DifferentialCoreCustomField', 'DifferentialReviewer' => 'Phobject', + 'DifferentialReviewerDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'DifferentialReviewerForRevisionEdgeType' => 'PhabricatorEdgeType', 'DifferentialReviewerStatus' => 'Phobject', 'DifferentialReviewersAddBlockingReviewersHeraldAction' => 'DifferentialReviewersHeraldAction', @@ -4757,6 +4780,8 @@ phutil_register_library_map(array( 'DifferentialRevisionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DifferentialRevisionRepositoryHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionRepositoryProjectsHeraldField' => 'DifferentialRevisionHeraldField', + 'DifferentialRevisionRequiredActionResultBucket' => 'DifferentialRevisionResultBucket', + 'DifferentialRevisionResultBucket' => 'PhabricatorSearchResultBucket', 'DifferentialRevisionReviewersHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionSearchEngine' => 'PhabricatorApplicationSearchEngine', 'DifferentialRevisionStatus' => 'Phobject', @@ -6286,14 +6311,22 @@ phutil_register_library_map(array( 'PhabricatorAuthDAO', 'PhabricatorPolicyInterface', 'PhabricatorDestructibleInterface', + 'PhabricatorApplicationTransactionInterface', ), 'PhabricatorAuthSSHKeyController' => 'PhabricatorAuthController', - 'PhabricatorAuthSSHKeyDeleteController' => 'PhabricatorAuthSSHKeyController', + 'PhabricatorAuthSSHKeyDeactivateController' => 'PhabricatorAuthSSHKeyController', 'PhabricatorAuthSSHKeyEditController' => 'PhabricatorAuthSSHKeyController', + 'PhabricatorAuthSSHKeyEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorAuthSSHKeyGenerateController' => 'PhabricatorAuthSSHKeyController', + 'PhabricatorAuthSSHKeyListController' => 'PhabricatorAuthSSHKeyController', 'PhabricatorAuthSSHKeyPHIDType' => 'PhabricatorPHIDType', 'PhabricatorAuthSSHKeyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorAuthSSHKeyReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', + 'PhabricatorAuthSSHKeySearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorAuthSSHKeyTableView' => 'AphrontView', + 'PhabricatorAuthSSHKeyTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorAuthSSHKeyTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorAuthSSHKeyViewController' => 'PhabricatorAuthSSHKeyController', 'PhabricatorAuthSSHPublicKey' => 'Phobject', 'PhabricatorAuthSession' => array( 'PhabricatorAuthDAO', @@ -6643,7 +6676,6 @@ phutil_register_library_map(array( 'PhabricatorCountdownTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorCountdownView' => 'AphrontView', 'PhabricatorCountdownViewController' => 'PhabricatorCountdownController', - 'PhabricatorCredentialsUsedByObjectEdgeType' => 'PhabricatorEdgeType', 'PhabricatorCursorPagedPolicyAwareQuery' => 'PhabricatorPolicyAwareQuery', 'PhabricatorCustomField' => 'Phobject', 'PhabricatorCustomFieldAttachment' => 'Phobject', @@ -7342,6 +7374,7 @@ phutil_register_library_map(array( 'PhabricatorOAuthServerDAO' => 'PhabricatorLiskDAO', 'PhabricatorOAuthServerEditEngine' => 'PhabricatorEditEngine', 'PhabricatorOAuthServerEditor' => 'PhabricatorApplicationTransactionEditor', + 'PhabricatorOAuthServerSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorOAuthServerScope' => 'Phobject', 'PhabricatorOAuthServerTestCase' => 'PhabricatorTestCase', 'PhabricatorOAuthServerTokenController' => 'PhabricatorOAuthServerController', @@ -7368,7 +7401,6 @@ phutil_register_library_map(array( 'PhabricatorObjectQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorObjectRemarkupRule' => 'PhutilRemarkupRule', 'PhabricatorObjectSelectorDialog' => 'Phobject', - 'PhabricatorObjectUsesCredentialsEdgeType' => 'PhabricatorEdgeType', 'PhabricatorOffsetPagedQuery' => 'PhabricatorQuery', 'PhabricatorOldWorldContentSource' => 'PhabricatorContentSource', 'PhabricatorOneTimeTriggerClock' => 'PhabricatorTriggerClock', @@ -7411,6 +7443,7 @@ phutil_register_library_map(array( 'PhabricatorOwnersPackageOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorOwnersPackagePHIDType' => 'PhabricatorPHIDType', 'PhabricatorOwnersPackageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorOwnersPackageRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'PhabricatorOwnersPackageSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorOwnersPackageTestCase' => 'PhabricatorTestCase', 'PhabricatorOwnersPackageTransaction' => 'PhabricatorApplicationTransaction', @@ -8007,6 +8040,8 @@ phutil_register_library_map(array( 'PhabricatorSearchOrderField' => 'PhabricatorSearchField', 'PhabricatorSearchPreferencesSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorSearchRelationship' => 'Phobject', + 'PhabricatorSearchResultBucket' => 'Phobject', + 'PhabricatorSearchResultBucketGroup' => 'Phobject', 'PhabricatorSearchResultView' => 'AphrontView', 'PhabricatorSearchSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorSearchSelectController' => 'PhabricatorSearchBaseController', diff --git a/src/applications/almanac/controller/AlmanacDeviceViewController.php b/src/applications/almanac/controller/AlmanacDeviceViewController.php index 000c8f8971..99cc9c2526 100644 --- a/src/applications/almanac/controller/AlmanacDeviceViewController.php +++ b/src/applications/almanac/controller/AlmanacDeviceViewController.php @@ -146,6 +146,7 @@ final class AlmanacDeviceViewController $keys = id(new PhabricatorAuthSSHKeyQuery()) ->setViewer($viewer) ->withObjectPHIDs(array($device_phid)) + ->withIsActive(true) ->execute(); $table = id(new PhabricatorAuthSSHKeyTableView()) @@ -156,38 +157,13 @@ final class AlmanacDeviceViewController ->setShowTrusted(true) ->setNoDataString(pht('This device has no associated SSH public keys.')); - try { - PhabricatorSSHKeyGenerator::assertCanGenerateKeypair(); - $can_generate = true; - } catch (Exception $ex) { - $can_generate = false; - } - - $generate_uri = '/auth/sshkey/generate/?objectPHID='.$device_phid; - $upload_uri = '/auth/sshkey/upload/?objectPHID='.$device_phid; + $menu_button = PhabricatorAuthSSHKeyTableView::newKeyActionsMenu( + $viewer, + $device); $header = id(new PHUIHeaderView()) ->setHeader(pht('SSH Public Keys')) - ->addActionLink( - id(new PHUIButtonView()) - ->setTag('a') - ->setHref($generate_uri) - ->setWorkflow(true) - ->setDisabled(!$can_edit || !$can_generate) - ->setText(pht('Generate Keypair')) - ->setIcon( - id(new PHUIIconView()) - ->setIcon('fa-lock'))) - ->addActionLink( - id(new PHUIButtonView()) - ->setTag('a') - ->setHref($upload_uri) - ->setWorkflow(true) - ->setDisabled(!$can_edit) - ->setText(pht('Upload Public Key')) - ->setIcon( - id(new PHUIIconView()) - ->setIcon('fa-upload'))); + ->addActionLink($menu_button); return id(new PHUIObjectBoxView()) ->setHeader($header) diff --git a/src/applications/almanac/management/AlmanacManagementRegisterWorkflow.php b/src/applications/almanac/management/AlmanacManagementRegisterWorkflow.php index 0493068eb4..ebe992469f 100644 --- a/src/applications/almanac/management/AlmanacManagementRegisterWorkflow.php +++ b/src/applications/almanac/management/AlmanacManagementRegisterWorkflow.php @@ -141,6 +141,7 @@ final class AlmanacManagementRegisterWorkflow $public_key = id(new PhabricatorAuthSSHKeyQuery()) ->setViewer($this->getViewer()) ->withKeys(array($key_object)) + ->withIsActive(true) ->executeOne(); if (!$public_key) { diff --git a/src/applications/almanac/management/AlmanacManagementTrustKeyWorkflow.php b/src/applications/almanac/management/AlmanacManagementTrustKeyWorkflow.php index 81ece51b72..c0bbc59ff0 100644 --- a/src/applications/almanac/management/AlmanacManagementTrustKeyWorkflow.php +++ b/src/applications/almanac/management/AlmanacManagementTrustKeyWorkflow.php @@ -35,6 +35,11 @@ final class AlmanacManagementTrustKeyWorkflow pht('No public key exists with ID "%s".', $id)); } + if (!$key->getIsActive()) { + throw new PhutilArgumentUsageException( + pht('Public key "%s" is not an active key.', $id)); + } + if ($key->getIsTrusted()) { throw new PhutilArgumentUsageException( pht('Public key with ID %s is already trusted.', $id)); diff --git a/src/applications/almanac/storage/AlmanacDevice.php b/src/applications/almanac/storage/AlmanacDevice.php index 6c7f3cb57f..d2a3e4a763 100644 --- a/src/applications/almanac/storage/AlmanacDevice.php +++ b/src/applications/almanac/storage/AlmanacDevice.php @@ -227,6 +227,14 @@ final class AlmanacDevice return $this->getName(); } + public function getSSHKeyNotifyPHIDs() { + // Devices don't currently have anyone useful to notify about SSH key + // edits, and they're usually a difficult vector to attack since you need + // access to a cluster host. However, it would be nice to make them + // subscribable at some point. + return array(); + } + /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 3df9013301..8018e5314a 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -159,7 +159,17 @@ final class PhabricatorAuditEditor $requests = mpull($requests, null, 'getAuditorPHID'); foreach ($add as $phid) { if (isset($requests[$phid])) { - continue; + $request = $requests[$phid]; + + // Only update an existing request if the current status is not + // an interesting status. + if ($request->isInteresting()) { + continue; + } + } else { + $request = id(new PhabricatorRepositoryAuditRequest()) + ->setCommitPHID($object->getPHID()) + ->setAuditorPHID($phid); } if ($this->getIsHeraldEditor()) { @@ -170,12 +180,13 @@ final class PhabricatorAuditEditor $audit_requested = PhabricatorAuditStatusConstants::AUDIT_REQUESTED; $audit_reason = $this->getAuditReasons($phid); } - $requests[] = id(new PhabricatorRepositoryAuditRequest()) - ->setCommitPHID($object->getPHID()) - ->setAuditorPHID($phid) + + $request ->setAuditStatus($audit_requested) ->setAuditReasons($audit_reason) ->save(); + + $requests[$phid] = $request; } $object->attachAudits($requests); diff --git a/src/applications/auth/application/PhabricatorAuthApplication.php b/src/applications/auth/application/PhabricatorAuthApplication.php index 808562e970..dfc855f315 100644 --- a/src/applications/auth/application/PhabricatorAuthApplication.php +++ b/src/applications/auth/application/PhabricatorAuthApplication.php @@ -75,10 +75,14 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { 'multifactor/' => 'PhabricatorAuthNeedsMultiFactorController', 'sshkey/' => array( + $this->getQueryRoutePattern('for/(?P[^/]+)/') + => 'PhabricatorAuthSSHKeyListController', 'generate/' => 'PhabricatorAuthSSHKeyGenerateController', 'upload/' => 'PhabricatorAuthSSHKeyEditController', 'edit/(?P\d+)/' => 'PhabricatorAuthSSHKeyEditController', - 'delete/(?P\d+)/' => 'PhabricatorAuthSSHKeyDeleteController', + 'deactivate/(?P\d+)/' + => 'PhabricatorAuthSSHKeyDeactivateController', + 'view/(?P\d+)/' => 'PhabricatorAuthSSHKeyViewController', ), ), diff --git a/src/applications/auth/conduit/PhabricatorAuthQueryPublicKeysConduitAPIMethod.php b/src/applications/auth/conduit/PhabricatorAuthQueryPublicKeysConduitAPIMethod.php index be91af7863..ae7c7f1391 100644 --- a/src/applications/auth/conduit/PhabricatorAuthQueryPublicKeysConduitAPIMethod.php +++ b/src/applications/auth/conduit/PhabricatorAuthQueryPublicKeysConduitAPIMethod.php @@ -28,7 +28,8 @@ final class PhabricatorAuthQueryPublicKeysConduitAPIMethod $viewer = $request->getUser(); $query = id(new PhabricatorAuthSSHKeyQuery()) - ->setViewer($viewer); + ->setViewer($viewer) + ->withIsActive(true); $ids = $request->getValue('ids'); if ($ids !== null) { diff --git a/src/applications/auth/controller/PhabricatorAuthSSHKeyController.php b/src/applications/auth/controller/PhabricatorAuthSSHKeyController.php index 86cf81778f..f6c14c0c56 100644 --- a/src/applications/auth/controller/PhabricatorAuthSSHKeyController.php +++ b/src/applications/auth/controller/PhabricatorAuthSSHKeyController.php @@ -3,18 +3,34 @@ abstract class PhabricatorAuthSSHKeyController extends PhabricatorAuthController { - protected function newKeyForObjectPHID($object_phid) { + private $keyObject; + + public function setSSHKeyObject(PhabricatorSSHPublicKeyInterface $object) { + $this->keyObject = $object; + return $this; + } + + public function getSSHKeyObject() { + return $this->keyObject; + } + + protected function loadSSHKeyObject($object_phid, $need_edit) { $viewer = $this->getViewer(); - $object = id(new PhabricatorObjectQuery()) + $query = id(new PhabricatorObjectQuery()) ->setViewer($viewer) - ->withPHIDs(array($object_phid)) - ->requireCapabilities( + ->withPHIDs(array($object_phid)); + + if ($need_edit) { + $query->requireCapabilities( array( PhabricatorPolicyCapability::CAN_VIEW, PhabricatorPolicyCapability::CAN_EDIT, - )) - ->executeOne(); + )); + } + + $object = $query->executeOne(); + if (!$object) { return null; } @@ -25,9 +41,38 @@ abstract class PhabricatorAuthSSHKeyController return null; } - return id(new PhabricatorAuthSSHKey()) - ->setObjectPHID($object_phid) - ->attachObject($object); + $this->keyObject = $object; + + return $object; + } + + protected function newKeyForObjectPHID($object_phid) { + $viewer = $this->getViewer(); + + $object = $this->loadSSHKeyObject($object_phid, true); + if (!$object) { + return null; + } + + return PhabricatorAuthSSHKey::initializeNewSSHKey($viewer, $object); + } + + protected function buildApplicationCrumbs() { + $crumbs = parent::buildApplicationCrumbs(); + $viewer = $this->getViewer(); + + $key_object = $this->getSSHKeyObject(); + if ($key_object) { + $object_phid = $key_object->getPHID(); + $handles = $viewer->loadHandles(array($object_phid)); + $handle = $handles[$object_phid]; + + $uri = $key_object->getSSHPublicKeyManagementURI($viewer); + + $crumbs->addTextCrumb($handle->getObjectName(), $uri); + } + + return $crumbs; } } diff --git a/src/applications/auth/controller/PhabricatorAuthSSHKeyDeleteController.php b/src/applications/auth/controller/PhabricatorAuthSSHKeyDeactivateController.php similarity index 53% rename from src/applications/auth/controller/PhabricatorAuthSSHKeyDeleteController.php rename to src/applications/auth/controller/PhabricatorAuthSSHKeyDeactivateController.php index 6c18e211cc..8eca02340d 100644 --- a/src/applications/auth/controller/PhabricatorAuthSSHKeyDeleteController.php +++ b/src/applications/auth/controller/PhabricatorAuthSSHKeyDeactivateController.php @@ -1,6 +1,6 @@ getObject()->getSSHPublicKeyManagementURI($viewer); + $cancel_uri = $key->getURI(); $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( $viewer, @@ -27,21 +27,33 @@ final class PhabricatorAuthSSHKeyDeleteController $cancel_uri); if ($request->isFormPost()) { - // TODO: It would be nice to write an edge transaction here or something. - $key->delete(); + $xactions = array(); + + $xactions[] = id(new PhabricatorAuthSSHKeyTransaction()) + ->setTransactionType(PhabricatorAuthSSHKeyTransaction::TYPE_DEACTIVATE) + ->setNewValue(true); + + id(new PhabricatorAuthSSHKeyEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->applyTransactions($key, $xactions); + return id(new AphrontRedirectResponse())->setURI($cancel_uri); } $name = phutil_tag('strong', array(), $key->getName()); return $this->newDialog() - ->setTitle(pht('Really delete SSH Public Key?')) + ->setTitle(pht('Deactivate SSH Public Key')) ->appendParagraph( pht( - 'The key "%s" will be permanently deleted, and you will not longer '. - 'be able to use the corresponding private key to authenticate.', + 'The key "%s" will be permanently deactivated, and you will no '. + 'longer be able to use the corresponding private key to '. + 'authenticate.', $name)) - ->addSubmitButton(pht('Delete Public Key')) + ->addSubmitButton(pht('Deactivate Public Key')) ->addCancelButton($cancel_uri); } diff --git a/src/applications/auth/controller/PhabricatorAuthSSHKeyEditController.php b/src/applications/auth/controller/PhabricatorAuthSSHKeyEditController.php index d09d52cc14..1023b0cd75 100644 --- a/src/applications/auth/controller/PhabricatorAuthSSHKeyEditController.php +++ b/src/applications/auth/controller/PhabricatorAuthSSHKeyEditController.php @@ -59,51 +59,45 @@ final class PhabricatorAuthSSHKeyEditController $v_key = $key->getEntireKey(); $e_key = strlen($v_key) ? null : true; - $errors = array(); + $validation_exception = null; if ($request->isFormPost()) { + $type_create = PhabricatorTransactions::TYPE_CREATE; + $type_name = PhabricatorAuthSSHKeyTransaction::TYPE_NAME; + $type_key = PhabricatorAuthSSHKeyTransaction::TYPE_KEY; + + $e_name = null; + $e_key = null; + $v_name = $request->getStr('name'); $v_key = $request->getStr('key'); - if (!strlen($v_name)) { - $errors[] = pht('You must provide a name for this public key.'); - $e_name = pht('Required'); - } else { - $key->setName($v_name); + $xactions = array(); + + if (!$key->getID()) { + $xactions[] = id(new PhabricatorAuthSSHKeyTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_CREATE); } - if (!strlen($v_key)) { - $errors[] = pht('You must provide a public key.'); - $e_key = pht('Required'); - } else { - try { - $public_key = PhabricatorAuthSSHPublicKey::newFromRawKey($v_key); + $xactions[] = id(new PhabricatorAuthSSHKeyTransaction()) + ->setTransactionType($type_name) + ->setNewValue($v_name); - $type = $public_key->getType(); - $body = $public_key->getBody(); - $comment = $public_key->getComment(); + $xactions[] = id(new PhabricatorAuthSSHKeyTransaction()) + ->setTransactionType($type_key) + ->setNewValue($v_key); - $key->setKeyType($type); - $key->setKeyBody($body); - $key->setKeyComment($comment); + $editor = id(new PhabricatorAuthSSHKeyEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true); - $e_key = null; - } catch (Exception $ex) { - $e_key = pht('Invalid'); - $errors[] = $ex->getMessage(); - } - } - - if (!$errors) { - try { - $key->save(); - return id(new AphrontRedirectResponse())->setURI($cancel_uri); - } catch (Exception $ex) { - $e_key = pht('Duplicate'); - $errors[] = pht( - 'This public key is already associated with another user or '. - 'device. Each key must unambiguously identify a single unique '. - 'owner.'); - } + try { + $editor->applyTransactions($key, $xactions); + return id(new AphrontRedirectResponse())->setURI($key->getURI()); + } catch (PhabricatorApplicationTransactionValidationException $ex) { + $validation_exception = $ex; + $e_name = $ex->getShortMessage($type_name); + $e_key = $ex->getShortMessage($type_key); } } @@ -134,7 +128,7 @@ final class PhabricatorAuthSSHKeyEditController return $this->newDialog() ->setTitle($title) ->setWidth(AphrontDialogView::WIDTH_FORM) - ->setErrors($errors) + ->setValidationException($validation_exception) ->appendForm($form) ->addSubmitButton($save_button) ->addCancelButton($cancel_uri); diff --git a/src/applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php b/src/applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php index 2cb6dc81ea..69d93548e5 100644 --- a/src/applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php +++ b/src/applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php @@ -36,13 +36,31 @@ final class PhabricatorAuthSSHKeyGenerateController $type = $public_key->getType(); $body = $public_key->getBody(); + $comment = pht('Generated'); - $key - ->setName($default_name) - ->setKeyType($type) - ->setKeyBody($body) - ->setKeyComment(pht('Generated')) - ->save(); + $entire_key = "{$type} {$body} {$comment}"; + + $type_create = PhabricatorTransactions::TYPE_CREATE; + $type_name = PhabricatorAuthSSHKeyTransaction::TYPE_NAME; + $type_key = PhabricatorAuthSSHKeyTransaction::TYPE_KEY; + + $xactions = array(); + + $xactions[] = id(new PhabricatorAuthSSHKeyTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_CREATE); + + $xactions[] = id(new PhabricatorAuthSSHKeyTransaction()) + ->setTransactionType($type_name) + ->setNewValue($default_name); + + $xactions[] = id(new PhabricatorAuthSSHKeyTransaction()) + ->setTransactionType($type_key) + ->setNewValue($entire_key); + + $editor = id(new PhabricatorAuthSSHKeyEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->applyTransactions($key, $xactions); // NOTE: We're disabling workflow on submit so the download works. We're // disabling workflow on cancel so the page reloads, showing the new diff --git a/src/applications/auth/controller/PhabricatorAuthSSHKeyListController.php b/src/applications/auth/controller/PhabricatorAuthSSHKeyListController.php new file mode 100644 index 0000000000..ae2bbb9cb5 --- /dev/null +++ b/src/applications/auth/controller/PhabricatorAuthSSHKeyListController.php @@ -0,0 +1,25 @@ +getURIData('forPHID'); + $object = $this->loadSSHKeyObject($object_phid, false); + if (!$object) { + return new Aphront404Response(); + } + + $engine = id(new PhabricatorAuthSSHKeySearchEngine()) + ->setSSHKeyObject($object); + + return id($engine) + ->setController($this) + ->buildResponse(); + } + +} diff --git a/src/applications/auth/controller/PhabricatorAuthSSHKeyViewController.php b/src/applications/auth/controller/PhabricatorAuthSSHKeyViewController.php new file mode 100644 index 0000000000..66c3a5f09b --- /dev/null +++ b/src/applications/auth/controller/PhabricatorAuthSSHKeyViewController.php @@ -0,0 +1,124 @@ +getViewer(); + + $id = $request->getURIData('id'); + + $ssh_key = id(new PhabricatorAuthSSHKeyQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if (!$ssh_key) { + return new Aphront404Response(); + } + + $this->setSSHKeyObject($ssh_key->getObject()); + + $title = pht('SSH Key %d', $ssh_key->getID()); + + $curtain = $this->buildCurtain($ssh_key); + $details = $this->buildPropertySection($ssh_key); + + $header = id(new PHUIHeaderView()) + ->setUser($viewer) + ->setHeader($ssh_key->getName()) + ->setHeaderIcon('fa-key'); + + if ($ssh_key->getIsActive()) { + $header->setStatus('fa-check', 'bluegrey', pht('Active')); + } else { + $header->setStatus('fa-ban', 'dark', pht('Deactivated')); + } + + $header->addActionLink( + id(new PHUIButtonView()) + ->setTag('a') + ->setText(pht('View Active Keys')) + ->setHref($ssh_key->getObject()->getSSHPublicKeyManagementURI($viewer)) + ->setIcon('fa-list-ul')); + + $crumbs = $this->buildApplicationCrumbs(); + $crumbs->addTextCrumb($title); + $crumbs->setBorder(true); + + $timeline = $this->buildTransactionTimeline( + $ssh_key, + new PhabricatorAuthSSHKeyTransactionQuery()); + $timeline->setShouldTerminate(true); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn( + array( + $details, + $timeline, + )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } + + private function buildCurtain(PhabricatorAuthSSHKey $ssh_key) { + $viewer = $this->getViewer(); + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $ssh_key, + PhabricatorPolicyCapability::CAN_EDIT); + + $id = $ssh_key->getID(); + + $edit_uri = $this->getApplicationURI("sshkey/edit/{$id}/"); + $deactivate_uri = $this->getApplicationURI("sshkey/deactivate/{$id}/"); + + $curtain = $this->newCurtainView($ssh_key); + + $curtain->addAction( + id(new PhabricatorActionView()) + ->setIcon('fa-pencil') + ->setName(pht('Edit SSH Key')) + ->setHref($edit_uri) + ->setWorkflow(true) + ->setDisabled(!$can_edit)); + + $curtain->addAction( + id(new PhabricatorActionView()) + ->setIcon('fa-times') + ->setName(pht('Deactivate SSH Key')) + ->setHref($deactivate_uri) + ->setWorkflow(true) + ->setDisabled(!$can_edit)); + + return $curtain; + } + + private function buildPropertySection( + PhabricatorAuthSSHKey $ssh_key) { + $viewer = $this->getViewer(); + + $properties = id(new PHUIPropertyListView()) + ->setUser($viewer); + + $properties->addProperty(pht('SSH Key Type'), $ssh_key->getKeyType()); + $properties->addProperty( + pht('Created'), + phabricator_datetime($ssh_key->getDateCreated(), $viewer)); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Details')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($properties); + } + +} diff --git a/src/applications/auth/editor/PhabricatorAuthSSHKeyEditor.php b/src/applications/auth/editor/PhabricatorAuthSSHKeyEditor.php new file mode 100644 index 0000000000..1fc61ffb2c --- /dev/null +++ b/src/applications/auth/editor/PhabricatorAuthSSHKeyEditor.php @@ -0,0 +1,244 @@ +getTransactionType()) { + case PhabricatorAuthSSHKeyTransaction::TYPE_NAME: + return $object->getName(); + case PhabricatorAuthSSHKeyTransaction::TYPE_KEY: + return $object->getEntireKey(); + case PhabricatorAuthSSHKeyTransaction::TYPE_DEACTIVATE: + return !$object->getIsActive(); + } + + } + + protected function getCustomTransactionNewValue( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case PhabricatorAuthSSHKeyTransaction::TYPE_NAME: + case PhabricatorAuthSSHKeyTransaction::TYPE_KEY: + return $xaction->getNewValue(); + case PhabricatorAuthSSHKeyTransaction::TYPE_DEACTIVATE: + return (bool)$xaction->getNewValue(); + } + } + + protected function applyCustomInternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + $value = $xaction->getNewValue(); + switch ($xaction->getTransactionType()) { + case PhabricatorAuthSSHKeyTransaction::TYPE_NAME: + $object->setName($value); + return; + case PhabricatorAuthSSHKeyTransaction::TYPE_KEY: + $public_key = PhabricatorAuthSSHPublicKey::newFromRawKey($value); + + $type = $public_key->getType(); + $body = $public_key->getBody(); + $comment = $public_key->getComment(); + + $object->setKeyType($type); + $object->setKeyBody($body); + $object->setKeyComment($comment); + return; + case PhabricatorAuthSSHKeyTransaction::TYPE_DEACTIVATE: + if ($value) { + $new = null; + } else { + $new = 1; + } + + $object->setIsActive($new); + return; + } + } + + protected function applyCustomExternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + return; + } + + protected function validateTransaction( + PhabricatorLiskDAO $object, + $type, + array $xactions) { + + $errors = parent::validateTransaction($object, $type, $xactions); + + switch ($type) { + case PhabricatorAuthSSHKeyTransaction::TYPE_NAME: + $missing = $this->validateIsEmptyTextField( + $object->getName(), + $xactions); + + if ($missing) { + $error = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Required'), + pht('SSH key name is required.'), + nonempty(last($xactions), null)); + + $error->setIsMissingFieldError(true); + $errors[] = $error; + } + break; + + case PhabricatorAuthSSHKeyTransaction::TYPE_KEY; + $missing = $this->validateIsEmptyTextField( + $object->getName(), + $xactions); + + if ($missing) { + $error = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Required'), + pht('SSH key material is required.'), + nonempty(last($xactions), null)); + + $error->setIsMissingFieldError(true); + $errors[] = $error; + } else { + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + try { + $public_key = PhabricatorAuthSSHPublicKey::newFromRawKey($new); + } catch (Exception $ex) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + $ex->getMessage(), + $xaction); + } + } + } + break; + + case PhabricatorAuthSSHKeyTransaction::TYPE_DEACTIVATE: + foreach ($xactions as $xaction) { + if (!$xaction->getNewValue()) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht('SSH keys can not be reactivated.'), + $xaction); + } + } + break; + } + + return $errors; + } + + protected function didCatchDuplicateKeyException( + PhabricatorLiskDAO $object, + array $xactions, + Exception $ex) { + + $errors = array(); + $errors[] = new PhabricatorApplicationTransactionValidationError( + PhabricatorAuthSSHKeyTransaction::TYPE_KEY, + pht('Duplicate'), + pht( + 'This public key is already associated with another user or device. '. + 'Each key must unambiguously identify a single unique owner.'), + null); + + throw new PhabricatorApplicationTransactionValidationException($errors); + } + + + protected function shouldSendMail( + PhabricatorLiskDAO $object, + array $xactions) { + return true; + } + + protected function getMailSubjectPrefix() { + return pht('[SSH Key]'); + } + + protected function getMailThreadID(PhabricatorLiskDAO $object) { + return 'ssh-key-'.$object->getPHID(); + } + + protected function getMailTo(PhabricatorLiskDAO $object) { + return $object->getObject()->getSSHKeyNotifyPHIDs(); + } + + protected function getMailCC(PhabricatorLiskDAO $object) { + return array(); + } + + protected function buildReplyHandler(PhabricatorLiskDAO $object) { + return id(new PhabricatorAuthSSHKeyReplyHandler()) + ->setMailReceiver($object); + } + + protected function buildMailTemplate(PhabricatorLiskDAO $object) { + $id = $object->getID(); + $name = $object->getName(); + $phid = $object->getPHID(); + + $mail = id(new PhabricatorMetaMTAMail()) + ->setSubject(pht('SSH Key %d: %s', $id, $name)) + ->addHeader('Thread-Topic', $phid); + + // The primary value of this mail is alerting users to account compromises, + // so force delivery. In particular, this mail should still be delievered + // even if "self mail" is disabled. + $mail->setForceDelivery(true); + + return $mail; + } + + protected function buildMailBody( + PhabricatorLiskDAO $object, + array $xactions) { + + $body = parent::buildMailBody($object, $xactions); + + $body->addTextSection( + pht('SECURITY WARNING'), + pht( + 'If you do not recognize this change, it may indicate your account '. + 'has been compromised.')); + + $detail_uri = $object->getURI(); + $detail_uri = PhabricatorEnv::getProductionURI($detail_uri); + + $body->addLinkSection(pht('SSH KEY DETAIL'), $detail_uri); + + return $body; + } + +} diff --git a/src/applications/auth/mail/PhabricatorAuthSSHKeyReplyHandler.php b/src/applications/auth/mail/PhabricatorAuthSSHKeyReplyHandler.php new file mode 100644 index 0000000000..e84d7a4efa --- /dev/null +++ b/src/applications/auth/mail/PhabricatorAuthSSHKeyReplyHandler.php @@ -0,0 +1,17 @@ + $handle) { $key = $objects[$phid]; $handle->setName(pht('SSH Key %d', $key->getID())); + + if (!$key->getIsActive()) { + $handle->setStatus(PhabricatorObjectHandle::STATUS_CLOSED); + } } } diff --git a/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php b/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php index f68969d0e8..4592d794fa 100644 --- a/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php +++ b/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php @@ -7,6 +7,7 @@ final class PhabricatorAuthSSHKeyQuery private $phids; private $objectPHIDs; private $keys; + private $isActive; public function withIDs(array $ids) { $this->ids = $ids; @@ -29,6 +30,11 @@ final class PhabricatorAuthSSHKeyQuery return $this; } + public function withIsActive($active) { + $this->isActive = $active; + return $this; + } + public function newResultObject() { return new PhabricatorAuthSSHKey(); } @@ -100,6 +106,19 @@ final class PhabricatorAuthSSHKeyQuery $where[] = implode(' OR ', $sql); } + if ($this->isActive !== null) { + if ($this->isActive) { + $where[] = qsprintf( + $conn, + 'isActive = %d', + 1); + } else { + $where[] = qsprintf( + $conn, + 'isActive IS NULL'); + } + } + return $where; } diff --git a/src/applications/auth/query/PhabricatorAuthSSHKeySearchEngine.php b/src/applications/auth/query/PhabricatorAuthSSHKeySearchEngine.php new file mode 100644 index 0000000000..0575b40b9c --- /dev/null +++ b/src/applications/auth/query/PhabricatorAuthSSHKeySearchEngine.php @@ -0,0 +1,105 @@ +sshKeyObject = $object; + return $this; + } + + public function getSSHKeyObject() { + return $this->sshKeyObject; + } + + public function canUseInPanelContext() { + return false; + } + + public function getResultTypeDescription() { + return pht('SSH Keys'); + } + + public function getApplicationClassName() { + return 'PhabricatorAuthApplication'; + } + + public function newQuery() { + $object = $this->getSSHKeyObject(); + $object_phid = $object->getPHID(); + + return id(new PhabricatorAuthSSHKeyQuery()) + ->withObjectPHIDs(array($object_phid)); + } + + protected function buildQueryFromParameters(array $map) { + $query = $this->newQuery(); + + return $query; + } + + + protected function buildCustomSearchFields() { + return array(); + } + + protected function getURI($path) { + $object = $this->getSSHKeyObject(); + $object_phid = $object->getPHID(); + + return "/auth/sshkey/for/{$object_phid}/{$path}"; + } + + protected function getBuiltinQueryNames() { + $names = array( + 'all' => pht('All Keys'), + ); + + return $names; + } + + public function buildSavedQueryFromBuiltin($query_key) { + + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + switch ($query_key) { + case 'all': + return $query; + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + + protected function renderResultList( + array $keys, + PhabricatorSavedQuery $query, + array $handles) { + assert_instances_of($keys, 'PhabricatorAuthSSHKey'); + + $viewer = $this->requireViewer(); + + $list = new PHUIObjectItemListView(); + $list->setUser($viewer); + foreach ($keys as $key) { + $item = id(new PHUIObjectItemView()) + ->setObjectName(pht('SSH Key %d', $key->getID())) + ->setHeader($key->getName()) + ->setHref($key->getURI()); + + if (!$key->getIsActive()) { + $item->setDisabled(true); + } + + $list->addItem($item); + } + + $result = new PhabricatorApplicationSearchResultView(); + $result->setObjectList($list); + $result->setNoDataString(pht('No matching SSH keys.')); + + return $result; + } +} diff --git a/src/applications/auth/query/PhabricatorAuthSSHKeyTransactionQuery.php b/src/applications/auth/query/PhabricatorAuthSSHKeyTransactionQuery.php new file mode 100644 index 0000000000..397a03f2b0 --- /dev/null +++ b/src/applications/auth/query/PhabricatorAuthSSHKeyTransactionQuery.php @@ -0,0 +1,10 @@ +getPHID(); + + return id(new self()) + ->setIsActive(1) + ->setObjectPHID($object_phid) + ->attachObject($object); + } + protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -26,13 +46,19 @@ final class PhabricatorAuthSSHKey 'keyBody' => 'text', 'keyComment' => 'text255', 'isTrusted' => 'bool', + 'isActive' => 'bool?', ), self::CONFIG_KEY_SCHEMA => array( 'key_object' => array( 'columns' => array('objectPHID'), ), - 'key_unique' => array( - 'columns' => array('keyIndex'), + 'key_active' => array( + 'columns' => array('isActive', 'objectPHID'), + ), + // NOTE: This unique key includes a nullable column, effectively + // constraining uniqueness on active keys only. + 'key_activeunique' => array( + 'columns' => array('keyIndex', 'isActive'), 'unique' => true, ), ), @@ -44,6 +70,12 @@ final class PhabricatorAuthSSHKey return parent::save(); } + public function getMailKey() { + // NOTE: We don't actually receive mail for these objects. It's OK for + // the mail key to be predictable until we do. + return PhabricatorHash::digestForIndex($this->getPHID()); + } + public function toPublicKey() { return PhabricatorAuthSSHPublicKey::newFromStoredKey($this); } @@ -71,6 +103,11 @@ final class PhabricatorAuthSSHKey PhabricatorAuthSSHKeyPHIDType::TYPECONST); } + public function getURI() { + $id = $this->getID(); + return "/auth/sshkey/view/{$id}/"; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -82,14 +119,29 @@ final class PhabricatorAuthSSHKey } public function getPolicy($capability) { + if (!$this->getIsActive()) { + if ($capability == PhabricatorPolicyCapability::CAN_EDIT) { + return PhabricatorPolicies::POLICY_NOONE; + } + } + return $this->getObject()->getPolicy($capability); } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + if (!$this->getIsActive()) { + return false; + } + return $this->getObject()->hasAutomaticCapability($capability, $viewer); } public function describeAutomaticCapability($capability) { + if (!$this->getIsACtive()) { + return pht( + 'Deactivated SSH keys can not be edited or reactivated.'); + } + return pht( 'SSH keys inherit the policies of the user or object they authenticate.'); } @@ -105,4 +157,26 @@ final class PhabricatorAuthSSHKey $this->saveTransaction(); } + +/* -( PhabricatorApplicationTransactionInterface )------------------------- */ + + + public function getApplicationTransactionEditor() { + return new PhabricatorAuthSSHKeyEditor(); + } + + public function getApplicationTransactionObject() { + return $this; + } + + public function getApplicationTransactionTemplate() { + return new PhabricatorAuthSSHKeyTransaction(); + } + + public function willRenderTimeline( + PhabricatorApplicationTransactionView $timeline, + AphrontRequest $request) { + return $timeline; + } + } diff --git a/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php b/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php new file mode 100644 index 0000000000..37edb7384d --- /dev/null +++ b/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php @@ -0,0 +1,59 @@ +getAuthorPHID(); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_CREATE: + return pht( + '%s created this key.', + $this->renderHandleLink($author_phid)); + case self::TYPE_NAME: + return pht( + '%s renamed this key from "%s" to "%s".', + $this->renderHandleLink($author_phid), + $old, + $new); + case self::TYPE_KEY: + return pht( + '%s updated the public key material for this SSH key.', + $this->renderHandleLink($author_phid)); + case self::TYPE_DEACTIVATE: + if ($new) { + return pht( + '%s deactivated this key.', + $this->renderHandleLink($author_phid)); + } else { + return pht( + '%s activated this key.', + $this->renderHandleLink($author_phid)); + } + + } + + return parent::getTitle(); + } + +} diff --git a/src/applications/auth/view/PhabricatorAuthSSHKeyTableView.php b/src/applications/auth/view/PhabricatorAuthSSHKeyTableView.php index e19a35fce6..0452a14521 100644 --- a/src/applications/auth/view/PhabricatorAuthSSHKeyTableView.php +++ b/src/applications/auth/view/PhabricatorAuthSSHKeyTableView.php @@ -8,6 +8,58 @@ final class PhabricatorAuthSSHKeyTableView extends AphrontView { private $showTrusted; private $showID; + public static function newKeyActionsMenu( + PhabricatorUser $viewer, + PhabricatorSSHPublicKeyInterface $object) { + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $object, + PhabricatorPolicyCapability::CAN_EDIT); + + try { + PhabricatorSSHKeyGenerator::assertCanGenerateKeypair(); + $can_generate = true; + } catch (Exception $ex) { + $can_generate = false; + } + + $object_phid = $object->getPHID(); + + $generate_uri = "/auth/sshkey/generate/?objectPHID={$object_phid}"; + $upload_uri = "/auth/sshkey/upload/?objectPHID={$object_phid}"; + $view_uri = "/auth/sshkey/for/{$object_phid}/"; + + $action_view = id(new PhabricatorActionListView()) + ->setUser($viewer) + ->addAction( + id(new PhabricatorActionView()) + ->setHref($upload_uri) + ->setWorkflow(true) + ->setDisabled(!$can_edit) + ->setName(pht('Upload Public Key')) + ->setIcon('fa-upload')) + ->addAction( + id(new PhabricatorActionView()) + ->setHref($generate_uri) + ->setWorkflow(true) + ->setDisabled(!$can_edit || !$can_generate) + ->setName(pht('Generate Keypair')) + ->setIcon('fa-lock')) + ->addAction( + id(new PhabricatorActionView()) + ->setHref($view_uri) + ->setName(pht('View History')) + ->setIcon('fa-list-ul')); + + return id(new PHUIButtonView()) + ->setTag('a') + ->setText(pht('SSH Key Actions')) + ->setHref('#') + ->setIcon('fa-gear') + ->setDropdownMenu($action_view); + } + public function setNoDataString($no_data_string) { $this->noDataString = $no_data_string; return $this; @@ -38,12 +90,6 @@ final class PhabricatorAuthSSHKeyTableView extends AphrontView { $keys = $this->keys; $viewer = $this->getUser(); - if ($this->canEdit) { - $delete_class = 'small grey button'; - } else { - $delete_class = 'small grey button disabled'; - } - $trusted_icon = id(new PHUIIconView()) ->setIcon('fa-star blue'); $untrusted_icon = id(new PHUIIconView()) @@ -56,22 +102,13 @@ final class PhabricatorAuthSSHKeyTableView extends AphrontView { javelin_tag( 'a', array( - 'href' => '/auth/sshkey/edit/'.$key->getID().'/', - 'sigil' => 'workflow', + 'href' => $key->getURI(), ), $key->getName()), $key->getIsTrusted() ? $trusted_icon : $untrusted_icon, $key->getKeyComment(), $key->getKeyType(), phabricator_datetime($key->getDateCreated(), $viewer), - javelin_tag( - 'a', - array( - 'href' => '/auth/sshkey/delete/'.$key->getID().'/', - 'class' => $delete_class, - 'sigil' => 'workflow', - ), - pht('Delete')), ); } @@ -85,7 +122,6 @@ final class PhabricatorAuthSSHKeyTableView extends AphrontView { pht('Comment'), pht('Type'), pht('Added'), - null, )) ->setColumnVisibility( array( @@ -101,7 +137,6 @@ final class PhabricatorAuthSSHKeyTableView extends AphrontView { '', '', 'right', - 'action', )); return $table; diff --git a/src/applications/calendar/storage/PhabricatorCalendarHoliday.php b/src/applications/calendar/storage/PhabricatorCalendarHoliday.php index d504a71ce0..be6dde58f5 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarHoliday.php +++ b/src/applications/calendar/storage/PhabricatorCalendarHoliday.php @@ -21,22 +21,4 @@ final class PhabricatorCalendarHoliday extends PhabricatorCalendarDAO { ) + parent::getConfiguration(); } - public static function getNthBusinessDay($epoch, $n) { - // Sadly, there are not many holidays. So we can load all of them. - $holidays = id(new PhabricatorCalendarHoliday())->loadAll(); - $holidays = mpull($holidays, null, 'getDay'); - $interval = ($n > 0 ? 1 : -1) * 24 * 60 * 60; - - $return = $epoch; - for ($i = abs($n); $i > 0; ) { - $return += $interval; - $weekday = date('w', $return); - if ($weekday != 0 && $weekday != 6 && // Sunday and Saturday - !isset($holidays[date('Y-m-d', $return)])) { - $i--; - } - } - return $return; - } - } diff --git a/src/applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php b/src/applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php index 3e92a19583..de9a4cddd8 100644 --- a/src/applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php +++ b/src/applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php @@ -16,24 +16,4 @@ final class PhabricatorCalendarHolidayTestCase extends PhabricatorTestCase { ->save(); } - public function testNthBusinessDay() { - $map = array( - array('2011-12-30', 1, '2012-01-03'), - array('2012-01-01', 1, '2012-01-03'), - array('2012-01-01', 0, '2012-01-01'), - array('2012-01-01', -1, '2011-12-30'), - array('2012-01-04', -1, '2012-01-03'), - ); - foreach ($map as $val) { - list($date, $n, $expect) = $val; - $actual = PhabricatorCalendarHoliday::getNthBusinessDay( - strtotime($date), - $n); - $this->assertEqual( - $expect, - date('Y-m-d', $actual), - pht("%d business days since '%s'", $n, $date)); - } - } - } diff --git a/src/applications/conduit/controller/PhabricatorConduitAPIController.php b/src/applications/conduit/controller/PhabricatorConduitAPIController.php index c4f3e65c35..690f6cc1da 100644 --- a/src/applications/conduit/controller/PhabricatorConduitAPIController.php +++ b/src/applications/conduit/controller/PhabricatorConduitAPIController.php @@ -204,6 +204,7 @@ final class PhabricatorConduitAPIController $stored_key = id(new PhabricatorAuthSSHKeyQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withKeys(array($public_key)) + ->withIsActive(true) ->executeOne(); if (!$stored_key) { return array( diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index 602d756d05..99d3f9962b 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -186,6 +186,10 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'Configuration of the notification server has changed substantially. '. 'For discussion, see T10794.'); + $stale_reason = pht( + 'The Differential revision list view age UI elements have been removed '. + 'to simplify the interface.'); + $ancient_config += array( 'phid.external-loaders' => pht( @@ -314,6 +318,9 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'metamta.differential.unified-comment-context' => pht( 'Inline comments are now always rendered with a limited amount '. 'of context.'), + + 'differential.days-fresh' => $stale_reason, + 'differential.days-stale' => $stale_reason, ); return $ancient_config; diff --git a/src/applications/differential/application/PhabricatorDifferentialApplication.php b/src/applications/differential/application/PhabricatorDifferentialApplication.php index 787bf52788..ed0053061b 100644 --- a/src/applications/differential/application/PhabricatorDifferentialApplication.php +++ b/src/applications/differential/application/PhabricatorDifferentialApplication.php @@ -102,68 +102,78 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication { ); } + public static function loadNeedAttentionRevisions(PhabricatorUser $viewer) { + if (!$viewer->isLoggedIn()) { + return array(); + } + + $viewer_phid = $viewer->getPHID(); + + $responsible_phids = id(new DifferentialResponsibleDatasource()) + ->setViewer($viewer) + ->evaluateTokens(array($viewer_phid)); + + $revision_query = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) + ->withResponsibleUsers($responsible_phids) + ->needReviewerStatus(true) + ->needRelationships(true) + ->needFlags(true) + ->needDrafts(true) + ->setLimit(self::MAX_STATUS_ITEMS); + + $revisions = $revision_query->execute(); + + $query = id(new PhabricatorSavedQuery()) + ->attachParameterMap( + array( + 'responsiblePHIDs' => $responsible_phids, + )); + + $groups = id(new DifferentialRevisionRequiredActionResultBucket()) + ->setViewer($viewer) + ->newResultGroups($query, $revisions); + + $include = array(); + foreach ($groups as $group) { + switch ($group->getKey()) { + case DifferentialRevisionRequiredActionResultBucket::KEY_MUSTREVIEW: + case DifferentialRevisionRequiredActionResultBucket::KEY_SHOULDREVIEW: + foreach ($group->getObjects() as $object) { + $include[] = $object; + } + break; + default: + break; + } + } + + return $include; + } + public function loadStatus(PhabricatorUser $user) { + $revisions = self::loadNeedAttentionRevisions($user); $limit = self::MAX_STATUS_ITEMS; - $revisions = id(new DifferentialRevisionQuery()) - ->setViewer($user) - ->withResponsibleUsers(array($user->getPHID())) - ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) - ->needRelationships(true) - ->setLimit($limit) - ->execute(); + if (count($revisions) >= $limit) { + $display_count = ($limit - 1); + $display_label = pht( + '%s+ Active Review(s)', + new PhutilNumber($display_count)); + } else { + $display_count = count($revisions); + $display_label = pht( + '%s Review(s) Need Attention', + new PhutilNumber($display_count)); + } $status = array(); - if (count($revisions) >= $limit) { - $all_count = count($revisions); - $all_count_str = pht( - '%s+ Active Review(s)', - new PhutilNumber($limit - 1)); - $type = PhabricatorApplicationStatusView::TYPE_WARNING; - $status[] = id(new PhabricatorApplicationStatusView()) - ->setType($type) - ->setText($all_count_str) - ->setCount($all_count); - } else { - list($blocking, $active, $waiting) = - DifferentialRevisionQuery::splitResponsible( - $revisions, - array($user->getPHID())); - - $blocking = count($blocking); - $blocking_str = pht( - '%s Review(s) Blocking Others', - new PhutilNumber($blocking)); - - $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; - $status[] = id(new PhabricatorApplicationStatusView()) - ->setType($type) - ->setText($blocking_str) - ->setCount($blocking); - - $active = count($active); - $active_str = pht( - '%s Review(s) Need Attention', - new PhutilNumber($active)); - - $type = PhabricatorApplicationStatusView::TYPE_WARNING; - $status[] = id(new PhabricatorApplicationStatusView()) - ->setType($type) - ->setText($active_str) - ->setCount($active); - - $waiting = count($waiting); - $waiting_str = pht( - '%s Review(s) Waiting on Others', - new PhutilNumber($waiting)); - - $type = PhabricatorApplicationStatusView::TYPE_INFO; - $status[] = id(new PhabricatorApplicationStatusView()) - ->setType($type) - ->setText($waiting_str) - ->setCount($waiting); - } + $status[] = id(new PhabricatorApplicationStatusView()) + ->setType(PhabricatorApplicationStatusView::TYPE_WARNING) + ->setText($display_label) + ->setCount($display_count); return $status; } diff --git a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php index 70f9fe77ed..110960aac5 100644 --- a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php +++ b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php @@ -229,25 +229,6 @@ final class PhabricatorDifferentialConfigOptions "\n\n". 'This sort of workflow is very unusual. Very few installs should '. 'need to change this option.')), - $this->newOption('differential.days-fresh', 'int', 1) - ->setSummary( - pht( - "For how many business days should a revision be considered ". - "'fresh'?")) - ->setDescription( - pht( - 'Revisions newer than this number of days are marked as fresh in '. - 'Action Required and Revisions Waiting on You views. Only work '. - 'days (not weekends and holidays) are included. Set to 0 to '. - 'disable this feature.')), - $this->newOption('differential.days-stale', 'int', 3) - ->setSummary( - pht("After this many days, a revision will be considered 'stale'.")) - ->setDescription( - pht( - "Similar to `%s` but marks stale revisions. ". - "If the revision is even older than it is when marked as 'old'.", - 'differential.days-fresh')), $this->newOption( 'metamta.differential.subject-prefix', 'string', diff --git a/src/applications/differential/customfield/DifferentialCustomField.php b/src/applications/differential/customfield/DifferentialCustomField.php index b4ef47c274..d0980736c4 100644 --- a/src/applications/differential/customfield/DifferentialCustomField.php +++ b/src/applications/differential/customfield/DifferentialCustomField.php @@ -35,27 +35,39 @@ abstract class DifferentialCustomField protected function parseObjectList( $value, array $types, - $allow_partial = false) { + $allow_partial = false, + array $suffixes = array()) { return id(new PhabricatorObjectListQuery()) ->setViewer($this->getViewer()) ->setAllowedTypes($types) ->setObjectList($value) ->setAllowPartialResults($allow_partial) + ->setSuffixes($suffixes) ->execute(); } - protected function renderObjectList(array $handles) { + protected function renderObjectList( + array $handles, + array $suffixes = array()) { + if (!$handles) { return null; } $out = array(); foreach ($handles as $handle) { + $phid = $handle->getPHID(); + if ($handle->getPolicyFiltered()) { - $out[] = $handle->getPHID(); + $token = $phid; } else if ($handle->isComplete()) { - $out[] = $handle->getObjectName(); + $token = $handle->getCommandLineObjectName(); } + + $suffix = idx($suffixes, $phid); + $token = $token.$suffix; + + $out[] = $token; } return implode(', ', $out); diff --git a/src/applications/differential/customfield/DifferentialProjectReviewersField.php b/src/applications/differential/customfield/DifferentialProjectReviewersField.php index 1eea4f51b9..c900f72659 100644 --- a/src/applications/differential/customfield/DifferentialProjectReviewersField.php +++ b/src/applications/differential/customfield/DifferentialProjectReviewersField.php @@ -8,7 +8,7 @@ final class DifferentialProjectReviewersField } public function getFieldName() { - return pht('Project Reviewers'); + return pht('Group Reviewers'); } public function getFieldDescription() { diff --git a/src/applications/differential/customfield/DifferentialReviewersField.php b/src/applications/differential/customfield/DifferentialReviewersField.php index 9ab77c26ac..d23dac53d9 100644 --- a/src/applications/differential/customfield/DifferentialReviewersField.php +++ b/src/applications/differential/customfield/DifferentialReviewersField.php @@ -36,43 +36,82 @@ final class DifferentialReviewersField } public function readValueFromRequest(AphrontRequest $request) { - // Compute a new set of reviewer objects. For reviewers who haven't been - // added or removed, retain their existing status. Also, respect the new - // order. - - $old_status = $this->getValue(); - $old_status = mpull($old_status, null, 'getReviewerPHID'); + $datasource = id(new DifferentialBlockingReviewerDatasource()) + ->setViewer($request->getViewer()); $new_phids = $request->getArr($this->getFieldKey()); - $new_phids = array_fuse($new_phids); + $new_phids = $datasource->evaluateTokens($new_phids); - $new_status = array(); - foreach ($new_phids as $new_phid) { - if (empty($old_status[$new_phid])) { - $new_status[$new_phid] = new DifferentialReviewer( - $new_phid, - array( - 'status' => DifferentialReviewerStatus::STATUS_ADDED, - )); + $reviewers = array(); + foreach ($new_phids as $spec) { + if (!is_array($spec)) { + $reviewers[$spec] = DifferentialReviewerStatus::STATUS_ADDED; } else { - $new_status[$new_phid] = $old_status[$new_phid]; + $reviewers[$spec['phid']] = $spec['type']; } } - $this->setValue($new_status); + $this->updateReviewers($this->getValue(), $reviewers); + } + + private function updateReviewers(array $old_reviewers, array $new_map) { + // Compute a new set of reviewer objects. We're going to respect the new + // reviewer order, add or remove any new or missing reviewers, and respect + // any blocking or unblocking changes. For reviewers who were there before + // and are still there, we're going to keep the old value because it + // may be something like "Accept", "Reject", etc. + + $old_map = mpull($old_reviewers, 'getStatus', 'getReviewerPHID'); + $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING; + + $new_reviewers = array(); + foreach ($new_map as $phid => $new) { + $old = idx($old_map, $phid); + + // If we have an old status and this didn't make the reviewer blocking + // or nonblocking, just retain the old status. This makes sure we don't + // throw away rejects, accepts, etc. + if ($old) { + $is_block = ($old !== $status_blocking && $new === $status_blocking); + $is_unblock = ($old === $status_blocking && $new !== $status_blocking); + if (!$is_block && !$is_unblock) { + $new_reviewers[$phid] = $old; + continue; + } + } + + $new_reviewers[$phid] = $new; + } + + foreach ($new_reviewers as $phid => $status) { + $new_reviewers[$phid] = new DifferentialReviewer( + $phid, + array( + 'status' => $status, + )); + } + + $this->setValue($new_reviewers); } public function renderEditControl(array $handles) { - $phids = array(); - if ($this->getValue()) { - $phids = mpull($this->getValue(), 'getReviewerPHID'); + $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING; + + $value = array(); + foreach ($this->getValue() as $reviewer) { + $phid = $reviewer->getReviewerPHID(); + if ($reviewer->getStatus() == $status_blocking) { + $value[] = 'blocking('.$phid.')'; + } else { + $value[] = $phid; + } } return id(new AphrontFormTokenizerControl()) ->setUser($this->getViewer()) ->setName($this->getFieldKey()) - ->setDatasource(new PhabricatorProjectOrUserDatasource()) - ->setValue($phids) + ->setDatasource(new DifferentialReviewerDatasource()) + ->setValue($value) ->setError($this->getFieldError()) ->setLabel($this->getFieldName()); } @@ -141,12 +180,17 @@ final class DifferentialReviewersField } public function parseValueFromCommitMessage($value) { - return $this->parseObjectList( + $results = $this->parseObjectList( $value, array( PhabricatorPeopleUserPHIDType::TYPECONST, PhabricatorProjectProjectPHIDType::TYPECONST, - )); + PhabricatorOwnersPackagePHIDType::TYPECONST, + ), + false, + array('!')); + + return $this->flattenReviewers($results); } public function getRequiredHandlePHIDsForCommitMessage() { @@ -154,29 +198,42 @@ final class DifferentialReviewersField } public function readValueFromCommitMessage($value) { - $current_reviewers = $this->getObject()->getReviewerStatus(); - $current_reviewers = mpull($current_reviewers, null, 'getReviewerPHID'); + $value = $this->inflateReviewers($value); $reviewers = array(); - foreach ($value as $phid) { - $reviewer = idx($current_reviewers, $phid); - if ($reviewer) { - $reviewers[] = $reviewer; + foreach ($value as $spec) { + $phid = $spec['phid']; + + $is_blocking = isset($spec['suffixes']['!']); + if ($is_blocking) { + $status = DifferentialReviewerStatus::STATUS_BLOCKING; } else { - $data = array( - 'status' => DifferentialReviewerStatus::STATUS_ADDED, - ); - $reviewers[] = new DifferentialReviewer($phid, $data); + $status = DifferentialReviewerStatus::STATUS_ADDED; } + + $reviewers[$phid] = $status; } - $this->setValue($reviewers); + $this->updateReviewers( + $this->getObject()->getReviewerStatus(), + $reviewers); return $this; } public function renderCommitMessageValue(array $handles) { - return $this->renderObjectList($handles); + $suffixes = array(); + + $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING; + + foreach ($this->getValue() as $reviewer) { + if ($reviewer->getStatus() == $status_blocking) { + $phid = $reviewer->getReviewerPHID(); + $suffixes[$phid] = '!'; + } + } + + return $this->renderObjectList($handles, $suffixes); } public function validateCommitMessageValue($value) { @@ -185,7 +242,9 @@ final class DifferentialReviewersField $config_self_accept_key = 'differential.allow-self-accept'; $allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key); - foreach ($value as $phid) { + foreach ($value as $spec) { + $phid = $spec['phid']; + if (($phid == $author_phid) && !$allow_self_accept) { throw new DifferentialFieldValidationException( pht('The author of a revision can not be a reviewer.')); @@ -224,4 +283,44 @@ final class DifferentialReviewersField return $warnings; } + public function getProTips() { + return array( + pht( + 'You can mark a reviewer as blocking by adding an exclamation '. + 'mark ("!") after their name.'), + ); + } + + private function flattenReviewers(array $values) { + // NOTE: For now, `arc` relies on this field returning only scalars, so we + // need to reduce the results into scalars. See T10981. + $result = array(); + + foreach ($values as $value) { + $result[] = $value['phid'].implode('', array_keys($value['suffixes'])); + } + + return $result; + } + + private function inflateReviewers(array $values) { + $result = array(); + + foreach ($values as $value) { + if (substr($value, -1) == '!') { + $value = substr($value, 0, -1); + $suffixes = array('!' => '!'); + } else { + $suffixes = array(); + } + + $result[] = array( + 'phid' => $value, + 'suffixes' => $suffixes, + ); + } + + return $result; + } + } diff --git a/src/applications/differential/customfield/DifferentialSubscribersField.php b/src/applications/differential/customfield/DifferentialSubscribersField.php index a1e8d1dbd6..b8423e9879 100644 --- a/src/applications/differential/customfield/DifferentialSubscribersField.php +++ b/src/applications/differential/customfield/DifferentialSubscribersField.php @@ -78,6 +78,7 @@ final class DifferentialSubscribersField array( PhabricatorPeopleUserPHIDType::TYPECONST, PhabricatorProjectProjectPHIDType::TYPECONST, + PhabricatorOwnersPackagePHIDType::TYPECONST, )); } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 3855edeac9..2d2671227d 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -7,6 +7,7 @@ final class DifferentialTransactionEditor private $isCloseByCommit; private $repositoryPHIDOverride = false; private $didExpandInlineState = false; + private $affectedPaths; public function getEditorApplicationClass() { return 'PhabricatorDifferentialApplication'; @@ -1200,7 +1201,13 @@ final class DifferentialTransactionEditor $body = new PhabricatorMetaMTAMailBody(); $body->setViewer($this->requireActor()); - $this->addHeadersAndCommentsToMailBody($body, $xactions); + $revision_uri = PhabricatorEnv::getProductionURI('/D'.$object->getID()); + + $this->addHeadersAndCommentsToMailBody( + $body, + $xactions, + pht('View Revision'), + $revision_uri); $type_inline = DifferentialTransaction::TYPE_INLINE; @@ -1226,7 +1233,7 @@ final class DifferentialTransactionEditor $body->addLinkSection( pht('REVISION DETAIL'), - PhabricatorEnv::getProductionURI('/D'.$object->getID())); + $revision_uri); $update_xaction = null; foreach ($xactions as $xaction) { @@ -1481,6 +1488,181 @@ final class DifferentialTransactionEditor return parent::shouldApplyHeraldRules($object, $xactions); } + protected function didApplyHeraldRules( + PhabricatorLiskDAO $object, + HeraldAdapter $adapter, + HeraldTranscript $transcript) { + + $repository = $object->getRepository(); + if (!$repository) { + return array(); + } + + if (!$this->affectedPaths) { + return array(); + } + + $packages = PhabricatorOwnersPackage::loadAffectedPackages( + $repository, + $this->affectedPaths); + + foreach ($packages as $key => $package) { + if ($package->isArchived()) { + unset($packages[$key]); + } + } + + if (!$packages) { + return array(); + } + + // Remove packages that the revision author is an owner of. If you own + // code, you don't need another owner to review it. + $authority = id(new PhabricatorOwnersPackageQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs(mpull($packages, 'getPHID')) + ->withAuthorityPHIDs(array($object->getAuthorPHID())) + ->execute(); + $authority = mpull($authority, null, 'getPHID'); + + foreach ($packages as $key => $package) { + $package_phid = $package->getPHID(); + if ($authority[$package_phid]) { + unset($packages[$key]); + continue; + } + } + + if (!$packages) { + return array(); + } + + $auto_subscribe = array(); + $auto_review = array(); + $auto_block = array(); + + foreach ($packages as $package) { + switch ($package->getAutoReview()) { + case PhabricatorOwnersPackage::AUTOREVIEW_SUBSCRIBE: + $auto_subscribe[] = $package; + break; + case PhabricatorOwnersPackage::AUTOREVIEW_REVIEW: + $auto_review[] = $package; + break; + case PhabricatorOwnersPackage::AUTOREVIEW_BLOCK: + $auto_block[] = $package; + break; + case PhabricatorOwnersPackage::AUTOREVIEW_NONE: + default: + break; + } + } + + $owners_phid = id(new PhabricatorOwnersApplication()) + ->getPHID(); + + $xactions = array(); + if ($auto_subscribe) { + $xactions[] = $object->getApplicationTransactionTemplate() + ->setAuthorPHID($owners_phid) + ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) + ->setNewValue( + array( + '+' => mpull($auto_subscribe, 'getPHID'), + )); + } + + $specs = array( + array($auto_review, false), + array($auto_block, true), + ); + + foreach ($specs as $spec) { + list($reviewers, $blocking) = $spec; + if (!$reviewers) { + continue; + } + + $phids = mpull($reviewers, 'getPHID'); + $xaction = $this->newAutoReviewTransaction($object, $phids, $blocking); + if ($xaction) { + $xactions[] = $xaction; + } + } + + return $xactions; + } + + private function newAutoReviewTransaction( + PhabricatorLiskDAO $object, + array $phids, + $is_blocking) { + + // TODO: This is substantially similar to DifferentialReviewersHeraldAction + // and both are needlessly complex. This logic should live in the normal + // transaction application pipeline. See T10967. + + $reviewers = $object->getReviewerStatus(); + $reviewers = mpull($reviewers, null, 'getReviewerPHID'); + + if ($is_blocking) { + $new_status = DifferentialReviewerStatus::STATUS_BLOCKING; + } else { + $new_status = DifferentialReviewerStatus::STATUS_ADDED; + } + + $new_strength = DifferentialReviewerStatus::getStatusStrength( + $new_status); + + $current = array(); + foreach ($phids as $phid) { + if (!isset($reviewers[$phid])) { + continue; + } + + // If we're applying a stronger status (usually, upgrading a reviewer + // into a blocking reviewer), skip this check so we apply the change. + $old_strength = DifferentialReviewerStatus::getStatusStrength( + $reviewers[$phid]->getStatus()); + if ($old_strength <= $new_strength) { + continue; + } + + $current[] = $phid; + } + + $phids = array_diff($phids, $current); + + if (!$phids) { + return null; + } + + $phids = array_fuse($phids); + + $value = array(); + foreach ($phids as $phid) { + $value[$phid] = array( + 'data' => array( + 'status' => $new_status, + ), + ); + } + + $edgetype_reviewer = DifferentialRevisionHasReviewerEdgeType::EDGECONST; + + $owners_phid = id(new PhabricatorOwnersApplication()) + ->getPHID(); + + return $object->getApplicationTransactionTemplate() + ->setAuthorPHID($owners_phid) + ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) + ->setMetadataValue('edge:type', $edgetype_reviewer) + ->setNewValue( + array( + '+' => $value, + )); + } + protected function buildHeraldAdapter( PhabricatorLiskDAO $object, array $xactions) { @@ -1547,6 +1729,10 @@ final class DifferentialTransactionEditor $paths[] = $path_prefix.'/'.$changeset->getFilename(); } + // Save the affected paths; we'll use them later to query Owners. This + // uses the un-expanded paths. + $this->affectedPaths = $paths; + // Mark this as also touching all parent paths, so you can see all pending // changes to any file within a directory. $all_paths = array(); diff --git a/src/applications/differential/herald/DifferentialReviewersAddBlockingReviewersHeraldAction.php b/src/applications/differential/herald/DifferentialReviewersAddBlockingReviewersHeraldAction.php index 79a00bc940..16d07e5240 100644 --- a/src/applications/differential/herald/DifferentialReviewersAddBlockingReviewersHeraldAction.php +++ b/src/applications/differential/herald/DifferentialReviewersAddBlockingReviewersHeraldAction.php @@ -22,7 +22,7 @@ final class DifferentialReviewersAddBlockingReviewersHeraldAction } protected function getDatasource() { - return new PhabricatorMetaMTAMailableDatasource(); + return new DiffusionAuditorDatasource(); } public function renderActionDescription($value) { diff --git a/src/applications/differential/herald/DifferentialReviewersAddReviewersHeraldAction.php b/src/applications/differential/herald/DifferentialReviewersAddReviewersHeraldAction.php index 45f209385e..d39a4370c5 100644 --- a/src/applications/differential/herald/DifferentialReviewersAddReviewersHeraldAction.php +++ b/src/applications/differential/herald/DifferentialReviewersAddReviewersHeraldAction.php @@ -22,7 +22,7 @@ final class DifferentialReviewersAddReviewersHeraldAction } protected function getDatasource() { - return new PhabricatorMetaMTAMailableDatasource(); + return new DiffusionAuditorDatasource(); } public function renderActionDescription($value) { diff --git a/src/applications/differential/herald/DifferentialReviewersHeraldAction.php b/src/applications/differential/herald/DifferentialReviewersHeraldAction.php index 5293af4311..3e98fdd92f 100644 --- a/src/applications/differential/herald/DifferentialReviewersHeraldAction.php +++ b/src/applications/differential/herald/DifferentialReviewersHeraldAction.php @@ -69,6 +69,7 @@ abstract class DifferentialReviewersHeraldAction $allowed_types = array( PhabricatorPeopleUserPHIDType::TYPECONST, PhabricatorProjectProjectPHIDType::TYPECONST, + PhabricatorOwnersPackagePHIDType::TYPECONST, ); $targets = $this->loadStandardTargets($phids, $allowed_types, $current); diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index 30731b6fed..f36a686eb5 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -91,18 +91,6 @@ final class DifferentialRevisionQuery return $this; } - /** - * Filter results to revisions with comments authored by the given PHIDs. - * - * @param array List of PHIDs of authors - * @return this - * @task config - */ - public function withDraftRepliesByAuthors(array $author_phids) { - $this->draftAuthors = $author_phids; - return $this; - } - /** * Filter results to revisions which CC one of the listed people. Calling this * function will clear anything set by previous calls to @{method:withCCs}. @@ -239,27 +227,6 @@ final class DifferentialRevisionQuery } - /** - * Set result ordering. Provide a class constant, such as - * `DifferentialRevisionQuery::ORDER_CREATED`. - * - * @task config - */ - public function setOrder($order_constant) { - switch ($order_constant) { - case self::ORDER_CREATED: - $this->setOrderVector(array('id')); - break; - case self::ORDER_MODIFIED: - $this->setOrderVector(array('updated', 'id')); - break; - default: - throw new Exception(pht('Unknown order "%s".', $order_constant)); - } - - return $this; - } - /** * Set whether or not the query will load and attach relationships. @@ -371,6 +338,11 @@ final class DifferentialRevisionQuery /* -( Query Execution )---------------------------------------------------- */ + public function newResultObject() { + return new DifferentialRevision(); + } + + /** * Execute the query as configured, returning matching * @{class:DifferentialRevision} objects. @@ -379,11 +351,9 @@ final class DifferentialRevisionQuery * @task exec */ protected function loadPage() { - $table = new DifferentialRevision(); - $conn_r = $table->establishConnection('r'); - $data = $this->loadData(); + $table = $this->newResultObject(); return $table->loadAllFromArray($data); } @@ -519,7 +489,7 @@ final class DifferentialRevisionQuery } private function loadData() { - $table = new DifferentialRevision(); + $table = $this->newResultObject(); $conn_r = $table->establishConnection('r'); $selects = array(); @@ -531,25 +501,17 @@ final class DifferentialRevisionQuery $basic_authors = $this->authors; $basic_reviewers = $this->reviewers; - $authority_projects = id(new PhabricatorProjectQuery()) - ->setViewer($this->getViewer()) - ->withMemberPHIDs($this->responsibles) - ->execute(); - $authority_phids = mpull($authority_projects, 'getPHID'); - try { // Build the query where the responsible users are authors. $this->authors = array_merge($basic_authors, $this->responsibles); + $this->reviewers = $basic_reviewers; $selects[] = $this->buildSelectStatement($conn_r); // Build the query where the responsible users are reviewers, or // projects they are members of are reviewers. $this->authors = $basic_authors; - $this->reviewers = array_merge( - $basic_reviewers, - $this->responsibles, - $authority_phids); + $this->reviewers = array_merge($basic_reviewers, $this->responsibles); $selects[] = $this->buildSelectStatement($conn_r); // Put everything back like it was. @@ -591,7 +553,7 @@ final class DifferentialRevisionQuery $joins = $this->buildJoinsClause($conn_r); $where = $this->buildWhereClause($conn_r); - $group_by = $this->buildGroupByClause($conn_r); + $group_by = $this->buildGroupClause($conn_r); $having = $this->buildHavingClause($conn_r); $this->buildingGlobalOrder = false; @@ -835,19 +797,37 @@ final class DifferentialRevisionQuery /** * @task internal */ - private function buildGroupByClause($conn_r) { + protected function shouldGroupQueryResultRows() { + $join_triggers = array_merge( $this->pathIDs, $this->ccs, $this->reviewers); - $needs_distinct = (count($join_triggers) > 1); - - if ($needs_distinct) { - return 'GROUP BY r.id'; - } else { - return ''; + if (count($join_triggers) > 1) { + return true; } + + return parent::shouldGroupQueryResultRows(); + } + + public function getBuiltinOrders() { + $orders = parent::getBuiltinOrders() + array( + 'updated' => array( + 'vector' => array('updated', 'id'), + 'name' => pht('Date Updated (Latest First)'), + 'aliases' => array(self::ORDER_MODIFIED), + ), + 'outdated' => array( + 'vector' => array('-updated', '-id'), + 'name' => pht('Date Updated (Oldest First)'), + ), + ); + + // Alias the "newest" builtin to the historical key for it. + $orders['newest']['aliases'][] = self::ORDER_CREATED; + + return $orders; } protected function getDefaultOrderVector() { @@ -1053,50 +1033,6 @@ final class DifferentialRevisionQuery } } - - public static function splitResponsible(array $revisions, array $user_phids) { - $blocking = array(); - $active = array(); - $waiting = array(); - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; - - // Bucket revisions into $blocking (revisions where you are blocking - // others), $active (revisions you need to do something about) and $waiting - // (revisions you're waiting on someone else to do something about). - foreach ($revisions as $revision) { - $needs_review = ($revision->getStatus() == $status_review); - $filter_is_author = in_array($revision->getAuthorPHID(), $user_phids); - if (!$revision->getReviewers()) { - $needs_review = false; - $author_is_reviewer = false; - } else { - $author_is_reviewer = in_array( - $revision->getAuthorPHID(), - $revision->getReviewers()); - } - - // If exactly one of "needs review" and "the user is the author" is - // true, the user needs to act on it. Otherwise, they're waiting on - // it. - if ($needs_review ^ $filter_is_author) { - if ($needs_review) { - array_unshift($blocking, $revision); - } else { - $active[] = $revision; - } - // User is author **and** reviewer. An exotic but configurable workflow. - // User needs to act on it double. - } else if ($needs_review && $author_is_reviewer) { - array_unshift($blocking, $revision); - $active[] = $revision; - } else { - $waiting[] = $revision; - } - } - - return array($blocking, $active, $waiting); - } - private function loadReviewerAuthority( array $revisions, array $edges, @@ -1105,9 +1041,13 @@ final class DifferentialRevisionQuery $revision_map = mpull($revisions, null, 'getPHID'); $viewer_phid = $this->getViewer()->getPHID(); - // Find all the project reviewers which the user may have authority over. + // Find all the project/package reviewers which the user may have authority + // over. $project_phids = array(); + $package_phids = array(); $project_type = PhabricatorProjectProjectPHIDType::TYPECONST; + $package_type = PhabricatorOwnersPackagePHIDType::TYPECONST; + $edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST; foreach ($edges as $src => $types) { if (!$allow_self) { @@ -1121,14 +1061,20 @@ final class DifferentialRevisionQuery } $edge_data = idx($types, $edge_type, array()); foreach ($edge_data as $dst => $data) { - if (phid_get_type($dst) == $project_type) { + $phid_type = phid_get_type($dst); + if ($phid_type == $project_type) { $project_phids[] = $dst; } + if ($phid_type == $package_type) { + $package_phids[] = $dst; + } } } - // Now, figure out which of these projects the viewer is actually a - // member of. + // The viewer has authority over themselves. + $user_authority = array_fuse(array($viewer_phid)); + + // And over any projects they are a member of. $project_authority = array(); if ($project_phids) { $project_authority = id(new PhabricatorProjectQuery()) @@ -1137,12 +1083,22 @@ final class DifferentialRevisionQuery ->withMemberPHIDs(array($viewer_phid)) ->execute(); $project_authority = mpull($project_authority, 'getPHID'); + $project_authority = array_fuse($project_authority); } - // Finally, the viewer has authority over themselves. - return array( - $viewer_phid => true, - ) + array_fuse($project_authority); + // And over any packages they own. + $package_authority = array(); + if ($package_phids) { + $package_authority = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($package_phids) + ->withAuthorityPHIDs(array($viewer_phid)) + ->execute(); + $package_authority = mpull($package_authority, 'getPHID'); + $package_authority = array_fuse($package_authority); + } + + return $user_authority + $project_authority + $package_authority; } public function getQueryApplicationClass() { diff --git a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php new file mode 100644 index 0000000000..b9fec508c5 --- /dev/null +++ b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php @@ -0,0 +1,206 @@ +objects = $objects; + + $phids = $query->getEvaluatedParameter('responsiblePHIDs', array()); + if (!$phids) { + throw new Exception( + pht( + 'You can not bucket results by required action without '. + 'specifying "Responsible Users".')); + } + $phids = array_fuse($phids); + + $groups = array(); + + $groups[] = $this->newGroup() + ->setName(pht('Must Review')) + ->setKey(self::KEY_MUSTREVIEW) + ->setNoDataString(pht('No revisions are blocked on your review.')) + ->setObjects($this->filterMustReview($phids)); + + $groups[] = $this->newGroup() + ->setName(pht('Ready to Review')) + ->setKey(self::KEY_SHOULDREVIEW) + ->setNoDataString(pht('No revisions are waiting on you to review them.')) + ->setObjects($this->filterShouldReview($phids)); + + $groups[] = $this->newGroup() + ->setName(pht('Ready to Land')) + ->setNoDataString(pht('No revisions are ready to land.')) + ->setObjects($this->filterShouldLand($phids)); + + $groups[] = $this->newGroup() + ->setName(pht('Ready to Update')) + ->setNoDataString(pht('No revisions are waiting for updates.')) + ->setObjects($this->filterShouldUpdate($phids)); + + $groups[] = $this->newGroup() + ->setName(pht('Waiting on Review')) + ->setNoDataString(pht('None of your revisions are waiting on review.')) + ->setObjects($this->filterWaitingForReview($phids)); + + $groups[] = $this->newGroup() + ->setName(pht('Waiting on Authors')) + ->setNoDataString(pht('No revisions are waiting on author action.')) + ->setObjects($this->filterWaitingOnAuthors($phids)); + + // Because you can apply these buckets to queries which include revisions + // that have been closed, add an "Other" bucket if we still have stuff + // that didn't get filtered into any of the previous buckets. + if ($this->objects) { + $groups[] = $this->newGroup() + ->setName(pht('Other Revisions')) + ->setObjects($this->objects); + } + + return $groups; + } + + private function filterMustReview(array $phids) { + $blocking = array( + DifferentialReviewerStatus::STATUS_BLOCKING, + DifferentialReviewerStatus::STATUS_REJECTED, + DifferentialReviewerStatus::STATUS_REJECTED_OLDER, + ); + $blocking = array_fuse($blocking); + + $objects = $this->getRevisionsUnderReview($this->objects, $phids); + + $results = array(); + foreach ($objects as $key => $object) { + if (!$this->hasReviewersWithStatus($object, $phids, $blocking)) { + continue; + } + + $results[$key] = $object; + unset($this->objects[$key]); + } + + return $results; + } + + private function filterShouldReview(array $phids) { + $reviewing = array( + DifferentialReviewerStatus::STATUS_ADDED, + DifferentialReviewerStatus::STATUS_COMMENTED, + ); + $reviewing = array_fuse($reviewing); + + $objects = $this->getRevisionsUnderReview($this->objects, $phids); + + $results = array(); + foreach ($objects as $key => $object) { + if (!$this->hasReviewersWithStatus($object, $phids, $reviewing)) { + continue; + } + + $results[$key] = $object; + unset($this->objects[$key]); + } + + return $results; + } + + private function filterShouldLand(array $phids) { + $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; + + $objects = $this->getRevisionsAuthored($this->objects, $phids); + + $results = array(); + foreach ($objects as $key => $object) { + if ($object->getStatus() != $status_accepted) { + continue; + } + + $results[$key] = $object; + unset($this->objects[$key]); + } + + return $results; + } + + private function filterShouldUpdate(array $phids) { + $statuses = array( + ArcanistDifferentialRevisionStatus::NEEDS_REVISION, + ArcanistDifferentialRevisionStatus::CHANGES_PLANNED, + ArcanistDifferentialRevisionStatus::IN_PREPARATION, + ); + $statuses = array_fuse($statuses); + + $objects = $this->getRevisionsAuthored($this->objects, $phids); + + $results = array(); + foreach ($objects as $key => $object) { + if (empty($statuses[$object->getStatus()])) { + continue; + } + + $results[$key] = $object; + unset($this->objects[$key]); + } + + return $results; + } + + private function filterWaitingForReview(array $phids) { + $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; + + $objects = $this->getRevisionsAuthored($this->objects, $phids); + + $results = array(); + foreach ($objects as $key => $object) { + if ($object->getStatus() != $status_review) { + continue; + } + + $results[$key] = $object; + unset($this->objects[$key]); + } + + return $results; + } + + private function filterWaitingOnAuthors(array $phids) { + $statuses = array( + ArcanistDifferentialRevisionStatus::ACCEPTED, + ArcanistDifferentialRevisionStatus::NEEDS_REVISION, + ArcanistDifferentialRevisionStatus::CHANGES_PLANNED, + ArcanistDifferentialRevisionStatus::IN_PREPARATION, + ); + $statuses = array_fuse($statuses); + + $objects = $this->getRevisionsNotAuthored($this->objects, $phids); + + $results = array(); + foreach ($objects as $key => $object) { + if (empty($statuses[$object->getStatus()])) { + continue; + } + + $results[$key] = $object; + unset($this->objects[$key]); + } + + return $results; + } + +} diff --git a/src/applications/differential/query/DifferentialRevisionResultBucket.php b/src/applications/differential/query/DifferentialRevisionResultBucket.php new file mode 100644 index 0000000000..3cf7d1b7ff --- /dev/null +++ b/src/applications/differential/query/DifferentialRevisionResultBucket.php @@ -0,0 +1,77 @@ +setAncestorClass(__CLASS__) + ->setUniqueMethod('getResultBucketKey') + ->execute(); + } + + protected function getRevisionsUnderReview(array $objects, array $phids) { + $results = array(); + + $objects = $this->getRevisionsNotAuthored($objects, $phids); + + $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; + foreach ($objects as $key => $object) { + if ($object->getStatus() != $status_review) { + continue; + } + + $results[$key] = $object; + } + + return $results; + } + + protected function getRevisionsAuthored(array $objects, array $phids) { + $results = array(); + + foreach ($objects as $key => $object) { + if (isset($phids[$object->getAuthorPHID()])) { + $results[$key] = $object; + } + } + + return $results; + } + + protected function getRevisionsNotAuthored(array $objects, array $phids) { + $results = array(); + + foreach ($objects as $key => $object) { + if (empty($phids[$object->getAuthorPHID()])) { + $results[$key] = $object; + } + } + + return $results; + } + + protected function hasReviewersWithStatus( + DifferentialRevision $revision, + array $phids, + array $statuses) { + + foreach ($revision->getReviewerStatus() as $reviewer) { + $reviewer_phid = $reviewer->getReviewerPHID(); + if (empty($phids[$reviewer_phid])) { + continue; + } + + $status = $reviewer->getStatus(); + if (empty($statuses[$status])) { + continue; + } + + return true; + } + + return false; + } + + +} diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php index 5e4b76a326..5cf6f82198 100644 --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -11,203 +11,80 @@ final class DifferentialRevisionSearchEngine return 'PhabricatorDifferentialApplication'; } + protected function newResultBuckets() { + return DifferentialRevisionResultBucket::getAllResultBuckets(); + } + public function newQuery() { return id(new DifferentialRevisionQuery()) ->needFlags(true) ->needDrafts(true) - ->needRelationships(true); + ->needRelationships(true) + ->needReviewerStatus(true); } - public function getPageSize(PhabricatorSavedQuery $saved) { - if ($saved->getQueryKey() == 'active') { - return 0xFFFF; - } - return parent::getPageSize($saved); - } + protected function buildQueryFromParameters(array $map) { + $query = $this->newQuery(); - public function buildSavedQueryFromRequest(AphrontRequest $request) { - $saved = new PhabricatorSavedQuery(); - - $saved->setParameter( - 'responsiblePHIDs', - $this->readUsersFromRequest($request, 'responsibles')); - - $saved->setParameter( - 'authorPHIDs', - $this->readUsersFromRequest($request, 'authors')); - - $saved->setParameter( - 'reviewerPHIDs', - $this->readUsersFromRequest( - $request, - 'reviewers', - array( - PhabricatorProjectProjectPHIDType::TYPECONST, - ))); - - $saved->setParameter( - 'subscriberPHIDs', - $this->readSubscribersFromRequest($request, 'subscribers')); - - $saved->setParameter( - 'repositoryPHIDs', - $request->getArr('repositories')); - - $saved->setParameter( - 'projects', - $this->readProjectsFromRequest($request, 'projects')); - - $saved->setParameter( - 'draft', - $request->getBool('draft')); - - $saved->setParameter( - 'order', - $request->getStr('order')); - - $saved->setParameter( - 'status', - $request->getStr('status')); - - return $saved; - } - - public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { - $query = id(new DifferentialRevisionQuery()) - ->needFlags(true) - ->needDrafts(true) - ->needRelationships(true); - - $user_datasource = id(new PhabricatorPeopleUserFunctionDatasource()) - ->setViewer($this->requireViewer()); - - $responsible_phids = $saved->getParameter('responsiblePHIDs', array()); - $responsible_phids = $user_datasource->evaluateTokens($responsible_phids); - if ($responsible_phids) { - $query->withResponsibleUsers($responsible_phids); + if ($map['responsiblePHIDs']) { + $query->withResponsibleUsers($map['responsiblePHIDs']); } - $this->setQueryProjects($query, $saved); - - $author_phids = $saved->getParameter('authorPHIDs', array()); - $author_phids = $user_datasource->evaluateTokens($author_phids); - if ($author_phids) { - $query->withAuthors($author_phids); + if ($map['authorPHIDs']) { + $query->withAuthors($map['authorPHIDs']); } - $reviewer_phids = $saved->getParameter('reviewerPHIDs', array()); - if ($reviewer_phids) { - $query->withReviewers($reviewer_phids); + if ($map['reviewerPHIDs']) { + $query->withReviewers($map['reviewerPHIDs']); } - $sub_datasource = id(new PhabricatorMetaMTAMailableFunctionDatasource()) - ->setViewer($this->requireViewer()); - $subscriber_phids = $saved->getParameter('subscriberPHIDs', array()); - $subscriber_phids = $sub_datasource->evaluateTokens($subscriber_phids); - if ($subscriber_phids) { - $query->withCCs($subscriber_phids); + if ($map['repositoryPHIDs']) { + $query->withRepositoryPHIDs($map['repositoryPHIDs']); } - $repository_phids = $saved->getParameter('repositoryPHIDs', array()); - if ($repository_phids) { - $query->withRepositoryPHIDs($repository_phids); - } - - $draft = $saved->getParameter('draft', false); - if ($draft && $this->requireViewer()->isLoggedIn()) { - $query->withDraftRepliesByAuthors( - array($this->requireViewer()->getPHID())); - } - - $status = $saved->getParameter('status'); - if (idx($this->getStatusOptions(), $status)) { - $query->withStatus($status); - } - - $order = $saved->getParameter('order'); - if (idx($this->getOrderOptions(), $order)) { - $query->setOrder($order); - } else { - $query->setOrder(DifferentialRevisionQuery::ORDER_CREATED); + if ($map['status']) { + $query->withStatus($map['status']); } return $query; } - public function buildSearchForm( - AphrontFormView $form, - PhabricatorSavedQuery $saved) { - - $responsible_phids = $saved->getParameter('responsiblePHIDs', array()); - $author_phids = $saved->getParameter('authorPHIDs', array()); - $reviewer_phids = $saved->getParameter('reviewerPHIDs', array()); - $subscriber_phids = $saved->getParameter('subscriberPHIDs', array()); - $repository_phids = $saved->getParameter('repositoryPHIDs', array()); - $only_draft = $saved->getParameter('draft', false); - $projects = $saved->getParameter('projects', array()); - - $form - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('Responsible Users')) - ->setName('responsibles') - ->setDatasource(new PhabricatorPeopleUserFunctionDatasource()) - ->setValue($responsible_phids)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('Authors')) - ->setName('authors') - ->setDatasource(new PhabricatorPeopleUserFunctionDatasource()) - ->setValue($author_phids)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('Reviewers')) - ->setName('reviewers') - ->setDatasource(new PhabricatorProjectOrUserDatasource()) - ->setValue($reviewer_phids)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('Subscribers')) - ->setName('subscribers') - ->setDatasource(new PhabricatorMetaMTAMailableFunctionDatasource()) - ->setValue($subscriber_phids)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('Repositories')) - ->setName('repositories') - ->setDatasource(new DiffusionRepositoryDatasource()) - ->setValue($repository_phids)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('Tags')) - ->setName('projects') - ->setDatasource(new PhabricatorProjectLogicalDatasource()) - ->setValue($projects)) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Status')) - ->setName('status') - ->setOptions($this->getStatusOptions()) - ->setValue($saved->getParameter('status'))); - - if ($this->requireViewer()->isLoggedIn()) { - $form - ->appendChild( - id(new AphrontFormCheckboxControl()) - ->addCheckbox( - 'draft', - 1, - pht('Show only revisions with a draft comment.'), - $only_draft)); - } - - $form - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Order')) - ->setName('order') - ->setOptions($this->getOrderOptions()) - ->setValue($saved->getParameter('order'))); + protected function buildCustomSearchFields() { + return array( + id(new PhabricatorSearchDatasourceField()) + ->setLabel(pht('Responsible Users')) + ->setKey('responsiblePHIDs') + ->setAliases(array('responsiblePHID', 'responsibles', 'responsible')) + ->setDatasource(new DifferentialResponsibleDatasource()) + ->setDescription( + pht('Find revisions that a given user is responsible for.')), + id(new PhabricatorUsersSearchField()) + ->setLabel(pht('Authors')) + ->setKey('authorPHIDs') + ->setAliases(array('author', 'authors', 'authorPHID')) + ->setDescription( + pht('Find revisions with specific authors.')), + id(new PhabricatorSearchDatasourceField()) + ->setLabel(pht('Reviewers')) + ->setKey('reviewerPHIDs') + ->setAliases(array('reviewer', 'reviewers', 'reviewerPHID')) + ->setDatasource(new DiffusionAuditorFunctionDatasource()) + ->setDescription( + pht('Find revisions with specific reviewers.')), + id(new PhabricatorSearchDatasourceField()) + ->setLabel(pht('Repositories')) + ->setKey('repositoryPHIDs') + ->setAliases(array('repository', 'repositories', 'repositoryPHID')) + ->setDatasource(new DiffusionRepositoryDatasource()) + ->setDescription( + pht('Find revisions from specific repositories.')), + id(new PhabricatorSearchSelectField()) + ->setLabel(pht('Status')) + ->setKey('status') + ->setOptions($this->getStatusOptions()) + ->setDescription( + pht('Find revisions with particular statuses.')), + ); } protected function getURI($path) { @@ -235,9 +112,12 @@ final class DifferentialRevisionSearchEngine switch ($query_key) { case 'active': + $bucket_key = DifferentialRevisionRequiredActionResultBucket::BUCKETKEY; + return $query ->setParameter('responsiblePHIDs', array($viewer->getPHID())) - ->setParameter('status', DifferentialRevisionQuery::STATUS_OPEN); + ->setParameter('status', DifferentialRevisionQuery::STATUS_OPEN) + ->setParameter('bucket', $bucket_key); case 'authored': return $query ->setParameter('authorPHIDs', array($viewer->getPHID())); @@ -260,13 +140,6 @@ final class DifferentialRevisionSearchEngine ); } - private function getOrderOptions() { - return array( - DifferentialRevisionQuery::ORDER_CREATED => pht('Created'), - DifferentialRevisionQuery::ORDER_MODIFIED => pht('Updated'), - ); - } - protected function renderResultList( array $revisions, PhabricatorSavedQuery $query, @@ -278,35 +151,26 @@ final class DifferentialRevisionSearchEngine ->setUser($viewer) ->setNoBox($this->isPanelContext()); + $bucket = $this->getResultBucket($query); + + $unlanded = $this->loadUnlandedDependencies($revisions); + $views = array(); - if ($query->getQueryKey() == 'active') { - $split = DifferentialRevisionQuery::splitResponsible( - $revisions, - $query->getParameter('responsiblePHIDs')); - list($blocking, $active, $waiting) = $split; + if ($bucket) { + $bucket->setViewer($viewer); - $views[] = id(clone $template) - ->setHeader(pht('Blocking Others')) - ->setNoDataString( - pht('No revisions are blocked on your action.')) - ->setHighlightAge(true) - ->setRevisions($blocking) - ->setHandles(array()); + try { + $groups = $bucket->newResultGroups($query, $revisions); - $views[] = id(clone $template) - ->setHeader(pht('Action Required')) - ->setNoDataString( - pht('No revisions require your action.')) - ->setHighlightAge(true) - ->setRevisions($active) - ->setHandles(array()); - - $views[] = id(clone $template) - ->setHeader(pht('Waiting on Others')) - ->setNoDataString( - pht('You have no revisions waiting on others.')) - ->setRevisions($waiting) - ->setHandles(array()); + foreach ($groups as $group) { + $views[] = id(clone $template) + ->setHeader($group->getName()) + ->setNoDataString($group->getNoDataString()) + ->setRevisions($group->getObjects()); + } + } catch (Exception $ex) { + $this->addError($ex->getMessage()); + } } else { $views[] = id(clone $template) ->setRevisions($revisions) @@ -325,6 +189,7 @@ final class DifferentialRevisionSearchEngine foreach ($views as $view) { $view->setHandles($handles); + $view->setUnlandedDependencies($unlanded); } if (count($views) == 1) { @@ -361,4 +226,56 @@ final class DifferentialRevisionSearchEngine return $view; } + private function loadUnlandedDependencies(array $revisions) { + $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; + + $phids = array(); + foreach ($revisions as $revision) { + if ($revision->getStatus() != $status_accepted) { + continue; + } + + $phids[] = $revision->getPHID(); + } + + if (!$phids) { + return array(); + } + + $query = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs($phids) + ->withEdgeTypes( + array( + DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST, + )); + + $query->execute(); + + $revision_phids = $query->getDestinationPHIDs(); + if (!$revision_phids) { + return array(); + } + + $viewer = $this->requireViewer(); + + $blocking_revisions = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withPHIDs($revision_phids) + ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) + ->execute(); + $blocking_revisions = mpull($blocking_revisions, null, 'getPHID'); + + $result = array(); + foreach ($revisions as $revision) { + $revision_phid = $revision->getPHID(); + $blocking_phids = $query->getDestinationPHIDs(array($revision_phid)); + $blocking = array_select_keys($blocking_revisions, $blocking_phids); + if ($blocking) { + $result[$revision_phid] = $blocking; + } + } + + return $result; + } + } diff --git a/src/applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php b/src/applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php new file mode 100644 index 0000000000..bea7056ea2 --- /dev/null +++ b/src/applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php @@ -0,0 +1,128 @@ + array( + 'name' => pht('Blocking: ...'), + 'arguments' => pht('reviewer'), + 'summary' => pht('Select a blocking reviewer.'), + 'description' => pht( + "This function allows you to add a reviewer as a blocking ". + "reviewer. For example, this will add `%s` as a blocking reviewer: ". + "\n\n%s\n\n", + 'alincoln', + '> blocking(alincoln)'), + ), + ); + } + + + protected function didLoadResults(array $results) { + foreach ($results as $result) { + $result + ->setColor('red') + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) + ->setIcon('fa-asterisk') + ->setPHID('blocking('.$result->getPHID().')') + ->setDisplayName(pht('Blocking: %s', $result->getDisplayName())) + ->setName($result->getName().' blocking'); + } + + return $results; + } + + protected function evaluateFunction($function, array $argv_list) { + $phids = array(); + foreach ($argv_list as $argv) { + $phids[] = head($argv); + } + + $phids = $this->resolvePHIDs($phids); + + $results = array(); + foreach ($phids as $phid) { + $results[] = array( + 'type' => DifferentialReviewerStatus::STATUS_BLOCKING, + 'phid' => $phid, + ); + } + + return $results; + } + + public function renderFunctionTokens($function, array $argv_list) { + $phids = array(); + foreach ($argv_list as $argv) { + $phids[] = head($argv); + } + + $phids = $this->resolvePHIDs($phids); + + $tokens = $this->renderTokens($phids); + foreach ($tokens as $token) { + $token->setColor(null); + if ($token->isInvalid()) { + $token + ->setValue(pht('Blocking: Invalid Reviewer')); + } else { + $token + ->setIcon('fa-asterisk') + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) + ->setColor('red') + ->setKey('blocking('.$token->getKey().')') + ->setValue(pht('Blocking: %s', $token->getValue())); + } + } + + return $tokens; + } + + private function resolvePHIDs(array $phids) { + $usernames = array(); + foreach ($phids as $key => $phid) { + if (phid_get_type($phid) != PhabricatorPeopleUserPHIDType::TYPECONST) { + $usernames[$key] = $phid; + } + } + + if ($usernames) { + $users = id(new PhabricatorPeopleQuery()) + ->setViewer($this->getViewer()) + ->withUsernames($usernames) + ->execute(); + $users = mpull($users, null, 'getUsername'); + foreach ($usernames as $key => $username) { + $user = idx($users, $username); + if ($user) { + $phids[$key] = $user->getPHID(); + } + } + } + + return $phids; + } + +} diff --git a/src/applications/differential/typeahead/DifferentialExactUserFunctionDatasource.php b/src/applications/differential/typeahead/DifferentialExactUserFunctionDatasource.php new file mode 100644 index 0000000000..8db39e164d --- /dev/null +++ b/src/applications/differential/typeahead/DifferentialExactUserFunctionDatasource.php @@ -0,0 +1,115 @@ +)...'); + } + + public function getDatasourceApplicationClass() { + return 'PhabricatorDifferentialApplication'; + } + + public function getComponentDatasources() { + return array( + new PhabricatorPeopleDatasource(), + ); + } + + public function getDatasourceFunctions() { + return array( + 'exact' => array( + 'name' => pht('Exact: ...'), + 'arguments' => pht('username'), + 'summary' => pht('Find results matching users exactly.'), + 'description' => pht( + "This function allows you to find results associated only with ". + "a user, exactly, and not any of their projects or packages. For ". + "example, this will find results associated with only `%s`:". + "\n\n%s\n\n", + 'alincoln', + '> exact(alincoln)'), + ), + ); + } + + protected function didLoadResults(array $results) { + foreach ($results as $result) { + $result + ->setColor(null) + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) + ->setIcon('fa-asterisk') + ->setPHID('exact('.$result->getPHID().')') + ->setDisplayName(pht('Exact User: %s', $result->getDisplayName())) + ->setName($result->getName().' exact'); + } + + return $results; + } + + protected function evaluateFunction($function, array $argv_list) { + $phids = array(); + foreach ($argv_list as $argv) { + $phids[] = head($argv); + } + + return $this->resolvePHIDs($phids); + } + + public function renderFunctionTokens($function, array $argv_list) { + $phids = array(); + foreach ($argv_list as $argv) { + $phids[] = head($argv); + } + + $phids = $this->resolvePHIDs($phids); + + $tokens = $this->renderTokens($phids); + foreach ($tokens as $token) { + $token->setColor(null); + if ($token->isInvalid()) { + $token + ->setValue(pht('Exact User: Invalid User')); + } else { + $token + ->setIcon('fa-asterisk') + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) + ->setKey('exact('.$token->getKey().')') + ->setValue(pht('Exact User: %s', $token->getValue())); + } + } + + return $tokens; + } + + private function resolvePHIDs(array $phids) { + $usernames = array(); + foreach ($phids as $key => $phid) { + if (phid_get_type($phid) != PhabricatorPeopleUserPHIDType::TYPECONST) { + $usernames[$key] = $phid; + } + } + + if ($usernames) { + $users = id(new PhabricatorPeopleQuery()) + ->setViewer($this->getViewer()) + ->withUsernames($usernames) + ->execute(); + $users = mpull($users, null, 'getUsername'); + foreach ($usernames as $key => $username) { + $user = idx($users, $username); + if ($user) { + $phids[$key] = $user->getPHID(); + } + } + } + + return $phids; + } + +} diff --git a/src/applications/differential/typeahead/DifferentialResponsibleDatasource.php b/src/applications/differential/typeahead/DifferentialResponsibleDatasource.php new file mode 100644 index 0000000000..5307ab1866 --- /dev/null +++ b/src/applications/differential/typeahead/DifferentialResponsibleDatasource.php @@ -0,0 +1,63 @@ +setViewer($viewer) + ->withMemberPHIDs($phids) + ->execute(); + foreach ($projects as $project) { + $phids[] = $project->getPHID(); + $values[] = $project->getPHID(); + } + + $packages = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withOwnerPHIDs($phids) + ->execute(); + foreach ($packages as $package) { + $values[] = $package->getPHID(); + } + + return $values; + } + +} diff --git a/src/applications/differential/typeahead/DifferentialResponsibleUserDatasource.php b/src/applications/differential/typeahead/DifferentialResponsibleUserDatasource.php new file mode 100644 index 0000000000..8ac7b36520 --- /dev/null +++ b/src/applications/differential/typeahead/DifferentialResponsibleUserDatasource.php @@ -0,0 +1,30 @@ +getViewer(), + $values); + } + +} diff --git a/src/applications/differential/typeahead/DifferentialResponsibleViewerFunctionDatasource.php b/src/applications/differential/typeahead/DifferentialResponsibleViewerFunctionDatasource.php new file mode 100644 index 0000000000..51eb2beb9a --- /dev/null +++ b/src/applications/differential/typeahead/DifferentialResponsibleViewerFunctionDatasource.php @@ -0,0 +1,77 @@ + array( + 'name' => pht('Current Viewer'), + 'summary' => pht('Use the current viewing user.'), + 'description' => pht( + 'Show revisions the current viewer is responsible for. This '. + 'function inclues revisions the viewer is responsible for through '. + 'membership in projects and packages.'), + ), + ); + } + + public function loadResults() { + if ($this->getViewer()->getPHID()) { + $results = array($this->renderViewerFunctionToken()); + } else { + $results = array(); + } + + return $this->filterResultsAgainstTokens($results); + } + + protected function canEvaluateFunction($function) { + if (!$this->getViewer()->getPHID()) { + return false; + } + + return parent::canEvaluateFunction($function); + } + + protected function evaluateFunction($function, array $argv_list) { + $results = array(); + foreach ($argv_list as $argv) { + $results[] = $this->getViewer()->getPHID(); + } + + return DifferentialResponsibleDatasource::expandResponsibleUsers( + $this->getViewer(), + $results); + } + + public function renderFunctionTokens($function, array $argv_list) { + $tokens = array(); + foreach ($argv_list as $argv) { + $tokens[] = PhabricatorTypeaheadTokenView::newFromTypeaheadResult( + $this->renderViewerFunctionToken()); + } + return $tokens; + } + + private function renderViewerFunctionToken() { + return $this->newFunctionResult() + ->setName(pht('Current Viewer')) + ->setPHID('viewer()') + ->setIcon('fa-user') + ->setUnique(true); + } + +} diff --git a/src/applications/differential/typeahead/DifferentialReviewerDatasource.php b/src/applications/differential/typeahead/DifferentialReviewerDatasource.php new file mode 100644 index 0000000000..18aa9e48b7 --- /dev/null +++ b/src/applications/differential/typeahead/DifferentialReviewerDatasource.php @@ -0,0 +1,27 @@ +unlandedDependencies = $unlanded_dependencies; + return $this; + } + + public function getUnlandedDependencies() { + return $this->unlandedDependencies; + } public function setNoDataString($no_data_string) { $this->noDataString = $no_data_string; @@ -65,20 +75,6 @@ final class DifferentialRevisionListView extends AphrontView { public function render() { $viewer = $this->getViewer(); - $fresh = PhabricatorEnv::getEnvConfig('differential.days-fresh'); - if ($fresh) { - $fresh = PhabricatorCalendarHoliday::getNthBusinessDay( - time(), - -$fresh); - } - - $stale = PhabricatorEnv::getEnvConfig('differential.days-stale'); - if ($stale) { - $stale = PhabricatorCalendarHoliday::getNthBusinessDay( - time(), - -$stale); - } - $this->initBehavior('phabricator-tooltips', array()); $this->requireResource('aphront-tooltip-css'); @@ -109,18 +105,6 @@ final class DifferentialRevisionListView extends AphrontView { $modified = $revision->getDateModified(); $status = $revision->getStatus(); - $show_age = ($fresh || $stale) && - $this->highlightAge && - !$revision->isClosed(); - - if ($stale && $modified < $stale) { - $object_age = PHUIObjectItemView::AGE_OLD; - } else if ($fresh && $modified < $fresh) { - $object_age = PHUIObjectItemView::AGE_STALE; - } else { - $object_age = PHUIObjectItemView::AGE_FRESH; - } - $status_name = ArcanistDifferentialRevisionStatus::getNameForRevisionStatus($status); @@ -143,15 +127,20 @@ final class DifferentialRevisionListView extends AphrontView { $item->addAttribute($draft); } - /* Most things 'Need Review', so accept it's the default */ - if ($status != ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { - $item->addAttribute($status_name); - } - // Author $author_handle = $this->handles[$revision->getAuthorPHID()]; $item->addByline(pht('Author: %s', $author_handle->renderLink())); + $unlanded = idx($this->unlandedDependencies, $phid); + if ($unlanded) { + $item->addAttribute( + array( + id(new PHUIIconView())->setIcon('fa-chain-broken', 'red'), + ' ', + pht('Open Dependencies'), + )); + } + $reviewers = array(); // TODO: As above, this should be based on `getReviewerStatus()`. foreach ($revision->getReviewers() as $reviewer) { @@ -164,7 +153,7 @@ final class DifferentialRevisionListView extends AphrontView { } $item->addAttribute(pht('Reviewers: %s', $reviewers)); - $item->setEpoch($revision->getDateModified(), $object_age); + $item->setEpoch($revision->getDateModified()); switch ($status) { case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index e2f0d51f4a..419be17898 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -455,7 +455,12 @@ final class DiffusionCommitController extends DiffusionController { if ($audit_requests) { $user_requests = array(); $other_requests = array(); + foreach ($audit_requests as $audit_request) { + if (!$audit_request->isInteresting()) { + continue; + } + if ($audit_request->isUser()) { $user_requests[] = $audit_request; } else { @@ -471,7 +476,7 @@ final class DiffusionCommitController extends DiffusionController { if ($other_requests) { $view->addProperty( - pht('Project/Package Auditors'), + pht('Group Auditors'), $this->renderAuditStatusView($other_requests)); } } diff --git a/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php b/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php index a7dbdde682..3c34876995 100644 --- a/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php +++ b/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php @@ -18,8 +18,13 @@ abstract class DiffusionAuditorsHeraldAction $object = $adapter->getObject(); $auditors = $object->getAudits(); - $auditors = mpull($auditors, null, 'getAuditorPHID'); - $current = array_keys($auditors); + + $current = array(); + foreach ($auditors as $auditor) { + if ($auditor->isInteresting()) { + $current[] = $auditor->getAuditorPHID(); + } + } $allowed_types = array( PhabricatorPeopleUserPHIDType::TYPECONST, diff --git a/src/applications/home/controller/PhabricatorHomeMainController.php b/src/applications/home/controller/PhabricatorHomeMainController.php index 950944188b..c5387a5fb7 100644 --- a/src/applications/home/controller/PhabricatorHomeMainController.php +++ b/src/applications/home/controller/PhabricatorHomeMainController.php @@ -205,40 +205,29 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController { } private function buildRevisionPanel() { - $user = $this->getRequest()->getUser(); - $user_phid = $user->getPHID(); + $viewer = $this->getViewer(); - $revision_query = id(new DifferentialRevisionQuery()) - ->setViewer($user) - ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) - ->withResponsibleUsers(array($user_phid)) - ->needRelationships(true) - ->needFlags(true) - ->needDrafts(true); + $revisions = PhabricatorDifferentialApplication::loadNeedAttentionRevisions( + $viewer); - $revisions = $revision_query->execute(); - - list($blocking, $active) = DifferentialRevisionQuery::splitResponsible( - $revisions, - array($user_phid)); - - if (!$blocking && !$active) { + if (!$revisions) { return $this->renderMiniPanel( pht('No Waiting Revisions'), pht('No revisions are waiting on you.')); } $title = pht('Revisions Waiting on You'); - $href = '/differential'; + $href = '/differential/'; $panel = new PHUIObjectBoxView(); $panel->setHeader($this->renderSectionHeader($title, $href)); $revision_view = id(new DifferentialRevisionListView()) ->setHighlightAge(true) - ->setRevisions(array_merge($blocking, $active)) - ->setUser($user); + ->setRevisions($revisions) + ->setUser($viewer); + $phids = array_merge( - array($user_phid), + array($viewer->getPHID()), $revision_view->getRequiredHandlePHIDs()); $handles = $this->loadViewerHandles($phids); diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php index 4c92f9d642..0287966fa3 100644 --- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php +++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php @@ -148,8 +148,7 @@ final class ManiphestTaskSearchEngine } protected function buildQueryFromParameters(array $map) { - $query = id(new ManiphestTaskQuery()) - ->needProjectPHIDs(true); + $query = $this->newQuery(); if ($map['assignedPHIDs']) { $query->withOwners($map['assignedPHIDs']); diff --git a/src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php b/src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php index e06da5f4e3..8657981836 100644 --- a/src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php +++ b/src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php @@ -24,6 +24,8 @@ final class PhabricatorMetaMTAMemberQuery extends PhabricatorQuery { } public function execute() { + $viewer = $this->getViewer(); + $phids = array_fuse($this->phids); $actors = array(); $type_map = array(); @@ -33,6 +35,33 @@ final class PhabricatorMetaMTAMemberQuery extends PhabricatorQuery { // TODO: Generalize this somewhere else. + + // If we have packages, break them down into their constituent user and + // project owners first. Then we'll resolve those and build the packages + // back up from the pieces. + $package_type = PhabricatorOwnersPackagePHIDType::TYPECONST; + $package_phids = idx($type_map, $package_type, array()); + unset($type_map[$package_type]); + + $package_map = array(); + if ($package_phids) { + $packages = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withPHIDs($package_phids) + ->execute(); + + foreach ($packages as $package) { + $package_owners = array(); + foreach ($package->getOwners() as $owner) { + $owner_phid = $owner->getUserPHID(); + $owner_type = phid_get_type($owner_phid); + $type_map[$owner_type][] = $owner_phid; + $package_owners[] = $owner_phid; + } + $package_map[$package->getPHID()] = $package_owners; + } + } + $results = array(); foreach ($type_map as $type => $phids) { switch ($type) { @@ -40,7 +69,7 @@ final class PhabricatorMetaMTAMemberQuery extends PhabricatorQuery { // NOTE: We're loading the projects here in order to respect policies. $projects = id(new PhabricatorProjectQuery()) - ->setViewer($this->getViewer()) + ->setViewer($viewer) ->withPHIDs($phids) ->needMembers(true) ->needWatchers(true) @@ -96,6 +125,21 @@ final class PhabricatorMetaMTAMemberQuery extends PhabricatorQuery { } } + // For any packages, stitch them back together from the resolved users + // and projects. + if ($package_map) { + foreach ($package_map as $package_phid => $owner_phids) { + $resolved = array(); + foreach ($owner_phids as $owner_phid) { + $resolved_phids = idx($results, $owner_phid, array()); + foreach ($resolved_phids as $resolved_phid) { + $resolved[] = $resolved_phid; + } + } + $results[$package_phid] = $resolved; + } + } + return $results; } diff --git a/src/applications/metamta/replyhandler/PhabricatorMailTarget.php b/src/applications/metamta/replyhandler/PhabricatorMailTarget.php index e7e79c5ef9..c607087b22 100644 --- a/src/applications/metamta/replyhandler/PhabricatorMailTarget.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailTarget.php @@ -107,11 +107,15 @@ final class PhabricatorMailTarget extends Phobject { $cc_handles = iterator_to_array($cc_handles); $body = ''; + if ($to_handles) { - $body .= "To: ".implode(', ', mpull($to_handles, 'getName'))."\n"; + $to_names = mpull($to_handles, 'getCommandLineObjectName'); + $body .= "To: ".implode(', ', $to_names)."\n"; } + if ($cc_handles) { - $body .= "Cc: ".implode(', ', mpull($cc_handles, 'getName'))."\n"; + $cc_names = mpull($cc_handles, 'getCommandLineObjectName'); + $body .= "Cc: ".implode(', ', $cc_names)."\n"; } return $body; diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index c4793ed918..1ae5884228 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -634,7 +634,7 @@ final class PhabricatorMetaMTAMail } $mailer->setBody($body); - $html_emails = false; + $html_emails = true; if ($use_prefs && $prefs) { $html_emails = $prefs->getPreference( PhabricatorUserPreferences::PREFERENCE_HTML_EMAILS, diff --git a/src/applications/metamta/typeahead/PhabricatorMetaMTAMailableDatasource.php b/src/applications/metamta/typeahead/PhabricatorMetaMTAMailableDatasource.php index 5fa7492fef..2e0e03bb3c 100644 --- a/src/applications/metamta/typeahead/PhabricatorMetaMTAMailableDatasource.php +++ b/src/applications/metamta/typeahead/PhabricatorMetaMTAMailableDatasource.php @@ -8,7 +8,7 @@ final class PhabricatorMetaMTAMailableDatasource } public function getPlaceholderText() { - return pht('Type a user, project, or mailing list name...'); + return pht('Type a user, project, package, or mailing list name...'); } public function getDatasourceApplicationClass() { @@ -19,6 +19,7 @@ final class PhabricatorMetaMTAMailableDatasource return array( new PhabricatorPeopleDatasource(), new PhabricatorProjectDatasource(), + new PhabricatorOwnersPackageDatasource(), ); } diff --git a/src/applications/oauthserver/query/PhabricatorOAuthServerSchemaSpec.php b/src/applications/oauthserver/query/PhabricatorOAuthServerSchemaSpec.php new file mode 100644 index 0000000000..49c2498d02 --- /dev/null +++ b/src/applications/oauthserver/query/PhabricatorOAuthServerSchemaSpec.php @@ -0,0 +1,10 @@ +buildEdgeSchemata(new PhabricatorOAuthServerClient()); + } + +} diff --git a/src/applications/owners/application/PhabricatorOwnersApplication.php b/src/applications/owners/application/PhabricatorOwnersApplication.php index 4b4841390a..3ef5f974d9 100644 --- a/src/applications/owners/application/PhabricatorOwnersApplication.php +++ b/src/applications/owners/application/PhabricatorOwnersApplication.php @@ -39,6 +39,12 @@ final class PhabricatorOwnersApplication extends PhabricatorApplication { return self::GROUP_UTILITIES; } + public function getRemarkupRules() { + return array( + new PhabricatorOwnersPackageRemarkupRule(), + ); + } + public function getRoutes() { return array( '/owners/' => array( diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php index 5da36ad473..c91de51cc6 100644 --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -144,7 +144,7 @@ final class PhabricatorOwnersDetailController } $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb($package->getName()); + $crumbs->addTextCrumb($package->getMonogram()); $crumbs->setBorder(true); $timeline = $this->buildTransactionTimeline( @@ -184,6 +184,19 @@ final class PhabricatorOwnersDetailController } $view->addProperty(pht('Owners'), $owner_list); + + $dominion = $package->getDominion(); + $dominion_map = PhabricatorOwnersPackage::getDominionOptionsMap(); + $spec = idx($dominion_map, $dominion, array()); + $name = idx($spec, 'short', $dominion); + $view->addProperty(pht('Dominion'), $name); + + $auto = $package->getAutoReview(); + $autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); + $spec = idx($autoreview_map, $auto, array()); + $name = idx($spec, 'name', $auto); + $view->addProperty(pht('Auto Review'), $name); + if ($package->getAuditingEnabled()) { $auditing = pht('Enabled'); } else { diff --git a/src/applications/owners/controller/PhabricatorOwnersPathsController.php b/src/applications/owners/controller/PhabricatorOwnersPathsController.php index 55aeb11b60..d1d6de760d 100644 --- a/src/applications/owners/controller/PhabricatorOwnersPathsController.php +++ b/src/applications/owners/controller/PhabricatorOwnersPathsController.php @@ -64,7 +64,7 @@ final class PhabricatorOwnersPathsController $editor->applyTransactions($package, $xactions); return id(new AphrontRedirectResponse()) - ->setURI('/owners/package/'.$package->getID().'/'); + ->setURI($package->getURI()); } else { $paths = $package->getPaths(); $path_refs = mpull($paths, 'getRef'); @@ -106,7 +106,7 @@ final class PhabricatorOwnersPathsController require_celerity_resource('owners-path-editor-css'); - $cancel_uri = '/owners/package/'.$package->getID().'/'; + $cancel_uri = $package->getURI(); $form = id(new AphrontFormView()) ->setUser($viewer) diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php index fc97be6fce..b20613c92e 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php @@ -51,8 +51,7 @@ final class PhabricatorOwnersPackageEditEngine } protected function getObjectViewURI($object) { - $id = $object->getID(); - return "/owners/package/{$id}/"; + return $object->getURI(); } protected function buildCustomEditFields($object) { @@ -85,6 +84,12 @@ applying a transaction of this type. EOTEXT ); + $autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); + $autoreview_map = ipull($autoreview_map, 'name'); + + $dominion_map = PhabricatorOwnersPackage::getDominionOptionsMap(); + $dominion_map = ipull($dominion_map, 'name'); + return array( id(new PhabricatorTextEditField()) ->setKey('name') @@ -101,6 +106,28 @@ EOTEXT ->setDatasource(new PhabricatorProjectOrUserDatasource()) ->setIsCopyable(true) ->setValue($object->getOwnerPHIDs()), + id(new PhabricatorSelectEditField()) + ->setKey('dominion') + ->setLabel(pht('Dominion')) + ->setDescription( + pht('Change package dominion rules.')) + ->setTransactionType( + PhabricatorOwnersPackageTransaction::TYPE_DOMINION) + ->setIsCopyable(true) + ->setValue($object->getDominion()) + ->setOptions($dominion_map), + id(new PhabricatorSelectEditField()) + ->setKey('autoReview') + ->setLabel(pht('Auto Review')) + ->setDescription( + pht( + 'Automatically trigger reviews for commits affecting files in '. + 'this package.')) + ->setTransactionType( + PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW) + ->setIsCopyable(true) + ->setValue($object->getAutoReview()) + ->setOptions($autoreview_map), id(new PhabricatorSelectEditField()) ->setKey('auditing') ->setLabel(pht('Auditing')) diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php index aa5ae1c6f3..5fb4a9af8a 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php @@ -20,6 +20,8 @@ final class PhabricatorOwnersPackageTransactionEditor $types[] = PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION; $types[] = PhabricatorOwnersPackageTransaction::TYPE_PATHS; $types[] = PhabricatorOwnersPackageTransaction::TYPE_STATUS; + $types[] = PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW; + $types[] = PhabricatorOwnersPackageTransaction::TYPE_DOMINION; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; @@ -47,6 +49,10 @@ final class PhabricatorOwnersPackageTransactionEditor return mpull($paths, 'getRef'); case PhabricatorOwnersPackageTransaction::TYPE_STATUS: return $object->getStatus(); + case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: + return $object->getAutoReview(); + case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: + return $object->getDominion(); } } @@ -58,6 +64,8 @@ final class PhabricatorOwnersPackageTransactionEditor case PhabricatorOwnersPackageTransaction::TYPE_NAME: case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: case PhabricatorOwnersPackageTransaction::TYPE_STATUS: + case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: + case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: return $xaction->getNewValue(); case PhabricatorOwnersPackageTransaction::TYPE_PATHS: $new = $xaction->getNewValue(); @@ -113,6 +121,12 @@ final class PhabricatorOwnersPackageTransactionEditor case PhabricatorOwnersPackageTransaction::TYPE_STATUS: $object->setStatus($xaction->getNewValue()); return; + case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: + $object->setAutoReview($xaction->getNewValue()); + return; + case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: + $object->setDominion($xaction->getNewValue()); + return; } return parent::applyCustomInternalTransaction($object, $xaction); @@ -127,6 +141,8 @@ final class PhabricatorOwnersPackageTransactionEditor case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: case PhabricatorOwnersPackageTransaction::TYPE_AUDITING: case PhabricatorOwnersPackageTransaction::TYPE_STATUS: + case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: + case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: return; case PhabricatorOwnersPackageTransaction::TYPE_OWNERS: $old = $xaction->getOldValue(); @@ -205,6 +221,61 @@ final class PhabricatorOwnersPackageTransactionEditor $error->setIsMissingFieldError(true); $errors[] = $error; } + + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + if (preg_match('([,!])', $new)) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht( + 'Package names may not contain commas (",") or exclamation '. + 'marks ("!"). These characters are ambiguous when package '. + 'names are parsed from the command line.'), + $xaction); + } + } + + break; + case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: + $map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + if (empty($map[$new])) { + $valid = array_keys($map); + + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht( + 'Autoreview setting "%s" is not valid. '. + 'Valid settings are: %s.', + $new, + implode(', ', $valid)), + $xaction); + } + } + break; + case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: + $map = PhabricatorOwnersPackage::getDominionOptionsMap(); + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + if (empty($map[$new])) { + $valid = array_keys($map); + + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht( + 'Dominion setting "%s" is not valid. '. + 'Valid settings are: %s.', + $new, + implode(', ', $valid)), + $xaction); + } + } break; case PhabricatorOwnersPackageTransaction::TYPE_PATHS: if (!$xactions) { @@ -331,8 +402,7 @@ final class PhabricatorOwnersPackageTransactionEditor $body = parent::buildMailBody($object, $xactions); - $detail_uri = PhabricatorEnv::getProductionURI( - '/owners/package/'.$object->getID().'/'); + $detail_uri = PhabricatorEnv::getProductionURI($object->getURI()); $body->addLinkSection( pht('PACKAGE DETAIL'), diff --git a/src/applications/owners/phid/PhabricatorOwnersPackagePHIDType.php b/src/applications/owners/phid/PhabricatorOwnersPackagePHIDType.php index fa667d4dcb..cfbaf6eeb2 100644 --- a/src/applications/owners/phid/PhabricatorOwnersPackagePHIDType.php +++ b/src/applications/owners/phid/PhabricatorOwnersPackagePHIDType.php @@ -9,7 +9,7 @@ final class PhabricatorOwnersPackagePHIDType extends PhabricatorPHIDType { } public function getTypeIcon() { - return 'fa-list-alt'; + return 'fa-shopping-bag'; } public function newObject() { @@ -36,12 +36,50 @@ final class PhabricatorOwnersPackagePHIDType extends PhabricatorPHIDType { foreach ($handles as $phid => $handle) { $package = $objects[$phid]; + $monogram = $package->getMonogram(); $name = $package->getName(); $id = $package->getID(); + $uri = $package->getURI(); - $handle->setName($name); - $handle->setURI("/owners/package/{$id}/"); + $handle + ->setName($monogram) + ->setFullName("{$monogram}: {$name}") + ->setCommandLineObjectName("{$monogram} {$name}") + ->setURI($uri); + + if ($package->isArchived()) { + $handle->setStatus(PhabricatorObjectHandle::STATUS_CLOSED); + } } } + public function canLoadNamedObject($name) { + return preg_match('/^O\d*[1-9]\d*$/i', $name); + } + + public function loadNamedObjects( + PhabricatorObjectQuery $query, + array $names) { + + $id_map = array(); + foreach ($names as $name) { + $id = (int)substr($name, 1); + $id_map[$id][] = $name; + } + + $objects = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($query->getViewer()) + ->withIDs(array_keys($id_map)) + ->execute(); + + $results = array(); + foreach ($objects as $id => $object) { + foreach (idx($id_map, $id, array()) as $name) { + $results[$name] = $object; + } + } + + return $results; + } + } diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php index 86bcb54f9d..c585f4022f 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php @@ -351,6 +351,7 @@ final class PhabricatorOwnersPackageQuery } $packages = $this->controlResults; + $weak_dominion = PhabricatorOwnersPackage::DOMINION_WEAK; $matches = array(); foreach ($packages as $package_id => $package) { @@ -373,6 +374,7 @@ final class PhabricatorOwnersPackageQuery if ($best_match && $include) { $matches[$package_id] = array( 'strength' => $best_match, + 'weak' => ($package->getDominion() == $weak_dominion), 'package' => $package, ); } @@ -381,6 +383,18 @@ final class PhabricatorOwnersPackageQuery $matches = isort($matches, 'strength'); $matches = array_reverse($matches); + $first_id = null; + foreach ($matches as $package_id => $match) { + if ($first_id === null) { + $first_id = $package_id; + continue; + } + + if ($match['weak']) { + unset($matches[$package_id]); + } + } + return array_values(ipull($matches, 'package')); } diff --git a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php index f92817d79e..728c3f42a8 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php @@ -136,9 +136,9 @@ final class PhabricatorOwnersPackageSearchEngine $item = id(new PHUIObjectItemView()) ->setObject($package) - ->setObjectName(pht('Package %d', $id)) + ->setObjectName($package->getMonogram()) ->setHeader($package->getName()) - ->setHref('/owners/package/'.$id.'/'); + ->setHref($package->getURI()); if ($package->isArchived()) { $item->setDisabled(true); diff --git a/src/applications/owners/remarkup/PhabricatorOwnersPackageRemarkupRule.php b/src/applications/owners/remarkup/PhabricatorOwnersPackageRemarkupRule.php new file mode 100644 index 0000000000..475f6c57be --- /dev/null +++ b/src/applications/owners/remarkup/PhabricatorOwnersPackageRemarkupRule.php @@ -0,0 +1,19 @@ +getEngine()->getConfig('viewer'); + + return id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withIDs($ids) + ->execute(); + } + +} diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php index adfc4d94d8..35f8ac58b5 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -14,12 +14,14 @@ final class PhabricatorOwnersPackage protected $name; protected $originalName; protected $auditingEnabled; + protected $autoReview; protected $description; protected $primaryOwnerPHID; protected $mailKey; protected $status; protected $viewPolicy; protected $editPolicy; + protected $dominion; private $paths = self::ATTACHABLE; private $owners = self::ATTACHABLE; @@ -28,6 +30,14 @@ final class PhabricatorOwnersPackage const STATUS_ACTIVE = 'active'; const STATUS_ARCHIVED = 'archived'; + const AUTOREVIEW_NONE = 'none'; + const AUTOREVIEW_SUBSCRIBE = 'subscribe'; + const AUTOREVIEW_REVIEW = 'review'; + const AUTOREVIEW_BLOCK = 'block'; + + const DOMINION_STRONG = 'strong'; + const DOMINION_WEAK = 'weak'; + public static function initializeNewPackage(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) ->setViewer($actor) @@ -41,6 +51,8 @@ final class PhabricatorOwnersPackage return id(new PhabricatorOwnersPackage()) ->setAuditingEnabled(0) + ->setAutoReview(self::AUTOREVIEW_NONE) + ->setDominion(self::DOMINION_STRONG) ->setViewPolicy($view_policy) ->setEditPolicy($edit_policy) ->attachPaths(array()) @@ -56,6 +68,36 @@ final class PhabricatorOwnersPackage ); } + public static function getAutoreviewOptionsMap() { + return array( + self::AUTOREVIEW_NONE => array( + 'name' => pht('No Autoreview'), + ), + self::AUTOREVIEW_SUBSCRIBE => array( + 'name' => pht('Subscribe to Changes'), + ), + self::AUTOREVIEW_REVIEW => array( + 'name' => pht('Review Changes'), + ), + self::AUTOREVIEW_BLOCK => array( + 'name' => pht('Review Changes (Blocking)'), + ), + ); + } + + public static function getDominionOptionsMap() { + return array( + self::DOMINION_STRONG => array( + 'name' => pht('Strong (Control All Paths)'), + 'short' => pht('Strong'), + ), + self::DOMINION_WEAK => array( + 'name' => pht('Weak (Control Unowned Paths)'), + 'short' => pht('Weak'), + ), + ); + } + protected function getConfiguration() { return array( // This information is better available from the history table. @@ -69,6 +111,8 @@ final class PhabricatorOwnersPackage 'auditingEnabled' => 'bool', 'mailKey' => 'bytes20', 'status' => 'text32', + 'autoReview' => 'text32', + 'dominion' => 'text32', ), ) + parent::getConfiguration(); } @@ -165,7 +209,7 @@ final class PhabricatorOwnersPackage foreach (array_chunk(array_keys($fragments), 128) as $chunk) { $rows[] = queryfx_all( $conn, - 'SELECT pkg.id, p.excluded, p.path + 'SELECT pkg.id, pkg.dominion, p.excluded, p.path FROM %T pkg JOIN %T p ON p.packageID = pkg.id WHERE p.path IN (%Ls) %Q', $package->getTableName(), @@ -207,35 +251,100 @@ final class PhabricatorOwnersPackage } public static function findLongestPathsPerPackage(array $rows, array $paths) { - $ids = array(); - foreach (igroup($rows, 'id') as $id => $package_paths) { - $relevant_paths = array_select_keys( - $paths, - ipull($package_paths, 'path')); + // Build a map from each path to all the package paths which match it. + $path_hits = array(); + $weak = array(); + foreach ($rows as $row) { + $id = $row['id']; + $path = $row['path']; + $length = strlen($path); + $excluded = $row['excluded']; - // For every package, remove all excluded paths. - $remove = array(); - foreach ($package_paths as $package_path) { - if ($package_path['excluded']) { - $remove += idx($relevant_paths, $package_path['path'], array()); - unset($relevant_paths[$package_path['path']]); - } + if ($row['dominion'] === self::DOMINION_WEAK) { + $weak[$id] = true; } - if ($remove) { - foreach ($relevant_paths as $fragment => $fragment_paths) { - $relevant_paths[$fragment] = array_diff_key($fragment_paths, $remove); - } - } - - $relevant_paths = array_filter($relevant_paths); - if ($relevant_paths) { - $ids[$id] = max(array_map('strlen', array_keys($relevant_paths))); + $matches = $paths[$path]; + foreach ($matches as $match => $ignored) { + $path_hits[$match][] = array( + 'id' => $id, + 'excluded' => $excluded, + 'length' => $length, + ); } } - return $ids; + // For each path, process the matching package paths to figure out which + // packages actually own it. + $path_packages = array(); + foreach ($path_hits as $match => $hits) { + $hits = isort($hits, 'length'); + + $packages = array(); + foreach ($hits as $hit) { + $package_id = $hit['id']; + if ($hit['excluded']) { + unset($packages[$package_id]); + } else { + $packages[$package_id] = $hit; + } + } + + $path_packages[$match] = $packages; + } + + // Remove packages with weak dominion rules that should cede control to + // a more specific package. + if ($weak) { + foreach ($path_packages as $match => $packages) { + $packages = isort($packages, 'length'); + $packages = array_reverse($packages, true); + + $first = null; + foreach ($packages as $package_id => $package) { + // If this is the first package we've encountered, note it and + // continue. We're iterating over the packages from longest to + // shortest match, so this package always has the strongest claim + // on the path. + if ($first === null) { + $first = $package_id; + continue; + } + + // If this is the first package we saw, its claim stands even if it + // is a weak package. + if ($first === $package_id) { + continue; + } + + // If this is a weak package and not the first package we saw, + // cede its claim to the stronger package. + if (isset($weak[$package_id])) { + unset($packages[$package_id]); + } + } + + $path_packages[$match] = $packages; + } + } + + // For each package that owns at least one path, identify the longest + // path it owns. + $package_lengths = array(); + foreach ($path_packages as $match => $hits) { + foreach ($hits as $hit) { + $length = $hit['length']; + $id = $hit['id']; + if (empty($package_lengths[$id])) { + $package_lengths[$id] = $length; + } else { + $package_lengths[$id] = max($package_lengths[$id], $length); + } + } + } + + return $package_lengths; } public static function splitPath($path) { @@ -289,6 +398,14 @@ final class PhabricatorOwnersPackage return isset($owner_phids[$phid]); } + public function getMonogram() { + return 'O'.$this->getID(); + } + + public function getURI() { + // TODO: Move these to "/O123" for consistency. + return '/owners/package/'.$this->getID().'/'; + } /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php index f59544b1f0..359a1fcb8a 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php @@ -10,6 +10,8 @@ final class PhabricatorOwnersPackageTransaction const TYPE_DESCRIPTION = 'owners.description'; const TYPE_PATHS = 'owners.paths'; const TYPE_STATUS = 'owners.status'; + const TYPE_AUTOREVIEW = 'owners.autoreview'; + const TYPE_DOMINION = 'owners.dominion'; public function getApplicationName() { return 'owners'; @@ -143,6 +145,30 @@ final class PhabricatorOwnersPackageTransaction '%s archived this package.', $this->renderHandleLink($author_phid)); } + case self::TYPE_AUTOREVIEW: + $map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); + $map = ipull($map, 'name'); + + $old = idx($map, $old, $old); + $new = idx($map, $new, $new); + + return pht( + '%s adjusted autoreview from "%s" to "%s".', + $this->renderHandleLink($author_phid), + $old, + $new); + case self::TYPE_DOMINION: + $map = PhabricatorOwnersPackage::getDominionOptionsMap(); + $map = ipull($map, 'short'); + + $old = idx($map, $old, $old); + $new = idx($map, $new, $new); + + return pht( + '%s adjusted package dominion rules from "%s" to "%s".', + $this->renderHandleLink($author_phid), + $old, + $new); } return parent::getTitle(); diff --git a/src/applications/owners/storage/__tests__/PhabricatorOwnersPackageTestCase.php b/src/applications/owners/storage/__tests__/PhabricatorOwnersPackageTestCase.php index fbe70f6789..7d24fc6e8d 100644 --- a/src/applications/owners/storage/__tests__/PhabricatorOwnersPackageTestCase.php +++ b/src/applications/owners/storage/__tests__/PhabricatorOwnersPackageTestCase.php @@ -4,9 +4,24 @@ final class PhabricatorOwnersPackageTestCase extends PhabricatorTestCase { public function testFindLongestPathsPerPackage() { $rows = array( - array('id' => 1, 'excluded' => 0, 'path' => 'src/'), - array('id' => 1, 'excluded' => 1, 'path' => 'src/releeph/'), - array('id' => 2, 'excluded' => 0, 'path' => 'src/releeph/'), + array( + 'id' => 1, + 'excluded' => 0, + 'dominion' => PhabricatorOwnersPackage::DOMINION_STRONG, + 'path' => 'src/', + ), + array( + 'id' => 1, + 'excluded' => 1, + 'dominion' => PhabricatorOwnersPackage::DOMINION_STRONG, + 'path' => 'src/releeph/', + ), + array( + 'id' => 2, + 'excluded' => 0, + 'dominion' => PhabricatorOwnersPackage::DOMINION_STRONG, + 'path' => 'src/releeph/', + ), ); $paths = array( @@ -29,6 +44,62 @@ final class PhabricatorOwnersPackageTestCase extends PhabricatorTestCase { 2 => strlen('src/releeph/'), ), PhabricatorOwnersPackage::findLongestPathsPerPackage($rows, $paths)); + + + // Test packages with weak dominion. Here, only package #2 should own the + // path. Package #1's claim is ceded to Package #2 because it uses weak + // rules. Package #2 gets the claim even though it also has weak rules + // because there is no more-specific package. + + $rows = array( + array( + 'id' => 1, + 'excluded' => 0, + 'dominion' => PhabricatorOwnersPackage::DOMINION_WEAK, + 'path' => 'src/', + ), + array( + 'id' => 2, + 'excluded' => 0, + 'dominion' => PhabricatorOwnersPackage::DOMINION_WEAK, + 'path' => 'src/applications/', + ), + ); + + $pvalue = array('src/applications/main/main.c' => true); + + $paths = array( + 'src/' => $pvalue, + 'src/applications/' => $pvalue, + ); + + $this->assertEqual( + array( + 2 => strlen('src/applications/'), + ), + PhabricatorOwnersPackage::findLongestPathsPerPackage($rows, $paths)); + + + // Now, add a more specific path to Package #1. This tests nested ownership + // in packages with weak dominion rules. This time, Package #1 should end + // up back on top, with Package #2 cedeing control to its more specific + // path. + $rows[] = array( + 'id' => 1, + 'excluded' => 0, + 'dominion' => PhabricatorOwnersPackage::DOMINION_WEAK, + 'path' => 'src/applications/main/', + ); + + $paths['src/applications/main/'] = $pvalue; + + $this->assertEqual( + array( + 1 => strlen('src/applications/main/'), + ), + PhabricatorOwnersPackage::findLongestPathsPerPackage($rows, $paths)); + + } } diff --git a/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php b/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php index 9230ce270e..41d5d6823a 100644 --- a/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php +++ b/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php @@ -27,9 +27,12 @@ final class PhabricatorOwnersPackageDatasource $packages = $this->executeQuery($query); foreach ($packages as $package) { + $name = $package->getName(); + $monogram = $package->getMonogram(); + $results[] = id(new PhabricatorTypeaheadResult()) - ->setName($package->getName()) - ->setURI('/owners/package/'.$package->getID().'/') + ->setName("{$monogram}: {$name}") + ->setURI($package->getURI()) ->setPHID($package->getPHID()); } diff --git a/src/applications/passphrase/conduit/PassphraseQueryConduitAPIMethod.php b/src/applications/passphrase/conduit/PassphraseQueryConduitAPIMethod.php index 18010efd73..387e01cb2e 100644 --- a/src/applications/passphrase/conduit/PassphraseQueryConduitAPIMethod.php +++ b/src/applications/passphrase/conduit/PassphraseQueryConduitAPIMethod.php @@ -63,9 +63,12 @@ final class PassphraseQueryConduitAPIMethod $material = array(); + $is_locked = $credential->getIsLocked(); + $allow_api = ($credential->getAllowConduit() && !$is_locked); + $secret = null; if ($request->getValue('needSecrets')) { - if ($credential->getAllowConduit()) { + if ($allow_api) { $secret = $credential->getSecret(); if ($secret) { $secret = $secret->openEnvelope(); @@ -102,7 +105,7 @@ final class PassphraseQueryConduitAPIMethod break; } - if (!$credential->getAllowConduit()) { + if (!$allow_api) { $material['noAPIAccess'] = pht( 'This private material for this credential is not accessible via '. 'API calls.'); diff --git a/src/applications/passphrase/controller/PassphraseCredentialConduitController.php b/src/applications/passphrase/controller/PassphraseCredentialConduitController.php index b86d18c227..ce8f21f62d 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialConduitController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialConduitController.php @@ -33,8 +33,22 @@ final class PassphraseCredentialConduitController throw new Exception(pht('Credential has invalid type "%s"!', $type)); } + $is_locked = $credential->getIsLocked(); + + if ($is_locked) { + return $this->newDialog() + ->setUser($viewer) + ->setTitle(pht('Credential Locked')) + ->appendChild( + pht( + 'This credential can not be made available via Conduit because '. + 'it is locked.')) + ->addCancelButton($view_uri); + } + if ($request->isFormPost()) { $xactions = array(); + $xactions[] = id(new PassphraseCredentialTransaction()) ->setTransactionType(PassphraseCredentialTransaction::TYPE_CONDUIT) ->setNewValue(!$credential->getAllowConduit()); diff --git a/src/applications/passphrase/controller/PassphraseCredentialEditController.php b/src/applications/passphrase/controller/PassphraseCredentialEditController.php index a814b5dc72..bdb1802880 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialEditController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialEditController.php @@ -270,8 +270,7 @@ final class PassphraseCredentialEditController extends PassphraseController { } if ($type->shouldRequireUsername()) { - $form - ->appendChild( + $form->appendChild( id(new AphrontFormTextControl()) ->setName('username') ->setLabel(pht('Login/Username')) @@ -279,13 +278,13 @@ final class PassphraseCredentialEditController extends PassphraseController { ->setDisabled($credential_is_locked) ->setError($e_username)); } - $form - ->appendChild( - $secret_control - ->setName('secret') - ->setLabel($type->getSecretLabel()) - ->setDisabled($credential_is_locked) - ->setValue($v_secret)); + + $form->appendChild( + $secret_control + ->setName('secret') + ->setLabel($type->getSecretLabel()) + ->setDisabled($credential_is_locked) + ->setValue($v_secret)); if ($type->shouldShowPasswordField()) { $form->appendChild( diff --git a/src/applications/passphrase/controller/PassphraseCredentialLockController.php b/src/applications/passphrase/controller/PassphraseCredentialLockController.php index 4a872d8667..9832705427 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialLockController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialLockController.php @@ -32,15 +32,17 @@ final class PassphraseCredentialLockController return $this->newDialog() ->setTitle(pht('Credential Already Locked')) ->appendChild( - pht( - 'This credential has been locked and the secret is '. - 'hidden forever. Anything relying on this credential will '. - 'still function. This operation can not be undone.')) + pht('This credential is already locked.')) ->addCancelButton($view_uri, pht('Close')); } if ($request->isFormPost()) { $xactions = array(); + + $xactions[] = id(new PassphraseCredentialTransaction()) + ->setTransactionType(PassphraseCredentialTransaction::TYPE_CONDUIT) + ->setNewValue(0); + $xactions[] = id(new PassphraseCredentialTransaction()) ->setTransactionType(PassphraseCredentialTransaction::TYPE_LOCK) ->setNewValue(1); @@ -48,6 +50,7 @@ final class PassphraseCredentialLockController $editor = id(new PassphraseCredentialTransactionEditor()) ->setActor($viewer) ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) ->setContentSourceFromRequest($request) ->applyTransactions($credential, $xactions); @@ -55,12 +58,13 @@ final class PassphraseCredentialLockController } return $this->newDialog() - ->setTitle(pht('Really lock credential?')) + ->setTitle(pht('Lock Credential')) ->appendChild( pht( - 'This credential will be locked and the secret will be '. - 'hidden forever. Anything relying on this credential will '. - 'still function. This operation can not be undone.')) + 'This credential will be locked and the secret will be hidden '. + 'forever. If Conduit access is enabled, it will be revoked. '. + 'Anything relying on this credential will still function. This '. + 'operation can not be undone.')) ->addSubmitButton(pht('Lock Credential')) ->addCancelButton($view_uri); } diff --git a/src/applications/passphrase/controller/PassphraseCredentialViewController.php b/src/applications/passphrase/controller/PassphraseCredentialViewController.php index 1bbea88ec6..aabb3821e0 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialViewController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialViewController.php @@ -119,6 +119,8 @@ final class PassphraseCredentialViewController extends PassphraseController { $credential, PhabricatorPolicyCapability::CAN_EDIT); + $can_conduit = ($can_edit && !$is_locked); + $curtain = $this->newCurtainView($credential); $curtain->addAction( @@ -161,7 +163,7 @@ final class PassphraseCredentialViewController extends PassphraseController { ->setName($credential_conduit_text) ->setIcon($credential_conduit_icon) ->setHref($this->getApplicationURI("conduit/{$id}/")) - ->setDisabled(!$can_edit) + ->setDisabled(!$can_conduit) ->setWorkflow(true)); $curtain->addAction( @@ -202,16 +204,6 @@ final class PassphraseCredentialViewController extends PassphraseController { $credential->getUsername()); } - $used_by_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( - $credential->getPHID(), - PhabricatorCredentialsUsedByObjectEdgeType::EDGECONST); - - if ($used_by_phids) { - $properties->addProperty( - pht('Used By'), - $viewer->renderHandleList($used_by_phids)); - } - $description = $credential->getDescription(); if (strlen($description)) { $properties->addSectionHeader( diff --git a/src/applications/passphrase/edge/PhabricatorCredentialsUsedByObjectEdgeType.php b/src/applications/passphrase/edge/PhabricatorCredentialsUsedByObjectEdgeType.php deleted file mode 100644 index fc08f844db..0000000000 --- a/src/applications/passphrase/edge/PhabricatorCredentialsUsedByObjectEdgeType.php +++ /dev/null @@ -1,16 +0,0 @@ -delete(); } - $keys = id(new PhabricatorAuthSSHKey())->loadAllWhere( - 'objectPHID = %s', - $this->getPHID()); + $keys = id(new PhabricatorAuthSSHKeyQuery()) + ->setViewer($engine->getViewer()) + ->withObjectPHIDs(array($this->getPHID())) + ->execute(); foreach ($keys as $key) { - $key->delete(); + $engine->destroyObject($key); } $emails = id(new PhabricatorUserEmail())->loadAllWhere( @@ -1341,6 +1342,12 @@ final class PhabricatorUser return 'id_rsa_phabricator'; } + public function getSSHKeyNotifyPHIDs() { + return array( + $this->getPHID(), + ); + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index 2b626f2e5d..f3c04d2a25 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -29,6 +29,7 @@ final class PhabricatorObjectHandle private $policyFiltered; private $subtitle; private $tokenIcon; + private $commandLineObjectName; public function setIcon($icon) { $this->icon = $icon; @@ -196,6 +197,19 @@ final class PhabricatorObjectHandle return $this->getName(); } + public function setCommandLineObjectName($command_line_object_name) { + $this->commandLineObjectName = $command_line_object_name; + return $this; + } + + public function getCommandLineObjectName() { + if ($this->commandLineObjectName !== null) { + return $this->commandLineObjectName; + } + + return $this->getObjectName(); + } + public function setTitle($title) { $this->title = $title; return $this; diff --git a/src/applications/phid/query/PhabricatorObjectListQuery.php b/src/applications/phid/query/PhabricatorObjectListQuery.php index c4bf0e9bba..5526ad4dff 100644 --- a/src/applications/phid/query/PhabricatorObjectListQuery.php +++ b/src/applications/phid/query/PhabricatorObjectListQuery.php @@ -6,6 +6,7 @@ final class PhabricatorObjectListQuery extends Phobject { private $objectList; private $allowedTypes = array(); private $allowPartialResults; + private $suffixes = array(); public function setAllowPartialResults($allow_partial_results) { $this->allowPartialResults = $allow_partial_results; @@ -16,6 +17,15 @@ final class PhabricatorObjectListQuery extends Phobject { return $this->allowPartialResults; } + public function setSuffixes(array $suffixes) { + $this->suffixes = $suffixes; + return $this; + } + + public function getSuffixes() { + return $this->suffixes; + } + public function setAllowedTypes(array $allowed_types) { $this->allowedTypes = $allowed_types; return $this; @@ -45,9 +55,80 @@ final class PhabricatorObjectListQuery extends Phobject { public function execute() { $names = $this->getObjectList(); - $names = array_unique(array_filter(preg_split('/[\s,]+/', $names))); - $objects = $this->loadObjects($names); + // First, normalize any internal whitespace so we don't get weird results + // if linebreaks hit in weird spots. + $names = preg_replace('/\s+/', ' ', $names); + + // Split the list on commas. + $names = explode(',', $names); + + // Trim and remove empty tokens. + foreach ($names as $key => $name) { + $name = trim($name); + + if (!strlen($name)) { + unset($names[$key]); + continue; + } + + $names[$key] = $name; + } + + // Remove duplicates. + $names = array_unique($names); + + $name_map = array(); + foreach ($names as $name) { + $parts = explode(' ', $name); + + // If this looks like a monogram, ignore anything after the first token. + // This allows us to parse "O123 Package Name" as though it was "O123", + // which we can look up. + if (preg_match('/^[A-Z]\d+\z/', $parts[0])) { + $name_map[$parts[0]] = $name; + } else { + // For anything else, split it on spaces and use each token as a + // value. This means "alincoln htaft", separated with a space instead + // of with a comma, is two different users. + foreach ($parts as $part) { + $name_map[$part] = $part; + } + } + } + + // If we're parsing with suffixes, strip them off any tokens and keep + // track of them for later. + $suffixes = $this->getSuffixes(); + if ($suffixes) { + $suffixes = array_fuse($suffixes); + $suffix_map = array(); + $stripped_map = array(); + foreach ($name_map as $key => $name) { + $found_suffixes = array(); + do { + $has_any_suffix = false; + foreach ($suffixes as $suffix) { + if (!$this->hasSuffix($name, $suffix)) { + continue; + } + + $key = $this->stripSuffix($key, $suffix); + $name = $this->stripSuffix($name, $suffix); + + $found_suffixes[] = $suffix; + $has_any_suffix = true; + break; + } + } while ($has_any_suffix); + + $stripped_map[$key] = $name; + $suffix_map[$key] = array_fuse($found_suffixes); + } + $name_map = $stripped_map; + } + + $objects = $this->loadObjects(array_keys($name_map)); $types = array(); foreach ($objects as $name => $object) { @@ -66,8 +147,8 @@ final class PhabricatorObjectListQuery extends Phobject { $invalid = array_mergev($invalid); $missing = array(); - foreach ($names as $name) { - if (empty($objects[$name])) { + foreach ($name_map as $key => $name) { + if (empty($objects[$key])) { $missing[] = $name; } } @@ -100,7 +181,18 @@ final class PhabricatorObjectListQuery extends Phobject { } } - return array_values(array_unique(mpull($objects, 'getPHID'))); + $result = array_unique(mpull($objects, 'getPHID')); + + if ($suffixes) { + foreach ($result as $key => $phid) { + $result[$key] = array( + 'phid' => $phid, + 'suffixes' => idx($suffix_map, $key, array()), + ); + } + } + + return array_values($result); } private function loadObjects($names) { @@ -146,5 +238,16 @@ final class PhabricatorObjectListQuery extends Phobject { return $results; } + private function hasSuffix($key, $suffix) { + return (substr($key, -strlen($suffix)) === $suffix); + } + + private function stripSuffix($key, $suffix) { + if ($this->hasSuffix($key, $suffix)) { + return substr($key, 0, -strlen($suffix)); + } + + return $key; + } } diff --git a/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php b/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php index e7347d5dbc..985690ecd9 100644 --- a/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php +++ b/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php @@ -13,7 +13,6 @@ final class PhabricatorObjectListQueryTestCase extends PhabricatorTestCase { $name = $user->getUsername(); $phid = $user->getPHID(); - $result = $this->parseObjectList("@{$name}"); $this->assertEqual(array($phid), $result); @@ -29,6 +28,47 @@ final class PhabricatorObjectListQueryTestCase extends PhabricatorTestCase { $result = $this->parseObjectList(''); $this->assertEqual(array(), $result); + $result = $this->parseObjectList("{$name}!", array(), false, array('!')); + $this->assertEqual( + array( + array( + 'phid' => $phid, + 'suffixes' => array('!' => '!'), + ), + ), + $result); + + $package = PhabricatorOwnersPackage::initializeNewPackage($user) + ->setName(pht('Query Test Package')) + ->save(); + + $package_phid = $package->getPHID(); + $package_mono = $package->getMonogram(); + + $result = $this->parseObjectList("{$package_mono} Any Ignored Text"); + $this->assertEqual(array($package_phid), $result); + + $result = $this->parseObjectList("{$package_mono} Any Text, {$name}"); + $this->assertEqual(array($package_phid, $phid), $result); + + $result = $this->parseObjectList( + "{$package_mono} Any Text!, {$name}", + array(), + false, + array('!')); + $this->assertEqual( + array( + array( + 'phid' => $package_phid, + 'suffixes' => array('!' => '!'), + ), + array( + 'phid' => $phid, + 'suffixes' => array(), + ), + ), + $result); + // Expect failure when loading a user if objects must be of type "DUCK". $caught = null; try { @@ -67,7 +107,8 @@ final class PhabricatorObjectListQueryTestCase extends PhabricatorTestCase { private function parseObjectList( $list, array $types = array(), - $allow_partial = false) { + $allow_partial = false, + $suffixes = array()) { $query = id(new PhabricatorObjectListQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) @@ -81,6 +122,10 @@ final class PhabricatorObjectListQueryTestCase extends PhabricatorTestCase { $query->setAllowPartialResults(true); } + if ($suffixes) { + $query->setSuffixes($suffixes); + } + return $query->execute(); } diff --git a/src/applications/pholio/view/PholioUploadedImageView.php b/src/applications/pholio/view/PholioUploadedImageView.php index 2ff3ba0390..e105f4a95f 100644 --- a/src/applications/pholio/view/PholioUploadedImageView.php +++ b/src/applications/pholio/view/PholioUploadedImageView.php @@ -42,12 +42,19 @@ final class PholioUploadedImageView extends AphrontView { PhabricatorFileThumbnailTransform::TRANSFORM_PINBOARD); $thumbnail_uri = $file->getURIForTransform($xform); + $thumb_img = phutil_tag( + 'img', + array( + 'class' => 'pholio-thumb-img', + 'src' => $thumbnail_uri, + )); + $thumb_frame = phutil_tag( 'div', array( 'class' => 'pholio-thumb-frame', - 'style' => 'background-image: url('.$thumbnail_uri.');', - )); + ), + $thumb_img); $handle = javelin_tag( 'div', diff --git a/src/applications/project/view/ProjectBoardTaskCard.php b/src/applications/project/view/ProjectBoardTaskCard.php index 7be0af2c2f..a7ee93dd76 100644 --- a/src/applications/project/view/ProjectBoardTaskCard.php +++ b/src/applications/project/view/ProjectBoardTaskCard.php @@ -100,9 +100,10 @@ final class ProjectBoardTaskCard extends Phobject { if ($points !== null) { $points_tag = id(new PHUITagView()) ->setType(PHUITagView::TYPE_SHADE) - ->setShade(PHUITagView::COLOR_BLUE) + ->setShade(PHUITagView::COLOR_GREY) ->setSlimShady(true) - ->setName($points); + ->setName($points) + ->addClass('phui-workcard-points'); $card->addAttribute($points_tag); } } diff --git a/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php b/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php index d9bae65931..f0a17718aa 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php +++ b/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php @@ -49,6 +49,16 @@ final class PhabricatorRepositoryAuditRequest return $this->assertAttached($this->commit); } + public function isInteresting() { + switch ($this->getAuditStatus()) { + case PhabricatorAuditStatusConstants::NONE: + case PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED: + return false; + } + + return true; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php index 61a615950d..3f41226b9f 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php @@ -33,108 +33,132 @@ final class PhabricatorRepositoryCommitOwnersWorker $repository, $commit, PhabricatorUser::getOmnipotentUser()); + $affected_packages = PhabricatorOwnersPackage::loadAffectedPackages( $repository, $affected_paths); - if ($affected_packages) { - $requests = id(new PhabricatorRepositoryAuditRequest()) - ->loadAllWhere( - 'commitPHID = %s', - $commit->getPHID()); - $requests = mpull($requests, null, 'getAuditorPHID'); - - foreach ($affected_packages as $package) { - $request = idx($requests, $package->getPHID()); - if ($request) { - // Don't update request if it exists already. - continue; - } - - if ($package->isArchived()) { - // Don't trigger audits if the package is archived. - continue; - } - - if ($package->getAuditingEnabled()) { - $reasons = $this->checkAuditReasons($commit, $package); - if ($reasons) { - $audit_status = - PhabricatorAuditStatusConstants::AUDIT_REQUIRED; - } else { - $audit_status = - PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED; - } - } else { - $reasons = array(); - $audit_status = PhabricatorAuditStatusConstants::NONE; - } - - $relationship = new PhabricatorRepositoryAuditRequest(); - $relationship->setAuditorPHID($package->getPHID()); - $relationship->setCommitPHID($commit->getPHID()); - $relationship->setAuditReasons($reasons); - $relationship->setAuditStatus($audit_status); - - $relationship->save(); - - $requests[$package->getPHID()] = $relationship; - } - - $commit->updateAuditStatus($requests); - $commit->save(); + if (!$affected_packages) { + return; } - } - - private function checkAuditReasons( - PhabricatorRepositoryCommit $commit, - PhabricatorOwnersPackage $package) { $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere( 'commitID = %d', $commit->getID()); + $commit->attachCommitData($data); - $reasons = array(); - - if ($data->getCommitDetail('vsDiff')) { - $reasons[] = pht('Changed After Revision Was Accepted'); - } - - $commit_author_phid = $data->getCommitDetail('authorPHID'); - if (!$commit_author_phid) { - $reasons[] = pht('Commit Author Not Recognized'); - } - + $author_phid = $data->getCommitDetail('authorPHID'); $revision_id = $data->getCommitDetail('differential.revisionID'); - - $revision_author_phid = null; - $commit_reviewedby_phid = null; - if ($revision_id) { $revision = id(new DifferentialRevisionQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withIDs(array($revision_id)) + ->needReviewerStatus(true) ->executeOne(); - if ($revision) { - $revision_author_phid = $revision->getAuthorPHID(); - $commit_reviewedby_phid = $data->getCommitDetail('reviewerPHID'); - if ($revision_author_phid !== $commit_author_phid) { - $reasons[] = pht('Author Not Matching with Revision'); - } - } else { - $reasons[] = pht('Revision Not Found'); - } - } else { - $reasons[] = pht('No Revision Specified'); + $revision = null; } - $owners_phids = PhabricatorOwnersOwner::loadAffiliatedUserPHIDs( - array($package->getID())); + $requests = id(new PhabricatorRepositoryAuditRequest()) + ->loadAllWhere( + 'commitPHID = %s', + $commit->getPHID()); + $requests = mpull($requests, null, 'getAuditorPHID'); - if (!($commit_author_phid && in_array($commit_author_phid, $owners_phids) || - $commit_reviewedby_phid && in_array($commit_reviewedby_phid, - $owners_phids))) { + + foreach ($affected_packages as $package) { + $request = idx($requests, $package->getPHID()); + if ($request) { + // Don't update request if it exists already. + continue; + } + + if ($package->isArchived()) { + // Don't trigger audits if the package is archived. + continue; + } + + if ($package->getAuditingEnabled()) { + $reasons = $this->checkAuditReasons( + $commit, + $package, + $author_phid, + $revision); + + if ($reasons) { + $audit_status = PhabricatorAuditStatusConstants::AUDIT_REQUIRED; + } else { + $audit_status = PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED; + } + } else { + $reasons = array(); + $audit_status = PhabricatorAuditStatusConstants::NONE; + } + + $relationship = new PhabricatorRepositoryAuditRequest(); + $relationship->setAuditorPHID($package->getPHID()); + $relationship->setCommitPHID($commit->getPHID()); + $relationship->setAuditReasons($reasons); + $relationship->setAuditStatus($audit_status); + + $relationship->save(); + + $requests[$package->getPHID()] = $relationship; + } + + $commit->updateAuditStatus($requests); + $commit->save(); + } + + private function checkAuditReasons( + PhabricatorRepositoryCommit $commit, + PhabricatorOwnersPackage $package, + $author_phid, + $revision) { + + $owner_phids = PhabricatorOwnersOwner::loadAffiliatedUserPHIDs( + array( + $package->getID(), + )); + $owner_phids = array_fuse($owner_phids); + + $reasons = array(); + + if (!$author_phid) { + $reasons[] = pht('Commit Author Not Recognized'); + } else if (isset($owner_phids[$author_phid])) { + return $reasons; + } + + if (!$revision) { + $reasons[] = pht('No Revision Specified'); + return $reasons; + } + + $accepted_statuses = array( + DifferentialReviewerStatus::STATUS_ACCEPTED, + DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER, + ); + $accepted_statuses = array_fuse($accepted_statuses); + + $found_accept = false; + foreach ($revision->getReviewerStatus() as $reviewer) { + $reviewer_phid = $reviewer->getReviewerPHID(); + + // If this reviewer isn't a package owner, just ignore them. + if (empty($owner_phids[$reviewer_phid])) { + continue; + } + + // If this reviewer accepted the revision and owns the package, we're + // all clear and do not need to trigger an audit. + if (isset($accepted_statuses[$reviewer->getStatus()])) { + $found_accept = true; + break; + } + } + + if (!$found_accept) { $reasons[] = pht('Owners Not Involved'); } diff --git a/src/applications/search/buckets/PhabricatorSearchResultBucket.php b/src/applications/search/buckets/PhabricatorSearchResultBucket.php new file mode 100644 index 0000000000..16e5a6150f --- /dev/null +++ b/src/applications/search/buckets/PhabricatorSearchResultBucket.php @@ -0,0 +1,54 @@ +pageSize = $page_size; + return $this; + } + + final public function getPageSize() { + if ($this->pageSize === null) { + return $this->getDefaultPageSize(); + } + + return $this->pageSize; + } + + public function setViewer(PhabricatorUser $viewer) { + $this->viewer = $viewer; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + + protected function getDefaultPageSize() { + return 1000; + } + + abstract public function getResultBucketName(); + abstract protected function buildResultGroups( + PhabricatorSavedQuery $query, + array $objects); + + final public function newResultGroups( + PhabricatorSavedQuery $query, + array $objects) { + return $this->buildResultGroups($query, $objects); + } + + final public function getResultBucketKey() { + return $this->getPhobjectClassConstant('BUCKETKEY'); + } + + final protected function newGroup() { + return new PhabricatorSearchResultBucketGroup(); + } + +} diff --git a/src/applications/search/buckets/PhabricatorSearchResultBucketGroup.php b/src/applications/search/buckets/PhabricatorSearchResultBucketGroup.php new file mode 100644 index 0000000000..2fcdad13e4 --- /dev/null +++ b/src/applications/search/buckets/PhabricatorSearchResultBucketGroup.php @@ -0,0 +1,47 @@ +noDataString = $no_data_string; + return $this; + } + + public function getNoDataString() { + return $this->noDataString; + } + + public function setName($name) { + $this->name = $name; + return $this; + } + + public function getName() { + return $this->name; + } + + public function setKey($key) { + $this->key = $key; + return $this; + } + + public function getKey() { + return $this->key; + } + + public function setObjects(array $objects) { + $this->objects = $objects; + return $this; + } + + public function getObjects() { + return $this->objects; + } + +} diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php index 3b30d5d30c..688a7db527 100644 --- a/src/applications/search/controller/PhabricatorApplicationSearchController.php +++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php @@ -213,6 +213,8 @@ final class PhabricatorApplicationSearchController if ($run_query) { + $exec_errors = array(); + $box->setAnchor( id(new PhabricatorAnchorView()) ->setAnchorName('R')); @@ -280,10 +282,18 @@ final class PhabricatorApplicationSearchController } } } catch (PhabricatorTypeaheadInvalidTokenException $ex) { - $errors[] = pht( + $exec_errors[] = pht( 'This query specifies an invalid parameter. Review the '. 'query parameters and correct errors.'); } + + // The engine may have encountered additional errors during rendering; + // merge them in and show everything. + foreach ($engine->getErrors() as $error) { + $exec_errors[] = $error; + } + + $errors = $exec_errors; } if ($errors) { diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index 702af44c40..96363ed947 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -28,6 +28,8 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { const CONTEXT_LIST = 'list'; const CONTEXT_PANEL = 'panel'; + const BUCKET_NONE = 'none'; + public function setController(PhabricatorController $controller) { $this->controller = $controller; return $this; @@ -96,8 +98,6 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { return $this->navigationItems; } - - public function canUseInPanelContext() { return true; } @@ -141,8 +141,8 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { * @param PhabricatorSavedQuery The saved query to operate on. * @return The result of the query. */ - public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { - $saved = clone $saved; + public function buildQueryFromSavedQuery(PhabricatorSavedQuery $original) { + $saved = clone $original; $this->willUseSavedQuery($saved); $fields = $this->buildSearchFields(); @@ -156,6 +156,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { $map[$field->getKey()] = $value; } + $original->attachParameterMap($map); $query = $this->buildQueryFromParameters($map); $object = $this->newResultObject(); @@ -266,6 +267,18 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { ->setOptions($orders); } + $buckets = $this->newResultBuckets(); + if ($query && $buckets) { + $bucket_options = array( + self::BUCKET_NONE => pht('No Bucketing'), + ) + mpull($buckets, 'getResultBucketName'); + + $fields[] = id(new PhabricatorSearchSelectField()) + ->setLabel(pht('Bucket')) + ->setKey('bucket') + ->setOptions($bucket_options); + } + $field_map = array(); foreach ($fields as $field) { $key = $field->getKey(); @@ -944,13 +957,37 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { /* -( Paging and Executing Queries )--------------------------------------- */ + protected function newResultBuckets() { + return array(); + } + + protected function getResultBucket(PhabricatorSavedQuery $saved) { + $key = $saved->getParameter('bucket'); + if ($key == self::BUCKET_NONE) { + return null; + } + + $buckets = $this->newResultBuckets(); + return idx($buckets, $key); + } + + public function getPageSize(PhabricatorSavedQuery $saved) { + $bucket = $this->getResultBucket($saved); + $limit = (int)$saved->getParameter('limit'); if ($limit > 0) { + if ($bucket) { + $bucket->setPageSize($limit); + } return $limit; } + if ($bucket) { + return $bucket->getPageSize(); + } + return 100; } diff --git a/src/applications/search/storage/PhabricatorSavedQuery.php b/src/applications/search/storage/PhabricatorSavedQuery.php index 71d9d9ccd2..8d3435f8b4 100644 --- a/src/applications/search/storage/PhabricatorSavedQuery.php +++ b/src/applications/search/storage/PhabricatorSavedQuery.php @@ -7,6 +7,8 @@ final class PhabricatorSavedQuery extends PhabricatorSearchDAO protected $queryKey; protected $engineClassName; + private $parameterMap = self::ATTACHABLE; + protected function getConfiguration() { return array( self::CONFIG_SERIALIZATION => array( @@ -52,6 +54,15 @@ final class PhabricatorSavedQuery extends PhabricatorSearchDAO return newv($this->getEngineClassName(), array()); } + public function attachParameterMap(array $map) { + $this->parameterMap = $map; + return $this; + } + + public function getEvaluatedParameter($key, $default = null) { + return $this->assertAttachedKey($this->parameterMap, $key, $default); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php b/src/applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php index 0d42149441..3396774065 100644 --- a/src/applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php @@ -76,7 +76,7 @@ final class PhabricatorEmailFormatSettingsPanel ? pht('Vary') : pht('Do Not Vary'); - $html_emails_default = pht('Plain Text'); + $html_emails_default = pht('HTML'); $re_prefix_value = $preferences->getPreference($pref_re_prefix); if ($re_prefix_value === null) { diff --git a/src/applications/settings/panel/PhabricatorSSHKeysSettingsPanel.php b/src/applications/settings/panel/PhabricatorSSHKeysSettingsPanel.php index 0faf620041..4c716e1dff 100644 --- a/src/applications/settings/panel/PhabricatorSSHKeysSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorSSHKeysSettingsPanel.php @@ -33,6 +33,7 @@ final class PhabricatorSSHKeysSettingsPanel extends PhabricatorSettingsPanel { $keys = id(new PhabricatorAuthSSHKeyQuery()) ->setViewer($viewer) ->withObjectPHIDs(array($user->getPHID())) + ->withIsActive(true) ->execute(); $table = id(new PhabricatorAuthSSHKeyTableView()) @@ -44,31 +45,12 @@ final class PhabricatorSSHKeysSettingsPanel extends PhabricatorSettingsPanel { $panel = new PHUIObjectBoxView(); $header = new PHUIHeaderView(); - $upload_button = id(new PHUIButtonView()) - ->setText(pht('Upload Public Key')) - ->setHref('/auth/sshkey/upload/?objectPHID='.$user->getPHID()) - ->setWorkflow(true) - ->setTag('a') - ->setIcon('fa-upload'); - - try { - PhabricatorSSHKeyGenerator::assertCanGenerateKeypair(); - $can_generate = true; - } catch (Exception $ex) { - $can_generate = false; - } - - $generate_button = id(new PHUIButtonView()) - ->setText(pht('Generate Keypair')) - ->setHref('/auth/sshkey/generate/?objectPHID='.$user->getPHID()) - ->setTag('a') - ->setWorkflow(true) - ->setDisabled(!$can_generate) - ->setIcon('fa-lock'); + $ssh_actions = PhabricatorAuthSSHKeyTableView::newKeyActionsMenu( + $viewer, + $user); $header->setHeader(pht('SSH Public Keys')); - $header->addActionLink($generate_button); - $header->addActionLink($upload_button); + $header->addActionLink($ssh_actions); $panel->setHeader($header); $panel->setTable($table); diff --git a/src/applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php b/src/applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php index baaf4f7af9..4e9d745540 100644 --- a/src/applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php +++ b/src/applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php @@ -108,9 +108,9 @@ EOBANNER; 'deleting complex or highly connected objects like repositories, '. 'projects and users.'. "\n\n". - 'These tattered edges are an expected consquence of destroying '. + 'These tattered edges are an expected consequence of destroying '. 'objects, and the Phabricator upstream will not help you fix '. - 'them. We strongly recomend disabling or archiving objects '. + 'them. We strongly recommend disabling or archiving objects '. 'instead.'))); $phids = mpull($named_objects, 'getPHID'); diff --git a/src/applications/transactions/edges/PhabricatorObjectUsesCredentialsEdgeType.php b/src/applications/transactions/edges/PhabricatorObjectUsesCredentialsEdgeType.php deleted file mode 100644 index abfe4f16fe..0000000000 --- a/src/applications/transactions/edges/PhabricatorObjectUsesCredentialsEdgeType.php +++ /dev/null @@ -1,16 +0,0 @@ -setEditPolicy($this->getActingAsPHID()); } - $xaction->setAuthorPHID($this->getActingAsPHID()); + // If the transaction already has an explicit author PHID, allow it to + // stand. This is used by applications like Owners that hook into the + // post-apply change pipeline. + if (!$xaction->getAuthorPHID()) { + $xaction->setAuthorPHID($this->getActingAsPHID()); + } + $xaction->setContentSource($this->getContentSource()); $xaction->attachViewer($actor); $xaction->attachObject($object); @@ -957,6 +963,12 @@ abstract class PhabricatorApplicationTransactionEditor if ($herald_xactions) { $xscript_id = $this->getHeraldTranscript()->getID(); foreach ($herald_xactions as $herald_xaction) { + // Don't set a transcript ID if this is a transaction from another + // application or source, like Owners. + if ($herald_xaction->getAuthorPHID()) { + continue; + } + $herald_xaction->setMetadataValue('herald:transcriptID', $xscript_id); } @@ -1217,6 +1229,7 @@ abstract class PhabricatorApplicationTransactionEditor $xaction, pht('You can not apply transactions which already have IDs/PHIDs!')); } + if ($xaction->getObjectPHID()) { throw new PhabricatorApplicationTransactionStructureException( $xaction, @@ -1224,13 +1237,7 @@ abstract class PhabricatorApplicationTransactionEditor 'You can not apply transactions which already have %s!', 'objectPHIDs')); } - if ($xaction->getAuthorPHID()) { - throw new PhabricatorApplicationTransactionStructureException( - $xaction, - pht( - 'You can not apply transactions which already have %s!', - 'authorPHIDs')); - } + if ($xaction->getCommentPHID()) { throw new PhabricatorApplicationTransactionStructureException( $xaction, @@ -1238,6 +1245,7 @@ abstract class PhabricatorApplicationTransactionEditor 'You can not apply transactions which already have %s!', 'commentPHIDs')); } + if ($xaction->getCommentVersion() !== 0) { throw new PhabricatorApplicationTransactionStructureException( $xaction, @@ -1569,6 +1577,14 @@ abstract class PhabricatorApplicationTransactionEditor $type = $xaction->getTransactionType(); if (isset($types[$type])) { foreach ($types[$type] as $other_key) { + $other_xaction = $result[$other_key]; + + // Don't merge transactions with different authors. For example, + // don't merge Herald transactions and owners transactions. + if ($other_xaction->getAuthorPHID() != $xaction->getAuthorPHID()) { + continue; + } + $merged = $this->mergeTransactions($result[$other_key], $xaction); if ($merged) { $result[$other_key] = $merged; @@ -2681,7 +2697,9 @@ abstract class PhabricatorApplicationTransactionEditor */ protected function addHeadersAndCommentsToMailBody( PhabricatorMetaMTAMailBody $body, - array $xactions) { + array $xactions, + $object_label = null, + $object_href = null) { $headers = array(); $comments = array(); @@ -2701,7 +2719,58 @@ abstract class PhabricatorApplicationTransactionEditor $comments[] = $comment; } } - $body->addRawSection(implode("\n", $headers)); + + $headers_text = implode("\n", $headers); + $body->addRawPlaintextSection($headers_text); + + $headers_html = phutil_implode_html(phutil_tag('br'), $headers); + + $header_button = null; + if ($object_label !== null) { + $button_style = array( + 'text-decoration: none;', + 'padding: 4px 8px;', + 'margin: 0 8px 8px;', + 'float: right;', + 'color: #464C5C;', + 'font-weight: bold;', + 'border-radius: 3px;', + 'background-color: #F7F7F9;', + 'background-image: linear-gradient(to bottom,#fff,#f1f0f1);', + 'display: inline-block;', + 'border: 1px solid rgba(71,87,120,.2);', + ); + + $header_button = phutil_tag( + 'a', + array( + 'style' => implode(' ', $button_style), + 'href' => $object_href, + ), + $object_label); + } + + $xactions_style = array( + ); + + $header_action = phutil_tag( + 'td', + array(), + $header_button); + + $header_action = phutil_tag( + 'td', + array( + 'style' => implode(' ', $xactions_style), + ), + $headers_html); + + $headers_html = phutil_tag( + 'table', + array(), + phutil_tag('tr', array(), array($header_action, $header_button))); + + $body->addRawHTMLSection($headers_html); foreach ($comments as $comment) { $body->addRemarkupSection(null, $comment); diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index ab22111647..1e6bb14481 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -613,7 +613,14 @@ abstract class PhabricatorApplicationTransaction case PhabricatorObjectMentionsObjectEdgeType::EDGECONST: case PhabricatorObjectMentionedByObjectEdgeType::EDGECONST: return true; - break; + case PhabricatorProjectObjectHasProjectEdgeType::EDGECONST: + // When an object is first created, we hide any corresponding + // project transactions in the web UI because you can just look at + // the UI element elsewhere on screen to see which projects it + // is tagged with. However, in mail there's no other way to get + // this information, and it has some amount of value to users, so + // we keep the transaction. See T10493. + return false; default: break; } diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php index db9224bf67..5726a4b6dd 100644 --- a/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php @@ -142,6 +142,14 @@ abstract class PhabricatorTypeaheadCompositeDatasource return parent::canEvaluateFunction($function); } + protected function evaluateValues(array $values) { + foreach ($this->getUsableDatasources() as $source) { + $values = $source->evaluateValues($values); + } + + return parent::evaluateValues($values); + } + protected function evaluateFunction($function, array $argv) { foreach ($this->getUsableDatasources() as $source) { if ($source->canEvaluateFunction($function)) { diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php index 00491428af..c1d1e96540 100644 --- a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php @@ -331,6 +331,14 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { } + /** + * @task functions + */ + protected function evaluateValues(array $values) { + return $values; + } + + /** * @task functions */ @@ -345,6 +353,8 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { } } + $results = $this->evaluateValues($results); + foreach ($evaluate as $function) { $function = self::parseFunction($function); if (!$function) { diff --git a/src/docs/user/cluster/cluster.diviner b/src/docs/user/cluster/cluster.diviner index d78c445c4a..29df43ce4c 100644 --- a/src/docs/user/cluster/cluster.diviner +++ b/src/docs/user/cluster/cluster.diviner @@ -69,8 +69,8 @@ and then deploy only Phabricator services into that VPC. If you have additional auxiliary hosts which run builds and tests via Drydock, you should //not// include them in the cluster address definition. For more -detailed discussion of the Drydock security model, see @{Drydock User Guide: -Security}. +detailed discussion of the Drydock security model, see +@{article:Drydock User Guide: Security}. Most other clustering features will not work until you define a cluster by configuring `cluster.addresses`. diff --git a/src/docs/user/cluster/cluster_ssh.diviner b/src/docs/user/cluster/cluster_ssh.diviner index 940ef98b05..bdd41776f5 100644 --- a/src/docs/user/cluster/cluster_ssh.diviner +++ b/src/docs/user/cluster/cluster_ssh.diviner @@ -26,7 +26,7 @@ at any time. First, deploy the Phabricator software and configuration to a host, then register the host as a cluster device if it is not already registered (for -help, see @{article:Cluster: Devices}. +help, see @{article:Cluster: Devices}.) Once the host is registered, start the SSH server, and then add the host to the SSH load balancer pool. @@ -36,7 +36,7 @@ production freely. You may also want to run web services on these hosts, since the service is very similar to SSH, also stateless, and it may be simpler to load balance the -services together. For details, see @{cluster: Web Servers}. +services together. For details, see @{article:Cluster: Web Servers}. Next Steps diff --git a/src/docs/user/cluster/cluster_webservers.diviner b/src/docs/user/cluster/cluster_webservers.diviner index 744696af66..7c7c3e8b1f 100644 --- a/src/docs/user/cluster/cluster_webservers.diviner +++ b/src/docs/user/cluster/cluster_webservers.diviner @@ -24,7 +24,7 @@ at any time. First, deploy the Phabricator software and configuration to a host, then register the host as a cluster device if it is not already registered (for -help, see @{article:Cluster: Devices}. +help, see @{article:Cluster: Devices}.) Once the host is registered, start the web server, and then add the host to the load balancer pool. @@ -34,7 +34,7 @@ production freely. You may also want to run SSH services on these hosts, since the service is very similar to HTTP, also stateless, and it may be simpler to load balance the -services together. For details, see @{cluster:SSH Servers}. +services together. For details, see @{article:Cluster: SSH Servers}. Next Steps diff --git a/src/docs/user/configuration/configuring_file_storage.diviner b/src/docs/user/configuration/configuring_file_storage.diviner index 120c44f61e..8ea60080ac 100644 --- a/src/docs/user/configuration/configuring_file_storage.diviner +++ b/src/docs/user/configuration/configuring_file_storage.diviner @@ -164,6 +164,8 @@ To enable file storage in S3, set these keys: - `amazon-s3.access-key`: Your AWS access key. - `amazon-s3.secret-key`: Your AWS secret key. + - `amazon-s3.region`: Your AWS S3 region. + - `amazon-s3.endpoint`: Your AWS S3 endpoint. - `storage.s3.bucket`: S3 bucket name where files should be stored. Testing Storage Engines diff --git a/src/docs/user/configuration/configuring_inbound_email.diviner b/src/docs/user/configuration/configuring_inbound_email.diviner index 7d57f01e3a..4b20881f58 100644 --- a/src/docs/user/configuration/configuring_inbound_email.diviner +++ b/src/docs/user/configuration/configuring_inbound_email.diviner @@ -128,6 +128,7 @@ like this: - Add a Mailgun route with a `catch_all()` rule which takes the action `forward("https://phabricator.example.com/mail/mailgun/")`. Replace the example domain with your actual domain. + - Set the `mailgun.api-key` config key to your Mailgun API key. = SendGrid Setup = diff --git a/src/docs/user/userguide/owners.diviner b/src/docs/user/userguide/owners.diviner index 9347ca9d42..a9a52bfab8 100644 --- a/src/docs/user/userguide/owners.diviner +++ b/src/docs/user/userguide/owners.diviner @@ -45,6 +45,88 @@ belonging to the package when you look at them in Diffusion, or look at changes which affect them in Diffusion or Differential. +Dominion +======== + +The **Dominion** option allows you to control how ownership cascades when +multiple packages own a path. The dominion rules are: + +**Strong Dominion.** This is the default. In this mode, the package will always +own all files matching its configured paths, even if another package also owns +them. + +For example, if the package owns `a/`, it will always own `a/b/c.z` even if +another package owns `a/b/`. In this case, both packages will own `a/b/c.z`. + +This mode prevents users from stealing files away from the package by defining +more narrow ownership rules in new packages, but enforces hierarchical +ownership rules. + +**Weak Dominion.** In this mode, the package will only own files which do not +match a more specific path in another package. + +For example, if the package owns `a/` but another package owns `a/b/`, the +package will no longer consider `a/b/c.z` to be a file it owns because another +package matches the path with a more specific rule. + +This mode lets you to define rules without implicit hierarchical ownership, +but allows users to steal files away from a package by defining a more +specific package. + +For more details on files which match multiple packages, see +"Files in Multiple Packages", below. + + +Auto Review +=========== + +You can configure **Auto Review** for packages. When a new code review is +created in Differential which affects code in a package, the package can +automatically be added as a subscriber or reviewer. + +The available settings are: + + - **No Autoreview**: This package will not be added to new reviews. + - **Subscribe to Changes**: This package will be added to reviews as a + subscriber. Owners will be notified of changes, but not required to act. + - **Review Changes**: This package will be added to reviews as a reviewer. + Reviews will appear on the dashboards of package owners. + - **Review Changes (Blocking)** This package will be added to reviews + as a blocking reviewer. A package owner will be required to accept changes + before they may land. + +NOTE: These rules **do not trigger** if the change author is a package owner. +They only apply to changes made by users who aren't already owners. + +These rules also do not trigger if the package has been archived. + +The intent of this feature is to make it easy to configure simple, reasonable +behaviors. If you want more tailored or specific triggers, you can write more +powerful rules by using Herald. + + +Auditing +======== + +You can automatically trigger audits on unreviewed code by configuring +**Auditing**. The available settings are: + + - **Disabled**: Do not trigger audits. + - **Enabled**: Trigger audits. + +When enabled, audits are triggered for commits which: + + - affect code owned by the package; + - were not authored by a package owner; and + - were not accepted by a package owner. + +Audits do not trigger if the package has been archived. + +The intent of this feature is to make it easy to configure simple auditing +behavior. If you want more powerful auditing behavior, you can use Herald to +write more sophisticated rules. + + Files in Multiple Packages ========================== @@ -67,4 +149,6 @@ configuration, these files are part of three packages: "iOS Application", "Android Application", and "Design Assets". (You can use an "exclude" rule if you want to make a different package with a -more specific claim the owner of a file or subdirectory.) +more specific claim the owner of a file or subdirectory. You can also change +the **Dominion** setting for a package to let it give up ownership of paths +owned by another package.) diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 85eb5ab437..e97841f3a3 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -60,12 +60,11 @@ vertical-align: top; white-space: pre-wrap; word-wrap: break-word; - padding: 0 8px 1px; - line-height: 16px; + padding: 1px 8px; } .device .differential-diff td { - padding: 0 4px 1px; + padding: 1px 4px; } .device .differential-diff .inline td { @@ -79,7 +78,7 @@ .differential-diff th { text-align: right; - padding: 2px 6px 0px 0px; + padding: 1px 6px 1px 0; vertical-align: top; background: {$lightbluebackground}; color: {$bluetext}; @@ -103,11 +102,11 @@ } .differential-diff td.old { - background: #ffd0d0; + background: rgba(251, 175, 175, 0.3); } .differential-diff td.new { - background: #d0ffd0; + background: rgba(151, 234, 151, .3); } .differential-diff td.old-rebase { @@ -120,12 +119,12 @@ .differential-diff td.old-full, .differential-diff td.old span.bright { - background: #ffaaaa; + background: rgba(251, 175, 175, 0.8); } .differential-diff td.new-full, .differential-diff td.new span.bright { - background: #aaffaa; + background: rgba(151, 234, 151, .8); } .differential-diff td.copy { diff --git a/webroot/rsrc/css/application/pholio/pholio-edit.css b/webroot/rsrc/css/application/pholio/pholio-edit.css index 3a91f0256d..5478a6f10b 100644 --- a/webroot/rsrc/css/application/pholio/pholio-edit.css +++ b/webroot/rsrc/css/application/pholio/pholio-edit.css @@ -17,8 +17,8 @@ .pholio-thumb-box { margin: 2px 0; float: left; - background: #f7f7f7; - border: 1px solid #D5D9DF; + background: {$lightgreybackground}; + border: 1px solid {$lightgreyborder}; border-radius: 3px; width: 296px; overflow: hidden; @@ -38,16 +38,16 @@ margin: 0 auto; } -.pholio-thumb-frame { - background-color: #ffffff; - background-position: center center; - background-repeat: no-repeat; - background-size: 280px 210px; - width: 280px; - height: 210px; +.pholio-thumb-img { + max-width: 280px; + max-height: 210px; padding: 8px; } +.pholio-thumb-frame { + background: url('/rsrc/image/checker_lighter.png'); +} + .device .pholio-thumb-frame { width: 100%; } diff --git a/webroot/rsrc/css/core/syntax.css b/webroot/rsrc/css/core/syntax.css index 35e3d299d4..27006f2e4c 100644 --- a/webroot/rsrc/css/core/syntax.css +++ b/webroot/rsrc/css/core/syntax.css @@ -7,17 +7,12 @@ } .remarkup-code .over-the-line { - color: #aa0066; + color: #aa0066; margin-right: 1px; } -.remarkup-code td > span, -.remarkup-code td > span > span { - padding: 1px 0 3px; -} - -.remarkup-code span.bright { - border-bottom: 1px solid transparent; +.remarkup-code td span { + display: inline-block; } .remarkup-code .rbw_r { color: red; } diff --git a/webroot/rsrc/css/phui/workboards/phui-workcard.css b/webroot/rsrc/css/phui/workboards/phui-workcard.css index f3d92d63e5..64792c6915 100644 --- a/webroot/rsrc/css/phui/workboards/phui-workcard.css +++ b/webroot/rsrc/css/phui/workboards/phui-workcard.css @@ -114,6 +114,19 @@ margin-bottom: 0; } +.phui-workcard .phui-object-item-attribute-spacer { + display: none; +} + +.phui-workcard .phui-workcard-points { + margin: 0 4px 2px 0; + display: inline-block; +} + +.phui-workcard .phui-object-item-attribute { + display: inline; +} + /* - Draggable Colors --------------------------------------------------------*/