Summary:
If a page generates warnings or errors, you only get a little red dot in
DarkConsole which is hard to see. DarkConsole is also fairly big and there are
plenty of reasons not to leave it open all the time.
Instead, unconditionally show a big message to developers if there are errors or
warnings.
We could make this more sophisticated eventually, but the value is just that you
see it.
Test Plan: Browsed pages with and without warnings, got the right banner state.
Reviewers: nh, btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan
Maniphest Tasks: T734
Differential Revision: https://secure.phabricator.com/D1307
Summary: After D1281, this has no callsites. I don't see us wanting to go back
to it.
Test Plan: Grepped for symbol name, no hits.
Reviewers: btrahan, jungejason
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: https://secure.phabricator.com/D1282
Summary:
- Old page was useless and dumb.
- New page looks a little less bad, functions a little less poorly.
- Still lots of work to be done.
Test Plan:
- Viewed a project.
- Clicked all the links on the left nav.
- Here is a screenshot:
https://secure.phabricator.com/file/view/PHID-FILE-4buzquotb3fo4dhlicrw/
Reviewers: btrahan, jungejason
Reviewed By: jungejason
CC: aran, jungejason
Maniphest Tasks: T681
Differential Revision: 1246
Summary:
add basic auditing functionalities. For the related commits for a
package, we detect the following conditions which might be suspicious to the
owners of the package:
* no revision specified
* revision not found
* author not match
* reviewedby not match
* owners not involved
* commit author not recognized
The owners of the package can change the status of the audit entries by
accepting it or specify concern.
The owner can turn on/off the auditing for a package.
Test Plan:
* verified that non-owner cannot see the details of the audit and cannot modify
it
* verified that all the audit reasons can be detected
* tested dropdown filtering and package search
* verified really normal change not detected
* verified accept/concern a commit
* tested enable/disable a package for auditing
* verified one audit applies to all <commit, packages> to the packages the
auditor owns
* verified that re-parsing a commit won't have effect if there exists a
relationship for <commit, package> already
Reviewers: epriestley, nh
Reviewed By: epriestley
CC: aran, benmathews, btrahan, mpodobnik, prithvi, TomL, epriestley
Differential Revision: 1242
Summary:
- Allow more than the 100 most recent projects to be viewed.
- Provide some useful filters.
- Default the view to your projects, not all projects.
- Put query logic in a query object.
- Put filter view logic in a view object. We can port more stuff to it later.
Test Plan: Looked at active/owned/all projects. Set page size to 5 and paged
through projects.
Reviewers: btrahan, jungejason, zeeg
Reviewed By: btrahan
CC: aran, btrahan
Differential Revision: 1227
Summary: make the change, kill the function. be sure to get a good $user or
$viewer variable
Test Plan:
for each controller or view, look at it in the ui. change timezone, refresh ui
and note change. i did not test the OAuthSettingsPanelController; not sure how
to get to that badboy and i got a bit lazy
Maniphest Tasks: T222
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, btrahan, epriestley
Maniphest Tasks: T222
Differential Revision: 1166
Summary: This is sort of silly but maybe useful? The real problem is that there
are like 500k conduit call logs and the real solution to that is better
filtering options, but this seems sort of okay.
Test Plan: Used "[" and "]" to switch between pages on the conduit call log.
Reviewers: btrahan, jungejason, nh, aran
Reviewed By: nh
CC: aran, nh, epriestley
Differential Revision: 1145
Summary: Provide a dirt-simple working example of client-side templating and
reactive programming.
Test Plan: Load the examples
Reviewers: epriestley, mroch, tomo
Reviewed By: epriestley
CC: ide, schrockn, aran, rzadorozny, epriestley
Differential Revision: 908
Summary:
Add possibility for not logged in users to browse and see Differential
revisions.
Test Plan:
Set 'differential.anonymous-access' config option to true, log out, you should
be able to browse Differential without logging back in.
Reviewers: epriestley, jungejason
Reviewed By: epriestley
CC: aran, epriestley, mareksapota
Differential Revision: 1044
Summary:
- We throw on a missing date right now, in the DateTime constructor. This can
happen in reasonable cases and this is display code, so handle it more
gracefully (see T520).
- This stuff is a little slow and we sometimes render many hundreds of dates
per page. I've been seeing it in profiles on and off. Memoize timezones to
improve performance.
- Some minor code duplication that would have become less-minor with the
constructor change, consolidate the logic.
- Add some unit tests and a little documentation.
Test Plan:
- Ran unit tests.
- Profiled 1,000 calls to phabricator_datetime(), cost dropped from ~49ms to
~19ms with addition of memoization. This is still slower than I'd like but I
don't think there's an easy way to squeeze it down further.
Reviewers: ajtrichards, jungejason, nh, tuomaspelkonen, aran
Reviewed By: ajtrichards
CC: aran, ajtrichards, epriestley
Differential Revision: 966
datasources
Summary:
The open source Phabricator has like 3,500 user accounts now and it takes a
while to pull/render them. Add an option to switch to ondemand for large
installs.
I'll follow up with a patch at some point to address a couple of name things:
- Denormalize last names into a keyed column (although this evidences some
bias toward the western world).
- Force all usernames to lowercase (sorry Girish, Makinde).
Also this patch is so clean it's crazy.
Didn't bother with other object types for now, I'm planning to dedicate a few
days to Projects at some point and I'll flesh out some auxiliary features like
this when I do that.
Test Plan: Switched to ondemand, verified data was queried dynamically. Switched
back, verified data was preloaded.
Reviewers: jungejason, nh, tuomaspelkonen, aran
Reviewed By: nh
CC: aran, epriestley, nh
Differential Revision: 923
Summary: Previously, this code accidentally did not use the best URI. Instead,
use the best URI. It's the best, obviously.
Test Plan: Uploaded a binary file and then clicked the preview.
Reviewers: hunterbridges, jungejason, nh, tuomaspelkonen, aran
Reviewed By: tuomaspelkonen
CC: aran, tuomaspelkonen
Differential Revision: 905
Summary:
We don't currently validate CSRF tokens on this workflow. This allows an
attacker to upload arbitrary files on the user's behalf. Although I believe the
tight list of servable mime-types means that's more or less the end of the
attack, this is still a vulnerability.
In the long term, the right solution is probably to pass CSRF tokens on all Ajax
requests in an HTTP header (or just a GET param) or something like that.
However, this endpoint is unique and this is the quickest and most direct way to
close the hole.
Test Plan:
- Drop-uploaded files to Files, Maniphest, Phriction and Differential.
- Modified CSRF vaidator to use __csrf__.'x' and verified uploads and form
submissions don't work.
Reviewers: andrewjcg, aran, jungejason, tuomaspelkonen, erling
Commenters: andrewjcg, pedram
CC: aran, epriestley, andrewjcg, pedram
Differential Revision: 758
Summary:
Not totally sure I'm in love with this but I think it's somewhat non-terrible,
despite the lack of lens flare.
Also made "Cancel" take you back to the task if you got to "Create" from "Create
Another Task".
Test Plan:
- Style:
https://secure.phabricator.com/file/view/PHID-FILE-ad37d3c1f3b2c7a7a7d1/
- Hit "Cancel" from "Create Another", got sent back to task.
- Hit "Cancel" from normal create, got sent back to list.
- Tried to save an invalid task after making changes to CC/Projects, changes
were preserved.
Reviewed By: codeblock
Reviewers: hunterbridges, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock
Differential Revision: 736
Summary:
This gets all the major pieces working. Allows you to drag-and-drop files in
Differential and Phriction, and embed files in remarkup with {Fxxx} references.
See also task.
I'm explicitly not documenting this yet since it's still pretty rough.
Test Plan: Dragged and dropped stuff into Differential and Phriction.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, tomo
Commenters: tomo
CC: aran, tomo, jungejason
Differential Revision: 674
Summary:
See T372. Always render commit messages on one display line, so the table
doesn't jump around as they AJAX in on browse views.
The goal here is to have the cell choose a size naturally and for its content to
render with "overflow: hidden" if the natural size isn't large enough to contain
the content. "white-space: pre" or "white-space: nowrap" would prevent wrapping
but potentially make the table exceed the display width when a better behavior
is to hide some of the commit message.
Also use utf8-aware shortening, now that we have a function for it.
Casting a wide net in case anyone has a better way to do the CSS here. It's kind
of nasty that we have to use so many DOM nodes.
Test Plan:
- Resized window while viewing browse and history views in Safari, Chrome and
Firefox. Table exhibited described behavior.
- Verified summaries render sensibly and are properly truncated to 100
characters.
Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen, tomo, mroch, cpojer
CC: aran, epriestley
Differential Revision: 750
Summary:
We currently cycle CSRF tokens every hour and check for the last two valid ones.
This means that a form could go stale in as little as an hour, and is certainly
stale after two.
When a stale form is submitted, you basically get a terrible heisen-state where
some of your data might persist if you're lucky but more likely it all just
vanishes. The .js file below outlines some more details.
This is a pretty terrible UX and we don't need to be as conservative about CSRF
validation as we're being. Remedy this problem by:
- Accepting the last 6 CSRF tokens instead of the last 1 (i.e., pages are
valid for at least 6 hours, and for as long as 7).
- Using JS to refresh the CSRF token every 55 minutes (i.e., pages connected
to the internet are valid indefinitely).
- Showing the user an explicit message about what went wrong when CSRF
validation fails so the experience is less bewildering.
They should now only be able to submit with a bad CSRF token if:
- They load a page, disconnect from the internet for 7 hours, reconnect, and
submit the form within 55 minutes; or
- They are actually the victim of a CSRF attack.
We could eventually fix the first one by tracking reconnects, which might be
"free" once the notification server gets built. It will probably never be an
issue in practice.
Test Plan:
- Reduced CSRF cycle frequency to 2 seconds, submitted a form after 15
seconds, got the CSRF exception.
- Reduced csrf-refresh cycle frequency to 3 seconds, submitted a form after 15
seconds, got a clean form post.
- Added debugging code the the csrf refresh to make sure it was doing sensible
things (pulling different tokens, finding all the inputs).
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley
Differential Revision: 660
Summary:
- Services: Show summary panel of total service call costs and relative page weight.
- Services: Add "Analyze Query Plans" button, which issues EXPLAIN for each query and flags problems.
- XHPRof: iframe the profile.
Test Plan: Used the new query plan analysis to find missing keys causing table scans, see D627.
Reviewers: jungejason, tuomaspelkonen, aran
CC:
Differential Revision: 628
Summary:
Replace some more date() calls with locale-aware calls.
Also, at least on my system, the DateTimeZone / DateTime stuff didn't actually
work and always rendered in UTC. Fixed that.
Test Plan:
Viewed daemon console, differential revisions, files, and maniphest timestamps
in multiple timezones.
Reviewed By: toulouse
Reviewers: toulouse, fratrik, jungejason, aran, tuomaspelkonen
CC: aran, toulouse
Differential Revision: 530
Summary: Added some change on the project's list view, to show information about
active tasks, population, etc. Also modified the "profile view", and added a class "PhabricatorProfileView" to render the profile, both on projects and users.
Test Plan: play around the project directory :)
Reviewers: epriestley ericfrenkiel
CC:
Differential Revision: 477
Summary:
Allows user-configurable timezones. Adds a preference panel, and migrates to the
new date rendering in easily-modified areas of the code. ***In progress***.
Test Plan:
Check database to make sure the field is being changed when the settings are
changed; check affected views to see how they render times.
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley, toulouse
Differential Revision: 475
Summary:
Allow duplicate tasks to be selected and merged in Maniphest.
I didn't create a separate transaction type for this because that implies a
bunch of really complicated rules which I don't want to sort out right now
(e.g., do we need to do cycle detection for merges? If so, what do we do when we
detect a cycle?) since I think it's unnecessary to get right for the initial
implementation (my Tasks merge implementation was similar to this and worked
quite well) and if/when we eventually need the metadata to be available in a
computer-readable form that need should inform the implementation.
Plenty of room for improvement here, of course.
Test Plan:
Merged duplicate tasks, tried to perform invalid merge operations (e.g., merge a
task into itself).
Tested existing attach workflows (task -> revision, revision -> task).
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: anjali, aran
Differential Revision: 459
Summary:
A few tweaks to hsb's Countdown implementation:
- Allow the page to be rendered "chromeless", suitable for display on one of
the dozens of monitors everyone has laying around.
- Show title of countdown in deletion dialog.
- When creating a new countdown default to time(), not Dec 31, 1969.
- Add extra "/" after editing to avoid needless redirect.
- Tweak some page titles.
- Show countdown author in list view.
- Highlight tab in list view.
- Tweak menu copy.
- Link countdown title in list view, separate buttons into different columns
so they pick up padding.
Test Plan:
Created, edited and deleted a timer. Viewed a timer and toggled chrome mode.
Viewed timer list.
Reviewed By: hsb
Reviewers: hsb, aran, jungejason, tuomaspelkonen
CC: aran, hsb, epriestley
Differential Revision: 454
errors.
Summary:
Make sure reviewers know what they are doing.
Test Plan:
Tested with different diffs that had lint and unit problems.
Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: grglr, aran, epriestley, tuomaspelkonen
Differential Revision: 432
Summary:
Provide a quick workflow for adding a new project. This ended up being sort of
complicated because we don't currently put forms in dialogs. I separated the
actual <form /> tag out of the display/layout of AphrontFormView to enable this
(the dialog is itself a form).
Limitations: if you create a new project and then remove it, it won't appear in
the tokenizer until you reload the page. We need to add the ability for the
datasource to drop its cache to enable this, which is super complicated.
Test Plan:
Used "Create new project" to add a new project when creating a task.
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: anjali, aran, epriestley
Differential Revision: 422
Summary:
If you resize your window to be very narrow, the menu bar spazzes out right now.
Prevent it from developing all sorts of weird internal linewrapping.
Test Plan:
Narrowed my browser window, header didn't spaz out.
Reviewed By: jungejason
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 428
Summary:
ReviewBoard has a fancier version of this feature that's more granular -- the
keyboard can focus on individual changes. I think that's good and intend to
implement something similar, but this gets us a step closer and gets rid of some
of the bookkeeping stuff like making shortcuts discoverable.
(I have another brnach with Maniphest merging which also uses fatcow icons,
which is why the README seems a little out of context.)
Test Plan:
Used "j" and "k" to jump between changesets. Pressed "?" and got a list of
available shortcuts.
Reviewed By: tuomaspelkonen
Reviewers: aran, jungejason, tuomaspelkonen
CC: moskov, aran, epriestley, tuomaspelkonen
Differential Revision: 412
Summary: Implements a simple infrastructure for keyboard shortcuts, see T184, and a "help" shortcut.
There's a lot of room for refinement here but I think it basically works. Each shortcut can also provide a "tooltip" handler which allows it to show help when the alt/option key is held down.
Test Plan: Pressed "?" and got help. Pressed "?" in various contexts where it should not activate (modifier keys, text input focused) and didn't get help.
Reviewers: aran, tuomaspelkonen, jungejason
CC: moskov
Differential Revision: 362
Summary:
If you name an attached file something like
"derpderpderpderpderdperdperpereprederpderplolololololo.png", the display
overflows. Crop and shorten it so that it looks decent. Show the full name as
the image title.
Test Plan:
Named a file "derpderpderpderpderdperdperpereprederpderplolololololo.png" and
got a sensible display out of it.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, tuomaspelkonen
Differential Revision: 389
Summary:
Allows you to link to comments with "D123#3" or "T123#3", then adds a pile of JS
to try to make it not terrible. :/
The thing I'm trying to avoid here is when someone says "look at this!
http://blog.com/#comment-239291" and you click and your browser jumps somewhere
random and you have no idea which comment they meant. Since I really hate this,
I've tried to avoid it by making sure the comment is always highlighted.
Test Plan:
Put T1#1 and D1#1 in remarkup and verified they linked properly.
Clicked anchors on individual comments.
Faked all comments hidden in Differential and verified they expanded on anchor
or anchor change.
Reviewed By: aran
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 383
Summary:
I ran a 99designs contest and this gear-eye thing was actually pretty okay. All
this stuff needs tweaks but at least it won't render with a big square on
windows anymore.
Test Plan:
Looked at menu, it seemed slightly more legitimate and designey than before?
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley, jungejason
Differential Revision: 363
Summary:
Show large thumbnails of attached files in Maniphest.
Test Plan:
Looked at large thumbnails in Maniphest.
Reviewed By: jungejason
Reviewers: tomo, aran, jungejason, tuomaspelkonen
CC: anjali, aran, epriestley, jungejason
Differential Revision: 335
Summary:
Make it discoverable, show uploading progress, show file thumbnails, allow you
to remove files, make it a generic form component.
Test Plan:
Uploaded ducks
Reviewed By: tomo
Reviewers: aran, tomo, jungejason, tuomaspelkonen
CC: anjali, aran, epriestley, tomo
Differential Revision: 334
Summary:
It was possible to submit a comment multiple times if the submit
button was pressed more than once quickly. Added javascript code
that disables the button when it is clicked.
Test Plan:
Tried to click the button multiple times very quickly, but the
button was disabled after the first click.
Reviewed By: epriestley
Reviewers: epriestley, jungejason
Commenters: aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 337
Summary:
This needs a bunch of UI polish (critically, it's totally undiscoverable) but it
basically works correctly. I'll clean it up in some followups.
Test Plan:
Uploaded some files via drag-and-drop, made comments, etc.
Reviewed By: aran
Reviewers: tomo, aran, jungejason, tuomaspelkonen
CC: anjali, aran
Differential Revision: 332
Summary:
Various CSS tweaks and fixes:
- Add remarkup styling to description change views, missed this before.
- Fix CSS so that transactions with only one item (e.g., changed priority)
don't have weird floater underneath them.
- Add more space between transaction items.
- Make default background color lighter and less heavy.
- Use beigey color for comment form in Maniphest.
- Share more CSS between Maniphest and Differential (previews, feedback).
- Move "Leap Into Action" call to Differential, replace Maniphest with
thematically-consistent "Weigh In" (obviously, Maniphest has a nautical theme).
Test Plan:
Browsed Maniphest and Differential in a couple browsers, styling all seems
correct.
Reviewed By: tomo
Reviewers: tomo, aran, jungejason, tuomaspelkonen
CC: anjali, aran, tomo
Differential Revision: 328
Summary:
Someone has "defaced" secure.phabricator.com with a helpful suggestion that I
actually do this; fair enough. :P
Test Plan:
Logged in as myself, unable to edit directory information. Logged out, logged in
as admin, was able to edit directory information.
I need to fix some more CSS stuff since some of these tabs render out hideous in
the admin background, but I can followup with that.
Reviewed By: tuomaspelkonen
Reviewers: aran, jungejason, tuomaspelkonen
Commenters: aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 296
Summary:
You can currently attach tasks to revisions from Differential, but not revisions
to tasks from Maniphest. Allow editing from either side.
This logic is kind of tricky but the alternative was massive code duplication.
Test Plan:
Added and removed revisions from maniphest. Added and removed tasks from
differential.
This should have no impact on the Facebook install since none of this is used
there.
Reviewed By: aran
Reviewers: tomo, tuomaspelkonen, jungejason, aran
CC: aran, epriestley
Differential Revision: 288
Summary:
Provide an "isAdmin" flag for users, to designate administrative users.
Restore the account editing interface and allow it to set role flags and reset
passwords.
Provide an "isDisabled" flag for users and shut down all system access for them.
Test Plan:
Created "admin" and "disabled" users. Did administrative things with the admin
user. Tried to do stuff with the disabled user and was rebuffed. Tried to access
administrative interfaces with a normal non-admin user and was denied.
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: ccheever, aran
Differential Revision: 278
Summary:
JX.Workflow does a JX.DOM.find() for the sigil __default__, but fails because we
render with phutil_render_tag() instead of javelin_render_tag() so sigils are
not converted correctly.
Test Plan:
Opened an inline comment in differential, got focus on the <textarea />
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 259
Summary:
Javelin is currently embedded in Phabricator via copy-and-paste of prebuilt
packages. This is not so great.
Pull it in as a submodule instead and make all the Phabriator resources declare
proper dependency trees. Add Javelin linting.
Test Plan:
I tried to run through pretty much all the JS functionality on the site. This is
still a high-risk change, but I did a pretty thorough test
Differential: inline comments, revealing diffs, list tokenizers, comment
preview, editing/deleting comments, add review action.
Maniphest: list tokenizer, comment actions
Herald: rule editing, tokenizers, add/remove rows
Reviewed By: tomo
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, tomo, epriestley
Differential Revision: 223
Summary:
- Provides an "all daemons" view to look at more than the first 15 daemons.
- Provides a "combined log" view with a large page size, to quickly look at
the log across all the daemons, making it easier to find issues when you have a
bunch of the same daemon and only one is having issues.
- When viewing the web console on the same host as a daemon, show whether it's
running or not.
Test Plan:
Clicked the various daemon log interfaces.
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran
Differential Revision: 215
Summary:
No workflow dialog when subscribing or unsubscribing from a Differential
Revision
Test Plan:
Repeatedly subscribe and unsubscribe to a revision. Enter refractory period due
to
speed of page gen.
Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Differential Revision: 128
Summary:
When function phlog() is called, stacktrace and detailed log information
is shown in DarkConsole.
Test Plan:
Called 'phlog' function from various places in Phabricator and checked that
the debug information was available in DarkConsole.
Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, epriestley
Differential Revision: 101
Summary:
Editing Maniphest tasks for a Differential Revision required user to hit
'search' every time he changed search parameters. Now select and text input
changes trigger search automatically.
Test Plan:
Tested that changing the select and entering text automatically gave the
correct results.
Reviewed By: epriestley
Reviewers: epriestley, jungejason
Commenters: jungejason
CC: epriestley, jungejason
Differential Revision: 102
