1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-22 02:08:47 +02:00
Commit graph

362 commits

Author SHA1 Message Date
epriestley
e0e339f16c Modernize People application
Summary: Move to application navigation, make it possible to get to /logs/ from the navigation.

Test Plan: Hit all interfaces, verified email.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1569, T631

Differential Revision: https://secure.phabricator.com/D3261
2012-08-13 15:27:21 -07:00
Bob Trahan
dd26bc6d1a Remove shield for Conduit API responses
Summary: 'cuz we don't need it and it's lame complexity for API clients of all kinds. Rip the band-aid off now.

Test Plan: used conduit console and verified no more shield. also did some JS stuff around the suite to verify I didn't kill JS

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T891

Differential Revision: https://secure.phabricator.com/D3265
2012-08-13 14:49:32 -07:00
epriestley
b2f12a09b5 Modernize MetaMTA
Summary:
  - Add an Application.
  - Move routes to the application.
  - Move nav out of tabs (which no longer exist).
  - Fix a couple of random things.

Test Plan: Viewed sent/received mail logs. Performed send/receive tests. Viewed email details.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T631, T1569

Differential Revision: https://secure.phabricator.com/D3255
2012-08-13 12:37:06 -07:00
epriestley
74b438db13 Move "Mailing Lists" to a separate application
Summary:
There's currently no way to get here from the UI since nav tabs don't exist anymore. It's also always been hard to find this feature even when we had the tabs, since it's surprising that it's inside "MetaMTA".

  - Move mailing lists to a separate application.
  - Add `buildApplicationPage()`, since we don't really need `buildStandardPageResponse()` any more -- we can infer all the information from `PhabricatorApplication`. This will let us get rid of a lot of the `PhabricatorXXXController` classes which just define application information.
  - Add `getApplicationURI()` to reduce code duplication, and in case we want to let you move applications around some day.

Test Plan: Looked/edited/saved mailing lists.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T631

Differential Revision: https://secure.phabricator.com/D3248
2012-08-12 19:19:46 -07:00
vrana
b1b2afce95 Support applications in aphrontpath.php
Test Plan:
  $ ./aphrontpath.php /diffusion/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3233
2012-08-10 10:48:08 -07:00
Pieter Hooimeijer
64472dd7b8 Adding Ponder-related files.
Summary:
Ponder is similar in spirit to the Wiki tool, but uses a Q&A
format and up/downvotes to signal user sentiment. Popular
questions are moved to the top of the feed on a 5-minute
cycle based on age (younger is better) and vote count (higher
is better).

Pre-apologies for noob diff.

Test Plan:
- `./bin/phd list` Should include `PonderHeatDaemon`; phd launch it
  if necessary.

- Navigate to /ponder/ ; observe sanity when adding questions,
  voting on them, and adding answers.

- Confirm that questions and answers are linkable using Q5 / Q5#A5 formatted object links.

- Confirm that searching for Ponder Questions works using built-in
  search.

Feedback on code / schema / whatever organization very welcome.

Reviewers: nh, vrana, epriestley

Reviewed By: epriestley

CC: gmarcotte, aran, Korvin, starruler

Differential Revision: https://secure.phabricator.com/D3136
2012-08-10 10:44:04 -07:00
epriestley
d74b84a729 Add a "Project" application
Summary: Move routes into a formal application class.

Test Plan: Checked /applications/, browsed various routes.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, floatinglomas

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3181
2012-08-07 11:54:49 -07:00
epriestley
ebb7807bb4 Fix an issue with URIs missing trailing slashes, like "/maniphest"
Summary:
In D3144, I made us look in application maps to find routing rules. However, we don't process //all// the maps when we 404 and try to do a "/" redirect. Process all of the maps.

Additionally, in D3146 I made the menu items application-driven. However, some pages (like 404) don't have a controller. Drop the requirement that the controller be nonnull.

Test Plan:
  - Visited "/maniphest", got a redirect after this patch.
  - Visited "/asldknfalksfn", got a 404 after this patch.

Reviewers: davidreuss, vrana, btrahan

Reviewed By: davidreuss

CC: aran

Maniphest Tasks: T1607

Differential Revision: https://secure.phabricator.com/D3158
2012-08-06 12:46:51 -07:00
epriestley
314c25365d Drive menu icons from Applications
Summary:
This makes no changes, it just moves the menu icons to the applications instead of hard-coded on the page.

I'm going to try to address some of the angst in T1593 next...

Test Plan: Loaded logged-in / logged out pages. Clicked menu items. Looked at /applications/.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1593, T1569

Differential Revision: https://secure.phabricator.com/D3146
2012-08-05 14:12:43 -07:00
epriestley
643653dc61 Store the "current" application in the controller
Summary:
When we match an application route, select it as the current application and store it on the controller.

Move routes for the major applications into their PhabricatorApplication classes so this works properly.

Test Plan: Added a var_dump() and made sure we picked the right app for all these applications.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1569

Differential Revision: https://secure.phabricator.com/D3144
2012-08-05 14:03:39 -07:00
vrana
606ef34d61 Load commit branches and tags by AJAX
Summary:
Each query takes over 2 seconds in FBCODE.
I didn't find a way how to speed them up.
There's also no easy way how to parallelize them at least.
So AJAX is the last instance.

Test Plan: Loaded commit with one branch and no tag.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3112
2012-07-31 17:01:41 -07:00
epriestley
fceabd42e8 Allow Fact app to draw charts
Summary: For any count fact, allow a chart to be drawn. INCREDIBLY POWERFUL DATA ANALYSIS PLATFORM.

Test Plan: Drew a chart of object counts. Drew the Maniphest burn chart.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1562

Differential Revision: https://secure.phabricator.com/D3099
2012-07-30 10:44:08 -07:00
epriestley
486f7c1e8e Add aggregated facts to the Facts application
Summary:
Some facts are aggregations of other facts. For example, we may compute how many times each macro is used in each object as a "raw fact":

  Dnnn uses macro "psyduck" 6 times.

But we want to present this data in aggregate form, e.g. "order macros by popularity". We can do this at runtime and it probably won't be too awful a query, but we can also aggregate it cheaply:

  Macro "psyduck" is used 3920 times across all objects.

...and then do a query like "select macros ordered by usage".

"Aggregate" facts support facts like this. The aggregate facts I've implemented are:

  - Count of all objects.
  - Count of objects of type X.
  - Last time facts were updated.

These clearly fit the "aggregate" facts template well. I'm not 100% sure macros do. We can use this table to answer a question like "What are the most popular macros, ordered by use?" We can also use it to answer a question like "What are the most popular macros in the last 6 months?", if we build a specific fact for that. But we can't use it to answer a question like "What are the most popular macros between times X and Y?". Maybe that's important; maybe not.

This seems like a good fit for at least some types of facts.

I'll de-magic the keys a bit in the next diff.

Test Plan: Ran the engines and got some aggregated facts about other facts.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1562

Differential Revision: https://secure.phabricator.com/D3089
2012-07-27 13:46:01 -07:00
Bob Trahan
321df38ccf Deleting unused private member variable from AphrontRequest
Summary: I was poking around to see how this class worked and noticed this variable does nothing.

Test Plan: Careful inspection and reasoning that unused private member variable can be deleted.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3068
2012-07-25 13:48:46 -07:00
epriestley
5d4a6bcf95 Break AphrontDatabaseConnection dependencies on PhabricatorEnv
Summary: We pull "retries" and a doc link from PhabricatorEnv directly. Break these dependencies so the classes can move to libphutil.

Test Plan: Browsed site, triggered a schema exception and verified I still got the useful footer text.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1283

Differential Revision: https://secure.phabricator.com/D3053
2012-07-24 10:47:27 -07:00
epriestley
be63cb386b Remove AphrontWriteGuard from phabricator/
Summary: See D3051.

Test Plan: Loaded some pages.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1283

Differential Revision: https://secure.phabricator.com/D3052
2012-07-24 10:46:07 -07:00
epriestley
d07934474e Break AphrontWriteGuard dependency on AphrontRequest
Summary:
I want to move queryfx() and family to libphutil, for @chad and others (see T1283). We need to break a few dependencies to do this.

Since AphrontWriteGuard is independently useful, I broke the dependency between it and AphrontRequest rather than between Connection and WriteGuard. I'll move its implementation to libphutil in a future diff.

Test Plan: Loaded site, submitted CSRF form successfully, monkeyed with CSRF token, submitted CSRF form, got error.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1283

Differential Revision: https://secure.phabricator.com/D3042
2012-07-23 15:17:59 -07:00
Bob Trahan
bc29a3e8a2 Make inline comment preview work in Diffusion
Summary: created a PhabricatorInlineCommentPreviewController so controllers in Diffusion and Differential respectively just have to handle the URI mapping and data loading like good little controllers.

Test Plan:
left inline comments on commits, deleted inline commits, submitted inline comments -- all worked well
did the same on some diffs

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1176

Differential Revision: https://secure.phabricator.com/D3034
2012-07-23 11:01:28 -07:00
Bob Trahan
ae13d33859 Phame - introduce blogs
Summary:
blogs are collections of posts. a blog also has metadata like a name, description and "bloggers" that can edit the metadata of the blog and contribute posts.

changes include the post edit flow where bloggers can now select which blogs to publish to. also made various small tweaks throughout the UI to make things sensical and clean as the concept of blogs is introduced.

there's edges powering this stuff.  bloggers <=> blogs and posts <=> blogs in particular.

Test Plan:
made blogs, deleted blogs, tried to make blogs with no bloggers. all went well.
verified ui to publish only showed up for public posts, published posts to blogs, un-published posts to blogs, re-published posts to blogs, deleted posts and verified they disappeared from blogs.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1373

Differential Revision: https://secure.phabricator.com/D3003
2012-07-19 09:03:10 -07:00
epriestley
dd70c59465 Use OpaqueEnvelopes for all passwords in Phabricator
Summary:
See D2991 / T1526. Two major changes here:

  - PHP just straight-up logs passwords on ldap_bind() failures. Suppress that with "@" and keep them out of DarkConsole by enabling discard mode.
  - Use PhutilOpaqueEnvelope whenever we send a password into a call stack.

Test Plan:
  - Created a new account.
  - Reset password.
  - Changed password.
  - Logged in with valid password.
  - Tried to login with bad password.
  - Changed password via accountadmin.
  - Hit various LDAP errors and made sure nothing appears in the logs.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2993
2012-07-17 12:06:33 -07:00
epriestley
ae2e73ce80 Add "stop on redirect" and "always profile" debugging options
Summary:
Currently, it's hard to debug performance issues on POST pages. Add flags to stop redirects and always collect profiles.

Also fix an issue with "all" profiles. This feature is mostly just for profiling DarkConsole itself and is rarely used, I think it's been broken for some time. There's no way to get to it with the UI.

NOTE: Some JS workflows don't stop on redirect because they use JS/AJAX redirects.

Test Plan: Enabled options, browsed, got stopped on redirects and had profiles generated. Disabled options and verified redirects and profiles work normally.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2990
2012-07-17 12:06:25 -07:00
Evan Priestley
62e039b748 Merge pull request #150 from linead/ldap_dir
Ldap dir
2012-07-11 18:37:36 -07:00
Bob Trahan
67691c196e Phame - add an "Everyone but Me" view and make published posts truly accessible to the whole world.
Summary:
accessibility covers not only a given post but also the various "published" views.

to keep the code relative clean, this diff also splits up the post list controller logic quite a bit. this also feels like good preparation for some other work around introducing "blogs" which are collections of published posts from bloggers with some fancy features around that.

Test Plan: clicked around various parts of the Phame application as a logged in user, a logged in user with no personal posts, and without any user logged in at all. various views all seemed reasonable.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1373

Differential Revision: https://secure.phabricator.com/D2898
2012-07-05 15:43:14 -07:00
linead
51d2d06e37 Added ldap import controller 2012-07-04 12:10:38 +10:00
vrana
b10428fe60 Link to a specific Phabricator version from stack trace
Summary: Line numbers and file paths may be different in current version.

Test Plan: Disabled editor, issued exception, clicked on the link.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2886
2012-06-28 16:22:06 -07:00
epriestley
1e3cd8afa8 Improve notification list view
Summary:
  - Provide a filter to show just unread notifications.
  - Visually show which notifications are unread.
  - Style tweaks to make notifications more readable.

Test Plan: {F13494}

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2883
2012-06-28 13:59:50 -07:00
vrana
0c321d8176 Delete old commented code 2012-06-28 10:45:45 -07:00
epriestley
083cd5fd9d Add a "Mark All Read" button for notifications
Summary: Adds a button to mark everything as read.

Test Plan: Clicked button.

Reviewers: btrahan, jungejason, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T974

Differential Revision: https://secure.phabricator.com/D2812
2012-06-20 13:51:19 -07:00
epriestley
8cd0997965 Add a list of all notifications
Summary: Add a "View All Notifications" link and page.

Test Plan: Viewed all notifications

Reviewers: jungejason, vrana

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T974

Differential Revision: https://secure.phabricator.com/D2780
2012-06-18 14:07:38 -07:00
epriestley
513abf00cf Add a server status page for notification server
Summary:
  - Add a /notification/status/ page which shows server status.
  - Remove various test controllers and routes.
  - Make the "no notifications" message look better.
  - Move port/URI configuration to config file.

Test Plan: Started server, hit /notification/status/, saw server status.

Reviewers: allenjohnashton, ddfisher, keebuhm, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T944

Differential Revision: https://secure.phabricator.com/D2756
2012-06-17 11:35:18 -07:00
Jonathan Lomas
4131fd34d3 Allow use of any whitespace or a comma as a task ID delimiter in custom query. 2012-06-15 15:54:09 -07:00
Espen Volden
726041584f Made it possible to login using LDAP
Summary: Made it possible to link and unlink LDAP accounts with  Phabricator accounts.

Test Plan:
I've tested this code locally and in production where I work.
I've tried creating an account from scratch by logging in with LDAP and linking and unlinking an LDAP account with an existing account. I've tried to associate the same LDAP account with different Phabricator accounts and it failed as expected.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, auduny, svemir

Maniphest Tasks: T742

Differential Revision: https://secure.phabricator.com/D2722
2012-06-13 08:58:46 -07:00
epriestley
d73a6d3dcd Fix username route for new "._-" usernames
Summary: We allow ".", "_" and "-" in usernames now, but not in the route.

Test Plan: Went to /p/.-_/, got 404'd at the route level before and now make it to the profile controller.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1348

Differential Revision: https://secure.phabricator.com/D2729
2012-06-13 08:39:02 -07:00
David Fisher
f8f195b329 Make Notifications Realtime
Summary:
Adds the node.js Aphlict server, the flash Aphlict client, and some
supporting javascript. Built on top of - and requires - D2703 (which is still
in progress).  Will likely work with no modification on top of the final
version, though.

The node server is currently run with

   sudo node support/aphlict/server/aphlict_server.js

Test Plan: tested locally

Reviewers: epriestley

Reviewed By: epriestley

CC: allenjohnashton, keebuhm, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2704
2012-06-11 17:51:12 -07:00
John-Ashton Allen
a11deec4d4 Adds the UI dropdown panel
Summary:
Add a dropdown to display notificaitons.  Right now
there is nothing real time about it, but we do update the panel
when the user clicks.  This panel is only displayed if the
install has notifications enabled and you have them enabled in
your preferences (not using them by default).

Test Plan: Turn off notifications for user1, left them on for user2.  Did things from user1 and from user2 on task both were cc'd on.  user2 recieved all notifications, user1 recieved nothing.  Made new user, made sure everything was switched off by default.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: keebuhm, ddfisher, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2703
2012-06-11 09:42:55 -07:00
John-Ashton Allen
3a6ee79190 Adds base notification application
Summary: First diff in a series of diffs to add notifications to Phabricator. This is the notification application ONLY. This commit does not include the changes to other applications that makes them add notifications. As such, no notifications will be generated beyond the initial database import.

Test Plan: This is part of the notifications architecture which has been running on http://theoryphabricator.com for the past several months.

Reviewers: epriestley, btrahan, ddfisher

Reviewed By: epriestley

CC: allenjohnashton, keebuhm, aran, Korvin, jungejason, nh

Maniphest Tasks: T974

Differential Revision: https://secure.phabricator.com/D2571
2012-06-08 06:32:02 -07:00
epriestley
3e87112d69 Minor, fix a path issue from the Great File Move. See comment about why this is written in this way. 2012-06-06 07:35:40 -07:00
epriestley
8d37576bd8 Move some aprhont/ stuff around a little bit
Summary:
"configuration/default" moved to "default". Rename it to "configuration" and put config stuff back there.

Put writeguard into writeguard/.

Test Plan: tools

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2639
2012-06-01 12:52:18 -07:00
vrana
6cc196a2e5 Move files in Phabricator one level up
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.

NOTE: `arc diff` timed out so I'm pushing it without review.

Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.

Auditors: epriestley

Maniphest Tasks: T1103
2012-06-01 12:32:44 -07:00
vrana
1ebf9186b4 Depend on class autoloading
Test Plan:
Run setup.
/differential/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2612
2012-05-30 16:57:21 -07:00
epriestley
09c8af4de0 Upgrade phabricator to libphutil v2
Summary: Mechanical changes from D2588. No "Class.php" moves yet.

Test Plan: See D2588.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2591
2012-05-30 14:26:29 -07:00
Bob Trahan
f5e842ebd9 dark console - introduce "request log" section
Summary: this section gets updated for each and every request. clicking a given entry updates the larger dark-console area to have the information from that request

Test Plan: clicked around in maniphest and observed request log populating correctly. clicked a few entries in request log and saw it updated properly. clicked a different tab in the dark-console and it worked. clicked a different request log entry and it opened the dark console to the proper request on the proper tab.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1136

Differential Revision: https://secure.phabricator.com/D2574
2012-05-25 10:14:17 -07:00
vrana
eb310888e5 Warn user before losing his data
Summary: Better solution would be to reload the page for user with valid token and all data he inserted but I guess that we don't have enough infrastructure for this.

Test Plan: Mangle token and send form.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2570
2012-05-24 18:04:44 -07:00
epriestley
77f546c572 Allow installs to require email verification
Summary:
Allow installs to require users to verify email addresses before they can use Phabricator. If a user logs in without a verified email address, they're given instructions to verify their address.

This isn't too useful on its own since we don't actually have arbitrary email registration, but the next step is to allow installs to restrict email to only some domains (e.g., @mycompany.com).

Test Plan:
  - Verification
    - Set verification requirement to `true`.
    - Tried to use Phabricator with an unverified account, was told to verify.
    - Tried to use Conduit, was given a verification error.
    - Verified account, used Phabricator.
    - Unverified account, reset password, verified implicit verification, used Phabricator.
  - People Admin Interface
    - Viewed as admin. Clicked "Administrate User".
    - Viewed as non-admin
  - Sanity Checks
    - Used Conduit normally from web/CLI with a verified account.
    - Logged in/out.
    - Sent password reset email.
    - Created a new user.
    - Logged in with an unverified user but with the configuration set to off.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran, csilvers

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2520
2012-05-21 12:47:38 -07:00
vrana
4626c40fe3 Display URI of HTTP profiler in DarkConsole
Summary: D2464

Test Plan: D2464

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2465
2012-05-14 14:25:42 -07:00
Aurelijus
ba98089426 Branch view improvements.
Summary:
Sorting by last commit date
Branch view limit to 25 branches
All branches table page with pagination on Git

Test Plan:
* Check repository view for expected behavior on branch table view
* Check all branches page & test pagination

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1200

Differential Revision: https://secure.phabricator.com/D2442
2012-05-10 11:47:46 -07:00
epriestley
87207b2f4e Allow users to have multiple email addresses, and verify emails
Summary:
  - Move email to a separate table.
  - Migrate existing email to new storage.
  - Allow users to add and remove email addresses.
  - Allow users to verify email addresses.
  - Allow users to change their primary email address.
  - Convert all the registration/reset/login code to understand these changes.
  - There are a few security considerations here but I think I've addressed them. Principally, it is important to never let a user acquire a verified email address they don't actually own. We ensure this by tightening the scoping of token generation rules to be (user, email) specific.
  - This should have essentially zero impact on Facebook, but may require some minor changes in the registration code -- I don't exactly remember how it is set up.

Not included here (next steps):

  - Allow configuration to restrict email to certain domains.
  - Allow configuration to require validated email.

Test Plan:
This is a fairly extensive, difficult-to-test change.

  - From "Email Addresses" interface:
    - Added new email (verified email verifications sent).
    - Changed primary email (verified old/new notificactions sent).
    - Resent verification emails (verified they sent).
    - Removed email.
    - Tried to add already-owned email.
  - Created new users with "accountadmin". Edited existing users with "accountadmin".
  - Created new users with "add_user.php".
  - Created new users with web interface.
  - Clicked welcome email link, verified it verified email.
  - Reset password.
  - Linked/unlinked oauth accounts.
  - Logged in with oauth account.
  - Logged in with email.
  - Registered with Oauth account.
  - Tried to register with OAuth account with duplicate email.
  - Verified errors for email verification with bad tokens, etc.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2393
2012-05-07 10:29:33 -07:00
epriestley
7b5f47b17d Enforce upload size limits and transport exceptions with appropriate response encoding
Summary:
  - When a user uploads an oversized file, throw an exception.
  - When an uncaught exception occurs during a Conduit request, return a Conduit response.
  - When an uncaught exception occurs during a non-workflow Ajax request, return an Ajax response.

Test Plan:
  - Uploaded overlarge files.
  - Hit an exception page with ?__ajax__=1 and ?__conduit__=1

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T875, T788

Differential Revision: https://secure.phabricator.com/D2385
2012-05-07 06:17:00 -07:00
vrana
87ff461470 Add /F123 shortcut
Summary:
I wanted to point someone on a file uploaded to Phabricator and the normal link is just too long.

I guess that this also improves security. Because pointing someone to the file directly reveals the secret key used in /data/ and it can be served without auth?

We already use `{F123}` so there will be no conflicts in future because we wouldn't want to reuse it for something else.

I promote the link on /file/ - it adds one redirect but I think it's worth it. I also considered making the link from the File ID column but there are already too many links (with some duplicity).

Test Plan:
/file/
/F123 (redirect)
/F9999999999 (404)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2380
2012-05-03 23:51:40 -07:00
epriestley
bf505ef51c Hard-code and unify "more applications" in Phabricator
Summary:
This is mostly intended to simplify D2323.

We currently allow users to edit and customize the links on the homepage, but as far as I know no one actually does this (no one complained when we redid the homepage earlier this year) and it creates a lot of mess in the database patches and quickstart dump. After D2331, this is the only data we load in the patch files. The patch files are also a mess with respect to this data and have various different versions of it.

Also the current UI is just kind of bad, it stretches stuff across too many screens and is generally ungood. Nuking this lets us nuke a lot of code in general.

(In the long term, I think we'll move toward an "application" model anyway, and this stuff will go away sooner or later.)

I'll add a drop-database patch some time later, just in case anyone does actually use this, so they can get their data out of MySQL.

Test Plan: Looked at home page, clicked "More Stuff", got a single list of other apps/things.

Reviewers: btrahan, vrana, edward, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T345

Differential Revision: https://secure.phabricator.com/D2332
2012-04-28 07:17:38 -07:00
vrana
d27a751339 Create overview of Conduit methods
Summary:
Also couple of small changes:

- Add method name to title.
- 404 for /conduit/method/x/.
- Remove utilities from side panel.
- Remove side panel from log.

Test Plan:
/conduit/
/conduit/method/x/
/conduit/method/user.whoami/
/conduit/log/

Reviewers: btrahan, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2326
2012-04-27 17:27:47 -07:00
epriestley
7b334f37bd Improve tag support in Diffusion
Summary:
  - When viewing a commit, show its tags.
  - For commits with many tags, show a list of all tags on the tag list interface.
  - Improve some handling of symbolic references.
  - When tags contain content, show it on the browse view reached by clicking the tag name.

Test Plan: Looked at commits with and without tags, clicked "More tags...", clicked tag names.

Reviewers: btrahan, vrana, davidreuss, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2290
2012-04-23 18:36:25 -07:00
vrana
6cf61980d2 Analyze fulltext search in DarkConsole
Test Plan: /search/.../?__analyze__=1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2289
2012-04-20 10:58:17 -07:00
epriestley
944049d871 Add a paginated list of all repository tags to Diffusion
Summary: Now supports more than 25 tags!

Test Plan: Set page size to 1, paginated. Verified SVN / Hg don't break/explode.

Reviewers: davidreuss, vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2268
2012-04-19 09:39:19 -07:00
vrana
1772410a2b Allow opening editor from DarkConsole error log
Test Plan:
Click on "passing a null index to idx()" in DarkConsole.
Click on entry in stack trace.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2275
2012-04-18 13:57:31 -07:00
epriestley
ded641ae32 Add basic per-object privacy policies
Summary:
Provides a basic start for access policies. Objects expose various capabilities, like CAN_VIEW, CAN_EDIT, etc., and set a policy for each capability. We currently implement three policies, PUBLIC (anyone, including logged-out), USERS (any logged-in) and NOONE (nobody). There's also a way to provide automatic capability grants (e.g., the owner of an object can always see it, even if some capability is set to "NOONE"), but I'm not sure how great the implementation feels and it might change.

Most of the code here is providing a primitive for efficient policy-aware list queries. The problem with doing queries naively is that you have to do crazy amounts of filtering, e.g. to show the user page 6, you need to filter at least 600 objects (and likely more) before you can figure out which ones are 500-600 for them. You can't just do "LIMIT 500, 100" because that might have only 50 results, or no results. Instead, the query looks like "WHERE id > last_visible_id", and then we fetch additional pages as necessary to satisfy the request.

The general idea is that we move all data access to Query classes and have them do object filtering. The ID paging primitive allows efficient paging in most cases, and the executeOne() method provides a concise way to do policy checks for edit/view screens.

We'll probably end up with mostly broader policy UIs or configuration-based policies, but there are at least a few cases for per-object privacy (e.g., marking tasks as "Security", and restricting things to the members of projects) so I figured we'd start with a flexible primitive and the simplify it in the UI where we can.

Test Plan: Unit tests, played around in the UI with various policy settings.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D2210
2012-04-14 10:13:29 -07:00
Bob Trahan
51418900f7 Phame V1 - Phabricator blogging software
Summary:
'cuz we need to be phamous!

V1 feature set

- posts
-- standard thing you'd expect - a title and a remarkup-powered body and...
-- "phame" title - a short string that can be used to reference the story. this gets auto-updated when you mess with the title.
-- configuration - for now, do you want Facebook, Disqus or no comments? this is a per-post thing but feeds from an instance-wide configuration

Please do toss out any must have features or changes.

Test Plan: played around with this bad boy like whoa

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, vrana

Maniphest Tasks: T1111

Differential Revision: https://secure.phabricator.com/D2202
2012-04-12 13:09:04 -07:00
epriestley
488b1cf641 Allow Maniphest queries to be saved
Summary:
There have been a couple of requests for this since bookmarks are "out this year like woah" and "totally uncool dude".

Allow users to save named custom queries and make them the /maniphest/ default if they so desire.

A little messy. :/

Test Plan: Saved, edited, deleted custom queries. Made custom query default; made 'no default' default. Verified default behavior. Issued a modified search from a custom query.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, 20after4

Maniphest Tasks: T923, T1034

Differential Revision: https://secure.phabricator.com/D1964
2012-04-10 09:46:04 -07:00
epriestley
a5903d2a53 Use head_key() and last_key() to explicitly communicate intent
Summary:
PHP arrays have an internal "current position" marker. (I think because foreach() wasn't introduced until PHP 4 and there was no way to get rid of it by then?)

A few functions affect the position of the marker, like reset(), end(), each(), next(), and prev(). A few functions read the position of the marker, like each(), next(), prev(), current() and key().

For the most part, no one uses any of this because foreach() is vastly easier and more natural. However, we sometimes want to select the first or last key from an array. Since key() returns the key //at the current position//, and you can't guarantee that no one will introduce some next() calls somewhere, the right way to do this is reset() + key(). This is cumbesome, so we introduced head_key() and last_key() (like head() and last()) in D2161.

Switch all the reset()/end() + key() (or omitted reset() since I was feeling like taking risks + key()) calls to head_key() or last_key().

Test Plan: Verified most of these by visiting the affected pages.

Reviewers: btrahan, vrana, jungejason, Koolvin

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2169
2012-04-09 11:08:59 -07:00
epriestley
e4df959064 Use Celerity to version all static resources
Summary:
We don't use versioned URIs for images, so when they change users may get old versions.

This was a particular issue with the recent logo change, which several users reported cache-related issues from.

Instead, use Celerity to manage image URI versions in addition to CSS/JS.

This is complicated, because we need to rewrite image URIs inside of CSS, which means the hash of a CSS file has to be derived from the current image data. Otherwise, when we updated an image the CSS wouldn't update, so we wouldn't be any better off.

So basically we:

  - Find all the "raw" files, and put them into the map.
  - Find all the CSS/JS, perform content-altering transformations on it (i.e., not minification) based on the partial map, and then put it into the map based on transformed hashes.

(If we wanted, we could now do CSS variables or whatever for "free", more or less.)

Test Plan:
  - Regenerated celerity map, browsed site, verified images generated with versioned URIs.
  - Moved "blue" flag image over "green" flag image, regenerated map, verified "green" flag image and the associated CSS changed hashes.
  - Added transformation unit tests; ran unit tests.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1073

Differential Revision: https://secure.phabricator.com/D2146
2012-04-08 10:07:51 -07:00
vrana
d4c5761f41 Customizable MySQL implementation
Test Plan:
- /
- upgrade_schema.php
- Setup
- Try disabling mysql_connect.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2133
2012-04-07 10:54:12 -07:00
vrana
7507981f22 Don't fatal with inaccessible database
Test Plan: Disable `mysql_connect()`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2137
2012-04-07 10:31:36 -07:00
vrana
58d0ee2de0 Improve Windows support
Test Plan: Display stacktrace, verify that filenames are links.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2132
2012-04-07 10:15:54 -07:00
vrana
e69ba98e20 Prepare for MySQLi support
Summary: This separates common MySQL stuff (identifiers and comments escaping, error codes, connection retries) from PHP extension specific stuff (connect, query, fetch, errors, escape string).

Test Plan:
/
Use `AphrontMySQLiDatabaseConnection` in `PhabricatorLiskDAO`, load homepage, edit task, save task.

Reviewers: epriestley

Reviewed By: epriestley

CC: nh, aran

Differential Revision: https://secure.phabricator.com/D2113
2012-04-06 12:43:56 -07:00
vrana
d1b7059a2d Open editor from stack trace
Summary:
I've considered that user may have set editor but not checked out Phabricator repositories.
But stack trace is useful mainly for developers.

Test Plan:
Click on path in Unhandled Exception.
Repeat with disabled editor.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2107
2012-04-04 18:19:14 -07:00
Bob Trahan
cc586b0afa For discussion -- Stripe integration
Summary:
various stripe stuff, including

- external stripe library
- payment form
- test controller to play with payment form, sample business logic

My main questions / discussion topics are...

- is the stripe PHP library too big? (ie should I write something more simple just for phabricator?)
-- if its cool, what is the best way to include the client? (ie should I make it a submodule rather than the flat copy here?)
- is the JS I wrote (too) ridiculous?
-- particularly unhappy with the error message stuff being in JS *but* it seemed the best choice given the most juicy error messages come from the stripe JS such that the overall code complexity is lowest this way.
- how should the stripe JS be included?
-- flat copy like I did here?
-- some sort of external?
-- can we just load it off stripe servers at request time? (I like that from the "if stripe is down, stripe is down" perspective)
- wasn't sure if the date control was too silly and should just be baked into the form?
-- for some reason I feel like its good to be prepared to walk away from Stripe / switch providers here, though I think this is on the wrong side of pragmatic

Test Plan: - played around with sample client form

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2096
2012-04-04 16:09:29 -07:00
vrana
67e10e60f2 Return $this from setters
Summary:
Most setters returns `$this` but some don't.
I guess it's not by purpose.

Test Plan:
  arc lint

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2085
2012-04-02 18:48:37 -07:00
epriestley
e7853e4801 Allow tasks to be subprioritized by drag-and-drop
Summary:
Like the title says, similar to Facebook Tasks.

Not sure how I really feel about this, but I guess it's kind of OK? I never used
this feature in Facebook Tasks but I think some people like it.

The drag-and-drop to repri across priorities feels okayish.

Because subpriority is a double and we just split the difference when
reprioritizing, you lose ~a bit of precision every time you repri two tasks
against each other and so you can break it by swapping the priorities of two
tasks ~50 times. This case is pretty silly and pathological. We can add some
code to deal with this at some point if necessary.

I think this also fixes the whacky task layout widths once and for all.

(There are a couple of minor UI glitches like headers not vanishing and header
counts not updating that I'm not fixing because I am lazy.)

Test Plan: Dragged and dropped tasks around.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, mgummelt

Maniphest Tasks: T859

Differential Revision: https://secure.phabricator.com/D1731
2012-04-02 12:12:04 -07:00
epriestley
698ec68327 General Herald refactoring pass
Summary:
**Who can delete global rules?**: I discussed this with @jungejason. The current behavior is that the rule author or any administrator can delete a global rule, but this
isn't consistent with who can edit a rule (anyone) and doesn't really make much sense (it's an artifact of the global/personal split). I proposed that anyone can delete a
rule but we don't actually delete them, and log the deletion. However, when it came time to actually write the code for this I backed off a bit and continued actually
deleting the rules -- I think this does a reasonable job of balancing accountability with complexity. So the new impelmentation is:

  - Personal rules can be deleted only by their owners.
  - Global rules can be deleted by any user.
  - All deletes are logged.
  - Logs are more detailed.
  - All logged actions can be viewed in aggregate.

**Minor Cleanup**

  - Merged `HomeController` and `AllController`.
  - Moved most queries to Query classes.
  - Use AphrontFormSelectControl::renderSelectTag() where appropriate (this is a fairly recent addition).
  - Use an AphrontErrorView to render the dry run notice (this didn't exist when I ported).
  - Reenable some transaction code (this works again now).
  - Removed the ability for admins to change rule authors (this was a little buggy, messy, and doesn't make tons of sense after the personal/global rule split).
  - Rules which depend on other rules now display the right options (all global rules, all your personal rules for personal rules).
  - Fix a bug in AphrontTableView where the "no data" cell would be rendered too wide if some columns are not visible.
  - Allow selectFilter() in AphrontNavFilterView to be called without a 'default' argument.

Test Plan:
  - Browsed, created, edited, deleted personal and gules.
  - Verified generated logs.
  - Did some dry runs.
  - Verified transcript list and transcript details.
  - Created/edited all/any rules; created/edited once/every time rules.
  - Filtered admin views by users.

Reviewers: jungejason, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2040
2012-03-30 10:49:55 -07:00
Bob Trahan
e696619dd1 Chatlog - add a pager
Summary: 'cuz I miss out on chat room goodness and can't paginate around in the current version

Test Plan: setup a phabot and spammed it in phabot-test. with new test data, set $page_limit = 1 and paged about -- looks good!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T990

Differential Revision: https://secure.phabricator.com/D2032
2012-03-27 16:53:47 -07:00
epriestley
7ad68e63e4 Add "Flags" to allow users to collect the things they love
Summary:
Flags are a personal collection of things you want to take a look at later. You can use several different colors and add notes.

Not really sure if this is actually a good idea or not but it was easy to build.

Planned features:

  - Allow Herald rules to add flags.
  - In the "edit flag" dialog, have a "[x] Subscribe Me" checkbox that CCs you.
  - Support Diffusion.
  - Support Phriction.
  - Always show flags on an object if you have them (in every view)?
  - Edit dialog feels a little heavy?
  - More filtering in /flag/ tool.
  - Add a top-level links somewhere?

Test Plan: Added, edited and removed flags from things. Viewed flags in flag view.

Reviewers: aran, btrahan

Reviewed By: btrahan

CC: aran, epriestley, Koolvin

Maniphest Tasks: T1041

Differential Revision: https://secure.phabricator.com/D2024
2012-03-27 16:22:40 -07:00
epriestley
a3028558ae Make includes free to the user (application support component)
Summary:
We spend a significant amount of time running includes, even with APC. However, we have rigidly structured includes and can safely run them all in workers before requests occur.

Right now, requests go like this:

  - Apache spawns a worker.
  - Client sends an HTTP request.
  - Apache interprets it.
  - Apache sees it's ".php", so it hands it off to the PHP SAPI.
  - The PHP SAPI starts the PHP interpreter in the worker.
  - The request is handled, etc.

Instead, we want to do this:

  - Worker spawns and loads the world.
  - Client sends an HTTP request.
  - Webeserver interprets it.
  - Sees it's a ".php", hands it off to the SAPI.
  - SAPI executes it on a loaded world.

No SAPIs I know of support this, but I added support to PHP-FPM fairly easily (in the sense that it took me 6 hours and I have a hacky, barely-working mess). Over HTTP (vs HTTPS) the performance improvement is pretty dramatic.

HPHP doesn't significantly defray this cost so we're probably quite a bit faster (to the user) under nginx+PHP-FPM than HPHP after this works for real.

I have the php-fpm half of this patch in a messy state, I'm going to try to port it to be vs php 5.4.

Test Plan: Ran a patched php-fpm, browsed around, site works, appears dramatically faster.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2030
2012-03-27 16:06:13 -07:00
epriestley
914f044b62 More Drydock Stuff
Summary:
  - Still really really rough.
  - Adds a full synchronous mode for debugging.
  - Adds some logging.
  - It can now allocate EC2 machines and put webroots on them in a hacky, terrible way.
  - Adds a base query class.

Test Plan: oh hey look a test page? http://ec2-50-18-65-151.us-west-1.compute.amazonaws.com:2011/

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D2026
2012-03-26 20:54:26 -07:00
epriestley
c0aac8267d Improve Diffusion behavior for externals
Summary:
  - Feature request from Airtime that I missed in the feedback notes, came up yesterday.
  - Identify git submodules as "FILE_SUBMODULE", not "FILE_NORMAL".
  - Link git submodules to an external resolver endpoint, which tries to find commits in tracked repositories.
  - Identify git symlinks as "FILE_SYMLINK", not "FILE_NORMAL".
  - Add folder, file, symlink and externals icons.

Test Plan:
  - externals/javelin is now identified as a submoudule and links to Javelin, not identified as a file and links to error.
  - bin/phd is now identified as a symlink.
  - Interfaces have pretty icons.

Reviewers: btrahan, cpiro, ddfisher, keebuhm, allenjohnashton

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1975
2012-03-21 14:01:20 -07:00
vrana
5c5ead2666 Fix links after D1921
Test Plan:
Search for some symbol. Click on the result. Verify that there is not // in URL.
Click on the link from generated exception.
View history in Diffusion, click on Browse.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1979
2012-03-21 13:33:57 -07:00
epriestley
9a5598118e When a URI fails lookup, just 404 if it's POST
Summary: It's rather confusing now since we'll "seamlessly" redirect you to the right URI, but drop the method and parameters.

Test Plan: Hit a bad URI with POST, got 404.

Reviewers: edward, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1965
2012-03-20 15:46:17 -07:00
epriestley
30ae22bfcf Fix many encoding and architecture problems in Diffusion request and URI handling
Summary:
Diffusion request/uri handling is currently a big, hastily ported mess. In particular, it has:

  - Tons and tons of duplicated code.
  - Bugs with handling unusual branch and file names.
  - An excessively large (and yet insufficiently expressive) API on DiffusionRequest, including a nonsensical concrete base class.
  - Other tools were doing hacky things like passing ":" branch names.

This diff attempts to fix these issues.

  - Make the base class abstract (it was concrete ONLY for "/diffusion/").
  - Move all URI generation to DiffusionRequest. Make the core static. Add unit tests.
  - Delete the 300 copies of URI generation code throughout Diffusion.
  - Move all URI parsing to DiffusionRequest. Make the core static. Add unit tests.
  - Add an appropriate static initializer for other callers.
  - Convert all code calling `newFromAphrontRequestDictionary` outside of Diffusion to the new `newFromDictionary` API.
  - Refactor static initializers to be sensibly-sized.
  - Refactor derived DiffusionRequest classes to remove duplicated code.
  - Properly encode branch names (fixes branches with "/", see <https://github.com/facebook/phabricator/issues/100>).
  - Properly encode path names (fixes issues in D1742).
  - Properly escape delimiter characters ";" and "$" in path names so files like "$100" are not interpreted as "line 100".
  - Fix a couple warnings.
  - Fix a couple lint issues.
  - Fix a bug where we would not parse filenames with spaces in them correctly in the Git browse query.
  - Fix a bug where Git change queries would fail unnecessarily.
  - Provide or improve some documentation.

This thing is pretty gigantic but also kind of hard to split up. If it's unreasonably difficult to review, let me know and I can take a stab at it though.

This supplants D1742.

Test Plan:
  - Used home, repository, branch, browse, change, history, diff (ajax), lastmodified (ajax) views of Diffusion.
  - Used Owners typeaheads and search.
  - Used diffusion.getrecentcommitsbypath method.
  - Pushed a change to an absurdly-named file on an absurdly-named branch, everything worked properly.

{F9185}

Reviewers: nh, vrana, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1921
2012-03-19 19:52:14 -07:00
epriestley
900190b2fe Add inline comments to Diffusion/Audit
Summary:
  - Add inline comments to Audits, like Differential.
  - Creates new storage for the comments in the Audits database.
  - Creates a new PhabricatorAuditInlineComment class, similar to DifferentialInlineComment.
  - Defines an Interface which Differential and Audit comments conform to.
  - Makes consumers of DifferentialInlineComments consume objects which implement that interface instead.
  - Adds save

NOTE: Some features are still missing! Wanted to cut this off before it got crazy:

  - Inline comments aren't shown in the main comment list.
  - Inline comments aren't shown in the emails.
  - Inline comments aren't previewed.

I'll followup with those but this was getting pretty big.

@vrana, does the SQL change look correct?

Test Plan:
  - Created, edited, deleted, replied to, reloaded and saved inline comments in Diffusion, on the left and right side of diffs.
  - Created, edited, deleted, replied to, reloaded and saved inline comments in Differentila, on the left and right side of primary and diff-versus-diff diffs.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1898
2012-03-14 12:56:01 -07:00
epriestley
11cccb98c2 Add "final" to more classes
Summary: No big surprises here, delted the unused "DarkConsole" class.

Test Plan: Ran 'testEverythingImplemented' to verify I wasn't finalizing anything we extend.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T795

Differential Revision: https://secure.phabricator.com/D1876
2012-03-13 11:18:11 -07:00
epriestley
f158b32a54 Minor, formalize changeset response class. 2012-03-12 21:39:05 -07:00
epriestley
b2890eeb0e Add "final" to all Phabricator "Controller" classes
Summary:
These are all unambiguously unextensible. Issues I hit:

  - Maniphest Change/Diff controllers, just consolidated them.
  - Some search controllers incorrectly extend from "Search" but should extend from "SearchBase". This has no runtime effects.
  - D1836 introduced a closure, which we don't handle correctly (somewhat on purpose; we target PHP 5.2). See T962.

Test Plan: Ran "testEverythingImplemented" unit test to identify classes extending from `final` classes. Resolved issues.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T795

Differential Revision: https://secure.phabricator.com/D1843
2012-03-09 15:46:25 -08:00
epriestley
180ccaffad Remove AprhontDefaultApplicationController
Summary: This class is unsused and completely useless.

Test Plan: Grepped for callsites.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1840
2012-03-09 07:53:40 -08:00
vrana
95eb3bcf09 Respect query string when redirecting from missing trailing /
Test Plan: /differential/filter/active/jakubv?status=all

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1798
2012-03-07 13:47:20 -08:00
epriestley
f8431bbfee Make Aphlict client somewhat more approachable
Summary: Provide a reasonable JS API for the Aphlict client. Provide an example behavior to invoke it.

Test Plan:
Ran "aphlict_server.js" with:

  $ sudo node aphlict_server.js

Loaded /aphlict/. Opened console. Got "hello" from the server every second.

Got reasonable errors with the server not present ("Security exception", but this is because it can't connect to port 843 to access the policy server).

Reviewers: ddfisher, keebuhm, allenjohnashton, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T944

Differential Revision: https://secure.phabricator.com/D1800
2012-03-06 20:14:03 -08:00
vrana
ad58491c6c Support /differential/filter/<filter>/<username>/
Summary: NOTE: I didn't add BC for ?phid=.

Test Plan:
/differential/
/differential/filter/active/
/differential/filter/active/epriestley/
/differential/filter/active/x/ - 404
/differential/filter/revisions/?status=open - search for epriestley
/differential/filter/revisions/epriestley/?status=open
/p/jakubv/

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T900

Differential Revision: https://secure.phabricator.com/D1797
2012-03-06 15:21:59 -08:00
David Fisher
639ed0faa6 Change All Search Boxes into Jump Navs
Summary:
- all search boxes are now jump navs (old functionality retained if none
  of the jump nav patterns match)
- added global keyboard shortcut '/' to focus the search box in the upper
  right

Test Plan:
- pressed '/' and noticed the search box gains keyboard focus
- triggered jump nav functionality from search box and saw it worked
- did a search which did not match a jump nav pattern and saw it worked
  (and searched in the selected context)
NOTE: The search box on the /search/ page is also changed to have jump
nav functionality. Old functionality is not impared. Still, this may not
be desirable.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1794
2012-03-05 19:52:26 -08:00
epriestley
f2caa6888e Simplify Owners interfaces to Audit
Summary:
  - Owners has "by user" commit views, but these are supplanted by the Audit views. Just nuke them.
  - Owners has "by package" commit views; consolidate these onto the package detail pages and link into Audit for full details.

Test Plan: Browsed all the Owners interfaces, clicked "View All ... Commits" buttons.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1764
2012-03-05 09:57:46 -08:00
vrana
eeb7d517c2 Always match full path in URI Mapper
Summary:
I am not sure if it is by purpose but Phabricator now process paths like
https://secure.phabricator.com/D1681-so-freaking-cool.
The reason is that there are bunch of rules with missing '$' at the end.
This mistake is so common and easy to create that I've rather removed all '$'
and changed the way how the key is processed.
I am not absolutelly sure if the '$' was missing in some rules by purpose but if
it is the case then we should rather add explicit '.*'.
This change is backwards compatible with custom maps ending with '$'. It is not
compatible with paths not ending with '$' by purpose.

Test Plan:
Visit /, /differential/, /differential/stats/revisions/, /D1681.
Run before and after:

  ./aphrontpath.php D123
  ./aphrontpath.php D123-cool
  ./aphrontpath.php /
  ./aphrontpath.php differential
  ./aphrontpath.php differential/
  ./aphrontpath.php differential/stats/revisions/
  ./aphrontpath.php /file/data/x/PHID-FILE-y/z

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1745
2012-03-01 15:27:03 -08:00
Bob Trahan
0327a5fc69 OAuthServer polish and random sauce
Summary:
This diff makes the OAuthServer more compliant with the spec by
- making it return well-formatted error codes with error types from the spec.
- making it respect the "state" variable, which is a transparent variable the
client passes and the server passes back
- making it be super, duper compliant with respect to redirect uris
-- if specified in authorization step, check if its valid relative to the client
registered URI and if so save it
-- if specified in authorization step, check if its been specified in the access
step and error if it doesn't match or doesn't exist
-- note we don't make any use of it in the access step which seems strange but
hey, that's what the spec says!
This diff makes the OAuthServer suck less by
- making the "cancel" button do something in the user authorization flow
- making the client list view and client edit view be a bit more usable around
client secrets
- fixing a few bugs I managed to introduce along the way

Test Plan:
- create a test phabricator client, updated my conf, and then linked and
unlinked phabricator to itself
- wrote some tests for PhabricatorOAuthServer -- they pass!
-- these validate the various validate URI checks
- tried a few important authorization calls
--
http://phabricator.dev/oauthserver/auth/?client_id=X&state=test&redirect_uri=http://www.evil.com
--- verified error'd from mismatching redirect uri's
--- verified state parameter in response
--- verified did not redirect to client redirect uri
-- http://phabricator.dev/oauthserver/auth/?client_id=X w/ existing
authorization
--- got redirected to proper client url with error that response_type not
specified
-- http://phabricator.dev/oauthserver/auth/?client_id=X&response_type=code w/
existing authorization
--- got redirected to proper client url with pertinent code!
- tried a few important access calls
-- verified appropriate errors if missing any required parameters
-- verified good access code with appropriate other variables resulted in an
access token
- verified that if redirect_uri set correctly in authorization required for
access and errors if differs at all / only succeeds if exactly the same

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, ajtrichards

Maniphest Tasks: T889, T906, T897

Differential Revision: https://secure.phabricator.com/D1727
2012-03-01 14:46:18 -08:00
epriestley
28f5d9f227 Remove old audit edit form in favor of Diffusion form
Summary: Since we embed comments/audits into Diffusion now, we don't need the
old edit interface.

Test Plan: Grepped for links to old interface.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1714
2012-02-28 21:13:14 -08:00
epriestley
1eeaeb62e4 Remove commit list from Diffusion in favor of Audit commit list
Summary:
We can drive this query better from the Audit tool now; get rid of the Diffusion
version.

Preserve usernames in URIs as per T900.

Test Plan: Clicked "Commits" from profile. Browsed audit commit filters.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1713
2012-02-28 21:12:08 -08:00
epriestley
280d7cd294 Add excel export to Maniphest
Summary:
Allow Maniphest result sets to be exported to Excel.

Spreadsheet_Excel_Writer is awful but comparatively easy to get working. There's
also a "PHPExcel" package but it has some autoload conflicts right now and this
seems good-enough.

Test Plan: Exported a bunch of tasks to Excel.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T923

Differential Revision: https://secure.phabricator.com/D1721
2012-02-28 21:07:12 -08:00
epriestley
c7094d2def Add preview and drafts to audits
Summary: Add comment previews and saved drafts to audits, like Maniphest /
Differential.

Test Plan: Typed stuff into the box. Got a preview. Reloaded page. Stuff was
still there. Submitted comment. Stuff is gone.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1699
2012-02-27 13:00:23 -08:00
epriestley
1094527072 Allow Herald to trigger audits for users or projects
Summary:
Allows you to write a commit rule that triggers an audit by a user (personal
rules) or a project (global rules).

Mostly this is trying to make auditing more lightweight and accessible in
environments where setting up Owners packages doesn't make sense.

For instance, Disqus wants a rule like "trigger an audit for everything that
didn't have a Differential revision". While not necessarily scalable, this is a
perfectly reasonable rule for a small company, but a lot of work to implement
with Owners (and you'll get a lot of collateral damage if you don't make every
committer a project owner).

Instead, they can create a project called 'Unreviewed Commits' and write a rule
like:

	- When: Differential revision does not exist
 	- Action: Trigger an Audit for project: "Unreviewed Commits"

Then whoever cares can join that project and they'll see those audits in their
queue, and when they approve/raise on commits their actions will affect the
project audit.

Similarly, if I want to look at all commits that match some other rule (say,
XSS) but only want to do it like once a month, I can just set up an audit rule
and go through the queue when I feel like it.

NOTE: This abuses the 'packagePHID' field to also store user and project PHIDs.
Through the magic of handles, this (apparently) works fine for now; I'll do a
big schema patch soon but have several other edits I want to make at the same
time.

Also:

	- Adds an "active" fiew for /audit/, eventually this will be like the
Differential "active" view (stuff that is relevant to you right now).
	- On commits, highlight triggered audits you are responsible for.

Test Plan: Added personal and global audit triggers to Herald, reparsed some
commits with --herald, got audits. Browsed all audit interfaces to make sure
nothing exploded. Viewed a commit where I was responsible for only some audits.
Performed audits and made sure the triggers I am supposed to be responsible for
updated properly.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1690
2012-02-27 09:36:30 -08:00
Korvin Szanto
e24a6acf58 Multiline Highlighting in Diffusion
Summary:
I added multiline highlighting with the syntax:

  http://site/path/to/file$from-to

NOTE: you can reverse the from and to

Test Plan: Open a file in diffusion and attempt to highlight multiple lines

Reviewers: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1693
2012-02-25 12:32:59 -08:00
epriestley
e5f3ad14e1 Allow audit comments to be added from Diffusion
Summary:
This is intended to supplant the existing "audit edit" interface. I've changed
them to both drive down the same write pathway, but the UIs are still different.
I'll fully merge them in a future diff.

Add a comment box (like Maniphest and Differential) to Diffusion. When users
make comments, their comments appear on the commit. Any audits triggers they are
responsible for are updated to reflect actions they take, as well.

Currently, audits can only be triggered by packages, but I intend to allow them
to be triggered by users and projects (via herald rules) in an upcoming diff.
Thus some of the language like "projects, users or packages" when the code is
clearly dealing only with "packagePHID".

Test Plan: Made audit updates via commit interface and via existing edit
interface. Verified both interfaces updated correctly, and that audit
responsibility rules were applied properly.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1688
2012-02-24 15:04:53 -08:00
Bob Trahan
3c4070a168 OAuth Server -- add controllers to RUD client authorizations and CRUD clients
Summary:
beyond the title, this diff tweaks the test console to have a bit more
functionality.  also makes a small change to CSS for AphrontFormControlMarkup,
which IMO fixes a display issue on
https://secure.phabricator.com/settings/page/profile/ where the Profile URI is
all up in the air and whatnot

I think this is missing pagination.  I am getting tired of the size though and
will add later.  See T905.

Test Plan:
viewed, updated and deleted client authorizations.  viewed, created,
updated and deleted clients

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T849, T850, T848

Differential Revision: https://secure.phabricator.com/D1683
2012-02-24 14:56:18 -08:00
epriestley
97ea6ea619 Add a basic first-class audit UI
Summary:
Currently, audits are only accessible through the Owners tool. Start moving them
to their own first-class tool in preparation for broader audit integration.

  - Lay some infrastructure groundwork (e.g. AuditQuery).
  - Build a basic /audit/ view.
  - Show audits on the commit page in Diffusion.

This has some code duplication with stuff we've already got, but I'll merge
everything together as we move forward on this.

Test Plan: Looked at /audit/ and a commit.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1685
2012-02-24 13:02:14 -08:00
epriestley
386dcfff7e Rough batch editor for Maniphest
Summary:
First stab at a batch editor for Maniphest. Basically, you can select a group of
tasks and then import them into the "batch" interface, where you can edit all of
them at once.

High level goal is to make it easier for users in PM/filer/support/QA roles to
deal with large numbers of tasks quickly.

This implementation has a few major limitations:

  - The only available actions are "add projects" and "remove projects".
  - There is no review / undo / log stuff.
  - All the changes are applied in-process, which may not scale terribly well.

However, the immediate need is just around projects and this seemed like a
reasonable place to draw the line for a minimal useful version of the tool.

Test Plan: Used batch editor to add and remove projects from groups of tasks.

Reviewers: btrahan, yairlivne

Reviewed By: btrahan

CC: aran, epriestley, sandra

Maniphest Tasks: T441

Differential Revision: https://secure.phabricator.com/D1680
2012-02-24 13:00:48 -08:00
epriestley
84afd3d469 Fix a minor parameter issue
Summary: D1595 split encodeJSONForHTTPResponse() into two methods, but left a
straggling $use_javelin_shield parameter which is no longer used.

Test Plan: Caught errors in error log.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1663
2012-02-22 10:06:06 -08:00