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

2818 commits

Author SHA1 Message Date
epriestley
ae2e73ce80 Add "stop on redirect" and "always profile" debugging options
Summary:
Currently, it's hard to debug performance issues on POST pages. Add flags to stop redirects and always collect profiles.

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

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

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

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2990
2012-07-17 12:06:25 -07:00
epriestley
9d26ef4248 Improve differential.createinline
Summary:
  - Share code between `createinline` and `getcomments`
  - Make them both extend the base class.
  - Sync up the parameters (this is more or less nonbreaking since the call is ~6 hours old).

Test Plan: Created and fetched inlines via the API.

Reviewers: alanh, vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1500

Differential Revision: https://secure.phabricator.com/D2988
2012-07-17 12:06:18 -07:00
Owen Jacobson
883e11f761 Retain pid files for dead daemons in 'phd status'.
'phd status' should have a stable result when invoked multiple times.
Automatically removing PID files for dead daemons every time 'phd status' is
invoked prevents tools from noticing that a daemon has died if something
happens to invoke 'phd status' before the tool looks. This affects Puppet
noticably, since it probably runs the status command every half hour.
2012-07-17 13:48:24 -04:00
Owen Jacobson
420d6426f9 'phd status' should exit with non-zero if daemons are not running.
'phd status' may be invoked by tools (such as puppet) which need to make
automated decisions about whether to start/restart the daemon. To enable this,
'phd status' now exits with 0 if all daemons are running, 1 if no daemons are
running, and 2 if some (but not all) daemons are running.
2012-07-17 13:48:15 -04:00
epriestley
02f40fd7ef Allow noncritical blocks in email bodies to be disabled via config
Summary:
See T931. LLVM users would also prefer simpler mail, and have similarly harsh opinions about the current state of affairs.

Allow installs to disable the hint blocks if they don't want them.

Test Plan: Sent myself mail with these settings on/off, got mail with/without the blocks.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T931

Differential Revision: https://secure.phabricator.com/D2968
2012-07-16 19:02:06 -07:00
epriestley
378feb3ffb Centralize rendering of application mail bodies
Summary: This is a minor quality-of-life improvement to prevent D2968 from being as nasty as it is.

Test Plan: Ran unit tests; generated Differential, Maniphest and Diffusion emails and verified the bodies looked sensible.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T931

Differential Revision: https://secure.phabricator.com/D2986
2012-07-16 19:01:43 -07:00
mkedia
508f1b19f9 [easy] Add limit param to recent commits
Summary: as title

Test Plan: https://phabricator.fb.com/conduit/method/diffusion.getrecentcommitsbypath/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2838
2012-07-16 17:46:42 -07:00
vrana
5daf08048b Fix path autocomplete
Summary: Paths autocompleter sometimes omits `/`.

Test Plan:
# Go to https://secure.phabricator.com/owners/new/.
# Write `/specs/h` to path.
# Delete the second slash resulting in `/specsh`.
# Don't find `/specshistorytest.html` in autocomplete.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2981
2012-07-16 13:28:16 -07:00
Alan Huang
8065d72899 Add a differential.createinline method to Conduit
Summary: a silly thing because I was bored

Test Plan: and I said "arc call-conduit", and there were comments

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1500

Differential Revision: https://secure.phabricator.com/D2971
2012-07-16 11:49:06 -07:00
epriestley
22660cff2a OMG
Summary: NOOOO

Test Plan: Image macros work again.

Reviewers: kdeggelman, btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2979
2012-07-16 10:35:36 -07:00
epriestley
cb8120551f Don't special-case LiskDAO->load(0)
Summary:
Lisk currently behaves in two different ways if you call it like `load("cow")` (throws) versus `load(99999999)` (returns null), where neither ID exists.

This was intended to catch programming errors as distinct from missing data, but in practice the former is very rare and you have to handle the latter in most cases anyway. The case where you pass "0" is particularly confusing. See D2971 for an example.

On the balance, I think this ends up being far more confusing than helpful. Instead, just return NULL if we're sure there's no such object.

Test Plan: Reasoned about program behavior.

Reviewers: alanh, btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2977
2012-07-16 09:50:23 -07:00
epriestley
8fd97f4352 When users submit a no-op edit in Phriction, don't update the document
Summary:
See T1501. When users mash "save", stop them if they didn't change anything.

Also, don't default-fill the "edit notes" field with the previous notes. This is meant to be more like a commit message for your changes.

Test Plan: Edited a document with no changes, got a dialog. Edited a document with a title change only and a description change only, things worked. Edited a document with a previous "Edit notes", got a blank default fill.

Reviewers: alanh, btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1501

Differential Revision: https://secure.phabricator.com/D2978
2012-07-16 09:40:10 -07:00
vrana
23f2ffb81c Add footer link to report a bug
Summary:
According to http://www.phabricator.com/docs/phabricator/article/Give_Feedback!_Get_Support!.html, we love feedback.
But there is no way to provide it without reading the docs.

Test Plan: Clicked the link.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, ldemailly

Differential Revision: https://secure.phabricator.com/D2975
2012-07-16 09:37:27 -07:00
Jason Ge
e0d4793e74 Show responsiveness' in differential stats
Summary: For "accept" and "reject" action, find the time between the action and last time the revision was updated by a new diff. Show it as the responsiveness row.

Test Plan: view it for a couple of engineers.

Reviewers: epriestley, vii

Reviewed By: epriestley

CC: vrana, nh, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2970
2012-07-13 10:38:27 -07:00
Evan Priestley
f5754ddadc Merge pull request #157 from avish/active-directory-ldap
Add active-directory domain-based ldap authentication support
2012-07-13 08:44:41 -07:00
Avishay Lavie
226cf288e9 Add active-directory domain-based ldap authentication support
Summary: Add active-directory domain-based ldap authentication support

Test Plan: Tested on a live install against Active Directory on a Windows Server

Reviewers: epriestley

CC: aran, epriestley

Maniphest Tasks: T1496

Differential Revision: https://secure.phabricator.com/D2966
2012-07-13 15:19:34 +03:00
epriestley
621bac74c7 Unfatal the "show change" link in Maniphest description edits
Summary: I missed this callsite in D2946. Transition it to the new markup cache.

Test Plan: Clicked "show change" on a description edit transaction, got the change instead of a fatal.

Reviewers: alanh, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1502

Differential Revision: https://secure.phabricator.com/D2972
2012-07-12 21:34:53 -07:00
epriestley
9574b91e55 Add options to include patches inline or attached for Diffusion commit emails
Summary:
See D818 for an older attempt at this. Support code has matured to the point where the patch is pretty straightforward.

@tido, this was a long-standing request from Aditya back in the day.

Test Plan: Used `reparse.php --herald` to send myself a bunch of emails with various patch configurations. Confirmed that limits are respected, reasonable errors arise when they're violated, etc. (Timeout is a little funky but that's out of scope here, I think.)

Reviewers: btrahan

Reviewed By: btrahan

CC: tido, aran

Maniphest Tasks: T456

Differential Revision: https://secure.phabricator.com/D2967
2012-07-12 13:33:35 -07:00
epriestley
2b690372de Fix several issues with Differential exception email
Summary:
  - Assigning $cc_phids clobbers the correct value assigned on line 80.
  - Remove stack trace noise, the trace is always meaningless and well-known.
  - Actually show the original body.

Test Plan: Piped mail to the mail receiver and verified the errors didn't CC revision CCs, no longer had traces, and included the original raw text body.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2969
2012-07-12 13:33:26 -07:00
Evan Priestley
62e039b748 Merge pull request #150 from linead/ldap_dir
Ldap dir
2012-07-11 18:37:36 -07:00
Evan Priestley
f5196e868d Merge pull request #151 from flxf/sever_typo
Fixed 'Sever' Typo
2012-07-11 17:22:18 -07:00
epriestley
6f20809a51 Improve Windows editor documentation with specific examples for popular editors
Summary: See D2955, D2956, D2957.

Test Plan: Read documentation. Ran all these commands from Git Bash and cmd.exe and used the specified editors to edit blocks of text.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1309

Differential Revision: https://secure.phabricator.com/D2958
2012-07-11 17:02:26 -07:00
epriestley
ff4774a970 Don't fatal on missing commit in DiffusionBrowseFileController
Summary:
Still not 100% sure what the repro case here is, but we do something similar on line 438 above. One case could be an SVN repo with a subpath specified, I think.

In any case, don't fatal if we're missing the commit.

Test Plan: Loaded some browse views, although I don't have a repro.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2959
2012-07-11 17:02:15 -07:00
Bob Trahan
dc75e79cb5 Make IRC Bot connect on both successful end of MOTD (376) and non-successful MOTD (422)
Summary: http://www.networksorcery.com/enp/protocol/irc.htm

Test Plan: augment code with an additional debug line (phlog('hi');) so I can see my case was trigged and it will fall through. setup an ill-configured IRC server with ngircd. Configure an ircbot to connect to said ill-configured IRC server. verify ircbot connected to channel. verify in irc bot logs that debug line was invoked.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1452

Differential Revision: https://secure.phabricator.com/D2962
2012-07-11 16:27:41 -07:00
epriestley
fcd04708d2 Add markup cache collection to the GC daemon
Summary: Allow the GC daemon to collect the new markup cache.

Test Plan: Ran gc daemon in "debug" mode, saw it collect cache entries.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2947
2012-07-11 11:40:18 -07:00
epriestley
5d8b75b4da Use the unified markup cache for Maniphest
Summary:
  - See D2945.
  - Drop `cache` field from ManiphestTransaction.
  - Render task descriptions and transactions through PhabricatorMarkupEngine.
  - Also pull the list of macros more lazily.

Test Plan:
  - Verified transactions and transaction preview work correctly and interact with cache correctly.
  - Verified tasks descriptions and task preview work correctly.
  - Verified we don't hit the imagemacro table when we're rendering everything from cache anymore.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2946
2012-07-11 11:40:10 -07:00
Felix Fung
523395590b Fixed 'Sever' Typo 2012-07-10 23:11:42 -07:00
Andrew Gallagher
eac8e0e7f3 Support postponed lint status in creatediff conduit call
Test Plan:
Created diff with a postponed linter and the lint status set to such
via arcanist.  Verified lint status showed as postponed in diff view.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1332

Differential Revision: https://secure.phabricator.com/D2932
2012-07-10 20:02:16 -07:00
epriestley
176b4a68a8 Fix some mercurial edge cases
Summary:
  - Old versions of Mercurial give different output for `hg log -- ''` and `hg log`. Just use `hg log`.
  - Branch names with spaces can't be specified in `--rev`. I talked with hstuart in #mercurial and apparently am not crazy.

Test Plan:
  - Viewed history of a repository.
  - Viewed history of a file.
  - Viewed branch `m m m m m 2:ffffffffffff (inactive)`
  - Learned that you checkout this branch with `hg checkout ':m m m m m 2:ffffffffffff (inactive)'`

Reviewers: dschleimer, btrahan

Reviewed By: dschleimer

CC: aran

Maniphest Tasks: T1268

Differential Revision: https://secure.phabricator.com/D2950
2012-07-10 10:36:33 -07:00
Daniel Fullarton
42dfee38e7 Refactor methods, fix style issues 2012-07-11 00:56:38 +10:00
epriestley
e2e9aed4fa Fix symbol handling in symbol query and IRC "Where is x?" handler
Summary: If a symbol's project has no linked repository, we currently explode. Instead, decline to generate a URI and fall back gracefully.

Test Plan: https://secure.phabricator.com/chatlog/channel/%23phabricator/?at=22345

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1465

Differential Revision: https://secure.phabricator.com/D2948
2012-07-09 17:51:42 -07:00
epriestley
2b0b9a1573 Add a generic multistep Markup cache
Summary:
The immediate issue this addresses is T1366, adding a rendering cache to Phriction. For wiki pages with code blocks especially, rerendering them each time is expensive.

The broader issue is that out markup caches aren't very good right now. They have three major problems:

**Problem 1: the data is stored in the wrong place.** We currently store remarkup caches on objects. This means we're always loading it and passing it around even when we don't need it, can't genericize cache management code (e.g., have one simple script to drop/GC caches), need to update authoritative rows to clear caches, and can't genericize rendering code since each object is different.

To solve this, I created a dedicated cache database that I plan to move all markup caches to use.

**Problem 2: time-variant rules break when cached.** Some rules like `**bold**` are time-invariant and always produce the same output, but some rules like `{Tnnn}` and `@username` are variant and may render differently (because a task was closed or a user is on vacation). Currently, we cache the raw output, so these time-variant rules get locked at whatever values they had when they were first rendered. This is the main reason Phriction doesn't have a cache right now -- I wanted `{Tnnn}` rules to reflect open/closed tasks.

To solve this, I split markup into a "preprocessing" phase (which does all the parsing and evaluates all time-invariant rules) and a "postprocessing" phase (which evaluates time-variant rules only). The preprocessing phase is most of the expense (and, notably, includes syntax highlighting) so this is nearly as good as caching the final output. I did most of the work here in D737 / D738, but we never moved to use it in Phabricator -- we currently just do the two operations serially in all cases.

This diff splits them apart and caches the output of preprocessing only, so we benefit from caching but also get accurate time-variant rendering.

**Problem 3: cache access isn't batched/pipelined optimally.** When we're rendering a list of markup blocks, we should be able to batch datafetching better than we do. D738 helped with this (fetching is batched within a single hunk of markup) and this improves batching on cache access. We could still do better here, but this is at least a step forward.

Also fixes a bug with generating a link in the Phriction history interface ($uri gets clobbered).

I'm using PHP serialization instead of JSON serialization because Remarkup does some stuff with non-ascii characters that might not survive JSON.

Test Plan:
  - Created a Phriction document and verified that previews don't go to cache (no rows appear in the cache table).
  - Verified that published documents come out of cache.
  - Verified that caches generate/regenerate correctly, time-variant rules render properly and old documents hit the right caches.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1366

Differential Revision: https://secure.phabricator.com/D2945
2012-07-09 15:20:56 -07:00
epriestley
918ac5755c Limit maximum size of Owners package queries
Summary:
Currently, a change may affect a very large number of paths. When we run the OwnersWorker on it, we'll execute a query which looks up packages for the paths. This may exceed "max_allowed_packet". Instead, break the list of paths into smaller chunks.

This is mostly to unblock r4nt / llvm, I'm going to add a more finessed approach to array_chunk() shortly.

Test Plan: Ran `reparse.php --owners` on a revision which affected packages, verified results are the same before and after the change. Set chunk size to 1, verified query results aggregated properly.

Reviewers: btrahan, jungejason, nh

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2943
2012-07-09 10:51:46 -07:00
epriestley
1089a48d4a Allow edges to be configured to prevent cycles
Summary:
Certain types of things we should be storing in edges (notably, Task X depends on Task Y) should always be acyclic. Allow `PhabricatorEdgeEditor` to enforce this, since we can't correctly enforce it outside of the editor without being vulnerable to races.

Each edge type can be marked acyclic. If an edge type is acyclic, we perform additional steps when writing new edges of that type:

  - We acquire a global lock on the edge type before performing any reads or writes. This ensures we can't produce a cycle as a result of a race where two edits add edges which independently do not produce a cycle, but do produce a cycle when combined.
  - After performing writes but before committing transactions, we load the edge graph for each acyclic type and verify that it is, in fact, acyclic. If we detect cycles, we abort the edit.
  - When we're done, we release the edge type locks.

This is a relatively high-complexity change, but gives us a simple way to flag an edge type as acyclic in a robust way.

Test Plan: Ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D2940
2012-07-09 10:39:38 -07:00
epriestley
bf8cbf55b1 Namespace GlobalLocks to storage namespaces
Summary:
Currently, multiple unit tests that acquire global locks will interfere with each other. Namespace the locks so they don't.

(Possibly we should also rename this to PhabricatorStorageNamespaceLock or something since it's not really global any more, but that's kind of unwieldy...)

Test Plan: Acquired locks with --trace and verified they were namespaced properly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D2939
2012-07-09 10:39:30 -07:00
epriestley
d86c4e0366 Store forced connections in the Lisk connection cache
Summary:
In unit tests which use fixtures, we open transactions on every connection we establish. However, since we don't track connections that are established with "$force_new" (currently, only GlobalLock connections) we never close these transactions normally.

Instead of not tracking these connections, track them using unique keys so we'll never get a cache hit on them.

Test Plan: Built unit tests on top of this, had them stop dying from unclosed transactions.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D2938
2012-07-09 10:39:21 -07:00
epriestley
7cf6313be9 Add a generic object for unit tests
Summary:
A later diff adds unit tests against edges, but we need real objects to connect with edges. Add some trivial objects to the Harbormaster database to compliment the similar HarbormasterScratchTable.
On its own, this does nothing interesting.

Test Plan: Built unit tests on this in a followup.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D2937
2012-07-09 10:39:14 -07:00
epriestley
bb714a2bef Update GitHub oauth docs
Summary: GitHub moved these pages around and made it easier to find your application list.

Test Plan: Read docs, verified links are correct.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2941
2012-07-09 10:38:32 -07:00
epriestley
3e15c3580d Simplify build file from data-or-hash code
Summary:
We have a bit more copy-paste than we need, consolidate a bit.

(Also switch Mercurial to download git diffs, which it handles well; we use them in "arc patch".)

Test Plan:
  - Downloaded a raw diff from Differential.
  - Downloaded a raw change from Diffusion.
  - Downloaded a raw file from Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2942
2012-07-09 10:38:25 -07:00
epriestley
18cfab0c36 Allow configuration of the default monospaced font style
Summary: This is a fairly contentious default that we can easily move to configuration.

Test Plan: Changed the default, changed my user setting, reverted my user setting, verified the "settings" page.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2935
2012-07-06 15:39:43 -07:00
Miles Shang
1b8ed98ddc Multi-line highlighting in Diffusion.
Summary: Currently, Diffusion supports highlighting of line ranges passed in the URI. It would be helpful to be able to highlight multiple line ranges.

Test Plan: Accessed directly via URL in my sandbox. Seems to work. I'm not sure what other components use this functionality, but this change should be backwards compatible.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D2921
2012-07-05 19:44:12 -07:00
Alan Huang
731e0df2b5 Add a differential.abandon Conduit method.
Summary:
Brazenly copied from differential.close. Ulterior motive
is to be able to automate discarding of useless commits from
arcanist testing.

Test Plan:
Called method in phabricator sandbox. Revisions were
successfully abandoned.

Reviewers: epriestley

Reviewed By: epriestley

CC: nh, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2928
2012-07-05 16:47:04 -07:00
epriestley
78d2f08fcd Add an Aphlict CLI client
Summary: Depends on D2926. Adds a simple CLI client for Aphlict to make it easier to debug stuff.

Test Plan: Ran client, saw debug messages.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2927
2012-07-05 16:04:04 -07:00
epriestley
63be89ba00 Improve error message for error 2006
Summary:
See discussion here:

https://secure.phabricator.com/chatlog/channel/%23phabricator/?at=21186

Basically, MySQL usually raises a good error if we exceed "max_allowed_packet":

  EXCEPTION: (AphrontQueryException) #1153: Got a packet bigger than 'max_allowed_packet' bytes

But sometimes it gives us a #2006 instead. This is documented, at least:

>"With some clients, you may also get a Lost connection to MySQL server during query error if the communication packet is too large."

  http://dev.mysql.com/doc/refman//5.5/en/packet-too-large.html

Try to improve the error message to point at this as a possible explanation.

Test Plan: Faked an error, had it throw, read exception message. See also chatlog.

Reviewers: btrahan, skrul

Reviewed By: skrul

CC: aran

Differential Revision: https://secure.phabricator.com/D2923
2012-07-05 16:03:58 -07:00
epriestley
ae6661ac3f Document event --trace added by D2918
Summary: Also fix a doc typo.

Test Plan: Read.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1092

Differential Revision: https://secure.phabricator.com/D2919
2012-07-05 16:03:49 -07:00
epriestley
ce360926b7 Allow PhabricatorGlobalLock to block
Summary: See D2924.

Test Plan: Ran locks with blocking timeouts.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2925
2012-07-05 16:03:43 -07:00
Bob Trahan
67691c196e Phame - add an "Everyone but Me" view and make published posts truly accessible to the whole world.
Summary:
accessibility covers not only a given post but also the various "published" views.

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

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

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1373

Differential Revision: https://secure.phabricator.com/D2898
2012-07-05 15:43:14 -07:00
epriestley
4dd5bcf1cd Don't fail in Diffusion if .gitmodules is missing
Summary:
See T1448. If this file isn't present, just move on instead of failing, since it's a (sort of) legitimate repository state.

Also fix some silliness a little later that got introduced in refactoring, I think.

Test Plan: Added an external to my test repo and removed ".gitmodules". Verified that the directory is now viewable after this patch.

Reviewers: btrahan, davidreuss, jungejason

Reviewed By: davidreuss

CC: aran

Maniphest Tasks: T1448

Differential Revision: https://secure.phabricator.com/D2922
2012-07-05 06:12:35 -07:00
epriestley
bf9cd55577 Minor, fix an issue where fail to extract the author PHID from an edge transaction. 2012-07-05 05:48:30 -07:00
linead
51d2d06e37 Added ldap import controller 2012-07-04 12:10:38 +10:00
epriestley
ddf67fce58 Add an example event listener, improve documentation, and add a commit discovery event
Summary: Improve documentation around Phabricator events.

Test Plan: Generated and read documentation. Ran test script.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T1092

Differential Revision: https://secure.phabricator.com/D2917
2012-07-03 16:46:27 -07:00
Jason Ge
61b79b5359 Use binary_safe_diff from arcanist
Summary:
binary_safe_diff is needed in arcanist too. Moved it over to
arcanist. See D2915.

Test Plan: diffusion page rendered correctly on binary file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2916
2012-07-03 13:51:37 -07:00
epriestley
dcf3a31944 Minor, unlock after discovery exception. 2012-07-03 07:46:38 -07:00
Jonathan Lomas
c821639e93 Opt-in approve your own Differential revisions
Summary: Did exactly what @epriestley suggested in T1428#2.

Test Plan: Turn it on in your config, post a revision, accept it.  Turn it off in your config, post a revision, can't accept it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2900
2012-07-03 06:30:48 -07:00
epriestley
948259ee92 Minor, restore missing limits from 310cf00fc3 2012-07-03 06:25:03 -07:00
epriestley
bda5c670bc Add useful text descriptions to edge transactions
Summary: See D2906. This just adds text so they render pretty.

Test Plan:
Got pretty emails and rendered transactions.

{F13706}

Reviewers: btrahan, davidreuss

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2907
2012-07-02 15:42:16 -07:00
epriestley
9f4cfd40bc Insert Maniphest transactions when edges are edited
Summary:
  - See D2741.
  - When EdgeEditor performs edits, emit events.
  - Listen for Maniphest edge events and save the changes as transactions.
  - Do all this in a reasonably generic way that won't take too much rewriting as we use edges more generally.

Test Plan: Attached and detached commits from tasks, saw reasonable-looking transactions spring into existence.

Reviewers: btrahan, davidreuss

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2906
2012-07-02 15:42:06 -07:00
epriestley
310cf00fc3 Consolidate feed query code
Summary: Simplify FeedQuery by making it extend from PhabricatorIDPagedPolicyQuery

Test Plan: Looked at feed on home, projects, user profile, and called `feed.query`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2905
2012-07-02 15:41:19 -07:00
epriestley
3a453f2cce Fix a typo
Summary: This should be "files".

Test Plan: Read carefully.

Reviewers: btrahan, nikil

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1435

Differential Revision: https://secure.phabricator.com/D2909
2012-07-02 15:22:35 -07:00
epriestley
a33e84e1e5 Add table markup to Phabricator
Summary: See D2902.

Test Plan: Made tables, generated docs.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2903
2012-07-02 14:44:38 -07:00
epriestley
534c0aa326 Minor, move all storage/query/db code to src/infrastructure/storage 2012-07-02 10:49:00 -07:00
epriestley
9ba6ebb97b Minor, move all remarkup code to src/infrastructure/markup/ 2012-07-02 10:44:37 -07:00
epriestley
d49b06c322 Consolidate PhabricatorFeedStory construction code
Summary: This code is duplicated in two places; share it.

Test Plan: Looked at feed and notifications.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2901
2012-07-02 10:37:22 -07:00
epriestley
f5f88d8698 Minor, fix an issue where %e is empty from git log. 2012-07-02 07:29:15 -07:00
epriestley
1c1fdf0448 Minor, move the "repair" instructions to the right place. 2012-07-02 07:22:56 -07:00
epriestley
16c20c92fe Minor, fix an issue where we may fail to select a translation for mail. 2012-07-02 07:17:56 -07:00
epriestley
fc45398ba9 Support basic notification aggregation
Summary: This is both only partially complete (supports Maniphest only) and somewhat overcomplicated (includes support for applying similar algorithms to Feed), but provides runtime aggregation of notifications.

Test Plan: {F13502}

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2884
2012-07-01 11:08:59 -07:00
epriestley
126f8e7240 Document git:branch-unique(*) and arc:bookmark
Summary: Explain how these work. Let me know if there's a clearer way to explain "arc:bookmark".

Test Plan: Generated / viewed documentation.

Reviewers: dschleimer

Reviewed By: dschleimer

CC: aran

Maniphest Tasks: T1233

Differential Revision: https://secure.phabricator.com/D2868
2012-07-01 11:07:05 -07:00
dschleimer
86fa4fd97f [Phabricator] track Mercurial bookmarks for differential diffs
Summary:
This adds all the changes necessary to track the active Mercurial
bookmark for differential diffs.  We render both branch and bookmark
information in the branch field of the Differential revison view, as
seen in
https://secure.phabricator.com/file/data/kzpmu3evfkukxdjyxrfz/PHID-FILE-eqorsqupxvwirqi2s5lo/bookmark_differential.jpg

The Arcanist half of this is https://secure.phabricator.com/D2896

Test Plan:
Mostly D2896.

Additionally, loaded a diff created with a bookmark, as per the link in the summary.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1331

Differential Revision: https://secure.phabricator.com/D2897
2012-06-30 15:41:58 -07:00
epriestley
f666c3d559 Minor, fix wording for some maniphest stories 2012-06-30 06:53:11 -07:00
David Reuss
fd94700d69 Minor, really fix empty reference properties in diffusion
Summary:
Oh man, after the explode/map, the output when no references is actually

  array(1) {
    [0]=> string(0) ""
  }

.. making the falsey check fail to work as expected.

Test Plan: same as D2892, but with a little more scrutiny :p

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2893
2012-06-30 06:43:33 -07:00
David Reuss
19d58564d1 Minor, don't show references in diffusion if there isn't any
Summary:
When there are no other references (and there will be none ususally,
if D2891 is accepted), we would show "References:" with empty contents.

Avoid that by testing for empty output after applying stupid filtering.

Test Plan: Looked in a standard repository with no special refs.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2892
2012-06-30 06:26:36 -07:00
vrana
2391c54c60 Delete method implemented in parent 2012-06-30 01:21:04 -07:00
vrana
273211ae3e Highlight also lines that were added on the other side in diff of diffs 2012-06-29 18:19:01 -07:00
vrana
a2f4d661b9 Fix computing line numbers in diff of diffs
Test Plan: Live in prod.

Auditors: jungejason
2012-06-29 18:03:26 -07:00
vrana
99df72b81c Unhighlight lines modified in rebase
Summary:
Diff of diffs display changes between new versions of file.
This is bad after rebase because there can be many unrelated changes so it is hard to spot the real change.

This diff unhighlights the lines that were added or removed in rebase.
The changes are still visible (they can be sometimes relevant) but very subtle.

Test Plan:
# Add, change and delete line. Display diff.
# Add and change some lines in parent. Rebase. Display diff. Display diff of diff.
# Change and add some lines. Display diff. Display diff to first diff. Display diff to second diff.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: jungejason, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2761
2012-06-29 17:21:23 -07:00
Nick Harper
d70f8dad3f Change lock in PullLocalDaemon to only lock on discovery
Summary:
The locks held by read-only pullLocal daemons were causing our discovery instance
to not get the lock and fail at discovery. We don't need to hold the lock while
pulling (only while discovering), so this moves the lock to the appropriate place.

Test Plan: tested in production

Reviewers: jungejason, epriestley, vrana

Reviewed By: jungejason

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2890
2012-06-29 16:19:32 -07:00
David Reuss
30b06c1ad0 Diffusion: turn branches/refs properties into links
Summary: They just beg to be clicked.

Test Plan: clicked furiously with my mouse until i got tired - went expected places.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2889
2012-06-29 14:14:15 -07:00
David Reuss
aa95ef2463 Handle encoding if git tells us it's still not UTF-8
Summary:
Even though `--encoding` is passed to the command, git still fails
in some cases to correctly convert the output. Attempt the conversion
ourselves if it's non UTF-8.

Test Plan: Reparsed message in a repository with ISO-8859-1 encoded commit messages.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T452

Differential Revision: https://secure.phabricator.com/D2888
2012-06-29 12:38:09 -07:00
epriestley
2bb8150506 Add specific close status queries to maniphest.find (and rename it to maniphest.query)
Summary:
  - Allow clients to query for specific closed statuses (invalid, resolved, wontfix, etc), not just "closed" tasks.
  - Rename this method to maniphest.query and deprecate maniphest.find as an alias to maniphest.query, for API consistency.

Test Plan: Ran queries for all tasks, "wontfix" tasks, closed tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2887
2012-06-29 09:17:19 -07:00
Andrew Gallagher
3223defe96 Add support for postponed linters
Summary:
This adds:
1) A new "arc:lint-postponed" diff property which stores a list of
lint names that are postponed and a finishpostponedlinters conduit
method which removes linters from this list.  Postponed linters are
shown in the lint details.
2) A updatelintmessages conduit message, which adds additional lint
messages to the "arc:lint" diff property.

In combination, this provides very basic support for running
asynchronous static analysis tools.  When the diff is being created,
a list of asynchronous static analysis runs can be added to the
diff's postponed linters list.  As these postponed linters finish
up, then can report new lint messages back to the diff then mark
themselves as complete.

The client is currently responsible for filtering the lint messages
by things like affected lines and files.

Test Plan:
Used conduit call API to add lint messages and remove postponed
linters from a test diff.

Reviewers: epriestley, vrana, nh, jungejason

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1332

Differential Revision: https://secure.phabricator.com/D2792
2012-06-28 18:09:38 -07:00
Andrew Gallagher
47cb5d3cc3 Revert "Revert "Mark date and time format translatable""
This reverts commit 24a025becb.
2012-06-28 18:08:59 -07:00
Andrew Gallagher
24a025becb Revert "Mark date and time format translatable"
This reverts commit e3eb8d55c5.
2012-06-28 18:08:25 -07:00
Jason Ge
081c3a24b0 Enable query activediff timestamp
Summary: people want to get the info about how long the revision has been active since the last time the authoer ran 'arc diff'.

Test Plan: call it from conduit console

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: Girish, aran, epriestley

Differential Revision: https://secure.phabricator.com/D2885
2012-06-28 17:52:31 -07:00
vrana
b10428fe60 Link to a specific Phabricator version from stack trace
Summary: Line numbers and file paths may be different in current version.

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

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2886
2012-06-28 16:22:06 -07:00
epriestley
46008ce3b3 Record initial branch when recording commits
Summary:
We have a race condition right now, where we may insert a commit without `seenOnBranches`. This means shouldAutocloseCommit will return false (since it's not on any autoclose branches) if the message parser runs fast enough, causing it to associate the commit but not close the revision. This happened for D2851.

Also prompt the user to repair broken repositories.

Test Plan: Ran discovery / repair. Ran discovery on new commits. Verified 'seenOnBranches' value. Deleted some data, verified "repair" error. Repaired repository.

Reviewers: jungejason, nh, vrana, btrahan

Reviewed By: btrahan

CC: aran

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

Test Plan: {F13494}

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2883
2012-06-28 13:59:50 -07:00
vrana
0c321d8176 Delete old commented code 2012-06-28 10:45:45 -07:00
vrana
7227d7d850 Fix displaying day numbers in calendar
Summary:
They were shifted by one because of calling `$day->setTime(24, 0, 0)`.

I've tried to clone the date and get the epoch from the clone but it was crashing for some reason.

Test Plan: /calendar/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2882
2012-06-28 10:28:56 -07:00
Bob Trahan
38e029205b Add a "Download Raw Diff" action to Differential
Summary: this lets people download diffs easily in case arc export, arc patch, etc isn't to their liking or whatever.

Test Plan: "Download Raw Diff" a few times with various things toggled in the "Revision Update History" widget

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1294

Differential Revision: https://secure.phabricator.com/D2855
2012-06-28 10:12:20 -07:00
epriestley
1bd0a1f0d9 Add a global lock for the RepositoryPullDaemon
Summary: Allow multiple daemons to run without contention.

Test Plan: Ran multiple daemons simultaneously in "debug" mode, observed them acquiring (and sometimes failing to acquire) locks.

Reviewers: btrahan, jungejason, nh

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1400

Differential Revision: https://secure.phabricator.com/D2877
2012-06-28 09:54:55 -07:00
epriestley
bbfb686db5 Allow 'repository.create' to set description and autoclose
Summary:
These are currently not available via Conduit.

Also fix a bug where bad JSON input triggers an error about undefined `$metadata`.

Test Plan: Ran 'repository.create' with and without a description and with and without autoclose. Verified the created repositories had the requested attributes.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2881
2012-06-28 09:54:48 -07:00
vrana
6c1c3c3a7a Fix line count in diffs with more hunks per changeset
Summary: This error usually doesn't occur because we have only one hunk per changeset most of the time.

Test Plan:
  $hunk = new ArcanistDiffHunk();
  $hunk->setAddLines(1);
  $hunk->setDelLines(1);

  $change = new ArcanistDiffChange();
  $change->addHunk($hunk);
  $change->addHunk($hunk);

  $diff = DifferentialDiff::newFromRawChanges(array($change));
  var_dump($diff->getLineCount());

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2878
2012-06-27 18:09:40 -07:00
epriestley
692e54ee36 Implement a MySQL-backed global lock
Summary: Implementation is a little crazy but this seems to work as advertised.

Test Plan: Acquired locks with "lock.php". Verified they held as long as the process reamined open and released properly on kill -9, ^C, etc.

Reviewers: nh, jungejason, vrana, btrahan, Girish, edward

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1400

Differential Revision: https://secure.phabricator.com/D2864
2012-06-27 13:59:12 -07:00
vrana
0a7973488f Simplify DifferentialHunk::getAddedLines()
Summary: I will also need `getRemovedLines()` so refactor this first.

Test Plan:
New test case.
Viewed uncached diff.
Verified that the only callsite of `getAddedLines()` trims lines.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2875
2012-06-27 13:54:02 -07:00
Jason Ge
89fd1204e2 Remove the actor itself from reviewer list in commandeering
Summary: When the commandeerer was a reviewer, after the commandeering, he stayed as a reviewer. He can no longer amend the diff. He has to go 'Edit Revision' to remove himself/herself. The fix is to remove it automatically.

Test Plan:
comamndeereded a revision and the behavior is correct now:
- I was removed from the revision list
- the comment transaction shows one more entry that I removed myself as a reviewer

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: nh, vrana, aran, Korvin

Maniphest Tasks: T1225

Differential Revision: https://secure.phabricator.com/D2872
2012-06-27 09:55:31 -07:00
vrana
7ca3401d03 Allow specifying custom syntax highlighter
Summary: Related to D2873.

Test Plan: Specified it and verified that highlighting still works.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2874
2012-06-26 19:37:45 -07:00
epriestley
6edd29de43 Prevent Firebug from crashing in Firefox 13 / Window 7
Summary:
Simpler fix for D2572. Not entirely sure why Firebug is crashing Firefox. It appears to be callstack depth related, possibly? You can sort of reproduce this like this:

  >>> var f = function(n) { n && f(n - 1); }
  >>> f(10000); // Takes a few ms to run.
  >>> f(40000); // Takes a few ms to run.
  >>> f(50000); // Hangs Firefox.

If there are 2,000 files, we currently hit a stack depth of around 4,000 with the pass() rules, so it seems like we should be 10x short of exploding.

Anyway, this keeps us from increasing stack depth for menus that aren't currently open and stops Firebug from crashing.

Test Plan: Clicked 2000-diff revision in Firefox.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2870
2012-06-26 18:22:53 -07:00
vrana
6b058d3efa Mark text output in docs 2012-06-26 17:32:42 -07:00
vrana
665769d505 Use whole days in Differential stats
Test Plan: /differential/stats/revisions/

Reviewers: vii, jungejason, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2867
2012-06-26 16:32:59 -07:00
Jason Ge
da45b09bb4 Use a format that's support by both zend php and hphp
Summary:
hphp doesn't support "24:00". Use "23:59:59 instead. This is causing the stat page only shows the '1 week' stat.

in hphp

  hphpd> =strtotime('1 week ago')
  1340143086
  hphpd> =strtotime('1 week ago 24:00')

  hphpd> =strtotime('1 week ago 23:59:59')
  1340175599

in zend php

  jungejasonmbp15:tmp jungejason$ cat timeformat.php
  <?php

  echo strtotime('1 week ago 24:00');
  echo "\n";
  echo strtotime('1 week ago 23:59:59');
  echo "\n";
  echo strtotime('1 week ago 24:00') - strtotime('1 week ago 23:59:59');;
  echo "\n";
  jungejasonmbp15:tmp jungejason$ php -f timeformat.php
  1340175600
  1340175599
  1

Test Plan: viewed the stat page.

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: nh, aran, epriestley

Differential Revision: https://secure.phabricator.com/D2865
2012-06-26 15:25:49 -07:00
epriestley
91284d17e1 Document arc:exec
Summary: docs

Test Plan: Generated / read docs.

Reviewers: dschleimer

Reviewed By: dschleimer

CC: aran, phleet

Maniphest Tasks: T1233

Differential Revision: https://secure.phabricator.com/D2831
2012-06-26 14:52:45 -07:00
Jason Ge
b644a2c3a7 Remove leading "/" so that "git cat-file" works
Summary:
When the absolute path is used for load file (loadFileContent(()), it fails in git. For example:

  /var/repo/page_admin_app
  > git cat-file blob '4d6c03923006d6c444660f2c734fe03e10fd20bd':'/ios/PageAdminApp/Resources/splash/De           fault-Portrait@2x~ipad.png'
  fatal: Not a valid object name 4d6c03923006d6c444660f2c734fe03e10fd20bd:/ios/PageAdminApp/Resources/s           plash/Default-Portrait@2x~ipad.png

This is breaking the auto-closing for about 8 revisions like

https://phabricator.fb.com/rPPA4d6c03923006d6c444660f2c734fe03e10fd20bd ...
https://phabricatorcator.fb.com/rPPA51acb7e482aab0c491b530ed19dddc741d50f673 ...

Test Plan:
- reparsed https://phabricator.fb.com/rPPA4d6c03923006d6c444660f2c734fe03e10fd20bd successfully with corresponding differential revision being closed.

- verified that without leading '/', loadFileContent for svn still works. Both of the following commands worked (note the double '/' right before 'tfb':

  svn cat svn+ssh://svn.vip.facebook.com/svnroot//tfb/trunk/www/flib/intern/cachearchiver/regenerators/wurfl/CacheArchiveWurflRegenerator.php@579700
  svn cat svn+ssh://svn.vip.facebook.com/svnroot/tfb/trunk/www/flib/intern/cachearchiver/regenerators/wurfl/CacheArchiveWurflRegenerator.php@579700

Reviewers: vrana

Reviewed By: vrana

CC: nh, aran, epriestley

Differential Revision: https://secure.phabricator.com/D2847
2012-06-26 11:58:34 -07:00
vrana
2b946459b5 Inform user about associated accounts in failed login
Summary:
Sometimes people create their account in Phabricator using some OAuth provider, forget about it and then tries to login with another provider.

Provide this information to them.

Test Plan: Tried to link an account linked to already existing user.

Reviewers: epriestley

Reviewed By: epriestley

CC: robarnold, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2857
2012-06-26 11:38:07 -07:00
epriestley
c0d48d0b2d Return attached hashes from differential.query
Summary: See T693. To do "arc branch" performantly in immutable history repositories, we need to be able to do a single query to identify revisions related by hash. Return hash information to enable this.

Test Plan: Ran `differential.query`.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T693

Differential Revision: https://secure.phabricator.com/D2859
2012-06-26 09:07:52 -07:00
Bob Trahan
ee6c6943b3 Robustify profile image controls ever so slightly
Summary: show project profile image on pertinent edit page. also add a "Use Default Image" checkbox for both project and user profiles. Also added a function for projects to get the profile picture to prevent some copy + paste action.

Test Plan: set my user profile and project profile image. clicked "Use Default Image" and got the default image back.

Reviewers: epriestley, floatinglomas

Reviewed By: floatinglomas

CC: aran, Korvin

Maniphest Tasks: T1307

Differential Revision: https://secure.phabricator.com/D2852
2012-06-26 08:14:15 -07:00
epriestley
b3900a399c Minor, allow feed stories to publish with no related PHIDs (commits, future notifications). 2012-06-26 05:48:21 -07:00
Evan Priestley
111ecabc5f Merge pull request #143 from dzhibas/exception-error-fix
Exception configuration directive fix (php.ini) upload_max_filesize
2012-06-26 05:44:10 -07:00
Nikolajus
3efa34ef49 Fixing upload_max_filesize configuration directive 2012-06-26 13:20:05 +02:00
epriestley
b5b8fca341 WIP 2012-06-25 17:24:41 -07:00
epriestley
7b50b2fbdc Use subprocess output streaming to improve performance of Git commit discovery
Summary:
Improve performance of large discovery tasks in Git by using subprocess streaming, like we do for Mercurial.

Basically, we save the cost of running many `git log` commands by running one big `git log` command but only parsing as much of it as we need to. This is pretty complicated, but we more or less need it for mercurial (which has ~100ms of 'hg' overhead instead of ~5ms of 'git' overhead) so we're already committed to most of the complexity costs. The git implementation is much simpler than the hg implementation because we don't need to handle all the weird parent rules (git gives us to them easily).

Test Plan:
Before, `discover --repair` on Phabricator took 35s:

  real	0m35.324s
  user	0m13.364s
  sys	0m21.088s

Now 7s:

  real	0m7.236s
  user	0m2.436s
  sys	0m3.444s

Note that most of the time is spent inserting rows after discover, the actual speedup of the git discovery part is much larger (subjectively, it runs in less than a second now, from ~28 seconds before).

Also ran discover/pull on single new commits in normal cases to verify that nothing broke in the common case.

Reviewers: jungejason, nh, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1401

Differential Revision: https://secure.phabricator.com/D2851
2012-06-25 15:21:48 -07:00
epriestley
ca31e3e84b Add a --repair flag to "bin/repository discover"
Summary:
If a repository is missing commits because they mysteriously vanished, there's no reasonable way to get them back right now. Provide a way to ignore the state in the database and rediscover the entire repository unconditionally.

We don't queue any reparses or anything, but when I move reparse into this script we can hook things up or something. This generally shouldn't be too important anyway.

Test Plan: Ran `repository discover --repair` on Phabricator.

Reviewers: jungejason, nh, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2850
2012-06-25 12:35:47 -07:00
epriestley
13d96e6377 Introduce "bin/repository" for repository management
Summary:
Nothing new or exciting here yet, just moving the random scripts/repositories/ things to bin/repository. Also add `repository list`.

(Console stuff comes from D2841.)

Test Plan: Ran `repository list`, `repository pull`, `repository discover`, `repository discover --verbose`, `repository help`.

Reviewers: jungejason, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2849
2012-06-25 12:35:37 -07:00
Daniel Fullarton
37df05c7a5 T1354 : Resize profile pics (specifically to handle github v3 api)
Reviewed by: epriestley
GitHub Pull: https://github.com/facebook/phabricator/pull/140
2012-06-25 06:29:19 -07:00
linead
e683236793 Extract ldap auth from password auth completely 2012-06-24 18:12:56 -07:00
epriestley
bedc9acf98 Minor, fix XSS in task description change notifications. 2012-06-24 15:15:34 -07:00
epriestley
a705f336a3 Add vebose logging to PhutilRepositoryPullDaemon
Summary: Add verbose logging. This logging is activated by setting "phd.verbose" in the config, running "phd debug", or explicitly in scripts/repository/pull.php and scripst/repository/discover.php

Test Plan:
  >>> orbital ~/devtools/phabricator $ ./scripts/repository/discover.php GTEST
  Discovering 'GTEST'...
  <VERB> PhabricatorRepositoryPullLocalDaemon Discovering commits in repository 'GTEST'...
  <VERB> PhabricatorRepositoryPullLocalDaemon Examining branch '()_+abcd$100', at a37bc285a12efa7224fe19f3df54cd90fa2b897a.
  <VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
  <VERB> PhabricatorRepositoryPullLocalDaemon Examining branch '+abcd$100', at a37bc285a12efa7224fe19f3df54cd90fa2b897a.
  <VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
  <VERB> PhabricatorRepositoryPullLocalDaemon Examining branch '_+abcd$100', at a37bc285a12efa7224fe19f3df54cd90fa2b897a.
  <VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
  <VERB> PhabricatorRepositoryPullLocalDaemon Examining branch 'abcd$100', at a37bc285a12efa7224fe19f3df54cd90fa2b897a.
  <VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
  <VERB> PhabricatorRepositoryPullLocalDaemon Examining branch 'arcpatch', at 774c7737b2d560a291697126bf4513204ccf661a.
  <VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
  <VERB> PhabricatorRepositoryPullLocalDaemon Examining branch 'arcpatch-1', at dc97539bee07293f95990d71f4638335a2531d69.
  <VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
  <VERB> PhabricatorRepositoryPullLocalDaemon Examining branch 'arcpatch-2', at 1acfaec313c46dd3caa90448800181fb91b0270f.

Reviewers: jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2843
2012-06-24 15:06:40 -07:00
Jason Ge
de46a2550c Fix auto-close in a repo with missing commits in Phabricator db
Summary:
After adding more error logging: https://secure.phabricator.com/differential/diff/5295/, discover.php prints the output: https://secure.phabricator.com/P427

The problem is that there are gaps in the commits and revisions in Phabricator's DB for the repo. For some git commits, both the phabricator commits and the differential revision are missing (because of a mysterious DB data loss. nharper should be able to recover it); for some other git commits, the phabricator commits are missing (probably also because of the daba loss mentioned above). This can also happen if the git commits were too big to be parsed by phabricator.

Because of the gap, the pull daemon detects them as new autoclose commits on the branch
  (https://secure.phabricator.com/diffusion/P/browse/master/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php;382bafa271ec4962$267)
  (https://secure.phabricator.com/diffusion/P/browse/master/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php;382bafa271ec4962$592)

Then tries to update it
  (https://secure.phabricator.com/diffusion/P/browse/master/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php;382bafa271ec4962$616)
and will fail because the commit doesn't exist in the DB.
  (https://secure.phabricator.com/diffusion/P/browse/master/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php;382bafa271ec4962$341)
After the exception is thrown, all the commits just detected are not marked as 'seenOnBranches', so they won't be marked as closed by the CommitMessageParserWorker.

This also explains the weird behavior where the parsing of the commits is always paused for 15 minutes and then gets started again. The parsing pauses because the process listed at https://secure.phabricator.com/P427 takes quite some time to finish before it can record tha tasks. The parsing happens all the time because the process will be executed again and again.

A better fix might be to put all the missing commits in the repository_badcommit table and check against it.

Test Plan: will run 'discover.php FBCODE' to test it.

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: nh, vrana, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2846
2012-06-23 09:47:20 -07:00
vrana
43121f6858 Provide a Conduit method to get inline comments
Summary:
See D2855 for usage.

This has a drawback that inlines without a comment (synthetic comments) are not attached anywhere.
I don't like adding more and more methods so I've chosen this solution.
Plus comments and inline comments are often useful together.

Test Plan: Called the method on a revision with inline comments.

Reviewers: epriestley

Reviewed By: epriestley

CC: vii, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2844
2012-06-23 09:34:14 -07:00
vrana
382bafa271 Don't treat links to redyoutube.com as YouTube
Test Plan: `http://redyoutube.com/?v=1`

Reviewers: tuomaspelkonen, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2842
2012-06-22 21:20:52 -07:00
vrana
d7b8bc892b Display revision number in history
Test Plan:
Displayed repository.
Displayed repository history.
Wondered that we actually have bunch of commits without a revision.
Displayed blame.
Didn't display merge commit.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2840
2012-06-22 17:10:45 -07:00
Jason Ge
969dcc5bae handle svn and hg for auto-close
Summary: in svn and hg (for now), no branch used.

Test Plan: will test live

Reviewers: epriestley

CC: nh, vrana, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2839
2012-06-22 17:05:06 -07:00
epriestley
c42f767e37 Destroy changeset parse cache when destroying a revision
Summary: When we delete a DifferentialChangeset, also delete the cache if it exists.

Test Plan:
Destroyed a revision, verified cache was destroyed. See marked line below.

        $ ./scripts/differential/destroy_revision.php D1 --trace
        >>> [0] <connect>
        <<< [0] <connect> 1,324 us
        >>> [1] <query> SELECT * FROM `differential_revision` WHERE `id` = 1
        <<< [1] <query> 517 us

            Really destroy 'D1: asdb' forever? [y/N] y

        >>> [2] <connect>
        <<< [2] <connect> 633 us
        >>> [3] <query> START TRANSACTION
        <<< [3] <query> 183 us
        >>> [4] <query> SELECT * FROM `differential_diff` WHERE revisionID = 1
        <<< [4] <query> 560 us
        >>> [5] <query> SAVEPOINT Aphront_Savepoint_1
        <<< [5] <query> 197 us
        >>> [6] <query> SELECT * FROM `differential_changeset` WHERE diffID = 3
        <<< [6] <query> 672 us
        >>> [7] <query> SAVEPOINT Aphront_Savepoint_2
        <<< [7] <query> 188 us
        >>> [8] <query> SELECT * FROM `differential_hunk` WHERE changesetID = 6
        <<< [8] <query> 946 us
        >>> [9] <query> DELETE FROM `differential_hunk` WHERE `id` = 6
        <<< [9] <query> 335 us
  ****  >>> [10] <query> DELETE FROM `differential_changeset_parse_cache` WHERE id = 6
        <<< [10] <query> 1,464 us
        >>> [11] <query> DELETE FROM `differential_changeset` WHERE `id` = 6
        <<< [11] <query> 313 us
        >>> [12] <query> SAVEPOINT Aphront_Savepoint_2
        <<< [12] <query> 151 us
        >>> [13] <query> SELECT * FROM `differential_hunk` WHERE changesetID = 7
        <<< [13] <query> 226 us
        >>> [14] <query> DELETE FROM `differential_hunk` WHERE `id` = 7
        <<< [14] <query> 164 us
        >>> [15] <query> DELETE FROM `differential_changeset_parse_cache` WHERE id = 7
        <<< [15] <query> 318 us
        >>> [16] <query> DELETE FROM `differential_changeset` WHERE `id` = 7
        <<< [16] <query> 189 us
        >>> [17] <query> SELECT * FROM `differential_diffproperty` WHERE diffID = 3
        <<< [17] <query> 500 us
        >>> [18] <query> DELETE FROM `differential_diffproperty` WHERE `id` = 1
        <<< [18] <query> 179 us
        >>> [19] <query> DELETE FROM `differential_diff` WHERE `id` = 3
        <<< [19] <query> 211 us
        >>> [20] <query> DELETE FROM `differential_relationship` WHERE revisionID = 1
        <<< [20] <query> 391 us
        >>> [21] <query> DELETE FROM `differential_commit` WHERE revisionID = 1
        <<< [21] <query> 397 us
        >>> [22] <query> SELECT * FROM `differential_comment` WHERE revisionID = 1
        <<< [22] <query> 448 us
        >>> [23] <query> DELETE FROM `differential_comment` WHERE `id` = 1
        <<< [23] <query> 212 us
        >>> [24] <query> DELETE FROM `differential_comment` WHERE `id` = 2
        <<< [24] <query> 160 us
        >>> [25] <query> SELECT * FROM `differential_inlinecomment` WHERE revisionID = 1
        <<< [25] <query> 549 us
        >>> [26] <query> SELECT * FROM `differential_auxiliaryfield` WHERE revisionPHID = 'PHID-DREV-orsh7alzcj764ubv2f34'
        <<< [26] <query> 531 us
        >>> [27] <query> SELECT * FROM `differential_affectedpath` WHERE revisionID = 1
        <<< [27] <query> 5,676 us
        >>> [28] <query> DELETE FROM `differential_revision` WHERE `id` = 1
        <<< [28] <query> 442 us
        >>> [29] <query> COMMIT
        <<< [29] <query> 324 us
        OK, destroyed revision.

Reviewers: csilvers, btrahan

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2835
2012-06-22 16:16:44 -07:00
vrana
e3eb8d55c5 Mark date and time format translatable
Summary:
This allows translations to specify custom date and time format.
We will need to call `setlocale()` or build system independent on it at some point but I guess it is good for now.

Test Plan: Translated 'D, M j, g:i A', saw it in diff.

Reviewers: btrahan, epriestley

Reviewed By: btrahan

CC: aran, Korvin

Maniphest Tasks: T1139, T1321

Differential Revision: https://secure.phabricator.com/D2809
2012-06-22 15:29:48 -07:00
vrana
a7a978d1f3 Link revision from blame
Summary: I need this much more than commit but it can be useful too.

Test Plan: Displayed blame, clicked on the link.

Reviewers: epriestley

Reviewed By: epriestley

CC: john, aran, Korvin, phunt

Differential Revision: https://secure.phabricator.com/D2820
2012-06-22 12:48:46 -07:00
vrana
d6ec905fe3 Allow overriding translations without creating PhabricatorTranslation
Test Plan: Overridden '%d Detail(s)', verified that it was used.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2815
2012-06-22 11:58:06 -07:00
vrana
c5c0324e1b Send several X-Differential-CC headers
Summary: People want to create filters checking if they are in CC which is almost impossible with multiplexing in Outlook.

Test Plan: Sent e-mail with multiple CCs, verified headers.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2829
2012-06-22 11:10:23 -07:00
epriestley
c5e504ff5e Add Z-index to notifications
Summary:
Currently, notifications aren't z-indexed so they can end up underneath some other elements:

{F13144}

Fix this so they float correctly.

Test Plan: {F13166}

Reviewers: jungejason, btrahan

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T1398

Differential Revision: https://secure.phabricator.com/D2834
2012-06-22 10:48:16 -07:00
Jason Ge
a75da4efac Revert "run record commit if its the first time seen"
Summary: This reverts commit 22e606d49e.

Test Plan: none

Reviewers: epriestley, nh

Reviewed By: nh

CC: nh, vrana, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2833
2012-06-21 20:26:07 -07:00
Jason Ge
22e606d49e run record commit if its the first time seen
Test Plan: test live.

Reviewers: nh

Reviewed By: nh

CC: epriestley, aran

Differential Revision: https://secure.phabricator.com/D2832
2012-06-21 19:47:25 -07:00
Jason Ge
06c976b738 check commit before calling its method
Summary: it's calling pull daemon's failure. This is actually Nick's fix.

Test Plan: Nick already manually ran it on daemon machine.

Reviewers: vrana, nh

Reviewed By: vrana

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2828
2012-06-21 18:36:31 -07:00
dschleimer
9444bfa861 Document Mercurial base commit DSL support
Summary: documentation changes for D2822

Test Plan:
Generated docs, but failed to read them because no one at Facebook
remembers how to get docs working in our sandbox.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1233

Differential Revision: https://secure.phabricator.com/D2827
2012-06-21 18:28:16 -07:00
vrana
ecf92ac00f Support petabytes
Test Plan: New test.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2821
2012-06-21 18:07:48 -07:00
vrana
e902875339 Use official PHP mirror 2012-06-21 10:48:37 -07:00
epriestley
083cd5fd9d Add a "Mark All Read" button for notifications
Summary: Adds a button to mark everything as read.

Test Plan: Clicked button.

Reviewers: btrahan, jungejason, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T974

Differential Revision: https://secure.phabricator.com/D2812
2012-06-20 13:51:19 -07:00
epriestley
5e184ee593 Improve debug support for notifications
Summary: Add a `notification.debug` setting that shows debug info in the browser. Also improve some logging/error handling stuff and fix a bug with host names.

Test Plan: {F13098}

Reviewers: jungejason, btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T944

Differential Revision: https://secure.phabricator.com/D2810
2012-06-20 13:20:47 -07:00
epriestley
addc2c8764 Determine unread count correctly when rendering notification panel
Summary:
Currently, when rendering the panel we count the number of unread notifications in the last 15, but this means we can never render a number larger than 15. If the user has more unread notifications than that, or unread notifications older than the most recent 15, there will be a flash of the higher number and then it will update to the lower number afterward.

Instead, count all unread notifications. This uses the same method used to render both numbers.

Test Plan: Loaded a page, checked the menu, nothing exploded.

Reviewers: btrahan, jungejason, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T974

Differential Revision: https://secure.phabricator.com/D2811
2012-06-20 13:20:41 -07:00
epriestley
a4e2eb3d8c Add a "differential.createrawdiff" Conduit method
Summary:
"differential.creatediff" requires a mostly-parsed diff, but there's no reason we can't make DifferentialDiffs out of raw diffs.

This mainly serves to lower the adoption barrier if getting "arc" distributed is too much of a hassle.

Test Plan: Made a diff out of a raw block of diff text.

Reviewers: ffx, vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2751
2012-06-20 12:35:04 -07:00
Bob Trahan
921b5dada1 Enable image macros on the wiki
Summary: D2230 did the heavy lifting spoken of in the comment this diff deletes. Also remove 'macro' => false.

Test Plan: Added an image macro to a wiki document and it showed up

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1023

Differential Revision: https://secure.phabricator.com/D2668
2012-06-20 12:06:35 -07:00
vrana
21656f0971 Include only whole days in Differential stats
Summary: This prevents "8 active days" for last week.

Test Plan: `strtotime('1 week ago 24:00')`

Reviewers: john, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2794
2012-06-20 11:57:26 -07:00
Jason Ge
3559d71e3a fix typo 2012-06-20 10:03:21 -07:00
epriestley
8ddfdb26e9 Publish feed stories about commits
Summary: When stuff gets committed, publish feed stories about it.

Test Plan: {F13061}

Reviewers: jungejason, vrana, voldern

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T1322

Differential Revision: https://secure.phabricator.com/D2805
2012-06-20 06:03:44 -07:00
epriestley
74e39bfe41 Provide basic documentation about Aphlict
Summary: Hopefully this is helpful? Also fixed a thing that wasn't using config.

Test Plan: Read documentation. Sent myself a notification over the server.

Reviewers: jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T944

Differential Revision: https://secure.phabricator.com/D2804
2012-06-19 18:37:58 -07:00
Jason Ge
0c088a695c Fix dropdown for file whose diff is not displayable
Summary: The current behavior is that, when the dropdown menu is clicked for a binary file, it navigates to the current revision page directly without showing the dropdown meu. The fix is to do nothing when there is no file displayed.

Test Plan:
- checked a diff with binary file
- checked a diff with normal files

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: epriestley, aran

Differential Revision: https://secure.phabricator.com/D2801
2012-06-19 18:28:28 -07:00
epriestley
d277908842 Minor, avoid branch loop nonsense for repositories not set to autoclose. See IRC. 2012-06-19 15:05:22 -07:00
vrana
606ca2831c Use pht() for plural 2012-06-19 15:04:36 -07:00
Nick Harper
4b10707d4d Add user.enable conduit method
Test Plan: enabled a disabled user

Reviewers: vrana, epriestley

Reviewed By: vrana

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2800
2012-06-19 14:36:08 -07:00
vrana
bf10eaf81c Wrap Differential property changes 2012-06-19 14:15:40 -07:00
epriestley
e48830eecb Don't rediscover an entire branch when a new commit appears at HEAD
Summary:
The autoclose logic is currently doing a little too much work. We want to parse each commit at most twice:

  # When it first appears in the repository.
  # When it first appears on an autoclose branch.

These two events might not be distinct (i.e., it might first appear on an autoclose branch).

Currently, to discover commits initially appearing on autoclose branches, we check each branch, determine if it's an autoclose branch or not, and determine if the HEAD is already a known commit on an autoclose branch. This is correct so far, and allows us to ignore branches which either haven't changed or have commits at HEAD which we've already examined.

However, if an autoclose branch has a new commit, we start working backward through it. Prior to this patch, we only stop when we hit commits that we've already discovered lie on this branch. If the branch is new, none of the commits will be discovered on it (they're discovered in general, and likely discovered on other autoclose branches, but not discovered on this branch), so we'll parse all the way back to the root.

Instead, we want to stop when we hit commits that we've already discovered on //any// autoclose branch.

So do that.

Test Plan: Pushed a new branch, then pushed a new commit at HEAD. Ran discovery, verified we rediscovered only 1 commit, not every commit in history.

Reviewers: vrana, jungejason, aurelijus

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1389

Differential Revision: https://secure.phabricator.com/D2798
2012-06-19 13:47:01 -07:00
epriestley
fabe52335e Add --verbose support to phd
Summary:
Support the `--verbose` flag added in D2795 in `phd`. See T1389.

Also simplify argument generation a little bit.

Test Plan: Ran "nice" daemon with debug,  daemon + verbose, daemon + no verbose.

Reviewers: vrana, jungejason, edward, aurelijus

Reviewed By: aurelijus

CC: aran

Maniphest Tasks: T1389

Differential Revision: https://secure.phabricator.com/D2797
2012-06-19 12:56:41 -07:00
epriestley
d4b6b095cb Provide a script to completely destroy revisions
Summary:
Someone may or may not have accidentally uploaded secrets to Differential. Provide an administrative mechanism to permanently destroy a revision.

Also fix some of the transaction handling code.

Test Plan:
  $ ./scripts/differential/destroy_revision.php --trace D1
  >>> [0] <connect>
  <<< [0] <connect> 1,060 us
  >>> [1] <query> SELECT * FROM `differential_revision` WHERE `id` = 1
  <<< [1] <query> 473 us

      Really destroy 'D1: asdbas' forever? [y/N] y

  >>> [2] <connect>
  <<< [2] <connect> 628 us
  >>> [3] <query> START TRANSACTION
  <<< [3] <query> 190 us
  >>> [4] <query> SELECT * FROM `differential_diff` WHERE revisionID = 1
  <<< [4] <query> 510 us
  >>> [5] <query> SAVEPOINT Aphront_Savepoint_1
  <<< [5] <query> 122 us
  >>> [6] <query> SELECT * FROM `differential_changeset` WHERE diffID = 1
  <<< [6] <query> 307 us
  >>> [7] <query> SAVEPOINT Aphront_Savepoint_2
  <<< [7] <query> 241 us
  >>> [8] <query> SELECT * FROM `differential_hunk` WHERE changesetID = 1
  <<< [8] <query> 212 us
  >>> [9] <query> DELETE FROM `differential_hunk` WHERE `id` = 1
  <<< [9] <query> 216 us
  >>> [10] <query> DELETE FROM `differential_changeset` WHERE `id` = 1
  <<< [10] <query> 154 us
  >>> [11] <query> SAVEPOINT Aphront_Savepoint_2
  <<< [11] <query> 118 us
  >>> [12] <query> SELECT * FROM `differential_hunk` WHERE changesetID = 2
  <<< [12] <query> 194 us
  >>> [13] <query> DELETE FROM `differential_hunk` WHERE `id` = 2
  <<< [13] <query> 179 us
  >>> [14] <query> DELETE FROM `differential_changeset` WHERE `id` = 2
  <<< [14] <query> 163 us
  >>> [15] <query> SAVEPOINT Aphront_Savepoint_2
  <<< [15] <query> 105 us
  >>> [16] <query> SELECT * FROM `differential_hunk` WHERE changesetID = 3
  <<< [16] <query> 211 us
  >>> [17] <query> DELETE FROM `differential_hunk` WHERE `id` = 3
  <<< [17] <query> 159 us
  >>> [18] <query> DELETE FROM `differential_changeset` WHERE `id` = 3
  <<< [18] <query> 152 us
  >>> [19] <query> SAVEPOINT Aphront_Savepoint_2
  <<< [19] <query> 124 us
  >>> [20] <query> SELECT * FROM `differential_hunk` WHERE changesetID = 4
  <<< [20] <query> 191 us
  >>> [21] <query> DELETE FROM `differential_hunk` WHERE `id` = 4
  <<< [21] <query> 155 us
  >>> [22] <query> DELETE FROM `differential_changeset` WHERE `id` = 4
  <<< [22] <query> 149 us
  >>> [23] <query> SELECT * FROM `differential_diffproperty` WHERE diffID = 1
  <<< [23] <query> 242 us
  >>> [24] <query> DELETE FROM `differential_diffproperty` WHERE `id` = 1
  <<< [24] <query> 196 us
  >>> [25] <query> DELETE FROM `differential_diff` WHERE `id` = 1
  <<< [25] <query> 169 us
  >>> [26] <query> DELETE FROM `differential_relationship` WHERE revisionID = 1
  <<< [26] <query> 178 us
  >>> [27] <query> DELETE FROM `differential_commit` WHERE revisionID = 1
  <<< [27] <query> 164 us
  >>> [28] <query> SELECT * FROM `differential_comment` WHERE revisionID = 1
  <<< [28] <query> 221 us
  >>> [29] <query> DELETE FROM `differential_comment` WHERE `id` = 1
  <<< [29] <query> 172 us
  >>> [30] <query> SELECT * FROM `differential_inlinecomment` WHERE revisionID = 1
  <<< [30] <query> 296 us
  >>> [31] <query> SELECT * FROM `differential_auxiliaryfield` WHERE revisionPHID = 'PHID-DREV-ooky7ozqukpmwget32oc'
  <<< [31] <query> 308 us
  >>> [32] <query> SELECT * FROM `differential_affectedpath` WHERE revisionID = 1
  <<< [32] <query> 4,173 us
  >>> [33] <query> DELETE FROM `differential_revision` WHERE `id` = 1
  <<< [33] <query> 231 us
  >>> [34] <query> COMMIT
  <<< [34] <query> 686 us
  OK, destroyed revision.

Reviewers: csilvers, vrana, jungejason

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2796
2012-06-19 11:52:50 -07:00
Edward Speyer
83a5f1479d Arrayish fields should always return arrays
Summary:
Array-like fields in HerladCommitAdapter should always return things of
type array.  If they return array(...) or null, then array_*() functions
sporadically break.

Test Plan:
Ran a PhabricatorRepositoryCommitHeraldWorker for a troublesome commit.
Failed before; fails no more.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, Korvin, vdt

Maniphest Tasks: T1385

Differential Revision: https://secure.phabricator.com/D2793
2012-06-19 11:18:38 -07:00
vrana
c53ed032c9 Link libphutil docs 2012-06-18 19:09:27 -07:00
vrana
8a8a48cc8f Fix displaying of inlines related both to visible and hidden diff
Summary: I tried also filling the column by empty space but this looks better.

Test Plan: Displayed a comment both with visible and hidden inlines.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1216

Differential Revision: https://secure.phabricator.com/D2789
2012-06-18 18:11:24 -07:00
vrana
c762050b7c Get rid of file_get_contents($uri)
Summary: It requires `allow_url_fopen` which we don't check in setup and our installation is about to disable it.

Test Plan:
Login with OAuth.
/oauth/facebook/diagnose/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2787
2012-06-18 17:45:45 -07:00
mkedia
a5b5128be9 Add path support to differential.query
Summary:
Allows clients to select open differential revisions that
affect a set of paths.

Test Plan:
http://phabricator.mkedia.dev2975.facebook.com/conduit/method/differential.query/ with

paths: [["E", "tfb/trunk/www/html/intern/css/tabloid"]]
status: "status-open"

(internal facebook repo, obvi)

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: vrana, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2788
2012-06-18 17:43:22 -07:00
vrana
2a09563533 Always use HTTPSFuture in elasticsearch.
Test Plan: Search elasticly.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2685
2012-06-18 15:16:46 -07:00
epriestley
76df7970ec Show notification text for non-reload notifications
Summary:
Show all notifications, but make the non-reload ones transient.

Depends on D2781, D2780

Test Plan: {F12986}

Reviewers: jungejason, vrana

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T944

Differential Revision: https://secure.phabricator.com/D2782
2012-06-18 14:08:10 -07:00
epriestley
7a4ca85d67 Show only one "reload" popup, use 'alert' style, click to reload
Summary: Use the features from D2758.

Test Plan: Updated T1 with two browser windows pointing at it, verified reload appeared, only one reload, and it appeared with 'alert' style.

Reviewers: jungejason, vrana

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T944

Differential Revision: https://secure.phabricator.com/D2781
2012-06-18 14:07:45 -07:00
epriestley
8cd0997965 Add a list of all notifications
Summary: Add a "View All Notifications" link and page.

Test Plan: Viewed all notifications

Reviewers: jungejason, vrana

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T974

Differential Revision: https://secure.phabricator.com/D2780
2012-06-18 14:07:38 -07:00
vrana
656c82f9b8 Fix principal error in makeChangesWithContext()
Summary: `array_fill()`, contrary to `range()`, doesn't accept the last element but the number of elements.

Test Plan: Reparsed commit not changed after the last diff but rebased which was previously reported as changed.

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2783
2012-06-18 13:52:45 -07:00
vrana
e84f9f9ec9 Send Differential e-mails in user's language
Summary:
Works this way:

- Select users' language with multiplexing.
- Select default language otherwise (it can be different from current user's language).
- Build body and subject for each user individually.
- Set the original language after sending the mails.

Test Plan:
- Comment on a diff of user with custom translation.
- Set default to a custom translation. Comment on a diff of user with default translation.
- Set default to a default translation. Comment on a diff of user with default translation.

Repeat with/without multiplexing.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2774
2012-06-18 12:41:09 -07:00
Jason Ge
112acf11cf Enable 'jumping to toc' on diffusion commit page
Summary:
Diffusion page is sharing the keyboard shortcuts code with
Differential page. But since the toc (Changes) panel doesn't have id
'differential-review-toc', the 'jumping to toc' doesn't work. The fix is
to add the ID. I don't like adding 'Differential' to the Diffusion page.
Later we should refactor the code to extract the shared components out of Differential.

Test Plan:
verified that 't' worked on the diffusion commit page.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: hwang, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2500
2012-06-18 11:09:25 -07:00
20after4
fea67c9858 Partial fix for owners bug where a package's repository doesn't exist.
Summary: Avoid requesting a non-existent repository...

Test Plan: Delete a repo that has an associated owners package. Then verify that the owners list page no longer throws an exception.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1372

Differential Revision: https://secure.phabricator.com/D2776
2012-06-18 06:04:36 -07:00
vrana
6b8295d100 Document internationalization
Test Plan: Generated docs.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2779
2012-06-17 13:22:55 -07:00
epriestley
1053a50f67 Add multiple notifications and notification types
Summary:
  - Allow more than one notification to be shown.
  - Allow notifications to be customized with extra classes.

Test Plan: {F12776}

Reviewers: allenjohnashton, ddfisher, keebuhm, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T944

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

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

Reviewers: allenjohnashton, ddfisher, keebuhm, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T944

Differential Revision: https://secure.phabricator.com/D2756
2012-06-17 11:35:18 -07:00
epriestley
cdd3683ed7 Allow applications to call Conduit directly
Summary:
Sorry this took so long, had a bunch of stuff going on today.

Separate the actual core part of making conduit calls from the controller, so the application can make conduit calls without needing to invoke HTTP or redo auth. Generally, this lets us build more parts of the application on top of Conduit, as appropriate.

This diff can be simplified, but I wanted to unblock you guys first. I'll followup with a cleanup patch once I have a chance.

Test Plan: Ran unit tests, ran calls from the conduit API console, and ran calls over arc.

Reviewers: nodren, 20after4, btrahan, vrana

Reviewed By: 20after4

CC: aran, svemir

Maniphest Tasks: T945

Differential Revision: https://secure.phabricator.com/D2718
2012-06-17 08:47:09 -07:00
epriestley
3e18acef9f Minor, use self:: instead of static:: for PHP 5.2 compatibility. See https://github.com/facebook/phabricator/issues/133 2012-06-17 08:46:22 -07:00
Jonathan Lomas
92fac31215 Fixes broken search if no task ids created by D2771.
Summary: I forgot to handle the case where there was no task ids entered in D2771, but this corrects that issue.

Test Plan: Search with no task ids...

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1365

Differential Revision: https://secure.phabricator.com/D2773
2012-06-16 06:27:35 -07:00
vrana
5b6713f3c1 Allow revision action links using VS diff
Test Plan: Created action link using VS diff, selected VS diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2770
2012-06-15 20:12:50 -07:00
Jonathan Lomas
7629d44175 Strip non-integers out of task id entries
Test Plan: search in task ids for `t123, 456, pickles` (assuming 123 and 456 exist), and you will see 123 and 456 listed.  This is done in the buildQueryFromRequest function, so it would process a hand-written GET string just fine too.

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T1365

Differential Revision: https://secure.phabricator.com/D2771
2012-06-15 17:38:49 -07:00
epriestley
957f9e2ec5 Add an administrative tool for deleting users
Summary:
Allow administrators to delete accounts if they jump through enough hoops.

Also remove bogus caption about usernames being uneditable since we let admins edit those too now.

Test Plan: Tried to delete myself. Deleted a non-myself user.

Reviewers: csilvers, vrana

Reviewed By: csilvers

CC: aran

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2767
2012-06-15 17:02:20 -07:00
epriestley
31fcd78c76 Add "verified" or "unverified" to role output in user.query
Summary: See discussion in rP2f138d0501887fd0aca0f8536176f092880f662c.

Test Plan: Ran `user.query` on verified and unverified users.

Reviewers: csilvers, vrana

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2768
2012-06-15 17:00:26 -07:00
epriestley
14648d6d7a Provide more informative messages when autoclosing revisions
Summary: This has been a point of some confusion, make the messages more explicit.

Test Plan:
Added var_dump() stuff and ran on some commits:

  $ ./scripts/repository/reparse.php --message rP9fc54f4dfb61f7338cb1cfe819bc72d2a3404264
  Running 'PhabricatorRepositoryGitCommitMessageParserWorker'...
  string(58) "Closed by commit rP9fc54f4dfb61 (authored by @epriestley)."
  $ ./scripts/repository/reparse.php --message rP444c634b6c6612fc7b36ddffab8023ef67372ab9
  Running 'PhabricatorRepositoryGitCommitMessageParserWorker'...
  string(83) "Closed by commit rP444c634b6c66 (authored by Ben Rogers, committed by @epriestley)."
  $ ./scripts/repository/reparse.php --message rP22d12fe499e3ecb62392397f2ac2a91768c974aa
  Running 'PhabricatorRepositoryGitCommitMessageParserWorker'...
  string(52) "Closed by commit rP22d12fe499e3 (authored by vrana)."
  $ ./scripts/repository/reparse.php --message rPe51958159483cd0acf00adcff51edf8717b4a23b
  Running 'PhabricatorRepositoryGitCommitMessageParserWorker'...
  string(85) "Closed by commit rPe51958159483 (authored by David Fisher, committed by @epriestley)."

Reviewers: csilvers, vrana

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2765
2012-06-15 17:00:08 -07:00
vrana
2f85be0243 Add 'description' to Diff dict 2012-06-15 16:16:03 -07:00
Evan Priestley
2ffc924fb5 Merge pull request #131 from floatingLomas/1305-filter-tasks-by-name
Add fulltext search to Maniphest custom query screen.
2012-06-15 16:10:32 -07:00
Evan Priestley
ee4cedce7f Merge pull request #130 from floatingLomas/1364-space-separated-task-ids
Allow use of any whitespace or a comma as a task ID delimiter in custom ...
2012-06-15 16:08:31 -07:00
Jonathan Lomas
4131fd34d3 Allow use of any whitespace or a comma as a task ID delimiter in custom query. 2012-06-15 15:54:09 -07:00
Jonathan Lomas
60da804d3d Add query limit of PHP_INT_MAX and handle the case where there are no fulltext results. 2012-06-15 15:42:31 -07:00
vrana
8a9da4b8d0 Add 'creationMethod' to Diff dict
Summary: We need it.

Test Plan: Ran 'differential.getdiff', saw it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2766
2012-06-15 15:30:19 -07:00
vrana
e9779b2db1 Bring back desciption to Maniphest attachment 2012-06-15 15:25:18 -07:00
vrana
ba9b4f5b66 Optimize SQL queries in differential.getdiff
Test Plan:
Verified that the method retuns the same output.
Verified the number of SQL queries.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2764
2012-06-15 15:15:23 -07:00
Jonathan Lomas
f91fc70b14 Add fulltext search to Maniphest custom query screen.
Summary: Allow fulltext search on custom query screen using the same fulltext search as the search page.

Test Plan: Enter search terms - with and without additional filters - see the expected results.  Don't enter search terms - with or without additional filters - and see the expected results.

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T1305

Differential Revision: https://secure.phabricator.com/D2763
2012-06-15 14:57:15 -07:00
Ben Rogers
444c634b6c Allows branches to appear in diffusion with out triggering closing things from the commit message
Summary: Implemented it how it was suggested in ticket comments

Test Plan: create a revision in a branch, push that branch up, verify it's visible in diffusion and also that revision is not closed, then merge and push to master, verify that revision closed

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, 20after4

Maniphest Tasks: T1210

Differential Revision: https://secure.phabricator.com/D2706
2012-06-15 14:10:15 -07:00
epriestley
904d44492a Document base commit DSL
Summary: Documentation for D2748.

Test Plan: Generated and read docs.

Reviewers: dschleimer, zeeg

Reviewed By: dschleimer

CC: aran, csilvers

Maniphest Tasks: T1233

Differential Revision: https://secure.phabricator.com/D2750
2012-06-15 14:02:19 -07:00
vrana
22d12fe499 Mark 'closed' as translatable
Summary:
This is to allow conservative people to bring back the old manners.

NOTE: It doesn't mark all occurrences but I think that's good enough. We will eventually mark them all.

Test Plan: Translated 'closed', displayed closed revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, asukhachev

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2760
2012-06-15 11:52:03 -07:00
vrana
9e3eb37a90 Use array_mergev() 2012-06-14 20:40:02 -07:00
vrana
325c2077ba Allow extending English translation
Test Plan: Displayed home.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2759
2012-06-14 19:27:29 -07:00
vrana
48ebcf0679 Allow user override translation and implement PhutilPerson
Test Plan:
Altered database.
Wrote a custom translation and selected it in preferences.
Verified that the text is custom translated.
Set language back to default.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2757
2012-06-14 18:33:00 -07:00
vrana
ec819c068c Delete methods unused since D2664 2012-06-14 18:23:47 -07:00
vrana
0acb7734cd Use pht()
Summary:
This is the first step in Phabricator internationalization.
It adds a translation selector and calls it at startup.
Installations can add custom selectors to override some texts.
We can add official translations in future.

Next step is to allow user to choose his translation which will override the global one.

This is currently used only for English plurals.

Test Plan: Displayed a diff with unit test error, verified that it says 'Detail' or 'Details' and not 'Detail(s)'.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2753
2012-06-14 16:25:20 -07:00
epriestley
357305507c Fix nginx configuration guide
Summary: We currently match "any PHP file path", which is wrong, since it will match things like `/diffusion/path/to/some/source/code.php`. Match only "index.php".

Test Plan: This is the config secure.phabricator.com / local / etc run, we just had out of date documentation.

Reviewers: Korvin, vrana, btrahan, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T1323

Differential Revision: https://secure.phabricator.com/D2754
2012-06-14 15:38:55 -07:00
vrana
892a2d1b61 Make Thread-Topic human readable
Summary:
Some e-mail clients display this header and it needs to be constant.

This is somehow involved but I doubt that there is a simpler solution.

Test Plan:
Applied SQL patch.
Commented on revision, commented on commit, changed package.
Verified that the `Thread-Topic` has constant and human readable value.

Reviewers: epriestley

Reviewed By: epriestley

CC: ola, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2745
2012-06-14 11:36:34 -07:00
Svemir Brkic
7463f9b3a9 Only get phid if project exists
Summary: If a project phriction page already exists, but object does not, avoid a fatal error.

Test Plan: See https://github.com/facebook/phabricator/issues/123

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2749
2012-06-14 10:43:17 -07:00
epriestley
2d7705ea30 Apply AMAZING DESIGN SKILLS to notification menu
Summary:
I am a fancy designer!

{F12665} {F12666}

Test Plan: Opened/closed menu. Viewed with-notification-count and without-notification count states.

Reviewers: allenjohnashton, ddfisher, keebuhm

Reviewed By: ddfisher

CC: aran, chad, joe

Maniphest Tasks: T974

Differential Revision: https://secure.phabricator.com/D2735
2012-06-14 06:13:53 -07:00
epriestley
86040227b0 Improve Aphlict server
Summary:
  - Move to port 22280 by default.
  - Warn when running as non-root.
  - Allow subscription and publish/admin ports to be configured.
  - Allow server to drop root after binding to 843.
  - Allow log path to be configured.
  - Add /status/ admin URI which shows server status.
  - Return HTTP 400 Bad Request for other requests, instead of hanging.
  - Minor formatting cleanup.

Test Plan:
Ran without root:

  $ node aphlict_server.js

...got a good error message. Ran with --user:

  $ sudo node aphlict_server.js --user=epriestley

...verified server dropped permissions. Ran with --port / --admin. Hit /status/ with GET, got status. Hit other URLs with GET, got 400.

Reviewers: allenjohnashton, ddfisher, keebuhm

Reviewed By: ddfisher

CC: aran

Differential Revision: https://secure.phabricator.com/D2737
2012-06-14 06:12:54 -07:00
epriestley
9a4243b4b3 Minor, fix MailImplementationTestAdapter constructor. See D2706. 2012-06-14 05:50:15 -07:00
vrana
56d316a208 Update Celerity map 2012-06-13 22:47:46 -07:00
David Fisher
e519581594 "Please Refresh" Notifications
Summary:
Based off D2704. Adds humane.js and a bit of plumbing. Currently does
not seem to load notification.css (which causes notifications not to display)
for reasons entirely opaque to me.

Test Plan:
tried locally. currently works except for the actual display due to
css loading difficulties

Reviewers: epriestley

Reviewed By: epriestley

CC: allenjohnashton, keebuhm, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2705
2012-06-13 17:28:58 -07:00
Alan Huang
d5e61f5250 Let the viewer collapse/expand individual files in a diff.
Summary:
Added a dropdown menu button and the keyboard shortcut 'h' to the
web diff view. These hide or show the annotated code display.

Test Plan:
Viewed an example diff that changed a large number of source files
and played around with keyboard shortcuts. Everything seemed to
work as expected.

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D2714
2012-06-13 16:52:42 -07:00
epriestley
d119b051e8 Add a basic notification UI element
Summary: This is //extremely// basic but dead simple and should cover us for v1, I think. Let me know what features you need.

Test Plan: Used UI example page.

Reviewers: allenjohnashton, ddfisher, keebuhm

Reviewed By: ddfisher

CC: aran, ender

Maniphest Tasks: T944

Differential Revision: https://secure.phabricator.com/D2732
2012-06-13 15:00:24 -07:00
vrana
0cdc4e008d Use textContent instead of innerHTML for linking symbols
Summary: Go to https://secure.phabricator.com/D2646#59cc9d49 and click on `array_mergev` to repro.

Test Plan: Checked if the property exists in DOM inspector.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2742
2012-06-13 14:45:45 -07:00
epriestley
4ac1fa1432 Don't log lost connection retries
Summary:
In D1460, we added initial-connection retries and additional logging in response to network/router instability at Facebook.

One of these logs occurs normally, though, when daemons get disconnected by 'wait_timeout' settings. This is noisy for a lot of installs and a few people have asked me about it.

I assume the situation has cleared up a bit at Facebook, so we can remove the "business as usual" log and keep error logs quieter for general installs?

Test Plan: Weathered the support storm.

Reviewers: nh, vrana

Reviewed By: nh

CC: Makinde, aran

Differential Revision: https://secure.phabricator.com/D2736
2012-06-13 14:42:34 -07:00
epriestley
c868ec7d11 Extend ManiphestAction from ManiphestConstants, not PhrictionConstants
Summary: Bad copy/paste.

Test Plan: arc liberate

Reviewers: svemir, allenjohnashton

Reviewed By: svemir

CC: aran

Differential Revision: https://secure.phabricator.com/D2738
2012-06-13 14:41:43 -07:00
Espen Volden
726041584f Made it possible to login using LDAP
Summary: Made it possible to link and unlink LDAP accounts with  Phabricator accounts.

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

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, auduny, svemir

Maniphest Tasks: T742

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

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

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1348

Differential Revision: https://secure.phabricator.com/D2729
2012-06-13 08:39:02 -07:00
vrana
371419344a Revert D2542
Summary: Not required after D2730.

Test Plan: `arc diff` with invalid reviewer in commit message.

Reviewers: nh

Reviewed By: nh

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2733
2012-06-12 17:33:10 -07:00
vrana
3718e49f9c Display link to change since last diff in Differential update e-mail
Test Plan: Updated revision, clicked on the link.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2731
2012-06-12 15:23:02 -07:00
epriestley
06371bc2cd Upgrade to GitHub v3 API
Summary: GitHub dropped support for the v2 API today, which breaks login and registration. Use the v3 API instead.

Test Plan: Registered and logged in with GitHub. Verified process pulled email/photo/name/username correctly.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2726
2012-06-12 12:13:27 -07:00
vrana
80de8c93c9 Stabilize Thread-Topic
Summary: NOTE: This can break current ongoing conversations.

Test Plan: Commented on a revision and checked the header in the e-mail.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1340

Differential Revision: https://secure.phabricator.com/D2723
2012-06-12 10:58:41 -07:00
epriestley
375c921bb0 Minor, fix issue with defaults: wildcard and repeat default to array and may not have a different default. 2012-06-12 07:05:57 -07:00
epriestley
133a2a1ac3 Minor, update Celerity map. 2012-06-12 05:46:01 -07:00
David Fisher
655bec9eeb Fix strange header appearance bug
Summary:
Text in the header changes appearance shortly after page loads. This
is due - for some reason - to adding the flash object. Making the width and
height of the flash object 0 solves this.

Test Plan: viewed locally, problem gone

Reviewers: epriestley

Reviewed By: epriestley

CC: allenjohnashton, keebuhm, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2719
2012-06-12 05:35:10 -07:00
vrana
2e484e257d Fix lint errors found by Nemo
Summary:
See also:

- https://github.com/tpyo/amazon-s3-php-class/pull/33
- https://github.com/stripe/stripe-php/pull/13

Test Plan: Ran a script analyzing sources by HPHP.

Reviewers: btrahan, jungejason, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2713
2012-06-11 19:09:42 -07:00
vrana
2793828795 Refactor setting e-mail subjects
Summary:
It seems that Outlook and Mail.app mostly ignores the threading headers and thread primarily by subject.
They are also very picky about the Re: part in the header.
I guess that's because users of these clients often hit Reply when they want to create a new message to the sender of an e-mail.

We need both of these applications to work with the same setting because we don't use multiplexing to prevent sending multiple e-mails to people in lists.
I also believe that the default behavior should just work in most setups.

I've tried several different combinations of putting "Re:" and none of them seems to always work in both clients.

This diff at least adds more abstraction to the code which should prevent copy/paste errors (two fixed by this diff!).

Test Plan: Sent several e-mails with varying subject, verified that they look as before in Outlook and Mail.app.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2709
2012-06-11 19:07:21 -07:00
epriestley
e1f2f7f2d9 Minor, update celerity map. 2012-06-11 17:52:17 -07:00
David Fisher
f8f195b329 Make Notifications Realtime
Summary:
Adds the node.js Aphlict server, the flash Aphlict client, and some
supporting javascript. Built on top of - and requires - D2703 (which is still
in progress).  Will likely work with no modification on top of the final
version, though.

The node server is currently run with

   sudo node support/aphlict/server/aphlict_server.js

Test Plan: tested locally

Reviewers: epriestley

Reviewed By: epriestley

CC: allenjohnashton, keebuhm, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2704
2012-06-11 17:51:12 -07:00
David Fisher
2bade93b76 Notification dropdown fixes
Summary: dropdown now displays properly and without flicker

Test Plan: tested locally

Reviewers: epriestley

Reviewed By: epriestley

CC: allenjohnashton, keebuhm, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2708
2012-06-11 17:17:48 -07:00
Nick Harper
865680ad30 Don't throw an exception if the author of a tag can't be found
Summary:
It's possible to have a tag with no tagger (or git used to allow this),
so some tags (like 26791a8bcf0e6d33f43aef7682bdb555236d56de in the linux
kernel) were causing trouble.

Test Plan:
opened diffusion to a page where I was previously getting a red box complaining
about being unable to parse the output of git for-each-ref.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, vrana

Differential Revision: https://secure.phabricator.com/D2711
2012-06-11 16:53:52 -07:00
vrana
a0b57cefb7 Declare that revision was closed by committer, not author
Summary: D2703#13 is confusing - it looks like that @allenjohnashton took the action but it was @epriestley.

Test Plan: I don't have a repro so I tested this block standalone.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2707
2012-06-11 11:30:27 -07:00
vrana
dd2b027905 Fix doc 2012-06-11 10:23:48 -07:00
epriestley
e1d6fe5840 Minor, fix library map. 2012-06-11 09:53:26 -07:00
vrana
11b3c6a4d5 Display e-mail preferences even if user can't change them
Test Plan: Displayed e-mail preferences with and without multiplexing.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

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

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

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: keebuhm, ddfisher, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2703
2012-06-11 09:42:55 -07:00
David Fisher
a57f5d1562 fixed overlong differential feed story titles
Summary:
Previously, the comment and/or summary would be added to the title. This
is incorrect behavior.

Test Plan: observed change

Reviewers: epriestley

Reviewed By: epriestley

CC: allenjohnashton, keebuhm, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2701
2012-06-10 17:38:30 -07:00
John-Ashton Allen
1f3b058daa Implemented fallback for unimplemented notifications with silly message.
Summary:
Most notifications are not implemented, a fallback message should
be displayed for these cases.

Test Plan: Commented out renderNotificationView in PhabricatorFeedStoryManiphest and watched the notifications render.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: ddfisher, keebuhm, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2700
2012-06-08 20:25:00 -07:00
epriestley
b695558223 Minor, remove abstract method until subclasses implement it. 2012-06-08 19:47:02 -07:00
epriestley
4d6df7dc14 Minor, fix insertNotifications() for unmigrated callsites. See discussion in rP3a6ee79190b709d84ff79865d1c1d6234e5c102a. 2012-06-08 19:16:31 -07:00
Keebuhm Park
284d7b6a46 Minor fixes to maniphest feed/notification
Summary:
Added `renderNotificationView()` abstract function to `PhabricatorFeedStory` base class.
Fixed duplicate line in `PhabricatorFeedStoryManiphest` class.
Fixed spacing/formatting in `ManiphestTransactionEditor`.

Test Plan: No functional changes

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: allenjohnashton, ddfisher, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2698
2012-06-08 19:11:53 -07:00
Keebuhm Park
c67f45734d Notification implementation for Differential
Summary: The notification implementation has been extended to Differential. Appropriate changes have been made to the Differential editors and Differential feed story.

Test Plan: Tested out various actions available for Differential and confirmed that the notifications get delivered correctly and feed is generated.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: allenjohnashton, ddfisher, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2696
2012-06-08 18:45:40 -07:00
vrana
6352f0f034 Avoid resolving path in celerity resource map
Summary:
HPHP doesn't like resolved symlinks.

Also I like this code better.

Test Plan: Used and not used custom Celerity map.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2694
2012-06-08 17:20:52 -07:00
Keebuhm Park
be0e16ff7d Fixed losing full size feed for maniphest task creation
Summary: After D2571, feed for maniphest task creation was being generated non-full-size. Fixed this by properly getting the maniphest action from story data.

Test Plan: Feed seems to work fine and the feed for task creation is full-sized.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: allenjohnashton, ddfisher, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2691
2012-06-08 14:31:10 -07:00
Keebuhm Park
d90a6cfbd1 Move files in notification one level up.
Summary: Moved the files in notification one level up to match the rest of code base. And ran arc liberate.

Test Plan: Made sure __phutil_library_map__ points to the right files.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: allenjohnashton, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2688
2012-06-08 13:00:42 -07:00
Keebuhm Park
207f101aee SQL patch for notification
Summary: Added `PhabricatorBuiltinPatchList` entry so that "storage upgrade" will update the database. Renamed and numbered the notification.sql patch.

Test Plan: Drop phabricator_feed.feed_storynotification table if it exists and run bin/storage upgrade to check if the patch is correctly applied.

Reviewers: epriestley, btrahan, allenjohnashton

Reviewed By: epriestley

CC: ddfisher, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2687
2012-06-08 12:42:59 -07:00
macvicar
1d94d49c17 [elasticsearch] Add timeout to API calls
Summary:
Default of 300 seconds is more than likely too much in most cases.
Provide the option to override.

Test Plan:
Blocked ElasticSearch with iptables
Set timeout to 5 seconds and make sure we error early

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2678
2012-06-08 12:16:49 -07:00
epriestley
5c5351b5e3 Minor, fix Dxxx in IRCBot. 2012-06-08 12:07:19 -07:00
John-Ashton Allen
ec37ce3db7 Add more detailed story actions for maniphest.
Summary:
TransactionType gives us more information than
update, open, close, assign.  We can display those in feed/notifications along with and comments on the actions.

Test Plan: did on local machine tested out.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: ddfisher, keebuhm, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2683
2012-06-08 11:28:49 -07:00
vrana
e962ad97fe Switch lint and unit fields in revision
Summary: D809#6

Test Plan: Displayed revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2682
2012-06-08 09:10:25 -07:00
epriestley
237a805e6c Minor, rebuild library map. 2012-06-08 08:21:53 -07:00
John-Ashton Allen
3a6ee79190 Adds base notification application
Summary: First diff in a series of diffs to add notifications to Phabricator. This is the notification application ONLY. This commit does not include the changes to other applications that makes them add notifications. As such, no notifications will be generated beyond the initial database import.

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

Reviewers: epriestley, btrahan, ddfisher

Reviewed By: epriestley

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

Maniphest Tasks: T974

Differential Revision: https://secure.phabricator.com/D2571
2012-06-08 06:32:02 -07:00
vrana
692296a4d4 Add newline to Differential review request e-mail 2012-06-07 23:59:45 -07:00
vrana
5752faecce Display Arcanist project in Differential e-mails
Summary:
I need this information quite often.
I don't know how many people are working on a single project and this information will be useless for them but I guess it won't hurt much?

Test Plan: Commented on accepted revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2674
2012-06-07 23:16:23 -07:00
vrana
bed3a9817b Merge author and committer
Summary: These columns holds the same value in most cases which irritates me.

Test Plan: Displayed history with same author and committer, emulated different committer.

Reviewers: hsb, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2671
2012-06-07 17:33:34 -07:00
Two9A
2d52881d4e Diffusion/BrowseFile: Set highlighted with blame as default view
Summary: As per title: when browsing files in Diffusion, set "Highlighted with blame" as the default view instead of falling back to "Highlighted".

Test Plan: Tested view with a local Diffusion setup, defaults correctly.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1278

Differential Revision: https://secure.phabricator.com/D2669
2012-06-07 16:56:26 -07:00
vrana
a8b5ca63bf Support Git renames in the middle of path
Test Plan: Blame previous revision with such rename.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2670
2012-06-07 12:00:44 -07:00
vrana
ee916859ea Drive Differential review request e-mail message by field specification
Summary:
This also changes some stuff:

- Reviewers used to be at top, now they are under comments.
- Primary reviewer is now rendered as first.

Test Plan: Added `renderValueForMail()` to a custom field, created diff, commented on it and verified e-mails.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2664
2012-06-06 18:07:47 -07:00
vrana
1406d6cdd0 Drive Differential e-mail message by field specification
Summary: Refactoring before the actual change.

Test Plan: None yet.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2661
2012-06-06 10:08:37 -07:00
epriestley
259638e900 Fix minor issues with D2630
Summary:
  - The config is called "resource-path" and the script references "resource-path", but the actual value checked for is "resource-map".
  - Use nonempty(), since defaulting with getEnvConfig() will give you null if the setting exists but is set to null. This default is nearly useless so maybe we should change it to use coalesce().
  - Remove Celerity map initialization from warmup. We don't currently initialize the environment in warmup, and Celerity initialization now depends on the environment.

Test Plan: Ran patch locally and on FPM-Warmup.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: hsb, aran

Differential Revision: https://secure.phabricator.com/D2662
2012-06-06 09:12:42 -07:00
epriestley
3e87112d69 Minor, fix a path issue from the Great File Move. See comment about why this is written in this way. 2012-06-06 07:35:40 -07:00
epriestley
0e1bbbd489 Allow administrators to change usernames
Summary:
Give them a big essay about how it's dangerous, but allow them to do it formally.

Because the username is part of the password salt, users must change their passwords after a username change.

Make password reset links work for already-logged-in-users since there's no reason not to (if you have a reset link, you can log out and use it) and it's much less confusing if you get this email and are already logged in.

Depends on: D2651

Test Plan: Changed a user's username to all kinds of crazy things. Clicked reset links in email. Tried to make invalid/nonsense name changes.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1303

Differential Revision: https://secure.phabricator.com/D2657
2012-06-06 07:09:56 -07:00
epriestley
0a7b4591ef Allow usernames to include ".", "-" and "_"
Summary:
See T1303, which presents a reasonable case for inclusion of these characters in valid usernames.

Also, unify username validity handling.

Test Plan: Created a new user with a valid name. Tried to create a new user with an invalid name. Ran unit tests.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1303

Differential Revision: https://secure.phabricator.com/D2651
2012-06-06 07:09:05 -07:00
epriestley
489303a057 Fix transaction handling in PhabricatorUserEditor->createNewUser()
Summary:
See https://github.com/facebook/phabricator/issues/117

  - The $user save can hit a duplicate key exception like the email, but we don't handle it correctly.
  - When the $user saves but the $email does not, the $user is left with a (rolled-back, invalid) ID. This makes the UI glitch out a bit. Wipe the ID if we abort the transaction.
  - We show the "Required" star marker even if the email is filled in.

The ID issue is sort of a general problem, but I think it's fairly rare: you must be doing inserts on related objects and the caller must catch the transaction failure and attempt to handle it in some way.

I can think of three approaches:

  - Manually "roll back" the objects inside the transaction, as here. Seems OK if this really is a rare problem.
  - Automatically roll back the 'id' and 'phid' columns (if they exist). Seems reasonable but maybe more complicated than necessary. Won't get every case right. For instance, if we inserted a third object here and that failed, $email would still have the userPHID set.
  - Automatically roll back the entire object. We can do this by cloning all the writable fields. Seems like it might be way too magical, but maybe the right solution? Might have weird bugs with nonwritable fields and other random stuff.

We can trigger the rollback by storing objects we updated on the transaction, and either throwing them away or rolling them back on saveTransaction() / killTransaction().

These fancier approaches all seem to have some tradeoffs though, and I don't think we need to pick one yet, since this has only caused problems in one case.

Test Plan: Tried to create a new user (via People -> Create New User) with a duplicate username. Got a proper UI message with no exception and no UI glitchiness.

Reviewers: btrahan, vrana, hgrimberg, hgrimberg01

Reviewed By: hgrimberg01

CC: aran

Differential Revision: https://secure.phabricator.com/D2650
2012-06-05 06:46:01 -07:00
KorvinSzanto
934246675e Quick ircbot differential
Summary: replace differential.find with differential.query and display in requested order

Test Plan: say D444 D222 D443 D442 and ensure they are in the correct order

Reviewers: epriestley

Reviewed By: epriestley

CC: Mnkras, aran

Differential Revision: https://secure.phabricator.com/D2656
2012-06-04 20:05:46 -07:00
vrana
8883c9494f Allow specifying custom celerity resource map
Summary:
We have custom static resources.
We currently include them in Phabricator's celerity resource map which is causing some pain - we need to regenerate the file without our custom resources before pushing upstream, we need to discard our changes before pulling from upstream and we need to rebuild with our changes to run Phabricator.

This diff allows writing and reading the map in other location.
The plan is this - I will run `celerity_mapper.php` twice - once to build Phabricator-only resources (to push to upstream) and once to build Phabricator + ours resoruces to put in our directory.

Better solution would be to create a map just with our resources and read and combine it with Phabricator resources.
But it is complicated because we have dependencies on Phabricator resources.

Test Plan:
`celerity_mapper.php webroot`
`celerity_mapper.php webroot ../facebook/src/__celerity_resource_map__.php`
Delete Phabricator's celerity map, set 'celerity.resource-path' and successfully load Phabricator.

Reviewers: epriestley, edward

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T721

Differential Revision: https://secure.phabricator.com/D2630
2012-06-04 18:45:03 -07:00
vrana
06b0f0d8ab Pass URI to Elastic engine from outside
Summary:
We need to generate the URI dynamically.
This code is also generally better.

Test Plan: Created custom search selector passing the custom URI to engine.

Reviewers: epriestley, edward

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2655
2012-06-04 18:33:42 -07:00
epriestley
5fee8c50ee Fail more softly if we can't execute "ps"
Summary: If, e.g., $PATH is broken we may not be able to run "ps". We'll explode pretty hard, currently. Instead, just show a harsher warning.

Test Plan: Changed "ps auxwww" to "psq", which doesn't exist on my system. Loaded page, got warning instead of explosion.

Reviewers: nathanws, vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2624
2012-06-02 14:05:27 -07:00
epriestley
cb1177497e Make ManiphestAction extend from ManiphestConstants, not PhrictionConstants
Summary: See rPee620bde6dbc. Copy/paste derp. Not @vrana!

Test Plan: arc liberate

Reviewers: allenjohnashton, vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2604
2012-06-02 14:01:06 -07:00
epriestley
6a8ac91599 Make chatlog a bit less awful
Summary:
  - Default to showing the newest page of chat.
  - Reformat for greater readability.
  - Add permalinks to specific lines.
  - Enable jump-to-date.

Test Plan: {F12200}

Reviewers: Koolvin, vrana, btrahan

Reviewed By: btrahan

CC: kdeggelman, aran

Maniphest Tasks: T837, T1065

Differential Revision: https://secure.phabricator.com/D2641
2012-06-02 14:00:08 -07:00
vrana
3102e15497 Replace array_mergev(array()) by array_merge()
Test Plan: Save owners package.

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2646
2012-06-01 21:47:15 -07:00
vrana
ec9589fb3b Ignore errors in svn diff
Summary: Otherwise attaching the commit diff doesn't work.

Test Plan: Reparsed previously failing commit message.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2605
2012-06-01 21:45:33 -07:00
vrana
7978264862 Display revision author if there is no diff author
Summary: Occurs for very old diffs.

Test Plan: Display revision with previously **An Unknown Object** author.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2645
2012-06-01 17:44:23 -07:00
vrana
fd10bcea48 Remove references to __init__.php from docs
Test Plan: Regenerate docs.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2640
2012-06-01 13:10:33 -07:00
epriestley
8d37576bd8 Move some aprhont/ stuff around a little bit
Summary:
"configuration/default" moved to "default". Rename it to "configuration" and put config stuff back there.

Put writeguard into writeguard/.

Test Plan: tools

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2639
2012-06-01 12:52:18 -07:00
epriestley
75dc602033 Move policy tests back into policy/
Summary: These were in an unusual location, but are better back in policy/

Test Plan: implicit arc unit

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

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

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

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

Auditors: epriestley

Maniphest Tasks: T1103
2012-06-01 12:32:44 -07:00
Jonathan Lomas
103ff94a40 Better format the Next and Updated columns in MetaMTA Mail Queue
Summary: They were only displaying seconds.  I found a function in viewutils.php that allowed for single-unit precision formatting, but I wanted more, so I wrote another function to allow more detail.

Test Plan: [site]/mail, and watch it work.  It's a new function, so it shouldn't break anything else.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1296

Differential Revision: https://secure.phabricator.com/D2616
2012-06-01 10:29:42 -07:00
epriestley
fd20b18c2d Improve lint integration documentation
Summary: Explain what lint is, how to configure it, what we ship with, etc. Also wax philosophical.

Test Plan: Generated docs, read "carefully".

Reviewers: csilvers, btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2628
2012-06-01 07:55:30 -07:00
epriestley
3512ba2b55 Remove "TB" unit tests
Summary: These fail on 32-bit machines and seem not valuable enough to try to make work with `bc`, etc.

Test Plan: Ran tests (depends on D2622).

Reviewers: floatinglomas, vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1298

Differential Revision: https://secure.phabricator.com/D2623
2012-06-01 07:51:56 -07:00
Craig Silverstein
06b2eb55f0 Fix a typo.
Test Plan: (None)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2627
2012-05-31 15:22:58 -07:00
vrana
1ebf9186b4 Depend on class autoloading
Test Plan:
Run setup.
/differential/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

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

Test Plan: See D2588.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2591
2012-05-30 14:26:29 -07:00
epriestley
c26062b43d Use heavy-check and heavy-x next to "Accept" and "Raise Concern" Audit actions
Summary: Mark these actions with the same markers we use in Differential.

Test Plan: {F12094}

Reviewers: csilvers, jungejason, btrahan

Reviewed By: csilvers

CC: aran

Maniphest Tasks: T1289

Differential Revision: https://secure.phabricator.com/D2601
2012-05-30 14:01:26 -07:00
vrana
9c2b67e2dc Add button to reveal all files in Differential
Summary: D2216 tried to ask the user, this one is explicit.

Test Plan: Click the button

Reviewers: epriestley, lucian

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2600
2012-05-30 10:44:37 -07:00
vrana
5e49de7b35 Use loadRelatives() in loadPrimaryEmail()
Summary: This is an example of code simplification with D2557.

Test Plan: Display user list, verify the SQL queries.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2558
2012-05-30 10:43:16 -07:00
vrana
6f10706852 Display and link lint errors on line 0
Summary:
Some lint errors (e.g. Javelin) don't have a line number.
Put them on the first line.

Putting them above the first line would be even nicer but much more complicated.

Test Plan: Display diff with lint error on line 0 (D2583).

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2599
2012-05-29 16:53:30 -07:00
vrana
c002b466b8 Fix doc links 2012-05-29 15:36:02 -07:00
vrana
37b1ac5a24 Load changesets with inline comments in large diffs
Summary: Also fix the notice text.

Test Plan:
Display diff with inline comments and lint errors.
Click on inline comment and lint link.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2594
2012-05-29 12:22:51 -07:00
Jason Ge
bbd5c8c187 Add audit entry when code changed after a revision was accepted
Summary: build on top of D2530 and D2540. Add an auditing entry when the code was changed after the revision was accepted.

Test Plan: ran reparse.php manually. It worked at https://phabricator.fb.com/rPHGIT461864d2e09dad04c28505658ef75a979e44d0d3. Look at the latest auditing entry for auditor 'phgit ha directory'.

Reviewers: vrana

Reviewed By: vrana

CC: nh, epriestley, aran

Maniphest Tasks: T201

Differential Revision: https://secure.phabricator.com/D2586
2012-05-29 11:32:09 -07:00
vrana
0044b9cca9 Throw defined error
Test Plan: `differential.setdiffproperty` with non-existent diff and property "arc:lint".

Reviewers: mgummelt, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2584
2012-05-26 10:07:44 -07:00
epriestley
557e508656 Allow restriction of permitted email domains
Summary:
Allow allowed email addresses to be restricted to certain domains. This implies email must be verified.

This probably isn't QUITE ready for prime-time without a few other tweaks (better administrative tools, notably) but we're nearly there.

Test Plan:
  - With no restrictions:
    - Registered with OAuth
    - Created an account with accountadmin
    - Added an email
  - With restrictions:
    - Tried to OAuth register with a restricted address, was prompted to provide a valid one.
    - Tried to OAuth register with a valid address, worked fine.
    - Tried to accountadmin a restricted address, got blocked.
    - Tried to accountadmin a valid address, worked fine.
    - Tried to add a restricted address, blocked.
    - Tried to add a valid address, worked fine.
    - Created a user with People with an invalid address, got blocked.
    - Created a user with People with a valid address, worked fine.

Reviewers: btrahan, csilvers

Reviewed By: csilvers

CC: aran, joe, csilvers

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2581
2012-05-26 06:04:35 -07:00
vrana
648c8aa499 Toggle DarkConsole request log by keyboard shortcut
Test Plan:
`
`

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2583
2012-05-25 22:06:04 -07:00
vrana
af6238ca4a Inform about changes made between last revision and commit
Summary:
This adds a link to [Closed] e-mail if it detects some changes.
It compares added and removed lines with 3 lines context.
The subtle form of informing is permissive to false negatives and positives.
I have an e-mail filter for [Closed] e-mails so I wouldn't personally notice this change - we should probably promote this feature a little bit.

Test Plan:
Reparse a diff with a change after last update.
Reparse a diff without a change after last update.

Reviewers: jungejason, epriestley

Reviewed By: jungejason

CC: aran, Koolvin, btrahan

Maniphest Tasks: T201

Differential Revision: https://secure.phabricator.com/D2540
2012-05-25 21:39:58 -07:00
vrana
0446e636ea Add an abstraction for preventing the 1+N queries problem
Summary:
Our code is quite complex in areas where we prevents the 1+N queries problem explained in [[ http://www.phabricator.com/docs/phabricator/article/Performance_N+1_Query_Problem.html | a performance chapter ]].

This diff adds an abstraction for preventing this code.

Test Plan:
Run all examples mentioned in the doc-comments with logging the queries.
Generate and read docs.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2557
2012-05-25 13:09:42 -07:00
vrana
1377d349e1 Use first diff author for summary and test plan in commandeered revisions
Summary:
I thought about it a little bit and this makes the most sense for me:

# Original author usually writes at least something and commander only updates it.
# There's a creation date of revision (= first diff) by these comments. I don't want to change this date because I use this information. Author should correspond to this date.
# It solves all our repros.

Test Plan: Display commandeered revision.

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, epriestley

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

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

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1136

Differential Revision: https://secure.phabricator.com/D2574
2012-05-25 10:14:17 -07:00
epriestley
70fd96037b Consolidate user editing code
Summary:
  - We currently have some bugs in account creation due to nontransactional user/email editing.
    - We save $user, then try to save $email. This may fail for various reasons, commonly because the email isn't unique.
    - This leaves us with a $user with no email.
  - Also, logging of edits is somewhat inconsistent across various edit mechanisms.
  - Move all editing to a `PhabricatorUserEditor` class.
  - Handle some broken-data cases more gracefully.

Test Plan:
  - Created and edited a user with `accountadmin`.
  - Created a user with `add_user.php`
  - Created and edited a user with People editor.
  - Created a user with OAuth.
  - Edited user information via Settings.
  - Tried to create an OAuth user with a duplicate email address, got a proper error.
  - Tried to create a user via People with a duplicate email address, got a proper error.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: tberman, aran

Maniphest Tasks: T1184

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

Test Plan: Mangle token and send form.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2570
2012-05-24 18:04:44 -07:00
vrana
96f725009f Destroy fixture explicitly
Summary:
Unittest databases are not always destroyed in our setup.
It could be caused by `__destruct()` not called in case of a fatal error.

Test Plan:
  arc unit src/applications/calendar/storage/holiday

Reviewers: edward, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2568
2012-05-24 16:09:42 -07:00
vrana
0da0632242 Display author of last manual diff in summary and test plan comments
Summary:
D2550 is not compatible with D2540.
Example: D2559.

Test Plan: Display commandeered revision.

Reviewers: nh

Reviewed By: nh

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2567
2012-05-24 15:38:45 -07:00
epriestley
0e3aeab1b3 Minor, remove getEmail() since D2494 removed these methods. 2012-05-24 15:37:33 -07:00
epriestley
79e8a637c2 Minor, fail gracefully if there are data integrity problems until I can fix oauth transactions. 2012-05-24 15:17:50 -07:00
vrana
a9cee4e923 Fix bad rebase in rPc2a9a807 2012-05-24 15:12:39 -07:00
epriestley
2f138d0501 Add a "roles" array to user.query
Summary:
  - Add "role" information, so clients can identify disabled users.
  - Formally deprecate `user.info`

Test Plan: Ran "user.query" and "user.whoami", inspected output. Verified "user.info" appears as deprecated in method list and console.

Reviewers: csilvers, btrahan

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2565
2012-05-24 12:10:47 -07:00
Nick Harper
0ddfd0b4fb Show less misleading summary, test plan authors
Summary:
Instead of assuming the test plan and summary are written by the author
of the differential revision, let's assume they are written by the author
of the latest differential diff.

Test Plan: viewed a drev that had been commandeered but not updated to check authors

Reviewers: epriestley, jungejason, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1235

Differential Revision: https://secure.phabricator.com/D2550
2012-05-23 14:57:54 -07:00
vrana
ec27d39b8b Use black bullet instead of gray background for disabled users
Summary: Just because I like it more.

Test Plan: View diff with comment from disabled user.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2525
2012-05-23 14:16:57 -07:00
vrana
978f6edf19 Fix code not working in HHVM
Summary:
It's also more readable so I think it's OK.
I've also filed a bug for HHVM.

Test Plan: `arc unit` in HHVM

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2551
2012-05-23 14:16:02 -07:00
epriestley
e12961802b Minor improvements to email management interface
Summary:
  - If you have an unverified primary email, we show a disabled "Primary" button right now in the "Status" column. Instead we should show an enabled "Verify" button, to allow you to re-send the verification email.
  - Sort addresses in a predictable way.

Test Plan:
  - Added, verified and removed a secondary email address.
  - Resent verification email for primary address.
  - Changed primary address.

Reviewers: btrahan, csilvers

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2548
2012-05-23 12:55:07 -07:00
Bob Trahan
65710ee2d2 Fix repository interactions for SVN repositories using the SVN protocol with SASL
Summary: also makes the UI more general for this username + password business.

Test Plan:
- configure a phabricator repository from the svn server @asherwin provided which is configured for svn protocol with SASL
- observed phabricator failing without my patch
- upgraded my SVN client to support SASL (protip for mac users - http://www.wandisco.com/subversion/download#osx)
- applied patch to phabricator
- restarted daemons
- noted daemon success - diffusion populating nicely

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1260

Differential Revision: https://secure.phabricator.com/D2549
2012-05-23 12:37:43 -07:00
Hafsteinn Baldvinsson
a438c87c52 Show the difference between a committer and an author
Summary:
Git and hg (supposedly..) differentiate between an author (who wrote the patch)
and a committer (who applied the patch).

This patch allows Phabricator to note when a patch is committed
by someone other than the Author.

Test Plan:
Created 2 accounts,
 - U (Account with a PHID)
 - U' (Account without a PHID)
and had them create and commit commits

testing if their username/real name would be displayed correctly in Diffusion,
  - BrowserTable
  - HistoryTable
  - Code revision

Teztz,
A(uthor)/C(ommitter)
If it's A/A then Author committed

UL = User link (<a href="/p/username">username</a>)
UN = User name ("Firstname Lastname")

Tezt | Expected in table  | Got
-------------------------------------------
A/A   | UL/UL             | UL/UL
A'/C  | UN/UL             | UN/UL
A/C'  | UL/UN             | UL/UN
A'/C' | UN/UN             | UN/UN

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T688

Differential Revision: https://secure.phabricator.com/D2541
2012-05-23 08:34:54 -07:00
Nick Harper
e4e56bb431 Return partial differential fields when there's an error parsing
Summary:
If someone typos one name in a cc or reviewer list, it would be nice if we
display all of the valid names in the field when running arc diff (in addition
to the error message).

Test Plan:
used the conduit console to check that calling differential.parsecommitmessage
with a list of some valid and some invalid ccs returns a result with both an
error and a list of some ccs. Also ran arc diff with that list of ccs to check
for the correct user experience.

Reviewers: epriestley, jungejason, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2542
2012-05-22 18:12:41 -07:00
vrana
ccd37afab8 Attach commit diff to its revision
Summary:
This attaches commit diff to its associated revision as any other diff.
The consequence is that the revision page now shows the actual commit instead of the last diff. It may be disturbing but it is desired.
Another consequence is that lint and unit results are displayed as skipped after the revision was committed. I want to fix it somehow.

My next plan is to automatically diff against the last normal diff and include the link to this diff in commit e-mail if non-empty.

Test Plan:
  reparse.php

Diff against last normal diff.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran, Koolvin, jungejason

Maniphest Tasks: T201

Differential Revision: https://secure.phabricator.com/D2530
2012-05-22 16:08:16 -07:00
epriestley
46af896364 Add self-links for Differential and Maniphest
Summary:
See:

https://groups.google.com/forum/?fromgroups#!topic/phabricator-dev/WolHZVVJB7k

Render the `D3`, `T132`, etc., in the title as a link instead of in grey text. Also clean up some related CSS.

Test Plan: Looked at a revision and a task.

Reviewers: btrahan, asherwin

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2534
2012-05-22 10:52:16 -07:00
mkedia
787c95abd8 Add owners.query method
Summary:
Allow callers to find all packages/paths owned by a given
user/project.

Test Plan:
Used conduit api page with --
user owner: PHID-USER-6ce1c976b86e5f3c34f6 (tried both loadPackagesFromProjects
true/false)

proj owner: PHID-PROJ-r5wnmmaawqsn4tvjmqm4

repo/path: E, /tfb/trunk/www/flib/privacy. Checked both path.getowners
and owners.query

(all of these are defined internally at fb)

Reviewers: epriestley, btrahan, nh

Reviewed By: btrahan

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2482
2012-05-22 10:19:58 -07:00
epriestley
a009c93350 Use "-b", not "--branch", when issuing "hg log" in Phabricator
Summary:
Mercurial renamed "--only-branch" to "--branch" about two years ago. "-b" exists in both versions.

http://www.selenic.com/pipermail/mercurial-devel/2010-April/020469.html

We have a few other cases where we use features that exist only in recent Mercurial (notably, 'ancestors' in log) but we can work around this one easily.

Test Plan: Looked at a Mercurial repo in Diffusion, verified that "log -b" commands issued and that the output was correct.

Reviewers: btrahan, Makinde, ipalaus

Reviewed By: ipalaus

CC: aran

Maniphest Tasks: T1264

Differential Revision: https://secure.phabricator.com/D2533
2012-05-22 07:14:55 -07:00
epriestley
3078778fc0 Support more aliases of "Test Plan" in commit messages
Summary: Add support for "tests", "testplan" and "tested" as alias of "Test Plan".

Test Plan: Created a diff with test plan specified in "tests".

Reviewers: csilvers, btrahan

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2531
2012-05-22 06:02:12 -07:00
epriestley
0461cd6e4f Prevent loops in received mail
Summary:
It's currently possible to configure Phabricator to send mail to some address it recognizes as relating to an object.

When we receive mail from Phabricator, drop it unconditionally.

Test Plan: Wrote two emails, one with the header and one without. Piped them to `mail_handler.php`, one was dropped immediately.

Reviewers: btrahan, nh, mikaaay, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2529
2012-05-22 06:02:05 -07:00
epriestley
463cb116bd Minor, fix a documentation link (thanks, @ipalaus!). 2012-05-22 05:59:40 -07:00
Nick Harper
c2a9a8079f Remove email from handles
Summary:
Since user emails aren't in the user table, we had to do extra data fetching
for handles, and the emails are only used in MetaMTA, so we move the email
code into MetaMTA and remove it from handles.

Test Plan: send test emails

Reviewers: jungejason, vrana, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2494
2012-05-21 17:37:26 -07:00
epriestley
8bbc724210 Bump Conduit server version
Summary:
We introduced a "user.query" call recently which is only about two weeks old. Bump versions so users get a forced upgrade.

Also, we raise a fairly confusing message when the user calls a nonexistent method. This is not the intent; `class_exists()` throws. Tailor this exception more carefully.

Test Plan:
  - Ran `echo {} | arc call-conduit derp.derp`, got a better exception.
  - Bumped version, ran `arc list`, got told to upgrade.

Reviewers: indiefan, nh, vrana, btrahan, jungejason, Makinde

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2527
2012-05-21 16:43:43 -07:00
Jason Ge
2b39a77fd8 variable name incorrect
Summary: as title

Test Plan: no

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2526
2012-05-21 15:34:35 -07:00
vrana
bb7285ab46 Don't mix different users on the same line in Calendar
Summary:
The current state is very confusing:
{F11734, size=full}

Test Plan: Display calendar for user with two different events in the same week.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Koolvin

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

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

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

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran, csilvers

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2520
2012-05-21 12:47:38 -07:00
epriestley
3fd93b594e Make XHProf easier for me to use with other users
Summary: Ideally there should be a "send epriestley this profile" button but this is a reasonable step forward. Add a "download .xhprof profile" button to profiles, since walking through these things remotely is pretty awkward. Also expand "Excl" and "Incl" acronyms.

Test Plan: Clicked download button.

Reviewers: csilvers, btrahan, vrana

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2523
2012-05-21 12:47:29 -07:00
epriestley
0548dc4c4c Show user status on calendar
Summary:
This is a rough cut, but gets some of the basics at least. Here's what it looks like:

{F11690}

Some things that would be nice for future diffs:

  - Different colors for different event types (tasks? MEETINGS?!)
  - When events span across multiple days, keep them in the same row.
  - Switch which month you're looking at.
  - Show specific users instead of all.
  - etc etc etc

Test Plan: See screenshot.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2514
2012-05-21 10:24:23 -07:00
Sigurd Holsen
c2d2f6ded1 bugfix: mercurial commit worker had an array error
It tried to get $path['targetPath'] istead of $change['targetPath']
when $path was a string
2012-05-21 09:04:48 -07:00
Bob Trahan
80f479d948 Make directories with spaces in their names work OK in diffusion end to end
Summary:
we were parsing the git log output slightly incorrectly and over-exploding on spaces. we also needed to escape the path %20 stuff`.

Not sure if there's something fancy to do given folks should reparse their repos if they are impacted by this issue.

Test Plan:
made a directory with spaces and some dummy revisions. observed diffs wouldn't load and links broken.
with patch, ran scripts/reparse.php for pertinent revisions and diffs loaded and links weren't broken.

Reviewers: floatinglomas, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1252

Differential Revision: https://secure.phabricator.com/D2510
2012-05-20 15:29:36 -07:00
epriestley
b624dc4e7b Increase result set to 100 from 25 for "attach" dialog
Summary: See T1254, until this gets paginated properly we can at least show more results. 25 is pretty anemic.

Test Plan: Tweaked limit, verified result count was affected.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1254

Differential Revision: https://secure.phabricator.com/D2513
2012-05-20 14:56:04 -07:00
epriestley
3a1ee00335 Select default branches more effectively in Diffusion
Summary: If you have an empty value saved in the "default branch" field, we default to empty string (or null, or whatever) instead of the correct default.

Test Plan:
  - Looked at a Git repo with an empty default branch, got a default to "master" instead of an error.
  - Looked at a Mercurial repo with an empty default branch, got a default to "default" instead of an error.

Reviewers: btrahan, csilvers

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1237

Differential Revision: https://secure.phabricator.com/D2512
2012-05-20 14:50:43 -07:00
epriestley
a89cef8e39 Remove PHID database, add Harbormaster database
Summary:
  - We currently write every PHID we generate to a table. This was motivated by two concerns:
    - **Understanding Data**: At Facebook, the data was sometimes kind of a mess. You could look at a random user in the ID tool and see 9000 assocs with random binary data attached to them, pointing at a zillion other objects with no idea how any of it got there. I originally created this table to have a canonical source of truth about PHID basics, at least. In practice, our data model has been really tidy and consistent, and we don't use any of the auxiliary data in this table (or even write it). The handle abstraction is powerful and covers essentially all of the useful data in the app, and we have human-readable types in the keys. So I don't think we have a real need here, and this table isn't serving it if we do.
    - **Uniqueness**: With a unique key, we can be sure they're unique, even if we get astronomically unlucky and get a collision. But every table we use them in has a unique key anyway. So we actually get pretty much nothing here, except maybe some vague guarantee that we won't reallocate a key later if the original object is deleted. But it's hard to imagine any install will ever have a collision, given that the key space is 36^20 per object type.
  - We also currently use PHIDs and Users in tests sometimes. This is silly and can break (see D2461).
  - Drop the PHID database.
  - Introduce a "Harbormaster" database (the eventual CI tool, after Drydock).
  - Add a scratch table to the Harbormaster database for doing unit test meta-tests.
  - Now, PHID generation does no writes, and unit tests are isolated from the application.
  - @csilvers: This should slightly improve the performance of the large query-bound tail in D2457.

Test Plan: Ran unit tests. Ran storage upgrade.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: csilvers, aran, nh, edward

Differential Revision: https://secure.phabricator.com/D2466
2012-05-20 14:46:01 -07:00
epriestley
a70cf3f675 Make "Dnn" and "Tnn" match case-insensitively in the "attach" dialog
Summary: These patterns are hard-coded, allow them to match case-insenstiviely.

Test Plan: Typed "d3" and "D3", got the right object in the attach dialog.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1253

Differential Revision: https://secure.phabricator.com/D2511
2012-05-20 13:51:43 -07:00
Bob Trahan
eb6041371b Fix diffusion browse links in the owner's tool
Summary: ...they were broken...

Test Plan: clicked links for both SVN and Git repos and got working results

Reviewers: vrana, floatinglomas, 20after4

Reviewed By: floatinglomas

CC: aran, epriestley

Maniphest Tasks: T1250

Differential Revision: https://secure.phabricator.com/D2505
2012-05-20 11:30:01 -07:00
Bob Trahan
a9000ea21c Phriction - lock down /project/ wiki docs
Summary:
only show the blank, "create new" wiki page for the project if the project actually exists; only allow edit if the project actually exists.
Small wrinkle here is not checking if the project actually exists if the page already exists.

Test Plan:
- viewed a project wiki page
- viewed a prokect wiki page for a fake project and got a 404
- edited a project wiki page
- edited a project wiki page for a fake project and got a 404

Reviewers: epriestley, jacktrades

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1248

Differential Revision: https://secure.phabricator.com/D2506
2012-05-20 08:54:25 -07:00
Bob Trahan
3d5d8d0f11 Fix task + commit associating
Summary: we weren't actually removing any edges. now we do.

Test Plan: had a commit associated with task x; removed association. had a commit associated with task x; removed association while adding a different one.

Reviewers: floatinglomas, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1256

Differential Revision: https://secure.phabricator.com/D2509
2012-05-20 08:53:48 -07:00
Bob Trahan
de1973b516 fix a small bug from new profile status code stuff
Summary: we need a user (the viewer in this case) for the status to render correctly with respect to timezone

Test Plan: my profile no longer fatals with an away status

Reviewers: davidreuss, vrana

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2504
2012-05-19 17:00:17 -07:00
Bob Trahan
7c42ade617 Fix D2490 (make macro handler correctly bail if there are no macros)
Summary: D2490 was not my finest hour and I incorrectly thought it was a null value from error. In reality this error is impossible and its just a valid empty array so instead use the empty predicate to bail.

Test Plan: with our logic combined, this be tested

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2503
2012-05-19 16:19:33 -07:00
Bob Trahan
912e414013 Update Conduit Maniphest CRUD API(s) to not accept crud
Summary: see T1241, T1242, T1244 for some examples of crud getting saved

Test Plan: threw some crud in my conduit console and got reasonable errors back

Reviewers: mikaaay, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1241, T1242, T1244

Differential Revision: https://secure.phabricator.com/D2487
2012-05-19 16:18:13 -07:00
vrana
4e687e0658 Highlight away status by bullet and display it everywhere
Summary: D2484#13

Test Plan:
Display revision list.
Display revision detail.

Reviewers: epriestley

Reviewed By: epriestley

CC: btrahan, aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2501
2012-05-19 13:13:06 -07:00
vrana
fa36dd513c Integrate user.getcurrentstatus into user.query
Summary: D2492#6

Test Plan:
`user.query` of user which is away.
`user.query` of user which is not away.
`user.whoami` - no information there.

Reviewers: epriestley

Reviewed By: epriestley

CC: btrahan, aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2502
2012-05-19 13:11:14 -07:00
Craig Silverstein
18deff557e Fix a typo.
Test Plan: (None.)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2483
2012-05-18 17:58:58 -07:00
Craig Silverstein
5af1dff06a Add a missing # in the markup.
Test Plan: (None)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2499
2012-05-18 17:38:13 -07:00
Bob Trahan
0b764e99dd Phriction - fix error log spew from misconfigured table column classes
Summary: there were 6 values, but we only need 4. delete the two empty ones as the others are all stylistically valid.

Test Plan: ...looks good!

Reviewers: ric03uec, vrana, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T1247

Differential Revision: https://secure.phabricator.com/D2498
2012-05-18 17:27:14 -07:00
vrana
8dca5cdb5a Add Conduit method to disable user
Summary: There's no method for enabling users somewhat intentionally.

Test Plan: Disable myself (oops, this is probably my last diff ever).

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2497
2012-05-18 16:29:36 -07:00
vrana
db1f94b0c0 Move getPrimaryReviewer() to DifferentialRevision
Test Plan: Display revision list both with last reviewer and without.

Reviewers: epriestley, btrahan

Reviewed By: btrahan

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2495
2012-05-18 14:32:19 -07:00
vrana
9f35a3ba45 Highlight away and sporadic users in revision list
Summary:
This is not so general as `getRequiredHandlePHIDs()`.
It allows bulk loading of user statuses only in revision list.

It also loads data in `render()`. I'm not sure if it's OK.

Maybe we can use the colorful point here.
Or maybe some unicode symbol?

Test Plan: {F11451, size=full}

Reviewers: btrahan, epriestley

Reviewed By: btrahan

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2484
2012-05-18 14:28:41 -07:00
vrana
d9b4fcb336 Display user status on user profile
Test Plan:
Display users with:

- Title.
- Status.
- Title and status.

Also display project.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2491
2012-05-18 00:28:31 -07:00
vrana
2476c872ff Add Conduit method for querying user status
Summary:
I want to use this to warn user if he specifies reviewers that are away.

We can also implement a general query method but I think that this usage is the
most useful not only for me but also in general case.

Test Plan:
Call the method for user which is away and which is not away.
Add user status through Conduit.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2492
2012-05-17 23:58:50 -07:00
vrana
f1f43f0acf Add colspan to Inline comment undo template
Summary: Broken by D2443.

Test Plan: Cancel non-empty inline comment.

Reviewers: epriestley, nh

Reviewed By: nh

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2486
2012-05-17 15:20:33 -07:00
Bob Trahan
5475a1bec7 IRC Macro Handler - check result from conduit call before operating on it
Summary: 'cuz github issue 114 came into existence. instead, just return false early here. note i am not sure if I should phlog that this is happening or not but its not exception worthy IMO.

Test Plan: lint-only 'cuz i don't want to setup an IRC server locally / somehow get my local phabricator instance accessible out there. happy to test end to end if there's an easier way...!

Reviewers: vrana, epriestley

Reviewed By: vrana

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2490
2012-05-17 14:55:18 -07:00
vrana
60696de095 Fix default limit in elasticsearch 2012-05-17 12:26:19 -07:00
vrana
6d5dcea3ba Inform about disabled and away users in Differential fields
Summary:
I've tried colors, italics, strike-through and this is a compromise between me and @leebyron.

NOTE: I don't want to disturb @epriestley from playing Diablo 3.

Test Plan: {F11439, size=full}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, leebyron

Differential Revision: https://secure.phabricator.com/D2481
2012-05-16 18:39:33 -07:00
Nick Harper
e9c3894861 Don't send email to bots subscribed to diffs
Summary:
Occasionally a bot will get subscribed to a differential revision; when
this happens we shouldn't send them an email, because otherwise the
author of the diff will get a bounce email if the bot has an invalid
emal address.

Test Plan:
re-sent a message that had a bot on the cc list, saw that it was no longer
included when it got sent.

Reviewers: epriestley, jungejason, vrana

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2479
2012-05-16 11:31:21 -07:00
Bob Trahan
6865f024c8 Make differentialRevisionListView always have the no data string be "No revisions found."
Summary: the existing, more custom text doesn't make sense when viewing someone else's revisions AND in someplaces its the less interesting "no data found".

Test Plan: clicked around the various http://phabricator.dev/differential/filter/* pages with various users.

Reviewers: epriestley, ry

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1222

Differential Revision: https://secure.phabricator.com/D2475
2012-05-15 14:44:03 -07:00
Bob Trahan
ed57869fc2 Fix setup from getRequiredClasses buggyboo
Summary:
D2470 added Package mailhandler, which was configured incorrectly in the getRequiredClasses function. this makes it like the other mail handlers

Reported at https://github.com/facebook/phabricator/issues/112

Test Plan: setup mode no longer fails

Reviewers: epriestley, jungejason, royklopper

Reviewed By: royklopper

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2476
2012-05-15 14:14:22 -07:00
Bob Trahan
7a19cf5a55 Fix lines of a large include path
Summary:
D2457 and D2459 made some changes here. This little guy just needed to be touched so it could be arc lint'd to have the proper updated include paths

Sorry to spam reviewers; Evan is in Diablo 3 somewhere. :D

Test Plan: no more fatal on test everything implemented (arc unit src/infrastructure/__tests__/)

Reviewers: epriestley, jungejason, nh, csilvers

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2474
2012-05-15 12:37:34 -07:00
Jason Ge
67d4f1ef4c Detect package change and send out email
Summary:
For package creation and deletion, send email to all the owners For
package modification, detect important fields such as owners and paths, and then
send out emails to all owners (including deleted owners and current owners)

Also start using transaction for package creation/deletion/modification.

Test Plan:
- tested mail creation and deletion
- tested modification to auditing enabled, primary owners, owners, paths

Reviewers: epriestley, nh, vrana

Reviewed By: epriestley

CC: prithvi, aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2470
2012-05-14 15:58:24 -07:00
Edward Speyer
bb96115fa6 Explicitly call __destruct() in PhabricatorEnvTestCase
Summary:
Required in order to run tests successfully in the HipHop interpreter.
Similar to D2362.

Test Plan: Run the tests in an HipHop runtime.

Reviewers: epriestley

Reviewed By: epriestley

CC: jungejason, aran, Koolvin, vrana

Differential Revision: https://secure.phabricator.com/D2365
2012-05-14 14:45:06 -07:00
vrana
4626c40fe3 Display URI of HTTP profiler in DarkConsole
Summary: D2464

Test Plan: D2464

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2465
2012-05-14 14:25:42 -07:00
vrana
d85425e654 Use transactions in saving comment
Summary: We had a real issue where revision was marked as accepted but the comment wasn't saved.

Test Plan: Reclaim abandoned revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: jungejason, aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2463
2012-05-14 14:21:05 -07:00
epriestley
820a6d407a Improve performance of repository discovery under Mercurial
Summary: Prelminary, I want to test this a bit more when I'm better rested. Provide a "bulk" import mode for Mercurial so we can do initial discovery more quickly.

Test Plan: Imported the Mercurial repository in 2m45s, blocked on MySQL I/O rather than Mercurial I/O.

Reviewers: csilvers, btrahan

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2457
2012-05-11 18:29:14 -07:00
vrana
b20ae2a07f Document transactions in docs
Summary:
Also add some formatting and links.

Also fix test broken by D2393.

Test Plan:
  diviner .

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2461
2012-05-11 16:10:18 -07:00
epriestley
81db220d1b Improve documentation on end-of-life for changes
Summary: This could use some additional explanation.

Test Plan: Read text.

Reviewers: btrahan, aurelijus

Reviewed By: aurelijus

CC: aran

Maniphest Tasks: T1204

Differential Revision: https://secure.phabricator.com/D2446
2012-05-10 14:19:06 -07:00
epriestley
61b728b19d Add a setting to blanket-disable autoclosing revisions in a repository
Summary: For git workflows where developers push personal feature branches to the origin, this is a quick workaround until T1210 is implemented properly. I'll also mention this in D2446.

Test Plan:
  - Committed a revision in an "autoclose" repository; it was autoclosed. Committed a revision in a no-autoclose repository, it was not autoclosed.
  - Edited repositories, saving the "autoclose" setting as enabled/disabled.

Reviewers: aurelijus, btrahan

Reviewed By: aurelijus

CC: aran

Maniphest Tasks: T1210

Differential Revision: https://secure.phabricator.com/D2448
2012-05-10 14:18:56 -07:00
epriestley
d24c81c5e2 Support branch offset/limit in Mercurial
Summary:
  - Support offset/limit as added by D2442, for Mercurial.
  - We just list everything and slice, no clear way to do better and this isn't a major perf issue.
  - No clear way to easily get by-update sorting, we can implement this by looking up revs if someone asks.
  - Bury some of the Git implementation details inside the Git query.

Test Plan:
  - Looked at a Git repo, clicked "View All Branches".
  - Looked at a Hg repo, clicked "View All Branches".
  - Limited page size to 1, made sure it limited properly.

Reviewers: aurelijus, btrahan, vrana

Reviewed By: aurelijus

CC: aran

Differential Revision: https://secure.phabricator.com/D2453
2012-05-10 14:18:49 -07:00
vrana
70e5ef92cf Use transactions in user status Conduit methods
Test Plan:
Add status, verify DB.
Remove status, verify DB.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

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

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

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1200

Differential Revision: https://secure.phabricator.com/D2442
2012-05-10 11:47:46 -07:00
epriestley
4921f3f13b Fix warnings for undefined $next_step and $verb
Summary: Revisions don't necessarily have a backing repository, as in "--raw".

Test Plan: Looked at a "--raw" revision in accepted and closed states.

Reviewers: nh, btrahan

Reviewed By: nh

CC: aran

Differential Revision: https://secure.phabricator.com/D2444
2012-05-10 11:43:35 -07:00
vrana
5a1d89227b Use subtler highlighting for copied and moved code
Summary:
The highlighting is distracting according to Nick Shrock and others.
Real designer, Lee Byron, helped me with this.

It also gives us unagressive target for jumping to the source line in future.

Another feature I will probably implement is highlighting also the source of copies/moves.
I will use the right side of the left column for it.

Test Plan:
Hover copied notifier.
Hover coverage notifier.

I've also checked that this doesn't break our super-flaky old/new code JavaScript detector.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin, leebyron, schrockn

Differential Revision: https://secure.phabricator.com/D2443
2012-05-10 10:25:24 -07:00
epriestley
1bf68e06a5 Improve Diffusion error messages and UI for partially imported repositories
Summary:
  - When you have an un-cloned repository, we currently throw random-looking Git/Hg exception. Instead, throw a useful error.
  - When you have a cloned but undiscovered repository, we show no commits. This is crazy confusing. Instead, show commits as "importing...".
  - Fix some warnings and errors for empty path table cases, etc.

Test Plan:
  - Wiped database.
  - Added Mercurial repo without running daemons. Viewed in Diffusion, got a good exception.
  - Pulled Mercurial repo without discovering it. Got "Importing...".
  - Discovered Mercurial repo without parsing it. Got "Importing..." plus date information.
  - Parsed Mercurial repo, got everything working properly.
  - Added Git repo without running daemons, did all the stuff above, same results.
  - This doesn't improve SVN much but that's a trickier case since we don't actually make SVN calls and rely only on the parse state.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T776

Differential Revision: https://secure.phabricator.com/D2439
2012-05-09 17:28:57 -07:00
epriestley
b98847d2b1 Show "show more..." link in "Local Commits" view in Differential
Summary: Javascript! Depends on D2437 to do anything useful.

Test Plan: Clicked "show more", saw more.

Reviewers: csilvers, btrahan

Reviewed By: csilvers

CC: aran

Maniphest Tasks: T1189

Differential Revision: https://secure.phabricator.com/D2438
2012-05-09 15:56:37 -07:00
epriestley
7e0c06716f Minor, fix obvious typo that I missed in testing somehow.
Auditors: csilvers
2012-05-09 12:43:57 -07:00
epriestley
905cc5ff32 Minor, fix OAuth registration, see D2431.
Auditors: btrahan
2012-05-09 11:19:15 -07:00
epriestley
b800df8c1b Simplify daemon management: "phd start"
Summary:
  - Merge CommitTask daemon into PullLocal daemon. This is another artifact of past instability (and order-dependent parsers). We still publish to the timeline, although this was the last consumer. Long term we'll probably delete timeline and move to webhooks, since everyone who has asked about this stuff has been eager to trade away the durability and ordering of the timeline for the ease of use of webhooks. There's also no reason to timeline this anymore since parsing is no longer order-dependent.
  - Add `phd start` to start all the daemons you need. Add `phd restart` to restart all the daemons you need. So cool~
  - Simplify and improve phd and Diffusion daemon documentation.

Test Plan:
  - Ran `phd start`.
  - Ran `phd restart`.
  - Generated/read documentation.
  - Imported some stuff, got clean parses.

Reviewers: btrahan, csilvers

Reviewed By: csilvers

CC: aran, jungejason, nh

Differential Revision: https://secure.phabricator.com/D2433
2012-05-09 10:29:37 -07:00
epriestley
907f1a3dee Raise a "you should upgrade your storage" note for a missing database
Summary: We raise an improved exception for missing tables/columns, but not databases.

Test Plan: Hit a "no such database error", got a better error message pointing me at storage upgrades.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2429
2012-05-09 10:01:53 -07:00
epriestley
ed34402313 "arc upgrade" (server changes)
Summary: When the server version is ahead of the client version, send a more exciting error!!!

Test Plan: omg~~~

Reviewers: nh, Makinde, btrahan, jungejason

Reviewed By: nh

CC: aran

Differential Revision: https://secure.phabricator.com/D2436
2012-05-09 10:01:46 -07:00
Aurelijus
872845cc6c Improve branch table view with history link & other features
Summary: Add history column & history link per branch on branch table view, also later add some more features like last commit date, etc.

Test Plan: Checked if History column is in browser table view & history links are properly linked.

Reviewers: epriestley

Reviewed By: epriestley

CC: davidreuss, aran, Koolvin

Maniphest Tasks: T1201, T1202, T1200

Differential Revision: https://secure.phabricator.com/D2432
2012-05-08 15:03:06 -07:00
epriestley
d2b01aead0 Use one daemon to discover commits in all repositories, not one per repository
Summary:
See D2418. This merges the commit discovery daemon into the same single daemon, and applies all the same rules to it.

There are relatively few implementation changes, but a few things did change:

  - I simplified/improved Mercurial importing, by finding full branch tip hashes with "--debug branches" and using "parents --template {node}" so we don't need to do separate "--debug id" calls.
  - Added a new "--not" flag to exclude repositories, since I switched to real arg parsing anyway.
  - I removed a web UI notification that you need to restart the daemons, this is no longer true.
  - I added a web UI notification that no pull daemon is running on the machine.

NOTE: @makinde, this doesn't change anything from your perspective, but it something breaks this is the likely cause.

This implicitly resolves T792, because discovery no longer runs before pulling.

Test Plan:

  - Swapped databases to a fresh install.
  - Ran "pulllocal" in debug mode. Verified it correctly does nothing (fixed a minor issue with min() on empty array).
  - Added an SVN repository. Verified it cloned and discovered correctly.
  - Added a Mercurial repository. Verified it cloned and discovered correctly.
  - Added a Git repository. Verified it cloned and discovered correctly.
  - Ran with arguments to verify behaviors: "--not MTEST --not STEST", "P --no-discovery", "P".

Reviewers: btrahan, csilvers, Makinde

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T792

Differential Revision: https://secure.phabricator.com/D2430
2012-05-08 12:53:41 -07:00
Bob Trahan
679f778235 OAuth -- add support for Disqus
Summary:
also fix some bugs where we weren't properly capturing the expiry value or scope of access tokens.

This code isn't the cleanest as some providers don't confirm what scope you've been granted. In that case, assume the access token is of the minimum scope Phabricator requires. This seems more useful to me as only Phabricator at the moment really easily / consistently lets the user increase / decrease the granted scope so its basically always the correct assumption at the time we make it.

Test Plan: linked and unlinked Phabricator, Github, Disqus and Facebook accounts from Phabricator instaneces

Reviewers: epriestley

Reviewed By: epriestley

CC: zeeg, aran, Koolvin

Maniphest Tasks: T1110

Differential Revision: https://secure.phabricator.com/D2431
2012-05-08 12:08:05 -07:00
epriestley
eb9645e9b4 When a user resigns from a commit they have authority over auditing projects for, write resign explicitly
Summary:
For most actions (like "accept"), we write a row only if you aren't acting on behalf of anything else. This avoids cases like every accept causing two relationships:

  Some Project | Accept
  Some User    | Accept

For "Resign", we must always write the row. Break the logic out and handle it separately.

Test Plan: Poked it locally, but let me know if this fixes things?

Reviewers: 20after4, btrahan

Reviewed By: 20after4

CC: aran

Differential Revision: https://secure.phabricator.com/D2423
2012-05-07 18:37:04 -07:00
epriestley
31142e6d0a Update arc documentation for D2424
Summary: Mention that you don't actually need an `.arcconfig` any more.

Test Plan: Read documentation.

Reviewers: btrahan, csilvers

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2425
2012-05-07 17:58:05 -07:00
epriestley
1c62a35710 Run one daemon to pull all working copies, not one daemon per working copy
Summary:
Allow the pull daemon to take a list of repositories. By default, pull all repositories.

Make some effort to respect pull frequencies, although we'll necessarily suffer a bit if running with only one process.

NOTE: We still launch one discovery daemon per working copy, so this only cuts the daemon count in half.

Test Plan:
  - Ran `phd debug pulllocal`, verified behavior.
  - Ran `pull.php P MTEST SVNTEST --trace`, verified it pulled the repos and ran the right commands.
  - Ran `phd repository-launch-master`, verified the right daemons launched, checked daemon console.
  - Ran `phd repository-launch-readonly`, verified the right daemon launched, checked daemon console.

Reviewers: btrahan, csilvers, davidreuss

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2418
2012-05-07 15:01:10 -07:00
Bob Trahan
8c6fa3e62d Conduit user.query
Summary: specify user table to make things not ambiguous

Test Plan: conduit console still works, including ID query...!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1075

Differential Revision: https://secure.phabricator.com/D2419
2012-05-07 13:47:06 -07:00
epriestley
758db327d3 Allow users to specify whether Maniphest custom fields should be copied or not when creating a "Similar Task"
Summary:
When you create a new task, the UI gives you the option to create another similar task. We copy some fields, but not others.

Currently, the field list is hard-coded and excludes auxiliary fields. Instead, allow auxiliary fields to elect to be copied.

Test Plan:
  - Created a new task, verified appropriate field defuaults.
  - Created a new "similar" task, verified 'copy' fields copied in.
  - Edited an existing task, verified appropriate values.
  - Edited-with-errors, verified new values didn't get reverted in the form.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1193

Differential Revision: https://secure.phabricator.com/D2410
2012-05-07 13:38:27 -07:00
epriestley
191f6d904f Fix a bug with "Resign" in Diffusion
Summary: The PHID list is a list, not a map -- I must have broken this in refactoring or something, since everything else works fine. See D2013.

Test Plan: Viewed a resignable revision, saw "Resign" (new after this commit), resignd.

Reviewers: 20after4, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D2417
2012-05-07 13:37:41 -07:00
Bob Trahan
c58bd95842 Conduit user.query method
Summary: also includes a PhabricatorPeopleQuery object

Test Plan: queried for various combinations username, realname, id, phid, and email. verified correct results.

Reviewers: epriestley

Reviewed By: epriestley

CC: zeeg, aran, Koolvin

Maniphest Tasks: T1075

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

Not included here (next steps):

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

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

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

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2393
2012-05-07 10:29:33 -07:00
epriestley
803dea1517 Make "user role" editing more clear
Summary: The various interfaces here are in conflict about what a role is and isn't. Make them all consistent.

Test Plan: Edited some users into various roles, verified they reported correctly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1190

Differential Revision: https://secure.phabricator.com/D2415
2012-05-07 10:25:36 -07:00
epriestley
2d9d2f1aed Allow "Differential Revision ID" field appear on create/edit messages
Summary:
  - In practice, 'edit' has two modes, 'create' and 'edit'. These seem like they should map to "create a revision" and "update a revision", but they are completely different.
    - We use the "create" mode:
      - When creating a message from the working copy.
      - When creating a message from a file.
      - When creating a message from a commit.
      - When creating a message from a user template.
      - When creating a message from an "--edit"!
    - We use the "edit" mode:
      - ONLY when updating a revision with `arc diff --verbatim`.
  - The only difference is in which fields may be overwritten. Under "create", all fields may be overwritten. Under "edit", only safe fields may be overwritten.
  - The "Differential Revision" field currently does not render in either edit mode. This is wrong. Even though it can not be updated in the "edit" mode, it should still render in both modes. This is the only material change this revision makes.
  - Without this change, when we "create" a new message from a working copy and the working copy has a "Differential Revision" field, we incorrectly discard it.
  - The only field which does not render on edit modes now is "Reviewed by" (not "Reviewers"), which is correct, since we do not read the value.

Test Plan: Ran "arc diff" to create/update revisions. Ran "arc diff --verbatim" to create/update revisions with implicit edits (with D2411). Ran "arc diff --edit" to update revisions with explicit edits.

Reviewers: jungejason, btrahan

Reviewed by: jungejason

CC: vrana, aran

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

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

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T875, T788

Differential Revision: https://secure.phabricator.com/D2385
2012-05-07 06:17:00 -07:00
epriestley
b80ea9e0e8 Revert cd63d9b2ce / D2403, see diff. This broke inline comments by making them not span enough columns. 2012-05-07 06:01:56 -07:00
epriestley
c30cb7669e Simplify transaction handling and restore read/write locks
Summary:
  - We used to have connection-level caching, so we needed getTransactionKey() to make sure there was one transaction state per real connection. We now cache in Lisk and each Connection object is guaranteed to represent a real, unique connection, so we can make this a non-static.
  - I kept the classes separate because it was a little easier, but maybe we should merge them?
  - Also track/implement read/write locking.
  - (The advantage of this over just writing LOCK IN SHARE MODE is that you can use, e.g., some Query class even if you don't have access to the queries it runs.)

Test Plan: Can you come up with a way to write unit tests for this? It seems like testing that it works requires deadlocking MySQL if the test is running in one process.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2398
2012-05-05 11:29:09 -07:00
vrana
cd63d9b2ce Use subtler highlighting for copied and moved code
Summary:
The highlighting is distracting according to Nick Shrock and others.
Real designer, Lee Byron, helped me with this.

It also gives us unagressive target for jumping to the source line in future.

Another feature I will probably implement is highlighting also the source of copies/moves.
I will use the right side of the left column for it.

Test Plan:
Hover copied notifier.
Hover coverage notifier.

I've also checked that this doesn't break our super-flaky old/new code JavaScript detector.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin, leebyron, schrockn

Differential Revision: https://secure.phabricator.com/D2403
2012-05-04 21:56:24 -07:00
vrana
1c5412ebe6 Unhighlight bright code in coverage
Summary:
It looks weird:
{F10953, size=full}

Test Plan: Hover coverage.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2401
2012-05-04 18:06:28 -07:00
epriestley
db20c74044 Add a missing (string) cast to repository.query
Summary: The method returns a PhutilURI object, which json_encode() encodes as "{}" since all the properties are private.

Test Plan:
Examined the output of "repository.query" more carefully:

  "0" : {
    "name"      : "Phabricator",
    "callsign"  : "P",
    "vcs"       : "git",
    "uri"       : "http:\/\/local.aphront.com:8080\/diffusion\/P\/",
    "remoteURI" : "git:\/\/github.com\/facebook\/phabricator.git",
    "tracking"  : true
  },

Reviewers: csilvers, btrahan, vrana, jungejason

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2402
2012-05-04 17:08:42 -07:00
epriestley
a122336b3e Try to diagnose App Login only for OAuth providers which support it
Summary: We currently try to do "app login" for all OAuth providers, but not all of them support it in a meaningful way. Particularly, it always fails for Google.

Test Plan: Ran google diagnostics on a working config, no longer got a diagnostic failure.

Reviewers: btrahan, vrana, csilvers

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2377
2012-05-04 16:16:29 -07:00
epriestley
049048765d Add "repository.create" and "repository.query" methods to Conduit
Summary: Primarily for @csilvers who has 92 million repositories or something. This is a touch hacky, but movitated by pragmatism.

Test Plan:
  - Ran "repository.create" to create repositories, "repository.query" to list them.
  - Tested most or maybe all of the error conditions, probably.

Reviewers: btrahan, vrana, csilvers

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2396
2012-05-04 16:16:22 -07:00
epriestley
e214536f38 Use "w" transactions for reading if they exist, so we can do transactional reads
Summary: Generally moves us toward having a sane approach to transaction handling.

Test Plan: See test case, which fails before this patch and passes afterwards.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2394
2012-05-04 13:25:53 -07:00
vrana
e4f4b6a3fe Add conduit method for deleting user status
Test Plan: All four eventualities.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2383
2012-05-04 13:03:02 -07:00
vrana
3fa310f3c0 Render shields before highlighting code
Summary:
It saves some time on non-highlighting generated and other not interesting code.

The code is quite complex (300 lines methods) so I'm not sure if everything is moved correctly.

P.S. I hope that moved code detector will work...

Test Plan:
Display generated file with all whitespace, verify that it is not highlighted.
Display normal file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1134

Differential Revision: https://secure.phabricator.com/D2358
2012-05-04 12:43:46 -07:00
vrana
8745374cb5 Mark optional conduit parameter 2012-05-04 00:33:00 -07:00
vrana
87ff461470 Add /F123 shortcut
Summary:
I wanted to point someone on a file uploaded to Phabricator and the normal link is just too long.

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

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

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

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

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2380
2012-05-03 23:51:40 -07:00
vrana
416e4e7b67 Allowing setting user status
Summary:
I will use it for highlighting users which are not currently available.

Maybe I will also use it in the nagging tool.

I don't plan creating a UI for it as API is currently enough for us.
Maybe I will visualize it at /calendar/ later.

I plan creating `user.deletestatus` method when this one will be done.

Test Plan:
`storage upgrade`
Call Conduit `user.addstatus`.
Verify DB.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2382
2012-05-03 18:24:30 -07:00
epriestley
9b2ededd48 Document configuration of file upload limits
Summary: I have a patch which makes uploads all fancy and adds progress bars, but document the landscape first since it's quite complicated.

Test Plan: Generated, read docs. Configured `storage.upload-size-limit` to various values.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T875

Differential Revision: https://secure.phabricator.com/D2381
2012-05-03 17:30:17 -07:00
Craig Silverstein
d6e2e4e4c5 Add docs for using lighttpd as the webserver.
lighttpd could support rules like this, to add efficiency:

    # Aggressively cache all static files
    $HTTP["url"] =~ "\.(jpg|gif|png|css|js|htc)" {
        expire.url = ( "" => "access 1 years" )
    }

    # Compress files for faster transfer
    compress.filetype = (
        "text/plain",
        "text/html",
        "text/javascript",
        "text/css",
        "text/xml"
    )

    compress.cache-dir = <would need to set to something>?

I don't know if that is necessary or useful.  Probably not a good idea
at this point, where the code is changing so rapidly: a 1 year cache
of javascript code could cause trouble.  And i think the default
lighttpd.conf already compresses text/html, text/plain, text/css, and
application/x-javascript by default, so we're ok there (could add
text/javascript and text/xml, I guess).
2012-05-03 13:42:35 -07:00
epriestley
6a04328430 Tighten scope requests with Google OAuth
Summary: We currently make a ludicrously gigantic permission request to do Google auth (read/write access to the entire address book), since I couldn't figure out how to do a more narrowly tailored request when I implemented it. @csilvers pointed me at some much more sensible APIs; we can now just ask for user ID, name, and email address.

Test Plan: Created a new account via Google Oauth. Linked/unlinked an existing account. Verified diagnostics page still works correctly. Logged in with a pre-existing Google account created with the old API (to verify user IDs are the same through both methods).

Reviewers: btrahan, vrana, csilvers, Makinde

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2378
2012-05-03 13:25:06 -07:00
vrana
631718f99e Implement getNthBusinessDay()
Summary: I will need it for nagging tool.

Test Plan:
None yet.
Please suggest me how to create a testing database (I need to insert some data in the table). I guess that it is now possible?
There is also probably some bug in `arc unit` - `setEnvConfig()` is not called before `getEnvConfig()` resulting in fatal error.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2376
2012-05-03 12:26:58 -07:00
vrana
45c662e4f7 Highlight disabled users in Remarkup
Test Plan:
  @btrahan
  @epriestley
  @xxx

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2374
2012-05-03 11:05:02 -07:00
vrana
73c82e5a94 Display holidays
Summary:
We will need it for two purposes:

- Status tool.
- Nagging tool - @aran suggested using "3 business days" and I don't want it to fall on New Year's Eve or such.

I don't plan working on any interface for editing this as this kind of data should be always imported.

Test Plan:
`bin/storage upgrade`
`scripts/calendar/import_us_holidays.php`
/calendar/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2375
2012-05-03 09:22:52 -07:00
vrana
826e5405b6 Open files in very large diffs inline
Summary: This allows writing inline comments and reduces different behavior between normal and very large diffs.

Test Plan:
Verify that normal diff works.
Verify that very large diff works.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2361
2012-05-02 17:51:35 -07:00
vrana
ec068ff453 Avoid fatal error if there's no Arcanist project or repository
Test Plan: Display diff without repository

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2373
2012-05-02 17:39:52 -07:00
vrana
fd3d510dc8 x
Summary:

Test Plan:

Reviewers:

CC:
2012-05-02 17:18:42 -07:00
Nick Harper
6039ca6fb5 Create option for differential custom fields to display warning on accept
Summary:
This adds support to differential fields to display warnings before a revision
gets accepted. Since lint and unit are differential fields, the code for their
warnings was moved into their respective field specification classes, so there
is only one code path for warnings (lint, unit, or custom).

Test Plan:
Select 'Accept' on a revision with lint/unit warnings and see messages appear
like they used to. Change it back to 'Comment' and they go away. Repeat with
a revision without lint/unit warnings and see no warnings appear. Checked
darkconsole to see no errors due to this.

Reviewers: jungejason, epriestley, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2363
2012-05-02 14:30:21 -07:00
vrana
81dd92fcdc Display Show Raw File link in Diffusion Change View
Summary: NOTE: `renderViewOptionsDropdown()` adds unnecessary parameters to URL but the link just redirects anyway.

Test Plan:
Show Raw File (Left and Right) in SVN and Git.
Verify also Added and Deleted files.

Reviewers: epriestley, aran

Reviewed By: epriestley

CC: Koolvin

Differential Revision: https://secure.phabricator.com/D2370
2012-05-02 14:18:35 -07:00
epriestley
63ce372480 Add "DiffusionRawDiffQuery"
Summary:
  - This is only slightly useful for updating Differential, since DiffQuery (vs RawDiffQuery) already gets you most of what you need. The only thing is that DiffQuery returns the diff for one path only right now(and the SVN version is very "special"). Should be easy to fix in the Git/HG cases at least, though (or maybe just use RawDiffQuery to avoid the SVN mess).
  - Added a "download raw diff" link.

Test Plan: Viewed Diffusion and raw commits for SVN, Mercurial and Git repositories.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2350
2012-05-02 13:43:45 -07:00
epriestley
5ab14d0879 Provide isolated, read/write storage fixtures for unit tests
Summary:
  - Unit tests can request storage fixtures.
  - We build one fixture across all tests in the process, which can quickstart (takes roughly 1s to build, 200ms to destroy for me). This is a one-time cost for running an arbitrary number of fixture-based tests.
  - We isolate all the connections inside transactions for each test, so individual tests don't affect one another.

Test Plan: Ran unit tests, which cover the important properties of fixtures.

Reviewers: btrahan, vrana, jungejason, edward

Reviewed By: btrahan

CC: aran, davidreuss

Maniphest Tasks: T140

Differential Revision: https://secure.phabricator.com/D2345
2012-05-02 12:42:23 -07:00
Nick Harper
56529f88e0 Show verb of closed diff based on backing vcs, not local vcs
Summary:
When choosing a verb to show with a closed differential revision, choose the
verb based on the upstream vcs, not the vcs used to create the diff, since these
are not the same thing. I also updated the documentation for the next step for
an accepted diff for the case where the local vcs and backing vcs aren't the
same (since arc land doesn't work for those).

Test Plan:
Loaded a committed diff and an accepted diff from fbcode and www to check that
they show the correct thing.

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1118

Differential Revision: https://secure.phabricator.com/D2360
2012-05-01 11:30:02 -07:00
vrana
b916722151 Index revision comments
Summary: Only inlines were indexed (contrary to what comment claims).

Test Plan: Index one revision, check database.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2359
2012-05-01 10:46:27 -07:00
epriestley
f04d8ab1a7 Further improve unit/lint rendering
Summary:
I think this improves things, let me know if you have feedback.

Also addresses T840.

Test Plan: See screenshots...

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran, zeeg

Maniphest Tasks: T840

Differential Revision: https://secure.phabricator.com/D2357
2012-05-01 10:15:56 -07:00
Bob Trahan
8988667dcc Make Oauth-registration flows a bit more resilient to failures from the providers
Summary: basically by validating we have good user data when we set the user data.

Test Plan: simulated a failure from a phabricator on phabricator oauth scenario. viewed ui that correctly told me there was an error with the provider and to try again.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1077

Differential Revision: https://secure.phabricator.com/D2337
2012-05-01 11:51:40 +02:00
Bob Trahan
458563c68a Phame - use post title for the actual web page title
Summary: 'cuz it looks dumb to use a URI slug

Test Plan: viewed a post liked the title

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2338
2012-05-01 11:50:51 +02:00
vrana
2f047dce43 Trim lines before detecting copied/moved code
Test Plan:
https://secure.phabricator.com/differential/diff/4043/
{F10761}

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2352
2012-04-30 17:00:32 -07:00
vrana
4cc61687aa Hide lint and unit error details
Summary:
Before: {F10754}

After: {F10753}

Test Plan:
View revision with lint warnings and unit errors.
Click on Details.
Click on Details.
Click on Details.
Click on Details.

Reviewers: asukhachev, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2351
2012-04-30 16:50:36 -07:00
vrana
4f9e5323ed Add image size to thumbnails in Remarkup
Test Plan:
View preview of comment with `{F10662}`.
Search for `>getThumb`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2347
2012-04-30 13:32:00 -07:00
epriestley
1dcc00646a Make "Commandeer" yellow
Summary: See T1178

Test Plan: {F10739}

Reviewers: nh, btrahan, vrana, jungejason

Reviewed By: nh

CC: aran

Maniphest Tasks: T1178

Differential Revision: https://secure.phabricator.com/D2346
2012-04-30 13:07:06 -07:00
vrana
ef5ed6647b Document image macro restriction
Summary: D2230

Test Plan: Read.

Reviewers: mkjones, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2343
2012-04-30 12:35:07 -07:00
epriestley
b27d1b3075 Minor, fix an issue created by D2341 + D2342 (namespace storage changed). 2012-04-30 12:14:37 -07:00
epriestley
307ad51404 Minor, use the same connection throughout applyPatchSQL() -- previously we hit caches, but no longer after D2342. 2012-04-30 12:10:35 -07:00
epriestley
a10b91a6a9 Move connection cache from "AphrontDatabaseConnection"-level to "LiskDAO"-level
Summary:
  - Currently, connections are responsible for connection caching. However, I want unit tests to be able to say "throw away the entire connection cache" with storage fixtures, and this is difficult/impossible when connections are responsible for the cache.
  - The only behavioral change is that previously we would use the same connection for read-mode and write-mode queries. We'll now establish two connections. No installs actually differentiate between the modes so it isn't particularly relevant what we do here. In the long term, we should probably check the "w" cache before building a new "r" connection, so transactional code which involves reads and writes works (we don't have any such code right now).

Test Plan: Loaded pages, verified only one connection was established per database. Ran unit tests.

Reviewers: btrahan, vrana, jungejason, edward

Reviewed By: vrana

CC: aran

Maniphest Tasks: T140

Differential Revision: https://secure.phabricator.com/D2342
2012-04-30 11:57:10 -07:00
epriestley
570feee199 Make default database namespace configurable
Summary: Allow the default namespace to be set in configuration, so you can juggle multiple copies of sandbox test data or whatever.

Test Plan: Changed default namespace, verified web UI and "storage" script respect it.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T345

Differential Revision: https://secure.phabricator.com/D2341
2012-04-30 11:56:58 -07:00
epriestley
c974cb3de8 Minor, futher sort out auth exceptions in Conduit + access log. 2012-04-30 11:18:19 -07:00
vrana
e08b4cbb2c Inform about moved code and prefer it over copied code
Summary:
Also reduce the memory usage a little bit (before increasing it again).

I use the same CSS class as for the copied code.

Test Plan: Parsed 100 diffs and checked about 10 of them - looks good.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2339
2012-04-30 11:01:15 -07:00
epriestley
087cc0808a Make SQL patch management DAG-based and provide namespace support
Summary:
This addresses three issues with the current patch management system:

  # Two people developing at the same time often pick the same SQL patch number, and then have to go rename it. The system catches this, but it's silly.
  # Second/third-party developers can't use the same system to manage auxiliary storage they may want to add.
  # There's no way to build mock databases for unit tests that need to do reads.

To resolve these things, you can now name your patches whatever you want and conflicts are just merge conflicts, which are less of a pain to fix than filename conflicts.

Dependencies are now a DAG, with implicit dependencies created on the prior patch if no dependencies are specified. Developers can add new concrete subclasses of `PhabricatorSQLPatchList` to add storage management, and define the dependency branchpoint of their patches so they apply in the correct order (although, generally, they should not depend on the mainline patches, presumably).

The commands `storage upgrade --namespace test1234` and `storage destroy --namespace test1234` will allow unit tests to build and destroy MySQL storage.

A "quickstart" mode allows an upgrade from scratch in ~1200ms. Destruction takes about 200ms. These seem like fairily reasonable costs to actually use in tests. Building from scratch patch-by-patch takes about 6000ms.

Test Plan:
  - Created new databases from scratch with and without quickstart in a separate test namespace. Pointed the webapp at the test namespaces, browsed around, everything looked good.
  - Compared quickstart and no-quickstart dump states, they're identical except for mysqldump timestamps and a few similar things.
  - Upgraded a legacy database to the new storage format.
  - Destroyed / dumped storage.

Reviewers: edward, vrana, btrahan, jungejason

Reviewed By: btrahan

CC: aran, nh

Maniphest Tasks: T140, T345

Differential Revision: https://secure.phabricator.com/D2323
2012-04-30 07:54:00 -07:00
epriestley
321b776148 Show README on repository screen in Diffusion
Summary:
  - Show README on the repository screen.
  - Move README to the bottom of the page for both repository and browse screens.
  - Support "README.rainbow".

Test Plan: Looked at repository, browse screens. Made a "README.rainbow".

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1104

Differential Revision: https://secure.phabricator.com/D2336
2012-04-30 07:47:41 -07:00
vrana
6fd99e287b Fix typos in detectCopiedCode()
Auditors: epriestley
2012-04-28 22:56:08 -07:00
vrana
13a48a79d7 Highlight copied/moved lines in Differential
Summary: The color used for this feature is pretty important and I am bad with colors.

Test Plan:
View diff created by D2320 with some copied lines and one line changed:

{F10604, size=full}

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2321
2012-04-28 21:46:31 -07:00
vrana
7affae9345 Detect copied code by own algorithm
Summary:
Required for D2321.
Deprecates D2320.
Uses algorithm described at D2320#16.

Complexity of this algorithm would be `O(N)` (`N` stands for number of lines) in most cases.
The worst case is `O(A*F)` (`A` stands for number of added lines, `F` for number of colliding lines) but it should be pretty rare. Real-world example is 100 modified files with moved license block (15 lines) in each. This will require 1500*100 comparisons because the algorithm will be trying to find the longest block in each file.

Test Plan:
`arc diff --only` on commit with copied code.
More tests on standalone algorithm.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2333
2012-04-28 21:39:31 -07:00
Jason Ge
6a9ef778fc Add "jump to table of content" keyboard shortcut
Summary:
Many times when I'm reading a big diff, I want to go to the
TOC. Add it.

Test Plan:
can navigate with 't'. It also shows up in '?'

Revert Plan:

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: nh, aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2335
2012-04-28 18:05:08 -07:00
epriestley
160ec660b0 Minor, fix an error when looking at a new, unparsed repository. 2012-04-28 16:37:41 -07:00
epriestley
7e9a2dc4d2 Clarify "Closed" revision status with VCS-specific language
Summary: Instead of rendering "Revision Status: Closed (Sat, Apr 28, 10:28 AM)", render "Revision Status: Closed (Pushed Sat, Apr 28, 10:28 AM)", or "Committed", as appropriate.

Test Plan: Looked at a committed revision.

Reviewers: btrahan, vrana, jungejason

Reviewed By: jungejason

CC: jeffmo, rdbayer, aran

Maniphest Tasks: T909

Differential Revision: https://secure.phabricator.com/D2334
2012-04-28 14:10:39 -07:00
epriestley
2909f0b06f Minor, fix access log issue with non-auth methods. 2012-04-28 13:08:13 -07:00
epriestley
07d8503647 Don't fatal on image macro list if a file has been deleted
Summary:
We added the ability to delete files a while ago, but this interface isn't happy about it.

I still render the macro so you can see/delete it, e.g.

Test Plan: Viewed a deleted macro page, got a page instead of an error. Also verified that the actual remarkup part doesn't have issues.

Reviewers: btrahan, vrana, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2328
2012-04-28 07:18:06 -07:00
epriestley
bf505ef51c Hard-code and unify "more applications" in Phabricator
Summary:
This is mostly intended to simplify D2323.

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

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

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

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

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

Reviewers: btrahan, vrana, edward, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T345

Differential Revision: https://secure.phabricator.com/D2332
2012-04-28 07:17:38 -07:00
epriestley
8ed48a89f4 Use a disk-based default avatar, not a database-based one
Summary:
This is mostly in an effort to simplify D2323. Currently, we load one image into the database by default. This is a weird special case that makes things more complicated than necessary.

Instead, use a disk-based default avatar.

Test Plan: Verified that a user without an image appears with the default avatar as a handle, in profile settings, and on their person page.

Reviewers: btrahan, vrana, edward, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T345

Differential Revision: https://secure.phabricator.com/D2331
2012-04-27 17:44:10 -07:00
vrana
d27a751339 Create overview of Conduit methods
Summary:
Also couple of small changes:

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

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

Reviewers: btrahan, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2326
2012-04-27 17:27:47 -07:00
vrana
11ffed7cfc Fix paging for 'before ID' queries
Test Plan: https://secure.phabricator.com/paste/filter/all/?before=157 - 158 is missing.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2330
2012-04-27 17:27:00 -07:00
epriestley
2bdde748d9 Fix DiffusionGitBrowseQuery to parse with "git config -l -f" instead of PHP ini parser
Summary: See discussion on rPc0aac8267dda74664acac5c93d9aeb5a0f9c4564.

Test Plan: Looked at externals/, got a correctly-behaving link.

Reviewers: vrana, davidreuss, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2317
2012-04-27 12:51:50 -07:00
epriestley
12cd0d0b67 Handle symbolic commit names better in Diffusion
Summary: We now allow symbolic commits, so let them through the pipeline.

Test Plan: {F10571}

Reviewers: davidreuss, btrahan, vrana

Reviewed By: davidreuss

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2315
2012-04-27 12:51:44 -07:00
vrana
38ffe45f8e Use committer date instead of author date for Git epoch
Summary:
This is somewhat controversial but push date is usually more useful than commit date (which can be for example a month before other people can see the commit).
We can also store both dates.

Test Plan:
  git log --pretty="%ct %at"

Reviewers: epriestley

Reviewed By: epriestley

CC: nh, aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2319
2012-04-26 16:25:56 -07:00
vrana
a0417d97c8 Add assert_instances_of()
Auditors: epriestley
2012-04-25 16:23:06 -07:00
vrana
b44f687ce2 Fix link to submodule's history
Test Plan:
https://secure.phabricator.com/diffusion/P/browse/master/externals/
Click on History of `javelin/`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2316
2012-04-25 10:32:44 -07:00
vrana
05718706ab Add comments 2012-04-25 09:57:06 -07:00
epriestley
3ce69b6306 Allow Phabricator to write an access log using PhutilDeferredLog
Summary: Provide a configurable access log.

Test Plan:
Got a sensible-looking log including logged-in, logged-out, conduit, 404, etc:

  [Mon, 23 Apr 2012 20:08:12 -0700]	32599	orbital	-	epriestley	DifferentialCommentPreviewController	-	/differential/comment/preview/42/	http://local.aphront.com:8080/D42	200	65406
  [Mon, 23 Apr 2012 20:08:12 -0700]	32881	orbital	-	epriestley	DifferentialChangesetViewController	-	/differential/changeset/	http://local.aphront.com:8080/D42	200	72669
  [Mon, 23 Apr 2012 20:08:39 -0700]	32882	orbital	127.0.0.1	epriestley	DifferentialRevisionListController	-	/differential/	http://local.aphront.com:8080/D42	200	106444
  [Mon, 23 Apr 2012 20:08:54 -0700]	32867	orbital	127.0.0.1	epriestley	DifferentialRevisionListController	-	/differential/	http://local.aphront.com:8080/differential/	200	112229
  [Mon, 23 Apr 2012 20:09:05 -0700]	32530	orbital	127.0.0.1	epriestley	PhabricatorDirectoryMainController	-	/	http://local.aphront.com:8080/differential/	200	141350
  [Mon, 23 Apr 2012 20:09:10 -0700]	32598	orbital	127.0.0.1	epriestley	PhabricatorDirectoryCategoryViewController	-	/directory/6/	http://local.aphront.com:8080/	200	43474
  [Mon, 23 Apr 2012 20:09:12 -0700]	32880	orbital	127.0.0.1	epriestley	PhabricatorConduitConsoleController	-	/conduit/	http://local.aphront.com:8080/directory/6/	200	139340
  [Mon, 23 Apr 2012 20:09:15 -0700]	32868	orbital	127.0.0.1	epriestley	PhabricatorConduitAPIController	arcanist.projectinfo	/api/arcanist.projectinfo	http://local.aphront.com:8080/conduit/	200	128774
  [Mon, 23 Apr 2012 20:10:04 -0700]	32599	orbital	127.0.0.1	epriestley	Phabricator404Controller	-	/asdbmabdmbsm	-	404	38782
  [Mon, 23 Apr 2012 20:10:04 -0700]	32881	orbital	127.0.0.1	-	CelerityResourceController	-	/res/c9a43002/rsrc/css/aphront/request-failure-view.css	http://local.aphront.com:8080/asdbmabdmbsm	200	25160
  [Mon, 23 Apr 2012 20:10:57 -0700]	32882	orbital	127.0.0.1	epriestley	PhabricatorLogoutController	-	/logout/	http://local.aphront.com:8080/asdbmabdmbsm	200	40810
  [Mon, 23 Apr 2012 20:10:57 -0700]	32867	orbital	127.0.0.1	-	PhabricatorLoginController	-	/login/	http://local.aphront.com:8080/asdbmabdmbsm	200	42526
  [Mon, 23 Apr 2012 20:10:59 -0700]	32919	orbital	127.0.0.1	-	PhabricatorLoginController	-	/login/	http://local.aphront.com:8080/asdbmabdmbsm	200	49052
  [Mon, 23 Apr 2012 20:10:59 -0700]	32880	orbital	127.0.0.1	-	CelerityResourceController	-	/res/c80156c4/rsrc/js/application/core/behavior-dark-console.js	http://local.aphront.com:8080/login/	200	33166
  [Mon, 23 Apr 2012 20:10:59 -0700]	32868	orbital	127.0.0.1	-	CelerityResourceController	-	/res/4965d970/rsrc/css/aphront/dark-console.css	http://local.aphront.com:8080/login/	200	38078
  [Mon, 23 Apr 2012 20:10:59 -0700]	32599	orbital	127.0.0.1	-	CelerityResourceController	-	/res/pkg/8a5de8a3/javelin.pkg.js	http://local.aphront.com:8080/login/	200	40534
  [Mon, 23 Apr 2012 20:10:59 -0700]	32882	orbital	127.0.0.1	-	CelerityResourceController	-	/res/pkg/9c4e265b/core.pkg.css	http://local.aphront.com:8080/login/	200	41262
  [Mon, 23 Apr 2012 20:10:59 -0700]	32881	orbital	127.0.0.1	-	CelerityResourceController	-	/res/pkg/0c96375e/core.pkg.js	http://local.aphront.com:8080/login/	200	43720
  [Mon, 23 Apr 2012 20:10:59 -0700]	32921	orbital	127.0.0.1	-	CelerityResourceController	-	/res/caa86a45/rsrc/js/javelin/core/init.js	http://local.aphront.com:8080/login/	200	47566
  [Mon, 23 Apr 2012 20:10:59 -0700]	32867	orbital	127.0.0.1	-	CelerityResourceController	-	/res/f46289e9/rsrc/js/application/core/behavior-error-log.js	http://local.aphront.com:8080/login/	200	29328
  [Mon, 23 Apr 2012 20:10:59 -0700]	32919	orbital	127.0.0.1	-	CelerityResourceController	-	/res/7e62ff40/rsrc/image/phabricator_logo.png	http://local.aphront.com:8080/login/	200	25583
  [Mon, 23 Apr 2012 20:10:59 -0700]	32880	orbital	127.0.0.1	-	CelerityResourceController	-	/res/8c6200d3/rsrc/image/sprite.png	http://local.aphront.com:8080/login/	200	29829
  [Mon, 23 Apr 2012 20:11:01 -0700]	32868	orbital	127.0.0.1	-	PhabricatorOAuthLoginController	-	/oauth/facebook/login/  http://local.aphront.com:8080/login/	200	855931
  [Mon, 23 Apr 2012 20:11:02 -0700]	32882	orbital	127.0.0.1	epriestley789	PhabricatorLoginValidateController	-	/login/validate/	http://local.aphront.com:8080/login/	200	29793
  [Mon, 23 Apr 2012 20:11:02 -0700]	32881	orbital	127.0.0.1	epriestley789	PhabricatorDirectoryMainController	-	/	http://local.aphront.com:8080/login/	200	91638

Reviewers: jungejason, btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2310
2012-04-25 07:24:08 -07:00
epriestley
284ee03919 Show refs in Diffusion for Git commits
Summary: In Diffusion, for Git, show commit refs (like "origin/master, origin/HEAD").

Test Plan: Looked at several Git, Hg and SVN commits.

Reviewers: davidreuss, btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2311
2012-04-25 07:21:03 -07:00
vrana
0732f2f9dd Hide selection to provide a visual cue about clipboard JS magic in Differential
Summary: Inspired by D2242.

Test Plan:
Select text in left pane.
Select text in right pane.
Select all.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2249
2012-04-24 18:53:48 -07:00
vrana
44bbdec9ac Improve elasticsearch
Summary: I thought that this will be fun but the elasticsearch API is horrible and the documentation is poor.

Test Plan:
Search for:

- string
- author
- author, owner
- string, author
- open
- string, open, author
- string, exclude
- several authors, several owners
- nothing
- probably all other combinations

Normally, such an exhaustive test plan wouldn't be required but each combination requires a completely different query.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, Koolvin, btrahan

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

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

Reviewers: btrahan, vrana, davidreuss, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2290
2012-04-23 18:36:25 -07:00
epriestley
e9bd842227 Allow "blame previous revision" to track file moves and handle edge cases
Summary:
  - Track + message through file moves.
  - Stop + message on file create.
  - Stop + message on first commit.

Test Plan:
  - Tested blaming through a move, through a create, and through the first commit.
  - Verified this doesn't break anything in SVN / Mercurial.

Reviewers: vrana, btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1091

Differential Revision: https://secure.phabricator.com/D2295
2012-04-23 18:16:38 -07:00
epriestley
20a5c9b261 Use "closed", not "committed", in Differential
Summary: "Committed" is SVN-specific language, and confusing in Git and Mercurial. Use neutral language instead.

Test Plan: Inspection.

Reviewers: btrahan, Makinde, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T909

Differential Revision: https://secure.phabricator.com/D2087
2012-04-23 17:40:57 -07:00
David Reuss
42b1c73f41 Allow CC's/Auditors added to audits
Test Plan:
Added CC's/Auditors, clicked the form elements, and saw correct
behaviour. Verified that metadata was present in the detail table.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, 20after4, Koolvin

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D2002
2012-04-23 13:50:25 -07:00
Bob Trahan
8069694640 Fix next_uri
Summary: we were using the "path" as the next_uri and that drops some delicious get parameters

Test Plan: see T1140; basically re-ran the steps listed there and they passed!

Reviewers: epriestley, njhartwell

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1140, T1009

Differential Revision: https://secure.phabricator.com/D2299
2012-04-22 20:39:54 +02:00
vrana
c53dd9968b Fix search select after D790
Auditors: epriestley
2012-04-21 00:28:13 -07:00
epriestley
18eac227c0 Minor, fix extra comma in SQL.
Auditors: vrana
2012-04-20 15:43:04 -07:00
epriestley
bbe2063443 [NO CLUE WHAT I'M DOING] Add an Elasticsearch engine
Summary:
I have no idea what I'm doing, but here's part of an elasticsearch engine. These things work:

  - Indexing stuff (??)
  - Searching for text/type?
  - Reconstructing things??

All the complicated stuff doesn't work. I'm having a hard time figuring out the best way to model things because elasticsearch's documentation is not exactly the most complete or illuminating.

@amckinley, does this look sane-ish so far? Particularly, the /phabricator/<type>/<phid>/ URI scheme and how I've set up the relationships and fields in the documents?

How should I model the relationship and field queries? I want, like, an "equal" query but it seems like I've got "text" or "term" to work with and neither are exact match? And "term" doesn't consider PHIDs to be terms since they have hyphens in them?

I'll keep kind of slogging my way forward here but if you have valuable wisdom to share it would probably get me to a better end state much faster. The whole query construction phase is pretty much black magic to me.

Test Plan: nyancat

Reviewers: amckinley, vrana

Reviewed By: vrana

CC: jungejason, tuomaspelkonen, aran, 20after4, vrana

Differential Revision: https://secure.phabricator.com/D790
2012-04-20 15:33:09 -07:00
vrana
6cf61980d2 Analyze fulltext search in DarkConsole
Test Plan: /search/.../?__analyze__=1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2289
2012-04-20 10:58:17 -07:00
vrana
c2e64110bb Fix typo in comment 2012-04-19 22:26:27 -07:00
vrana
549a86cd96 Add sex
Summary:
We will need it for intl.

I've put it to User instead of UserProfile to be easier accessible.

Test Plan:
Apply SQL patch.
Change sex to Male.
Change sex to Unknown.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2287
2012-04-19 16:05:25 -07:00
vrana
9b8eaa5fd5 Document enum database type
Test Plan: Read.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2286
2012-04-19 16:04:37 -07:00
vrana
1f2cf78c1b Display committed date in Revision Status field
Summary:
This is slightly more complicated for this reason:

- We don't set `dateCommitted` for normal commits, only for markcommitted.
-- We need to add this date to old revisions now.

Test Plan:
Reparse a revision - commit date was set.
Conduit `markcommitted` - commit date was set.
Run SQL script.
Display closed revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2282
2012-04-19 15:05:09 -07:00
vrana
d9ce80aa17 Explicitly set URL parameter separator for external URLs
Summary:
PHP has this crazy [[ http://php.net/arg_separator.output | arg_separator.output ]] INI setting which allows setting different string for URL parameters separator instead of `&` (e.g. in `?a=1&b=2`).

Don't use it for external URLs.

Test Plan: Log in through OAuth.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2284
2012-04-19 14:54:47 -07:00
vrana
9fd3958567 Fix typo 2012-04-19 14:13:07 -07:00
vrana
0141bb8ee4 Allow/fix sending forms to new tab or window
Test Plan:
Ctrl+click on Show Diff in Chrome - button is not grayed-out, new tab is opened.
Click on it - button is grayed out.
Repeat in Firefox.

Reviewers: epriestley, tuomaspelkonen

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1137

Differential Revision: https://secure.phabricator.com/D2278
2012-04-19 09:49:45 -07:00
epriestley
6cdbfe860f Remove getFileType() and getChangeType() from DifferentialChangeset
Summary:
These are explicit copies of implicitly-generated Lisk methods.

See brief discussion in rPdec8bac3a3af6065166d485db80fffa70dc2abe3.

Test Plan: Looked at a diff in Differential.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2283
2012-04-19 09:45:50 -07:00
epriestley
944049d871 Add a paginated list of all repository tags to Diffusion
Summary: Now supports more than 25 tags!

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

Reviewers: davidreuss, vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2268
2012-04-19 09:39:19 -07:00
vrana
9be054443f Handle refreshing profile image with expired OAuth token
Summary:
If OAuth token is expired then refreshing profile image doesn't work.
This diffs solves it this way:

- Hide Refresh Profile Image button with expired token.
- Display Refresh Token with expired token.
- Update token after logging-in.

Test Plan:
Wait until token expires.
/settings/page/facebook/ - no Refresh Profile Image button.
Refresh Token.
Refresh Profile Image.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: michalburger1, aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2281
2012-04-19 09:30:25 -07:00
epriestley
bdcba43f21 Formalize a mechanism for marking Conduit methods deprecated/unstable
Summary:
This is better than writing "(UNSTABLE!!!)" in front of the text description.

I'll add a wiki to keep track of API changes, too.

See also D2087, which motivates this.

Test Plan: Browsed console, saw "deprecated" and "unstable" on appropriate methods.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T909

Differential Revision: https://secure.phabricator.com/D2271
2012-04-18 14:25:27 -07:00
vrana
bddcf288d8 Format metrics and parents in XHProf Profile
Test Plan: /xhprof/profile/PHID-FILE-.../?symbol=queryfx_all

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

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

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2275
2012-04-18 13:57:31 -07:00
vrana
3c22bebc9b Allow disabling JX.Workflow with keyboard modifier
Summary:
Ctrl+click opens the link to a new tab in most browsers.
Shift+click to a new window.
Alt+click or Meta+click downloads the target.

This diff respects these conventions by disabling JX.Workflow for these modifiers.

Test Plan:
Click Flag Task - inline dialog.
Ctrl+click Flag Task - new tab with standalone dialog.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2276
2012-04-18 13:47:47 -07:00
Jason Ge
401ac19aa6 Reorder the daemon console's display of the panels
Summary:
move up the panels which generally has short length.

Test Plan:
view the page.

Reviewers: blair, vrana

Reviewed By: vrana

CC: epriestley, aran

Differential Revision: https://secure.phabricator.com/D2228
2012-04-18 13:47:08 -07:00
vrana
81244015c1 Display author in paste detail
Summary: This is not very nice.

Test Plan: /P1

Reviewers: codeblock

Reviewed By: codeblock

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2267
2012-04-18 11:46:59 -07:00
vrana
380c70fcd5 Fix counterexample CSS after D2067 2012-04-18 11:43:57 -07:00
vrana
c9311c3ced Indicate buttons disabled in JX.Workflow
Summary:
We are marking disabled buttons with 'disabled' class in `behavior-form`.
But we ignore `JX.Workflow` there because it has its own handling.
But this handling doesn't set class so the button is disabled but it is not indicated to user.

It causes troubles in Clowncopterize where users report that browser freezes before doing anything after clicking it. It probably happens also on other places.

This diff solves it by using CSS3 selector on attribute (contrary to explicitly setting class in JX.Workflow).

Test Plan:
Add `sleep(3)` to `DifferentialCommentSaveController`.
Clowncopterize empty comment.

Reviewers: epriestley, tuomaspelkonen

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2264
2012-04-18 11:04:42 -07:00
vrana
1050bf383c Document database schema
Summary:
Please review carefully, me not very well on English.

I am just guessing what were the design decisions in most parts of this document.
Feel free to correct me or add more information.

Test Plan:
`diviner .`
/docs/
/docs/article/Database_Schema.html
Copy the text to Word and proofread.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: nh, jungejason, aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2258
2012-04-18 10:57:41 -07:00
vrana
aaf344cd02 Don't process links in Remarkup literal block
Test Plan:
  %%%Text that won't be processed by remarkup
  [[http://www.example.com | example]]
  %%%

> http://www.phabricator.com/docs/phabricator/article/Remarkup_Reference.html#literal-blocks

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2266
2012-04-18 10:48:20 -07:00
vrana
7e571994bc Don't display inline comment's Reply in standalone view
Summary: NOTE: This is starting to be too hacky.

Test Plan:
View revision with inline diffs, verify that Reply is there.
View standalone - no Reply.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2263
2012-04-18 10:44:24 -07:00
epriestley
dec8acd803 Add very basic tag support to Diffusion
Summary: Lists the 25 most recent tags on the "Repository" page.

Test Plan: Looked at a git repository with a tag, saw it. Looked at HG/SVN repos, they didn't break.

Reviewers: davidreuss, 20after4, btrahan, vrana, jungejason

Reviewed By: davidreuss

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2255
2012-04-18 08:02:08 -07:00
vrana
091248ebe6 Link to "large" view from inline comment for previous revision
Summary: Also link to `D1?id=` instead of `?id=` because some IE versions linked to root in this case.

Test Plan: Click on old diff's inline comment link on large revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2260
2012-04-17 20:16:01 -07:00
vrana
752d742085 Fix copy/paste error from D2227
Auditors: epriestley
2012-04-17 16:24:49 -07:00
epriestley
68d90ef698 Allow revisions to be commandeered
Summary:
  - Adds "Commandeer Revision", to allow you to plunder revisions from those lost to sea (e.g., interns who have left or co-workers who are dealing with a family emergency).
  - Removes admin-abandon to simplify things, since you can just Commandeer + Abandon now.
  - There are other workarounds available but this is the natural/expected workflow (and the one everyone always asks for) and there's no real reason not to allow it.

Test Plan: Swashbuckled.

Reviewers: cpiro, btrahan, vrana, jungejason

Reviewed By: cpiro

CC: aran, zeeg

Differential Revision: https://secure.phabricator.com/D2257
2012-04-17 14:59:31 -07:00
vrana
74cdad29d0 Don't save highlithing errors to cache
Summary:
If I have Pygments enabled in config but `pygmentize` doesn't work then unhighlighted source is stored to cache.
If I later make `pygmentize` work then the unhighlighted source is still loaded from the cache.

Test Plan:
Break `pygmentize`.
View a diff with JS files.
Fix `pygmenize`.
View the diff again.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, gatos99, Koolvin

Differential Revision: https://secure.phabricator.com/D2227
2012-04-17 12:48:27 -07:00
vrana
1f1c7a34b7 Improve image macros
Summary:
Couple of small improvements:

- Delete `randomon` macro.
- Make name unique (deleting current conflicts randomly).
- Image macro must be alone on the line.
- Filter by name.

Test Plan:
Run SQL.
/file/macro/
/file/macro/?name=imagemacro
Try to create conflicting name.
Write this comment:

  Test imagemacro.
  imagemacro

Reviewers: aran, epriestley

Reviewed By: epriestley

CC: epriestley, Koolvin

Differential Revision: https://secure.phabricator.com/D2230
2012-04-17 12:16:58 -07:00
20after4
1ff68376f5 New maniphest event TYPE_MANIPHEST_DIDEDITTASK
Summary:
This event is fired after a task is created and assigned with an id.
Use case is sending an email notification to everyone in a project when a new task is
submitted to said project.

Test Plan:
Implement the event listener, submit a new task to a project, see if the project members
receive an email notification. I will submit the event handler in a separate diff once it's a bit
prettier and tested more thoroughly.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, jungejason

Differential Revision: https://secure.phabricator.com/D2159
2012-04-17 12:09:58 -07:00
vrana
6929ac539e Print Maniphest tasks in Differential e-mail
Test Plan: Send diff with attached task, read e-mail.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1128

Differential Revision: https://secure.phabricator.com/D2256
2012-04-17 11:43:15 -07:00
vrana
6c0e22b3a3 Support IE8
Test Plan:
View revision in Chrome.
View revision in Firefox.
View revision in IE8.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2251
2012-04-17 11:01:30 -07:00
vrana
dd7087f4db Don't send empty testplan in e-mail
Summary: Allowed after D2193.

Test Plan: Disable `differential.require-test-plan-field`, create diff without test plan.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2252
2012-04-17 10:54:36 -07:00
epriestley
fbfccf5ddc Improve Policy options
Summary:
  - Add an "Administrators" policy.
  - Allow "Public" to be completely disabled in configuration.
  - Simplify unit tests, and cover the new policies.

Test Plan: Ran unit tests.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D2238
2012-04-17 07:52:10 -07:00
epriestley
0f06287dc5 Allow PhabricatorEnv to be temporarily made mutable for unit tests
Summary: Introduces a scope-guarded way to override the env config, for unit tests which are sensitive to config values.

Test Plan: Ran unit tests.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2237
2012-04-17 07:52:01 -07:00
epriestley
9531496d66 Add "X-Auto-Response-Suppress" header to all outgoing mail
Summary: This appears to sometimes be effective (for MS clients), and we've seen it in the wild on inbound mail.

Test Plan: Sent myself some mail, verified it had the right header.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T571

Differential Revision: https://secure.phabricator.com/D2241
2012-04-17 06:35:28 -07:00
epriestley
3f6f8421b2 Update package definitions
Summary: So much Javascript. :(

Test Plan: Verified major Diffusion, Differential, Maniphest interfaces pull in no unpackaged CSS/JS. Rebuilt Celerity map.

Reviewers: btrahan, vrana, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2248
2012-04-16 17:51:30 -07:00
epriestley
5f0d8e09d8 Use "user-select: none" to provide a visual cue about copy/paste JS magic
Summary:
  - For line numbers, use "user-select: none" to make them unselectable. This provides a stronger visual cue that copy/paste is enchanted.
  - In Paste, make it look sensible again after the blame-on-blame refactor in Diffusion. See also TODO to share this code formally.
  - In Diffusion, use the "phabricator-oncopy" behavior.

NOTE: I left blame/commit columns selectable in Diffusion, since you might reasonably want to copy/paste them?

NOTE: In Differential, the left side of the diff still highlights, even though it will be copied only if you select part of a line on the left and nothing else. But this seemed like a reasonable behavior, so I left it.

Test Plan:
  - Looked at Paste. Saw a nice line number column. Selected text, got the expected selection. Copied text, got the expected copy.
  - Looked at Diffusion. Saw a nice line number column, still. Selected text, got expected selection. Copied text, got expected copy.
  - Looked at Differential. Highlighted stuff, got expected results. Copied stuff, got expected results.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1123

Differential Revision: https://secure.phabricator.com/D2242
2012-04-16 15:55:16 -07:00
vrana
c3635e4464 Hide oncopy <pre> in Firefox
Test Plan:
Comment `JX.DOM.remove(pre)` to better see the problem in old code.
Apply this diff and verify that the pre is not shown.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2245
2012-04-16 15:03:15 -07:00
epriestley
283fee7c0a Actually (???) fix mail multiplexing. I am awful at programming. 2012-04-16 10:36:38 -07:00
epriestley
2353af7464 Fix missing whitespace in rP23f25edd97f052ff4c1c5d8c4be962b4da149bca
Summary: See rP23f25edd97f052ff4c1c5d8c4be962b4da149bca.

Test Plan: RAN LINT AND UNIT TESTS. VERIFIED THERE ARE NO SYNTAX ERRORS.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2240
2012-04-15 21:15:58 -07:00
epriestley
23f25edd97 Derp derp. 2012-04-15 10:18:23 -07:00
epriestley
3bbba619ed Derp. 2012-04-15 10:14:22 -07:00
epriestley
3a1928215b Minor, don't use private "reply-to" if not enabled, even if multiplexing is forced. 2012-04-15 10:03:11 -07:00
vrana
ced84470a9 Move invalid session error to login page
Summary:
With invalid session (which happens for me when I change production and dev db but can of course happen in other cases), Phabricator displays an ugly unhandled exception dialog suggesting to logging in again.
But there's no login dialog on that page.

This also changes how users with invalid session are treated on pages not requiring logging.
Previously, an exception was thrown on them. Now they are treated as unlogged users.

Test Plan: Corrupt session, go to /, login.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2236
2012-04-14 22:19:04 -07:00
vrana
d17be1d824 Fix SVN commit change parser for files moved from deleted directory
Summary:
This continues work started at D2215.
Files moved from deleted directory were marked as Copied Here instead of Moved Here.

Test Plan: Reparsed two commits which was previously wrong, now correct.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1114

Differential Revision: https://secure.phabricator.com/D2229
2012-04-14 22:18:27 -07:00
vrana
ef990703fa Fix SVN commit change parser for directories copied from the same path
Summary: See [[ https://secure.phabricator.com/D2215?id=3773#inline-2451 | D2215#inline-2451 ]].

Test Plan: Reparsed commit which was wrong, now correct.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2232
2012-04-14 21:46:02 -07:00
vrana
cec641a81e Don't mark diff as "Unit Tests Skipped" when posponed test was skipped
Summary:
Following logic at https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/unit/ArcanistUnitWorkflow.php;ea0f737e85644d5a$242-255.

Also unrequire coverage.

Test Plan:
  echo '{"diff_id": 1, "file": "empty", "name": "name", "result": "skip", "message": "test"}' | arc call-conduit differential.updateunitresults

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, epriestley

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

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

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

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

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

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D2210
2012-04-14 10:13:29 -07:00
Bob Trahan
6be9f6f3a8 Make Maniphest Transaction preview tokenizer aware
Summary:
...pretty sure the JS is too hack-tastic but it works...! :D

also fixed a small error from assert_instances_of change where a null value is all errors and what have you

Test Plan: played around with tasks in firefox and safari. made cc, owner, and project changes, as well as priority, etc.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1074

Differential Revision: https://secure.phabricator.com/D2234
2012-04-14 07:05:58 -07:00
Bob Trahan
304948f039 some fixes for code layout doc
Summary: use arc layout, include about the __tests__ folder, upsell unit testing

Test Plan: read the docs!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2235
2012-04-14 07:05:29 -07:00
Bob Trahan
5b32a19a08 fixing a typo
Test Plan: verus is wrong; versus is correct

Reviewers: vrana

CC:
2012-04-13 09:51:52 -07:00
Bob Trahan
9bfb28253e Add a phabricator code layout doc
Summary: tried to cover the basics and sprinkle in lots of class references, etc. would really appreciate feedback...!  :D

Test Plan: read the docs!

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T359

Differential Revision: https://secure.phabricator.com/D2223
2012-04-13 09:49:12 -07:00
vrana
4af3bb9f4b Allow View Standalone in Diff preview
Test Plan: Click View Standalone.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2225
2012-04-12 22:35:13 -07:00
vrana
50e3114896 Link to TOC in very large diff link
Test Plan: Visit the link.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2224
2012-04-12 22:04:13 -07:00
Bob Trahan
4e97491ee3 Various documentation updates focused around surfacing feedback article
Summary:
missed doing this for phame, so i poked around a bit and added it to
similar verbage as well as to a few "Next Steps" where I thought the feedback
might be provocative.

Test Plan: read the docs

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2220
2012-04-12 19:09:33 -07:00
vrana
17b0277ec5 Fix SVN commit change parser for files moved from deleted directory
Summary: This is not perfect. Moved files are reported as deleted but I'm happy with it.

Test Plan: Reparsed two commits which was previously wrong, now semi-correct.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1114

Differential Revision: https://secure.phabricator.com/D2215
2012-04-12 17:47:19 -07:00
epriestley
9a29107d01 Properly detect InnoDB setups which are "NO" or "DISABLED"
Summary: See D2160, http://dev.mysql.com/doc/refman/5.5/en/show-engines.html

Test Plan: Ran setup.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

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

V1 feature set

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

Please do toss out any must have features or changes.

Test Plan: played around with this bad boy like whoa

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, vrana

Maniphest Tasks: T1111

Differential Revision: https://secure.phabricator.com/D2202
2012-04-12 13:09:04 -07:00
vrana
3d6b8bff34 Fix reticle with edit inline comment
Test Plan:
- Hover left comment on diff of diff.
- Hover right comment on diff of diff.
- Hover inline comment preview.
- Hover inline comment edit.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1076

Differential Revision: https://secure.phabricator.com/D2213
2012-04-12 11:42:50 -07:00
epriestley
c458768415 Fix various threading issues, particularly in Gmail
Summary:
  - Add an explicit multiplexing option, and enable it by default. This is necessary for Mail.app to coexist with other clients ("Re:" breaks outlook at the very least, and generally sucks in the common case), and allows users with flexible clients to enable subject variance.
  - Add an option for subject line variance. Default to not varying the subject, so mail no longer says [Committed], [Closed], etc. This is so the defaults thread correctly in Gmail (not entirely sure this actually works).
  - Add a preference to enable subject line variance.
  - Unless all mail is multiplexed, don't enable or respect the "Re" or "vary subject" preferences. These are currently shown and respected in non-multiplex cases, which creates inconsistent results.

NOTE: @jungejason @nh @vrana This changes the default behavior (from non-multiplexing to multiplexing), and might break Facebook's integration. You should be able to keep the same behavior by setting the options appropriately, although if you can get the new defaults working they're probably better.

Test Plan:
Send mail from Maniphest, Differential and Audit. Updated preferences. Enabled/disabled multiplexing. Things seem OK?

NOTE: I haven't actually been able to repro the Gmail threading issue so I'm not totally sure what's going on there, maybe it started respecting "Re:" (or always has), but @cpiro and @20after4 both reported it independently. This fixes a bunch of bugs in any case and gives us more conservative set of defaults.

I'll see if I can buff out the Gmail story a bit but every client is basically a giant black box of mystery. :/

Reviewers: btrahan, vrana, jungejason, nh

Reviewed By: btrahan

CC: cpiro, 20after4, aran

Maniphest Tasks: T1097, T847

Differential Revision: https://secure.phabricator.com/D2206
2012-04-12 09:31:03 -07:00
Aizat Faiz
f0e89b7723 Fix typo 'retrive' to 'retrieve'. 2012-04-12 17:17:30 +08:00
vrana
0ad7a01b8d Add typehints to queryfx() et al.
Test Plan: /D123

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2203
2012-04-11 12:02:11 -07:00
epriestley
cd2bca664c Detect alternate Danish outlook reply pattern
Summary: Sometimes we get a lowercase "Meddelelse" in Danish outlook. Relax the patterns since the risk of hitting false positives here is essentially nonexistant.

Test Plan: Unit tests.

Reviewers: davidreuss, btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2205
2012-04-11 10:31:04 -07:00
Edward Speyer
8f70d891fa AphrontFormSelectControl: Add <optgroup> to <select>
Summary:
Add <optgroup> style selects, if the array of options is actually an
array-of-arrays.

Test Plan: Made one, it looked OK.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2177
2012-04-10 21:35:51 -07:00
vrana
dea4901bb6 Unrequire filling Owners in Owners tool
Summary:
Owners field is filled by Primary Owner which is required.
So that it is not neccessary to require filling Owners explicitly.

Test Plan: Don't fill Owners and successfully save the form //before// this change.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2201
2012-04-10 21:30:56 -07:00
epriestley
2360504462 Fix null test plan database error
Summary:
Some Differential fields are not nullable; when Test Plan is switched to non-required mode we can end up trying to save a null value to a non-nullable column (see D2193).

(I should probably just alter the schema to make these fields nullable, but that might have farther-reaching effects.)

Test Plan: Reproduced error, applied patch, no more error.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2200
2012-04-10 16:54:05 -07:00
vrana
01bd844926 Display cursor hand on line number in revision but not in standalone view
Summary: Partially broken by D2166.

Test Plan:
Hover line number in revision.
Hover line number in standalone view.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2196
2012-04-10 15:05:54 -07:00
Bob Trahan
1175784d5d PhabricatorSlug
Summary:
This is to be used in Phame so the logic is shared where possible. The change has three main things going on

- broke out functionality from PhrictionDocument that isn't Phriction specific.
- swept up code base to use new PhabricatorSlug class.
- altered the regex ever so slightly per discussion and http://stackoverflow.com/questions/2028022/javascript-how-to-convert-unicode-string-to-ascii

I think maybe we should punt on unicode here for quite a bit -- http://www.456bereastreet.com/archive/201006/be_careful_with_non-ascii_characters_in_urls/ -- but we'll be well-positioned to add it with the code here.

Test Plan: used phriction to create, edit, view documents. used a tool (codemod) for the codebase sweeping

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2195
2012-04-10 14:18:20 -07:00
epriestley
01907bcccc Allow "Test Plan" to be disabled in config
Summary:
This is a somewhat common request, and far more difficult than necessary currently.

I think the field is useful enough to leave it default-enabled, but there's wide diversity in testing philosophy.

Test Plan: Verified "test plan" field appeared. Disabled config. Verified "test plan" field vanished.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran, asouza

Differential Revision: https://secure.phabricator.com/D2193
2012-04-10 13:36:05 -07:00
epriestley
5f615c1e6e Fix a warning when viewing a revision not attached to a repository
Summary: We'll get a typehint warning on the repository if there's no repository. Check outside the method instead.

Test Plan: Loaded page, no warning.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2194
2012-04-10 13:34:31 -07:00
epriestley
fe9ba6bc67 Improve DifferentialRevisionQuery and add the ability to query by arcanist project
Summary:
  - We currently post-filter by branches, but should do this in SQL. See T799.
  - We currently identify branch-name-matches as being in the working copy even if they belong to a different project (e.g., two different projects with commits on the branch "master"). See T1100.
  - Denormalize branch and project information into DifferentialRevision.
  - Expose project information in the API.

Test Plan: Ran conduit API queries with branches and arc project IDs, got reasonable results.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1100, T799

Differential Revision: https://secure.phabricator.com/D2190
2012-04-10 12:51:34 -07:00
vrana
b5adde88d4 Simplify selecting authors and revisions in blame
Summary: Déjà vu: D1736.

Test Plan: Double click besides author in blame.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2166
2012-04-10 11:41:31 -07:00
vrana
935f3657b5 Allow %f2 and other escape sequences in editor link
Test Plan: Open in Editor.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2184
2012-04-10 11:36:37 -07:00
vrana
e87e1786a6 Fix docs links after D2181
Test Plan:
  diviner .

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2188
2012-04-10 11:33:26 -07:00
vrana
76b534b560 Don't fetch all commits without blame in Diffusion
Summary:
Otherwise useless query is executed:

  lang=sql
  SELECT c.*
  FROM `repository_commit` c
  ORDER BY c.epoch DESC

Test Plan: /diffusion/X/browse/x

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2186
2012-04-10 09:58:36 -07:00
vrana
347bc357fd Display Browse in Diffusion and Open in Editor links in commit detail
Test Plan:
/rX1
Browse in Diffusion
Open in Editor

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

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

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

A little messy. :/

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

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, 20after4

Maniphest Tasks: T923, T1034

Differential Revision: https://secure.phabricator.com/D1964
2012-04-10 09:46:04 -07:00
vrana
62a172af90 Fix reticle
Summary:
Now I understand that [[ https://secure.phabricator.com/diffusion/P/browse/master/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js;32f12d1f8fb7aeca$174-176 | behavior-edit-inline-comments.js:174-176 ]] and [[ https://secure.phabricator.com/diffusion/P/browse/master/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php;32f12d1f8fb7aeca$72-99 | DifferentialChangesetViewController.php:72-99 ]] need to stay in sync:

- 1 - isOnRight equals isNewFile
- 1/-1 - left is new, right is missing
- 1/2 - both are new

Test Plan: Hover inline comment.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1076

Differential Revision: https://secure.phabricator.com/D2179
2012-04-09 22:09:02 -07:00
epriestley
32f12d1f8f Don't fatal when generating patch emails for diffs with binaries
Summary:
When Phabricator is configured to generate patch email, we'll fatal if the patch contains binaries and is generating to Git because ArcanistBundle can't load the binary data. Provide a callback to load the data. See D2174.

(This may cause us to generate absolutely enormous emails, but you get what you asked for...)

Test Plan: Created a diff with an image under "send git patches" email configuration.

Reviewers: Makinde, btrahan, vrana, jungejason

Reviewed By: Makinde

CC: aran

Differential Revision: https://secure.phabricator.com/D2175
2012-04-09 17:35:01 -07:00
vrana
974b576df0 Fix whitespace 2012-04-09 16:57:17 -07:00
vrana
6d313a1676 Improve speed of user feed
Test Plan: /p/x/feed/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1099

Differential Revision: https://secure.phabricator.com/D2176
2012-04-09 16:01:16 -07:00
vrana
e69c8abc15 Don't set feed width on chromeless page
Summary: Otherwise browser displays horizontal scrollbar at http://phabricator.org/.

Test Plan:
Add this rule in Firebug and display http://phabricator.org/.

/feed/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2173
2012-04-09 15:11:41 -07:00
vrana
32d2395a45 Unify links to www.phabricator.com and phabricator.com
Test Plan:
  scripts/sql/upgrade_schema.php

Verify links at /directory/2/.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1096

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

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

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

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

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

Reviewers: btrahan, vrana, jungejason, Koolvin

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2169
2012-04-09 11:08:59 -07:00
vrana
db2fef4c87 Don't display "Foul Magicks" in Maniphest
Summary:
There was a typo:
`PHID-!!!!-NO_PROJECT` instead of
`PHID-!!!!-NO-PROJECT`

Also use `<em>` to differentiate from project named "(No Project)".

Test Plan:
/maniphest/report/project/
Click on (No Project).

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2167
2012-04-09 08:22:49 -07:00
vrana
aa0d0396a6 Highlighting blame is broken if there is an unavailable commit
Test Plan: .../PhotoSnowlift.js?view=blame

Reviewers: jungejason

Reviewed By: jungejason

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2165
2012-04-09 01:14:36 -07:00
vrana
7451c1f6c9 Support NO_BACKSLASH_ESCAPES in escapeStringForLikeClause()
Summary: Also simplify this clunky code.

Test Plan: /owners/view/search/?name=%25

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2114
2012-04-08 21:37:31 -07:00
vrana
a662b09e73 Fix reticle with inline comments editor
Test Plan: Hover textarea in inline comment editor

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2152
2012-04-08 21:35:47 -07:00
vrana
13775fde01 Prefer external [[]] links in Remarkup
Test Plan: I didn't repro it probably because of custom rules.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1090

Differential Revision: https://secure.phabricator.com/D2150
2012-04-08 21:33:25 -07:00
vrana
2c8e6f99bd Standardize mysql.configuration-provider
Summary: NOTE: BC break!

Test Plan: /

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, nh

Differential Revision: https://secure.phabricator.com/D2130
2012-04-08 21:32:15 -07:00
epriestley
6c1e2cd8b2 Minor, make sure burnup buckets are unique year-over-year
Auditors: btrahan, vrana, jungejason
2012-04-08 16:33:51 -07:00
epriestley
6eb91b2a0e Improve documentation for System Agents and other account roles
Summary: Explain this stuff better and add some documentation links.

Test Plan: Read documentation, viewed account edit interfaces.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T834

Differential Revision: https://secure.phabricator.com/D2158
2012-04-08 15:10:00 -07:00
epriestley
62e41040f0 Improve exception behavior for storage engine failures
Summary: See T1021. Raise configuration or implementation exceptions immediately. When all engines fail, raise an aggregate exception with details.

Test Plan: Forced all engines to fail, received an aggregate exception. Forced an engine to fail with a config exception, recevied it immediately.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1021

Differential Revision: https://secure.phabricator.com/D2157
2012-04-08 15:07:34 -07:00
epriestley
06367c4801 Improve insert behavior for drag-and-drop and paste uploads
Summary: In Safari, Firefox and Chrome, respect cursor position and selection ranges.

Test Plan: Dragged-and-dropped files into the middle of text, end of text, and a selected text range in Safari, Firefox and Chrome. Copy/pasted files into similar cases in Chrome. Got expected, normal behavior in all cases.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1016

Differential Revision: https://secure.phabricator.com/D2155
2012-04-08 15:05:40 -07:00
epriestley
23fd936b47 Add some basic signature stripping
Summary: See discussion in T789. Covered the obvious cases, at least. We can refine this as we get a larger sample size.

Test Plan: Unit test coverage.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T789

Differential Revision: https://secure.phabricator.com/D2154
2012-04-08 15:04:12 -07:00
epriestley
056fd755da Detect missing InnoDB in MySQL
Summary: See T993. MySQL fails very very softly if you request an engine which does not exist. Detect and fail if the InnoDB engine is missing or broken.

Test Plan: Faked InnoDB missing, got a failure. Ran normally, got success.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T993

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

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

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

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

So basically we:

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

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

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

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1073

Differential Revision: https://secure.phabricator.com/D2146
2012-04-08 10:07:51 -07:00
epriestley
dd21f7e37c Make error views look less awful
Summary: These elements look heavy and out of place right now.

Test Plan: Looked at error views in uiexample page.

Reviewers: btrahan, vrana, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2144
2012-04-07 17:25:31 -07:00
epriestley
780da42126 Show user a notice when they view "My Projects" for feed but haven't joined any projects
Summary: Currently, we show them everything. Instead, show them an explicit notice.

Test Plan: Looked at "My Projects" feed with no projects.

Reviewers: btrahan, vrana, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T1015

Differential Revision: https://secure.phabricator.com/D2143
2012-04-07 17:25:24 -07:00
epriestley
ee278a302e Improve Diffusion blame views
Summary:
  - Make some effort to simplify the code.
  - Make "Skip Past This Commit" work in Git and Mercurial.
  - Make blame work in Mercurial.
  - Add tooltip hover state to show more information about commits.

Test Plan: Viewed blame views in SVN, Git, Hg. Clicked line numbers, hovered/clicked commits, hovered/clicked "blame past..."

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T378

Differential Revision: https://secure.phabricator.com/D2142
2012-04-07 17:24:35 -07:00
vrana
df67401e24 Add typehints to Diffusion browse file controller
Test Plan: Display commit.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2139
2012-04-07 16:03:55 -07:00
Jason Ge
bbeb850d63 Fix missing inline comments issue
Summary:
some inline comments are missing in the revision page. The
reason is that the inline comments got overwritten if multiple groups of
comments are modifying the same file
(https://secure.phabricator.com/diffusion/P/browse/master/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php;f6748bc1907d946ffe5a0957964a5eb5fe90514f$299).

Test Plan: the comments used to be missing renders now.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: hwang, aran

Differential Revision: https://secure.phabricator.com/D2135
2012-04-07 14:05:52 -07:00
Jason Ge
4f5ce3d9a3 Update ibrary map file
Summary: Update ibrary map file. It's probably caused by D2113.

Test Plan: arc unit passed for the testEverythingImplemented test case.

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2134
2012-04-07 14:04:58 -07:00
epriestley
d4839f402c Minor, test fix for D2140? I think this fixes the issue in D2140, pushing to verify...
Auditors: 20after4, btrahan
2012-04-07 12:59:06 -07:00
vrana
5493c0e58e Fix typo 2012-04-07 11:45:31 -07:00
epriestley
ff4f72fdab Minor documentation updates.
Auditors: vrana, btrahan
2012-04-07 11:05:58 -07:00
epriestley
6c2e032dbe Minor, fix a heading in documentation.
Auditors: vrana, btrahan
2012-04-07 11:02:48 -07:00
epriestley
c894344505 Minor, fix some doc links.
Auditors: vrana, btrahan
2012-04-07 10:59:14 -07:00
vrana
d4c5761f41 Customizable MySQL implementation
Test Plan:
- /
- upgrade_schema.php
- Setup
- Try disabling mysql_connect.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2133
2012-04-07 10:54:12 -07:00
epriestley
34ca4a9ba7 Update arcanist documentation to reflect "land", a sane relative commit, and "--auto"
Summary: See D2080. The introduction of `arc land`, defaulting to `origin/master`, and --auto enormously simplifies the documentation.

Test Plan: Read documentation.

Reviewers: btrahan

Reviewed By: btrahan

CC: 20after4, aran

Maniphest Tasks: T894

Differential Revision: https://secure.phabricator.com/D2082
2012-04-07 10:39:51 -07:00
epriestley
869f24bf33 Fix reticle for diff-of-diffs
Summary:
The older logic was incorrect:

  - It chose `change.left` for `data.on_right` being true.
  - 'O' and 'N' mean 'old' and 'new', not 'left' and 'right'. In diff-of-diffs, both sides are 'N'.

So, select the changeset ID correctly (pick the right side one for on_right), and select the new file prefix correctly (N for new, O for old).

Test Plan: Waved my mouse over some inline comments in a diff-of-diffs, got reasonable-looking reticles.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1076

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

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

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

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2132
2012-04-07 10:15:54 -07:00
epriestley
3fdd8c497c Possible fix for T1076, pushing to verify.
I think the issue is that we don't set the left-side changesetID correctly. This seems to work correctly locally, but I'm not sure I got a good repro. Pushing to verify the production test cases provided in T1076.

Auditors: vrana, btrahan
2012-04-07 10:01:28 -07:00
Jason Ge
f6748bc190 The existing Audit code forgot to set authorPHID
Summary:
The audit tools has many false positive about Author Not
Matching with Revision. The fix is to set the authorPHID which was
missing in the existing code

Test Plan:
run reparse.php and it doesn't generate false positive result
anymore.

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2119
2012-04-06 14:16:42 -07:00
epriestley
f7c74e3fb8 Remove an extra </p>
Summary: renderMiniPanel() renders the entire <p>.

Test Plan: Looked at page source for homepage, verified there was no double </p>.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1079

Differential Revision: https://secure.phabricator.com/D2128
2012-04-06 14:09:24 -07:00
vrana
a234a712cd Disable autoload in search for internal class
Test Plan:
/diffusion/symbol/Exception/?jump=1&type=class&lang=php
/diffusion/symbol/Countable/?jump=1&type=class&lang=php

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

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

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

Reviewers: epriestley

Reviewed By: epriestley

CC: nh, aran

Differential Revision: https://secure.phabricator.com/D2113
2012-04-06 12:43:56 -07:00
vrana
2211a0b07e Don't render <div> inside <span>
Test Plan: D1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2121
2012-04-06 10:15:44 -07:00
vrana
23b65c13f4 Use Filesystem::readRandomBytes() in setup
Test Plan: Run setup.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2123
2012-04-06 10:10:33 -07:00
vrana
36ee5dba51 Jump to TOC after Show Diff
Summary: I've found it useful mainly on smaller screen or with lots of comments.

Test Plan: Show Diff

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2122
2012-04-06 09:56:28 -07:00
vrana
1f2028adf0 Render valid HTML
Summary: Also delete some dead code.

Test Plan: /D1

Reviewers: tuomaspelkonen, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2120
2012-04-06 09:56:14 -07:00
vrana
23988ca482 Support Windows
Test Plan:
Enable setup.
Disable setup.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2118
2012-04-06 09:34:06 -07:00
vrana
426d6f2b1f Make getTransactionKey() protected
Test Plan: Search for `getTransactionKey`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2115
2012-04-06 09:33:37 -07:00
vrana
5f341c79cb Pass connection to mysql_set_charset() 2012-04-05 17:23:29 -07:00
vrana
64facecb87 Fix typo in AphrontMySQLDatabaseConnection 2012-04-05 17:19:07 -07:00
vrana
f698e860cf Remove duplicate remarkup rule
Summary: D2110

Test Plan:
  [[wiki]]
  [[http://example.com]]
  [[http://example.com | example.com]]

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2111
2012-04-05 16:31:17 -07:00
Nick Harper
efb49a6a09 Fix differential.createrevision conduit call
Summary:
This introduces some boundary checking for
PhabricatorOwnersOwner::loadAffiliatedUserPHIDs() if it gets passed an empty
array, which happened when I ran arc diff and it called
differential.createrevision.

Test Plan: ran arc diff

Reviewers: epriestley, meitros, jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2112
2012-04-05 15:28:58 -07:00
vrana
ff2b58dab7 Fix MySQL column escaping
Summary:
MySQL doesn't treat `\` as escaping character in ##``##.
This isn't probably SQL injection hole because I've found no calls of this method with user input.
But better safe than sorry.

See also [[http://dev.mysql.com/doc/refman/5.1/en/server-sql-mode.html#sqlmode_no_backslash_escapes | NO_BACKSLASH_ESCAPES]].

Test Plan:
  lang=sql
  SELECT `a\`b`; -- Throws: Syntax error near '`'.
  -- Should throw: Unknown column 'a`b'.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2109
2012-04-05 15:11:05 -07:00
vrana
23d5d7a1a6 Delete unused CSS
Summary: Unused since D2062.

Test Plan:
View commit.
View task.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

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

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

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2107
2012-04-04 18:19:14 -07:00
Bob Trahan
09172a1937 Add pagers to server clients and client authorizations in OAuth Server GUI
Summary: ...also make the pager usage in ChatLog use the nice formatWhereClause functionality

Test Plan: set $page_size = 2 and paged around the data a bit

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T905

Differential Revision: https://secure.phabricator.com/D2106
2012-04-04 17:51:16 -07:00
epriestley
05b4c90bfd Allow Commits to be attached to Tasks using edges
Summary: Use Edges to attach Commits and Tasks. Note, no "edit attached commits" interface from tasks yet since the search backend needs a little work to list commits in a sensible way.

Test Plan: Attached commits to tasks. Looked at commits, looked at tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D2105
2012-04-04 17:34:25 -07:00
Bob Trahan
cc586b0afa For discussion -- Stripe integration
Summary:
various stripe stuff, including

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

My main questions / discussion topics are...

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

Test Plan: - played around with sample client form

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2096
2012-04-04 16:09:29 -07:00
epriestley
877cb136e8 Add an assocations-like "Edges" framework
Summary:
We have a lot of cases where we store object relationships, but it's all kind of messy and custom. Some particular problems:

  - We go to great lengths to enforce order stability in Differential revisions, but the implementation is complex and inelegant.
  - Some relationships are stored on-object, so we can't pull the inverses easily. For example, Maniphest shows child tasks but not parent tasks.
  - I want to add more of these and don't want to continue building custom stuff.
  - UIs like the "attach stuff to other stuff" UI need custom branches for each object type.
  - Stuff like "allow commits to close tasks" is notrivial because of nonstandard metadata storage.

Provide an association-like "edge" framework to fix these problems. This is nearly identical to associations, with a few differences:

  - I put edge metadata in a separate table and don't load it by default, to keep edge rows small and allow large metadata if necessary. The on-edge metadata seemed to get abused a lot at Facebook.
  - I put a 'seq' column on the edges to ensure they have an explicit, stable ordering within a source and type.

This isn't actually used anywhere yet, but my first target is attaching commits to tasks for T904.

Test Plan: Made a mock page that used Editor and Query. Verified adding and removing edges, overwriting edges, writing and loading edge data, sequence number generation.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, 20after4

Differential Revision: https://secure.phabricator.com/D2088
2012-04-04 15:30:21 -07:00
vrana
bc61f36beb Replace elseif by else if
Summary:
Mostly written by me.
Omit external libraries.

Test Plan: http://phabricator.com/docs/phabricator/article/PHP_Coding_Standards.html

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2104
2012-04-04 15:24:47 -07:00
vrana
b8cb52a9da Return $this from shortcuts
Test Plan:
Search for `>setLineWidthFromChangesets(`.
Search for `>loadAndAttachAuxiliaryAttributes(`.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2102
2012-04-04 15:12:20 -07:00
vrana
582fc847f2 Use assert_instances_of() in Differential
Summary: NOTE: This is not produced by a script so there might be errors. Please review carefully.

Test Plan: Browse around Differential.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2103
2012-04-04 15:11:30 -07:00
epriestley
01767c482d Add a datepicker control
Summary: I looooove JS! It makes me giddy with glee!

Test Plan: Picked dates. See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2086
2012-04-04 12:14:10 -07:00
vrana
84398fc581 Allow system agents in commit message object lists
Summary:
When system agent adds a comment then he is added to CC.
When I amend and update then I get message "Commit message references nonexistent ..."

Test Plan: Update revision with system agent in CC.

Reviewers: epriestley

Reviewed By: epriestley

CC: michalburger1, aran

Differential Revision: https://secure.phabricator.com/D2100
2012-04-04 10:46:00 -07:00
vrana
6a0b25c188 Fix wrong assert_instances_of() added by D2091
Test Plan: Display revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2095
2012-04-03 19:26:05 -07:00
vrana
a309d5ba2f Replace leading double underscore in function names by single underscore
Summary:
> PHP reserves all symbols starting with __ as magical. http://php.net/userlandnaming.rules

I didn't touch third-party S3 library.

Test Plan: /

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2077
2012-04-03 18:55:52 -07:00
vrana
7dfdf63948 Fix Jump to HEAD link
Test Plan:
Jump to head.
Go to doctor.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2097
2012-04-03 18:39:58 -07:00
vrana
6f855c8b52 Don't show current revision in dependencies
Test Plan: Edit Dependencies, don't see current revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2098
2012-04-03 18:39:34 -07:00
vrana
c241f50d77 Use assert_instances_of() in Diffusion
Summary: NOTE: This is not produced by a script so there might be errors. Please review carefully.

Test Plan: Browse around Diffusion.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2094
2012-04-03 16:31:10 -07:00
mkedia
591d50008f Allow projects to own packages
Summary:
- The UI is pretty straightforward, since Handle just works (tm)
- Added two methods to the owners object to handle the new layer of
  indirection. Then ran git grep PhabricatorOwnersOwner and changed
  callsites as appropriate.

Sending this to get a round of feedback before I test the non-trivial
changes in this diff.

Test Plan:
- owners tool: edit, view, list for basic functionality.
- phlog for the two new methods I added

Reviewers: epriestley, blair, jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2079
2012-04-03 16:20:07 -07:00