1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 08:12:40 +01:00
Commit graph

4593 commits

Author SHA1 Message Date
epriestley
c0e1a63a63 Implement an approval queue
Summary:
  - Add an option for the queue.
  - By default, enable it.
  - Dump new users into the queue.
  - Send admins an email to approve them.

Test Plan:
  - Registered new accounts with queue on and off.
  - As an admin, approved accounts and disabled the queue from email.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7576
2013-11-13 11:24:56 -08:00
epriestley
0fa411083f Show an "approval queue" item on the home page for admins, and sort out menu item visibility
Summary:
  - If you're an administrator and there are users waiting for approval, show a count on the home page.
  - Sort out the `isUserActivated()` access check.
  - Hide all the menu widgets except "Logout" for disabled and unapproved users.
  - Add a "Log In" item.
  - Add a bunch of unit tests.

Test Plan: Ran unit tests, clicked around as unapproved/approved/logged-in/logged-out users.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, chad

Differential Revision: https://secure.phabricator.com/D7574
2013-11-13 11:24:38 -08:00
epriestley
c8320923c4 Implement most of the administrative UI for approval queues
Summary:
Nothing fancy here, just:

  - UI to show users needing approval.
  - "Approve" and "Disable" actions.
  - Send "Approved" email on approve.
  - "Approve" edit + log operations.
  - "Wait for Approval" state for users who need approval.

There's still no natural way for users to end up not-approved -- you have to write directly to the database.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7573
2013-11-13 11:24:18 -08:00
epriestley
a3c811f281 Accept case-insensitive mail replies
Summary:
Mailbox sometimes (?) changes the case of the email address (?). Be more liberal in what we accept.

Also fix a minor output bug.

Test Plan: Sent mail to `e1+...` instead of `E1+...`, verified it arrived.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7575
2013-11-12 21:23:23 -08:00
epriestley
7f11e8d740 Improve handling of email verification and "activated" accounts
Summary:
Small step forward which improves existing stuff or lays groudwork for future stuff:

  - Currently, to check for email verification, we have to single-query the email address on every page. Instead, denoramlize it into the user object.
    - Migrate all the existing users.
    - When the user verifies an email, mark them as `isEmailVerified` if the email is their primary email.
    - Just make the checks look at the `isEmailVerified` field.
  - Add a new check, `isUserActivated()`, to cover email-verified plus disabled. Currently, a non-verified-but-not-disabled user could theoretically use Conduit over SSH, if anyone deployed it. Tighten that up.
  - Add an `isApproved` flag, which is always true for now. In a future diff, I want to add a default-on admin approval queue for new accounts, to prevent configuration mistakes. The way it will work is:
    - When the queue is enabled, registering users are created with `isApproved = false`.
    - Admins are sent an email, "[Phabricator] New User Approval (alincoln)", telling them that a new user is waiting for approval.
    - They go to the web UI and approve the user.
    - Manually-created accounts are auto-approved.
    - The email will have instructions for disabling the queue.

I think this queue will be helpful for new installs and give them peace of mind, and when you go to disable it we have a better opportunity to warn you about exactly what that means.

Generally, I want to improve the default safety of registration, since if you just blindly coast through the path of least resistance right now your install ends up pretty open, and realistically few installs are on VPNs.

Test Plan:
  - Ran migration, verified `isEmailVerified` populated correctly.
  - Created a new user, checked DB for verified (not verified).
  - Verified, checked DB (now verified).
  - Used Conduit, People, Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Differential Revision: https://secure.phabricator.com/D7572
2013-11-12 14:37:04 -08:00
epriestley
cd73fe78db Roadblock users trying to register with external accounts that have invalid emails
Summary:
Ref T3472. Currently, if an install only allows "@mycompany.com" emails and you try to register with an "@personal.com" account, we let you pick an "@mycompany.com" address instead. This is secure: you still have to verify the email. However, it defies user expectation -- it's somewhat confusing that we let you register. Instead, provide a hard roadblock.

(These accounts can still be linked, just not used for registration.)

Test Plan: See screenshot.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3472

Differential Revision: https://secure.phabricator.com/D7571
2013-11-12 14:36:49 -08:00
epriestley
30a51dac36 Clarify registration rules more aggressively when configuring auth
Summary: See private chatter. Make it explicitly clear when adding a provider that anyone who can browse to Phabricator can register.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7570
2013-11-12 10:56:47 -08:00
epriestley
62794e4494 Don't allow "autoclose only" to be set in Mercurial
Summary: We don't actually support this yet, so hide the configuration.

Test Plan: Edited branches for an hg repo.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7563
2013-11-11 16:26:58 -08:00
epriestley
d4eca25774 Don't implement SVN over HTTP
Summary:
Ref T2230. As far as I can tell, getting SVN working over HTTP is incredibly complicated. It's all DAV-based and doesn't appear to have any kind of binary we can just execute and pass requests through to. Don't support it for now.

  - Disable it in the UI.
  - Make sure all the error messages are reasonable.

Test Plan: Tried to HTTP an SVN repo. Tried to clone a Git repo with SVN, got a good error message.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7562
2013-11-11 16:10:41 -08:00
epriestley
c818e6e159 Remove differential.anonymous-access
Summary:
Fixes T3034. This is obsoleted by modern policies.

This was written by a Facebook intern and is rarely used -- the Hive install might be the only use in the wild. It has never really worked correctly.

Test Plan: `grep`; browsed Differential.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3034

Differential Revision: https://secure.phabricator.com/D7568
2013-11-11 16:05:19 -08:00
Bob Trahan
819bd7f03b Add arcanist project phid to differential.query
Summary: Fixes T3535. Also, flip flop on that spacing thing and make the spaces purdy

Test Plan: got an arcanist projected phid in the json dict

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3535

Differential Revision: https://secure.phabricator.com/D7565
2013-11-11 15:01:21 -08:00
Bob Trahan
f35ce505a1 Herald - add ability to conditionalize on Maniphest Task projects
Summary: adds FIELD_PROJECTS and deploys it to Maniphest Task Herald Adapter. Went with "projects" because it feels like that could go well in other Adapters that want to conditionalize based on project.

Test Plan: made a new herald rule to be cc'd if project foo was on a task. it worked!

Reviewers: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7564
2013-11-11 14:20:31 -08:00
epriestley
a4e8fd2289 Wait for the Git client to disconnect before exiting in Git SSH workflows
Summary:
Ref T2230. Very rarely, even though we've flushed the connection and sent all the data, we'll close the connection before Git is happy with it and it will flip out with an error like this:

  fatal: The remote end hung up unexpectedly
  fatal: early EOF
  fatal: index-pack failed

This is hard to reproduce because it depends on the order of read/write operations we can't directly control. I only saw it about 2% of the time, by just running `git pull` over and over again.

Waiting for Git to close its side of the connection seems to fix it.

Test Plan: Ran `git clone` a ton of times without seeing the error again. Ran `git push` a ton of times with new commits.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7558
2013-11-11 12:27:28 -08:00
epriestley
85f505465e Support serving SVN repositories over SSH
Summary:
Ref T2230. The SVN protocol has a sensible protocol format with a good spec here:

http://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_ra_svn/protocol

Particularly, compare this statement to the clown show that is the Mercurial wire protocol:

> It is possible to parse an item without knowing its type in advance.

WHAT A REASONABLE STATEMENT TO BE ABLE TO MAKE ABOUT A WIRE PROTOCOL

Although it makes substantially more sense than Mercurial, it's much heavier-weight than the Git or Mercurial protocols, since it isn't distributed.

It's also not possible to figure out if a request is a write request (or even which repository it is against) without proxying some of the protocol frames. Finally, several protocol commands embed repository URLs, and we need to reach into the protocol and translate them.

Test Plan: Ran various SVN commands over SSH (`svn log`, `svn up`, `svn commit`, etc).

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7556
2013-11-11 12:19:06 -08:00
epriestley
8840f60218 Enable Mercurial reads and writes over SSH
Summary:
Ref T2230. This is substantially more complicated than Git, but mostly because Mercurial's protocol is a like 50 ad-hoc extensions cobbled together. Because we must decode protocol frames in order to determine if a request is read or write, 90% of this is implementing a stream parser for the protocol.

Mercurial's own parser is simpler, but relies on blocking reads. Since we don't even have methods for blocking reads right now and keeping the whole thing non-blocking is conceptually better, I made the parser nonblocking. It ends up being a lot of stuff. I made an effort to cover it reasonably well with unit tests, and to make sure we fail closed (i.e., reject requests) if there are any parts of the protocol I got wrong.

A lot of the complexity is sharable with the HTTP stuff, so it ends up being not-so-bad, just very hard to verify by inspection as clearly correct.

Test Plan:
  - Ran `hg clone` over SSH.
  - Ran `hg fetch` over SSH.
  - Ran `hg push` over SSH, to a read-only repo (error) and a read-write repo (success).

Reviewers: btrahan, asherkin

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7553
2013-11-11 12:18:27 -08:00
epriestley
ac7c739226 Fix --depth N clones in Git
Summary: Ref T2230. Fixes T4079. As it turns out, this is Git being weird. See comments for some detials about what's going on here.

Test Plan: Created shallow and deep Git clones.

Reviewers: hach-que, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4079, T2230

Differential Revision: https://secure.phabricator.com/D7554
2013-11-11 12:17:47 -08:00
epriestley
f2938bacd9 Generalize SSH passthru for repository hosting
Summary:
Ref T2230. In Git, we can determine if a command is read-only or read/write from the command itself, but this isn't the case in Mercurial or SVN.

For Mercurial and SVN, we need to proxy the protocol that's coming over the wire, look at each request from the client, and then check if it's a read or a write. To support this, provide a more flexible version of `passthruIO`.

The way this will work is:

  - The SSH IO channel is wrapped in a `ProtocolChannel` which can parse the the incoming stream into message objects.
  - The `willWriteCallback` will look at those messages and determine if they're reads or writes.
    - If they're writes, it will check for write permission.
    - If we're good to go, the message object is converted back into a byte stream and handed to the underlying command.

Test Plan: Executed `git clone`, `git clone --depth 3`, `git push` (against no-write repo, got error), `git push` (against valid repo).

Reviewers: btrahan

Reviewed By: btrahan

CC: hach-que, asherkin, aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7551
2013-11-11 12:12:21 -08:00
epriestley
ae5fbe034e Fix an issue where Ponder rename stories tried to render question bodies
Summary: Missing some `break;`, pretty sure this is causing the issue on `secure.phabricator.com`.

Test Plan: Will push.

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7559
2013-11-11 11:17:06 -08:00
Jakub Vrana
a29b5b070f Replace some hsprintf() by phutil_tag()
Test Plan: Looked at a diff with inline comment.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7549
2013-11-11 09:23:23 -08:00
Brecht Van Lommel
7ec42dbbea Fix 404 clicking Find Owners in diffusion, if Owners application is disabled.
Test Plan: Install/uninstall Owners application, Find Owners action shown/hidden as expected.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7557
2013-11-11 09:12:09 -08:00
Ryan Freebern
71adee75b0 Use accounts typeahead endpoint for project membership
Reviewed by: epriestley

See: <https://github.com/facebook/phabricator/pull/436>
2013-11-10 16:46:30 -08:00
epriestley
5bb646a838 Fix incorrect check for CAN_EDIT in macro enable/disable controller
Summary: This CAN_EDIT capability doesn't exist. `PhabricatorMacroCapabilityManage::CAPABILITY` (checked on line 15) is used instead.

Test Plan: Disabled, then re-enabled a macro.

Reviewers: hach-que, btrahan

Reviewed By: hach-que

CC: aran

Differential Revision: https://secure.phabricator.com/D7550
2013-11-09 16:34:26 -08:00
James Rhodes
eccbbce9a2 Allow users to create buildables from diffs
Summary:
Now that diffs have PHIDs we can create buildables for them.

This also adds `buildable.diff` in the variables list so the diff ID is available, and it also fixes the Cancel button on "Edit Plan" page so it redirects to the right place.

Test Plan: Created a buildable from a diff, ran a build plan against it that had `echo ${buildable.diff}` and got the right ID.  Also tested the "Edit Plan" cancel redirect.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7546
2013-11-09 15:08:34 -08:00
James Rhodes
79ef667dfd Render build status on revisions and commits
Summary:
This uses an event listener to render the status of builds on their buildables.  The revision and commit view now renders out the status of each of the builds.

Currently the revision controller has the results for the latest diff rendered out.  We might want to show the status of previous diffs in the future, but for now I think the latest diff should do fine.

There's also a number of bug fixes in this diff, including a particularly nasty one where builds would have a build plan PHID generated for them, which resulted in handle lookups always returning invalid objects.

Test Plan: Ran builds against diffs and commits, saw them appear on the revision and commit view controllers.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7544
2013-11-09 15:04:00 -08:00
epriestley
56c65e33ef Add a "Make an HTTP Request" build step
Summary:
Ref T1049. This is very minimal, but does what it says.

I merged the variable replacement code so Remote + HTTP can share more stuff.

Test Plan:
Ran "HTTP" and "Remote" build plans.

{F79886}

{F79887}

Reviewers: hach-que, btrahan

Reviewed By: hach-que

CC: zeeg, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7541
2013-11-09 07:16:12 -08:00
James Rhodes
43847d6bd7 Prevent crashes when more than one buildable for an object exists
Summary: This prevents a crash in applying build plans when more than one buildable exists for the same object.  It also adds a check into the "New Manual Build" page to ensure that users can't create a buildable for an object that already has one.

Test Plan: Tried to create a buildable for an object that already has one and a nice friendly error appeared.  Applied a build plan to a buildable whose object has two buildables and didn't get a crash any more.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7543
2013-11-09 07:03:18 -08:00
James Rhodes
3eaac9eda9 Added build.id variable and other improvements / fixes
Summary: This adds a `build.id` variable, cleans up the naming convention of other variables and also fixes an issue in the remote command to read the buffers after the command finishes.

Test Plan: Ran a build with `/bin/echo ${build.id}` and saw the build ID come through.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7540
2013-11-08 19:06:45 -08:00
James Rhodes
197e9b6f49 Put back the stronger variable replacement
Summary: This puts back the stronger variable replacement that was missed the last update to D7519.

Test Plan: Re-ran a remote build that had variables in the command and everything worked as expected.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7539
2013-11-08 18:36:48 -08:00
James Rhodes
8f52778f32 Fix content being missed in the build logs
Summary: This fixes an issue where content would be discarded when the content to append is larger than the chunk size limit.

Test Plan: Tested running a remote command that does `I=0; while true; do echo "$I"; I=$[$I+1]; done` and all of the outputted numbers matched the line numbers in the logs.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7537
2013-11-08 18:28:15 -08:00
James Rhodes
e64efa05c3 Implement "Run Remote Command" build step implementation.
Summary: This adds a build step implementation for running a command on a remote machine over SSH.  It supports merging in various variables about the build (such as the commit hash / revision ID, repository call sign, version control type and clone URI).

Test Plan: Configured a build plan to run `/bin/true` on localhost and the build passed.  Configured a build plan to run `/bin/false` on localhost and the build failed.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7519
2013-11-08 18:15:14 -08:00
James Rhodes
0ac1be7094 Implemented support for build logs
Summary:
Depends on D7519.

This implements support for build logs in Harbormaster.  This includes support for appending to a log from the "Run Remote Command" build step.

It also adds the ability to cancel builds.

Currently the build view page doesn't update the logs live; I'm sure this can be achieved with Javelin, but I don't have enough experience with Javelin to actually make it poll from updates to content in the background.

{F79151}

{F79153}

{F79150}

{F79152}

Test Plan:
Tested this by setting up SSH on a Windows machine and using a Remote Command configured with:

```
C:\Windows\system32\cmd.exe /C cd C:\Build && mkdir Build_${timestamp} && cd Build_${timestamp} && git clone --recursive https://github.com/hach-que/Tychaia.git && cd Tychaia && Protobuild.exe && C:\Windows\Microsoft.NET\Framework\v4.0.30319\MSBuild.exe Tychaia.Windows.sln
```

and observed the output of the build stream from the Windows machine into Phabricator.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7521
2013-11-08 18:15:07 -08:00
Bob Trahan
d0de4dab24 Pholio - fix a bug replacing multiple images
Summary: these transactions should //never// merge since they are always created for a 1:1 replacement. (ie any merging would be implicitly erroneous).  Fixes T4081

Test Plan: made a mock with three images and replaced all three successfully. replaced image A with image B, did not save, replaced image B with image C, then saved and verified transaction correctly showed image A replaced with image C.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4081

Differential Revision: https://secure.phabricator.com/D7536
2013-11-08 17:13:36 -08:00
James Rhodes
7ffea0463e Use herald to trigger builds of revisions and commits.
Summary:
Depends on D7500.

This seemed like a pretty good idea once I thought of it.  Instead of having some custom triggering logic instead Harbormaster, I figured it best to leverage all of Herald's power so that users can create rules to apply builds to commits and differential revisions.  This gives the added advantage that they can trigger off builds for particular types of revisions and commits, which seems like it could be really useful (e.g. run extra tests against revisions that touch sensitive areas of the code).

Test Plan: Ran the usual daemons + the Harbormaster daemon.  Pushed a commit to the repository and saw both the buildable and build get created when the commit worked picked it up.  Submitted a diff and saw both the buildable and build get created when the Herald rules were evaluated for the diff.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran, hwinkel

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7501
2013-11-08 16:58:39 -08:00
Bob Trahan
3de4e91dac Naunce - capalities for Source object
Summary: another little piece here that basically just adds some permissions to source editing. serving it up before I do anything too complicated to make sure it seems kosher. in terms of what comes next this form needs to be dynamic based on source type so there'll be some fun there. That said, I plan to implement a more simple "phabricator form" only version to start here and flesh out a few other things like queues with that.

Test Plan: set permission to no one for source edit and got a nice error page.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7535
2013-11-08 12:45:14 -08:00
Asher Baker
d700e7f22d Add support for landing to hosted Mercurial repos.
Summary: I've kept this as close as possible to the Git version for ease of review and later refactoring of them both together. At minimum, the functions to get the working dir should probably be cleaned up one day.

Test Plan: Landed a revision.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7534
2013-11-08 11:39:32 -08:00
epriestley
1c73d6cd06 Tighten a regular expression in repository.create
Summary:
See <https://github.com/facebook/phabricator/issues/433>. We were missing a "^" here.

This should be moved over to transactions soon and then we can get rid of the duplication. :/

Test Plan: Tried to create a repository with callsign "9X", got a helpful error about "ALL UPPERCASE LETTERS".

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7531
2013-11-08 11:09:09 -08:00
Anirudh Sanjeev
1bcf478c69 Fix weird issue where INF is not respected in PHP 5.5
Summary:
After upgrading to PHP 5.5, the conduit list was not fully visible because
INF was being treated as "0" for some reason. Fixed by making it a PHP_MAX_INT

Test Plan: Checked on PHP 5.5

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7530
2013-11-08 07:07:10 -08:00
epriestley
f68d67530a Fix an issue with pulling Subversion blame data
Summary:
Fixes T4067. The way `DiffusionCommitQuery` works prevents it from loading SVN identifiers in some cases without additional constraints, since "12345" might be an SVN revision 12345, or it might be the first 5 characters of a Git commit hash.

Introduce `withRepository()` as a shorthand for `withDefaultRepository()` + `withRepositoryIDs()`. This tells the query to:

  - Only look in the given repository; and
  - use the more liberal identifier resolution rules while doing so.

The practical impact this has is that blame tooltips in SVN work again. The other queries which are fixed here were never run in SVN (which doesn't have first-class branches or tags); I've cleaned them up only for completeness.

Test Plan:
  - Viewed blame in SVN, saw information again instead of empty tooltip.
  - Viewed brnaches/tags in Mercurial and Git.

{F79226}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4067

Differential Revision: https://secure.phabricator.com/D7523
2013-11-07 12:10:43 -08:00
Bob Trahan
9084f1fe8c Phriction - make the check for project sub pages more fine-grained
Summary:
we were just checking if projects/ was in the URI before barfing. Use some more fun utility functions such that we only complain if there is no project.

Fixes T4071.

Test Plan: made a subpage under a project - success! tried to make a project wiki page where there was no project - successful failure! tried to make a project wiki sub page where there was no project - successful failure!

Reviewers: epriestley, chad

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4071

Differential Revision: https://secure.phabricator.com/D7527
2013-11-07 12:06:42 -08:00
epriestley
0e65740c2d Support ecdsa-sha2-nistp256 SSH keys
Summary:
See discussion in <https://github.com/facebook/phabricator/issues/430>.

(If we end up with more than like 5 of these we should probably make this a warning or something instead, the only goal is to prevent user error.)

Test Plan:
{F79196}

{F79197}

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran, enko

Differential Revision: https://secure.phabricator.com/D7522
2013-11-07 11:33:46 -08:00
epriestley
038f5323c0 Allow Herald's "Reviewers" field to select project reviewers
Summary: The "Reviewers" condition in Differential Revision rules has the wrong typeahead and can't select projects, but should be able to.

Test Plan: {F79273}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7526
2013-11-07 11:23:31 -08:00
epriestley
6324669748 Allow Phabricator to serve Mercurial repositories over HTTP
Summary: Ref T2230. This is easily the worst thing I've had to write in a while. I'll leave some notes inline.

Test Plan: Ran `hg clone http://...` on a hosted repo. Ran `hg push` on the same. Changed sync'd both ways.

Reviewers: asherkin, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7520
2013-11-06 18:00:42 -08:00
epriestley
44a40eaf57 Split Diffusion VSC serve code into its own controller
Summary: This is starting to get a bit sizable and it turns out Mercurial is sort of a beast, so split the VCS serve stuff into a separate controller.

Test Plan: Pushed and pulled an authenticated Git repository.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, hach-que

Differential Revision: https://secure.phabricator.com/D7494
2013-11-06 17:55:46 -08:00
epriestley
5b873a74de Move Mercurial discovery to PhabricatorRepositoryDiscoveryEngine
Summary: Ref T4068. Partly, this moves discovery to the more unit-testable PhabricatorRepositoryDiscoveryEngine. It also fixes some issues, see inlines.

Test Plan: In a Mercurial repository, ran `bin/repository discover --repair`, verified commits came out topographically sorted. Ran without `--repair` and in various other contexts, like with no commits to discover and some-but-not-all commits to discover.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4068

Differential Revision: https://secure.phabricator.com/D7518
2013-11-06 17:20:22 -08:00
Bob Trahan
66ae64f7bc Nuance - get some scaffolding up there
Summary:
I updated the wiki too - https://secure.phabricator.com/w/projects/pebkac/ - with what I am thinking right now. Rough plan here is

 - next diff:
  - implement editors and transactions
  - implement "web type" for contact source
   - /pebkac/item/new/ will be the entry point for this
  - implement "actions" on a contact
  - probably some "polish" on the scaffolding laid out here; like "create" permissions maybs
 - diffs after that:
  - implement "twitter" type for source
  - implement email reply handler stuff for item and source

Probs a great time to blast huge holes in all this stuff. :D

Test Plan: these pages load and arc lint doesn't complain

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran, chad

Differential Revision: https://secure.phabricator.com/D7465
2013-11-06 17:00:09 -08:00
epriestley
4f0f95f7b5 Assign PHIDs to all diffs
Summary:
Ref T1049. Ref T2222. `DifferentialDiff` does not currently have a PHID, but we need it for Harbormaster and ApplicationTransactions. See some discussion in D7501.

(I split the SQL into two sections so we can't fail in the middle. At some point, I'd like to do a pass on the migration stuff and get this happening automatically, and also simplify the PatchList.)

Test Plan:
  - Ran `bin/storage upgrade`.
  - Checked for valid PHIDs in the database.
  - Used `phid.query` to look up a diff by PHID.
  - Created a new diff and verified it got a PHID.

Reviewers: btrahan, hach-que

Reviewed By: btrahan

CC: aran, vrana

Maniphest Tasks: T2222, T1049

Differential Revision: https://secure.phabricator.com/D7513
2013-11-06 13:59:06 -08:00
epriestley
bd29784a32 Add an administrative bin/repository importing command to list importing commits
Summary: Ref T4068. Adds a command to list all commits in an "importing" status. This will allow users to use `reparse.php` to diagnose and repair issues.

Test Plan:
  - Ran `bin/repository importing P`, etc.
  - Used `reparse.php` to reparse some commit stages and saw status update correctly.
  - Ran on a repo with no importing commits.
  - Ran with `... --simple | xargs`, which saves us having to put an `awk` or something in there for users.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4068

Differential Revision: https://secure.phabricator.com/D7515
2013-11-06 11:26:41 -08:00
epriestley
e3a5ab1f8c Add an administrative bin/repository mark-imported command
Summary:
Ref T4068. In some cases like that one, I anticipate a repository not fully importing when a handful of random commits are broken. In the long run we should just deal with that properly, but in the meantime provide an administrative escape hatch so you can mark the repository as imported and get it running normally.

The major reason to do this is that Herald, Feed, Harbormaster, etc., won't activate until a repository is "imported".

Test Plan:
  - Tried to mark an imported repository as imported, got an "already imported" message.
  - Same for not-imported.
  - Marked a repository not-imported.
  - Marked a repository imported.
  - Marked a repository not-imported, then waited for the daemons to mark it imported again automatically.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, kbrownlees

Maniphest Tasks: T4068

Differential Revision: https://secure.phabricator.com/D7514
2013-11-06 11:26:24 -08:00
epriestley
377742c23a Get some level of meaningful status information into Harbormaster buildable list
Summary: Ref T1049. Nothing fancy, but shows red for fail/error and green for pass. See discussion in D7502.

Test Plan: {F78839}

Reviewers: hach-que, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7512
2013-11-06 11:26:17 -08:00
James Rhodes
8cae7f999f Make the buildables list a little nicer.
Summary:
Depends on D7501.

This just renders the buildable's actual object name onto the list, so you can see at a glance what the buildable represents.  I'd like to also pull across a list of builds of this buildable and change the bar color, but I'm not quite sure how to do that in the search architecture without N+1 querying.

Test Plan:
Looked at the buildable list and it looked like this:

{F78555}

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7502
2013-11-06 06:14:26 -08:00
Chad Little
b6c88bd6c1 Merge branch 'master' of github.com:facebook/phabricator 2013-11-05 19:05:22 -08:00
Chad Little
f691e35195 Add Fluid-ish view to AphrontMultiColumnView
Summary: Cleans up some CSS while adding lots of other... Mainly, this allow min-width "tables" that trigger a scroll-bar, but go full width if larger than min.

Test Plan: Tested Workboard Examples and some Project pages, Chrome, Tablet and Mobile Layouts

Reviewers: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7509
2013-11-05 18:57:52 -08:00
epriestley
ebcd60eae5 Fix an issue with non-bare Git repositories and non-master branches
Summary: This is a little funky but fixes an issue with Git repos that are
non-bare needing "origin/" to resolve branches other than "master". Eventually
this should get cleaned up.

Test Plan: Reporting user verified this fixed their issue.

Auditors: btrahan
2013-11-05 17:38:15 -08:00
epriestley
d5f41ef70e Return HTTP 500, not HTTP 200, on exception pages
Summary: Ref T4064. The response code here isn't normally relevant, but we can hit these via `git clone http://../`, etc., and it's clearly more correct to use HTTP 500.

Test Plan: Added a fake `throw new Exception()` and verified I got an HTTP 500 response.

Reviewers: jamesr, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4064

Differential Revision: https://secure.phabricator.com/D7507
2013-11-05 15:25:12 -08:00
epriestley
ce48375951 Don't throw when user tries to use an empty password via HTTP auth
Summary: Fixes T4064. See discussion there.

Test Plan: Tried `git clone http://...` with empty password, got 403. Retried with actual password, got a clone.

Reviewers: jamesr, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4064

Differential Revision: https://secure.phabricator.com/D7508
2013-11-05 15:24:58 -08:00
James Rhodes
e4569e7e7e Implement interface for adding, editing and deleting build steps on plans.
Summary: This implements an interface for adding new build steps, editing existing build steps and deleting build steps from build plans.  It uses the settings definitions on the build implementation to work out what fields should be displayed on the edit page.

Test Plan:
See screenshots:

{F78529}

{F78532}

{F78528}

{F78531}

{F78527}

{F78530}

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7500
2013-11-05 14:08:44 -08:00
James Rhodes
c514d34b94 Add build step implementation infrastructure and sleep build step.
Summary:
Depends on D7498.

This implements support for a "build step implementation".  Build steps have an associated class name (which makes the class in PHP) and a details field, which is serialized JSON (same as PhabricatorRepository).

This also implements a SleepBuildStepImplementation which just pauses the build for a specified period of seconds.

Test Plan:
Inserted a build step with `insert into harbormaster_buildstep (phid, buildPlanPHID, className, details, dateCreated, dateModified) values ('', 'PHID-HMCP-zkh5w6czfbfpk2gxwdeo', 'SleepBuildStepImplementation', '{"seconds":5}', NOW(), NOW());` (adjusting the build plan PHID as appropriate).

Started the daemon and applied the build plan to a buildable, and saw the daemon take a 5 second delay after creating `SleepBuildStepImplementation`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran, chad

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7499
2013-11-05 13:34:44 -08:00
Aviv Eyal
5c0edc9351 Land Revision button for hosted git repos
Summary:
ref T182.

Simple approach of clone, patch, push. While waiting for drydock, implement a hackish mutex
setup for the workspace, which should work ok as long as there's only one committer who is
carefull about theses things.

Less obvious note: This is taking the both author and commiter's 'primary email' for the commit -
which might rub some people wrong.

Test Plan:
With a hosted repo, created some diffs and landed them.
Also clicked button for some error cases, got the right error message.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: hach-que, Korvin, epriestley, aran

Maniphest Tasks: T182

Differential Revision: https://secure.phabricator.com/D7486
2013-11-05 13:00:13 -08:00
James Rhodes
ca5400d14b Implement basic Harbormaster daemon and start builds.
Summary: This implements a basic Harbormaster daemon that takes pending builds and builds them (currently just sleeps 15 seconds before moving to passed state).  It also implements an interface to apply a build plan to a buildable, so that users can kick off builds for a buildable.

Test Plan: Ran `bin/phd debug PhabricatorHarbormasterBuildDaemon` and used the interface to start some builds by applying a build plan.  Observed them move from 'pending' to 'building' to 'passed'.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7498
2013-11-05 12:48:36 -08:00
epriestley
5cc26f065d Expand "local working copy" conditional check in Diffusion
Summary: We don't care about any disk resources at all for non-hosted SVN repositories. See <https://github.com/facebook/phabricator/issues/428>.

Test Plan: Looked at a non-hosted SVN repo, saw no storage info.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7506
2013-11-05 10:01:44 -08:00
epriestley
71e6386a73 Fix issue with new branch query data getting used incorrectly
See <https://github.com/facebook/phabricator/issues/429>
2013-11-05 09:59:31 -08:00
epriestley
7414777856 Select all available bodies when rendering a feed story
Summary:
Fixes T4060. The logic here is:

  - When you take several actions at once, we show a single feed story for all of them.
  - We choose the "most interesting" title for the feed story. For example, "close task" is more interesting than "add CC".

Currently, the issue with this is:

  - "Add comment" is the //least interesting// title. I think this is correct: all other actions are more interesting than the fact that you added a comment.
  - We try to conserve the number of objects we need to load by rendering only the most interesting transaction.

To fix this:

  - Stop being so conservative; load all of the transactions and all of their PHIDs.
  - Add bodies from any transactions which render bodies. In all cases (I think?) this is a maximum of one comment adding a body.

The end result is a story like this:

  epriestley closed T123: the building is on fire.

  "Okay guys I put the fire out"

Test Plan: See screenshot.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran, asherkin

Maniphest Tasks: T4060

Differential Revision: https://secure.phabricator.com/D7504
2013-11-05 09:03:59 -08:00
epriestley
9d069963ed Add more status checks for binaries
Summary:
Expands on D7488, which looks way better than the config checks. I'm leaving the config checks for now, but maybe we should just get rid of them? This advice is delivered in a far more timely way.

  - Check for normal VCS binaries too.
  - Link to `environment.append-paths`.
  - Get rid of untranslated names (I think they're probably not too useful?)

Test Plan: See screenshots.

Reviewers: hach-que, btrahan

Reviewed By: hach-que

CC: aran

Differential Revision: https://secure.phabricator.com/D7495
2013-11-04 15:14:01 -08:00
James Rhodes
10659ece8e Warn when 'git-http-backend' is not on the PATH.
Summary:
Currently if 'git-http-backend' is not on the PATH, there is no visible message to the user other than "info/refs: is this a valid git repository?" when trying to clone.  This adds a setup check so that if there are any Git repositories in use, it will check for the existance of the "git-http-backend" binary in the PATH.

I believe this is shipped by default alongside the git package on most distros, but in some (such as OpenSUSE), this binary isn't on the PATH by default.

Test Plan: Removed `/usr/lib/git` from my `environment.append-paths` and saw the message appear.  Added it back and the message went away.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4050

Differential Revision: https://secure.phabricator.com/D7488
2013-11-04 15:03:08 -08:00
epriestley
4f20530856 Merge "expandshortcommitquery" and "stablecommitnamequery" into "resolverefs"
Summary:
Ref T1493. Diffusion has some garbagey behavior for things we can't resolve. Common cases are:

  - Looking at a branch that doesn't exist.
  - Looking at a repository with no branches.
  - Looking at a commit that doesn't exist.
  - Looking at an empty repository.

In these cases, we generally fatal unhelpfully. I want to untangle this mess.

This doesn't help much, but does clean things up a bit. We currently have two separate query paths, "stablecommitname" and "expandshortcommit". These are pretty much doing the same thing -- taking some ref like "master" or "default" or a tag name or part of a commit name, and turning it into a full commit name. Merge them into a single "resolverefs" method.

This simplifies the code a fair bit, and gives us better error messages. They still aren't great, but they're like this now:

  Ref "7498aec194ecf2d333e0e2baddd9d5cdf922d7f1" is ambiguous or does not exist.

...instead of just:

  ERR-INVALID-COMMIT

Test Plan: Looked at Git, Mercurial and Subversion repositories that were empty and non-empty. Looked at branches/heads. Tried to look at invalid commits. Looked at tags. All of this still works, and some behaviors are a bit better than they used to be.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1493

Differential Revision: https://secure.phabricator.com/D7484
2013-11-04 14:13:07 -08:00
epriestley
8f3ae81143 Fix tag content display in Git
Summary: Fixes the junk I broke in D7484. Before that, tag content was a side effect of resolving the ref name. Now, fetch it explicitly in `diffusion.tagsquery`.

Test Plan: Looked at a tag, saw the annotation/message.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7485
2013-11-04 12:16:53 -08:00
epriestley
90a9e90675 Record Conduit calls in the service profiler
Summary: This puts Conduit calls into the "Services" tab. They aren't always real service calls, but I think they're big enough to belong there and be useful.

Test Plan: Viewed "Services" tab, saw conduit calls.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7482
2013-11-04 12:15:48 -08:00
epriestley
b90e51ab0e Clean up hg --debug branches calls
Summary: Ref T1493. Consolidate these a bit; they might need some more magic once we do `--noupdate` checkouts. Mostly just trying to clean up and centralize this code a bit.

Test Plan: Viewed and `bin/repository discover`'d Mercurial repos with and without any branches.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1493

Differential Revision: https://secure.phabricator.com/D7480
2013-11-04 12:15:32 -08:00
Jakub Vrana
00fb8c22e4 Decentralize phtize
Summary: The idea is to have all `phtize` definitions in applications to allow their separation.

Test Plan: Clicked View Options after mangling the translation.

Reviewers: epriestley

Reviewed By: epriestley

CC: btrahan, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7345
2013-11-04 11:38:28 -08:00
Chad Little
1c31ea3a60 Add header icons to PHUIPropertyListView
Summary: Adds summary (description) and test plan icons to make these area's more unique and differentiated over general sections.

Test Plan: Test a diff, a commit, a task

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7493
2013-11-04 11:07:51 -08:00
Chad Little
5e2de53522 Touch up Phriction History page
Summary: Adds an ObjectBox. Fixes T4047

Test Plan: View updated Phriction History Page.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4047

Differential Revision: https://secure.phabricator.com/D7487
2013-11-04 08:54:00 -08:00
James Rhodes
3e2efaf00e Disable CSRF checks on Git push when updating repository.
Summary: This disables CSRF checking around the `$repository->writeStatusMessage` so that pushing changes over HTTP to Git repositories doesn't fail miserably.

Test Plan: Applied this fix and I could `git push` to hosted repositories again.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4052

Differential Revision: https://secure.phabricator.com/D7490
2013-11-04 07:43:29 -08:00
James Rhodes
0ceb53bfae Fix issue where Git authentication would always 403 on non-public install.
Summary: This fixes an issue where Git authentication would always fail on an install with `policy.allow-public` set to false.  This is because when public access is allowed, anonymous users can query the user list.  However, when public access is not allowed, you have to be authenticated before you can read any of the user objects.

Test Plan:
Prior to this fix, I get:

```
james@james-laptop:~/git/8> git clone http://phabricator.local/diffusion/TEST/
Cloning into 'TEST'...
fatal: unable to access 'http://phabricator.local/diffusion/TEST/': The requested URL returned error: 403
```

when `policy.allow-public` is false.  After this fix I get:

```
james@james-laptop:~/git/8> git clone http://phabricator.local/diffusion/TEST/
Cloning into 'TEST'...
remote: Counting objects: 102, done.
remote: Compressing objects: 100% (71/71), done.
remote: Total 102 (delta 6), reused 0 (delta 0)
Receiving objects: 100% (102/102), 9.89 KiB | 0 bytes/s, done.
Resolving deltas: 100% (6/6), done.
Checking connectivity... done
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4049

Differential Revision: https://secure.phabricator.com/D7489
2013-11-04 07:29:21 -08:00
epriestley
3dcef4f37b Minor, restore a missing "break;" that I derped somehow. 2013-11-02 18:18:12 -07:00
James Rhodes
24579e2956 Add phame.queryblog and phame.querypost Conduit calls.
Summary: This implements Conduit calls for querying Phame blogs and Phame posts.

Test Plan: Made some calls and they seem to generally work.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3695

Differential Revision: https://secure.phabricator.com/D7478
2013-11-02 16:31:30 -07:00
James Rhodes
10a9f6501f Support processing Remarkup in bulk with remarkup.processbulk Conduit method
Summary: This adds support for a Conduit method to process Remarkup content in bulk.  It also updates the `getEngineContexts` methods to support any missing contexts.

Test Plan: Ran the command and processed a few sets of text.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4046

Differential Revision: https://secure.phabricator.com/D7479
2013-11-02 16:30:11 -07:00
Chad Little
7498aec194 Fix Phriction email language
Summary: This adds pht's and such english to Phriction email body.

Test Plan: Edited a Document, Moved a Document. Got new emails. Such Wow.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7477
2013-11-02 08:43:53 -07:00
epriestley
cd674931fc When creating a repository in Diffusion, prompt for "Create" or "Import" first
Summary:
Ref T2230. This will need some more refinement, but basically it adds a "Create" vs "Import" step before we go through the paged workflow.

  - If you choose "Create", we skip the remote URI / auth stuff, and then set the "hosted" flag.
  - If you choose "Import", we do what we do now.

Test Plan: Created and imported repos.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7475
2013-11-01 17:39:35 -07:00
epriestley
3607bd487c Survive pull/discover for hosted repositories in all VCSes
Summary:
Hosted repositories only sometimes survive the pull/discover phases right now, due to issues like:

  - Pull tries to `git clone`, but should `git init`.
  - Mercurial doesn't handle empty repositories with on branches.
  - SVN tries to connect to an invalid remote.
  - None of them set the INIT repo flag correctly, so status doesn't get updated properly in the UI.

Fix all this stuff.

Test Plan:
  - For each of Git, SVN and Mercurial:
    - Created a new repository from the web UI in a deactivated state.
    - Made it hosted.
    - Manually ran pull/discover.
    - Verified we end up with initialized, empty repositories in consistent states.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7474
2013-11-01 17:36:30 -07:00
epriestley
a0e820ad9a Improve repository hinting and feedback
Summary:
  - Warn about "Read/Write" instead of disabling it, to prevent edits which mutate it after changing a hosted repository to an unhosted one.
  - Warn about authenticated connections with HTTPS auth disabled, and link to the relevant setting.
  - When "Autoclose" is disabled, show that "Autoclose Branches" won't have an effect.
  - For hosted repositories, show the HTTP and SSH clone URIs.
    - Make them easy to copy/paste.
    - Link to credential management.
    - Show if they're read-only.
    - This could be a bit nicer-looking than it is.

Test Plan: Looked at repositories in a bunch of states and made various edits to them.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7471
2013-11-01 17:35:43 -07:00
epriestley
d1649d176f Mark IMPORTED_CHANGE more consistently
Summary:
See <https://github.com/facebook/phabricator/issues/425>. There are some ways that the change parsers may not reach `finishParse()`, but we now need them to in order to mark the commit imported, advance the progress bar, and eventually kick the repository out of IMPORTING status.

Take all the copy/pasted code in the parsers and move it into the parent. Specifically, this is:

  - Printing a status message about starting a parse;
  - checking for bad commits;
  - queueing the next parse stage; and
  - marking the import step complete.

Test Plan: Used `reparse.php --change` to reparse Git, SVN and Mercurial repos.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7470
2013-11-01 12:54:10 -07:00
epriestley
43fd567ef4 Implement writes over HTTP for Git.
Summary: Depends on D7642.  This updates the authentication logic so that HTTP writes can be made to Git repositories hosted by Phabricator.

Test Plan: Set the policy to allow me to push and I was able to.  Changed the policy to disallow push and I was no longer able to push.

Reviewers: #blessed_reviewers, hach-que

Reviewed By: hach-que

CC: Korvin, epriestley, aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7468
2013-11-01 08:44:37 -07:00
epriestley
0278b15ceb Implementation of VCS passwords against user.
Summary: This allows users to set their HTTP access passwords via Diffusion interface.

Test Plan: Clicked the "Set HTTP Access Password" link, set a password and saw it appear in the DB.

Reviewers: #blessed_reviewers, hach-que, btrahan

Reviewed By: hach-que

CC: Korvin, epriestley, aran, jamesr

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7462
2013-11-01 08:34:11 -07:00
epriestley
40b0818207 Show additional status information during repository import
Summary:
Ref T2350. Fixes T2231.

  - Adds log flags around discovery.
  - Adds message flags for "needs update". This is basically an out-of-band hint to the daemons that a repository should be pulled sooner than normal. We set the flag when users push a revision, and expose a Conduit method that `arc land` will be able to use.

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2350, T2231

Differential Revision: https://secure.phabricator.com/D7467
2013-10-31 15:46:57 -07:00
epriestley
2f7057ad99 Preserve linebreaks again in Summary / Test Plan for revisions
Summary:
I pulled these into the property list recently, which made them more consistent, but that dropped "preserve linebreaks". Since these usually come from the CLI, render with linebreaks preserved.

@csilvers, you'll need to `bin/cache purge --purge-remarkup` after this if you want to fix existing revisions.

Test Plan: Made a revision with some poetry, saw poetry preserved.

Reviewers: btrahan, csilvers

Reviewed By: btrahan

CC: csilvers, aran

Differential Revision: https://secure.phabricator.com/D7464
2013-10-30 17:20:06 -07:00
Aviv Eyal
2250ee6aa6 Allow null for token expiration date
Summary: At least under GitHub, the token value is stored as "null", and not missing. And `null > anything` is false, so Phabricator thinks the token is expired or not there.

Test Plan: http://ph.vm/settings/panel/external/ before shows "No OAuth Access Token," and after it says "Active OAuth Token".

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7466
2013-10-30 17:19:46 -07:00
epriestley
3a39b01233 Add "RepositoryStatusMessage" and detailed information about initilization
Summary:
`RepositoryStatusMessage` is basically a key/value table associated with a repository that I'm using to let the daemons store the most recent event of a given type, so we can easily show it on the status dashboard. I think this will be a lot easier for users to figure out than digging through logfiles.

I'm also going to write the "this needs a pull" status here eventually, for reducing the time lapse between pushes and discovery.

  - Add storage for these messages.
  - Have the pull engine populate the INIT phase. I'll do the FETCH phase next.
  - Update the status readout to show all the various states.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7461
2013-10-30 16:04:19 -07:00
Chad Little
edd8bea85b Clean up legalpad sign UI
Summary: Moves stuff to ObjectBox, Error UI

Test Plan: Signed Document, Missed a Form Field, Resigned Form Document. Fixes T4037

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7463
2013-10-30 15:50:46 -07:00
epriestley
5ef4954a36 Remove "daemons not running" code from Repositories
Summary: This moved into Diffusion in D7458 and is now presented in a much cleaner, more targeted way.

Test Plan: Loaded `/repository/`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7459
2013-10-30 13:15:41 -07:00
epriestley
2d01da14fd Provide detailed status information about repository state/progress
Summary:
Replace the blanket "daemons not running" warning with a lot more specific detail, to try to make it easier for users to figure out how to set up repositories correctly.

The next change here will add some additional status information from the daemons, so this panel can report results in greater detail.

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7458
2013-10-30 13:15:32 -07:00
epriestley
93d7a1451b Clean up file browse view a bit
Summary:
  - Use DiffusionCommitQuery
  - Get rid of the "Author" column.
  - Collapse commit + revision together.
  - Better tooltips to cover for the removed information.
  - Colorize only the "line" column.
  - Generally, reduce the amount of visual noise and non-code-stuff going on in this interface.
  - I'd like to make the "<<" thing look nicer too but that might take some actual design.

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: chad

CC: chad, aran

Differential Revision: https://secure.phabricator.com/D7457
2013-10-30 13:15:26 -07:00
epriestley
d51ae49f61 Clean up Diffusion dedicated tag table view
Summary: Minor cleanup. Make the "imported" check less strict (we don't need owners or herald to show change status). Export the "imported" flag over Conduit.

Test Plan: Viewed tag table. Viewed partially imported repositories.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7455
2013-10-30 13:15:14 -07:00
epriestley
bf2fffe264 Clean up dedicated branch table view
Summary: Swap to DiffusionCommitQuery, other minor cleanup.

Test Plan: Viewed page, forced error view and looked at it.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7454
2013-10-30 13:14:56 -07:00
epriestley
05884e5042 Add a quoted body stripping pattern for "> On ... at ... wrote:"
Summary: See @scottmac's reply in T3982. It looks like his email client uses the standard quote string, but includes it in the quoted block.

Test Plan: Added a failing unit test, made it pass.

Reviewers: btrahan

Reviewed By: btrahan

CC: scottmac, aran

Differential Revision: https://secure.phabricator.com/D7440
2013-10-30 13:07:18 -07:00
epriestley
f08908ff35 Raise a setup warning for missing or invalid local repository directory
Summary: I'm planning to add more detailed info to Diffusion itself, but catch the big issue here.

Test Plan: Hit config issue locally, then resolved it.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7439
2013-10-30 13:07:09 -07:00
epriestley
d5bc8197ec Remove "isUnparsed" flag from commits
Summary: The new "importStatus" property provides a much stronger and more consistent version of this flag. The only callsite was removed by D7452.

Test Plan: Used `grep` to check for callsites and found none.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7453
2013-10-30 13:06:57 -07:00
epriestley
25656b311a Clean up Diffusion tag list query a bit
Summary:
  - Use DiffusionCommitQuery.
  - Use denormalized summary.
  - Use stronger "Importing" test.

Test Plan: Viewed `/diffusion/X/` for repos with tags. Saw tags; saw importing commits marked as "Importing" instead of "Unknown".

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7452
2013-10-30 13:06:43 -07:00
epriestley
d1c4b5081c Clean up Diffusion branch query a bit
Summary:
Ref T2716.

  - Serve from `DiffusionCommitQuery`, not `PhabricatorAuditCommitQuery` (which should probably die).
  - Fix logic for `limit`, which incorrectly failed to display the "Showing %d branches." text.
  - Clean up things a touch.
  - I didn't end up actually needing `needCommitData()`, but left it in there since I think it will be needed soon.
  - Removed a "TODO" because I don't remember what "etc etc" means.

Test Plan: Looked at branches in several repositories.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2716

Differential Revision: https://secure.phabricator.com/D7451
2013-10-30 13:06:28 -07:00
epriestley
70b53c49fd Fix an issue with viewing an undiscovered commit in Diffusion
Summary: If you load Diffusion between a repository being pulled and discovered, you can end up with a valid commit reference that hasn't been discovered yet. Don't fatal.

Test Plan: Saw somewhat-helpful error page instead of fatal.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7448
2013-10-30 13:06:15 -07:00
epriestley
3bf372c60c Consolidate querying of things which we can use git for-each-ref for
Summary: Ref T2230. This cleans up D7442, by using `git for-each-ref` everywhere we can, in a basically reasonable way.

Test Plan:
In bare and non-bare repositories:

  - Ran discovery with `bin/repository discover`;
  - listed branches on `/diffusion/X/`;
  - listed tags on `/diffusion/X/`;
  - listed tags, branches and refs on `/diffusion/rXnnnn`.

Reviewers: btrahan, avivey

Reviewed By: avivey

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7447
2013-10-30 13:06:09 -07:00