Summary: Fixes a few issues. The author of the commit is more prominent / not cut off. Auditors is in a more consistent location. More space is available for reasons. Commits by themselves look much less janky. Only downside is actual Audits are now 3 lines vs. 2, but the extra space is used well.
Test Plan:
Test list of audits and commits.
{F309237}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11817
Summary: Third times the charm?
Test Plan: pray
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11816
Summary: Fixes T7319. These need a "%s" for the count where they had a "%d"
Test Plan: plan in D11812 is no longer a lie! (watcher added / removed strings render correctly)
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7319
Differential Revision: https://secure.phabricator.com/D11813
Summary: If we don't have a state in PHUIActionPanelView, don't set the extra padding to display it.
Test Plan: Review in UIExamples.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11814
Summary: Fixes T7319. ...except I can't get this working in my sandbox? Changes to the translation file don't seem to show up. TEST PLAN IS A LIE
Test Plan: became a watcher, un became a watcher - saw sensical translated strings
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7319
Differential Revision: https://secure.phabricator.com/D11812
Summary: Fixes T7299. Also re-direct the user to the initial request uri if the signature was required.
Test Plan: made a signature required legalpad doc. visit the instance at a specific uri, signed the document, and ended up at that specific uri
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7299
Differential Revision: https://secure.phabricator.com/D11809
Summary:
Fixes the long uptake we saw on `meta.phacility.com`. I regressed this in D11795.
We make three calls to this method, but only one actually consumes the messages. The other two are just checking to see if there are any messages.
Only move the cursor up if we're actually going to process the messages.
Test Plan: Sort of tricky to test convincingly since it's inherently race-prone, but ran `debug pulllocal` and pushed update messages and saw it pick them up.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11808
Summary:
I am hitting this error when generating Diviner documentation:
```
COMMAND
'/usr/src/phabricator/bin/diviner' atomize --ugly --book $SOME_BOOK --atomizer 'DivinerPHPAtomizer' -- $SOME_PATHS
STDOUT
(empty)
STDERR
[2015-02-18 23:05:01] EXCEPTION: (RuntimeException) Undefined variable: type at [<phutil>/src/error/PhutilErrorHandler.php:210]
#0 PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phabricator>/src/applications/diviner/atomizer/DivinerPHPAtomizer.php:315]
#1 DivinerPHPAtomizer::parseReturnType(DivinerAtom, XHPASTNode) called at [<phabricator>/src/applications/diviner/atomizer/DivinerPHPAtomizer.php:116]
#2 DivinerPHPAtomizer::executeAtomize(string, string) called at [<phabricator>/src/applications/diviner/atomizer/DivinerAtomizer.php:23]
#3 DivinerAtomizer::atomize(string, string, array) called at [<phabricator>/src/applications/diviner/workflow/DivinerAtomizeWorkflow.php:109]
#4 DivinerAtomizeWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:396]
#5 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:292]
#6 PhutilArgument... (87 more bytes) ... at [<phutil>/src/future/exec/ExecFuture.php:416]
#0 ExecFuture::resolvex(NULL) called at [<phutil>/src/future/exec/ExecFuture.php:438]
#1 ExecFuture::resolveJSON() called at [<phabricator>/src/applications/diviner/workflow/DivinerGenerateWorkflow.php:349]
#2 DivinerGenerateWorkflow::resolveAtomizerFutures(array, array) called at [<phabricator>/src/applications/diviner/workflow/DivinerGenerateWorkflow.php:209]
#3 DivinerGenerateWorkflow::buildAtomCache() called at [<phabricator>/src/applications/diviner/workflow/DivinerGenerateWorkflow.php:170]
#4 DivinerGenerateWorkflow::generateBook(string, PhutilArgumentParser) called at [<phabricator>/src/applications/diviner/workflow/DivinerGenerateWorkflow.php:74]
#5 DivinerGenerateWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:396]
#6 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:292]
#7 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/diviner/diviner.php:21]
```
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11807
Summary: Use modern components, pht
Test Plan: I have no data locally, expect @epriestley to commandeer
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11805
Summary: Ref T7202.
Test Plan: Visited edit subscription page and it worked. Clicked edit link from subscription view page and got to the right place.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7202
Differential Revision: https://secure.phabricator.com/D11803
Summary: This change wraps the icon inline with the text, so smaller width icons have equal spacing between the border and text.
Test Plan:
review a number of different tag with icons, also UIExamples.
{F309048}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11802
Summary: Fixes T7317, allows public to be set on this list controller.
Test Plan: Tested a list of subscribers on a logged in and logged out Diff.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7317
Differential Revision: https://secure.phabricator.com/D11801
Summary:
Ref T6941. In the cluster (and in other reasonable setups) we've separated SSH load balancers from HTTP load balancers.
In particular, ELBs will not let you load balance port 22, so this is likely a reasonable/common issue in larger clusters in AWS.
Allow users to specify an alternate host for SSH traffic.
Test Plan: Set host to someting different, saw it reflected in UI.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6941
Differential Revision: https://secure.phabricator.com/D11800
Summary: Pretty basic, but you can now search panels by type (query, text, tab).
Test Plan: Searched for a few different types of panels, results look correct
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11782
Summary: Sets everything up flush, I think this is the only indented block left.
Test Plan:
Write some code.
{F289438}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley, #design
Differential Revision: https://secure.phabricator.com/D11721
Summary: This port is always HTTP, so use HTTP even if users have set the URI to "https".
Test Plan: Launched server and hit status page, status good.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11799
Summary: Fixes T7106. If you have bad credentials AND you've pushed an "update this repository" message into the queue, the loop above this level ends up resetting the timer every time we go through it, so the daemon spins in a loop failing forever.
Test Plan:
- Created a repo with bad credentials.
- Clicekd "updated now" to queue an update message.
- Saw daemon run in a loop.
- Applied patch, no loop.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7106
Differential Revision: https://secure.phabricator.com/D11795
Summary: Fixes T7284. We were initialized the project name to the empty string, which was making things work like a rename, including automagically adding the old slug.
Test Plan: made a project and no more "empty" tag being made. also don't have that bad transaction story anymore.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7284
Differential Revision: https://secure.phabricator.com/D11794
Summary: Even if you --force, we can't kill PID 0. This sends the process itself the signal, and terminates it.
Test Plan: See D11786.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11787
Summary: Fixes T7263. Last bit there was to upgrade this dialogue to let users know they are letting their primary email address be exposed in these flows. Depends on D11791, D11792, at least in terms of being accurate to the user as the code ended up strangely decoupled.
Test Plan: wordsmithin'
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7263
Differential Revision: https://secure.phabricator.com/D11793
Summary: Ref T7263. We need this in the oauth case and otherwise it makes sense to include.
Test Plan: used the conduit console and saw my email address included in the results!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7263
Differential Revision: https://secure.phabricator.com/D11791
Summary: Ref T7123. Turns out that we might throw ConduitClientException now in proxied scenarios. For all but one callsite remove the try / catch bit and don't issue the call for SVN. For the remaining callsite, also don't issue the call for SVN but keep in the exception logic since its renders a pretty error message in the non-proxied case?
Test Plan: played around with diffusion and things looked okay.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7123
Differential Revision: https://secure.phabricator.com/D11789
Summary: Fixes T7256.
Test Plan: Looked at rXPRF0a7a5f69f5d7 in a local instance. things looked great both pre and post patch.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7256
Differential Revision: https://secure.phabricator.com/D11790
Summary: Fixes T7294. This lets legalpad store other documents that don't need signatures but conceptually belong in legalpad.
Test Plan: made a document with signature type "no one" and it saved. viewed the document and noted no signing UI was present.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7294
Differential Revision: https://secure.phabricator.com/D11788
Summary:
In the cluster, the box has a ton of stuff that "looks like a daemon" beacuse it is some other instance's daemon.
Stop `phd restart` from complaining about this if given a "--gently" flag, which is like the opposite of "--force".
(I'll make it `stop --force` at the beginning of a whole-box restart to kill stragglers.)
Test Plan: Ran `bin/phd restart --gently`, etc.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11784
Summary:
Fixes T7291. There are a class of spam/annoyance attacks here that we should be more strict about preventing, since you can add an individual's address as a mailing list.
This application is likely on the way out so I didn't bother trying to do per-object policies.
Test Plan: Set policy restrictively and could no longer create or edit mailing lists.
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7291
Differential Revision: https://secure.phabricator.com/D11783
Summary: Fixes T7295. Humbling debugging experience but I got it.
Test Plan: saved a legalpad doc without edits over and over and saw no "requires signature" transaction. toggled "requires signature", saved, and saw the transaction.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7295
Differential Revision: https://secure.phabricator.com/D11785
Summary: Fixes T7252. The UI is slightly different than in Maniphest - in Maniphest the error shows up at the bottom and here it shows up the top - but I think the UI here makes sense as you see the error right away on the newly returned dialogue?
Test Plan: set "created after" to "assdaasds" and got an error back. set filter to something that should work and it worked
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7252
Differential Revision: https://secure.phabricator.com/D11760
Summary:
Fixes T7130. Fixes T7041. Fixes T7012.
Major change here is partitioning clients. In the Phacility cluster, being able to get a huge pile of instances on a single server -- without needing to run a process per instance -- is desirable.
To accomplish this, just bucket clients by the path they connect with. This will let us set client URIs to `/instancename/` and then route connections to a small set of servers. This degrades cleanly in the common case and has no effect on installs which don't do instancing.
Also fix two unrelated issues:
- Fix the timeouts, which were incorrectly initializing in `open()` (which is called during reconnect, causing them to reset every time). Instead, initialize in the constructor. Cap timeout at 5 minutes.
- Probably fix subscriptions, which were using a property with an object definition. Since this is by-ref, all concrete instances of the object share the same property, so all users would be subscribed to everything. Probably.
Test Plan:
- Hit notification status page, saw version bump and instance/path name.
- Saw instance/path name in client and server logs.
- Stopped server, saw reconnects after 2, 4, 16, ... seconds.
- Sent test notification; received test notification.
- Didn't explicitly test the subscription thing but it should be obvious by looking at `/notification/status/` shortly after a push.
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7041, T7012, T7130
Differential Revision: https://secure.phabricator.com/D11769
Summary: Fixes T7275. This makes the error stuff a little more consistent with other modern UIs.
Test Plan: {F307286}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7275
Differential Revision: https://secure.phabricator.com/D11778
Summary:
Right now, taskmasters on empty queues sleep for 30 seconds. With a default setup (4 taskmasters), this averages out to 7.5 seconds between the time you do anything that queues something and the time that the taskmasters start work on it.
On instances, which currently launch a smaller number of taskmasters, this wait is even longer.
Instead, sleep for the number of seconds that there are taskmasters, with a random offset. This makes the average wait to start a task from an empty queue 1 second, and the average maximum load of an empty queue also one query per second.
On loaded instances this doesn't matter, but this should dramatically improve behavior for less-loaded instances without any real tradeoffs.
Test Plan: Started several taskmasters, saw them jitter out of sync and then use short sleeps to give an empty queue about a 1s delay.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11772
Summary:
Fixes T7285. If the user tries to view a subscription they don't have permission to view, we may filter all the subscriptions out, then still try to load related data. This can fatal because it's invalid.
Instead, bail if we filtered everything.
Test Plan: Subscritption detail page of another user's subscription is now 404 instead of fatal.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7285
Differential Revision: https://secure.phabricator.com/D11780
Summary:
At least one user wanted to type more text here, and it seems reasonable that administrators may want to write a couple of paragraphs.
I didn't make this short for any particular reason, I just wasn't sure what the workflow would look like as I was building it.
Test Plan: Loaded page, saw normal height text area.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11779
Summary:
This mentions "like GitHub", but we purged all the issues and no longer accept them.
Generally, feature requests should be coming to the upstream only nowadays.
Also, don't overpromise IRC.
Test Plan: Read documentation.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11777
Summary: Removes odd margin. Also removes min-height on content.
Test Plan: Test a fresh Phriction, and a deleted page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11775
Summary: We use font-smoothing to make better looking buttons, but it makes text buttons look bad.
Test Plan: Test a button in an actionlist, font looks same as other action list items.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11776
Summary: Fixes T7273. This shows a better title (like "No Such Instance") instead of a generic one ("Unhandled Exception") when the user hits an AphrontUsageException.
Test Plan: Visited a nonexistent instance, got a nice title.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7273
Differential Revision: https://secure.phabricator.com/D11771
Summary:
The correct conf variable is notification.pidfile, not notification.pid.
See also:
* src/applications/config/option/PhabricatorNotificationConfigOptions.php:58
* src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php:34
Test Plan:
Grepped the whole source of Phabricator for occurences of `notification.pid`,
but all matches pointed to `notification.pidfile` instead.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11768
Summary: Ref T7150. Show some basic information instead of nothing.
Test Plan: Used these in Instances.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7150
Differential Revision: https://secure.phabricator.com/D11767
Summary:
If your install isn't public, users can't see the Auth or People applications while logged out, so we can't load their invites.
Allow this query to go through no matter who the viewing user is.
Test Plan: Invite flow on `admin.phacility.com` now works better.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11765
Summary:
Ref T7185. These settings shouldn't be unlocked anywhere. Specifically:
- `reply-handler`: These are on the way out.
- `reply-handler-domain`: Also hopefully on the way out; locked because a compromised administrator account can redirect replies.
- `phabricator.cookie-prefix`: Not dangerous per se, but an admin could have a hard time fixing this if they changed it by accident since their session would become invalid immediately.
Test Plan: Browsed Config.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7185
Differential Revision: https://secure.phabricator.com/D11764
Summary:
Ref T7185. We currently have "locked", "masked", and "hidden" config.
However, "masked" does not really do anything. It was intended to mask values in DarkConsole, but Config got built out instead and "hidden" is strictly better in modern usage and protects against compromised administrator accounts. "hidden" implies "locked", so it's now strictly more powerful than just locked.
Remove "masked" and upgrade all "masked" config to "hidden". In particular, this hides some API keys and secret keys much more aggressively in Config, which is desirable.
Test Plan: Browsed things like S3 API keys in config and could no longer see them.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7185
Differential Revision: https://secure.phabricator.com/D11763
Summary: Pretty sure this was me derping, not trying to make a joke.
Test Plan: New text makes sense.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11762
Summary: Fixes T7159.
Test Plan:
Created a legalpad document that needed a signature and I was required to sign it no matter what page I hit. Signed it and things worked! Added a new legalpad document and I had to sign again!
Ran unit tests and they passed!
Logged out as a user who was roadblocked into signing a bunch of stuff and it worked!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7159
Differential Revision: https://secure.phabricator.com/D11759
Summary: Fixes T7088. Mainly this updates the documentation but I also snuck in tweaking how the domain reply handler is built. This does two main things -- makes the behavior consistent as some applications who didn't override this behavior would send out emails with reply tos AND makes it easier for us to deprecate the custom domain thing on a per application basis, which is just silly. On that note, the main documentation doesn't get into how this can be overridden, though I left in that mini blurb on the config setting itself. We could deprecate this harder and LOCK things if you want as well.
Test Plan: read docs, looked good. reasoned through re-factor
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7088
Differential Revision: https://secure.phabricator.com/D11725
