1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-01 03:02:43 +01:00
Commit graph

5710 commits

Author SHA1 Message Date
Chad Little
00d9aae894 Merge branch 'master' of github.com:facebook/phabricator 2013-10-29 16:13:49 -07:00
Chad Little
f82cb0e063 Clean up Phriction History View
Summary: Fixes T4028, makes the table a bit easier to read and navigate changesets.

Test Plan: Tested a big history, back to original and latest.

Reviewers: epriestley, btrahan

CC: Korvin, epriestley, aran

Maniphest Tasks: T4028

Differential Revision: https://secure.phabricator.com/D7438
2013-10-29 16:07:13 -07:00
epriestley
c749fcc192 Reject SCP-style URIs with explicit protocols in Diffusion
Summary:
Fixes T3619. These URIs are valid:

  git@domain.com:/path        (Git SCP-style implicit SSH)
  ssh://git@domain.com/path   (Explicit SSH)

This URI, arrived at by adding "ssh://" to the front of an SCP-style URI, is not:

  ssh://git@domain.com:/path

Detect URIs in this form and reject them. See T3619.

Test Plan:
{F75486}

Also set some valid URIs.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3619

Differential Revision: https://secure.phabricator.com/D7431
2013-10-29 15:32:51 -07:00
epriestley
59922b78b9 Make Phabricator clone bare git repositories
Summary:
This doesn't really impact anything very much, but is a little cleaner than cloning repositories with a working copy. It's somewhat important for allowing pushes, because you can't push to a checked-out branch.

Mercurial has a similar option (`--noupdate`) but leave that alone for now.

The origin stuff was mostly for sanity/explicitness purposes -- I believe it's safe to remove in all non-ridiculous cases. Git fails with it in bare repositories (it automatically creates an `origin`, but doesn't create the local refs for it, or something).

Test Plan: Nuked a repo, re-cloned it, pulled and updated it several times. Browsed both bare and non-bare repos in Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7430
2013-10-29 15:32:41 -07:00
epriestley
d8bda7c66e Add an "importing" state to repositories and clean up the UI
Summary:
Fixes T3217. Ref T776. Ref T1493. Broadly, this introduces a mechanism which works like this:

  - When a repository is created, we set an "importing" flag.
  - After discovery completes, we check if a repository has no importing commits. Basically, this is the first time we catch up to HEAD.
  - If we're caught up, clear the "importing" flag.

This flag lets us fix some issues:

  - T3217. Currently, when you import a new repository and users have rules like "Email me on every commit ever" or "trigger an audit on every commit", we take a bunch of publish actions. Instead, implicitly disable publishing during import.
  - An imported but un-pulled repository currently has an incomprehensible error on `/diffusion/X/`. Fix that.
  - Show more cues in the UI about importing.
  - Made some exceptions more specific.

Test Plan:
This is the new screen for a completely new repo, replacing a giant exception:

{F75443}

  - Created a repository, saw it "importing".
  - Pulled and discovered it.
  - Processed its commits.
  - Ran discovery again, saw import flag clear.
  - Also this repository was empty, which hit some of the other code.

This is the new "parsed empty repository" UI, which isn't good, but is less broken:

{F75446}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, hach-que

Maniphest Tasks: T3607, T1493, T776, T3217

Differential Revision: https://secure.phabricator.com/D7429
2013-10-29 15:32:41 -07:00
epriestley
9125095587 Distinguish between empty and unparsed commits in Diffusion
Summary:
Fixes T3416. Fixes T1733.

  - Adds a flag to the commit table showing whether or not we have parsed it.
  - The flag is set to `0` initially when the commit is discovered.
  - The flag is set to `1` when the changes are parsed.
  - The UI can now use the flag to distinguish between "empty commit" and "commit which we haven't imported changes for yet".
  - Simplify rendering code a little bit.
  - Fix an issue with the Message parser for empty commits.
  - There's a key on the flag so we can do `SELECT * FROM repository_commit WHERE repositoryID = %d AND importStatus = 0 LIMIT 1` soon, to determine if a repository is fully imported or not. This will let us improve the UI (Ref T776, Ref T3217).

Test Plan:
  - Ran `bin/storage upgrade -f`.
  - Created an empty commit.
  - Without the daemons running, ran `bin/repository pull GTEST` and `bin/repository discover GTEST`.
  - Viewed web UI to get the first screenshot ("Still Importing...").
  - Ran the message and change steps with `scripts/repository/reparse.php`.
  - Viewed web UI to get the second screenshot ("Empty Commit").

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T776, T1733, T3416, T3217

Differential Revision: https://secure.phabricator.com/D7428
2013-10-29 15:32:41 -07:00
epriestley
0965f0041a Don't pull updates for repositories marked as hosted
Summary:
  - Don't try to pull hosted repos.
  - Also, fix the `--verbose` + `--trace` interaction for `bin/repository`.
  - Also, fix a couple of unit tests which got tweaked earlier.

Test Plan:
  $ ./bin/repository pull GTEST --verbose
  Pulling 'GTEST'...
  Repository "GTEST" is hosted, so Phabricator does not pull updates for it.
  Done.

Reviewers: btrahan, hach-que

Reviewed By: hach-que

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7427
2013-10-29 15:32:41 -07:00
epriestley
d02202cde2 Enable "SSH Keys" auth panel unconditionally
Summary: We've had support for this for a long time, but it was conditional on config. Since it more-or-less actually does something now, just enable it unconditionally.

Test Plan: Settings -> SSH Public Keys

Reviewers: btrahan

Reviewed By: btrahan

CC: hach-que, aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7426
2013-10-29 15:32:41 -07:00
epriestley
c6665b1907 Serve git writes over SSH
Summary: Looks like this is pretty straightforward; same as the reads except mark it as needing PUSH.

Test Plan: Ran `git push`, pushed over SSH to a hosted repo.

Reviewers: btrahan

Reviewed By: btrahan

CC: hach-que, aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7425
2013-10-29 15:32:41 -07:00
epriestley
9a2e45ef12 Serve Git reads over SSH
Summary: Like D7423, but for SSH.

Test Plan: Ran `git clone ssh://...`, got a clone.

Reviewers: btrahan

Reviewed By: btrahan

CC: hach-que, aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7424
2013-10-29 15:32:40 -07:00
epriestley
7d9dfb561d Serve Git reads over HTTP
Summary: Mostly ripped from D7391. No writes yet.

Test Plan: Ran `git clone` against a local over HTTP, got a clone.

Reviewers: btrahan, hach-que

Reviewed By: hach-que

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7423
2013-10-29 15:32:40 -07:00
epriestley
bb4904553f Route some VCS connections over SSH
Summary:
  - Add web UI for configuring SSH hosting.
  - Route git reads (`git-upload-pack` over SSH).

Test Plan:
  >>> orbital ~ $ git clone ssh://127.0.0.1/
  Cloning into '127.0.0.1'...
  Exception: Unrecognized repository path "/". Expected a path like "/diffusion/X/".
  fatal: Could not read from remote repository.

  Please make sure you have the correct access rights
  and the repository exists.
  >>> orbital ~ $ git clone ssh://127.0.0.1/diffusion/X/
  Cloning into 'X'...
  Exception: No repository "X" exists!
  fatal: Could not read from remote repository.

  Please make sure you have the correct access rights
  and the repository exists.
  >>> orbital ~ $ git clone ssh://127.0.0.1/diffusion/MT/
  Cloning into 'MT'...
  Exception: This repository is not available over SSH.
  fatal: Could not read from remote repository.

  Please make sure you have the correct access rights
  and the repository exists.
  >>> orbital ~ $ git clone ssh://127.0.0.1/diffusion/P/
  Cloning into 'P'...
  Exception: TODO: Implement serve over SSH.
  fatal: Could not read from remote repository.

  Please make sure you have the correct access rights
  and the repository exists.

Reviewers: btrahan

Reviewed By: btrahan

CC: hach-que, aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7421
2013-10-29 15:32:40 -07:00
epriestley
888b3839e7 Prepare to route VCS connections through SSH
Summary:
Fixes T2229. This sets the stage for a patch similar to D7417, but for SSH. In particular, SSH 6.2 introduced an `AuthorizedKeysCommand` directive, which lets us do this in a mostly-reasonable way without needing users to patch sshd (if they have a recent enough version, at least).

The way the `AuthorizedKeysCommand` works is that it gets run and produces an `authorized_keys`-style file fragment. This isn't ideal, because we have to dump every key into the result, but should be fine for most installs. The earlier patch against `sshd` passes the public key itself, which allows the script to just look up the key. We might use this eventually, since it can scale much better, so I haven't removed it.

Generally, auth is split into two scripts now which mostly do the same thing:

  - `ssh-auth` is the AuthorizedKeysCommand auth, which takes nothing and dumps the whole keyfile.
  - `ssh-auth-key` is the slightly cleaner and more scalable (but patch-dependent) version, which takes the public key and dumps only matching options.

I also reworked the argument parsing to be a bit more sane.

Test Plan:
This is somewhat-intentionally a bit obtuse since I don't really want anyone using it yet, but basically:

  - Copy `phabricator-ssh-hook.sh` to somewhere like `/usr/libexec/openssh/`, chown it `root` and chmod it `500`.
    - This script should probably also do a username check in the future.
  - Create a copy of `sshd_config` and fix the paths/etc. Point the KeyScript at your copy of the hook.
  - Start a copy of sshd (6.2 or newer) with `-f <your config file>` and maybe `-d -d -d` to foreground and debug.
  - Run `ssh -p 2222 localhost` or similar.

Specifically, I did this setup and then ran a bunch of commands like:

  - `ssh host` (denied, no command)
  - `ssh host ls` (denied, not supported)
  - `echo '{}' | ssh host conduit conduit.ping` (works)

Reviewers: btrahan

Reviewed By: btrahan

CC: hach-que, aran

Maniphest Tasks: T2229, T2230

Differential Revision: https://secure.phabricator.com/D7419
2013-10-29 15:32:40 -07:00
epriestley
c7f23f522a Accept and route VCS HTTP requests
Summary:
Mostly ripped from D7391, with some changes:

  - Serve repositories at `/diffusion/X/`, with no special `/git/` or `/serve/` URI component.
    - This requires a little bit of magic, but I got the magic working for Git, Mercurial and SVN, and it seems reasonable.
    - I think having one URI for everything will make it easier for users to understand.
    - One downside is that git will clone into `X` by default, but I think that's not a big deal, and we can work around that in the future easily enough.
  - Accept HTTP requests for Git, SVN and Mercurial repositories.
  - Auth logic is a little different in order to be more consistent with how other things work.
  - Instead of AphrontBasicAuthResponse, added "VCSResponse". Mercurial can print strings we send it on the CLI if we're careful, so support that. I did a fair amount of digging and didn't have any luck with git or svn.
  - Commands we don't know about are assumed to require "Push" capability by default.

No actual VCS data going over the wire yet.

Test Plan:
Ran a bunch of stuff like this:

  $ hg clone http://local.aphront.com:8080/diffusion/P/
  abort: HTTP Error 403: This repository is not available over HTTP.

...and got pretty reasonable-seeming errors in all cases. All this can do is produce errors for now.

Reviewers: hach-que, btrahan

Reviewed By: hach-que

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7417
2013-10-29 15:32:40 -07:00
epriestley
bb35f8ec9f Add hosting, serving, and push policy options to repository edit
Summary:
Basically straight from D7391. The differences are basically:

  - Policy stuff is all application-scope instead of global-scope.
  - Made a few strings a little nicer.
  - Deleted a bit of dead code.
  - Added a big "THIS DOESN'T WORK YET" warning.

Test Plan: See screenshots.

Reviewers: hach-que, btrahan

Reviewed By: hach-que

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7416
2013-10-29 15:32:40 -07:00
epriestley
86fe020a97 Add global "push" policy to Repositories
Summary: No editing or view yet, just adds the schema and a policy default. Part of D7391.

Test Plan: `bin/storage upgrade`

Reviewers: hach-que, btrahan

Reviewed By: hach-que

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7415
2013-10-29 15:32:40 -07:00
epriestley
ba63c5e426 Ship "Repositories" create button to new Diffusion workflow
Summary: Ref T2231. Get rid of the old create controller and make the button go to the new stuff instead. This will eventually get cleaned up more, but I don't have a clear plan for Arcanist Projects yet.

Test Plan: Clicked button, hit new workflow.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7414
2013-10-29 15:32:39 -07:00
epriestley
5f6ea9f662 Activate the new Repository creation workflow
Summary:
Ref T2231. This:

  - Activates the new multi-step workflow, and exposes it in the UI.
  - Adds "can create", "default view" and "default edit" capabilities.
  - Provides a default value for `repository.default-local-path` and forces repositories into it by default. It's still editable, but Phabricator gets it correct (for some definition of correct) by default now.

Test Plan: Created some new repositories with the new workflow.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1286, T2231

Differential Revision: https://secure.phabricator.com/D7413
2013-10-29 15:32:39 -07:00
epriestley
c585f97e90 Remove horrible old repository edit controller
Summary:
Ref T2231. I didn't port these options over, so they're still supported but have no edit UI:

  - Pull Frequency (confusing/not useful, I think?)
  - Default Owners Path (probably used only by Facebook and only in the E repository)
  - Show user in public repository URL (probably mostly obsolete with hosting?)

We can add those back if users notice, but they seem like the three least useful options so I'm going to see if we can get away with removing them.

Test Plan: Clicked "Edit" from Repositories, got kicked into the nice new Diffusion edit UI instead of the old one.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7410
2013-10-29 15:32:39 -07:00
Chad Little
6bdedfe679 Add ObjectBox to Condiut Cert Page
Summary: adds a box, Fixes T4033

Test Plan: look at box

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4033

Differential Revision: https://secure.phabricator.com/D7437
2013-10-29 14:54:10 -07:00
Chad Little
ec83a844f8 Proteins 2013-10-29 14:23:30 -07:00
epriestley
d1ed816e61 Move "Delete Repository" stuff to Diffusion
Summary: Ref T2231. This just moves the "Delete" dialog from Repositories to Diffusion. This dialog just shows instructions and isn't interesting.

Test Plan: {F75093}

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7412
2013-10-29 12:26:07 -07:00
epriestley
7c23960de8 Use status header stuff for "Edit Repository"
Summary: Ref T2231. Use status info element instead of tags.

Test Plan: {F75092}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7411
2013-10-29 12:24:03 -07:00
epriestley
c4cdb5c5f0 Move editing "Local Path" to modern UI/controller/etc
Summary: Fixes T1286. Ref T2231. See previous diffs; same as the others but does "Local Path".

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1286, T2231

Differential Revision: https://secure.phabricator.com/D7409
2013-10-29 12:20:26 -07:00
Chad Little
e1683f353e Add plain status icon colors
Summary: This adds a set of plain (blueish) icons for use instead of black.

Test Plan: Photoshop

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7434
2013-10-28 09:25:18 -07:00
Chad Little
e04eeea7a5 Revert changeset css changes
Summary: I changed some CSS here that was not intended.

Test Plan: Reload a few Diffs, no bottom padding at last diff now.

Reviewers: epriestley, btrahan

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7433
2013-10-28 08:27:41 -07:00
Chad Little
7757c9c867 Touch up comments are hidden box
Summary: Adds consistent spacing, color to this interactive box. Also, I changed it to yellow, if its too much lemme know but feels ok.

Test Plan: tested new colors in differential

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7422
2013-10-26 14:42:32 -07:00
Chad Little
fe8751bea6 Fix Remarkup sprite, tweak Transactions
Summary: This fixes the sprite for remarkup and does some minor tweaks for transactions (so Differential looks a little more like timeline)

Test Plan: tested remarkup, differential

Reviewers: epriestley, btrahan

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7418
2013-10-26 08:43:34 -07:00
epriestley
796db9f9c6 Sort out application crumbs in new repository edit workflow
Summary: Ref T2231. Crumbs in the Diffusion edit workflow are a bit wonky, with stuff like "rP (master)" which isn't very useful and no link back to the main "Edit" page. Make them consistent across all the screens.

Test Plan: Loaded a bunch of these screens and saw sane crumbs on all of them.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7407
2013-10-25 15:58:58 -07:00
epriestley
1975bdfb36 Work around a bug in PHP 5.3-ish with abstract methods in interfaces
Summary:
@chad is hitting an issue described in P961, which I think is this bug in PHP: https://bugs.php.net/bug.php?id=43200

Work around it by defining a "PHIDInterface" and having both "Flaggable" and "Policy" extend it, so that there is only one `getPHID()` declaration.

Test Plan: shrug~

Reviewers: chad, btrahan

Reviewed By: chad

CC: chad, aran

Differential Revision: https://secure.phabricator.com/D7408
2013-10-25 15:58:17 -07:00
epriestley
e81bad9ba2 Improve consistency of policy enforcement on new repository edit UI
Summary: Ref T2231. The policy rules are a little murky right now: the "Edit Repository" link requires CAN_EDIT, but the actualy page doesn't. Instead, require CAN_EDIT for the edit page.

Test Plan: As a user without CAN_EDIT, viewed a repository and clicked the edit link.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7406
2013-10-25 15:58:02 -07:00
epriestley
b57b72368c Move "Remote URI" to modern repository editor thing
Summary:
Ref T2231. Allows you to edit the remote URI and credentials.

This is a little bit funky because I'm reusing some of the pages on the new (not-yet-hooked-up) create form. Specifically, it had pages like this:

  - Repo Type
  - Name/Callsign/Remote
  - Auth
  - Done

I split "Name/Callsign/Remote" into "Name/Callsign" and "Remote", then when editing the remote I just take you through "Remote" and "Auth" and then back. This lets us reuse the giant pile of protocol/URI sanity checking logic and ends up being pretty clean, although it's a little weird that the "Create" controller does both full-create and edit-remote.

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7405
2013-10-25 13:59:02 -07:00
epriestley
52d4e66883 Move repository actions (notify, autoclose) to new UI
Summary: Ref T2231. Brings "Notify/Publish" and "Autoclose" to the new UI.

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7402
2013-10-25 13:58:15 -07:00
epriestley
49670e1a56 Move Subversion repository information to the new interface
Summary: Ref T2231. Moves "UUID" and "Subpath/Import Only" to Diffusion.

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7400
2013-10-25 13:58:03 -07:00
epriestley
dcb0b1b64f Add support for branch-related configuration to new Repository edit workflow
Summary: Ref T2231. Modernizes editing "Default Branch", "Track Only", and "Autoclose Only".

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D7399
2013-10-25 13:57:14 -07:00
Bob Trahan
64f3c9b3da Forgot a pht on my last diff
Summary: also add a few more words

Test Plan: looks good!

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7404
2013-10-25 12:57:42 -07:00
Bob Trahan
da84546058 Add filter by object ability to flag query
Summary: See title. Fixes T1809.

Test Plan:
verified each type that has flaggable interface still can be flagged

verified that new custom query filter works

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1809

Differential Revision: https://secure.phabricator.com/D7392
2013-10-25 12:52:00 -07:00
Erik Fercak
203d82083a Allow prefilling a task's assignee by his PHID
Summary: Some scripts might find it easier to work with PHIDs instead of user names.

Test Plan:
Use ?assign=<username> and ?assign=<PHID-USER> with the create task URI.
See assignee input being filled correctly.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: epriestley, aran, Korvin

Differential Revision: https://secure.phabricator.com/D7401
2013-10-25 11:35:35 -07:00
Chad Little
e478706769 PHUIInfoPanel
Summary: First cut of an 'info panel' for phabricator. Basic concept is for display a list of items with a bit more info and depth and an object item list. Projects could be a good first example.

Test Plan: UIExamples

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7398
2013-10-25 11:09:06 -07:00
Erik Fercak
1fb2f39fc0 Enable prefilling of projects and priority fields in Maniphest task creation
Summary:
Projects and priority inputs can be prefilled similar to how title
and description fields work.

Prefilling of projects already worked but used PHIDs instead of
more user friendly name so I changed that too.

Test Plan:
Visit [[/maniphest/task/create/?projects=Maniphest;Easy&priority=100&assign=vrana&title=Hip-hip&description=hooray!|example]]
and see prefilled form fields.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7394
2013-10-25 10:20:32 -07:00
epriestley
6a169de967 Update install documentation to include FreeBSD
Summary:
Fixes T4024.

  - Mention FreeBSD by name.
  - Make "install this on a normal computer" more explicit.
  - Make "install this on a entire domain" more explicit.

Test Plan:
{F74892}

{F74891}

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T4024

Differential Revision: https://secure.phabricator.com/D7397
2013-10-25 08:58:42 -07:00
epriestley
0c2b4dbfff Fix header of "Change Password" form
Summary: Fixes T4025.

Test Plan: {F74873}

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T4025

Differential Revision: https://secure.phabricator.com/D7395
2013-10-25 08:57:35 -07:00
epriestley
9e87172166 Make remarkup rules runtime-pluggable in a reasonable way
Summary:
Gets rid of some old Differential-specific nonsense and replaces it with general runtime-pluggable Remarkup rules.

Facebook: This removes two options which may be in use. Have any classes being added via config here just subclass the new abstract bases instead. This should take 5 seconds to fix. You can adjust order by overriding `getPriority()` on the rules, if necessary.

Test Plan: See comments.

Reviewers: btrahan

Reviewed By: btrahan

CC: FacebookPOC, andrewjcg, aran

Differential Revision: https://secure.phabricator.com/D7393
2013-10-24 17:26:07 -07:00
Chad Little
87a440888d Minor Differential CSS
Summary: More Diffusion/Differential touch ups, ToC, etc.

Test Plan: Look at colors, see that they match or look better.

Reviewers: epriestley, btrahan

Reviewed By: btrahan

CC: Korvin, epriestley, aran

Maniphest Tasks: T3952

Differential Revision: https://secure.phabricator.com/D7386
2013-10-23 13:35:20 -07:00
Chad Little
18931ec4c0 Update Apps Installed icons to match Projects.
Summary: Changes to checkmark and crossed circle to match active projects

Test Plan: installed and uninstalled an application. poor conpherence.

Reviewers: epriestley, btrahan

Reviewed By: btrahan

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7390
2013-10-23 13:28:47 -07:00
Chad Little
2b21b4e880 Add search app icon
Summary: It's an icon. For Search. Fixes T3966

Test Plan: Search

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3966

Differential Revision: https://secure.phabricator.com/D7389
2013-10-23 11:30:52 -07:00
Chad Little
e73ecb96e0 White hover policy icons
Summary: Makes a white hover icon show on the policy dropdown. Also fixed some spacing. Fixes T4017

Test Plan: hover over the policy dropdown

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4017

Differential Revision: https://secure.phabricator.com/D7388
2013-10-23 10:16:09 -07:00
epriestley
2c3d071a26 Improve exception behavior for Herald commit rules which fail to load diff context
Summary:
This code is a little funky right now, and can return `array("error message")` and then try to call `getHunks()` on it. Additionally, each field loads the commit's changes separately.

Instead, load the commit's changes once and cache them, and handle exceptions appropriately.

Test Plan:
  - Created a rule like "changed, added, removed content all match /.*/" to force all fields to generate.
  - Ran it successfully.
  - Faked an error and ran it, got reasonable results.

Reviewers: btrahan

Reviewed By: btrahan

CC: bigo, aran

Differential Revision: https://secure.phabricator.com/D7384
2013-10-23 08:28:58 -07:00
epriestley
b5a009337f Harbormaster v(-2)
Summary:
Ref T1049. I don't really want to sink too much time into this right now, but a seemingly reasonable architecture came to me in a dream. Here's a high-level overview of how things fit together:

  - **"Build"**: In Harbormaster, "build" means any process we want to run against a working copy. It might actually be building an executable, but it might also be running lint, running unit tests, generating documentation, generating symbols, running a deploy, setting up a sandcastle, etc.
  - `HarbormasterBuildable`: A "buildable" is some piece of code which build operations can run on. Generally, this is either a Differential diff or a Diffusion commit. The Buildable class just wraps those objects and provides a layer of abstraction. Currently, you can manually create a buildable from a commit. In the future, this will be done automatically.
  - `HarbormasterBuildStep`: A "build step" is an individual build operation, like "run lint", "run unit", "build docs", etc. The step defines how to perform the operation (for example, "run unit tests by executing 'arc unit'"). In this diff, this barely exists.
  - `HarbormasterBuildPlan`: This glues together build steps into groups or sequences. For example, you might want to "run unit", and then "deploy" if the tests pass. You can create a build plan which says "run step "unit tests", then run step "deploy" on success" or whatever. In the future, these will also contain triggers/conditions ("Automatically run this build plan against every commit") and probably be able to define failure actions ("If this plan fails, send someone an email"). Because build plans will run commands, only administrators can manage them.
  - `HarbormasterBuild`: This is the concrete result of running a `BuildPlan` against a `Buildable`. It tracks the build status and collects results, so you can see if the build is running/successful/failed. A `Buildable` may have several `Build`s, because you can execute more than one `BuildPlan` against it. For example, you might have a "documentation" build plan which you run continuously against HEAD, but a "unit" build plan which you want to run against every commit.
  - `HarbormasterBuildTarget`: This is the concrete result of running a `BuildStep` against a `Buildable`. These are children of `Build`. A step might be able to produce multiple targets, but generally this is something like "Unit Tests" or "Lint" and has an overall status, so you can see at a glance that unit tests were fine but lint had some issues.
  - `HarbormasterBuildItem`: An optional subitem for a target. For lint, this might be an individual file. For unit tests, an individual test. For normal builds, an executable. For deploys, a server. For documentation generation, there might just not be subitems.
  - `HarbormasterBuildLog`: Provides extra information, like command/execution transcripts. This is where stdout/stderr will get dumped, and general details and other messages.
  - `HarbormasterBuildArtifact`: Stores side effects or results from build steps. For example, something which builds a binary might put the binary in "Files" and then put its PHID here. Unit tests might put coverage information here. Generally, any build step which produces some high-level output object can use this table to record its existence.

This diff implements almost nothing and does nothing useful, but puts most of these object relationships in place. The two major things you can't easily do with these objects are:

  1) Run arbitrary cron jobs. Jenkins does this, but it feels tacked on and I don't know of anyone using it for that. We could create fake Buildables to get a similar effect, but if we need to do this I'd rather do it elsewhere in general. Build and cron/service/monitoring feel like pretty different problems to me.
  2) Run parameterized/matrix steps (maybe?). Bamboo has this plan/stage/task/job breakdown where a build step can generate a zillion actual jobs, like "build client on x86", "build server on x86", "build client on ARM", "build server on ARM", etc. We can sort of do this by having a Step map to multiple Targets, but I haven't really thought about it too much and it may end up being not-great. I'd guess we have like an 80% chance of getting a clean implementation if/when we get there. I suspect no one actually needs this, or when they do they'll just implement a custom Step and it can be parameterized at that level. I'm not too worried about this overall.

The major difference between this and Jenkins/Bamboo/TravisCI is that all three of those are **plan-centric**: the primary object in the system is a build plan, and the dashboard shows you all your build plans and the current status. I don't think this is the right model. One disadvantage is that you basically end up with top-level messaging that says "Trunk is broken", not "Trunk was broken by commit af32f392f". Harbormaster is **buildable-centric**: the primary object in the system is stuff you can run build operations against (commits/branches/revisions), and actual build plans are secondary. The main view will be "recent commits on this branch, and whether they're good or not" -- which I think is what's most important in a larger/more complex product -- not the pass/fail status of all jobs. This also makes it easier and more natural to integrate with Differential and Diffusion, which both care about the overall status of the commit/revision, not the current status of jobs.

Test Plan: Poked around, but this doesn't really do anything yet.

Reviewers: btrahan

Reviewed By: btrahan

CC: zeeg, chad, aran, seporaitis

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7368
2013-10-22 15:01:06 -07:00
epriestley
fd27538e89 Add project history and title strings
Summary: Ref T4010. Adds a history page and restores the transaction title strings, which previously sort-of existed in the defunct feed story class.

Test Plan: See screenshots.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T4010

Differential Revision: https://secure.phabricator.com/D7371
2013-10-22 13:49:37 -07:00
epriestley
9b89e137cf Move Project transaction storage to modern tables
Summary:
Ref T4010. Projects have a weird proto-version of ApplicationTransactions which is very similar but not quite the same.

Move the storage to a modern format, but keep all the other code for now.

Test Plan: Migrated project transactions; edited projects.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4010

Differential Revision: https://secure.phabricator.com/D7370
2013-10-22 13:49:28 -07:00
epriestley
90b83d7a92 Fix logged-out Diffusion calls to Conduit
Summary:
Conduit doesn't currently have an analog to "shouldAllowPublic", so the recent policy checks added here caught legitimate Conduit calls when viewing Diffusion as a logged-out user.

Add `shouldAllowPublic()` and set it for all the Diffusion queries.

(More calls probably need this, but we can add it when we hit them.)

Test Plan: Looked at Diffusion as a logged-out user with public access enabled.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7380
2013-10-22 13:47:52 -07:00
epriestley
7dd31a16d9 Fix chatlog application query integration
Summary: `class_exists()` is case-insensitive, but `PhabricatorApplication::getByClass()` is not.

Test Plan: Fixed unit test to fail, then fixed code to pass unit test.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7379
2013-10-22 13:47:47 -07:00
Chad Little
e14ba56394 Add top border back to section-headers
Summary: This adds back the top border on section headers and cleans up the tab CSS just a hair.

Test Plan: tested files, tasks, and custom field profile.

Reviewers: epriestley, btrahan

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7378
2013-10-22 08:49:13 -07:00
epriestley
d7a276346f Add a secret board view to Projects
Summary:
Ref T1344. This is //very// rough. Some UI issues:

  - Empty states for the board and columns are junky.
  - Column widths are crazy. I think we need to set them to fixed-width, since we may have an arbitrarily large number of columns?
  - I don't think we have the header UI elements in M10 yet and that mock is pretty old, so I sort of very roughly approximated it.
  - What should we do when you click a task title? Popping the whole task in a dialog is possible but needs a bunch of work to actually work. Might need to build "sheets" or something.
  - Icons are slightly clipped for some reason.
  - All the backend stuff is totally faked.

Generally, my plan is just to use these to implement all of T390. Specifically:

  - "Kanban" projects will have "Backlog" on the left. You'll drag them toward the right as you make progress.
  - "Milestone" projects will have "No Milestone" on the left, then "Milestone 9", "Milestone 8", etc.
  - "Sprint" projects will have "Backlog" on the left, then "Sprint 31", "Sprint 30", etc.

So all of these things end up being pretty much exactly the same, with some minor text changes and new columns showing up on the left vs the right or whatever.

Test Plan: See screenshot.

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: chad, aran, sascha-egerer

Maniphest Tasks: T1344

Differential Revision: https://secure.phabricator.com/D7374
2013-10-21 21:11:36 -07:00
epriestley
2a5c987c71 Lock policy queries to their applications
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.

This has several parts:

  - For PolicyAware queries, provide an application class name method.
  - If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
  - For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.

Test Plan:
  - Added a unit test to verify I got all the class names right.
  - Browsed around, logged in/out as a normal user with public policies on and off.
  - Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7367
2013-10-21 17:20:27 -07:00
epriestley
32dd8af9e5 Reduce surface area of DifferentialComment API
Summary:
Ref T2222. Shrink the API to make it easier to move this object's storage to ApplicationTransactions.

Fixes T3415. This moves the "Summary" and "Test Plan" into the property list, and thereby fixes all the attribution problems associated with commandeering, creating a revision from another user's diff, etc.

Test Plan: Browsed several revisions.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3415, T2222

Differential Revision: https://secure.phabricator.com/D7375
2013-10-21 17:01:27 -07:00
epriestley
cf1c06e157 Add custom field storage to Projects
Summary: Ref T4010. Adds storage and indexes for custom fields. These tables are the same as people/maniphest/differential.

Test Plan: Ran `bin/storage upgrade`.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T4010

Differential Revision: https://secure.phabricator.com/D7369
2013-10-21 17:01:13 -07:00
epriestley
8994a81b35 Make event-triggered actions more aware of application access
Summary:
Fixes T3675.

  - Maniphest had a couple of old non-event listeners; move them to events.
  - Make most of the similar listeners a little more similar.
  - Add checks for access to the application.

Test Plan:
  - Viewed profile, project, task, revision.
  - Clicked all the actions.
  - Blocked access to various applications and verified the actions vanished.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3675

Differential Revision: https://secure.phabricator.com/D7365
2013-10-21 17:00:50 -07:00
epriestley
d66972c9f2 Tie application event listeners to the applications they listen for
Summary:
Ref T3675. Some of these listeners shouldn't do their thing if the viewer doesn't have access to an application (for example, users without access to Differential should not be able to "Edit Tasks"). Set the stage for that:

  - Introduce `PhabricatorEventListener`, which has an application.
  - Populate this for event listeners installed by applications.
  - Rename the "PeopleMenu" listeners to "ActionMenu" listeners, which better describes their modern behavior.

This doesn't actually change any behaviors.

Test Plan: Viewed Maniphest, Differntial, People.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3675

Differential Revision: https://secure.phabricator.com/D7364
2013-10-21 17:00:21 -07:00
epriestley
c69beb7988 Stop writes to the old Relationship table
Summary: Ref T1279. The new stuff seems stable, so stop writes to the old tables.

Test Plan:
  - Added and removed reviewers.
  - Grepped for `::RELATIONSHIP_TABLE` to verify we really have no more reads.
  - Grepped for `::RELATION_REVIEWER`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1279

Differential Revision: https://secure.phabricator.com/D7360
2013-10-21 16:59:22 -07:00
epriestley
7fedfacbca Add capabilities for editing task triage details (priority, assignee, etc)
Summary:
This is primarily a client request, and a little bit use-case specific, but policies seem to be holding up well and I'm getting more comfortable about maintaining this. Much if it can run through ApplicationTransactions.

Allow the ability to edit status, policies, priorities, assignees and projects of a task to be restricted to some subset of users. Also allow bulk edit to be locked. This affects the editor itself and the edit, view and list interfaces.

Test Plan: As a restricted user, created, edited and commented on tasks. Tried to drag them around.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7357
2013-10-21 16:59:06 -07:00
epriestley
00bf47f973 Fix "Manage herald rules" link by removing it
Summary: Fixes T4001. I broke this some time ago and no one has complained. I don't think it gets much use, and we haven't added it for the newer apps. Just get rid of it rather than adapt the URIs for ApplicationSearch.

Test Plan: Unit tests, sent myself some email.

Reviewers: zeeg, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4001

Differential Revision: https://secure.phabricator.com/D7355
2013-10-21 16:58:56 -07:00
epriestley
baf2ea5b32 Remove "ManiphestTransactionEditorPro"
Summary: Drop the "Pro" bit.

Test Plan: Created/edited tasks, moved tasks around, generally made a mess. Nothing burned down.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7352
2013-10-21 16:58:37 -07:00
epriestley
83c99be423 Clean up supplemental capabilitiy checks in transaction edits
Summary:
We have this commented-out chunk of code now which was originally buggy and is now just nonfunctional.

For now, the core edit types don't always require CAN_EDIT (e.g., subscribe, comment, add edges), except for editing the edit policy itself, which always does. Add a supplemental capability check there and let everything else go through with CAN_VIEW. We can buff the policy checks on application editors over time, they all require appropriate capabilities to get to in the first place anyway.

Test Plan: Created and edited some tasks without getting overwhelmed with policy exceptions.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7351
2013-10-21 16:58:21 -07:00
epriestley
f5c7dd68d2 Clean up some ordering and strata edge cases in Phrequent
Summary:
Ref T3569. Two issues:

  # Since `sort()` is not stable, instantaneous events (ending on the same second they start) would sometime sort wrong and produce the wrong results. Guarantee they sort correctly.
  # Because events can end at any time, there are some additional special cases the algorithm didn't handle properly. Draw a bunch of ASCII art diagrams so these cases work properly.

Test Plan:
  - No more fatal when tracking an object for the first time.
  - Unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: skyronic, aran

Maniphest Tasks: T3569

Differential Revision: https://secure.phabricator.com/D7350
2013-10-21 16:58:12 -07:00
epriestley
da9a362169 Shuffle project information around on detail page
Summary:
Ref T4007. Fixes T4009. Ref T4008.

  - Move blurb to a text section.
  - Make it render as remarkup.
  - Put policy information and status information in header.

Test Plan: See screenshot.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T4009, T4007, T4008

Differential Revision: https://secure.phabricator.com/D7373
2013-10-21 11:34:45 -07:00
epriestley
ee254b5c2a Remove PhabricatorFeedStoryManiphest and ManiphestAction
Summary:
I'll hold this for a couple weeks.

These classes are now only used to render legacy feed stories. I don't plan to migrate the stories since I don't think they're particularly valuable, and migrating them would be complex and time consuming.

With these classes removed, legacy Maniphest feed stories simply vanish from feed.

Test Plan: `grep`, viewed feed, verified it worked but omitted old-style stories.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7114
2013-10-20 16:13:34 -07:00
epriestley
31c474a7ff Duct-tape the "z" haunted comment panel mode back together
Summary: Fixes T3898. This feature needs generalization at some point, but just unbreak it for now since a surprising number of users like it.

Test Plan: Pressed "z".

Reviewers: chad, btrahan

Reviewed By: chad

CC: chad, aran, spicyj

Maniphest Tasks: T3898

Differential Revision: https://secure.phabricator.com/D7366
2013-10-19 16:43:33 -07:00
epriestley
87ccca32b6 Add printable support to CSS
Summary:
Fixes T2146. This is a really simple approach, you just do:

  !print .rule {
    whatever: blah;
  }

And it transforms it into:

  .printable .rule {
    whatever: blah;
  }

  @media print {
    .rule {
      whatever: blah;
    }
  }

So we end up with these rules twice, but they should compress well and we shouldn't need that many of them, and this fix is way way simpler than all the nonsense I discussed in T2146.

Test Plan:
  - Added a unit test.
  - Added a simple rule to throw away the menubar when printing.
  - Checked the latter with `/?__print__=1`.

Reviewers: chad, btrahan

Reviewed By: chad

CC: chad, aran

Maniphest Tasks: T2146

Differential Revision: https://secure.phabricator.com/D7363
2013-10-19 14:23:19 -07:00
epriestley
3643fe1498 Use property tabs in Files
Summary:
See screenshots. Some simplifications:

  - Tabbed and non-tabbed lists are now allowed to be mixed. We just make the non-tabbed lists permanent and put them on the bottom (e.g., image and audio data in Files).
  - You can provide a tab name instead of an entire tab object and we'll build an object for you.
  - We respect `setSelected()` on the tab objects now.

Test Plan: See screenshots.

Reviewers: chad, btrahan

Reviewed By: chad

CC: chad, aran

Differential Revision: https://secure.phabricator.com/D7362
2013-10-19 12:08:06 -07:00
epriestley
2228029201 Wire up object box tabs
Summary: Make tabs do stuff when you click 'em.

Test Plan:
  - Clicked object box tabs in UIExample.
  - Viewed some existing non-tab UIs (Differential, Maniphest).
  - Viewed some existing non-tab, multiple-list UIs (Diffusion).
  - Grepped for methods I changed.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7361
2013-10-19 12:07:50 -07:00
epriestley
f010730e49 Migrate all Differential inline comments to ApplicationTransactions
Summary:
Ref T2222. This implements step (1) described there, which is moving over all the inline comments.

The old and new tables are simliar. The only real trick here is that `transactionPHID` and `legacyCommentID` mean roughly the same thing (`null` if the inline is a draft, non-null if it has been submitted) but we don't have real `transactionPHID`s yet. We just make some up -- we'll backfill them later.

Two risks here:

  - I need to take a second look at the keys on this table. I think we need to tweak them a bit, and it will be less disruptive to do that before this migration than after.
  - This will take a while for Facebook, and other large installs with tens of thousands of revisions. I'll communicate this.

I'm otherwise pretty satisfied with this, seems to work well and is pretty low risk / non-disruptive.

Test Plan:
  - Before migrating, then after migrating:
    - Made a bunch of inlines (drafts, submitted).
    - Edited and deleted inlines.
    - Verified inlines showed up in preview.
    - Verified that inlines aren't indexed when they're drafts (`bin/search index D935`).
    - Verified that inlines ARE indexed when they're not drafts.
    - Verified that drafts inlines make revisions appear as "with draft" in the revision list.
  - Made left, right, and draft inlines.
  - Migrated (`bin/storage upgrade`).
  - Verified that my inlines from before the migration still showed up.
  - (Repeated all the stuff above.)
  - Manually inspected the inline comment table.

Reviewers: btrahan

Reviewed By: btrahan

CC: FacebookPOC, aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D7139
2013-10-19 05:03:25 -07:00
epriestley
460051c1d8 Drop maniphest_savedquery table
Summary: This data was migrated by D6977 and is now obsolete. I'll hold this patch for a week or two in case we get reports of migration errors.

Test Plan: Ran storage upgrade, saw the table vanish. Grepped for references to the table.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6997
2013-10-19 04:56:17 -07:00
epriestley
7180199d13 Fix daemon auth issue
Summary: I touched this code recently but it needs an unusual special case because we call through with the "omnipotent user" from the daemons. As per the TODO below, this will all get cleaned up at some point.

Test Plan: Will make @poop verify.

Reviewers: btrahan, poop

Reviewed By: poop

CC: poop, aran

Differential Revision: https://secure.phabricator.com/D7356
2013-10-18 18:29:36 -07:00
epriestley
7e815a06f8 Fix audio editing!
Summary: This capability was replaced with an application-wide "manage" capability. It's checked for just above.

Test Plan: Edited audio!

Reviewers: btrahan, ljalonen, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7353
2013-10-18 16:23:16 -07:00
epriestley
943080a4de Make Phrequent time accounting aware of the stack
Summary:
Ref T3569. Fixes T3567. When figuring out how much time has been spent on an object, subtract "preemptive" events which interrupted the object.

Also, make the UI look vaguely sane:

{F72773}

Test Plan: Added a bunch of unit tests, mucked around in the UI.

Reviewers: btrahan

Reviewed By: btrahan

CC: hach-que, skyronic, aran

Maniphest Tasks: T3567, T3569

Differential Revision: https://secure.phabricator.com/D7349
2013-10-18 12:47:36 -07:00
Jakub Vrana
82e57ab8a7 Add Link button to Remarkup assist
Summary: Believe it or not, I forgot how to create a link in Remarkup.

Test Plan: Clicked on it with selected URL, selected text and without a selection.

Reviewers: chad, epriestley

Reviewed By: epriestley

CC: epriestley, aran, chad

Differential Revision: https://secure.phabricator.com/D7336
2013-10-17 18:57:40 -07:00
Chad Little
43e58e8d31 Link Remarkup Icon
Summary: Icon for a link

Test Plan: photoshop

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7344
2013-10-17 15:49:13 -07:00
epriestley
0b22777f68 Remove UI warnings about policies being a janky mess
Summary: Ref T603. While policies aren't completely perfect, they are substantially functional to the best of my knowledge -- definitely in good enough shape that we want to hear about issues with them, now.

Test Plan: Edited a task, repository, and project.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7343
2013-10-17 12:59:40 -07:00
epriestley
5171e3684c Require application "Can Use" capability to call Conduit methods
Summary: Ref T603. If you don't have access to an application, prevent execution of its (authenticated) methods.

Test Plan: Restricted Tokens to only admins, then tried to view/call Token methods as a non-admin.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7342
2013-10-17 12:51:19 -07:00
epriestley
32dca4b553 Fix lightbox downloads for embeded images and a warning
Summary:
I refactored this recently and accidentally dropped the download URI.

Also fix a warning with, e.g., files named `README`.

Test Plan: Clicked a thumb, clicked "Download", got a file.

Reviewers: chad, btrahan, dctrwatson

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7341
2013-10-17 11:21:01 -07:00
epriestley
95c2b03fc8 Distinguish between invalid/broken handles and filtered handles
Summary:
Ref T603. Currently, we render handles the user doesn't have permission to see in a manner identical to handles that don't exist. This is confusing, and not required by policies (which restrict content, but permit knowledge that an object exists).

Instead, render them in different styles. Bad/invalid objects look like:

  Unknown Object (Task)

Restricted objects look like:

  [o] Restricted Task

...where `[o]` is the padlock icon.

Test Plan:
{F71100}

{F71101}

It's possible this renders weird somewhere, but I wasn't immediately able to find any issues. Yell if you see something.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7334
2013-10-17 10:49:21 -07:00
epriestley
cf8d7da292 Add some missing non-context translations
Summary: We're missing some strings here.

Test Plan: {F71857}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7340
2013-10-17 10:48:44 -07:00
epriestley
4f05736175 Add an icon+background selector for project images
Summary: Makes it easy to choose distinctive icons for projects.

Test Plan:
{F71018}

{F71020}

{F71019}

{F71021}

Reviewers: btrahan, chad

Reviewed By: chad

CC: chad, aran

Differential Revision: https://secure.phabricator.com/D7333
2013-10-17 09:32:34 -07:00
Chad Little
70b7b9a869 White policy icons
Summary: White policy icons for hover states, rebuilt 2x icons

Test Plan: photoshop

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7337
2013-10-17 07:41:02 -07:00
epriestley
b55cf56d2e Implement Graphviz, Figlet and Cowsay as Remarkup interpreter blocks
Summary: Fixes T3274. Fixes T3964. Ref T3591.

Test Plan: {F70928}

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T3274, T3964, T3591

Differential Revision: https://secure.phabricator.com/D7332
2013-10-16 19:12:14 -07:00
Jakub Vrana
07b8c2cfd0 Phtize phabricator-remarkup-assist
Test Plan: Translated 'bold text' as 'txt', clicked on B without selection, saw 'txt'.

CC: epriestley, aran

Differential Revision: https://secure.phabricator.com/D7335
2013-10-16 19:04:47 -07:00
Jakub Vrana
a9a3bdf3cc Delete unintentional phlog()
Leaked in D7329.
2013-10-16 13:59:23 -07:00
Chad Little
89d35b98c8 Misc Diffusion/Differential CSS tweaks
Summary: Various tweaks and fixes. Adds a File Contents view in Diffusion, normalizes spaces, colors.

Test Plan: tested differential and diffusion in my sandbox.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3952

Differential Revision: https://secure.phabricator.com/D7325
2013-10-16 13:09:12 -07:00
epriestley
314673f4f6 Fix an issue with rendering PHID lists containing null in Maniphest
Summary: See IRC. Someone got a `null` in CCPHIDs somehow. Moving to subscriptions should prevent this, but paper over it for now.

Test Plan: Will have @dctrwatson check.

Reviewers: btrahan, dctrwatson

Reviewed By: btrahan

CC: dctrwatson, aran

Differential Revision: https://secure.phabricator.com/D7330
2013-10-16 12:46:34 -07:00
epriestley
3410cbd53e Add application and object level policy controls to Countdown
Summary: Ref T603. Give countdowns proper UI-level policy controls, and an application-level default policy. Put policy information in the header.

Test Plan:
  - Adjusted default policy.
  - Created new countdowns.
  - Edited countdowns.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7322
2013-10-16 10:36:08 -07:00
epriestley
e381022bc7 Provide application and object level policy controls in Slowvote
Summary: Ref T603. Gives the create/edit interface a policy control, and adds an application-level default.

Test Plan: Created and edited polls.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7321
2013-10-16 10:36:00 -07:00
epriestley
8c1c6fec5a Modernize policies in Paste and Macro
Summary:
Ref T603. Fixes T2823. This updates Paste and Macro.

  - **Paste**
    - Added default view policy.
    - I didn't add a "create" policy, since I can't come up with any realistic scenario where you'd give users access to pastes but not let them create them.
  - **Macro**
    - Added a "manage" policy, which covers creating and editing macros. This lets an install only allow "People With An Approved Sense of Humor" or whatever to create macros.
    - Removed the "edit" policy, since giving individual users access to specific macros doesn't make much sense to me.
    - Changed the view policy to the "most public" policy the install allows.
    - Added view policy information to the header.

Also fix a couple of minor things in Maniphest.

Test Plan:
  - Set Paste policy, created pastes via web and Conduit, saw they got the right default policies.
  - Set Macro policy, tried to create/edit macros with valid and unauthorized users.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2823, T603

Differential Revision: https://secure.phabricator.com/D7317
2013-10-16 10:35:52 -07:00
Jakub Vrana
29391a658e Disallow <! in <script>
Summary:
HTML5 has this crazy script escaping states:

- Script data escaped dash dash state
- Script data double escaped state

https://communities.coverity.com/blogs/security/2012/11/16/did-i-do-that-html-5-js-escapers-3

Perhaps `<!` is too aggressive but I didn't spend much time searching for a more fine grained expression.

Test Plan: Searched for `renderInlineScript()`.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7329
2013-10-16 09:28:37 -07:00
Chad Little
d34143c723 Celerity Map
Summary: I forgot my veggies last night

Test Plan: n/a

Reviewers: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7324
2013-10-15 09:38:36 -07:00
Chad Little
451dfb9afb Differential Changeset header icons
Summary: Adds filetype icons, applying to differential file headers. The main issue is with all the lightening, I wanted something to still anchor 'new file' on the page and adding a sharp icons does that pretty well for me. Feedback is cool too.

Test Plan: Add some new icons, test in previous commits.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7320
2013-10-15 07:34:48 -07:00
epriestley
19968e31f4 Generalize Herald account sources
Summary: The "user" and "user/project" sources exclude system agents and disabled users, but should not.

Test Plan: Added system agents to Herald rules.

Reviewers: btrahan, bigo

Reviewed By: bigo

CC: aran

Differential Revision: https://secure.phabricator.com/D7319
2013-10-14 19:38:35 -07:00
epriestley
197d3817bc Give disabled crumb actions a distinct visual style
Summary: Ref T603. When you aren't allowed to take a top-level action (usually "Create Thing"), visually disable the action.

Test Plan: {F69596}

Reviewers: chad

Reviewed By: chad

CC: chad, aran, asherkin

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7318
2013-10-14 18:21:06 -07:00
epriestley
76dfeb95ba Allow "Custom" policies to be selected in the policy control
Summary: Ref T603. When a user selects "Custom", we pop open the rules dialog and let them create a new rule or edit the existing rule.

Test Plan: Set some objects to have custom policies.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7300
2013-10-14 16:59:16 -07:00
epriestley
3a4c08d7f1 Simplify custom policies before saving, and reject meaningless policies
Summary:
Ref T603. Do a little more sanity checking on custom policies, so policies like this:

  [ Allow ] [ Users ] [ <no users> ]

...that don't specify anything and thus which aren't meaningful raise errors.

Test Plan: {F69570}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7314
2013-10-14 16:48:41 -07:00
Chad Little
d2895249ee Add Persona login icon
Summary: Adds the new icon 1x and 2x

Test Plan: photoshop

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3958

Differential Revision: https://secure.phabricator.com/D7316
2013-10-14 16:33:49 -07:00
Scott MacVicar
4d1709651e [releeph] Conduit failure with bad IDs
Summary:
Instead of returning a blank result it throws exceptions. Fix this up a
little so we get some consistency with differential

Test Plan:
Loaded a bad phid for releeph, returns empty list.
Try a good phid and get 2 releeph merges.

Reviewers: epriestley, elenaperezrioja, dschleimer, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7302
2013-10-14 16:07:17 -07:00
epriestley
c4abf160cc Fix some file policy issues and add a "Query Workspace"
Summary:
Ref T603. Several issues here:

  1. Currently, `FileQuery` does not actually respect object attachment edges when doing policy checks. Everything else works fine, but this was missing an `array_keys()`.
  2. Once that's fixed, we hit a bunch of recursion issues. For example, when loading a User we load the profile picture, and then that loads the User, and that loads the profile picture, etc.
  3. Introduce a "Query Workspace", which holds objects we know we've loaded and know we can see but haven't finished filtering and/or attaching data to. This allows subqueries to look up objects instead of querying for them.
    - We can probably generalize this a bit to make a few other queries more efficient. Pholio currently has a similar (but less general) "mock cache". However, it's keyed by ID instead of PHID so it's not easy to reuse this right now.

This is a bit complex for the problem being solved, but I think it's the cleanest approach and I believe the primitive will be useful in the future.

Test Plan: Looked at pastes, macros, mocks and projects as a logged-in and logged-out user.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7309
2013-10-14 14:36:06 -07:00
epriestley
073cb0e78c Make PhabricatorPolicyInterface require a getPHID() method
Summary:
Ref T603. This cleans up an existing callsite in the policy filter, and opens up some stuff in the future.

Some policy objects don't have real PHIDs:

  PhabricatorTokenGiven
  PhabricatorSavedQuery
  PhabricatorNamedQuery
  PhrequentUserTime
  PhabricatorFlag
  PhabricatorDaemonLog
  PhabricatorConduitMethodCallLog
  ConduitAPIMethod
  PhabricatorChatLogEvent
  PhabricatorChatLogChannel

Although it would be reasonable to add real PHIDs to some of these (like `ChatLogChannel`), it probably doesn't make much sense for others (`DaemonLog`, `MethodCallLog`). Just let them return `null`.

Also remove some duplicate `$id` and `$phid` properties. These are declared on `PhabricatorLiskDAO` and do not need to be redeclared.

Test Plan: Ran the `testEverythingImplemented` unit test, which verifies that all classes conform to the interface.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7306
2013-10-14 14:35:47 -07:00
epriestley
0ce4f6d176 Add Persona auth provider
Summary: Ref T3958. Adds a provider for Mozilla's Persona auth.

Test Plan:
  - Created a Persona provider.
  - Registered a new account with Persona.
  - Logged in with Persona.
  - Linked an account with Persona.
  - Dissolved an account link with Persona.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3958

Differential Revision: https://secure.phabricator.com/D7313
2013-10-14 14:34:57 -07:00
epriestley
502c6f2d48 Render public content as "Public" in headers, not "Public (No Login Required)"
Summary:
Ref T603. Although I think the parenthetical is valuable when //setting// policies to make sure no one accidentally opens content up, it's super annoying in headers.

This makes headers say "Public". Everything else still says "Public (No Login Required)".

Test Plan: {F69469}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7310
2013-10-14 14:17:25 -07:00
Chad Little
e1e17a3378 Minor Differential CSS tweaks
Summary: Fixes notices, uses standard colors for TODOs. Mutes comments just a hair.

Test Plan: Will test on epriestley. Also Differentail and Diffusion.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3952

Differential Revision: https://secure.phabricator.com/D7311
2013-10-14 13:12:36 -07:00
Bob Trahan
cb64bef3c5 Maniphest - make instructions !assign savvy
Summary: forgot this in last diff.  Ref T3937.

Test Plan: looks good

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3937

Differential Revision: https://secure.phabricator.com/D7308
2013-10-14 12:33:38 -07:00
Bob Trahan
d0127f95e5 Maniphest - add support for !assign command
Summary:
also try to centralize some of the command parsing logic. note that differential is still an exception here. it uses a whitelist-style regex. i think long-term we should have this for every app but changing it seemed too big for this diff.

Fixes T3937.

Test Plan:
echo '!assign btrahan' | ./bin/mail receive-test --as xerxes --to T22 ; echo '!claim' | ./bin/mail receive-test --as xerxes --to T22

unit tests passed, though my new one is silly

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3937

Differential Revision: https://secure.phabricator.com/D7307
2013-10-14 12:29:41 -07:00
epriestley
13178ec279 Prepare the policy rule edit endpoint for integration
Summary: Ref T603. Allow the endpoint to take an existing policy PHID to populate the editor and return a useful datastructure.

Test Plan: In the next revision, actually hooked this up.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7299
2013-10-14 12:07:31 -07:00
epriestley
5e5b7576a6 Make PhabricatorPolicyQuery a CursorPagedPolicyAwareQuery
Summary:
Ref T603. Make these actually implement policy interfaces, so shared infrastructure (like handle loading) works as expected. They don't actually have meaningful policies, and we short circuit all the checks.

(I don't plan to let you set policy controls on policies themselves)

Test Plan: Loaded handles for Policy objects via common infrastructure.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7298
2013-10-14 12:06:52 -07:00
epriestley
7364a3bedd Add some missing strings for custom policies
Summary: Ref T603. Fix/provide some rendering stuff related to custom policies.

Test Plan: After setting stuff to custom policies (made easier by future diffs), looked at the various places strings appear in the UI and saw more sensible ones.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7297
2013-10-14 12:05:43 -07:00
epriestley
6c1b00fa40 Rename ACTION_ACCEPT into ACTION_ALLOW
Summary: Ref T603. This is "Allow" in the UI, I just mistyped it when I created the constant.

Test Plan: `grep`

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7296
2013-10-14 12:05:11 -07:00
epriestley
3f5a7b31c8 Add a synthetic DOM event to JX.Workflow for form submission
Summary: Currently, when a dialog is submitted the Workflow itself emits an event but no DOM event is emitted. The workflow event is fine for handlers which only use JS, but there's currently no way for a handler to act more like a normal form handler. This event gives normal form handlers a way to capture Workflow submits and muck around with form contents, etc.

Test Plan: In a future diff, edited policies via a Workflow dialog.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7295
2013-10-14 11:58:38 -07:00
epriestley
181aff8556 Make some dropdown CSS more specific
Summary:
Dropdowns have some `span` rules and such currently. Give them class-based rules instead.

(This allows me to add another <span> to menu items later on without it picking up silly styles.)

Test Plan: In a future diff, added menu items with additional <span>s inside them.

Reviewers: btrahan, chad

Reviewed By: chad

CC: chad, aran

Differential Revision: https://secure.phabricator.com/D7294
2013-10-14 11:50:05 -07:00
epriestley
994823524b Don't try to activate disabled items in dropdown menus
Summary: Fixes an issue where the user could click a disabled dropdown menu item and get an exception or some other nonsense. Instead, just don't activate anything.

Test Plan: Clicked a disabled header, like "Members of project" in the policy dropdown.

Reviewers: btrahan, va.multimoney, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7293
2013-10-14 11:48:21 -07:00
epriestley
67cca8f7fa Fix breadcrumbs for login screen triggered when a logged-out user fails a policy check
Summary:
Ref T603. Currently, if you're logged out and try to view some object which requires you to be logged in, the login screen is missing the application breadcrumb and just says "Login".

Add the application in context so we get the keys icon.

Test Plan: {F69255}

Reviewers: chad, btrahan, asherkin

Reviewed By: chad

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7303
2013-10-14 11:46:26 -07:00
epriestley
67b17239b8 Allow custom policies to be loaded and exeucuted by the policy filter
Summary: Ref T603. Adds code to actually execute custom policies. (There's still no way to select them in the UI.)

Test Plan:
  - Added and executed unit tests.
  - Edited policies in existing applications.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7292
2013-10-14 11:46:14 -07:00
Chad Little
4733a8ef14 Add border, transparent indicators to images in file property view
Summary: Fixes T3950. This centers the images, adds a thin blue border, and a transparent background.

Test Plan: Tested a file in Files, Diffusion, and Macro.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3950

Differential Revision: https://secure.phabricator.com/D7305
2013-10-14 11:40:19 -07:00
Chad Little
b3ca73ff1b Fix subscribers in PHUIDocument
Summary: This resolves a wrapping issue in phui-document-css

Test Plan: reload page, subscribers no longer wrap.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7301
2013-10-14 11:38:05 -07:00
Chad Little
503f413789 Clean up spacing on Diffusion headers
Summary: This adds some controllable space between paths in Diffusion headers. Fixes T3951

Test Plan: Tested new links in diffusion.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3951

Differential Revision: https://secure.phabricator.com/D7304
2013-10-14 09:40:05 -07:00
Chad Little
1f0b40b61d Projects Icons Series 1
Summary: A set of random icons for use as project identifiers. 42, white.

Test Plan: photoshop, epriestley

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7290
2013-10-12 19:15:38 -07:00
epriestley
a0913db5df Improve an awkward initialization behavior in Javelin in Firefox
Summary: See comment; see IRC. This didn't cause anything bad, but ended up with `document.body.id == "null"` in Firefox, which is a bit weird.

Test Plan: Loaded page, put `document.body.null` into the console, got consistent results.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7288
2013-10-12 17:08:56 -07:00
epriestley
122f643d6f Fix two issues where StandardPageView assumes it will always have access to a user
Summary: If we hit an exception very early, the request may not have a user yet. Don't assume it will.

Test Plan: Broke MySQL config on purpose to test some stuff, loaded Phabricator, got a nice error page instead of a super-fatal one.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7286
2013-10-12 17:08:47 -07:00
epriestley
130a15b51b Highlight the currently selected policy in the policy dropdown control thing
Summary: Ref T603. Make this a little easier to use by highlighting the current value.

Test Plan: See screenshot.

Reviewers: chad, btrahan

Reviewed By: chad

CC: chad, aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7289
2013-10-12 17:08:35 -07:00
epriestley
5af031ec9b Make the policy control a JS dropdown with icons
Summary: Ref T603. After thinking about this for a bit I can't really come up with anything better than what Facebook does, so I'm going to implement something similar for choosing custom policies. To start with, swap this over to a JS-driven dropdown.

Test Plan: See screenshot.

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7285
2013-10-12 17:08:11 -07:00
Chad Little
538d8f27b5 Fix empty property list in Phriction
Summary: Fixes T3944

Test Plan: Will test on secure

Reviewers: epriestley, staticshock

CC: Korvin, epriestley, aran

Maniphest Tasks: T3944

Differential Revision: https://secure.phabricator.com/D7291
2013-10-11 14:20:21 -07:00
Chad Little
97c690fc0f PHUIPropertyListView
Summary: This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.

Test Plan: Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7283
2013-10-11 07:53:56 -07:00
Asher Baker
f8d963a77e Rename "Upload Image" dialog to "Upload File" to match purpose
Summary: Fixes T3940.

Test Plan: Clicked button, looked at dialog.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: nmalcolm, Korvin, epriestley, aran

Maniphest Tasks: T3940

Differential Revision: https://secure.phabricator.com/D7287
2013-10-11 05:15:45 -07:00
epriestley
5899ae08b3 Add storage for custom policies
Summary: Ref T603. Allows custom policies to be saved. No integration with policy controls yet.

Test Plan:
  mysql> select * from policy where id = 3\G
  *************************** 1. row ***************************
             id: 3
           phid: PHID-PLCY-e4v2fnbyuibi4supl5tn
          rules: [{"action":"allow","rule":"PhabricatorPolicyRuleAdministrators","value":null},{"action":"allow","rule":"PhabricatorPolicyRuleProjects","value":["PHID-PROJ-cwovm5gn2ilubjehcdgd"]},{"action":"allow","rule":"PhabricatorPolicyRuleLunarPhase","value":"new"}]
  defaultAction: deny
    dateCreated: 1381437466
   dateModified: 1381437466
  1 row in set (0.00 sec)

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7282
2013-10-10 16:09:51 -07:00
Bob Trahan
db71bf6128 Fix issue reported from github
Summary:
we filter the $actors above such that its possible to have no $actor anymore (if $actor is not a deliverable email address). ergo, make sure we have actor before we start calling methods.

Fixes github issue 403

Test Plan: logic on this one - not 100% sure how to easily reproduce

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7284
2013-10-10 15:17:37 -07:00
Asher Baker
ff4fa1885b Improve pagination in ChatLog application
Summary:
- Add an extra paginator at the top.
- Add a link to jump to the bottom (where the latest messages are).
- Align paginators with edge of content rather than the page.

Test Plan: Looked at the chatlog.

Reviewers: epriestley, chad, #blessed_reviewers

CC: chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7280
2013-10-10 04:49:04 -07:00
Asher Baker
8cc64a9678 Add a 'create' policy to Project
Summary: UX on this could probably be better 'disabled' crumbs don't appear to have any visible difference, and the policy error has to load the /create page rather than being a modal - not sure on the way to fix these.

Test Plan: Tried to create a project with and without access, saw suitable error.

Reviewers: epriestley, #blessed_reviewers

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7279
2013-10-10 04:33:18 -07:00
epriestley
c39b10aa7a Fix non-public capabilities in Application edit
Summary: Ref T603. I nuked this check by accident and neglected to test the negative case.

Test Plan: Saved a non-public policy (Herald Global) and a public policy (Maniphest View).

Reviewers: asherkin, btrahan

Reviewed By: asherkin

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7278
2013-10-09 16:23:36 -07:00
Asher Baker
e3005fad09 Hide Audit information on Home when the application is uninstalled
Test Plan: Looked at Home with Audit installed and uninstalled.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7277
2013-10-09 15:25:03 -07:00
epriestley
f4582dc49d Allow "Default View" policies to be set to Public
Summary: Ref T603. Currently, we hard-code defense against setting policies to "Public" in several places, and special case only the CAN_VIEW policy. In fact, other policies (like Default View) should also be able to be set to public. Instead of hard-coding this, move it to the capability definitions.

Test Plan: Set default view policy in Maniphest to "Public", created a task, verified default policy.

Reviewers: btrahan, asherkin

Reviewed By: asherkin

CC: asherkin, aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7276
2013-10-09 15:06:18 -07:00
epriestley
11fbd213b1 Custom Policy Editor
Summary:
Ref T603. This isn't remotely usable yet, but I wanted to get any feedback before I build it out anymore.

I think this is a reasonable interface for defining custom policies? It's basically similar to Herald, although it's a bit simpler.

I imagine users will rarely interact with this, but this will service the high end of policy complexity (and allow the definition of things like "is member of LDAP group" or whatever).

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran, asherkin

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7217
2013-10-09 14:05:10 -07:00
epriestley
436a403357 Add a "default view" policy to Differential
Summary:
Ref T603. Allows the Differential view policy to be configured with a default.

I've omitted "edit" because I want to wait and see how comment/comment-action policies work out. I could imagine locking "edit" down to only the owner at some point, and providing a wider "interact" capability, or something like that, which would cover accept/reject/commandeer. Users in this group could still edit indirectly by commandeering first.

Test Plan: Created new revisions from the CLI and conduit.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7269
2013-10-09 13:58:00 -07:00
epriestley
650dc0cc30 Remove the "create rules" Herald capability
Summary:
Ref T603. In thinking about this, I think I went mad with power in creating this capability. I can't imagine any reason to give users access to Herald but not let them create rules.

We can restore this later if some install comes up with a good reason to have it, but in the interest of keeping policies as simple as possible, I think we're better off without it. In particular, if you don't want a group of users creating rules, just lock them out of the application entirely.

The "Manage Global Rules" capability is still around, I think that one's super good.

Test Plan: Edited Herald policies, created a rule.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7268
2013-10-09 13:55:44 -07:00
epriestley
1ee455c441 Add defualt view and default edit policies for tasks
Summary: Ref T603. Allow global default policies to be configured for tasks.

Test Plan:
  - Created task via web UI.
  - Created task via Conduit.
  - Created task via email.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7267
2013-10-09 13:53:17 -07:00
epriestley
3147a6ca57 Improve messaging of special policy rules in applications
Summary: Ref T603. When the user encounters an action which is controlled by a special policy rule in the application, make it easier for applications to show the user what policy controls the action and what the setting is. I took this about halfway before and left a TODO, but turn it into something more useful.

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: chad

CC: chad, aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7265
2013-10-09 13:52:04 -07:00
epriestley
45f38c549b Use header status/policy elements in Applications meta-application
Summary: Ref T603. Use more modern elements.

Test Plan: See screenshot.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7264
2013-10-09 13:46:45 -07:00
epriestley
7a97a71e20 Move Herald application capabilities to newer infrastructure
Summary: Ref T603. Use the new hotness.

Test Plan: Edited Herald in Applications, tried to create rules / global rules without capabilities, got reasonable error messages.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7263
2013-10-09 13:44:41 -07:00
epriestley
82a061b485 Make UIStatusList background element transparent instead of white
Summary: Fixes T3933.

Test Plan: See screenshot.

Reviewers: chad

Reviewed By: chad

CC: chad, aran

Maniphest Tasks: T3933

Differential Revision: https://secure.phabricator.com/D7272
2013-10-08 17:18:57 -07:00
Chad Little
ac868f56db Maniphest mobile CSS tweaks
Summary: Fixes a few minor quirks when viewing maniphest on mobile.

Test Plan: shrink maniphest screen, easier to read.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7270
2013-10-08 14:35:56 -07:00
Chad Little
fe73f9da73 Adds status colors to navbar tabs
Summary: Adds the abilit to set a status color of warning or fail to navbar tab lists (for objectheaders)

Test Plan: uiexamples, photoshop

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7266
2013-10-07 19:29:05 -07:00
epriestley
de67f00d0e Remove AphrontRedirectException
Summary: Fixes T3909. Waiting on Facebook to confirm this is unused.

Test Plan: `grep`

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3909

Differential Revision: https://secure.phabricator.com/D7193
2013-10-07 13:29:05 -07:00
epriestley
b1b1ff83f2 Allow applications to define new policy capabilities
Summary:
Ref T603. I want to let applications define new capabilities (like "can manage global rules" in Herald) and get full support for them, including reasonable error strings in the UI.

Currently, this is difficult for a couple of reasons. Partly this is just a code organization issue, which is easy to fix. The bigger thing is that we have a bunch of strings which depend on both the policy and capability, like: "You must be an administrator to view this object." "Administrator" is the policy, and "view" is the capability.

That means every new capability has to add a string for each policy, and every new policy (should we introduce any) needs to add a string for each capability. And we can't do any piecemeal "You must be a {$role} to {$action} this object" becuase it's impossible to translate.

Instead, make all the strings depend on //only// the policy, //only// the capability, or //only// the object type. This makes the dialogs read a little more strangely, but I think it's still pretty easy to understand, and it makes adding new stuff way way easier.

Also provide more context, and more useful exception messages.

Test Plan:
  - See screenshots.
  - Also triggered a policy exception and verified it was dramatically more useful than it used to be.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7260
2013-10-07 13:28:58 -07:00
epriestley
68c854b967 Remove dead rejectImpossiblePolicy() method
Summary: Ref T603. Apparently we made all policies possible at some point. Go us! This has no callsites.

Test Plan: `grep`, notice it's a private method

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7259
2013-10-07 12:52:01 -07:00