diff --git a/resources/celerity/map.php b/resources/celerity/map.php index e5eb50bb58..3b86ebeeb4 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'e68cf1fa', 'conpherence.pkg.js' => '15191c65', - 'core.pkg.css' => 'cff4ff6f', + 'core.pkg.css' => '9d1148a4', 'core.pkg.js' => '4bde473b', 'differential.pkg.css' => '06dc617c', 'differential.pkg.js' => 'ef0b989b', @@ -127,7 +127,7 @@ return array( 'rsrc/css/phui/calendar/phui-calendar-list.css' => '576be600', 'rsrc/css/phui/calendar/phui-calendar-month.css' => '21154caf', 'rsrc/css/phui/calendar/phui-calendar.css' => 'f1ddf11c', - 'rsrc/css/phui/object-item/phui-oi-big-ui.css' => '628f59de', + 'rsrc/css/phui/object-item/phui-oi-big-ui.css' => '7a7c22af', 'rsrc/css/phui/object-item/phui-oi-color.css' => 'cd2b9b77', 'rsrc/css/phui/object-item/phui-oi-drag-ui.css' => '08f4ccc3', 'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '9d9685d6', @@ -214,7 +214,7 @@ return array( 'rsrc/externals/javelin/core/__tests__/stratcom.js' => '88bf7313', 'rsrc/externals/javelin/core/__tests__/util.js' => 'e251703d', 'rsrc/externals/javelin/core/init.js' => '8d83d2a1', - 'rsrc/externals/javelin/core/init_node.js' => 'c234aded', + 'rsrc/externals/javelin/core/init_node.js' => 'f7732951', 'rsrc/externals/javelin/core/install.js' => '05270951', 'rsrc/externals/javelin/core/util.js' => '93cc50d6', 'rsrc/externals/javelin/docs/Base.js' => '74676256', @@ -469,6 +469,7 @@ return array( 'rsrc/js/core/behavior-keyboard-shortcuts.js' => '01fca1f0', 'rsrc/js/core/behavior-lightbox-attachments.js' => '6b31879a', 'rsrc/js/core/behavior-line-linker.js' => '66a62306', + 'rsrc/js/core/behavior-linked-container.js' => '291da458', 'rsrc/js/core/behavior-more.js' => 'a80d0378', 'rsrc/js/core/behavior-object-selector.js' => '77c1f0b0', 'rsrc/js/core/behavior-oncopy.js' => '2926fff2', @@ -616,6 +617,7 @@ return array( 'javelin-behavior-launch-icon-composer' => '48086888', 'javelin-behavior-lightbox-attachments' => '6b31879a', 'javelin-behavior-line-chart' => 'e4232876', + 'javelin-behavior-linked-container' => '291da458', 'javelin-behavior-maniphest-batch-selector' => 'ad54037e', 'javelin-behavior-maniphest-list-editor' => 'a9f88de2', 'javelin-behavior-maniphest-subpriority-editor' => '71237763', @@ -832,7 +834,7 @@ return array( 'phui-lightbox-css' => '0a035e40', 'phui-list-view-css' => '38f8c9bd', 'phui-object-box-css' => '9cff003c', - 'phui-oi-big-ui-css' => '628f59de', + 'phui-oi-big-ui-css' => '7a7c22af', 'phui-oi-color-css' => 'cd2b9b77', 'phui-oi-drag-ui-css' => '08f4ccc3', 'phui-oi-flush-ui-css' => '9d9685d6', @@ -1027,6 +1029,10 @@ return array( 'javelin-uri', 'phabricator-notification', ), + '291da458' => array( + 'javelin-behavior', + 'javelin-dom', + ), '2926fff2' => array( 'javelin-behavior', 'javelin-dom', @@ -1348,9 +1354,6 @@ return array( 'javelin-magical-init', 'javelin-util', ), - '628f59de' => array( - 'phui-oi-list-view-css', - ), '62dfea03' => array( 'javelin-install', 'javelin-util', @@ -1508,6 +1511,9 @@ return array( 'owners-path-editor', 'javelin-behavior', ), + '7a7c22af' => array( + 'phui-oi-list-view-css', + ), '7cbe244b' => array( 'javelin-install', 'javelin-util', diff --git a/resources/sql/autopatches/20181213.auth.01.sessionphid.sql b/resources/sql/autopatches/20181213.auth.01.sessionphid.sql new file mode 100644 index 0000000000..34b5aa5bf6 --- /dev/null +++ b/resources/sql/autopatches/20181213.auth.01.sessionphid.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.phabricator_session + ADD phid VARBINARY(64) NOT NULL; diff --git a/resources/sql/autopatches/20181213.auth.02.populatephid.php b/resources/sql/autopatches/20181213.auth.02.populatephid.php new file mode 100644 index 0000000000..314eaf87a3 --- /dev/null +++ b/resources/sql/autopatches/20181213.auth.02.populatephid.php @@ -0,0 +1,18 @@ +establishConnection('w'); + +foreach ($iterator as $session) { + if (strlen($session->getPHID())) { + continue; + } + + queryfx( + $conn, + 'UPDATE %R SET phid = %s WHERE id = %d', + $table, + $session->generatePHID(), + $session->getID()); +} diff --git a/resources/sql/autopatches/20181213.auth.03.phidkey.sql b/resources/sql/autopatches/20181213.auth.03.phidkey.sql new file mode 100644 index 0000000000..6bc11b3e55 --- /dev/null +++ b/resources/sql/autopatches/20181213.auth.03.phidkey.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.phabricator_session + ADD UNIQUE KEY `key_phid` (phid); diff --git a/resources/sql/autopatches/20181213.auth.04.longerhashes.sql b/resources/sql/autopatches/20181213.auth.04.longerhashes.sql new file mode 100644 index 0000000000..2bffb4c4a8 --- /dev/null +++ b/resources/sql/autopatches/20181213.auth.04.longerhashes.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.phabricator_session + CHANGE sessionKey sessionKey VARBINARY(64) NOT NULL; diff --git a/resources/sql/autopatches/20181213.auth.05.longerloghashes.sql b/resources/sql/autopatches/20181213.auth.05.longerloghashes.sql new file mode 100644 index 0000000000..dc8638d91c --- /dev/null +++ b/resources/sql/autopatches/20181213.auth.05.longerloghashes.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.user_log + CHANGE session session VARBINARY(64); diff --git a/resources/sql/patches/20130219.commitsummarymig.php b/resources/sql/patches/20130219.commitsummarymig.php index 60bdd1542c..f47016804d 100644 --- a/resources/sql/patches/20130219.commitsummarymig.php +++ b/resources/sql/patches/20130219.commitsummarymig.php @@ -12,9 +12,9 @@ foreach ($commits as $commit) { continue; } - $data = $commit->loadOneRelative( - new PhabricatorRepositoryCommitData(), - 'commitID'); + $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere( + 'commitID = %d', + $commit->getID()); if (!$data) { continue; diff --git a/resources/sql/patches/20130409.commitdrev.php b/resources/sql/patches/20130409.commitdrev.php index fb556f1846..a264e8edeb 100644 --- a/resources/sql/patches/20130409.commitdrev.php +++ b/resources/sql/patches/20130409.commitdrev.php @@ -8,7 +8,9 @@ $commit_table->establishConnection('w'); $edges = 0; foreach (new LiskMigrationIterator($commit_table) as $commit) { - $data = $commit->loadOneRelative($data_table, 'commitID'); + $data = $data_table->loadOneWhere( + 'commitID = %d', + $commit->getID()); if (!$data) { continue; } diff --git a/resources/sql/patches/20130530.sessionhash.php b/resources/sql/patches/20130530.sessionhash.php index 771dac61e3..1e09ee32fd 100644 --- a/resources/sql/patches/20130530.sessionhash.php +++ b/resources/sql/patches/20130530.sessionhash.php @@ -1,22 +1,7 @@ openTransaction(); -$conn = $table->establishConnection('w'); - -$sessions = queryfx_all( - $conn, - 'SELECT userPHID, type, sessionKey FROM %T FOR UPDATE', - PhabricatorUser::SESSION_TABLE); - -foreach ($sessions as $session) { - queryfx( - $conn, - 'UPDATE %T SET sessionKey = %s WHERE userPHID = %s AND type = %s', - PhabricatorUser::SESSION_TABLE, - PhabricatorHash::weakDigest($session['sessionKey']), - $session['userPHID'], - $session['type']); -} - -$table->saveTransaction(); +// See T13225. Long ago, this upgraded session key storage from unhashed to +// HMAC-SHA1 here. We later upgraded storage to HMAC-SHA256, so this is initial +// upgrade is now fairly pointless. Dropping this migration entirely only logs +// users out of installs that waited more than 5 years to upgrade, which seems +// like a reasonable behavior. diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cd260f497d..b0408e127d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1633,7 +1633,6 @@ phutil_register_library_map(array( 'LegalpadTransactionView' => 'applications/legalpad/view/LegalpadTransactionView.php', 'LiskChunkTestCase' => 'infrastructure/storage/lisk/__tests__/LiskChunkTestCase.php', 'LiskDAO' => 'infrastructure/storage/lisk/LiskDAO.php', - 'LiskDAOSet' => 'infrastructure/storage/lisk/LiskDAOSet.php', 'LiskDAOTestCase' => 'infrastructure/storage/lisk/__tests__/LiskDAOTestCase.php', 'LiskEphemeralObjectException' => 'infrastructure/storage/lisk/LiskEphemeralObjectException.php', 'LiskFixtureTestCase' => 'infrastructure/storage/lisk/__tests__/LiskFixtureTestCase.php', @@ -1760,6 +1759,7 @@ phutil_register_library_map(array( 'ManiphestTaskStatusTestCase' => 'applications/maniphest/constants/__tests__/ManiphestTaskStatusTestCase.php', 'ManiphestTaskStatusTransaction' => 'applications/maniphest/xaction/ManiphestTaskStatusTransaction.php', 'ManiphestTaskSubpriorityTransaction' => 'applications/maniphest/xaction/ManiphestTaskSubpriorityTransaction.php', + 'ManiphestTaskSubtaskController' => 'applications/maniphest/controller/ManiphestTaskSubtaskController.php', 'ManiphestTaskSubtypeDatasource' => 'applications/maniphest/typeahead/ManiphestTaskSubtypeDatasource.php', 'ManiphestTaskTestCase' => 'applications/maniphest/__tests__/ManiphestTaskTestCase.php', 'ManiphestTaskTitleHeraldField' => 'applications/maniphest/herald/ManiphestTaskTitleHeraldField.php', @@ -2084,6 +2084,7 @@ phutil_register_library_map(array( 'PhabricatorAmazonAuthProvider' => 'applications/auth/provider/PhabricatorAmazonAuthProvider.php', 'PhabricatorAnchorView' => 'view/layout/PhabricatorAnchorView.php', 'PhabricatorAphlictManagementDebugWorkflow' => 'applications/aphlict/management/PhabricatorAphlictManagementDebugWorkflow.php', + 'PhabricatorAphlictManagementNotifyWorkflow' => 'applications/aphlict/management/PhabricatorAphlictManagementNotifyWorkflow.php', 'PhabricatorAphlictManagementRestartWorkflow' => 'applications/aphlict/management/PhabricatorAphlictManagementRestartWorkflow.php', 'PhabricatorAphlictManagementStartWorkflow' => 'applications/aphlict/management/PhabricatorAphlictManagementStartWorkflow.php', 'PhabricatorAphlictManagementStatusWorkflow' => 'applications/aphlict/management/PhabricatorAphlictManagementStatusWorkflow.php', @@ -2295,6 +2296,7 @@ phutil_register_library_map(array( 'PhabricatorAuthSessionEngineExtensionModule' => 'applications/auth/engine/PhabricatorAuthSessionEngineExtensionModule.php', 'PhabricatorAuthSessionGarbageCollector' => 'applications/auth/garbagecollector/PhabricatorAuthSessionGarbageCollector.php', 'PhabricatorAuthSessionInfo' => 'applications/auth/data/PhabricatorAuthSessionInfo.php', + 'PhabricatorAuthSessionPHIDType' => 'applications/auth/phid/PhabricatorAuthSessionPHIDType.php', 'PhabricatorAuthSessionQuery' => 'applications/auth/query/PhabricatorAuthSessionQuery.php', 'PhabricatorAuthSessionRevoker' => 'applications/auth/revoker/PhabricatorAuthSessionRevoker.php', 'PhabricatorAuthSetPasswordController' => 'applications/auth/controller/PhabricatorAuthSetPasswordController.php', @@ -3503,7 +3505,6 @@ phutil_register_library_map(array( 'PhabricatorNotificationServersConfigType' => 'applications/notification/config/PhabricatorNotificationServersConfigType.php', 'PhabricatorNotificationStatusView' => 'applications/notification/view/PhabricatorNotificationStatusView.php', 'PhabricatorNotificationTestController' => 'applications/notification/controller/PhabricatorNotificationTestController.php', - 'PhabricatorNotificationTestFeedStory' => 'applications/notification/feed/PhabricatorNotificationTestFeedStory.php', 'PhabricatorNotificationUIExample' => 'applications/uiexample/examples/PhabricatorNotificationUIExample.php', 'PhabricatorNotificationsApplication' => 'applications/notification/application/PhabricatorNotificationsApplication.php', 'PhabricatorNotificationsSetting' => 'applications/settings/setting/PhabricatorNotificationsSetting.php', @@ -4596,6 +4597,7 @@ phutil_register_library_map(array( 'PhabricatorUnknownContentSource' => 'infrastructure/contentsource/PhabricatorUnknownContentSource.php', 'PhabricatorUnsubscribedFromObjectEdgeType' => 'applications/transactions/edges/PhabricatorUnsubscribedFromObjectEdgeType.php', 'PhabricatorUser' => 'applications/people/storage/PhabricatorUser.php', + 'PhabricatorUserApproveTransaction' => 'applications/people/xaction/PhabricatorUserApproveTransaction.php', 'PhabricatorUserBadgesCacheType' => 'applications/people/cache/PhabricatorUserBadgesCacheType.php', 'PhabricatorUserBlurbField' => 'applications/people/customfield/PhabricatorUserBlurbField.php', 'PhabricatorUserCache' => 'applications/people/storage/PhabricatorUserCache.php', @@ -4622,6 +4624,7 @@ phutil_register_library_map(array( 'PhabricatorUserLogView' => 'applications/people/view/PhabricatorUserLogView.php', 'PhabricatorUserMessageCountCacheType' => 'applications/people/cache/PhabricatorUserMessageCountCacheType.php', 'PhabricatorUserNotificationCountCacheType' => 'applications/people/cache/PhabricatorUserNotificationCountCacheType.php', + 'PhabricatorUserNotifyTransaction' => 'applications/people/xaction/PhabricatorUserNotifyTransaction.php', 'PhabricatorUserPHIDResolver' => 'applications/phid/resolver/PhabricatorUserPHIDResolver.php', 'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php', 'PhabricatorUserPreferencesCacheType' => 'applications/people/cache/PhabricatorUserPreferencesCacheType.php', @@ -4643,6 +4646,7 @@ phutil_register_library_map(array( 'PhabricatorUserTransaction' => 'applications/people/storage/PhabricatorUserTransaction.php', 'PhabricatorUserTransactionEditor' => 'applications/people/editor/PhabricatorUserTransactionEditor.php', 'PhabricatorUserTransactionType' => 'applications/people/xaction/PhabricatorUserTransactionType.php', + 'PhabricatorUserUsernameTransaction' => 'applications/people/xaction/PhabricatorUserUsernameTransaction.php', 'PhabricatorUsersEditField' => 'applications/transactions/editfield/PhabricatorUsersEditField.php', 'PhabricatorUsersPolicyRule' => 'applications/people/policyrule/PhabricatorUsersPolicyRule.php', 'PhabricatorUsersSearchField' => 'applications/people/searchfield/PhabricatorUsersSearchField.php', @@ -7197,7 +7201,6 @@ phutil_register_library_map(array( 'Phobject', 'AphrontDatabaseTableRefInterface', ), - 'LiskDAOSet' => 'Phobject', 'LiskDAOTestCase' => 'PhabricatorTestCase', 'LiskEphemeralObjectException' => 'Exception', 'LiskFixtureTestCase' => 'PhabricatorTestCase', @@ -7347,6 +7350,7 @@ phutil_register_library_map(array( 'ManiphestTaskStatusTestCase' => 'PhabricatorTestCase', 'ManiphestTaskStatusTransaction' => 'ManiphestTaskTransactionType', 'ManiphestTaskSubpriorityTransaction' => 'ManiphestTaskTransactionType', + 'ManiphestTaskSubtaskController' => 'ManiphestController', 'ManiphestTaskSubtypeDatasource' => 'PhabricatorTypeaheadDatasource', 'ManiphestTaskTestCase' => 'PhabricatorTestCase', 'ManiphestTaskTitleHeraldField' => 'ManiphestTaskHeraldField', @@ -7700,6 +7704,7 @@ phutil_register_library_map(array( 'PhabricatorAmazonAuthProvider' => 'PhabricatorOAuth2AuthProvider', 'PhabricatorAnchorView' => 'AphrontView', 'PhabricatorAphlictManagementDebugWorkflow' => 'PhabricatorAphlictManagementWorkflow', + 'PhabricatorAphlictManagementNotifyWorkflow' => 'PhabricatorAphlictManagementWorkflow', 'PhabricatorAphlictManagementRestartWorkflow' => 'PhabricatorAphlictManagementWorkflow', 'PhabricatorAphlictManagementStartWorkflow' => 'PhabricatorAphlictManagementWorkflow', 'PhabricatorAphlictManagementStatusWorkflow' => 'PhabricatorAphlictManagementWorkflow', @@ -7945,6 +7950,7 @@ phutil_register_library_map(array( 'PhabricatorAuthSessionEngineExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorAuthSessionGarbageCollector' => 'PhabricatorGarbageCollector', 'PhabricatorAuthSessionInfo' => 'Phobject', + 'PhabricatorAuthSessionPHIDType' => 'PhabricatorPHIDType', 'PhabricatorAuthSessionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorAuthSessionRevoker' => 'PhabricatorAuthRevoker', 'PhabricatorAuthSetPasswordController' => 'PhabricatorAuthController', @@ -9320,7 +9326,6 @@ phutil_register_library_map(array( 'PhabricatorNotificationServersConfigType' => 'PhabricatorJSONConfigType', 'PhabricatorNotificationStatusView' => 'AphrontTagView', 'PhabricatorNotificationTestController' => 'PhabricatorNotificationController', - 'PhabricatorNotificationTestFeedStory' => 'PhabricatorFeedStory', 'PhabricatorNotificationUIExample' => 'PhabricatorUIExample', 'PhabricatorNotificationsApplication' => 'PhabricatorApplication', 'PhabricatorNotificationsSetting' => 'PhabricatorInternalSetting', @@ -10642,6 +10647,7 @@ phutil_register_library_map(array( 'PhabricatorConduitResultInterface', 'PhabricatorAuthPasswordHashInterface', ), + 'PhabricatorUserApproveTransaction' => 'PhabricatorUserTransactionType', 'PhabricatorUserBadgesCacheType' => 'PhabricatorUserCacheType', 'PhabricatorUserBlurbField' => 'PhabricatorUserCustomField', 'PhabricatorUserCache' => 'PhabricatorUserDAO', @@ -10674,6 +10680,7 @@ phutil_register_library_map(array( 'PhabricatorUserLogView' => 'AphrontView', 'PhabricatorUserMessageCountCacheType' => 'PhabricatorUserCacheType', 'PhabricatorUserNotificationCountCacheType' => 'PhabricatorUserCacheType', + 'PhabricatorUserNotifyTransaction' => 'PhabricatorUserTransactionType', 'PhabricatorUserPHIDResolver' => 'PhabricatorPHIDResolver', 'PhabricatorUserPreferences' => array( 'PhabricatorUserDAO', @@ -10700,6 +10707,7 @@ phutil_register_library_map(array( 'PhabricatorUserTransaction' => 'PhabricatorModularTransaction', 'PhabricatorUserTransactionEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorUserTransactionType' => 'PhabricatorModularTransactionType', + 'PhabricatorUserUsernameTransaction' => 'PhabricatorUserTransactionType', 'PhabricatorUsersEditField' => 'PhabricatorTokenizerEditField', 'PhabricatorUsersPolicyRule' => 'PhabricatorPolicyRule', 'PhabricatorUsersSearchField' => 'PhabricatorSearchTokenizerField', diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementNotifyWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementNotifyWorkflow.php new file mode 100644 index 0000000000..a7653e74b5 --- /dev/null +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementNotifyWorkflow.php @@ -0,0 +1,81 @@ +setName('notify') + ->setSynopsis(pht('Send a notification to a user.')) + ->setArguments( + array( + array( + 'name' => 'user', + 'param' => 'username', + 'help' => pht('User to notify.'), + ), + array( + 'name' => 'message', + 'param' => 'text', + 'help' => pht('Message to send.'), + ), + )); + } + + public function execute(PhutilArgumentParser $args) { + $viewer = $this->getViewer(); + + $username = $args->getArg('user'); + if (!strlen($username)) { + throw new PhutilArgumentUsageException( + pht( + 'Specify a user to notify with "--user".')); + } + + $user = id(new PhabricatorPeopleQuery()) + ->setViewer($viewer) + ->withUsernames(array($username)) + ->executeOne(); + + if (!$user) { + throw new PhutilArgumentUsageException( + pht( + 'No user with username "%s" exists.', + $username)); + } + + $message = $args->getArg('message'); + if (!strlen($message)) { + throw new PhutilArgumentUsageException( + pht( + 'Specify a message to send with "--message".')); + } + + $application_phid = id(new PhabricatorNotificationsApplication()) + ->getPHID(); + + $content_source = $this->newContentSource(); + + $xactions = array(); + + $xactions[] = id(new PhabricatorUserTransaction()) + ->setTransactionType( + PhabricatorUserNotifyTransaction::TRANSACTIONTYPE) + ->setNewValue($message) + ->setForceNotifyPHIDs(array($user->getPHID())); + + $editor = id(new PhabricatorUserTransactionEditor()) + ->setActor($viewer) + ->setActingAsPHID($application_phid) + ->setContentSource($content_source); + + $editor->applyTransactions($user, $xactions); + + echo tsprintf( + "%s\n", + pht('Sent notification.')); + + return 0; + } + +} diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 3f65cc80f8..165f8ef84f 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -251,47 +251,16 @@ final class PhabricatorAuditEditor case PhabricatorTransactions::TYPE_COMMENT: $this->didExpandInlineState = true; - $actor_phid = $this->getActingAsPHID(); - $author_phid = $object->getAuthorPHID(); - $actor_is_author = ($actor_phid == $author_phid); + $query_template = id(new DiffusionDiffInlineCommentQuery()) + ->withCommitPHIDs(array($object->getPHID())); - $state_map = PhabricatorTransactions::getInlineStateMap(); + $state_xaction = $this->newInlineStateTransaction( + $object, + $query_template); - $query = id(new DiffusionDiffInlineCommentQuery()) - ->setViewer($this->getActor()) - ->withCommitPHIDs(array($object->getPHID())) - ->withFixedStates(array_keys($state_map)); - - $inlines = array(); - - $inlines[] = id(clone $query) - ->withAuthorPHIDs(array($actor_phid)) - ->withHasTransaction(false) - ->execute(); - - if ($actor_is_author) { - $inlines[] = id(clone $query) - ->withHasTransaction(true) - ->execute(); + if ($state_xaction) { + $xactions[] = $state_xaction; } - - $inlines = array_mergev($inlines); - - if (!$inlines) { - break; - } - - $old_value = mpull($inlines, 'getFixedState', 'getPHID'); - $new_value = array(); - foreach ($old_value as $key => $state) { - $new_value[$key] = $state_map[$state]; - } - - $xactions[] = id(new PhabricatorAuditTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE) - ->setIgnoreOnNoEffect(true) - ->setOldValue($old_value) - ->setNewValue($new_value); break; } } diff --git a/src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php b/src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php index fa58977c90..413d0306b9 100644 --- a/src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php +++ b/src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php @@ -16,8 +16,9 @@ final class PhabricatorAuthTerminateSessionController $query->withIDs(array($id)); } - $current_key = PhabricatorHash::weakDigest( - $request->getCookie(PhabricatorCookies::COOKIE_SESSION)); + $current_key = PhabricatorAuthSession::newSessionDigest( + new PhutilOpaqueEnvelope( + $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); $sessions = $query->execute(); foreach ($sessions as $key => $session) { diff --git a/src/applications/auth/controller/PhabricatorAuthUnlinkController.php b/src/applications/auth/controller/PhabricatorAuthUnlinkController.php index 6211e78110..e6e1493e5a 100644 --- a/src/applications/auth/controller/PhabricatorAuthUnlinkController.php +++ b/src/applications/auth/controller/PhabricatorAuthUnlinkController.php @@ -56,7 +56,8 @@ final class PhabricatorAuthUnlinkController id(new PhabricatorAuthSessionEngine())->terminateLoginSessions( $viewer, - $request->getCookie(PhabricatorCookies::COOKIE_SESSION)); + new PhutilOpaqueEnvelope( + $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); return id(new AphrontRedirectResponse())->setURI($this->getDoneURI()); } diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php index 4ce86e8f97..2020e4a542 100644 --- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php @@ -109,36 +109,49 @@ final class PhabricatorAuthSessionEngine extends Phobject { $session_table = new PhabricatorAuthSession(); $user_table = new PhabricatorUser(); - $conn_r = $session_table->establishConnection('r'); - $session_key = PhabricatorHash::weakDigest($session_token); + $conn = $session_table->establishConnection('r'); - $cache_parts = $this->getUserCacheQueryParts($conn_r); + // TODO: See T13225. We're moving sessions to a more modern digest + // algorithm, but still accept older cookies for compatibility. + $session_key = PhabricatorAuthSession::newSessionDigest( + new PhutilOpaqueEnvelope($session_token)); + $weak_key = PhabricatorHash::weakDigest($session_token); + + $cache_parts = $this->getUserCacheQueryParts($conn); list($cache_selects, $cache_joins, $cache_map, $types_map) = $cache_parts; $info = queryfx_one( - $conn_r, + $conn, 'SELECT s.id AS s_id, + s.phid AS s_phid, s.sessionExpires AS s_sessionExpires, s.sessionStart AS s_sessionStart, s.highSecurityUntil AS s_highSecurityUntil, s.isPartial AS s_isPartial, s.signedLegalpadDocuments as s_signedLegalpadDocuments, + IF(s.sessionKey = %P, 1, 0) as s_weak, u.* %Q - FROM %T u JOIN %T s ON u.phid = s.userPHID - AND s.type = %s AND s.sessionKey = %P %Q', + FROM %R u JOIN %R s ON u.phid = s.userPHID + AND s.type = %s AND s.sessionKey IN (%P, %P) %Q', + new PhutilOpaqueEnvelope($weak_key), $cache_selects, - $user_table->getTableName(), - $session_table->getTableName(), + $user_table, + $session_table, $session_type, new PhutilOpaqueEnvelope($session_key), + new PhutilOpaqueEnvelope($weak_key), $cache_joins); if (!$info) { return null; } + // TODO: Remove this, see T13225. + $is_weak = (bool)$info['s_weak']; + unset($info['s_weak']); + $session_dict = array( 'userPHID' => $info['phid'], 'sessionKey' => $session_key, @@ -201,6 +214,19 @@ final class PhabricatorAuthSessionEngine extends Phobject { unset($unguarded); } + // TODO: Remove this, see T13225. + if ($is_weak) { + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $conn_w = $session_table->establishConnection('w'); + queryfx( + $conn_w, + 'UPDATE %T SET sessionKey = %P WHERE id = %d', + $session->getTableName(), + new PhutilOpaqueEnvelope($session_key), + $session->getID()); + unset($unguarded); + } + $user->attachSession($session); return $user; } @@ -240,7 +266,8 @@ final class PhabricatorAuthSessionEngine extends Phobject { // This has a side effect of validating the session type. $session_ttl = PhabricatorAuthSession::getSessionTypeTTL($session_type); - $digest_key = PhabricatorHash::weakDigest($session_key); + $digest_key = PhabricatorAuthSession::newSessionDigest( + new PhutilOpaqueEnvelope($session_key)); // Logging-in users don't have CSRF stuff yet, so we have to unguard this // write. @@ -298,7 +325,7 @@ final class PhabricatorAuthSessionEngine extends Phobject { */ public function terminateLoginSessions( PhabricatorUser $user, - $except_session = null) { + PhutilOpaqueEnvelope $except_session = null) { $sessions = id(new PhabricatorAuthSessionQuery()) ->setViewer($user) @@ -306,7 +333,8 @@ final class PhabricatorAuthSessionEngine extends Phobject { ->execute(); if ($except_session !== null) { - $except_session = PhabricatorHash::weakDigest($except_session); + $except_session = PhabricatorAuthSession::newSessionDigest( + $except_session); } foreach ($sessions as $key => $session) { diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php index 7d7aec921f..ae3608d525 100644 --- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php @@ -2,6 +2,8 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { + const DIGEST_TEMPORARY_KEY = 'mfa.totp.sync'; + public function getFactorKey() { return 'totp'; } @@ -34,12 +36,16 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { // (We store and verify the hash of the key, not the key itself, to limit // how useful the data in the table is to an attacker.) + $token_code = PhabricatorHash::digestWithNamedKey( + $key, + self::DIGEST_TEMPORARY_KEY); + $temporary_token = id(new PhabricatorAuthTemporaryTokenQuery()) ->setViewer($user) ->withTokenResources(array($user->getPHID())) ->withTokenTypes(array($totp_token_type)) ->withExpired(false) - ->withTokenCodes(array(PhabricatorHash::weakDigest($key))) + ->withTokenCodes(array($token_code)) ->executeOne(); if (!$temporary_token) { // If we don't have a matching token, regenerate the key below. @@ -53,12 +59,16 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { // Mark this key as one we generated, so the user is allowed to submit // a response for it. + $token_code = PhabricatorHash::digestWithNamedKey( + $key, + self::DIGEST_TEMPORARY_KEY); + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); id(new PhabricatorAuthTemporaryToken()) ->setTokenResource($user->getPHID()) ->setTokenType($totp_token_type) ->setTokenExpires(time() + phutil_units('1 hour in seconds')) - ->setTokenCode(PhabricatorHash::weakDigest($key)) + ->setTokenCode($token_code) ->save(); unset($unguarded); } diff --git a/src/applications/auth/phid/PhabricatorAuthSessionPHIDType.php b/src/applications/auth/phid/PhabricatorAuthSessionPHIDType.php new file mode 100644 index 0000000000..e031c4ae88 --- /dev/null +++ b/src/applications/auth/phid/PhabricatorAuthSessionPHIDType.php @@ -0,0 +1,34 @@ +withPHIDs($phids); + } + + public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects) { + return; + } + +} diff --git a/src/applications/auth/query/PhabricatorAuthSessionQuery.php b/src/applications/auth/query/PhabricatorAuthSessionQuery.php index 25928e72c1..00a663e964 100644 --- a/src/applications/auth/query/PhabricatorAuthSessionQuery.php +++ b/src/applications/auth/query/PhabricatorAuthSessionQuery.php @@ -4,6 +4,7 @@ final class PhabricatorAuthSessionQuery extends PhabricatorCursorPagedPolicyAwareQuery { private $ids; + private $phids; private $identityPHIDs; private $sessionKeys; private $sessionTypes; @@ -28,19 +29,17 @@ final class PhabricatorAuthSessionQuery return $this; } + public function withPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + + public function newResultObject() { + return new PhabricatorAuthSession(); + } + protected function loadPage() { - $table = new PhabricatorAuthSession(); - $conn_r = $table->establishConnection('r'); - - $data = queryfx_all( - $conn_r, - 'SELECT * FROM %T %Q %Q %Q', - $table->getTableName(), - $this->buildWhereClause($conn_r), - $this->buildOrderClause($conn_r), - $this->buildLimitClause($conn_r)); - - return $table->loadAllFromArray($data); + return $this->loadStandardPage($this->newResultObject()); } protected function willFilterPage(array $sessions) { @@ -65,8 +64,8 @@ final class PhabricatorAuthSessionQuery return $sessions; } - protected function buildWhereClause(AphrontDatabaseConnection $conn) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); if ($this->ids !== null) { $where[] = qsprintf( @@ -75,6 +74,13 @@ final class PhabricatorAuthSessionQuery $this->ids); } + if ($this->phids !== null) { + $where[] = qsprintf( + $conn, + 'phid IN (%Ls)', + $this->phids); + } + if ($this->identityPHIDs !== null) { $where[] = qsprintf( $conn, @@ -85,7 +91,8 @@ final class PhabricatorAuthSessionQuery if ($this->sessionKeys !== null) { $hashes = array(); foreach ($this->sessionKeys as $session_key) { - $hashes[] = PhabricatorHash::weakDigest($session_key); + $hashes[] = PhabricatorAuthSession::newSessionDigest( + new PhutilOpaqueEnvelope($session_key)); } $where[] = qsprintf( $conn, @@ -100,9 +107,7 @@ final class PhabricatorAuthSessionQuery $this->sessionTypes); } - $where[] = $this->buildPagingClause($conn); - - return $this->formatWhereClause($conn, $where); + return $where; } public function getQueryApplicationClass() { diff --git a/src/applications/auth/storage/PhabricatorAuthSession.php b/src/applications/auth/storage/PhabricatorAuthSession.php index cf707a053d..6d54dda781 100644 --- a/src/applications/auth/storage/PhabricatorAuthSession.php +++ b/src/applications/auth/storage/PhabricatorAuthSession.php @@ -6,6 +6,8 @@ final class PhabricatorAuthSession extends PhabricatorAuthDAO const TYPE_WEB = 'web'; const TYPE_CONDUIT = 'conduit'; + const SESSION_DIGEST_KEY = 'session.digest'; + protected $userPHID; protected $type; protected $sessionKey; @@ -17,12 +19,19 @@ final class PhabricatorAuthSession extends PhabricatorAuthDAO private $identityObject = self::ATTACHABLE; + public static function newSessionDigest(PhutilOpaqueEnvelope $session_token) { + return PhabricatorHash::digestWithNamedKey( + $session_token->openEnvelope(), + self::SESSION_DIGEST_KEY); + } + protected function getConfiguration() { return array( self::CONFIG_TIMESTAMPS => false, + self::CONFIG_AUX_PHID => true, self::CONFIG_COLUMN_SCHEMA => array( 'type' => 'text32', - 'sessionKey' => 'bytes40', + 'sessionKey' => 'text64', 'sessionStart' => 'epoch', 'sessionExpires' => 'epoch', 'highSecurityUntil' => 'epoch?', @@ -74,6 +83,10 @@ final class PhabricatorAuthSession extends PhabricatorAuthDAO } } + public function getPHIDType() { + return PhabricatorAuthSessionPHIDType::TYPECONST; + } + public function isHighSecuritySession() { $until = $this->getHighSecurityUntil(); diff --git a/src/applications/differential/editor/DifferentialRevisionEditEngine.php b/src/applications/differential/editor/DifferentialRevisionEditEngine.php index ffae7fab16..07b693044f 100644 --- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php +++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php @@ -279,36 +279,17 @@ final class DifferentialRevisionEditEngine $object); $inlines = msort($inlines, 'getID'); - foreach ($inlines as $inline) { - $xactions[] = id(new DifferentialTransaction()) - ->setTransactionType(DifferentialTransaction::TYPE_INLINE) - ->attachComment($inline); - } + $editor = $object->getApplicationTransactionEditor() + ->setActor($viewer); - $viewer_phid = $viewer->getPHID(); - $viewer_is_author = ($object->getAuthorPHID() == $viewer_phid); - if ($viewer_is_author) { - $state_map = PhabricatorTransactions::getInlineStateMap(); + $query_template = id(new DifferentialDiffInlineCommentQuery()) + ->withRevisionPHIDs(array($object->getPHID())); - $inlines = id(new DifferentialDiffInlineCommentQuery()) - ->setViewer($viewer) - ->withRevisionPHIDs(array($object->getPHID())) - ->withFixedStates(array_keys($state_map)) - ->execute(); - if ($inlines) { - $old_value = mpull($inlines, 'getFixedState', 'getPHID'); - $new_value = array(); - foreach ($old_value as $key => $state) { - $new_value[$key] = $state_map[$state]; - } - - $xactions[] = id(new DifferentialTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE) - ->setIgnoreOnNoEffect(true) - ->setOldValue($old_value) - ->setNewValue($new_value); - } - } + $xactions = $editor->newAutomaticInlineTransactions( + $object, + $inlines, + DifferentialTransaction::TYPE_INLINE, + $query_template); return $xactions; } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 853b024e27..8072c19e9a 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -247,50 +247,16 @@ final class DifferentialTransactionEditor case DifferentialTransaction::TYPE_INLINE: $this->didExpandInlineState = true; - $actor_phid = $this->getActingAsPHID(); - $author_phid = $object->getAuthorPHID(); - $actor_is_author = ($actor_phid == $author_phid); + $query_template = id(new DifferentialDiffInlineCommentQuery()) + ->withRevisionPHIDs(array($object->getPHID())); - $state_map = PhabricatorTransactions::getInlineStateMap(); + $state_xaction = $this->newInlineStateTransaction( + $object, + $query_template); - $query = id(new DifferentialDiffInlineCommentQuery()) - ->setViewer($this->getActor()) - ->withRevisionPHIDs(array($object->getPHID())) - ->withFixedStates(array_keys($state_map)); - - $inlines = array(); - - // We're going to undraft any "done" marks on your own inlines. - $inlines[] = id(clone $query) - ->withAuthorPHIDs(array($actor_phid)) - ->withHasTransaction(false) - ->execute(); - - // If you're the author, we also undraft any "done" marks on other - // inlines. - if ($actor_is_author) { - $inlines[] = id(clone $query) - ->withHasTransaction(true) - ->execute(); + if ($state_xaction) { + $results[] = $state_xaction; } - - $inlines = array_mergev($inlines); - - if (!$inlines) { - break; - } - - $old_value = mpull($inlines, 'getFixedState', 'getPHID'); - $new_value = array(); - foreach ($old_value as $key => $state) { - $new_value[$key] = $state_map[$state]; - } - - $results[] = id(new DifferentialTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE) - ->setIgnoreOnNoEffect(true) - ->setOldValue($old_value) - ->setNewValue($new_value); break; } } diff --git a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php index 8392504def..667af46550 100644 --- a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php +++ b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php @@ -170,36 +170,17 @@ final class DiffusionCommitEditEngine $raw = true); $inlines = msort($inlines, 'getID'); - foreach ($inlines as $inline) { - $xactions[] = $object->getApplicationTransactionTemplate() - ->setTransactionType(PhabricatorAuditActionConstants::INLINE) - ->attachComment($inline); - } + $editor = $object->getApplicationTransactionEditor() + ->setActor($viewer); - $viewer_phid = $viewer->getPHID(); - $viewer_is_author = ($object->getAuthorPHID() == $viewer_phid); - if ($viewer_is_author) { - $state_map = PhabricatorTransactions::getInlineStateMap(); + $query_template = id(new DiffusionDiffInlineCommentQuery()) + ->withCommitPHIDs(array($object->getPHID())); - $inlines = id(new DiffusionDiffInlineCommentQuery()) - ->setViewer($viewer) - ->withCommitPHIDs(array($object->getPHID())) - ->withFixedStates(array_keys($state_map)) - ->execute(); - if ($inlines) { - $old_value = mpull($inlines, 'getFixedState', 'getPHID'); - $new_value = array(); - foreach ($old_value as $key => $state) { - $new_value[$key] = $state_map[$state]; - } - - $xactions[] = $object->getApplicationTransactionTemplate() - ->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE) - ->setIgnoreOnNoEffect(true) - ->setOldValue($old_value) - ->setNewValue($new_value); - } - } + $xactions = $editor->newAutomaticInlineTransactions( + $object, + $inlines, + PhabricatorAuditActionConstants::INLINE, + $query_template); return $xactions; } diff --git a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php index 29dc588b45..8907a22ce5 100644 --- a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php +++ b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php @@ -212,11 +212,8 @@ final class DiffusionRepositoryEditEngine ->setObject($object) ->execute(); - $track_value = $object->getDetail('branch-filter', array()); - $track_value = array_keys($track_value); - - $autoclose_value = $object->getDetail('close-commits-filter', array()); - $autoclose_value = array_keys($autoclose_value); + $track_value = $object->getTrackOnlyRules(); + $autoclose_value = $object->getAutocloseOnlyRules(); $automation_instructions = pht( "Configure **Repository Automation** to allow Phabricator to ". diff --git a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php index e3ace3a5a5..bdd0a3f71d 100644 --- a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php @@ -23,8 +23,8 @@ final class DiffusionRepositoryBranchesManagementPanel $has_any = $repository->getDetail('default-branch') || - $repository->getDetail('branch-filter') || - $repository->getDetail('close-commits-filter'); + $repository->getTrackOnlyRules() || + $repository->getAutocloseOnlyRules(); if ($has_any) { return 'fa-code-fork'; @@ -74,17 +74,21 @@ final class DiffusionRepositoryBranchesManagementPanel ->setViewer($viewer); $default_branch = nonempty( - $repository->getHumanReadableDetail('default-branch'), + $repository->getDetail('default-branch'), phutil_tag('em', array(), $repository->getDefaultBranch())); $view->addProperty(pht('Default Branch'), $default_branch); + $track_only_rules = $repository->getTrackOnlyRules(); + $track_only_rules = implode(', ', $track_only_rules); $track_only = nonempty( - $repository->getHumanReadableDetail('branch-filter', array()), + $track_only_rules, phutil_tag('em', array(), pht('Track All Branches'))); $view->addProperty(pht('Track Only'), $track_only); + $autoclose_rules = $repository->getAutocloseOnlyRules(); + $autoclose_rules = implode(', ', $autoclose_rules); $autoclose_only = nonempty( - $repository->getHumanReadableDetail('close-commits-filter', array()), + $autoclose_rules, phutil_tag('em', array(), pht('Autoclose On All Branches'))); $autoclose_disabled = false; diff --git a/src/applications/diffusion/management/DiffusionRepositorySubversionManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositorySubversionManagementPanel.php index f87a5d9b85..2d95bc2292 100644 --- a/src/applications/diffusion/management/DiffusionRepositorySubversionManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositorySubversionManagementPanel.php @@ -68,7 +68,7 @@ final class DiffusionRepositorySubversionManagementPanel ->setViewer($viewer); $default_branch = nonempty( - $repository->getHumanReadableDetail('svn-subpath'), + $repository->getDetail('svn-subpath'), phutil_tag('em', array(), pht('Import Entire Repository'))); $view->addProperty(pht('Import Only'), $default_branch); diff --git a/src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php b/src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php index 8ae9b3d203..850c06f105 100644 --- a/src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php +++ b/src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php @@ -43,12 +43,13 @@ final class DiffusionPathChangeQuery extends Phobject { $conn_r = $repository->establishConnection('r'); - $limit = ''; if ($this->limit) { $limit = qsprintf( $conn_r, 'LIMIT %d', $this->limit + 1); + } else { + $limit = qsprintf($conn_r, ''); } $raw_changes = queryfx_all( diff --git a/src/applications/drydock/controller/DrydockConsoleController.php b/src/applications/drydock/controller/DrydockConsoleController.php index 54adcf5ca5..d0ae358768 100644 --- a/src/applications/drydock/controller/DrydockConsoleController.php +++ b/src/applications/drydock/controller/DrydockConsoleController.php @@ -34,6 +34,7 @@ final class DrydockConsoleController extends DrydockController { ->setHeader(pht('Blueprints')) ->setImageIcon('fa-map-o') ->setHref($this->getApplicationURI('blueprint/')) + ->setClickable(true) ->addAttribute( pht( 'Configure blueprints so Drydock can build resources, like '. @@ -44,6 +45,7 @@ final class DrydockConsoleController extends DrydockController { ->setHeader(pht('Resources')) ->setImageIcon('fa-map') ->setHref($this->getApplicationURI('resource/')) + ->setClickable(true) ->addAttribute( pht('View and manage resources Drydock has built, like hosts.'))); @@ -52,6 +54,7 @@ final class DrydockConsoleController extends DrydockController { ->setHeader(pht('Leases')) ->setImageIcon('fa-link') ->setHref($this->getApplicationURI('lease/')) + ->setClickable(true) ->addAttribute(pht('Manage leases on resources.'))); $menu->addItem( @@ -59,6 +62,7 @@ final class DrydockConsoleController extends DrydockController { ->setHeader(pht('Repository Operations')) ->setImageIcon('fa-fighter-jet') ->setHref($this->getApplicationURI('operation/')) + ->setClickable(true) ->addAttribute(pht('Review the repository operation queue.'))); $crumbs = $this->buildApplicationCrumbs(); diff --git a/src/applications/fact/extract/PhabricatorFactUpdateIterator.php b/src/applications/fact/extract/PhabricatorFactUpdateIterator.php index df77f4dd33..c0d229084f 100644 --- a/src/applications/fact/extract/PhabricatorFactUpdateIterator.php +++ b/src/applications/fact/extract/PhabricatorFactUpdateIterator.php @@ -12,11 +12,8 @@ final class PhabricatorFactUpdateIterator extends PhutilBufferedIterator { private $position; private $ignoreUpdatesDuration = 15; - private $set; - public function __construct(LiskDAO $object) { - $this->set = new LiskDAOSet(); - $this->object = $object->putInSet($this->set); + $this->object = $object; } public function setPosition($position) { @@ -41,8 +38,6 @@ final class PhabricatorFactUpdateIterator extends PhutilBufferedIterator { } protected function loadPage() { - $this->set->clearSet(); - if ($this->object->hasProperty('dateModified')) { if ($this->cursor) { list($after_epoch, $after_id) = explode(':', $this->cursor); diff --git a/src/applications/feed/query/PhabricatorFeedQuery.php b/src/applications/feed/query/PhabricatorFeedQuery.php index ce472a6026..a35f14da57 100644 --- a/src/applications/feed/query/PhabricatorFeedQuery.php +++ b/src/applications/feed/query/PhabricatorFeedQuery.php @@ -34,7 +34,15 @@ final class PhabricatorFeedQuery } protected function willFilterPage(array $data) { - return PhabricatorFeedStory::loadAllFromRows($data, $this->getViewer()); + $stories = PhabricatorFeedStory::loadAllFromRows($data, $this->getViewer()); + + foreach ($stories as $key => $story) { + if (!$story->isVisibleInFeed()) { + unset($stories[$key]); + } + } + + return $stories; } protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index c3984573c7..e0c65c7dc2 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -418,6 +418,14 @@ abstract class PhabricatorFeedStory return array(); } + public function isVisibleInFeed() { + return true; + } + + public function isVisibleInNotifications() { + return true; + } + /* -( PhabricatorPolicyInterface Implementation )-------------------------- */ diff --git a/src/applications/herald/engine/HeraldEngine.php b/src/applications/herald/engine/HeraldEngine.php index 739e83e4e8..d853e3eb9a 100644 --- a/src/applications/herald/engine/HeraldEngine.php +++ b/src/applications/herald/engine/HeraldEngine.php @@ -243,9 +243,9 @@ final class HeraldEngine extends Phobject { } queryfx( $conn_w, - 'INSERT IGNORE INTO %T (phid, ruleID) VALUES %Q', + 'INSERT IGNORE INTO %T (phid, ruleID) VALUES %LQ', HeraldRule::TABLE_RULE_APPLIED, - implode(', ', $sql)); + $sql); } } } diff --git a/src/applications/maniphest/application/PhabricatorManiphestApplication.php b/src/applications/maniphest/application/PhabricatorManiphestApplication.php index 1ed4096660..35c1efb6e8 100644 --- a/src/applications/maniphest/application/PhabricatorManiphestApplication.php +++ b/src/applications/maniphest/application/PhabricatorManiphestApplication.php @@ -52,6 +52,7 @@ final class PhabricatorManiphestApplication extends PhabricatorApplication { 'task/' => array( $this->getEditRoutePattern('edit/') => 'ManiphestTaskEditController', + 'subtask/(?P[1-9]\d*)/' => 'ManiphestTaskSubtaskController', ), 'subpriority/' => 'ManiphestSubpriorityController', ), diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php index 4458dc3636..609db0d1b6 100644 --- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php +++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php @@ -338,6 +338,8 @@ dictionary with these keys: - `tag` //Optional string.// Tag text for this subtype. - `color` //Optional string.// Display color for this subtype. - `icon` //Optional string.// Icon for the subtype. + - `children` //Optional map.// Configure options shown to the user when + they "Create Subtask". See below. Each subtype must have a unique key, and you must define a subtype with the key "%s", which is used as a default subtype. @@ -345,6 +347,54 @@ the key "%s", which is used as a default subtype. The tag text (`tag`) is used to set the text shown in the subtype tag on list views and workboards. If you do not configure it, the default subtype will have no subtype tag and other subtypes will use their name as tag text. + +The `children` key allows you to configure which options are presented to the +user when they "Create Subtask" from a task of this subtype. You can specify +these keys: + + - `subtypes`: //Optional list.// Show users creation forms for these + task subtypes. + - `forms`: //Optional list.// Show users these specific forms, + in order. + +If you don't specify either constraint, users will be shown creation forms +for the same subtype. + +For example, if you have a "quest" subtype and do not configure `children`, +users who click "Create Subtask" will be presented with all create forms for +"quest" tasks. + +If you want to present them with forms for a different task subtype or set of +subtypes instead, use `subtypes`: + +``` + { + ... + "children": { + "subtypes": ["objective", "boss", "reward"] + } + ... + } +``` + +If you want to present them with specific forms, use `forms` and specify form +IDs: + +``` + { + ... + "children": { + "forms": [12, 16] + } + ... + } +``` + +When specifying forms by ID explicitly, the order you specify the forms in will +be used when presenting options to the user. + +If only one option would be presented, the user will be taken directly to the +appropriate form instead of being prompted to choose a form. EOTEXT , $subtype_default_key)); diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index a11cc4d85f..0f96d76b91 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -281,29 +281,39 @@ final class ManiphestTaskDetailController extends ManiphestController { ->setDisabled(!$can_edit) ->setWorkflow($workflow_edit)); - $edit_config = $edit_engine->loadDefaultEditConfiguration($task); - $can_create = (bool)$edit_config; + $subtype_map = $task->newEditEngineSubtypeMap(); + $subtask_options = $subtype_map->getCreateFormsForSubtype( + $edit_engine, + $task); - if ($can_create) { - $form_key = $edit_config->getIdentifier(); - $edit_uri = id(new PhutilURI("/task/edit/form/{$form_key}/")) + // If no forms are available, we want to show the user an error. + // If one form is available, we take them user directly to the form. + // If two or more forms are available, we give the user a choice. + + // The "subtask" controller handles the first case (no forms) and the + // third case (more than one form). In the case of one form, we link + // directly to the form. + $subtask_uri = "/task/subtask/{$id}/"; + $subtask_workflow = true; + + if (count($subtask_options) == 1) { + $subtask_form = head($subtask_options); + $form_key = $subtask_form->getIdentifier(); + $subtask_uri = id(new PhutilURI("/task/edit/form/{$form_key}/")) ->setQueryParam('parent', $id) ->setQueryParam('template', $id) ->setQueryParam('status', ManiphestTaskStatus::getDefaultStatus()); - $edit_uri = $this->getApplicationURI($edit_uri); - } else { - // TODO: This will usually give us a somewhat-reasonable error page, but - // could be a bit cleaner. - $edit_uri = "/task/edit/{$id}/"; - $edit_uri = $this->getApplicationURI($edit_uri); + $subtask_workflow = false; } + $subtask_uri = $this->getApplicationURI($subtask_uri); + $subtask_item = id(new PhabricatorActionView()) ->setName(pht('Create Subtask')) - ->setHref($edit_uri) + ->setHref($subtask_uri) ->setIcon('fa-level-down') - ->setDisabled(!$can_create) - ->setWorkflow(!$can_create); + ->setDisabled(!$subtask_options) + ->setWorkflow($subtask_workflow); $relationship_list = PhabricatorObjectRelationshipList::newForObject( $viewer, diff --git a/src/applications/maniphest/controller/ManiphestTaskSubtaskController.php b/src/applications/maniphest/controller/ManiphestTaskSubtaskController.php new file mode 100644 index 0000000000..5256c1bd26 --- /dev/null +++ b/src/applications/maniphest/controller/ManiphestTaskSubtaskController.php @@ -0,0 +1,71 @@ +getViewer(); + $id = $request->getURIData('id'); + + $task = id(new ManiphestTaskQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if (!$task) { + return new Aphront404Response(); + } + + $cancel_uri = $task->getURI(); + + $edit_engine = id(new ManiphestEditEngine()) + ->setViewer($viewer) + ->setTargetObject($task); + + $subtype_map = $task->newEditEngineSubtypeMap(); + + $subtype_options = $subtype_map->getCreateFormsForSubtype( + $edit_engine, + $task); + + if (!$subtype_options) { + return $this->newDialog() + ->setTitle(pht('No Forms')) + ->appendParagraph( + pht( + 'You do not have access to any forms which can be used to '. + 'create a subtask.')) + ->addCancelButton($cancel_uri, pht('Close')); + } + + $menu = id(new PHUIObjectItemListView()) + ->setUser($viewer) + ->setBig(true) + ->setFlush(true); + + foreach ($subtype_options as $form_key => $subtype_form) { + $subtype_key = $subtype_form->getSubtype(); + $subtype = $subtype_map->getSubtype($subtype_key); + + $subtask_uri = id(new PhutilURI("/task/edit/form/{$form_key}/")) + ->setQueryParam('parent', $id) + ->setQueryParam('template', $id) + ->setQueryParam('status', ManiphestTaskStatus::getDefaultStatus()); + $subtask_uri = $this->getApplicationURI($subtask_uri); + + $item = id(new PHUIObjectItemView()) + ->setHeader($subtype_form->getDisplayName()) + ->setHref($subtask_uri) + ->setClickable(true) + ->setImageIcon($subtype->newIconView()) + ->addAttribute($subtype->getName()); + + $menu->addItem($item); + } + + return $this->newDialog() + ->setTitle(pht('Choose Subtype')) + ->appendChild($menu) + ->addCancelButton($cancel_uri); + } + +} diff --git a/src/applications/maniphest/editor/ManiphestEditEngine.php b/src/applications/maniphest/editor/ManiphestEditEngine.php index 0b0b4d6758..dc9c56f840 100644 --- a/src/applications/maniphest/editor/ManiphestEditEngine.php +++ b/src/applications/maniphest/editor/ManiphestEditEngine.php @@ -169,7 +169,9 @@ EODOCS ->setConduitDocumentation($column_documentation) ->setAliases(array('columnPHID', 'columns', 'columnPHIDs')) ->setTransactionType(PhabricatorTransactions::TYPE_COLUMNS) - ->setIsFormField(false) + ->setIsReorderable(false) + ->setIsDefaultable(false) + ->setIsLockable(false) ->setCommentActionLabel(pht('Move on Workboard')) ->setCommentActionOrder(2000) ->setColumnMap($column_map), diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index d1932a7597..fc5097f4d3 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -641,9 +641,9 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { if ($this->hasOpenSubtasks !== null) { if ($this->hasOpenSubtasks) { - $join_type = 'JOIN'; + $join_type = qsprintf($conn, 'JOIN'); } else { - $join_type = 'LEFT JOIN'; + $join_type = qsprintf($conn, 'LEFT JOIN'); } $joins[] = qsprintf( diff --git a/src/applications/notification/builder/PhabricatorNotificationBuilder.php b/src/applications/notification/builder/PhabricatorNotificationBuilder.php index 026f55149c..b9a79be3f3 100644 --- a/src/applications/notification/builder/PhabricatorNotificationBuilder.php +++ b/src/applications/notification/builder/PhabricatorNotificationBuilder.php @@ -160,15 +160,6 @@ final class PhabricatorNotificationBuilder extends Phobject { 'href' => $story->getURI(), 'icon' => $story->getImageURI(), ); - } else if ($story instanceof PhabricatorNotificationTestFeedStory) { - $dict[] = array( - 'showAnyNotification' => $web_ready, - 'showDesktopNotification' => $desktop_ready, - 'title' => pht('Test Notification'), - 'body' => $story->renderText(), - 'href' => null, - 'icon' => PhabricatorUser::getDefaultProfileImageURI(), - ); } else { $dict[] = array( 'showWebNotification' => false, diff --git a/src/applications/notification/controller/PhabricatorNotificationTestController.php b/src/applications/notification/controller/PhabricatorNotificationTestController.php index 5c76e53357..f49f9d31a1 100644 --- a/src/applications/notification/controller/PhabricatorNotificationTestController.php +++ b/src/applications/notification/controller/PhabricatorNotificationTestController.php @@ -6,34 +6,31 @@ final class PhabricatorNotificationTestController public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); - $story_type = 'PhabricatorNotificationTestFeedStory'; - $story_data = array( - 'title' => pht( - 'This is a test notification, sent at %s.', - phabricator_datetime(time(), $viewer)), - ); - - $viewer_phid = $viewer->getPHID(); - - // NOTE: Because we don't currently show you your own notifications, make - // sure this comes from a different PHID. - $application_phid = id(new PhabricatorNotificationsApplication()) - ->getPHID(); - - // TODO: When it's easier to get these buttons to render as forms, this - // would be slightly nicer as a more standard isFormPost() check. - if ($request->validateCSRF()) { - id(new PhabricatorFeedStoryPublisher()) - ->setStoryType($story_type) - ->setStoryData($story_data) - ->setStoryTime(time()) - ->setStoryAuthorPHID($application_phid) - ->setRelatedPHIDs(array($viewer_phid)) - ->setPrimaryObjectPHID($viewer_phid) - ->setSubscribedPHIDs(array($viewer_phid)) - ->setNotifyAuthor(true) - ->publish(); + $message_text = pht( + 'This is a test notification, sent at %s.', + phabricator_datetime(time(), $viewer)); + + // NOTE: Currently, the FeedStoryPublisher explicitly filters out + // notifications about your own actions. Send this notification from + // a different actor to get around this. + $application_phid = id(new PhabricatorNotificationsApplication()) + ->getPHID(); + + $xactions = array(); + + $xactions[] = id(new PhabricatorUserTransaction()) + ->setTransactionType( + PhabricatorUserNotifyTransaction::TRANSACTIONTYPE) + ->setNewValue($message_text) + ->setForceNotifyPHIDs(array($viewer->getPHID())); + + $editor = id(new PhabricatorUserTransactionEditor()) + ->setActor($viewer) + ->setActingAsPHID($application_phid) + ->setContentSourceFromRequest($request); + + $editor->applyTransactions($viewer, $xactions); } return id(new AphrontAjaxResponse()); diff --git a/src/applications/notification/feed/PhabricatorNotificationTestFeedStory.php b/src/applications/notification/feed/PhabricatorNotificationTestFeedStory.php deleted file mode 100644 index ad984431ba..0000000000 --- a/src/applications/notification/feed/PhabricatorNotificationTestFeedStory.php +++ /dev/null @@ -1,27 +0,0 @@ -getAuthorPHID(); - } - - public function renderView() { - $data = $this->getStoryData(); - - $author_phid = $data->getAuthorPHID(); - - $view = $this->newStoryView(); - - $view->setTitle($data->getValue('title')); - $view->setImage($this->getHandle($author_phid)->getImageURI()); - - return $view; - } - - public function renderText() { - $data = $this->getStoryData(); - return $data->getValue('title'); - } - -} diff --git a/src/applications/notification/query/PhabricatorNotificationQuery.php b/src/applications/notification/query/PhabricatorNotificationQuery.php index b40bdee066..0063c06e45 100644 --- a/src/applications/notification/query/PhabricatorNotificationQuery.php +++ b/src/applications/notification/query/PhabricatorNotificationQuery.php @@ -53,13 +53,13 @@ final class PhabricatorNotificationQuery $data = queryfx_all( $conn, - 'SELECT story.*, notif.hasViewed FROM %T notif - JOIN %T story ON notif.chronologicalKey = story.chronologicalKey + 'SELECT story.*, notif.hasViewed FROM %R notif + JOIN %R story ON notif.chronologicalKey = story.chronologicalKey %Q ORDER BY notif.chronologicalKey DESC %Q', - $notification_table->getTableName(), - $story_table->getTableName(), + $notification_table, + $story_table, $this->buildWhereClause($conn), $this->buildLimitClause($conn)); @@ -93,7 +93,7 @@ final class PhabricatorNotificationQuery (int)!$this->unread); } - if ($this->keys) { + if ($this->keys !== null) { $where[] = qsprintf( $conn, 'notif.chronologicalKey IN (%Ls)', @@ -103,6 +103,16 @@ final class PhabricatorNotificationQuery return $where; } + protected function willFilterPage(array $stories) { + foreach ($stories as $key => $story) { + if (!$story->isVisibleInNotifications()) { + unset($stories[$key]); + } + } + + return $stories; + } + protected function getResultCursor($item) { return $item->getChronologicalKey(); } diff --git a/src/applications/people/controller/PhabricatorPeopleApproveController.php b/src/applications/people/controller/PhabricatorPeopleApproveController.php index 0e97ad6ee6..013f4371f6 100644 --- a/src/applications/people/controller/PhabricatorPeopleApproveController.php +++ b/src/applications/people/controller/PhabricatorPeopleApproveController.php @@ -24,30 +24,18 @@ final class PhabricatorPeopleApproveController } if ($request->isFormPost()) { - id(new PhabricatorUserEditor()) + $xactions = array(); + + $xactions[] = id(new PhabricatorUserTransaction()) + ->setTransactionType(PhabricatorUserApproveTransaction::TRANSACTIONTYPE) + ->setNewValue(true); + + id(new PhabricatorUserTransactionEditor()) ->setActor($viewer) - ->approveUser($user, true); - - $title = pht( - 'Phabricator Account "%s" Approved', - $user->getUsername()); - - $body = sprintf( - "%s\n\n %s\n\n", - pht( - 'Your Phabricator account (%s) has been approved by %s. You can '. - 'login here:', - $user->getUsername(), - $viewer->getUsername()), - PhabricatorEnv::getProductionURI('/')); - - $mail = id(new PhabricatorMetaMTAMail()) - ->addTos(array($user->getPHID())) - ->addCCs(array($viewer->getPHID())) - ->setSubject('[Phabricator] '.$title) - ->setForceDelivery(true) - ->setBody($body) - ->saveAndSend(); + ->setContentSourceFromRequest($request) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->applyTransactions($user, $xactions); return id(new AphrontRedirectResponse())->setURI($done_uri); } diff --git a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php index 9759a375c7..046726f39e 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php @@ -36,12 +36,18 @@ final class PhabricatorPeopleProfileManageController $crumbs->addTextCrumb(pht('Manage')); $crumbs->setBorder(true); + $timeline = $this->buildTransactionTimeline( + $user, + new PhabricatorPeopleTransactionQuery()); + $timeline->setShouldTerminate(true); + $manage = id(new PHUITwoColumnView()) ->setHeader($header) ->addClass('project-view-home') ->addClass('project-view-people-home') ->setCurtain($curtain) - ->addPropertySection(pht('Details'), $properties); + ->addPropertySection(pht('Details'), $properties) + ->setMainColumn($timeline); return $this->newPage() ->setTitle( @@ -51,10 +57,7 @@ final class PhabricatorPeopleProfileManageController )) ->setNavigation($nav) ->setCrumbs($crumbs) - ->appendChild( - array( - $manage, - )); + ->appendChild($manage); } private function buildPropertyView(PhabricatorUser $user) { diff --git a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php index 5dce872c4b..0c32075932 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php @@ -74,6 +74,10 @@ final class PhabricatorPeopleProfileViewController ->setTitle($user->getUsername()) ->setNavigation($nav) ->setCrumbs($crumbs) + ->setPageObjectPHIDs( + array( + $user->getPHID(), + )) ->appendChild( array( $home, diff --git a/src/applications/people/controller/PhabricatorPeopleRenameController.php b/src/applications/people/controller/PhabricatorPeopleRenameController.php index cd552717e0..42ff2e7988 100644 --- a/src/applications/people/controller/PhabricatorPeopleRenameController.php +++ b/src/applications/people/controller/PhabricatorPeopleRenameController.php @@ -22,36 +22,29 @@ final class PhabricatorPeopleRenameController $request, $done_uri); - $errors = array(); - - $v_username = $user->getUsername(); - $e_username = true; + $validation_exception = null; + $username = $user->getUsername(); if ($request->isFormPost()) { - $v_username = $request->getStr('username'); + $username = $request->getStr('username'); + $xactions = array(); - if (!strlen($v_username)) { - $e_username = pht('Required'); - $errors[] = pht('New username is required.'); - } else if ($v_username == $user->getUsername()) { - $e_username = pht('Invalid'); - $errors[] = pht('New username must be different from old username.'); - } else if (!PhabricatorUser::validateUsername($v_username)) { - $e_username = pht('Invalid'); - $errors[] = PhabricatorUser::describeValidUsername(); + $xactions[] = id(new PhabricatorUserTransaction()) + ->setTransactionType( + PhabricatorUserUsernameTransaction::TRANSACTIONTYPE) + ->setNewValue($username); + + $editor = id(new PhabricatorUserTransactionEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnMissingFields(true); + + try { + $editor->applyTransactions($user, $xactions); + return id(new AphrontRedirectResponse())->setURI($done_uri); + } catch (PhabricatorApplicationTransactionValidationException $ex) { + $validation_exception = $ex; } - if (!$errors) { - try { - id(new PhabricatorUserEditor()) - ->setActor($viewer) - ->changeUsername($user, $v_username); - - return id(new AphrontRedirectResponse())->setURI($done_uri); - } catch (AphrontDuplicateKeyQueryException $ex) { - $e_username = pht('Not Unique'); - $errors[] = pht('Another user already has that username.'); - } - } } $inst1 = pht( @@ -87,18 +80,13 @@ final class PhabricatorPeopleRenameController ->appendChild( id(new AphrontFormTextControl()) ->setLabel(pht('New Username')) - ->setValue($v_username) - ->setName('username') - ->setError($e_username)); - - if ($errors) { - $errors = id(new PHUIInfoView())->setErrors($errors); - } + ->setValue($username) + ->setName('username')); return $this->newDialog() ->setWidth(AphrontDialogView::WIDTH_FORM) ->setTitle(pht('Change Username')) - ->appendChild($errors) + ->setValidationException($validation_exception) ->appendParagraph($inst1) ->appendParagraph($inst2) ->appendParagraph($inst3) diff --git a/src/applications/people/editor/PhabricatorUserEditEngine.php b/src/applications/people/editor/PhabricatorUserEditEngine.php index c4c1abf1e3..4b3a60a2fc 100644 --- a/src/applications/people/editor/PhabricatorUserEditEngine.php +++ b/src/applications/people/editor/PhabricatorUserEditEngine.php @@ -75,6 +75,16 @@ final class PhabricatorUserEditEngine ->setConduitDescription(pht('Disable or enable the user.')) ->setConduitTypeDescription(pht('True to disable the user.')) ->setValue($object->getIsDisabled()), + id(new PhabricatorBoolEditField()) + ->setKey('approved') + ->setOptions(pht('Approved'), pht('Unapproved')) + ->setLabel(pht('Approved')) + ->setDescription(pht('Approve the user.')) + ->setTransactionType(PhabricatorUserApproveTransaction::TRANSACTIONTYPE) + ->setIsFormField(false) + ->setConduitDescription(pht('Approve or reject the user.')) + ->setConduitTypeDescription(pht('True to approve the user.')) + ->setValue($object->getIsApproved()), ); } diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php index 58c2ba1ff1..8092824a0c 100644 --- a/src/applications/people/editor/PhabricatorUserEditor.php +++ b/src/applications/people/editor/PhabricatorUserEditor.php @@ -129,53 +129,6 @@ final class PhabricatorUserEditor extends PhabricatorEditor { } - /** - * @task edit - */ - public function changeUsername(PhabricatorUser $user, $username) { - $actor = $this->requireActor(); - - if (!$user->getID()) { - throw new Exception(pht('User has not been created yet!')); - } - - if (!PhabricatorUser::validateUsername($username)) { - $valid = PhabricatorUser::describeValidUsername(); - throw new Exception(pht('Username is invalid! %s', $valid)); - } - - $old_username = $user->getUsername(); - - $user->openTransaction(); - $user->reload(); - $user->setUsername($username); - - try { - $user->save(); - } catch (AphrontDuplicateKeyQueryException $ex) { - $user->setUsername($old_username); - $user->killTransaction(); - throw $ex; - } - - $log = PhabricatorUserLog::initializeNewLog( - $actor, - $user->getPHID(), - PhabricatorUserLog::ACTION_CHANGE_USERNAME); - $log->setOldValue($old_username); - $log->setNewValue($username); - $log->save(); - - $user->saveTransaction(); - - // The SSH key cache currently includes usernames, so dirty it. See T12554 - // for discussion. - PhabricatorAuthSSHKeyQuery::deleteSSHKeyCache(); - - $user->sendUsernameChangeEmail($actor, $old_username); - } - - /* -( Editing Roles )------------------------------------------------------ */ @@ -293,45 +246,6 @@ final class PhabricatorUserEditor extends PhabricatorEditor { return $this; } - /** - * @task role - */ - public function approveUser(PhabricatorUser $user, $approve) { - $actor = $this->requireActor(); - - if (!$user->getID()) { - throw new Exception(pht('User has not been created yet!')); - } - - $user->openTransaction(); - $user->beginWriteLocking(); - - $user->reload(); - if ($user->getIsApproved() == $approve) { - $user->endWriteLocking(); - $user->killTransaction(); - return $this; - } - - $log = PhabricatorUserLog::initializeNewLog( - $actor, - $user->getPHID(), - PhabricatorUserLog::ACTION_APPROVE); - $log->setOldValue($user->getIsApproved()); - $log->setNewValue($approve); - - $user->setIsApproved($approve); - $user->save(); - - $log->save(); - - $user->endWriteLocking(); - $user->saveTransaction(); - - return $this; - } - - /* -( Adding, Removing and Changing Email )-------------------------------- */ diff --git a/src/applications/people/editor/PhabricatorUserTransactionEditor.php b/src/applications/people/editor/PhabricatorUserTransactionEditor.php index c0dfa941af..929b2e224c 100644 --- a/src/applications/people/editor/PhabricatorUserTransactionEditor.php +++ b/src/applications/people/editor/PhabricatorUserTransactionEditor.php @@ -11,4 +11,18 @@ final class PhabricatorUserTransactionEditor return pht('Users'); } + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { + return true; + } + + protected function getMailTo(PhabricatorLiskDAO $object) { + return array(); + } + + protected function getMailCC(PhabricatorLiskDAO $object) { + return array(); + } + } diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php index 131dc7c588..542b685e29 100644 --- a/src/applications/people/query/PhabricatorPeopleQuery.php +++ b/src/applications/people/query/PhabricatorPeopleQuery.php @@ -163,14 +163,7 @@ final class PhabricatorPeopleQuery } protected function loadPage() { - $table = new PhabricatorUser(); - $data = $this->loadStandardPageRows($table); - - if ($this->needPrimaryEmail) { - $table->putInSet(new LiskDAOSet()); - } - - return $table->loadAllFromArray($data); + return $this->loadStandardPage($this->newResultObject()); } protected function didFilterPage(array $users) { diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 7717fbf18c..5d310378e0 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -458,14 +458,9 @@ final class PhabricatorUser } public function loadPrimaryEmail() { - $email = new PhabricatorUserEmail(); - $conn = $email->establishConnection('r'); - - return $this->loadOneRelative( - $email, - 'userPHID', - 'getPHID', - qsprintf($conn, '(isPrimary = 1)')); + return id(new PhabricatorUserEmail())->loadOneWhere( + 'userPHID = %s AND isPrimary = 1', + $this->getPHID()); } diff --git a/src/applications/people/storage/PhabricatorUserLog.php b/src/applications/people/storage/PhabricatorUserLog.php index 82819f8f5b..12cb4cb626 100644 --- a/src/applications/people/storage/PhabricatorUserLog.php +++ b/src/applications/people/storage/PhabricatorUserLog.php @@ -150,7 +150,7 @@ final class PhabricatorUserLog extends PhabricatorUserDAO 'actorPHID' => 'phid?', 'action' => 'text64', 'remoteAddr' => 'text64', - 'session' => 'bytes40?', + 'session' => 'text64?', ), self::CONFIG_KEY_SCHEMA => array( 'actorPHID' => array( diff --git a/src/applications/people/xaction/PhabricatorUserApproveTransaction.php b/src/applications/people/xaction/PhabricatorUserApproveTransaction.php new file mode 100644 index 0000000000..e458c5822c --- /dev/null +++ b/src/applications/people/xaction/PhabricatorUserApproveTransaction.php @@ -0,0 +1,96 @@ +getIsApproved(); + } + + public function generateNewValue($object, $value) { + return (bool)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setIsApproved((int)$value); + } + + public function applyExternalEffects($object, $value) { + $user = $object; + $this->newUserLog(PhabricatorUserLog::ACTION_APPROVE) + ->setOldValue((bool)$user->getIsApproved()) + ->setNewValue((bool)$value) + ->save(); + + $actor = $this->getActor(); + $title = pht( + 'Phabricator Account "%s" Approved', + $user->getUsername()); + + $body = sprintf( + "%s\n\n %s\n\n", + pht( + 'Your Phabricator account (%s) has been approved by %s. You can '. + 'login here:', + $user->getUsername(), + $actor->getUsername()), + PhabricatorEnv::getProductionURI('/')); + + $mail = id(new PhabricatorMetaMTAMail()) + ->addTos(array($user->getPHID())) + ->addCCs(array($actor->getPHID())) + ->setSubject('[Phabricator] '.$title) + ->setForceDelivery(true) + ->setBody($body) + ->saveAndSend(); + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s approved this user.', + $this->renderAuthor()); + } else { + return pht( + '%s rejected this user.', + $this->renderAuthor()); + } + } + + public function shouldHideForFeed() { + return true; + } + + public function validateTransactions($object, array $xactions) { + $actor = $this->getActor(); + $errors = array(); + + foreach ($xactions as $xaction) { + $is_approved = (bool)$object->getIsApproved(); + + if ((bool)$xaction->getNewValue() === $is_approved) { + continue; + } + + if (!$actor->getIsAdmin()) { + $errors[] = $this->newInvalidError( + pht('You must be an administrator to approve users.')); + } + } + + return $errors; + } + + public function getRequiredCapabilities( + $object, + PhabricatorApplicationTransaction $xaction) { + + // Unlike normal user edits, approvals require admin permissions, which + // is enforced by validateTransactions(). + + return null; + } +} diff --git a/src/applications/people/xaction/PhabricatorUserDisableTransaction.php b/src/applications/people/xaction/PhabricatorUserDisableTransaction.php index ab399a28e0..7a8a1c7966 100644 --- a/src/applications/people/xaction/PhabricatorUserDisableTransaction.php +++ b/src/applications/people/xaction/PhabricatorUserDisableTransaction.php @@ -15,7 +15,9 @@ final class PhabricatorUserDisableTransaction public function applyInternalEffects($object, $value) { $object->setIsDisabled((int)$value); + } + public function applyExternalEffects($object, $value) { $this->newUserLog(PhabricatorUserLog::ACTION_DISABLE) ->setOldValue((bool)$object->getIsDisabled()) ->setNewValue((bool)$value) @@ -35,19 +37,10 @@ final class PhabricatorUserDisableTransaction } } - public function getTitleForFeed() { - $new = $this->getNewValue(); - if ($new) { - return pht( - '%s disabled %s.', - $this->renderAuthor(), - $this->renderObject()); - } else { - return pht( - '%s enabled %s.', - $this->renderAuthor(), - $this->renderObject()); - } + public function shouldHideForFeed() { + // Don't publish feed stories about disabling users, since this can be + // a sensitive action. + return true; } public function validateTransactions($object, array $xactions) { diff --git a/src/applications/people/xaction/PhabricatorUserNotifyTransaction.php b/src/applications/people/xaction/PhabricatorUserNotifyTransaction.php new file mode 100644 index 0000000000..4a527f8265 --- /dev/null +++ b/src/applications/people/xaction/PhabricatorUserNotifyTransaction.php @@ -0,0 +1,38 @@ +renderAuthor()); + } + + public function getTitleForFeed() { + return $this->getNewValue(); + } + + public function shouldHideForNotifications() { + return false; + } + + public function shouldHideForFeed() { + return true; + } + + public function shouldHideForMail() { + return true; + } + +} diff --git a/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php b/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php new file mode 100644 index 0000000000..db134a5c78 --- /dev/null +++ b/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php @@ -0,0 +1,92 @@ +getUsername(); + } + + public function generateNewValue($object, $value) { + return $value; + } + + public function applyInternalEffects($object, $value) { + $object->setUsername($value); + } + + public function applyExternalEffects($object, $value) { + $user = $object; + + $this->newUserLog(PhabricatorUserLog::ACTION_CHANGE_USERNAME) + ->setOldValue($this->getOldValue()) + ->setNewValue($value) + ->save(); + + // The SSH key cache currently includes usernames, so dirty it. See T12554 + // for discussion. + PhabricatorAuthSSHKeyQuery::deleteSSHKeyCache(); + + $user->sendUsernameChangeEmail($this->getActor(), $this->getOldValue()); + } + + public function getTitle() { + return pht( + '%s renamed this user from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function validateTransactions($object, array $xactions) { + $actor = $this->getActor(); + $errors = array(); + + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + $old = $xaction->getOldValue(); + + if ($old === $new) { + continue; + } + + if (!$actor->getIsAdmin()) { + $errors[] = $this->newInvalidError( + pht('You must be an administrator to rename users.')); + } + + if (!strlen($new)) { + $errors[] = $this->newRequiredError( + pht('New username is required.'), $xaction); + } else if (!PhabricatorUser::validateUsername($new)) { + $errors[] = $this->newInvalidError( + PhabricatorUser::describeValidUsername(), $xaction); + } + + $user = id(new PhabricatorPeopleQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withUsernames(array($new)) + ->executeOne(); + + if ($user) { + $errors[] = $this->newInvalidError( + pht('Another user already has that username.'), $xaction); + } + + } + + return $errors; + } + + public function getRequiredCapabilities( + $object, + PhabricatorApplicationTransaction $xaction) { + + // Unlike normal user edits, renames require admin permissions, which + // is enforced by validateTransactions(). + + return null; + } +} diff --git a/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php b/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php index 9d97ed7f8c..d1e400a485 100644 --- a/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php +++ b/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php @@ -41,12 +41,19 @@ final class PhrictionEditConduitAPIMethod extends PhrictionConduitAPIMethod { } $xactions = array(); - $xactions[] = id(new PhrictionTransaction()) - ->setTransactionType(PhrictionDocumentTitleTransaction::TRANSACTIONTYPE) - ->setNewValue($request->getValue('title')); - $xactions[] = id(new PhrictionTransaction()) - ->setTransactionType(PhrictionDocumentContentTransaction::TRANSACTIONTYPE) - ->setNewValue($request->getValue('content')); + if ($request->getValue('title')) { + $xactions[] = id(new PhrictionTransaction()) + ->setTransactionType( + PhrictionDocumentTitleTransaction::TRANSACTIONTYPE) + ->setNewValue($request->getValue('title')); + } + + if ($request->getValue('content')) { + $xactions[] = id(new PhrictionTransaction()) + ->setTransactionType( + PhrictionDocumentContentTransaction::TRANSACTIONTYPE) + ->setNewValue($request->getValue('content')); + } $editor = id(new PhrictionTransactionEditor()) ->setActor($request->getUser()) diff --git a/src/applications/phriction/controller/PhrictionController.php b/src/applications/phriction/controller/PhrictionController.php index 51ffbcab1f..06f0102526 100644 --- a/src/applications/phriction/controller/PhrictionController.php +++ b/src/applications/phriction/controller/PhrictionController.php @@ -2,6 +2,14 @@ abstract class PhrictionController extends PhabricatorController { + private $showingWelcomeDocument = false; + + public function setShowingWelcomeDocument($show_welcome) { + $this->showingWelcomeDocument = $show_welcome; + return $this; + + } + public function buildSideNavView($for_app = false) { $user = $this->getRequest()->getUser(); @@ -37,12 +45,14 @@ abstract class PhrictionController extends PhabricatorController { ->setIcon('fa-home')); } - $crumbs->addAction( - id(new PHUIListItemView()) - ->setName(pht('New Document')) - ->setHref('/phriction/new/?slug='.$this->getDocumentSlug()) - ->setWorkflow(true) - ->setIcon('fa-plus-square')); + if (!$this->showingWelcomeDocument) { + $crumbs->addAction( + id(new PHUIListItemView()) + ->setName(pht('New Document')) + ->setHref('/phriction/new/?slug='.$this->getDocumentSlug()) + ->setWorkflow(true) + ->setIcon('fa-plus-square')); + } return $crumbs; } diff --git a/src/applications/phriction/controller/PhrictionDocumentController.php b/src/applications/phriction/controller/PhrictionDocumentController.php index 8da9807064..bf7282b35a 100644 --- a/src/applications/phriction/controller/PhrictionDocumentController.php +++ b/src/applications/phriction/controller/PhrictionDocumentController.php @@ -9,6 +9,7 @@ final class PhrictionDocumentController return true; } + public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); $this->slug = $request->getURIData('slug'); @@ -35,15 +36,15 @@ final class PhrictionDocumentController ->needContent(true) ->executeOne(); if (!$document) { - $document = PhrictionDocument::initializeNewDocument($viewer, $slug); - if ($slug == '/') { $title = pht('Welcome to Phriction'); $subtitle = pht('Phriction is a simple and easy to use wiki for '. 'keeping track of documents and their changes.'); $page_title = pht('Welcome'); $create_text = pht('Edit this Document'); + $this->setShowingWelcomeDocument(true); + } else { $title = pht('No Document Here'); diff --git a/src/applications/releeph/conduit/ReleephGetBranchesConduitAPIMethod.php b/src/applications/releeph/conduit/ReleephGetBranchesConduitAPIMethod.php index 6e225ec8e3..5516b4edb6 100644 --- a/src/applications/releeph/conduit/ReleephGetBranchesConduitAPIMethod.php +++ b/src/applications/releeph/conduit/ReleephGetBranchesConduitAPIMethod.php @@ -30,19 +30,16 @@ final class ReleephGetBranchesConduitAPIMethod extends ReleephConduitAPIMethod { foreach ($projects as $project) { $repository = $project->getRepository(); - $branches = $project->loadRelatives( - id(new ReleephBranch()), - 'releephProjectID', - 'getID', - 'isActive = 1'); + $branches = id(new ReleephBranch())->loadAllWhere( + 'releephProjectID = %d AND isActive = 1', + $project->getID()); foreach ($branches as $branch) { $full_branch_name = $branch->getName(); - $cut_point_commit = $branch->loadOneRelative( - id(new PhabricatorRepositoryCommit()), - 'phid', - 'getCutPointCommitPHID'); + $cut_point_commit = id(new PhabricatorRepositoryCommit())->loadOneWhere( + 'phid = %s', + $branch->getCutPointCommitPHID()); $results[] = array( 'project' => $project->getName(), diff --git a/src/applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php b/src/applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php index 5975f208e6..20c8d5e226 100644 --- a/src/applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php @@ -22,16 +22,17 @@ final class ReleephDiffSizeFieldSpecification } $diff_rev = $requested_object; - $diffs = $diff_rev->loadRelatives( - new DifferentialDiff(), - 'revisionID', - 'getID', - 'creationMethod <> "commit"'); + $diffs = id(new DifferentialDiff())->loadAllWhere( + 'revisionID = %d AND creationMethod != %s', + $diff_rev->getID(), + 'commit'); $all_changesets = array(); $most_recent_changesets = null; foreach ($diffs as $diff) { - $changesets = $diff->loadRelatives(new DifferentialChangeset(), 'diffID'); + $changesets = id(new DifferentialChangeset())->loadAllWhere( + 'diffID = %d', + $diff->getID()); $all_changesets += $changesets; $most_recent_changesets = $changesets; } diff --git a/src/applications/releeph/storage/ReleephRequest.php b/src/applications/releeph/storage/ReleephRequest.php index c071600ec7..a877192107 100644 --- a/src/applications/releeph/storage/ReleephRequest.php +++ b/src/applications/releeph/storage/ReleephRequest.php @@ -257,18 +257,17 @@ final class ReleephRequest extends ReleephDAO /* -( Loading external objects )------------------------------------------- */ public function loadPhabricatorRepositoryCommit() { - return $this->loadOneRelative( - new PhabricatorRepositoryCommit(), - 'phid', - 'getRequestCommitPHID'); + return id(new PhabricatorRepositoryCommit())->loadOneWhere( + 'phid = %s', + $this->getRequestCommitPHID()); } public function loadPhabricatorRepositoryCommitData() { $commit = $this->loadPhabricatorRepositoryCommit(); if ($commit) { - return $commit->loadOneRelative( - new PhabricatorRepositoryCommitData(), - 'commitID'); + return id(new PhabricatorRepositoryCommitData())->loadOneWhere( + 'commitID = %d', + $commit->getID()); } } diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php index 3743045c34..7ca725de81 100644 --- a/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php @@ -15,11 +15,11 @@ final class PhabricatorRepositoryManagementThawWorkflow array( array( 'name' => 'demote', - 'param' => 'device/service', + 'param' => 'device|service', 'help' => pht( 'Demote a device (or all devices in a service) discarding '. - 'local changes. Clears stuck write locks and recovers from '. - 'lost leaders.'), + 'unsynchronized changes. Clears stuck write locks and recovers '. + 'from lost leaders.'), ), array( 'name' => 'promote', diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index c961e06f27..12c2b9e8a3 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -239,20 +239,6 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return idx($this->details, $key, $default); } - public function getHumanReadableDetail($key, $default = null) { - $value = $this->getDetail($key, $default); - - switch ($key) { - case 'branch-filter': - case 'close-commits-filter': - $value = array_keys($value); - $value = implode(', ', $value); - break; - } - - return $value; - } - public function setDetail($key, $value) { $this->details[$key] = $value; return $this; @@ -1202,6 +1188,26 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return null; } + public function getAutocloseOnlyRules() { + return array_keys($this->getDetail('close-commits-filter', array())); + } + + public function setAutocloseOnlyRules(array $rules) { + $rules = array_fill_keys($rules, true); + $this->setDetail('close-commits-filter', $rules); + return $this; + } + + public function getTrackOnlyRules() { + return array_keys($this->getDetail('branch-filter', array())); + } + + public function setTrackOnlyRules(array $rules) { + $rules = array_fill_keys($rules, true); + $this->setDetail('branch-filter', $rules); + return $this; + } + /* -( Repository URI Management )------------------------------------------ */ diff --git a/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php b/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php index f78c3374ed..ea83aa9d56 100644 --- a/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php +++ b/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php @@ -37,11 +37,7 @@ final class PhabricatorRepositoryTestCase $repo->shouldTrackBranch('imaginary'), pht('Track all branches by default.')); - $repo->setDetail( - 'branch-filter', - array( - 'master' => true, - )); + $repo->setTrackOnlyRules(array('master')); $this->assertTrue( $repo->shouldTrackBranch('master'), diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php index b544228561..f5d3af7fd8 100644 --- a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php +++ b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php @@ -114,40 +114,36 @@ abstract class PhabricatorRepositoryCommitChangeParserWorker PhabricatorRepositoryCommit $commit, array $changes) { + $conn = $repository->establishConnection('w'); + $repository_id = (int)$repository->getID(); $commit_id = (int)$commit->getID(); - // NOTE: This SQL is being built manually instead of with qsprintf() - // because some SVN changes affect an enormous number of paths (millions) - // and this showed up as significantly slow on a profile at some point. - $changes_sql = array(); foreach ($changes as $change) { - $values = array( + $changes_sql[] = qsprintf( + $conn, + '(%d, %d, %d, %nd, %nd, %d, %d, %d, %d)', $repository_id, (int)$change->getPathID(), $commit_id, - nonempty((int)$change->getTargetPathID(), 'null'), - nonempty((int)$change->getTargetCommitID(), 'null'), + nonempty((int)$change->getTargetPathID(), null), + nonempty((int)$change->getTargetCommitID(), null), (int)$change->getChangeType(), (int)$change->getFileType(), (int)$change->getIsDirect(), - (int)$change->getCommitSequence(), - ); - $changes_sql[] = '('.implode(', ', $values).')'; + (int)$change->getCommitSequence()); } - $conn_w = $repository->establishConnection('w'); - queryfx( - $conn_w, + $conn, 'DELETE FROM %T WHERE commitID = %d', PhabricatorRepository::TABLE_PATHCHANGE, $commit_id); foreach (PhabricatorLiskDAO::chunkSQL($changes_sql) as $chunk) { queryfx( - $conn_w, + $conn, 'INSERT INTO %T (repositoryID, pathID, commitID, targetPathID, targetCommitID, changeType, fileType, isDirect, commitSequence) diff --git a/src/applications/repository/xaction/PhabricatorRepositoryAutocloseOnlyTransaction.php b/src/applications/repository/xaction/PhabricatorRepositoryAutocloseOnlyTransaction.php index feacc73c44..5ff17c4b05 100644 --- a/src/applications/repository/xaction/PhabricatorRepositoryAutocloseOnlyTransaction.php +++ b/src/applications/repository/xaction/PhabricatorRepositoryAutocloseOnlyTransaction.php @@ -6,11 +6,11 @@ final class PhabricatorRepositoryAutocloseOnlyTransaction const TRANSACTIONTYPE = 'repo:autoclose-only'; public function generateOldValue($object) { - return array_keys($object->getDetail('close-commits-filter', array())); + return $object->getAutocloseOnlyRules(); } public function applyInternalEffects($object, $value) { - $object->setDetail('close-commits-filter', array_fill_keys($value, true)); + $object->setAutocloseOnlyRules($value); } public function getTitle() { diff --git a/src/applications/repository/xaction/PhabricatorRepositoryTrackOnlyTransaction.php b/src/applications/repository/xaction/PhabricatorRepositoryTrackOnlyTransaction.php index 20ff1baf08..5fdc26e792 100644 --- a/src/applications/repository/xaction/PhabricatorRepositoryTrackOnlyTransaction.php +++ b/src/applications/repository/xaction/PhabricatorRepositoryTrackOnlyTransaction.php @@ -6,11 +6,11 @@ final class PhabricatorRepositoryTrackOnlyTransaction const TRANSACTIONTYPE = 'repo:track-only'; public function generateOldValue($object) { - return array_keys($object->getDetail('branch-filter', array())); + return $object->getTrackOnlyRules(); } public function applyInternalEffects($object, $value) { - $object->setDetail('branch-filter', array_fill_keys($value, true)); + $object->setTrackOnlyRules($value); } public function getTitle() { diff --git a/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php b/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php index 2e2e174e0f..51f1747b9f 100644 --- a/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php +++ b/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php @@ -63,7 +63,7 @@ final class PhabricatorSettingsTimezoneController $server_offset = $viewer->getTimeZoneOffset(); - if ($client_offset == $server_offset || $did_calibrate) { + if (($client_offset == $server_offset) || $did_calibrate) { return $this->newDialog() ->setTitle(pht('Timezone Calibrated')) ->appendParagraph( @@ -113,12 +113,13 @@ final class PhabricatorSettingsTimezoneController } private function formatOffset($offset) { - $offset = $offset / 60; - - if ($offset >= 0) { - return pht('UTC-%d', $offset); + $hours = $offset / 60; + // Non-integer number of hours off UTC? + if ($offset % 60) { + $minutes = abs($offset % 60); + return pht('UTC%+d:%02d', $hours, $minutes); } else { - return pht('UTC+%d', -$offset); + return pht('UTC%+d', $hours); } } diff --git a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php index ae653e0f70..5fada0bbed 100644 --- a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php @@ -193,7 +193,8 @@ final class PhabricatorMultiFactorSettingsPanel id(new PhabricatorAuthSessionEngine())->terminateLoginSessions( $user, - $request->getCookie(PhabricatorCookies::COOKIE_SESSION)); + new PhutilOpaqueEnvelope( + $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); return id(new AphrontRedirectResponse()) ->setURI($this->getPanelURI('?id='.$config->getID())); diff --git a/src/applications/settings/panel/PhabricatorNotificationsSettingsPanel.php b/src/applications/settings/panel/PhabricatorNotificationsSettingsPanel.php index e75c2b99ee..797bcafcb3 100644 --- a/src/applications/settings/panel/PhabricatorNotificationsSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorNotificationsSettingsPanel.php @@ -143,7 +143,7 @@ final class PhabricatorNotificationsSettingsPanel ->setCaption( pht( 'Phabricator can send real-time notifications to your web browser '. - 'or to your desktop. Select where you\'d want to receive these '. + 'or to your desktop. Select where you want to receive these '. 'real-time updates.')) ->initBehavior( 'desktop-notifications-control', diff --git a/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php b/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php index 9fb8838cf7..79d7610f2f 100644 --- a/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php @@ -121,7 +121,8 @@ final class PhabricatorPasswordSettingsPanel extends PhabricatorSettingsPanel { id(new PhabricatorAuthSessionEngine())->terminateLoginSessions( $user, - $request->getCookie(PhabricatorCookies::COOKIE_SESSION)); + new PhutilOpaqueEnvelope( + $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); return id(new AphrontRedirectResponse())->setURI($next); } diff --git a/src/applications/settings/panel/PhabricatorSessionsSettingsPanel.php b/src/applications/settings/panel/PhabricatorSessionsSettingsPanel.php index eab18002a1..314d68f69d 100644 --- a/src/applications/settings/panel/PhabricatorSessionsSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorSessionsSettingsPanel.php @@ -44,8 +44,9 @@ final class PhabricatorSessionsSettingsPanel extends PhabricatorSettingsPanel { ->withPHIDs($identity_phids) ->execute(); - $current_key = PhabricatorHash::weakDigest( - $request->getCookie(PhabricatorCookies::COOKIE_SESSION)); + $current_key = PhabricatorAuthSession::newSessionDigest( + new PhutilOpaqueEnvelope( + $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); $rows = array(); $rowc = array(); diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 4e189df164..86cc014102 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -358,7 +358,7 @@ abstract class PhabricatorEditEngine return $this->editEngineConfiguration; } - private function newConfigurationQuery() { + public function newConfigurationQuery() { return id(new PhabricatorEditEngineConfigurationQuery()) ->setViewer($this->getViewer()) ->withEngineKeys(array($this->getEngineKey())); diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php index 23fac106d7..9e754a3ca8 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php @@ -11,6 +11,8 @@ final class PhabricatorEditEngineSubtype private $icon; private $tagText; private $color; + private $childSubtypes = array(); + private $childIdentifiers = array(); public function setKey($key) { $this->key = $key; @@ -57,6 +59,24 @@ final class PhabricatorEditEngineSubtype return $this->color; } + public function setChildSubtypes(array $child_subtypes) { + $this->childSubtypes = $child_subtypes; + return $this; + } + + public function getChildSubtypes() { + return $this->childSubtypes; + } + + public function setChildFormIdentifiers(array $child_identifiers) { + $this->childIdentifiers = $child_identifiers; + return $this; + } + + public function getChildFormIdentifiers() { + return $this->childIdentifiers; + } + public function hasTagView() { return (bool)strlen($this->getTagText()); } @@ -118,6 +138,7 @@ final class PhabricatorEditEngineSubtype 'tag' => 'optional string', 'color' => 'optional string', 'icon' => 'optional string', + 'children' => 'optional map', )); $key = $value['key']; @@ -141,6 +162,27 @@ final class PhabricatorEditEngineSubtype 'no name. Subtypes must have a name.', $key)); } + + $children = idx($value, 'children'); + if ($children) { + PhutilTypeSpec::checkMap( + $children, + array( + 'subtypes' => 'optional list', + 'forms' => 'optional list', + )); + + $child_subtypes = idx($children, 'subtypes'); + $child_forms = idx($children, 'forms'); + + if ($child_subtypes && $child_forms) { + throw new Exception( + pht( + 'Subtype configuration is invalid: subtype with key "%s" '. + 'specifies both child subtypes and child forms. Specify one '. + 'or the other, but not both.')); + } + } } if (!isset($map[self::SUBTYPE_DEFAULT])) { @@ -179,10 +221,27 @@ final class PhabricatorEditEngineSubtype $subtype->setColor($color); } + $children = idx($entry, 'children', array()); + $child_subtypes = idx($children, 'subtypes'); + $child_forms = idx($children, 'forms'); + + if ($child_subtypes) { + $subtype->setChildSubtypes($child_subtypes); + } + + if ($child_forms) { + $subtype->setChildFormIdentifiers($child_forms); + } + $map[$key] = $subtype; } return new PhabricatorEditEngineSubtypeMap($map); } + public function newIconView() { + return id(new PHUIIconView()) + ->setIcon($this->getIcon(), $this->getColor()); + } + } diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php index cd8e0674ae..638b665184 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php @@ -39,4 +39,45 @@ final class PhabricatorEditEngineSubtypeMap return $this->subtypes[$subtype_key]; } + public function getCreateFormsForSubtype( + PhabricatorEditEngine $edit_engine, + PhabricatorEditEngineSubtypeInterface $object) { + + $subtype_key = $object->getEditEngineSubtype(); + $subtype = $this->getSubtype($subtype_key); + + $select_identifiers = $subtype->getChildFormIdentifiers(); + $select_subtypes = $subtype->getChildSubtypes(); + + $query = $edit_engine->newConfigurationQuery() + ->withIsDisabled(false); + + if ($select_identifiers) { + $query->withIdentifiers($select_identifiers); + } else { + // If we're selecting by subtype rather than selecting specific forms, + // only select create forms. + $query->withIsDefault(true); + + if ($select_subtypes) { + $query->withSubtypes($select_subtypes); + } else { + $query->withSubtypes(array($subtype_key)); + } + } + + $forms = $query->execute(); + $forms = mpull($forms, null, 'getIdentifier'); + + // If we're selecting by ID, respect the order specified in the + // constraint. Otherwise, use the create form sort order. + if ($select_identifiers) { + $forms = array_select_keys($forms, $select_identifiers) + $forms; + } else { + $forms = msort($forms, 'getCreateSortKey'); + } + + return $forms; + } + } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index f354de6a1a..738fcf098b 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -3378,9 +3378,28 @@ abstract class PhabricatorApplicationTransactionEditor PhabricatorLiskDAO $object, array $xactions) { - return array_unique(array_merge( - $this->getMailTo($object), - $this->getMailCC($object))); + // If some transactions are forcing notification delivery, add the forced + // recipients to the notify list. + $force_list = array(); + foreach ($xactions as $xaction) { + $force_phids = $xaction->getForceNotifyPHIDs(); + + if (!$force_phids) { + continue; + } + + foreach ($force_phids as $force_phid) { + $force_list[] = $force_phid; + } + } + + $to_list = $this->getMailTo($object); + $cc_list = $this->getMailCC($object); + + $full_list = array_merge($force_list, $to_list, $cc_list); + $full_list = array_fuse($full_list); + + return array_keys($full_list); } @@ -3409,7 +3428,19 @@ abstract class PhabricatorApplicationTransactionEditor array $xactions, array $mailed_phids) { - $xactions = mfilter($xactions, 'shouldHideForFeed', true); + // Remove transactions which don't publish feed stories or notifications. + // These never show up anywhere, so we don't need to do anything with them. + foreach ($xactions as $key => $xaction) { + if (!$xaction->shouldHideForFeed()) { + continue; + } + + if (!$xaction->shouldHideForNotifications()) { + continue; + } + + unset($xactions[$key]); + } if (!$xactions) { return; @@ -4594,4 +4625,87 @@ abstract class PhabricatorApplicationTransactionEditor return $mail; } + public function newAutomaticInlineTransactions( + PhabricatorLiskDAO $object, + array $inlines, + $transaction_type, + PhabricatorCursorPagedPolicyAwareQuery $query_template) { + + $xactions = array(); + + foreach ($inlines as $inline) { + $xactions[] = $object->getApplicationTransactionTemplate() + ->setTransactionType($transaction_type) + ->attachComment($inline); + } + + $state_xaction = $this->newInlineStateTransaction( + $object, + $query_template); + + if ($state_xaction) { + $xactions[] = $state_xaction; + } + + return $xactions; + } + + protected function newInlineStateTransaction( + PhabricatorLiskDAO $object, + PhabricatorCursorPagedPolicyAwareQuery $query_template) { + + $actor_phid = $this->getActingAsPHID(); + $author_phid = $object->getAuthorPHID(); + $actor_is_author = ($actor_phid == $author_phid); + + $state_map = PhabricatorTransactions::getInlineStateMap(); + + $query = id(clone $query_template) + ->setViewer($this->getActor()) + ->withFixedStates(array_keys($state_map)); + + $inlines = array(); + + $inlines[] = id(clone $query) + ->withAuthorPHIDs(array($actor_phid)) + ->withHasTransaction(false) + ->execute(); + + if ($actor_is_author) { + $inlines[] = id(clone $query) + ->withHasTransaction(true) + ->execute(); + } + + $inlines = array_mergev($inlines); + + if (!$inlines) { + return null; + } + + $old_value = mpull($inlines, 'getFixedState', 'getPHID'); + $new_value = array(); + foreach ($old_value as $key => $state) { + $new_value[$key] = $state_map[$state]; + } + + // See PHI995. Copy some information about the inlines into the transaction + // so we can tailor rendering behavior. In particular, we don't want to + // render transactions about users marking their own inlines as "Done". + + $inline_details = array(); + foreach ($inlines as $inline) { + $inline_details[$inline->getPHID()] = array( + 'authorPHID' => $inline->getAuthorPHID(), + ); + } + + return $object->getApplicationTransactionTemplate() + ->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE) + ->setIgnoreOnNoEffect(true) + ->setMetadataValue('inline.details', $inline_details) + ->setOldValue($old_value) + ->setNewValue($new_value); + } + } diff --git a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php index 305e38ab09..0e8531a22d 100644 --- a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php +++ b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php @@ -36,12 +36,7 @@ class PhabricatorApplicationTransactionFeedStory // time because it's cheap and gets us better results when things change // by letting the changes apply retroactively. - $xaction_phids = $this->getValue('transactionPHIDs'); - - $xactions = array(); - foreach ($xaction_phids as $xaction_phid) { - $xactions[] = $this->getObject($xaction_phid); - } + $xactions = $this->getTransactions(); foreach ($xactions as $key => $xaction) { if ($xaction->shouldHideForFeed()) { @@ -52,7 +47,7 @@ class PhabricatorApplicationTransactionFeedStory if ($xactions) { $primary_phid = head($xactions)->getPHID(); } else { - $primary_phid = head($xaction_phids); + $primary_phid = head($this->getValue('transactionPHIDs')); } $this->primaryTransactionPHID = $primary_phid; @@ -61,6 +56,41 @@ class PhabricatorApplicationTransactionFeedStory return $this->primaryTransactionPHID; } + public function isVisibleInNotifications() { + $xactions = $this->getTransactions(); + + foreach ($xactions as $key => $xaction) { + if (!$xaction->shouldHideForNotifications()) { + return true; + } + } + + return false; + } + + public function isVisibleInFeed() { + $xactions = $this->getTransactions(); + + foreach ($xactions as $key => $xaction) { + if (!$xaction->shouldHideForFeed()) { + return true; + } + } + + return false; + } + + private function getTransactions() { + $xaction_phids = $this->getValue('transactionPHIDs'); + + $xactions = array(); + foreach ($xaction_phids as $xaction_phid) { + $xactions[] = $this->getObject($xaction_phid); + } + + return $xactions; + } + public function getPrimaryTransaction() { return $this->getObject($this->getPrimaryTransactionPHID()); } diff --git a/src/applications/transactions/query/PhabricatorEditEngineConfigurationSearchEngine.php b/src/applications/transactions/query/PhabricatorEditEngineConfigurationSearchEngine.php index 52d48f528b..694b4c48b6 100644 --- a/src/applications/transactions/query/PhabricatorEditEngineConfigurationSearchEngine.php +++ b/src/applications/transactions/query/PhabricatorEditEngineConfigurationSearchEngine.php @@ -115,14 +115,6 @@ final class PhabricatorEditEngineConfigurationSearchEngine ->setHeader($config->getDisplayName()); $id = $config->getID(); - if ($id) { - $item->addIcon('fa-file-text-o bluegrey', pht('Form %d', $id)); - $key = $id; - } else { - $item->addIcon('fa-file-text bluegrey', pht('Builtin')); - $key = $config->getBuiltinKey(); - } - $item->setHref("/transactions/editengine/{$engine_key}/view/{$key}/"); if ($config->getIsDefault()) { $item->addAttribute(pht('Default Create Form')); @@ -139,6 +131,31 @@ final class PhabricatorEditEngineConfigurationSearchEngine $item->setStatusIcon('fa-file-text-o green', pht('Enabled')); } + $subtype_key = $config->getSubtype(); + if ($subtype_key !== PhabricatorEditEngineSubtype::SUBTYPE_DEFAULT) { + $engine = $config->getEngine(); + if ($engine->supportsSubtypes()) { + $map = $engine->newSubtypeMap(); + if ($map->isValidSubtype($subtype_key)) { + $subtype = $map->getSubtype($subtype_key); + + $icon = $subtype->getIcon(); + $color = $subtype->getColor(); + + $item->addIcon("{$icon} {$color}", $subtype->getName()); + } + } + } + + if ($id) { + $item->setObjectName(pht('Form %d', $id)); + $key = $id; + } else { + $item->addIcon('fa-file-text bluegrey', pht('Builtin')); + $key = $config->getBuiltinKey(); + } + $item->setHref("/transactions/editengine/{$engine_key}/view/{$key}/"); + $list->addItem($item); } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index c65adcac6b..f38c56acb3 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -664,6 +664,16 @@ abstract class PhabricatorApplicationTransaction break; } break; + + case PhabricatorTransactions::TYPE_INLINESTATE: + list($done, $undone) = $this->getInterestingInlineStateChangeCounts(); + + if (!$done && !$undone) { + return true; + } + + break; + } return false; @@ -749,6 +759,10 @@ abstract class PhabricatorApplicationTransaction return $this->shouldHide(); } + public function shouldHideForNotifications() { + return $this->shouldHideForFeed(); + } + public function getTitleForMail() { return id(clone $this)->setRenderingTarget('text')->getTitle(); } @@ -1007,15 +1021,7 @@ abstract class PhabricatorApplicationTransaction } case PhabricatorTransactions::TYPE_INLINESTATE: - $done = 0; - $undone = 0; - foreach ($new as $phid => $state) { - if ($state == PhabricatorInlineCommentInterface::STATE_DONE) { - $done++; - } else { - $undone++; - } - } + list($done, $undone) = $this->getInterestingInlineStateChangeCounts(); if ($done && $undone) { return pht( '%s marked %s inline comment(s) as done and %s inline comment(s) '. @@ -1582,6 +1588,49 @@ abstract class PhabricatorApplicationTransaction return $moves; } + private function getInterestingInlineStateChangeCounts() { + // See PHI995. Newer inline state transactions have additional details + // which we use to tailor the rendering behavior. These details are not + // present on older transactions. + $details = $this->getMetadataValue('inline.details', array()); + + $new = $this->getNewValue(); + + $done = 0; + $undone = 0; + foreach ($new as $phid => $state) { + $is_done = ($state == PhabricatorInlineCommentInterface::STATE_DONE); + + // See PHI995. If you're marking your own inline comments as "Done", + // don't count them when rendering a timeline story. In the case where + // you're only affecting your own comments, this will hide the + // "alice marked X comments as done" story entirely. + + // Usually, this happens when you pre-mark inlines as "done" and submit + // them yourself. We'll still generate an "alice added inline comments" + // story (in most cases/contexts), but the state change story is largely + // just clutter and slightly confusing/misleading. + + $inline_details = idx($details, $phid, array()); + $inline_author_phid = idx($inline_details, 'authorPHID'); + if ($inline_author_phid) { + if ($inline_author_phid == $this->getAuthorPHID()) { + if ($is_done) { + continue; + } + } + } + + if ($is_done) { + $done++; + } else { + $undone++; + } + } + + return array($done, $undone); + } + /* -( PhabricatorPolicyInterface Implementation )-------------------------- */ @@ -1617,6 +1666,15 @@ abstract class PhabricatorApplicationTransaction return null; } + public function setForceNotifyPHIDs(array $phids) { + $this->setMetadataValue('notify.force', $phids); + return $this; + } + + public function getForceNotifyPHIDs() { + return $this->getMetadataValue('notify.force', array()); + } + /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php index eef4b8e5d9..f6aece2a7f 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransaction.php +++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php @@ -108,6 +108,17 @@ abstract class PhabricatorModularTransaction return parent::shouldHideForMail($xactions); } + final public function shouldHideForNotifications() { + $hide = $this->getTransactionImplementation()->shouldHideForNotifications(); + + // Returning "null" means "use the default behavior". + if ($hide === null) { + return parent::shouldHideForNotifications(); + } + + return $hide; + } + /* final */ public function getIcon() { $icon = $this->getTransactionImplementation()->getIcon(); if ($icon !== null) { diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php index ca82af1c66..35dd3ac197 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -59,6 +59,10 @@ abstract class PhabricatorModularTransactionType return false; } + public function shouldHideForNotifications() { + return null; + } + public function getIcon() { return null; } diff --git a/src/infrastructure/storage/lisk/LiskDAO.php b/src/infrastructure/storage/lisk/LiskDAO.php index aa12f8e614..81005ab30d 100644 --- a/src/infrastructure/storage/lisk/LiskDAO.php +++ b/src/infrastructure/storage/lisk/LiskDAO.php @@ -193,8 +193,6 @@ abstract class LiskDAO extends Phobject private static $connections = array(); - private $inSet = null; - protected $id; protected $phid; protected $dateCreated; @@ -659,179 +657,11 @@ abstract class LiskDAO extends Phobject } else { $result[] = $obj->loadFromArray($row); } - if ($this->inSet) { - $this->inSet->addToSet($obj); - } } return $result; } - /** - * This method helps to prevent the 1+N queries problem. It happens when you - * execute a query for each row in a result set. Like in this code: - * - * COUNTEREXAMPLE, name=Easy to write but expensive to execute - * $diffs = id(new DifferentialDiff())->loadAllWhere( - * 'revisionID = %d', - * $revision->getID()); - * foreach ($diffs as $diff) { - * $changesets = id(new DifferentialChangeset())->loadAllWhere( - * 'diffID = %d', - * $diff->getID()); - * // Do something with $changesets. - * } - * - * One can solve this problem by reading all the dependent objects at once and - * assigning them later: - * - * COUNTEREXAMPLE, name=Cheaper to execute but harder to write and maintain - * $diffs = id(new DifferentialDiff())->loadAllWhere( - * 'revisionID = %d', - * $revision->getID()); - * $all_changesets = id(new DifferentialChangeset())->loadAllWhere( - * 'diffID IN (%Ld)', - * mpull($diffs, 'getID')); - * $all_changesets = mgroup($all_changesets, 'getDiffID'); - * foreach ($diffs as $diff) { - * $changesets = idx($all_changesets, $diff->getID(), array()); - * // Do something with $changesets. - * } - * - * The method @{method:loadRelatives} abstracts this approach which allows - * writing a code which is simple and efficient at the same time: - * - * name=Easy to write and cheap to execute - * $diffs = $revision->loadRelatives(new DifferentialDiff(), 'revisionID'); - * foreach ($diffs as $diff) { - * $changesets = $diff->loadRelatives( - * new DifferentialChangeset(), - * 'diffID'); - * // Do something with $changesets. - * } - * - * This will load dependent objects for all diffs in the first call of - * @{method:loadRelatives} and use this result for all following calls. - * - * The method supports working with set of sets, like in this code: - * - * $diffs = $revision->loadRelatives(new DifferentialDiff(), 'revisionID'); - * foreach ($diffs as $diff) { - * $changesets = $diff->loadRelatives( - * new DifferentialChangeset(), - * 'diffID'); - * foreach ($changesets as $changeset) { - * $hunks = $changeset->loadRelatives( - * new DifferentialHunk(), - * 'changesetID'); - * // Do something with hunks. - * } - * } - * - * This code will execute just three queries - one to load all diffs, one to - * load all their related changesets and one to load all their related hunks. - * You can try to write an equivalent code without using this method as - * a homework. - * - * The method also supports retrieving referenced objects, for example authors - * of all diffs (using shortcut @{method:loadOneRelative}): - * - * foreach ($diffs as $diff) { - * $author = $diff->loadOneRelative( - * new PhabricatorUser(), - * 'phid', - * 'getAuthorPHID'); - * // Do something with author. - * } - * - * It is also possible to specify additional conditions for the `WHERE` - * clause. Similarly to @{method:loadAllWhere}, you can specify everything - * after `WHERE` (except `LIMIT`). Contrary to @{method:loadAllWhere}, it is - * allowed to pass only a constant string (`%` doesn't have a special - * meaning). This is intentional to avoid mistakes with using data from one - * row in retrieving other rows. Example of a correct usage: - * - * $status = $author->loadOneRelative( - * new PhabricatorCalendarEvent(), - * 'userPHID', - * 'getPHID', - * '(UNIX_TIMESTAMP() BETWEEN dateFrom AND dateTo)'); - * - * @param LiskDAO Type of objects to load. - * @param string Name of the column in target table. - * @param string Method name in this table. - * @param string Additional constraints on returned rows. It supports no - * placeholders and requires putting the WHERE part into - * parentheses. It's not possible to use LIMIT. - * @return list Objects of type $object. - * - * @task load - */ - public function loadRelatives( - LiskDAO $object, - $foreign_column, - $key_method = 'getID', - $where = '') { - - if (!$this->inSet) { - id(new LiskDAOSet())->addToSet($this); - } - $relatives = $this->inSet->loadRelatives( - $object, - $foreign_column, - $key_method, - $where); - return idx($relatives, $this->$key_method(), array()); - } - - /** - * Load referenced row. See @{method:loadRelatives} for details. - * - * @param LiskDAO Type of objects to load. - * @param string Name of the column in target table. - * @param string Method name in this table. - * @param string Additional constraints on returned rows. It supports no - * placeholders and requires putting the WHERE part into - * parentheses. It's not possible to use LIMIT. - * @return LiskDAO Object of type $object or null if there's no such object. - * - * @task load - */ - final public function loadOneRelative( - LiskDAO $object, - $foreign_column, - $key_method = 'getID', - $where = '') { - - $relatives = $this->loadRelatives( - $object, - $foreign_column, - $key_method, - $where); - - if (!$relatives) { - return null; - } - - if (count($relatives) > 1) { - throw new AphrontCountQueryException( - pht( - 'More than one result from %s!', - __FUNCTION__.'()')); - } - - return reset($relatives); - } - - final public function putInSet(LiskDAOSet $set) { - $this->inSet = $set; - return $this; - } - - final protected function getInSet() { - return $this->inSet; - } - /* -( Examining Objects )-------------------------------------------------- */ diff --git a/src/infrastructure/storage/lisk/LiskDAOSet.php b/src/infrastructure/storage/lisk/LiskDAOSet.php deleted file mode 100644 index 90eba708ea..0000000000 --- a/src/infrastructure/storage/lisk/LiskDAOSet.php +++ /dev/null @@ -1,101 +0,0 @@ -addToSet($author); - * foreach ($reviewers as $reviewer) { - * $users->addToSet($reviewer); - * } - * foreach ($ccs as $cc) { - * $users->addToSet($cc); - * } - * // Preload e-mails of all involved users and return e-mails of author. - * $author_emails = $author->loadRelatives( - * new PhabricatorUserEmail(), - * 'userPHID', - * 'getPHID'); - */ -final class LiskDAOSet extends Phobject { - private $daos = array(); - private $relatives = array(); - private $subsets = array(); - - public function addToSet(LiskDAO $dao) { - if ($this->relatives) { - throw new Exception( - pht( - "Don't call %s after loading data!", - __FUNCTION__.'()')); - } - $this->daos[] = $dao; - $dao->putInSet($this); - return $this; - } - - /** - * The main purpose of this method is to break cyclic dependency. - * It removes all objects from this set and all subsets created by it. - */ - public function clearSet() { - $this->daos = array(); - $this->relatives = array(); - foreach ($this->subsets as $set) { - $set->clearSet(); - } - $this->subsets = array(); - return $this; - } - - - /** - * See @{method:LiskDAO::loadRelatives}. - */ - public function loadRelatives( - LiskDAO $object, - $foreign_column, - $key_method = 'getID', - $where = '') { - - $relatives = &$this->relatives[ - get_class($object)."-{$foreign_column}-{$key_method}-{$where}"]; - - if ($relatives === null) { - $ids = array(); - foreach ($this->daos as $dao) { - $id = $dao->$key_method(); - if ($id !== null) { - $ids[$id] = $id; - } - } - if (!$ids) { - $relatives = array(); - } else { - $set = new LiskDAOSet(); - $this->subsets[] = $set; - - $conn = $object->establishConnection('r'); - - if (strlen($where)) { - $where_clause = qsprintf($conn, 'AND %Q', $where); - } else { - $where_clause = qsprintf($conn, ''); - } - - $relatives = $object->putInSet($set)->loadAllWhere( - '%C IN (%Ls) %Q', - $foreign_column, - $ids, - $where_clause); - $relatives = mgroup($relatives, 'get'.$foreign_column); - } - } - - return $relatives; - } - -} diff --git a/src/infrastructure/storage/lisk/LiskMigrationIterator.php b/src/infrastructure/storage/lisk/LiskMigrationIterator.php index a46512f866..edd31c8123 100644 --- a/src/infrastructure/storage/lisk/LiskMigrationIterator.php +++ b/src/infrastructure/storage/lisk/LiskMigrationIterator.php @@ -17,11 +17,9 @@ final class LiskMigrationIterator extends PhutilBufferedIterator { private $object; private $cursor; - private $set; public function __construct(LiskDAO $object) { - $this->set = new LiskDAOSet(); - $this->object = $object->putInSet($this->set); + $this->object = $object; } protected function didRewind() { @@ -33,15 +31,15 @@ final class LiskMigrationIterator extends PhutilBufferedIterator { } protected function loadPage() { - $this->set->clearSet(); - $results = $this->object->loadAllWhere( 'id > %d ORDER BY id ASC LIMIT %d', $this->cursor, $this->getPageSize()); + if ($results) { $this->cursor = last($results)->getID(); } + return $results; } diff --git a/src/infrastructure/util/PhabricatorHash.php b/src/infrastructure/util/PhabricatorHash.php index ce48b0966b..d717778eb3 100644 --- a/src/infrastructure/util/PhabricatorHash.php +++ b/src/infrastructure/util/PhabricatorHash.php @@ -187,6 +187,16 @@ final class PhabricatorHash extends Phobject { } public static function digestHMACSHA256($message, $key) { + if (!is_string($message)) { + throw new Exception( + pht('HMAC-SHA256 can only digest strings.')); + } + + if (!is_string($key)) { + throw new Exception( + pht('HMAC-SHA256 keys must be strings.')); + } + if (!strlen($key)) { throw new Exception( pht('HMAC-SHA256 requires a nonempty key.')); @@ -194,7 +204,9 @@ final class PhabricatorHash extends Phobject { $result = hash_hmac('sha256', $message, $key, $raw_output = false); - if ($result === false) { + // Although "hash_hmac()" is documented as returning `false` when it fails, + // it can also return `null` if you pass an object as the "$message". + if ($result === false || $result === null) { throw new Exception( pht('Unable to compute HMAC-SHA256 digest of message.')); } diff --git a/src/view/AphrontDialogView.php b/src/view/AphrontDialogView.php index 12bec92843..52681845d2 100644 --- a/src/view/AphrontDialogView.php +++ b/src/view/AphrontDialogView.php @@ -404,7 +404,7 @@ final class AphrontDialogView $header), phutil_tag('div', array( - 'class' => 'aphront-dialog-body phabricator-remarkup grouped', + 'class' => 'aphront-dialog-body grouped', ), $children), $tail, diff --git a/src/view/phui/PHUIObjectItemView.php b/src/view/phui/PHUIObjectItemView.php index 0657086c49..e1c67c7f32 100644 --- a/src/view/phui/PHUIObjectItemView.php +++ b/src/view/phui/PHUIObjectItemView.php @@ -28,6 +28,7 @@ final class PHUIObjectItemView extends AphrontTagView { private $sideColumn; private $coverImage; private $description; + private $clickable; private $selectableName; private $selectableValue; @@ -179,6 +180,15 @@ final class PHUIObjectItemView extends AphrontTagView { return $this; } + public function setClickable($clickable) { + $this->clickable = $clickable; + return $this; + } + + public function getClickable() { + return $this->clickable; + } + public function setEpoch($epoch) { $date = phabricator_datetime($epoch, $this->getUser()); $this->addIcon('none', $date); @@ -332,6 +342,13 @@ final class PHUIObjectItemView extends AphrontTagView { $item_classes[] = 'phui-oi-with-image-icon'; } + if ($this->getClickable()) { + Javelin::initBehavior('linked-container'); + + $item_classes[] = 'phui-oi-linked-container'; + $sigils[] = 'linked-container'; + } + return array( 'class' => $item_classes, 'sigil' => $sigils, @@ -443,15 +460,6 @@ final class PHUIObjectItemView extends AphrontTagView { ), $spec['label']); - if (isset($spec['attributes']['href'])) { - $icon_href = phutil_tag( - 'a', - array('href' => $spec['attributes']['href']), - array($icon, $label)); - } else { - $icon_href = array($icon, $label); - } - $classes = array(); $classes[] = 'phui-oi-icon'; if (isset($spec['attributes']['class'])) { @@ -463,7 +471,10 @@ final class PHUIObjectItemView extends AphrontTagView { array( 'class' => implode(' ', $classes), ), - $icon_href); + array( + $icon, + $label, + )); } $icons[] = phutil_tag( diff --git a/webroot/rsrc/css/phui/object-item/phui-oi-big-ui.css b/webroot/rsrc/css/phui/object-item/phui-oi-big-ui.css index 67ce566733..6f60560a2e 100644 --- a/webroot/rsrc/css/phui/object-item/phui-oi-big-ui.css +++ b/webroot/rsrc/css/phui/object-item/phui-oi-big-ui.css @@ -59,3 +59,16 @@ background-color: {$hoverblue}; border-radius: 3px; } + +.device-desktop .phui-oi-linked-container { + cursor: pointer; +} + +.device-desktop .phui-oi-linked-container:hover { + background-color: {$hoverblue}; + border-radius: 3px; +} + +.device-desktop .phui-oi-linked-container a:hover { + text-decoration: none; +} diff --git a/webroot/rsrc/externals/javelin/core/init_node.js b/webroot/rsrc/externals/javelin/core/init_node.js index 19834664b4..1a7999e253 100644 --- a/webroot/rsrc/externals/javelin/core/init_node.js +++ b/webroot/rsrc/externals/javelin/core/init_node.js @@ -49,7 +49,7 @@ JX.require = function(thing) { } vm.createScript(content, path) - .runInNewContext(sandbox, path); + .runInNewContext(sandbox); }; exports.JX = JX; diff --git a/webroot/rsrc/js/core/behavior-linked-container.js b/webroot/rsrc/js/core/behavior-linked-container.js new file mode 100644 index 0000000000..9721f91b22 --- /dev/null +++ b/webroot/rsrc/js/core/behavior-linked-container.js @@ -0,0 +1,47 @@ +/** + * @provides javelin-behavior-linked-container + * @requires javelin-behavior javelin-dom + */ + +JX.behavior('linked-container', function() { + + JX.Stratcom.listen( + 'click', + 'linked-container', + function(e) { + + // If the user clicked some link inside the container, bail out and just + // click the link. + if (e.getNode('tag:a')) { + return; + } + + // If this is some sort of unusual click, bail out. Note that we'll + // handle "Left Click" and "Command + Left Click" differently, below. + if (!e.isLeftButton()) { + return; + } + + var container = e.getNode('linked-container'); + + // Find the first link in the container. We're going to pretend the user + // clicked it. + var link = JX.DOM.scry(container, 'a')[0]; + if (!link) { + return; + } + + // If the click is a "Command + Left Click", change the target of the + // link so we open it in a new tab. + var is_command = !!e.getRawEvent().metaKey; + if (is_command) { + var old_target = link.target; + link.target = '_blank'; + link.click(); + link.target = old_target; + } else { + link.click(); + } + }); + +});