From f94cee8628883f17d49a868af6ba38b83d7a3fc1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 25 Jun 2018 05:59:35 -0700 Subject: [PATCH 1/9] Fix querying for transactions over "transaction.search" when the object does not support comments Summary: See PHI725. Ref T13151. We currently try to load comments unconditionally, but not all objects (like projects) have comments. Only try to load comments if an object actually has comments. Test Plan: Queried for an object with no comments, like project `#masonry`, via `transaction.search`. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13151 Differential Revision: https://secure.phabricator.com/D19507 --- .../TransactionSearchConduitAPIMethod.php | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php index 17f53cfeee..3b040ea2c3 100644 --- a/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php +++ b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php @@ -85,22 +85,23 @@ final class TransactionSearchConduitAPIMethod $xactions = $xaction_query->executeWithCursorPager($pager); + $comment_map = array(); if ($xactions) { $template = head($xactions)->getApplicationTransactionCommentObject(); + if ($template) { - $query = new PhabricatorApplicationTransactionTemplatedCommentQuery(); + $query = new PhabricatorApplicationTransactionTemplatedCommentQuery(); - $comment_map = $query - ->setViewer($viewer) - ->setTemplate($template) - ->withTransactionPHIDs(mpull($xactions, 'getPHID')) - ->execute(); + $comment_map = $query + ->setViewer($viewer) + ->setTemplate($template) + ->withTransactionPHIDs(mpull($xactions, 'getPHID')) + ->execute(); - $comment_map = msort($comment_map, 'getCommentVersion'); - $comment_map = array_reverse($comment_map); - $comment_map = mgroup($comment_map, 'getTransactionPHID'); - } else { - $comment_map = array(); + $comment_map = msort($comment_map, 'getCommentVersion'); + $comment_map = array_reverse($comment_map); + $comment_map = mgroup($comment_map, 'getTransactionPHID'); + } } $modular_classes = array(); From 11f1c139155537ae14608ed43e6367220cef40c9 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Tue, 26 Jun 2018 15:06:13 +0000 Subject: [PATCH 2/9] Append the intermediate chain to the "cert" parameter in Aphlict Summary: Per the documentation[1], any intermediate chain is to be appended to the "cert" parameter. The "ca" parameter controls the root CA used to authenticate the client certificate, if one is provided, and is not used for intermediate certificate chains -- nor has it ever been. It is not clear how this could have worked in the past[2]. [1] https://nodejs.org/api/tls.html#tls_tls_createsecurecontext_options [2] D15709 Test Plan: Before this diff, with node 4.2.6 from Ubuntu packages: ``` $ openssl s_client -connect phabricator.dropboxer.net:22280 -verify 5 -CApath /etc/ssl/certs/ verify depth is 5 CONNECTED(00000003) depth=0 C = US, ST = California, L = San Francisco, O = "Dropbox, Inc", OU = Dropbox Ops, CN = phabricator.dropboxer.net verify error:num=20:unable to get local issuer certificate verify return:1 depth=0 C = US, ST = California, L = San Francisco, O = "Dropbox, Inc", OU = Dropbox Ops, CN = phabricator.dropboxer.net verify error:num=27:certificate not trusted verify return:1 depth=0 C = US, ST = California, L = San Francisco, O = "Dropbox, Inc", OU = Dropbox Ops, CN = phabricator.dropboxer.net verify error:num=21:unable to verify the first certificate verify return:1 --- Certificate chain 0 s:/C=US/ST=California/L=San Francisco/O=Dropbox, Inc/OU=Dropbox Ops/CN=phabricator.dropboxer.net i:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert SHA2 High Assurance Server CA ``` After: ``` $ openssl s_client -connect phabricator.dropboxer.net:22280 -verify 5 -CApath /etc/ssl/certs/ verify depth is 5 CONNECTED(00000003) depth=2 C = US, O = DigiCert Inc, OU = www.digicert.com, CN = DigiCert High Assurance EV Root CA verify return:1 depth=1 C = US, O = DigiCert Inc, OU = www.digicert.com, CN = DigiCert SHA2 High Assurance Server CA verify return:1 depth=0 C = US, ST = California, L = San Francisco, O = "Dropbox, Inc", OU = Dropbox Ops, CN = phabricator.dropboxer.net verify return:1 --- Certificate chain 0 s:/C=US/ST=California/L=San Francisco/O=Dropbox, Inc/OU=Dropbox Ops/CN=phabricator.dropboxer.net i:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert SHA2 High Assurance Server CA 1 s:/C=US/ST=California/L=San Francisco/O=Dropbox, Inc/OU=Dropbox Ops/CN=phabricator.dropboxer.net i:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert SHA2 High Assurance Server CA 2 s:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert SHA2 High Assurance Server CA i:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert High Assurance EV Root CA ``` Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D18181 --- support/aphlict/server/aphlict_server.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/support/aphlict/server/aphlict_server.js b/support/aphlict/server/aphlict_server.js index f162e4937c..34bddf4fa9 100644 --- a/support/aphlict/server/aphlict_server.js +++ b/support/aphlict/server/aphlict_server.js @@ -103,10 +103,9 @@ for (ii = 0; ii < config.servers.length; ii++) { if (spec['ssl.cert']){ spec['ssl.cert'] = fs.readFileSync(spec['ssl.cert']); - } - - if (spec['ssl.chain']){ - spec['ssl.chain'] = fs.readFileSync(spec['ssl.chain']); + if (spec['ssl.chain']){ + spec['ssl.cert'] += "\n" + fs.readFileSync(spec['ssl.chain']); + } } servers.push(spec); @@ -140,10 +139,6 @@ for (ii = 0; ii < servers.length; ii++) { cert: server['ssl.cert'], }; - if (server['ssl.chain']) { - https_config.ca = server['ssl.chain']; - } - http_server = https.createServer(https_config); } else { http_server = http.createServer(); From a94528ee4aa1d847fcdad2343257998a66658d52 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 27 Jun 2018 09:48:18 -0700 Subject: [PATCH 3/9] Expose Differential actions for "transaction.search" in a basic way Summary: See PHI725. Ref T13151. These actions are somewhat unusual and I considered different ways to represent them (make them look like "status" transactions; build multiple synthetic transactions) but ultimately landed on the simplest approach of just exposing them more or less as they exist internally. I haven't included data for any of them. Most don't really have any data, but "accept" does. I'm holding off on providing more data until after T731, which may shake up the internal format. Test Plan: Applied most of these transactions against a revision, queried for it with `transaction.search`, got distinguishable transactions out. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13151 Differential Revision: https://secure.phabricator.com/D19509 --- .../xaction/DifferentialRevisionAbandonTransaction.php | 8 ++++++++ .../xaction/DifferentialRevisionAcceptTransaction.php | 8 ++++++++ .../xaction/DifferentialRevisionCloseTransaction.php | 1 - .../xaction/DifferentialRevisionCommandeerTransaction.php | 8 ++++++++ .../DifferentialRevisionPlanChangesTransaction.php | 8 ++++++++ .../xaction/DifferentialRevisionReclaimTransaction.php | 8 ++++++++ .../xaction/DifferentialRevisionRejectTransaction.php | 8 ++++++++ .../xaction/DifferentialRevisionReopenTransaction.php | 8 ++++++++ .../DifferentialRevisionRequestReviewTransaction.php | 8 ++++++++ .../xaction/DifferentialRevisionResignTransaction.php | 8 ++++++++ 10 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php index 6232594aae..c2baef034f 100644 --- a/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php @@ -86,4 +86,12 @@ final class DifferentialRevisionAbandonTransaction $this->renderObject()); } + public function getTransactionTypeForConduit($xaction) { + return 'abandon'; + } + + public function getFieldValuesForConduit($object, $data) { + return array(); + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php index 42cd798584..3d2df14899 100644 --- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php @@ -234,4 +234,12 @@ final class DifferentialRevisionAcceptTransaction $this->renderObject()); } + public function getTransactionTypeForConduit($xaction) { + return 'accept'; + } + + public function getFieldValuesForConduit($object, $data) { + return array(); + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php index 7ef0f2ba60..f20ba7a011 100644 --- a/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php @@ -154,5 +154,4 @@ final class DifferentialRevisionCloseTransaction ); } - } diff --git a/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php b/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php index 561e57c7c4..5aafa536f3 100644 --- a/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php @@ -87,4 +87,12 @@ final class DifferentialRevisionCommandeerTransaction $this->renderObject()); } + public function getTransactionTypeForConduit($xaction) { + return 'commandeer'; + } + + public function getFieldValuesForConduit($object, $data) { + return array(); + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php index bdbe28d5cb..f4fb0a3eb1 100644 --- a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php @@ -119,4 +119,12 @@ final class DifferentialRevisionPlanChangesTransaction return (bool)$this->getMetadataValue('draft.demote'); } + public function getTransactionTypeForConduit($xaction) { + return 'plan-changes'; + } + + public function getFieldValuesForConduit($object, $data) { + return array(); + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php index 6d74589b64..e023dda0a1 100644 --- a/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php @@ -85,4 +85,12 @@ final class DifferentialRevisionReclaimTransaction $this->renderObject()); } + public function getTransactionTypeForConduit($xaction) { + return 'reclaim'; + } + + public function getFieldValuesForConduit($object, $data) { + return array(); + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php index 0ed6db96bf..0001ce09ca 100644 --- a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php @@ -99,4 +99,12 @@ final class DifferentialRevisionRejectTransaction $this->renderObject()); } + public function getTransactionTypeForConduit($xaction) { + return 'request-changes'; + } + + public function getFieldValuesForConduit($object, $data) { + return array(); + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php index 4fc9f3a6f1..a2d25287bf 100644 --- a/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php @@ -72,4 +72,12 @@ final class DifferentialRevisionReopenTransaction $this->renderObject()); } + public function getTransactionTypeForConduit($xaction) { + return 'reopen'; + } + + public function getFieldValuesForConduit($object, $data) { + return array(); + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php index 08c590a9ae..a3d2699c74 100644 --- a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php @@ -84,4 +84,12 @@ final class DifferentialRevisionRequestReviewTransaction $this->renderObject()); } + public function getTransactionTypeForConduit($xaction) { + return 'request-review'; + } + + public function getFieldValuesForConduit($object, $data) { + return array(); + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php index 53b0b8da01..a2b4fd4337 100644 --- a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php @@ -93,4 +93,12 @@ final class DifferentialRevisionResignTransaction $this->renderObject()); } + public function getTransactionTypeForConduit($xaction) { + return 'resign'; + } + + public function getFieldValuesForConduit($object, $data) { + return array(); + } + } From ca1349ab45d8f8c87ac78d86581f13a5c0a94e65 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 27 Jun 2018 10:43:03 -0700 Subject: [PATCH 4/9] Add an explicit example of constraining "transaction.search" to the webhook documentation Summary: Depends on D19509. Ref T13151. See PHI725. `transaction.search` supports "constraints" and the documentation mentions it in passing, but doesn't really show how to do it, and the method is not automatically self-documenting because it isn't a "real" `*.search` method. Add an example of how to use the `contraints` parameter to retrieve information about only the relevant transactions. Also remove a refernece to `requestb.in`, which is now defunct. Test Plan: Called `transaction.search` with similar parameters. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13151 Differential Revision: https://secure.phabricator.com/D19510 --- src/docs/user/userguide/webhooks.diviner | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/docs/user/userguide/webhooks.diviner b/src/docs/user/userguide/webhooks.diviner index 51521b462b..10d1f36da0 100644 --- a/src/docs/user/userguide/webhooks.diviner +++ b/src/docs/user/userguide/webhooks.diviner @@ -38,9 +38,6 @@ options: phabricator/ $ ./bin/webhook call --id 42 --object D123 ``` -You can use a tool like [[ https://requestb.in | RequestBin ]] to inspect -the headers and payload for calls to hooks. - Verifying Requests ================== @@ -157,6 +154,20 @@ Hooks that are interested in changes should generally make a call to `transaction.search`, passing the transaction PHIDs as a constraint to retrieve details about the transactions. +For example, your call to `transaction.search` may look something like this: + +```lang=json +{ + "objectIdentifier": "PHID-XXXX-abcdef", + "constraints": { + "phids": [ + "PHID-XACT-XXXX-11111111", + "PHID-XACT-XXXX-22222222" + ] + } +} +``` + The `phid.query` method can also be used to retrieve generic information about a list of objects. From 4214b56a4f4f378e8defdf0f29d6b9228f356560 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 27 Jun 2018 10:52:42 -0700 Subject: [PATCH 5/9] Make the dashboard panel datasource work properly with hundreds of panels Summary: Ref T13151. See PHI727. Update the dashboard widget/panel datasource to actually query results using what the user typed. The current approach is blind to what the user typed when pulling results from the database, and gets limited to an artificially small number of results somewhere in the pipeline. Test Plan: - Queried for panels with text queries. - Queried for panels with `W123` queries. - This is substantially similar to the Owners datasource, which received a similar update in D17142 and has worked well since then. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13151 Differential Revision: https://secure.phabricator.com/D19511 --- .../PhabricatorDashboardPanelDatasource.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/applications/dashboard/typeahead/PhabricatorDashboardPanelDatasource.php b/src/applications/dashboard/typeahead/PhabricatorDashboardPanelDatasource.php index 8bbdb1a5e5..883b6d32bb 100644 --- a/src/applications/dashboard/typeahead/PhabricatorDashboardPanelDatasource.php +++ b/src/applications/dashboard/typeahead/PhabricatorDashboardPanelDatasource.php @@ -20,13 +20,22 @@ final class PhabricatorDashboardPanelDatasource return $this->filterResultsAgainstTokens($results); } - protected function renderSpecialTokens(array $values) { return $this->renderTokensFromResults($this->buildResults(), $values); } public function buildResults() { - $query = id(new PhabricatorDashboardPanelQuery()); + $query = new PhabricatorDashboardPanelQuery(); + + $raw_query = $this->getRawQuery(); + if (preg_match('/^[wW]\d+\z/', $raw_query)) { + $id = trim($raw_query, 'wW'); + $id = (int)$id; + $query->withIDs(array($id)); + } else { + $query->withNameNgrams($raw_query); + } + $panels = $this->executeQuery($query); $results = array(); From 185c28f307e404a0f729a10b04bb8024f4fb1193 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 9 Jul 2018 14:22:29 -0700 Subject: [PATCH 6/9] Update parent/child revision timeline messages to use modern language ("parent revision") Summary: See PHI746. See also T11833, perhaps. Ref T13151. Long ago, parent revisions were called "dependent revisions". This was changed to "parent revisions" in the action UI to improve clarity, but not changed in the timeline stories. Update the timeline stories to use the same language the actions in the UI use. Test Plan: {F5732876} {F5732877} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13151 Differential Revision: https://secure.phabricator.com/D19514 --- ...alRevisionDependedOnByRevisionEdgeType.php | 12 ++-- ...ntialRevisionDependsOnRevisionEdgeType.php | 12 ++-- .../PhabricatorUSEnglishTranslation.php | 66 +++++++++++-------- 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/src/applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php b/src/applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php index a6940b536e..1f1adbc83a 100644 --- a/src/applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php +++ b/src/applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php @@ -33,7 +33,7 @@ final class DifferentialRevisionDependedOnByRevisionEdgeType $add_edges) { return pht( - '%s added %s dependent revision(s): %s.', + '%s added %s child revision(s): %s.', $actor, $add_count, $add_edges); @@ -45,7 +45,7 @@ final class DifferentialRevisionDependedOnByRevisionEdgeType $rem_edges) { return pht( - '%s removed %s dependent revision(s): %s.', + '%s removed %s child revision(s): %s.', $actor, $rem_count, $rem_edges); @@ -60,7 +60,7 @@ final class DifferentialRevisionDependedOnByRevisionEdgeType $rem_edges) { return pht( - '%s edited dependent revision(s), added %s: %s; removed %s: %s.', + '%s edited child revision(s), added %s: %s; removed %s: %s.', $actor, $add_count, $add_edges, @@ -75,7 +75,7 @@ final class DifferentialRevisionDependedOnByRevisionEdgeType $add_edges) { return pht( - '%s added %s dependent revision(s) for %s: %s.', + '%s added %s child revision(s) for %s: %s.', $actor, $add_count, $object, @@ -89,7 +89,7 @@ final class DifferentialRevisionDependedOnByRevisionEdgeType $rem_edges) { return pht( - '%s removed %s dependent revision(s) for %s: %s.', + '%s removed %s child revision(s) for %s: %s.', $actor, $rem_count, $object, @@ -106,7 +106,7 @@ final class DifferentialRevisionDependedOnByRevisionEdgeType $rem_edges) { return pht( - '%s edited dependent revision(s) for %s, added %s: %s; removed %s: %s.', + '%s edited child revision(s) for %s, added %s: %s; removed %s: %s.', $actor, $object, $add_count, diff --git a/src/applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php b/src/applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php index e826f554c6..a972b5359e 100644 --- a/src/applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php +++ b/src/applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php @@ -36,7 +36,7 @@ final class DifferentialRevisionDependsOnRevisionEdgeType $add_edges) { return pht( - '%s added %s dependencie(s): %s.', + '%s added %s parent revision(s): %s.', $actor, $add_count, $add_edges); @@ -48,7 +48,7 @@ final class DifferentialRevisionDependsOnRevisionEdgeType $rem_edges) { return pht( - '%s removed %s dependencie(s): %s.', + '%s removed %s parent revision(s): %s.', $actor, $rem_count, $rem_edges); @@ -63,7 +63,7 @@ final class DifferentialRevisionDependsOnRevisionEdgeType $rem_edges) { return pht( - '%s edited dependencie(s), added %s: %s; removed %s: %s.', + '%s edited parent revision(s), added %s: %s; removed %s: %s.', $actor, $add_count, $add_edges, @@ -78,7 +78,7 @@ final class DifferentialRevisionDependsOnRevisionEdgeType $add_edges) { return pht( - '%s added %s dependencie(s) for %s: %s.', + '%s added %s parent revision(s) for %s: %s.', $actor, $add_count, $object, @@ -92,7 +92,7 @@ final class DifferentialRevisionDependsOnRevisionEdgeType $rem_edges) { return pht( - '%s removed %s dependencie(s) for %s: %s.', + '%s removed %s parent revision(s) for %s: %s.', $actor, $rem_count, $object, @@ -109,7 +109,7 @@ final class DifferentialRevisionDependsOnRevisionEdgeType $rem_edges) { return pht( - '%s edited dependencie(s) for %s, added %s: %s; removed %s: %s.', + '%s edited parent revision(s) for %s, added %s: %s; removed %s: %s.', $actor, $object, $add_count, diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index a1e7ae4eae..c2241995e7 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -595,70 +595,80 @@ final class PhabricatorUSEnglishTranslation '%s changed files, attached: %3$s; detached: %5$s.', - '%s added %s dependencie(s): %s.' => array( + '%s added %s parent revision(s): %s.' => array( array( - '%s added a dependency: %3$s.', - '%s added dependencies: %3$s.', + '%s added a parent revision: %3$s.', + '%s added parent revisions: %3$s.', ), ), - '%s added %s dependencie(s) for %s: %s.' => array( + '%s added %s parent revision(s) for %s: %s.' => array( array( - '%s added a dependency for %3$s: %4$s.', - '%s added dependencies for %3$s: %4$s.', + '%s added a parent revision for %3$s: %4$s.', + '%s added parent revisions for %3$s: %4$s.', ), ), - '%s removed %s dependencie(s): %s.' => array( + '%s removed %s parent revision(s): %s.' => array( array( - '%s removed a dependency: %3$s.', - '%s removed dependencies: %3$s.', + '%s removed a parent revision: %3$s.', + '%s removed parent revisions: %3$s.', ), ), - '%s removed %s dependencie(s) for %s: %s.' => array( + '%s removed %s parent revision(s) for %s: %s.' => array( array( - '%s removed a dependency for %3$s: %4$s.', - '%s removed dependencies for %3$s: %4$s.', + '%s removed a parent revision for %3$s: %4$s.', + '%s removed parent revisions for %3$s: %4$s.', ), ), - '%s edited dependencie(s), added %s: %s; removed %s: %s.' => array( - '%s edited dependencies, added: %3$s; removed: %5$s.', + '%s edited parent revision(s), added %s: %s; removed %s: %s.' => array( + '%s edited parent revisions, added: %3$s; removed: %5$s.', ), - '%s edited dependencie(s) for %s, added %s: %s; removed %s: %s.' => array( - '%s edited dependencies for %s, added: %3$s; removed: %5$s.', + '%s edited parent revision(s) for %s, '. + 'added %s: %s; removed %s: %s.' => array( + '%s edited parent revisions for %s, added: %3$s; removed: %5$s.', ), - '%s added %s dependent revision(s): %s.' => array( + '%s added %s child revision(s): %s.' => array( array( - '%s added a dependent revision: %3$s.', - '%s added dependent revisions: %3$s.', + '%s added a child revision: %3$s.', + '%s added child revisions: %3$s.', ), ), - '%s added %s dependent revision(s) for %s: %s.' => array( + '%s added %s child revision(s) for %s: %s.' => array( array( - '%s added a dependent revision for %3$s: %4$s.', - '%s added dependent revisions for %3$s: %4$s.', + '%s added a child revision for %3$s: %4$s.', + '%s added child revisions for %3$s: %4$s.', ), ), - '%s removed %s dependent revision(s): %s.' => array( + '%s removed %s child revision(s): %s.' => array( array( - '%s removed a dependent revision: %3$s.', - '%s removed dependent revisions: %3$s.', + '%s removed a child revision: %3$s.', + '%s removed child revisions: %3$s.', ), ), - '%s removed %s dependent revision(s) for %s: %s.' => array( + '%s removed %s child revision(s) for %s: %s.' => array( array( - '%s removed a dependent revision for %3$s: %4$s.', - '%s removed dependent revisions for %3$s: %4$s.', + '%s removed a child revision for %3$s: %4$s.', + '%s removed child revisions for %3$s: %4$s.', ), ), + '%s edited child revision(s), added %s: %s; removed %s: %s.' => array( + '%s edited child revisions, added: %3$s; removed: %5$s.', + ), + + '%s edited child revision(s) for %s, '. + 'added %s: %s; removed %s: %s.' => array( + '%s edited child revisions for %s, added: %3$s; removed: %5$s.', + ), + '%s added %s commit(s): %s.' => array( array( '%s added a commit: %3$s.', From 67283c7a4526854409a621b484b3859302330653 Mon Sep 17 00:00:00 2001 From: Austin McKinley Date: Thu, 19 Jul 2018 20:46:36 -0700 Subject: [PATCH 7/9] Add test plan to differential.revision.search Summary: Ref T13151. Ref PHI622. Test Plan: Loaded a revision in the Conduit UI; observed presence of `testPlan` field. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T13151 Differential Revision: https://secure.phabricator.com/D19518 --- .../differential/storage/DifferentialRevision.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 2f7cee110f..a432c02a96 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -1148,6 +1148,10 @@ final class DifferentialRevision extends DifferentialDAO ->setKey('summary') ->setType('string') ->setDescription(pht('Revision summary.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('testPlan') + ->setType('string') + ->setDescription(pht('Revision test plan.')), ); } @@ -1167,6 +1171,7 @@ final class DifferentialRevision extends DifferentialDAO 'repositoryPHID' => $this->getRepositoryPHID(), 'diffPHID' => $this->getActiveDiffPHID(), 'summary' => $this->getSummary(), + 'testPlan' => $this->getTestPlan(), ); } From eb80a5ede1fbe5273c4d248d87bcd2b13d2153b9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 17 Jul 2018 16:04:00 -0700 Subject: [PATCH 8/9] Make the Conduit auth error for an unrecognized public key a little more useful Summary: Ref T13168. This is just a small quality-of-life fix: we can disclose which public key we're talking about because public keys are public. Test Plan: - Hit public key error (through my own bumbling / not reading or following instructions). Specifically, I haven't associated the key with a device in Almanac. - Before: vague error. - After: more specific error with enough key material that I could grep for it. Reviewers: amckinley Reviewed By: amckinley Subscribers: yelirekim Maniphest Tasks: T13168 Differential Revision: https://secure.phabricator.com/D19516 --- .../conduit/controller/PhabricatorConduitAPIController.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/applications/conduit/controller/PhabricatorConduitAPIController.php b/src/applications/conduit/controller/PhabricatorConduitAPIController.php index 12127a1e27..aea3fc0908 100644 --- a/src/applications/conduit/controller/PhabricatorConduitAPIController.php +++ b/src/applications/conduit/controller/PhabricatorConduitAPIController.php @@ -211,9 +211,14 @@ final class PhabricatorConduitAPIController ->withIsActive(true) ->executeOne(); if (!$stored_key) { + $key_summary = id(new PhutilUTF8StringTruncator()) + ->setMaximumBytes(64) + ->truncateString($raw_key); return array( 'ERR-INVALID-AUTH', - pht('No user or device is associated with that public key.'), + pht( + 'No user or device is associated with the public key "%s".', + $key_summary), ); } From 96a8b89fa8a82b79bab07674d6464b361cdf0efb Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 20 Jul 2018 14:36:07 -0700 Subject: [PATCH 9/9] When building a config stack, stop SiteSource objects from poisoning the cache Summary: Ref T13168. I'm not sure how this worked before, but I ran into this issue on my new laptop. SiteSource accesses `PhabrictatorEnv::getEnvConfig('phabricator.base-uri')` when local, which may poison the cache and lock the value since we don't later discard the cache. Specifically, when I access `http://locala.phacility.com`, I was getting an error like "You made a request for locala.phacility.com, but no configured site can serve this request.". This was because the base-uri was being incorrectly frozen as "local.phacility.com". The expectation is that it will match, so the standard PlatformSite will serve the request. Test Plan: - Before change: "no configured site" error. - After change: local instance works properly. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13168 Differential Revision: https://secure.phabricator.com/D19526 --- src/infrastructure/env/PhabricatorEnv.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index ba05d0409d..8b2af38d3d 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -221,6 +221,10 @@ final class PhabricatorEnv extends Phobject { foreach ($site_sources as $site_source) { $stack->pushSource($site_source); + + // If the site source did anything which reads config, throw it away + // to make sure any additional site sources get clean reads. + self::dropConfigCache(); } $masters = PhabricatorDatabaseRef::getMasterDatabaseRefs(); @@ -259,6 +263,10 @@ final class PhabricatorEnv extends Phobject { throw $ex; } } + + // Drop the config cache one final time to make sure we're getting clean + // reads now that we've finished building the stack. + self::dropConfigCache(); } public static function repairConfig($key, $value) {