diff --git a/resources/celerity/map.php b/resources/celerity/map.php index ce069dbd11..30c04ffc92 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ return array( 'core.pkg.js' => '4c79d74f', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '45951e9e', - 'differential.pkg.js' => 'b71b8c5d', + 'differential.pkg.js' => 'ae6460e0', 'diffusion.pkg.css' => 'a2d17c7d', 'diffusion.pkg.js' => '6134c5a1', 'favicon.ico' => '30672e08', @@ -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', @@ -444,7 +444,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', @@ -572,7 +572,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', @@ -690,7 +690,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', @@ -1546,6 +1546,12 @@ return array( '7f243deb' => array( 'javelin-install', ), + '7fe9bc12' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-uri', + ), '834a1173' => array( 'javelin-behavior', 'javelin-scrollbar', @@ -2073,12 +2079,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/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 93efcc65e1..7180a7ae04 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -439,6 +439,8 @@ 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', @@ -458,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', @@ -547,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', @@ -1328,6 +1332,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', @@ -1351,6 +1356,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', @@ -1388,6 +1394,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', @@ -4774,6 +4782,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', @@ -5429,6 +5438,7 @@ phutil_register_library_map(array( 'HarbormasterBuildkiteBuildableInterface', 'PhabricatorApplicationTransactionInterface', 'PhabricatorDestructibleInterface', + 'PhabricatorConduitResultInterface', ), 'DifferentialDiffAffectedFilesHeraldField' => 'DifferentialDiffHeraldField', 'DifferentialDiffAuthorHeraldField' => 'DifferentialDiffHeraldField', @@ -5447,6 +5457,8 @@ phutil_register_library_map(array( 'DifferentialDiffQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DifferentialDiffRepositoryHeraldField' => 'DifferentialDiffHeraldField', 'DifferentialDiffRepositoryProjectsHeraldField' => 'DifferentialDiffHeraldField', + 'DifferentialDiffSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', + 'DifferentialDiffSearchEngine' => 'PhabricatorApplicationSearchEngine', 'DifferentialDiffTestCase' => 'PhutilTestCase', 'DifferentialDiffTransaction' => 'PhabricatorApplicationTransaction', 'DifferentialDiffTransactionQuery' => 'PhabricatorApplicationTransactionQuery', @@ -5466,6 +5478,7 @@ phutil_register_library_map(array( 'DifferentialGetWorkingCopy' => 'Phobject', 'DifferentialGitSVNIDCommitMessageField' => 'DifferentialCommitMessageField', 'DifferentialHarbormasterField' => 'DifferentialCustomField', + 'DifferentialHeraldStateReasons' => 'HeraldStateReasons', 'DifferentialHiddenComment' => 'DifferentialDAO', 'DifferentialHostField' => 'DifferentialCustomField', 'DifferentialHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', @@ -5580,6 +5593,7 @@ phutil_register_library_map(array( 'DifferentialRevisionHasTaskRelationship' => 'DifferentialRevisionRelationship', 'DifferentialRevisionHeraldField' => 'HeraldField', 'DifferentialRevisionHeraldFieldGroup' => 'HeraldFieldGroup', + 'DifferentialRevisionHoldDraftTransaction' => 'DifferentialRevisionTransactionType', 'DifferentialRevisionIDCommitMessageField' => 'DifferentialCommitMessageField', 'DifferentialRevisionInlineTransaction' => 'PhabricatorModularTransactionType', 'DifferentialRevisionInlinesController' => 'DifferentialController', @@ -6457,6 +6471,7 @@ phutil_register_library_map(array( 'HeraldApplicationActionGroup' => 'HeraldActionGroup', 'HeraldApplyTranscript' => 'Phobject', 'HeraldBasicFieldGroup' => 'HeraldFieldGroup', + 'HeraldBuildableState' => 'HeraldState', 'HeraldCommitAdapter' => array( 'HeraldAdapter', 'HarbormasterBuildableAdapterInterface', @@ -6486,6 +6501,7 @@ phutil_register_library_map(array( 'HeraldGroup' => 'Phobject', 'HeraldInvalidActionException' => 'Exception', 'HeraldInvalidConditionException' => 'Exception', + 'HeraldMailableState' => 'HeraldState', 'HeraldManageGlobalRulesCapability' => 'PhabricatorPolicyCapability', 'HeraldManiphestTaskAdapter' => 'HeraldAdapter', 'HeraldNewController' => 'HeraldController', @@ -6530,6 +6546,8 @@ phutil_register_library_map(array( 'HeraldSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'HeraldSelectFieldValue' => 'HeraldFieldValue', 'HeraldSpaceField' => 'HeraldField', + 'HeraldState' => 'Phobject', + 'HeraldStateReasons' => 'Phobject', 'HeraldSubscribersField' => 'HeraldField', 'HeraldSupportActionGroup' => 'HeraldActionGroup', 'HeraldSupportFieldGroup' => 'HeraldFieldGroup', @@ -10539,6 +10557,7 @@ phutil_register_library_map(array( 'PhabricatorDestructibleInterface', 'PhabricatorSpacesInterface', 'PhabricatorFulltextInterface', + 'PhabricatorFerretInterface', ), 'PonderQuestionAnswerTransaction' => 'PonderQuestionTransactionType', 'PonderQuestionAnswerWikiTransaction' => 'PonderQuestionTransactionType', @@ -10548,6 +10567,7 @@ phutil_register_library_map(array( 'PonderQuestionEditController' => 'PonderController', 'PonderQuestionEditEngine' => 'PhabricatorEditEngine', 'PonderQuestionEditor' => 'PonderEditor', + 'PonderQuestionFerretEngine' => 'PhabricatorFerretEngine', 'PonderQuestionFulltextEngine' => 'PhabricatorFulltextEngine', 'PonderQuestionHistoryController' => 'PonderController', 'PonderQuestionListController' => 'PonderController', 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/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 index e7ed2bedb2..5d625e3ce9 100644 --- a/src/applications/differential/customfield/DifferentialDraftField.php +++ b/src/applications/differential/customfield/DifferentialDraftField.php @@ -36,6 +36,12 @@ final class DifferentialDraftField 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( 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 7c9bb01b07..26c440e866 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -9,6 +9,7 @@ final class DifferentialTransactionEditor private $didExpandInlineState = false; private $hasReviewTransaction = false; private $affectedPaths; + private $firstBroadcast = false; public function getEditorApplicationClass() { return 'PhabricatorDifferentialApplication'; @@ -27,7 +28,7 @@ final class DifferentialTransactionEditor } public function isFirstBroadcast() { - return $this->getIsNewObject(); + return $this->firstBroadcast; } public function getDiffUpdateTransaction(array $xactions) { @@ -137,8 +138,7 @@ final class DifferentialTransactionEditor $object->setRepositoryPHID($diff->getRepositoryPHID()); } $object->attachActiveDiff($diff); - - // TODO: Update the `diffPHID` once we add that. + $object->setActiveDiffPHID($diff->getPHID()); return; } @@ -630,6 +630,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 +1007,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 +1196,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 +1453,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,12 +1562,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); + } + + // Mark this as the first broadcast we're sending about the revision + // so mail can generate specially. + $this->firstBroadcast = true; + $xaction = $object->getApplicationTransactionTemplate() + ->setAuthorPHID($author_phid) ->setTransactionType( DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE) ->setOldValue(false) 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/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 76c7c4da95..bf4bec0abc 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,7 @@ final class DifferentialRevision extends DifferentialDAO const RELATION_SUBSCRIBED = 'subd'; const PROPERTY_CLOSED_FROM_ACCEPTED = 'wasAcceptedBeforeClose'; + const PROPERTY_DRAFT_HOLD = 'draft.hold'; public static function initializeNewRevision(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) @@ -708,6 +711,14 @@ 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 loadActiveBuilds(PhabricatorUser $viewer) { $diff = $this->getActiveDiff(); @@ -721,9 +732,10 @@ final class DifferentialRevision extends DifferentialDAO return array(); } - $builds = id(new HarbormasterBuildQuery()) + return id(new HarbormasterBuildQuery()) ->setViewer($viewer) ->withBuildablePHIDs(mpull($buildables, 'getPHID')) + ->withAutobuilds(false) ->withBuildStatuses( array( HarbormasterBuildStatus::STATUS_INACTIVE, @@ -735,29 +747,7 @@ final class DifferentialRevision extends DifferentialDAO HarbormasterBuildStatus::STATUS_PAUSED, HarbormasterBuildStatus::STATUS_DEADLOCKED, )) - ->needBuildTargets(true) ->execute(); - if (!$builds) { - return array(); - } - - $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; - } - } - - return $active; } @@ -996,6 +986,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.')), ); } @@ -1012,6 +1014,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/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/DifferentialRevisionHoldDraftTransaction.php b/src/applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php new file mode 100644 index 0000000000..5bc257ab62 --- /dev/null +++ b/src/applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php @@ -0,0 +1,47 @@ +getHoldAsDraft(); + } + + public function generateNewValue($object, $value) { + return (bool)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setHoldAsDraft($value); + } + + 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/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/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/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/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/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 9636b5b017..c87623c68c 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/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/ManiphestTaskStatusTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php index 5e1cd44611..a3780e81b9 100644 --- a/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskStatusTransaction.php @@ -229,4 +229,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/metamta/herald/PhabricatorMetaMTAEmailHeraldAction.php b/src/applications/metamta/herald/PhabricatorMetaMTAEmailHeraldAction.php index 2b29fe2443..74fb879fe7 100644 --- a/src/applications/metamta/herald/PhabricatorMetaMTAEmailHeraldAction.php +++ b/src/applications/metamta/herald/PhabricatorMetaMTAEmailHeraldAction.php @@ -6,6 +6,12 @@ abstract class PhabricatorMetaMTAEmailHeraldAction const DO_SEND = 'do.send'; const DO_FORCE = 'do.force'; + public function getRequiredAdapterStates() { + return array( + HeraldMailableState::STATECONST, + ); + } + public function supportsObject($object) { // NOTE: This implementation lacks generality, but there's no great way to // figure out if something generates email right now. diff --git a/src/applications/phame/storage/PhameBlogTransaction.php b/src/applications/phame/storage/PhameBlogTransaction.php index c605510d7d..d3d6a79d0a 100644 --- a/src/applications/phame/storage/PhameBlogTransaction.php +++ b/src/applications/phame/storage/PhameBlogTransaction.php @@ -15,6 +15,10 @@ final class PhameBlogTransaction return PhabricatorPhameBlogPHIDType::TYPECONST; } + public function getApplicationTransactionCommentObject() { + return null; + } + public function getBaseTransactionClass() { return 'PhameBlogTransactionType'; } diff --git a/src/applications/ponder/search/PonderQuestionFerretEngine.php b/src/applications/ponder/search/PonderQuestionFerretEngine.php new file mode 100644 index 0000000000..72345f60c0 --- /dev/null +++ b/src/applications/ponder/search/PonderQuestionFerretEngine.php @@ -0,0 +1,18 @@ +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/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/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/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/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); } }); }