diff --git a/resources/celerity/map.php b/resources/celerity/map.php index d682369496..3708f83d02 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,8 +9,8 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '3dc188c0', - 'core.pkg.js' => 'ee320ca2', + 'core.pkg.css' => 'af983028', + 'core.pkg.js' => '8225dc58', 'differential.pkg.css' => '8d8360fb', 'differential.pkg.js' => '67e02996', 'diffusion.pkg.css' => '42c75c37', @@ -112,7 +112,7 @@ return array( 'rsrc/css/application/tokens/tokens.css' => 'ce5a50bd', 'rsrc/css/application/uiexample/example.css' => 'b4795059', 'rsrc/css/core/core.css' => '1b29ed61', - 'rsrc/css/core/remarkup.css' => '9e627d41', + 'rsrc/css/core/remarkup.css' => 'f06cc20e', 'rsrc/css/core/syntax.css' => '4234f572', 'rsrc/css/core/z-index.css' => '99c0f5eb', 'rsrc/css/diviner/diviner-shared.css' => '4bd263b0', @@ -151,7 +151,7 @@ return array( 'rsrc/css/phui/phui-document-summary.css' => 'b068eed1', 'rsrc/css/phui/phui-document.css' => '52b748a5', 'rsrc/css/phui/phui-feed-story.css' => 'a0c05029', - 'rsrc/css/phui/phui-fontkit.css' => '9b714a5e', + 'rsrc/css/phui/phui-fontkit.css' => '1ec937e5', 'rsrc/css/phui/phui-form-view.css' => '01b796c0', 'rsrc/css/phui/phui-form.css' => '159e2d9c', 'rsrc/css/phui/phui-head-thing.css' => 'd7f293df', @@ -253,7 +253,7 @@ return array( 'rsrc/externals/javelin/lib/URI.js' => '2e255291', 'rsrc/externals/javelin/lib/Vector.js' => 'e9c80beb', 'rsrc/externals/javelin/lib/WebSocket.js' => 'fdc13e4e', - 'rsrc/externals/javelin/lib/Workflow.js' => '958e9045', + 'rsrc/externals/javelin/lib/Workflow.js' => '851f642d', 'rsrc/externals/javelin/lib/__tests__/Cookie.js' => 'ca686f71', 'rsrc/externals/javelin/lib/__tests__/DOM.js' => '4566e249', 'rsrc/externals/javelin/lib/__tests__/JSON.js' => '710377ae', @@ -752,7 +752,7 @@ return array( 'javelin-workboard-header' => '111bfd2d', 'javelin-workboard-header-template' => 'ebe83a6b', 'javelin-workboard-order-template' => '03e8891f', - 'javelin-workflow' => '958e9045', + 'javelin-workflow' => '851f642d', 'maniphest-report-css' => '3d53188b', 'maniphest-task-edit-css' => '272daa84', 'maniphest-task-summary-css' => '61d1667e', @@ -793,7 +793,7 @@ return array( 'phabricator-object-selector-css' => 'ee77366f', 'phabricator-phtize' => '2f1db1ed', 'phabricator-prefab' => '5793d835', - 'phabricator-remarkup-css' => '9e627d41', + 'phabricator-remarkup-css' => 'f06cc20e', 'phabricator-search-results-css' => '9ea70ace', 'phabricator-shaped-request' => 'abf88db8', 'phabricator-slowvote-css' => '1694baed', @@ -838,7 +838,7 @@ return array( 'phui-document-view-pro-css' => 'b9613a10', 'phui-feed-story-css' => 'a0c05029', 'phui-font-icon-base-css' => 'd7994e06', - 'phui-fontkit-css' => '9b714a5e', + 'phui-fontkit-css' => '1ec937e5', 'phui-form-css' => '159e2d9c', 'phui-form-view-css' => '01b796c0', 'phui-head-thing-view-css' => 'd7f293df', @@ -1607,6 +1607,17 @@ return array( 'javelin-resource', 'javelin-routable', ), + '851f642d' => array( + 'javelin-stratcom', + 'javelin-request', + 'javelin-dom', + 'javelin-vector', + 'javelin-install', + 'javelin-util', + 'javelin-mask', + 'javelin-uri', + 'javelin-routable', + ), '87428eb2' => array( 'javelin-behavior', 'javelin-diffusion-locate-file-source', @@ -1712,17 +1723,6 @@ return array( 'javelin-stratcom', 'javelin-vector', ), - '958e9045' => array( - 'javelin-stratcom', - 'javelin-request', - 'javelin-dom', - 'javelin-vector', - 'javelin-install', - 'javelin-util', - 'javelin-mask', - 'javelin-uri', - 'javelin-routable', - ), '9623adc1' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/resources/sql/autopatches/20180208.maniphest.02.populate.php b/resources/sql/autopatches/20180208.maniphest.02.populate.php index e11267496c..4b4e549574 100644 --- a/resources/sql/autopatches/20180208.maniphest.02.populate.php +++ b/resources/sql/autopatches/20180208.maniphest.02.populate.php @@ -26,6 +26,7 @@ foreach (new LiskMigrationIterator($table) as $task) { $xactions = id(new ManiphestTransactionQuery()) ->setViewer($viewer) ->withObjectPHIDs(array($task->getPHID())) + ->needHandles(false) ->withTransactionTypes( array( $type_merge, diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d10ec7d420..ab5a58dda6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1560,6 +1560,7 @@ phutil_register_library_map(array( 'HeraldRuleIndexEngineExtension' => 'applications/herald/engineextension/HeraldRuleIndexEngineExtension.php', 'HeraldRuleListController' => 'applications/herald/controller/HeraldRuleListController.php', 'HeraldRuleListView' => 'applications/herald/view/HeraldRuleListView.php', + 'HeraldRuleManagementWorkflow' => 'applications/herald/management/HeraldRuleManagementWorkflow.php', 'HeraldRuleNameTransaction' => 'applications/herald/xaction/HeraldRuleNameTransaction.php', 'HeraldRulePHIDType' => 'applications/herald/phid/HeraldRulePHIDType.php', 'HeraldRuleQuery' => 'applications/herald/query/HeraldRuleQuery.php', @@ -2171,9 +2172,11 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionCommentView' => 'applications/transactions/view/PhabricatorApplicationTransactionCommentView.php', 'PhabricatorApplicationTransactionController' => 'applications/transactions/controller/PhabricatorApplicationTransactionController.php', 'PhabricatorApplicationTransactionDetailController' => 'applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php', + 'PhabricatorApplicationTransactionDetailView' => 'applications/transactions/view/PhabricatorApplicationTransactionDetailView.php', 'PhabricatorApplicationTransactionEditor' => 'applications/transactions/editor/PhabricatorApplicationTransactionEditor.php', 'PhabricatorApplicationTransactionFeedStory' => 'applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php', 'PhabricatorApplicationTransactionInterface' => 'applications/transactions/interface/PhabricatorApplicationTransactionInterface.php', + 'PhabricatorApplicationTransactionJSONDiffDetailView' => 'applications/transactions/view/PhabricatorApplicationTransactionJSONDiffDetailView.php', 'PhabricatorApplicationTransactionNoEffectException' => 'applications/transactions/exception/PhabricatorApplicationTransactionNoEffectException.php', 'PhabricatorApplicationTransactionNoEffectResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionNoEffectResponse.php', 'PhabricatorApplicationTransactionPublishWorker' => 'applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php', @@ -3150,16 +3153,26 @@ phutil_register_library_map(array( 'PhabricatorEditEngineConfigurationTransactionQuery' => 'applications/transactions/query/PhabricatorEditEngineConfigurationTransactionQuery.php', 'PhabricatorEditEngineConfigurationViewController' => 'applications/transactions/controller/PhabricatorEditEngineConfigurationViewController.php', 'PhabricatorEditEngineController' => 'applications/transactions/controller/PhabricatorEditEngineController.php', + 'PhabricatorEditEngineCreateOrderTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineCreateOrderTransaction.php', 'PhabricatorEditEngineDatasource' => 'applications/transactions/typeahead/PhabricatorEditEngineDatasource.php', + 'PhabricatorEditEngineDefaultCreateTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineDefaultCreateTransaction.php', 'PhabricatorEditEngineDefaultLock' => 'applications/transactions/editengine/PhabricatorEditEngineDefaultLock.php', + 'PhabricatorEditEngineDefaultTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineDefaultTransaction.php', + 'PhabricatorEditEngineDisableTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineDisableTransaction.php', + 'PhabricatorEditEngineEditOrderTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineEditOrderTransaction.php', 'PhabricatorEditEngineExtension' => 'applications/transactions/engineextension/PhabricatorEditEngineExtension.php', 'PhabricatorEditEngineExtensionModule' => 'applications/transactions/engineextension/PhabricatorEditEngineExtensionModule.php', + 'PhabricatorEditEngineIsEditTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineIsEditTransaction.php', 'PhabricatorEditEngineListController' => 'applications/transactions/controller/PhabricatorEditEngineListController.php', 'PhabricatorEditEngineLock' => 'applications/transactions/editengine/PhabricatorEditEngineLock.php', 'PhabricatorEditEngineLockableInterface' => 'applications/transactions/editengine/PhabricatorEditEngineLockableInterface.php', + 'PhabricatorEditEngineLocksTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineLocksTransaction.php', 'PhabricatorEditEngineMFAEngine' => 'applications/transactions/editengine/PhabricatorEditEngineMFAEngine.php', 'PhabricatorEditEngineMFAInterface' => 'applications/transactions/editengine/PhabricatorEditEngineMFAInterface.php', + 'PhabricatorEditEngineNameTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineNameTransaction.php', + 'PhabricatorEditEngineOrderTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineOrderTransaction.php', 'PhabricatorEditEnginePointsCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEnginePointsCommentAction.php', + 'PhabricatorEditEnginePreambleTransaction' => 'applications/transactions/xaction/PhabricatorEditEnginePreambleTransaction.php', 'PhabricatorEditEngineProfileMenuItem' => 'applications/search/menuitem/PhabricatorEditEngineProfileMenuItem.php', 'PhabricatorEditEngineQuery' => 'applications/transactions/query/PhabricatorEditEngineQuery.php', 'PhabricatorEditEngineSearchEngine' => 'applications/transactions/query/PhabricatorEditEngineSearchEngine.php', @@ -3170,7 +3183,9 @@ phutil_register_library_map(array( 'PhabricatorEditEngineSubtypeInterface' => 'applications/transactions/editengine/PhabricatorEditEngineSubtypeInterface.php', 'PhabricatorEditEngineSubtypeMap' => 'applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php', 'PhabricatorEditEngineSubtypeTestCase' => 'applications/transactions/editengine/__tests__/PhabricatorEditEngineSubtypeTestCase.php', + 'PhabricatorEditEngineSubtypeTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineSubtypeTransaction.php', 'PhabricatorEditEngineTokenizerCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineTokenizerCommentAction.php', + 'PhabricatorEditEngineTransactionType' => 'applications/transactions/xaction/PhabricatorEditEngineTransactionType.php', 'PhabricatorEditField' => 'applications/transactions/editfield/PhabricatorEditField.php', 'PhabricatorEditPage' => 'applications/transactions/editengine/PhabricatorEditPage.php', 'PhabricatorEditType' => 'applications/transactions/edittype/PhabricatorEditType.php', @@ -7381,6 +7396,7 @@ phutil_register_library_map(array( 'HeraldRuleIndexEngineExtension' => 'PhabricatorEdgeIndexEngineExtension', 'HeraldRuleListController' => 'HeraldController', 'HeraldRuleListView' => 'AphrontView', + 'HeraldRuleManagementWorkflow' => 'HeraldManagementWorkflow', 'HeraldRuleNameTransaction' => 'HeraldRuleTransactionType', 'HeraldRulePHIDType' => 'PhabricatorPHIDType', 'HeraldRuleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', @@ -8089,8 +8105,10 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionCommentView' => 'AphrontView', 'PhabricatorApplicationTransactionController' => 'PhabricatorController', 'PhabricatorApplicationTransactionDetailController' => 'PhabricatorApplicationTransactionController', + 'PhabricatorApplicationTransactionDetailView' => 'AphrontView', 'PhabricatorApplicationTransactionEditor' => 'PhabricatorEditor', 'PhabricatorApplicationTransactionFeedStory' => 'PhabricatorFeedStory', + 'PhabricatorApplicationTransactionJSONDiffDetailView' => 'PhabricatorApplicationTransactionDetailView', 'PhabricatorApplicationTransactionNoEffectException' => 'Exception', 'PhabricatorApplicationTransactionNoEffectResponse' => 'AphrontProxyResponse', 'PhabricatorApplicationTransactionPublishWorker' => 'PhabricatorWorker', @@ -8101,7 +8119,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionShowOlderController' => 'PhabricatorApplicationTransactionController', 'PhabricatorApplicationTransactionStructureException' => 'Exception', 'PhabricatorApplicationTransactionTemplatedCommentQuery' => 'PhabricatorApplicationTransactionCommentQuery', - 'PhabricatorApplicationTransactionTextDiffDetailView' => 'AphrontView', + 'PhabricatorApplicationTransactionTextDiffDetailView' => 'PhabricatorApplicationTransactionDetailView', 'PhabricatorApplicationTransactionTransactionPHIDType' => 'PhabricatorPHIDType', 'PhabricatorApplicationTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorApplicationTransactionValidationError' => 'Phobject', @@ -9226,18 +9244,28 @@ phutil_register_library_map(array( 'PhabricatorEditEngineConfigurationSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorEditEngineConfigurationSortController' => 'PhabricatorEditEngineController', 'PhabricatorEditEngineConfigurationSubtypeController' => 'PhabricatorEditEngineController', - 'PhabricatorEditEngineConfigurationTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorEditEngineConfigurationTransaction' => 'PhabricatorModularTransaction', 'PhabricatorEditEngineConfigurationTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorEditEngineConfigurationViewController' => 'PhabricatorEditEngineController', 'PhabricatorEditEngineController' => 'PhabricatorApplicationTransactionController', + 'PhabricatorEditEngineCreateOrderTransaction' => 'PhabricatorEditEngineTransactionType', 'PhabricatorEditEngineDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorEditEngineDefaultCreateTransaction' => 'PhabricatorEditEngineTransactionType', 'PhabricatorEditEngineDefaultLock' => 'PhabricatorEditEngineLock', + 'PhabricatorEditEngineDefaultTransaction' => 'PhabricatorEditEngineTransactionType', + 'PhabricatorEditEngineDisableTransaction' => 'PhabricatorEditEngineTransactionType', + 'PhabricatorEditEngineEditOrderTransaction' => 'PhabricatorEditEngineTransactionType', 'PhabricatorEditEngineExtension' => 'Phobject', 'PhabricatorEditEngineExtensionModule' => 'PhabricatorConfigModule', + 'PhabricatorEditEngineIsEditTransaction' => 'PhabricatorEditEngineTransactionType', 'PhabricatorEditEngineListController' => 'PhabricatorEditEngineController', 'PhabricatorEditEngineLock' => 'Phobject', + 'PhabricatorEditEngineLocksTransaction' => 'PhabricatorEditEngineTransactionType', 'PhabricatorEditEngineMFAEngine' => 'Phobject', + 'PhabricatorEditEngineNameTransaction' => 'PhabricatorEditEngineTransactionType', + 'PhabricatorEditEngineOrderTransaction' => 'PhabricatorEditEngineTransactionType', 'PhabricatorEditEnginePointsCommentAction' => 'PhabricatorEditEngineCommentAction', + 'PhabricatorEditEnginePreambleTransaction' => 'PhabricatorEditEngineTransactionType', 'PhabricatorEditEngineProfileMenuItem' => 'PhabricatorProfileMenuItem', 'PhabricatorEditEngineQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorEditEngineSearchEngine' => 'PhabricatorApplicationSearchEngine', @@ -9247,7 +9275,9 @@ phutil_register_library_map(array( 'PhabricatorEditEngineSubtype' => 'Phobject', 'PhabricatorEditEngineSubtypeMap' => 'Phobject', 'PhabricatorEditEngineSubtypeTestCase' => 'PhabricatorTestCase', + 'PhabricatorEditEngineSubtypeTransaction' => 'PhabricatorEditEngineTransactionType', 'PhabricatorEditEngineTokenizerCommentAction' => 'PhabricatorEditEngineCommentAction', + 'PhabricatorEditEngineTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorEditField' => 'Phobject', 'PhabricatorEditPage' => 'Phobject', 'PhabricatorEditType' => 'Phobject', diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 7995e2a36d..7fdb586f56 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -485,6 +485,11 @@ final class PhabricatorAuditEditor return $phids; } + protected function getObjectLinkButtonLabelForMail( + PhabricatorLiskDAO $object) { + return pht('View Commit'); + } + protected function buildMailBody( PhabricatorLiskDAO $object, array $xactions) { diff --git a/src/applications/auth/controller/PhabricatorLogoutController.php b/src/applications/auth/controller/PhabricatorLogoutController.php index 47592e0a2d..dccf6bb45b 100644 --- a/src/applications/auth/controller/PhabricatorLogoutController.php +++ b/src/applications/auth/controller/PhabricatorLogoutController.php @@ -4,7 +4,25 @@ final class PhabricatorLogoutController extends PhabricatorAuthController { public function shouldRequireLogin() { - return true; + // See T13310. We allow access to the "Logout" controller even if you are + // not logged in: otherwise, users who do not have access to any Spaces can + // not log out. + + // When you try to access a controller which requires you be logged in, + // and you do not have access to any Spaces, an access check fires first + // and prevents access with a "No Access to Spaces" error. If this + // controller requires users be logged in, users who are trying to log out + // and also have no access to Spaces get the error instead of a logout + // workflow and are trapped. + + // By permitting access to this controller even if you are not logged in, + // we bypass the Spaces check and allow users who have no access to Spaces + // to log out. + + // This incidentally allows users who are already logged out to access the + // controller, but this is harmless: we just no-op these requests. + + return false; } public function shouldRequireEmailVerification() { diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php index e4956335a0..7d73cb194d 100644 --- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php @@ -485,7 +485,7 @@ final class PhabricatorAuthSessionEngine extends Phobject { // change the order of prompts for users, but the alternative is that the // Settings panel order disagrees with the prompt order, which seems more // disruptive. - $factors = msort($factors, 'newSortVector'); + $factors = msortv($factors, 'newSortVector'); // If the account has no associated multi-factor auth, just issue a token // without putting the session into high security mode. This is generally diff --git a/src/applications/auth/engineextension/PhabricatorAuthMFAEditEngineExtension.php b/src/applications/auth/engineextension/PhabricatorAuthMFAEditEngineExtension.php index adad1f9274..f138193d69 100644 --- a/src/applications/auth/engineextension/PhabricatorAuthMFAEditEngineExtension.php +++ b/src/applications/auth/engineextension/PhabricatorAuthMFAEditEngineExtension.php @@ -38,6 +38,7 @@ final class PhabricatorAuthMFAEditEngineExtension ->setLabel(pht('MFA')) ->setIsFormField(false) ->setCommentActionLabel(pht('Sign With MFA')) + ->setCanApplyWithoutEditCapability(true) ->setCommentActionOrder(12000) ->setActionDescription( pht('You will be prompted to provide MFA when you submit.')) diff --git a/src/applications/auth/provider/PhabricatorAsanaAuthProvider.php b/src/applications/auth/provider/PhabricatorAsanaAuthProvider.php index 8a0fccec28..4067e522af 100644 --- a/src/applications/auth/provider/PhabricatorAsanaAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAsanaAuthProvider.php @@ -59,6 +59,14 @@ final class PhabricatorAsanaAuthProvider return null; } + if (strlen($uri->getFragment())) { + return null; + } + + if ($uri->getQueryParamsAsPairList()) { + return null; + } + $context_id = $matches[1]; $task_id = $matches[2]; diff --git a/src/applications/auth/provider/PhabricatorJIRAAuthProvider.php b/src/applications/auth/provider/PhabricatorJIRAAuthProvider.php index 23f7e7f706..c26c70ce37 100644 --- a/src/applications/auth/provider/PhabricatorJIRAAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorJIRAAuthProvider.php @@ -341,6 +341,14 @@ final class PhabricatorJIRAAuthProvider return null; } + if (strlen($uri->getFragment())) { + return null; + } + + if ($uri->getQueryParamsAsPairList()) { + return null; + } + $domain = $matches[1]; $issue = $matches[2]; diff --git a/src/applications/config/option/PhabricatorCoreConfigOptions.php b/src/applications/config/option/PhabricatorCoreConfigOptions.php index 48f6f24491..10f3e75b62 100644 --- a/src/applications/config/option/PhabricatorCoreConfigOptions.php +++ b/src/applications/config/option/PhabricatorCoreConfigOptions.php @@ -54,6 +54,20 @@ production instance were both in operation. EOREMARKUP )); + $timezone_description = $this->deformat(pht(<<newOption('phabricator.base-uri', 'string', null) @@ -96,14 +110,7 @@ EOREMARKUP $this->newOption('phabricator.timezone', 'string', null) ->setSummary( pht('The timezone Phabricator should use.')) - ->setDescription( - pht( - "PHP requires that you set a timezone in your php.ini before ". - "using date functions, or it will emit a warning. If this isn't ". - "possible (for instance, because you are using HPHP) you can set ". - "some valid constant for %s here and Phabricator will set it on ". - "your behalf, silencing the warning.", - 'date_default_timezone_set()')) + ->setDescription($timezone_description) ->addExample('America/New_York', pht('US East (EDT)')) ->addExample('America/Chicago', pht('US Central (CDT)')) ->addExample('America/Boise', pht('US Mountain (MDT)')) @@ -302,12 +309,10 @@ EOREMARKUP if (!$ok) { throw new PhabricatorConfigValidationException( pht( - "Config option '%s' is invalid. The timezone identifier must ". - "be a valid timezone identifier recognized by PHP, like '%s'. "." - You can find a list of valid identifiers here: %s", + 'Config option "%s" is invalid. The timezone identifier must '. + 'be a valid timezone identifier recognized by PHP, like "%s".', $key, - 'America/Los_Angeles', - 'http://php.net/manual/timezones.php')); + 'America/Los_Angeles')); } } } diff --git a/src/applications/differential/command/DifferentialActionEmailCommand.php b/src/applications/differential/command/DifferentialActionEmailCommand.php index cbf94487e9..1b1f8d64e3 100644 --- a/src/applications/differential/command/DifferentialActionEmailCommand.php +++ b/src/applications/differential/command/DifferentialActionEmailCommand.php @@ -56,7 +56,7 @@ final class DifferentialActionEmailCommand public function getCommandObjects() { $actions = DifferentialRevisionActionTransaction::loadAllActions(); - $actions = msort($actions, 'getRevisionActionOrderVector'); + $actions = msortv($actions, 'getRevisionActionOrderVector'); $objects = array(); foreach ($actions as $action) { diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 9544e70c16..b0471e7c5f 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1033,12 +1033,6 @@ final class DifferentialRevisionViewController } - /** - * Note this code is somewhat similar to the buildPatch method in - * @{class:DifferentialReviewRequestMail}. - * - * @return @{class:AphrontRedirectResponse} - */ private function buildRawDiffResponse( DifferentialRevision $revision, array $changesets, @@ -1100,15 +1094,17 @@ final class DifferentialRevisionViewController } $file_name .= 'diff'; - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - $file = PhabricatorFile::newFromFileData( - $raw_diff, - array( - 'name' => $file_name, - 'ttl.relative' => phutil_units('24 hours in seconds'), - 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, - )); + $iterator = new ArrayIterator(array($raw_diff)); + $source = id(new PhabricatorIteratorFileUploadSource()) + ->setName($file_name) + ->setMIMEType('text/plain') + ->setRelativeTTL(phutil_units('24 hours in seconds')) + ->setViewPolicy(PhabricatorPolicies::POLICY_NOONE) + ->setIterator($iterator); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $file = $source->uploadFile(); $file->attachToObject($revision->getPHID()); unset($unguarded); diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index eba641771e..88f13ff828 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -601,6 +601,11 @@ final class DifferentialTransactionEditor return $xactions; } + protected function getObjectLinkButtonLabelForMail( + PhabricatorLiskDAO $object) { + return pht('View Revision'); + } + protected function buildMailBody( PhabricatorLiskDAO $object, array $xactions) { @@ -610,14 +615,13 @@ final class DifferentialTransactionEditor $body = id(new PhabricatorMetaMTAMailBody()) ->setViewer($viewer); - $revision_uri = $object->getURI(); - $revision_uri = PhabricatorEnv::getProductionURI($revision_uri); + $revision_uri = $this->getObjectLinkButtonURIForMail($object); $new_uri = $revision_uri.'/new/'; $this->addHeadersAndCommentsToMailBody( $body, $xactions, - pht('View Revision'), + $this->getObjectLinkButtonLabelForMail($object), $revision_uri); $type_inline = DifferentialTransaction::TYPE_INLINE; diff --git a/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php index 6832108dc3..1a6017a02f 100644 --- a/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php @@ -44,6 +44,11 @@ final class DifferentialRevisionCloseTransaction $object->setProperty( DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED, $was_accepted); + + // See T13300. When a revision is closed, we promote it out of "Draft" + // immediately. This usually happens when a user creates a draft revision + // and then lands the associated commit before the revision leaves draft. + $object->setShouldBroadcast(true); } protected function validateAction($object, PhabricatorUser $viewer) { diff --git a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php index b2e382652f..43b4d31252 100644 --- a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php @@ -62,6 +62,23 @@ final class DiffusionRepositoryBranchesManagementPanel ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); + $drequest = DiffusionRequest::newFromDictionary( + array( + 'user' => $viewer, + 'repository' => $repository, + )); + + $view_uri = $drequest->generateURI( + array( + 'action' => 'branches', + )); + + $action_list->addAction( + id(new PhabricatorActionView()) + ->setIcon('fa-code-fork') + ->setName(pht('View Branches')) + ->setHref($view_uri)); + return $this->newCurtainView() ->setActionList($action_list); } @@ -111,98 +128,6 @@ final class DiffusionRepositoryBranchesManagementPanel $content[] = $this->newBox(pht('Branches'), $view); - if (!$repository->isImporting()) { - $request = $this->getRequest(); - $pager = id(new PHUIPagerView()) - ->readFromRequest($request); - - $params = array( - 'offset' => $pager->getOffset(), - 'limit' => $pager->getPageSize() + 1, - 'repository' => $repository->getID(), - ); - - $branches = id(new ConduitCall('diffusion.branchquery', $params)) - ->setUser($viewer) - ->execute(); - $branches = DiffusionRepositoryRef::loadAllFromDictionaries($branches); - $branches = $pager->sliceResults($branches); - $can_close_branches = ($repository->isHg()); - - $publisher = $repository->newPublisher(); - - $rows = array(); - foreach ($branches as $branch) { - $branch_name = $branch->getShortName(); - $permanent = $publisher->shouldPublishRef($branch); - - $default = $repository->getDefaultBranch(); - $icon = null; - if ($default == $branch->getShortName()) { - $icon = id(new PHUIIconView()) - ->setIcon('fa-code-fork'); - } - - $fields = $branch->getRawFields(); - $closed = idx($fields, 'closed'); - if ($closed) { - $status = pht('Closed'); - } else { - $status = pht('Open'); - } - - if ($publishing_disabled) { - $permanent_status = pht('Publishing Disabled'); - } else { - if ($permanent) { - $permanent_status = pht('Permanent'); - } else { - $permanent_status = pht('Not Permanent'); - } - } - - $rows[] = array( - $icon, - $branch_name, - $status, - $permanent_status, - ); - } - $branch_table = new AphrontTableView($rows); - $branch_table->setHeaders( - array( - '', - pht('Branch'), - pht('Status'), - pht('Permanent'), - )); - $branch_table->setColumnClasses( - array( - '', - 'pri', - 'narrow', - 'wide', - )); - $branch_table->setColumnVisibility( - array( - true, - true, - $can_close_branches, - true, - )); - - $box = $this->newBox(pht('Branch Status'), $branch_table); - $box->setPager($pager); - $content[] = $box; - } else { - $content[] = id(new PHUIInfoView()) - ->setSeverity(PHUIInfoView::SEVERITY_NOTICE) - ->appendChild( - pht( - 'Branch status is unavailable while the repository is still '. - 'importing.')); - } - return $content; } diff --git a/src/applications/diffusion/protocol/DiffusionGitUploadPackWireProtocol.php b/src/applications/diffusion/protocol/DiffusionGitUploadPackWireProtocol.php index 451ad74c9e..724c1c83b2 100644 --- a/src/applications/diffusion/protocol/DiffusionGitUploadPackWireProtocol.php +++ b/src/applications/diffusion/protocol/DiffusionGitUploadPackWireProtocol.php @@ -187,61 +187,76 @@ final class DiffusionGitUploadPackWireProtocol // \0 // // ... + // + // See T13309. The end of this list (which may be empty if a repository + // does not have any refs) has a list of zero or more of these: + // + // shallow + // + // These entries are present if the repository is a shallow clone + // which was made with the "--depth" flag. + // + // Note that "shallow" frames do not advertise capabilities, and if + // a repository has only "shallow" frames, capabilities are never + // advertised. $bytes = $frame['bytes']; $matches = array(); if ($is_first) { - $ok = preg_match( - '('. - '^'. - '(?P[0-9a-f]{40})'. - ' '. - '(?P[^\0\n]+)'. - '\0'. - '(?P[^\n]+)'. - '\n'. - '\z'. - ')', - $bytes, - $matches); - if (!$ok) { + $capabilities_pattern = '\0(?P[^\n]+)'; + } else { + $capabilities_pattern = ''; + } + + $ok = preg_match( + '('. + '^'. + '(?:'. + '(?P[0-9a-f]{40}) (?P[^\0\n]+)'.$capabilities_pattern. + '|'. + 'shallow (?P[0-9a-f]{40})'. + ')'. + '\n'. + '\z'. + ')', + $bytes, + $matches); + + if (!$ok) { + if ($is_first) { throw new Exception( pht( 'Unexpected "git upload-pack" initial protocol frame: expected '. - '" \0\n", got "%s".', + '" \0\n", or '. + '"shallow \n", got "%s".', $bytes)); - } - } else { - $ok = preg_match( - '('. - '^'. - '(?P[0-9a-f]{40})'. - ' '. - '(?P[^\0\n]+)'. - '\n'. - '\z'. - ')', - $bytes, - $matches); - if (!$ok) { + } else { throw new Exception( pht( 'Unexpected "git upload-pack" protocol frame: expected '. - '" \n", got "%s".', + '" \n", or "shallow \n", got "%s".', $bytes)); } } - $hash = $matches['hash']; - $name = $matches['name']; + if (isset($matches['shallow'])) { + $name = null; + $hash = $matches['shallow']; + $is_shallow = true; + } else { + $name = $matches['name']; + $hash = $matches['hash']; + $is_shallow = false; + } - if ($is_first) { + if (isset($matches['capabilities'])) { $capabilities = $matches['capabilities']; } $refs[] = array( 'hash' => $hash, 'name' => $name, + 'shallow' => $is_shallow, ); } @@ -252,10 +267,16 @@ final class DiffusionGitUploadPackWireProtocol ->setCapabilities($capabilities); foreach ($refs as $ref) { - $ref_list->addRef( - id(new DiffusionGitWireProtocolRef()) - ->setName($ref['name']) - ->setHash($ref['hash'])); + $wire_ref = id(new DiffusionGitWireProtocolRef()) + ->setHash($ref['hash']); + + if ($ref['shallow']) { + $wire_ref->setIsShallow(true); + } else { + $wire_ref->setName($ref['name']); + } + + $ref_list->addRef($wire_ref); } // TODO: Here, we have a structured list of refs. In a future change, @@ -275,10 +296,19 @@ final class DiffusionGitUploadPackWireProtocol // a little surprising, but is consistent with the native behavior of the // protocol. + // Likewise, we don't send back any capabilities if we're sending only + // "shallow" frames. + $output = array(); $is_first = true; foreach ($refs as $ref) { - if ($is_first) { + $is_shallow = $ref->getIsShallow(); + + if ($is_shallow) { + $result = sprintf( + "shallow %s\n", + $ref->getHash()); + } else if ($is_first) { $result = sprintf( "%s %s\0%s\n", $ref->getHash(), diff --git a/src/applications/diffusion/protocol/DiffusionGitWireProtocolRef.php b/src/applications/diffusion/protocol/DiffusionGitWireProtocolRef.php index bf5238c219..bd1672317a 100644 --- a/src/applications/diffusion/protocol/DiffusionGitWireProtocolRef.php +++ b/src/applications/diffusion/protocol/DiffusionGitWireProtocolRef.php @@ -5,6 +5,7 @@ final class DiffusionGitWireProtocolRef private $name; private $hash; + private $isShallow; public function setName($name) { $this->name = $name; @@ -24,9 +25,19 @@ final class DiffusionGitWireProtocolRef return $this->hash; } + public function setIsShallow($is_shallow) { + $this->isShallow = $is_shallow; + return $this; + } + + public function getIsShallow() { + return $this->isShallow; + } + public function newSortVector() { return id(new PhutilSortVector()) - ->addString($this->getName()); + ->addInt((int)$this->getIsShallow()) + ->addString((string)$this->getName()); } } diff --git a/src/applications/diffusion/xaction/DiffusionCommitActionTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitActionTransaction.php index d2e5f17ccb..1d351ffa5d 100644 --- a/src/applications/diffusion/xaction/DiffusionCommitActionTransaction.php +++ b/src/applications/diffusion/xaction/DiffusionCommitActionTransaction.php @@ -63,7 +63,7 @@ abstract class DiffusionCommitActionTransaction return false; } - return ($viewer->getPHID() === $commit->getAuthorPHID()); + return ($viewer->getPHID() === $commit->getEffectiveAuthorPHID()); } public function newEditField( diff --git a/src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php b/src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php index 909e45c31f..58d5155aed 100644 --- a/src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php +++ b/src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php @@ -46,10 +46,44 @@ final class PhabricatorFilesManagementMigrateWorkflow 'name' => 'names', 'wildcard' => true, ), + array( + 'name' => 'from-engine', + 'param' => 'engine', + 'help' => pht('Migrate files from the named storage engine.'), + ), + array( + 'name' => 'local-disk-source', + 'param' => 'path', + 'help' => pht( + 'When migrating from a local disk source, use the specified '. + 'path as the root directory.'), + ), )); } public function execute(PhutilArgumentParser $args) { + + // See T13306. This flag allows you to import files from a backup of + // local disk storage into some other engine. When the caller provides + // the flag, we override the local disk engine configuration and treat + // it as though it is configured to use the specified location. + + $local_disk_source = $args->getArg('local-disk-source'); + if (strlen($local_disk_source)) { + $path = Filesystem::resolvePath($local_disk_source); + try { + Filesystem::assertIsDirectory($path); + } catch (FilesystemException $ex) { + throw new PhutilArgumentUsageException( + pht( + 'The "--local-disk-source" argument must point to a valid, '. + 'readable directory on local disk.')); + } + + $env = PhabricatorEnv::beginScopedEnv(); + $env->overrideEnvConfig('storage.local-disk.path', $path); + } + $target_key = $args->getArg('engine'); if (!$target_key) { throw new PhutilArgumentUsageException( diff --git a/src/applications/files/management/PhabricatorFilesManagementWorkflow.php b/src/applications/files/management/PhabricatorFilesManagementWorkflow.php index e94fa1d96a..44d43dc66a 100644 --- a/src/applications/files/management/PhabricatorFilesManagementWorkflow.php +++ b/src/applications/files/management/PhabricatorFilesManagementWorkflow.php @@ -4,23 +4,48 @@ abstract class PhabricatorFilesManagementWorkflow extends PhabricatorManagementWorkflow { protected function buildIterator(PhutilArgumentParser $args) { + $viewer = $this->getViewer(); $names = $args->getArg('names'); - if ($args->getArg('all')) { - if ($names) { - throw new PhutilArgumentUsageException( - pht( - 'Specify either a list of files or `%s`, but not both.', - '--all')); - } - return new LiskMigrationIterator(new PhabricatorFile()); + $is_all = $args->getArg('all'); + $from_engine = $args->getArg('from-engine'); + + $any_constraint = ($from_engine || $names); + + if (!$is_all && !$any_constraint) { + throw new PhutilArgumentUsageException( + pht( + 'Use "--all" to migrate all files, or choose files to migrate '. + 'with "--names" or "--from-engine".')); } + if ($is_all && $any_constraint) { + throw new PhutilArgumentUsageException( + pht( + 'You can not migrate all files with "--all" and also migrate only '. + 'a subset of files with "--from-engine" or "--names".')); + } + + // If we're migrating specific named files, convert the names into IDs + // first. + $ids = null; if ($names) { - return $this->loadFilesWithNames($names); + $files = $this->loadFilesWithNames($names); + $ids = mpull($files, 'getID'); } - return null; + $query = id(new PhabricatorFileQuery()) + ->setViewer($viewer); + + if ($ids) { + $query->withIDs($ids); + } + + if ($from_engine) { + $query->withStorageEngines(array($from_engine)); + } + + return new PhabricatorQueryIterator($query); } protected function loadFilesWithNames(array $names) { @@ -36,7 +61,7 @@ abstract class PhabricatorFilesManagementWorkflow if (empty($files[$name])) { throw new PhutilArgumentUsageException( pht( - "No file '%s' exists!", + 'No file "%s" exists.', $name)); } } diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php index 4205ab5c5d..c19574acaa 100644 --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -19,6 +19,7 @@ final class PhabricatorFileQuery private $needTransforms; private $builtinKeys; private $isBuiltin; + private $storageEngines; public function withIDs(array $ids) { $this->ids = $ids; @@ -137,6 +138,11 @@ final class PhabricatorFileQuery $ngrams); } + public function withStorageEngines(array $engines) { + $this->storageEngines = $engines; + return $this; + } + public function showOnlyExplicitUploads($explicit_uploads) { $this->explicitUploads = $explicit_uploads; return $this; @@ -469,6 +475,13 @@ final class PhabricatorFileQuery } } + if ($this->storageEngines !== null) { + $where[] = qsprintf( + $conn, + 'storageEngine IN (%Ls)', + $this->storageEngines); + } + return $where; } diff --git a/src/applications/harbormaster/engine/HarbormasterBuildGraph.php b/src/applications/harbormaster/engine/HarbormasterBuildGraph.php index dc9e4b7691..64c12c2e8f 100644 --- a/src/applications/harbormaster/engine/HarbormasterBuildGraph.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildGraph.php @@ -25,8 +25,7 @@ final class HarbormasterBuildGraph extends AbstractDirectedGraph { $graph = id(new HarbormasterBuildGraph($steps_by_phid)) ->addNodes($step_phids); - $raw_results = - $graph->getBestEffortTopographicallySortedNodes(); + $raw_results = $graph->getNodesInRoughTopologicalOrder(); $results = array(); foreach ($raw_results as $node) { diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 69f538afcc..cb8545b71d 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -130,6 +130,14 @@ abstract class HeraldAdapter extends Phobject { abstract public function getHeraldName(); + final public function willGetHeraldField($field_key) { + // This method is called during rule evaluation, before we engage the + // Herald profiler. We make sure we have a concrete implementation so time + // spent loading fields out of the classmap is not mistakenly attributed to + // whichever field happens to evaluate first. + $this->requireFieldImplementation($field_key); + } + public function getHeraldField($field_key) { return $this->requireFieldImplementation($field_key) ->getHeraldFieldValue($this->getObject()); diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php index 60dd6dc6ed..09a23408d3 100644 --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -77,6 +77,7 @@ final class HeraldTranscriptController extends HeraldController { $this->buildTransactionsTranscriptPanel( $object, $xscript), + $this->buildProfilerTranscriptPanel($xscript), ); } @@ -597,4 +598,98 @@ final class HeraldTranscriptController extends HeraldController { } + private function buildProfilerTranscriptPanel(HeraldTranscript $xscript) { + $viewer = $this->getViewer(); + + $object_xscript = $xscript->getObjectTranscript(); + + $profile = $object_xscript->getProfile(); + + // If this is an older transcript without profiler information, don't + // show anything. + if ($profile === null) { + return null; + } + + $profile = isort($profile, 'elapsed'); + $profile = array_reverse($profile); + + $phids = array(); + foreach ($profile as $frame) { + if ($frame['type'] === 'rule') { + $phids[] = $frame['key']; + } + } + $handles = $viewer->loadHandles($phids); + + $field_map = HeraldField::getAllFields(); + + $rows = array(); + foreach ($profile as $frame) { + $cost = $frame['elapsed']; + $cost = 1000000 * $cost; + $cost = pht('%sus', new PhutilNumber($cost)); + + $type = $frame['type']; + switch ($type) { + case 'rule': + $type_display = pht('Rule'); + break; + case 'field': + $type_display = pht('Field'); + break; + default: + $type_display = $type; + break; + } + + $key = $frame['key']; + switch ($type) { + case 'field': + $field_object = idx($field_map, $key); + if ($field_object) { + $key_display = $field_object->getHeraldFieldName(); + } else { + $key_display = $key; + } + break; + case 'rule': + $key_display = $handles[$key]->renderLink(); + break; + default: + $key_display = $key; + break; + } + + $rows[] = array( + $type_display, + $key_display, + $cost, + pht('%s', new PhutilNumber($frame['count'])), + ); + } + + $table_view = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Type'), + pht('What'), + pht('Cost'), + pht('Count'), + )) + ->setColumnClasses( + array( + null, + 'wide', + 'right', + 'right', + )); + + $box_view = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Profile')) + ->setTable($table_view); + + return $box_view; + } + } diff --git a/src/applications/herald/engine/HeraldEngine.php b/src/applications/herald/engine/HeraldEngine.php index 2d96532882..9ca72821e7 100644 --- a/src/applications/herald/engine/HeraldEngine.php +++ b/src/applications/herald/engine/HeraldEngine.php @@ -16,6 +16,9 @@ final class HeraldEngine extends Phobject { private $forbiddenActions = array(); private $skipEffects = array(); + private $profilerStack = array(); + private $profilerFrames = array(); + public function setDryRun($dry_run) { $this->dryRun = $dry_run; return $this; @@ -137,7 +140,8 @@ final class HeraldEngine extends Phobject { ->setName($object->getHeraldName()) ->setType($object->getAdapterContentType()) ->setFields($this->fieldCache) - ->setAppliedTransactionPHIDs($xaction_phids); + ->setAppliedTransactionPHIDs($xaction_phids) + ->setProfile($this->getProfile()); $this->transcript->setObjectTranscript($object_transcript); @@ -329,7 +333,36 @@ final class HeraldEngine extends Phobject { break; } - $match = $this->doesConditionMatch($rule, $condition, $object); + // Here, we're profiling the cost to match the condition value against + // whatever test is configured. Normally, this cost should be very + // small (<<1ms) since it amounts to a single comparison: + // + // [ Task author ][ is any of ][ alice ] + // + // However, it may be expensive in some cases, particularly if you + // write a rule with a very creative regular expression that backtracks + // explosively. + // + // At time of writing, the "Another Herald Rule" field is also + // evaluated inside the matching function. This may be arbitrarily + // expensive (it can prompt us to execute any finite number of other + // Herald rules), although we'll push the profiler stack appropriately + // so we don't count the evaluation time against this rule in the final + // profile. + + $caught = null; + + $this->pushProfilerRule($rule); + try { + $match = $this->doesConditionMatch($rule, $condition, $object); + } catch (Exception $ex) { + $caught = $ex; + } + $this->popProfilerRule($rule); + + if ($caught) { + throw $ex; + } if (!$all && $match) { $reason = pht('Any condition matched.'); @@ -421,7 +454,25 @@ final class HeraldEngine extends Phobject { public function getObjectFieldValue($field) { if (!array_key_exists($field, $this->fieldCache)) { - $this->fieldCache[$field] = $this->object->getHeraldField($field); + $adapter = $this->object; + + $adapter->willGetHeraldField($field); + + $caught = null; + + $this->pushProfilerField($field); + try { + $value = $adapter->getHeraldField($field); + } catch (Exception $ex) { + $caught = $ex; + } + $this->popProfilerField($field); + + if ($caught) { + throw $caught; + } + + $this->fieldCache[$field] = $value; } return $this->fieldCache[$field]; @@ -637,4 +688,98 @@ final class HeraldEngine extends Phobject { return $is_forbidden; } +/* -( Profiler )----------------------------------------------------------- */ + + private function pushProfilerField($field_key) { + return $this->pushProfilerStack('field', $field_key); + } + + private function popProfilerField($field_key) { + return $this->popProfilerStack('field', $field_key); + } + + private function pushProfilerRule(HeraldRule $rule) { + return $this->pushProfilerStack('rule', $rule->getPHID()); + } + + private function popProfilerRule(HeraldRule $rule) { + return $this->popProfilerStack('rule', $rule->getPHID()); + } + + private function pushProfilerStack($type, $key) { + $this->profilerStack[] = array( + 'type' => $type, + 'key' => $key, + 'start' => microtime(true), + ); + + return $this; + } + + private function popProfilerStack($type, $key) { + if (!$this->profilerStack) { + throw new Exception( + pht( + 'Unable to pop profiler stack: profiler stack is empty.')); + } + + $frame = last($this->profilerStack); + if (($frame['type'] !== $type) || ($frame['key'] !== $key)) { + throw new Exception( + pht( + 'Unable to pop profiler stack: expected frame of type "%s" with '. + 'key "%s", but found frame of type "%s" with key "%s".', + $type, + $key, + $frame['type'], + $frame['key'])); + } + + // Accumulate the new timing information into the existing profile. If this + // is the first time we've seen this particular rule or field, we'll + // create a new empty frame first. + + $elapsed = microtime(true) - $frame['start']; + $frame_key = sprintf('%s/%s', $type, $key); + + if (!isset($this->profilerFrames[$frame_key])) { + $current = array( + 'type' => $type, + 'key' => $key, + 'elapsed' => 0, + 'count' => 0, + ); + } else { + $current = $this->profilerFrames[$frame_key]; + } + + $current['elapsed'] += $elapsed; + $current['count']++; + + $this->profilerFrames[$frame_key] = $current; + + array_pop($this->profilerStack); + } + + private function getProfile() { + if ($this->profilerStack) { + $frame = last($this->profilerStack); + $frame_type = $frame['type']; + $frame_key = $frame['key']; + $frame_count = count($this->profilerStack); + + throw new Exception( + pht( + 'Unable to retrieve profile: profiler stack is not empty. The '. + 'stack has %s frame(s); the final frame has type "%s" and key '. + '"%s".', + new PhutilNumber($frame_count), + $frame_type, + $frame_key)); + } + + return array_values($this->profilerFrames); + } + + } diff --git a/src/applications/herald/management/HeraldRuleManagementWorkflow.php b/src/applications/herald/management/HeraldRuleManagementWorkflow.php new file mode 100644 index 0000000000..3380b21da7 --- /dev/null +++ b/src/applications/herald/management/HeraldRuleManagementWorkflow.php @@ -0,0 +1,106 @@ +setName('rule') + ->setExamples('**rule** --rule __rule__ --disable') + ->setSynopsis( + pht( + 'Modify a rule, bypassing policies. This workflow can disable '. + 'problematic personal rules.')) + ->setArguments( + array( + array( + 'name' => 'rule', + 'param' => 'rule', + 'help' => pht('Apply changes to this rule.'), + ), + array( + 'name' => 'disable', + 'help' => pht('Disable the rule.'), + ), + array( + 'name' => 'enable', + 'help' => pht('Enable the rule.'), + ), + )); + } + + public function execute(PhutilArgumentParser $args) { + $viewer = $this->getViewer(); + + $rule_name = $args->getArg('rule'); + if (!strlen($rule_name)) { + throw new PhutilArgumentUsageException( + pht('Specify a rule to edit with "--rule ".')); + } + + if (preg_match('/^H\d+/', $rule_name)) { + $rule_id = substr($rule_name, 1); + } else { + $rule_id = $rule_name; + } + + $rule = id(new HeraldRuleQuery()) + ->setViewer($viewer) + ->withIDs(array($rule_id)) + ->executeOne(); + if (!$rule) { + throw new PhutilArgumentUsageException( + pht( + 'Unable to load Herald rule with ID or monogram "%s".', + $rule_name)); + } + + $is_disable = $args->getArg('disable'); + $is_enable = $args->getArg('enable'); + + $xactions = array(); + + if ($is_disable && $is_enable) { + throw new PhutilArgumentUsageException( + pht( + 'Specify "--enable" or "--disable", but not both.')); + } else if ($is_disable || $is_enable) { + $xactions[] = $rule->getApplicationTransactionTemplate() + ->setTransactionType(HeraldRuleDisableTransaction::TRANSACTIONTYPE) + ->setNewValue($is_disable); + } + + if (!$xactions) { + throw new PhutilArgumentUsageException( + pht( + 'Use flags to specify at least one edit to apply to the '. + 'rule (for example, use "--disable" to disable a rule).')); + } + + $herald_phid = id(new PhabricatorHeraldApplication())->getPHID(); + + $editor = $rule->getApplicationTransactionEditor() + ->setActor($viewer) + ->setActingAsPHID($herald_phid) + ->setContentSource($this->newContentSource()) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true); + + echo tsprintf( + "%s\n", + pht( + 'Applying changes to %s: %s...', + $rule->getMonogram(), + $rule->getName())); + + $editor->applyTransactions($rule, $xactions); + + echo tsprintf( + "%s\n", + pht('Done.')); + + + return 0; + } + +} diff --git a/src/applications/herald/storage/transcript/HeraldObjectTranscript.php b/src/applications/herald/storage/transcript/HeraldObjectTranscript.php index 9d331b271e..b5f9aa783f 100644 --- a/src/applications/herald/storage/transcript/HeraldObjectTranscript.php +++ b/src/applications/herald/storage/transcript/HeraldObjectTranscript.php @@ -7,6 +7,7 @@ final class HeraldObjectTranscript extends Phobject { protected $name; protected $fields; protected $appliedTransactionPHIDs; + protected $profile; public function setPHID($phid) { $this->phid = $phid; @@ -48,6 +49,15 @@ final class HeraldObjectTranscript extends Phobject { return $this->fields; } + public function setProfile(array $profile) { + $this->profile = $profile; + return $this; + } + + public function getProfile() { + return $this->profile; + } + public function setAppliedTransactionPHIDs($phids) { $this->appliedTransactionPHIDs = $phids; return $this; diff --git a/src/applications/herald/xaction/HeraldRuleEditTransaction.php b/src/applications/herald/xaction/HeraldRuleEditTransaction.php index c4b03983fb..653d300906 100644 --- a/src/applications/herald/xaction/HeraldRuleEditTransaction.php +++ b/src/applications/herald/xaction/HeraldRuleEditTransaction.php @@ -40,17 +40,10 @@ final class HeraldRuleEditTransaction public function newChangeDetailView() { $viewer = $this->getViewer(); - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $json = new PhutilJSON(); - $old_json = $json->encodeFormatted($old); - $new_json = $json->encodeFormatted($new); - - return id(new PhabricatorApplicationTransactionTextDiffDetailView()) + return id(new PhabricatorApplicationTransactionJSONDiffDetailView()) ->setViewer($viewer) - ->setOldText($old_json) - ->setNewText($new_json); + ->setOld($this->getOldValue()) + ->setNew($this->getNewValue()); } } diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index fd5bfe0cdc..6255903eff 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -206,6 +206,11 @@ final class ManiphestTransactionEditor ->setSubject("T{$id}: {$title}"); } + protected function getObjectLinkButtonLabelForMail( + PhabricatorLiskDAO $object) { + return pht('View Task'); + } + protected function buildMailBody( PhabricatorLiskDAO $object, array $xactions) { diff --git a/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php b/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php index 5c17b11321..73917a2d84 100644 --- a/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php +++ b/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php @@ -29,6 +29,12 @@ final class PhabricatorMailManagementReceiveTestWorkflow 'param' => 'object', 'help' => pht('Simulate mail delivery "To:" the given object.'), ), + array( + 'name' => 'cc', + 'param' => 'address', + 'help' => pht('Simulate a mail delivery "Cc:" address.'), + 'repeat' => true, + ), )); } @@ -84,6 +90,8 @@ final class PhabricatorMailManagementReceiveTestWorkflow $from = $user->loadPrimaryEmail()->getAddress(); } + $cc = $args->getArg('cc'); + $console->writeErr("%s\n", pht('Reading message body from stdin...')); $body = file_get_contents('php://stdin'); @@ -91,6 +99,7 @@ final class PhabricatorMailManagementReceiveTestWorkflow $header_content = array( 'Message-ID' => Filesystem::readRandomCharacters(12), 'From' => $from, + 'Cc' => implode(', ', $cc), ); if (preg_match('/.+@.+/', $to)) { diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php index 64528ea949..c620bea30a 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php @@ -104,24 +104,43 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { } public function loadAllRecipientPHIDs() { - $addresses = array_merge( - $this->getToAddresses(), - $this->getCCAddresses()); + $addresses = $this->newTargetAddresses(); - return $this->loadPHIDsFromAddresses($addresses); - } + // See T13317. Don't allow reserved addresses (like "noreply@...") to + // match user PHIDs. + foreach ($addresses as $key => $address) { + if (PhabricatorMailUtil::isReservedAddress($address)) { + unset($addresses[$key]); + } + } - public function loadCCPHIDs() { - return $this->loadPHIDsFromAddresses($this->getCCAddresses()); - } - - private function loadPHIDsFromAddresses(array $addresses) { - if (empty($addresses)) { + if (!$addresses) { return array(); } - $users = id(new PhabricatorUserEmail()) - ->loadAllWhere('address IN (%Ls)', $addresses); - return mpull($users, 'getUserPHID'); + + $address_strings = array(); + foreach ($addresses as $address) { + $address_strings[] = phutil_string_cast($address->getAddress()); + } + + // See T13317. If a verified email address is in the "To" or "Cc" line, + // we'll count the user who owns that address as a recipient. + + // We require the address be verified because we'll trigger behavior (like + // adding subscribers) based on the recipient list, and don't want to add + // Alice as a subscriber if she adds an unverified "internal-bounces@" + // address to her account and this address gets caught in the crossfire. + // In the best case this is confusing; in the worst case it could + // some day give her access to objects she can't see. + + $recipients = id(new PhabricatorUserEmail()) + ->loadAllWhere( + 'address IN (%Ls) AND isVerified = 1', + $address_strings); + + $recipient_phids = mpull($recipients, 'getUserPHID'); + + return $recipient_phids; } public function processReceivedMail() { diff --git a/src/applications/owners/storage/PhabricatorOwnersPath.php b/src/applications/owners/storage/PhabricatorOwnersPath.php index 7022cb887c..5bb32f8407 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPath.php +++ b/src/applications/owners/storage/PhabricatorOwnersPath.php @@ -79,15 +79,27 @@ final class PhabricatorOwnersPath extends PhabricatorOwnersDAO { public static function getSetFromTransactionValue(array $v) { $set = array(); foreach ($v as $ref) { - $set[$ref['repositoryPHID']][$ref['path']][$ref['excluded']] = true; + $key = self::getScalarKeyForRef($ref); + $set[$key] = true; } return $set; } public static function isRefInSet(array $ref, array $set) { - return isset($set[$ref['repositoryPHID']][$ref['path']][$ref['excluded']]); + $key = self::getScalarKeyForRef($ref); + return isset($set[$key]); } + private static function getScalarKeyForRef(array $ref) { + return sprintf( + 'repository=%s path=%s display=%s excluded=%d', + $ref['repositoryPHID'], + $ref['path'], + $ref['display'], + $ref['excluded']); + } + + /** * Get the number of directory matches between this path specification and * some real path. diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php index 753b6ff9e9..a8bb1cc259 100644 --- a/src/applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php +++ b/src/applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php @@ -12,12 +12,33 @@ final class PhabricatorOwnersPackagePathsTransaction public function generateNewValue($object, $value) { $new = $value; + foreach ($new as $key => $info) { - $new[$key]['excluded'] = (int)idx($info, 'excluded'); + $info['excluded'] = (int)idx($info, 'excluded'); + + // The input has one "path" key with the display path. + // Move it to "display", then normalize the value in "path". + + $display_path = $info['path']; + $raw_path = rtrim($display_path, '/').'/'; + + $info['path'] = $raw_path; + $info['display'] = $display_path; + + $new[$key] = $info; } + return $new; } + public function getTransactionHasEffect($object, $old, $new) { + list($add, $rem) = PhabricatorOwnersPath::getTransactionValueChanges( + $old, + $new); + + return ($add || $rem); + } + public function validateTransactions($object, array $xactions) { $errors = array(); @@ -110,8 +131,8 @@ final class PhabricatorOwnersPackagePathsTransaction $display_map = array(); $seen_map = array(); foreach ($new as $key => $spec) { - $display_path = $spec['path']; - $raw_path = rtrim($display_path, '/').'/'; + $raw_path = $spec['path']; + $display_path = $spec['display']; // If the user entered two paths in the same repository which normalize // to the same value (like "src/main.c" and "src/main.c/"), discard the @@ -193,11 +214,18 @@ final class PhabricatorOwnersPackagePathsTransaction $rowc = array(); foreach ($rows as $key => $row) { $rowc[] = $row['class']; + + if (array_key_exists('display', $row)) { + $display_path = $row['display']; + } else { + $display_path = $row['path']; + } + $rows[$key] = array( $row['change'], $row['excluded'] ? pht('Exclude') : pht('Include'), $this->renderHandle($row['repositoryPHID']), - $row['path'], + $display_path, ); } diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php index 038fa416f8..c1bb9190b3 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php @@ -398,7 +398,7 @@ final class PhabricatorRepositoryDiscoveryEngine } } - // Now, sort them topographically. + // Now, sort them topologically. $commits = $this->reduceGraph($graph); $refs = array(); @@ -437,7 +437,7 @@ final class PhabricatorRepositoryDiscoveryEngine $graph = new PhutilDirectedScalarGraph(); $graph->addNodes($edges); - $commits = $graph->getTopographicallySortedNodes(); + $commits = $graph->getNodesInTopologicalOrder(); // NOTE: We want the most ancestral nodes first, so we need to reverse the // list we get out of AbstractDirectedGraph. diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php index e0aa8b4c5b..ebd6edff97 100644 --- a/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php @@ -252,12 +252,24 @@ final class PhabricatorRepositoryManagementReparseWorkflow ); foreach ($classes as $class) { - PhabricatorWorker::scheduleTask( - $class, - $spec, - array( - 'priority' => PhabricatorWorker::PRIORITY_IMPORT, - )); + try { + PhabricatorWorker::scheduleTask( + $class, + $spec, + array( + 'priority' => PhabricatorWorker::PRIORITY_IMPORT, + )); + } catch (PhabricatorWorkerPermanentFailureException $ex) { + // See T13315. We expect some reparse steps to occasionally raise + // permanent failures: for example, because they are no longer + // reachable. This is a routine condition, not a catastrophic + // failure, so let the user know something happened but continue + // reparsing any remaining commits. + echo tsprintf( + "** %s ** %s\n", + pht('WARN'), + $ex->getMessage()); + } } $progress->update(1); diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php index 6b8ac687b3..b5c2fde82a 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -466,6 +466,18 @@ final class PhabricatorRepositoryCommit return $data->getCommitDetail('authorPHID'); } + public function getEffectiveAuthorPHID() { + if ($this->hasAuthorIdentity()) { + $identity = $this->getAuthorIdentity(); + if ($identity->hasEffectiveUser()) { + return $identity->getCurrentEffectiveUserPHID(); + } + } + + $data = $this->getCommitData(); + return $data->getCommitDetail('authorPHID'); + } + public function getAuditStatusObject() { $status = $this->getAuditStatus(); return DiffusionCommitAuditStatus::newForStatus($status); diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php index 17074f8cb4..0b0a194806 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php @@ -147,7 +147,7 @@ final class PhabricatorRepositoryCommitPublishWorker $data = $commit->getCommitData(); - $author_phid = $data->getCommitDetail('authorPHID'); + $author_phid = $commit->getEffectiveAuthorPHID(); $revision = DiffusionCommitRevisionQuery::loadRevisionForCommit( $viewer, @@ -223,13 +223,22 @@ final class PhabricatorRepositoryCommitPublishWorker // If auditing is configured to trigger on unreviewed changes, check if // the revision was "Accepted" when it landed. If not, trigger an audit. + + // We may be running before the revision actually closes, so we'll count + // either an "Accepted" or a "Closed, Previously Accepted" revision as + // good enough. + if ($audit_unreviewed) { $commit_unreviewed = true; if ($revision) { - $was_accepted = DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED; - if ($revision->isPublished()) { - if ($revision->getProperty($was_accepted)) { - $commit_unreviewed = false; + if ($revision->isAccepted()) { + $commit_unreviewed = false; + } else { + $was_accepted = DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED; + if ($revision->isPublished()) { + if ($revision->getProperty($was_accepted)) { + $commit_unreviewed = false; + } } } } diff --git a/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php b/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php index 6a0ba19d03..8ae2704161 100644 --- a/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php +++ b/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php @@ -20,7 +20,8 @@ final class PhabricatorSettingsTimezoneController $zone = new DateTimeZone($identifier); $offset = -($zone->getOffset($now) / 60); if ($offset == $client_offset) { - $options[$identifier] = $identifier; + $name = PhabricatorTime::getTimezoneDisplayName($identifier); + $options[$identifier] = $name; } } diff --git a/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php b/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php index 60389e1590..d20782d2b5 100644 --- a/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php @@ -94,7 +94,7 @@ final class PhabricatorExternalAccountsSettingsPanel ->setViewer($viewer) ->withIsEnabled(true) ->execute(); - $configs = msort($configs, 'getSortVector'); + $configs = msortv($configs, 'getSortVector'); $account_map = mgroup($accounts, 'getProviderConfigPHID'); diff --git a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php index 09193f3c96..abbb88c0a5 100644 --- a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php @@ -54,7 +54,7 @@ final class PhabricatorMultiFactorSettingsPanel ->setViewer($viewer) ->withUserPHIDs(array($user->getPHID())) ->execute(); - $factors = msort($factors, 'newSortVector'); + $factors = msortv($factors, 'newSortVector'); $rows = array(); $rowc = array(); diff --git a/src/applications/settings/setting/PhabricatorTimezoneSetting.php b/src/applications/settings/setting/PhabricatorTimezoneSetting.php index 52fce77428..207ad95f62 100644 --- a/src/applications/settings/setting/PhabricatorTimezoneSetting.php +++ b/src/applications/settings/setting/PhabricatorTimezoneSetting.php @@ -81,9 +81,16 @@ final class PhabricatorTimezoneSetting } sort($group); + + $group_map = array(); + foreach ($group as $identifier) { + $name = PhabricatorTime::getTimezoneDisplayName($identifier); + $group_map[$identifier] = $name; + } + $option_groups[] = array( 'label' => $label, - 'options' => array_fuse($group), + 'options' => $group_map, ); } diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultCreateController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultCreateController.php index d7ea8810a5..039bfb0f44 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultCreateController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultCreateController.php @@ -15,7 +15,7 @@ final class PhabricatorEditEngineConfigurationDefaultCreateController $key = $config->getIdentifier(); $cancel_uri = "/transactions/editengine/{$engine_key}/view/{$key}/"; - $type = PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULTCREATE; + $type = PhabricatorEditEngineDefaultCreateTransaction::TRANSACTIONTYPE; if ($request->isFormPost()) { $xactions = array(); diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultsController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultsController.php index f7361d50cf..3d63a4a098 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultsController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultsController.php @@ -52,7 +52,7 @@ final class PhabricatorEditEngineConfigurationDefaultsController $field->readValueFromSubmit($request); } - $type = PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULT; + $type = PhabricatorEditEngineDefaultTransaction::TRANSACTIONTYPE; $xactions = array(); foreach ($fields as $field) { diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDisableController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDisableController.php index 24a32c5598..a3311b2d49 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDisableController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationDisableController.php @@ -15,7 +15,7 @@ final class PhabricatorEditEngineConfigurationDisableController $key = $config->getIdentifier(); $cancel_uri = "/transactions/editengine/{$engine_key}/view/{$key}/"; - $type = PhabricatorEditEngineConfigurationTransaction::TYPE_DISABLE; + $type = PhabricatorEditEngineDisableTransaction::TRANSACTIONTYPE; if ($request->isFormPost()) { $xactions = array(); diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationIsEditController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationIsEditController.php index 970a2512f1..b93390ff66 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationIsEditController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationIsEditController.php @@ -15,8 +15,7 @@ final class PhabricatorEditEngineConfigurationIsEditController $key = $config->getIdentifier(); $cancel_uri = "/transactions/editengine/{$engine_key}/view/{$key}/"; - $type = PhabricatorEditEngineConfigurationTransaction::TYPE_ISEDIT; - + $type = PhabricatorEditEngineIsEditTransaction::TRANSACTIONTYPE; if ($request->isFormPost()) { $xactions = array(); diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationLockController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationLockController.php index 34b099b9f0..1375124585 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationLockController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationLockController.php @@ -30,8 +30,7 @@ final class PhabricatorEditEngineConfigurationLockController $xactions = array(); $locks = $request->getArr('locks'); - $type_locks = PhabricatorEditEngineConfigurationTransaction::TYPE_LOCKS; - + $type_locks = PhabricatorEditEngineLocksTransaction::TRANSACTIONTYPE; $xactions[] = id(new PhabricatorEditEngineConfigurationTransaction()) ->setTransactionType($type_locks) ->setNewValue($locks); diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationReorderController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationReorderController.php index 6ff36cdfa4..563c3141b2 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationReorderController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationReorderController.php @@ -31,8 +31,7 @@ final class PhabricatorEditEngineConfigurationReorderController $xactions = array(); $key_order = $request->getStrList('keyOrder'); - $type_order = PhabricatorEditEngineConfigurationTransaction::TYPE_ORDER; - + $type_order = PhabricatorEditEngineOrderTransaction::TRANSACTIONTYPE; $xactions[] = id(new PhabricatorEditEngineConfigurationTransaction()) ->setTransactionType($type_order) ->setNewValue($key_order); diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSortController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSortController.php index 613a847326..5e8680b651 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSortController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSortController.php @@ -70,10 +70,10 @@ final class PhabricatorEditEngineConfigurationSortController if ($is_create) { $xaction_type = - PhabricatorEditEngineConfigurationTransaction::TYPE_CREATEORDER; + PhabricatorEditEngineCreateOrderTransaction::TRANSACTIONTYPE; } else { $xaction_type = - PhabricatorEditEngineConfigurationTransaction::TYPE_EDITORDER; + PhabricatorEditEngineEditOrderTransaction::TRANSACTIONTYPE; } $xactions[] = id(new PhabricatorEditEngineConfigurationTransaction()) diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSubtypeController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSubtypeController.php index 9ce8d7a0e5..918782cd81 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSubtypeController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSubtypeController.php @@ -35,9 +35,7 @@ final class PhabricatorEditEngineConfigurationSubtypeController $xactions = array(); $subtype = $request->getStr('subtype'); - $type_subtype = - PhabricatorEditEngineConfigurationTransaction::TYPE_SUBTYPE; - + $type_subtype = PhabricatorEditEngineSubtypeTransaction::TRANSACTIONTYPE; $xactions[] = id(new PhabricatorEditEngineConfigurationTransaction()) ->setTransactionType($type_subtype) ->setNewValue($subtype); diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index f2ca9883ca..da5e4d3634 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -3084,6 +3084,11 @@ abstract class PhabricatorApplicationTransactionEditor ->withObjectPHIDs(array($object->getPHID())) ->withPHIDs($xaction_phids) ->execute(); + + // Sort the mail transactions in the input order. + $mail_xactions = mpull($mail_xactions, null, 'getPHID'); + $mail_xactions = array_select_keys($mail_xactions, $xaction_phids); + $mail_xactions = array_values($mail_xactions); } else { $mail_xactions = array(); } @@ -3262,7 +3267,7 @@ abstract class PhabricatorApplicationTransactionEditor protected function getStrongestAction( PhabricatorLiskDAO $object, array $xactions) { - return head(msort($xactions, 'newActionStrengthSortVector')); + return head(msortv($xactions, 'newActionStrengthSortVector')); } @@ -3417,12 +3422,39 @@ abstract class PhabricatorApplicationTransactionEditor ->setViewer($this->requireActor()) ->setContextObject($object); - $this->addHeadersAndCommentsToMailBody($body, $xactions); + $button_label = $this->getObjectLinkButtonLabelForMail($object); + $button_uri = $this->getObjectLinkButtonURIForMail($object); + + $this->addHeadersAndCommentsToMailBody( + $body, + $xactions, + $button_label, + $button_uri); + $this->addCustomFieldsToMailBody($body, $object, $xactions); return $body; } + protected function getObjectLinkButtonLabelForMail( + PhabricatorLiskDAO $object) { + return null; + } + + protected function getObjectLinkButtonURIForMail( + PhabricatorLiskDAO $object) { + + // Most objects define a "getURI()" method which does what we want, but + // this isn't formally part of an interface at time of writing. Try to + // call the method, expecting an exception if it does not exist. + + try { + $uri = $object->getURI(); + return PhabricatorEnv::getProductionURI($uri); + } catch (Exception $ex) { + return null; + } + } /** * @task mail @@ -3445,7 +3477,7 @@ abstract class PhabricatorApplicationTransactionEditor PhabricatorMetaMTAMailBody $body, array $xactions, $object_label = null, - $object_href = null) { + $object_uri = null) { // First, remove transactions which shouldn't be rendered in mail. foreach ($xactions as $key => $xaction) { @@ -3511,7 +3543,7 @@ abstract class PhabricatorApplicationTransactionEditor $headers_html = phutil_implode_html(phutil_tag('br'), $headers_html); $header_button = null; - if ($object_label !== null) { + if ($object_label !== null && $object_uri !== null) { $button_style = array( 'text-decoration: none;', 'padding: 4px 8px;', @@ -3530,7 +3562,7 @@ abstract class PhabricatorApplicationTransactionEditor 'a', array( 'style' => implode(' ', $button_style), - 'href' => $object_href, + 'href' => $object_uri, ), $object_label); } diff --git a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php index a9d96337ed..5a30fbfdde 100644 --- a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php +++ b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php @@ -99,14 +99,14 @@ final class PhabricatorEditEngineConfigurationEditEngine ->setLabel(pht('Name')) ->setDescription(pht('Name of the form.')) ->setTransactionType( - PhabricatorEditEngineConfigurationTransaction::TYPE_NAME) + PhabricatorEditEngineNameTransaction::TRANSACTIONTYPE) ->setValue($object->getName()), id(new PhabricatorRemarkupEditField()) ->setKey('preamble') ->setLabel(pht('Preamble')) ->setDescription(pht('Optional instructions, shown above the form.')) ->setTransactionType( - PhabricatorEditEngineConfigurationTransaction::TYPE_PREAMBLE) + PhabricatorEditEnginePreambleTransaction::TRANSACTIONTYPE) ->setValue($object->getPreamble()), ); } diff --git a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php index ccadf9b819..34b7653001 100644 --- a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php +++ b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php @@ -13,191 +13,9 @@ final class PhabricatorEditEngineConfigurationEditor public function getTransactionTypes() { $types = parent::getTransactionTypes(); - $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_NAME; - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_PREAMBLE; - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_ORDER; - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULT; - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_LOCKS; - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_SUBTYPE; - $types[] = - PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULTCREATE; - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_ISEDIT; - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_DISABLE; - - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_CREATEORDER; - $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_EDITORDER; - return $types; } - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - switch ($type) { - case PhabricatorEditEngineConfigurationTransaction::TYPE_NAME: - $missing = $this->validateIsEmptyTextField( - $object->getName(), - $xactions); - - if ($missing) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('Form name is required.'), - nonempty(last($xactions), null)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - break; - case PhabricatorEditEngineConfigurationTransaction::TYPE_SUBTYPE: - if ($xactions) { - $map = $object->getEngine() - ->setViewer($this->getActor()) - ->newSubtypeMap(); - foreach ($xactions as $xaction) { - $new = $xaction->getNewValue(); - - if ($map->isValidSubtype($new)) { - continue; - } - - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht('Subtype "%s" is not a valid subtype.', $new), - $xaction); - } - } - break; - } - - return $errors; - } - - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorEditEngineConfigurationTransaction::TYPE_NAME: - return $object->getName(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_PREAMBLE; - return $object->getPreamble(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_ORDER: - return $object->getFieldOrder(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULT: - $field_key = $xaction->getMetadataValue('field.key'); - return $object->getFieldDefault($field_key); - case PhabricatorEditEngineConfigurationTransaction::TYPE_LOCKS: - return $object->getFieldLocks(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_SUBTYPE: - return $object->getSubtype(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULTCREATE: - return (int)$object->getIsDefault(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_ISEDIT: - return (int)$object->getIsEdit(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_DISABLE: - return (int)$object->getIsDisabled(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_CREATEORDER: - return (int)$object->getCreateOrder(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_EDITORDER: - return (int)$object->getEditOrder(); - - } - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorEditEngineConfigurationTransaction::TYPE_NAME: - case PhabricatorEditEngineConfigurationTransaction::TYPE_PREAMBLE; - case PhabricatorEditEngineConfigurationTransaction::TYPE_ORDER: - case PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULT: - case PhabricatorEditEngineConfigurationTransaction::TYPE_LOCKS: - case PhabricatorEditEngineConfigurationTransaction::TYPE_SUBTYPE: - return $xaction->getNewValue(); - case PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULTCREATE: - case PhabricatorEditEngineConfigurationTransaction::TYPE_ISEDIT: - case PhabricatorEditEngineConfigurationTransaction::TYPE_DISABLE: - case PhabricatorEditEngineConfigurationTransaction::TYPE_CREATEORDER: - case PhabricatorEditEngineConfigurationTransaction::TYPE_EDITORDER: - return (int)$xaction->getNewValue(); - } - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorEditEngineConfigurationTransaction::TYPE_NAME: - $object->setName($xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_PREAMBLE; - $object->setPreamble($xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_ORDER: - $object->setFieldOrder($xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULT: - $field_key = $xaction->getMetadataValue('field.key'); - $object->setFieldDefault($field_key, $xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_LOCKS: - $object->setFieldLocks($xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_SUBTYPE: - $object->setSubtype($xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULTCREATE: - $object->setIsDefault($xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_ISEDIT: - $object->setIsEdit($xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_DISABLE: - $object->setIsDisabled($xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_CREATEORDER: - $object->setCreateOrder($xaction->getNewValue()); - return; - case PhabricatorEditEngineConfigurationTransaction::TYPE_EDITORDER: - $object->setEditOrder($xaction->getNewValue()); - return; - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorEditEngineConfigurationTransaction::TYPE_NAME: - case PhabricatorEditEngineConfigurationTransaction::TYPE_PREAMBLE; - case PhabricatorEditEngineConfigurationTransaction::TYPE_ORDER; - case PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULT: - case PhabricatorEditEngineConfigurationTransaction::TYPE_ISEDIT: - case PhabricatorEditEngineConfigurationTransaction::TYPE_LOCKS: - case PhabricatorEditEngineConfigurationTransaction::TYPE_SUBTYPE: - case PhabricatorEditEngineConfigurationTransaction::TYPE_DEFAULTCREATE: - case PhabricatorEditEngineConfigurationTransaction::TYPE_DISABLE: - case PhabricatorEditEngineConfigurationTransaction::TYPE_CREATEORDER: - case PhabricatorEditEngineConfigurationTransaction::TYPE_EDITORDER: - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 9850d66a7f..775d59d4db 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -127,7 +127,19 @@ abstract class PhabricatorApplicationTransaction } public function hasComment() { - return $this->getComment() && strlen($this->getComment()->getContent()); + if (!$this->getComment()) { + return false; + } + + $content = $this->getComment()->getContent(); + + // If the content is empty or consists of only whitespace, don't count + // this as comment. + if (!strlen(trim($content))) { + return false; + } + + return true; } public function getComment() { diff --git a/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php b/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php index 71048e96c5..a9dca32e3d 100644 --- a/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php @@ -1,19 +1,7 @@ getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $type = $this->getTransactionType(); - switch ($type) { - case PhabricatorTransactions::TYPE_CREATE: - return pht( - '%s created this form configuration.', - $this->renderHandleLink($author_phid)); - case self::TYPE_NAME: - if (strlen($old)) { - return pht( - '%s renamed this form from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $old, - $new); - } else { - return pht( - '%s named this form "%s".', - $this->renderHandleLink($author_phid), - $new); - } - case self::TYPE_PREAMBLE: - return pht( - '%s updated the preamble for this form.', - $this->renderHandleLink($author_phid)); - case self::TYPE_ORDER: - return pht( - '%s reordered the fields in this form.', - $this->renderHandleLink($author_phid)); - case self::TYPE_DEFAULT: - $key = $this->getMetadataValue('field.key'); - return pht( - '%s changed the default value for field "%s".', - $this->renderHandleLink($author_phid), - $key); - case self::TYPE_LOCKS: - return pht( - '%s changed locked and hidden fields.', - $this->renderHandleLink($author_phid)); - case self::TYPE_DEFAULTCREATE: - if ($new) { - return pht( - '%s added this form to the "Create" menu.', - $this->renderHandleLink($author_phid)); - } else { - return pht( - '%s removed this form from the "Create" menu.', - $this->renderHandleLink($author_phid)); - } - case self::TYPE_ISEDIT: - if ($new) { - return pht( - '%s marked this form as an edit form.', - $this->renderHandleLink($author_phid)); - } else { - return pht( - '%s unmarked this form as an edit form.', - $this->renderHandleLink($author_phid)); - } - case self::TYPE_DISABLE: - if ($new) { - return pht( - '%s disabled this form.', - $this->renderHandleLink($author_phid)); - } else { - return pht( - '%s enabled this form.', - $this->renderHandleLink($author_phid)); - } - case self::TYPE_SUBTYPE: - return pht( - '%s changed the subtype of this form from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $old, - $new); - } - - return parent::getTitle(); - } - - public function getColor() { - $author_phid = $this->getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $type = $this->getTransactionType(); - switch ($type) { - case PhabricatorTransactions::TYPE_CREATE: - return 'green'; - case self::TYPE_DISABLE: - if ($new) { - return 'indigo'; - } else { - return 'green'; - } - } - - return parent::getColor(); - } - - public function getIcon() { - $author_phid = $this->getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $type = $this->getTransactionType(); - switch ($type) { - case PhabricatorTransactions::TYPE_CREATE: - return 'fa-plus'; - case self::TYPE_DISABLE: - if ($new) { - return 'fa-ban'; - } else { - return 'fa-check'; - } - } - - return parent::getIcon(); - } - - protected function newRemarkupChanges() { - $changes = array(); - - $type = $this->getTransactionType(); - switch ($type) { - case self::TYPE_PREAMBLE: - $changes[] = $this->newRemarkupChange() - ->setOldValue($this->getOldValue()) - ->setNewValue($this->getNewValue()); - break; - } - - return $changes; + public function getBaseTransactionClass() { + return 'PhabricatorEditEngineTransactionType'; } } diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionDetailView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionDetailView.php new file mode 100644 index 0000000000..1220e5bc7f --- /dev/null +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionDetailView.php @@ -0,0 +1,172 @@ +newText = $new_text; + return $this; + } + + public function setOldText($old_text) { + $this->oldText = $old_text; + return $this; + } + + public function renderForMail() { + $diff = $this->buildDiff(); + + $viewer = $this->getViewer(); + $old_bright = $viewer->getCSSValue('old-bright'); + $new_bright = $viewer->getCSSValue('new-bright'); + + $old_styles = array( + 'padding: 0 2px;', + 'color: #333333;', + "background: {$old_bright};", + ); + $old_styles = implode(' ', $old_styles); + + $new_styles = array( + 'padding: 0 2px;', + 'color: #333333;', + "background: {$new_bright};", + ); + $new_styles = implode(' ', $new_styles); + + $omit_styles = array( + 'padding: 8px 0;', + ); + $omit_styles = implode(' ', $omit_styles); + + $result = array(); + foreach ($diff->getSummaryParts() as $part) { + $type = $part['type']; + $text = $part['text']; + switch ($type) { + case '.': + $result[] = phutil_tag( + 'div', + array( + 'style' => $omit_styles, + ), + pht('...')); + break; + case '-': + $result[] = phutil_tag( + 'span', + array( + 'style' => $old_styles, + ), + $text); + break; + case '+': + $result[] = phutil_tag( + 'span', + array( + 'style' => $new_styles, + ), + $text); + break; + case '=': + $result[] = $text; + break; + } + } + + $styles = array( + 'white-space: pre-wrap;', + 'color: #74777D;', + ); + + // Beyond applying "pre-wrap", convert newlines to "
" explicitly + // to improve behavior in clients like Airmail. + $result = phutil_escape_html_newlines($result); + + return phutil_tag( + 'div', + array( + 'style' => implode(' ', $styles), + ), + $result); + } + + public function render() { + $diff = $this->buildDiff(); + + require_celerity_resource('differential-changeset-view-css'); + + $result = array(); + foreach ($diff->getParts() as $part) { + $type = $part['type']; + $text = $part['text']; + switch ($type) { + case '-': + $result[] = phutil_tag( + 'span', + array( + 'class' => 'old', + ), + $text); + break; + case '+': + $result[] = phutil_tag( + 'span', + array( + 'class' => 'new', + ), + $text); + break; + case '=': + $result[] = $text; + break; + } + } + + $diff_view = phutil_tag( + 'div', + array( + 'class' => 'prose-diff', + ), + $result); + + $old_view = phutil_tag( + 'div', + array( + 'class' => 'prose-diff', + ), + $this->oldText); + + $new_view = phutil_tag( + 'div', + array( + 'class' => 'prose-diff', + ), + $this->newText); + + return id(new PHUITabGroupView()) + ->addTab( + id(new PHUITabView()) + ->setKey('old') + ->setName(pht('Old')) + ->appendChild($old_view)) + ->addTab( + id(new PHUITabView()) + ->setKey('new') + ->setName(pht('New')) + ->appendChild($new_view)) + ->addTab( + id(new PHUITabView()) + ->setKey('diff') + ->setName(pht('Diff')) + ->appendChild($diff_view)) + ->selectTab('diff'); + } + + private function buildDiff() { + $engine = new PhutilProseDifferenceEngine(); + return $engine->getDiff($this->oldText, $this->newText); + } +} diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionJSONDiffDetailView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionJSONDiffDetailView.php new file mode 100644 index 0000000000..91111c1e0b --- /dev/null +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionJSONDiffDetailView.php @@ -0,0 +1,17 @@ +setNewText($json->encodeFormatted($new_object)); + return $this; + } + + public function setOld($old_object) { + $json = new PhutilJSON(); + $this->setOldText($json->encodeFormatted($old_object)); + return $this; + } +} diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php index 755d6a9fcf..d778d8e95d 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php @@ -1,174 +1,4 @@ newText = $new_text; - return $this; - } - - public function setOldText($old_text) { - $this->oldText = $old_text; - return $this; - } - - public function renderForMail() { - $diff = $this->buildDiff(); - - $viewer = $this->getViewer(); - $old_bright = $viewer->getCSSValue('old-bright'); - $new_bright = $viewer->getCSSValue('new-bright'); - - $old_styles = array( - 'padding: 0 2px;', - 'color: #333333;', - "background: {$old_bright};", - ); - $old_styles = implode(' ', $old_styles); - - $new_styles = array( - 'padding: 0 2px;', - 'color: #333333;', - "background: {$new_bright};", - ); - $new_styles = implode(' ', $new_styles); - - $omit_styles = array( - 'padding: 8px 0;', - ); - $omit_styles = implode(' ', $omit_styles); - - $result = array(); - foreach ($diff->getSummaryParts() as $part) { - $type = $part['type']; - $text = $part['text']; - switch ($type) { - case '.': - $result[] = phutil_tag( - 'div', - array( - 'style' => $omit_styles, - ), - pht('...')); - break; - case '-': - $result[] = phutil_tag( - 'span', - array( - 'style' => $old_styles, - ), - $text); - break; - case '+': - $result[] = phutil_tag( - 'span', - array( - 'style' => $new_styles, - ), - $text); - break; - case '=': - $result[] = $text; - break; - } - } - - $styles = array( - 'white-space: pre-wrap;', - 'color: #74777D;', - ); - - // Beyond applying "pre-wrap", convert newlines to "
" explicitly - // to improve behavior in clients like Airmail. - $result = phutil_escape_html_newlines($result); - - return phutil_tag( - 'div', - array( - 'style' => implode(' ', $styles), - ), - $result); - } - - public function render() { - $diff = $this->buildDiff(); - - require_celerity_resource('differential-changeset-view-css'); - - $result = array(); - foreach ($diff->getParts() as $part) { - $type = $part['type']; - $text = $part['text']; - switch ($type) { - case '-': - $result[] = phutil_tag( - 'span', - array( - 'class' => 'old', - ), - $text); - break; - case '+': - $result[] = phutil_tag( - 'span', - array( - 'class' => 'new', - ), - $text); - break; - case '=': - $result[] = $text; - break; - } - } - - $diff_view = phutil_tag( - 'div', - array( - 'class' => 'prose-diff', - ), - $result); - - $old_view = phutil_tag( - 'div', - array( - 'class' => 'prose-diff', - ), - $this->oldText); - - $new_view = phutil_tag( - 'div', - array( - 'class' => 'prose-diff', - ), - $this->newText); - - return id(new PHUITabGroupView()) - ->addTab( - id(new PHUITabView()) - ->setKey('old') - ->setName(pht('Old')) - ->appendChild($old_view)) - ->addTab( - id(new PHUITabView()) - ->setKey('new') - ->setName(pht('New')) - ->appendChild($new_view)) - ->addTab( - id(new PHUITabView()) - ->setKey('diff') - ->setName(pht('Diff')) - ->appendChild($diff_view)) - ->selectTab('diff'); - } - - private function buildDiff() { - $engine = new PhutilProseDifferenceEngine(); - return $engine->getDiff($this->oldText, $this->newText); - } - -} + extends PhabricatorApplicationTransactionDetailView {} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineCreateOrderTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineCreateOrderTransaction.php new file mode 100644 index 0000000000..9a9483c227 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineCreateOrderTransaction.php @@ -0,0 +1,26 @@ +getCreateOrder(); + } + + public function generateNewValue($object, $value) { + return (int)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setCreateOrder($value); + } + + public function getTitle() { + return pht( + '%s changed the order in which this form appears in the "Create" menu.', + $this->renderAuthor()); + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineDefaultCreateTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineDefaultCreateTransaction.php new file mode 100644 index 0000000000..2c4a544013 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineDefaultCreateTransaction.php @@ -0,0 +1,35 @@ +getIsDefault(); + } + + public function generateNewValue($object, $value) { + return (int)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setIsDefault($value); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + if ($new) { + return pht( + '%s added this form to the "Create" menu.', + $this->renderAuthor()); + } else { + return pht( + '%s removed this form from the "Create" menu.', + $this->renderAuthor()); + } + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineDefaultTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineDefaultTransaction.php new file mode 100644 index 0000000000..7b980cdb1a --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineDefaultTransaction.php @@ -0,0 +1,71 @@ +getMetadataValue('field.key'); + return $object->getFieldDefault($field_key); + } + + public function applyInternalEffects($object, $value) { + $field_key = $this->getMetadataValue('field.key'); + $object->setFieldDefault($field_key, $value); + } + + public function getTitle() { + $key = $this->getMetadataValue('field.key'); + $object = $this->getObject(); + $engine = $object->getEngine(); + $fields = $engine->getFieldsForConfig($object); + $field = idx($fields, $key); + + if (!$field) { + return pht( + '%s changed the default values for field %s.', + $this->renderAuthor(), + $this->renderValue($key)); + } + + return pht( + '%s changed the default value for field %s.', + $this->renderAuthor(), + $this->renderValue($field->getLabel())); + } + + public function hasChangeDetailView() { + return true; + } + + public function newChangeDetailView() { + $viewer = $this->getViewer(); + $old = $this->renderDefaultValueAsFallbackText($this->getOldValue()); + $new = $this->renderDefaultValueAsFallbackText($this->getNewValue()); + + return id(new PhabricatorApplicationTransactionTextDiffDetailView()) + ->setViewer($viewer) + ->setOldText($old) + ->setNewText($new); + } + + private function renderDefaultValueAsFallbackText($default_value) { + // See T13319. When rendering an "alice changed the default value for + // field X." story on custom forms, we may fall back to a generic + // rendering. Try to present the value change in a comprehensible way + // even if it isn't especially human readable (for example, it may + // contain PHIDs or other internal identifiers). + + if (is_scalar($default_value) || is_null($default_value)) { + return $default_value; + } + + if (phutil_is_natural_list($default_value)) { + return id(new PhutilJSON())->encodeAsList($default_value); + } + + return id(new PhutilJSON())->encodeAsObject($default_value); + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineDisableTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineDisableTransaction.php new file mode 100644 index 0000000000..ef64c20974 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineDisableTransaction.php @@ -0,0 +1,51 @@ +getIsDisabled(); + } + + public function generateNewValue($object, $value) { + return (int)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setIsDisabled($value); + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s disabled this form.', + $this->renderAuthor()); + } else { + return pht( + '%s enabled this form.', + $this->renderAuthor()); + } + } + + public function getColor() { + $new = $this->getNewValue(); + if ($new) { + return 'indigo'; + } else { + return 'green'; + } + } + + public function getIcon() { + $new = $this->getNewValue(); + if ($new) { + return 'fa-ban'; + } else { + return 'fa-check'; + } + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineEditOrderTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineEditOrderTransaction.php new file mode 100644 index 0000000000..a9109e6475 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineEditOrderTransaction.php @@ -0,0 +1,26 @@ +getEditOrder(); + } + + public function generateNewValue($object, $value) { + return (int)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setEditOrder($value); + } + + public function getTitle() { + return pht( + '%s changed the order in which this form appears in the "Edit" menu.', + $this->renderAuthor()); + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineIsEditTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineIsEditTransaction.php new file mode 100644 index 0000000000..a4a3a38543 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineIsEditTransaction.php @@ -0,0 +1,34 @@ +getIsEdit(); + } + + public function generateNewValue($object, $value) { + return (int)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setIsEdit($value); + } + + public function getTitle() { + $new = $this->getNewValue(); + + if ($new) { + return pht( + '%s marked this form as an edit form.', + $this->renderAuthor()); + } else { + return pht( + '%s unmarked this form as an edit form.', + $this->renderAuthor()); + } + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineLocksTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineLocksTransaction.php new file mode 100644 index 0000000000..b919a4de08 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineLocksTransaction.php @@ -0,0 +1,35 @@ +getFieldLocks(); + } + + public function applyInternalEffects($object, $value) { + $object->setFieldLocks($value); + } + + public function getTitle() { + return pht( + '%s changed locked and hidden fields.', + $this->renderAuthor()); + } + + public function hasChangeDetailView() { + return true; + } + + public function newChangeDetailView() { + $viewer = $this->getViewer(); + + return id(new PhabricatorApplicationTransactionJSONDiffDetailView()) + ->setViewer($viewer) + ->setOld($this->getOldValue()) + ->setNew($this->getNewValue()); + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineNameTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineNameTransaction.php new file mode 100644 index 0000000000..0ab4b7029f --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineNameTransaction.php @@ -0,0 +1,54 @@ +getName(); + } + + public function applyInternalEffects($object, $value) { + $object->setName($value); + } + + public function getTitle() { + if (strlen($this->getOldValue())) { + return pht( + '%s renamed this form from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } else { + return pht( + '%s named this form %s.', + $this->renderAuthor(), + $this->renderNewValue()); + } + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + if (!strlen($new)) { + $errors[] = $this->newRequiredError( + pht('Form name is required.'), + $xaction); + continue; + } + } + + if (!$errors) { + if ($this->isEmptyTextTransaction($object->getName(), $xactions)) { + $errors[] = $this->newRequiredError( + pht('Forms must have a name.')); + } + } + + return $errors; + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineOrderTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineOrderTransaction.php new file mode 100644 index 0000000000..96376113c4 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineOrderTransaction.php @@ -0,0 +1,35 @@ +getFieldOrder(); + } + + public function applyInternalEffects($object, $value) { + $object->setFieldOrder($value); + } + + public function getTitle() { + return pht( + '%s reordered the fields in this form.', + $this->renderAuthor()); + } + + public function hasChangeDetailView() { + return true; + } + + public function newChangeDetailView() { + $viewer = $this->getViewer(); + + return id(new PhabricatorApplicationTransactionJSONDiffDetailView()) + ->setViewer($viewer) + ->setOld($this->getOldValue()) + ->setNew($this->getNewValue()); + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEnginePreambleTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEnginePreambleTransaction.php new file mode 100644 index 0000000000..e450bc592d --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEnginePreambleTransaction.php @@ -0,0 +1,45 @@ +getPreamble(); + } + + public function applyInternalEffects($object, $value) { + $object->setPreamble($value); + } + + public function getTitle() { + return pht( + '%s updated the preamble for this form.', + $this->renderAuthor()); + } + + public function hasChangeDetailView() { + return true; + } + + public function newChangeDetailView() { + $viewer = $this->getViewer(); + + return id(new PhabricatorApplicationTransactionTextDiffDetailView()) + ->setViewer($viewer) + ->setOldText($this->getOldValue()) + ->setNewText($this->getNewValue()); + } + + public function newRemarkupChanges() { + $changes = array(); + + $changes[] = $this->newRemarkupChange() + ->setOldValue($this->getOldValue()) + ->setNewValue($this->getNewValue()); + + return $changes; + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineSubtypeTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineSubtypeTransaction.php new file mode 100644 index 0000000000..53ec221631 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineSubtypeTransaction.php @@ -0,0 +1,48 @@ +getSubtype(); + } + + public function applyInternalEffects($object, $value) { + $object->setSubtype($value); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + return pht( + '%s changed the subtype of this form from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function validateTransactions($object, array $xactions) { + $map = $object->getEngine() + ->setViewer($this->getActor()) + ->newSubtypeMap(); + + $errors = array(); + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + if ($map->isValidSubtype($new)) { + continue; + } + + $errors[] = $this->newInvalidError( + pht('Subtype "%s" is not a valid subtype.', $new), + $xaction); + } + + return $errors; + } + +} diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineTransactionType.php b/src/applications/transactions/xaction/PhabricatorEditEngineTransactionType.php new file mode 100644 index 0000000000..c49d1e4654 --- /dev/null +++ b/src/applications/transactions/xaction/PhabricatorEditEngineTransactionType.php @@ -0,0 +1,4 @@ + backup.sql.gz + phabricator/ $ ./bin/storage dump --compress --output backup.sql.gz Then store the backup somewhere safe, like in a box buried under an old tree stump. No one will ever think to look for it there. diff --git a/src/infrastructure/graph/PhabricatorObjectGraph.php b/src/infrastructure/graph/PhabricatorObjectGraph.php index 48e0fcbe73..67130e8c05 100644 --- a/src/infrastructure/graph/PhabricatorObjectGraph.php +++ b/src/infrastructure/graph/PhabricatorObjectGraph.php @@ -189,7 +189,7 @@ abstract class PhabricatorObjectGraph $order = id(new PhutilDirectedScalarGraph()) ->addNodes($ancestry) - ->getTopographicallySortedNodes(); + ->getNodesInTopologicalOrder(); $ancestry = array_select_keys($ancestry, $order); diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php index d5f1da9ecf..c3b9a32327 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php @@ -51,24 +51,59 @@ final class PhabricatorStorageManagementDumpWorkflow } public function didExecute(PhutilArgumentParser $args) { + $output_file = $args->getArg('output'); + $is_compress = $args->getArg('compress'); + $is_overwrite = $args->getArg('overwrite'); + + if ($is_compress) { + if ($output_file === null) { + throw new PhutilArgumentUsageException( + pht( + 'The "--compress" flag can only be used alongside "--output".')); + } + + if (!function_exists('gzopen')) { + throw new PhutilArgumentUsageException( + pht( + 'The "--compress" flag requires the PHP "zlib" extension, but '. + 'that extension is not available. Install the extension or '. + 'omit the "--compress" option.')); + } + } + + if ($is_overwrite) { + if ($output_file === null) { + throw new PhutilArgumentUsageException( + pht( + 'The "--overwrite" flag can only be used alongside "--output".')); + } + } + + if ($output_file !== null) { + if (Filesystem::pathExists($output_file)) { + if (!$is_overwrite) { + throw new PhutilArgumentUsageException( + pht( + 'Output file "%s" already exists. Use "--overwrite" '. + 'to overwrite.', + $output_file)); + } + } + } + $api = $this->getSingleAPI(); $patches = $this->getPatches(); - $console = PhutilConsole::getConsole(); - $with_indexes = !$args->getArg('no-indexes'); $applied = $api->getAppliedPatches(); if ($applied === null) { - $namespace = $api->getNamespace(); - $console->writeErr( + throw new PhutilArgumentUsageException( pht( - '**Storage Not Initialized**: There is no database storage '. - 'initialized in this storage namespace ("%s"). Use '. - '**%s** to initialize storage.', - $namespace, - './bin/storage upgrade')); - return 1; + 'There is no database storage initialized in the current storage '. + 'namespace ("%s"). Use "bin/storage upgrade" to initialize '. + 'storage or use "--namespace" to choose a different namespace.', + $api->getNamespace())); } $ref = $api->getRef(); @@ -141,38 +176,6 @@ final class PhabricatorStorageManagementDumpWorkflow } } - $output_file = $args->getArg('output'); - $is_compress = $args->getArg('compress'); - $is_overwrite = $args->getArg('overwrite'); - - if ($is_compress) { - if ($output_file === null) { - throw new PhutilArgumentUsageException( - pht( - 'The "--compress" flag can only be used alongside "--output".')); - } - } - - if ($is_overwrite) { - if ($output_file === null) { - throw new PhutilArgumentUsageException( - pht( - 'The "--overwrite" flag can only be used alongside "--output".')); - } - } - - if ($output_file !== null) { - if (Filesystem::pathExists($output_file)) { - if (!$is_overwrite) { - throw new PhutilArgumentUsageException( - pht( - 'Output file "%s" already exists. Use "--overwrite" '. - 'to overwrite.', - $output_file)); - } - } - } - $argv = array(); $argv[] = '--hex-blob'; $argv[] = '--single-transaction'; diff --git a/src/infrastructure/time/PhabricatorTime.php b/src/infrastructure/time/PhabricatorTime.php index b221285d13..67378ca8df 100644 --- a/src/infrastructure/time/PhabricatorTime.php +++ b/src/infrastructure/time/PhabricatorTime.php @@ -78,4 +78,14 @@ final class PhabricatorTime extends Phobject { return $datetime; } + public static function getTimezoneDisplayName($raw_identifier) { + + // Internal identifiers have names like "America/Los_Angeles", but this is + // just an implementation detail and we can render them in a more human + // readable format with spaces. + $name = str_replace('_', ' ', $raw_identifier); + + return $name; + } + } diff --git a/webroot/rsrc/css/core/remarkup.css b/webroot/rsrc/css/core/remarkup.css index aa5bb8e8c1..3354be9663 100644 --- a/webroot/rsrc/css/core/remarkup.css +++ b/webroot/rsrc/css/core/remarkup.css @@ -174,40 +174,29 @@ list-style-type: none; } -.phabricator-remarkup h1.remarkup-header { - font-size: 24px; - line-height: 1.625em; - margin: 24px 0 4px; -} - .phabricator-remarkup h2.remarkup-header { - font-size: 20px; - line-height: 1.5em; - margin: 20px 0 4px; + font-size: 28px; + margin: 1em 0 0.75em; } .phabricator-remarkup h3.remarkup-header { - font-size: 18px; - line-height: 1.375em; - margin: 20px 0 4px; + font-size: 24px; + margin: 1em 0 0.75em; } .phabricator-remarkup h4.remarkup-header { - font-size: 16px; - line-height: 1.25em; - margin: 12px 0 4px; + font-size: 22px; + margin: 1em 0 0.75em; } .phabricator-remarkup h5.remarkup-header { - font-size: 15px; - line-height: 1.125em; - margin: 8px 0 4px; + font-size: 18px; + margin: 1em 0 0.75em; } .phabricator-remarkup h6.remarkup-header { - font-size: 14px; - line-height: 1em; - margin: 4px 0; + font-size: 16px; + margin: 1em 0 0.75em; } .phabricator-remarkup blockquote { diff --git a/webroot/rsrc/css/phui/phui-fontkit.css b/webroot/rsrc/css/phui/phui-fontkit.css index 43162ea618..1bc56826d7 100644 --- a/webroot/rsrc/css/phui/phui-fontkit.css +++ b/webroot/rsrc/css/phui/phui-fontkit.css @@ -6,15 +6,6 @@ color: {$blacktext}; } -.phui-document-view .phabricator-remarkup .remarkup-header { - margin-bottom: 8px; -} - -.phui-document-view .phabricator-remarkup h2.remarkup-header { - padding: 0 24px 8px 0; - margin: 32px 0 4px; -} - .phui-document-view .phabricator-remarkup .remarkup-header strong { color: #586BE9; } diff --git a/webroot/rsrc/externals/javelin/lib/Workflow.js b/webroot/rsrc/externals/javelin/lib/Workflow.js index d767a58780..d398d33774 100644 --- a/webroot/rsrc/externals/javelin/lib/Workflow.js +++ b/webroot/rsrc/externals/javelin/lib/Workflow.js @@ -75,6 +75,7 @@ JX.install('Workflow', { var workflow = new JX.Workflow(link.href); return workflow; }, + _push : function(workflow) { JX.Mask.show(); JX.Workflow._stack.push(workflow); @@ -85,8 +86,36 @@ JX.install('Workflow', { dialog._destroy(); JX.Mask.hide(); }, - disable : function() { - JX.Workflow._disabled = true; + _onlink: function(event) { + // See T13302. When a user clicks a link in a dialog and that link + // triggers a navigation event, we want to close the dialog as though + // they had pressed a button. + + // When Quicksand is enabled, this is particularly relevant because + // the dialog will stay in the foreground while the page content changes + // in the background if we do not dismiss the dialog. + + // If this is a Command-Click, the link will open in a new window. + var is_command = !!event.getRawEvent().metaKey; + if (is_command) { + return; + } + + var link = event.getNode('tag:a'); + + // If the link is an anchor, or does not go anywhere, ignore the event. + var href = '' + link.getAttribute('href'); + if (!href.length || href[0] === '#') { + return; + } + + // This link will open in a new window. + if (link.target === '_blank') { + return; + } + + // Close the dialog. + JX.Workflow._pop(); }, _onbutton : function(event) { @@ -94,10 +123,6 @@ JX.install('Workflow', { return; } - if (JX.Workflow._disabled) { - return; - } - // Get the button (which is sometimes actually another tag, like an ) // which triggered the event. In particular, this makes sure we get the // right node if there is a