diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 2098d76356..65d2b073fe 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,11 +9,11 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'e68cf1fa', 'conpherence.pkg.js' => '15191c65', - 'core.pkg.css' => '16369829', + 'core.pkg.css' => '8ccb0ba3', 'core.pkg.js' => '4c79d74f', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '45951e9e', - 'differential.pkg.js' => 'b71b8c5d', + 'differential.pkg.js' => '500a75c5', 'diffusion.pkg.css' => 'a2d17c7d', 'diffusion.pkg.js' => '6134c5a1', 'favicon.ico' => '4d48ee79', @@ -73,7 +73,7 @@ return array( 'rsrc/css/application/diffusion/diffusion-icons.css' => '0c15255e', 'rsrc/css/application/diffusion/diffusion-readme.css' => '419dd5b6', 'rsrc/css/application/diffusion/diffusion-repository.css' => 'ee6f20ec', - 'rsrc/css/application/diffusion/diffusion-source.css' => '69ac9399', + 'rsrc/css/application/diffusion/diffusion-source.css' => '5f35a3bd', 'rsrc/css/application/diffusion/diffusion.css' => '45727264', 'rsrc/css/application/feed/feed.css' => 'ecd4ec57', 'rsrc/css/application/files/global-drag-and-drop.css' => 'b556a948', @@ -176,7 +176,7 @@ return array( 'rsrc/css/phui/phui-spacing.css' => '042804d6', 'rsrc/css/phui/phui-status.css' => 'd5263e49', 'rsrc/css/phui/phui-tag-view.css' => 'b4719c50', - 'rsrc/css/phui/phui-timeline-view.css' => 'f21db7ca', + 'rsrc/css/phui/phui-timeline-view.css' => 'e2ef62b1', 'rsrc/css/phui/phui-two-column-view.css' => '44ec4951', 'rsrc/css/phui/workboards/phui-workboard-color.css' => '783cdff5', 'rsrc/css/phui/workboards/phui-workboard.css' => '3bc85455', @@ -398,7 +398,7 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', 'rsrc/js/application/diff/DiffChangeset.js' => '99abf4cd', - 'rsrc/js/application/diff/DiffChangesetList.js' => '8f1cd52c', + 'rsrc/js/application/diff/DiffChangesetList.js' => '3b77efdd', 'rsrc/js/application/diff/DiffInline.js' => 'e83d28f3', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07', @@ -446,7 +446,7 @@ return array( 'rsrc/js/application/releeph/releeph-preview-branch.js' => 'b2b4fbaf', 'rsrc/js/application/releeph/releeph-request-state-change.js' => 'a0b57eb8', 'rsrc/js/application/releeph/releeph-request-typeahead.js' => 'de2e896f', - 'rsrc/js/application/repository/repository-crossreference.js' => 'e5339c43', + 'rsrc/js/application/repository/repository-crossreference.js' => '7fe9bc12', 'rsrc/js/application/search/behavior-reorder-profile-menu-items.js' => 'e2e0a072', 'rsrc/js/application/search/behavior-reorder-queries.js' => 'e9581f08', 'rsrc/js/application/slowvote/behavior-slowvote-embed.js' => '887ad43f', @@ -574,7 +574,7 @@ return array( 'diffusion-icons-css' => '0c15255e', 'diffusion-readme-css' => '419dd5b6', 'diffusion-repository-css' => 'ee6f20ec', - 'diffusion-source-css' => '69ac9399', + 'diffusion-source-css' => '5f35a3bd', 'diviner-shared-css' => '896f1d43', 'font-fontawesome' => 'e838e088', 'font-lato' => 'c7ccd872', @@ -692,7 +692,7 @@ return array( 'javelin-behavior-reorder-applications' => '76b9fc3e', 'javelin-behavior-reorder-columns' => 'e1d25dfb', 'javelin-behavior-reorder-profile-menu-items' => 'e2e0a072', - 'javelin-behavior-repository-crossreference' => 'e5339c43', + 'javelin-behavior-repository-crossreference' => '7fe9bc12', 'javelin-behavior-scrollbar' => '834a1173', 'javelin-behavior-search-reorder-queries' => 'e9581f08', 'javelin-behavior-select-content' => 'bf5374ef', @@ -777,7 +777,7 @@ return array( 'phabricator-darkmessage' => 'c48cccdd', 'phabricator-dashboard-css' => 'fe5b1869', 'phabricator-diff-changeset' => '99abf4cd', - 'phabricator-diff-changeset-list' => '8f1cd52c', + 'phabricator-diff-changeset-list' => '3b77efdd', 'phabricator-diff-inline' => 'e83d28f3', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', @@ -874,7 +874,7 @@ return array( 'phui-status-list-view-css' => 'd5263e49', 'phui-tag-view-css' => 'b4719c50', 'phui-theme-css' => '9f261c6b', - 'phui-timeline-view-css' => 'f21db7ca', + 'phui-timeline-view-css' => 'e2ef62b1', 'phui-two-column-view-css' => '44ec4951', 'phui-workboard-color-css' => '783cdff5', 'phui-workboard-view-css' => '3bc85455', @@ -1137,6 +1137,10 @@ return array( 'javelin-dom', 'javelin-magical-init', ), + '3b77efdd' => array( + 'javelin-install', + 'phuix-button-view', + ), '3cb0b2fc' => array( 'javelin-behavior', 'javelin-dom', @@ -1549,6 +1553,12 @@ return array( '7f243deb' => array( 'javelin-install', ), + '7fe9bc12' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-uri', + ), '834a1173' => array( 'javelin-behavior', 'javelin-scrollbar', @@ -1607,10 +1617,6 @@ return array( '8e1baf68' => array( 'phui-button-css', ), - '8f1cd52c' => array( - 'javelin-install', - 'phuix-button-view', - ), '8f29b364' => array( 'javelin-behavior', 'javelin-stratcom', @@ -2076,12 +2082,6 @@ return array( 'javelin-behavior', 'javelin-dom', ), - 'e5339c43' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-uri', - ), 'e5822781' => array( 'javelin-behavior', 'javelin-dom', diff --git a/resources/sql/autopatches/20171026.ferret.01.ponder.doc.sql b/resources/sql/autopatches/20171026.ferret.01.ponder.doc.sql new file mode 100644 index 0000000000..38c86a4134 --- /dev/null +++ b/resources/sql/autopatches/20171026.ferret.01.ponder.doc.sql @@ -0,0 +1,9 @@ +CREATE TABLE {$NAMESPACE}_ponder.ponder_question_fdocument ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + objectPHID VARBINARY(64) NOT NULL, + isClosed BOOL NOT NULL, + authorPHID VARBINARY(64), + ownerPHID VARBINARY(64), + epochCreated INT UNSIGNED NOT NULL, + epochModified INT UNSIGNED NOT NULL +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20171026.ferret.02.ponder.field.sql b/resources/sql/autopatches/20171026.ferret.02.ponder.field.sql new file mode 100644 index 0000000000..871f0d8f5b --- /dev/null +++ b/resources/sql/autopatches/20171026.ferret.02.ponder.field.sql @@ -0,0 +1,8 @@ +CREATE TABLE {$NAMESPACE}_ponder.ponder_question_ffield ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + documentID INT UNSIGNED NOT NULL, + fieldKey VARCHAR(4) NOT NULL COLLATE {$COLLATE_TEXT}, + rawCorpus LONGTEXT NOT NULL COLLATE {$COLLATE_SORT}, + termCorpus LONGTEXT NOT NULL COLLATE {$COLLATE_SORT}, + normalCorpus LONGTEXT NOT NULL COLLATE {$COLLATE_SORT} +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20171026.ferret.03.ponder.ngrams.sql b/resources/sql/autopatches/20171026.ferret.03.ponder.ngrams.sql new file mode 100644 index 0000000000..3d2a3024b8 --- /dev/null +++ b/resources/sql/autopatches/20171026.ferret.03.ponder.ngrams.sql @@ -0,0 +1,5 @@ +CREATE TABLE {$NAMESPACE}_ponder.ponder_question_fngrams ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + documentID INT UNSIGNED NOT NULL, + ngram CHAR(3) NOT NULL COLLATE {$COLLATE_TEXT} +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20171026.ferret.04.ponder.cngrams.sql b/resources/sql/autopatches/20171026.ferret.04.ponder.cngrams.sql new file mode 100644 index 0000000000..49b66e0d39 --- /dev/null +++ b/resources/sql/autopatches/20171026.ferret.04.ponder.cngrams.sql @@ -0,0 +1,7 @@ +CREATE TABLE {$NAMESPACE}_ponder.ponder_question_fngrams_common ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + ngram CHAR(3) NOT NULL COLLATE {$COLLATE_TEXT}, + needsCollection BOOL NOT NULL, + UNIQUE KEY `key_ngram` (ngram), + KEY `key_collect` (needsCollection) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20171026.ferret.05.ponder.index.php b/resources/sql/autopatches/20171026.ferret.05.ponder.index.php new file mode 100644 index 0000000000..20489846d2 --- /dev/null +++ b/resources/sql/autopatches/20171026.ferret.05.ponder.index.php @@ -0,0 +1,11 @@ +getPHID(), + array( + 'force' => true, + )); +} diff --git a/resources/sql/autopatches/20171101.diff.01.active.sql b/resources/sql/autopatches/20171101.diff.01.active.sql new file mode 100644 index 0000000000..aee8c5aa13 --- /dev/null +++ b/resources/sql/autopatches/20171101.diff.01.active.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_differential.differential_revision + ADD activeDiffPHID VARBINARY(64) NOT NULL; diff --git a/resources/sql/autopatches/20171101.diff.02.populate.php b/resources/sql/autopatches/20171101.diff.02.populate.php new file mode 100644 index 0000000000..b41b0aa51d --- /dev/null +++ b/resources/sql/autopatches/20171101.diff.02.populate.php @@ -0,0 +1,24 @@ +establishConnection('w'); +$diff_table = new DifferentialDiff(); + +foreach (new LiskMigrationIterator($table) as $revision) { + $revision_id = $revision->getID(); + + $diff_row = queryfx_one( + $conn, + 'SELECT phid FROM %T WHERE revisionID = %d ORDER BY id DESC LIMIT 1', + $diff_table->getTableName(), + $revision_id); + + if ($diff_row) { + queryfx( + $conn, + 'UPDATE %T SET activeDiffPHID = %s WHERE id = %d', + $table->getTableName(), + $diff_row['phid'], + $revision_id); + } +} diff --git a/scripts/mail/mail_handler.php b/scripts/mail/mail_handler.php index 2ff23adb0f..1c3c71f305 100755 --- a/scripts/mail/mail_handler.php +++ b/scripts/mail/mail_handler.php @@ -35,16 +35,23 @@ $args->parse( $parser = new MimeMailParser(); $parser->setText(file_get_contents('php://stdin')); -$text_body = $parser->getMessageBody('text'); +$content = array(); +foreach (array('text', 'html') as $part) { + $part_body = $parser->getMessageBody($part); -$text_body_headers = $parser->getMessageBodyHeaders('text'); -$content_type = idx($text_body_headers, 'content-type'); -if ( - !phutil_is_utf8($text_body) && - (preg_match('/charset="(.*?)"/', $content_type, $matches) || - preg_match('/charset=(\S+)/', $content_type, $matches)) -) { - $text_body = phutil_utf8_convert($text_body, 'UTF-8', $matches[1]); + if (strlen($part_body) && !phutil_is_utf8($part_body)) { + $part_headers = $parser->getMessageBodyHeaders($part); + if (!is_array($part_headers)) { + $part_headers = array(); + } + $content_type = idx($part_headers, 'content-type'); + if (preg_match('/charset="(.*?)"/', $content_type, $matches) || + preg_match('/charset=(\S+)/', $content_type, $matches)) { + $part_body = phutil_utf8_convert($part_body, 'UTF-8', $matches[1]); + } + } + + $content[$part] = $part_body; } $headers = $parser->getHeaders(); @@ -57,10 +64,7 @@ if ($args->getArg('process-duplicates')) { $received = new PhabricatorMetaMTAReceivedMail(); $received->setHeaders($headers); -$received->setBodies(array( - 'text' => $text_body, - 'html' => $parser->getMessageBody('html'), -)); +$received->setBodies($content); $attachments = array(); foreach ($parser->getAttachments() as $attachment) { diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 117e01157a..2dc5d819c0 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -439,11 +439,14 @@ phutil_register_library_map(array( 'DifferentialDiffQuery' => 'applications/differential/query/DifferentialDiffQuery.php', 'DifferentialDiffRepositoryHeraldField' => 'applications/differential/herald/DifferentialDiffRepositoryHeraldField.php', 'DifferentialDiffRepositoryProjectsHeraldField' => 'applications/differential/herald/DifferentialDiffRepositoryProjectsHeraldField.php', + 'DifferentialDiffSearchConduitAPIMethod' => 'applications/differential/conduit/DifferentialDiffSearchConduitAPIMethod.php', + 'DifferentialDiffSearchEngine' => 'applications/differential/query/DifferentialDiffSearchEngine.php', 'DifferentialDiffTestCase' => 'applications/differential/storage/__tests__/DifferentialDiffTestCase.php', 'DifferentialDiffTransaction' => 'applications/differential/storage/DifferentialDiffTransaction.php', 'DifferentialDiffTransactionQuery' => 'applications/differential/query/DifferentialDiffTransactionQuery.php', 'DifferentialDiffViewController' => 'applications/differential/controller/DifferentialDiffViewController.php', 'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php', + 'DifferentialDraftField' => 'applications/differential/customfield/DifferentialDraftField.php', 'DifferentialExactUserFunctionDatasource' => 'applications/differential/typeahead/DifferentialExactUserFunctionDatasource.php', 'DifferentialFieldParseException' => 'applications/differential/exception/DifferentialFieldParseException.php', 'DifferentialFieldValidationException' => 'applications/differential/exception/DifferentialFieldValidationException.php', @@ -457,6 +460,7 @@ phutil_register_library_map(array( 'DifferentialGetWorkingCopy' => 'applications/differential/DifferentialGetWorkingCopy.php', 'DifferentialGitSVNIDCommitMessageField' => 'applications/differential/field/DifferentialGitSVNIDCommitMessageField.php', 'DifferentialHarbormasterField' => 'applications/differential/customfield/DifferentialHarbormasterField.php', + 'DifferentialHeraldStateReasons' => 'applications/differential/herald/DifferentialHeraldStateReasons.php', 'DifferentialHiddenComment' => 'applications/differential/storage/DifferentialHiddenComment.php', 'DifferentialHostField' => 'applications/differential/customfield/DifferentialHostField.php', 'DifferentialHovercardEngineExtension' => 'applications/differential/engineextension/DifferentialHovercardEngineExtension.php', @@ -546,6 +550,7 @@ phutil_register_library_map(array( 'DifferentialRevisionHasTaskRelationship' => 'applications/differential/relationships/DifferentialRevisionHasTaskRelationship.php', 'DifferentialRevisionHeraldField' => 'applications/differential/herald/DifferentialRevisionHeraldField.php', 'DifferentialRevisionHeraldFieldGroup' => 'applications/differential/herald/DifferentialRevisionHeraldFieldGroup.php', + 'DifferentialRevisionHoldDraftTransaction' => 'applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php', 'DifferentialRevisionIDCommitMessageField' => 'applications/differential/field/DifferentialRevisionIDCommitMessageField.php', 'DifferentialRevisionInlineTransaction' => 'applications/differential/xaction/DifferentialRevisionInlineTransaction.php', 'DifferentialRevisionInlinesController' => 'applications/differential/controller/DifferentialRevisionInlinesController.php', @@ -589,6 +594,7 @@ phutil_register_library_map(array( 'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/DifferentialRevisionUpdateHistoryView.php', 'DifferentialRevisionViewController' => 'applications/differential/controller/DifferentialRevisionViewController.php', 'DifferentialRevisionVoidTransaction' => 'applications/differential/xaction/DifferentialRevisionVoidTransaction.php', + 'DifferentialRevisionWrongStateTransaction' => 'applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php', 'DifferentialSchemaSpec' => 'applications/differential/storage/DifferentialSchemaSpec.php', 'DifferentialSetDiffPropertyConduitAPIMethod' => 'applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php', 'DifferentialStoredCustomField' => 'applications/differential/customfield/DifferentialStoredCustomField.php', @@ -763,6 +769,7 @@ phutil_register_library_map(array( 'DiffusionMercurialBlameQuery' => 'applications/diffusion/query/blame/DiffusionMercurialBlameQuery.php', 'DiffusionMercurialCommandEngine' => 'applications/diffusion/protocol/DiffusionMercurialCommandEngine.php', 'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php', + 'DiffusionMercurialFlagInjectionException' => 'applications/diffusion/exception/DiffusionMercurialFlagInjectionException.php', 'DiffusionMercurialRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php', 'DiffusionMercurialRequest' => 'applications/diffusion/request/DiffusionMercurialRequest.php', 'DiffusionMercurialResponse' => 'applications/diffusion/response/DiffusionMercurialResponse.php', @@ -825,6 +832,7 @@ phutil_register_library_map(array( 'DiffusionQueryCommitsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionQueryCommitsConduitAPIMethod.php', 'DiffusionQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php', 'DiffusionQueryPathsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php', + 'DiffusionQuickSearchEngineExtension' => 'applications/diffusion/engineextension/DiffusionQuickSearchEngineExtension.php', 'DiffusionRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php', 'DiffusionRawDiffQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionRawDiffQueryConduitAPIMethod.php', 'DiffusionReadmeView' => 'applications/diffusion/view/DiffusionReadmeView.php', @@ -1327,6 +1335,7 @@ phutil_register_library_map(array( 'HeraldApplicationActionGroup' => 'applications/herald/action/HeraldApplicationActionGroup.php', 'HeraldApplyTranscript' => 'applications/herald/storage/transcript/HeraldApplyTranscript.php', 'HeraldBasicFieldGroup' => 'applications/herald/field/HeraldBasicFieldGroup.php', + 'HeraldBuildableState' => 'applications/herald/state/HeraldBuildableState.php', 'HeraldCommitAdapter' => 'applications/diffusion/herald/HeraldCommitAdapter.php', 'HeraldCondition' => 'applications/herald/storage/HeraldCondition.php', 'HeraldConditionTranscript' => 'applications/herald/storage/transcript/HeraldConditionTranscript.php', @@ -1350,6 +1359,7 @@ phutil_register_library_map(array( 'HeraldGroup' => 'applications/herald/group/HeraldGroup.php', 'HeraldInvalidActionException' => 'applications/herald/engine/exception/HeraldInvalidActionException.php', 'HeraldInvalidConditionException' => 'applications/herald/engine/exception/HeraldInvalidConditionException.php', + 'HeraldMailableState' => 'applications/herald/state/HeraldMailableState.php', 'HeraldManageGlobalRulesCapability' => 'applications/herald/capability/HeraldManageGlobalRulesCapability.php', 'HeraldManiphestTaskAdapter' => 'applications/maniphest/herald/HeraldManiphestTaskAdapter.php', 'HeraldNewController' => 'applications/herald/controller/HeraldNewController.php', @@ -1387,6 +1397,8 @@ phutil_register_library_map(array( 'HeraldSchemaSpec' => 'applications/herald/storage/HeraldSchemaSpec.php', 'HeraldSelectFieldValue' => 'applications/herald/value/HeraldSelectFieldValue.php', 'HeraldSpaceField' => 'applications/spaces/herald/HeraldSpaceField.php', + 'HeraldState' => 'applications/herald/state/HeraldState.php', + 'HeraldStateReasons' => 'applications/herald/state/HeraldStateReasons.php', 'HeraldSubscribersField' => 'applications/subscriptions/herald/HeraldSubscribersField.php', 'HeraldSupportActionGroup' => 'applications/herald/action/HeraldSupportActionGroup.php', 'HeraldSupportFieldGroup' => 'applications/herald/field/HeraldSupportFieldGroup.php', @@ -2529,6 +2541,8 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldEditField' => 'infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php', 'PhabricatorCustomFieldEditType' => 'infrastructure/customfield/editor/PhabricatorCustomFieldEditType.php', 'PhabricatorCustomFieldFulltextEngineExtension' => 'infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php', + 'PhabricatorCustomFieldHeraldAction' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldAction.php', + 'PhabricatorCustomFieldHeraldActionGroup' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldActionGroup.php', 'PhabricatorCustomFieldHeraldField' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldField.php', 'PhabricatorCustomFieldHeraldFieldGroup' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldFieldGroup.php', 'PhabricatorCustomFieldImplementationIncompleteException' => 'infrastructure/customfield/exception/PhabricatorCustomFieldImplementationIncompleteException.php', @@ -3194,6 +3208,7 @@ phutil_register_library_map(array( 'PhabricatorMetronomicTriggerClock' => 'infrastructure/daemon/workers/clock/PhabricatorMetronomicTriggerClock.php', 'PhabricatorModularTransaction' => 'applications/transactions/storage/PhabricatorModularTransaction.php', 'PhabricatorModularTransactionType' => 'applications/transactions/storage/PhabricatorModularTransactionType.php', + 'PhabricatorMonogramQuickSearchEngineExtension' => 'applications/typeahead/engineextension/PhabricatorMonogramQuickSearchEngineExtension.php', 'PhabricatorMonospacedFontSetting' => 'applications/settings/setting/PhabricatorMonospacedFontSetting.php', 'PhabricatorMonospacedTextareasSetting' => 'applications/settings/setting/PhabricatorMonospacedTextareasSetting.php', 'PhabricatorMotivatorProfileMenuItem' => 'applications/search/menuitem/PhabricatorMotivatorProfileMenuItem.php', @@ -3517,6 +3532,7 @@ phutil_register_library_map(array( 'PhabricatorPeopleProfileTasksController' => 'applications/people/controller/PhabricatorPeopleProfileTasksController.php', 'PhabricatorPeopleProfileViewController' => 'applications/people/controller/PhabricatorPeopleProfileViewController.php', 'PhabricatorPeopleQuery' => 'applications/people/query/PhabricatorPeopleQuery.php', + 'PhabricatorPeopleQuickSearchEngineExtension' => 'applications/people/engineextension/PhabricatorPeopleQuickSearchEngineExtension.php', 'PhabricatorPeopleRenameController' => 'applications/people/controller/PhabricatorPeopleRenameController.php', 'PhabricatorPeopleRevisionsProfileMenuItem' => 'applications/people/menuitem/PhabricatorPeopleRevisionsProfileMenuItem.php', 'PhabricatorPeopleSearchEngine' => 'applications/people/query/PhabricatorPeopleSearchEngine.php', @@ -3777,6 +3793,9 @@ phutil_register_library_map(array( 'PhabricatorQueryOrderItem' => 'infrastructure/query/order/PhabricatorQueryOrderItem.php', 'PhabricatorQueryOrderTestCase' => 'infrastructure/query/order/__tests__/PhabricatorQueryOrderTestCase.php', 'PhabricatorQueryOrderVector' => 'infrastructure/query/order/PhabricatorQueryOrderVector.php', + 'PhabricatorQuickSearchApplicationEngineExtension' => 'applications/meta/engineextension/PhabricatorQuickSearchApplicationEngineExtension.php', + 'PhabricatorQuickSearchEngine' => 'applications/search/engine/PhabricatorQuickSearchEngine.php', + 'PhabricatorQuickSearchEngineExtension' => 'applications/search/engineextension/PhabricatorQuickSearchEngineExtension.php', 'PhabricatorRateLimitRequestExceptionHandler' => 'aphront/handler/PhabricatorRateLimitRequestExceptionHandler.php', 'PhabricatorRecaptchaConfigOptions' => 'applications/config/option/PhabricatorRecaptchaConfigOptions.php', 'PhabricatorRedirectController' => 'applications/base/controller/PhabricatorRedirectController.php', @@ -4778,6 +4797,7 @@ phutil_register_library_map(array( 'PonderQuestionEditController' => 'applications/ponder/controller/PonderQuestionEditController.php', 'PonderQuestionEditEngine' => 'applications/ponder/editor/PonderQuestionEditEngine.php', 'PonderQuestionEditor' => 'applications/ponder/editor/PonderQuestionEditor.php', + 'PonderQuestionFerretEngine' => 'applications/ponder/search/PonderQuestionFerretEngine.php', 'PonderQuestionFulltextEngine' => 'applications/ponder/search/PonderQuestionFulltextEngine.php', 'PonderQuestionHistoryController' => 'applications/ponder/controller/PonderQuestionHistoryController.php', 'PonderQuestionListController' => 'applications/ponder/controller/PonderQuestionListController.php', @@ -4809,6 +4829,7 @@ phutil_register_library_map(array( 'ProjectDefaultViewCapability' => 'applications/project/capability/ProjectDefaultViewCapability.php', 'ProjectEditConduitAPIMethod' => 'applications/project/conduit/ProjectEditConduitAPIMethod.php', 'ProjectQueryConduitAPIMethod' => 'applications/project/conduit/ProjectQueryConduitAPIMethod.php', + 'ProjectQuickSearchEngineExtension' => 'applications/project/engineextension/ProjectQuickSearchEngineExtension.php', 'ProjectRemarkupRule' => 'applications/project/remarkup/ProjectRemarkupRule.php', 'ProjectRemarkupRuleTestCase' => 'applications/project/remarkup/__tests__/ProjectRemarkupRuleTestCase.php', 'ProjectReplyHandler' => 'applications/project/mail/ProjectReplyHandler.php', @@ -5433,6 +5454,7 @@ phutil_register_library_map(array( 'HarbormasterBuildkiteBuildableInterface', 'PhabricatorApplicationTransactionInterface', 'PhabricatorDestructibleInterface', + 'PhabricatorConduitResultInterface', ), 'DifferentialDiffAffectedFilesHeraldField' => 'DifferentialDiffHeraldField', 'DifferentialDiffAuthorHeraldField' => 'DifferentialDiffHeraldField', @@ -5451,11 +5473,14 @@ phutil_register_library_map(array( 'DifferentialDiffQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DifferentialDiffRepositoryHeraldField' => 'DifferentialDiffHeraldField', 'DifferentialDiffRepositoryProjectsHeraldField' => 'DifferentialDiffHeraldField', + 'DifferentialDiffSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', + 'DifferentialDiffSearchEngine' => 'PhabricatorApplicationSearchEngine', 'DifferentialDiffTestCase' => 'PhutilTestCase', 'DifferentialDiffTransaction' => 'PhabricatorApplicationTransaction', 'DifferentialDiffTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'DifferentialDiffViewController' => 'DifferentialController', 'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'DoorkeeperFeedStoryPublisher', + 'DifferentialDraftField' => 'DifferentialCoreCustomField', 'DifferentialExactUserFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'DifferentialFieldParseException' => 'Exception', 'DifferentialFieldValidationException' => 'Exception', @@ -5469,6 +5494,7 @@ phutil_register_library_map(array( 'DifferentialGetWorkingCopy' => 'Phobject', 'DifferentialGitSVNIDCommitMessageField' => 'DifferentialCommitMessageField', 'DifferentialHarbormasterField' => 'DifferentialCustomField', + 'DifferentialHeraldStateReasons' => 'HeraldStateReasons', 'DifferentialHiddenComment' => 'DifferentialDAO', 'DifferentialHostField' => 'DifferentialCustomField', 'DifferentialHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', @@ -5583,6 +5609,7 @@ phutil_register_library_map(array( 'DifferentialRevisionHasTaskRelationship' => 'DifferentialRevisionRelationship', 'DifferentialRevisionHeraldField' => 'HeraldField', 'DifferentialRevisionHeraldFieldGroup' => 'HeraldFieldGroup', + 'DifferentialRevisionHoldDraftTransaction' => 'DifferentialRevisionTransactionType', 'DifferentialRevisionIDCommitMessageField' => 'DifferentialCommitMessageField', 'DifferentialRevisionInlineTransaction' => 'PhabricatorModularTransactionType', 'DifferentialRevisionInlinesController' => 'DifferentialController', @@ -5626,6 +5653,7 @@ phutil_register_library_map(array( 'DifferentialRevisionUpdateHistoryView' => 'AphrontView', 'DifferentialRevisionViewController' => 'DifferentialController', 'DifferentialRevisionVoidTransaction' => 'DifferentialRevisionTransactionType', + 'DifferentialRevisionWrongStateTransaction' => 'DifferentialRevisionTransactionType', 'DifferentialSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'DifferentialSetDiffPropertyConduitAPIMethod' => 'DifferentialConduitAPIMethod', 'DifferentialStoredCustomField' => 'DifferentialCustomField', @@ -5803,6 +5831,7 @@ phutil_register_library_map(array( 'DiffusionMercurialBlameQuery' => 'DiffusionBlameQuery', 'DiffusionMercurialCommandEngine' => 'DiffusionCommandEngine', 'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery', + 'DiffusionMercurialFlagInjectionException' => 'Exception', 'DiffusionMercurialRawDiffQuery' => 'DiffusionRawDiffQuery', 'DiffusionMercurialRequest' => 'DiffusionRequest', 'DiffusionMercurialResponse' => 'AphrontResponse', @@ -5865,6 +5894,7 @@ phutil_register_library_map(array( 'DiffusionQueryCommitsConduitAPIMethod' => 'DiffusionConduitAPIMethod', 'DiffusionQueryConduitAPIMethod' => 'DiffusionConduitAPIMethod', 'DiffusionQueryPathsConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', + 'DiffusionQuickSearchEngineExtension' => 'PhabricatorQuickSearchEngineExtension', 'DiffusionRawDiffQuery' => 'DiffusionFileFutureQuery', 'DiffusionRawDiffQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', 'DiffusionReadmeView' => 'DiffusionView', @@ -6460,6 +6490,7 @@ phutil_register_library_map(array( 'HeraldApplicationActionGroup' => 'HeraldActionGroup', 'HeraldApplyTranscript' => 'Phobject', 'HeraldBasicFieldGroup' => 'HeraldFieldGroup', + 'HeraldBuildableState' => 'HeraldState', 'HeraldCommitAdapter' => array( 'HeraldAdapter', 'HarbormasterBuildableAdapterInterface', @@ -6489,6 +6520,7 @@ phutil_register_library_map(array( 'HeraldGroup' => 'Phobject', 'HeraldInvalidActionException' => 'Exception', 'HeraldInvalidConditionException' => 'Exception', + 'HeraldMailableState' => 'HeraldState', 'HeraldManageGlobalRulesCapability' => 'PhabricatorPolicyCapability', 'HeraldManiphestTaskAdapter' => 'HeraldAdapter', 'HeraldNewController' => 'HeraldController', @@ -6533,6 +6565,8 @@ phutil_register_library_map(array( 'HeraldSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'HeraldSelectFieldValue' => 'HeraldFieldValue', 'HeraldSpaceField' => 'HeraldField', + 'HeraldState' => 'Phobject', + 'HeraldStateReasons' => 'Phobject', 'HeraldSubscribersField' => 'HeraldField', 'HeraldSupportActionGroup' => 'HeraldActionGroup', 'HeraldSupportFieldGroup' => 'HeraldFieldGroup', @@ -7855,6 +7889,8 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldEditField' => 'PhabricatorEditField', 'PhabricatorCustomFieldEditType' => 'PhabricatorEditType', 'PhabricatorCustomFieldFulltextEngineExtension' => 'PhabricatorFulltextEngineExtension', + 'PhabricatorCustomFieldHeraldAction' => 'HeraldAction', + 'PhabricatorCustomFieldHeraldActionGroup' => 'HeraldActionGroup', 'PhabricatorCustomFieldHeraldField' => 'HeraldField', 'PhabricatorCustomFieldHeraldFieldGroup' => 'HeraldFieldGroup', 'PhabricatorCustomFieldImplementationIncompleteException' => 'Exception', @@ -8595,6 +8631,7 @@ phutil_register_library_map(array( 'PhabricatorMetronomicTriggerClock' => 'PhabricatorTriggerClock', 'PhabricatorModularTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorModularTransactionType' => 'Phobject', + 'PhabricatorMonogramQuickSearchEngineExtension' => 'PhabricatorQuickSearchEngineExtension', 'PhabricatorMonospacedFontSetting' => 'PhabricatorStringSetting', 'PhabricatorMonospacedTextareasSetting' => 'PhabricatorSelectSetting', 'PhabricatorMotivatorProfileMenuItem' => 'PhabricatorProfileMenuItem', @@ -8987,6 +9024,7 @@ phutil_register_library_map(array( 'PhabricatorPeopleProfileTasksController' => 'PhabricatorPeopleProfileController', 'PhabricatorPeopleProfileViewController' => 'PhabricatorPeopleProfileController', 'PhabricatorPeopleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorPeopleQuickSearchEngineExtension' => 'PhabricatorQuickSearchEngineExtension', 'PhabricatorPeopleRenameController' => 'PhabricatorPeopleController', 'PhabricatorPeopleRevisionsProfileMenuItem' => 'PhabricatorProfileMenuItem', 'PhabricatorPeopleSearchEngine' => 'PhabricatorApplicationSearchEngine', @@ -9302,6 +9340,9 @@ phutil_register_library_map(array( 'Phobject', 'Iterator', ), + 'PhabricatorQuickSearchApplicationEngineExtension' => 'PhabricatorQuickSearchEngineExtension', + 'PhabricatorQuickSearchEngine' => 'Phobject', + 'PhabricatorQuickSearchEngineExtension' => 'Phobject', 'PhabricatorRateLimitRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorRecaptchaConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorRedirectController' => 'PhabricatorController', @@ -10547,6 +10588,7 @@ phutil_register_library_map(array( 'PhabricatorDestructibleInterface', 'PhabricatorSpacesInterface', 'PhabricatorFulltextInterface', + 'PhabricatorFerretInterface', ), 'PonderQuestionAnswerTransaction' => 'PonderQuestionTransactionType', 'PonderQuestionAnswerWikiTransaction' => 'PonderQuestionTransactionType', @@ -10556,6 +10598,7 @@ phutil_register_library_map(array( 'PonderQuestionEditController' => 'PonderController', 'PonderQuestionEditEngine' => 'PhabricatorEditEngine', 'PonderQuestionEditor' => 'PonderEditor', + 'PonderQuestionFerretEngine' => 'PhabricatorFerretEngine', 'PonderQuestionFulltextEngine' => 'PhabricatorFulltextEngine', 'PonderQuestionHistoryController' => 'PonderController', 'PonderQuestionListController' => 'PonderController', @@ -10587,6 +10630,7 @@ phutil_register_library_map(array( 'ProjectDefaultViewCapability' => 'PhabricatorPolicyCapability', 'ProjectEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', 'ProjectQueryConduitAPIMethod' => 'ProjectConduitAPIMethod', + 'ProjectQuickSearchEngineExtension' => 'PhabricatorQuickSearchEngineExtension', 'ProjectRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'ProjectRemarkupRuleTestCase' => 'PhabricatorTestCase', 'ProjectReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', diff --git a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php index f3e2562a1c..27e03485ca 100644 --- a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php +++ b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php @@ -9,9 +9,27 @@ final class PhabricatorAuthNeedsMultiFactorController return false; } + public function shouldRequireEnabledUser() { + // Users who haven't been approved yet are allowed to enroll in MFA. We'll + // kick disabled users out later. + return false; + } + + public function shouldRequireEmailVerification() { + // Users who haven't verified their email addresses yet can still enroll + // in MFA. + return false; + } + public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); + if ($viewer->getIsDisabled()) { + // We allowed unapproved and disabled users to hit this controller, but + // want to kick out disabled users now. + return new Aphront400Response(); + } + $panel = id(new PhabricatorMultiFactorSettingsPanel()) ->setUser($viewer) ->setViewer($viewer) diff --git a/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php b/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php index d9d251ab07..d9fb5d013b 100644 --- a/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php +++ b/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php @@ -9,6 +9,10 @@ final class PhabricatorAuthMainMenuBarExtension return true; } + public function shouldRequireFullSession() { + return false; + } + public function getExtensionOrder() { return 900; } diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index f923eb7b73..5952a806ed 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -137,10 +137,6 @@ abstract class PhabricatorController extends AphrontController { } if ($this->shouldRequireEnabledUser()) { - if ($user->isLoggedIn() && !$user->getIsApproved()) { - $controller = new PhabricatorAuthNeedsApprovalController(); - return $this->delegateToController($controller); - } if ($user->getIsDisabled()) { $controller = new PhabricatorDisabledUserController(); return $this->delegateToController($controller); @@ -160,6 +156,15 @@ abstract class PhabricatorController extends AphrontController { } } + // Require users sign Legalpad documents before we check if they have + // MFA. If we don't do this, they can get stuck in a state where they + // can't add MFA until they sign, and can't sign until they add MFA. + // See T13024 and PHI223. + $result = $this->requireLegalpadSignatures(); + if ($result !== null) { + return $result; + } + // Check if the user needs to configure MFA. $need_mfa = $this->shouldRequireMultiFactorEnrollment(); $have_mfa = $user->getIsEnrolledInMultiFactor(); @@ -224,46 +229,15 @@ abstract class PhabricatorController extends AphrontController { ->withPHIDs(array($application->getPHID())) ->executeOne(); } - } - - if (!$this->shouldAllowLegallyNonCompliantUsers()) { - $legalpad_class = 'PhabricatorLegalpadApplication'; - $legalpad = id(new PhabricatorApplicationQuery()) - ->setViewer($user) - ->withClasses(array($legalpad_class)) - ->withInstalled(true) - ->execute(); - $legalpad = head($legalpad); - - $doc_query = id(new LegalpadDocumentQuery()) - ->setViewer($user) - ->withSignatureRequired(1) - ->needViewerSignatures(true); - - if ($user->hasSession() && - !$user->getSession()->getIsPartial() && - !$user->getSession()->getSignedLegalpadDocuments() && - $user->isLoggedIn() && - $legalpad) { - - $sign_docs = $doc_query->execute(); - $must_sign_docs = array(); - foreach ($sign_docs as $sign_doc) { - if (!$sign_doc->getUserSignature($user->getPHID())) { - $must_sign_docs[] = $sign_doc; - } - } - if ($must_sign_docs) { - $controller = new LegalpadDocumentSignController(); - $this->getRequest()->setURIMap(array( - 'id' => head($must_sign_docs)->getID(), - )); - $this->setCurrentApplication($legalpad); + // If users need approval, require they wait here. We do this near the + // end so they can take other actions (like verifying email, signing + // documents, and enrolling in MFA) while waiting for an admin to take a + // look at things. See T13024 for more discussion. + if ($this->shouldRequireEnabledUser()) { + if ($user->isLoggedIn() && !$user->getIsApproved()) { + $controller = new PhabricatorAuthNeedsApprovalController(); return $this->delegateToController($controller); - } else { - $engine = id(new PhabricatorAuthSessionEngine()) - ->signLegalpadDocuments($user, $sign_docs); } } } @@ -558,6 +532,81 @@ abstract class PhabricatorController extends AphrontController { return $this->buildApplicationCrumbs(); } + private function requireLegalpadSignatures() { + if (!$this->shouldRequireLogin()) { + return null; + } + + if ($this->shouldAllowLegallyNonCompliantUsers()) { + return null; + } + + $viewer = $this->getViewer(); + + if (!$viewer->hasSession()) { + return null; + } + + $session = $viewer->getSession(); + if ($session->getIsPartial()) { + // If the user hasn't made it through MFA yet, require they survive + // MFA first. + return null; + } + + if ($session->getSignedLegalpadDocuments()) { + return null; + } + + if (!$viewer->isLoggedIn()) { + return null; + } + + $must_sign_docs = array(); + $sign_docs = array(); + + $legalpad_class = 'PhabricatorLegalpadApplication'; + $legalpad_installed = PhabricatorApplication::isClassInstalledForViewer( + $legalpad_class, + $viewer); + if ($legalpad_installed) { + $sign_docs = id(new LegalpadDocumentQuery()) + ->setViewer($viewer) + ->withSignatureRequired(1) + ->needViewerSignatures(true) + ->setOrder('oldest') + ->execute(); + + foreach ($sign_docs as $sign_doc) { + if (!$sign_doc->getUserSignature($viewer->getPHID())) { + $must_sign_docs[] = $sign_doc; + } + } + } + + if (!$must_sign_docs) { + // If nothing needs to be signed (either because there are no documents + // which require a signature, or because the user has already signed + // all of them) mark the session as good and continue. + $engine = id(new PhabricatorAuthSessionEngine()) + ->signLegalpadDocuments($viewer, $sign_docs); + + return null; + } + + $request = $this->getRequest(); + $request->setURIMap( + array( + 'id' => head($must_sign_docs)->getID(), + )); + + $application = PhabricatorApplication::getByClass($legalpad_class); + $this->setCurrentApplication($application); + + $controller = new LegalpadDocumentSignController(); + return $this->delegateToController($controller); + } + /* -( Deprecated )--------------------------------------------------------- */ diff --git a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php index b6c268d88c..98fa948722 100644 --- a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php +++ b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php @@ -159,10 +159,10 @@ final class PhabricatorAccessControlTestCase extends PhabricatorTestCase { $u_unverified, $u_admin, $u_public, + $u_notapproved, ), array( $u_disabled, - $u_notapproved, )); @@ -224,7 +224,7 @@ final class PhabricatorAccessControlTestCase extends PhabricatorTestCase { )); $this->checkAccess( - pht('Application Controller'), + pht('Application Controller, No Login Required'), id(clone $app_controller)->setConfig('login', false), $request, array( @@ -232,10 +232,10 @@ final class PhabricatorAccessControlTestCase extends PhabricatorTestCase { $u_unverified, $u_admin, $u_public, + $u_notapproved, ), array( $u_disabled, - $u_notapproved, )); } diff --git a/src/applications/daemon/storage/PhabricatorDaemonLogEvent.php b/src/applications/daemon/storage/PhabricatorDaemonLogEvent.php index afdbd1f621..c5d2cbd1d4 100644 --- a/src/applications/daemon/storage/PhabricatorDaemonLogEvent.php +++ b/src/applications/daemon/storage/PhabricatorDaemonLogEvent.php @@ -18,6 +18,9 @@ final class PhabricatorDaemonLogEvent extends PhabricatorDaemonDAO { 'logID' => array( 'columns' => array('logID', 'epoch'), ), + 'key_epoch' => array( + 'columns' => array('epoch'), + ), ), ) + parent::getConfiguration(); } diff --git a/src/applications/differential/conduit/DifferentialDiffSearchConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialDiffSearchConduitAPIMethod.php new file mode 100644 index 0000000000..d4222510d7 --- /dev/null +++ b/src/applications/differential/conduit/DifferentialDiffSearchConduitAPIMethod.php @@ -0,0 +1,18 @@ +parseFields($corpus); $errors = $parser->getErrors(); + $xactions = $parser->getTransactions(); $revision_id_value = idx( $field_map, @@ -49,6 +50,7 @@ final class DifferentialParseCommitMessageConduitAPIMethod 'value' => $revision_id_value, 'validDomain' => $revision_id_valid_domain, ), + 'transactions' => $xactions, ); } diff --git a/src/applications/differential/conduit/DifferentialQueryDiffsConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialQueryDiffsConduitAPIMethod.php index 7ff8600f81..ade7a3fd28 100644 --- a/src/applications/differential/conduit/DifferentialQueryDiffsConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialQueryDiffsConduitAPIMethod.php @@ -11,6 +11,16 @@ final class DifferentialQueryDiffsConduitAPIMethod return pht('Query differential diffs which match certain criteria.'); } + public function getMethodStatus() { + return self::METHOD_STATUS_FROZEN; + } + + public function getMethodStatusDescription() { + return pht( + 'This method is frozen and will eventually be deprecated. New code '. + 'should use "differential.diff.search" instead.'); + } + protected function defineParamTypes() { return array( 'ids' => 'optional list', diff --git a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php index 7ff886664f..0d00207b43 100644 --- a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php +++ b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php @@ -86,6 +86,7 @@ EOHELP array( '/\.py$/', '/\.l?hs$/', + '/\.ya?ml$/', )) ->setDescription( pht( diff --git a/src/applications/differential/constants/DifferentialLegacyQuery.php b/src/applications/differential/constants/DifferentialLegacyQuery.php index 26d2c4aee2..ff7944afe9 100644 --- a/src/applications/differential/constants/DifferentialLegacyQuery.php +++ b/src/applications/differential/constants/DifferentialLegacyQuery.php @@ -32,14 +32,7 @@ final class DifferentialLegacyQuery } private static function getMap() { - $all = array( - DifferentialRevisionStatus::NEEDS_REVIEW, - DifferentialRevisionStatus::NEEDS_REVISION, - DifferentialRevisionStatus::CHANGES_PLANNED, - DifferentialRevisionStatus::ACCEPTED, - DifferentialRevisionStatus::PUBLISHED, - DifferentialRevisionStatus::ABANDONED, - ); + $all = array_keys(DifferentialRevisionStatus::getAll()); $open = array(); $closed = array(); @@ -61,6 +54,9 @@ final class DifferentialLegacyQuery ), self::STATUS_NEEDS_REVIEW => array( DifferentialRevisionStatus::NEEDS_REVIEW, + + // For legacy callers, "Draft" is treated as "Needs Review". + DifferentialRevisionStatus::DRAFT, ), self::STATUS_NEEDS_REVISION => array( DifferentialRevisionStatus::NEEDS_REVISION, diff --git a/src/applications/differential/customfield/DifferentialDraftField.php b/src/applications/differential/customfield/DifferentialDraftField.php new file mode 100644 index 0000000000..5d625e3ce9 --- /dev/null +++ b/src/applications/differential/customfield/DifferentialDraftField.php @@ -0,0 +1,96 @@ +getViewer(); + $revision = $this->getObject(); + + if (!$revision->isDraft()) { + return array(); + } + + // If the author has held this revision as a draft explicitly, don't + // show any misleading messages about it autosubmitting later. + if ($revision->getHoldAsDraft()) { + return array(); + } + + $warnings = array(); + + $blocking_map = array( + HarbormasterBuildStatus::STATUS_FAILED, + HarbormasterBuildStatus::STATUS_ABORTED, + HarbormasterBuildStatus::STATUS_ERROR, + HarbormasterBuildStatus::STATUS_PAUSED, + HarbormasterBuildStatus::STATUS_DEADLOCKED, + ); + $blocking_map = array_fuse($blocking_map); + + $builds = $revision->loadActiveBuilds($viewer); + + $waiting = array(); + $blocking = array(); + foreach ($builds as $build) { + if (isset($blocking_map[$build->getBuildStatus()])) { + $blocking[] = $build; + } else { + $waiting[] = $build; + } + } + + $blocking_list = $viewer->renderHandleList(mpull($blocking, 'getPHID')) + ->setAsInline(true); + $waiting_list = $viewer->renderHandleList(mpull($waiting, 'getPHID')) + ->setAsInline(true); + + if ($blocking) { + $warnings[] = pht( + 'This draft revision will not be submitted for review because %s '. + 'build(s) failed: %s.', + phutil_count($blocking), + $blocking_list); + $warnings[] = pht( + 'Fix build failures and update the revision.'); + } else if ($waiting) { + $warnings[] = pht( + 'This draft revision will be sent for review once %s '. + 'build(s) pass: %s.', + phutil_count($waiting), + $waiting_list); + } else { + $warnings[] = pht( + 'This is a draft revision that has not yet been submitted for '. + 'review.'); + } + + return $warnings; + } + +} diff --git a/src/applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php b/src/applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php index 02a24effcf..a6940b536e 100644 --- a/src/applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php +++ b/src/applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php @@ -13,6 +13,20 @@ final class DifferentialRevisionDependedOnByRevisionEdgeType return true; } + public function getConduitKey() { + return 'revision.child'; + } + + public function getConduitName() { + return pht('Revision Has Child'); + } + + public function getConduitDescription() { + return pht( + 'The source revision makes changes required by the destination '. + 'revision.'); + } + public function getTransactionAddString( $actor, $add_count, diff --git a/src/applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php b/src/applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php index 4613ad8a34..e826f554c6 100644 --- a/src/applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php +++ b/src/applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php @@ -17,6 +17,19 @@ final class DifferentialRevisionDependsOnRevisionEdgeType return true; } + public function getConduitKey() { + return 'revision.parent'; + } + + public function getConduitName() { + return pht('Revision Has Parent'); + } + + public function getConduitDescription() { + return pht( + 'The source revision depends on changes in the destination revision.'); + } + public function getTransactionAddString( $actor, $add_count, diff --git a/src/applications/differential/editor/DifferentialRevisionEditEngine.php b/src/applications/differential/editor/DifferentialRevisionEditEngine.php index 9aad031a7c..2bd3a5cdc3 100644 --- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php +++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php @@ -235,6 +235,22 @@ final class DifferentialRevisionEditEngine $fields[] = $action->newEditField($object, $viewer); } + $fields[] = id(new PhabricatorBoolEditField()) + ->setKey('draft') + ->setLabel(pht('Hold as Draft')) + ->setIsConduitOnly(true) + ->setOptions( + pht('Autosubmit Once Builds Finish'), + pht('Hold as Draft')) + ->setTransactionType( + DifferentialRevisionHoldDraftTransaction::TRANSACTIONTYPE) + ->setDescription(pht('Hold revision as as draft.')) + ->setConduitDescription( + pht( + 'Change autosubmission from draft state after builds finish.')) + ->setConduitTypeDescription(pht('New "Hold as Draft" setting.')) + ->setValue($object->getHoldAsDraft()); + return $fields; } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 253d49c1ec..b2fc179002 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -9,6 +9,8 @@ final class DifferentialTransactionEditor private $didExpandInlineState = false; private $hasReviewTransaction = false; private $affectedPaths; + private $firstBroadcast = false; + private $wasDraft = false; public function getEditorApplicationClass() { return 'PhabricatorDifferentialApplication'; @@ -27,7 +29,7 @@ final class DifferentialTransactionEditor } public function isFirstBroadcast() { - return $this->getIsNewObject(); + return $this->firstBroadcast; } public function getDiffUpdateTransaction(array $xactions) { @@ -137,8 +139,7 @@ final class DifferentialTransactionEditor $object->setRepositoryPHID($diff->getRepositoryPHID()); } $object->attachActiveDiff($diff); - - // TODO: Update the `diffPHID` once we add that. + $object->setActiveDiffPHID($diff->getPHID()); return; } @@ -166,6 +167,8 @@ final class DifferentialTransactionEditor } } + $this->wasDraft = $object->isDraft(); + return parent::expandTransactions($object, $xactions); } @@ -630,6 +633,10 @@ final class DifferentialTransactionEditor $phids = array(); $phids[] = $object->getAuthorPHID(); foreach ($object->getReviewers() as $reviewer) { + if ($reviewer->isResigned()) { + continue; + } + $phids[] = $reviewer->getReviewerPHID(); } return $phids; @@ -1003,26 +1010,7 @@ final class DifferentialTransactionEditor protected function shouldApplyHeraldRules( PhabricatorLiskDAO $object, array $xactions) { - - if ($this->getIsNewObject()) { - return true; - } - - foreach ($xactions as $xaction) { - switch ($xaction->getTransactionType()) { - case DifferentialTransaction::TYPE_UPDATE: - if (!$this->getIsCloseByCommit()) { - return true; - } - break; - case DifferentialRevisionCommandeerTransaction::TRANSACTIONTYPE: - // When users commandeer revisions, we may need to trigger - // signatures or author-based rules. - return true; - } - } - - return parent::shouldApplyHeraldRules($object, $xactions); + return true; } protected function didApplyHeraldRules( @@ -1211,6 +1199,49 @@ final class DifferentialTransactionEditor $revision, $revision->getActiveDiff()); + // If the object is still a draft, prevent "Send me an email" and other + // similar rules from acting yet. + if (!$object->shouldBroadcast()) { + $adapter->setForbiddenAction( + HeraldMailableState::STATECONST, + DifferentialHeraldStateReasons::REASON_DRAFT); + } + + // If this edit didn't actually change the diff (for example, a user + // edited the title or changed subscribers), prevent "Run build plan" + // and other similar rules from acting yet, since the build results will + // not (or, at least, should not) change unless the actual source changes. + // We also don't run Differential builds if the update was caused by + // discovering a commit, as the expectation is that Diffusion builds take + // over once things land. + $has_update = false; + $has_commit = false; + + $type_update = DifferentialTransaction::TYPE_UPDATE; + foreach ($xactions as $xaction) { + if ($xaction->getTransactionType() != $type_update) { + continue; + } + + if ($xaction->getMetadataValue('isCommitUpdate')) { + $has_commit = true; + } else { + $has_update = true; + } + + break; + } + + if ($has_commit) { + $adapter->setForbiddenAction( + HeraldBuildableState::STATECONST, + DifferentialHeraldStateReasons::REASON_LANDED); + } else if (!$has_update) { + $adapter->setForbiddenAction( + HeraldBuildableState::STATECONST, + DifferentialHeraldStateReasons::REASON_UNCHANGED); + } + return $adapter; } @@ -1425,11 +1456,13 @@ final class DifferentialTransactionEditor protected function getCustomWorkerState() { return array( 'changedPriorToCommitURI' => $this->changedPriorToCommitURI, + 'firstBroadcast' => $this->firstBroadcast, ); } protected function loadCustomWorkerState(array $state) { $this->changedPriorToCommitURI = idx($state, 'changedPriorToCommitURI'); + $this->firstBroadcast = idx($state, 'firstBroadcast'); return $this; } @@ -1532,19 +1565,31 @@ final class DifferentialTransactionEditor protected function didApplyTransactions($object, array $xactions) { // If a draft revision has no outstanding builds and we're automatically // making drafts public after builds finish, make the revision public. - $auto_undraft = true; + $auto_undraft = !$object->getHoldAsDraft(); if ($object->isDraft() && $auto_undraft) { $active_builds = $this->hasActiveBuilds($object); if (!$active_builds) { + // When Harbormaster moves a revision out of the draft state, we + // attribute the action to the revision author since this is more + // natural and more useful. + $author_phid = $object->getAuthorPHID(); + + // Additionally, we change the acting PHID for the transaction set + // to the author if it isn't already a user so that mail comes from + // the natural author. + $acting_phid = $this->getActingAsPHID(); + $user_type = PhabricatorPeopleUserPHIDType::TYPECONST; + if (phid_get_type($acting_phid) != $user_type) { + $this->setActingAsPHID($author_phid); + } + $xaction = $object->getApplicationTransactionTemplate() + ->setAuthorPHID($author_phid) ->setTransactionType( DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE) - ->setOldValue(false) ->setNewValue(true); - $xaction = $this->populateTransaction($object, $xaction); - // If we're creating this revision and immediately moving it out of // the draft state, mark this as a create transaction so it gets // hidden in the timeline and mail, since it isn't interesting: it @@ -1553,15 +1598,36 @@ final class DifferentialTransactionEditor $xaction->setIsCreateTransaction(true); } - $object->openTransaction(); - $object - ->setStatus(DifferentialRevisionStatus::NEEDS_REVIEW) - ->save(); + // Queue this transaction and apply it separately after the current + // batch of transactions finishes so that Herald can fire on the new + // revision state. See T13027 for discussion. + $this->queueTransaction($xaction); + } + } - $xaction->save(); - $object->saveTransaction(); + // If the revision is new or was a draft, and is no longer a draft, we + // might be sending the first email about it. - $xactions[] = $xaction; + // This might mean it was created directly into a non-draft state, or + // it just automatically undrafted after builds finished, or a user + // explicitly promoted it out of the draft state with an action like + // "Request Review". + + // If we haven't sent any email about it yet, mark this email as the first + // email so the mail gets enriched with "SUMMARY" and "TEST PLAN". + + $is_new = $this->getIsNewObject(); + $was_draft = $this->wasDraft; + + if (!$object->isDraft() && ($was_draft || $is_new)) { + if (!$object->getHasBroadcast()) { + // Mark this as the first broadcast we're sending about the revision + // so mail can generate specially. + $this->firstBroadcast = true; + + $object + ->setHasBroadcast(true) + ->save(); } } @@ -1570,58 +1636,12 @@ final class DifferentialTransactionEditor private function hasActiveBuilds($object) { $viewer = $this->requireActor(); - $diff = $object->getActiveDiff(); - $buildables = id(new HarbormasterBuildableQuery()) - ->setViewer($viewer) - ->withContainerPHIDs(array($object->getPHID())) - ->withBuildablePHIDs(array($diff->getPHID())) - ->withManualBuildables(false) - ->execute(); - if (!$buildables) { - return false; - } - - $builds = id(new HarbormasterBuildQuery()) - ->setViewer($viewer) - ->withBuildablePHIDs(mpull($buildables, 'getPHID')) - ->withBuildStatuses( - array( - HarbormasterBuildStatus::STATUS_INACTIVE, - HarbormasterBuildStatus::STATUS_PENDING, - HarbormasterBuildStatus::STATUS_BUILDING, - HarbormasterBuildStatus::STATUS_FAILED, - HarbormasterBuildStatus::STATUS_ABORTED, - HarbormasterBuildStatus::STATUS_ERROR, - HarbormasterBuildStatus::STATUS_PAUSED, - HarbormasterBuildStatus::STATUS_DEADLOCKED, - )) - ->needBuildTargets(true) - ->execute(); + $builds = $object->loadActiveBuilds($viewer); if (!$builds) { return false; } - $active = array(); - foreach ($builds as $key => $build) { - foreach ($build->getBuildTargets() as $target) { - if ($target->isAutotarget()) { - // Ignore autotargets when looking for active of failed builds. If - // local tests fail and you continue anyway, you don't need to - // double-confirm them. - continue; - } - - // This build has at least one real target that's doing something. - $active[$key] = $build; - break; - } - } - - if (!$active) { - return false; - } - return true; } diff --git a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php index 952c76c63f..c426c411e7 100644 --- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php +++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php @@ -268,6 +268,16 @@ final class DifferentialDiffExtractionEngine extends Phobject { $xactions = array(); + // If the revision isn't closed or "Accepted", write a warning into the + // transaction log. This makes it more clear when users bend the rules. + if (!$revision->isClosed() && !$revision->isAccepted()) { + $wrong_type = DifferentialRevisionWrongStateTransaction::TRANSACTIONTYPE; + + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType($wrong_type) + ->setNewValue($revision->getModernRevisionStatus()); + } + $xactions[] = id(new DifferentialTransaction()) ->setTransactionType(DifferentialTransaction::TYPE_UPDATE) ->setIgnoreOnNoEffect(true) diff --git a/src/applications/differential/herald/DifferentialHeraldStateReasons.php b/src/applications/differential/herald/DifferentialHeraldStateReasons.php new file mode 100644 index 0000000000..92d2ca3067 --- /dev/null +++ b/src/applications/differential/herald/DifferentialHeraldStateReasons.php @@ -0,0 +1,26 @@ + pht( + 'This revision is still an unsubmitted draft, so mail will not '. + 'be sent yet.'), + self::REASON_UNCHANGED => pht( + 'The update which triggered Herald did not update the diff for '. + 'this revision, so builds will not run.'), + self::REASON_LANDED => pht( + 'The update which triggered Herald was an automatic update in '. + 'response to discovering a commit, so builds will not run.'), + ); + + return idx($reasons, $reason); + } + +} diff --git a/src/applications/differential/parser/DifferentialCommitMessageParser.php b/src/applications/differential/parser/DifferentialCommitMessageParser.php index 7e5a50e61f..3a73428e4b 100644 --- a/src/applications/differential/parser/DifferentialCommitMessageParser.php +++ b/src/applications/differential/parser/DifferentialCommitMessageParser.php @@ -28,6 +28,7 @@ final class DifferentialCommitMessageParser extends Phobject { private $errors; private $commitMessageFields; private $raiseMissingFieldErrors = true; + private $xactions; public static function newStandardParser(PhabricatorUser $viewer) { $key_title = DifferentialTitleCommitMessageField::FIELDKEY; @@ -134,6 +135,7 @@ final class DifferentialCommitMessageParser extends Phobject { */ public function parseCorpus($corpus) { $this->errors = array(); + $this->xactions = array(); $label_map = $this->getLabelMap(); $key_title = $this->titleKey; @@ -284,12 +286,25 @@ final class DifferentialCommitMessageParser extends Phobject { try { $result = $field->parseFieldValue($text_value); $result_map[$field_key] = $result; + + try { + $xactions = $field->getFieldTransactions($result); + foreach ($xactions as $xaction) { + $this->xactions[] = $xaction; + } + } catch (Exception $ex) { + $this->errors[] = pht( + 'Error extracting field transactions from "%s": %s', + $field->getFieldName(), + $ex->getMessage()); + } } catch (DifferentialFieldParseException $ex) { $this->errors[] = pht( 'Error parsing field "%s": %s', $field->getFieldName(), $ex->getMessage()); } + } if ($this->getRaiseMissingFieldErrors()) { @@ -317,6 +332,14 @@ final class DifferentialCommitMessageParser extends Phobject { } + /** + * @task parse + */ + public function getTransactions() { + return $this->xactions; + } + + /* -( Support Methods )---------------------------------------------------- */ diff --git a/src/applications/differential/query/DifferentialDiffQuery.php b/src/applications/differential/query/DifferentialDiffQuery.php index f779e40c26..3b1dd3ae62 100644 --- a/src/applications/differential/query/DifferentialDiffQuery.php +++ b/src/applications/differential/query/DifferentialDiffQuery.php @@ -6,6 +6,7 @@ final class DifferentialDiffQuery private $ids; private $phids; private $revisionIDs; + private $revisionPHIDs; private $commitPHIDs; private $hasRevision; @@ -27,6 +28,11 @@ final class DifferentialDiffQuery return $this; } + public function withRevisionPHIDs(array $revision_phids) { + $this->revisionPHIDs = $revision_phids; + return $this; + } + public function withCommitPHIDs(array $phids) { $this->commitPHIDs = $phids; return $this; @@ -160,6 +166,25 @@ final class DifferentialDiffQuery } } + if ($this->revisionPHIDs !== null) { + $viewer = $this->getViewer(); + + $revisions = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->setParentQuery($this) + ->withPHIDs($this->revisionPHIDs) + ->execute(); + $revision_ids = mpull($revisions, 'getID'); + if (!$revision_ids) { + throw new PhabricatorEmptyQueryException(); + } + + $where[] = qsprintf( + $conn, + 'revisionID IN (%Ls)', + $revision_ids); + } + return $where; } diff --git a/src/applications/differential/query/DifferentialDiffSearchEngine.php b/src/applications/differential/query/DifferentialDiffSearchEngine.php new file mode 100644 index 0000000000..89feeb5c3e --- /dev/null +++ b/src/applications/differential/query/DifferentialDiffSearchEngine.php @@ -0,0 +1,79 @@ +newQuery(); + + if ($map['revisionPHIDs']) { + $query->withRevisionPHIDs($map['revisionPHIDs']); + } + + return $query; + } + + protected function buildCustomSearchFields() { + return array( + id(new PhabricatorPHIDsSearchField()) + ->setLabel(pht('Revisions')) + ->setKey('revisionPHIDs') + ->setAliases(array('revision', 'revisions', 'revisionPHID')) + ->setDescription( + pht('Find diffs attached to a particular revision.')), + ); + } + + protected function getURI($path) { + return '/differential/diff/'.$path; + } + + protected function getBuiltinQueryNames() { + $names = array(); + + $names['all'] = pht('All Diffs'); + + return $names; + } + + public function buildSavedQueryFromBuiltin($query_key) { + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + $viewer = $this->requireViewer(); + + switch ($query_key) { + case 'all': + return $query; + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + + protected function renderResultList( + array $revisions, + PhabricatorSavedQuery $query, + array $handles) { + assert_instances_of($revisions, 'DifferentialDiff'); + + $viewer = $this->requireViewer(); + + // NOTE: This is only exposed to Conduit, so we don't currently render + // results. + + return id(new PhabricatorApplicationSearchResultView()); + } + +} diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index 8fd91cbd7e..727a4ca184 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -38,8 +38,6 @@ final class DifferentialRevisionQuery private $needDrafts; private $needFlags; - private $buildingGlobalOrder; - /* -( Query Configuration )------------------------------------------------ */ @@ -484,12 +482,11 @@ final class DifferentialRevisionQuery } if (count($selects) > 1) { - $this->buildingGlobalOrder = true; $query = qsprintf( $conn_r, '%Q %Q %Q', implode(' UNION DISTINCT ', $selects), - $this->buildOrderClause($conn_r), + $this->buildOrderClause($conn_r, true), $this->buildLimitClause($conn_r)); } else { $query = head($selects); @@ -513,7 +510,6 @@ final class DifferentialRevisionQuery $group_by = $this->buildGroupClause($conn_r); $having = $this->buildHavingClause($conn_r); - $this->buildingGlobalOrder = false; $order_by = $this->buildOrderClause($conn_r); $limit = $this->buildLimitClause($conn_r); @@ -758,17 +754,9 @@ final class DifferentialRevisionQuery } public function getOrderableColumns() { - $primary = ($this->buildingGlobalOrder ? null : 'r'); - return array( - 'id' => array( - 'table' => $primary, - 'column' => 'id', - 'type' => 'int', - 'unique' => true, - ), 'updated' => array( - 'table' => $primary, + 'table' => $this->getPrimaryTableAlias(), 'column' => 'dateModified', 'type' => 'int', ), diff --git a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php index f3971f8571..711f70afb3 100644 --- a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php +++ b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php @@ -123,6 +123,14 @@ final class DifferentialRevisionRequiredActionResultBucket $reviewing = array( DifferentialReviewerStatus::STATUS_ADDED, DifferentialReviewerStatus::STATUS_COMMENTED, + + // If an author has used "Request Review" to put an accepted revision + // back into the "Needs Review" state, include "Accepted" reviewers + // whose reviews have been voided in the "Should Review" bucket. + + // If we don't do this, they end up in "Waiting on Other Reviewers", + // even if there are no other reviewers. + DifferentialReviewerStatus::STATUS_ACCEPTED, ); $reviewing = array_fuse($reviewing); @@ -130,7 +138,7 @@ final class DifferentialRevisionRequiredActionResultBucket $results = array(); foreach ($objects as $key => $object) { - if (!$this->hasReviewersWithStatus($object, $phids, $reviewing)) { + if (!$this->hasReviewersWithStatus($object, $phids, $reviewing, true)) { continue; } diff --git a/src/applications/differential/query/DifferentialRevisionResultBucket.php b/src/applications/differential/query/DifferentialRevisionResultBucket.php index 54705649eb..c5cc5c0e6c 100644 --- a/src/applications/differential/query/DifferentialRevisionResultBucket.php +++ b/src/applications/differential/query/DifferentialRevisionResultBucket.php @@ -53,7 +53,8 @@ abstract class DifferentialRevisionResultBucket protected function hasReviewersWithStatus( DifferentialRevision $revision, array $phids, - array $statuses) { + array $statuses, + $include_voided = null) { foreach ($revision->getReviewers() as $reviewer) { $reviewer_phid = $reviewer->getReviewerPHID(); @@ -66,6 +67,15 @@ abstract class DifferentialRevisionResultBucket continue; } + if ($include_voided !== null) { + if ($status == DifferentialReviewerStatus::STATUS_ACCEPTED) { + $is_voided = (bool)$reviewer->getVoidedPHID(); + if ($is_voided !== $include_voided) { + continue; + } + } + } + return true; } diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php index 7a694cfd98..586680d1b8 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php @@ -41,6 +41,20 @@ final class DifferentialChangesetOneUpRenderer $column_width = 4; + $aural_minus = javelin_tag( + 'span', + array( + 'aural' => true, + ), + '- '); + + $aural_plus = javelin_tag( + 'span', + array( + 'aural' => true, + ), + '+ '); + $out = array(); foreach ($primitives as $k => $p) { $type = $p['type']; @@ -55,8 +69,10 @@ final class DifferentialChangesetOneUpRenderer if ($is_old) { if ($p['htype']) { $class = 'left old'; + $aural = $aural_minus; } else { $class = 'left'; + $aural = null; } if ($type == 'old-file') { @@ -79,14 +95,20 @@ final class DifferentialChangesetOneUpRenderer ), $line); + $render = $p['render']; + if ($aural !== null) { + $render = array($aural, $render); + } + $cells[] = phutil_tag('th', array('class' => $class)); $cells[] = $no_copy; - $cells[] = phutil_tag('td', array('class' => $class), $p['render']); + $cells[] = phutil_tag('td', array('class' => $class), $render); $cells[] = $no_coverage; } else { if ($p['htype']) { $class = 'right new'; $cells[] = phutil_tag('th', array('class' => $class)); + $aural = $aural_plus; } else { $class = 'right'; if ($left_prefix) { @@ -98,6 +120,7 @@ final class DifferentialChangesetOneUpRenderer $oline = $p['oline']; $cells[] = phutil_tag('th', array('id' => $left_id), $oline); + $aural = null; } if ($type == 'new-file') { @@ -120,8 +143,13 @@ final class DifferentialChangesetOneUpRenderer ), $line); + $render = $p['render']; + if ($aural !== null) { + $render = array($aural, $render); + } + $cells[] = $no_copy; - $cells[] = phutil_tag('td', array('class' => $class), $p['render']); + $cells[] = phutil_tag('td', array('class' => $class), $render); $cells[] = $no_coverage; } diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index f7e85cd835..d70cdbbdf5 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -9,7 +9,8 @@ final class DifferentialDiff HarbormasterCircleCIBuildableInterface, HarbormasterBuildkiteBuildableInterface, PhabricatorApplicationTransactionInterface, - PhabricatorDestructibleInterface { + PhabricatorDestructibleInterface, + PhabricatorConduitResultInterface { protected $revisionID; protected $authorPHID; @@ -740,4 +741,82 @@ final class DifferentialDiff $this->saveTransaction(); } + +/* -( PhabricatorConduitResultInterface )---------------------------------- */ + + + public function getFieldSpecificationsForConduit() { + return array( + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('revisionPHID') + ->setType('phid') + ->setDescription(pht('Associated revision PHID.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('authorPHID') + ->setType('phid') + ->setDescription(pht('Revision author PHID.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('repositoryPHID') + ->setType('phid') + ->setDescription(pht('Associated repository PHID.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('refs') + ->setType('map') + ->setDescription(pht('List of related VCS references.')), + ); + } + + public function getFieldValuesForConduit() { + $refs = array(); + + $branch = $this->getBranch(); + if (strlen($branch)) { + $refs[] = array( + 'type' => 'branch', + 'name' => $branch, + ); + } + + $onto = $this->loadTargetBranch(); + if (strlen($onto)) { + $refs[] = array( + 'type' => 'onto', + 'name' => $onto, + ); + } + + $base = $this->getSourceControlBaseRevision(); + if (strlen($base)) { + $refs[] = array( + 'type' => 'base', + 'identifier' => $base, + ); + } + + $bookmark = $this->getBookmark(); + if (strlen($bookmark)) { + $refs[] = array( + 'type' => 'bookmark', + 'name' => $bookmark, + ); + } + + $revision_phid = null; + if ($this->getRevisionID()) { + $revision_phid = $this->getRevision()->getPHID(); + } + + return array( + 'revisionPHID' => $revision_phid, + 'authorPHID' => $this->getAuthorPHID(), + 'repositoryPHID' => $this->getRepositoryPHID(), + 'refs' => $refs, + ); + } + + public function getConduitSearchAttachments() { + return array(); + } + + } diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index e29db36b14..73f22c91e4 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -35,6 +35,8 @@ final class DifferentialRevision extends DifferentialDAO protected $mailKey; protected $branchName; protected $repositoryPHID; + protected $activeDiffPHID; + protected $viewPolicy = PhabricatorPolicies::POLICY_USER; protected $editPolicy = PhabricatorPolicies::POLICY_USER; protected $properties = array(); @@ -57,6 +59,8 @@ final class DifferentialRevision extends DifferentialDAO const RELATION_SUBSCRIBED = 'subd'; const PROPERTY_CLOSED_FROM_ACCEPTED = 'wasAcceptedBeforeClose'; + const PROPERTY_DRAFT_HOLD = 'draft.hold'; + const PROPERTY_HAS_BROADCAST = 'draft.broadcast'; public static function initializeNewRevision(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) @@ -67,13 +71,19 @@ final class DifferentialRevision extends DifferentialDAO $view_policy = $app->getPolicy( DifferentialDefaultViewCapability::CAPABILITY); + if (PhabricatorEnv::getEnvConfig('phabricator.show-prototypes')) { + $initial_state = DifferentialRevisionStatus::DRAFT; + } else { + $initial_state = DifferentialRevisionStatus::NEEDS_REVIEW; + } + return id(new DifferentialRevision()) ->setViewPolicy($view_policy) ->setAuthorPHID($actor->getPHID()) ->attachRepository(null) ->attachActiveDiff(null) ->attachReviewers(array()) - ->setModernRevisionStatus(DifferentialRevisionStatus::NEEDS_REVIEW); + ->setModernRevisionStatus($initial_state); } protected function getConfiguration() { @@ -702,6 +712,54 @@ final class DifferentialRevision extends DifferentialDAO return false; } + public function getHoldAsDraft() { + return $this->getProperty(self::PROPERTY_DRAFT_HOLD, false); + } + + public function setHoldAsDraft($hold) { + return $this->setProperty(self::PROPERTY_DRAFT_HOLD, $hold); + } + + public function getHasBroadcast() { + return $this->getProperty(self::PROPERTY_HAS_BROADCAST, false); + } + + public function setHasBroadcast($has_broadcast) { + return $this->setProperty(self::PROPERTY_HAS_BROADCAST, $has_broadcast); + } + + + public function loadActiveBuilds(PhabricatorUser $viewer) { + $diff = $this->getActiveDiff(); + + $buildables = id(new HarbormasterBuildableQuery()) + ->setViewer($viewer) + ->withContainerPHIDs(array($this->getPHID())) + ->withBuildablePHIDs(array($diff->getPHID())) + ->withManualBuildables(false) + ->execute(); + if (!$buildables) { + return array(); + } + + return id(new HarbormasterBuildQuery()) + ->setViewer($viewer) + ->withBuildablePHIDs(mpull($buildables, 'getPHID')) + ->withAutobuilds(false) + ->withBuildStatuses( + array( + HarbormasterBuildStatus::STATUS_INACTIVE, + HarbormasterBuildStatus::STATUS_PENDING, + HarbormasterBuildStatus::STATUS_BUILDING, + HarbormasterBuildStatus::STATUS_FAILED, + HarbormasterBuildStatus::STATUS_ABORTED, + HarbormasterBuildStatus::STATUS_ERROR, + HarbormasterBuildStatus::STATUS_PAUSED, + HarbormasterBuildStatus::STATUS_DEADLOCKED, + )) + ->execute(); + } + /* -( HarbormasterBuildableInterface )------------------------------------- */ @@ -938,6 +996,18 @@ final class DifferentialRevision extends DifferentialDAO ->setKey('status') ->setType('map') ->setDescription(pht('Information about revision status.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('repositoryPHID') + ->setType('phid?') + ->setDescription(pht('Revision repository PHID.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('diffPHID') + ->setType('phid') + ->setDescription(pht('Active diff PHID.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('summary') + ->setType('string') + ->setDescription(pht('Revision summary.')), ); } @@ -954,6 +1024,9 @@ final class DifferentialRevision extends DifferentialDAO 'title' => $this->getTitle(), 'authorPHID' => $this->getAuthorPHID(), 'status' => $status_info, + 'repositoryPHID' => $this->getRepositoryPHID(), + 'diffPHID' => $this->getActiveDiffPHID(), + 'summary' => $this->getSummary(), ); } diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index 307d424b0e..59f73b5465 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -301,6 +301,9 @@ final class DifferentialChangesetListView extends AphrontView { 'Hide or show all inline comments.' => pht('Hide or show all inline comments.'), + + 'Finish editing inline comments before changing display modes.' => + pht('Finish editing inline comments before changing display modes.'), ), )); diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php index 33aad25289..f88669e539 100644 --- a/src/applications/differential/view/DifferentialReviewersView.php +++ b/src/applications/differential/view/DifferentialReviewersView.php @@ -47,6 +47,7 @@ final class DifferentialReviewersView extends AphrontView { $action_phid = $reviewer->getLastActionDiffPHID(); $is_current_action = $this->isCurrent($action_phid); + $is_voided = (bool)$reviewer->getVoidedPHID(); $comment_phid = $reviewer->getLastCommentDiffPHID(); $is_current_comment = $this->isCurrent($comment_phid); @@ -86,7 +87,7 @@ final class DifferentialReviewersView extends AphrontView { break; case DifferentialReviewerStatus::STATUS_ACCEPTED: - if ($is_current_action) { + if ($is_current_action && !$is_voided) { $icon = PHUIStatusItemView::ICON_ACCEPT; $color = 'green'; if ($authority_name !== null) { @@ -97,7 +98,12 @@ final class DifferentialReviewersView extends AphrontView { } else { $icon = 'fa-check-circle-o'; $color = 'bluegrey'; - if ($authority_name !== null) { + + if (!$is_current_action && $is_voided) { + // The reviewer accepted the revision, but later the author + // used "Request Review" to request an updated review. + $label = pht('Accepted Earlier'); + } else if ($authority_name !== null) { $label = pht('Accepted Prior Diff (by %s)', $authority_name); } else { $label = pht('Accepted Prior Diff'); diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php index d52e2dbeb7..190a6a4920 100644 --- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php @@ -162,6 +162,11 @@ final class DifferentialRevisionAcceptTransaction 'closed. Only open revisions can be accepted.')); } + if ($object->isDraft()) { + throw new Exception( + pht('You can not accept a draft revision.')); + } + $config_key = 'differential.allow-self-accept'; if (!PhabricatorEnv::getEnvConfig($config_key)) { if ($this->isViewerRevisionAuthor($object, $viewer)) { diff --git a/src/applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php b/src/applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php new file mode 100644 index 0000000000..2562f18209 --- /dev/null +++ b/src/applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php @@ -0,0 +1,56 @@ +getHoldAsDraft(); + } + + public function generateNewValue($object, $value) { + return (bool)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setHoldAsDraft($value); + + // If draft isn't the default state but we're creating a new revision + // and holding it as a draft, put it in draft mode. See PHI206. + // TODO: This can probably be removed once Draft is the universal default. + if ($this->isNewObject()) { + if ($object->isNeedsReview()) { + $object->setModernRevisionStatus(DifferentialRevisionStatus::DRAFT); + } + } + } + + public function getTitle() { + if ($this->getNewValue()) { + return pht( + '%s held this revision as a draft.', + $this->renderAuthor()); + } else { + return pht( + '%s set this revision to automatically submit once builds complete.', + $this->renderAuthor()); + } + } + + public function getTitleForFeed() { + if ($this->getNewValue()) { + return pht( + '%s held %s as a draft.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s set %s to automatically submit once builds complete.', + $this->renderAuthor(), + $this->renderObject()); + } + } + +} diff --git a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php index 465b5234d5..ed0d20a0b6 100644 --- a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php @@ -57,11 +57,15 @@ final class DifferentialRevisionRequestReviewTransaction 'revisions.')); } - if (!$this->isViewerRevisionAuthor($object, $viewer)) { - throw new Exception( - pht( - 'You can not request review of this revision because you are not '. - 'the author of the revision.')); + // When revisions automatically promote out of "Draft" after builds finish, + // the viewer may be acting as the Harbormaster application. + if (!$viewer->isOmnipotent()) { + if (!$this->isViewerRevisionAuthor($object, $viewer)) { + throw new Exception( + pht( + 'You can not request review of this revision because you are not '. + 'the author of the revision.')); + } } } diff --git a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php index 40a512202d..53b0b8da01 100644 --- a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php @@ -64,11 +64,6 @@ final class DifferentialRevisionResignTransaction 'been closed. You can only resign from open revisions.')); } - if ($object->isDraft()) { - throw new Exception( - pht('You can not resign from a draft revision.')); - } - $resigned = DifferentialReviewerStatus::STATUS_RESIGNED; if ($this->getViewerReviewerStatus($object, $viewer) == $resigned) { throw new Exception( diff --git a/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php b/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php index 615ce38bcf..9c8b3767df 100644 --- a/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php @@ -74,10 +74,10 @@ final class DifferentialRevisionStatusTransaction return 'status'; } - public function getFieldValuesForConduit($object, $data) { + public function getFieldValuesForConduit($xaction, $data) { return array( - 'old' => $object->getOldValue(), - 'new' => $object->getNewValue(), + 'old' => $xaction->getOldValue(), + 'new' => $xaction->getNewValue(), ); } diff --git a/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php b/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php new file mode 100644 index 0000000000..e19e54199c --- /dev/null +++ b/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php @@ -0,0 +1,41 @@ +getNewValue(); + + $status = DifferentialRevisionStatus::newForStatus($new_value); + + return pht( + 'This revision was not accepted when it landed; it landed in state %s.', + $this->renderValue($status->getDisplayName())); + } + + public function getTitleForFeed() { + return null; + } +} diff --git a/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php index 4280230002..2d4a221171 100644 --- a/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php @@ -47,7 +47,7 @@ final class DiffusionExistsQueryConduitAPIMethod $commit = $request->getValue('commit'); list($err, $stdout) = $repository->execLocalCommand( 'id --rev %s', - $commit); + hgsprintf('%s', $commit)); return !$err; } diff --git a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php index 200be8567b..4c1d39e8c8 100644 --- a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php @@ -123,21 +123,23 @@ final class DiffusionHistoryQueryConduitAPIMethod // branches). if (strlen($path)) { - $path_arg = csprintf('-- %s', $path); - $branch_arg = ''; + $path_arg = csprintf('%s', $path); + $revset_arg = hgsprintf( + 'reverse(ancestors(%s))', + $commit_hash); } else { $path_arg = ''; - // NOTE: --branch used to be called --only-branch; use -b for - // compatibility. - $branch_arg = csprintf('-b %s', $drequest->getBranch()); + $revset_arg = hgsprintf( + 'reverse(ancestors(%s)) and branch(%s)', + $drequest->getBranch(), + $commit_hash); } list($stdout) = $repository->execxLocalCommand( - 'log --debug --template %s --limit %d %C --rev %s %C', + 'log --debug --template %s --limit %d --rev %s -- %C', '{node};{parents}\\n', ($offset + $limit), // No '--skip' in Mercurial. - $branch_arg, - hgsprintf('reverse(ancestors(%s))', $commit_hash), + $revset_arg, $path_arg); $stdout = DiffusionMercurialCommandEngine::filterMercurialDebugOutput( diff --git a/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php index 9d2d6caadc..79587a2e5e 100644 --- a/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php @@ -77,7 +77,7 @@ final class DiffusionMergedCommitsQueryConduitAPIMethod list($parents) = $repository->execxLocalCommand( 'parents --template=%s --rev %s', '{node}\\n', - $commit); + hgsprintf('%s', $commit)); $parents = explode("\n", trim($parents)); if (count($parents) < 2) { diff --git a/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php index 389f931c0f..af973f102d 100644 --- a/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php @@ -97,7 +97,7 @@ final class DiffusionSearchQueryConduitAPIMethod $results = array(); $future = $repository->getLocalCommandFuture( - 'grep --rev %s --print0 --line-number %s %s', + 'grep --rev %s --print0 --line-number -- %s %s', hgsprintf('ancestors(%s)', $drequest->getStableCommit()), $grep, $path); diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index b4c0352d53..4985ac2bb7 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -969,6 +969,24 @@ final class DiffusionBrowseController extends DiffusionController { $handles = $viewer->loadHandles($phids); + $author_phids = array(); + $author_map = array(); + foreach ($blame_commits as $commit) { + $commit_identifier = $commit->getCommitIdentifier(); + + $author_phid = ''; + if (isset($revision_map[$commit_identifier])) { + $revision_id = $revision_map[$commit_identifier]; + $revision = $revisions[$revision_id]; + $author_phid = $revision->getAuthorPHID(); + } else { + $author_phid = $commit->getAuthorPHID(); + } + + $author_map[$commit_identifier] = $author_phid; + $author_phids[$author_phid] = $author_phid; + } + $colors = array(); if ($blame_commits) { $epochs = array(); @@ -1113,6 +1131,7 @@ final class DiffusionBrowseController extends DiffusionController { // blame outputs. $commit_links = $this->renderCommitLinks($blame_commits, $handles); $revision_links = $this->renderRevisionLinks($revisions, $handles); + $author_links = $this->renderAuthorLinks($author_map, $handles); if ($this->coverage) { require_celerity_resource('differential-changeset-view-css'); @@ -1127,6 +1146,10 @@ final class DiffusionBrowseController extends DiffusionController { )); } + $skip_text = pht('Skip Past This Commit'); + $skip_icon = id(new PHUIIconView()) + ->setIcon('fa-caret-square-o-left'); + foreach ($display as $line_index => $line) { $row = array(); @@ -1141,15 +1164,15 @@ final class DiffusionBrowseController extends DiffusionController { $revision_link = null; $commit_link = null; + $author_link = null; $before_link = null; - $commit_date = null; - $style = 'border-right: 3px solid '.$line['color'].';'; + $style = 'background: '.$line['color'].';'; if ($identifier && !$line['duplicate']) { if (isset($commit_links[$identifier])) { - $commit_link = $commit_links[$identifier]['link']; - $commit_date = $commit_links[$identifier]['date']; + $commit_link = $commit_links[$identifier]; + $author_link = $author_links[$author_map[$identifier]]; } if (isset($revision_map[$identifier])) { @@ -1160,10 +1183,6 @@ final class DiffusionBrowseController extends DiffusionController { } $skip_href = $line_href.'?before='.$identifier.'&view=blame'; - $skip_text = pht('Skip Past This Commit'); - $icon = id(new PHUIIconView()) - ->setIcon('fa-caret-square-o-left'); - $before_link = javelin_tag( 'a', array( @@ -1175,7 +1194,7 @@ final class DiffusionBrowseController extends DiffusionController { 'size' => 300, ), ), - $icon); + $skip_icon); } if ($show_blame) { @@ -1186,41 +1205,34 @@ final class DiffusionBrowseController extends DiffusionController { ), $before_link); - $row[] = phutil_tag( - 'th', - array( - 'class' => 'diffusion-rev-link', - ), - $commit_link); - - if ($revision_map) { - $row[] = phutil_tag( - 'th', - array( - 'class' => 'diffusion-blame-revision', - ), - $revision_link); + $object_links = array(); + $object_links[] = $author_link; + $object_links[] = $commit_link; + if ($revision_link) { + $object_links[] = phutil_tag('span', array(), '/'); + $object_links[] = $revision_link; } $row[] = phutil_tag( 'th', array( - 'class' => 'diffusion-blame-date', + 'class' => 'diffusion-rev-link', ), - $commit_date); + $object_links); } $line_link = phutil_tag( 'a', array( 'href' => $line_href, + 'style' => $style, ), $line_number); $row[] = javelin_tag( 'th', array( - 'class' => 'diffusion-line-link ', + 'class' => 'diffusion-line-link', 'sigil' => 'phabricator-source-line', 'style' => $style, ), @@ -1536,6 +1548,33 @@ final class DiffusionBrowseController extends DiffusionController { return head($parents); } + private function renderRevisionTooltip( + DifferentialRevision $revision, + $handles) { + $viewer = $this->getRequest()->getUser(); + + $date = phabricator_date($revision->getDateModified(), $viewer); + $id = $revision->getID(); + $title = $revision->getTitle(); + $header = "D{$id} {$title}"; + + $author = $handles[$revision->getAuthorPHID()]->getName(); + + return "{$header}\n{$date} \xC2\xB7 {$author}"; + } + + private function renderCommitTooltip( + PhabricatorRepositoryCommit $commit, + $author) { + + $viewer = $this->getRequest()->getUser(); + + $date = phabricator_date($commit->getEpoch(), $viewer); + $summary = trim($commit->getSummary()); + + return "{$summary}\n{$date} \xC2\xB7 {$author}"; + } + protected function markupText($text) { $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine(); $engine->setConfig('viewer', $this->getRequest()->getUser()); @@ -1743,6 +1782,9 @@ final class DiffusionBrowseController extends DiffusionController { ->setViewer($viewer) ->withRepository($repository) ->withIdentifiers($identifiers) + // TODO: We only fetch this to improve author display behavior, but + // shouldn't really need to? + ->needCommitData(true) ->execute(); $commits = mpull($commits, null, 'getCommitIdentifier'); } else { @@ -1752,29 +1794,59 @@ final class DiffusionBrowseController extends DiffusionController { return array($identifiers, $commits); } + private function renderAuthorLinks(array $authors, $handles) { + $links = array(); + + foreach ($authors as $phid) { + if (!strlen($phid)) { + // This means we couldn't identify an author for the commit or the + // revision. We just render a blank for alignment. + $style = null; + $href = null; + } else { + $src = $handles[$phid]->getImageURI(); + $style = 'background-image: url('.$src.');'; + $href = $handles[$phid]->getURI(); + } + + $links[$phid] = javelin_tag( + $href ? 'a' : 'span', + array( + 'class' => 'diffusion-author-link', + 'style' => $style, + 'href' => $href, + 'sigil' => 'has-tooltip', + 'meta' => array( + 'tip' => $handles[$phid]->getName(), + 'align' => 'E', + ), + )); + } + + return $links; + } + private function renderCommitLinks(array $commits, $handles) { $links = array(); - $viewer = $this->getViewer(); foreach ($commits as $identifier => $commit) { - $date = phabricator_date($commit->getEpoch(), $viewer); - $summary = trim($commit->getSummary()); + $tooltip = $this->renderCommitTooltip( + $commit, + $commit->renderAuthorShortName($handles)); - $commit_link = phutil_tag( + $commit_link = javelin_tag( 'a', array( 'href' => $commit->getURI(), + 'sigil' => 'has-tooltip', + 'meta' => array( + 'tip' => $tooltip, + 'align' => 'E', + 'size' => 600, + ), ), - $summary); + $commit->getLocalName()); - $commit_date = phutil_tag( - 'a', - array( - 'href' => $commit->getURI(), - ), - $date); - - $links[$identifier]['link'] = $commit_link; - $links[$identifier]['date'] = $commit_date; + $links[$identifier] = $commit_link; } return $links; @@ -1785,10 +1857,19 @@ final class DiffusionBrowseController extends DiffusionController { foreach ($revisions as $revision) { $revision_id = $revision->getID(); - $revision_link = phutil_tag( + + $tooltip = $this->renderRevisionTooltip($revision, $handles); + + $revision_link = javelin_tag( 'a', array( 'href' => '/'.$revision->getMonogram(), + 'sigil' => 'has-tooltip', + 'meta' => array( + 'tip' => $tooltip, + 'align' => 'E', + 'size' => 600, + ), ), $revision->getMonogram()); diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index d7734856f0..11463ef97d 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -39,20 +39,23 @@ final class DiffusionCommitController extends DiffusionController { return $this->buildRawDiffResponse($drequest); } - $commit = id(new DiffusionCommitQuery()) + $commits = id(new DiffusionCommitQuery()) ->setViewer($viewer) ->withRepository($repository) ->withIdentifiers(array($commit_identifier)) ->needCommitData(true) ->needAuditRequests(true) - ->executeOne(); + ->setLimit(100) + ->execute(); + + $multiple_results = count($commits) > 1; $crumbs = $this->buildCrumbs(array( - 'commit' => true, + 'commit' => !$multiple_results, )); $crumbs->setBorder(true); - if (!$commit) { + if (!$commits) { if (!$this->getCommitExists()) { return new Aphront404Response(); } @@ -70,7 +73,40 @@ final class DiffusionCommitController extends DiffusionController { ->setTitle($title) ->setCrumbs($crumbs) ->appendChild($error); + } else if ($multiple_results) { + $warning_message = + pht( + 'The identifier %s is ambiguous and matches more than one commit.', + phutil_tag( + 'strong', + array(), + $commit_identifier)); + + $error = id(new PHUIInfoView()) + ->setTitle(pht('Ambiguous Commit')) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->appendChild($warning_message); + + $list = id(new DiffusionCommitListView()) + ->setViewer($viewer) + ->setCommits($commits) + ->setNoDataString(pht('No recent commits.')); + + $crumbs->addTextCrumb(pht('Ambiguous Commit')); + + $matched_commits = id(new PHUITwoColumnView()) + ->setFooter(array( + $error, + $list, + )); + + return $this->newPage() + ->setTitle(pht('Ambiguous Commit')) + ->setCrumbs($crumbs) + ->appendChild($matched_commits); + } else { + $commit = head($commits); } $audit_requests = $commit->getAudits(); @@ -438,7 +474,8 @@ final class DiffusionCommitController extends DiffusionController { $repository = $drequest->getRepository(); $view = id(new PHUIPropertyListView()) - ->setUser($this->getRequest()->getUser()); + ->setUser($this->getRequest()->getUser()) + ->setObject($commit); $edge_query = id(new PhabricatorEdgeQuery()) ->withSourcePHIDs(array($commit_phid)) diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php index f64b72e1f8..35fce7f03d 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php @@ -46,8 +46,20 @@ final class DiffusionRepositoryController extends DiffusionController { ->withRepositoryPHIDs(array($repository->getPHID())) ->withRefTypes(array(PhabricatorRepositoryRefCursor::TYPE_BRANCH)) ->withRefNames(array($drequest->getBranch())) + ->needPositions(true) ->execute(); - if ($ref_cursors) { + + // It's possible that this branch previously existed, but has been + // deleted. Make sure we have valid cursor positions, not just cursors. + $any_positions = false; + foreach ($ref_cursors as $ref_cursor) { + if ($ref_cursor->getPositions()) { + $any_positions = true; + break; + } + } + + if ($any_positions) { // This is a valid branch, so we necessarily have some content. $page_has_content = true; } else { diff --git a/src/applications/diffusion/editor/DiffusionURIEditor.php b/src/applications/diffusion/editor/DiffusionURIEditor.php index 674efbc158..90ced865f0 100644 --- a/src/applications/diffusion/editor/DiffusionURIEditor.php +++ b/src/applications/diffusion/editor/DiffusionURIEditor.php @@ -347,11 +347,11 @@ final class DiffusionURIEditor continue; } - $io_type = $uri->getIoType(); + $io_type = $uri->getEffectiveIOType(); if ($io_type == PhabricatorRepositoryURI::IO_READWRITE) { if ($no_readwrite) { - $readwite_conflict = $uri; + $readwrite_conflict = $uri; break; } } diff --git a/src/applications/diffusion/engineextension/DiffusionQuickSearchEngineExtension.php b/src/applications/diffusion/engineextension/DiffusionQuickSearchEngineExtension.php new file mode 100644 index 0000000000..80c705b4f8 --- /dev/null +++ b/src/applications/diffusion/engineextension/DiffusionQuickSearchEngineExtension.php @@ -0,0 +1,12 @@ +splitArguments($test_command); + + foreach ($test_args as $test_arg) { + if (preg_match('/^--(config|debugger)/i', $test_arg)) { + throw new DiffusionMercurialFlagInjectionException( + pht( + 'Mercurial command appears to contain unsafe injected "--config" '. + 'or "--debugger": %s', + $test_command)); + } + } + // NOTE: Here, and in Git and Subversion, we override the SSH command even // if the repository does not use an SSH remote, since our SSH wrapper // defuses an attack against older versions of Mercurial, Git and diff --git a/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php b/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php index 1df3ea3522..50b7d4a5e2 100644 --- a/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php +++ b/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php @@ -123,6 +123,62 @@ final class DiffusionCommandEngineTestCase extends PhabricatorTestCase { 'argv' => 'xyz', 'protocol' => 'https', )); + + // Test that filtering defenses for "--config" and "--debugger" flag + // injections in Mercurial are functional. See T13012. + + $caught = null; + try { + $this->assertCommandEngineFormat( + '', + array(), + array( + 'vcs' => $type_hg, + 'argv' => '--debugger', + )); + } catch (DiffusionMercurialFlagInjectionException $ex) { + $caught = $ex; + } + + $this->assertTrue( + ($caught instanceof DiffusionMercurialFlagInjectionException), + pht('Expected "--debugger" injection in Mercurial to throw.')); + + + $caught = null; + try { + $this->assertCommandEngineFormat( + '', + array(), + array( + 'vcs' => $type_hg, + 'argv' => '--config=x', + )); + } catch (DiffusionMercurialFlagInjectionException $ex) { + $caught = $ex; + } + + $this->assertTrue( + ($caught instanceof DiffusionMercurialFlagInjectionException), + pht('Expected "--config" injection in Mercurial to throw.')); + + $caught = null; + try { + $this->assertCommandEngineFormat( + '', + array(), + array( + 'vcs' => $type_hg, + 'argv' => (string)csprintf('%s', '--config=x'), + )); + } catch (DiffusionMercurialFlagInjectionException $ex) { + $caught = $ex; + } + + $this->assertTrue( + ($caught instanceof DiffusionMercurialFlagInjectionException), + pht('Expected quoted "--config" injection in Mercurial to throw.')); + } private function assertCommandEngineFormat( diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php index da8937910e..8996bab628 100644 --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -177,7 +177,63 @@ final class DiffusionCommitQuery } protected function loadPage() { - return $this->loadStandardPage($this->newResultObject()); + $table = $this->newResultObject(); + $conn = $table->establishConnection('r'); + + $subqueries = array(); + if ($this->responsiblePHIDs) { + $base_authors = $this->authorPHIDs; + $base_auditors = $this->auditorPHIDs; + + $responsible_phids = $this->responsiblePHIDs; + if ($base_authors) { + $all_authors = array_merge($base_authors, $responsible_phids); + } else { + $all_authors = $responsible_phids; + } + + if ($base_auditors) { + $all_auditors = array_merge($base_auditors, $responsible_phids); + } else { + $all_auditors = $responsible_phids; + } + + $this->authorPHIDs = $all_authors; + $this->auditorPHIDs = $base_auditors; + $subqueries[] = $this->buildStandardPageQuery( + $conn, + $table->getTableName()); + + $this->authorPHIDs = $base_authors; + $this->auditorPHIDs = $all_auditors; + $subqueries[] = $this->buildStandardPageQuery( + $conn, + $table->getTableName()); + } else { + $subqueries[] = $this->buildStandardPageQuery( + $conn, + $table->getTableName()); + } + + if (count($subqueries) > 1) { + foreach ($subqueries as $key => $subquery) { + $subqueries[$key] = '('.$subquery.')'; + } + + $query = qsprintf( + $conn, + '%Q %Q %Q', + implode(' UNION DISTINCT ', $subqueries), + $this->buildOrderClause($conn, true), + $this->buildLimitClause($conn)); + } else { + $query = head($subqueries); + } + + $rows = queryfx_all($conn, '%Q', $query); + $rows = $this->didLoadRawRows($rows); + + return $table->loadAllFromArray($rows); } protected function willFilterPage(array $commits) { @@ -487,18 +543,10 @@ final class DiffusionCommitQuery $this->auditorPHIDs); } - if ($this->responsiblePHIDs !== null) { - $where[] = qsprintf( - $conn, - '(audit.auditorPHID IN (%Ls) OR commit.authorPHID IN (%Ls))', - $this->responsiblePHIDs, - $this->responsiblePHIDs); - } - if ($this->statuses !== null) { $where[] = qsprintf( $conn, - 'commit.auditStatus IN (%Ls)', + 'commit.auditStatus IN (%Ld)', $this->statuses); } @@ -541,10 +589,6 @@ final class DiffusionCommitQuery return ($this->auditIDs || $this->auditorPHIDs); } - private function shouldJoinAudit() { - return (bool)$this->responsiblePHIDs; - } - private function shouldJoinOwners() { return (bool)$this->packagePHIDs; } @@ -560,13 +604,6 @@ final class DiffusionCommitQuery $audit_request->getTableName()); } - if ($this->shouldJoinAudit()) { - $join[] = qsprintf( - $conn, - 'LEFT JOIN %T audit ON commit.phid = audit.commitPHID', - $audit_request->getTableName()); - } - if ($this->shouldJoinOwners()) { $join[] = qsprintf( $conn, @@ -584,10 +621,6 @@ final class DiffusionCommitQuery return true; } - if ($this->shouldJoinAudit()) { - return true; - } - if ($this->shouldJoinOwners()) { return true; } diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php index b649ca65a8..f9e9f74774 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php @@ -256,6 +256,66 @@ final class DiffusionLowLevelResolveRefsQuery return $results; } + // If some of the refs look like hashes, try to bulk resolve them. This + // workflow happens via RefEngine and bulk resolution is dramatically + // faster than individual resolution. See PHI158. + + $hashlike = array(); + foreach ($unresolved as $key => $ref) { + if (preg_match('/^[a-f0-9]{40}\z/', $ref)) { + $hashlike[$key] = $ref; + } + } + + if (count($hashlike) > 1) { + $hashlike_map = array(); + + $hashlike_groups = array_chunk($hashlike, 64, true); + foreach ($hashlike_groups as $hashlike_group) { + $hashlike_arg = array(); + foreach ($hashlike_group as $hashlike_ref) { + $hashlike_arg[] = hgsprintf('%s', $hashlike_ref); + } + $hashlike_arg = '('.implode(' or ', $hashlike_arg).')'; + + list($err, $refs) = $repository->execLocalCommand( + 'log --template=%s --rev %s', + '{node}\n', + $hashlike_arg); + if ($err) { + // NOTE: If any ref fails to resolve, Mercurial will exit with an + // error. We just give up on the whole group and resolve it + // individually below. In theory, we could split it into subgroups + // but the pathway where this bulk resolution matters rarely tries + // to resolve missing refs (see PHI158). + continue; + } + + $refs = phutil_split_lines($refs, false); + + foreach ($refs as $ref) { + $hashlike_map[$ref] = true; + } + } + + foreach ($unresolved as $key => $ref) { + if (!isset($hashlike_map[$ref])) { + continue; + } + + $results[$ref][] = array( + 'type' => 'commit', + 'identifier' => $ref, + ); + + unset($unresolved[$key]); + } + } + + if (!$unresolved) { + return $results; + } + // If we still have unresolved refs (which might be things like "tip"), // try to resolve them individually. diff --git a/src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php b/src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php index 39eed01226..9edcbb6380 100644 --- a/src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php +++ b/src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php @@ -16,14 +16,14 @@ final class DiffusionMercurialRawDiffQuery extends DiffusionRawDiffQuery { // If `$commit` has no parents (usually because it's the first commit // in the repository), we want to diff against `null`. This revset will // do that for us automatically. - $against = '('.$commit.'^ or null)'; + $against = hgsprintf('(%s^ or null)', $commit); } $future = $repository->getLocalCommandFuture( 'diff -U %d --git --rev %s --rev %s -- %s', $this->getLinesOfContext(), $against, - $commit, + hgsprintf('%s', $commit), $path); return $future; diff --git a/src/applications/diffusion/view/DiffusionPatternSearchView.php b/src/applications/diffusion/view/DiffusionPatternSearchView.php index a679b51400..93ef42d46d 100644 --- a/src/applications/diffusion/view/DiffusionPatternSearchView.php +++ b/src/applications/diffusion/view/DiffusionPatternSearchView.php @@ -96,10 +96,11 @@ final class DiffusionPatternSearchView extends DiffusionView { $path_title = Filesystem::readablePath($this->path, $drequest->getPath()); - $href = $drequest->generateURI(array( - 'action' => 'browse', - 'path' => $path_title, - )); + $href = $drequest->generateURI( + array( + 'action' => 'browse', + 'path' => $this->path, + )); $title = phutil_tag('a', array('href' => $href), $path_title); diff --git a/src/applications/drydock/storage/DrydockLease.php b/src/applications/drydock/storage/DrydockLease.php index 0dd4e36b48..e60529afe4 100644 --- a/src/applications/drydock/storage/DrydockLease.php +++ b/src/applications/drydock/storage/DrydockLease.php @@ -83,6 +83,9 @@ final class DrydockLease extends DrydockDAO 'key_resource' => array( 'columns' => array('resourcePHID', 'status'), ), + 'key_status' => array( + 'columns' => array('status'), + ), ), ) + parent::getConfiguration(); } diff --git a/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php b/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php index 4e461ec3f5..fcd6f97601 100644 --- a/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php +++ b/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php @@ -58,6 +58,14 @@ final class PhabricatorFilesComposeAvatarBuiltinFile } private function composeImage($color, $image, $border) { + // If we don't have the GD extension installed, just return a static + // default profile image rather than trying to compose a dynamic one. + if (!function_exists('imagecreatefromstring')) { + $root = dirname(phutil_get_library_root('phabricator')); + $default_path = $root.'/resources/builtin/profile.png'; + return Filesystem::readFile($default_path); + } + $color_const = hexdec(trim($color, '#')); $true_border = self::rgba2gd($border); $image_map = self::getImageMap(); diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php index a2375b9699..cb5c25336c 100644 --- a/src/applications/files/controller/PhabricatorFileDataController.php +++ b/src/applications/files/controller/PhabricatorFileDataController.php @@ -14,6 +14,10 @@ final class PhabricatorFileDataController extends PhabricatorFileController { return false; } + public function shouldAllowPartialSessions() { + return true; + } + public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); $this->phid = $request->getURIData('phid'); diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index cbb2fcd051..b151dee5ed 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -134,6 +134,9 @@ final class PhabricatorFile extends PhabricatorFileDAO 'columns' => array('builtinKey'), 'unique' => true, ), + 'key_engine' => array( + 'columns' => array('storageEngine', 'storageHandle(64)'), + ), ), ) + parent::getConfiguration(); } diff --git a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php index 76bdf9ccc3..8c718e5f5d 100644 --- a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php +++ b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php @@ -7,6 +7,12 @@ final class HarbormasterRunBuildPlansHeraldAction const ACTIONCONST = 'harbormaster.build'; + public function getRequiredAdapterStates() { + return array( + HeraldBuildableState::STATECONST, + ); + } + public function getActionGroupKey() { return HeraldSupportActionGroup::ACTIONGROUPKEY; } diff --git a/src/applications/harbormaster/query/HarbormasterBuildQuery.php b/src/applications/harbormaster/query/HarbormasterBuildQuery.php index cf6db6115b..770ca4413b 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildQuery.php +++ b/src/applications/harbormaster/query/HarbormasterBuildQuery.php @@ -10,6 +10,7 @@ final class HarbormasterBuildQuery private $buildPlanPHIDs; private $initiatorPHIDs; private $needBuildTargets; + private $autobuilds; public function withIDs(array $ids) { $this->ids = $ids; @@ -41,6 +42,11 @@ final class HarbormasterBuildQuery return $this; } + public function withAutobuilds($with_autobuilds) { + $this->autobuilds = $with_autobuilds; + return $this; + } + public function needBuildTargets($need_targets) { $this->needBuildTargets = $need_targets; return $this; @@ -141,50 +147,87 @@ final class HarbormasterBuildQuery if ($this->ids !== null) { $where[] = qsprintf( $conn, - 'id IN (%Ld)', + 'b.id IN (%Ld)', $this->ids); } if ($this->phids !== null) { $where[] = qsprintf( $conn, - 'phid in (%Ls)', + 'b.phid in (%Ls)', $this->phids); } if ($this->buildStatuses !== null) { $where[] = qsprintf( $conn, - 'buildStatus in (%Ls)', + 'b.buildStatus in (%Ls)', $this->buildStatuses); } if ($this->buildablePHIDs !== null) { $where[] = qsprintf( $conn, - 'buildablePHID IN (%Ls)', + 'b.buildablePHID IN (%Ls)', $this->buildablePHIDs); } if ($this->buildPlanPHIDs !== null) { $where[] = qsprintf( $conn, - 'buildPlanPHID IN (%Ls)', + 'b.buildPlanPHID IN (%Ls)', $this->buildPlanPHIDs); } if ($this->initiatorPHIDs !== null) { $where[] = qsprintf( $conn, - 'initiatorPHID IN (%Ls)', + 'b.initiatorPHID IN (%Ls)', $this->initiatorPHIDs); } + if ($this->autobuilds !== null) { + if ($this->autobuilds) { + $where[] = qsprintf( + $conn, + 'p.planAutoKey IS NOT NULL'); + } else { + $where[] = qsprintf( + $conn, + 'p.planAutoKey IS NULL'); + } + } + return $where; } + protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { + $joins = parent::buildJoinClauseParts($conn); + + if ($this->shouldJoinPlanTable()) { + $joins[] = qsprintf( + $conn, + 'JOIN %T p ON b.buildPlanPHID = p.phid', + id(new HarbormasterBuildPlan())->getTableName()); + } + + return $joins; + } + + private function shouldJoinPlanTable() { + if ($this->autobuilds !== null) { + return true; + } + + return false; + } + public function getQueryApplicationClass() { return 'PhabricatorHarbormasterApplication'; } + protected function getPrimaryTableAlias() { + return 'b'; + } + } diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php b/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php index 6825bf6646..461ef4b06f 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php @@ -43,6 +43,9 @@ final class HarbormasterBuildArtifact extends HarbormasterDAO 'key_target' => array( 'columns' => array('buildTargetPHID', 'artifactType'), ), + 'key_index' => array( + 'columns' => array('artifactIndex'), + ), ), ) + parent::getConfiguration(); } diff --git a/src/applications/herald/action/HeraldAction.php b/src/applications/herald/action/HeraldAction.php index 091f3e9bc2..a2d589f9f2 100644 --- a/src/applications/herald/action/HeraldAction.php +++ b/src/applications/herald/action/HeraldAction.php @@ -17,6 +17,7 @@ abstract class HeraldAction extends Phobject { const DO_STANDARD_PERMISSION = 'do.standard.permission'; const DO_STANDARD_INVALID_ACTION = 'do.standard.invalid-action'; const DO_STANDARD_WRONG_RULE_TYPE = 'do.standard.wrong-rule-type'; + const DO_STANDARD_FORBIDDEN = 'do.standard.forbidden'; abstract public function getHeraldActionName(); abstract public function supportsObject($object); @@ -25,6 +26,10 @@ abstract class HeraldAction extends Phobject { abstract public function renderActionDescription($value); + public function getRequiredAdapterStates() { + return array(); + } + protected function renderActionEffectDescription($type, $data) { return null; } @@ -336,6 +341,11 @@ abstract class HeraldAction extends Phobject { 'color' => 'red', 'name' => pht('Wrong Rule Type'), ), + self::DO_STANDARD_FORBIDDEN => array( + 'icon' => 'fa-ban', + 'color' => 'violet', + 'name' => pht('Forbidden'), + ), ); } @@ -381,6 +391,8 @@ abstract class HeraldAction extends Phobject { return pht( 'This action does not support rules of type "%s".', $data); + case self::DO_STANDARD_FORBIDDEN: + return HeraldStateReasons::getExplanation($data); } return null; diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 78ce862945..1c50c0b5ee 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -37,6 +37,7 @@ abstract class HeraldAdapter extends Phobject { private $fieldMap; private $actionMap; private $edgeCache = array(); + private $forbiddenActions = array(); public function getEmailPHIDs() { return array_values($this->emailPHIDs); @@ -1116,4 +1117,38 @@ abstract class HeraldAdapter extends Phobject { return $this->edgeCache[$type]; } + +/* -( Forbidden Actions )-------------------------------------------------- */ + + + final public function getForbiddenActions() { + return array_keys($this->forbiddenActions); + } + + final public function setForbiddenAction($action, $reason) { + $this->forbiddenActions[$action] = $reason; + return $this; + } + + final public function getRequiredFieldStates($field_key) { + return $this->requireFieldImplementation($field_key) + ->getRequiredAdapterStates(); + } + + final public function getRequiredActionStates($action_key) { + return $this->requireActionImplementation($action_key) + ->getRequiredAdapterStates(); + } + + final public function getForbiddenReason($action) { + if (!isset($this->forbiddenActions[$action])) { + throw new Exception( + pht( + 'Action "%s" is not forbidden!', + $action)); + } + + return $this->forbiddenActions[$action]; + } + } diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php index a9509beb33..dc7ca676e3 100644 --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -273,7 +273,11 @@ final class HeraldTranscriptController extends HeraldController { ->setTarget(phutil_tag('strong', array(), pht('Conditions')))); foreach ($cond_xscripts as $cond_xscript) { - if ($cond_xscript->getResult()) { + if ($cond_xscript->isForbidden()) { + $icon = 'fa-ban'; + $color = 'indigo'; + $result = pht('Forbidden'); + } else if ($cond_xscript->getResult()) { $icon = 'fa-check'; $color = 'green'; $result = pht('Passed'); @@ -284,12 +288,17 @@ final class HeraldTranscriptController extends HeraldController { } if ($cond_xscript->getNote()) { + $note_text = $cond_xscript->getNote(); + if ($cond_xscript->isForbidden()) { + $note_text = HeraldStateReasons::getExplanation($note_text); + } + $note = phutil_tag( 'div', array( 'class' => 'herald-condition-note', ), - $cond_xscript->getNote()); + $note_text); } else { $note = null; } @@ -310,7 +319,12 @@ final class HeraldTranscriptController extends HeraldController { $cond_list->addItem($cond_item); } - if ($rule_xscript->getResult()) { + if ($rule_xscript->isForbidden()) { + $last_icon = 'fa-ban'; + $last_color = 'indigo'; + $last_result = pht('Forbidden'); + $last_note = pht('Object state prevented rule evaluation.'); + } else if ($rule_xscript->getResult()) { $last_icon = 'fa-check-circle'; $last_color = 'green'; $last_result = pht('Passed'); diff --git a/src/applications/herald/engine/HeraldEngine.php b/src/applications/herald/engine/HeraldEngine.php index df81117b80..b8e1db5626 100644 --- a/src/applications/herald/engine/HeraldEngine.php +++ b/src/applications/herald/engine/HeraldEngine.php @@ -12,6 +12,9 @@ final class HeraldEngine extends Phobject { protected $object; private $dryRun; + private $forbiddenFields = array(); + private $forbiddenActions = array(); + public function setDryRun($dry_run) { $this->dryRun = $dry_run; return $this; @@ -76,39 +79,42 @@ final class HeraldEngine extends Phobject { // This is not a dry run, and this rule is only supposed to be // applied a single time, and it's already been applied... // That means automatic failure. - $xscript = id(new HeraldRuleTranscript()) - ->setRuleID($rule->getID()) + $this->newRuleTranscript($rule) ->setResult(false) - ->setRuleName($rule->getName()) - ->setRuleOwner($rule->getAuthorPHID()) ->setReason( pht( 'This rule is only supposed to be repeated a single time, '. 'and it has already been applied.')); - $this->transcript->addRuleTranscript($xscript); + $rule_matches = false; } else { - $rule_matches = $this->doesRuleMatch($rule, $object); + if ($this->isForbidden($rule, $object)) { + $this->newRuleTranscript($rule) + ->setResult(HeraldRuleTranscript::RESULT_FORBIDDEN) + ->setReason( + pht( + 'Object state is not compatible with rule.')); + + $rule_matches = false; + } else { + $rule_matches = $this->doesRuleMatch($rule, $object); + } } } catch (HeraldRecursiveConditionsException $ex) { $names = array(); - foreach ($this->stack as $rule_id => $ignored) { - $names[] = '"'.$rules[$rule_id]->getName().'"'; + foreach ($this->stack as $rule_phid => $ignored) { + $names[] = '"'.$rules[$rule_phid]->getName().'"'; } $names = implode(', ', $names); - foreach ($this->stack as $rule_id => $ignored) { - $xscript = new HeraldRuleTranscript(); - $xscript->setRuleID($rule_id); - $xscript->setResult(false); - $xscript->setReason( - pht( - "Rules %s are recursively dependent upon one another! ". - "Don't do this! You have formed an unresolvable cycle in the ". - "dependency graph!", - $names)); - $xscript->setRuleName($rules[$rule_id]->getName()); - $xscript->setRuleOwner($rules[$rule_id]->getAuthorPHID()); - $this->transcript->addRuleTranscript($xscript); + foreach ($this->stack as $rule_phid => $ignored) { + $this->newRuleTranscript($rules[$rule_phid]) + ->setResult(false) + ->setReason( + pht( + "Rules %s are recursively dependent upon one another! ". + "Don't do this! You have formed an unresolvable cycle in the ". + "dependency graph!", + $names)); } $rule_matches = false; } @@ -309,14 +315,9 @@ final class HeraldEngine extends Phobject { } } - $rule_transcript = new HeraldRuleTranscript(); - $rule_transcript->setRuleID($rule->getID()); - $rule_transcript->setResult($result); - $rule_transcript->setReason($reason); - $rule_transcript->setRuleName($rule->getName()); - $rule_transcript->setRuleOwner($rule->getAuthorPHID()); - - $this->transcript->addRuleTranscript($rule_transcript); + $this->newRuleTranscript($rule) + ->setResult($result) + ->setReason($reason); return $result; } @@ -327,16 +328,7 @@ final class HeraldEngine extends Phobject { HeraldAdapter $object) { $object_value = $this->getConditionObjectValue($condition, $object); - $test_value = $condition->getValue(); - - $cond = $condition->getFieldCondition(); - - $transcript = new HeraldConditionTranscript(); - $transcript->setRuleID($rule->getID()); - $transcript->setConditionID($condition->getID()); - $transcript->setFieldName($condition->getFieldName()); - $transcript->setCondition($cond); - $transcript->setTestValue($test_value); + $transcript = $this->newConditionTranscript($rule, $condition); try { $result = $object->doesConditionMatch( @@ -351,8 +343,6 @@ final class HeraldEngine extends Phobject { $transcript->setResult($result); - $this->transcript->addConditionTranscript($transcript); - return $result; } @@ -446,4 +436,136 @@ final class HeraldEngine extends Phobject { return false; } + private function newRuleTranscript(HeraldRule $rule) { + $xscript = id(new HeraldRuleTranscript()) + ->setRuleID($rule->getID()) + ->setRuleName($rule->getName()) + ->setRuleOwner($rule->getAuthorPHID()); + + $this->transcript->addRuleTranscript($xscript); + + return $xscript; + } + + private function newConditionTranscript( + HeraldRule $rule, + HeraldCondition $condition) { + + $xscript = id(new HeraldConditionTranscript()) + ->setRuleID($rule->getID()) + ->setConditionID($condition->getID()) + ->setFieldName($condition->getFieldName()) + ->setCondition($condition->getFieldCondition()) + ->setTestValue($condition->getValue()); + + $this->transcript->addConditionTranscript($xscript); + + return $xscript; + } + + private function newApplyTranscript( + HeraldAdapter $adapter, + HeraldRule $rule, + HeraldActionRecord $action) { + + $effect = id(new HeraldEffect()) + ->setObjectPHID($adapter->getPHID()) + ->setAction($action->getAction()) + ->setTarget($action->getTarget()) + ->setRule($rule); + + $xscript = new HeraldApplyTranscript($effect, false); + + $this->transcript->addApplyTranscript($xscript); + + return $xscript; + } + + private function isForbidden( + HeraldRule $rule, + HeraldAdapter $adapter) { + + $forbidden = $adapter->getForbiddenActions(); + if (!$forbidden) { + return false; + } + + $forbidden = array_fuse($forbidden); + + $is_forbidden = false; + + foreach ($rule->getConditions() as $condition) { + $field_key = $condition->getFieldName(); + + if (!isset($this->forbiddenFields[$field_key])) { + $reason = null; + + try { + $states = $adapter->getRequiredFieldStates($field_key); + } catch (Exception $ex) { + $states = array(); + } + + foreach ($states as $state) { + if (!isset($forbidden[$state])) { + continue; + } + $reason = $adapter->getForbiddenReason($state); + break; + } + + $this->forbiddenFields[$field_key] = $reason; + } + + $forbidden_reason = $this->forbiddenFields[$field_key]; + if ($forbidden_reason !== null) { + $this->newConditionTranscript($rule, $condition) + ->setResult(HeraldConditionTranscript::RESULT_FORBIDDEN) + ->setNote($forbidden_reason); + + $is_forbidden = true; + } + } + + foreach ($rule->getActions() as $action_record) { + $action_key = $action_record->getAction(); + + if (!isset($this->forbiddenActions[$action_key])) { + $reason = null; + + try { + $states = $adapter->getRequiredActionStates($action_key); + } catch (Exception $ex) { + $states = array(); + } + + foreach ($states as $state) { + if (!isset($forbidden[$state])) { + continue; + } + $reason = $adapter->getForbiddenReason($state); + break; + } + + $this->forbiddenActions[$action_key] = $reason; + } + + $forbidden_reason = $this->forbiddenActions[$action_key]; + if ($forbidden_reason !== null) { + $this->newApplyTranscript($adapter, $rule, $action_record) + ->setAppliedReason( + array( + array( + 'type' => HeraldAction::DO_STANDARD_FORBIDDEN, + 'data' => $forbidden_reason, + ), + )); + + $is_forbidden = true; + } + } + + return $is_forbidden; + } + } diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php index 2abed0ff1d..408a76f7fb 100644 --- a/src/applications/herald/field/HeraldField.php +++ b/src/applications/herald/field/HeraldField.php @@ -20,6 +20,10 @@ abstract class HeraldField extends Phobject { return null; } + public function getRequiredAdapterStates() { + return array(); + } + protected function getHeraldFieldStandardType() { throw new PhutilMethodNotImplementedException(); } diff --git a/src/applications/herald/state/HeraldBuildableState.php b/src/applications/herald/state/HeraldBuildableState.php new file mode 100644 index 0000000000..d19ae87c36 --- /dev/null +++ b/src/applications/herald/state/HeraldBuildableState.php @@ -0,0 +1,7 @@ +setAncestorClass(__CLASS__) + ->execute(); + } + + final public static function getExplanation($reason) { + $reasons = self::getAllReasons(); + + foreach ($reasons as $reason_implementation) { + $explanation = $reason_implementation->explainReason($reason); + if ($explanation !== null) { + return $explanation; + } + } + + return pht('Unknown reason ("%s").', $reason); + } + +} diff --git a/src/applications/herald/storage/transcript/HeraldConditionTranscript.php b/src/applications/herald/storage/transcript/HeraldConditionTranscript.php index 36a534e6b0..a7096b6880 100644 --- a/src/applications/herald/storage/transcript/HeraldConditionTranscript.php +++ b/src/applications/herald/storage/transcript/HeraldConditionTranscript.php @@ -10,6 +10,8 @@ final class HeraldConditionTranscript extends Phobject { protected $note; protected $result; + const RESULT_FORBIDDEN = 'forbidden'; + public function setRuleID($rule_id) { $this->ruleID = $rule_id; return $this; @@ -72,4 +74,9 @@ final class HeraldConditionTranscript extends Phobject { public function getResult() { return $this->result; } + + public function isForbidden() { + return ($this->getResult() === self::RESULT_FORBIDDEN); + } + } diff --git a/src/applications/herald/storage/transcript/HeraldRuleTranscript.php b/src/applications/herald/storage/transcript/HeraldRuleTranscript.php index d12f1f9dd9..8928a74cf6 100644 --- a/src/applications/herald/storage/transcript/HeraldRuleTranscript.php +++ b/src/applications/herald/storage/transcript/HeraldRuleTranscript.php @@ -9,6 +9,12 @@ final class HeraldRuleTranscript extends Phobject { protected $ruleName; protected $ruleOwner; + const RESULT_FORBIDDEN = 'forbidden'; + + public function isForbidden() { + return ($this->getResult() === self::RESULT_FORBIDDEN); + } + public function setResult($result) { $this->result = $result; return $this; diff --git a/src/applications/legalpad/query/LegalpadDocumentQuery.php b/src/applications/legalpad/query/LegalpadDocumentQuery.php index 655da955e4..3d79e9f3a1 100644 --- a/src/applications/legalpad/query/LegalpadDocumentQuery.php +++ b/src/applications/legalpad/query/LegalpadDocumentQuery.php @@ -77,23 +77,12 @@ final class LegalpadDocumentQuery return $this; } + public function newResultObject() { + return new LegalpadDocument(); + } + protected function loadPage() { - $table = new LegalpadDocument(); - $conn_r = $table->establishConnection('r'); - - $data = queryfx_all( - $conn_r, - 'SELECT d.* FROM %T d %Q %Q %Q %Q %Q', - $table->getTableName(), - $this->buildJoinClause($conn_r), - $this->buildWhereClause($conn_r), - $this->buildGroupClause($conn_r), - $this->buildOrderClause($conn_r), - $this->buildLimitClause($conn_r)); - - $documents = $table->loadAllFromArray($data); - - return $documents; + return $this->loadStandardPage($this->newResultObject()); } protected function willFilterPage(array $documents) { @@ -134,12 +123,12 @@ final class LegalpadDocumentQuery return $documents; } - protected function buildJoinClause(AphrontDatabaseConnection $conn_r) { - $joins = array(); + protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { + $joins = parent::buildJoinClauseParts($conn); if ($this->contributorPHIDs !== null) { $joins[] = qsprintf( - $conn_r, + $conn, 'JOIN edge contributor ON contributor.src = d.phid AND contributor.type = %d', PhabricatorObjectHasContributorEdgeType::EDGECONST); @@ -147,79 +136,81 @@ final class LegalpadDocumentQuery if ($this->signerPHIDs !== null) { $joins[] = qsprintf( - $conn_r, + $conn, 'JOIN %T signer ON signer.documentPHID = d.phid AND signer.signerPHID IN (%Ls)', id(new LegalpadDocumentSignature())->getTableName(), $this->signerPHIDs); } - return implode(' ', $joins); + return $joins; } - protected function buildGroupClause(AphrontDatabaseConnection $conn_r) { - if ($this->contributorPHIDs || $this->signerPHIDs) { - return 'GROUP BY d.id'; - } else { - return ''; + protected function shouldGroupQueryResultRows() { + if ($this->contributorPHIDs) { + return true; } + + if ($this->signerPHIDs) { + return true; + } + + return parent::shouldGroupQueryResultRows(); } - protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); if ($this->ids !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'd.id IN (%Ld)', $this->ids); } if ($this->phids !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'd.phid IN (%Ls)', $this->phids); } if ($this->creatorPHIDs !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'd.creatorPHID IN (%Ls)', $this->creatorPHIDs); } if ($this->dateCreatedAfter !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'd.dateCreated >= %d', $this->dateCreatedAfter); } if ($this->dateCreatedBefore !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'd.dateCreated <= %d', $this->dateCreatedBefore); } if ($this->contributorPHIDs !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'contributor.dst IN (%Ls)', $this->contributorPHIDs); } if ($this->signatureRequired !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'd.requireSignature = %d', $this->signatureRequired); } - $where[] = $this->buildPagingClause($conn_r); - - return $this->formatWhereClause($where); + return $where; } private function loadDocumentBodies(array $documents) { @@ -275,4 +266,8 @@ final class LegalpadDocumentQuery return 'PhabricatorLegalpadApplication'; } + protected function getPrimaryTableAlias() { + return 'd'; + } + } diff --git a/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php b/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php index 8b218cf821..1b608b02e3 100644 --- a/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php +++ b/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php @@ -11,105 +11,66 @@ final class LegalpadDocumentSearchEngine return 'PhabricatorLegalpadApplication'; } - public function buildSavedQueryFromRequest(AphrontRequest $request) { - $saved = new PhabricatorSavedQuery(); - $saved->setParameter( - 'creatorPHIDs', - $this->readUsersFromRequest($request, 'creators')); - - $saved->setParameter( - 'contributorPHIDs', - $this->readUsersFromRequest($request, 'contributors')); - - $saved->setParameter( - 'withViewerSignature', - $request->getBool('withViewerSignature')); - - $saved->setParameter('createdStart', $request->getStr('createdStart')); - $saved->setParameter('createdEnd', $request->getStr('createdEnd')); - - return $saved; + public function newQuery() { + return id(new LegalpadDocumentQuery()) + ->needViewerSignatures(true); } - public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { - $query = id(new LegalpadDocumentQuery()) - ->needViewerSignatures(true); + protected function buildCustomSearchFields() { + return array( + id(new PhabricatorUsersSearchField()) + ->setLabel(pht('Signed By')) + ->setKey('signerPHIDs') + ->setAliases(array('signer', 'signers', 'signerPHID')) + ->setDescription( + pht('Search for documents signed by given users.')), + id(new PhabricatorUsersSearchField()) + ->setLabel(pht('Creators')) + ->setKey('creatorPHIDs') + ->setAliases(array('creator', 'creators', 'creatorPHID')) + ->setDescription( + pht('Search for documents with given creators.')), + id(new PhabricatorUsersSearchField()) + ->setLabel(pht('Contributors')) + ->setKey('contributorPHIDs') + ->setAliases(array('contributor', 'contributors', 'contributorPHID')) + ->setDescription( + pht('Search for documents with given contributors.')), + id(new PhabricatorSearchDateField()) + ->setLabel(pht('Created After')) + ->setKey('createdStart'), + id(new PhabricatorSearchDateField()) + ->setLabel(pht('Created Before')) + ->setKey('createdEnd'), + ); + } - $creator_phids = $saved->getParameter('creatorPHIDs', array()); - if ($creator_phids) { - $query->withCreatorPHIDs($creator_phids); + protected function buildQueryFromParameters(array $map) { + $query = $this->newQuery(); + + if ($map['signerPHIDs']) { + $query->withSignerPHIDs($map['signerPHIDs']); } - $contributor_phids = $saved->getParameter('contributorPHIDs', array()); - if ($contributor_phids) { - $query->withContributorPHIDs($contributor_phids); + if ($map['contributorPHIDs']) { + $query->withContributorPHIDs($map['creatorPHIDs']); } - if ($saved->getParameter('withViewerSignature')) { - $viewer_phid = $this->requireViewer()->getPHID(); - if ($viewer_phid) { - $query->withSignerPHIDs(array($viewer_phid)); - } + if ($map['creatorPHIDs']) { + $query->withCreatorPHIDs($map['creatorPHIDs']); } - $start = $this->parseDateTime($saved->getParameter('createdStart')); - $end = $this->parseDateTime($saved->getParameter('createdEnd')); - - if ($start) { - $query->withDateCreatedAfter($start); + if ($map['createdStart']) { + $query->withDateCreatedAfter($map['createdStart']); } - if ($end) { - $query->withDateCreatedBefore($end); + if ($map['createdEnd']) { + $query->withDateCreatedAfter($map['createdStart']); } return $query; } - public function buildSearchForm( - AphrontFormView $form, - PhabricatorSavedQuery $saved_query) { - - $creator_phids = $saved_query->getParameter('creatorPHIDs', array()); - $contributor_phids = $saved_query->getParameter( - 'contributorPHIDs', array()); - - $viewer_signature = $saved_query->getParameter('withViewerSignature'); - if (!$this->requireViewer()->getPHID()) { - $viewer_signature = false; - } - - $form - ->appendChild( - id(new AphrontFormCheckboxControl()) - ->addCheckbox( - 'withViewerSignature', - 1, - pht('Show only documents I have signed.'), - $viewer_signature) - ->setDisabled(!$this->requireViewer()->getPHID())) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorPeopleDatasource()) - ->setName('creators') - ->setLabel(pht('Creators')) - ->setValue($creator_phids)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorPeopleDatasource()) - ->setName('contributors') - ->setLabel(pht('Contributors')) - ->setValue($contributor_phids)); - - $this->buildDateRange( - $form, - $saved_query, - 'createdStart', - pht('Created After'), - 'createdEnd', - pht('Created Before')); - } - protected function getURI($path) { return '/legalpad/'.$path; } @@ -130,10 +91,11 @@ final class LegalpadDocumentSearchEngine $query = $this->newSavedQuery(); $query->setQueryKey($query_key); + $viewer = $this->requireViewer(); + switch ($query_key) { case 'signed': - return $query - ->setParameter('withViewerSignature', true); + return $query->setParameter('signerPHIDs', array($viewer->getPHID())); case 'all': return $query; } @@ -141,12 +103,6 @@ final class LegalpadDocumentSearchEngine return parent::buildSavedQueryFromBuiltin($query_key); } - protected function getRequiredHandlePHIDsForResultList( - array $documents, - PhabricatorSavedQuery $query) { - return array(); - } - protected function renderResultList( array $documents, PhabricatorSavedQuery $query, diff --git a/src/applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php index 8b0d0496cf..357d118bc4 100644 --- a/src/applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php +++ b/src/applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php @@ -21,6 +21,16 @@ final class ManiphestGetTaskTransactionsConduitAPIMethod return 'nonempty list>'; } + public function getMethodStatus() { + return self::METHOD_STATUS_FROZEN; + } + + public function getMethodStatusDescription() { + return pht( + 'This method is frozen and will eventually be deprecated. New code '. + 'should use "transaction.search" instead.'); + } + protected function execute(ConduitAPIRequest $request) { $results = array(); $task_ids = $request->getValue('ids'); diff --git a/src/applications/maniphest/xaction/ManiphestTaskDescriptionTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskDescriptionTransaction.php index 009327ed9b..fec1a87604 100644 --- a/src/applications/maniphest/xaction/ManiphestTaskDescriptionTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskDescriptionTransaction.php @@ -57,5 +57,15 @@ final class ManiphestTaskDescriptionTransaction return $changes; } + public function getTransactionTypeForConduit($xaction) { + return 'description'; + } + + public function getFieldValuesForConduit($xaction, $data) { + return array( + 'old' => $xaction->getOldValue(), + 'new' => $xaction->getNewValue(), + ); + } } diff --git a/src/applications/maniphest/xaction/ManiphestTaskOwnerTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskOwnerTransaction.php index d510fe8fbc..caaf84f542 100644 --- a/src/applications/maniphest/xaction/ManiphestTaskOwnerTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskOwnerTransaction.php @@ -154,5 +154,15 @@ final class ManiphestTaskOwnerTransaction } + public function getTransactionTypeForConduit($xaction) { + return 'owner'; + } + + public function getFieldValuesForConduit($xaction, $data) { + return array( + 'old' => $xaction->getOldValue(), + 'new' => $xaction->getNewValue(), + ); + } } diff --git a/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php index 8953664f27..5a69199874 100644 --- a/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php @@ -107,4 +107,16 @@ final class ManiphestTaskPointsTransaction return $value; } + public function getTransactionTypeForConduit($xaction) { + return 'points'; + } + + public function getFieldValuesForConduit($xaction, $data) { + return array( + 'old' => $xaction->getOldValue(), + 'new' => $xaction->getNewValue(), + ); + } + + } diff --git a/src/applications/maniphest/xaction/ManiphestTaskPriorityTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskPriorityTransaction.php index 2ed7c4e15b..83f38fc659 100644 --- a/src/applications/maniphest/xaction/ManiphestTaskPriorityTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskPriorityTransaction.php @@ -6,20 +6,17 @@ final class ManiphestTaskPriorityTransaction const TRANSACTIONTYPE = 'priority'; public function generateOldValue($object) { - if ($this->isNewObject()) { - return null; - } - return $object->getPriority(); + return (string)$object->getPriority(); } public function generateNewValue($object, $value) { // `$value` is supposed to be a keyword, but if the priority // assigned to a task has been removed from the config, // no such keyword will be available. Other edits to the task - // should still be allowed, even if the priority is no longer + // should still be allowed, even if the priority is no longer // valid, so treat this as a no-op. if ($value === ManiphestTaskPriority::UNKNOWN_PRIORITY_KEYWORD) { - return $object->getPriority(); + return (string)$object->getPriority(); } return (string)ManiphestTaskPriority::getTaskPriorityFromKeyword($value); diff --git a/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php index 5e1cd44611..dd51a63799 100644 --- a/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php @@ -6,9 +6,6 @@ final class ManiphestTaskStatusTransaction const TRANSACTIONTYPE = 'status'; public function generateOldValue($object) { - if ($this->isNewObject()) { - return null; - } return $object->getStatus(); } @@ -229,4 +226,15 @@ final class ManiphestTaskStatusTransaction } + public function getTransactionTypeForConduit($xaction) { + return 'status'; + } + + public function getFieldValuesForConduit($xaction, $data) { + return array( + 'old' => $xaction->getOldValue(), + 'new' => $xaction->getNewValue(), + ); + } + } diff --git a/src/applications/maniphest/xaction/ManiphestTaskTitleTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskTitleTransaction.php index 506817b0fc..e4ec2a132f 100644 --- a/src/applications/maniphest/xaction/ManiphestTaskTitleTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskTitleTransaction.php @@ -72,4 +72,15 @@ final class ManiphestTaskTitleTransaction return $errors; } + public function getTransactionTypeForConduit($xaction) { + return 'title'; + } + + public function getFieldValuesForConduit($xaction, $data) { + return array( + 'old' => $xaction->getOldValue(), + 'new' => $xaction->getNewValue(), + ); + } + } diff --git a/src/applications/meta/engineextension/PhabricatorQuickSearchApplicationEngineExtension.php b/src/applications/meta/engineextension/PhabricatorQuickSearchApplicationEngineExtension.php new file mode 100644 index 0000000000..8161c863ba --- /dev/null +++ b/src/applications/meta/engineextension/PhabricatorQuickSearchApplicationEngineExtension.php @@ -0,0 +1,11 @@ +isLoggedIn(); } + public function shouldRequireFullSession() { + return false; + } + public function getExtensionOrder() { return 1200; } @@ -65,42 +69,44 @@ final class PeopleMainMenuBarExtension $view = id(new PhabricatorActionListView()) ->setViewer($viewer); - $view->addAction( - id(new PhabricatorActionView()) - ->appendChild($user_view)); + if ($this->getIsFullSession()) { + $view->addAction( + id(new PhabricatorActionView()) + ->appendChild($user_view)); - $view->addAction( - id(new PhabricatorActionView()) - ->setType(PhabricatorActionView::TYPE_DIVIDER)); + $view->addAction( + id(new PhabricatorActionView()) + ->setType(PhabricatorActionView::TYPE_DIVIDER)); - $view->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Profile')) - ->setHref('/p/'.$viewer->getUsername().'/')); + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Profile')) + ->setHref('/p/'.$viewer->getUsername().'/')); - $view->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Settings')) - ->setHref('/settings/user/'.$viewer->getUsername().'/')); + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Settings')) + ->setHref('/settings/user/'.$viewer->getUsername().'/')); - $view->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Manage')) - ->setHref('/people/manage/'.$viewer->getID().'/')); + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Manage')) + ->setHref('/people/manage/'.$viewer->getID().'/')); - if ($application) { - $help_links = $application->getHelpMenuItems($viewer); - if ($help_links) { - foreach ($help_links as $link) { - $view->addAction($link); + if ($application) { + $help_links = $application->getHelpMenuItems($viewer); + if ($help_links) { + foreach ($help_links as $link) { + $view->addAction($link); + } } } - } - $view->addAction( - id(new PhabricatorActionView()) - ->addSigil('logout-item') - ->setType(PhabricatorActionView::TYPE_DIVIDER)); + $view->addAction( + id(new PhabricatorActionView()) + ->addSigil('logout-item') + ->setType(PhabricatorActionView::TYPE_DIVIDER)); + } $view->addAction( id(new PhabricatorActionView()) diff --git a/src/applications/people/engineextension/PhabricatorPeopleQuickSearchEngineExtension.php b/src/applications/people/engineextension/PhabricatorPeopleQuickSearchEngineExtension.php new file mode 100644 index 0000000000..565838de72 --- /dev/null +++ b/src/applications/people/engineextension/PhabricatorPeopleQuickSearchEngineExtension.php @@ -0,0 +1,11 @@ +createUser(); $user->save(); - $user2 = $this->createUser(); - $user2->save(); + $user->setAllowInlineCacheGeneration(true); $proj = $this->createProject($user); @@ -1289,12 +1288,19 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { $new_name = $proj->getName().' '.mt_rand(); - $xaction = new PhabricatorProjectTransaction(); - $xaction->setTransactionType( - PhabricatorProjectNameTransaction::TRANSACTIONTYPE); - $xaction->setNewValue($new_name); + $params = array( + 'objectIdentifier' => $proj->getID(), + 'transactions' => array( + array( + 'type' => 'name', + 'value' => $new_name, + ), + ), + ); - $this->applyTransactions($proj, $user, array($xaction)); + id(new ConduitCall('project.edit', $params)) + ->setUser($user) + ->execute(); return true; } diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php index adf4a3138b..2764ce6322 100644 --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -120,16 +120,6 @@ final class PhabricatorProjectTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectNameTransaction::TRANSACTIONTYPE: - case PhabricatorProjectStatusTransaction::TRANSACTIONTYPE: - case PhabricatorProjectImageTransaction::TRANSACTIONTYPE: - case PhabricatorProjectIconTransaction::TRANSACTIONTYPE: - case PhabricatorProjectColorTransaction::TRANSACTIONTYPE: - PhabricatorPolicyFilter::requireCapability( - $this->requireActor(), - $object, - PhabricatorPolicyCapability::CAN_EDIT); - return; case PhabricatorProjectLockTransaction::TRANSACTIONTYPE: PhabricatorPolicyFilter::requireCapability( $this->requireActor(), diff --git a/src/applications/project/engine/PhabricatorBoardRenderingEngine.php b/src/applications/project/engine/PhabricatorBoardRenderingEngine.php index ca8b0633da..d76497bc21 100644 --- a/src/applications/project/engine/PhabricatorBoardRenderingEngine.php +++ b/src/applications/project/engine/PhabricatorBoardRenderingEngine.php @@ -67,7 +67,9 @@ final class PhabricatorBoardRenderingEngine extends Phobject { $project_phids = $object->getProjectPHIDs(); $project_handles = array_select_keys($this->handles, $project_phids); if ($project_handles) { - $card->setProjectHandles($project_handles); + $card + ->setHideArchivedProjects(true) + ->setProjectHandles($project_handles); } $cover_phid = $object->getCoverImageThumbnailPHID(); diff --git a/src/applications/project/engineextension/ProjectQuickSearchEngineExtension.php b/src/applications/project/engineextension/ProjectQuickSearchEngineExtension.php new file mode 100644 index 0000000000..6bb50e021e --- /dev/null +++ b/src/applications/project/engineextension/ProjectQuickSearchEngineExtension.php @@ -0,0 +1,11 @@ +getValue('object'); + switch ($event->getType()) { case PhabricatorEventType::TYPE_UI_WILLRENDERPROPERTIES: + // Hacky solution so that property list view on Diffusion + // commits shows build status, but not Projects, Subscriptions, + // or Tokens. + if ($object instanceof PhabricatorRepositoryCommit) { + return; + } $this->handlePropertyEvent($event); break; } diff --git a/src/applications/project/storage/PhabricatorProjectTransaction.php b/src/applications/project/storage/PhabricatorProjectTransaction.php index 158c2480c0..a8b2bb0d4a 100644 --- a/src/applications/project/storage/PhabricatorProjectTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectTransaction.php @@ -19,6 +19,10 @@ final class PhabricatorProjectTransaction return PhabricatorProjectProjectPHIDType::TYPECONST; } + public function getApplicationTransactionCommentObject() { + return null; + } + public function getBaseTransactionClass() { return 'PhabricatorProjectTransactionType'; } diff --git a/src/applications/project/view/ProjectBoardTaskCard.php b/src/applications/project/view/ProjectBoardTaskCard.php index 1a6807ec45..3a7016ca74 100644 --- a/src/applications/project/view/ProjectBoardTaskCard.php +++ b/src/applications/project/view/ProjectBoardTaskCard.php @@ -8,6 +8,7 @@ final class ProjectBoardTaskCard extends Phobject { private $owner; private $canEdit; private $coverImageFile; + private $hideArchivedProjects; public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; @@ -35,6 +36,15 @@ final class ProjectBoardTaskCard extends Phobject { return $this->coverImageFile; } + public function setHideArchivedProjects($hide_archived_projects) { + $this->hideArchivedProjects = $hide_archived_projects; + return $this; + } + + public function getHideArchivedProjects() { + return $this->hideArchivedProjects; + } + public function setTask(ManiphestTask $task) { $this->task = $task; return $this; @@ -126,10 +136,12 @@ final class ProjectBoardTaskCard extends Phobject { $project_handles = $this->getProjectHandles(); // Remove any archived projects from the list. - if ($project_handles) { - foreach ($project_handles as $key => $handle) { - if ($handle->getStatus() == PhabricatorObjectHandle::STATUS_CLOSED) { - unset($project_handles[$key]); + if ($this->hideArchivedProjects) { + if ($project_handles) { + foreach ($project_handles as $key => $handle) { + if ($handle->getStatus() == PhabricatorObjectHandle::STATUS_CLOSED) { + unset($project_handles[$key]); + } } } } diff --git a/src/applications/repository/query/PhabricatorRepositoryQuery.php b/src/applications/repository/query/PhabricatorRepositoryQuery.php index 0b681713da..e1246cfc3e 100644 --- a/src/applications/repository/query/PhabricatorRepositoryQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryQuery.php @@ -425,6 +425,7 @@ final class PhabricatorRepositoryQuery 'type' => 'string', 'unique' => true, 'reverse' => true, + 'null' => 'tail', ), 'name' => array( 'table' => 'r', diff --git a/src/applications/search/engine/PhabricatorQuickSearchEngine.php b/src/applications/search/engine/PhabricatorQuickSearchEngine.php new file mode 100644 index 0000000000..4b67dca659 --- /dev/null +++ b/src/applications/search/engine/PhabricatorQuickSearchEngine.php @@ -0,0 +1,8 @@ +setAncestorClass(__CLASS__) + ->execute(); + + $datasources = array(); + foreach ($extensions as $extension) { + $datasources[] = $extension->newQuickSearchDatasources(); + } + return array_mergev($datasources); + } +} diff --git a/src/applications/search/typeahead/PhabricatorSearchDatasource.php b/src/applications/search/typeahead/PhabricatorSearchDatasource.php index 2a17ad01eb..ed17a31185 100644 --- a/src/applications/search/typeahead/PhabricatorSearchDatasource.php +++ b/src/applications/search/typeahead/PhabricatorSearchDatasource.php @@ -16,14 +16,8 @@ final class PhabricatorSearchDatasource } public function getComponentDatasources() { - $sources = array( - new PhabricatorPeopleDatasource(), - new PhabricatorProjectDatasource(), - new PhabricatorApplicationDatasource(), - new PhabricatorTypeaheadMonogramDatasource(), - new DiffusionRepositoryDatasource(), - new DiffusionSymbolDatasource(), - ); + $sources = id(new PhabricatorQuickSearchEngine()) + ->getAllDatasources(); // These results are always rendered in the full browse display mode, so // set the browse flag on all component sources. diff --git a/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php b/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php index 09b26819c0..5f371d69f3 100644 --- a/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php +++ b/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php @@ -9,11 +9,19 @@ final class PhabricatorSubscriptionsUIEventListener } public function handleEvent(PhutilEvent $event) { + $object = $event->getValue('object'); + switch ($event->getType()) { case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS: $this->handleActionEvent($event); break; case PhabricatorEventType::TYPE_UI_WILLRENDERPROPERTIES: + // Hacky solution so that property list view on Diffusion + // commits shows build status, but not Projects, Subscriptions, + // or Tokens. + if ($object instanceof PhabricatorRepositoryCommit) { + return; + } $this->handlePropertyEvent($event); break; } diff --git a/src/applications/tokens/event/PhabricatorTokenUIEventListener.php b/src/applications/tokens/event/PhabricatorTokenUIEventListener.php index bbf3438b62..047c5504bf 100644 --- a/src/applications/tokens/event/PhabricatorTokenUIEventListener.php +++ b/src/applications/tokens/event/PhabricatorTokenUIEventListener.php @@ -9,11 +9,19 @@ final class PhabricatorTokenUIEventListener } public function handleEvent(PhutilEvent $event) { + $object = $event->getValue('object'); + switch ($event->getType()) { case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS: $this->handleActionEvent($event); break; case PhabricatorEventType::TYPE_UI_WILLRENDERPROPERTIES: + // Hacky solution so that property list view on Diffusion + // commits shows build status, but not Projects, Subscriptions, + // or Tokens. + if ($object instanceof PhabricatorRepositoryCommit) { + return; + } $this->handlePropertyEvent($event); break; } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index cf15fed267..047b44d141 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -70,6 +70,8 @@ abstract class PhabricatorApplicationTransactionEditor private $feedRelatedPHIDs = array(); private $modularTypes; + private $transactionQueue = array(); + const STORAGE_ENCODING_BINARY = 'binary'; /** @@ -1174,6 +1176,8 @@ abstract class PhabricatorApplicationTransactionEditor 'priority' => PhabricatorWorker::PRIORITY_ALERTS, )); + $this->flushTransactionQueue($object); + return $xactions; } @@ -3867,4 +3871,39 @@ abstract class PhabricatorApplicationTransactionEditor return pht('%s created an object: %s.', $author, $object); } +/* -( Queue )-------------------------------------------------------------- */ + + protected function queueTransaction( + PhabricatorApplicationTransaction $xaction) { + $this->transactionQueue[] = $xaction; + return $this; + } + + private function flushTransactionQueue($object) { + if (!$this->transactionQueue) { + return; + } + + $xactions = $this->transactionQueue; + $this->transactionQueue = array(); + + $editor = $this->newQueueEditor(); + + return $editor->applyTransactions($object, $xactions); + } + + private function newQueueEditor() { + $editor = id(newv(get_class($this), array())) + ->setActor($this->getActor()) + ->setContentSource($this->getContentSource()) + ->setContinueOnNoEffect($this->getContinueOnNoEffect()) + ->setContinueOnMissingFields($this->getContinueOnMissingFields()); + + if ($this->actingAsPHID !== null) { + $editor->setActingAsPHID($this->actingAsPHID); + } + + return $editor; + } + } diff --git a/src/applications/typeahead/engineextension/PhabricatorMonogramQuickSearchEngineExtension.php b/src/applications/typeahead/engineextension/PhabricatorMonogramQuickSearchEngineExtension.php new file mode 100644 index 0000000000..078cb44685 --- /dev/null +++ b/src/applications/typeahead/engineextension/PhabricatorMonogramQuickSearchEngineExtension.php @@ -0,0 +1,11 @@ +shouldAppearInTransactionMail(); case self::ROLE_HERALD: return $this->shouldAppearInHerald(); + case self::ROLE_HERALDACTION: + return $this->shouldAppearInHeraldActions(); case self::ROLE_EDITENGINE: return $this->shouldAppearInEditView() || $this->shouldAppearInEditEngine(); @@ -1476,4 +1479,56 @@ abstract class PhabricatorCustomField extends Phobject { } + public function shouldAppearInHeraldActions() { + if ($this->proxy) { + return $this->proxy->shouldAppearInHeraldActions(); + } + return false; + } + + + public function getHeraldActionName() { + if ($this->proxy) { + return $this->proxy->getHeraldActionName(); + } + + return null; + } + + + public function getHeraldActionStandardType() { + if ($this->proxy) { + return $this->proxy->getHeraldActionStandardType(); + } + + return null; + } + + + public function getHeraldActionDescription($value) { + if ($this->proxy) { + return $this->proxy->getHeraldActionDescription($value); + } + + return null; + } + + + public function getHeraldActionEffectDescription($value) { + if ($this->proxy) { + return $this->proxy->getHeraldActionEffectDescription($value); + } + + return null; + } + + + public function getHeraldActionDatasource() { + if ($this->proxy) { + return $this->proxy->getHeraldActionDatasource(); + } + + return null; + } + } diff --git a/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldAction.php b/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldAction.php new file mode 100644 index 0000000000..1d35ba59a2 --- /dev/null +++ b/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldAction.php @@ -0,0 +1,105 @@ +customField = $custom_field; + return $this; + } + + public function getCustomField() { + return $this->customField; + } + + public function getActionGroupKey() { + return PhabricatorCustomFieldHeraldActionGroup::ACTIONGROUPKEY; + } + + public function supportsObject($object) { + return ($object instanceof PhabricatorCustomFieldInterface); + } + + public function supportsRuleType($rule_type) { + return true; + } + + public function getActionsForObject($object) { + $viewer = PhabricatorUser::getOmnipotentUser(); + $role = PhabricatorCustomField::ROLE_HERALDACTION; + + $field_list = PhabricatorCustomField::getObjectFields($object, $role) + ->setViewer($viewer) + ->readFieldsFromStorage($object); + + $map = array(); + foreach ($field_list->getFields() as $field) { + $key = $field->getFieldKey(); + $map[$key] = id(new self()) + ->setCustomField($field); + } + + return $map; + } + + public function applyEffect($object, HeraldEffect $effect) { + $field = $this->getCustomField(); + $value = $effect->getTarget(); + $adapter = $this->getAdapter(); + + $old_value = $field->getOldValueForApplicationTransactions(); + $new_value = id(clone $field) + ->setValueFromApplicationTransactions($value) + ->getValueForStorage(); + + $xaction = $adapter->newTransaction() + ->setTransactionType(PhabricatorTransactions::TYPE_CUSTOMFIELD) + ->setMetadataValue('customfield:key', $field->getFieldKey()) + ->setOldValue($old_value) + ->setNewValue($new_value); + + $adapter->queueTransaction($xaction); + + $this->logEffect(self::DO_SET_FIELD, $value); + } + + public function getHeraldActionName() { + return $this->getCustomField()->getHeraldActionName(); + } + + public function getHeraldActionStandardType() { + return $this->getCustomField()->getHeraldActionStandardType(); + } + + protected function getDatasource() { + return $this->getCustomField()->getHeraldActionDatasource(); + } + + public function renderActionDescription($value) { + return $this->getCustomField()->getHeraldActionDescription($value); + } + + protected function getActionEffectMap() { + return array( + self::DO_SET_FIELD => array( + 'icon' => 'fa-pencil', + 'color' => 'green', + 'name' => pht('Set Field Value'), + ), + ); + } + + protected function renderActionEffectDescription($type, $data) { + switch ($type) { + case self::DO_SET_FIELD: + return $this->getCustomField()->getHeraldActionEffectDescription($data); + } + } + + +} diff --git a/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldActionGroup.php b/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldActionGroup.php new file mode 100644 index 0000000000..b8f16ab202 --- /dev/null +++ b/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldActionGroup.php @@ -0,0 +1,16 @@ +getFieldName()); + } + + public function getHeraldActionDescription($value) { + $list = $this->renderHeraldHandleList($value); + return pht('Set "%s" to: %s.', $this->getFieldName(), $list); + } + + public function getHeraldActionEffectDescription($value) { + return $this->renderHeraldHandleList($value); + } + + public function getHeraldActionStandardType() { + return HeraldAction::STANDARD_PHID_LIST; + } + + public function getHeraldActionDatasource() { + return $this->getDatasource(); + } + + private function renderHeraldHandleList($value) { + if (!is_array($value)) { + return pht('(Invalid List)'); + } else { + return $this->getViewer() + ->renderHandleList($value) + ->setAsInline(true) + ->render(); + } + } + } diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 9bebfe6957..63daa6df79 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -111,7 +111,19 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery AphrontDatabaseConnection $conn, $table_name) { - $rows = queryfx_all( + $query = $this->buildStandardPageQuery($conn, $table_name); + + $rows = queryfx_all($conn, '%Q', $query); + $rows = $this->didLoadRawRows($rows); + + return $rows; + } + + protected function buildStandardPageQuery( + AphrontDatabaseConnection $conn, + $table_name) { + + return qsprintf( $conn, '%Q FROM %T %Q %Q %Q %Q %Q %Q %Q', $this->buildSelectClause($conn), @@ -123,10 +135,6 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery $this->buildHavingClause($conn), $this->buildOrderClause($conn), $this->buildLimitClause($conn)); - - $rows = $this->didLoadRawRows($rows); - - return $rows; } protected function didLoadRawRows(array $rows) { @@ -1032,7 +1040,10 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery /** * @task order */ - final protected function buildOrderClause(AphrontDatabaseConnection $conn) { + final protected function buildOrderClause( + AphrontDatabaseConnection $conn, + $for_union = false) { + $orderable = $this->getOrderableColumns(); $vector = $this->getOrderVector(); @@ -1045,7 +1056,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery $parts[] = $part; } - return $this->formatOrderClause($conn, $parts); + return $this->formatOrderClause($conn, $parts, $for_union); } @@ -1054,7 +1065,8 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery */ protected function formatOrderClause( AphrontDatabaseConnection $conn, - array $parts) { + array $parts, + $for_union = false) { $is_query_reversed = false; if ($this->getBeforeID()) { @@ -1075,6 +1087,13 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } $table = idx($part, 'table'); + + // When we're building an ORDER BY clause for a sequence of UNION + // statements, we can't refer to tables from the subqueries. + if ($for_union) { + $table = null; + } + $column = $part['column']; if ($table !== null) { @@ -1546,6 +1565,13 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery return $select; } + $vector = $this->getOrderVector(); + if (!$vector->containsKey('rank')) { + // We only need to SELECT the virtual "_ft_rank" column if we're + // actually sorting the results by rank. + return $select; + } + if (!$this->ferretEngine) { $select[] = '0 _ft_rank'; return $select; diff --git a/src/view/page/menu/PhabricatorMainMenuBarExtension.php b/src/view/page/menu/PhabricatorMainMenuBarExtension.php index fede2fdb85..e66b1f2c7d 100644 --- a/src/view/page/menu/PhabricatorMainMenuBarExtension.php +++ b/src/view/page/menu/PhabricatorMainMenuBarExtension.php @@ -5,6 +5,7 @@ abstract class PhabricatorMainMenuBarExtension extends Phobject { private $viewer; private $application; private $controller; + private $isFullSession; public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; @@ -33,6 +34,15 @@ abstract class PhabricatorMainMenuBarExtension extends Phobject { return $this->controller; } + public function setIsFullSession($is_full_session) { + $this->isFullSession = $is_full_session; + return $this; + } + + public function getIsFullSession() { + return $this->isFullSession; + } + final public function getExtensionKey() { return $this->getPhobjectClassConstant('MAINMENUBARKEY'); } @@ -41,6 +51,10 @@ abstract class PhabricatorMainMenuBarExtension extends Phobject { return true; } + public function shouldRequireFullSession() { + return true; + } + public function isExtensionEnabledForViewer(PhabricatorUser $viewer) { if (!$viewer->isLoggedIn()) { return false; diff --git a/src/view/page/menu/PhabricatorMainMenuView.php b/src/view/page/menu/PhabricatorMainMenuView.php index f9e4032d87..ad9510f348 100644 --- a/src/view/page/menu/PhabricatorMainMenuView.php +++ b/src/view/page/menu/PhabricatorMainMenuView.php @@ -46,7 +46,9 @@ final class PhabricatorMainMenuView extends AphrontView { $app_button = ''; $aural = null; - if ($viewer->isLoggedIn() && $viewer->isUserActivated()) { + $is_full = $this->isFullSession($viewer); + + if ($is_full) { list($menu, $dropdowns, $aural) = $this->renderNotificationMenu(); if (array_filter($menu)) { $alerts[] = $menu; @@ -54,14 +56,18 @@ final class PhabricatorMainMenuView extends AphrontView { $menu_bar = array_merge($menu_bar, $dropdowns); $app_button = $this->renderApplicationMenuButton(); $search_button = $this->renderSearchMenuButton($header_id); - } else { + } else if (!$viewer->isLoggedIn()) { $app_button = $this->renderApplicationMenuButton(); if (PhabricatorEnv::getEnvConfig('policy.allow-public')) { $search_button = $this->renderSearchMenuButton($header_id); } } - $search_menu = $this->renderPhabricatorSearchMenu(); + if ($search_button) { + $search_menu = $this->renderPhabricatorSearchMenu(); + } else { + $search_menu = null; + } if ($alerts) { $alerts = javelin_tag( @@ -84,7 +90,9 @@ final class PhabricatorMainMenuView extends AphrontView { $extensions = PhabricatorMainMenuBarExtension::getAllEnabledExtensions(); foreach ($extensions as $extension) { - $extension->setViewer($viewer); + $extension + ->setViewer($viewer) + ->setIsFullSession($is_full); $controller = $this->getController(); if ($controller) { @@ -96,6 +104,14 @@ final class PhabricatorMainMenuView extends AphrontView { } } + if (!$is_full) { + foreach ($extensions as $key => $extension) { + if ($extension->shouldRequireFullSession()) { + unset($extensions[$key]); + } + } + } + foreach ($extensions as $key => $extension) { if (!$extension->isExtensionEnabledForViewer($extension->getViewer())) { unset($extensions[$key]); @@ -677,4 +693,38 @@ final class PhabricatorMainMenuView extends AphrontView { ); } + private function isFullSession(PhabricatorUser $viewer) { + if (!$viewer->isLoggedIn()) { + return false; + } + + if (!$viewer->isUserActivated()) { + return false; + } + + if (!$viewer->hasSession()) { + return false; + } + + $session = $viewer->getSession(); + if ($session->getIsPartial()) { + return false; + } + + if (!$session->getSignedLegalpadDocuments()) { + return false; + } + + $mfa_key = 'security.require-multi-factor-auth'; + $need_mfa = PhabricatorEnv::getEnvConfig($mfa_key); + if ($need_mfa) { + $have_mfa = $viewer->getIsEnrolledInMultiFactor(); + if (!$have_mfa) { + return false; + } + } + + return true; + } + } diff --git a/webroot/rsrc/css/application/diffusion/diffusion-source.css b/webroot/rsrc/css/application/diffusion/diffusion-source.css index 3b8b9a8a16..a2c67cf16d 100644 --- a/webroot/rsrc/css/application/diffusion/diffusion-source.css +++ b/webroot/rsrc/css/application/diffusion/diffusion-source.css @@ -13,22 +13,24 @@ -webkit-overflow-scrolling: touch; } -.diffusion-source tr.phabricator-source-highlight th, -.diffusion-source tr.phabricator-source-highlight td { - background: {$gentle.highlight}; +.diffusion-source tr.phabricator-source-highlight { + background: {$sh-yellowbackground}; } .diffusion-source th { text-align: right; vertical-align: top; - color: {$darkbluetext}; + background: {$lightgreybackground}; + color: {$bluetext}; border-right: 1px solid {$thinblueborder}; } .diffusion-source td { vertical-align: top; white-space: pre-wrap; - padding: 3px 12px; + padding-top: 1px; + padding-bottom: 1px; + padding-left: 8px; width: 100%; word-break: break-all; } @@ -43,18 +45,12 @@ } .diffusion-blame-link, -.diffusion-rev-link, -.diffusion-blame-date { +.diffusion-rev-link { white-space: nowrap; } -.diffusion-blame-date, -.diffusion-blame-link, -.diffusion-blame-revision, -.diffusion-rev-link { - background: {$lightgreybackground}; - font: {$basefont}; - font-size: {$smallerfontsize}; +.diffusion-blame-link { + min-width: 28px; } .diffusion-source th.diffusion-rev-link { @@ -62,27 +58,20 @@ min-width: 130px; } -.diffusion-source a { +.diffusion-blame-link a, +.diffusion-rev-link a, +.diffusion-line-link a { color: {$darkbluetext}; } .diffusion-rev-link a { - max-width: 300px; - overflow: hidden; - white-space: nowrap; - text-overflow: ellipsis; - margin: 3px 8px; - display: block; -} - -.diffusion-blame-date a, -.diffusion-blame-revision a { - float: right; - margin: 3px 8px; + margin: 0 8px 0 0; + display: inline-block; } .diffusion-rev-link span { - margin-right: -4px; + display: inline-block; + margin-right: 4px; margin-left: -4px; color: {$lightgreytext}; } @@ -91,19 +80,7 @@ .diffusion-line-link a { /* Give the user a larger click target. */ display: block; - padding: 4px 8px 3px; -} - -.diffusion-line-link a { - color: {$lightgreytext}; -} - -.diffusion-blame-link a .phui-icon-view { - color: {$bluetext}; -} - -.diffusion-blame-link a:hover .phui-icon-view { - color: {$sky}; + padding: 2px 8px; } .diffusion-line-link { @@ -113,3 +90,13 @@ -ms-user-select: none; user-select: none; } + +.diffusion-rev-link .diffusion-author-link { + display: inline-block; + padding: 0; + margin: 2px 6px -4px 8px; + width: 16px; + height: 16px; + background-size: 100% 100%; + background-repeat: no-repeat; +} diff --git a/webroot/rsrc/css/phui/phui-timeline-view.css b/webroot/rsrc/css/phui/phui-timeline-view.css index 33e0f8ed0c..b0ae9c0044 100644 --- a/webroot/rsrc/css/phui/phui-timeline-view.css +++ b/webroot/rsrc/css/phui/phui-timeline-view.css @@ -315,6 +315,10 @@ background-color: {$violet}; } +.phui-timeline-icon-fill-pink { + background-color: {$pink}; +} + .phui-timeline-icon-fill-grey { background-color: #888; } diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js index 3f47f1ed35..ec0270ac12 100644 --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -465,7 +465,7 @@ JX.install('DiffChangesetList', { new JX.Notification() .setContent(message) .alterClassName('jx-notification-alert', true) - .setDuration(1000) + .setDuration(3000) .show(); }, @@ -691,6 +691,7 @@ JX.install('DiffChangesetList', { 'div', 'differential-changeset'); + var changeset_list = this; var changeset = this.getChangesetForNode(node); var menu = new JX.PHUIXDropdownMenu(button); @@ -738,6 +739,22 @@ JX.install('DiffChangesetList', { var up_item = new JX.PHUIXActionView() .setHandler(function(e) { if (changeset.isLoaded()) { + + // Don't let the user swap display modes if a comment is being + // edited, since they might lose their work. See PHI180. + var inlines = changeset.getInlines(); + for (var ii = 0; ii < inlines.length; ii++) { + if (inlines[ii].isEditing()) { + changeset_list._warnUser( + pht( + 'Finish editing inline comments before changing display ' + + 'modes.')); + e.prevent(); + menu.close(); + return; + } + } + var renderer = changeset.getRenderer(); if (renderer == '1up') { renderer = '2up'; diff --git a/webroot/rsrc/js/application/repository/repository-crossreference.js b/webroot/rsrc/js/application/repository/repository-crossreference.js index bfb849c759..138ce4bf3e 100644 --- a/webroot/rsrc/js/application/repository/repository-crossreference.js +++ b/webroot/rsrc/js/application/repository/repository-crossreference.js @@ -46,8 +46,19 @@ JX.behavior('repository-crossreference', function(config, statics) { if (!isSignalkey(e)) { return; } + + var target = e.getTarget(); + + try { + // If we're in an inline comment, don't link symbols. + if (JX.DOM.findAbove(target, 'div', 'differential-inline-comment')) { + return; + } + } catch (ex) { + // Continue if we're not inside an inline comment. + } + if (e.getType() === 'mouseover') { - var target = e.getTarget(); while (target !== document.body) { if (JX.DOM.isNode(target, 'span') && (target.className in class_map)) { @@ -58,7 +69,7 @@ JX.behavior('repository-crossreference', function(config, statics) { target = target.parentNode; } } else if (e.getType() === 'click') { - openSearch(e.getTarget(), lang); + openSearch(target, lang); } }); }