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

1059 commits

Author SHA1 Message Date
epriestley
04eb1f98e2 Always mark revisions as "updated" when users comment on them
Summary: See T773 and the explanatory inline comment.

Test Plan: Made no-action comments and comments that did something (reject, plan
changes) to revisions. Saw them always jump to the top of the action list.

Reviewers: jungejason, simpkins, btrahan

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T773

Differential Revision: https://secure.phabricator.com/D1386
2012-01-12 20:08:26 -08:00
epriestley
f5e1e3377c Add basic draft support to Phriction
Summary:
  - When a user is creating a Phriction document, save a draft as
"phriction:<slug>".
  - When a user is editing a Phriction document, save a draft as "<document
phid>:<document version>".
  - If a user has an available draft, use that instead of the native content.
  - If using a draft, tell the user and give them an option to discard it.
  - If a page is updated, your draft is lost (we show new page content
unconditionally) but this should be rare and is the simplest way to resolve this
issue in a realtively consistent way.

Test Plan:
  - Recovered drafts for new and edited pages.
  - Used "nodraft" to discard drafts.

Reviewers: davidreuss, btrahan, jungejason

Reviewed By: davidreuss

CC: aran, davidreuss

Maniphest Tasks: T769

Differential Revision: https://secure.phabricator.com/D1378
2012-01-12 18:04:20 -08:00
Jason Ge
cf35a640ec Enable filtering for committed revisions
Summary:
engineers requested to supporting filtering by 'committed'
revisions, and I think it makes sense.

Test Plan: verified that all the three options worked

Reviewers: epriestley, btrahan, nh

Reviewed By: nh

CC: nh, wolffiex, aran

Differential Revision: https://secure.phabricator.com/D1383
2012-01-12 17:02:20 -08:00
epriestley
bfbe6ec594 Prevent login brute forcing with captchas
Summary: If a remote address has too many recent login failures, require they
fill out a captcha before they can attempt to login.

Test Plan: Tried to login a bunch of times, then submitted the CAPTHCA form with
various combinations of valid/invalid passwords and valid/invalid captchas.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, epriestley, jungejason

Maniphest Tasks: T765

Differential Revision: https://secure.phabricator.com/D1379
2012-01-12 15:22:05 -08:00
epriestley
7f7710a24d Add @phutil-external-symbol declarations to Phabricator
Summary: See D1381.

Test Plan: Ran "arc liberate src/ --all" and got a clean rebuild.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T762

Differential Revision: https://secure.phabricator.com/D1382
2012-01-12 15:20:18 -08:00
epriestley
d84c0a5be5 Validate all fields in differential.parsecommitmessage
Summary:
  - We currently run ##parseValueFromCommitMessage()## on all fields present in
the message, but not ##validateField()##.
  - This detects value errors (e.g., an invalid reviewer) but not higher-level
errors (e.g., a missing field).
  - This can break the stacked-commits Git mutable history workflow by
recognizing too many commit messages as valid ("multiple valid commit messages,
this is ambiguous").
  - This also gives you some errors ("Missing test plan") too late in "arc diff
--create" (after the diff has been built).

Test Plan:
  - Grepped for validateField() calls, removed a couple of calls that had the
same implementation as the base class.
  - Grepped for other calls to this to make sure I'm not stumbling into
unintended side effects, but it only runs from the diff workflow.
  - Ran "arc diff --create" with an invalid test plan, got a good error early in
the process.
  - Ran "arc diff master" with stacked local commits, got a correct selection of
the intended message.

Reviewers: cpiro, btrahan, jungejason

Reviewed By: cpiro

CC: aran, cpiro

Differential Revision: https://secure.phabricator.com/D1373
2012-01-12 15:20:11 -08:00
epriestley
02fb5fea89 Allow configuration of a minimum password length, unify password reset
interfaces

Summary:
  - We have a hard-coded minimum length of 3 right now (and 1 in the other
interface), which is sort of silly.
  - Provide a more reasonable default, and allow it to be configured.
  - We have two password reset interfaces, one of which no longer actually
requires you to verify you own the account. This is more than a bit derp.
  - Merge the interfaces into one, using either an email token or the account's
current password to let you change the password.

Test Plan:
  - Reset password on an account.
  - Changed password on an account.
  - Created a new account, logged in, set the password.
  - Tried to set a too-short password, got an error.

Reviewers: btrahan, jungejason, nh

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T766

Differential Revision: https://secure.phabricator.com/D1374
2012-01-12 07:39:13 -08:00
epriestley
8b3ab97d64 Fix fatal on project list
Summary:
Until T605 gets fixed, you might end up with a Project without a Profile if the
Profile insert failed. This fatals the list view; instead, don't fatal if a
profile is missing.

(At some point we should probably just merge this field into the Project object,
I was just mimicking the user/profile separation but we have partial-field
object support now and Projects aren't super heavily used or very big.)

Test Plan:
  - Viewed list view including a project with a missing profile.
  - Edited the project, creating its profile.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: arice, aran, btrahan

Differential Revision: https://secure.phabricator.com/D1368
2012-01-12 06:52:24 -08:00
epriestley
65a56c6ce0 Improve mailing list edit form
Summary:
  - Add some captions to make it more clear what these fields mean.
  - Require "name", since tokenizers use it exclusively.
  - Limit URI to allowed protocols, since admins can currently XSS users by
entering a "javascript:" URI and then tricking the user into clicking the
mailing list name. This exploit is dumb, but technically privilege escallation.

Test Plan:
  - Created a new mailing list.
  - Edited a mailing list.
  - Tested URI: valid, invalid, omitted.
  - Tested name: valid, omitted.

Reviewers: btrahan, jungejason, davidreuss

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1365
2012-01-11 15:48:21 -08:00
kevin
beaee478d3 Added conduit method to get maniphest transactions
Summary:
Added a Conduit API method to return all transactions for a
given set of task_ids. This will be used to comments and other important
information about the tasks.

Test Plan:
Use Conduit to execute ##maniphest.gettasktransactions## and
visually verify that transaction information is returned.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1361
2012-01-11 09:13:59 -08:00
epriestley
b10fa8023f Support Git implicit file:// URIs
Summary: @s reported an issue with implicit file:// URIs in Git, see P270.
Recognize and handle URIs in this format. For URIs we don't understand, raise an
exception.

Test Plan:
  - Added failing tests.
  - Fixed code.
  - Tests pass.

Reviewers: btrahan, jungejason, s

Reviewed By: s

CC: aran, epriestley, s

Differential Revision: https://secure.phabricator.com/D1362
2012-01-11 09:00:18 -08:00
epriestley
d75007cf42 Validate logins, and simplify email password resets
Summary:
  - There are some recent reports of login issues, see T755 and T754. I'm not
really sure what's going on, but this is an attempt at getting some more
information.
  - When we login a user by setting 'phusr' and 'phsid', send them to
/login/validate/ to validate that the cookies actually got set.
  - Do email password resets in two steps: first, log the user in. Redirect them
through validate, then give them the option to reset their password.
  - Don't CSRF logged-out users. It technically sort of works most of the time
right now, but is silly. If we need logged-out CSRF we should generate it in
some more reliable way.

Test Plan:
  - Logged in with username/password.
  - Logged in with OAuth.
  - Logged in with email password reset.
  - Sent bad values to /login/validate/, got appropriate errors.
  - Reset password.
  - Verified next_uri still works.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan, j3kuntz

Maniphest Tasks: T754, T755

Differential Revision: https://secure.phabricator.com/D1353
2012-01-11 08:25:55 -08:00
Andrew Gallagher
840eb46d03 Match unittest results by name or file
Summary:
Just talked to @tuomaspelkonen, and turns out there is a case where
postponed tests results use the filepath for both the name and file
parameters.  Then, after the tests have completed, the unittest
results are updated with the class name as the test name.  To handle
this, this diff matches the stored unittest results name against
either the name or file component of the updated unittest info.

Not sure of great way to generally handle these situations.  Perhaps,
long term, we can just use a placeholder unittest result, mark that
as passed (or delete it?) then add a new test result with the correct
name.

Test Plan: updated unittest result with new name (but file was the same).

Reviewers: epriestley, tuomaspelkonen

Reviewed By: epriestley

CC: aran, epriestley, andrewjcg

Differential Revision: https://secure.phabricator.com/D1356
2012-01-10 16:51:14 -08:00
Andrew Gallagher
48f53ba095 Allow updating diff with results for new unit tests
Summary:
When using postponed unittests to make 'arc diff' faster, there
are some situations where it is difficult to know exactly how
many unittests will be run.  This is the case for many of our
C++ unittests, which we can't really know until we compile the
tests (which is slow, and probably isn't reasonable to be done
before posting the diff).  I suppose we could make sure we
explicitly which tests a C++ unittest will run in some way, but
this would require a lot of change to our backend test infra.
Also, it seems that this is a pretty general issue of not knowing
how many unittests will be run until they actually run.

This diff adds an optional "create" parameter to updateunitresults
which wil create a new unit tests result rather than updating an
existing one.  I am not sure if this really fits here or should
be its own method, but there is a lot of code re-use between them
so I consolidated.

Test Plan: updated a diff with a new unit test result

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley, andrewjcg, tuomaspelkonen

Differential Revision: https://secure.phabricator.com/D1352
2012-01-10 16:18:50 -08:00
Bob Trahan
84ea5c53e4 Kill PhabricatorFileURI
Summary:
we used to need this function for security purposes, but no longer need
it.   remove it so that some call sites can be optimized via smarter data
fetching, and so the whole codebase can have one less thing in it.

Test Plan:
verified the images displayed properly for each of the following
- viewed a diff with added images.
- viewed a user feed
- viewed a user profile
- viewed all image macros
- viewed a paste and clicked through "raw link"
weakness in testing around proxy files and transformed files.  not sure what
these are.  changes here are very programmatic however.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Maniphest Tasks: T672

Differential Revision: https://secure.phabricator.com/D1354
2012-01-10 15:21:39 -08:00
Bob Trahan
2a29a51080 Deploy new ArcanistManyWordsAboutDifferentialConstants class from D1328 into
Phabricator

Summary: ...this breaks without D1328.   Used good ole "codemod" to do this
work, with lots of manual edits around 80 chars.

Test Plan: clicked around phabricator tool suite, particular differential, a
bunch

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1351
2012-01-10 11:49:20 -08:00
epriestley
a4ceba9101 Add more information to differential.getdiff
Summary: We need some additional fields to heuristically match revisions to the
working copy in arc.

Test Plan: Executed conduit method, got correct values in fields

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: https://secure.phabricator.com/D1347
2012-01-10 10:40:57 -08:00
epriestley
5f8711ebf8 Minor, fix CSRF error caused by D1329. 2012-01-09 13:47:33 -08:00
Jason Ge
79d6d252b7 Display base correctly for git-svn in update history
Summary:
for a git repo which is using git-svn, we should show the
revisioni number. For example, it is show "svn+ssh" if the git url
starts with "svn+ssh". Show the svn revision number instead.

Before fix:
https://secure.phabricator.com/file/view/PHID-FILE-j5rogyqjkgifoxtpwzcu/

After fix:
https://secure.phabricator.com/file/view/PHID-FILE-4vsnptcjzuzuyj4zo25x/

Test Plan:
- verified a diff with git-svn worked
- verified a pure git diff still worked

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1344
2012-01-08 12:18:22 -08:00
epriestley
79205481e6 Minor, fix \n in MetaMTA headers. 2012-01-08 10:26:16 -08:00
Jason Ge
3a142c8788 Fix undefined index header.generate-toc in Differential
Summary:
getMarkupEngineDefaultConfiguration() is not setting
header.generate-toc. This is causing "undefined index" in Differential
page.

Test Plan:
  - Differential reviion page renders correctly
  - Phriction page renders correctly

Reviewers: epriestley, tuomaspelkonen, nh

Reviewed By: nh

CC: aran, nh

Differential Revision: https://secure.phabricator.com/D1342
2012-01-06 23:52:39 -08:00
Nick Harper
13d930b232 Fix table layout bug for inline comments
Summary:
The filename header for inline comments used to span 2 columns - the line number
and the comment. With the addition of a column for the diff (to link to inline
comments on previous diffs), the filename header should now span 3 columns
instead of just the line number and diff, leaving the comment squished to the
right.

Test Plan:
Opened a differential revision with an inline comment from a previous diff, and
saw that the filename header continued across the comment. Also checked an
inline comment on a current diff, and saw that it looks fine.

Reviewers: epriestley, btrahan, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1340
2012-01-06 14:55:18 -08:00
epriestley
5e486db59d Enable Table of Contents in Phriction
Summary: Use the TOC stuff introduced in D1334.

Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-lttr6jszx2y2rqjpalr6/

Reviewers: btrahan, jungejason, davidreuss

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1335
2012-01-06 11:52:50 -08:00
epriestley
4579f23f63 Add a "maniphest.update" Conduit method
Summary:
  - Add maniphest.update
  - Add support for auxiliary fields to maniphest.createtask

Test Plan:
  - Created tasks with maniphest.createtask
  - Updated tasks with maniphest.update

Reviewers: btrahan, jungejason, zeeg

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1330
2012-01-06 11:52:00 -08:00
epriestley
3c9551e96d Add "Reveal Entire File" option to Diffusion
Summary: Clicks all the "Show All" links for you at the touch of a button.

Test Plan:
  - Used "reveal entire file" on revealable files.
  - Opened on already-visible files, got "entire file shown".
  - Used other menu options.
  - Used normal "show more" links.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T497

Differential Revision: https://secure.phabricator.com/D1331
2012-01-06 11:51:10 -08:00
epriestley
e5c0bfc8e3 Link to undisplayed inline comments
Summary:
We currently don't link to comments which aren't visible. Link to the
appropriate diff in a new window, indicating where the comment lives.

Test Plan: Clicked visible, not-so-visible comments.

Reviewers: btrahan, jungejason, davidreuss

Reviewed By: btrahan

CC: aran, btrahan, epriestley

Maniphest Tasks: T555, T449

Differential Revision: https://secure.phabricator.com/D1333
2012-01-06 11:50:59 -08:00
epriestley
58f2cb2509 Provide a script for batch creating user accounts
Summary: Make it a little easier to create a bunch of accounts if your company
has more than like 5 employees.

Test Plan: Ran "add_user.php" to create new users. Created new users from the
web console.

Reviewers: btrahan, jungejason, rguerin

Reviewed By: btrahan

CC: aran, btrahan, rguerin

Differential Revision: https://secure.phabricator.com/D1336
2012-01-06 11:50:51 -08:00
epriestley
d16454d45d Improve a race condition in session establishment code
Summary:
If you try to establish several sessions quickly (e.g., by running several
copies of "arc" at once, as in "arc x | arc y"), the current logic has a high
chance of making them all pick the same conduit session to refresh (since it's
the oldest one when each process selects the current sessions). This means they
all issue updates against "conduit-3" (or whatever) and one ends up with a bogus
session.

Instead, do an update against the table with the session key we read, so only
one process wins the race. If we don't win the race, try again until we do or
have tried every session slot.

Test Plan:
  - Wiped conduit sessions, ran arc commands to verify the fresh session case.
  - Ran a bunch of arc piped to itself, e.g. "arc list | arc list | arc list |
...". It succeeds up to the session limit, and above that gets failures as
expected.
  - Manually checked the session table to make sure things seemed reasonable
there.
  - Generally ran a bunch of arc commands.
  - Logged out and logged in on the web interface.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T687

Differential Revision: https://secure.phabricator.com/D1329
2012-01-06 11:33:03 -08:00
epriestley
1903bb80bb Add a link from Differential to Diffusion
Summary:
Provide an easy way to jump to Diffusion from Differential if we have
the data we need to connect them.

Test Plan: Tested menu in linked and unlinked diffs. Used menu item.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T309

Differential Revision: https://secure.phabricator.com/D1326
2012-01-05 18:03:08 -08:00
epriestley
da3a972400 Retry failed page anchor navigation
Summary:
When the user loads a page with an anchor on it like #thing, or clicks a link to
#thing, and #thing doesn't exist, keep trying to navigate to #thing for a few
seconds.

This allows anchors to work when the target is in content which is later ajaxed
in. In particular, this affects inline comments in Differential.

Test Plan: Opened inline comment links in a new tab, was in the right place when
I switched tabs.

Reviewers: nh, btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan, epriestley

Maniphest Tasks: T492

Differential Revision: https://secure.phabricator.com/D1327
2012-01-05 18:02:02 -08:00
epriestley
9c5a6601d6 Remove some Mercurial and Google caveats
Summary: These seem to work relatively reasonably and don't have any known
deal-breaking failures.

Test Plan: shrug~

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1324
2012-01-05 14:14:06 -08:00
Bob Trahan
e478c0074c Add support for querying by commit hashes to DifferentialRevisionQuery class and
corresponding ConduitAPI

Summary: reasonable title...  also made this new functionality used by the
repository worker for parsing diffs

Test Plan:
- looked at the conduit console and queried for various types of hashes,
including hashes with no match.  got correct results.
- identified a reasonable diff from a local git repo.  set the revision status
to 2 (ACCEPTED) in the database.  augmented the worker parser code to var_dump
and die after finding revision id.   ran scripts/repository/reparse.php
--message rX and verified my var_dumps.  removed var_dumps and die and ran
reparse.php again with same paramters.  verified revision looked good in
diffusion and there were no errors.
- repeated the above reparse.php jonx for a mercurial repo.  note svn isn't in
this hash game so that test was particularly exciting no-op'dness i did not
bother with

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: https://secure.phabricator.com/D1315
2012-01-05 13:51:51 -08:00
epriestley
7abdf3afe0 Add a dropdown menu for Differential view options
Summary:
There are several open Differential tasks that are basically blocked on not
having reasonable places in the UI to put things. Replace the "View Standalone /
Raw" button with a "View Options" dropdown menu so we can shove things like
"Expand All", "Fold / Unfold File", and "View in Diffusion" in there.

This doesn't change any behavior, just puts the existing options in a menu.

Test Plan:
  - Toggled menu open by clicking button.
  - Clicked menu items.
  - Toggled menu closed by clicking button.
  - Toggled menu closed by clicking document.
  - Toggled menu closed by opening another menu.
  - Toggled menu closed by selecting an item.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T497, T309

Differential Revision: https://secure.phabricator.com/D1316
2012-01-05 12:58:38 -08:00
epriestley
275472d70f Fix appearance of "Differential Revision" on edit interfaces
Summary: The recent change to the field causes us to render "http://junk.com/D"
in some cases, just null the field if there's no data.

Test Plan: Ran "arc diff --create".

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1321
2012-01-05 12:57:40 -08:00
epriestley
1cb81936e7 Minor, fix array_merge() issue. 2012-01-05 09:11:49 -08:00
vrana
460efc4489 Include added reviewers and ccs in preview
Summary: Preview of Add Reviewers looks silly without actually showing them

Test Plan:
Go to any diff
Leap into action: Add Reviewers
Add some reviewers
Write some comment
Preview including Added reviewers should be displayed
Change action to Comment
Added reviewers should disappear
Repeat with Add CCs

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, vrana

Differential Revision: https://secure.phabricator.com/D1276
2012-01-04 17:08:13 -08:00
epriestley
4077ef2555 Show lint issues inline in Differential
Summary:
Take lint information and use it to render synthetic inline comments in
Differential.

Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-2uapwmf5tqhgscbuty3b/

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan, epriestley, pad

Maniphest Tasks: T24

Differential Revision: https://secure.phabricator.com/D1309
2012-01-04 14:17:33 -08:00
epriestley
28b606394b Fix undefined variable warning in unit test field
Summary:
See D1295. $unit_messages may be undefined.

I'll see if I can improve the visibility of warnings, the red dot in DarkConsole
is easy to miss right now. See T734.

Test Plan: Loaded a revision with no unit failures, didn't receive a warning.

Reviewers: nh, btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1306
2012-01-04 10:20:53 -08:00
epriestley
99231ebba4 Allow Git repositories to track only some branches
Summary:
Some installs use Git as the backbone of a CI framework or use a Git remote to
share patches. The tracker scripts currently recognize associated revisions as
"Committed" when they appear in any branch, even if that branch is
"alincoln-personal-development_test_hack" or whatever.

To address the broadest need here, allow Git repositories to be configured to
track only certain branches instead of all branches.

This doesn't allow you to import a branch into Diffusion but ignore it in
Differential. Supporting that is somewhat technically complicated because the
parser currently goes like this:

  - Look at HEAD of all branches.
  - For any commits we haven't seen before, follow them back to something we
have seen (or the root).
  - "Discover" everything new.

Since this doesn't track <branch, commit> pairs, we currently don't have enough
information to tell when a commit appears in a branch for the first time, so we
don't have anywhere we can put a test for whether that branch is tracked and do
the Differential hook only if it is.

However, I think this cruder patch satisfies most of the need and is simple and
obvious in its implementation.

See also D1263.

Test Plan:
  - Updated a Git repository with various filters: "", "master, remote", "derp",
"  ,,, master   ,,,,,"
  - Edited SVN and Mercurial repositories to verify they didn't get caught in
the crossfire.
  - Ran daemon in debug mode on libphutil with filter "derp", got exception
about no tracked branches. Ran with filter "master", got tracking. Ran with no
filter, got tracking.
  - Looked at Diffusion with "derp" and "master", saw no branches and "master"
respectively.
  - Added unit tests to cover filtering logic.

Reviewers: btrahan, jungejason, nh, fratrik

Reviewed By: fratrik

CC: aran, fratrik, epriestley

Maniphest Tasks: T270

Differential Revision: https://secure.phabricator.com/D1290
2012-01-04 10:20:34 -08:00
epriestley
ec1df21bef Add getStrList() to AphrontRequest
Summary:
  - We have a few places where we do some kind of ad-hoc comma list tokenizing,
and I'm adding another one in D1290. Add a helper to the request object.
  - Add some unit tests.

Test Plan:
  - Ran unit tests.
  - Used PHID manager, Maniphest custom view, and Repository project editor.

Reviewers: btrahan, fratrik, jungejason

Reviewed By: btrahan

CC: aran, btrahan, epriestley

Differential Revision: https://secure.phabricator.com/D1302
2012-01-04 10:18:46 -08:00
Andrew Gallagher
cd0386bf8b Remarkup unittest result messages
Summary: This diffs adds support for marking up unittest result messages.

Test Plan: Verified that links in unittest results were markup'd.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley, zeeg

Differential Revision: https://secure.phabricator.com/D1298
2012-01-03 18:23:34 -08:00
Andrew Gallagher
48f72b57b2 Enable use of remarkup literal block
Summary:
D1293 adds support for a literal block in remarkup.  This diff enables
it in phabricator with a few basic rules (for line breaks, escaping HTML,
and linkifying URLs).

Test Plan: Tested in sandbox

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1297
2012-01-03 18:00:45 -08:00
vrana
7a9be605ae Avoid double escaping in render()
Test Plan: View any comment

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1296
2012-01-03 16:46:05 -08:00
Nick Harper
4a77906141 Don't show detailed unittest result box if all tests passed
Summary:
When all unit tests pass, a box appears between the unit test results and lint
status (for test failures to go in). This checks if there's anything to put
in that div/ul before putting it on the page.

Test Plan:
Loaded a revision with unit tests OK and saw no box. Loaded a revision with
failing unittests, and saw the same box from before.

Reviewers: tuomaspelkonen, epriestley

Reviewed By: epriestley

CC: jungejason, aran, nh, epriestley

Differential Revision: https://secure.phabricator.com/D1295
2012-01-03 14:58:22 -08:00
Nick Harper
369079dd45 Change default status to 'all' in DifferentialRevisionList
Summary: Makes it easier to discover the list of all revisions for a user.

Test Plan:
Opened up /differential/filter/revisions/, and saw that it defaulted
to status of all. Clicked between tabs, and it stayed on all. Selected
open, it only displayed open revisions, including as I switched between
tabs.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1278
2012-01-03 14:49:30 -08:00
Evan Priestley
71e1911dfc Lock down MetaMTA functionality to administrators
Summary:
We have a debug interface for sending various sorts of email, but normal users
don't really need to use it. In particular, they can:

  - Send arbitrary email to other users;
  - Discover other users' email addresses fairly easily (CC everyone);
  - Send arbitrary email to arbitrary addresses in conjunction with "Mailing
Lists"

In fact, normal users don't need to get to the MetaMTA web interface at all and
it has some somewhat-sensitive things beacuse it has a lot of detailed
information about mail. For instance, users can look at mail records to discover
things like password reset links and per-user object email addresses.

We should smooth out the UI here but I think I can do something about T21 fairly
soon and cover it then.

Test Plan:
Went to /mail/ with a non-admin, got 404'd. Went to /mail/ with an
admin, everything works, got a red admin header.

Reviewers: jungejason, btrahan

Reviewed By: btrahan

CC: aran, btrahan, jungejason

Maniphest Tasks: T718

Differential Revision: https://secure.phabricator.com/D1292
2011-12-30 14:37:13 -08:00
Bob Trahan
890f0ff7fa ...fix my fat finger period to a comma
Summary: see above

Test Plan: no errors on herald controller

Reviewers: epriestley
2011-12-29 08:55:54 -08:00
Bob Trahan
46baa3b7ae Herald - Kill Tabs
Summary: makes a nice side filter for most UI elements.  only place this getds a
little funky is on the test console; a second, inner filter list appears for the
"affected" filters.

Test Plan: viewed each side filter and verified ui.  for each filter, interacted
with the ui and made sure things looked right and there were no errors

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T631

Differential Revision: https://secure.phabricator.com/D1289
2011-12-29 08:51:21 -08:00
epriestley
efb0fa739f Make tracked git repositories use an implicit 'origin' remote
Summary:
See T624. I originally wrote this to require an explicit remote, but this
creates an ugly "origin:" in all the URIs and makes T270 more difficult.

Treat all branch names as implying 'origin/'.

Test Plan:
  - Pulled and imported a fresh copy of libphutil without issues.
  - Browsed various git repositories.
  - Browsed Javelin's various branches.
  - Ran upgrade script, got a bunch of clean 'origin/master' -> 'master'
conversions.
  - Tried to specify an explicit remote in a default branch name.
  - Unit tests.

Reviewers: nh, jungejason, btrahan

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T624

Differential Revision: https://secure.phabricator.com/D1269
2011-12-29 08:35:32 -08:00
epriestley
1a1da7d6c2 Integrate auxiliary fields into Maniphest transactions
Summary:
  - When changing auxiliary field values, use transactions.
  - Clean up some of the load/save logic for auxiliary fields so it's a little
more performant.

NOTE: The transaction display of auxiliary fields is incredibly hacky, I'll
follow up with a more nuanced approach but wanted to limit scope here.

Test Plan: Created and edited tasks with custom fields configured; created and
edited tasks without custom fields configured.

Reviewers: btrahan, jungejason, zeeg

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T418

Differential Revision: https://secure.phabricator.com/D1283
2011-12-27 16:01:49 -08:00
epriestley
abd8efc358 Further relax same origin checks
Summary: Allow paths to match even if they differ by trailing slashes and
".git".

Test Plan: Ran unit tests.

Reviewers: jungejason, btrahan

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T710

Differential Revision: https://secure.phabricator.com/D1286
2011-12-26 12:01:59 -08:00
epriestley
aba5b48202 Minor: cosmetic fix to always use the small 50x50 picture in the new profile layout. 2011-12-24 09:12:01 -08:00
epriestley
bdbe9df65e Remove support for GitHub post-receive notifications
Summary:
  - These never actually did anything.
  - I don't even really remember why I built them, maybe the Open Source team
was pushing for more GitHub integration or something? I really have no idea.
  - Anyway, repository tailers do everything these could do (and much more).

Test Plan:
  - Ran tailers off GitHub for many months without needing post-receive hooks.
  - Grepped for relevant strings, couldn't find any references.
  - Used "Repository" edit interface for a Git repository.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T706

Differential Revision: https://secure.phabricator.com/D1273
2011-12-24 09:00:08 -08:00
epriestley
f4f8ea0b34 Add support for a "bool" type to Maniphest's default field extensions
Summary:
  - On the edit view, this is represented as a checkbox.
  - On the detail view, it renders with a user-selectable string.

Test Plan: Added a bool field to my local install, checked and unchecked it.

Reviewers: zeeg, jungejason, btrahan

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: https://secure.phabricator.com/D1277
2011-12-24 08:57:13 -08:00
epriestley
e786c44b6f Relax the same-origin check in the Git commit discovery daemon
Summary:
Git accepts either "git@x:/path" or "ssh://git@x.com/path" URIs to mean the
exact same thing, which is causing some false positives and confusion,
particularly because we sometimes mutate URIs.

Since this is just a sanity check, we don't really care about the username,
domain or credentials -- matching the paths is good enough. We're just trying to
make it hard to shoot yourself in the foot by copy-pasting the same local path
into two repositories and forgetting to change one, like I did. :P

Relax the check to only verify the paths are the same.

Test Plan:
  - Ran unit tests, which should fully cover things.
  - Ran commit discovery daemon in debug mode on incorrectly and correctly
configured repositories.

Reviewers: ajtrichards, jungejason, btrahan

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T710

Differential Revision: https://secure.phabricator.com/D1279
2011-12-24 08:54:44 -08:00
epriestley
2e6ab9b9a6 Convert user profile to be more useful and use the new Project profile style
layout

Summary:
  - Use new less-horrible layout.
  - Organize information more completely and sensibly.

Test Plan: Looked at some profiles.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: https://secure.phabricator.com/D1281
2011-12-24 08:54:23 -08:00
epriestley
5065db5a2a Reduce spew of some daemons
Summary:
It used to be more useful for daemons to spew random debugging information, but
features like "phd debug" and some fixes to error reporting like D1101 provide
better ways to debug, test, develop and diagnose daemons.

  - Stop writing "." every time MetaMTA sends a message.
  - Stop spewing the entire IRC protocol from the IRC bot unless in debug mode.
  - Stop writing GC daemon log entries about collecting daemon logs (DURRR)
unless in debug mode.

Test Plan: Ran daemons in debug and non-debug modes, got expected level of
noisiness.

Reviewers: jungejason, nh, btrahan

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: https://secure.phabricator.com/D1268
2011-12-22 17:49:33 -08:00
epriestley
2cec36d858 Minor style/layout tweaks to Project profiles and feed
Summary: Just trying to improve the design/layout a little here.

Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-xglufh3wxy34evec4o4u/

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: https://secure.phabricator.com/D1265
2011-12-22 17:47:43 -08:00
vrana
cce212dac7 Link from Blame Revision
Summary: There can be Dxxx, rXXXxxx or even full URL in //Blame Revision// field
so just highlighting it as normal text would work probably best

Test Plan:
Go to https://secure.phabricator.com/D277
You should see a link from //Blame Revision// (if it would be displayed)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1274
2011-12-22 15:57:53 -08:00
vrana
a83a54004d Avoid double escaping in renderLink()
Summary: ##phutil_render_tag()## escapes attributes automatically

Test Plan: View any diff

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, vrana

Differential Revision: https://secure.phabricator.com/D1271
2011-12-22 14:49:36 -08:00
Jason Ge
682fb6f519 Fix issue when a path is '/' in a package
Summary:
when a path is '/' in defining a package, D1251 is generating
an extra '//'.

Test Plan: veryfied adding path '/', '/src' and '/src/' all worked.

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1266
2011-12-22 09:58:19 -08:00
epriestley
99a9c082f9 Fix a fatal on the commit author list
Summary: I think we only hit this because I mucked around with the database to
recover from the runaway parse of the Diviner repository (now prevented by
D1253), but be more robust against missing data in this interface.

Test Plan: After applying this patch, no longer received a fatal on the commit
history page for users linked to nonexistant/bogus commits.

Reviewers: jack, btrahan, jungejason, aran

Reviewed By: aran

CC: aran

Maniphest Tasks: T701

Differential Revision: https://secure.phabricator.com/D1264
2011-12-22 07:10:34 -08:00
epriestley
3b65864ee1 Fix two minor bugs with recent patches:
- Use the computed remote URI (which may have an explicit 'ssh://' under Git in some cases).
  - Use '$id' correctly rather than casting the URI to an int in the message parser.
2011-12-22 06:57:04 -08:00
epriestley
b258095124 Expose symbol information over Conduit
Summary: I want to add a command like "where is ArcanistUnitTestEngine" to
phabot. I also want to add a symbol typeahead to Diffusion and generally finish
up that feature since it's useful but only half-implemented. Consolidate the
query logic and expose the data over Conduit.

Test Plan: Used /symbol/ and Conduit to lookup symbols.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T315

Differential Revision: https://secure.phabricator.com/D1260
2011-12-22 06:44:55 -08:00
epriestley
f901befcf5 Bump Phabricator server version to 3
Summary: See D1257. Also make the error message more friendly, and remove a very
very old Facebook-specific error.

Test Plan:
  - Tried to diff with an older arc.
  - Tried to diff with a newer arc.
  - Diffed with the right arc.

Reviewers: btrahan, jungejason, aran

Reviewed By: aran

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1258
2011-12-22 06:44:48 -08:00
epriestley
9d8b5481ae Emit full URIs to identify Differential revisions
Summary:
  - Previously, used IDs like "33" to match a commit to a Differential revision.
This has a namespacing problem because we now have an arbitrarily large number
of Phabricator installs in the world, and they may want to track commits from
other installs.
  - In Differential, parse raw IDs or full URIs. Emit only full URIs.
  - In Repositories, parse only full URIs.
  - This might cause a few commits to not be picked up in rare circumstances.
Users can fix them with "arc mark-committed". This should be exceedingly rare
because of hash matching.
  - There are some caveats for reparsing older repositories, see comments
inline. I don't think there's much broad impact here.

Test Plan:
  - Created a new revision, got a full URI.
  - Updated revision, worked correctly.
  - Ran unit tests.
  - Monkeyed with "Differential Revision" field.
  - Reviewers: btrahan, jungejason

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, epriestley, jungejason

Maniphest Tasks: T54, T692

Differential Revision: 1250
2011-12-20 19:58:22 -08:00
epriestley
a810469929 Improve performance of differential.query
Summary:
  - Instead of running (1 + N * 4) queries, run 5 queries.
  - Add 'dateModified' field.

Test Plan:
  - Ran various queries against various revisions, data seemed unchanged.
  - Checked query log.

Reviewers: nh, jonathanhester, jungejason, btrahan

Reviewed By: jonathanhester

CC: aran, epriestley, jonathanhester, jungejason

Differential Revision: 1252
2011-12-20 19:52:18 -08:00
epriestley
92f9741779 Add a safety check to commit discovery daemon
Summary:
Although I couldn't repro the issue in T692, I did manage to point the "Diviner"
repository at the "Phabricator" working copy and screw some stuff up on
secure.phabricator.com.

Before discovering commits in a repository, ensure the 'origin' remote points at
the configured URI. This prevents issues where the working copy gets configured
to point at an existing (but incorrect) checkout.

Test Plan:
  - Ran gitcommitdiscovery daemon normally under "phd debug", saw it execute the
"remote show -n" command and then start working.
  - Intentionally botched the config, got an exception:

  (Exception) Working copy '/INSECURE/repos/phabricator' has origin URL
  'ssh://git@github.com/facebook/phabricator.git', but the configured URL
  'git://github.com/facebook/diviner.git' is expected. Refusing to proceed.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T692

Differential Revision: 1253
2011-12-20 19:52:08 -08:00
epriestley
bd3c2355e8 Match usernames in any case in commit message object lists (CC, Reviewers, etc.)
Summary:
Allow entry of "CC: alincoln" to match user "ALincoln".

Put both variations in the map and try the exact case version first since we'll
also match email addresses and mailables, and theoretically some mailable might
have the same name as a user, as we're effectively abandoning restriction of
which characters can appear in usernames.

Test Plan: Created a local revision with a reviewer in CrAzY CaPs.

Reviewers: jungejason, btrahan

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T697

Differential Revision: 1255
2011-12-20 19:48:56 -08:00
Nick Harper
7075222b4b Validate paths before saving them when editing an owners package
Summary:
Paths in owners packages when referring to a directory should always end with
a trailing slash. (Otherwise, some things break, like loading the owning
packages for a path.) With this change, PhabricatorOwnersPackage now requires
that the path provided for a package is valid, and if the path is for a
directory, it adds a trailing slash if one was not provided.

Test Plan:
Edited a path in a package and left off the trailing slash. Saw that the slash
was added. Tried again with the trailing slash, and checked that another slash
was not added. Did this with a path in both a git and svn repository.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1251
2011-12-20 17:31:16 -08:00
epriestley
43430e154d Rough cut of Project profile improvements
Summary:
  - Old page was useless and dumb.
  - New page looks a little less bad, functions a little less poorly.
  - Still lots of work to be done.

Test Plan:
  - Viewed a project.
  - Clicked all the links on the left nav.
  - Here is a screenshot:
https://secure.phabricator.com/file/view/PHID-FILE-4buzquotb3fo4dhlicrw/

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T681

Differential Revision: 1246
2011-12-20 17:19:55 -08:00
epriestley
540400a806 Make Maniphest use FilterNavView instead of NavView
Summary: Share more code; reduce the number of ad-hoc versions of this rendering
loop.

Test Plan: Clicked all the filters.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 1247
2011-12-20 17:14:40 -08:00
epriestley
21ba07d5bd Provide wiki pages for projects
Summary:
Provide tighter integration between Projects and Phriction. Partly, I have most
of a rewrite for the Projects homepage ready but it's not currently possible to
publish feed stories about a project so all the feeds are empty/boring. This
partly makes them more useful and partly just provides a tool integration point.

  - When you create a project, all the wiki pages in projects/<project_name>/*
are associated with it.
  - Publish updates to those pages as being related to the project so they'll
show up in project feeds.
  - Show a project link on those pages.

This is very "convention over configuration" but I think it's the right
approach. We could provide some sort of, like, "@project=derp" tag to let you
associated arbitrary pages to projects later, but just letting you move pages is
probably far better.

Test Plan:
  - Ran upgrade scripts against stupidly named projects ("der", "  der", "  der
", "der (2)", "  der (2) (2)", etc). Ended up with uniquely named projects.
  - Ran unit tests.
  - Created /projects/ wiki documents and made sure they displayed correctly.
  - Verified feed stories publish as project-related.
  - Edited projects, including perfomring a name-colliding edit.
  - Created projects, including performing a name-colliding create.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T681

Differential Revision: 1231
2011-12-20 14:03:12 -08:00
jungejason
c80d1480d5 Add Basic Auditing Functionalities
Summary:
add basic auditing functionalities. For the related commits for a
package, we detect the following conditions which might be suspicious to the
owners of the package:

* no revision specified
* revision not found
* author not match
* reviewedby not match
* owners not involved
* commit author not recognized

The owners of the package can change the status of the audit entries by
accepting it or specify concern.

The owner can turn on/off the auditing for a package.

Test Plan:
*  verified that non-owner cannot see the details of the audit and cannot modify
it
*  verified that all the audit reasons can be detected
*  tested dropdown filtering and package search
*  verified really normal change not detected
*  verified accept/concern a commit
*  tested enable/disable a package for auditing
*  verified one audit applies to all <commit, packages> to the packages the
auditor owns
*  verified that re-parsing a commit won't have effect if there exists a
 relationship for <commit, package> already

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, benmathews, btrahan, mpodobnik, prithvi, TomL, epriestley

Differential Revision: 1242
2011-12-20 13:36:53 -08:00
epriestley
125e5b16db Remove "PHID Manager" Tool list interface
Summary: This was well-intentioned but has not actually proven to be useful.

Test Plan:
  - No list tab shows up anymore.
  - Looked up a PHID.

Reviewers: btrahan, jungejason, Girish

Reviewed By: Girish

CC: aran, jungejason, edward, emiraga, Girish, nh, tuomaspelkonen, epriestley

Maniphest Tasks: T631

Differential Revision: 1234
2011-12-20 09:58:52 -08:00
epriestley
ee620bde6d Publish feed stories from Maniphest
Summary: I didn't get around to this earlier; add Feed/Maniphest integration.
This is partly motivated by wanting Projects to not be terrible. Pretty
straightforward.

Test Plan:
  - Created, updated, reassigned and closed a task.
  - Verified feed stories render reasonably.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T681

Differential Revision: 1232
2011-12-20 09:55:29 -08:00
epriestley
b5c9b9d059 Use remote credentials for 'git fetch' and 'hg pull' commands
Summary: These are "local" commands, but need remote credentials. If the daemon
runs as a user who does not have credentials, the initial clone will work but
subsequent updates will fail.

Test Plan:
  - Nuked a local copy of a Git repo.
  - Ran "phd debug fetch <phid>" as root (or any other user with no natural SSH
keys). Verified initial clone worked (since it passes credentials to the command
correctly).
  - Killed daemon, re-ran, verified "fetch" failed (no credentials passed).
  - Applied this patch.
  - Re-ran "phd debug fetch <phid>", verified it passed credentials and
succeeded.
  - Did all these steps for a Mercurial repo.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T686

Differential Revision: 1236
2011-12-20 08:28:38 -08:00
epriestley
0634009720 Share feed building code
Summary:
I pretty much copy/pasted this code; rather than do that again now that I want
to add feeds to projects, share the code.

This "Builder" is a little weird -- I don't want to call it a "View" because it
does data access. "Builder" seemed okay. We don't really have much code that
does this sort of thing right now, elsewhere.

Test Plan:
  - Viewed public feed.
  - Viewed private feed.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T681

Differential Revision: 1233
2011-12-20 08:28:31 -08:00
epriestley
e45ffda55a Move most remaining sha1() calls to HMAC
Summary:
  - For context, see T547. This is the last (maybe?) in a series of diffs that
moves us off raw sha1() calls in order to make it easier to audit the codebase
for correct use of hash functions.
  - This breaks CSRF tokens. Any open forms will generate an error when
submitted, so maybe upgrade off-peak.
  - We now generate HMAC mail keys but accept MAC or HMAC. In a few months, we
can remove the MAC version.
  - The only remaining callsite is Conduit. We can't use HMAC since Arcanist
would need to know the key. {T550} provides a better solution to this, anyway.

Test Plan:
  - Verified CSRF tokens generate properly.
  - Manually changed CSRF to an incorrect value and got an error.
  - Verified mail generates with a new mail hash.
  - Verified Phabricator accepts both old and new mail hashes.
  - Verified Phabricator rejects bad mail hashes.
  - Checked user log, things look OK.

Reviewers: btrahan, jungejason, benmathews

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T547

Differential Revision: 1237
2011-12-19 08:56:53 -08:00
David Reuss
5f1b3937e5 Added Outlook boundaries for email parser
Summary:
Outlook wraps a message in 5 dashes on each side when doing replies.
This strips english and danish versions.

Test Plan:
Tried parsing emails with different messages and saw the
expected behaviour with patch applied. Ran arc unit, and saw test
passed.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1239
2011-12-19 08:40:18 -08:00
David Reuss
673f0d6c81 Display changed attachments as other changes in Maniphest
Summary:
When changing attachments, the removed part is squished together
with the added parts, making it hard to read. This changes the output
so it looks like other changes, seperating each action by a semicolon.

Test Plan:
Viewed a task where i had attached and deleted revisions, and
saw the output look as other changes of same type.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1238
2011-12-19 08:37:29 -08:00
epriestley
afc2f8526b Allow Phriction documents to be deleted
Summary:
  - Add a "delete" operation. Delete is just a special edit which removes the
page from indexes and shows a notice that the document has been deleted.
  - When a user deletes all the content on a page, treat it as a delete.
  - When a conduit call deletes all the content on a page, treat it as a delete.
  - Add page status to Conduit.
  - Add change type field to history.
  - Added a couple of constants to support a future 'move' change, which would
move content from one document to another.

Test Plan:
  - Verified deleted pages vanish from the document index (and restoring them
puts them back).
  - Verified deleted pages show "This page has been deleted...".
  - Created, edited and deleted a document via Conduit.
  - Deleted pages via "delete" button.
  - Deleted pages via editing content to nothing.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: skrul, aran, btrahan, epriestley

Maniphest Tasks: T680

Differential Revision: 1230
2011-12-17 11:45:25 -08:00
epriestley
9d372dd54c Simplify project lists
Summary: Remove a bunch of relatively useless stuff from the Project list
interfaces.

Test Plan: Looked at project lists, less random busy junk.

Reviewers: btrahan, jungejason, zeeg

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T681

Differential Revision: 1229
2011-12-17 11:45:18 -08:00
jhester
1270b0b5bd Add properties to differential.getdiff
Summary: Add 'addLines' and 'delLines' properties to differential.getdiff return
dictionary.  These properties are aggregated from the changesets.

Test Plan: Issue a differential.getdiff query via conduit and verify that
'addLines' and 'removeLines' properties are included and accurate

Reviewers: epriestley

Reviewed By: epriestley

CC: epriestley, aran, jonathanhester

Differential Revision: 1209
2011-12-16 18:02:35 -08:00
epriestley
93d5d29541 Remove "Former" project members
Summary:
This is a needlessly confusing/complex feature that I originally wrote sort of
speculativley. I think we can better serve what little need may exist here with
project feeds.

I'm probably going to get rid of or deemphasize "role" too and just add "Join
Project" and "Leave Project" buttons.

Test Plan: Viewed project list, project profile. Edited project profile and
affiliation.

Reviewers: btrahan, jungejason, zeeg

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T681

Differential Revision: 1228
2011-12-16 17:46:02 -08:00
epriestley
81acf588e2 Take the first step on the long journey of fixing "Projects"
Summary:
  - Allow more than the 100 most recent projects to be viewed.
  - Provide some useful filters.
  - Default the view to your projects, not all projects.
  - Put query logic in a query object.
  - Put filter view logic in a view object. We can port more stuff to it later.

Test Plan: Looked at active/owned/all projects. Set page size to 5 and paged
through projects.

Reviewers: btrahan, jungejason, zeeg

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1227
2011-12-16 17:23:48 -08:00
Evan Priestley
770beb5cd5 Merge pull request #85 from dcramer/hide-empty-fields-in-maniphest
Hide auxiliary fields that have no value set
2011-12-16 16:51:58 -08:00
David Cramer
2a80bdd448 Hide auxiliary fields that have no value set
Reviewers: epriestly
2011-12-16 16:49:45 -08:00
epriestley
fbd7db6898 This reverts commit 0386ac9aa2. 2011-12-16 16:14:20 -08:00
epriestley
c97fcd12bc Share Revision/Task attaching code
Summary:
We have this code in two places; split it into an editor class so we can share
it.

This also fixes some probems with this field not //detaching// tasks properly.

Test Plan:
  - Created a revision with no attached tasks.
  - Attached it to a task.
  - Updated it.
  - Detached it.
  - Used web UI to attach/detach tasks/revisions.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan, epriestley

Differential Revision: 1225
2011-12-16 16:13:00 -08:00
epriestley
0386ac9aa2 projquery 2011-12-16 13:44:10 -08:00
epriestley
43fc7820e9 August comes AFTER July. 2011-12-16 13:34:36 -08:00
epriestley
dfc3506240 Prevent "Maniphest Tasks" field from creating empty comment when revisions are
updated

Summary:
  - If you update a revision with a nonempty "Maniphest Tasks" field, an empty
comment is posted (see T586).
  - The transaction email currently says "Attached revision 'Unknown
Differential Revision'", move attaching to "didWriteRevision()" to make sure the
object has been written.

Test Plan:   - Attached; updated a revision.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T685

Differential Revision: 1223
2011-12-16 13:32:52 -08:00
epriestley
3cd92ab075 Minor fix to D1186 to enforce ordering. 2011-12-16 13:32:16 -08:00
epriestley
4dd87f1ad3 Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
  - Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
  - Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
  - Allow views to be filtered and sorted more flexibly.
  - Allow anonymous users to use the per-user views, just don't default them
there.

NOTE: This might have performance implications! I need some help evaluating
them.

@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?

The "active revisions" view is built much differently now. Before, we issued two
queries:

  - SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
  - SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)

These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.

Now, we issue only one query:

  - SELECT (open revisions you authored or are reviewing)

Then we divide them into the two tables in PHP. That query is available in P246.

On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?

In particular:

  - Run the queries and make sure the new version doesn't take too long.
  - Run the queries with EXPLAIN and give me the output maybe?

Test Plan:
  - Looked at different filters.
  - Changed "View User" PHID.
  - Changed open/all.
  - Changed sort order.
  - Ran EXPLAIN / select against secure.phabricator.com corpus.

Reviewers: btrahan, nh, jungejason

Reviewed By: btrahan

CC: cpiro, aran, btrahan, epriestley, jungejason, nh

Maniphest Tasks: T586

Differential Revision: 1186
2011-12-16 13:21:54 -08:00
Bob Trahan
b595f8447b Fix comment from D1221 to have updated variable
Test Plan: re-ran D1221 test plan

Reviewers: epriestley
2011-12-15 14:37:44 -08:00
Bob Trahan
ecb0ab4847 Phriction - kill tabs
Summary:
...except that pesky help tab which remains.

Pertinent bits here...
- move "History" into button "View History" that is grey and next to "Edit Page"
- for history page, add breadcrumb similar to the one on "diff" page.  This
unifies the experiencing on history <=> diffs as well as gives the user a link
back to the document, which was a tab on the History page before this diff.

Thoughts for next time...
- I'd like to further unify the breadcrumbs between "View" and "History / Diff".
- The "Document Index" is pretty sweet and feels a bit buried.  I wonder if
unifying breadcrumbs is the key here?

Test Plan: clicked around phriction.   viewed a document, viewed its history.
verified links in breadcrumbs were correct

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T631

Differential Revision: 1221
2011-12-15 14:35:38 -08:00
Bob Trahan
128b7584da Files - kill tabs
Summary:
kill tabs for Files application.  Technique is the "filter list" on the left
hand side, with separation for "Files" versus "Image Macros".   UI quirks
include:

- the page title does not change for the 3 files filters while it does change
for each of the two image macro filters.
- standalone "file" pages do not have the filter view
- you can visit /file/upload/ standalone and it doesn't have the pretty filter
list on it

Please do give direction on these quirks if you like.  :)

This change also neuters the ?author= functionality for files.  The code is
written such that it can easily be brought back.

Test Plan: clicked around on the filters, liked what I saw.  uploaded files
fancy-like and basic-like and it worked!  made image macros and it worked!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Maniphest Tasks: T631

Differential Revision: 1219
2011-12-15 14:32:12 -08:00
Bob Trahan
0e7049e8aa Countdown - kill tabs
Summary: pretty simple stuff here.   "View" controller had a 'view' tab selected
which DNE.

Test Plan: viewed countdown, noted no tabs

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T631

Differential Revision: 1222
2011-12-15 14:31:25 -08:00
jungejason
c13b7da290 Add Related Commits for Owners
Summary:
For each commit, find the affected packages, and provide a way to
search by package.

Test Plan:
create commits that touch and don't touch two packages, and verify
that they display correctly in all the UI pages.

Reviewers: epriestley, blair, nh, tuomaspelkonen

Reviewed By: epriestley

CC: benmathews, aran, epriestley, btrahan, jungejason, mpodobnik, prithvi

Maniphest Tasks: T83

Differential Revision: 1208
2011-12-14 22:48:57 -08:00
David Cramer
2677217244 Revised unit display to resemble lint output
Reviewers: epriestley

Test Plan: View a differential with unit results and compare display.

Differential Revision: 1216
2011-12-14 16:11:55 -08:00
Bob Trahan
522b16ed12 Paste - fix N query problem for file URIs
Summary: grab all the files in one big fetch, rather than serially fetching
them.   follow up from D1198.

Test Plan: viewed paste and there were no errors!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, jungejason, btrahan, epriestley

Differential Revision: 1202
2011-12-14 12:30:10 -08:00
Marek Sapota
4ef18d35ac Fix error introduced with D1195.
Summary:
Not my best commit=/

Reviewers: epriestley

Test plan:
After the change commit parser started working again.

Differential Revision: 1206
2011-12-13 20:37:18 -08:00
epriestley
51b8168253 Fix fatal in commit message parser
Summary:
See D1195, which fataled this daemon.

https://secure.phabricator.com/daemon/log/2966/

Test Plan: Applied this patch to secure.phabricator.com, restarted daemon, it
picked up D1203.

Reviewers: btrahan, jungejason, mareksapota

Reviewed By: btrahan

CC: aran, btrahan, mareksapota

Differential Revision: 1204
2011-12-13 18:56:26 -08:00
epriestley
2efd8fe971 Fix a bad %d for PHID
Summary: D1174 caught this issue -- we mean to load all //your// rules, but
actually load //all// rules. Use %s correctly.

Test Plan: Hit /herald/rule/ without an exception.

Reviewers: fmoo, btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Differential Revision: 1203
2011-12-13 18:30:53 -08:00
Bob Trahan
6f1dfbb658 Paste - kill tabs
Summary:
merge paste create and paste list into a single controller.  Add a "filter list"
to the left hand side and have new "create w/ recent", "my" and "all" views.  UI
wrinkle -- "create w/ recent" does not paginate the recent pastes and instead
upsells the user to the new "all" view.

Also includes a business logic clean up or two for simplicity of code.

Test Plan:
- created a paste from the UI
- tried to create a paste with title and no body
- tried to create a paste with no title and no body
- viewed the paste list on "create" view
- viewed the paste list on "author" view
- viewed the paste list on "all" view
- viewed page 2 of the paste list for "author" and "all" views
- "forked" a given paste through completion

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, btrahan

Maniphest Tasks: T631

Differential Revision: 1198
2011-12-13 17:53:41 -08:00
Marek Sapota
2d232674df Allow customized patterns for marking generated files.
Test Plan:
Created a listener that adds some patterns to $matches array, reloaded
Differential, some changesets were not shown as generated.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, mareksapota

Differential Revision: 1200
2011-12-13 17:14:25 -08:00
jhester
1fec5fd727 Add author to differential.getrevisionfeedback
Summary: Add the author PHID to the differential.getrevisionfeedback conduit api
method

Test Plan: issue differential.getrevisionfeedback query via conduit against a
valid revision and verify author phid is included in results

Reviewers: epriestley

Reviewed By: epriestley

CC: epriestley, aran, jungejason, tuomaspelkonen, jonathanhester

Differential Revision: 1190
2011-12-13 16:35:57 -08:00
Marek Sapota
9292cfd6a3 Recognise better who committed a revision.
Test Plan: Commit as not the author and see what shows up.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, mareksapota, zeeg

Differential Revision: 1195
2011-12-13 14:00:13 -08:00
Bob Trahan
4fc37c3dde Add diff view for Maniphest Task "description changed" transactions
Summary:
use the handy DifferentialChangesetParser to do most of the heavy lifting inside
the pertinent view object.   update the controller to be aware of the "show
more" calls coming from the new ui and update the transactionID appropriately.

also snuck in a small change to AprontRequest to all getting all the request
data.  I used it to debug building this.

Test Plan: made a task and entered a bunch of test data.  had descriptions of
various lengths, as well as really long descriptions that i did not change to
much.   verified the diff looked correct and various "show more" links worked as
expected

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1187
2011-12-08 18:15:19 -08:00
epriestley
682e0aa468 Add "responsible users" and "subscribers" to DifferentialRevisionQuery, plus a
couple bug fixes

Summary:
  - Add the ability to query for "responsible users" (author or reviewer).
  - Add the ability to query for "subscribers" (reviewer or CC).
  - Fix an issue where CC and Reviewer used the same join table alias and were
incompatible.
  - Remove support for 'paths' for the moment, since each path needs a
repository ID. (There are no clients for this.)
  - Remove single withX() methods that have no callsites -- withPath() is
singular because it accepts two arguments and I didn't want to have an ad-hoc
type format, but I think we can get away without these for other conditions.
  - Include GROUP BY in more cases where may need it. This doesn't actually
change program behavior since we uniquify in loadFromArray(), it just means less
data over the wire.

These new query classes are to support rewriting the Differential list view on
top of DifferentialRevisionQuery.

Test Plan:
  - Issued queries via conduit for "responsible users".
  - Issued queries via conduit for "subscribers".
  - Issued queries via conduit for "cc" with "reviewer" at the same time.
  - Issued queries via conduit for "cc", "reviewer", "responsible users" and
"subscribers" at the same time.
  - Issued a "subscribers" and "reviewers" query which returned duplicates;
verified GROUP BY took effect.

Reviewers: nh, btrahan, jungejason

Reviewed By: nh

CC: aran, nh

Differential Revision: 1182
2011-12-08 09:38:52 -08:00
epriestley
bd12a2b839 Fix a final (?) task field issue which slipped through the cracks
Summary:
Derped this one up; while my testing was successful in preventing runaway
attaching I missed the bit where it doesn't actually work.

This resolves the "Unknown Object" link seen on T661.

Test Plan:
  - Created two new revisions, each attached to a local task.
  - Verified that they attached additively, Maniphest and Differential were
linked to the right places, and nothign else bad happened.

Reviewers: btrahan, fratrik

Reviewed By: fratrik

CC: aran, fratrik, btrahan

Differential Revision: 1181
2011-12-07 08:53:19 -08:00
epriestley
d13906ff96 Add "tabindex" to Remarkup reference lists
Summary:
Prevent keyboard focus of these links so we don't disrupt tab order from
comments to "Submit".

Arguably I should make a "function" for this or something but there's nowhere to
really put it that makes any sense right now.

Test Plan: Verified Firefox skips these links in tab order.

Reviewers: fratrik, btrahan, jungejason

Reviewed By: fratrik

CC: aran, fratrik

Maniphest Tasks: T661

Differential Revision: 1180
2011-12-07 08:53:12 -08:00
epriestley
4fd81150be Remove "Updated" view from Differential
Summary:
This landed during my review drama embargo and is a generally good idea but had
some implementation issues.

@elynde reports it has been broken for some time, although it still works on
secure.phabricator.com so I'm guessing it's just taking a zillion years to run
at Facebook. It's up to more than a second for me on secure.phabricator.com:

https://secure.phabricator.com/file/view/PHID-FILE-v4ql4c66u3xnkarmrpm4/

The basic problem is that some of the data architecture around this
implementation is hard to scale. I want to pursue a similar feature eventually,
but drive it off notifications that we'll ship through real-time infrastructure
too.

I'm also trying to get rid of DifferentialRevisionListData and this simplifies
that somewhat.

Test Plan:
  - Grepped for table name, table constant, query constant, and class name; no
hits.
  - Applied SQL patch.
  - Verified that Differential no longer shows "Updated".

Reviewers: elynde, btrahan, jungejason

Reviewed By: elynde

CC: aran, elynde

Differential Revision: 1178
2011-12-07 06:55:03 -08:00
Nick Harper
74f710a437 Add sanity to DifferentialRevisionQuery
Summary:
Changed cc/reviewer search to be a union/or instead of intersection/and within
each list. Also added support to search for multiple authors (same behavior as
cc/reviewer), and updated conduit call to match. (See discussion on D1158.)

Test Plan:
Used the conduit call to search for revisions with one of 2 people on the cc
list, and checked the results to see that it wasn't constraining to requiring
both be on the cc list.

Reviewers: epriestley, jungejason, btrahan

Reviewed By: epriestley

CC: aran, nh, epriestley

Differential Revision: 1179
2011-12-06 14:47:12 -08:00
epriestley
2797903776 Fix bad query edge condition when updating a revision with no attached tasks. 2011-12-04 13:25:32 -08:00
Bob Trahan
588b959c03 phabricator_format_timestamp => phabricator_datetime
Summary: make the change, kill the function.   be sure to get a good $user or
$viewer variable

Test Plan:
for each controller or view, look at it in the ui.   change timezone, refresh ui
and note change.   i did not test the OAuthSettingsPanelController; not sure how
to get to that badboy and i got a bit lazy

Maniphest Tasks: T222

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Maniphest Tasks: T222

Differential Revision: 1166
2011-12-04 08:51:34 -08:00
Jakub Vrana
9c1697383c Add a link to Remarkup Reference below comment
Summary:
To reduce blindness, all textareas with some kind of special syntax should have
an information about this syntax and a link to its documentation. Preview
function is a nice complement but it doesn't replace this information.

I've added this information and the link below the comment field.

Please note that <a target> is a valid attribute in HTML5.

Test Plan:
Go to https://secure.phabricator.com/D1164#comment-content
There should be a link to Remarkup Reference
This link should open Remarkup Reference in a new window (to not discard the
comment)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, vrana

Differential Revision: 1164
2011-12-03 12:28:21 -08:00
Bob Trahan
83efa6e1c5 make arc diff link maniphest tasks with revisions
Summary:
add "Maniphest Task:" or "Maniphest Tasks:" followed by text that has TX in it.
foreach TX the task will be attached to the revision and the revision will be
attached to the task.  parsing is pretty... ummm, robust such that it will pick
up any TX substring and parse that as a Maniphest Task just fine.   it errors
out if there is not an actual task for TX and otherwise churns along pretty
nicely.

Also, make sure the PhabricatorObjectHandle loads the task ID as the alternateID
since we need that here and it should be that way anyhoo.

Test Plan:
made a diff and in the commit message added Maniphest Task(s): TX combination.
Tried various combinations of TX -- single, multiple with commas, multiple many
lines, single bad, multiple bad, multiple mix of bad and good. verified that the
good tasks were attached to the diff and diff was attached to the good tasks.

Maniphest Tasks: T137

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1165
2011-12-03 11:34:55 -08:00
epriestley
10cc5f2660 Set user on auxiliary fields before validating them on template workflow
Summary: Some fields need this data in some circumstances in order to validate
-- see D1153.

Test Plan: Ran "arc diff" against local, no longer got an exception for access
of this field from the 'Reviewers' validator.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1160
2011-12-02 15:14:39 -08:00
Nick Harper
8f5e28bf59 Add differential.query conduit method
Summary:
Created a differential.query conduit method that is built on top of
DifferentialRevisionQuery. I also added support for querying by author, ccs, and
reviewers to DifferentialRevisionQuery, so feature parity can be brought up to
match differential.find and its backing class DifferentialRevisionListData.

Test Plan:
Tried a few calls to the conduit call using the web interface, and got back
reasonable looking data.

Reviewers: epriestley, jungejason, btrahan

Reviewed By: epriestley

CC: aran, nh, epriestley

Differential Revision: 1158
2011-12-02 13:06:43 -08:00
Bob Trahan
519a443eba kill differential tabs in favor of a create diff button
Summary:
kill the tabs and make it a create button instead.  pertinent notes:
* added a "Filter diffs" button to the form.  optional, but i thought it
necessary with the new green button
* linked to Arcanist user guide on the create diff page.  somewhat unrelated but
i think create diff will get more traffic now so linking to help seemed like a
reasonable add on here.

Test Plan:
viewed differential homepage
* clicked left hand filter elements.  noted "Create Diff" button on filters
within user revisions and no button on filters within all revisions.
* entered another user into Select User UI and viewed their diffs via button and
pressing enter

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1157
2011-12-02 10:39:36 -08:00
David Reuss
c2054bab09 Support limiting maniphest queries to specific ids
Summary:
This limits a maniphest task query to only contain certain ids set
by the tasks query parameter.

Test Plan:
none yet, i wrote this at a computer with no phabricator
install while bored and eating dinner.

Reviewers: skrul, epriestley

Reviewed By: epriestley

CC: aran, davidreuss, epriestley, skrul

Differential Revision: 1137
2011-12-02 07:30:20 -08:00
epriestley
19f2110e74 Allow "differential.getcommitmessage" to be called without a revision ID in
order to generate a template

Summary: See T614. This allows us to generate an empty template by calling
Conduit, so we can build command-line editing workflows for SVN, Mercurial, and
conservative-Git.

Test Plan: Used web console to invoke Conduit method; got a reasonable empty
template out of it.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Differential Revision: 1156
2011-12-02 07:28:55 -08:00
epriestley
462ad4169c Remove obsolete 'error' field from differential.parsecommitmessage
Summary: As of D1154, we don't need this anymore. See that change for context.

Test Plan: See D1154.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1155
2011-12-02 07:28:49 -08:00
epriestley
40221feed9 Validate commit message fields on the server side
Summary:
See T643. We have some hard-coded checks in Arcanist for the existence of
'testPlan' and 'title', and don't properly validate those fields on the server.
Add a validation pass in the Conduit-based edit pathway.

In particular, this means that if you disable the "Test Plan" field, Arcanist
won't block you anymore.

Test Plan: Disabled Arcanist checks and ran "arc diff"; got blocked on the
server side.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1153
2011-12-02 07:28:43 -08:00
epriestley
bd520076f9 Add optional keystroke support for AphrontPagerView
Summary: This is sort of silly but maybe useful? The real problem is that there
are like 500k conduit call logs and the real solution to that is better
filtering options, but this seems sort of okay.

Test Plan: Used "[" and "]" to switch between pages on the conduit call log.

Reviewers: btrahan, jungejason, nh, aran

Reviewed By: nh

CC: aran, nh, epriestley

Differential Revision: 1145
2011-12-01 10:16:12 -08:00
epriestley
77a5a3ab00 Add a basic Conduit log view
Summary:
The conduit access to Differential kind of sucks and we want to break
back-compat in order to fix it (see D1114).

To make it easier to pull this off, I want to build out the Conduit logging a
bit so administrators can identify which users are making deprecated calls.

We should probably build a little more infrastructure around this too (API
versions?), but this is at least a reasonable step forward which gives us more
insight into the use of Conduit and more tools to smooth the deprecation
process.

This initial commit is super basic but the interface currently says "stuff",
I'll build this out a little more in a bit.

Test Plan: Looked at call logs.

Reviewers: btrahan, nh, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1144
2011-12-01 10:15:51 -08:00
epriestley
c53511e9b4 Minor remarkup updates
Summary:
  - Update documentation for changes in D1148.
  - Link to Remarkup documentation from Maniphest.
  - Support "Note:" syntax in Phabricator (previously, it was only supported in
Diviner, but I've found it pretty good and useful).

Test Plan: Regenerated and perused documentation; made a "NOTE:".

Reviewers: btrahan, broofa, fugalh, jungejason, nh, aran

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1149
2011-12-01 10:15:38 -08:00
Bob Trahan
9f201ef897 change '' => null from D1139
Summary: feedback from D1139 i missed before i git pushed.  :/

Test Plan: re-did test plan in d1139

Reviewers: epriestley

CC:
2011-11-30 11:17:14 -08:00
Bob Trahan
d0d33094d6 add projectName to conduit.getdiff
Summary: some ground work for T479

Test Plan:
called up a diff via the conduit api console
it had the right project name and did not error

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1139
2011-11-30 11:08:15 -08:00
Bob Trahan
ec8dbfd05f Dedupe DIRECTORY w/ Directory tab in directory header
Summary: the tab is a bit silly right next to DIRECTORY

Test Plan:
viewed phabricator with an admin account
* looks good on load
* clicked Categories and Items; looked good
viewed phabricator with a non-admin account
* looks good on load
* nothing else to click in the header

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1131
2011-11-28 13:03:46 -08:00
epriestley
e0a56cb938 Clean up two more sha1 instances
Summary: See T547. One of these I just missed in D1000; the comment change just
makes it easier to audit use of hash functions by cleaning up "grep" output.

Test Plan: Ran isolation unit test.

Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 1124
2011-11-20 14:21:26 -08:00
Brian Pane
5dba8abceb Add a "createcomment" method to Differential
Summary:
Added a new method differential.createcomment

Task ID: #752014

Test Plan:
I created a test diff and called this method via the conduit
from a client PHP script to add comments.  I confirmed that
1) the comment appeared on the revision, 2) URLs within the
comment were turned into hyperlinks, and 3) Phabricator
sent a notification email to the people watching the test
diff.

Reviewers: nh, jungejason, epriestley

Reviewed By: nh

CC: aran, nh

Differential Revision: 1128
2011-11-18 14:53:01 -08:00
Marek Sapota
e9693f25f8 Move event framework from Phabricator to libphutil
Summary:
Move event framework from Phabricator to libphutil so it can be used in other
phutil projects, such as Arcanist.

Test plan:
Use along with path to libphutil, events should work as expected.

Reviewers: epriestley

Differential Revision: 1098
2011-11-16 16:34:45 -08:00
epriestley
98c8e150b0 Prevent delivery of email to disabled objects
Summary: See T625. Facebook's REST-based MTA layer had a check for this so I
overlooked it in porting it out. We should not attempt to deliver email to
disabled users.

Test Plan:
Used MetaMTA console to send email to:

  - No users: received "no To" exception.
  - A disabled user: received "all To disabled" exception.
  - A valid user: received email.
  - A valid user and a disabled user: received email to valid user only.

(Note that you can't easily send to disabled users directly since they don't
appear in the typeahead, but you can prefill it and then disable the user by
hitting "Send".)

Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: skrul, aran, epriestley

Differential Revision: 1120
2011-11-16 11:07:50 -08:00
epriestley
ef020f711e Ensure Maniphest CC PHID list is always a list in maniphest.info
Summary: See T626. Use array_values() to discard keys, for consistency and so
this will always encode as a list (JSON array) over the wire.

Test Plan: Added and removed CCs from a task while calling maniphest.info on it;
CCs worked and I always received a list.

Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: skrul, aran, btrahan, epriestley

Differential Revision: 1118
2011-11-16 11:07:42 -08:00
Jason Ge
42383214ea Enable admin to view and delete other users' herald rules
Summary:
enable admin to delete user's herald rules. This is useful for
managing non-active users' rules. For example, ex-employees' rules. The
code change includes:

 - Added a 'All' tab which is only accessible to admin.
 - Refactor out a HeraldRuleListView which is used by both the home
   controller and the all rule controller

Test Plan:
delete an ex-employee rule as an admin; disable myself as
admin and verified that I don't have access to view other user's rules
and I'am not be able to delete them; also verified that as a non-admin,
I can still view, create and delete my own rules.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, epriestley, jungejason

Differential Revision: 1064
2011-11-15 16:21:51 -08:00
John Stockdale
501c90bb30 Use Git's encoding flag instead of MBString
Summary:
0d5b0f21ad added string conversion but MBString always needs an argument for endcoding.

It looks like we can get away with doing this in git instead, with the --encoding='UTF-8' flag. Then we should be safe to remove the test for output type, and stay UTF-8 safe.

Test Plan:
Run updaters with change. Verify commits are updated.

Reviewers: epriestley

CC:

Differential Revision: 1108
2011-11-12 01:01:41 +00:00
Bob Trahan
5c21b5345d execx ==> execxLocalCommand for git libraries in diffusion
Summary:
this was fairly mechanical at the end of the day

note that future/exec got removed by the code generation robots
post this change

Test Plan:
clicked around diffusion a bunch looking for errors.

For a given repo (say http://phabricator.dev/diffusion/BOBALIE/)
 - http://phabricator.dev/diffusion/BOBALIE/history/
 - http://phabricator.dev/diffusion/BOBALIE/history/origin:master/.arcconfig
 - http://phabricator.dev/diffusion/BOBALIE/browse/origin:master/
 - http://phabricator.dev/diffusion/BOBALIE/browse/origin:master/.arcconfig
 - http://phabricator.dev/rBOBALIEbfede2e8ea9435644968e2e76c0bac8949fb7d06

For a given file (say
http://phabricator.dev/diffusion/BOBALIE/change/origin:master/.arcconfig;bfede2e8ea9435644968e2e76c0bac8949fb7d06)
 - history view*
 - browse view
 - change view

* found a bug where the history view doesn't have the change view in the left
hand UI
will fix laters

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1095
2011-11-09 16:21:59 -08:00
Marek Sapota
b71a55900a Allow tweaking of Differential mail by using events
Summary: Allow tweaking Differential mail before sending.

Test Plan:
Wrote a listener renaming Differential attachments and it worked without
problems.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, mareksapota, davidreuss

Differential Revision: 1091
2011-11-09 10:13:53 -08:00
epriestley
802dcd4cfb Add attchment support to SendGridAdapter
Summary:
  - Add attachment support for SendGrid.
  - Add attachment support to the MetaMTA test console.

Test Plan:
  - Sent myself a file with Amazon SES via test console.
  - Sent myself a file with SendGrid via test console.

Reviewers: mareksapota, jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 1089
2011-11-08 14:28:10 -08:00
Emil Hesslow
88dc9c471d Add actAsUser to API
Summary: createrevision creates the revision as the user which certificate is
used. Add a meta parameter to API calls named actAsUser so one user can create
revisions for someone else. Right now there is no authentication.

Test Plan: Called createrevision with one users cert and set actAsUser to
someone else. The revision was created as the actAsUser user.

Reviewers: epriestley, nh, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1087
2011-11-08 08:12:31 -08:00
adonohue
7d2a18d883 Examples using JX.View
Summary: Provide a dirt-simple working example of client-side templating and
reactive programming.

Test Plan: Load the examples

Reviewers: epriestley, mroch, tomo

Reviewed By: epriestley

CC: ide, schrockn, aran, rzadorozny, epriestley

Differential Revision: 908
2011-11-06 15:17:00 -08:00
Jason Ge
f46e12d0ca Refactor some Herald code
Summary:
I was reading herald code for a task and realized that the method was
really long. So I refactor it to shorter methods.

Test Plan:
was still able to create a differential rule and commit rule; and
verified that dry-run still worked.

Reviewers: epriestley, tuomaspelkonen

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1077
2011-11-04 16:32:00 -07:00
epriestley
fbfb263cd9 Provide a configuration flag to disable silliness in the UI
Summary: See comments. A few installs have remarked that their organizations
would prefer buttons labled "Submit" to buttons labeled "Clowncopterize".

Test Plan:
  - In "serious" mode, verified Differential and Maniphest have serious strings,
tasks can not be closed out of spite, and reset/welcome emails are extremely
serious.
  - In unserious mode, verified Differential and Maniphest have normal strings,
tasks can be closed out of spite, and reset/welcome emails are silly.
  - This does not disable the "fax these changes" message in Arcanist (no
reasonable way for it to read the config value) or the rainbow syntax
highlighter (already removable though configuration).

Reviewers: moskov, jungejason, nh, tuomaspelkonen, aran

Reviewed By: moskov

CC: aran, moskov

Differential Revision: 1081
2011-11-04 15:24:54 -07:00
Jason Ge
4cdfc6d1cb Fix exception when encoding is not defined
Summary:
the code tries to access 'encoding' property even when the
repository is empty. The fix is to set it to null in that case.

Test Plan: run the conduit method on my sandbox and it works now.

Reviewers: grglr, epriestley, nh

Reviewed By: grglr

CC: aran, grglr

Differential Revision: 1075
2011-11-02 21:41:29 -07:00
Marek Sapota
f1de90d7ef Mark person that actually runs arc commit as committer instead of the author.
Summary:
`arc commit` and `arc mark-committed` would only add comments <author> committed
this revision, since now everyone can run this commands it makes more sense to
show the actual committer instead of the author.

Test Plan:
Commit (or mark committed) not your revision, Phabricator should add <you>
committed this revision comment instead of <author> committed this revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1067
2011-11-01 15:26:56 -07:00
Marek Sapota
874ae4b7ee Allow anonymous access to conduit getdiff method.
Summary:
Allow anonymous access to conduit getdiff method, which is needed for anonymous
`arc patch`.

Test Plan: Running getdiff with an unauthenticated conduit should work.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1068
2011-11-01 15:26:32 -07:00
Marek Sapota
74d1983c68 Move Abandon Revision action to the bottom if it's an admin action.
Test Plan:
Login as admin, look at an open revision you don't own, you should be able to
choose '(Admin) Abandon Revision', the option should be on the bottom and should
abandon the revision after sending the comment.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1060
2011-10-28 09:20:18 -07:00