Summary:
Fixes T9874.
- Stop using the phrase "restart your webserver". Instead, say "restart Phabricator".
- Write a document explaining that "Restart Phabricator" means to restart all of the server processes, depending on how your configuration is set up, and approximately how to do that.
- Link to this document.
- In places where we are not specifically giving instructions and the user isn't expected to do anything, be intentionally vague so as to avoid being misleading.
Test Plan:
- Read document.
- Hit "exetnsion" and "PHP config" setup checks, got "restart Phabricator" with documentation links in both cases.
Reviewers: chad
Maniphest Tasks: T9874
Differential Revision: https://secure.phabricator.com/D14636
Summary:
Ref T9132. We have several places in the code that sometimes need to parse complex types. For example, we accept all of these in ApplicationSearch and now in ApplicationEditor:
> /?subscribers=cat,dog
> /?subscribers=PHID-USER-1111
> /?subscribers[]=cat&subscribers[]=PHID-USER-2222
..etc. The logic to parse this stuff isn't too complex, but it isn't trivial either.
Right now it lives in some odd places. Notably, `PhabricatorApplicationSearchEngine` has some weird helper methods for this stuff. Rather than give `EditEngine` the same set of weird helper methods, pull all this stuff out into "HTTPParameterTypes".
Future diffs will add "Projects" and "Users" types where all the custom parsing/lookup logic can live. Then eventually the Search stuff can reuse these.
Generally, this just breaks the code up into smaller pieces that have more specific responsibilities.
Test Plan: {F944142}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14402
Summary: Ref T8099, Missed a few various edge cases here. Cleans up the 'next' UI.
Test Plan: Review a current setup issue, then ignore it.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13529
Summary:
Hit this locally, with an error like:
> Version <empty string> is older than 1.9, the minimum supported version.
(Where `<empty string>` was just the empty string, not literally the text `<empty string>`.)
Be more careful about parsing versions, and parse the newer string.
Test Plan: Got "unknown version" with intentionally-broken test data, then clean readout.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11558
Summary:
Ref T4331. Ref T5968. Users sometimes have trouble figuring out how to ignore issues. The option is a bit hard to spot, especially if you aren't familiar with interfaces yet.
Make it a button on the issue page itself instead.
Test Plan:
Normal issue:
{F199225}
Ignored issue:
{F199226}
Fatal issue:
{F199227}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4331, T5968
Differential Revision: https://secure.phabricator.com/D10420
Summary: Fixes T6011. See that task for discussion. We can detect when `memory_limit` will be the limiting factor for drag-and-drop uploads and warn administrators about it.
Test Plan: Fiddled configuration values and hit, then resolved, the issue.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6011
Differential Revision: https://secure.phabricator.com/D10413
Summary: Fixes T2605. Provide some instructions on configuring RDS properly. The "DB Parameter Group" thing in the web UI seems pretty easy to use, it's just not obvious that it's what you should be using.
Test Plan: Jiggled these warnings to trigger them, viewed the output, saw a table of values and a hint about RDS.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2605
Differential Revision: https://secure.phabricator.com/D10343
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.
Test Plan: Eyeballed it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9431
Summary:
From @chad. This setup link should open in a new window so you don't lose your context in resolving setup issues.
@chad, this was the only one I could find immediately, let me know if you remember seeing others that I missed.
Test Plan: Faked an error, clicked the link, got a new tab.
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D7991
Summary: Ref T3780. Facebook has some environmental / itermittent stuff which would be easier to debug with host information on the setup issue screen.
Test Plan:
Checked both in-chrome and out-of-chrome versions of this screen, both looked reasonable.
{F56694}
Reviewers: wez, btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T3780
Differential Revision: https://secure.phabricator.com/D6842
Summary:
Fixes T3436. Currently, when installs have configuration options which we don't know about, we raise a fairly confusing/ambiguous message about the options being unknown. Instead:
- Keep a list of previously valid (but now deleted) config, with explanatory reasons for what happened to it. Present this information, along with altenate wording ("Obsolete Config" instead of "Unknown Config") where applicable.
- Show a list of all the places the config is defined.
- Provide an active link to delete it from the web UI.
- Provide a command to delete it from the CLI.
- Allow `bin/config delete` to delete configuration options which no longer have a definition.
Test Plan:
- Set an auth key in database, local and file config.
- Walked through the setup issue, cleaning it up.
- Set an invalid key and made sure I still got a reasonable error (this now has better cleanup instructions).
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3436
Differential Revision: https://secure.phabricator.com/D6317
Summary:
Currently PhabricatorSetupIssueView will show the current values of
configuration options regardless of whether or not they are defined
as hidden options. This means that if the MySQL server stops, Phabricator
will present the MySQL connection credentials to anyone who can access
the Phabricator page.
Test Plan:
Stop the MySQL server for a Phabricator instance. It should display 'hidden'
instead of the MySQL password.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5596
Summary: After D5305, this method does nothing since we automatically figure out what we need to do.
Test Plan:
- Viewed a page with the main menu on it (MainMenuView).
- Viewed a revision with transactions on it (TransactionView).
- Viewed timeline UIExample (TimelineView, TimelineEventView).
- Viewed a revision (PropertyListView).
- Viewed a profile (ProfileHeaderView).
- Viewed Pholio list (PinboardView, PinboardItemView).
- Viewed Config (ObjectItemView, ObjectItemListView).
- Viewed Home (MenuView).
- Viewed a revision (HeaderView, CrumbsView, ActionListView).
- Viewed a revision with an inline comment (anchorview).
- Viewed a Phriction diff page (AphrontCrumbsView).
- Filed T2721 to get rid of this.
- Looked at Pholio and made inlines and comments (mockimages, pholioinlinecomment/save/edit).
- Looked at conpherences.
- Browsed around.
Reviewers: chad, vrana
Reviewed By: chad
CC: edward, aran
Differential Revision: https://secure.phabricator.com/D5307
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such
Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5002
Summary: They are same because render() returns safe HTML and raw strings are automatically escaped.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4909
Summary:
I like this abstraction better.
Result of `phutil_implode_html()` may be also used as a param of `hsprintf()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4904
Summary: These are pretty straightforward, they just have a fair amount of instructional text with inline markup.
Test Plan: Added and viewed a UIExample.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4686
Summary:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, pht('...'))
The searched for `<` and `&` by sgrep.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4504
Summary:
Going to /config/issue/config.unknown.phabricator.setup/ fataled with
Call to a member function getLocked() on a non-object
Test Plan: Went to /config/issue/config.unknown.phabricator.setup/ and saw the page render.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4633
Summary:
Fixes T2293.
We currently hard-require this in setup. We do not need to; we don't actually need it until we start running daemons. Move it to post-install and provide more guidance.
We could make this even easier in the future, but we'd need to special case it, since it's dangerous to let it be set to any value (if you set it to the wrong value, you can't log in). We could safely have a workflow which writes the current request URI into the database configuration, or a two-stage workflow where we set the URI and then verify it, but these both imply some special casing and complication. This should be a step forward from where we are today, regardless.
Test Plan:
Removed "phabricator.base-uri" from my configuration. Verified Phabricator still works.
Without "phabricator.base-uri" configured, logged in from multiple host names (127.0.0.1:8080, local.aphront.com:8080).
Configured "phabricator.base-uri". Verified my unblessed session no longer worked. Verified setup issue went away.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2293
Differential Revision: https://secure.phabricator.com/D4580
Summary:
When configuration is set incorrectly (e.g., of the wrong type), detect and repair it by setting it to the default value. A setup warning will be raised separately.
Notably, this removes the need to hard-code all the class types.
This runs separately from the "invalid config" check because we need to run it on every page, but do setup checks only once per restart (some of them are slow).
Also dirty setup when we edit configuration.
Test Plan: Set config incorrectly on purpose, saw Phabricator correct it on restart and on every subsequent page load until it was fixed.
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2292
Differential Revision: https://secure.phabricator.com/D4492
Summary:
- Move `prettyPrintJSON()` and make it static.
- Use it from `PhabricatorSetupIssueView`
- Update other `config/` places that use it to call it from the new class.
This fixes a bug in `PhabricatorSetupIssueView` which showed up if the value
was an array and couldn't be rendered by `phutil_escape_html()`.
Test Plan:
- Rendered some config options.
- Went to /config/issue/config.unknown.phame.skins/ without error.
Reviewers: epriestley, btrahan, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4411
Summary:
- When a setup issue is nonfatal (i.e., a warning), instruct the user to edit the value from the web UI instead of using `bin/config`.
- When the user edits configuration in response to a setup issue, send them back to the issue when they're done.
- When an issue relates to PHP configuration, link to the PHP documentation on configuration.
- Add new-style setup check for timezone issues.
Test Plan: Mucked with my timezone config, resolved the issues I created.
Reviewers: codeblock, btrahan, vrana
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2221, T2228
Differential Revision: https://secure.phabricator.com/D4298
Summary:
This is basicaly a light version of D4286. The major problem with D4286 is that it's a huge leap and completely replaces the setup process in one step.
Instead, I want to do this:
- Add the post-setup warnings (yellow bar with "6 unresolved warnings...").
- Copy all setup checks into post-setup warnings (so every check has an old-style check and a new-style check).
- Run that for a little bit and make sure it's stable.
- Implement fatal post-setup checks (the red screen, vs the yellow bar).
- Run that for a little bit.
- Nuke setup mode and delete all the old checks.
This should give us a bunch of very gradual steps toward the brave new world of simpler setup.
Test Plan:
- Faked APC setup failures, saw warnings raise.
- Verified that this runs after restart (get + set).
- Verified that this costs us only one cache hit after first-run (get only).
Reviewers: btrahan, codeblock, vrana, chad
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4295
