From 99889a6321a75ad19de62ea5bb60b1f177ed0270 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 14 Aug 2016 06:43:22 -0700 Subject: [PATCH 01/22] Execute Harbormaster buildable filtering properly from HarbormasterBuildSearchEngine Summary: Ref T11473. When running `harbormaster.build.search` with a `buildables` constraint, the constraint doesn't get passed to the Query and so currently has no effect. This piece of logic was just accidentally omitted from D16356. It is probably not used anywhere today and doesn't show up in the UI, so it's easy to overlook (I missed it in review, too). Test Plan: Ran `harbormaster.build.search` with a `buildables` constraint, got expected filtering. Reviewers: yelirekim, chad Reviewed By: chad Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T11473 Differential Revision: https://secure.phabricator.com/D16395 --- .../harbormaster/query/HarbormasterBuildSearchEngine.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php index 948fea389c..4cf6a83701 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php +++ b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php @@ -61,6 +61,10 @@ final class HarbormasterBuildSearchEngine $query->withBuildPlanPHIDs($map['plans']); } + if ($map['buildables']) { + $query->withBuildablePHIDs($map['buildables']); + } + if ($map['statuses']) { $query->withBuildStatuses($map['statuses']); } From 07082d28678c0604ce567ad5547318da42942d9a Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 14 Aug 2016 07:22:23 -0700 Subject: [PATCH 02/22] Don't allow empty list constraints in Conduit calls Summary: Ref T11473. If you write a method like `get_stuff(ids)` and then call it with an empty list of IDs, you can end up passing an empty constraint to Conduit. If you run a `*.search` method with such a constraint, like this one: ``` { "ids": [] } ``` ...we have three possible beahviors: # Treat it like the user passed no constraint (basically, ignore the constraint). # Respect the constraint (return no results). # Error. Currently, we do (1). However, this is pretty confusing and I think clearly the worst option, since it means `get_stuff(array())` in client code will often tend to return a ton of results. We could do (2) instead, but this is also sort of confusing (it may not be obvious why nothing matched, even though it's an application bug) and I think most reasonable client code should be doing an `if ($ids)` test: this test makes clients a little more complicated, but they can save a network call, and I think they often need to do this test anyway (for example, to show the user a different message). This implements (3), and just considers these to be errors: this is the least tricky behavior, it's consistent with what we do in PHP, makes fairly good sense, and the only cost for this is that client code may need to be slightly more complex, but this slightly more complex code is usually better code. Test Plan: Ran Conduit `*.search` queries with `"ids":[]` and `"phids":[]`, got sensible errors instead of runaway result sets. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11473 Differential Revision: https://secure.phabricator.com/D16396 --- .../parametertype/ConduitListParameterType.php | 18 ++++++++++++++++++ .../parametertype/ConduitParameterType.php | 6 +++++- .../search/field/PhabricatorIDsSearchField.php | 3 ++- .../field/PhabricatorPHIDsSearchField.php | 3 ++- .../field/PhabricatorSearchDatasourceField.php | 3 ++- 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/applications/conduit/parametertype/ConduitListParameterType.php b/src/applications/conduit/parametertype/ConduitListParameterType.php index 10c81e8c5d..6ec3898ac2 100644 --- a/src/applications/conduit/parametertype/ConduitListParameterType.php +++ b/src/applications/conduit/parametertype/ConduitListParameterType.php @@ -3,6 +3,17 @@ abstract class ConduitListParameterType extends ConduitParameterType { + private $allowEmptyList = true; + + public function setAllowEmptyList($allow_empty_list) { + $this->allowEmptyList = $allow_empty_list; + return $this; + } + + public function getAllowEmptyList() { + return $this->allowEmptyList; + } + protected function getParameterValue(array $request, $key) { $value = parent::getParameterValue($request, $key); @@ -27,6 +38,13 @@ abstract class ConduitListParameterType pht('Expected a list, but value is an object.')); } + if (!$value && !$this->getAllowEmptyList()) { + $this->raiseValidationException( + $request, + $key, + pht('Expected a nonempty list, but value is an empty list.')); + } + return $value; } diff --git a/src/applications/conduit/parametertype/ConduitParameterType.php b/src/applications/conduit/parametertype/ConduitParameterType.php index 8f02057c0f..6468b099a0 100644 --- a/src/applications/conduit/parametertype/ConduitParameterType.php +++ b/src/applications/conduit/parametertype/ConduitParameterType.php @@ -61,7 +61,11 @@ abstract class ConduitParameterType extends Phobject { protected function raiseValidationException(array $request, $key, $message) { // TODO: Specialize this so we can give users more tailored messages from // Conduit. - throw new Exception($message); + throw new Exception( + pht( + 'Error while reading "%s": %s', + $key, + $message)); } diff --git a/src/applications/search/field/PhabricatorIDsSearchField.php b/src/applications/search/field/PhabricatorIDsSearchField.php index 909cce9c4a..cb8c27ef75 100644 --- a/src/applications/search/field/PhabricatorIDsSearchField.php +++ b/src/applications/search/field/PhabricatorIDsSearchField.php @@ -24,7 +24,8 @@ final class PhabricatorIDsSearchField } protected function newConduitParameterType() { - return new ConduitIntListParameterType(); + return id(new ConduitIntListParameterType()) + ->setAllowEmptyList(false); } } diff --git a/src/applications/search/field/PhabricatorPHIDsSearchField.php b/src/applications/search/field/PhabricatorPHIDsSearchField.php index 459233c472..c88ef7670a 100644 --- a/src/applications/search/field/PhabricatorPHIDsSearchField.php +++ b/src/applications/search/field/PhabricatorPHIDsSearchField.php @@ -24,7 +24,8 @@ final class PhabricatorPHIDsSearchField } protected function newConduitParameterType() { - return new ConduitPHIDListParameterType(); + return id(new ConduitPHIDListParameterType()) + ->setAllowEmptyList(false); } } diff --git a/src/applications/search/field/PhabricatorSearchDatasourceField.php b/src/applications/search/field/PhabricatorSearchDatasourceField.php index 6ba322936f..d8c1242ef4 100644 --- a/src/applications/search/field/PhabricatorSearchDatasourceField.php +++ b/src/applications/search/field/PhabricatorSearchDatasourceField.php @@ -22,7 +22,8 @@ final class PhabricatorSearchDatasourceField protected function newConduitParameterType() { if (!$this->conduitParameterType) { - return new ConduitStringListParameterType(); + return id(new ConduitStringListParameterType()) + ->setAllowEmptyList(false); } return $this->conduitParameterType; From 8c4122662465215de5d81699d28821ee04d3e6a3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 14 Aug 2016 08:58:17 -0700 Subject: [PATCH 03/22] Survive receipt of too much workboard column position information Summary: Fixes T11468. I was able to find //a// reproduction case for the issue, at least, described in my comments there. When the server sends back position information for columns we aren't displaying, for whatever reason, just ignore that information. Future work in T4900 to do more synchronization may revisit this. Test Plan: See T11468#190177 for steps. Did that stuff with this patch, got a clean edit. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11468 Differential Revision: https://secure.phabricator.com/D16397 --- resources/celerity/map.php | 22 +++++++++---------- .../js/application/projects/WorkboardBoard.js | 12 +++++++++- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 9296082d99..65f36d734d 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -422,7 +422,7 @@ return array( 'rsrc/js/application/phortune/phortune-credit-card-form.js' => '2290aeef', 'rsrc/js/application/policy/behavior-policy-control.js' => 'd0c516d5', 'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '5e9f347c', - 'rsrc/js/application/projects/WorkboardBoard.js' => '52291776', + 'rsrc/js/application/projects/WorkboardBoard.js' => 'fe7cb52a', 'rsrc/js/application/projects/WorkboardCard.js' => 'c587b80f', 'rsrc/js/application/projects/WorkboardColumn.js' => 'bae58312', 'rsrc/js/application/projects/WorkboardController.js' => '55baf5ed', @@ -750,7 +750,7 @@ return array( 'javelin-view-renderer' => '6c2b09a2', 'javelin-view-visitor' => 'efe49472', 'javelin-websocket' => 'e292eaf4', - 'javelin-workboard-board' => '52291776', + 'javelin-workboard-board' => 'fe7cb52a', 'javelin-workboard-card' => 'c587b80f', 'javelin-workboard-column' => 'bae58312', 'javelin-workboard-controller' => '55baf5ed', @@ -1297,15 +1297,6 @@ return array( 'javelin-vector', 'javelin-typeahead-static-source', ), - 52291776 => array( - 'javelin-install', - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-workflow', - 'phabricator-draggable-list', - 'javelin-workboard-column', - ), '5359e785' => array( 'javelin-install', 'javelin-util', @@ -2219,6 +2210,15 @@ return array( 'javelin-view-visitor', 'javelin-util', ), + 'fe7cb52a' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-workflow', + 'phabricator-draggable-list', + 'javelin-workboard-column', + ), 'fea0eb47' => array( 'javelin-install', ), diff --git a/webroot/rsrc/js/application/projects/WorkboardBoard.js b/webroot/rsrc/js/application/projects/WorkboardBoard.js index 4506041cca..82187d9f78 100644 --- a/webroot/rsrc/js/application/projects/WorkboardBoard.js +++ b/webroot/rsrc/js/application/projects/WorkboardBoard.js @@ -216,8 +216,18 @@ JX.install('WorkboardBoard', { } var column_maps = response.columnMaps; + var natural_column; for (var natural_phid in column_maps) { - this.getColumn(natural_phid).setNaturalOrder(column_maps[natural_phid]); + natural_column = this.getColumn(natural_phid); + if (!natural_column) { + // Our view of the board may be out of date, so we might get back + // information about columns that aren't visible. Just ignore the + // position information for any columns we aren't displaying on the + // client. + continue; + } + + natural_column.setNaturalOrder(column_maps[natural_phid]); } var property_maps = response.propertyMaps; From 15021a0bcc919cea77c5a8b03188e09c6b20f0f5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 14 Aug 2016 13:04:45 -0700 Subject: [PATCH 04/22] Fix bad array index test in Differential package code Summary: This needs an `isset()` for cases when authority and packages don't completely overlap. Test Plan: - With a package set to trigger autoreview, created a revision. - Observed error log, saw no more error. - Saw package trigger autoreview properly. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16398 --- .../differential/editor/DifferentialTransactionEditor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index bf2df0acee..7eaf5ef52d 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1533,7 +1533,7 @@ final class DifferentialTransactionEditor foreach ($packages as $key => $package) { $package_phid = $package->getPHID(); - if ($authority[$package_phid]) { + if (isset($authority[$package_phid])) { unset($packages[$key]); continue; } From f50e550c9ea069a345ff3614802200120bf8316e Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 15 Aug 2016 10:06:38 -0700 Subject: [PATCH 05/22] Correct various spelling errors Summary: Fixes T11477. Test Plan: Grep for Mulitple Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T11477 Differential Revision: https://secure.phabricator.com/D16399 --- .../config/option/PhabricatorApplicationConfigOptions.php | 2 +- .../settings/setting/PhabricatorEditorMultipleSetting.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/applications/config/option/PhabricatorApplicationConfigOptions.php b/src/applications/config/option/PhabricatorApplicationConfigOptions.php index dab28af776..34d74e88ab 100644 --- a/src/applications/config/option/PhabricatorApplicationConfigOptions.php +++ b/src/applications/config/option/PhabricatorApplicationConfigOptions.php @@ -234,7 +234,7 @@ abstract class PhabricatorApplicationConfigOptions extends Phobject { if (isset($options[$key])) { throw new Exception( pht( - "Mulitple %s subclasses contain an option named '%s'!", + "Multiple %s subclasses contain an option named '%s'!", __CLASS__, $key)); } diff --git a/src/applications/settings/setting/PhabricatorEditorMultipleSetting.php b/src/applications/settings/setting/PhabricatorEditorMultipleSetting.php index a5d64b9fcb..88e36e55d4 100644 --- a/src/applications/settings/setting/PhabricatorEditorMultipleSetting.php +++ b/src/applications/settings/setting/PhabricatorEditorMultipleSetting.php @@ -9,7 +9,7 @@ final class PhabricatorEditorMultipleSetting const VALUE_SINGLE = 'disable'; public function getSettingName() { - return pht('Edit Mulitple Files'); + return pht('Edit Multiple Files'); } public function getSettingPanelKey() { From 95cf83f14eadfab486a5129ee74c023c94fb541a Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 16 Aug 2016 14:00:00 -0700 Subject: [PATCH 06/22] Convert some whiny exceptions into quiet MalformedRequest exceptions Summary: Fixes T11480. This cleans up the error logs a little by quieting three common errors which are really malformed requests: - The CSRF error happens when bots hit anything which does write checks. - The "wrong cookie domain" errors happen when bots try to use the `security.alternate-file-domain` to browse stuff like `/auth/start/`. - The "no phcid" errors happen when bots try to go through the login flow. All of these are clearly communicated to human users, commonly encountered by bots, and not useful to log. I collapsed the `CSRFException` type into a standard malformed request exception, since nothing catches it and I can't really come up with a reason why anything would ever care. Test Plan: Hit each error through some level of `curl -H ...` and/or fakery. Verified that they showed to users before/after, but no longer log. Hit some other real errors, verified that they log. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11480 Differential Revision: https://secure.phabricator.com/D16402 --- src/__phutil_library_map__.php | 2 -- src/aphront/AphrontRequest.php | 35 +++++++++++-------- .../exception/AphrontCSRFException.php | 3 -- ...bricatorDefaultRequestExceptionHandler.php | 14 ++++++-- .../auth/provider/PhabricatorAuthProvider.php | 6 ++-- 5 files changed, 37 insertions(+), 23 deletions(-) delete mode 100644 src/aphront/exception/AphrontCSRFException.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index fc75e65838..b5426ac82e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -132,7 +132,6 @@ phutil_register_library_map(array( 'AphrontApplicationConfiguration' => 'aphront/configuration/AphrontApplicationConfiguration.php', 'AphrontBarView' => 'view/widget/bars/AphrontBarView.php', 'AphrontBoolHTTPParameterType' => 'aphront/httpparametertype/AphrontBoolHTTPParameterType.php', - 'AphrontCSRFException' => 'aphront/exception/AphrontCSRFException.php', 'AphrontCalendarEventView' => 'applications/calendar/view/AphrontCalendarEventView.php', 'AphrontController' => 'aphront/AphrontController.php', 'AphrontCursorPagerView' => 'view/control/AphrontCursorPagerView.php', @@ -4566,7 +4565,6 @@ phutil_register_library_map(array( 'AphrontApplicationConfiguration' => 'Phobject', 'AphrontBarView' => 'AphrontView', 'AphrontBoolHTTPParameterType' => 'AphrontHTTPParameterType', - 'AphrontCSRFException' => 'AphrontException', 'AphrontCalendarEventView' => 'AphrontView', 'AphrontController' => 'Phobject', 'AphrontCursorPagerView' => 'AphrontView', diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index a49821e10b..45b383c839 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -261,25 +261,30 @@ final class AphrontRequest extends Phobject { // Add some diagnostic details so we can figure out if some CSRF issues // are JS problems or people accessing Ajax URIs directly with their // browsers. - $more_info = array(); + $info = array(); + + $info[] = pht( + 'You are trying to save some data to Phabricator, but the request '. + 'your browser made included an incorrect token. Reload the page '. + 'and try again. You may need to clear your cookies.'); if ($this->isAjax()) { - $more_info[] = pht('This was an Ajax request.'); + $info[] = pht('This was an Ajax request.'); } else { - $more_info[] = pht('This was a Web request.'); + $info[] = pht('This was a Web request.'); } if ($token) { - $more_info[] = pht('This request had an invalid CSRF token.'); + $info[] = pht('This request had an invalid CSRF token.'); } else { - $more_info[] = pht('This request had no CSRF token.'); + $info[] = pht('This request had no CSRF token.'); } // Give a more detailed explanation of how to avoid the exception // in developer mode. if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { // TODO: Clean this up, see T1921. - $more_info[] = pht( + $info[] = pht( "To avoid this error, use %s to construct forms. If you are already ". "using %s, make sure the form 'action' uses a relative URI (i.e., ". "begins with a '%s'). Forms using absolute URIs do not include CSRF ". @@ -299,16 +304,16 @@ final class AphrontRequest extends Phobject { 'setRenderAsForm(true)'); } + $message = implode("\n", $info); + // This should only be able to happen if you load a form, pull your // internet for 6 hours, and then reconnect and immediately submit, // but give the user some indication of what happened since the workflow // is incredibly confusing otherwise. - throw new AphrontCSRFException( - pht( - 'You are trying to save some data to Phabricator, but the request '. - 'your browser made included an incorrect token. Reload the page '. - 'and try again. You may need to clear your cookies.')."\n\n". - implode("\n", $more_info)); + throw new AphrontMalformedRequestException( + pht('Invalid Request (CSRF)'), + $message, + true); } return true; @@ -480,7 +485,8 @@ final class AphrontRequest extends Phobject { $configured_as = PhabricatorEnv::getEnvConfig('phabricator.base-uri'); $accessed_as = $this->getHost(); - throw new Exception( + throw new AphrontMalformedRequestException( + pht('Bad Host Header'), pht( 'This Phabricator install is configured as "%s", but you are '. 'using the domain name "%s" to access a page which is trying to '. @@ -488,7 +494,8 @@ final class AphrontRequest extends Phobject { 'domain or a configured alternate domain. Phabricator will not '. 'set cookies on other domains for security reasons.', $configured_as, - $accessed_as)); + $accessed_as), + true); } $base_domain = $base_domain_uri->getDomain(); diff --git a/src/aphront/exception/AphrontCSRFException.php b/src/aphront/exception/AphrontCSRFException.php deleted file mode 100644 index c7ff45e82f..0000000000 --- a/src/aphront/exception/AphrontCSRFException.php +++ /dev/null @@ -1,3 +0,0 @@ -getViewer($request); - // Always log the unhandled exception. - phlog($ex); + // Some types of uninteresting request exceptions don't get logged, usually + // because they are caused by the background radiation of bot traffic on + // the internet. These include requests with bad CSRF tokens and + // questionable "Host" headers. + $should_log = true; + if ($ex instanceof AphrontMalformedRequestException) { + $should_log = !$ex->getIsUnlogged(); + } + + if ($should_log) { + phlog($ex); + } $class = get_class($ex); $message = $ex->getMessage(); diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php index c949764c9a..a3f77618e4 100644 --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -464,12 +464,14 @@ abstract class PhabricatorAuthProvider extends Phobject { public function getAuthCSRFCode(AphrontRequest $request) { $phcid = $request->getCookie(PhabricatorCookies::COOKIE_CLIENTID); if (!strlen($phcid)) { - throw new Exception( + throw new AphrontMalformedRequestException( + pht('Missing Client ID Cookie'), pht( 'Your browser did not submit a "%s" cookie with client state '. 'information in the request. Check that cookies are enabled. '. 'If this problem persists, you may need to clear your cookies.', - PhabricatorCookies::COOKIE_CLIENTID)); + PhabricatorCookies::COOKIE_CLIENTID), + true); } return PhabricatorHash::digest($phcid); From 05f7227329d0c27cf092127b84faae7e4a1e4b16 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 16 Aug 2016 15:57:00 -0700 Subject: [PATCH 07/22] Document how to manually close revisions Summary: Fixes T11484. These mechanisms aren't necessarily obvious and make sense to document here. Test Plan: Read documentation. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11484 Differential Revision: https://secure.phabricator.com/D16404 --- .../user/userguide/diffusion_autoclose.diviner | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/docs/user/userguide/diffusion_autoclose.diviner b/src/docs/user/userguide/diffusion_autoclose.diviner index 6d6f76bf87..f2533bbdc6 100644 --- a/src/docs/user/userguide/diffusion_autoclose.diviner +++ b/src/docs/user/userguide/diffusion_autoclose.diviner @@ -59,6 +59,24 @@ not necessarily that the repository is still importing. - //Field Not Present// This commit was processed before we implemented this diagnostic feature, and no information is available. + +Manually Closing Revisions +========================== + +If autoclose didn't activate for some reason and you want to manually close +revisions, you can do so in several ways: + +**Close Revision**: The revision author can use the "Close Revision" action +from the web UI, located in the action dropdown on the revision page (where +reviewers would "Accept Revision" or "Request Changes"). + +`differential.always-allow-close`: If you set this option in {nav Config}, +any user can take the "Close Revision" action in the web UI. + +`arc close-revision`: You can close revisions from the command line by using +`arc close-revision`. + + Next Steps ========== From a35b03ac6ad99de93727a2662aa7ce3a8b7895cf Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 16 Aug 2016 15:46:47 -0700 Subject: [PATCH 08/22] Update Preamble documentation for clusters with mixed request sources and loadbalancer chains Summary: Fixes T11487. Improve documentation for three situations: - When you configure a cluster behind a load balancer, all requests are trusted but not all have an "X-Forwarded-For" header. Change the suggested snippet to read this header only if it exists. - When a request goes through a series of load balancers (as with a CDN) they can end up writing a list of IPs to the header. Parse these. - Remove the "rate limiting" stuff -- this got disabled/removed a long time ago and is misleading/incorrect. Test Plan: Read documentation. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11487 Differential Revision: https://secure.phabricator.com/D16403 --- .../configuring_preamble.diviner | 85 +++++++++---------- 1 file changed, 41 insertions(+), 44 deletions(-) diff --git a/src/docs/user/configuration/configuring_preamble.diviner b/src/docs/user/configuration/configuring_preamble.diviner index a8710598bf..bb76600d3b 100644 --- a/src/docs/user/configuration/configuring_preamble.diviner +++ b/src/docs/user/configuration/configuring_preamble.diviner @@ -4,7 +4,8 @@ Adjust environmental settings (SSL, remote IP, rate limiting) using a preamble script. -= Overview = +Overview +======== If Phabricator is deployed in an environment where HTTP headers behave oddly (usually, because it is behind a load balancer), it may not be able to detect @@ -37,27 +38,52 @@ If present, this script will be executed at the very beginning of each web request, allowing you to adjust the environment. For common adjustments and examples, see the next sections. -= Adjusting Client IPs = +Adjusting Client IPs +==================== If your install is behind a load balancer, Phabricator may incorrectly detect -all requests as originating from the load balancer, rather than from the correct -client IPs. If this is the case and some other header (like `X-Forwarded-For`) -is known to be trustworthy, you can overwrite the `REMOTE_ADDR` setting so -Phabricator can figure out the client IP correctly: +all requests as originating from the load balancer, rather than from the +correct client IPs. + +If this is the case and some other header (like `X-Forwarded-For`) is known to +be trustworthy, you can read the header and overwrite the `REMOTE_ADDR` value +so Phabricator can figure out the client IP correctly. + +You should do this //only// if the `X-Forwarded-For` header is known to be +trustworthy. In particular, if users can make requests to the web server +directly, they can provide an arbitrary `X-Forwarded-For` header, and thereby +spoof an arbitrary client IP. + +The `X-Forwarded-For` header may also contain a list of addresses if a request +has been forwarded through multiple loadbalancers. Using a snippet like this +will usually handle most situations correctly: ``` name=Overwrite REMOTE_ADDR with X-Forwarded-For Date: Tue, 16 Aug 2016 17:28:43 -0700 Subject: [PATCH 09/22] Fix a typo in "Internationalization" documentation Summary: Caught this while linking to it from D16405. Test Plan: Consulted a dictionary. Reviewers: chad, alexmv Reviewed By: alexmv Differential Revision: https://secure.phabricator.com/D16406 --- src/docs/contributor/internationalization.diviner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/docs/contributor/internationalization.diviner b/src/docs/contributor/internationalization.diviner index f00d09a5db..e9cdb9dfcd 100644 --- a/src/docs/contributor/internationalization.diviner +++ b/src/docs/contributor/internationalization.diviner @@ -367,7 +367,7 @@ produce a consistent message about a common error state in a convenient way. There are a handful of error strings in the codebase which may be used before the translation framework is loaded, or may be used during handling other -errors, possibly rasised from within the translation framework. This handful +errors, possibly raised from within the translation framework. This handful of special cases are left untranslated to prevent fatals and cycles in the error handler. From f659b8743a019ed3647454d1609dbc1d9ee27394 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 17 Aug 2016 08:34:59 -0700 Subject: [PATCH 10/22] Fix Herald test adapter for commits Summary: Fixes T11488. I broke this in D16360, I think by doing a little extra refactoring after testing it. This code is very old, before commits always needed to have repositories attached in order to do policy checks. Modernize it by mostly just using the repository which is present on the Commit object, and using the existing edge cache. Test Plan: Ran a commit through the Herald test adapter. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11488 Differential Revision: https://secure.phabricator.com/D16413 --- .../diffusion/herald/HeraldCommitAdapter.php | 36 +++++++------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php index 538753f669..bd34fe23cc 100644 --- a/src/applications/diffusion/herald/HeraldCommitAdapter.php +++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php @@ -7,7 +7,6 @@ final class HeraldCommitAdapter protected $diff; protected $revision; - protected $repository; protected $commit; protected $commitData; private $commitDiff; @@ -42,6 +41,7 @@ final class HeraldCommitAdapter public function setObject($object) { $this->commit = $object; + return $this; } @@ -86,41 +86,26 @@ final class HeraldCommitAdapter } public function getTriggerObjectPHIDs() { + $project_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST; + return array_merge( array( - $this->repository->getPHID(), + $this->getRepository()->getPHID(), $this->getPHID(), ), - $this->repository->getProjectPHIDs()); + $this->loadEdgePHIDs($project_type)); } public function explainValidTriggerObjects() { return pht('This rule can trigger for **repositories** and **projects**.'); } - public static function newLegacyAdapter( - PhabricatorRepository $repository, - PhabricatorRepositoryCommit $commit, - PhabricatorRepositoryCommitData $commit_data) { - - $object = new HeraldCommitAdapter(); - - $commit->attachRepository($repository); - - $object->repository = $repository; - $object->commit = $commit; - $object->commitData = $commit_data; - - return $object; - } - public function setCommit(PhabricatorRepositoryCommit $commit) { $viewer = PhabricatorUser::getOmnipotentUser(); $repository = id(new PhabricatorRepositoryQuery()) ->setViewer($viewer) ->withIDs(array($commit->getRepositoryID())) - ->needProjectPHIDs(true) ->executeOne(); if (!$repository) { throw new Exception(pht('Unable to load repository!')); @@ -137,7 +122,6 @@ final class HeraldCommitAdapter $this->commit->attachRepository($repository); $this->commit->attachCommitData($data); - $this->repository = $repository; $this->commitData = $data; return $this; @@ -150,7 +134,7 @@ final class HeraldCommitAdapter public function loadAffectedPaths() { if ($this->affectedPaths === null) { $result = PhabricatorOwnerPathQuery::loadAffectedPaths( - $this->repository, + $this->getRepository(), $this->commit, PhabricatorUser::getOmnipotentUser()); $this->affectedPaths = $result; @@ -161,7 +145,7 @@ final class HeraldCommitAdapter public function loadAffectedPackages() { if ($this->affectedPackages === null) { $packages = PhabricatorOwnersPackage::loadAffectedPackages( - $this->repository, + $this->getRepository(), $this->loadAffectedPaths()); $this->affectedPackages = $packages; } @@ -314,7 +298,7 @@ final class HeraldCommitAdapter $drequest = DiffusionRequest::newFromDictionary( array( 'user' => $viewer, - 'repository' => $this->repository, + 'repository' => $this->getRepository(), 'commit' => $this->commit->getCommitIdentifier(), )); @@ -325,6 +309,10 @@ final class HeraldCommitAdapter $params); } + private function getRepository() { + return $this->getObject()->getRepository(); + } + /* -( HarbormasterBuildableAdapterInterface )------------------------------ */ From 37b8ec5bb73eedb1820da27e39944941039eef2e Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 17 Aug 2016 06:29:46 -0700 Subject: [PATCH 11/22] Provide an "--output" mode for "bin/storage dump" with better error handling Summary: Ref T6996. If you do this kind of thing in the shell, you don't get a good error by default if the `dump` command fails: ``` $ bin/storage dump | gzip > output.sql.gz ``` This can be worked around with some elaborate bash tricks, but they're really clunky and uninintuitive. We also need to do this in several places (while writing backups; while performing exports), and I don't want to copy clunky bash tricks all over the codebase. Instead, provide `--output` and `--compress` flags which just do this processing inside `bin/storage dump`. It will fail appropriately if any of the underlying operations fail. This also makes the write a little safer (refuses to overwrite) and the code more reusable. Test Plan: - Did three dumps, with no flags, `--output`, and `--output --compress`. - Verified all three took similar amounts of time and were identical except for "Date Exported" timestamps in comments (except that the compressed one was compressed). - Used `gunzip` to examine the compressed one, verified it was really compressed. - Faked a write error, saw properly command behavior (clean up file + exit with error). Reviewers: chad Reviewed By: chad Maniphest Tasks: T6996 Differential Revision: https://secure.phabricator.com/D16407 --- ...abricatorStorageManagementDumpWorkflow.php | 128 +++++++++++++++++- 1 file changed, 125 insertions(+), 3 deletions(-) diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php index 1aec53fd63..6a481d4702 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php @@ -16,6 +16,26 @@ final class PhabricatorStorageManagementDumpWorkflow 'Add __--master-data__ to the __mysqldump__ command, '. 'generating a CHANGE MASTER statement in the output.'), ), + array( + 'name' => 'output', + 'param' => 'file', + 'help' => pht( + 'Write output directly to disk. This handles errors better '. + 'than using pipes. Use with __--compress__ to gzip the '. + 'output.'), + ), + array( + 'name' => 'compress', + 'help' => pht( + 'With __--output__, write a compressed file to disk instead '. + 'of a plaintext file.'), + ), + array( + 'name' => 'overwrite', + 'help' => pht( + 'With __--output__, overwrite the output file if it already '. + 'exists.'), + ), )); } @@ -55,6 +75,38 @@ final class PhabricatorStorageManagementDumpWorkflow } } + $output_file = $args->getArg('output'); + $is_compress = $args->getArg('compress'); + $is_overwrite = $args->getArg('overwrite'); + + if ($is_compress) { + if ($output_file === null) { + throw new PhutilArgumentUsageException( + pht( + 'The "--compress" flag can only be used alongside "--output".')); + } + } + + if ($is_overwrite) { + if ($output_file === null) { + throw new PhutilArgumentUsageException( + pht( + 'The "--overwrite" flag can only be used alongside "--output".')); + } + } + + if ($output_file !== null) { + if (Filesystem::pathExists($output_file)) { + if (!$is_overwrite) { + throw new PhutilArgumentUsageException( + pht( + 'Output file "%s" already exists. Use "--overwrite" '. + 'to overwrite.', + $output_file)); + } + } + } + $argv = array(); $argv[] = '--hex-blob'; $argv[] = '--single-transaction'; @@ -79,13 +131,83 @@ final class PhabricatorStorageManagementDumpWorkflow $argv[] = $database; } + if ($has_password) { - $err = phutil_passthru('mysqldump -p%P %Ls', $password, $argv); + $command = csprintf('mysqldump -p%P %Ls', $password, $argv); } else { - $err = phutil_passthru('mysqldump %Ls', $argv); + $command = csprintf('mysqldump %Ls', $argv); } - return $err; + // If we aren't writing to a file, just passthru the command. + if ($output_file === null) { + return phutil_passthru('%C', $command); + } + + // If we are writing to a file, stream the command output to disk. This + // mode makes sure the whole command fails if there's an error (commonly, + // a full disk). See T6996 for discussion. + + if ($is_compress) { + $file = gzopen($output_file, 'wb'); + } else { + $file = fopen($output_file, 'wb'); + } + + if (!$file) { + throw new Exception( + pht( + 'Failed to open file "%s" for writing.', + $file)); + } + + $future = new ExecFuture('%C', $command); + + $lines = new LinesOfALargeExecFuture($future); + + try { + foreach ($lines as $line) { + $line = $line."\n"; + if ($is_compress) { + $ok = gzwrite($file, $line); + } else { + $ok = fwrite($file, $line); + } + + if ($ok !== strlen($line)) { + throw new Exception( + pht( + 'Failed to write %d byte(s) to file "%s".', + new PhutilNumber(strlen($line)), + $output_file)); + } + } + + if ($is_compress) { + $ok = gzclose($file); + } else { + $ok = fclose($file); + } + + if ($ok !== true) { + throw new Exception( + pht( + 'Failed to close file "%s".', + $output_file)); + } + } catch (Exception $ex) { + // If we might have written a partial file to disk, try to remove it so + // we don't leave any confusing artifacts laying around. + + try { + Filesystem::remove($output_file); + } catch (Exception $ex) { + // Ignore any errors we hit. + } + + throw $ex; + } + + return 0; } } From 2c5b1dc20a6bd0ba593237dc430585e9a5827b8a Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 17 Aug 2016 07:59:59 -0700 Subject: [PATCH 12/22] Provide "--output" flags for "bin/storage renamespace" Summary: Ref T6996. Depends on D16407. This does the same stuff as D16407, but for `bin/storage renamespace`. In particular: - Support writing directly to a file (so we can get good errors on failure). - Support in-process compression. Also add support for reading out of a `storage dump` subprocess, so we don't have to do a dump-to-disk + renamespace + compress dance and can just stream out of MySQL directly to a compressed file on disk. This is used in the second stage of instance exports (see T7148). It would be nice to share more code with `bin/storage dump`, and possibly to just make this a flag for it, although we still do need to do the file-based version when importing (vs exporting). I figured that was better left for another time. Test Plan: Ran `bin/storage renamespace --live --output x --from A --to B --compress --overwrite` and similar commands. Verified that a compressed, renamespaced dump came out of the other end. Reviewers: chad Reviewed By: chad Maniphest Tasks: T6996 Differential Revision: https://secure.phabricator.com/D16410 --- ...orStorageManagementRenamespaceWorkflow.php | 158 +++++++++++++++--- 1 file changed, 137 insertions(+), 21 deletions(-) diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php index e64242344b..ad5afac7fb 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php @@ -8,15 +8,20 @@ final class PhabricatorStorageManagementRenamespaceWorkflow ->setName('renamespace') ->setExamples( '**renamespace** [__options__] '. - '--in __dump.sql__ --from __old__ --to __new__ > __out.sql__') + '--input __dump.sql__ --from __old__ --to __new__ > __out.sql__') ->setSynopsis(pht('Change the database namespace of a .sql dump file.')) ->setArguments( array( array( - 'name' => 'in', + 'name' => 'input', 'param' => 'file', 'help' => pht('SQL dumpfile to process.'), ), + array( + 'name' => 'live', + 'help' => pht( + 'Generate a live dump instead of processing a file on disk.'), + ), array( 'name' => 'from', 'param' => 'namespace', @@ -27,6 +32,21 @@ final class PhabricatorStorageManagementRenamespaceWorkflow 'param' => 'namespace', 'help' => pht('Desired database namespace for output.'), ), + array( + 'name' => 'output', + 'param' => 'file', + 'help' => pht('Write output directly to a file on disk.'), + ), + array( + 'name' => 'compress', + 'help' => pht('Emit gzipped output instead of plain text.'), + ), + array( + 'name' => 'overwrite', + 'help' => pht( + 'With __--output__, write to disk even if the file already '. + 'exists.'), + ), )); } @@ -37,12 +57,13 @@ final class PhabricatorStorageManagementRenamespaceWorkflow public function didExecute(PhutilArgumentParser $args) { $console = PhutilConsole::getConsole(); - $in = $args->getArg('in'); - if (!strlen($in)) { + $input = $args->getArg('input'); + $is_live = $args->getArg('live'); + if (!strlen($input) && !$is_live) { throw new PhutilArgumentUsageException( pht( - 'Specify the dumpfile to read with %s.', - '--in')); + 'Specify the dumpfile to read with "--in", or use "--live" to '. + 'generate one automatically.')); } $from = $args->getArg('from'); @@ -61,6 +82,62 @@ final class PhabricatorStorageManagementRenamespaceWorkflow '--to')); } + + $output_file = $args->getArg('output'); + $is_overwrite = $args->getArg('overwrite'); + $is_compress = $args->getArg('compress'); + + if ($is_overwrite) { + if ($output_file === null) { + throw new PhutilArgumentUsageException( + pht( + 'The "--overwrite" flag can only be used alongside "--output".')); + } + } + + if ($output_file !== null) { + if (Filesystem::pathExists($output_file)) { + if (!$is_overwrite) { + throw new PhutilArgumentUsageException( + pht( + 'Output file "%s" already exists. Use "--overwrite" '. + 'to overwrite.', + $output_file)); + } + } + } + + if ($is_live) { + $root = dirname(phutil_get_library_root('phabricator')); + + $future = new ExecFuture( + '%R dump', + $root.'/bin/storage'); + + $lines = new LinesOfALargeExecFuture($future); + } else { + $lines = new LinesOfALargeFile($input); + } + + if ($output_file === null) { + $file = fopen('php://stdout', 'wb'); + $output_name = pht('stdout'); + } else { + if ($is_compress) { + $file = gzopen($output_file, 'wb'); + } else { + $file = fopen($output_file, 'wb'); + } + $output_name = $output_file; + } + + if (!$file) { + throw new Exception( + pht( + 'Failed to open output file "%s" for writing.', + $output_name)); + } + $patterns = array( 'use' => '@^(USE `)([^_]+)(_.*)$@', 'create' => '@^(CREATE DATABASE /\*.*?\*/ `)([^_]+)(_.*)$@', @@ -68,27 +145,66 @@ final class PhabricatorStorageManagementRenamespaceWorkflow $found = array_fill_keys(array_keys($patterns), 0); - $matches = null; - foreach (new LinesOfALargeFile($in) as $line) { + try { + $matches = null; + foreach ($lines as $line) { - foreach ($patterns as $key => $pattern) { - if (preg_match($pattern, $line, $matches)) { - $namespace = $matches[2]; - if ($namespace != $from) { - throw new Exception( - pht( - 'Expected namespace "%s", found "%s": %s.', - $from, - $namespace, - $line)); + foreach ($patterns as $key => $pattern) { + if (preg_match($pattern, $line, $matches)) { + $namespace = $matches[2]; + if ($namespace != $from) { + throw new Exception( + pht( + 'Expected namespace "%s", found "%s": %s.', + $from, + $namespace, + $line)); + } + + $line = $matches[1].$to.$matches[3]; + $found[$key]++; } + } - $line = $matches[1].$to.$matches[3]; - $found[$key]++; + $data = $line."\n"; + + if ($is_compress) { + $bytes = gzwrite($file, $data); + } else { + $bytes = fwrite($file, $data); + } + + if ($bytes !== strlen($data)) { + throw new Exception( + pht( + 'Failed to write %d byte(s) to "%s".', + new PhutilNumber(strlen($data)), + $output_name)); } } - echo $line."\n"; + if ($is_compress) { + $ok = gzclose($file); + } else { + $ok = fclose($file); + } + + if ($ok !== true) { + throw new Exception( + pht( + 'Failed to close file "%s".', + $output_file)); + } + } catch (Exception $ex) { + try { + if ($output_file !== null) { + Filesystem::remove($output_file); + } + } catch (Exception $ex) { + // Ignore any exception. + } + + throw $ex; } // Give the user a chance to catch things if the results are crazy. From 94c746e1d67fcee8b181e34581552df02e7837ff Mon Sep 17 00:00:00 2001 From: Chad Little Date: Wed, 17 Aug 2016 09:36:32 -0700 Subject: [PATCH 13/22] Rough in Guides Application Summary: Ref T11132, Ref T11478. Builds out a basic PHUICMSView and Guides Application, no content / modules. Test Plan: Go to /guides/, see blank states for new guides. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T11132, T11478 Differential Revision: https://secure.phabricator.com/D16414 --- resources/celerity/map.php | 2 + src/__phutil_library_map__.php | 12 ++ .../PhabricatorGuideApplication.php | 41 ++++++ .../controller/PhabricatorGuideController.php | 21 ++++ .../PhabricatorGuideInstallController.php | 46 +++++++ .../PhabricatorGuideQuickStartController.php | 46 +++++++ .../PhabricatorGuideWelcomeController.php | 46 +++++++ src/view/phui/PHUICMSView.php | 118 ++++++++++++++++++ webroot/rsrc/css/phui/phui-cms.css | 47 +++++++ 9 files changed, 379 insertions(+) create mode 100644 src/applications/guides/application/PhabricatorGuideApplication.php create mode 100644 src/applications/guides/controller/PhabricatorGuideController.php create mode 100644 src/applications/guides/controller/PhabricatorGuideInstallController.php create mode 100644 src/applications/guides/controller/PhabricatorGuideQuickStartController.php create mode 100644 src/applications/guides/controller/PhabricatorGuideWelcomeController.php create mode 100644 src/view/phui/PHUICMSView.php create mode 100644 webroot/rsrc/css/phui/phui-cms.css diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 65f36d734d..b45c7d3280 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -127,6 +127,7 @@ return array( 'rsrc/css/phui/phui-box.css' => '5c8387cf', 'rsrc/css/phui/phui-button.css' => '4a5fbe3d', 'rsrc/css/phui/phui-chart.css' => '6bf6f78e', + 'rsrc/css/phui/phui-cms.css' => '33064557', 'rsrc/css/phui/phui-crumbs-view.css' => '9dac418c', 'rsrc/css/phui/phui-curtain-view.css' => '7148ae25', 'rsrc/css/phui/phui-document-pro.css' => 'dc3d46ed', @@ -832,6 +833,7 @@ return array( 'phui-calendar-list-css' => 'fcc9fb41', 'phui-calendar-month-css' => '8e10e92c', 'phui-chart-css' => '6bf6f78e', + 'phui-cms-css' => '33064557', 'phui-crumbs-view-css' => '9dac418c', 'phui-curtain-view-css' => '7148ae25', 'phui-document-summary-view-css' => '9ca48bdf', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index b5426ac82e..d98c735f08 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1597,6 +1597,7 @@ phutil_register_library_map(array( 'PHUIButtonBarView' => 'view/phui/PHUIButtonBarView.php', 'PHUIButtonExample' => 'applications/uiexample/examples/PHUIButtonExample.php', 'PHUIButtonView' => 'view/phui/PHUIButtonView.php', + 'PHUICMSView' => 'view/phui/PHUICMSView.php', 'PHUICalendarDayView' => 'view/phui/calendar/PHUICalendarDayView.php', 'PHUICalendarListView' => 'view/phui/calendar/PHUICalendarListView.php', 'PHUICalendarMonthView' => 'view/phui/calendar/PHUICalendarMonthView.php', @@ -2629,6 +2630,11 @@ phutil_register_library_map(array( 'PhabricatorGlobalLock' => 'infrastructure/util/PhabricatorGlobalLock.php', 'PhabricatorGlobalUploadTargetView' => 'applications/files/view/PhabricatorGlobalUploadTargetView.php', 'PhabricatorGoogleAuthProvider' => 'applications/auth/provider/PhabricatorGoogleAuthProvider.php', + 'PhabricatorGuideApplication' => 'applications/guides/application/PhabricatorGuideApplication.php', + 'PhabricatorGuideController' => 'applications/guides/controller/PhabricatorGuideController.php', + 'PhabricatorGuideInstallController' => 'applications/guides/controller/PhabricatorGuideInstallController.php', + 'PhabricatorGuideQuickStartController' => 'applications/guides/controller/PhabricatorGuideQuickStartController.php', + 'PhabricatorGuideWelcomeController' => 'applications/guides/controller/PhabricatorGuideWelcomeController.php', 'PhabricatorHTTPParameterTypeTableView' => 'applications/config/view/PhabricatorHTTPParameterTypeTableView.php', 'PhabricatorHandleList' => 'applications/phid/handle/pool/PhabricatorHandleList.php', 'PhabricatorHandleObjectSelectorDataView' => 'applications/phid/handle/view/PhabricatorHandleObjectSelectorDataView.php', @@ -6243,6 +6249,7 @@ phutil_register_library_map(array( 'PHUIButtonBarView' => 'AphrontTagView', 'PHUIButtonExample' => 'PhabricatorUIExample', 'PHUIButtonView' => 'AphrontTagView', + 'PHUICMSView' => 'AphrontTagView', 'PHUICalendarDayView' => 'AphrontView', 'PHUICalendarListView' => 'AphrontTagView', 'PHUICalendarMonthView' => 'AphrontView', @@ -7429,6 +7436,11 @@ phutil_register_library_map(array( 'PhabricatorGlobalLock' => 'PhutilLock', 'PhabricatorGlobalUploadTargetView' => 'AphrontView', 'PhabricatorGoogleAuthProvider' => 'PhabricatorOAuth2AuthProvider', + 'PhabricatorGuideApplication' => 'PhabricatorApplication', + 'PhabricatorGuideController' => 'PhabricatorController', + 'PhabricatorGuideInstallController' => 'PhabricatorGuideController', + 'PhabricatorGuideQuickStartController' => 'PhabricatorGuideController', + 'PhabricatorGuideWelcomeController' => 'PhabricatorGuideController', 'PhabricatorHTTPParameterTypeTableView' => 'AphrontView', 'PhabricatorHandleList' => array( 'Phobject', diff --git a/src/applications/guides/application/PhabricatorGuideApplication.php b/src/applications/guides/application/PhabricatorGuideApplication.php new file mode 100644 index 0000000000..66f678489a --- /dev/null +++ b/src/applications/guides/application/PhabricatorGuideApplication.php @@ -0,0 +1,41 @@ + array( + '' => 'PhabricatorGuideWelcomeController', + 'install/' + => 'PhabricatorGuideInstallController', + 'quickstart/' + => 'PhabricatorGuideQuickStartController', + ), + ); + } + +} diff --git a/src/applications/guides/controller/PhabricatorGuideController.php b/src/applications/guides/controller/PhabricatorGuideController.php new file mode 100644 index 0000000000..5aac89ba00 --- /dev/null +++ b/src/applications/guides/controller/PhabricatorGuideController.php @@ -0,0 +1,21 @@ +setBaseURI(new PhutilURI($this->getApplicationURI())); + $nav->addLabel(pht('Guides')); + $nav->addFilter('/', pht('Welcome')); + $nav->addFilter('install/', pht('Installation Guide')); + $nav->addFilter('quickstart/', pht('Quick Start Guide')); + + return $nav; + } + + public function buildApplicationMenu() { + return $this->buildSideNavView(null, true)->getMenu(); + } + +} diff --git a/src/applications/guides/controller/PhabricatorGuideInstallController.php b/src/applications/guides/controller/PhabricatorGuideInstallController.php new file mode 100644 index 0000000000..6781edaeac --- /dev/null +++ b/src/applications/guides/controller/PhabricatorGuideInstallController.php @@ -0,0 +1,46 @@ +getViewer(); + + $title = pht('Installation Guide'); + + $nav = $this->buildSideNavView(); + $nav->selectFilter('install/'); + + $header = id(new PHUIHeaderView()) + ->setHeader($title) + ->setProfileHeader(true); + + $crumbs = $this->buildApplicationCrumbs() + ->addTextCrumb(pht('Installation')); + + $content = null; + + $view = id(new PHUICMSView()) + ->setCrumbs($crumbs) + ->setNavigation($nav) + ->setHeader($header) + ->setContent($content); + + return $this->newPage() + ->setTitle($title) + ->addClass('phui-cms-body') + ->appendChild($view); + + } + + private function getGuideContent() { + + $guide = null; + + return $guide; + } +} diff --git a/src/applications/guides/controller/PhabricatorGuideQuickStartController.php b/src/applications/guides/controller/PhabricatorGuideQuickStartController.php new file mode 100644 index 0000000000..0b6d315642 --- /dev/null +++ b/src/applications/guides/controller/PhabricatorGuideQuickStartController.php @@ -0,0 +1,46 @@ +getViewer(); + + $title = pht('Quick Start Guide'); + + $nav = $this->buildSideNavView(); + $nav->selectFilter('quickstart/'); + + $header = id(new PHUIHeaderView()) + ->setHeader($title) + ->setProfileHeader(true); + + $crumbs = $this->buildApplicationCrumbs() + ->addTextCrumb(pht('Quick Start')); + + $content = null; + + $view = id(new PHUICMSView()) + ->setCrumbs($crumbs) + ->setNavigation($nav) + ->setHeader($header) + ->setContent($content); + + return $this->newPage() + ->setTitle($title) + ->addClass('phui-cms-body') + ->appendChild($view); + + } + + private function getGuideContent() { + + $guide = null; + + return $guide; + } +} diff --git a/src/applications/guides/controller/PhabricatorGuideWelcomeController.php b/src/applications/guides/controller/PhabricatorGuideWelcomeController.php new file mode 100644 index 0000000000..62a1a57567 --- /dev/null +++ b/src/applications/guides/controller/PhabricatorGuideWelcomeController.php @@ -0,0 +1,46 @@ +getViewer(); + + $title = pht('Welcome to Phabricator'); + + $nav = $this->buildSideNavView(); + $nav->selectFilter('/'); + + $header = id(new PHUIHeaderView()) + ->setHeader($title) + ->setProfileHeader(true); + + $crumbs = $this->buildApplicationCrumbs() + ->addTextCrumb(pht('Welcome')); + + $content = null; + + $view = id(new PHUICMSView()) + ->setCrumbs($crumbs) + ->setNavigation($nav) + ->setHeader($header) + ->setContent($content); + + return $this->newPage() + ->setTitle($title) + ->addClass('phui-cms-body') + ->appendChild($view); + + } + + private function getGuideContent() { + + $guide = null; + + return $guide; + } +} diff --git a/src/view/phui/PHUICMSView.php b/src/view/phui/PHUICMSView.php new file mode 100644 index 0000000000..5898c8ee87 --- /dev/null +++ b/src/view/phui/PHUICMSView.php @@ -0,0 +1,118 @@ +header = $header; + return $this; + } + + public function setNavigation(AphrontSideNavFilterView $nav) { + $this->nav = $nav; + return $this; + } + + public function setCrumbs(PHUICrumbsView $crumbs) { + $this->crumbs = $crumbs; + return $this; + } + + public function setContent($content) { + $this->content = $content; + return $this; + } + + public function setToc($toc) { + $this->toc = $toc; + return $this; + } + + public function setComments($comments) { + $this->comments = $comments; + } + + protected function getTagName() { + return 'div'; + } + + protected function getTagAttributes() { + require_celerity_resource('phui-cms-css'); + + $classes = array(); + $classes[] = 'phui-cms-view'; + + if ($this->comments) { + $classes[] = 'phui-cms-has-comments'; + } + + return array( + 'class' => implode(' ', $classes), + ); + + } + + protected function getTagContent() { + + $content = phutil_tag( + 'div', + array( + 'class' => 'phui-cms-page-content', + ), + array( + $this->header, + $this->content, + )); + + $comments = null; + if ($this->comments) { + $comments = phutil_tag( + 'div', + array( + 'class' => 'phui-cms-comments', + ), + array( + $this->comments, + )); + } + + $navigation = $this->nav; + $navigation->appendChild($content); + $navigation->appendChild($comments); + + $page = phutil_tag( + 'div', + array( + 'class' => 'phui-cms-inner', + ), + array( + $navigation, + )); + + $cms_view = phutil_tag( + 'div', + array( + 'class' => 'phui-cms-wrap', + ), + array( + $this->crumbs, + $page, + )); + + $classes = array(); + $classes[] = 'phui-cms-page'; + + return phutil_tag( + 'div', + array( + 'class' => implode(' ', $classes), + ), + $cms_view); + } +} diff --git a/webroot/rsrc/css/phui/phui-cms.css b/webroot/rsrc/css/phui/phui-cms.css new file mode 100644 index 0000000000..16b47c73ac --- /dev/null +++ b/webroot/rsrc/css/phui/phui-cms.css @@ -0,0 +1,47 @@ +/** + * @provides phui-cms-css + */ + + +.phui-cms-body { + background-color: #f0f0f2; +} + +.phui-cms-view .phui-crumbs-view { + border-bottom: 1px solid {$thinblueborder}; +} + +.phui-cms-page { + max-width: 1140px; + margin: 0 auto; + background-color: #fff; + border-left: 1px solid {$lightblueborder}; + border-right: 1px solid {$lightblueborder}; + border-bottom: 1px solid {$lightblueborder}; + margin-bottom: 20px; +} + +.phui-cms-view .phui-basic-nav.phui-navigation-shell .phabricator-nav-local { + background-color: {$page.background}; + width: 240px; + border-right: 1px solid {$thinblueborder}; +} + +.phui-cms-view .phabricator-nav-content { + padding: 0; +} + +.phui-cms-view .phui-document-container { + border: none; +} + +.phui-cms-view .phui-profile-header { + padding: 32px 20px; + border-bottom: 1px solid {$thinblueborder}; +} + +.phui-cms-view .phui-document-view.phui-document-view-pro { + width: auto; + padding: 32px 20px; + margin: 0; +} From aba4366020b768d88ee1e3dbb411229fc15dc5be Mon Sep 17 00:00:00 2001 From: Chad Little Date: Wed, 17 Aug 2016 14:57:21 -0700 Subject: [PATCH 14/22] Restrict aprhont-form-width to certain input types Summary: Fixes T11491, I believe these are the only two that need to be specified. Test Plan: Checked Create pages for diffs, tasks, projects, events, blogs, login, etc. Mobile/Desktop. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T11491 Differential Revision: https://secure.phabricator.com/D16416 --- resources/celerity/map.php | 6 +++--- webroot/rsrc/css/phui/phui-form-view.css | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index b45c7d3280..343052c0ce 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => '340c5b75', + 'core.pkg.css' => '3cea8606', 'core.pkg.js' => 'b562c3db', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '3fb7f532', @@ -135,7 +135,7 @@ return array( 'rsrc/css/phui/phui-document.css' => 'c32e8dec', 'rsrc/css/phui/phui-feed-story.css' => 'aa49845d', 'rsrc/css/phui/phui-fontkit.css' => '9cda225e', - 'rsrc/css/phui/phui-form-view.css' => 'fab0a10f', + 'rsrc/css/phui/phui-form-view.css' => '76b4a46c', 'rsrc/css/phui/phui-form.css' => 'aac1d51d', 'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f', 'rsrc/css/phui/phui-header-view.css' => '06385974', @@ -843,7 +843,7 @@ return array( 'phui-font-icon-base-css' => '6449bce8', 'phui-fontkit-css' => '9cda225e', 'phui-form-css' => 'aac1d51d', - 'phui-form-view-css' => 'fab0a10f', + 'phui-form-view-css' => '76b4a46c', 'phui-head-thing-view-css' => 'fd311e5f', 'phui-header-view-css' => '06385974', 'phui-hovercard' => '1bd28176', diff --git a/webroot/rsrc/css/phui/phui-form-view.css b/webroot/rsrc/css/phui/phui-form-view.css index a3fdac4574..3cf5111692 100644 --- a/webroot/rsrc/css/phui/phui-form-view.css +++ b/webroot/rsrc/css/phui/phui-form-view.css @@ -109,7 +109,8 @@ -webkit-font-smoothing: antialiased; } -.aphront-form-input input { +.aphront-form-input input[type="text"], +.aphront-form-input input[type="password"] { width: 100%; } From 1cca7fbccef8a4ac5ab4b1a626dd4faad5fc5f16 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 18 Aug 2016 12:24:35 -0700 Subject: [PATCH 15/22] Simplify PHUIObjectItemList a bit Summary: I don't think we use footicons, removing that CSS. States were added but only used in Auth, convert them to statusIcon instead. Test Plan: Visit Auth, UIExamples, grep for `setState` Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16418 --- resources/celerity/map.php | 6 +- .../config/PhabricatorAuthListController.php | 6 +- .../examples/PHUIObjectItemListExample.php | 49 ----------- src/view/phui/PHUIObjectItemListView.php | 9 -- src/view/phui/PHUIObjectItemView.php | 88 +------------------ .../css/phui/phui-object-item-list-view.css | 69 --------------- 6 files changed, 8 insertions(+), 219 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 343052c0ce..413027ffdc 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => '3cea8606', + 'core.pkg.css' => 'ba6d7e7a', 'core.pkg.js' => 'b562c3db', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '3fb7f532', @@ -147,7 +147,7 @@ return array( 'rsrc/css/phui/phui-info-view.css' => '28efab79', 'rsrc/css/phui/phui-list.css' => '9da2aa00', 'rsrc/css/phui/phui-object-box.css' => '6b487c57', - 'rsrc/css/phui/phui-object-item-list-view.css' => '8d99e42b', + 'rsrc/css/phui/phui-object-item-list-view.css' => 'aefe157c', 'rsrc/css/phui/phui-pager.css' => 'bea33d23', 'rsrc/css/phui/phui-pinboard-view.css' => '2495140e', 'rsrc/css/phui/phui-profile-menu.css' => '8a3fc181', @@ -856,7 +856,7 @@ return array( 'phui-inline-comment-view-css' => '5953c28e', 'phui-list-view-css' => '9da2aa00', 'phui-object-box-css' => '6b487c57', - 'phui-object-item-list-view-css' => '8d99e42b', + 'phui-object-item-list-view-css' => 'aefe157c', 'phui-pager-css' => 'bea33d23', 'phui-pinboard-view-css' => '2495140e', 'phui-profile-menu-css' => '8a3fc181', diff --git a/src/applications/auth/controller/config/PhabricatorAuthListController.php b/src/applications/auth/controller/config/PhabricatorAuthListController.php index cb9c21b35f..df168dfe14 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthListController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthListController.php @@ -53,7 +53,7 @@ final class PhabricatorAuthListController } if ($config->getIsEnabled()) { - $item->setState(PHUIObjectItemView::STATE_SUCCESS); + $item->setStatusIcon('fa-check-circle green'); $item->addAction( id(new PHUIListItemView()) ->setIcon('fa-times') @@ -61,8 +61,8 @@ final class PhabricatorAuthListController ->setDisabled(!$can_manage) ->addSigil('workflow')); } else { - $item->setState(PHUIObjectItemView::STATE_FAIL); - $item->addIcon('fa-times grey', pht('Disabled')); + $item->setStatusIcon('fa-ban red'); + $item->addIcon('fa-ban grey', pht('Disabled')); $item->addAction( id(new PHUIListItemView()) ->setIcon('fa-plus') diff --git a/src/applications/uiexample/examples/PHUIObjectItemListExample.php b/src/applications/uiexample/examples/PHUIObjectItemListExample.php index c85cc311fc..4a75ceede1 100644 --- a/src/applications/uiexample/examples/PHUIObjectItemListExample.php +++ b/src/applications/uiexample/examples/PHUIObjectItemListExample.php @@ -340,55 +340,6 @@ final class PHUIObjectItemListExample extends PhabricatorUIExample { $out[] = $box; - $list = id(new PHUIObjectItemListView()) - ->setStates(true); - - $list->addItem( - id(new PHUIObjectItemView()) - ->setObjectName('X1200') - ->setHeader(pht('Action Passed')) - ->addAttribute(pht('That went swimmingly, go you')) - ->setHref('#') - ->setState(PHUIObjectItemView::STATE_SUCCESS)); - - $list->addItem( - id(new PHUIObjectItemView()) - ->setObjectName('X1201') - ->setHeader(pht('Action Failed')) - ->addAttribute(pht('Whoopsies, might want to fix that')) - ->setHref('#') - ->setState(PHUIObjectItemView::STATE_FAIL)); - - $list->addItem( - id(new PHUIObjectItemView()) - ->setObjectName('X1202') - ->setHeader(pht('Action Warning')) - ->addAttribute(pht('We need to talk about things')) - ->setHref('#') - ->setState(PHUIObjectItemView::STATE_WARN)); - - $list->addItem( - id(new PHUIObjectItemView()) - ->setObjectName('X1203') - ->setHeader(pht('Action Noted')) - ->addAttribute(pht('The weather seems nice today')) - ->setHref('#') - ->setState(PHUIObjectItemView::STATE_NOTE)); - - $list->addItem( - id(new PHUIObjectItemView()) - ->setObjectName('X1203') - ->setHeader(pht('Action In Progress')) - ->addAttribute(pht('Outlook fuzzy, try again later')) - ->setHref('#') - ->setState(PHUIObjectItemView::STATE_BUILD)); - - $box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('State Icons')) - ->setObjectList($list); - - $out[] = array($box); - return $out; } } diff --git a/src/view/phui/PHUIObjectItemListView.php b/src/view/phui/PHUIObjectItemListView.php index 55c0c8aede..0e5747cf48 100644 --- a/src/view/phui/PHUIObjectItemListView.php +++ b/src/view/phui/PHUIObjectItemListView.php @@ -9,7 +9,6 @@ final class PHUIObjectItemListView extends AphrontTagView { private $flush; private $simple; private $allowEmptyList; - private $states; private $itemClass = 'phui-object-item-standard'; public function setAllowEmptyList($allow_empty_list) { @@ -51,11 +50,6 @@ final class PHUIObjectItemListView extends AphrontTagView { return $this; } - public function setStates($states) { - $this->states = $states; - return $this; - } - public function setItemClass($item_class) { $this->itemClass = $item_class; return $this; @@ -69,9 +63,6 @@ final class PHUIObjectItemListView extends AphrontTagView { $classes = array(); $classes[] = 'phui-object-item-list-view'; - if ($this->states) { - $classes[] = 'phui-object-list-states'; - } if ($this->flush) { $classes[] = 'phui-object-list-flush'; } diff --git a/src/view/phui/PHUIObjectItemView.php b/src/view/phui/PHUIObjectItemView.php index 4b56d746a7..3ea1c81930 100644 --- a/src/view/phui/PHUIObjectItemView.php +++ b/src/view/phui/PHUIObjectItemView.php @@ -19,8 +19,6 @@ final class PHUIObjectItemView extends AphrontTagView { private $headIcons = array(); private $disabled; private $imageURI; - private $state; - private $fontIcon; private $imageIcon; private $titleText; private $badge; @@ -29,16 +27,6 @@ final class PHUIObjectItemView extends AphrontTagView { private $launchButton; private $coverImage; - const AGE_FRESH = 'fresh'; - const AGE_STALE = 'stale'; - const AGE_OLD = 'old'; - - const STATE_SUCCESS = 'green'; - const STATE_FAIL = 'red'; - const STATE_WARN = 'yellow'; - const STATE_NOTE = 'blue'; - const STATE_BUILD = 'sky'; - public function setDisabled($disabled) { $this->disabled = $disabled; return $this; @@ -156,63 +144,9 @@ final class PHUIObjectItemView extends AphrontTagView { return $this; } - public function setState($state) { - $this->state = $state; - switch ($state) { - case self::STATE_SUCCESS: - $fi = 'fa-check-circle green'; - break; - case self::STATE_FAIL: - $fi = 'fa-times-circle red'; - break; - case self::STATE_WARN: - $fi = 'fa-exclamation-circle yellow'; - break; - case self::STATE_NOTE: - $fi = 'fa-info-circle blue'; - break; - case self::STATE_BUILD: - $fi = 'fa-refresh ph-spin sky'; - break; - } - $this->setIcon($fi); - return $this; - } - - public function setIcon($icon) { - $this->fontIcon = id(new PHUIIconView()) - ->setIcon($icon); - return $this; - } - - public function setEpoch($epoch, $age = self::AGE_FRESH) { + public function setEpoch($epoch) { $date = phabricator_datetime($epoch, $this->getUser()); - - $days = floor((time() - $epoch) / 60 / 60 / 24); - - switch ($age) { - case self::AGE_FRESH: - $this->addIcon('none', $date); - break; - case self::AGE_STALE: - $attr = array( - 'tip' => pht('Stale (%s day(s))', new PhutilNumber($days)), - 'class' => 'icon-age-stale', - ); - - $this->addIcon('fa-clock-o yellow', $date, $attr); - break; - case self::AGE_OLD: - $attr = array( - 'tip' => pht('Old (%s day(s))', new PhutilNumber($days)), - 'class' => 'icon-age-old', - ); - $this->addIcon('fa-clock-o red', $date, $attr); - break; - default: - throw new Exception(pht("Unknown age '%s'!", $age)); - } - + $this->addIcon('none', $date); return $this; } @@ -309,10 +243,6 @@ final class PHUIObjectItemView extends AphrontTagView { $item_classes[] = 'phui-object-item-disabled'; } - if ($this->state) { - $item_classes[] = 'phui-object-item-state-'.$this->state; - } - switch ($this->effect) { case 'highlighted': $item_classes[] = 'phui-object-item-highlighted'; @@ -338,10 +268,6 @@ final class PHUIObjectItemView extends AphrontTagView { $item_classes[] = 'phui-object-item-with-image-icon'; } - if ($this->fontIcon) { - $item_classes[] = 'phui-object-item-with-ficon'; - } - return array( 'class' => $item_classes, ); @@ -596,16 +522,6 @@ final class PHUIObjectItemView extends AphrontTagView { $this->getImageIcon()); } - $ficon = null; - if ($this->fontIcon) { - $image = phutil_tag( - 'div', - array( - 'class' => 'phui-object-item-ficon', - ), - $this->fontIcon); - } - if ($image && $this->href) { $image = phutil_tag( 'a', diff --git a/webroot/rsrc/css/phui/phui-object-item-list-view.css b/webroot/rsrc/css/phui/phui-object-item-list-view.css index 3556d48672..a3e9bccc9c 100644 --- a/webroot/rsrc/css/phui/phui-object-item-list-view.css +++ b/webroot/rsrc/css/phui/phui-object-item-list-view.css @@ -487,46 +487,6 @@ ul.phui-object-item-list-view .phui-object-item-selected } -/* - Foot Icons ---------------------------------------------------------------- - - Object counts shown in the footer. - -*/ - -.phui-object-item-foot-icons { - margin-left: 10px; - bottom: 0; - position: absolute; -} - -.phui-object-item-with-foot-icons .phui-object-item-content, -.device-phone .phui-object-item-with-foot-icons .phui-object-item-col2 { - padding-bottom: 24px; -} - -.device-phone .phui-object-item-with-foot-icons .phui-object-item-content { - padding-bottom: 0; -} - -.phui-object-item-foot-icon { - display: inline-block; - background: {$lightgreyborder}; - color: #ffffff; - font-weight: bold; - margin-right: 3px; - padding: 3px 6px 0; - height: 17px; - vertical-align: middle; - position: relative; - font-size: 12px; - -webkit-font-smoothing: antialiased; -} - -.phui-object-item-foot-icon .phui-icon-view { - margin-right: 4px; -} - - /* - Handle Icons -------------------------------------------------------------- Shows owners, reviewers, etc., using profile picture icons. @@ -621,35 +581,6 @@ ul.phui-object-item-list-view .phui-object-item-selected list-style: none; } - -/* - State --------------------------------------------------------------------- - - Provides a list of object status or states, success or fail, etc - -*/ - -.phui-object-item-ficon { - width: 48px; - height: 26px; - margin-top: 12px; - position: absolute; - text-align: center; - font-size: 24px; -} - -.phui-object-item-with-ficon .phui-object-item-content-box { - margin-left: 38px; -} - -.phui-object-box .phui-object-list-states { - padding: 0; -} - -.phui-object-list-states .phui-info-view { - margin: 0; - border: none; -} - /* - Badges ---------------------------------------------------------------- */ .phui-object-item-col0.phui-object-item-badge { From 1ecce605890c94b2d2bb7a6b6d534a4635ffd690 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 19 Aug 2016 04:52:43 -0700 Subject: [PATCH 16/22] Keep setIcon() working for now on PHUIObjectItemView Summary: Ref T11501. This method was removed in D16418, but still has some callsites. I know of four: - Config - Settings - Drydock main page - Almanac main page Since I might be missing some and it's close to the release cut, just put the method back for now until we can clean it up more properly. Test Plan: Viewed Settings, Config, Drydock, Almanac. No more fatal on this method being missing. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11501 Differential Revision: https://secure.phabricator.com/D16420 --- src/view/phui/PHUIObjectItemView.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/view/phui/PHUIObjectItemView.php b/src/view/phui/PHUIObjectItemView.php index 3ea1c81930..73f4ff7824 100644 --- a/src/view/phui/PHUIObjectItemView.php +++ b/src/view/phui/PHUIObjectItemView.php @@ -167,6 +167,11 @@ final class PHUIObjectItemView extends AphrontTagView { return $this; } + public function setIcon($icon) { + // TODO: Remove this in favor of setStatusIcon()? + return $this->setStatusIcon($icon); + } + public function setStatusIcon($icon, $label = null) { $this->statusIcon = array( 'icon' => $icon, From e7aa874f5eba4757bf007dfab010b9fbcf375e5c Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 19 Aug 2016 09:29:57 -0700 Subject: [PATCH 17/22] Fix getIcon calls in PHUIObjectListItem Summary: Fixes T11501. Let's you pass in a full PHUIIconView or just the icon name to give ObjectListItem a large icon. Test Plan: Alamanac, Applications, Drydock, Settings, Search Typeahead, Config page... Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T11501 Differential Revision: https://secure.phabricator.com/D16421 --- .../controller/AlmanacConsoleController.php | 10 +++++----- .../controller/PhabricatorConfigListController.php | 2 +- .../controller/DrydockConsoleController.php | 8 ++++---- .../meta/query/PhabricatorAppSearchEngine.php | 14 +------------- .../PhabricatorUserPreferencesSearchEngine.php | 4 ++-- src/view/phui/PHUIObjectItemView.php | 9 ++++----- 6 files changed, 17 insertions(+), 30 deletions(-) diff --git a/src/applications/almanac/controller/AlmanacConsoleController.php b/src/applications/almanac/controller/AlmanacConsoleController.php index cd9ebf610a..74c57c384f 100644 --- a/src/applications/almanac/controller/AlmanacConsoleController.php +++ b/src/applications/almanac/controller/AlmanacConsoleController.php @@ -16,7 +16,7 @@ final class AlmanacConsoleController extends AlmanacController { id(new PHUIObjectItemView()) ->setHeader(pht('Devices')) ->setHref($this->getApplicationURI('device/')) - ->setIcon('fa-server') + ->setImageIcon('fa-server') ->addAttribute( pht( 'Create an inventory of physical and virtual hosts and '. @@ -26,7 +26,7 @@ final class AlmanacConsoleController extends AlmanacController { id(new PHUIObjectItemView()) ->setHeader(pht('Services')) ->setHref($this->getApplicationURI('service/')) - ->setIcon('fa-plug') + ->setImageIcon('fa-plug') ->addAttribute( pht( 'Create and update services, and map them to interfaces on '. @@ -36,7 +36,7 @@ final class AlmanacConsoleController extends AlmanacController { id(new PHUIObjectItemView()) ->setHeader(pht('Networks')) ->setHref($this->getApplicationURI('network/')) - ->setIcon('fa-globe') + ->setImageIcon('fa-globe') ->addAttribute( pht( 'Manage public and private networks.'))); @@ -45,7 +45,7 @@ final class AlmanacConsoleController extends AlmanacController { id(new PHUIObjectItemView()) ->setHeader(pht('Namespaces')) ->setHref($this->getApplicationURI('namespace/')) - ->setIcon('fa-asterisk') + ->setImageIcon('fa-asterisk') ->addAttribute( pht('Control who can create new named services and devices.'))); @@ -56,7 +56,7 @@ final class AlmanacConsoleController extends AlmanacController { id(new PHUIObjectItemView()) ->setHeader(pht('Documentation')) ->setHref($docs_uri) - ->setIcon('fa-book') + ->setImageIcon('fa-book') ->addAttribute(pht('Browse documentation for Almanac.'))); $crumbs = $this->buildApplicationCrumbs(); diff --git a/src/applications/config/controller/PhabricatorConfigListController.php b/src/applications/config/controller/PhabricatorConfigListController.php index 8b0ff42e50..e6af666931 100644 --- a/src/applications/config/controller/PhabricatorConfigListController.php +++ b/src/applications/config/controller/PhabricatorConfigListController.php @@ -51,7 +51,7 @@ final class PhabricatorConfigListController ->setHeader($group->getName()) ->setHref('/config/group/'.$group->getKey().'/') ->addAttribute($group->getDescription()) - ->setIcon($group->getIcon()); + ->setImageIcon($group->getIcon()); $list->addItem($item); } } diff --git a/src/applications/drydock/controller/DrydockConsoleController.php b/src/applications/drydock/controller/DrydockConsoleController.php index 1d79f52c14..ac524cb8a5 100644 --- a/src/applications/drydock/controller/DrydockConsoleController.php +++ b/src/applications/drydock/controller/DrydockConsoleController.php @@ -31,7 +31,7 @@ final class DrydockConsoleController extends DrydockController { $menu->addItem( id(new PHUIObjectItemView()) ->setHeader(pht('Blueprints')) - ->setIcon('fa-map-o') + ->setImageIcon('fa-map-o') ->setHref($this->getApplicationURI('blueprint/')) ->addAttribute( pht( @@ -41,7 +41,7 @@ final class DrydockConsoleController extends DrydockController { $menu->addItem( id(new PHUIObjectItemView()) ->setHeader(pht('Resources')) - ->setIcon('fa-map') + ->setImageIcon('fa-map') ->setHref($this->getApplicationURI('resource/')) ->addAttribute( pht('View and manage resources Drydock has built, like hosts.'))); @@ -49,14 +49,14 @@ final class DrydockConsoleController extends DrydockController { $menu->addItem( id(new PHUIObjectItemView()) ->setHeader(pht('Leases')) - ->setIcon('fa-link') + ->setImageIcon('fa-link') ->setHref($this->getApplicationURI('lease/')) ->addAttribute(pht('Manage leases on resources.'))); $menu->addItem( id(new PHUIObjectItemView()) ->setHeader(pht('Repository Operations')) - ->setIcon('fa-fighter-jet') + ->setImageIcon('fa-fighter-jet') ->setHref($this->getApplicationURI('operation/')) ->addAttribute(pht('Review the repository operation queue.'))); diff --git a/src/applications/meta/query/PhabricatorAppSearchEngine.php b/src/applications/meta/query/PhabricatorAppSearchEngine.php index 98a9495fa0..62e1c1020e 100644 --- a/src/applications/meta/query/PhabricatorAppSearchEngine.php +++ b/src/applications/meta/query/PhabricatorAppSearchEngine.php @@ -214,18 +214,6 @@ final class PhabricatorAppSearchEngine $icon = 'application'; } - // TODO: This sheet doesn't work the same way other sheets do so it - // ends up with the wrong classes if we try to use PHUIIconView. This - // is probably all changing in the redesign anyway. - - $icon_view = javelin_tag( - 'span', - array( - 'class' => 'phui-icon-view phui-font-fa '.$icon, - 'aural' => false, - ), - ''); - $description = $application->getShortDescription(); $configure = id(new PHUIButtonView()) @@ -241,7 +229,7 @@ final class PhabricatorAppSearchEngine $item = id(new PHUIObjectItemView()) ->setHeader($name) - ->setImageIcon($icon_view) + ->setImageIcon($icon) ->setSubhead($description) ->setLaunchButton($configure); diff --git a/src/applications/settings/query/PhabricatorUserPreferencesSearchEngine.php b/src/applications/settings/query/PhabricatorUserPreferencesSearchEngine.php index 4b1f135c6a..dc01a5191b 100644 --- a/src/applications/settings/query/PhabricatorUserPreferencesSearchEngine.php +++ b/src/applications/settings/query/PhabricatorUserPreferencesSearchEngine.php @@ -65,8 +65,8 @@ final class PhabricatorUserPreferencesSearchEngine $item = id(new PHUIObjectItemView()) ->setHeader($setting->getDisplayName()) ->setHref($setting->getEditURI()) - ->setImageURI(PhabricatorUser::getDefaultProfileImageURI()) - ->setIcon('fa-globe') + // TODO: Replace this with NUX Style bg / white icon when built + ->setImageIcon('fa-globe') ->addAttribute(pht('Edit global default settings for all users.')); $list->addItem($item); diff --git a/src/view/phui/PHUIObjectItemView.php b/src/view/phui/PHUIObjectItemView.php index 73f4ff7824..28d683e4d3 100644 --- a/src/view/phui/PHUIObjectItemView.php +++ b/src/view/phui/PHUIObjectItemView.php @@ -131,6 +131,10 @@ final class PHUIObjectItemView extends AphrontTagView { } public function setImageIcon($image_icon) { + if (!$image_icon instanceof PHUIIconView) { + $image_icon = id(new PHUIIconView()) + ->setIcon($image_icon); + } $this->imageIcon = $image_icon; return $this; } @@ -167,11 +171,6 @@ final class PHUIObjectItemView extends AphrontTagView { return $this; } - public function setIcon($icon) { - // TODO: Remove this in favor of setStatusIcon()? - return $this->setStatusIcon($icon); - } - public function setStatusIcon($icon, $label = null) { $this->statusIcon = array( 'icon' => $icon, From 4d175ac709702851eb15965f356e1bffe6dc3dae Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 19 Aug 2016 10:43:41 -0700 Subject: [PATCH 18/22] Simplify how tag lists manage their handles Summary: Fixes T11493. This code is a little bit weird/clever, simplify it so that we always cast the handles to an array early on. Test Plan: {F1767668} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11493 Differential Revision: https://secure.phabricator.com/D16422 --- .../phid/view/PHUIHandleTagListView.php | 44 +++++++++++-------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/applications/phid/view/PHUIHandleTagListView.php b/src/applications/phid/view/PHUIHandleTagListView.php index eb909d4b86..c4ef3762c3 100644 --- a/src/applications/phid/view/PHUIHandleTagListView.php +++ b/src/applications/phid/view/PHUIHandleTagListView.php @@ -61,15 +61,21 @@ final class PHUIHandleTagListView extends AphrontTagView { } } - if ($this->limit && (count($handles) > $this->limit)) { - if (!is_array($handles)) { - $handles = iterator_to_array($handles); - } - $handles = array_slice($handles, 0, $this->limit); + // We may be passed a PhabricatorHandleList; if we are, convert it into + // a normal array. + if (!is_array($handles)) { + $handles = iterator_to_array($handles); + } + + $over_limit = $this->limit && (count($handles) > $this->limit); + if ($over_limit) { + $visible = array_slice($handles, 0, $this->limit); + } else { + $visible = $handles; } $list = array(); - foreach ($handles as $handle) { + foreach ($visible as $handle) { $tag = $handle->renderTag(); if ($this->showHovercards) { $tag->setPHID($handle->getPHID()); @@ -84,21 +90,21 @@ final class PHUIHandleTagListView extends AphrontTagView { )); } - if ($this->limit) { - if (count($this->handles) > $this->limit) { - $tip_text = implode(', ', mpull($this->handles, 'getName')); + if ($over_limit) { + $tip_text = implode(', ', mpull($handles, 'getName')); - $more = $this->newPlaceholderTag() - ->setName("\xE2\x80\xA6") - ->addSigil('has-tooltip') - ->setMetadata( - array( - 'tip' => $tip_text, - 'size' => 200, - )); + Javelin::initBehavior('phabricator-tooltips'); - $list[] = $this->newItem($more); - } + $more = $this->newPlaceholderTag() + ->setName("\xE2\x80\xA6") + ->addSigil('has-tooltip') + ->setMetadata( + array( + 'tip' => $tip_text, + 'size' => 200, + )); + + $list[] = $this->newItem($more); } return $list; From 3bd0da0ec24315d8a029d8c3086c37fcb829e922 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 19 Aug 2016 11:29:28 -0700 Subject: [PATCH 19/22] Add a missing table key to improve performance of "Recently Completed Tasks" query Summary: Fixes T11490. Currently, this query can not use a key and the table size may be quite large. Adjust the query so it can use a key for both selection and ordering, and add that key. Test Plan: Ran `EXPLAIN` on the old query in production, then added the key and ran `EXPLAIN` on the new query. Saw key in use, and "rows" examined drop from 29,273 to 15. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11490 Differential Revision: https://secure.phabricator.com/D16423 --- .../daemon/workers/query/PhabricatorWorkerArchiveTaskQuery.php | 2 ++ .../daemon/workers/storage/PhabricatorWorkerArchiveTask.php | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/infrastructure/daemon/workers/query/PhabricatorWorkerArchiveTaskQuery.php b/src/infrastructure/daemon/workers/query/PhabricatorWorkerArchiveTaskQuery.php index 69abac27b0..adc0992473 100644 --- a/src/infrastructure/daemon/workers/query/PhabricatorWorkerArchiveTaskQuery.php +++ b/src/infrastructure/daemon/workers/query/PhabricatorWorkerArchiveTaskQuery.php @@ -104,6 +104,8 @@ final class PhabricatorWorkerArchiveTaskQuery if ($this->dateCreatedBefore) { return qsprintf($conn_r, 'ORDER BY dateCreated DESC, id DESC'); + } else if ($this->dateModifiedSince) { + return qsprintf($conn_r, 'ORDER BY dateModified DESC, id DESC'); } else { return qsprintf($conn_r, 'ORDER BY id DESC'); } diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php index 9797373eb7..fe1164e532 100644 --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php @@ -31,6 +31,9 @@ final class PhabricatorWorkerArchiveTask extends PhabricatorWorkerTask { 'leaseOwner' => array( 'columns' => array('leaseOwner', 'priority', 'id'), ), + 'key_modified' => array( + 'columns' => array('dateModified'), + ), ) + $parent[self::CONFIG_KEY_SCHEMA]; return $config; From f379858874a3336dc36cc8b7e77b0d0b698b2fff Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 19 Aug 2016 12:49:38 -0700 Subject: [PATCH 20/22] Add setBackground to PHUIIconView Summary: Ref T11132. Adds a background color option to PHUIIconView, for use whereever, and NUX. Also normalize icon placement for mixed image/icon result list. Test Plan: Test in UIExamples, and Global Settings. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T11132 Differential Revision: https://secure.phabricator.com/D16424 --- resources/celerity/map.php | 14 +++---- ...PhabricatorUserPreferencesSearchEngine.php | 7 +++- .../uiexample/examples/PHUIIconExample.php | 22 ++++++++++- src/view/phui/PHUIIconView.php | 10 +++++ webroot/rsrc/css/font/phui-font-icon-base.css | 39 +++++++++++++++++++ webroot/rsrc/css/phui/phui-icon.css | 17 ++++++++ .../css/phui/phui-object-item-list-view.css | 34 +++++----------- 7 files changed, 108 insertions(+), 35 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 413027ffdc..b7c80fbaed 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => 'ba6d7e7a', + 'core.pkg.css' => 'ed7ae7bb', 'core.pkg.js' => 'b562c3db', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '3fb7f532', @@ -112,7 +112,7 @@ return array( 'rsrc/css/font/font-aleo.css' => '8bdb2835', 'rsrc/css/font/font-awesome.css' => '2b7ebbcc', 'rsrc/css/font/font-lato.css' => 'c7ccd872', - 'rsrc/css/font/phui-font-icon-base.css' => '6449bce8', + 'rsrc/css/font/phui-font-icon-base.css' => '4e8274c4', 'rsrc/css/layout/phabricator-filetree-view.css' => 'fccf9f82', 'rsrc/css/layout/phabricator-source-code-view.css' => 'cbeef983', 'rsrc/css/phui/calendar/phui-calendar-day.css' => '572b1893', @@ -141,13 +141,13 @@ return array( 'rsrc/css/phui/phui-header-view.css' => '06385974', 'rsrc/css/phui/phui-hovercard.css' => 'de1a2119', 'rsrc/css/phui/phui-icon-set-selector.css' => '1ab67aad', - 'rsrc/css/phui/phui-icon.css' => 'd0534b71', + 'rsrc/css/phui/phui-icon.css' => 'b1dbd620', 'rsrc/css/phui/phui-image-mask.css' => 'a8498f9c', 'rsrc/css/phui/phui-info-panel.css' => '27ea50a1', 'rsrc/css/phui/phui-info-view.css' => '28efab79', 'rsrc/css/phui/phui-list.css' => '9da2aa00', 'rsrc/css/phui/phui-object-box.css' => '6b487c57', - 'rsrc/css/phui/phui-object-item-list-view.css' => 'aefe157c', + 'rsrc/css/phui/phui-object-item-list-view.css' => '40010767', 'rsrc/css/phui/phui-pager.css' => 'bea33d23', 'rsrc/css/phui/phui-pinboard-view.css' => '2495140e', 'rsrc/css/phui/phui-profile-menu.css' => '8a3fc181', @@ -840,7 +840,7 @@ return array( 'phui-document-view-css' => 'c32e8dec', 'phui-document-view-pro-css' => 'dc3d46ed', 'phui-feed-story-css' => 'aa49845d', - 'phui-font-icon-base-css' => '6449bce8', + 'phui-font-icon-base-css' => '4e8274c4', 'phui-fontkit-css' => '9cda225e', 'phui-form-css' => 'aac1d51d', 'phui-form-view-css' => '76b4a46c', @@ -849,14 +849,14 @@ return array( 'phui-hovercard' => '1bd28176', 'phui-hovercard-view-css' => 'de1a2119', 'phui-icon-set-selector-css' => '1ab67aad', - 'phui-icon-view-css' => 'd0534b71', + 'phui-icon-view-css' => 'b1dbd620', 'phui-image-mask-css' => 'a8498f9c', 'phui-info-panel-css' => '27ea50a1', 'phui-info-view-css' => '28efab79', 'phui-inline-comment-view-css' => '5953c28e', 'phui-list-view-css' => '9da2aa00', 'phui-object-box-css' => '6b487c57', - 'phui-object-item-list-view-css' => 'aefe157c', + 'phui-object-item-list-view-css' => '40010767', 'phui-pager-css' => 'bea33d23', 'phui-pinboard-view-css' => '2495140e', 'phui-profile-menu-css' => '8a3fc181', diff --git a/src/applications/settings/query/PhabricatorUserPreferencesSearchEngine.php b/src/applications/settings/query/PhabricatorUserPreferencesSearchEngine.php index dc01a5191b..a677ed0e66 100644 --- a/src/applications/settings/query/PhabricatorUserPreferencesSearchEngine.php +++ b/src/applications/settings/query/PhabricatorUserPreferencesSearchEngine.php @@ -62,11 +62,14 @@ final class PhabricatorUserPreferencesSearchEngine ->setViewer($viewer); foreach ($settings as $setting) { + $icon = id(new PHUIIconView()) + ->setIcon('fa-globe') + ->setBackground('bg-sky'); + $item = id(new PHUIObjectItemView()) ->setHeader($setting->getDisplayName()) ->setHref($setting->getEditURI()) - // TODO: Replace this with NUX Style bg / white icon when built - ->setImageIcon('fa-globe') + ->setImageIcon($icon) ->addAttribute(pht('Edit global default settings for all users.')); $list->addItem($item); diff --git a/src/applications/uiexample/examples/PHUIIconExample.php b/src/applications/uiexample/examples/PHUIIconExample.php index 1db4aa202f..8e01d6e16a 100644 --- a/src/applications/uiexample/examples/PHUIIconExample.php +++ b/src/applications/uiexample/examples/PHUIIconExample.php @@ -130,6 +130,17 @@ final class PHUIIconExample extends PhabricatorUIExample { ->addClass('mmr'); } + $squares = array('fa-briefcase', 'fa-code', 'fa-globe', 'fa-home'); + $squareview = array(); + foreach ($squares as $icon) { + $squareview[] = + id(new PHUIIconView()) + ->setIcon($icon) + ->setBackground('bg-blue') + ->setHref('#') + ->addClass('mmr'); + } + $layout_cicons = id(new PHUIBoxView()) ->appendChild($cicons) ->addMargin(PHUI::MARGIN_LARGE); @@ -155,6 +166,10 @@ final class PHUIIconExample extends PhabricatorUIExample { ->addMargin(PHUI::MARGIN_MEDIUM); $layout5 = id(new PHUIBoxView()) + ->appendChild($squareview) + ->addMargin(PHUI::MARGIN_MEDIUM); + + $layout6 = id(new PHUIBoxView()) ->appendChild($loginview) ->addMargin(PHUI::MARGIN_MEDIUM); @@ -187,9 +202,13 @@ final class PHUIIconExample extends PhabricatorUIExample { ->appendChild($layout4); $wrap5 = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Authentication')) + ->setHeaderText(pht('Squares')) ->appendChild($layout5); + $wrap6 = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Authentication')) + ->appendChild($layout6); + return phutil_tag( 'div', array( @@ -202,6 +221,7 @@ final class PHUIIconExample extends PhabricatorUIExample { $wrap3, $wrap4, $wrap5, + $wrap6, )); } } diff --git a/src/view/phui/PHUIIconView.php b/src/view/phui/PHUIIconView.php index af9af313ee..cdc3825135 100644 --- a/src/view/phui/PHUIIconView.php +++ b/src/view/phui/PHUIIconView.php @@ -17,6 +17,7 @@ final class PHUIIconView extends AphrontTagView { private $spriteSheet; private $iconFont; private $iconColor; + private $iconBackground; public function setHref($href) { $this->href = $href; @@ -54,6 +55,11 @@ final class PHUIIconView extends AphrontTagView { return $this; } + public function setBackground($color) { + $this->iconBackground = $color; + return $this; + } + protected function getTagName() { $tag = 'span'; if ($this->href) { @@ -79,6 +85,10 @@ final class PHUIIconView extends AphrontTagView { if ($this->iconColor) { $classes[] = $this->iconColor; } + if ($this->iconBackground) { + $classes[] = 'phui-icon-square'; + $classes[] = $this->iconBackground; + } } else { if ($this->headSize) { $classes[] = $this->headSize; diff --git a/webroot/rsrc/css/font/phui-font-icon-base.css b/webroot/rsrc/css/font/phui-font-icon-base.css index 23194b3bda..a1b0d61d0d 100644 --- a/webroot/rsrc/css/font/phui-font-icon-base.css +++ b/webroot/rsrc/css/font/phui-font-icon-base.css @@ -151,6 +151,45 @@ color: rgba({$alphagrey},0.3); } +/* Backgrounds */ + +.phui-icon-view.bg-dark { + background-color: {$darkgreytext}; +} +.phui-icon-view.bg-bluegrey { + background-color: {$bluetext}; +} +.phui-icon-view.bg-red { + background-color: {$red}; +} +.phui-icon-view.bg-orange { + background-color: {$orange}; +} +.phui-icon-view.bg-yellow { + background-color: {$yellow}; +} +.phui-icon-view.bg-green { + background-color: {$green} +} +.phui-icon-view.bg-blue { + background-color: {$blue}; +} +.phui-icon-view.bg-sky { + background-color: {$sky}; +} +.phui-icon-view.bg-indigo { + background-color: {$indigo}; +} +.phui-icon-view.bg-pink { + background-color: {$pink}; +} +.phui-icon-view.bg-fire { + background-color: {$fire}; +} +.phui-icon-view.bg-violet { + background-color: {$violet}; +} + /* Hovers */ .device-desktop a.phui-icon-view.lightgreytext:hover, diff --git a/webroot/rsrc/css/phui/phui-icon.css b/webroot/rsrc/css/phui/phui-icon.css index aa362034ed..e2124c28b8 100644 --- a/webroot/rsrc/css/phui/phui-icon.css +++ b/webroot/rsrc/css/phui/phui-icon.css @@ -80,3 +80,20 @@ a.phui-icon-circle:hover { a.phui-icon-circle:hover .phui-icon-view { color: {$sky}; } + +/* - Icon in a Square ------------------------------------------------------- */ + +.phui-icon-view.phui-icon-square { + height: 40px; + width: 40px; + color: #fff; + font-size: 26px; + text-align: center; + line-height: 38px; + border-radius: 3px; +} + +a.phui-icon-view.phui-icon-square:hover { + text-decoration: none; + color: #fff; +} diff --git a/webroot/rsrc/css/phui/phui-object-item-list-view.css b/webroot/rsrc/css/phui/phui-object-item-list-view.css index a3e9bccc9c..506ffc7985 100644 --- a/webroot/rsrc/css/phui/phui-object-item-list-view.css +++ b/webroot/rsrc/css/phui/phui-object-item-list-view.css @@ -651,37 +651,19 @@ ul.phui-object-item-list-view .phui-object-item-selected .phui-object-item-image-icon { background: none; - width: 30px; - height: 30px; - margin: 4px 0; + width: 40px; + height: 40px; + margin: 8px 6px; position: absolute; } .phui-object-item-image-icon .phui-icon-view { position: absolute; - width: 24px; - height: 24px; - left: 6px; - top: 10px; - font-size: 24px; + width: 40px; + height: 40px; + font-size: 26px; text-align: center; - vertical-align: bottom; -} - -.phui-object-item-with-image-icon .phui-object-item-frame { - min-height: 48px; -} - -.phui-object-item-with-image-icon .phui-object-item-content-box { - margin-left: 36px; -} - -.device-desktop .phui-object-item-launcher-list .phui-object-item-content { - margin-right: 0; -} - -.device-desktop .phui-object-item-launcher-list .phui-object-icon-pane { - width: auto; + line-height: 36px; } .phui-object-item-image { @@ -693,10 +675,12 @@ ul.phui-object-item-list-view .phui-object-item-selected position: absolute; } +.phui-object-item-with-image-icon .phui-object-item-frame, .phui-object-item-with-image .phui-object-item-frame { min-height: 52px; } +.phui-object-item-with-image-icon .phui-object-item-content-box, .phui-object-item-with-image .phui-object-item-content-box { margin-left: 46px; } From 56bc84a73b35f5501694f1b4a0f263954adfedba Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sat, 20 Aug 2016 13:11:02 -0700 Subject: [PATCH 21/22] Rough in of NUX Guide steps Summary: Ref T11132. Will work on CSS tomorrow, but wanted to rough in the UI Steps to get guidance. Not sure what you have in mind for the "app" part, if you want to explain it and I build or you build. Test Plan: Visit each page and click on links. Very rough and unfinished. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T11132 Differential Revision: https://secure.phabricator.com/D16419 --- resources/celerity/map.php | 6 +- src/__phutil_library_map__.php | 4 + .../PhabricatorGuideInstallController.php | 158 +++++++++++++++- .../PhabricatorGuideQuickStartController.php | 172 +++++++++++++++++- .../PhabricatorGuideWelcomeController.php | 15 +- .../guides/view/PhabricatorGuideItemView.php | 67 +++++++ .../guides/view/PhabricatorGuideListView.php | 45 +++++ .../rsrc/css/application/guides/guides.css | 34 ++++ webroot/rsrc/css/phui/phui-cms.css | 5 +- 9 files changed, 490 insertions(+), 16 deletions(-) create mode 100644 src/applications/guides/view/PhabricatorGuideItemView.php create mode 100644 src/applications/guides/view/PhabricatorGuideListView.php create mode 100644 webroot/rsrc/css/application/guides/guides.css diff --git a/resources/celerity/map.php b/resources/celerity/map.php index b7c80fbaed..bec64fab6e 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -70,6 +70,7 @@ return array( 'rsrc/css/application/feed/feed.css' => 'ecd4ec57', 'rsrc/css/application/files/global-drag-and-drop.css' => '5c1b47c2', 'rsrc/css/application/flag/flag.css' => '5337623f', + 'rsrc/css/application/guides/guides.css' => '1d5414e5', 'rsrc/css/application/harbormaster/harbormaster.css' => 'f491c9f4', 'rsrc/css/application/herald/herald-test.css' => 'a52e323e', 'rsrc/css/application/herald/herald.css' => 'dc31f6e9', @@ -127,7 +128,7 @@ return array( 'rsrc/css/phui/phui-box.css' => '5c8387cf', 'rsrc/css/phui/phui-button.css' => '4a5fbe3d', 'rsrc/css/phui/phui-chart.css' => '6bf6f78e', - 'rsrc/css/phui/phui-cms.css' => '33064557', + 'rsrc/css/phui/phui-cms.css' => 'be43c8a8', 'rsrc/css/phui/phui-crumbs-view.css' => '9dac418c', 'rsrc/css/phui/phui-curtain-view.css' => '7148ae25', 'rsrc/css/phui/phui-document-pro.css' => 'dc3d46ed', @@ -573,6 +574,7 @@ return array( 'font-fontawesome' => '2b7ebbcc', 'font-lato' => 'c7ccd872', 'global-drag-and-drop-css' => '5c1b47c2', + 'guides-app-css' => '1d5414e5', 'harbormaster-css' => 'f491c9f4', 'herald-css' => 'dc31f6e9', 'herald-rule-editor' => 'd6a7e717', @@ -833,7 +835,7 @@ return array( 'phui-calendar-list-css' => 'fcc9fb41', 'phui-calendar-month-css' => '8e10e92c', 'phui-chart-css' => '6bf6f78e', - 'phui-cms-css' => '33064557', + 'phui-cms-css' => 'be43c8a8', 'phui-crumbs-view-css' => '9dac418c', 'phui-curtain-view-css' => '7148ae25', 'phui-document-summary-view-css' => '9ca48bdf', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d98c735f08..93a7bb66c8 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2633,6 +2633,8 @@ phutil_register_library_map(array( 'PhabricatorGuideApplication' => 'applications/guides/application/PhabricatorGuideApplication.php', 'PhabricatorGuideController' => 'applications/guides/controller/PhabricatorGuideController.php', 'PhabricatorGuideInstallController' => 'applications/guides/controller/PhabricatorGuideInstallController.php', + 'PhabricatorGuideItemView' => 'applications/guides/view/PhabricatorGuideItemView.php', + 'PhabricatorGuideListView' => 'applications/guides/view/PhabricatorGuideListView.php', 'PhabricatorGuideQuickStartController' => 'applications/guides/controller/PhabricatorGuideQuickStartController.php', 'PhabricatorGuideWelcomeController' => 'applications/guides/controller/PhabricatorGuideWelcomeController.php', 'PhabricatorHTTPParameterTypeTableView' => 'applications/config/view/PhabricatorHTTPParameterTypeTableView.php', @@ -7439,6 +7441,8 @@ phutil_register_library_map(array( 'PhabricatorGuideApplication' => 'PhabricatorApplication', 'PhabricatorGuideController' => 'PhabricatorController', 'PhabricatorGuideInstallController' => 'PhabricatorGuideController', + 'PhabricatorGuideItemView' => 'Phobject', + 'PhabricatorGuideListView' => 'AphrontView', 'PhabricatorGuideQuickStartController' => 'PhabricatorGuideController', 'PhabricatorGuideWelcomeController' => 'PhabricatorGuideController', 'PhabricatorHTTPParameterTypeTableView' => 'AphrontView', diff --git a/src/applications/guides/controller/PhabricatorGuideInstallController.php b/src/applications/guides/controller/PhabricatorGuideInstallController.php index 6781edaeac..46e992b438 100644 --- a/src/applications/guides/controller/PhabricatorGuideInstallController.php +++ b/src/applications/guides/controller/PhabricatorGuideInstallController.php @@ -8,6 +8,7 @@ final class PhabricatorGuideInstallController } public function handleRequest(AphrontRequest $request) { + require_celerity_resource('guides-app-css'); $viewer = $request->getViewer(); $title = pht('Installation Guide'); @@ -22,7 +23,7 @@ final class PhabricatorGuideInstallController $crumbs = $this->buildApplicationCrumbs() ->addTextCrumb(pht('Installation')); - $content = null; + $content = $this->getGuideContent($viewer); $view = id(new PHUICMSView()) ->setCrumbs($crumbs) @@ -37,10 +38,159 @@ final class PhabricatorGuideInstallController } - private function getGuideContent() { + private function getGuideContent($viewer) { + $guide_items = new PhabricatorGuideListView(); - $guide = null; + $title = pht('Resolve Setup Issues'); + $issues_resolved = !PhabricatorSetupCheck::getOpenSetupIssueKeys(); + $href = PhabricatorEnv::getURI('/config/issue/'); + if ($issues_resolved) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + "You've resolved (or ignored) all outstanding setup issues."); + } else { + $icon = 'fa-warning'; + $icon_bg = 'bg-red'; + $skip = '#'; + $description = + pht('You have some unresolved setup issues to take care of.'); + } - return $guide; + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + $configs = id(new PhabricatorAuthProviderConfigQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->execute(); + + $title = pht('Login and Registration'); + $href = PhabricatorEnv::getURI('/auth/'); + $have_auth = (bool)$configs; + if ($have_auth) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + "You've configured at least one authentication provider."); + } else { + $icon = 'fa-key'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = pht( + 'Authentication providers allow users to register accounts and '. + 'log in to Phabricator.'); + } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + + $title = pht('Configure Phabricator'); + $href = PhabricatorEnv::getURI('/config/'); + + // Just load any config value at all; if one exists the install has figured + // out how to configure things. + $have_config = (bool)id(new PhabricatorConfigEntry())->loadAllWhere( + '1 = 1 LIMIT 1'); + + if ($have_config) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + "You've configured at least one setting from the web interface."); + } else { + $icon = 'fa-sliders'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = pht( + 'Learn how to configure mail and other options in Phabricator.'); + } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + + $title = pht('User Account Settings'); + $href = PhabricatorEnv::getURI('/settings/'); + $preferences = id(new PhabricatorUserPreferencesQuery()) + ->setViewer($viewer) + ->withUsers(array($viewer)) + ->executeOne(); + + $have_settings = ($preferences && $preferences->getPreferences()); + if ($have_settings) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + "You've adjusted at least one setting on your account."); + } else { + $icon = 'fa-wrench'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = pht( + 'Configure account settings for all users, or just yourself'); + } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + + $title = pht('Notification Server'); + $href = PhabricatorEnv::getURI('/config/notifications/'); + // TODO: Wire up a notifications check + $have_notifications = false; + if ($have_notifications) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + "You've set up a real-time notification server."); + } else { + $icon = 'fa-bell'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = pht( + 'Phabricator can deliver notifications in real-time with WebSockets.'); + } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + + $guide_items->addItem($item); + + return $guide_items; } } diff --git a/src/applications/guides/controller/PhabricatorGuideQuickStartController.php b/src/applications/guides/controller/PhabricatorGuideQuickStartController.php index 0b6d315642..36ad87b869 100644 --- a/src/applications/guides/controller/PhabricatorGuideQuickStartController.php +++ b/src/applications/guides/controller/PhabricatorGuideQuickStartController.php @@ -8,6 +8,7 @@ final class PhabricatorGuideQuickStartController } public function handleRequest(AphrontRequest $request) { + require_celerity_resource('guides-app-css'); $viewer = $request->getViewer(); $title = pht('Quick Start Guide'); @@ -22,7 +23,7 @@ final class PhabricatorGuideQuickStartController $crumbs = $this->buildApplicationCrumbs() ->addTextCrumb(pht('Quick Start')); - $content = null; + $content = $this->getGuideContent($viewer); $view = id(new PHUICMSView()) ->setCrumbs($crumbs) @@ -37,10 +38,173 @@ final class PhabricatorGuideQuickStartController } - private function getGuideContent() { + private function getGuideContent($viewer) { + $guide_items = new PhabricatorGuideListView(); - $guide = null; + $title = pht('Configure Applications'); + $apps_check = true; + $href = PhabricatorEnv::getURI('/applications/'); + if ($apps_check) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + "You've uninstalled any unneeded applications for now."); + } else { + $icon = 'fa-globe'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = + pht('Use all our applications, or uninstall the ones you don\'t want.'); + } - return $guide; + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + + $title = pht('Invite Collaborators'); + $people_check = true; + $href = PhabricatorEnv::getURI('/people/invite/'); + if ($people_check) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + 'You will not be alone on this journey.'); + } else { + $icon = 'fa-group'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = + pht('Invite the rest of your team to get started on Phabricator.'); + } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + + $title = pht('Create a Repository'); + $repository_check = true; + $href = PhabricatorEnv::getURI('/diffusion/'); + if ($repository_check) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + "You've created at least one repository."); + } else { + $icon = 'fa-code'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = + pht('If you are here for code review, let\'s set up your first '. + 'repository.'); + } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + + $title = pht('Create a Project'); + $project_check = true; + $href = PhabricatorEnv::getURI('/project/'); + if ($project_check) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + "You've created at least one project."); + } else { + $icon = 'fa-briefcase'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = + pht('Project tags define everything. Create them for teams, tags, '. + 'or actual projects.'); + } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + + $title = pht('Build a Dashboard'); + $dashboard_check = true; + $href = PhabricatorEnv::getURI('/dashboard/'); + if ($dashboard_check) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + "You've created at least one dashboard."); + } else { + $icon = 'fa-dashboard'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = + pht('Customize the default homepage layout and items.'); + } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + + $title = pht('Personalize your Install'); + $ui_check = true; + $href = PhabricatorEnv::getURI('/config/group/ui/'); + if ($dashboard_check) { + $icon = 'fa-check'; + $icon_bg = 'bg-green'; + $skip = null; + $description = pht( + 'It looks amazing, good work. Home Sweet Home.'); + } else { + $icon = 'fa-home'; + $icon_bg = 'bg-sky'; + $skip = '#'; + $description = + pht('Change the name and add your company logo, just to give it a '. + 'little extra polish.'); + } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setSkipHref($skip) + ->setDescription($description); + $guide_items->addItem($item); + + return $guide_items; } } diff --git a/src/applications/guides/controller/PhabricatorGuideWelcomeController.php b/src/applications/guides/controller/PhabricatorGuideWelcomeController.php index 62a1a57567..065504bacb 100644 --- a/src/applications/guides/controller/PhabricatorGuideWelcomeController.php +++ b/src/applications/guides/controller/PhabricatorGuideWelcomeController.php @@ -8,6 +8,7 @@ final class PhabricatorGuideWelcomeController } public function handleRequest(AphrontRequest $request) { + require_celerity_resource('guides-app-css'); $viewer = $request->getViewer(); $title = pht('Welcome to Phabricator'); @@ -22,7 +23,8 @@ final class PhabricatorGuideWelcomeController $crumbs = $this->buildApplicationCrumbs() ->addTextCrumb(pht('Welcome')); - $content = null; + $content = id(new PHUIDocumentViewPro()) + ->appendChild($this->getGuideContent($viewer)); $view = id(new PHUICMSView()) ->setCrumbs($crumbs) @@ -37,10 +39,15 @@ final class PhabricatorGuideWelcomeController } - private function getGuideContent() { + private function getGuideContent($viewer) { - $guide = null; + $content = pht( + 'You have successfully installed Phabricator. These next guides will '. + 'take you through configuration and new user orientation. '. + 'These steps are optional, and you can go through them in any order. '. + 'If you want to get back to this guide later on, you can find it in '. + 'the **Config** application under **Welcome Guide**.'); - return $guide; + return new PHUIRemarkupView($viewer, $content); } } diff --git a/src/applications/guides/view/PhabricatorGuideItemView.php b/src/applications/guides/view/PhabricatorGuideItemView.php new file mode 100644 index 0000000000..e32515195c --- /dev/null +++ b/src/applications/guides/view/PhabricatorGuideItemView.php @@ -0,0 +1,67 @@ +title = $title; + return $this; + } + + public function setDescription($description) { + $this->description = $description; + return $this; + } + + public function setHref($href) { + $this->href = $href; + return $this; + } + + public function setIcon($icon) { + $this->icon = $icon; + return $this; + } + + public function setIconBackground($background) { + $this->iconBackground = $background; + return $this; + } + + public function setSkipHref($href) { + $this->skipHref = $href; + return $this; + } + + public function getTitle() { + return $this->title; + } + + public function getDescription() { + return $this->description; + } + + public function getHref() { + return $this->href; + } + + public function getIcon() { + return $this->icon; + } + + public function getIconBackground() { + return $this->iconBackground; + } + + public function getSkipHref() { + return $this->skipHref; + } + + +} diff --git a/src/applications/guides/view/PhabricatorGuideListView.php b/src/applications/guides/view/PhabricatorGuideListView.php new file mode 100644 index 0000000000..4f65a021ba --- /dev/null +++ b/src/applications/guides/view/PhabricatorGuideListView.php @@ -0,0 +1,45 @@ +items[] = $item; + return $this; + } + + public function render() { + require_celerity_resource('guides-app-css'); + + $list = id(new PHUIObjectItemListView()) + ->addClass('guides-app'); + + foreach ($this->items as $item) { + $icon = id(new PHUIIconView()) + ->setIcon($item->getIcon()) + ->setBackground($item->getIconBackground()); + + $list_item = id(new PHUIObjectItemView()) + ->setHeader($item->getTitle()) + ->setHref($item->getHref()) + ->setImageIcon($icon) + ->addAttribute($item->getDescription()); + + $skip_href = $item->getSkipHref(); + if ($skip_href) { + $skip = id(new PHUIButtonView()) + ->setText(pht('Skip')) + ->setTag('a') + ->setHref($skip_href) + ->setColor(PHUIButtonView::GREY); + $list_item->setLaunchButton($skip); + } + $list->addItem($list_item); + } + + return $list; + + } + +} diff --git a/webroot/rsrc/css/application/guides/guides.css b/webroot/rsrc/css/application/guides/guides.css new file mode 100644 index 0000000000..8a871758c8 --- /dev/null +++ b/webroot/rsrc/css/application/guides/guides.css @@ -0,0 +1,34 @@ +/** + * @provides guides-app-css + */ + +.guides-app ul.phui-object-item-list-view { + margin: 0; + padding: 20px; +} + +.guides-app .phui-object-item-no-bar .phui-object-item-frame { + border: 0; +} + +.guides-app .phui-object-item-image-icon { + margin: 8px 2px 12px; +} + +.guides-app a.phui-object-item-link { + color: #000; + font-size: {$biggestfontsize}; +} + +.guides-app .phui-object-item-name { + padding-top: 6px; +} + +.guides-app .phui-object-item-launch-button a.button { + font-size: {$normalfontsize}; + padding: 3px 12px 4px; +} + +.device-desktop .guides-app .phui-object-item { + margin-bottom: 8px; +} diff --git a/webroot/rsrc/css/phui/phui-cms.css b/webroot/rsrc/css/phui/phui-cms.css index 16b47c73ac..5284c69834 100644 --- a/webroot/rsrc/css/phui/phui-cms.css +++ b/webroot/rsrc/css/phui/phui-cms.css @@ -35,13 +35,14 @@ border: none; } -.phui-cms-view .phui-profile-header { +.phui-cms-view .phui-cms-page-content .phui-profile-header { padding: 32px 20px; border-bottom: 1px solid {$thinblueborder}; } .phui-cms-view .phui-document-view.phui-document-view-pro { width: auto; - padding: 32px 20px; + max-width: inherit; + padding: 0 20px; margin: 0; } From fcb20cb79989b6a42b484901e508fd2941110c7a Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 20 Aug 2016 14:06:34 -0700 Subject: [PATCH 22/22] Add a "--force" flag to "bin/repository move-paths" Summary: Ref T7148. The automated export process runs this via daemon, which can't answer "Y" to this prompt. Let it "--force" instead. (Some of my test instances didn't have any repositories, which is why I didn't catch this sooner.) Test Plan: Ran `bin/repository move-paths --force ...`, saw change applied without a prompt. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7148 Differential Revision: https://secure.phabricator.com/D16426 --- .../PhabricatorRepositoryManagementMovePathsWorkflow.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementMovePathsWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementMovePathsWorkflow.php index baee437883..ced8663b3c 100644 --- a/src/applications/repository/management/PhabricatorRepositoryManagementMovePathsWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementMovePathsWorkflow.php @@ -19,6 +19,10 @@ final class PhabricatorRepositoryManagementMovePathsWorkflow 'param' => 'prefix', 'help' => pht('Replace matching prefixes with this string.'), ), + array( + 'name' => 'force', + 'help' => pht('Apply changes without prompting.'), + ), )); } @@ -47,6 +51,8 @@ final class PhabricatorRepositoryManagementMovePathsWorkflow 'You must specify a path prefix to move to with --to.')); } + $is_force = $args->getArg('force'); + $rows = array(); $any_changes = false; @@ -118,7 +124,7 @@ final class PhabricatorRepositoryManagementMovePathsWorkflow } $prompt = pht('Apply these changes?'); - if (!phutil_console_confirm($prompt)) { + if (!$is_force && !phutil_console_confirm($prompt)) { throw new Exception(pht('Declining to apply changes.')); }