From 58db64c81f5c92ed6afd8e7a73916b0bb9ddf60d Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sat, 5 Aug 2017 20:05:04 -0700 Subject: [PATCH 01/24] Hide curtainview on mobile if it's empty Summary: If we don't have any panels, just an action list, we want to hide the entire box on mobile since it's just an empty line. Test Plan: Review Owners, Differential curtains on mobile, desktop. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18350 --- resources/celerity/map.php | 6 +++--- src/view/layout/PHUICurtainView.php | 9 ++++++++- webroot/rsrc/css/phui/phui-curtain-view.css | 4 ++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index d50a470c34..aa02c547de 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' => 'b5b51108', - 'core.pkg.css' => 'cc0772c6', + 'core.pkg.css' => '04da74af', 'core.pkg.js' => '5d80e0db', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '45951e9e', @@ -149,7 +149,7 @@ return array( 'rsrc/css/phui/phui-comment-form.css' => 'ac68149f', 'rsrc/css/phui/phui-comment-panel.css' => 'f50152ad', 'rsrc/css/phui/phui-crumbs-view.css' => '6ece3bbb', - 'rsrc/css/phui/phui-curtain-view.css' => '55dd0e59', + 'rsrc/css/phui/phui-curtain-view.css' => 'ca363f15', 'rsrc/css/phui/phui-document-pro.css' => '8af7ea27', 'rsrc/css/phui/phui-document-summary.css' => '9ca48bdf', 'rsrc/css/phui/phui-document.css' => 'c32e8dec', @@ -835,7 +835,7 @@ return array( 'phui-comment-form-css' => 'ac68149f', 'phui-comment-panel-css' => 'f50152ad', 'phui-crumbs-view-css' => '6ece3bbb', - 'phui-curtain-view-css' => '55dd0e59', + 'phui-curtain-view-css' => 'ca363f15', 'phui-document-summary-view-css' => '9ca48bdf', 'phui-document-view-css' => 'c32e8dec', 'phui-document-view-pro-css' => '8af7ea27', diff --git a/src/view/layout/PHUICurtainView.php b/src/view/layout/PHUICurtainView.php index af02ceb932..49ff55384a 100644 --- a/src/view/layout/PHUICurtainView.php +++ b/src/view/layout/PHUICurtainView.php @@ -46,10 +46,17 @@ final class PHUICurtainView extends AphrontTagView { $panels = $this->renderPanels(); - return id(new PHUIObjectBoxView()) + $box = id(new PHUIObjectBoxView()) ->appendChild($action_list) ->appendChild($panels) ->addClass('phui-two-column-properties'); + + // We want to hide this UI on mobile if there are no child panels + if (!$panels) { + $box->addClass('curtain-no-panels'); + } + + return $box; } private function renderPanels() { diff --git a/webroot/rsrc/css/phui/phui-curtain-view.css b/webroot/rsrc/css/phui/phui-curtain-view.css index 0d7d4f942f..8bd8a331c4 100644 --- a/webroot/rsrc/css/phui/phui-curtain-view.css +++ b/webroot/rsrc/css/phui/phui-curtain-view.css @@ -46,3 +46,7 @@ .phui-side-column .phui-curtain-panel-body .phui-tag-view { white-space: pre-wrap; } + +.device .curtain-no-panels { + display: none; +} From 8ca29a607a2bf82bf83f1a6dcc61d85de6d6367d Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sat, 5 Aug 2017 19:32:26 -0700 Subject: [PATCH 02/24] Remove incorrect policy language on Diff reviewers Summary: Fixes T12952. This never work AFAIK, so resolves this mis-information. See T4411 for follow up. Test Plan: Click on policy for a diff, no longer see text. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T12952 Differential Revision: https://secure.phabricator.com/D18349 --- src/applications/differential/storage/DifferentialRevision.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 0e968dccf4..7ad01c02ca 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -523,7 +523,6 @@ final class DifferentialRevision extends DifferentialDAO switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: - $description[] = pht("A revision's reviewers can always view it."); $description[] = pht( 'If a revision belongs to a repository, other users must be able '. 'to view the repository in order to view the revision.'); From ec5f20b399e53cf8378e996e82591cc4f2cf5dfd Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sat, 5 Aug 2017 15:55:31 -0700 Subject: [PATCH 03/24] Move PHUIInfoView Summary: Just moves this because I can never easily find it. Test Plan: Check UIExamples Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18348 --- src/__phutil_library_map__.php | 2 +- src/view/{form => phui}/PHUIInfoView.php | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/view/{form => phui}/PHUIInfoView.php (100%) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index afaccaacf9..5741de915b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1769,7 +1769,7 @@ phutil_register_library_map(array( 'PHUIImageMaskExample' => 'applications/uiexample/examples/PHUIImageMaskExample.php', 'PHUIImageMaskView' => 'view/phui/PHUIImageMaskView.php', 'PHUIInfoExample' => 'applications/uiexample/examples/PHUIInfoExample.php', - 'PHUIInfoView' => 'view/form/PHUIInfoView.php', + 'PHUIInfoView' => 'view/phui/PHUIInfoView.php', 'PHUIInvisibleCharacterTestCase' => 'view/phui/__tests__/PHUIInvisibleCharacterTestCase.php', 'PHUIInvisibleCharacterView' => 'view/phui/PHUIInvisibleCharacterView.php', 'PHUILeftRightExample' => 'applications/uiexample/examples/PHUILeftRightExample.php', diff --git a/src/view/form/PHUIInfoView.php b/src/view/phui/PHUIInfoView.php similarity index 100% rename from src/view/form/PHUIInfoView.php rename to src/view/phui/PHUIInfoView.php From ca8ae2d4ca5d0f4f8655426e8c99ad35fd3039d0 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sat, 5 Aug 2017 14:57:36 -0700 Subject: [PATCH 04/24] Add a red button to PHUIButtonView Summary: Danger Danger Test Plan: UIExamples {F5084035} Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18347 --- resources/celerity/map.php | 6 +++--- .../CelerityDefaultPostprocessor.php | 3 +++ .../uiexample/examples/PHUIButtonExample.php | 1 + src/view/phui/PHUIButtonView.php | 1 + webroot/rsrc/css/phui/button/phui-button.css | 20 ++++++++++++++++++- 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index aa02c547de..fc4556782c 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' => 'b5b51108', - 'core.pkg.css' => '04da74af', + 'core.pkg.css' => 'cea08376', 'core.pkg.js' => '5d80e0db', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '45951e9e', @@ -127,7 +127,7 @@ return array( 'rsrc/css/layout/phabricator-source-code-view.css' => 'aea41829', 'rsrc/css/phui/button/phui-button-bar.css' => 'f1ff5494', 'rsrc/css/phui/button/phui-button-simple.css' => '8e1baf68', - 'rsrc/css/phui/button/phui-button.css' => '3ca51caa', + 'rsrc/css/phui/button/phui-button.css' => '340f55c1', 'rsrc/css/phui/calendar/phui-calendar-day.css' => '572b1893', 'rsrc/css/phui/calendar/phui-calendar-list.css' => '576be600', 'rsrc/css/phui/calendar/phui-calendar-month.css' => '21154caf', @@ -824,7 +824,7 @@ return array( 'phui-big-info-view-css' => 'acc3492c', 'phui-box-css' => '745e881d', 'phui-button-bar-css' => 'f1ff5494', - 'phui-button-css' => '3ca51caa', + 'phui-button-css' => '340f55c1', 'phui-button-simple-css' => '8e1baf68', 'phui-calendar-css' => 'f1ddf11c', 'phui-calendar-day-css' => '572b1893', diff --git a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php index 39a6c849e1..61f6176f15 100644 --- a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php +++ b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php @@ -231,6 +231,9 @@ final class CelerityDefaultPostprocessor 'green.button.color' => '#139543', 'green.button.gradient' => 'linear-gradient(to bottom, #23BB5B, #139543)', 'green.button.hover' => 'linear-gradient(to bottom, #23BB5B, #178841)', + 'red.button.color' => '#b33225', + 'red.button.gradient' => 'linear-gradient(to bottom, #d25454, #b33225)', + 'red.button.hover' => 'linear-gradient(to bottom, #d25454, #982115)', 'grey.button.color' => '#F7F7F9', 'grey.button.gradient' => 'linear-gradient(to bottom, #ffffff, #f1f0f1)', 'grey.button.hover' => 'linear-gradient(to bottom, #ffffff, #eeebec)', diff --git a/src/applications/uiexample/examples/PHUIButtonExample.php b/src/applications/uiexample/examples/PHUIButtonExample.php index a5ece198fd..c7107250ff 100644 --- a/src/applications/uiexample/examples/PHUIButtonExample.php +++ b/src/applications/uiexample/examples/PHUIButtonExample.php @@ -20,6 +20,7 @@ final class PHUIButtonExample extends PhabricatorUIExample { $colors = array( null, PHUIButtonView::GREEN, + PHUIButtonView::RED, PHUIButtonView::GREY, ); $sizes = array(null, PHUIButtonView::SMALL); diff --git a/src/view/phui/PHUIButtonView.php b/src/view/phui/PHUIButtonView.php index 1aaa1cbaad..8d6eca9ec7 100644 --- a/src/view/phui/PHUIButtonView.php +++ b/src/view/phui/PHUIButtonView.php @@ -5,6 +5,7 @@ final class PHUIButtonView extends AphrontTagView { const GREEN = 'green'; const GREY = 'grey'; const BLUE = 'blue'; + const RED = 'red'; const DISABLED = 'disabled'; const SMALL = 'small'; diff --git a/webroot/rsrc/css/phui/button/phui-button.css b/webroot/rsrc/css/phui/button/phui-button.css index 8cdd818f12..a36d69d4bc 100644 --- a/webroot/rsrc/css/phui/button/phui-button.css +++ b/webroot/rsrc/css/phui/button/phui-button.css @@ -50,7 +50,9 @@ input[type="submit"] { button .phui-icon-view, a.button .phui-icon-view, button.button-green .phui-icon-view, -a.button.button-green .phui-icon-view { +a.button.button-green .phui-icon-view, +button.button-red .phui-icon-view, +a.button.button-red .phui-icon-view { color: white; } @@ -76,6 +78,14 @@ a.button-green.button:visited { background-image: {$green.button.gradient}; } +button.button-red, +a.button-red.button, +a.button-red.button:visited { + background-color: {$red.button.color}; + border-color: {$red.button.color}; + background-image: {$red.button.gradient}; +} + button.button-grey, input[type="submit"].button-grey, a.button-grey, @@ -123,6 +133,14 @@ button.button-green:hover { transition: 0.1s; } +a.button.button-red:hover, +button.button-red:hover { + border-color: #79150b; + background-color: #0DAD48; + background-image: {$red.button.hover}; + transition: 0.1s; +} + body a.button.disabled:hover, body button.disabled:hover, body a.button.disabled:active, From fd3cb18fe4a4da81cefcb96db34b19d7ad5f45f9 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 7 Aug 2017 13:34:00 +0000 Subject: [PATCH 05/24] Don't force buttons to grey with PHUIInfoView Summary: I'd like to use red buttons. Test Plan: Set a button to red in InfoView, see red button. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18352 --- src/view/phui/PHUIInfoView.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/view/phui/PHUIInfoView.php b/src/view/phui/PHUIInfoView.php index 8ba74056b8..18f5af3374 100644 --- a/src/view/phui/PHUIInfoView.php +++ b/src/view/phui/PHUIInfoView.php @@ -95,7 +95,6 @@ final class PHUIInfoView extends AphrontTagView { } public function addButton(PHUIButtonView $button) { - $button->setColor(PHUIButtonView::GREY); $this->buttons[] = $button; return $this; } From 7119c9874475b2a82f2e564790b98b65f8b6dc87 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 7 Aug 2017 13:53:19 -0700 Subject: [PATCH 06/24] Add a UIExamples page for PHUIBigInfoView Summary: Fixes the icon bug and builds a basic examples page for future testing. Test Plan: Visit uiexampls and various types of info views. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18356 --- src/__phutil_library_map__.php | 2 + .../uiexample/examples/PHUIBigInfoExample.php | 48 +++++++++++++++++++ src/view/phui/PHUIBigInfoView.php | 1 + 3 files changed, 51 insertions(+) create mode 100644 src/applications/uiexample/examples/PHUIBigInfoExample.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5741de915b..d44e9370bb 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1710,6 +1710,7 @@ phutil_register_library_map(array( 'PHUIBadgeExample' => 'applications/uiexample/examples/PHUIBadgeExample.php', 'PHUIBadgeMiniView' => 'view/phui/PHUIBadgeMiniView.php', 'PHUIBadgeView' => 'view/phui/PHUIBadgeView.php', + 'PHUIBigInfoExample' => 'applications/uiexample/examples/PHUIBigInfoExample.php', 'PHUIBigInfoView' => 'view/phui/PHUIBigInfoView.php', 'PHUIBoxExample' => 'applications/uiexample/examples/PHUIBoxExample.php', 'PHUIBoxView' => 'view/phui/PHUIBoxView.php', @@ -6867,6 +6868,7 @@ phutil_register_library_map(array( 'PHUIBadgeExample' => 'PhabricatorUIExample', 'PHUIBadgeMiniView' => 'AphrontTagView', 'PHUIBadgeView' => 'AphrontTagView', + 'PHUIBigInfoExample' => 'PhabricatorUIExample', 'PHUIBigInfoView' => 'AphrontTagView', 'PHUIBoxExample' => 'PhabricatorUIExample', 'PHUIBoxView' => 'AphrontTagView', diff --git a/src/applications/uiexample/examples/PHUIBigInfoExample.php b/src/applications/uiexample/examples/PHUIBigInfoExample.php new file mode 100644 index 0000000000..519cdba88e --- /dev/null +++ b/src/applications/uiexample/examples/PHUIBigInfoExample.php @@ -0,0 +1,48 @@ +getRequest(); + $viewer = $request->getUser(); + + $image = PhabricatorFile::loadBuiltin($viewer, + 'projects/v3/rocket.png'); + + $button = id(new PHUIButtonView()) + ->setTag('a') + ->setText(pht('Launch Away')) + ->setColor(PHUIButtonView::GREEN) + ->setHref('#'); + + $views = array(); + $views[] = id(new PHUIBigInfoView()) + ->setTitle(pht('Simply Slim')) + ->setDescription(pht('A simple description')) + ->addAction($button); + + $views[] = id(new PHUIBigInfoView()) + ->setTitle(pht('Basicly Basic')) + ->setIcon('fa-rocket') + ->setDescription(pht('A more basic description')) + ->addAction($button); + + $views[] = id(new PHUIBigInfoView()) + ->setTitle(pht('A Modern Example')) + ->setImage($image->getBestURI()) + ->setDescription(pht('A modern description with lots of frills.')) + ->addAction($button); + + + return phutil_tag_div('ml', $views); + } +} diff --git a/src/view/phui/PHUIBigInfoView.php b/src/view/phui/PHUIBigInfoView.php index ad10a9fa4a..31a458ed60 100644 --- a/src/view/phui/PHUIBigInfoView.php +++ b/src/view/phui/PHUIBigInfoView.php @@ -49,6 +49,7 @@ final class PHUIBigInfoView extends AphrontTagView { protected function getTagContent() { require_celerity_resource('phui-big-info-view-css'); + $icon = null; if ($this->icon) { $icon = id(new PHUIIconView()) ->setIcon($this->icon) From 80b18278b8c55dbc44121c846518ac285e626dbe Mon Sep 17 00:00:00 2001 From: Chad Little Date: Tue, 8 Aug 2017 16:39:11 -0700 Subject: [PATCH 07/24] Center page tabs on mobile Summary: Centers tabs when used above the page header when on mobile. Test Plan: Test mobile and desktop layouts of Instances. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18368 --- resources/celerity/map.php | 6 +++--- .../css/application/base/standard-page-view.css | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index fc4556782c..a8b40f8e88 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' => 'b5b51108', - 'core.pkg.css' => 'cea08376', + 'core.pkg.css' => '5a682e14', 'core.pkg.js' => '5d80e0db', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '45951e9e', @@ -42,7 +42,7 @@ return array( 'rsrc/css/application/base/main-menu-view.css' => '16053029', 'rsrc/css/application/base/notification-menu.css' => '73fefdfa', 'rsrc/css/application/base/phui-theme.css' => '9f261c6b', - 'rsrc/css/application/base/standard-page-view.css' => 'a0dae682', + 'rsrc/css/application/base/standard-page-view.css' => 'c581d2ac', 'rsrc/css/application/chatlog/chatlog.css' => 'd295b020', 'rsrc/css/application/conduit/conduit-api.css' => '7bc725c4', 'rsrc/css/application/config/config-options.css' => 'd55ed093', @@ -802,7 +802,7 @@ return array( 'phabricator-shaped-request' => '7cbe244b', 'phabricator-slowvote-css' => 'a94b7230', 'phabricator-source-code-view-css' => 'aea41829', - 'phabricator-standard-page-view' => 'a0dae682', + 'phabricator-standard-page-view' => 'c581d2ac', 'phabricator-textareautils' => '320810c8', 'phabricator-title' => '485aaa6c', 'phabricator-tooltip' => '358b8c04', diff --git a/webroot/rsrc/css/application/base/standard-page-view.css b/webroot/rsrc/css/application/base/standard-page-view.css index f697b1f7a8..d930a73797 100644 --- a/webroot/rsrc/css/application/base/standard-page-view.css +++ b/webroot/rsrc/css/application/base/standard-page-view.css @@ -184,6 +184,20 @@ a.handle-status-closed:hover { box-shadow: 0 0 3px 0 rgba(0,0,0,0.2); } +.device .phabricator-standard-page-tabs { + margin-bottom: 20px; +} + +.device-phone .phabricator-standard-page-tabs { + text-align: center; +} + +.device-phone + .phabricator-standard-page-tabs.phui-list-view.phui-list-tabbar > li { + display: inline-block; + float: none; +} + .phabricator-standard-page-tabs.phui-list-tabbar .phui-list-item-href { padding: 12px 24px; } From b1e3cf627de11d5140fa3b6f050df24468427a56 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Tue, 8 Aug 2017 17:45:56 -0700 Subject: [PATCH 08/24] Add more repo images Summary: Just a few more. Test Plan: Edit Picture, see new image, choose image. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18370 --- resources/builtin/repo/repo-git.png | Bin 0 -> 4899 bytes resources/builtin/repo/repo-hg.png | Bin 0 -> 4828 bytes resources/builtin/repo/repo-svn.png | Bin 0 -> 5058 bytes ...ffusionRepositoryProfilePictureController.php | 3 +++ 4 files changed, 3 insertions(+) create mode 100644 resources/builtin/repo/repo-git.png create mode 100644 resources/builtin/repo/repo-hg.png create mode 100644 resources/builtin/repo/repo-svn.png diff --git a/resources/builtin/repo/repo-git.png b/resources/builtin/repo/repo-git.png new file mode 100644 index 0000000000000000000000000000000000000000..b8dfed8ad28218651f6213bfd6917298fc508266 GIT binary patch literal 4899 zcmb_gdpwi>+uz0r6;hLm@Ff7;ML+xv8=3b@+50d>eT9@c=istWgjRt^e&$`-pQyc~Ys+?)@%N z`7!I$x1WWE)JHez^tDQBY78b@lr>yx33w_kD6Pexik&-hvDSt%v2UW~-mBz1{@}68 z^O_c8D<_xqkma*8KE)fKRe^Fh&bF$|0ih&oZqcxKl~EZt_QW*n&&gUpZEp^ro~$~R zJ-99oU_zpO%iR=Bw{?osyjpAA2t!tP_q^H9hx@){6k_yFB1>Q^b=getuXRl^DhQIFF_v!F-8v$J6y|MbBWqVWV~I{8HAp-^o@mF} z_{idG(4(wBkG>}yE!WWG>bnj~ zpw|{WXsznrNZ;ZdAksgEaf}A44zSNtanT@$mBo(b=La4|Kr*wHfgUF+%en=~7jYIx zK^zgPesHjA{@`0>oZtxwZ-P7o4jytmgRMx3)-B(BtW+mddLNizoxOzh!J9ZiyeZNB z6!ADe{uBRyx)gvAdS1XuA;0{sgzJUO2@NU(L9R}ybI(dw%V3*S)Ur}%3N|WlRrVnz zRzd%U3RBzNy~ zpGe@mG|z3NVF5WFfV8kY67nmo#U{n0m|i94WJg}Od2Mw~I-wABXHDzxP(Ip7*Aq$LE2NMlyPuYb)1%VGXD92x%~18 z$oiIWVm9?>S)TouHl@|yVeo%4K{C)D%mg}8Z8L&S+NDMgU?y-aKmCvW<1swv4L$Ls ze676YcqsJUw}o$d5Pv4Uh?N?H(M}Z;gQLMbu|D)#qf$B0uIO`6W3{O70Vs9ohP6jj z;!Cg%`B~0Q?qkEqJ5xBYDv(gTkN*FJo|inl(ECN5NU+ZN;eWusD*pf%!#LUC0Y zLFG%~oE_1jKN-P7MpZ{tBp1xFsC&&UTc_F92~gxW3K-ULs`AFK8b`a->!KbsZxomm zbD{+^|E6mHo$1oShXPg=XmqPadf1|k9r@eL5!b@Ch^T`vbqq?1hT)*2jF9+&#&{$) zv+Z;9$-JyKMS1|Y`@SFHXP0wK7I1}w%5nB{l}wC$e+~wAOc~%TH?TedIMkl~d1Q8Y zHI1Kx>@7d?oB4i7H)GSbv!nSJ6M!co4`h-se&K7WeKm(}2K>ScK!!Cr$I@)ik{7p3 zcX@^827M)UYLsiQjO&2;4kT60s_j|~@-=ZMw%ZVwH-W!Zor-t za@mSxb*WcI-D)Ye!E1ZL1FhcL#!56+EUsy+!nT<}AF0)m6X}1Q)^^azRlc|88k;td zIdw``R`T+rh1Ir&vt3_BMM)wFKs6!ige%8Z_b(X;H{zjN^4^SxW$6iWExm&FR%Nkq z+Xq?`jSOxpYpcIqd5pC&amkd5aML?9cxPu}@<{ahNryGpy$nuhA$RO7HpLPKCN$1~ zF83tp&#l~aouNI+iKxsp%vW3Wjn>|&E=L7{Fy&N22P!(jf6Nx=noFU0PUK2Jjlr(v zs!ZF-MiIGP{V|S&tu3YXiGtUbYt+|Tdqeb%&P|>rWt>BCE8={+IA(eU78kQw;(H-a z_vFPo6)T&`gjHJzhUr!pc6*=c70op`t){WCd>uohLk`PFv0BqB>+O`%#oxIRz7=AL z*#~xoM;B{5JsCZliRA~&j;u`JOs}0nV%}^3)q#b|wG_j8UKj6?DL%Y%f<?C8 zAu!^3!_B52HXB*I$nlQ8OK@Y{*pFM7cv%C4(Fx_o2Qo{ zp>TlxMpw7`mT^)B?q!!=7`F^^FW7L=%rSkLE3227zdEq$yO2=VpaqO-&tlca#aLVH zrn^ff#?C^A2s9z`I+p$Z(Im?WDQWnRWQyVMsksAz6(U%jgh-wgZNad3c)mi19l1W6 zHgW`V2%4r`t8wMd*a*iqqJkjZ0)_~RQq*M)AxS+0hz1-dUHQ~n*WiXtf4YA*4YNt3 zcj2DTX#(A`Q5}*;;tT0rQ&xI!+r=D;GE<~d!McOUx|Y`iR?^d~-_(-8{c7V&+huu( zByK=`yP1Jt!QBb*rI+JkiQ|yPq})Yp-MIr@_L5E)F$U_*KHGRyp4}eVr-qbp?l}Qh z_8v=bUKaaxfk&%>{>;Quz`gJGQCG?{^CvDmlMad3t2pfNoKkQ*tEKS*VOa|qx+$gAvtjl7n@flUO|5)3T@k=AIy_OHMSSn+-wIO8r zcb_db@%PWJR*mxDv7?_EM+e4zXFDr(p7U~v!H`Q4HlnWi`yMD%foA*4GVKQ6l6q3zoqGbDMfb3HW3XehEA1hb#Y6^b@ZHaODBA~>p93_Jr z=kzC&T(tTA=It@kdbXnEAORbHNE>;9VK3-F*xJjlyp9E*f&h<#A3-GX_(i+)ChPsr zRy~i1TJQo{5tE^v;2}S3^pdcq9AKKwr^jxepn(H3*i_q!q+KZG|^}OaAXOlM*xsVFmH|&D$%a zSdUdCh6DO1=ezs6MzW%7MrJNlM0$DD-@Pi`24{YUc`FrLqdtWx^s1v{&Lik0nAhxI z)}|nNMQ*D5N9h`d%<3&ie)xBq*ZuW6K0^vIr*U9uF%= zuDWscKxbIY%#MkiCc2*>9`Bw5Ww#k#Zthbd;r(vio5am69R>`f0i;XmkG{JKEeujb z6g}BY#B_;W@(2YOY^P?0e=L6N-Z*y+av+AvNN&*4yHzU(212M-ij@fI%QtI@1sal zN7JBJ6a-nXgyaFU?yQD2ytu^u-4EpAH!CVg9GJN9b7X4F5Fv?S>JOg8b z%VGN$xBcTmW|>14vy=l4#4$qf>753-x2Afx{)uQkv4*<@3L)6BCcOQD${-k zya0c_My%r_UsMksE7s}x&F?S}POr7#nfCrlN9Dr9yqhG5o5Ue zqmLSKE$lWi0mT;GbTXjKDcR6Kj1!GPxus z1T+T>NG0cd$T!>3gcq%e<>06Z=3P&9OZJPNYK;6HZq+8K$pK3L`oF9!lopmBCUikx zL(}pc|6EXXcx&d^Tz#`$PXQ1IJN1)2)JFk(L$5iV;|lx${L0hz%ydcz*VEh9p9_=a z_?*|mZKQP#m~i~_*B{cmWo_!z46LzE7B4ROccB=<49G#$oYM^;g?x??eeqJwL5xS;m?_9G8*vFwJZ7 z;eM@Z7=BTsDtaga=<@5|);CEf0gri! zM8ie!q2$y~1c`_w>*G#)!YyeCoFI-8`i=-bvQ-j$LNRt7S2i~fdfLx#F7rk5bsT6r zsKlOob~`umKPlh+Z+sIT{#udnCjL5JVG}=!901`D9L956f{k#-@2-*@L2i!7e30E; z9FNR8e(BXetQI!O$bjZRs-83|C-O9m6@C7{MAi*(ZJyG7PTy~2KZ4=pUSbBJWkr}M zfI&E@Ei+dD*84Sl=$FfUBkFdED&-&S!poR{V3yn-_a$3rJ2Sq~V#jnGp~ob+=q2R^ zQNf&+%#SulkmTW(JE-zPcxXpUmIuti9o#bc-v}%UMU*aUMq#7^pnq(cAGb0sJO0A-2eap literal 0 HcmV?d00001 diff --git a/resources/builtin/repo/repo-hg.png b/resources/builtin/repo/repo-hg.png new file mode 100644 index 0000000000000000000000000000000000000000..d12c2e533941ee38c08ee61023fe092c0bccc876 GIT binary patch literal 4828 zcmb_gc{tQ<_n#SCNRkIxq7o{DB4jTQ3E69&u}_QTv6hVNjir<|$spN7nUOI_mSMEm zmyi(#m6*nm#8^gnzu(dO`{(yu*Zar&x~{pd?|trbpL0Ivoco+}yKil2BEToX2Z2BY zOwSwHLLg8$_}j$|P(mV-(GbYKB2%NY7lWa5EhnJ>@%6!tQjIr}DB$y-{;YFI4Z;4h zj>0GO8yj&^n{#)wLW17hZa>}AEv5RW&&Wfwmzf=V)B-cLdiM6Zm2CM%Bz#ZK3z~g| zyJl(i);=3EuJy0q?6!DjUX=IB_vVIOt?TTN=(gZ;oS#6g zOg(SvolWw*6$Dqi;aHpCH^3p}%YmPm^Y+sje;Rj~#fxIPY|~a`HR(0?03L}M0SgQl zQsZJ!ny3*c62pZs0+um;#3$UtryqufIJ`_s?&@D7Jn7rhKJ?{W9fEH->jQ>naBHqI zmRk4TNwv_*N&|{(+vuAd_0S`jeb{0kf_{DVi^o&o>zq4LkIgu(`O3mI5y28WX#QKJend#TPWm5hZzamNKoDtjpq7w2O zMKa*Y!!Wov41SnQl>e_kj{*|#x%`CcDo4eEC*7tuPK*td71?VckJ2L@VF%gvvGxxA zY>&?~e-`H)JJy~80EaNFpGVO4xl=sOa1tPj!5H;b_pm;>#t>`+95j&l)#>%BR>oe? zNzvGazG?wnL7edJCJs>)mmooT{zZ)dUa7wkvkP?6omtOvxNM3%;tw3l2+?%oPJ^NV zc+fCtBizo?|L^&%*wOp$_WHV=uS8H#>>&`d`TrVzLPJ@->@&aW;enabfcmrf)COI_ zg9RKQnmfi7HHF5mW}Qic7JH#^GM_Z+!1G5;ZC_KtMu-pym(S^EdEgj%z%PfU3^<3M zfY-_(?D-vUYO6-B`Pu{7TPoaA;R+zTc5~6F24O~43%YZ9sO5^Om79wV-t5njA9j+v zKnc~79ufhKHa3k8qo2*}C-gk`*8-P@9s2(jb})JH(z^-O-W%S;n=cMJ7Oi7n_H=Z0 ztP{hhjt4_TQ8fq2qq=Fs-sPUX-pA_DFL=RxZQ+2AyH%r4WYwB-|3>fS3kH9DiM9_W z3PhB)Zyy*}H_dCKcZWVnn0&vkdBNK)31Qhi~N z>2Dbs|IV>#;+&<3!|0Zijq<|tsJKHQbRyo$l2WXyt}vx?Apw!pP2c9$1@m*rIU*RZ zylG|GA-Qc(`D{y}DQ>9A&MxI!2O7J{6N9(CcL zOvT+Ffuw{Y!~!=A?Joc9%&lZ6Fbtn!bqd0c1pC|+V3`og9<&gORsV;@AK(r`46G6h zKO3J=Z6EbmnhnGxznzRXT~5mU79j0>J|h2#H^adwC-wH_TlPUu)oi7*MLc|++^NGw z+qvQaMy|BV^=PX0f*>klo<}i63)$PXVd4csbdyDD_NhM=x9GgvCZ9Bk>uHouA9PGm zT=B5JP-b_t$u&h;^YPf;jHQN7!A5sA?61z%OY*e9V{DI(z$o|cMDy&PklUEL+mJn7 zd>yONGh4$^dMido#x-2*RYhjlr(u69HxOwTH(^=Dd%sk5C9QONaPYH?aNj(>BlosICm2nGG)MqTcRwF5sw;YiH)O|hz#M@*vHho*(H8-^(BU(l)=-K1JHf<8Z ze6aHP0Uou8ai*F87{XZuVdX1PsjWcBf;*I??djJP`C~hf;J=hF`i zm?X3&cRTAK(`y8?UCr0nhN%*|@6jJc%{`E<`yBgrB2h^)t@T2Hy`8AM%j3le-|?Sy z8hQ!}8e(5(qskI4SOh+P{Z7yAz{TN~6&D`%rM#PWzkRHxky`iBDZmA*PfXI4T#V}! zwu|H;Fl~BPA*_jTMATAM2I@7eK`iUwRuX-Rkl?D;OG zRgde{S;IQ5E>N~?4N`TZ)rwz^_uAQW9uKo$6yI!492O4^J^mq(H|Ha3=5a2Ffu9gK z*S3xB5k6ty-(o6WQPkzNdw4Ok=6mye$0<^oue%c=`yA&rTo$b-yHKe7~{I$!un=WpGEC5Z7~VA z#JUgMSITkD1c6_H_Etv;D;bSE-%DMx{$B4hqQ!x(0XDr|4~cQZogYwx2!R_?V00aV zFzBbLn%&qupZS-qVjNq#zd@5Hf3dvESs~xTmC1Zyg+MtQ_~udwIvKtreMPz_`H?Na zaTvU(I@3WROhQa2d(faX(r_0tid}&9qT1FZG_y0-GTlEgzW8Bh_Jd?RVG8;9d;Q-1 zIES~1h3xi=ar60^E^d(dP4zIBr@g|$Q`Xd699SXhA?AWUOy2k}C9V}!6q#U>q=#g^ z-+KZm50^$Ajt+s_n`+F2qX9ny8H z{&vIViAUQ%i+J@GX-nSOO%qcG_kf;*!QXuG?92KgO%MLKxc>Uf8?|1|#L3dCSuneB z9P1-FpIBL}_^5R800$yHS30w7={ZI3ii?AAn4)}KS)JVHx}f~obrS1nnJYU-ayo84 zuoq0V-W%<*;aT;G_l)dgpY`N|_}>0=E@H+E)6=Yl&?lL>zlzSpY12(gjeuegMU<+} zHN0pLizUS@J$@46cDooCDY{rQF?FRrmIp+jomoiyK-U^= zz9988QtCrsN(hK$)`xYpI3W)0!D+!$6{E)&8eUd;9s7)ZQIo&MfXsIuWOajC)+y&*THOPeCE(cKq|f z{Y3rb*Ta^+N-*q6K;b~!`Kj@~0<&IC7ZzH_R{Tw>OG4{XxU(2&^7$4dCH8%tz>V>& zv$s6o`(9g~EIyg`hT+ zpOK0kQ6Bj0AX`tH`nM(-3!Jw}U|lw@?Ntxd|LfkoTX&CdVpm*SV$3R=zNJu~JK^x? zwWr~*;mfbdWvBGLAHU}VGwl!Qimr|0toCPlu-R~>J`G8=+x;K0B3 zmTfGUzX!PZUZ_5B8Sl3p$cm&0(cW>|w8fJiK;UGSS3gZ^d{!#KF&|$l%Ygdz{`hr%o(}9)wJ{7sIMf1*E?s7mb4VaL#!|Hz} znE}=o=hdIcx5NxFp_Fe$$&JnLqBV~J4+RQTp?}lg8hlZFTS}&m^kq**19BMW<29SO zfPHMT^Z00&B%6q9jM5~@frnZ+W9G5oyGab!NN2-9Mqjl8w4ld{S@Ylq_`{%!KezW+ z%f_XV1{V`$WqdgJ0X4Z$^S0K974o5>6+dPk$kwh#aRX|GP;}M2x90Bn&;sA7d??Vf z)sZx_(D|YMA8(SW*NPzk4+%AYYafmqKA&^vG&o<4@KKu9W?>93;nQG&;3u#Lj_{pY zCPL4~dQE1kCoR7NtAKY+bWNTA`N+}LSapk43t(tL|A}GJmB~ad;Boab=ztOlnn~ruzEzuG2f+ z&cyM=reR8N`39^U$As3A40h}oO5K`R=2t40w8J-j3%^_*{tE5%eJ%x@nt%zBlozsT zXd<25wfNQHotKSRJmYIj&(#2L*J@v2AK*BkzG+)kVRh72a4x{DsImFP@w~arN7-Q> zx-yc`1OW5vPRa z(Bf9B#f2n&iL^~M$(`P-J#o^&Q$X*JjPk27APX>&%xh93XXPW!NJLYbC)lq5Z{fEC zgnJgo5nD;TORi?@C#Z-Q)dD;A!A{LK_@6M{PoJx(?)h1k6zKB|aq-FOtl6c_>AczzIh&uZkG*BVnQh zRwZ8n7aYou+(h1a55r=(@O$tsKRu!5WEeskSLJ1|=hn|A>mm{={*S(=8Y72-DkfhN zW6KwLS0=n8|H}WDZhhfHTyZv(FHf|bqMMm88&3hL^>nlV*E#*Tb7N@Hfc|G|7A^ncm!p-k8lKP u{tzQ)N`A-*H-%w=6+q%ef~$XKh=6sn6uhok%k+T%k}@^6G!T$PzKa*Pak1TL>8sg`p54OC$T5BwLgjlq6diJofB^D_do2 zY*SgXWLK8#?lau){r&N~_jTtr^UOJ)v%SyzoM&D$6M5Z0>j)D!6AT7BqN9Dq7zTr* zsV@dP=)@-=76*fIM(SKqzu^s^{}Mv00Y4T#rl9`(3cOr1s!Uh*QrTrvhTQj4@I(<; zdj3n7EJs?|9D-$_*#Gn(RkCicoK5Y0;X0EztMzdO`TR`$EZg%%NAfmU zrf-r*r<2hVl7drq$f!G6SgebY7I&R_L)nEfK1w6r1`=hQ!~ z<(JCW#)=2aeQthr8ei>cqPQ&i{;K0@+}d1R-^YKs!VJt+Sr@GHsVm5eH&mZ1oV41e zmm+b1!HG70W?|(1h^s%k`xVB2Pk6fLZ!m#KG}i7rA12InUr^B9GHu{aet zGtl6I>MY-wdW;mo;*=p>P<)quyQ=4S2$_=_y)ISX?y$|76ijA@)U90eog)1AKt7-@ z0m?N=FUaZ|Km({ehZLaD_WyoA0*r@;f4;?By+u*rb09dm+ZP8|4x8&`XOI4h zgJzF~W{;w>4h2KPL2K0MKo*D$1RhDWIszQ%kOQRu_mJw7U@*?dOk$R%MfARCS^1_! zxT3t7&SwgOfeIF{btC_!a)aLBshYbB{A$!}7p&S{2~zuBrv+wcpwe=?z7;i(tSt+} zgqoo;HIF1#ZH6s1sGeeVgf5o+7&e2gNxv;Bgl~%Avy*_bmfqiO`=)cht~69u!)|#2HF9{@|k~ghqSV;9>822dz^%IP9GfP~icE z{^o=h{og}yGFbTB*52mCmF|{$jhoIasOL0@hK3;+)HKqoqnbyB3HWdE52 zRcgu{C-TS74 ziuJ1i;SYZ&+mw&(MQ-#T7>H$-!={}qM2Or=NGLN#kGF$xyLmUYC2!gF`>YJavIj|b zYxSOr!Hs-(tXE+H?>YRBsYY6Fh>LxO|GB#P12lN*_8&AGEKHpb2k5Kv|8I-@%0tJw zR!Yo!uQ%L&_8##gitNoru~$c!1*MnAwWDS4e|AZFVrf#P*id>Z&H9>}?5~!rmO(<7 zR#uQ0_w7>m6}JMxqOJim!uX~1<<6DJt-)(w70W87S2~*qv>e_JD{#?Ag>U$V%e|45 zlx!LMoi?P*Y^mgNmPu{7D9A_QGZzC$3{Z|x{+cJbl86DHYb2Yg-kkNt=mwa#=0`> z|LlJT#47|H3tx0h)%eW88Tf5sMYGJi$W4Qh&t9K;Bry$LE&lC`qeIu+O(}x+8@dR8 zF66^^P~R7{S=~yuT;CUZ2{;D=^BjQH6)Zd88FoPc9`NRz7~iLN@x6Rq^i#_Y&(k+% zy|D2pfhL_laM_lMxFT7R3eAZ}+7BK`ywx9As@%^gwXZM}1R2nXPPwPc`LmJ4&26$9 z;*_&3r8oW>|I*rolS!?i!t3Ld0Ym$i^{)H1G^ev4hWMAxk(ok-pO6GVDQzhr`>kj4 zc*>vJK4P9pRvO=0K*PWs|qjuKDzyr*GiPjO>YI5V=oT?!8=H-@+~RTN^=X7EX# z50TB*%63z98CQO8KN8^;wn}P^ms{Fidg(bexRo@BQW8*G^2#`SH5PUxVS<&EQVm|v z!^T8GSOag(=0=Wgr_QGMi0)9LcqIA^QJ}575`F5q@$Uy8E@bXW zSO$kL^(ZFF5Pl0^acg5kYy{WlsD9wsg1~5Ezso~N$A z&=rMg3b(%S^sOW)P`HnQ94AG=*rK@PmKEHUIL?uWNfp*#y?tGf&Il@^qr8k&Iqyu; zo>V`(9^_pt{q*A%+*=>_ZUHg(_USj5Zp)5O&b+aYov=+uBv)1I;L zmF(`|)y_3twc4to6gr9%l0MMUp}$~#0;a9kxUOBxWh6?o1Ftyp^26fFQ~39rRAfb? zUBY^3Njd?R<&p##_{Fqa&H8;JKUQ7iCl3V=txgn)yS-$T-GuW-bX-Dv2=lH$%Cpz$ z1W{mZja(Ot=e%5aP6|6lL0WsMA0Sti=rL;qJ0>5drRhy7zg5W>3Y0vr6|->n^gtCt zk8)|@v^fX$Zpq5$d5)QR;9XaHlA%_rz*vDbf~A1A4*>_9mB)j%k|K+4=V5bw(DeISsNhFV$MkKqN2Hb-Y&Dug2se z>0%q(f~<^;)#}BsV?Q1F8bT|h4tWWH^DCE=TyqW5ObmVJn%22n#5)^TbQ7(PZEtx| z%qE`rzYMfE3ISpVeiUW^C%>ege2~ueDlj^bhJYW{a+MhMv-MSCe=&%k_+AQ@OSWE_dwFcNg-rD4!y~<3{)p z_1JTqm^1fph@aRnsguMqxTR0AoQ?2~eY4x@=Fwf}#T9#Ezu1#4s^qp2>@mf#JLwO| z_h!MdLATI_XEIE~tI!4dT|@aWn<_7kOColtZ8$`fn+0$w1S)WXWk}#kaF!5B0D+c+ zqu8;zjNlWPo_??<5_pP03y_RCvFvbs3M`5N{Bq!W_TR3wbU;K-3_gIfwe7Pz29Y34GOzE6cTy)&TUoU(ue`wVlATt7t8(aCvQ2i@c!9V zspM`E@kn`?Ci@W(=4|SeG*LsAnLq+PN5G0H`nLDqhmSpzTvtOObfU_VE|#`1MS-N5$t`ff=mU*_t`+$+z6K+x4DtS#ONtOf*DU$alGix z`2u;!KSZ zvg3~L(#%q&8P=-?l15UQRGBCG_%56pC$|2VM?hWZfjoS{sL_oBj>2xX78VwoO1+|b zmR=P*X&^vC5My2^ge_1{BBp+xxCWh|RVqiJZ+BQrV=p^p=(h(n07&Cay?=)yBzW`e z4MXL#JW8c3=sY?!Hjg zplq)vVv<9j+Y-x3R38*^?L*WNAW#HVHA>aFh&b0;(uE3`BIFM}AqJU2F&63-mjAuI z^OQ(N_KP0~6c&!T)o3vDvMN>4`{bnZD`ei@CzhO@&lYq*TBw$*jBb|YWTG(7Oqk-q`LqM&*xT8W<( zznb?i%?;K{d$L_ z?>)wkG@fjp{5T$c%hK$g++lD9t;FmLbtS&s{e`x9BM)CAi}S>HiQ>-8c3wR2d`Bu^ z5qU-@A6H_W$@81fv|E^nFTLhqle}lF!?``MF>sek1JrlFAy?>HYkz-P zW7VjFFgE-s9CNW_I5VA$lW1E#V{ZORw=HF>XOCx1?WHuDQyj$GweBrB_muPu@7yIl z)mgdMtI@)QW>OBq#htD}42)TfOJgk`hnCyGj3}<@n2A zLn{;=oy$*ojBI)^>J_S`z1rDx9$9Sk^xMJfnf$yDIofYy`9Lpnc7&-<$V7tUg zUk`wUT;gya2daq!BbvWg_9z^D#MP0fCSh|1Z4+KHYk1(2PTV?lF~hv{w`kG;nmQbC zCa6ilmw50f9{g<{V?6H2$ZW$Xo@T0bV1MdBPCT%^HOFNOqaO(q0nE2Umlvi%2QEx@ z@p0I=UU*adT*K zL#);ABb$x8@nVNxm1O>3XK^>qcY69rR0b^!z-ggB#Yo^@f%T6Qn6aIzPzF>m7S4d; o2VkMJ{J?<>U8pj2@E?qq6qTgNnnM?ZsDG~MXc}B8)WC-R6HvkoeE Date: Fri, 4 Aug 2017 05:29:49 -0700 Subject: [PATCH 09/24] Reduce callsites to "ArcanistDifferentialRevisionStatus" in Phabricator Summary: Ref T2543. These are currently numeric values, like "0" and "3". I want to replace them with strings, like "accepted", and move definitions from Arcanist to Phabricator. To set the stage for this, reduce the number of callsites where Phabricator invokes `ArcanistDifferentialRevisionStatus`. This is just the easy ones. I'll hold this until the release cut. Test Plan: - Called `differential.find`. - Called `differential.getrevision`. - Called `differential.query`. - Removed all reviewers from a revision, saw warning. - Abandoned the no-reviewers revision, no more warning. - Attached a revision to a task to get it to show the state icon with the status on a tooltip. - Viewed revision bucketing on dashboard. - Used `bin/search index` to reindex a revision. - Hit the "Land Revision" endpoint. I didn't explicitly test these cases: - Doorkeeper Asana integration, since setup takes a thousand years. - Disambiguation logic when multiple hashes match, since setup is also very involved. - Releeph because it's Releeph. Reviewers: chad Reviewed By: chad Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T2543 Differential Revision: https://secure.phabricator.com/D18339 --- .../DifferentialFindConduitAPIMethod.php | 4 +- ...ifferentialGetRevisionConduitAPIMethod.php | 4 +- .../DifferentialQueryConduitAPIMethod.php | 4 +- .../customfield/DifferentialBranchField.php | 9 ++-- .../DifferentialReviewersField.php | 3 +- ...alDoorkeeperRevisionFeedStoryPublisher.php | 6 +-- .../phid/DifferentialRevisionPHIDType.php | 3 +- ...tialRevisionRequiredActionResultBucket.php | 15 ++---- .../DifferentialRevisionResultBucket.php | 3 +- .../DifferentialRevisionSearchEngine.php | 4 +- .../DifferentialRevisionFulltextEngine.php | 3 +- .../storage/DifferentialRevision.php | 5 ++ .../DiffusionLowLevelCommitFieldsQuery.php | 3 +- .../DrydockLandRepositoryOperation.php | 49 +++++++++---------- ...entialReleephRequestFieldSpecification.php | 3 +- 15 files changed, 49 insertions(+), 69 deletions(-) diff --git a/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php index 92dce067fe..79d3fc5d7d 100644 --- a/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php @@ -88,9 +88,7 @@ final class DifferentialFindConduitAPIMethod 'uri' => PhabricatorEnv::getProductionURI('/D'.$id), 'dateCreated' => $revision->getDateCreated(), 'authorPHID' => $revision->getAuthorPHID(), - 'statusName' => - ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( - $revision->getStatus()), + 'statusName' => $revision->getStatusDisplayName(), 'sourcePath' => $diff->getSourcePath(), ); } diff --git a/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php index b05efb5c15..9109849292 100644 --- a/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php @@ -83,9 +83,7 @@ final class DifferentialGetRevisionConduitAPIMethod 'uri' => PhabricatorEnv::getURI('/D'.$revision->getID()), 'title' => $revision->getTitle(), 'status' => $revision->getStatus(), - 'statusName' => - ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( - $revision->getStatus()), + 'statusName' => $revision->getStatusDisplayName(), 'summary' => $revision->getSummary(), 'testPlan' => $revision->getTestPlan(), 'lineCount' => $revision->getLineCount(), diff --git a/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php index 3b88087a67..1051983324 100644 --- a/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php @@ -221,9 +221,7 @@ final class DifferentialQueryConduitAPIMethod 'dateModified' => $revision->getDateModified(), 'authorPHID' => $revision->getAuthorPHID(), 'status' => $revision->getStatus(), - 'statusName' => - ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( - $revision->getStatus()), + 'statusName' => $revision->getStatusDisplayName(), 'properties' => $revision->getProperties(), 'branch' => $diff->getBranch(), 'summary' => $revision->getSummary(), diff --git a/src/applications/differential/customfield/DifferentialBranchField.php b/src/applications/differential/customfield/DifferentialBranchField.php index 16be0ce0c9..2387e3cd3f 100644 --- a/src/applications/differential/customfield/DifferentialBranchField.php +++ b/src/applications/differential/customfield/DifferentialBranchField.php @@ -76,16 +76,17 @@ final class DifferentialBranchField PhabricatorApplicationTransactionEditor $editor, array $xactions) { - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; + $revision = $this->getObject(); // Show the "BRANCH" section only if there's a new diff or the revision // is "Accepted". - if ((!$editor->getDiffUpdateTransaction($xactions)) && - ($this->getObject()->getStatus() != $status_accepted)) { + $is_update = (bool)$editor->getDiffUpdateTransaction($xactions); + $is_accepted = $revision->isAccepted(); + if (!$is_update && !$is_accepted) { return; } - $branch = $this->getBranchDescription($this->getObject()->getActiveDiff()); + $branch = $this->getBranchDescription($revision->getActiveDiff()); if ($branch === null) { return; } diff --git a/src/applications/differential/customfield/DifferentialReviewersField.php b/src/applications/differential/customfield/DifferentialReviewersField.php index 7633bfb492..f835854e2f 100644 --- a/src/applications/differential/customfield/DifferentialReviewersField.php +++ b/src/applications/differential/customfield/DifferentialReviewersField.php @@ -68,8 +68,7 @@ final class DifferentialReviewersField public function getWarningsForRevisionHeader(array $handles) { $revision = $this->getObject(); - $status_needs_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; - if ($revision->getStatus() != $status_needs_review) { + if (!$revision->isNeedsReview()) { return array(); } diff --git a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php index 9583486c98..59223f20da 100644 --- a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php +++ b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php @@ -35,8 +35,7 @@ final class DifferentialDoorkeeperRevisionFeedStoryPublisher } public function getActiveUserPHIDs($object) { - $status = $object->getStatus(); - if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { + if ($object->isNeedsReview()) { return $object->getReviewerPHIDs(); } else { return array(); @@ -44,8 +43,7 @@ final class DifferentialDoorkeeperRevisionFeedStoryPublisher } public function getPassiveUserPHIDs($object) { - $status = $object->getStatus(); - if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { + if ($object->isNeedsReview()) { return array(); } else { return $object->getReviewerPHIDs(); diff --git a/src/applications/differential/phid/DifferentialRevisionPHIDType.php b/src/applications/differential/phid/DifferentialRevisionPHIDType.php index 5a6cf701ae..8547ec9e83 100644 --- a/src/applications/differential/phid/DifferentialRevisionPHIDType.php +++ b/src/applications/differential/phid/DifferentialRevisionPHIDType.php @@ -50,8 +50,7 @@ final class DifferentialRevisionPHIDType extends PhabricatorPHIDType { $icon = DifferentialRevisionStatus::getRevisionStatusIcon($status); $color = DifferentialRevisionStatus::getRevisionStatusColor($status); - $name = ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( - $status); + $name = $revision->getStatusDisplayName(); $handle ->setStateIcon($icon) diff --git a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php index 6d83d97a47..3e6daa6317 100644 --- a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php +++ b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php @@ -134,13 +134,11 @@ final class DifferentialRevisionRequiredActionResultBucket } private function filterShouldLand(array $phids) { - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; - $objects = $this->getRevisionsAuthored($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { - if ($object->getStatus() != $status_accepted) { + if (!$object->isAccepted()) { continue; } @@ -175,13 +173,11 @@ final class DifferentialRevisionRequiredActionResultBucket } private function filterWaitingForReview(array $phids) { - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; - $objects = $this->getRevisionsAuthored($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { - if ($object->getStatus() != $status_review) { + if (!$object->isNeedsReview()) { continue; } @@ -217,16 +213,11 @@ final class DifferentialRevisionRequiredActionResultBucket } private function filterWaitingOnOtherReviewers(array $phids) { - $statuses = array( - ArcanistDifferentialRevisionStatus::NEEDS_REVIEW, - ); - $statuses = array_fuse($statuses); - $objects = $this->getRevisionsNotAuthored($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { - if (!isset($statuses[$object->getStatus()])) { + if (!$object->isNeedsReview()) { continue; } diff --git a/src/applications/differential/query/DifferentialRevisionResultBucket.php b/src/applications/differential/query/DifferentialRevisionResultBucket.php index 762e2d97f4..54705649eb 100644 --- a/src/applications/differential/query/DifferentialRevisionResultBucket.php +++ b/src/applications/differential/query/DifferentialRevisionResultBucket.php @@ -15,9 +15,8 @@ abstract class DifferentialRevisionResultBucket $objects = $this->getRevisionsNotAuthored($objects, $phids); - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; foreach ($objects as $key => $object) { - if ($object->getStatus() != $status_review) { + if (!$object->isNeedsReview()) { continue; } diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php index 4248119cb9..ef88c0ea1c 100644 --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -235,11 +235,9 @@ final class DifferentialRevisionSearchEngine } private function loadUnlandedDependencies(array $revisions) { - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; - $phids = array(); foreach ($revisions as $revision) { - if ($revision->getStatus() != $status_accepted) { + if (!$revision->isAccepted()) { continue; } diff --git a/src/applications/differential/search/DifferentialRevisionFulltextEngine.php b/src/applications/differential/search/DifferentialRevisionFulltextEngine.php index 45639edbbe..cdb9f1a86d 100644 --- a/src/applications/differential/search/DifferentialRevisionFulltextEngine.php +++ b/src/applications/differential/search/DifferentialRevisionFulltextEngine.php @@ -34,8 +34,7 @@ final class DifferentialRevisionFulltextEngine // If a revision needs review, the owners are the reviewers. Otherwise, the // owner is the author (e.g., accepted, rejected, closed). - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; - if ($revision->getStatus() == $status_review) { + if ($revision->isNeedsReview()) { $reviewers = $revision->getReviewerPHIDs(); $reviewers = array_fuse($reviewers); diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 7ad01c02ca..cd6df4b6e5 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -626,6 +626,11 @@ final class DifferentialRevision extends DifferentialDAO return ($this->getStatus() == $status_accepted); } + public function isNeedsReview() { + $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; + return ($this->getStatus() == $status_review); + } + public function getStatusIcon() { $map = array( ArcanistDifferentialRevisionStatus::NEEDS_REVIEW diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php index ab8f5e3e2e..a0548d5178 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php @@ -122,9 +122,8 @@ final class DiffusionLowLevelCommitFieldsQuery $revisions = array_reverse($revisions); // Try to find an accepted revision first. - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; foreach ($revisions as $revision) { - if ($revision->getStatus() == $status_accepted) { + if ($revision->isAccepted()) { return $revision; } } diff --git a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php index 4e306772a3..1ccc82eb7b 100644 --- a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php +++ b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php @@ -289,31 +289,30 @@ final class DrydockLandRepositoryOperation ); } - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; - if ($revision->getStatus() != $status_accepted) { - switch ($revision->getStatus()) { - case ArcanistDifferentialRevisionStatus::CLOSED: - return array( - 'title' => pht('Revision Closed'), - 'body' => pht( - 'This revision has already been closed. Only open, accepted '. - 'revisions may land.'), - ); - case ArcanistDifferentialRevisionStatus::ABANDONED: - return array( - 'title' => pht('Revision Abandoned'), - 'body' => pht( - 'This revision has been abandoned. Only accepted revisions '. - 'may land.'), - ); - default: - return array( - 'title' => pht('Revision Not Accepted'), - 'body' => pht( - 'This revision is still under review. Only revisions which '. - 'have been accepted may land.'), - ); - } + if ($revision->isAccepted()) { + // We can land accepted revisions, so continue below. Otherwise, raise + // an error with tailored messaging for the most common cases. + } else if ($revision->isAbandoned()) { + return array( + 'title' => pht('Revision Abandoned'), + 'body' => pht( + 'This revision has been abandoned. Only accepted revisions '. + 'may land.'), + ); + } else if ($revision->isClosed()) { + return array( + 'title' => pht('Revision Closed'), + 'body' => pht( + 'This revision has already been closed. Only open, accepted '. + 'revisions may land.'), + ); + } else { + return array( + 'title' => pht('Revision Not Accepted'), + 'body' => pht( + 'This revision is still under review. Only revisions which '. + 'have been accepted may land.'), + ); } // Check for other operations. Eventually this should probably be more diff --git a/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php b/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php index bdb63e6c9c..dd67f5bc59 100644 --- a/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php +++ b/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php @@ -79,8 +79,7 @@ final class DifferentialReleephRequestFieldSpecification extends Phobject { return null; } - $status = $this->getRevision()->getStatus(); - if ($status == ArcanistDifferentialRevisionStatus::CLOSED) { + if ($this->getRevision()->isClosed()) { $verb = $tense[$this->releephAction]['past']; } else { $verb = $tense[$this->releephAction]['future']; From 70088f7eec8fe8e33c7f6deae9dc289395ace42c Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Aug 2017 08:08:18 -0700 Subject: [PATCH 10/24] Continue reducing callsites to ArcanistDifferentialRevisionStatus Summary: Ref T2543. Further consolidates status management into DifferentialRevisionStatus. One change I'm making here is internally renaming "CLOSED" to "PUBLISHED". The UI will continue to say "Closed", at least for now, but this should make the code more clear because we care about "is closed, exactly" vs "is any closed status (closed, abandoned, sometimes accepted)". This distinction is more obvious as `isClosed()` vs `isPublished()` than, e.g., `isClosedWithExactlyTheClosedStatus()` or something. I think "Published" is generally more clear, too, and more consistent with modern language (e.g., "pre-publish review" replacing "pre-commit review" to make it more clear what we mean in Git/Mercurial). I've removed the IN_PREPARATION status since this was just earlier groundwork for "Draft" and not actually used, and under the newer plan I'm trying to just abandon `ArcanistDifferentialRevisionStatus` entirely (or, at least, substantially). Test Plan: - Viewed revisions. - Viewed revision list. - Viewed revisions linked to a task in Maniphest. - Viewed revision graph of dependencies in Differential. - Grepped for `COLOR_STATUS_...` constants. - Grepped for removed method `getRevisionStatusIcon()` (no callsites). - Grepped for removed method `renderFullDescription()` (one callsite, replaced with just building a `TagView` inline). - Grepped for removed method `isClosedStatus()` (no callsites after other changes). Reviewers: chad Reviewed By: chad Maniphest Tasks: T2543 Differential Revision: https://secure.phabricator.com/D18340 --- .../constants/DifferentialRevisionStatus.php | 178 +++++++++++------- .../DifferentialRevisionViewController.php | 10 +- .../phid/DifferentialRevisionPHIDType.php | 4 +- .../storage/DifferentialRevision.php | 39 ++-- .../view/DifferentialRevisionListView.php | 5 +- .../graph/DifferentialRevisionGraph.php | 4 +- 6 files changed, 144 insertions(+), 96 deletions(-) diff --git a/src/applications/differential/constants/DifferentialRevisionStatus.php b/src/applications/differential/constants/DifferentialRevisionStatus.php index 38e6a2dd49..0b2abe51fe 100644 --- a/src/applications/differential/constants/DifferentialRevisionStatus.php +++ b/src/applications/differential/constants/DifferentialRevisionStatus.php @@ -1,74 +1,130 @@ - self::COLOR_STATUS_DEFAULT, - ArcanistDifferentialRevisionStatus::NEEDS_REVISION => - self::COLOR_STATUS_RED, - ArcanistDifferentialRevisionStatus::CHANGES_PLANNED => - self::COLOR_STATUS_RED, - ArcanistDifferentialRevisionStatus::ACCEPTED => - self::COLOR_STATUS_GREEN, - ArcanistDifferentialRevisionStatus::CLOSED => - self::COLOR_STATUS_DARK, - ArcanistDifferentialRevisionStatus::ABANDONED => - self::COLOR_STATUS_DARK, - ArcanistDifferentialRevisionStatus::IN_PREPARATION => - self::COLOR_STATUS_BLUE, - ); - return idx($map, $status, $default); + public function getIcon() { + return idx($this->spec, 'icon'); } - public static function getRevisionStatusIcon($status) { - $default = 'fa-square-o bluegrey'; - - $map = array( - ArcanistDifferentialRevisionStatus::NEEDS_REVIEW => - 'fa-square-o bluegrey', - ArcanistDifferentialRevisionStatus::NEEDS_REVISION => - 'fa-refresh', - ArcanistDifferentialRevisionStatus::CHANGES_PLANNED => - 'fa-headphones', - ArcanistDifferentialRevisionStatus::ACCEPTED => - 'fa-check', - ArcanistDifferentialRevisionStatus::CLOSED => - 'fa-check-square-o', - ArcanistDifferentialRevisionStatus::ABANDONED => - 'fa-plane', - ArcanistDifferentialRevisionStatus::IN_PREPARATION => - 'fa-question-circle', - ); - return idx($map, $status, $default); + public function getIconColor() { + return idx($this->spec, 'color.icon', 'bluegrey'); } - public static function renderFullDescription($status) { - $status_name = - ArcanistDifferentialRevisionStatus::getNameForRevisionStatus($status); + public function getTagColor() { + return idx($this->spec, 'color.tag', 'bluegrey'); + } - $tag = id(new PHUITagView()) - ->setName($status_name) - ->setIcon(self::getRevisionStatusIcon($status)) - ->setColor(self::getRevisionStatusColor($status)) - ->setType(PHUITagView::TYPE_SHADE); + public function getDisplayName() { + return idx($this->spec, 'name'); + } - return $tag; + public function isClosedStatus() { + return idx($this->spec, 'closed'); + } + + public function isAbandoned() { + return ($this->key === self::ABANDONED); + } + + public function isAccepted() { + return ($this->key === self::ACCEPTED); + } + + public function isNeedsReview() { + return ($this->key === self::NEEDS_REVIEW); + } + + public static function newForLegacyStatus($legacy_status) { + $result = new self(); + + $map = self::getMap(); + foreach ($map as $key => $spec) { + if (!isset($spec['legacy'])) { + continue; + } + + if ($spec['legacy'] != $legacy_status) { + continue; + } + + $result->key = $key; + $result->spec = $spec; + break; + } + + return $result; + } + + private static function getMap() { + $close_on_accept = PhabricatorEnv::getEnvConfig( + 'differential.close-on-accept'); + + return array( + self::NEEDS_REVIEW => array( + 'name' => pht('Needs Review'), + 'legacy' => 0, + 'icon' => 'fa-code', + 'closed' => false, + 'color.icon' => 'grey', + 'color.tag' => 'bluegrey', + 'color.ansi' => 'magenta', + ), + self::NEEDS_REVISION => array( + 'name' => pht('Needs Revision'), + 'legacy' => 1, + 'icon' => 'fa-refresh', + 'closed' => false, + 'color.icon' => 'red', + 'color.tag' => 'red', + 'color.ansi' => 'red', + ), + self::CHANGES_PLANNED => array( + 'name' => pht('Changes Planned'), + 'legacy' => 5, + 'icon' => 'fa-headphones', + 'closed' => false, + 'color.icon' => 'red', + 'color.tag' => 'red', + 'color.ansi' => 'red', + ), + self::ACCEPTED => array( + 'name' => pht('Accepted'), + 'legacy' => 2, + 'icon' => 'fa-check', + 'closed' => $close_on_accept, + 'color.icon' => 'green', + 'color.tag' => 'green', + 'color.ansi' => 'green', + ), + self::PUBLISHED => array( + 'name' => pht('Closed'), + 'legacy' => 3, + 'icon' => 'fa-check-square-o', + 'closed' => true, + 'color.icon' => 'black', + 'color.tag' => 'indigo', + 'color.ansi' => 'cyan', + ), + self::ABANDONED => array( + 'name' => pht('Abandoned'), + 'legacy' => 4, + 'icon' => 'fa-plane', + 'closed' => true, + 'color.icon' => 'black', + 'color.tag' => 'indigo', + 'color.ansi' => null, + ), + ); } public static function getClosedStatuses() { @@ -100,8 +156,4 @@ final class DifferentialRevisionStatus extends Phobject { ); } - public static function isClosedStatus($status) { - return in_array($status, self::getClosedStatuses()); - } - } diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index ab03ee6d81..5c7f5104f5 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -508,11 +508,13 @@ final class DifferentialRevisionViewController extends DifferentialController { ->setPolicyObject($revision) ->setHeaderIcon('fa-cog'); - $status = $revision->getStatus(); - $status_name = - DifferentialRevisionStatus::renderFullDescription($status); + $status_tag = id(new PHUITagView()) + ->setName($revision->getStatusDisplayName()) + ->setIcon($revision->getStatusIcon()) + ->setColor($revision->getStatusIconColor()) + ->setType(PHUITagView::TYPE_SHADE); - $view->addProperty(PHUIHeaderView::PROPERTY_STATUS, $status_name); + $view->addProperty(PHUIHeaderView::PROPERTY_STATUS, $status_tag); return $view; } diff --git a/src/applications/differential/phid/DifferentialRevisionPHIDType.php b/src/applications/differential/phid/DifferentialRevisionPHIDType.php index 8547ec9e83..d652a8c056 100644 --- a/src/applications/differential/phid/DifferentialRevisionPHIDType.php +++ b/src/applications/differential/phid/DifferentialRevisionPHIDType.php @@ -48,8 +48,8 @@ final class DifferentialRevisionPHIDType extends PhabricatorPHIDType { $status = $revision->getStatus(); - $icon = DifferentialRevisionStatus::getRevisionStatusIcon($status); - $color = DifferentialRevisionStatus::getRevisionStatusColor($status); + $icon = $revision->getStatusIcon($status); + $color = $revision->getStatusIconColor($status); $name = $revision->getStatusDisplayName(); $handle diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index cd6df4b6e5..c2856f5d20 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -613,47 +613,36 @@ final class DifferentialRevision extends DifferentialDAO } public function isClosed() { - return DifferentialRevisionStatus::isClosedStatus($this->getStatus()); + return $this->getStatusObject()->isClosedStatus(); } public function isAbandoned() { - $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED; - return ($this->getStatus() == $status_abandoned); + return $this->getStatusObject()->isAbandoned(); } public function isAccepted() { - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; - return ($this->getStatus() == $status_accepted); + return $this->getStatusObject()->isAccepted(); } public function isNeedsReview() { - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; - return ($this->getStatus() == $status_review); + return $this->getStatusObject()->isNeedsReview(); } public function getStatusIcon() { - $map = array( - ArcanistDifferentialRevisionStatus::NEEDS_REVIEW - => 'fa-code grey', - ArcanistDifferentialRevisionStatus::NEEDS_REVISION - => 'fa-refresh red', - ArcanistDifferentialRevisionStatus::CHANGES_PLANNED - => 'fa-headphones red', - ArcanistDifferentialRevisionStatus::ACCEPTED - => 'fa-check green', - ArcanistDifferentialRevisionStatus::CLOSED - => 'fa-check-square-o black', - ArcanistDifferentialRevisionStatus::ABANDONED - => 'fa-plane black', - ); - - return idx($map, $this->getStatus()); + return $this->getStatusObject()->getIcon(); } public function getStatusDisplayName() { + return $this->getStatusObject()->getDisplayName(); + } + + public function getStatusIconColor() { + return $this->getStatusObject()->getIconColor(); + } + + public function getStatusObject() { $status = $this->getStatus(); - return ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( - $status); + return DifferentialRevisionStatus::newForLegacyStatus($status); } public function getFlag(PhabricatorUser $viewer) { diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php index 5373192b31..ed97435746 100644 --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -145,8 +145,11 @@ final class DifferentialRevisionListView extends AphrontView { $item->setDisabled(true); } + $icon = $revision->getStatusIcon(); + $color = $revision->getStatusIconColor(); + $item->setStatusIcon( - $revision->getStatusIcon(), + "{$icon} {$color}", $revision->getStatusDisplayName()); $list->addItem($item); diff --git a/src/infrastructure/graph/DifferentialRevisionGraph.php b/src/infrastructure/graph/DifferentialRevisionGraph.php index 892540f610..715820e9e3 100644 --- a/src/infrastructure/graph/DifferentialRevisionGraph.php +++ b/src/infrastructure/graph/DifferentialRevisionGraph.php @@ -27,10 +27,12 @@ final class DifferentialRevisionGraph if ($object) { $status_icon = $object->getStatusIcon(); + $status_color = $object->getStatusIconColor(); $status_name = $object->getStatusDisplayName(); $status = array( - id(new PHUIIconView())->setIcon($status_icon), + id(new PHUIIconView()) + ->setIcon($status_icon, $status_color), ' ', $status_name, ); From 03ab7224bb4ef1b89fa9f813259f946e2e6a1c9d Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Aug 2017 08:20:20 -0700 Subject: [PATCH 11/24] Reduce STATUS_CLOSED (now internally "Published") revision status callsites Summary: Ref T2543. Add `isPublished()` to mean: exactly the status 'closed', which is now interally called 'published', but still shown as 'closed' to users. We have some callsites which are about "exactly that status", vs "any 'closed' status", e.g. including "abandoned". This also introduces `isChangePlanned()`, which felt less awkward than `isChangesPlanned()` but more consistent than `hasChangesPlanned()` or `isStatusChangesPlanned()` or similar. Test Plan: `grep`, loaded revisions, requested review. Reviewers: chad Reviewed By: chad Maniphest Tasks: T2543 Differential Revision: https://secure.phabricator.com/D18341 --- .../DifferentialUpdateRevisionConduitAPIMethod.php | 2 +- .../differential/constants/DifferentialRevisionStatus.php | 8 ++++++++ .../differential/storage/DifferentialRevision.php | 8 ++++++++ .../DifferentialRevisionPlanChangesTransaction.php | 4 +--- .../xaction/DifferentialRevisionReopenTransaction.php | 5 +---- .../DifferentialRevisionRequestReviewTransaction.php | 3 +-- .../PhabricatorRepositoryCommitMessageParserWorker.php | 5 +---- 7 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php index 3ad5b07564..d9b0779211 100644 --- a/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php @@ -69,7 +69,7 @@ final class DifferentialUpdateRevisionConduitAPIMethod throw new ConduitException('ERR_BAD_REVISION'); } - if ($revision->getStatus() == ArcanistDifferentialRevisionStatus::CLOSED) { + if ($revision->isPublished()) { throw new ConduitException('ERR_CLOSED'); } diff --git a/src/applications/differential/constants/DifferentialRevisionStatus.php b/src/applications/differential/constants/DifferentialRevisionStatus.php index 0b2abe51fe..caca7ed85b 100644 --- a/src/applications/differential/constants/DifferentialRevisionStatus.php +++ b/src/applications/differential/constants/DifferentialRevisionStatus.php @@ -44,6 +44,14 @@ final class DifferentialRevisionStatus extends Phobject { return ($this->key === self::NEEDS_REVIEW); } + public function isPublished() { + return ($this->key === self::PUBLISHED); + } + + public function isChangePlanned() { + return ($this->key === self::CHANGES_PLANNED); + } + public static function newForLegacyStatus($legacy_status) { $result = new self(); diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index c2856f5d20..a6ce99080a 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -628,6 +628,14 @@ final class DifferentialRevision extends DifferentialDAO return $this->getStatusObject()->isNeedsReview(); } + public function isChangePlanned() { + return $this->getStatusObject()->isChangePlanned(); + } + + public function isPublished() { + return $this->getStatusObject()->isPublished(); + } + public function getStatusIcon() { return $this->getStatusObject()->getIcon(); } diff --git a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php index e7cdaa2455..58152bdb67 100644 --- a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php @@ -56,9 +56,7 @@ final class DifferentialRevisionPlanChangesTransaction } protected function validateAction($object, PhabricatorUser $viewer) { - $status_planned = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED; - - if ($object->getStatus() == $status_planned) { + if ($object->isChangePlanned()) { throw new Exception( pht( 'You can not request review of this revision because this '. diff --git a/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php index 84015b9286..e2b217603a 100644 --- a/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php @@ -39,10 +39,7 @@ final class DifferentialRevisionReopenTransaction } protected function validateAction($object, PhabricatorUser $viewer) { - // Note that we're testing for "Closed", exactly, not just any closed - // status. - $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; - if ($object->getStatus() != $status_closed) { + if ($object->isPublished()) { throw new Exception( pht( 'You can not reopen this revision because it is not closed. '. diff --git a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php index 32fcb3271e..65b96d6d8e 100644 --- a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php @@ -37,8 +37,7 @@ final class DifferentialRevisionRequestReviewTransaction } protected function validateAction($object, PhabricatorUser $viewer) { - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; - if ($object->getStatus() == $status_review) { + if ($object->isNeedsReview()) { throw new Exception( pht( 'You can not request review of this revision because this '. diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index f7e3a735a4..ab11a83b38 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -203,10 +203,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $revision->getID(), $commit->getPHID()); - $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; - $should_close = ($revision->getStatus() != $status_closed) && - $should_autoclose; - + $should_close = !$revision->isPublished() && $should_autoclose; if ($should_close) { $commit_close_xaction = id(new DifferentialTransaction()) ->setTransactionType(DifferentialTransaction::TYPE_ACTION) From 46d1596bf797890f9455147186f5e74369b5cec4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Aug 2017 10:19:28 -0700 Subject: [PATCH 12/24] Pull legacy revision query status filters out of the main Query class Summary: Ref T2543. Currently, Differential uses a set of hard-coded query filters (like "open" and "closed") to query revisions by status (for example, "open" means any of "review, revision, changes planned, accepted [usually]"). In other applications, like Maniphest, we've replaced this with a low level list of the actual statuses, plus higher level convenience UI through tokenizer functions. This basically has all of the benefits of the hard-coded filters with none of the drawbacks, and is generally more flexible. I'd like to do that in Differential, too, although we'll need to keep the legacy maps around for a while because they're used by `differential.find` and `differential.getrevision`. To prepare for this, pull all the legacy stuff out into a separate class. Then I'll modernize where I can, and we can get rid of this junk some day. Test Plan: Grepped for `RevisionQuery::STATUS`. Ran queries via Differential UI. Reviewers: chad Reviewed By: chad Maniphest Tasks: T2543 Differential Revision: https://secure.phabricator.com/D18343 --- src/__phutil_library_map__.php | 2 + .../DifferentialFindConduitAPIMethod.php | 4 +- .../DifferentialQueryConduitAPIMethod.php | 7 +- .../constants/DifferentialLegacyQuery.php | 86 +++++++++++++++++++ .../constants/DifferentialRevisionStatus.php | 49 +++++------ .../DifferentialDiffViewController.php | 2 +- .../DifferentialRevisionViewController.php | 2 +- .../query/DifferentialRevisionQuery.php | 73 ++-------------- .../DifferentialRevisionSearchEngine.php | 18 ++-- .../controller/DiffusionBrowseController.php | 2 +- 10 files changed, 130 insertions(+), 115 deletions(-) create mode 100644 src/applications/differential/constants/DifferentialLegacyQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d44e9370bb..4089df2bbb 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -471,6 +471,7 @@ phutil_register_library_map(array( 'DifferentialJIRAIssuesCommitMessageField' => 'applications/differential/field/DifferentialJIRAIssuesCommitMessageField.php', 'DifferentialJIRAIssuesField' => 'applications/differential/customfield/DifferentialJIRAIssuesField.php', 'DifferentialLegacyHunk' => 'applications/differential/storage/DifferentialLegacyHunk.php', + 'DifferentialLegacyQuery' => 'applications/differential/constants/DifferentialLegacyQuery.php', 'DifferentialLineAdjustmentMap' => 'applications/differential/parser/DifferentialLineAdjustmentMap.php', 'DifferentialLintField' => 'applications/differential/customfield/DifferentialLintField.php', 'DifferentialLintStatus' => 'applications/differential/constants/DifferentialLintStatus.php', @@ -5447,6 +5448,7 @@ phutil_register_library_map(array( 'DifferentialJIRAIssuesCommitMessageField' => 'DifferentialCommitMessageCustomField', 'DifferentialJIRAIssuesField' => 'DifferentialStoredCustomField', 'DifferentialLegacyHunk' => 'DifferentialHunk', + 'DifferentialLegacyQuery' => 'Phobject', 'DifferentialLineAdjustmentMap' => 'Phobject', 'DifferentialLintField' => 'DifferentialHarbormasterField', 'DifferentialLintStatus' => 'Phobject', diff --git a/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php index 79d3fc5d7d..a199f599e8 100644 --- a/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php @@ -52,12 +52,12 @@ final class DifferentialFindConduitAPIMethod switch ($type) { case 'open': $query - ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) + ->withStatus(DifferentialLegacyQuery::STATUS_OPEN) ->withAuthors($guids); break; case 'committable': $query - ->withStatus(DifferentialRevisionQuery::STATUS_ACCEPTED) + ->withStatus(DifferentialLegacyQuery::STATUS_ACCEPTED) ->withAuthors($guids); break; case 'revision-ids': diff --git a/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php index 1051983324..329a44c857 100644 --- a/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php @@ -25,12 +25,7 @@ final class DifferentialQueryConduitAPIMethod $hash_types = ArcanistDifferentialRevisionHash::getTypes(); $hash_const = $this->formatStringConstants($hash_types); - $status_types = array( - DifferentialRevisionQuery::STATUS_ANY, - DifferentialRevisionQuery::STATUS_OPEN, - DifferentialRevisionQuery::STATUS_ACCEPTED, - DifferentialRevisionQuery::STATUS_CLOSED, - ); + $status_types = DifferentialLegacyQuery::getAllConstants(); $status_const = $this->formatStringConstants($status_types); $order_types = array( diff --git a/src/applications/differential/constants/DifferentialLegacyQuery.php b/src/applications/differential/constants/DifferentialLegacyQuery.php new file mode 100644 index 0000000000..4a20267f10 --- /dev/null +++ b/src/applications/differential/constants/DifferentialLegacyQuery.php @@ -0,0 +1,86 @@ +getLegacyKey(); + if ($legacy_key !== null) { + $values[] = $legacy_key; + } + } + + return $values; + } + + private static function getMap() { + $all = array( + DifferentialRevisionStatus::NEEDS_REVIEW, + DifferentialRevisionStatus::NEEDS_REVISION, + DifferentialRevisionStatus::CHANGES_PLANNED, + DifferentialRevisionStatus::ACCEPTED, + DifferentialRevisionStatus::PUBLISHED, + DifferentialRevisionStatus::ABANDONED, + ); + + $open = array(); + $closed = array(); + + foreach ($all as $status) { + $status_object = DifferentialRevisionStatus::newForStatus($status); + if ($status_object->isClosedStatus()) { + $closed[] = $status_object->getKey(); + } else { + $open[] = $status_object->getKey(); + } + } + + return array( + self::STATUS_ANY => $all, + self::STATUS_OPEN => $open, + self::STATUS_ACCEPTED => array( + DifferentialRevisionStatus::ACCEPTED, + ), + self::STATUS_NEEDS_REVIEW => array( + DifferentialRevisionStatus::NEEDS_REVIEW, + ), + self::STATUS_NEEDS_REVISION => array( + DifferentialRevisionStatus::NEEDS_REVISION, + ), + self::STATUS_CLOSED => $closed, + self::STATUS_ABANDONED => array( + DifferentialRevisionStatus::ABANDONED, + ), + ); + } + +} diff --git a/src/applications/differential/constants/DifferentialRevisionStatus.php b/src/applications/differential/constants/DifferentialRevisionStatus.php index caca7ed85b..37b51f1b3b 100644 --- a/src/applications/differential/constants/DifferentialRevisionStatus.php +++ b/src/applications/differential/constants/DifferentialRevisionStatus.php @@ -12,6 +12,14 @@ final class DifferentialRevisionStatus extends Phobject { private $key; private $spec = array(); + public function getKey() { + return $this->key; + } + + public function getLegacyKey() { + return idx($this->spec, 'legacy'); + } + public function getIcon() { return idx($this->spec, 'icon'); } @@ -52,6 +60,18 @@ final class DifferentialRevisionStatus extends Phobject { return ($this->key === self::CHANGES_PLANNED); } + public static function newForStatus($status) { + $result = new self(); + + $map = self::getMap(); + if (isset($map[$status])) { + $result->key = $status; + $result->spec = $map[$status]; + } + + return $result; + } + public static function newForLegacyStatus($legacy_status) { $result = new self(); @@ -135,33 +155,4 @@ final class DifferentialRevisionStatus extends Phobject { ); } - public static function getClosedStatuses() { - $statuses = array( - ArcanistDifferentialRevisionStatus::CLOSED, - ArcanistDifferentialRevisionStatus::ABANDONED, - ); - - if (PhabricatorEnv::getEnvConfig('differential.close-on-accept')) { - $statuses[] = ArcanistDifferentialRevisionStatus::ACCEPTED; - } - - return $statuses; - } - - public static function getOpenStatuses() { - return array_diff(self::getAllStatuses(), self::getClosedStatuses()); - } - - public static function getAllStatuses() { - return array( - ArcanistDifferentialRevisionStatus::NEEDS_REVIEW, - ArcanistDifferentialRevisionStatus::NEEDS_REVISION, - ArcanistDifferentialRevisionStatus::CHANGES_PLANNED, - ArcanistDifferentialRevisionStatus::ACCEPTED, - ArcanistDifferentialRevisionStatus::CLOSED, - ArcanistDifferentialRevisionStatus::ABANDONED, - ArcanistDifferentialRevisionStatus::IN_PREPARATION, - ); - } - } diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php index dae65bb200..488d2cec82 100644 --- a/src/applications/differential/controller/DifferentialDiffViewController.php +++ b/src/applications/differential/controller/DifferentialDiffViewController.php @@ -177,7 +177,7 @@ final class DifferentialDiffViewController extends DifferentialController { $revisions = id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withAuthors(array($viewer->getPHID())) - ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) + ->withStatus(DifferentialLegacyQuery::STATUS_OPEN) ->requireCapabilities( array( PhabricatorPolicyCapability::CAN_VIEW, diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 5c7f5104f5..81b2aebda6 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -808,7 +808,7 @@ final class DifferentialRevisionViewController extends DifferentialController { $query = id(new DifferentialRevisionQuery()) ->setViewer($this->getRequest()->getUser()) - ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) + ->withStatus(DifferentialLegacyQuery::STATUS_OPEN) ->withUpdatedEpochBetween($recent, null) ->setOrder(DifferentialRevisionQuery::ORDER_MODIFIED) ->setLimit(10) diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index 701463e6a7..f53ec46a9f 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -1,13 +1,6 @@ withStatus(DifferentialRevisionQuery::STATUS_OPEN) - * ->execute(); - * * @task config Query Configuration * @task exec Query Execution * @task internal Internals @@ -18,13 +11,6 @@ final class DifferentialRevisionQuery private $pathIDs = array(); private $status = 'status-any'; - const STATUS_ANY = 'status-any'; - const STATUS_OPEN = 'status-open'; - const STATUS_ACCEPTED = 'status-accepted'; - const STATUS_NEEDS_REVIEW = 'status-needs-review'; - const STATUS_NEEDS_REVISION = 'status-needs-revision'; - const STATUS_CLOSED = 'status-closed'; - const STATUS_ABANDONED = 'status-abandoned'; private $authors = array(); private $draftAuthors = array(); @@ -149,7 +135,7 @@ final class DifferentialRevisionQuery /** * Filter results to revisions with a given status. Provide a class constant, - * such as `DifferentialRevisionQuery::STATUS_OPEN`. + * such as `DifferentialLegacyQuery::STATUS_OPEN`. * * @param const Class STATUS constant, like STATUS_OPEN. * @return this @@ -711,57 +697,12 @@ final class DifferentialRevisionQuery // NOTE: Although the status constants are integers in PHP, the column is a // string column in MySQL, and MySQL won't use keys on string columns if // you put integers in the query. - - switch ($this->status) { - case self::STATUS_ANY: - break; - case self::STATUS_OPEN: - $where[] = qsprintf( - $conn_r, - 'r.status IN (%Ls)', - DifferentialRevisionStatus::getOpenStatuses()); - break; - case self::STATUS_NEEDS_REVIEW: - $where[] = qsprintf( - $conn_r, - 'r.status IN (%Ls)', - array( - ArcanistDifferentialRevisionStatus::NEEDS_REVIEW, - )); - break; - case self::STATUS_NEEDS_REVISION: - $where[] = qsprintf( - $conn_r, - 'r.status IN (%Ls)', - array( - ArcanistDifferentialRevisionStatus::NEEDS_REVISION, - )); - break; - case self::STATUS_ACCEPTED: - $where[] = qsprintf( - $conn_r, - 'r.status IN (%Ls)', - array( - ArcanistDifferentialRevisionStatus::ACCEPTED, - )); - break; - case self::STATUS_CLOSED: - $where[] = qsprintf( - $conn_r, - 'r.status IN (%Ls)', - DifferentialRevisionStatus::getClosedStatuses()); - break; - case self::STATUS_ABANDONED: - $where[] = qsprintf( - $conn_r, - 'r.status IN (%Ls)', - array( - ArcanistDifferentialRevisionStatus::ABANDONED, - )); - break; - default: - throw new Exception( - pht("Unknown revision status filter constant '%s'!", $this->status)); + $statuses = DifferentialLegacyQuery::getQueryValues($this->status); + if ($statuses !== null) { + $where[] = qsprintf( + $conn_r, + 'r.status IN (%Ls)', + $statuses); } $where[] = $this->buildWhereClauseParts($conn_r); diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php index ef88c0ea1c..8cb8ffd2fb 100644 --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -115,7 +115,7 @@ final class DifferentialRevisionSearchEngine return $query ->setParameter('responsiblePHIDs', array($viewer->getPHID())) - ->setParameter('status', DifferentialRevisionQuery::STATUS_OPEN) + ->setParameter('status', DifferentialLegacyQuery::STATUS_OPEN) ->setParameter('bucket', $bucket_key); case 'authored': return $query @@ -129,13 +129,13 @@ final class DifferentialRevisionSearchEngine private function getStatusOptions() { return array( - DifferentialRevisionQuery::STATUS_ANY => pht('All'), - DifferentialRevisionQuery::STATUS_OPEN => pht('Open'), - DifferentialRevisionQuery::STATUS_ACCEPTED => pht('Accepted'), - DifferentialRevisionQuery::STATUS_NEEDS_REVIEW => pht('Needs Review'), - DifferentialRevisionQuery::STATUS_NEEDS_REVISION => pht('Needs Revision'), - DifferentialRevisionQuery::STATUS_CLOSED => pht('Closed'), - DifferentialRevisionQuery::STATUS_ABANDONED => pht('Abandoned'), + DifferentialLegacyQuery::STATUS_ANY => pht('All'), + DifferentialLegacyQuery::STATUS_OPEN => pht('Open'), + DifferentialLegacyQuery::STATUS_ACCEPTED => pht('Accepted'), + DifferentialLegacyQuery::STATUS_NEEDS_REVIEW => pht('Needs Review'), + DifferentialLegacyQuery::STATUS_NEEDS_REVISION => pht('Needs Revision'), + DifferentialLegacyQuery::STATUS_CLOSED => pht('Closed'), + DifferentialLegacyQuery::STATUS_ABANDONED => pht('Abandoned'), ); } @@ -267,7 +267,7 @@ final class DifferentialRevisionSearchEngine $blocking_revisions = id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withPHIDs($revision_phids) - ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) + ->withStatus(DifferentialLegacyQuery::STATUS_OPEN) ->execute(); $blocking_revisions = mpull($blocking_revisions, null, 'getPHID'); diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index 06c8336f5c..32381207fa 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -1758,7 +1758,7 @@ final class DiffusionBrowseController extends DiffusionController { $revisions = id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withPath($repository->getID(), $path_id) - ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) + ->withStatus(DifferentialLegacyQuery::STATUS_OPEN) ->withUpdatedEpochBetween($recent, null) ->setOrder(DifferentialRevisionQuery::ORDER_MODIFIED) ->setLimit(10) From bd47d001b5482d8f8160a011ac8a0dd76f147e13 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Wed, 9 Aug 2017 22:49:25 +0000 Subject: [PATCH 13/24] Remove chatbot example configuration Summary: This is no longer needed after the chatbot was removed in D17756. Test Plan: N/A Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18378 --- resources/chatbot/example_config.json | 24 ------------------- .../files/storage/PhabricatorFile.php | 8 +++++-- 2 files changed, 6 insertions(+), 26 deletions(-) delete mode 100644 resources/chatbot/example_config.json diff --git a/resources/chatbot/example_config.json b/resources/chatbot/example_config.json deleted file mode 100644 index 7d150e65ae..0000000000 --- a/resources/chatbot/example_config.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "server" : "irc.freenode.net", - "port" : 6667, - "nick" : "phabot", - "join" : [ - "#phabot-test" - ], - "handlers" : [ - "PhabricatorBotObjectNameHandler", - "PhabricatorBotSymbolHandler", - "PhabricatorBotLogHandler", - "PhabricatorBotFeedNotificationHandler", - "PhabricatorBotWhatsNewHandler", - "PhabricatorBotMacroHandler" - ], - - "conduit.uri" : null, - "conduit.token" : null, - - "macro.size" : 48, - "macro.aspect" : 0.66, - - "notification.channels" : ["#phabot-test"] -} diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 19f0bed90a..97c2ef6acc 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -854,7 +854,7 @@ final class PhabricatorFile extends PhabricatorFileDAO return $this->getTransformedURI($transform->getTransformKey()); } - private function getTransformedURI($transform) { + private function getTransformedURI($transform, $cdn) { $parts = array(); $parts[] = 'file'; $parts[] = 'xform'; @@ -871,7 +871,11 @@ final class PhabricatorFile extends PhabricatorFileDAO $path = implode('/', $parts); $path = $path.'/'; - return PhabricatorEnv::getCDNURI($path); + if ($cdn) { + return PhabricatorEnv::getCDNURI($path); + } else { + return PhabricatorEnv::getURI($path); + } } public function isViewableInBrowser() { From 3ba196152f9bfb4bbcc909b94b4fafa7847f49c3 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Wed, 9 Aug 2017 20:01:31 -0700 Subject: [PATCH 14/24] Clean up some dialog spacing Summary: Makes dialogs a little wider, form dialogs a lot wider (space controls). Also cleans up Passphrase dialogs. Fixes T12833. I think forms probably need to move to tables for better layout flexibility like veritical alignment. Test Plan: Passphrase create, edit, etc. Other dialogs. Reviewers: epriestley Subscribers: Korvin Maniphest Tasks: T12833 Differential Revision: https://secure.phabricator.com/D18382 --- resources/celerity/map.php | 10 +++++----- .../passphrase/view/PassphraseCredentialControl.php | 5 +++-- webroot/rsrc/css/aphront/dialog-view.css | 4 ++-- webroot/rsrc/css/phui/phui-form-view.css | 1 + 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index a8b40f8e88..c0bc1bf629 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' => 'b5b51108', - 'core.pkg.css' => '5a682e14', + 'core.pkg.css' => '0e4a68ad', 'core.pkg.js' => '5d80e0db', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '45951e9e', @@ -26,7 +26,7 @@ return array( 'rsrc/audio/basic/ting.mp3' => '17660001', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', 'rsrc/css/aphront/dark-console.css' => 'f7b071f1', - 'rsrc/css/aphront/dialog-view.css' => '685c7e2d', + 'rsrc/css/aphront/dialog-view.css' => '6bfc244b', 'rsrc/css/aphront/list-filter-view.css' => '5d6f0526', 'rsrc/css/aphront/multi-column.css' => '84cc6640', 'rsrc/css/aphront/notification.css' => '3f6c89c9', @@ -155,7 +155,7 @@ return array( 'rsrc/css/phui/phui-document.css' => 'c32e8dec', 'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9', 'rsrc/css/phui/phui-fontkit.css' => '1320ed01', - 'rsrc/css/phui/phui-form-view.css' => '6175808d', + 'rsrc/css/phui/phui-form-view.css' => 'ae9f8d16', 'rsrc/css/phui/phui-form.css' => '7aaa04e3', 'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f', 'rsrc/css/phui/phui-header-view.css' => 'e7de7ee2', @@ -539,7 +539,7 @@ return array( 'almanac-css' => 'dbb9b3af', 'aphront-bars' => '231ac33c', 'aphront-dark-console-css' => 'f7b071f1', - 'aphront-dialog-view-css' => '685c7e2d', + 'aphront-dialog-view-css' => '6bfc244b', 'aphront-list-filter-view-css' => '5d6f0526', 'aphront-multi-column-view-css' => '84cc6640', 'aphront-panel-view-css' => '8427b78d', @@ -843,7 +843,7 @@ return array( 'phui-font-icon-base-css' => '870a7360', 'phui-fontkit-css' => '1320ed01', 'phui-form-css' => '7aaa04e3', - 'phui-form-view-css' => '6175808d', + 'phui-form-view-css' => 'ae9f8d16', 'phui-head-thing-view-css' => 'fd311e5f', 'phui-header-view-css' => 'e7de7ee2', 'phui-hovercard' => '1bd28176', diff --git a/src/applications/passphrase/view/PassphraseCredentialControl.php b/src/applications/passphrase/view/PassphraseCredentialControl.php index 411afd6f85..1c189e30e7 100644 --- a/src/applications/passphrase/view/PassphraseCredentialControl.php +++ b/src/applications/passphrase/view/PassphraseCredentialControl.php @@ -113,11 +113,12 @@ final class PassphraseCredentialControl extends AphrontFormControl { 'a', array( 'href' => '#', - 'class' => 'button button-grey', + 'class' => 'button button-grey mll', 'sigil' => 'passphrase-credential-add', 'mustcapture' => true, + 'style' => 'height: 20px;', // move aphront-form to tables ), - pht('Add Credential')); + pht('Add New Credential')); } else { $button = null; } diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css index 5809ab117e..153685548e 100644 --- a/webroot/rsrc/css/aphront/dialog-view.css +++ b/webroot/rsrc/css/aphront/dialog-view.css @@ -3,7 +3,7 @@ */ .aphront-dialog-view { - width: 560px; + width: 580px; margin: 32px auto 16px; border: 1px solid {$lightblueborder}; border-radius: 3px; @@ -32,7 +32,7 @@ } .aphront-dialog-view-width-form { - width: 640px; + width: 820px; } .aphront-dialog-view-width-full { diff --git a/webroot/rsrc/css/phui/phui-form-view.css b/webroot/rsrc/css/phui/phui-form-view.css index da1a524abb..1b7400656a 100644 --- a/webroot/rsrc/css/phui/phui-form-view.css +++ b/webroot/rsrc/css/phui/phui-form-view.css @@ -196,6 +196,7 @@ .aphront-form-control-markup .aphront-form-input { font-size: {$normalfontsize}; + padding: 3px 0; } .aphront-form-control-static .aphront-form-input { From 71eaf3e8c4d61203ab2887ab1a294ea81bd6ae83 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Thu, 10 Aug 2017 08:59:52 +1000 Subject: [PATCH 15/24] Remove code that was accidentally landed Summary: See some discussion in D18378. Test Plan: N/A Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18380 --- src/applications/files/storage/PhabricatorFile.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 97c2ef6acc..19f0bed90a 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -854,7 +854,7 @@ final class PhabricatorFile extends PhabricatorFileDAO return $this->getTransformedURI($transform->getTransformKey()); } - private function getTransformedURI($transform, $cdn) { + private function getTransformedURI($transform) { $parts = array(); $parts[] = 'file'; $parts[] = 'xform'; @@ -871,11 +871,7 @@ final class PhabricatorFile extends PhabricatorFileDAO $path = implode('/', $parts); $path = $path.'/'; - if ($cdn) { - return PhabricatorEnv::getCDNURI($path); - } else { - return PhabricatorEnv::getURI($path); - } + return PhabricatorEnv::getCDNURI($path); } public function isViewableInBrowser() { From 92c49c3772d856472f3b457f4f5d4e32f96c0e9b Mon Sep 17 00:00:00 2001 From: Chad Little Date: Wed, 9 Aug 2017 19:31:05 -0700 Subject: [PATCH 16/24] Minor UX tweaks to Phortune autopay Summary: Fixes T12958. Adds a success message when card is added, also switches to use radio buttons for clarity. Updated redirect uri for deleting methods as well. Test Plan: Add cards, remove cards. {F5091084} Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T12958 Differential Revision: https://secure.phabricator.com/D18381 --- .../PhortunePaymentMethodCreateController.php | 3 ++- ...PhortunePaymentMethodDisableController.php | 5 ++-- .../PhortuneSubscriptionEditController.php | 27 ++++++++++++++----- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php b/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php index 794f97aa8e..87bddd4d33 100644 --- a/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php +++ b/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php @@ -123,7 +123,8 @@ final class PhortunePaymentMethodCreateController $next_uri = $this->getApplicationURI( "cart/{$cart_id}/checkout/?paymentMethodID=".$method->getID()); } else if ($subscription_id) { - $next_uri = $cancel_uri; + $next_uri = new PhutilURI($cancel_uri); + $next_uri->setQueryParam('added', true); } else { $account_uri = $this->getApplicationURI($account->getID().'/'); $next_uri = new PhutilURI($account_uri); diff --git a/src/applications/phortune/controller/payment/PhortunePaymentMethodDisableController.php b/src/applications/phortune/controller/payment/PhortunePaymentMethodDisableController.php index d00db2725f..f5feec8a29 100644 --- a/src/applications/phortune/controller/payment/PhortunePaymentMethodDisableController.php +++ b/src/applications/phortune/controller/payment/PhortunePaymentMethodDisableController.php @@ -25,11 +25,12 @@ final class PhortunePaymentMethodDisableController } $account = $method->getAccount(); - $account_uri = $this->getApplicationURI($account->getID().'/'); + $account_id = $account->getID(); + $account_uri = $this->getApplicationURI("/account/billing/{$account_id}/"); if ($request->isFormPost()) { - // TODO: ApplicationTransactions! + // TODO: ApplicationTransactions!!!! $method ->setStatus(PhortunePaymentMethod::STATUS_DISABLED) ->save(); diff --git a/src/applications/phortune/controller/subscription/PhortuneSubscriptionEditController.php b/src/applications/phortune/controller/subscription/PhortuneSubscriptionEditController.php index 185bbdbcc6..e7287f3d29 100644 --- a/src/applications/phortune/controller/subscription/PhortuneSubscriptionEditController.php +++ b/src/applications/phortune/controller/subscription/PhortuneSubscriptionEditController.php @@ -4,6 +4,7 @@ final class PhortuneSubscriptionEditController extends PhortuneController { public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); + $added = $request->getBool('added'); $subscription = id(new PhortuneSubscriptionQuery()) ->setViewer($viewer) @@ -112,6 +113,7 @@ final class PhortuneSubscriptionEditController extends PhortuneController { pht('Subscription %d', $subscription->getID()), $view_uri); $crumbs->addTextCrumb(pht('Edit')); + $crumbs->setBorder(true); $uri = $this->getApplicationURI($account->getID().'/card/new/'); @@ -127,15 +129,19 @@ final class PhortuneSubscriptionEditController extends PhortuneController { ), pht('Add Payment Method...')); + $radio = id(new AphrontFormRadioButtonControl()) + ->setName('defaultPaymentMethodPHID') + ->setLabel(pht('Autopay With')) + ->setValue($current_phid) + ->setError($e_method); + + foreach ($options as $key => $value) { + $radio->addButton($key, $value, null); + } + $form = id(new AphrontFormView()) ->setUser($viewer) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setName('defaultPaymentMethodPHID') - ->setLabel(pht('Autopay With')) - ->setValue($current_phid) - ->setError($e_method) - ->setOptions($options)) + ->appendChild($radio) ->appendChild( id(new AphrontFormMarkupControl()) ->setValue($add_method_button)) @@ -151,6 +157,13 @@ final class PhortuneSubscriptionEditController extends PhortuneController { ->setFormErrors($errors) ->appendChild($form); + if ($added) { + $info_view = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_SUCCESS) + ->appendChild(pht('Payment method has been successfully added.')); + $box->setInfoView($info_view); + } + $header = id(new PHUIHeaderView()) ->setHeader(pht('Edit %s', $subscription->getSubscriptionName())) ->setHeaderIcon('fa-pencil'); From e89087fc5136444fb669aa74751ad825214976c0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Aug 2017 03:31:20 -0700 Subject: [PATCH 17/24] Fix a hang in fancy date picker for Ye Olde Time Years Summary: Fixes T12960. When the user enters a date like "1917", we currently loop ~20 million times. Instead: - Be a little more careful about parsing. - Javascript's default behavior of interpreting "2001-02-31" as "2001-03-03" ("February 31" -> "March 3") already seems reasonable, so just let it do that. Test Plan: Verified these behaviors: - `2017-08-08` - Valid, recent. - `17-08-08` - Valid, recent. - `1917-08-08` - Valid, a century ago, no loop. - `2017-02-31` - "February 31", interpreted as "March 3" by Javascsript, seems reasonable. - `Quack` - Default, current time. - `0/0/0`, `0/99/0` - Default, current time. Reviewers: avivey, chad Reviewed By: chad Maniphest Tasks: T12960 Differential Revision: https://secure.phabricator.com/D18383 --- resources/celerity/map.php | 18 ++++---- .../rsrc/js/core/behavior-fancy-datepicker.js | 44 ++++++++++++------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index c0bc1bf629..54832d23ae 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -484,7 +484,7 @@ return array( 'rsrc/js/core/behavior-device.js' => 'bb1dd507', 'rsrc/js/core/behavior-drag-and-drop-textarea.js' => '484a6e22', 'rsrc/js/core/behavior-error-log.js' => '6882e80a', - 'rsrc/js/core/behavior-fancy-datepicker.js' => 'a9210d03', + 'rsrc/js/core/behavior-fancy-datepicker.js' => 'ecf4e799', 'rsrc/js/core/behavior-file-tree.js' => '88236f00', 'rsrc/js/core/behavior-form.js' => '5c54cbf3', 'rsrc/js/core/behavior-gesture.js' => '3ab51e2c', @@ -633,7 +633,7 @@ return array( 'javelin-behavior-editengine-reorder-fields' => 'b59e1e96', 'javelin-behavior-error-log' => '6882e80a', 'javelin-behavior-event-all-day' => 'b41537c9', - 'javelin-behavior-fancy-datepicker' => 'a9210d03', + 'javelin-behavior-fancy-datepicker' => 'ecf4e799', 'javelin-behavior-global-drag-and-drop' => '960f6a39', 'javelin-behavior-herald-rule-editor' => '7ebaeed3', 'javelin-behavior-high-security-warning' => 'a464fe03', @@ -1716,13 +1716,6 @@ return array( 'javelin-uri', 'phabricator-keyboard-shortcut', ), - 'a9210d03' => array( - 'javelin-behavior', - 'javelin-util', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-vector', - ), 'a9f88de2' => array( 'javelin-behavior', 'javelin-dom', @@ -2105,6 +2098,13 @@ return array( 'javelin-dom', 'phabricator-draggable-list', ), + 'ecf4e799' => array( + 'javelin-behavior', + 'javelin-util', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-vector', + ), 'edf8a145' => array( 'javelin-behavior', 'javelin-uri', diff --git a/webroot/rsrc/js/core/behavior-fancy-datepicker.js b/webroot/rsrc/js/core/behavior-fancy-datepicker.js index 15558dbfe1..afd5ff25ad 100644 --- a/webroot/rsrc/js/core/behavior-fancy-datepicker.js +++ b/webroot/rsrc/js/core/behavior-fancy-datepicker.js @@ -287,26 +287,38 @@ JX.behavior('fancy-datepicker', function(config, statics) { }; function getValidDate() { - var written_date = new Date(value_y, value_m-1, value_d); + var year_int = parseInt(value_y, 10); + if (isNaN(year_int) || year_int < 0) { + return new Date(); + } + + // If the user enters "11" for the year, interpret it as "2011" (which + // is almost certainly what they mean) not "1911" (which is the default + // behavior of Javascript). + if (year_int < 70) { + year_int += 2000; + } + + var month_int = parseInt(value_m, 10); + if (isNaN(month_int) || month_int < 1 || month_int > 12) { + return new Date(); + } + + // In Javascript, January is "0", not "1", so adjust the value down. + month_int = month_int - 1; + + var day_int = parseInt(value_d, 10); + if (isNaN(day_int) || day_int < 1 || day_int > 31) { + return new Date(); + } + + var written_date = new Date(year_int, month_int, day_int); if (isNaN(written_date.getTime())) { return new Date(); - } else { - //year 01 should be 2001, not 1901 - if (written_date.getYear() < 70) { - value_y += 2000; - written_date = new Date(value_y, value_m-1, value_d); - } - - // adjust for a date like February 31 - var adjust = 1; - while (written_date.getMonth() !== value_m-1) { - written_date = new Date(value_y, value_m-1, value_d-adjust); - adjust++; - } - - return written_date; } + + return written_date; } From 8443366f32d3f8bda19e67744386a68c0cf4387f Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Aug 2017 06:04:52 -0700 Subject: [PATCH 18/24] Remove `bin/files purge` workflow Summary: Fixes T12948. See that task for substantial discussion and context. Briefly: - This workflow is very old, and won't work for large (>2GB) files. - This workflow has become more dangerous than it once was, and can fail in several ways that delete data and/or make recovery much more difficult (see T12948 for more discussion). - This was originally added in D6068, which is a bit muddled, but looks like "one install ran into a weird issue so I wrote a script for them"; this would be a Consulting/Support issue and not come upstream today. I can't identify any arguments for retaining this workflow there, at least. Test Plan: - Grepped for `files purge`, got nothing. - Grepped for `purge`, looked for anything that looked like instructions or documentation, got nothing. I don't recall recommending anyone run this script in many years, and didn't even remember that it existed or what it did when T12948 was reported, so I believe it is not in widespread use. Reviewers: joshuaspence, chad Reviewed By: joshuaspence Maniphest Tasks: T12948 Differential Revision: https://secure.phabricator.com/D18384 --- src/__phutil_library_map__.php | 2 - ...habricatorFilesManagementPurgeWorkflow.php | 71 ------------------- 2 files changed, 73 deletions(-) delete mode 100644 src/applications/files/management/PhabricatorFilesManagementPurgeWorkflow.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4089df2bbb..2b45c3856a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2900,7 +2900,6 @@ phutil_register_library_map(array( 'PhabricatorFilesManagementGenerateKeyWorkflow' => 'applications/files/management/PhabricatorFilesManagementGenerateKeyWorkflow.php', 'PhabricatorFilesManagementIntegrityWorkflow' => 'applications/files/management/PhabricatorFilesManagementIntegrityWorkflow.php', 'PhabricatorFilesManagementMigrateWorkflow' => 'applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php', - 'PhabricatorFilesManagementPurgeWorkflow' => 'applications/files/management/PhabricatorFilesManagementPurgeWorkflow.php', 'PhabricatorFilesManagementRebuildWorkflow' => 'applications/files/management/PhabricatorFilesManagementRebuildWorkflow.php', 'PhabricatorFilesManagementWorkflow' => 'applications/files/management/PhabricatorFilesManagementWorkflow.php', 'PhabricatorFilesOnDiskBuiltinFile' => 'applications/files/builtin/PhabricatorFilesOnDiskBuiltinFile.php', @@ -8238,7 +8237,6 @@ phutil_register_library_map(array( 'PhabricatorFilesManagementGenerateKeyWorkflow' => 'PhabricatorFilesManagementWorkflow', 'PhabricatorFilesManagementIntegrityWorkflow' => 'PhabricatorFilesManagementWorkflow', 'PhabricatorFilesManagementMigrateWorkflow' => 'PhabricatorFilesManagementWorkflow', - 'PhabricatorFilesManagementPurgeWorkflow' => 'PhabricatorFilesManagementWorkflow', 'PhabricatorFilesManagementRebuildWorkflow' => 'PhabricatorFilesManagementWorkflow', 'PhabricatorFilesManagementWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorFilesOnDiskBuiltinFile' => 'PhabricatorFilesBuiltinFile', diff --git a/src/applications/files/management/PhabricatorFilesManagementPurgeWorkflow.php b/src/applications/files/management/PhabricatorFilesManagementPurgeWorkflow.php deleted file mode 100644 index bf2c04a584..0000000000 --- a/src/applications/files/management/PhabricatorFilesManagementPurgeWorkflow.php +++ /dev/null @@ -1,71 +0,0 @@ -setName('purge') - ->setSynopsis(pht('Delete files with missing data.')) - ->setArguments( - array( - array( - 'name' => 'all', - 'help' => pht('Update all files.'), - ), - array( - 'name' => 'dry-run', - 'help' => pht('Show what would be updated.'), - ), - array( - 'name' => 'names', - 'wildcard' => true, - ), - )); - } - - public function execute(PhutilArgumentParser $args) { - $console = PhutilConsole::getConsole(); - - $iterator = $this->buildIterator($args); - if (!$iterator) { - throw new PhutilArgumentUsageException( - pht( - 'Either specify a list of files to purge, or use `%s` '. - 'to purge all files.', - '--all')); - } - - $is_dry_run = $args->getArg('dry-run'); - - foreach ($iterator as $file) { - $fid = 'F'.$file->getID(); - - try { - $file->loadFileData(); - $okay = true; - } catch (Exception $ex) { - $okay = false; - } - - if ($okay) { - $console->writeOut( - "%s\n", - pht('%s: File data is OK, not purging.', $fid)); - } else { - if ($is_dry_run) { - $console->writeOut( - "%s\n", - pht('%s: Would purge (dry run).', $fid)); - } else { - $console->writeOut( - "%s\n", - pht('%s: Purging.', $fid)); - $file->delete(); - } - } - } - - return 0; - } -} From 7f90ef2d82610f0d30a41d41ac8c7856872af3df Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Aug 2017 08:18:56 -0700 Subject: [PATCH 19/24] Make the "Requested Changes to Prior Diff" reviewer icon red, not bluegrey Summary: See PHI31. The "Accepted Older Revision" icon is (more reasonably) bluegrey, but that rule spilled over here where it doesn't make much sense. "Requested Changes to Prior Diff" remains in effect across updates, but the coloration implies otherwise. Test Plan: "Requested Changes to This Diff" (unchanged): {F5092019} "Requested Changes to Prior Diff" (now red, previously bluegrey): {F5092020} Note that the icons are different so this is technically colorblind-safe, and it's normally not important to distinguish between these two reds anyway. Reviewers: chad, lvital Reviewed By: lvital Subscribers: lvital Differential Revision: https://secure.phabricator.com/D18385 --- .../differential/view/DifferentialReviewersView.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php index 034bf99edb..8ddea7c8c0 100644 --- a/src/applications/differential/view/DifferentialReviewersView.php +++ b/src/applications/differential/view/DifferentialReviewersView.php @@ -116,7 +116,7 @@ final class DifferentialReviewersView extends AphrontView { } } else { $icon = 'fa-times-circle-o'; - $color = 'bluegrey'; + $color = 'red'; if ($authority_name !== null) { $label = pht( 'Requested Changes to Prior Diff (by %s)', From a7124f8f7a116d844e979506f2cb91cccb183a68 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 10 Aug 2017 13:22:46 -0700 Subject: [PATCH 20/24] Add status table to Diffusion Branch manage page Summary: Fixes T12832. Adds a basic table (not paginated?) to view tracking and autoclose status. Test Plan: Review a large repository (Krita) with setting various states of tracking and autoclose. {F5092117} Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T12832 Differential Revision: https://secure.phabricator.com/D18386 --- ...usionRepositoryBranchesManagementPanel.php | 61 ++++++++++++++++++- .../DiffusionRepositoryManagementPanel.php | 4 ++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php index aa2bb49ccb..a735b8d580 100644 --- a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php @@ -67,6 +67,7 @@ final class DiffusionRepositoryBranchesManagementPanel public function buildManagementPanelContent() { $repository = $this->getRepository(); $viewer = $this->getViewer(); + $content = array(); $view = id(new PHUIPropertyListView()) ->setViewer($viewer); @@ -90,8 +91,66 @@ final class DiffusionRepositoryBranchesManagementPanel } $view->addProperty(pht('Autoclose Only'), $autoclose_only); + $content[] = $this->newBox(pht('Branches'), $view); - return $this->newBox(pht('Branches'), $view); + // Branch Autoclose Table + if (!$repository->isImporting()) { + $request = $this->getRequest(); + $pager = id(new PHUIPagerView()) + ->readFromRequest($request); + + $params = array( + 'offset' => $pager->getOffset(), + 'limit' => $pager->getPageSize() + 1, + 'repository' => $repository->getID(), + ); + + $branches = id(new ConduitCall('diffusion.branchquery', $params)) + ->setUser($viewer) + ->execute(); + $branches = DiffusionRepositoryRef::loadAllFromDictionaries($branches); + $branches = $pager->sliceResults($branches); + + $rows = array(); + foreach ($branches as $branch) { + $branch_name = $branch->getShortName(); + $tracking = $repository->shouldTrackBranch($branch_name); + $autoclosing = $repository->shouldAutocloseBranch($branch_name); + + $rows[] = array( + $branch_name, + $tracking ? pht('Tracking') : pht('Off'), + $autoclosing ? pht('Autoclose On') : pht('Off'), + ); + } + $branch_table = new AphrontTableView($rows); + $branch_table->setHeaders( + array( + pht('Branch'), + pht('Track'), + pht('Autoclose'), + )); + $branch_table->setColumnClasses( + array( + 'pri', + 'narrow', + 'wide', + )); + + $box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Branch Status')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setTable($branch_table) + ->setPager($pager); + $content[] = $box; + } else { + $content[] = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_NOTICE) + ->appendChild(pht('Branch status in unavailable while the repository '. + 'is still importing.')); + } + + return $content; } } diff --git a/src/applications/diffusion/management/DiffusionRepositoryManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryManagementPanel.php index 0098fe1df9..f9e76eb523 100644 --- a/src/applications/diffusion/management/DiffusionRepositoryManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryManagementPanel.php @@ -25,6 +25,10 @@ abstract class DiffusionRepositoryManagementPanel return $this->repository; } + final public function getRequest() { + return $this->controller->getRequest(); + } + final public function setController(PhabricatorController $controller) { $this->controller = $controller; return $this; From 0860b7f27c749dba15aa9936959516440c143c39 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 10 Aug 2017 13:51:09 -0700 Subject: [PATCH 21/24] Update Autoclose document language Summary: Rewords the document to note new location and status table. Test Plan: Read, reread. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D18387 --- .../user/userguide/diffusion_autoclose.diviner | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/docs/user/userguide/diffusion_autoclose.diviner b/src/docs/user/userguide/diffusion_autoclose.diviner index f2533bbdc6..796a3b0404 100644 --- a/src/docs/user/userguide/diffusion_autoclose.diviner +++ b/src/docs/user/userguide/diffusion_autoclose.diviner @@ -17,21 +17,19 @@ troubleshoot it. Troubleshooting Autoclose ========================= -You can check if a branch is currently configured to autoclose on the main -repository view, or in the branches list view. Hover over the {icon check} or -{icon times} icon and you should see one of these messages: +You can check if a branch is currently configured to autoclose on the +management page for the given repository under the branches menu item. +You should see one of these statuses next to the name of the branch: - - {icon check} **Autoclose Enabled** Autoclose is active for this branch. - - {icon times} **Repository Importing** This repository is still importing. + - **Autoclose On** Autoclose is active for this branch. + - **Repository Importing** This repository is still importing. Autoclose does not activate until a repository finishes importing for the first time. This prevents situations where you import a repository and accidentally close hundreds of related objects during import. Autoclose will activate for new commits after the initial import completes. - - {icon times} **Repository Autoclose Disabled** Autoclose is disabled for - this entire repository. You can enable it in **Edit Repository**. - - {icon times} **Branch Untracked** This branch is not tracked. Because it + - **Tracking Off** This branch is not tracked. Because it is not tracked, commits on it won't be seen and won't be discovered. - - {icon times} **Branch Autoclose Disabled** Autoclose is not enabled for + - **Autoclose Off** Autoclose is not enabled for this branch. You can adjust which branches autoclose in **Edit Repository**. This option is only available in Git. From 794e185bf90eefa61cac3a48f41859e1020f90df Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Aug 2017 17:04:11 -0700 Subject: [PATCH 22/24] Pass SSH wrappers to VCS commands unconditonally, not just if there's an SSH remote Summary: Ref T12961. In Mercurial, it's possible to have "subrepos" which may use a different protocol than the main repository. By putting an SSH repository inside an HTTP repository, an attacker can theoretically get us to execute `hg` without overriding `ui.ssh`, then execute code via the SSH hostname attack. As an immediate mitigation to this attack, specify `ui.ssh` unconditionally. Normally, this will have no effect (it will just be ignored). In the specific case of an SSH repo inside an HTTP repo, it will defuse the `ssh` protocol. For good measure and consistency, do the same for Subversion and Git. However, we don't normally maintain working copies for either Subversion or Git so it's unlikely that similar attacks exist there. Test Plan: - Put an SSH subrepo with an attack URI inside an HTTP outer repo in Mercurial. - Ran `hg up` with and without `ui.ssh` specified. - Got dangerous badness without `ui.ssh` and safe `ssh` subprocesses with `ui.ssh`. I'm not yet able to confirm that `hg pull -u -- ` can actually trigger this, but this can't hurt and our SSH wrapper is safer than the native behavior for all Subversion, Git and Mercurial versions released prior to today. Reviewers: chad Reviewed By: chad Subscribers: cspeckmim Maniphest Tasks: T12961 Differential Revision: https://secure.phabricator.com/D18389 --- .../protocol/DiffusionGitCommandEngine.php | 4 +--- .../protocol/DiffusionMercurialCommandEngine.php | 14 ++++++++------ .../protocol/DiffusionSubversionCommandEngine.php | 4 +--- .../__tests__/DiffusionCommandEngineTestCase.php | 4 ++-- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php b/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php index a16416ae1d..168b18caa5 100644 --- a/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php +++ b/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php @@ -26,9 +26,7 @@ final class DiffusionGitCommandEngine $env['HOME'] = PhabricatorEnv::getEmptyCWD(); - if ($this->isAnySSHProtocol()) { - $env['GIT_SSH'] = $this->getSSHWrapper(); - } + $env['GIT_SSH'] = $this->getSSHWrapper(); if ($this->isAnyHTTPProtocol()) { $uri = $this->getURI(); diff --git a/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php b/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php index 49c618b085..9499b8f5f1 100644 --- a/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php +++ b/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php @@ -11,12 +11,14 @@ final class DiffusionMercurialCommandEngine protected function newFormattedCommand($pattern, array $argv) { $args = array(); - if ($this->isAnySSHProtocol()) { - $pattern = "hg --config ui.ssh=%s {$pattern}"; - $args[] = $this->getSSHWrapper(); - } else { - $pattern = "hg {$pattern}"; - } + // NOTE: Here, and in Git and Subversion, we override the SSH command even + // if the repository does not use an SSH remote, since our SSH wrapper + // defuses an attack against older versions of Mercurial, Git and + // Subversion (see T12961) and it's possible to execute this attack + // in indirect ways, like by using an SSH subrepo inside an HTTP repo. + + $pattern = "hg --config ui.ssh=%s {$pattern}"; + $args[] = $this->getSSHWrapper(); return array($pattern, array_merge($args, $argv)); } diff --git a/src/applications/diffusion/protocol/DiffusionSubversionCommandEngine.php b/src/applications/diffusion/protocol/DiffusionSubversionCommandEngine.php index d8e9e6ff17..75b8785a6e 100644 --- a/src/applications/diffusion/protocol/DiffusionSubversionCommandEngine.php +++ b/src/applications/diffusion/protocol/DiffusionSubversionCommandEngine.php @@ -44,9 +44,7 @@ final class DiffusionSubversionCommandEngine protected function newCustomEnvironment() { $env = array(); - if ($this->isAnySSHProtocol()) { - $env['SVN_SSH'] = $this->getSSHWrapper(); - } + $env['SVN_SSH'] = $this->getSSHWrapper(); return $env; } diff --git a/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php b/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php index 76d9ad9170..1df3ea3522 100644 --- a/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php +++ b/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php @@ -26,7 +26,7 @@ final class DiffusionCommandEngineTestCase extends PhabricatorTestCase { )); $this->assertCommandEngineFormat( - 'hg xyz', + (string)csprintf('hg --config ui.ssh=%s xyz', $ssh_wrapper), array( 'LANG' => 'en_US.UTF-8', 'HGPLAIN' => '1', @@ -102,7 +102,7 @@ final class DiffusionCommandEngineTestCase extends PhabricatorTestCase { )); $this->assertCommandEngineFormat( - 'hg xyz', + (string)csprintf('hg --config ui.ssh=%s xyz', $ssh_wrapper), array( 'LANG' => 'en_US.UTF-8', 'HGPLAIN' => '1', From 2c150076b00f73918d6b3c86dc28e424317fc826 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Aug 2017 17:22:32 -0700 Subject: [PATCH 23/24] Stop populating or updating working copies in observed Mercurial repositories Summary: Ref T12961. Fixes T4416. Currently, for observed Mercurial repositories, we build a working copy with `pull -u` (for "update"). This should be unnecessary, and we don't do it for hosted Mercurial repositories. We also stopped doing it years ago for Git repositories. We also don't clone Mercurial repositories with a working copy. It's possible something has slipped through the cracks here so I'll hold this until after the release cut, but I believe there are no actual technical blockers here. Test Plan: - Observed a public Mercurial repository on Bitbucket. - Let it import. - Browsed commits, branches, file content, etc., without any apparent issues. Reviewers: chad Reviewed By: chad Subscribers: cspeckmim Maniphest Tasks: T12961, T4416 Differential Revision: https://secure.phabricator.com/D18390 --- .../repository/engine/PhabricatorRepositoryPullEngine.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php index 32e99e619d..10eddd38d4 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php @@ -529,7 +529,7 @@ final class PhabricatorRepositoryPullEngine // This is a local command, but needs credentials. $remote = $repository->getRemoteURIEnvelope(); - $future = $repository->getRemoteCommandFuture('pull -u -- %P', $remote); + $future = $repository->getRemoteCommandFuture('pull -- %P', $remote); $future->setCWD($path); try { From 45b0fd8f9b53c5a3712bc0ffc8f78eb416c1e70d Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Fri, 11 Aug 2017 05:50:43 -0700 Subject: [PATCH 24/24] Remove a debugging "echo" that crept in in dccd799b Summary: This echo was accidentally added in dccd799b Test Plan: Inspection. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D18391 --- src/applications/differential/view/DifferentialReviewersView.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php index 8ddea7c8c0..3fcb1c0ccf 100644 --- a/src/applications/differential/view/DifferentialReviewersView.php +++ b/src/applications/differential/view/DifferentialReviewersView.php @@ -158,7 +158,6 @@ final class DifferentialReviewersView extends AphrontView { private function isCurrent($action_phid) { if (!$this->diff) { - echo "A\n"; return true; }