Summary:
Ref T2015. Not directly related to Drydock, but I've wanted to do this for a bit.
Introduce a common base class for all the workflows in the scripts in `bin/*`. This slightly reduces code duplication by moving `isExecutable()` to the base, but also provides `getViewer()`. This is a little nicer than `PhabricatorUser::getOmnipotentUser()` and gives us a layer of indirection if we ever want to introduce more general viewer mechanisms in scripts.
Test Plan: Lint; ran some of the scripts.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D7838
Summary:
Fixes two issues:
- When rendering a task's details, we currently issue a policy-oblivious query. Instead, issue a policy-aware query.
- The formatting is a little bit weird, with the top half in a box and the bottom half with an older style. Make them consistent.
Test Plan: Looked at the detail pages for several tasks in queue.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7812
Summary: We currently have a lot of calls to `addCrumb(id(new PhabricatorCrumbView())->...)` which can be expressed much more simply with a convenience method. Nearly all crumbs are only textual.
Test Plan:
- This was mostly automated, then I cleaned up a few unusual sites manually.
- Bunch of grep / randomly clicking around.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: hach-que, aran
Differential Revision: https://secure.phabricator.com/D7787
Summary:
This came up recently in a discussion with @lifeihuang, and then tangentally with @hach-que. Make it easier for users to get a sense of whether they might need to add more daemons. Although we've improved the transparency of daemons, it's not easy for non-experts to determine at a glance how close to overflowing the queue is.
This number is approximate, but should be good enough for determining if your queue is more like 25% or 95% full.
If this goes over, say, 80%, it's probably a good idea to think about adding a couple of daemons. If it's under that, you should generally be fine.
Test Plan: {F88331}
Reviewers: btrahan, hach-que, lifeihuang
Reviewed By: btrahan
CC: hach-que, lifeihuang, aran, chad
Differential Revision: https://secure.phabricator.com/D7747
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.
This has several parts:
- For PolicyAware queries, provide an application class name method.
- If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
- For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.
Test Plan:
- Added a unit test to verify I got all the class names right.
- Browsed around, logged in/out as a normal user with public policies on and off.
- Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7367
Summary:
Ref T3675. Some of these listeners shouldn't do their thing if the viewer doesn't have access to an application (for example, users without access to Differential should not be able to "Edit Tasks"). Set the stage for that:
- Introduce `PhabricatorEventListener`, which has an application.
- Populate this for event listeners installed by applications.
- Rename the "PeopleMenu" listeners to "ActionMenu" listeners, which better describes their modern behavior.
This doesn't actually change any behaviors.
Test Plan: Viewed Maniphest, Differntial, People.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3675
Differential Revision: https://secure.phabricator.com/D7364
Summary:
Ref T603. This cleans up an existing callsite in the policy filter, and opens up some stuff in the future.
Some policy objects don't have real PHIDs:
PhabricatorTokenGiven
PhabricatorSavedQuery
PhabricatorNamedQuery
PhrequentUserTime
PhabricatorFlag
PhabricatorDaemonLog
PhabricatorConduitMethodCallLog
ConduitAPIMethod
PhabricatorChatLogEvent
PhabricatorChatLogChannel
Although it would be reasonable to add real PHIDs to some of these (like `ChatLogChannel`), it probably doesn't make much sense for others (`DaemonLog`, `MethodCallLog`). Just let them return `null`.
Also remove some duplicate `$id` and `$phid` properties. These are declared on `PhabricatorLiskDAO` and do not need to be redeclared.
Test Plan: Ran the `testEverythingImplemented` unit test, which verifies that all classes conform to the interface.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7306
Summary: This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.
Test Plan: Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7283
Summary:
Ref T603. Adds clarifying text which expands on policies and explains exceptions and rules. The goal is to provide an easy way for users to learn about special policy rules, like "task owners can always see a task".
This presentation might be a little aggressive. That's probably OK as we introduce policies, but something a little more tempered might be better down the road.
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7150
Summary: The adds the ability to set 'properties' such as state, privacy, due date to the header of objects.
Test Plan: Implemented in Paste, Pholio. Tested various states.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7016
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:
^\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:
We currently check if daemons are running using the filesystem and process list. These checks reach the wrong result for a lot of users because their webservers can't read the filesystem or process list. They also reach the wrong result for daemons running on other machines.
Instead, query the active daemon list to see if daemons are running. This should be significantly more reliable.
(We didn't do this before because the running daemon list mechanism didn't exist when the check was written, and at the time it was more complex than doing a simple filesystem/process list thing.)
Test Plan: Viewed `/repositories/` with and without daemons running, saw appropriate warning or lack of warning.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6722
Summary: Fixes T3680. One description was wrong, and clean up some of the other stuff.
Test Plan: Ran `phd`.
Reviewers: btrahan, Korvin
Reviewed By: Korvin
CC: aran, jifriedman, Korvin
Maniphest Tasks: T3680
Differential Revision: https://secure.phabricator.com/D6683
Summary: Fixes T3557. One thing which made T3557 kind of a mess was the lack of information about progress through temporary failures. Add a column which records a task's last failure time, and surface it in the console.
Test Plan: {F51277}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6550
Summary: Ref T3557. Make it easier to access full daemon logs from the CLI.
Test Plan: {F51265}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6547
Summary:
Ref T3557. We summarize long messages, but don't let you see the entire message. This is occasionally inconvenient, and I'm planning to add more prefix junk to some messages for T2569.
Provide a link you can click to see the full message.
This isn't javascripted because a ton of these can make the page ridiculously enormous and it seems unlikely you'd care much about all of them.
Test Plan: {F51261} {F51262}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6546
Summary:
Ref T3557. This stuff does a bunch of nonsense in the View right now. Instead, do it in a real Query class.
Fixes a long-standing bug which prevented "all daemons" from showing more than 3 days' worth of data.
Test Plan: Viewed `/daemon/`, viewed "All Daemons".
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6544
Summary: Ref T3557. I had misremembered this value; show it in the UI. Depends on D6542.
Test Plan: Viewed UI for a waiting daemon. Saw it restart after 5s.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6543
Summary:
Ref T3557. The major goals here are:
- Modernize use of UI elements.
- Present daemon status with more clarity. Particularly, the "Waiting" status is called out and explained in detail.
Test Plan:
{F51247}
{F51248}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6541
Summary:
Ref T3557. Slightly improves display of daemons:
- Makes status more clear (through colors, explanatory text, icons, and explicit descriptions instead of symbols).
- Particularly, the "wait" status is now communicated as a normal status ("waiting a moment...") with a calm blue color.
- Uses modern responsive elements.
Test Plan: {F51232}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6539
Summary: Fixes T1670. These are now unused.
Test Plan: Grepped for callsites.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1670
Differential Revision: https://secure.phabricator.com/D6538
Summary: Ref T1670. Use events and direct database writes instead of Conduit. Deprecate the Conduit methods.
Test Plan: Ran daemons, used the console to review daemon event logs.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1670
Differential Revision: https://secure.phabricator.com/D6536
Summary:
Ref T1670. Prepare for the overseers to talk directly to the database instead of using Conduit. See T1670 for discussion.
This shouldn't impact anything, except it has a very small chance of destabilizing the overseers.
Test Plan:
Ran `phd launch`, `phd debug`, `phd start`.
Ran with `--trace-memory` and verified elevated but mostly steady memory usage (8MB / overseer). This climbed by 0.05KB / sec (4MB / day) but the source of the leaks seems to be the cURL calls we're making over Conduit so this will actually fix that. Disabling `--conduit-uri` reported steady memory usage. I wasn't able to identify anything leaking within code we control. This may be something like a dynamic but capped buffer in cURL, since we haven't seen any issues in the wild.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1670
Differential Revision: https://secure.phabricator.com/D6534
Summary:
Ref T1670. Mostly, use PhutilArgumentParser. This breaks up the mismash of functional stuff and PhabriatorDaemonControl into proper argumentparser Workflows.
There are no functional changes, except that I removed the "pingConduit()" call prior to starting daemons, because I intend to remove all Conduit integration.
Test Plan:
- Ran `phd list`.
- Ran `phd status` (running daemons).
- Ran `phd status` (no running daemons).
- Ran `phd stop <pid>` (dead task).
- Ran `phd stop <pid>` (live task).
- Ran `phd stop zebra` (invalid PID).
- Ran `phd stop 1` (bad PID).
- Ran `phd stop`.
- Ran `phd debug zebra` (no match).
- Ran `phd debug e` (ambiguous).
- Ran `phd debug task`.
- Ran `phd launch task`.
- Ran `phd launch 0 task` (invalid arg).
- Ran `phd launch 2 task`.
- Ran `phd help`.
- Ran `phd help list`.
- Ran `phd start`.
- Ran `phd restart`.
- Looked at Repositories (daemon running).
- Looked at Repositories (daemon not running).
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1670
Differential Revision: https://secure.phabricator.com/D6490
Summary:
Fixes T2691. Now, all PhabricatorActionListViews in the codebase setObjectHref to $request->getRequestURI. This value is passed over to PhabricatorActionItems right before they are rendered. If a PhabricatorActionItem is a workflow and there is no user OR the user is logged out, we used this objectURI to construct a log in URI.
Potentially added some undesirable behavior to aggressively setUser (and later setObjectURI) from within the List on Actions... This should be okay-ish unless there was a vision of actions having different user objects associated with them. I think this is a safe assumption.
Test Plan: played around with a mock all logged out (Ref T2652) and it worked!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2691
Differential Revision: https://secure.phabricator.com/D6416
Summary:
Ref T2852. Asana sync tasks currently have a standard retry/backoff schedule, but the defaults are quite aggressive (retry every 60s forever). Instead, retry at increasing intervals and stop retrying after a few tries.
- Retry at intervals and stop retrying after a few iterations.
- Modernize some interfaces.
- Add better information about retry behaviors to the web UI.
Test Plan: {F49194}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6381
Summary:
this suppresses displaying entries that have not been in the running
state for more than 3 days.
note: We should probably kill off these dead rows similar
to the code at the top of the loop, but without checking to see if we're
the same host and the item is still live, or have some other cruft removal
job to effect the same.
Test Plan: load /daemon/ and rejoice that there are fewer crufty rows.
Reviewers: vrana, nh
Reviewed By: nh
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D5299
Summary: Scan Daemons app for pht and fix a form or two.
Test Plan: Used Daemons app as much as I could, check all caps language.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5133
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:
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:
Done by searching for `AphrontDialogView` and then `appendChild()`.
Also added some `pht()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4882
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:
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: 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:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, '...')
Then searched for `&` and `<` in the output and replaced them.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4503
Summary:
Makes various fixes to the Daemon console UI:
- Removes timeline, timeline cursors, and timeline-related controllers. This abstraction is all but dead and just waiting on an eventual cleanup effort with Facebook (see T2003). There's no need to inspect or debug it anymore.
- Instead of showing the 15 most recently launched non-exited daemons, show all the running daemons. With the old rule, "dead" daemons tended to build up at the bottom of the list -- e.g., secure.phabricator.com shows the 7 active daemons, then 8 dead daemons from as far back as Aug 2012. Showing running daemons is far more useful.
- Simplify the two "Running Daemons" and "All Daemons" subviews into one "All Daemons" subview. The main console now has "running daemons", effectively.
- Create a "Recently completed tasks" view, which shows how many tasks of each task class have completed in the last 15 minutes and how long they took on average. Understanding how quickly tasks are completing is one of the most common uses of the daemon console, and it's currently almost useless for that. Now that we archive tasks, we can show this information in an easily digestable form.
- Partially modernize all of the remaining views.
Test Plan: Looked at daemon console.
Reviewers: btrahan, chad, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2372, T2003
Differential Revision: https://secure.phabricator.com/D4573
Summary:
No fancy-pants smarty stuff yet, but merges /applications/ and the awful application buttons into the dark navigation.
Hover state is maybe a little weird.
Test Plan: {F29324}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran, btrahan, codeblock
Differential Revision: https://secure.phabricator.com/D4431
Summary: This removes all calls to addSpacer and the method. We were applying it inconsistently and it was causing spacing issues with redesigning the sidenav. My feeling is we can recreate the space in CSS if the design dictates, which would apply it consistently.
Test Plan: Go to Applications, click on every application.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4420
Summary: This is used in every other view.
Test Plan: Browsed around.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4248
Summary:
Load the data for daemon worker tasks when viewing them, and present
the information in a useful way. This defaults to printing the json data,
but for some classes of worker it will also link to the corresponding
object, to make debugging problems with workers easier.
Test Plan:
load /daemon/task/NNN for a CommitParserWorker and a MetaMTAWorker, and
see the addition of a data field with useful content and link.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4226
