Summary: Ref T603. Ref D6941.
Test Plan: Clicked around all over - looked good. I plan to re-test D6941 to make sure the executeOne case works now as intended
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D6944
Summary: Adds plain support for object lists that just look like lists
Test Plan: review UIexamples and a number of other applications
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6922
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:
This attempts some consistency in form layouts. Notably, they all now contain headers and are 16px off the sides and tops of pages. Also updated dialogs to the same look and feel. I think I got 98% of forms with this pass, but it's likely I missed some buried somewhere.
TODO: will take another pass as consolidating these colors and new gradients in another diff.
Test Plan: Played in my sandbox all week. Please play with it too and let me know how they feel.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6806
Summary:
We can get this out of PHIDType reasonably in all cases and simplify implementation here.
None of these translate correctly anyway so they're basically debugging/development strings.
Test Plan: `grep`, browsed some transactions
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6786
Summary:
^\s+(['"])dust\1\s*=>\s*true,?\s*$\n
Test Plan: Looked through the diff.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6769
Summary: See IRC. This is dumb but I think we should try to work by default on Debian, and it doesn't cost us too much. See inline comment for more.
Test Plan:
- No `disable_functions`, restarted, worked fine.
- Set `disable_functions = pcntl_derp`, restarted, worked fine.
- Set `disable_functions = derp`, restarted, setup fatal.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6741
Summary:
Fixes T3709. PHP has two configuration options ('disable_functions', 'disable_classes') which allow functions and classes to be blacklisted at runtime.
Since these break things in an unclear way, raise a setup fatal if they are set.
We take a slightly more tailored approach to these in `phd` already, but I'd rather try just saying "no, this is bad" and see if we can get away with it. I suspect we can, and there's no legitimate reason to blacklist functions given that Phabricator must have access to, e.g., `proc_open()`.
Test Plan: {F54058}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3709
Differential Revision: https://secure.phabricator.com/D6739
Summary: Fixes T3710. The text on these options is switched around.
Test Plan: {F54051} {F54052}
Reviewers: btrahan, nmalcolm, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3710
Differential Revision: https://secure.phabricator.com/D6737
Summary: Fixes T1144. Though actually I think T1144 wanted some handy way to email from the command-line / arc, this is cooler. :D
Test Plan: set conf properly and then ./bin/mail receive-test --as btrahan --to pasties@phabricator.dev | README --> it worked...! couldn't test files as easily but verified exception thrown when I tried to test.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1144
Differential Revision: https://secure.phabricator.com/D6622
Summary:
Ref T3373. The submit listener doesn't properly scope the form it listens to right now, so several forms on the page mean that comments post to one of them more or less at random.
Scope it properly by telling it which object PHID it is associated with.
Test Plan: Made Question comments, saw comments Ajax in on the question itself rather than on an arbitrary answer.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6611
Summary:
Fixes T3632. Cleans up a bunch of DarkConsole stuff:
- The config setting had out-of-date instructions. Modernize the instructions.
- The setting was sort of hidden under "Display Preferences". Move it to a new "Developer Preferences".
- The setting magically appeared if DarkConsole was enabled on the install. Instead, always show it but explain why it isn't availalbe.
- When the user enables the console, also force it to actually be shown.
- Call out instructions about use of the "`" key more clearly.
Test Plan: Viewed config setting. Viewed settings panel. Changed setting. Enabling the setting showed DarkConsole.
Reviewers: garoevans, chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3632
Differential Revision: https://secure.phabricator.com/D6594
Summary: The `phabricator.allowed-uris` config setting is not checked properly when trying to set cookies.
Test Plan:
Set an alternate URI, then accessed Phabricator. No longer received a secondary cookie error.
Hit the new exceptions to test them:
{F51131}
{F51132}
Reviewers: btrahan, garoevans
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6528
Summary:
See discussion in T2715. Currently, PHIDs are all hard coded in the PHID application. In the long run, we need to move them out into actual applications.
A specific immediate issue is Releeph, which uses a very very old and very broken mechanism to inject PHIDs in a way that only sort of works.
Moving forward, every PHID type will be provided by a `PhabricatorPHIDType` subclass, which will manage loading it, etc.
This also moves toward cleaning up the "load objects by name" (where "name" means something like `D12`) code, which is an //enormous// mess and spread across at least 4-5 callsites.
Test Plan: Used `phid.lookup` and `phid.query` to load Slowvotes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6502
Summary: Fixes T3453. Makes it easier for me to get version info when supporting users, since they can copy/paste from the web UI instead of running a bunch of `git` commands.
Test Plan: {F50749}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3453
Differential Revision: https://secure.phabricator.com/D6491
Summary:
Ref T2852. It's a little tricky to figure out Asana workspace IDs right now. If the viewer has a linked account, just pull their workspaces and show them which IDs are available.
(In theory, we could use a `<select>`, but it would have more edge cases; this seems like a pretty solid fix.)
Test Plan: {F49938}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6437
Summary: Fixes T3525. This feels way better, although it's still a little hard for me to pick out of lists with otherwise default-colored items.
Test Plan: {F49910} {F49911}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3525
Differential Revision: https://secure.phabricator.com/D6435
Summary:
- The dialog has no body text.
- It uses an "Ignore" link because it predates action items.
Test Plan:
{F49894}
{F49895}
{F49896}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6433
Summary:
These versions are broken, but package distros seem to be picking them up. :/
Since the error you get is completely useless, fatal immediately with a useful message.
Ref T2594.
Test Plan: Faked verisions and hit the issue.
Reviewers: btrahan
Reviewed By: btrahan
CC: brennantaylor, Arijit, aran
Maniphest Tasks: T2594
Differential Revision: https://secure.phabricator.com/D6415
Summary: Fixes T3501. `apc.stat` should generally be 0 in production and 1 in development. Raise a setup warning if it isn't.
Test Plan:
Hit both setup warnings.
{F49176}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3501
Differential Revision: https://secure.phabricator.com/D6376
Summary:
- Add GC support to conduit logs.
- Add Query support to conduit logs.
- Record the actual user PHID.
- Show client name.
- Support querying by specific method, so I can link to this from a setup issue.
@wez, this migration may not be fast. It took about 8 seconds for me to migrate 800,000 rows in the `conduit_methodcalllog` table. This adds a GC which should keep the table at a more manageable size in the future.
You can safely delete all data older than 30 days from this table, although you should do it by `id` instead of `dateCreated` since there's no key on `dateCreated` until this patch.
Test Plan:
- Ran GC.
- Looked at log UI.
- Ran Conduit methods.
Reviewers: btrahan
Reviewed By: btrahan
CC: wez, aran
Differential Revision: https://secure.phabricator.com/D6332
Summary: Fixes T3400. Users are crafty. Attempt to outwit them.
Test Plan: Added all kinds of nonsense to my PATH to hit all the errors. Verified sensible-looking error messages which I couldn't figure out any way to misread or outwit.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3400
Differential Revision: https://secure.phabricator.com/D6318
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: When I ignore setup issues, I want them to look dealt with, and keep yellow for new ones. Also updated callout colors.
Test Plan: Ignored a number of issues.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6300
Summary: Ref T3322. Depends on D6297. Here are some Phabricator tweaks to complment D6297.
Test Plan: {F47522}
Reviewers: garoevans
Reviewed By: garoevans
CC: aran, chad
Maniphest Tasks: T3322
Differential Revision: https://secure.phabricator.com/D6298
Summary: Generally prefer 'cards' to represent individual 'items' or 'action items', so I think it works here.
Test Plan: Reload setup issues pages.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6252
Summary:
Ref T1536.
- Move all the provider-specific help into contextual help in Auth.
- This provides help much more contextually, and we can just tell the user the right values to use to configure things.
- Rewrite account/registration help to reflect the newer state of the word.
- Also clean up a few other loose ends.
Test Plan: {F46937}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6247
Summary: Ref T1536. This is the last major migration. Moves us over to the DB and drops all the config stuff.
Test Plan:
- Ran the migration.
- Saw all my old config brought forward and respected, with accurate settings.
- Ran LDAP import.
- Grepped for all removed config options.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran, wez
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6243
Summary:
Ref T1536. Currently, when you install Phabricator you're dumped on the login screen and have to consult the documentation to learn about `bin/accountadmin`.
Instead, detect that an install is running first-time setup:
- It has no configured providers; and
- it has no user accounts.
We can safely deduce that such an install isn't configured yet, and let the user create an admin account from the web UI.
After they login, we raise a setup issue and lead them to configure authentication.
(This could probably use some UI and copy tweaks.)
Test Plan:
{F46738}
{F46739}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6228
Summary:
Ref T3354. There's no way for us to test most of the config options which actually affect this limit, so the Phabricator config is basically a canary value to indicate "the administrator hasn't configured anything yet".
Raise a setup issue if it isn't set. There's a trail to get here from Files, but we've de-emphasized the old-school upload form so it's hard to unearth.
Emphasize the warning that you need to read the documentation and configure like 30 other things to make this work.
Test Plan: Cleared my config, verified I got the issue, read it, set my config, issue went away.
Reviewers: jamesr, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3354
Differential Revision: https://secure.phabricator.com/D6185
Summary:
Ref T1703. This sets the stage for (but does not yet implement) custom UI types for config. In particular, a draggable list for custom fields.
I might make all the builtin types go through this at some point too, but don't really want to bother for the moment. It would be very slightly cleaner but woudn't get us much of anything.
Test Plan:
UI now renders via custom code, although that code does nothing (produces an unadorned text field):
{F45693}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1703
Differential Revision: https://secure.phabricator.com/D6154
Summary: At the global level, truncate emails at a user-configured size.
Test Plan: Untested, as I could not get PHP to send emails on my box, but if you can this should be very easy to test. Just set the max size to something like .001 kilobytes and make sure it does the right thing.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T1392
Differential Revision: https://secure.phabricator.com/D6118
Summary:
These are a bit tricky because we don't want to require you to install a VCS you don't use just to use Phabricator. Test that repositories exist before performing the checks.
I'll couple this with additional checks during repository creation.
Test Plan: Changed binary names to nonexistent ones, verified setup issues raised properly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6040
Summary:
A few more of these issues have cropped up recently. Basically:
- Webservers often (by default, I guess?) have a different or nonexistent $PATH.
- Users have a hard time figuring this out, since it's not obvious that the webserver might have a different configuration than the CLI, and they can run "git" and such themselves fine, and they don't normally use SetEnv or similar in webserver config.
I've been pursuing one prong of attack here (better detection and more tailored errors); this is a second prong (try to just guess the configuration correctly).
In 99% of cases, the binaries in question are in one of these three places, so just make them the default appended paths. If users have wacky configs they can override the setting.
Test Plan: Viewed config locally.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6039
Summary:
Kind of a quick look at an idea for T2184
Ref T2184
Test Plan: Make sure the site still loads
Reviewers: epriestley
CC: aran, Korvin, mbishopim3
Maniphest Tasks: T2184
Differential Revision: https://secure.phabricator.com/D6045
Summary: I introduced this helper at some point, clean up all the code duplication around content sources.
Test Plan: Grepped; hit edit interfaces for most/all of these.
Reviewers: btrahan, chad, edward
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6030
Summary:
Spent an hour or two helping a user figure this out. Make sure I never do that again.
If the webserver is configured with an empty or bogus PATH, binaries like 'which' and 'diff' (and 'git', and 'svn', etc.) may not be available. In most cases, this is fine, because we get an error like "sh: whatever-command not found", which is obvious to diagnose.
In the case of 'diff', we don't get this, because 'diff' is expected to exit with a nonzero code for differing files -- so we interpret the "sh: whatever-command not found" as "files differ" and then try to parse the empty output.
Explicitly check for 'which' (on Windows, 'where') and 'diff' during setup (I plan to refine the behavior around 'git', 'svn' and 'hg' at some point, but this is less pressing since the errors are trivial to support).
Test Plan: Faked failures on all modes, verified setup warnings look reasonable.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6008
Summary:
See <https://github.com/facebook/phabricator/issues/320>. We have a soft dependency on 'fileinfo', which we try to recover from (with `file`) but won't be able to on Windows and apparently FreeBSD systems. Since users can ignore setup checks anyway now, just raise a warning during install.
I believe almost all installs should have this extension, it has been part of the core for a long time.
Test Plan: Faked setup failure, looked at warning. "Solved" setup failure, saw it go away.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5952
Summary: Using PhabricatorExternalAccount in place maniphest.default-public-author.
Test Plan:
Using receivemail to see if the a new entry is made in the 'phabircator_user.user_externalaccount' table. Few things, I noticed that phabricator creates table 'user_externalaccout'. And now it throws up error 'Unknown column 'dateCreated' in 'field list''. Awaiting your comments.
{F41370}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T1205
Differential Revision: https://secure.phabricator.com/D5747
Summary:
Currently, `isInstalled()` and `getAllInstalledApplications()` are inconsistent:
- `isInstalled()` returns true for beta apps, even if `phabricator.show-beta-applications` is false.
- `getAllInstalledApplications()` omits beta apps if `phabricator.show-beta-applications` is false.
Making the beta config control installs (not just homepage visibility) makes far more sense as we roll out more thorough application integrations.
Make `isInstalled()` respect beta, and clean up some callsites.
D5602 builds on this.
Test Plan: Installed/uninstalled beta apps, verified Conpherence menu/panel and other application integrations dropped out of the UI.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5603
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:
See D5561. Ref T2378.
- Add `support/bin/*` to .gitignore so any symlinks or binaries won't get picked up by Git.
- Add a README so Git preserves the directory and there's at least //some// documentation of its existence.
Test Plan: ummmmm
Reviewers: jevripio, codeblock, btrahan
Reviewed By: jevripio
CC: aran
Maniphest Tasks: T2378
Differential Revision: https://secure.phabricator.com/D5562
Summary:
Use correct spelling of 'environment.append-paths' so that the current
value of the variable will display as expected in the
'pygmentize Not Found' setup issue screen.
Test Plan:
* Enabled Pygments but haven't installed it
* Follow 'unresolved setup issues' link to 'Not Found' screen
* See that 'envinronment.append-paths' is None
* Set 'environment.append-paths'
* See that 'envinronment.append-paths' is still None
* Apply this fix
* See that 'environment.append-paths' is now '/usr/bin'
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5555
Summary:
We have a fair number of conditionals on the existence of the access log. Instead, always build it and just don't write it if the user doesn't want a version on disk.
Also, formalize logged-in user PHID (avoids object existence juggling) in the access log and move microseconds-since-startup to PhabricatorStartup (simplifies index.php).
Depends on D5532. Fixes T2860. Ref T2870.
Test Plan: Disabled access log, verified XHProf writes occurred correctly.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2860, T2870
Differential Revision: https://secure.phabricator.com/D5533
Summary: Use UA strings to detect platform; override general monospaced settings with platform-specific ones. Fixes T2868.
Test Plan: whatcouldgowrong
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2868
Differential Revision: https://secure.phabricator.com/D5526
Summary:
The welcome.enabled feature is turned on by default; unsure of
whether this is desired by upstream.
Test Plan:
Apply the patch and see the welcome content. It's configurable
in the config.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5339
Summary:
Initial pass at elements appearing on M10.
Glaring omissions:
- I cut a single icon out of M10 in a haphazard way.
- No linear graident texture on the cards.
Test Plan:
{F35248}
{F35249}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5311
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: T2381
Test Plan:
Click on the ignore link in /config/issue/ and respond to the dialog box.
Also, test uninstalling and reinstalling an application in the web UI (to verify that refractoring didn't break anything).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2381
Differential Revision: https://secure.phabricator.com/D5234
Summary: Adds a hover state, helpful for large stacked lists. Also applied dust to projects and config to go with the hover changes.
Test Plan: Config, Projects
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5223
Summary: Fixes T2659. These didn't exist until recently.
Test Plan: {F34556}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2659
Differential Revision: https://secure.phabricator.com/D5221
Summary:
Provide a viewer to all remarkup engines.
This fixes commit summaries in Diffusion, which were failing to link because they didn't have a user and thus couldn't see/load `D123`, e.g.
Test Plan: Grepped for engine creation.
Reviewers: vrana
Reviewed By: vrana
CC: aran, edward
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D5152
Summary: We use mysqli if it's available by default. Don't require installs to build with mysql.
Test Plan: Applied to new secure.phabricator.com install.
Reviewers: chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5194
Summary: hehehe
Test Plan: Reloaded /config/, no more bogus setup issuse.
Reviewers: kwadwon, staticshock, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5158
Summary: Check if pygmentize is runnable if pygments is enabled
Test Plan: Enable pygments with pygmentize unavailable in path
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5157
Summary: T2381
Test Plan:
Include existing setup issues in the ignore config option,
reduces the number of setup issues in the status bar, moves ignored
issues to the bottom of the list, and marks them as ignored.
Also include a string corresponding to no setup issue, and verify that
application does not break.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5072
Summary: Makes sense with `QueryFuture`.
Test Plan: Switched secure.phabricator.com to MySQLi and nothing exploded.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5066
Test Plan: Enter in a url and create a macro. :)
Reviewers: epriestley
Reviewed By: epriestley
CC: epriestley, aran, dctrwatson, Korvin
Differential Revision: https://secure.phabricator.com/D5039
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:
Some transactions (like editing configuration values, task descriptions, or Conpherence images) can't be simply explained and need an additional larger element to show them fully (like a text diff).
Support change details like this in ApplicationTransactions. Implements the element in Config, so you can see changes.
Test Plan: {F32974}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2213
Differential Revision: https://secure.phabricator.com/D4984
Summary: Sgrepped for `"=~/</"` and manually changed every HTML.
Test Plan: This doesn't work yet but it is hopefully one of the last diffs before Phabricator will be undoubtedly HTML safe.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4927
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:
Lots of killed `phutil_escape_html()`.
Done by searching for `AphrontTableView` and then `$rows` (usually) backwards.
Test Plan:
Looked at homepage.
echo id(new AphrontTableView(array(array('<'))))->render();
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4884
Summary: Route all `$_SERVER['HTTP_...']` stuff through AphrontRequest (it would be nice to make this non-static, but the stack is a bit tangled right now...)
Test Plan: Verified CSRF and cascading profiling. `var_dump()`'d User-Agent and Referer and verified they are populated and returned correct values when accessed. Restarted server to trigger setup checks.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4888
Summary: T2361
Test Plan:
Set value to metamta.mail-adapter and reload page. Defaults to assigned value.
Performed same test with metamta.can-send-as-user to check that functionality is not broken for config 'boolean' options.
Reviewers: epriestley
Reviewed By: epriestley
CC: kwadwon, aran, Korvin
Maniphest Tasks: T2361
Differential Revision: https://secure.phabricator.com/D4881
Summary: Preserving animation of GIF profile Pictures
Test Plan: Uploaded Animated images as profile pictures to check if the animation of gif images is preserved and it does :) somewhat !
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4833
Summary: Done by searching for `AphrontErrorView` and then `appendChild()`.
Test Plan:
Looked at Commit Detail.
Looked at Revision Detail.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4843
Summary:
- Make the warning describe rationale and point at the MySQL manual explicitly.
- Add a reference to the developer mode config, in case the user wants to resolve the probelm by disabling developer mode.
- Now that the message is huge, provide a summary.
- Move from "Database" to "MySQL" setup checks -- this is kind of arbitrary, but the former is used for fatals (pre-install) and the latter for warnings (post-install) right now. This has no practical impact on anything and is purely stylistic.
Test Plan:
{F31798}
{F31799}
Reviewers: edward, blc
Reviewed By: edward
CC: aran
Differential Revision: https://secure.phabricator.com/D4835
Summary:
I attempted to test every interface. I probably missed some stuff, but I at least have some level of confidence that the `phutil_tag` branch is fairly stable.
Fixed these issues:
[1] Fixed a Herald issue with object links in transcripts. Some actions return
links; this was previously hard-coded.
[2] DarkConsole refactoring created an issue where the "`" event handler registered too many times.
[3] Fixed a bug where `strlen($value)` was being checked, but fields may now return array(). Possibly we should implement phutil_is_empty_html() or similar.
[4] Fixed a undefined variable issue for image edit transactions.
[5] Fixed an issue with rendering participant transactions. This required phutil_safe_html() because `pht()` can't handle `array()` for `%s`.
[6] Fixed an issue where feed was entirely overescaped by reverting an overly ambitious render_tag -> tag.
[7] Fixed an issue with strict tables and inserting `''` instead of `0` into an integer column.
[8] Fixed an issue where • was shown escaped.
[9] Fixed an issue where "no data" was overescaped.
[10] Fixed an issue with strict tables and inserting `''` instead of `0` into an integer column.
[11] Fixed an issue with strict tables and inserting `''`.
[12] Fixed an issue with missing space after ":" for mini panels.
Encountered (but did not fix) these issues:
[X1] "e" works incorrectly on comments you are not allowed to edit. Did not fix.
[X2] Coverage currently interacts incorrectly with "--everything" for Phutil tests.
Test Plan:
- Viewed Differential.
- Created a diff via copy/paste.
- Viewed standalone diff.
- Jumped to diff via changeset table.
- Created a revision.
- Updated revision.
- Added a comment.
- Edited revision dependencies.
- Edited revision tasks.
- Viewed MetaMTA transcripts.
- Viewed Herald transcripts [1].
- Downloaded raw diff.
- Flagged / unflagged revision.
- Added/edited/deleted inline comment.
- Collapsed/expanded file.
- Did show raw left.
- Did show raw right.
- Checked previews for available actions.
- Clicked remarkup buttons
- Used filetree view.
- Used keyboard: F, j, k, J, K, n, p, t, h, "?" [2] [X1].
- Created a meme.
- Uploaded a file via drag and drop.
- Viewed a revision with no reviewers.
- Viewed a revision with >100 files.
- Viewed various other revisions [3].
- Viewed an image diff.
- Added image diff inline comments.
- Viewed Maniphest.
- Ran various queries.
- Created task.
- Created similar task.
- Added comments to tasks.
- Ran custom query.
- Saved custom query.
- Edited custom queries.
- Drag-reordered tasks.
- Batch edited tasks.
- Exported tasks to excel.
- Looked at reports (issue in T2311 notwithstanding).
- Viewed Diffusion.
- Browsed Git, SVN, HG repositories.
- Looked at history, browse, change, commit views.
- Viewed audit.
- Performed various audit searches.
- Viewed Paste.
- Performed paste searches.
- Created, edited, forked paste.
- Viewed Phriction.
- Edited a page.
- Viewed edit history.
- Used search typeahead to search for user / application.
- Used search to search for text.
- Viewed Phame.
- Viewed Blog, Post.
- Viewed live post.
- Published/unpublished post.
- Previewed post.
- Viewed Pholio.
- Edited/commented mock.
- Viewed ponder.
- Viewed question.
- Added answer/comment.
- Viewed Diviner.
- Viewed Conpherence [4] [5].
- Made Conpherence updates.
- Viewed calendar.
- Created status.
- Viewed status.
- Viewed Feed [6].
- Viewed Projects.
- Viewed project detail.
- Edited project.
- Viewed Owners.
- Viewed package detail.
- Edited package [7].
- Viewed flags.
- Edited flag.
- Deleted flag.
- Viewed Herald.
- Viewed rules.
- Created rule.
- Edited rule.
- Viewed edit log.
- Viewed transcripts.
- Inspected a transcript.
- Viewed People.
- Viewed list.
- Administrated user.
- Checked username/delete stuff.
- Looked at create/import LDAP/activity logs.
- Looked at a user profile.
- Looked at user about page.
- Looked at Repositories.
- Edited repository.
- Edited arcanist project.
- Looked at daemons.
- Looked at all daemons [8].
- Viewed combined log.
- Looked at configuration.
- Edited configuration.
- Looked at setup issues [9].
- Looked at current settings.
- Looked at application list.
- Installed / uninstalled applications [10].
- Looked at mailing lists.
- Created a mailing list.
- Edited a mailing list.
- Looked at sent mail.
- Looked at received mail.
- Looked at send/receive tests.
- Looked at settings.
- Clicked through all the panels.
- Looked at slowvote.
- Created a slowvote [11].
- Voted in a slowvote.
- Looked at Macro.
- Created a macro.
- Edited a macro.
- Commented on a macro.
- Looked at Countdown.
- Created a Countdown.
- Looked at it.
- Looked at Drydock.
- Poked around a bit.
- Looked at Fact.
- Poked around a bit.
- Looked at files.
- Looked at a file.
- Uploaded a file.
- Looked at Conduit.
- Made a Conduit call.
- Looked at UIExamples.
- Looked at PHPAST.
- Looked at PHIDs.
- Looked at notification menu.
- Looked at notification detail.
- Logged out.
- Logged in.
- Looked at homepage [12].
- Ran `arc unit --everything --no-coverage` [X2].
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4807
Summary: Suggest the MySQL mode STRICT_ALL_TABLES during setup if it is not set. Small improvement to the phabricator.developer-mode comments.
Test Plan: Set the global sql_mode to include or exclude STRICT_ALL_TABLES and check for desired behavior.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4803
Summary: Refactor options related to verbose error reporting and forcing disk reads into a single developer option.
Test Plan: Run Phabricator with the developer-mode option set and check that errors print stack traces, static assets are always reloaded, etc.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4780
Summary: If the check is too much, let me know. I noticed you send over __ajax__=true, so I figured it was safest to evaluate existance and value.
Test Plan: Included unit test. Would have included a test where __ajax__ and __conduit__ are not set, but without mocking this gives an uncatchable Fatal Error. If you want me to include it, just direct me on the mocking strategy.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2401
Differential Revision: https://secure.phabricator.com/D4719
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 Applications application which allows uninstallation & installation of application.
Test Plan: In "Applications" application, clicked on uninstalled the application by cliking Uninstall and chekcing whether they are really uninstalled(Disabling URI & in appearance in the side pane). Then Clicked on the install button of the uninstalled application to check whether they are installed.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4715
Summary: This works after pht() + html got sorted out.
Test Plan: Looked at some object attribute lists.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4645
Summary: Add installation check for a dot in the domain, which is necessary for some browsers to set cookies.
Test Plan: Restart web server to force the setup procedures to run again.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4710
Summary: Added a reply handler. A few problems -- first, I can't seem to get this to actually send me email so I haven't been able to reply (which I would have done by generating a reply, then copying the raw email into scripts/mail_handler.php). Second, the subject is often terrible on these emails -- unless the conpherence is named its something gross like "E4:" Third, on create I am noticing an error on array_combine() which I think is related to the need to write array_combine_not_broken or what have you I saw go by... (PhabricatorTransactionEditor does array_combine(xaction->getOldValue(), xaction->getOldValue()) and complains that the arrays are empty)
Test Plan: noted that /mail/ said mails were being sent
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D4656
Summary:
This adds a new method for rendering the object list as a stackable set of items. Good for certain renderings like Config.
u
Test Plan: Review list on iOS, Chrome, FF.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4637
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:
There were a few defaults that got changed when porting to PHP. Most of them
seem to be accidental, so this diff sets them back to correctness.
Test Plan:
php> require '../libphutil/src/__phutil_library_init__.php';
php> require 'src/__phutil_library_init__.php'
php> $a = PhabricatorApplicationConfigOptions::loadAllOptions()
php> $b = require 'conf/default.conf.php';
php> $x = array();
php> foreach($a as $key => $obj) { $x[$key] = $obj->getDefault(); }
php> foreach($x as $key => $default) { if ($b[$key] != $default) { echo "$key has different default.\n"; } }
log.access.format has different default.
(seems to be intentional)
PHP Notice: Undefined index: phabricator.env in /usr/lib/python2.7/site-packages/phpsh/phpsh.php(577) : eval()'d code on line 1
(no longer in config file)
PHP Notice: Undefined index: test.value in /usr/lib/python2.7/site-packages/phpsh/phpsh.php(577) : eval()'d code on line 1
(not in config file)
metamta.default-address has different default.
(intentional)
metamta.domain has different default.
(intentional)
PHP Notice: Undefined index: phid.external-loaders in /usr/lib/python2.7/site-packages/phpsh/phpsh.php(577) : eval()'d code on line 1
(no longer in config file)
phame.skins has different default.
(fixed in D4618)
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4621
Summary: I heard this HTTP thing is pretty good.
Test Plan: @starruler did things which confirmed this is less bad than D4611.
Reviewers: starruler
Reviewed By: starruler
CC: aran
Differential Revision: https://secure.phabricator.com/D4612
Summary: Fixes T2392.
Test Plan: grepped for others, this is the only `set` with non-array default
Reviewers: chad, starruler
Reviewed By: starruler
CC: aran
Maniphest Tasks: T2392
Differential Revision: https://secure.phabricator.com/D4611
Summary: These should default to array() so they're safe to `foreach` over.
Test Plan: Grepped for 'list<string>'.
Reviewers: codeblock, btrahan, starruler, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4600
Summary:
- PHABRICATOR_ENV is now optional. If you don't specify it, we won't load a config file.
- PhabricatorSetup is now gone.
- I removed the alternate file domain check for now, see T2380.
- `phabricator.setup` config is now gone.
- Rewrote documentation:
- No more mentions of `phabricator.setup`.
- Normal install guide no longer mentions PHABRICATOR_ENV. This is now an advanced topic.
- Clarified that you only need to set up one of apache, nginx or lighttpd.
- Tweaked a few things I've seen users have difficulty with.
This should have no effect on any existing installs, but make the process much simpler for future installs.
Closes T2221.
Closes T2223.
Closes T2228.
Test Plan:
- Removed my PHABRICATOR_ENV and went through the install process.
- Generated and read documentation.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2221, T2223, T2228
Differential Revision: https://secure.phabricator.com/D4596
Summary: Port the database checks over.
Test Plan: Triggered all the checks via intentional misconfiguration.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4590
Summary:
- Allow new-style setup to raise fatal setup errors.
- Port extension checks to new-style setup as fatal errors.
- When fatal errors are raised, abort setup and show them in a chrome-free response.
Test Plan: {F29981}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4587
Summary:
We no longer need to do PHP CLI checks (D4568) or run `git submodule` (D4581) so we don't need $PATH to be set to complete setup. Move it to post-install.
Drop the instructions about PHP-FPM because the Phabricator config is dramatically easier now that we have it.
Test Plan: Set environment.append-paths to various things, faked lack of $PATH, verified I got the warning when I expected to setting Phabricator config cleared it.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4585
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: Might not be the cleanest way to do this, but seems to work.
Test Plan:
- Saved an option which used the new enum type.
- Changed it.
- Saw it show up on the list view.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4572
Summary: Fixed T2349
Test Plan: Temporarily added a VERSION file with some text and it rendered it correctly.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2349
Differential Revision: https://secure.phabricator.com/D4544
Summary:
T2345
getConfig throws an Exception when the key does not exist.
Also removes dead code that throws an Exception.
Test Plan:
Reloaded the Phabricator home page. In the process, found
2 Exceptions thrown due to nonexistent keys. After addressing these problems,
the home page loads without Exceptions.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4541
Summary: Fixed T2349
Test Plan:
Could not visibly see version at footer any more. Appeared in the top of /config.
Does not appear as a config option in /config.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2349
Differential Revision: https://secure.phabricator.com/D4539
Summary: Technically we should have these for all the OAuth providers but I don't think anyone really has trouble with them and it can probably be done generically after T1536. Preserve the functionality, at least.
Test Plan: Broke my config, verified warnings appeared.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4506
Summary: These are nonblocking warnings and can move to post-install.
Test Plan: Broke my environment and observed the warnings.
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran, asherkin
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4505
Summary:
Migrate to the new hotness. Also:
- Remove a string test, which is now impossible since the config will repair itself and raise a type error.
- Restore the header even in /config/ -- this check is kind of hacky and it feels a bit more natural now that it's above the menu.
Test Plan: Set my local disk path to something invalid, verified I got a setup error.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4497
Summary: Fixes T2269. If the user manages to mess up both the PHP and Phabricator configurations, set the timezone to UTC. We basically never use this anyway (we always render into the user's time), PHP just gets angry at us if we don't set it. (We do use it for logged-out users, I suppose.)
Test Plan: Set PHP and Phabricator timezones to goofy nonsense, verified we recover sensibly from it.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2228, T2269
Differential Revision: https://secure.phabricator.com/D4496
Summary: Because the Default configuration provider is loaded before custom libraries, any config options specified in them don't get a default values.
Test Plan: Looked at /config/
Reviewers: epriestley, codeblock, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4532
Summary: Minor spacing tweaks to Config app. Added label for consistency.
Test Plan: Review pages in the Config app for spacing.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4523
Summary: Ports mail stuff from the existing setup process to the more modular setup checks.
Test Plan: Configured my local install to have all these errors, verified setup raised them.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4494
Summary: We should set the value if it's valid, not if it's invalid. derpp~
Test Plan: Set `files.viewable-mime-types`.
Reviewers: asherkin, codeblock, btrahan, vrana
Reviewed By: asherkin
CC: aran
Differential Revision: https://secure.phabricator.com/D4510
Summary:
If your configuration overrides the connection adapter, we need to load libraries before we can setup the database config source.
Also lock this since it won't work when edited from the web anymore, and so sneaky users can't upload stuff and then edit their config to run arbitrary code.
Test Plan: See chatlog in #phabricator. This is a problem for Facebook only.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4498
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: The remaining hash/key values are already-migrated, I am just bad at grep. Also implement a "set" type.
Test Plan: Looked at set, edited set.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4476
Summary: Oops, missed this -- alphabetical is probably a better sort order than by-group-then-by-definition.
Test Plan: Looked at alphabetical options.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4474
Summary:
Currently, we have a "config" panel in DarkConsole. It's useful to have a table of all effective config values, but it doesn't need to be in DarkConsole. Move it to Config instead. Basically:
- You don't need to activate DarkConsole to see it anymore;
- now visible only to admins;
- respects config mask/hide;
- somewhat prettier;
- links to config edit;
- no longer ships down on every DarkConsole request with a giant table of rarely-used data.
Test Plan: Looked at the table. Looked at lack of table in darkconsole.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4473
Summary:
Allow extra options to be locked, hidden or masked via config. These options are themselves locked and can not be edited via the web UI.
The primary goal here is to let us lock or hide things from SaaS installs (e.g., keys, etc.), or to let server administrators lock or hide information from web UI administrators if they want to for some reason.
The secondary goal is to remove the `darkconsole.config-mask` option, although I might just remove the panel entirely and put it in the config app, since that probably makes far more sense. Yeahhhhh... probably doing that.
These options need masks when ported (they haven't been ported yet):
phabricator.csrf-key
phabricator.mail-key
security.hmac-key
Test Plan: Artifically tweaked lock/hide settings on options, verified the UI respected them.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4472
Summary:
It's obvious that they're Phabricator related (why else would we provide
settings for them) and nothing else is prefaced.
Test Plan: Looked at /config/
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4471
Summary: Implement Diffusion MetaMTA options. Also make the fake '{{config.option}}' rule work, and use Remarkup to render summaries as well as descriptions.
Test Plan: Looked at Diffusion rules, edited some, looked at setup issues, verified '{{config.option}}' linked to the right option.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4466
Summary:
These are technically in MetaMTA right now, but I put them in Differential since I think it's probably a better primary category fit and having 120 options in MetaMTA won't do anyone any favors. (We can do some kind of cross-categorization or "related options" later if we get feedback that people are having trouble finding options like these which have multiple reasonable categories.)
Also improve the readability of displayed JSON literals with forward slashes.
Test Plan: Looked at options, edited a couple of them. Looked at JSON literal values, saw them rendered more readably.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4464
Summary:
Port PHPMailer options. Also:
- Don't show values on config lists if they're masked (this is mostly for passwords, to prevent them from being idly/accidentally disclosed).
- Don't show "default" icon -- just show an icon if the value has been customized. This makes it easier to pick out custom values.
Test Plan: Looked at / edited mailer values.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4441
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:
T2255 lists it as "???" and we don't have a "Misc" category and it seems silly
to make it for one option, so stick this in core for now.
Test Plan: Went to the setting page and saw the bool options.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4389
Test Plan: Quick view of each option in the web interface.
Reviewers: epriestley, btrahan, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4386
Summary:
Rather than throwing if we don't `setOptions()`, let's just default to `true`
and `false`.
Test Plan:
Removed a `setOptions()` call temporarily and saw options default to
`true` / `false`.
Reviewers: epriestley, btrahan, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4368
Summary: See discussion in D4355, this fixes reversed bool logic.
Test Plan:
- Quickly viewed in the web interface to make sure it didn't break anything.
- Saved `ldap.auth-enabled` with correct boolean value in the db.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4357
Test Plan: Looked at them in the web UI.
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4355
Test Plan: Looked at the setting and available options from the dropdown.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4354
Summary:
Adds the translations group as per T2255. Currently `translation.override` is
`wild` -- it should be changed to dict<string, string> when that exists.
Also fixes a small bug from D4326 which caused "class" types to not ever
validate.
Test Plan:
- Looked at the settings.
- Successfully saved a setting relating to classes.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4350
Summary: Bring these over. Also sort the group list.
Test Plan: Viewed config.
Reviewers: btrahan, codeblock, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4338
Summary:
- Ports MySQL settings to PHP.
- Removes "mysql.retries" -- this existed only because Magic Numbers Are Bad, but there is no concievable reason it should ever be set to anything other than 3.
- Introduced "Hidden" config, which isn't visible from the web (for SaaS, we'll just mark anything with secret keys as "hidden").
- Introduced "Masked" config, which will be masked in darkconsole once that gets updated.
- "Hidden" implies "Masked" and "Locked".
- Moved "storage.default-namespace" here -- it probably makes more sense than core; this was my bad in T2255.
- Put cancel button back for hidden/locked config.
- Introduce 'class' config type.
Test Plan: Viewed MySQL options. None are editable.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4326
Summary:
- Adds the rest of the group as per T2255.
- Adds a pht() around the `$developer_warning` in `PhabricatorStandardPageView`.
Test Plan:
- Viewed new config options.
- Triggered a fake warning to make sure I didn't break error callouts.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4333
Summary: Added all the "Security" group options listed in T2255.
Test Plan:
- Looked at all the options.
- Tested validation on `security.alternate-file-domain`
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4334
Summary: Refs T2255 and takes care of the "EXTENDING PHABRICATOR" group thereof.
Test Plan: Looked at each of the new options.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4325
Summary: See title.
Test Plan: Checked that the strings still rendered.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4327
Summary: This config section is weak (poorly documented) and inconsistent (keys with "_" instead of "-") but I'm going to keep punting on improving it until after T1536.
Test Plan: Loaded, examined LDAP config.
Reviewers: btrahan, codeblock
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4322
Summary:
- Move GC options into PHP.
- Remove the "run at" and "run for" options. The GC daemon doesn't actually do any table scans, is very gentle, and runs for like 3 seconds per day in any normal install. Just limit it to running once every 4 hours when it's caught up and call it a day.
Test Plan: Edited GC options.
Reviewers: btrahan, codeblock
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4321
Summary: Some config shouldn't reasonably be edited from the web interface because it immediately torpedoes the install if you make a mistake. Block edits to "locked" config.
Test Plan: Tried to edit locked config, got denied. Viewed locked config on edit and list screens.
Reviewers: codeblock, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4320
Summary:
Add validation for timezones, since date_default_timezone_set() returns a usable error code.
Note that we could also list all the timezones using timezone_identifiers_list(), but the list is enormous (many hundreds of entries) and impossible to use (~160 entries in "America" alone). I listed the likely US values as examples but left it as a string input text field.
Test Plan: Tried to save an invalid setting. Saved a valid setting.
Reviewers: codeblock, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4318
Summary:
Refs #2255 and completes the first group ("CORE") in @epriestley's comment
thereof.
Test Plan: Saw the new options appear in the list and save correctly.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4317
Summary: This is more or less a copy of the validation which lives in `webroot/index.php` right now, but I don't want to wipe that out just yet because there's no way for normal users to see this new validation.
Test Plan: Tried to set "phabricator.base-uri" to crazy nonsense, was harshly rebuffed.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4316
Summary:
- Allows us to implement setup warnings for edits which don't go through the web UI, e.g. "you edited a config file and set value X to something goofy".
- Allows us to implement more sophisticated validations, beyond basic type checks (e.g., "phabricator.base-uri" must be a URI).
- Fixes T358 (or, close enough -- fixes it for all options which have been migrated as per T2255.
Test Plan: Set "darkconsole.enabled" to "xyz" in my config, observed setup warning. Added fake validation, observed web UI edit error.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255, T358
Differential Revision: https://secure.phabricator.com/D4315
Summary: Use ApplicationTransactions in Config to create an edit history. Resolves T2256.
Test Plan: {F28477}
Reviewers: btrahan, codeblock
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2256
Differential Revision: https://secure.phabricator.com/D4314
Summary:
- When viewing a config list, show the current effective value.
- Add an icon showing default vs nondefault values.
Test Plan: {F28475}
Reviewers: btrahan, codeblock
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4313
Summary: Show the value for all loaded configuration sources.
Test Plan:
{F28469}
{F28470}
{F28471}
Reviewers: btrahan, codeblock
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4312
Summary: Show example config values to the user when available.
Test Plan:
{F28465}
{F28466}
Reviewers: btrahan, codeblock
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2221, T2255
Differential Revision: https://secure.phabricator.com/D4311
Summary: Also improve behavior for the "unknown config" warning.
Test Plan: Looked at configs, went through unknown config workflow.
Reviewers: btrahan, codeblock
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4310
Summary:
- Add a "developer" option group.
- Add an "access log" option group.
- Render the types "bool", "int" and "string" in a more tailored way.
- Add a config check for dead config. Right now this serves as a "TODO" list of things that need to be migrated.
Test Plan: Looked at config options, setup issues. Edited bool, int, string options.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4308
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: Ref T2255. Ref T2221. Lay the groundwork to move configuration into PHP, so we can show descriptions in the web UI, do typechecking, disable application options when an application is uninstalled, etc.
Test Plan:
{F28421}
{F28420}
{F28422}
Reviewers: codeblock, btrahan, vrana
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2221, T2255
Differential Revision: https://secure.phabricator.com/D4306
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
Summary:
* When we restored to the default value, we did, in fact delete the row from the
database, but then a few lines later down, we saved it again. This patch causes
the controller to return early on delete, like it was supposed to do to begin
with.
* When checking the user's input value for `null` (since PHP's JSON encoder will
return `null` on failure), check the value that the user gave, not the value
that we default to (which is often `null` anyway). Oops.
Test Plan:
* Saved an empty text field and saw the delete work properly and NOT get
re-added.
* Put `null` in the text field, and saved successfully.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4300
Summary:
As mentioned by @epriestley in an inline on D4290, we should show what happens
if the user leaves the box blank.
Test Plan: Went to edit a setting and saw the default below the text box.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, asherkin
Differential Revision: https://secure.phabricator.com/D4293
Summary:
This is somewhat clowny, particularly in how it handles JSON encode/decode, but
I've commented why I did things the way I did. The goal is to store minified JSON
but show pretty-printed JSON where possible, to the user editing it.
Test Plan:
* Went to /config/ and saw a list of keys from the `default` config.
* Clicked on one of them, submitted the default value successfully.
* Changed the value to invalid JSON and got a decent error.
* Changed the value to valid JSON and checked the DB to confirm it saved.
* Confirmed the DB values were minified.
* Confirmed the user-facing values were pretty-printed where they could be.
* Confirmed that PHIDs were getting assigned properly and that isDeleted
properly defaulted to false/0.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2246
Differential Revision: https://secure.phabricator.com/D4290
