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

1227 commits

Author SHA1 Message Date
epriestley
30473549ac Add a basic pull event log for debugging repository cloning
Summary:
Ref T10228. This is currently quite limited:

  - No UI.
  - No SSH support.

My primary goal is to debug the issue in T10228. In the long run we can expand this to be a bit fancier.

Test Plan:
Made various valid and invalid clones, got sucess responses and not-so-successful responses, viewed the log table for general corresponding messages and broad sanity.

Ran GC via `bin/phd debug trigger`, no issues.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10228

Differential Revision: https://secure.phabricator.com/D15127
2016-01-28 08:18:34 -08:00
Chad Little
36158dbdc0 Convert all calls to 'IconFont' to just 'Icon'
Summary: Mostly for consistency, we're not using other forms of icons and this makes all classes that use an icon call it in the same way.

Test Plan: tested uiexamples, lots of other random pages.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15125
2016-01-27 20:59:27 -08:00
epriestley
b53d61c909 Fix bad call to getShortName()
Summary: Fixes T10212. This method was removed in D14990, but I missed a callsite.

Test Plan: Disabling blame now works nicely.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10212

Differential Revision: https://secure.phabricator.com/D15100
2016-01-23 17:07:27 -08:00
epriestley
b51a859636 Allow diffusion.filecontentquery to load data for arbitrarily large files
Summary:
Fixes T10186. After D14970, `diffusion.filecontentquery` puts the content in a file and returns the file PHID.

However, it does this in a way that doesn't go through the chunking engine, so it will fail for files larger than the chunk threshold (generally, 8MB).

Instead, stream the file from the underlying command directly into chunked storage.

Test Plan:
  - Made a commit including a really big file: 4dcd4c492b
  - Used `diffusion.filecontentquery` to load file content.
  - Parsed/imported commit locally.
  - Used `diffusion.filecontentquery` to load content for smaller files (README, etc).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10186

Differential Revision: https://secure.phabricator.com/D15072
2016-01-21 09:52:43 -08:00
Aviv Eyal
98d2370672 Add link to Herald Transaction when blocking pushes
Summary: Ref T9719.

Test Plan: Tried to push, get dragon and a link.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T9719

Differential Revision: https://secure.phabricator.com/D15027
2016-01-15 03:01:47 +00:00
epriestley
1eab16c395 Move repository URIs to a dedicated index
Summary:
Ref T4705 (there are also some other adjacent related tasks dealing with URIs).

Currently, we issue a "get repositories matching URIs: ..." query by loading every possible repository and then checking their URIs in PHP.

Instead, put URIs in a separate table. I plan for each repository to potentially have multiple URIs soon, so this prepares for that.

Test Plan:
  - Ran migrations.
  - Looked at index table, made sure it appeared sensible.
  - Ran some queries by `uri` to find repositories, found the repositories I expected.
  - Updated the remote URI of a repository, saw queries / index update appropriately.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4705

Differential Revision: https://secure.phabricator.com/D15005
2016-01-13 09:34:31 -08:00
epriestley
a061bd2d09 Parse and display commit authorship date in Git in Diffusion
Summary: Fixes T8826. Git tracks an "author date", which may be different from the "committed date". We don't currently extract/show this; do so.

Test Plan: {F1059235}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8826

Differential Revision: https://secure.phabricator.com/D14995
2016-01-11 09:32:37 -08:00
epriestley
9fb929dff3 Show repositories in global autocomplete dropdown
Summary: Fixes T9523.

Test Plan: {F1059225}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9523

Differential Revision: https://secure.phabricator.com/D14994
2016-01-11 09:32:23 -08:00
epriestley
b848ab87b3 Introduce "local names" for commits
Summary: Ref T4245. Full commit display names (like `rPaaaa`) are going to be obnoxious soon in some cases (e.g., `rPaaaa` becomes `R123:aaaa`, which is much uglier) so reduce how often we show the repository in cases where it isn't really necessary to include it.

Test Plan:
  - Saw no more `rX` on repository list view for Git/Mercurial (still present for Subversion).
  - Saw no more `rX` on various repository detail views, except when referencing other commits (e.g., mentions).
  - Grepped for removed `getShortName()`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14990
2016-01-11 09:18:30 -08:00
epriestley
96ebd35824 Change repository "Clone/Checkout As" to "Short Name"
Summary:
Ref T4245.

  - Rename "Clone/Checkout As" to "Short Name" in the UI.
  - Allow any repository to have a short name, not just hosted repositories.

Test Plan:
  - Ran migration.
  - Reviewed old transactions, saw they looked good.
  - Edited an existing repository's short name.
  - Gave an imported repository a new short name.
  - Removed a repository's short name.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14989
2016-01-11 09:17:32 -08:00
epriestley
0b3d10c3da Enforce sensible, unique clone/checkout names for repositories
Summary:
Fixes T7938.

  - Primarily, users can currently shoot themselves in the foot by putting `../../etc/passwd` and other similar nonsense in these fields (this is not dangerous, but also does not work). Require sensible names.
  - Enforce uniqueness so these names can be used in URIs and as identifiers in the future.
  - (This doesn't start actually using them for anything fancy yet.)

Test Plan:
  - Gave several repositories clone names: a valid name, two duplicate names, an invalid, name, some with no names.
  - Ran migrations.
  - Got clean conversion for valid names, appropriate errors for invalid/duplicate names.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7938

Differential Revision: https://secure.phabricator.com/D14986
2016-01-11 02:06:44 -08:00
epriestley
c7520cd9f2 Improve rendering of commit branching graph
Summary:
Fixes T9323. Two minor fixes:

  - On the first commit, don't render a downward line.
  - Clean up a 1px spacing issue that had cropped up a while ago when we added icons or something, I think.

Test Plan:
Before:

{F1057248}

After:

{F1057249}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9323

Differential Revision: https://secure.phabricator.com/D14974
2016-01-08 11:52:07 -08:00
epriestley
d1fb2f7fb9 Make diffusion.filecontentquery return file PHIDs instead of raw content
Summary:
Fixes T9319. Proxied requests (e.g., in the cluster) for binary files (like images) currently fail because we can not return binary data over Conduit in JSON.

Although Conduit will eventually support binary-safe encodings, a cleaner approach to this is just to return a `filePHID` instead of the raw content. This is generally faster and more flexible, and gives us more opportunities to add caching later.

After making the call, the client pulls the file data separately.

We also no longer need to return a complex data structure because we don't do blame over this call any longer.

Test Plan:
  - Viewed images in Diffusion.
  - Viewed READMEs in Diffusion.
  - Used `bin/differential attach-commit rX Dy` to hit attach pathway.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9319

Differential Revision: https://secure.phabricator.com/D14970
2016-01-08 09:29:16 -08:00
epriestley
449da36c2f Use a path digest when building blame cache keys
Keys have a maximum length of 128, and long paths could cause key lengths to exceed this.

Auditors: chad
2016-01-06 19:12:57 -08:00
epriestley
d725dedb1e Fix two minor issues with blame that involves revisions
Summary: I was looking at some random un-revisioney repository for most of my testing and missed these.

Test Plan: Viewed blame of a file with some revisions.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14965
2016-01-06 18:55:48 -08:00
epriestley
438100691d Don't let blame run for longer than 15 seconds
Summary: Fixes T2450. If we spend more than 15 seconds in blame, just cut it off.

Test Plan:
  - Changed timeout to 0.01 seconds.
  - Did blame on a non-highlighted file, got no blame, saw warning.
  - Did blame on a highlighted file, got no blame.
    - Note: you don't get a warning here because of Ajax stuff. It'd be kind of tricky to add and doesn't seem like a big deal so I'm planning to leave it as-is for now.

Reviewers: chad

Reviewed By: chad

Subscribers: 20after4, chasemp

Maniphest Tasks: T2450

Differential Revision: https://secure.phabricator.com/D14964
2016-01-06 18:44:20 -08:00
epriestley
9ab1b5a22d Make mundane performance improvements to Diffusion browse views
Summary:
Ref T2450. This reorganizes code to improve performance.

Mostly, there are a lot of things which are unique per commit (author name, links, short name, etc), but we were rendering them for every line.

This often meant we'd render the same author's name thousands of times. This is slower than rendering it only once.

In 99% of interfaces this doesn't matter, but blame is weird and it's significant on big files.

Test Plan:
Locally, `__phutil_library_map__.php` now has costs of roughly:

  - 550ms for main content (from 650ms before the patch).
  - 1,500ms for blame content (frrom 1,800ms before the patch).

So this isn't huge, is a decent ~20%-ish performance gain for shuffling some stuff around.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2450

Differential Revision: https://secure.phabricator.com/D14963
2016-01-06 18:43:51 -08:00
Fabian Stelzer
e8d3071452 Implement a git blame cache
Summary: Ref T2450. Ref T2453. Add a repository_blamecache table and cache git blame information

Test Plan: View files in Diffusion with enabled blame

Reviewers: fabe, chad, #blessed_reviewers

Reviewed By: chad, #blessed_reviewers

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T2453, T2450

Differential Revision: https://secure.phabricator.com/D10600
2016-01-06 18:43:30 -08:00
epriestley
0759b84d77 Improve construction of commit queries from blame lookups
Summary:
Ref T2450. File blame tends to have the same commit a lot of times, and we don't do lookups like this efficiently right now.

In particular, for a file like `__phutil_library_map__.php`, we would issue a query with ~9,000 clauses like this:

```
(repositoryID = 1 AND commitIdentifier LIKE "XYZ%")
```

...but only a few hundred of those identifiers were unique. Instead, issue only one clause per unique identifier.

MySQL also seems to do a little better on "commitIdentifier = X" if we have the full hash, so special case that slightly.

Test Plan:
  - Issuing a query for only unique identifiers dropped the cost from 400ms to 100ms locally.
  - Swapping to `=` if we have the full hash dropped the cost from 100ms to 75ms locally.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2450

Differential Revision: https://secure.phabricator.com/D14962
2016-01-06 18:43:04 -08:00
epriestley
741118a08f Improve Diffusion behavior for directories with impressive numbers of files
Summary:
Fixes T4366. Two years ago, Facebook put 16,000 files in a directory. Today, the page has nearly loaded.

Paginate large directories.

Test Plan:
  - Viewed home and browse views in Git, Mercurial and Subversion.

I put an artificially small page size (5) on home:

{F1055653}

I pushed 16,000 files to a directory and paged through them. Here's the last page, which rendered in about 200ms:

{F1055655}

Our behavior is a bit better than GitHub here, which shows only the first 1,000 files, disables pagination, and can't retrieve history for the files:

{F1055656}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4366

Differential Revision: https://secure.phabricator.com/D14956
2016-01-06 14:19:55 -08:00
epriestley
91a01e5703 Improve Diffusion browse performance for large files
Summary:
When looking at a large file in Diffusion:

  - disable highlighting if it's huge and show a note about why;
  - pick up a few other optimizations.

Test Plan: Locally, this improves the main render of `__phutil_library_map__.php` from 3,200ms to 600ms for me, at the cost of syntax highlighting (we can eventually add view options and let users re-enable it).

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14959
2016-01-06 09:24:35 -08:00
epriestley
9728c65e93 Drive blame generation through diffusion.blame
Summary:
Ref T2450. Ref T9319. This is still a bit messy, but not quite so bad as it was: instead of using a single call to get both blame information and file content, use `diffusion.blame` for blame information.

This will make optimizations to both blame and file content easier.

Test Plan: Viewed a bunch of blame (color on/off, blame on/off).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2450, T9319

Differential Revision: https://secure.phabricator.com/D14958
2016-01-06 09:24:21 -08:00
epriestley
f561dc172d Implement a dedicated "diffusion.blame" API method
Summary:
Fixes T2451. Several motivations here, from strongest to weakest:

  - Currently, getting blame and file content are closely entwined. This makes fixing T9319 more difficult, and I want to fix it. I want to separate blame from content so there's more flexibility in how we approach this issue.
  - This makes pursuing T2450 easier, if it turns out to be a meaningful win.
  - If we can get a win on blame performance, we can do `arc blame` eventually if we want.

Test Plan:
  - Blamed in SVN, Git and Mercurial.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2451

Differential Revision: https://secure.phabricator.com/D14957
2016-01-06 09:24:03 -08:00
epriestley
94d79c11a9 Show import progress on repository main page
Summary: Fixes T9192.

Test Plan: {F1055042}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9192

Differential Revision: https://secure.phabricator.com/D14951
2016-01-05 14:02:59 -08:00
epriestley
f9a5cd2bbd Fix all remaining weird Diffusion request processing
Summary: Ref T4245. This is the last of it, and covers the clone/push stuff.

Test Plan:
  - Cloned git.
  - Pushed git.
  - Cloned mercurial.
  - Pushed mercurial.
  - Visited a `blah.git` URL in my browser just because; got redirected to a human-facing UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14949
2016-01-05 14:01:53 -08:00
epriestley
38f2008e68 Modernize Diffusion lint controllers
Summary: Ref T4245. On their best day these don't work all that well, but I'm pretty sure I didn't make anything worse.

Test Plan:
  - Viewed global lint.
  - Viewed lint for a repository.
  - Viewed lint details for a particular message.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14948
2016-01-05 14:01:20 -08:00
epriestley
3cbc239bc6 Modernize most somewhat-weird Diffusion controllers
Summary: Ref T4245. This gets everything else except serving HTTP requests (complicated) and lint (quite weird).

Test Plan:
  - Viewed a diff.
  - Viewed externals.
  - Viewed history table to see last modified.
  - Did path completion and validation in Owners.
  - Did tree path search in Diffusion.
  - Viewed a repository.
  - Created a new repository.
  - Looked up symbols.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14947
2016-01-05 14:00:57 -08:00
epriestley
649f882720 Slightly modernize all Diffusion edit endpoints
Summary: Ref T4245. Prepares edit endpoints for more flexible repository identifiers.

Test Plan:
  - Added, edited, deleted mirror.
  - Created repository.
  - Edited basic repository information.
  - Edited policies for a repository.
  - Activated/deactivated repository.
  - Updated a repository.
  - Hit "Delete" dialog for a repository.
  - Edited hosting.
  - Toggled dangerous changes.
  - Edited branches.
  - Edited automation.
  - Tested automation.
  - Edited storage.
  - Edited staging.
  - Edited encoding.
  - Edited symbols.
  - Edited branches.
  - Edited actions.
  - Tried to do edits as an unprivileged user.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14945
2016-01-05 14:00:36 -08:00
epriestley
2bfc5ff92e Modernize more Diffusion controllers
Summary: Ref T4245. Standardize how context is read, minor updates / modernizations / consistency tweaks.

Test Plan:
  - Viewed a change.
  - Viewed brnaches.
  - Edited a commit.
  - Viewed tags.
  - Viewed history.
  - Added, edited and deleted a mirror.
  - Viewed push events.
  - Viewed a particular event.
  - Viewed ref disambiguation.
  - Viewed repository list.
  - Ran automation test.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14944
2016-01-05 14:00:20 -08:00
epriestley
f1c298203a Return no results from grep repository queries on error
Summary: Fixes T7852. Although `1` could also indicate other kinds of problems, assume it means "no results".

Test Plan: Searched for nonsense strings in Git and Mercurial. Searched for valid strings in Git and Mercurial.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7852

Differential Revision: https://secure.phabricator.com/D14943
2016-01-05 13:59:12 -08:00
epriestley
8b6edaa4e2 Merge and modernize Browse controllers in Diffusion
Summary:
Ref T4245. Browsing is huge and currently split across 5 files using controller delegation.

Although having a huge file isn't great, I think the way it is split up is currently worse, and it gets weird with more flexible repository identifiers.

So this is mostly merging five controllers into one, then a bit of modernization.

I think this can probably be split up better by pulling some of it out into views, instead of using delegation.

Test Plan: Browsed files, directories, and search results.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14942
2016-01-05 13:58:55 -08:00
epriestley
7de17fb75e Modernize tag and branch controllers in Diffusion
Summary: Ref T4245. Prepares these controllers to accept alternate identifers, plus minor spacing and layout fixes.

Test Plan: Viewed tags, viewed branches.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14941
2016-01-05 13:58:36 -08:00
epriestley
fb3b4ee532 Make CommitController more flexible about handling URIs
Summary:
Ref T4245. This adds support for both ID-based and callsign-based routes, although the ID-based routes don't occur anywhere.

Also moves toward simplifying the DiffusionRequest stuff.

Test Plan: Visited normal callsign-based commit pages; visited new ID-based commit pages.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14940
2016-01-05 13:56:27 -08:00
epriestley
07e2596aa1 Move generateDiffusionURI() into PhabricatorRepository
Summary: Ref T4245. This further reduces the reliance on callsigns in Diffusion.

Test Plan:
  - Pretty reasonable test coverage already exists.
  - Browsed repository list, browse view, history view, content view, change view, commit view, tag view, branch view of repositories.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14937
2016-01-05 04:47:06 -08:00
epriestley
bcfd6bdd81 Move various other callsites away from callsigns
Summary: Ref T4245. These mostly relate to building URIs.

Test Plan: Tried to hunt down as many of these in the UI as I could. Some are a bit tricky but they should be low-risk.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14933
2016-01-04 06:54:42 -08:00
epriestley
9febfb26a0 Convert diffusion.looksoon to use repository identifiers instead of callsigns
Summary:
Ref T4245. Like everything else, accept more identifiers.

This needs a change in `arc`, which I've made a note about elsewhere.

Test Plan: Used "Update Now" from web UI, saw update get scheduled.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14932
2016-01-04 06:54:20 -08:00
epriestley
be5b89687e Separate external editor integration from callsigns
Summary: Ref T4245. Pass the whole repository in so it can do something else in a future change.

Test Plan: Loaded changesets in Diffusion.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14931
2016-01-04 06:54:01 -08:00
epriestley
b1388c5ca1 Remove diffusion.getcommits Conduit API method
Summary:
Ref T4245. This was obsoleted long ago and has no callers in Phabricator or Arcanist.

Also some minor cleanup.

Test Plan: `grep` for callers everywhere.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14930
2016-01-04 06:53:45 -08:00
epriestley
35e7a1f3c0 Continue reducing callers to getCallsign()
Summary: Ref T4245. More of the same, just narrowing down the easy cases.

Test Plan:
- Called `diffusion.querycommit`.
- Browsed branches.
- Browsed repository.
- Browsed directory.
- Searched for stuff.
- Viewed a commit.
- Viewed a file diff.
- Edited a commit.
- Viewed history.
- Viewed tags.
- Viewed push log.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14929
2016-01-04 06:53:32 -08:00
epriestley
9d84eb4c74 Allow Conduit API methods in Diffusion to accept any repository identifier
Summary:
Ref T4245. Broaden support to include "ABCD", "rABCD", "1234", "R1234", etc.

This doesn't change the old behavior, just accepts more stuff.

Test Plan:
  - Browsed Diffusion.
  - Made various calls via API console.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14928
2016-01-04 06:51:04 -08:00
epriestley
37532a0bf0 Remove various additional calls to getCallsign()
Summary: Ref T4245. These are all straightforward to remove.

Test Plan:
- Edited paths in a package.
- Ran `bin/audit delete --repositories ...` with various identifiers.
- Searched by repository for `R3`, `rAAAA` in Harbormaster.
- Did a Herald dry run on a commit.
- Browsed commits, made comments.
- Viewed a Releeph product list.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14927
2016-01-02 11:04:22 -08:00
epriestley
1b4f5e38ce Fix an issue with rendering some commit hovercards
Summary: This logic wasn't quite right.

Test Plan: Hovered over a recognized commit, got a valid hovercard

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14925
2016-01-02 11:03:12 -08:00
epriestley
84a570a61b fix broken link for project creating button
Summary: fix T10074

Test Plan: click the "create project" button

Reviewers: joshuaspence, #blessed_reviewers, epriestley

Reviewed By: joshuaspence, #blessed_reviewers, epriestley

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T10074

Differential Revision: https://secure.phabricator.com/D14917
2015-12-31 05:59:05 -08:00
epriestley
bdc517485c Modernize Hovercard implementation
Summary:
Ref T8980. Move away from events to EngineExtensions.

This also simplifies hovercards a bit:

  - Removes tasks from revision cards.
  - Removes blockers/blocked from task cards.
  - Removes "Send Message" from user cards.

These mostly felt cluttery to me. Open to arguments to retain them. I think we can make better use of the space, though (e.g., flags, projects + board columns).

Test Plan:
  - Viewed people, task, revision, commit and project hovercards.

{F1043256}

{F1043257}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8980

Differential Revision: https://secure.phabricator.com/D14878
2015-12-24 12:18:28 -08:00
epriestley
2868a69f65 Remove all setObjectURI() from ActionListViews
Summary:
Ref T10004. After D14804, we get this behavior by default and no longer need to set it explicitly.

(If some endpoint did eventually need to set it explicitly, it could just change what it passes to `setHref()`, but I believe we currently have no such endpoints and do not foresee ever having any.)

Test Plan:
  - As a logged out user, clicked various links in Differential, Maniphest, Files, etc., always got redirected to a sensible place after login.
  - Grepped for `setObjectURI()`, `getObjectURI()` (there are a few remaining callsites, but to a different method with the same name in Doorkeeper).

Reviewers: chad

Reviewed By: chad

Subscribers: hach-que

Maniphest Tasks: T10004

Differential Revision: https://secure.phabricator.com/D14805
2015-12-17 08:30:22 -08:00
epriestley
8dcdc7534d Add a DiffusionRefDatasource for typeahead'ing branches, tags, bookmarks and refs
Summary: Ref T9952. This will let me put a "Branch: [____]" control on the "Land Revision" dialog so users can choose a branch to target.

Test Plan: Used `/typeahead/class/` to vet basic behavior.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9952

Differential Revision: https://secure.phabricator.com/D14732
2015-12-10 14:21:24 -08:00
epriestley
b4681aa0c8 Remove dead link to Perforce blog about Git Fusion
Summary: Fixes T9941. I think someone from Perforce emailed us about 10 years ago and I added this link in response, but I haven't seen other interest in Perforce since then. Link is now dead.

Test Plan:
  - {nav Diffusion > Create Repository > Import Existing}, no more Perforce link.
  - Grepped for `perforce`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9941

Differential Revision: https://secure.phabricator.com/D14720
2015-12-09 08:51:25 -08:00
epriestley
2353121dee Add "Is merge commit?" to commit fields in Herald
Summary: Fixes T5788. We already have this as a pre-commit field, add it as a post-commit field too.

Test Plan: Ran this rule on a merge commit. Also ran it on a non-merge commit. Both got the correct value.

Reviewers: avivey, chad

Reviewed By: avivey, chad

Subscribers: avivey

Maniphest Tasks: T5788

Differential Revision: https://secure.phabricator.com/D14685
2015-12-05 13:07:56 -08:00
epriestley
5963c4c9e0 Fix history link in Diffusion non-root browse views
Summary:
Fixes T9798. That task has good repro instructions.

In sub-views, we don't link the "History" icon correctly -- we only link it to `history/README` instead of `history/path/to/README`. Add the full path.

Also canonicalize the paths in a slightly prettier and more consistenty way.

Test Plan: Viewed root and non-root browse tables, saw links show up properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9798

Differential Revision: https://secure.phabricator.com/D14491
2015-11-16 09:00:29 -08:00
David Fisher
c589af51e8 add "update" mode to Diffusion coverage Conduit
Summary:
This diff adds a new mode argument to the Diffusion Conduit API with two options:
- "overwrite": the default, maintains the current behavior of deleting all coverage
  in the specified branch before uploading the new coverage
- "update": does not delete old coverage, but will overwrite previous
  coverage information if it's for the same file and commit

`DiffusionRequest::loadCoverage` already loads a file's coverage from the
latest available commit, so uploading coverage for different files in different
commits with "update" will result in seeing the latest uploaded coverage in
Diffusion.

Test Plan: manual local verification

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14428
2015-11-09 16:52:34 -08:00
Joshua Spence
c35b564f4d Various translation improvements
Summary: Depends on D14070.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D14073
2015-11-03 07:02:46 +11:00
Aviv Eyal
724f6ddda5 return this in DiffusionCommitQuery
Test Plan: chain another call after this

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D14364
2015-10-28 23:25:41 +00:00
epriestley
a763f9510e Add some Drydock documentation plus "Test Configuration" for repository automation
Summary:
Ref T182. Ref T9252.

  - Adds a "Test" repository operation that just runs `git status` to see if things work.
  - Adds a button for it in Edit Repository.
  - Shows operation status on the operation detail view to make this workflow work a little better.
  - Adds a lot of words. Words words words words.

Test Plan:
  - Tested repository operation.
  - Read words.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T182, T9252

Differential Revision: https://secure.phabricator.com/D14349
2015-10-27 18:04:02 +00:00
epriestley
4c1463eb56 Probably fix bad URI construction for Diffusion symbols
Summary: Ref T9532.

Test Plan: I don't have this configured locally but this seems very likely to be the correct fix. This list should be a list of PHIDs, but is a list of PHIDs followed by one PhabricatorRepository object.

Reviewers: avivey, chad

Reviewed By: chad

Maniphest Tasks: T9532

Differential Revision: https://secure.phabricator.com/D14311
2015-10-20 09:03:47 -07:00
epriestley
df5a031b54 Allow "Repository Automation" to be configured for repositories
Summary:
Ref T182. This allows you to assign blueprints that a repository can use to perform working copy operations. Eventually, this will support "merge this" in Differential, etc.

This is just UI for now, with no material effects.

Most of this diff is just taking logic that was in the existing "Blueprints" CustomField and putting it in more general places so Diffusion (which does not use CustomFields) can also access it.

Test Plan:
  - Configured repository automation for a repository.
  - Removed repository automation for a repository.

Reviewers: chad

Reviewed By: chad

Subscribers: avivey

Maniphest Tasks: T182

Differential Revision: https://secure.phabricator.com/D14259
2015-10-13 15:45:59 -07:00
Christopher Speck
812c41a18a Conditionally use hg files vs. hg locate depending on version of Mercurial
Summary:
In Mercurial 3.2 the `locate` command was deprecated in favor of `files` command. This change updates the DiffusionLowLevelMercurialPathsQuery command to conditionally use `locate` or `files` based on the version of Mercurial used.

Closes T7375

Test Plan:
My test/develop Phabricator instance is setup to run Mercurial 3.5.1.

The test procedure to verify valid file listings are being returned:
 1. I navigated to `http://192.168.0.133/conduit/method/diffusion.querypaths/`
 2. I populated the following fields:
  - path: `"/"`
  - commit: `"d721d5b57fc9ef72e47ff9d4e0c583d74a46590c"`
  - callsign: `"HGTEST"`
 3. I submitted request and verified that result contained all files in the repository:
```
{
  "0": "README",
  "1": "alpha/beta/trifle",
  "2": "test/Chupacabra.cow",
  "3": "test/socket.ks"
}
```

I repeated the above steps after setting up Mercurial 2.6.2, which I installed in the following manner:
 1. I downloaded Mercurial 2.6.2 source and run `make local` which will only compile it to work from its own directory (`/opt/mercurial-2.6.2`)
 2. I linked `/usr/local/bin/hg -> /opt/mercurial-2.6.2/hg` (there's also a `/usr/bin/hg` which is a link to `/usr/local/bin/hg`)
 3. I navigated to my home directory and verify that `hg --version` returns 2.6.2.
 4. I restarted phabricator services (probably unnecessary).

With the Multimeter application active
 1. I verified that `/usr/local/bin/hg` referred to version 2.6
 2. I ran the same conduit call from the conduit application
 3. I verified that `http://192.168.0.133/multimeter/?type=2&group=label` incremented values for `bin.hg locate`.
 4. I swapped out mercurial versions for 3.5.1
 5. I ran the same conduit call from the conduit application
 6. I verified that `http://192.168.0.133/multimeter/?type=2&group=label` incremented values for `bin.hg files`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T7375

Differential Revision: https://secure.phabricator.com/D14253
2015-10-12 17:50:26 -07:00
Christopher Speck
32d4ae8cb2 Added an intercept to Mercurial's capabilities command to remove bundle2.
Summary:
If Mercurial 3.4+ is used to host repositories in Phabricator, any clients using 3.5+ will receive an exception after the bundle is pushed up. Clients will also fail to update phases for changesets pushed up.

Before directly responding to mercurial clients with all capabilities, this change filters out the 'bundle2' capability so the client negotiates using a legacy bundle wire format instead.

Test Plan:
Server: Mercurial 3.5
Client: Mercurial 3.4

Test with both HTTP and SSH protocols:
1. Create a local commit on client
2. Push commit to server
3. Verify the client emits something like:
```
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
```

Closes T9450

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T9450

Differential Revision: https://secure.phabricator.com/D14241
2015-10-10 07:14:48 -07:00
epriestley
2bfa0e087e Improve consistency and Harbormaster integration of Diffusion
Summary:
Ref T9123. Two major Harbormaster-related UI changes in Diffusion:

  - Tags table now shows tag build status.
  - Branches table now shows branch build status.

Then some minor consistency / qualtiy of life changes:

  - Picked a nicer looking "history" icon?
  - Branches table now uses the same "history" icon as other tables.
  - Tags table now has a "history" link.
  - Browse table now has a "history" link.
  - Dates now use more consistent formatting.
  - Column order is now more consistent.
  - Use of style is now more consistent.

Test Plan:
{F865056}

{F865057}

{F865058}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9123

Differential Revision: https://secure.phabricator.com/D14242
2015-10-06 07:38:15 -07:00
epriestley
b2e89a9e48 Fix several error handling issues with Subversion commits in Diffusion
Summary:
Ref T9513. I checked this briefly but didn't do a very thorough job of it.

  - Don't try to query merges for Subversion, since it doesn't support them.
  - Fix up "existsquery" to work properly (and efficiently) for both hosted and imported repositories.
  - Fix up "parentsquery" to have similar behavior on invalid commits to other VCSes (throw an exception).

Test Plan:
  - No more merges warning on SVN.
  - Hosted SVN gets the right exists result now.
  - Visiting "r23980283789287" now 404's instead of "not parsed yet".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9513

Differential Revision: https://secure.phabricator.com/D14239
2015-10-05 15:57:41 -07:00
epriestley
0db86cce7d Improve Diffusion behavior for no-longer-existing commits
Summary:
Ref T9028. When users push a commit, then later delete it (e.g., by deleting the branch which contained it) we currently explode when trying to view it.

Instead, degrade gradually if some information is not available.

Test Plan:
  - Looked at valid commits with parents, refs, branches and merges.
  - Looked at invalid commits.
  - Looked at a previously valid, now-deleted + gc'd commit:

{F859273}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9028

Differential Revision: https://secure.phabricator.com/D14227
2015-10-02 16:11:03 -07:00
epriestley
2728a9f964 Allow builds to have parameters
Summary: Ref T9352. See D13635. Build targets can have variables already, but let builds have them too. This mostly enables future use cases (sub-builds, more sophisticated build triggers).

Test Plan: With a custom Herald rule + action like the one in T9352, updated a revision and saw it generate multiple builds with varying parameters.

Reviewers: chad, hach-que

Reviewed By: hach-que

Maniphest Tasks: T9352

Differential Revision: https://secure.phabricator.com/D14222
2015-10-02 06:32:08 -07:00
epriestley
6d5c9e897d Hide "Revision" column in Diffusion history view if Differential is uninstalled
Summary: Fixes T9481. If the viewer does not have access to Differential (for example, because it is not installed), hide the "Revision" column in Diffusion.

Test Plan:
  - Viewed history, saw "Revision" column.
  - Uninstalled Differential, reloaded, no "Revision" column.

Reviewers: chad

Reviewed By: chad

Subscribers: revi

Maniphest Tasks: T9481

Differential Revision: https://secure.phabricator.com/D14188
2015-09-29 07:09:12 -07:00
epriestley
fa943f744b Stop all object mentions from matching after "@"
Summary:
Fixes T9479. Currently, `@aaaaaaaa` may try to match as a commit hash, and `@C123456` may try to match as a Countdown reference. These should only match as user mentions.

Prevent object mention rules from matching after `@`. We already prevent them after `-` and `#`, and already prevented the username rule after `@` (i.e., preventing `@@user`).

Test Plan:
Created some "interesting" users locally and `@mentioned` them:

{F850779}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9479

Differential Revision: https://secure.phabricator.com/D14186
2015-09-29 06:43:49 -07:00
Chad Little
799d86dc65 Show file UI box as collapsed in Diffusion
Summary: This UI should have a Collapsed PHUIObjectBoxView.

Test Plan: review a file in sandbox

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14139
2015-09-21 12:29:17 -07:00
Chad Little
666f19e504 Make icon setting in Section Headers easier/consistent
Summary: You can already pass other icons, but this makes it a bit simpler.

Test Plan: Test Maniphest, Badges

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14131
2015-09-19 11:29:01 -07:00
Chad Little
d199560a6b Add a box around the pager in Diffusion
Summary: Fixes T9392, adds some sweet sweet margin to the pager.

Test Plan: See pager with new padding, test different pages, breakpoints.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9392

Differential Revision: https://secure.phabricator.com/D14098
2015-09-11 08:59:56 -07:00
epriestley
4e181a5611 Clean up browse/history links in Diffusion
Summary:
Fixes T9126. In particular:

  - Add "Browse" links to all history views.
  - Use icons to show "Browse" and "History" links, instead of text.
  - Use FontAwesome.
  - Generally standardize handling of these elements.

This might need a little design attention, but I think it's an improvement overall.

Test Plan:
  - Viewed repository history.
  - Viewed branch history.
  - Viewed file history.
  - Viewed table of contents on a commit.
  - Viewed merged changes on a merge commit.
  - Viewed a directory containing an external.
  - Viewed a deleted file.

{F788419}

{F788420}

{F788421}

{F788422}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9126

Differential Revision: https://secure.phabricator.com/D14096
2015-09-10 19:28:49 -07:00
epriestley
20ce1a905f Replace AphrontUsageException with AphrontMalformedRequestException
Summary:
Ref T1806. Ref T7173. Context here is that I want to fix "you can not log in to this instance" being a confusing mess with an opaque error. To do this without hacks, I want to:

  - clean up some exception handling behavior (this diff);
  - modularize exception handling (next diff);
  - replace confusing, over-general exceptions with tailored ones in the Phacility cluster, using the new modular stuff.

This cleans up an awkward "AphrontUsageException" which does some weird stuff right now. In particular, it is extensible and extended in one place in Diffusion, but that extension is meaningless.

Realign this as "AphrontMalformedRequestException", which is a better description of what it is and does: raises errors before we can get as far as normal routing and site handling.

Test Plan: Hit some of these exceptions, saw the expected "abandon all hope" error page.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T1806, T7173

Differential Revision: https://secure.phabricator.com/D14047
2015-09-03 10:04:17 -07:00
epriestley
28621244ad Fix abrupt failure mode for uncloned repositories in Diffusion
Summary:
Fixes T9080. We try to list alternatives for the current ref (for example, if you're viewing a branch named "master" but there's also a tag named "master", or, in Mercurial, there are several branches named "master") but fail to abruptly if we can't get the list.

It's fine if we can't get the list; just continue. This is common when the repository hasn't cloned yet.

Test Plan: In a local repository with bad credentials, tried to do anything before and after. Before: completely blocked by error; after: things work normally.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9080

Differential Revision: https://secure.phabricator.com/D14044
2015-09-03 10:03:31 -07:00
epriestley
13516cf35f Fix an issue with "packages(...)" in typeaheads
Summary:
Fixes T9302. This datasource wasn't resolving package PHIDs correctly for the actual query.

Also fixes an issue with the "Affected packages that need audit" Herald rule.

Test Plan: Ran a "Needs Audit" query with only packages, and only `packages(user)`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9302

Differential Revision: https://secure.phabricator.com/D14029
2015-09-01 15:35:25 -07:00
epriestley
bce0698a0f Modernize Audit search engine
Summary:
Fixes T9279. Modernizes the SearchEngine and Query classes. User-facing changes:

  - Added order by commit date, default to order by commit date with newest commits first.
  - Added explicit "Needs Audit by".
  - Added new `packages(...)` typeahead function.
  - Picked up automatic subscribers, projects, and order fields.

This changes behavior a little bit: we previously attempted to exclude, e.g., commits which a package you own needs to audit, but which you have resigned from. This is difficult in general and I think it needs a more comprehensive solution. This shouldn't impact users much, anyway.

Test Plan: {F767628}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9279

Differential Revision: https://secure.phabricator.com/D14013
2015-08-31 10:17:54 -07:00
epriestley
9790f93a83 Show all packages with a claim on a file in the table of contents view
Summary:
Ref T9218. See discussion there for rationale; I think this is the right behavior to pursue.

The screenshot below is pretty ugly. I think it's a lot worse than most real-world cases will be, since you have to sort of opt-in to having crazy levels of overlapping packages, and it's perfectly normal/reasonable for files owned by one package. Owners is powerful enough to let you specify sub-packages with exclusive ownership.

That said, this may be more typical than I hope. I don't think we can reduce the complexity here much for free, but it would might be reasonable to add some view options (e.g.: group by package?, show only packages I own?, show packages as icons with a tooltip?) if it's an issue.

Test Plan: {F734956}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9218

Differential Revision: https://secure.phabricator.com/D13940
2015-08-20 04:46:17 -07:00
Chad Little
e0faa66772 Allow Owners Packages to be archived
Summary: Fixes T8428. Adds status to packages, allows setting and application search. I presume though these need checked elsewhere?

Test Plan: New package, edit package, archive package, run search queries.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T8428

Differential Revision: https://secure.phabricator.com/D13925
2015-08-18 13:36:05 -07:00
epriestley
6010e80e5c Show packages in table of contents views in Diffusion and Differential
Summary:
Fixes T8004.

  - For paths which are part of a package, show the package.
  - Highlight paths which are part of a package you (the viewer) have authority over.

Test Plan:
{F725418}

  - Viewed owned and unowned chagnes in Diffusion and Differential.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8004

Differential Revision: https://secure.phabricator.com/D13923
2015-08-17 10:14:22 -07:00
epriestley
a3393c3ecb Fix Owners lookups for the repository root
Summary:
Ref T9201. At the root of a repository the current path is `null`, but the OwnersQuery wants strings.

This could be resolved a couple different ways, but just cast the arguments to strings since that seems reasonable enough.

Test Plan: Browsed root of a repository.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9201

Differential Revision: https://secure.phabricator.com/D13919
2015-08-16 17:59:14 -07:00
epriestley
36a50ccbae Provide a more modern way to load packages owning a set of files
Summary:
Ref T8320. Ref T8004. This just tries to generally modernize

It also replaces the nonfunctional "Find Owners" link with a new property that just shows owning packages.

Test Plan:
  - Created and edited packages.

{F720478}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8004, T8320

Differential Revision: https://secure.phabricator.com/D13911
2015-08-15 13:06:10 -07:00
epriestley
8149399312 Convert commits to use unified table of contents
Summary:
Fixes T2183. We now use the same rendering element in both places.

Intentional changes:

  - Package highlighting is out, coming back to both apps in next diff.
  - removed redundant-feeling "Change" link. The information is now shown with a character ("M", "V", etc.) and the page is a click away under "History". Clicking the path also jumps you to substantially similar content. (We could restore it fairly easily, I just think it's probably the least useful thing in the table right now.)

Test Plan: Viewed a bunch of commits in Diffusion.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2183

Differential Revision: https://secure.phabricator.com/D13910
2015-08-15 12:57:20 -07:00
Joshua Spence
368f359114 Use PhutilClassMapQuery instead of PhutilSymbolLoader
Summary: Use `PhutilClassMaQuery` instead of `PhutilSymbolLoader`, mostly for consistency. Depends on D13588.

Test Plan: Poked around a bunch of pages.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13589
2015-08-14 07:49:01 +10:00
Joshua Spence
79f2e81f38 Various linter fixes
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13808
2015-08-08 10:19:45 +10:00
epriestley
386d2b62f8 Fix some copy/paste slop with BLOCK actions in Herald transcripts
Summary: Fixes T9060. These actions still work fine, but the transcripts got messed up a bit.

Test Plan: Viewed transcripts with blocking actions.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9060

Differential Revision: https://secure.phabricator.com/D13782
2015-08-04 07:32:19 -07:00
epriestley
a3e2f655eb Modularize the Diffusion/Differential "Block" Herald actions
Also removes HeraldCustomAction. This completes action modularization.

Ref T8726.
2015-08-03 14:33:28 -07:00
epriestley
6f6d88794b Modularize the Diffusion "Add Auditors" Herald action
Ref T8726.
2015-08-03 14:33:27 -07:00
epriestley
776caa507b Modularize the Harbormaster "Run build plan" Herald action
Ref T8726. Modularizes "Run build plan" in Differential and Diffusion.
2015-08-03 14:33:26 -07:00
epriestley
51fead17cf Modularize "Send an Email" Herald actions
Summary: Ref T8726. No surprises.

Test Plan:
Created rules using both action variants, applied upgrade, saw rules still work correctly.

{F658842}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13701
2015-08-03 14:33:23 -07:00
epriestley
8ae08a3de7 Make "Add Subscribers" and "Remove Subscribers" Herald actions modular
Summary: Ref T8726. Converts these actions to be modular. No real surprises in this change.

Test Plan:
{F658709}

  - Wrote some rules.
  - Migrated them forward.
  - Used a bunch of these rules.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13699
2015-08-03 14:33:22 -07:00
epriestley
8d9bd791f7 Modularize Herald "flag" action, plus update transcripts
Summary:
Ref T8726. This modularizes "Mark with flag", plus rebuilds transcripts in a more modern/flexible way. The big transcript stuff is:

  - Transcripts are now translatable.
  - Transcripts can now show multiple outputs from a single action. For example, an action like "add A, B, C to subscribers" can now say "added A; B is invalid; C was already subscribed".

Test Plan: {F637784}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, eadler, epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13649
2015-08-03 14:33:21 -07:00
epriestley
56dd5211f0 Make Herald action modularization more aggressive
Summary:
Ref T8726. Herald actions are technically sort-of modular already, but make them more aggressively modular similar to `HeraldField`.

I plan to obsolete and replace `HeraldCustomAction`.

Test Plan: Saw actions in nice groups; created and ran a "Do Nothing" action. Transcripts are a bit rough for now.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13646
2015-08-03 14:33:20 -07:00
epriestley
f1222f956a Correctly clear draft markers when deleting an inline comment
Summary:
Fixes T8917. Prior to T2618, deleting inlines prompted users, then really deleted the rows.

After T2618, we delete immediately and offer "Undo". However, some interactions with drafts were missed, and we were only clearing the "this revision has a draft" flag on one of the delete pathways (when you delete all the comment text, then save the comment).

Make both the "Delete" action and the "Delete All Comment Text + Save" workflows do the same thing: mark the row as deleted, and clear any relevant drafts.

Test Plan:
  - Made an inline comment on a clean revision with no "draft comments" marker in the list view.
  - Used "delete" to delete it.
  - After applying the patch, verified that no "draft commetns" marker appears in the list view.
  - Used Delete and Edit + Remove Text + Save to delete comments in Differential and Diffusion.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T8917

Differential Revision: https://secure.phabricator.com/D13665
2015-07-21 11:36:46 -07:00
epriestley
0306eb70ed Group and order Herald fields
Summary:
Ref T8726. Some adapters now have a large number of fields, and we lost the sort-of-human-readable implicit ordering when fields were modularized.

Instead, group and sort fields.

Test Plan: {F603066}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13619
2015-07-16 14:13:13 -07:00
epriestley
715233fb61 Fully modularize Herald field values
Summary: Ref T8726. This gets rid of all the `VALUE_*` constants and lets Fields provide arbitrary typeaheads without upstream/JS changes.

Test Plan: Used all tokenizers. Used "Another Herald Rule". Grepped for all removed constants.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13616
2015-07-16 14:12:44 -07:00
epriestley
ed3dfd491f Further modularize Herald values
Summary: Ref T8726. Modularize action values. Fully modularize "text", "none" and "select" controls. Only tokenizers remain.

Test Plan:
  - Used all affected value types in UI.
  - Reviewed rules using new modular rendering, saw sensible output.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13614
2015-07-16 14:12:23 -07:00
epriestley
b2b739c709 Reduce code duplication in Herald fields
Summary:
Ref T8726. I want to modularize values and reduce how hard-coded / copypasta'd they are.

  - Rename `get...StandardCondition()` to `get...StandardType()`, since we can drive both conditions and values from it.
  - Rename `STANDARD_LIST` to `STANDARD_PHID_LIST` for consistency: all "lists" are lists of PHIDs.
  - For all standard types which don't require typehaeads, lift their logic into the base class.
  - I'll lift typeaheads soon, but need to generalize them first.

Test Plan: Edited various Herald rules, saw value UI generate correctly.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13612
2015-07-16 14:11:44 -07:00
epriestley
e8f063de25 Modularize Herald Diffusion pre-commit content fields
Summary: Ref T8726. The gruntwork part of this is finally over.

Test Plan:
  - Made a huge rule with every field.
  - Applied migration.
  - Verified the rule was still the same.
  - Pushed a bunch of commits and verified transcripts.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13601
2015-07-08 12:26:57 -07:00
epriestley
f9134fc331 Modularize all Herald pre-commit ref fields
Summary: Ref T8726. Just (pre-commit content) one more left.

Test Plan:
  - Created a big rule with every field.
  - Migrated it.
  - Verified the rule was still the same.
  - Pushed a bunch of changes and reviewed the transcripts.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13600
2015-07-08 12:26:00 -07:00
epriestley
00b98eca54 Modularize all DifferentialRevision fields
Summary: Ref T8726. Make all the DifferentialRevision stuff modular.

Test Plan:
  - Created a rule with all fields.
  - Ran upgrade.
  - Saw all fields preserved with new modular versions.
  - Used test console to run rule with all fields, verified field values as broadly sensible.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13598
2015-07-08 12:25:48 -07:00
epriestley
50491d2057 Use standard project rendering in Diffusion
Summary: Fixes T8791.

Test Plan: {F599915}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T8791

Differential Revision: https://secure.phabricator.com/D13603
2015-07-08 07:00:17 -07:00
epriestley
98ac0a022c Modularize all Diffusion commit Herald fields
Summary: Ref T8726.

Test Plan:
Created a giant rule with every commit field:

{F594686}

Ran the upgrade, got the same rule with new fields:

{F594688}

Used "Test Console" to run transcripts, saw all the fields populate correctly.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13567
2015-07-07 11:53:17 -07:00
epriestley
59c294457f Modularize more Herald fields
Summary: Ref T8726. Continue making Herald fields more modular than they currently are.

Test Plan:
  - Created a rule using all the affected fields.
  - Ran the rule.
  - Saw reasonable object field values.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: eadler, joshuaspence, epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13495
2015-07-06 13:15:47 -07:00
epriestley
4034c24b43 Fix redesign branch repository controller for Subversion
Summary: We access this variable, but may never initialize it.

Test Plan: Viewed SVN repository.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D13544
2015-07-03 10:19:43 -07:00
epriestley
14a395ed8e Merge branch 'master' into redesign-2015 2015-06-25 10:06:50 -07:00
epriestley
fcb35a55fd Support Spaces in Diffusion
Summary:
Ref T8493. Diffusion is probably the strongest upstream use case we have for Spaces right now, so I want to get us on it to kick the tires a bit.

Small amount of hackiness around the multi-page form thing but it shouldn't create any problems.

Test Plan:
  - Created a new repo.
  - Edited a repo.
  - Tried invalid edits, saw value preserved.
  - Viewed edit full detail screen, saw space info.
  - Viewed repo detail view, saw space.
  - Viewed repo list view, saw space.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8493

Differential Revision: https://secure.phabricator.com/D13414
2015-06-24 10:56:46 -07:00
Eitan Adler
3c7f4e5c5b Remove duplicate duplicate words which are not requires
Summary: Change 'the the' to 'the' where appropriate.

Test Plan: eyeball it

Reviewers: joshuaspence, chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D13412
2015-06-23 17:26:14 -07:00
epriestley
b55f9b6120 Merge branch 'master' into redesign-2015 2015-06-22 12:26:41 -07:00
epriestley
d1983560a6 Show when objects have a non-default policy
Summary:
Fixes T6787. I'm kind of cheating a little bit here by not unifying default selection with `initializeNew(...)` methods, but I figure we can let this settle for a bit and then go do that later. It's pretty minor.

Since we're not doing templates I kind of want to swap the `'template'` key to `'type'` so maybe I'll do that too at some point.

@chad, freel free to change these, I was just trying to make them pretty obvious. I //do// think it's good for them to stand out, but my approach is probably a bit inconsistent/heavy-handed in the new design.

Test Plan:
{F525024}

{F525025}

{F525026}

{F525027}

Reviewers: btrahan, chad

Reviewed By: btrahan

Subscribers: johnny-bit, joshuaspence, chad, epriestley

Maniphest Tasks: T6787

Differential Revision: https://secure.phabricator.com/D13387
2015-06-22 11:46:59 -07:00
epriestley
803c65663d Merge branch 'master' into redesign-2015 2015-06-20 06:10:54 -07:00
epriestley
04516d256b Add an "--importing" flag to bin/repository reparse
Summary: Fixes T6839. Sometimes, worker tasks go astray for whatever reason. This automates the step of `bin/repository importing | xargs | mangle mangle | bin/repostiory reparse`.

Test Plan: Ran various flavors of the command, got good looking results.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6839

Differential Revision: https://secure.phabricator.com/D13362
2015-06-20 05:25:44 -07:00
epriestley
7d7e13d79b Merge branch 'master' into redesign-2015 2015-06-19 08:33:30 -07:00
epriestley
e96cd29eff Reduce badness in viewing large files in the Diffusion web UI
Summary:
Fixes T8597. Second issue there is that if you look at a huge file in Diffusion (like `/path/to/300MB.pdf`) we pull the whole thing over Conduit upfront, then try to shove it into file storage.

Instead, pull no more than the chunk limit (normally 4MB) and don't spend more than 10s pulling data.

If we get 4MB of data and/or time out, just fail with a message in the vein of "this is a really big file".

Eventually, we could improve this:

  - We can determine the //size// of very large files correctly in at least some VCSes, this just takes a little more work. This would let us show the true filesize, at least.
  - We could eventually stream the data out of the VCS, but we can't stream data over Conduit right now and this is a lot of work.

This is just "stop crashing".

Test Plan: Changed limits to 0.01 seconds and 8 bytes and saw reasonable errors. Changed them back and got normal beahvior.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8597

Differential Revision: https://secure.phabricator.com/D13348
2015-06-18 13:06:01 -07:00
epriestley
53ef057b1b Merge branch 'master' into redesign-2015 2015-06-15 08:06:23 -07:00
Joshua Spence
62648237c2 Fix a few more undeclared properties
Summary: Ref T8538. Ref T8539. Ref T8541.

Test Plan: Eyeball it.

Reviewers: epriestley, de_jean_7777, benoittgt, #blessed_reviewers

Reviewed By: benoittgt

Subscribers: epriestley, Korvin

Maniphest Tasks: T8541, T8538, T8539

Differential Revision: https://secure.phabricator.com/D13293
2015-06-15 19:39:04 +10:00
Joshua Spence
b6d745b666 Extend from Phobject
Summary: All classes should extend from some other class. See D13275 for some explanation.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13283
2015-06-15 18:02:27 +10:00
epriestley
2fbc65e396 Call didRejectResult() in DiffusionCommitQuery properly
Summary:
Ref T4345. This error is per object-type in the query implementations, not a mail/permissions issue.

Without `didRejectResult()`, we can't distinguish between "restricted" and "unknown" for objects filtered by `willFilterPage()`.

  - Call `didRejectResult()` on commits.
  - Make `didRejectResult()` handle both existing policy exceptions and filtering.
  - Recover from partial objects (like commits) which are missing attached data required to figure out policies.

Test Plan: Saw "Restricted Diffusion Commit" instead of "Unknown Object (Diffusion Commit)" when viewing nonvisible commit handle in Maniphest.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4345

Differential Revision: https://secure.phabricator.com/D13289
2015-06-14 15:35:32 -07:00
Joshua Spence
ea7397f7e4 Rename PassphraseCredentialType subclasses for consistency
Summary: Ref T5655.

Test Plan: `arc unit` + `grep`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D13266
2015-06-14 14:11:55 +10:00
Chad Little
9170e90112 [Redesign] Move basefont to Lato, remove Source Sans Pro
Summary: Working towards a more unified look and feel. This brings in Lato as a complete base font over Helvetica Neue, as well as removing Source Sans Pro from DocumentView and Conpherence. Design-wise Lato provides the nice readability at larger font sizes that Source Sans Pro did, with the ability to scale down to tables and UI widgets with ease. This gives us one font instead of two, and now Object descriptions and Timeline posts all can benefit from a consistent, readable font.

Test Plan:
Test main UI, smaller elements like tables, menus, DocumentViews, Previews, Conpherence.

{F498135}

{F498136}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13276
2015-06-13 20:32:45 +01:00
epriestley
52f8756c3c Add a "template" parameter to application default policies
Summary:
Ref T5681. Ref T6860. This doesn't do anything interesting on its own, just makes the next diff smaller.

In the next diff, policies become aware of the types of objects they're acting on. We need to specify which object type all the "Default View/Edit" settings are for so they get the right rules.

For example, a rule like "Allow task author" is OK for "View Policy" on a task, and also OK for "Default View Policy" on ManiphestApplication. But it's not OK for "Can Create Tasks" on ManiphestApplication.

So annotate all the "template"/"default" policies with their types. The next diff will use these to let you select appropriate rules for the given object type.

Test Plan:
  - Used `grep` to find these.
  - This change has no effect.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5681, T6860

Differential Revision: https://secure.phabricator.com/D13251
2015-06-11 13:25:30 -07:00
epriestley
57b898af9a Merge branch 'master' into redesign-2015 2015-06-10 07:44:58 -07:00
Joshua Spence
f47e69c015 Mark some strings for translation
Summary: Add some more `pht`izations.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13200
2015-06-09 23:06:52 +10:00
epriestley
ce434e821c Make ACTION_NOTHING a "standard" action in Herald
Summary: Ref T8455. Multiple adapters implement essentially identical effects for ACTION_NOTHING. Consolidate them.

Test Plan: Created several `ACTION_NOTHING` rules and updated the corresponding objects.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8455

Differential Revision: https://secure.phabricator.com/D13178
2015-06-08 10:30:09 -07:00
epriestley
992c199577 Add "Mailing List" users
Summary:
Ref T8387. Adds new mailing list users.

This doesn't migrate anything yet. I also need to update the "Email Addresses" panel to let administrators change the list address.

Test Plan:
  - Created and edited a mailing list user.
  - Viewed profile.
  - Viewed People list.
  - Searched for lists / nonlists.
  - Grepped for all uses of `getIsDisabled()` / `getIsSystemAgent()` and added relevant corresponding behaviors.
  - Hit the web/api/ssh session blocks.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: eadler, tycho.tatitscheff, epriestley

Maniphest Tasks: T8387

Differential Revision: https://secure.phabricator.com/D13123
2015-06-03 18:42:33 -07:00
Chad Little
e4788df9a5 [Redesign] PHUIPagerView
Summary: Ref T8099, Moves AphrontPagerView to PHUIPagerView, converts to standard PHUIButtons and adds some additional features for icon placement on buttons.

Test Plan: Tested Advanced Search and Searching files in Diffusion. Works as expected.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T8342, T8099

Differential Revision: https://secure.phabricator.com/D13092
2015-06-02 14:34:04 -07:00
Joshua Spence
ebde9357c7 Fix reverting commit language
Summary: I think the language here is inverted? On a commit which is reverted, the text states `$author added a reverted commit: $commit`. In reality, `$commit` is the //reverting// commit, not the //reverted// commit.

Test Plan: Think about English.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13067
2015-06-01 09:54:30 +10:00
Chad Little
3dcb672076 [Redesign] Better Headers, more options
Summary: Ref T8099, Adds ability to set header as "tall" and provide better control over height of actual header text. Also fixed some color issues with multiple object boxes.

Test Plan: Review a commit, review dashboards, review a diff, review home.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T8099

Differential Revision: https://secure.phabricator.com/D13077
2015-05-31 08:36:44 -07:00
Aviv Eyal
8ea13f3ce9 Framework for external symbol search
Summary:
Ref T7984. With this, an install can add an ExternalSymbolsSource to src/extensions, which will include whatever
source they have.

Test Plan: search for php and python builtins.

Reviewers: joshuaspence, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7984

Differential Revision: https://secure.phabricator.com/D13036
2015-05-31 07:37:06 -07:00
epriestley
3699253e49 Merge branch 'master' into redesign-2015 2015-05-28 12:00:06 -07:00
epriestley
fcac85d807 Allow configuration of a "staging area" for each repository
Summary:
Ref T8238. This allows configuration of a "staging area" for Git repositories, which is the URI to some Git repository (possibly the same repository).

If a staging area is configured, `arc` will push a copy of anything it creates a diff for there (see next revision). This primarily makes handoff to build systems easier.

This is a bit leaky and I intend for it to eventually be positioned as a less-preferred solution, but from the perspective of build systems it's the same as the real (virtual ref) solution that I want to build.

Test Plan: Ran `arc diff` with various flags, saw appropriate changes copied into the staging area. See also discussion in T8238.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: cburroughs, epriestley

Maniphest Tasks: T8238

Differential Revision: https://secure.phabricator.com/D13019
2015-05-27 10:28:27 -07:00
Joshua Spence
4fc74a6758 Remove arcanist project datasource
Summary: Ref T7604. This class is no longer used.

Test Plan: `grep`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D13004
2015-05-26 07:10:10 +10:00
epriestley
ea131bb2ef Improve UX for Diffusion mail fields
Summary:
  - Give the fields names and descriptions.
  - When new, default-disabled fields are added, disable them by default even if there's already a config.
  - Be a bit less hacky about `$faux_spec`.

Test Plan: {F432383}

Reviewers: joshuaspence, fabe

Reviewed By: fabe

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D13006
2015-05-25 07:18:48 -07:00
Joshua Spence
be19b31a64 Add repository to audit emails
Summary: This is vaguely related to T5791. Add a "REPOSITORY" header to audit emails so that they can be filtered in Gmail.

Test Plan: Commented on an audit and used `./bin/mail show-outbound` to inspect outbound email.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: fabe, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12997
2015-05-25 22:43:42 +10:00
Fabian Stelzer
f0d16b3047 show merged commits in herald emails for merge commits
Summary:
Includes a new Block in Herad emails which tells the user about which commits were merged in a merge commit
Otherwise the email would just say "Merge branch XYZ". Ref T8295

Test Plan: imported various commits (and merges) and watched resulting herald emails for all of them

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T8295

Differential Revision: https://secure.phabricator.com/D12993
2015-05-25 04:49:13 -07:00
Joshua Spence
1b12249b2c Fix some format strings
Summary: These format strings use `%d` instead of `%s`.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12996
2015-05-25 21:29:30 +10:00
epriestley
bfca11dbba Merge branch 'master' into redesign-2015 2015-05-22 12:57:32 -07:00
Chad Little
3cd27a0881 [Redesign] PHUIHeaderView goes to table layout
Summary: Ref T8099. This changes makes flexible layouts easier, and aligns icons, buttons better vertically.

Test Plan: Test various header locations, Phriction, Conpherence, Dashboards, Workboards etc.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T8099

Differential Revision: https://secure.phabricator.com/D12961
2015-05-22 08:22:25 -07:00
Joshua Spence
36e2d02d6e phtize all the things
Summary: `pht`ize a whole bunch of strings in rP.

Test Plan: Intense eyeballing.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12797
2015-05-22 21:16:39 +10:00
Aviv Eyal
f21972a01f Only link symbols if there might be any
Summary:
fixes T8260. Only turn on symbol links if:
- The repository has any configuration about symbols, or
- There actually are symbols in the repository.

Test Plan: Look at revisions and files in various states of configurations and having symbols.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: joshuaspence, Korvin, epriestley

Maniphest Tasks: T8260

Differential Revision: https://secure.phabricator.com/D12946
2015-05-21 08:25:35 -07:00
Chad Little
667159eb07 [Redesign] Add Table, Collapse support to ObjectBox 2015-05-19 23:14:22 -07:00
Joshua Spence
69940f2b9e Replace ArcanistPhutilTestCase refs with PhutilTestCase
Summary: Ref T7977. Remove the `PhabricatorTestCase::getLink` method. Depends on D12665.

Test Plan: `arc unit`.

Reviewers: avivey, #blessed_reviewers, epriestley

Reviewed By: avivey, #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7977

Differential Revision: https://secure.phabricator.com/D12667
2015-05-20 09:40:39 +10:00
Joshua Spence
c896aeb62e Various linter fixes
Summary: Apply various linter fixes.

Test Plan: Unit tests + eyeballing.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12390
2015-05-20 07:27:41 +10:00
Joshua Spence
2f80c01236 Remove arcanist projects from DiffusionRequest
Summary: Ref T7604. This data is no longer used. Depends on D12894.

Test Plan: `grep`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12895
2015-05-20 06:58:25 +10:00
Joshua Spence
f8b7f03145 Change "lint save" to not use Arcanist Projects
Summary: Ref T7604. Change `DiffusionLintSaveRunner` to use repositories instead of Arcanist Projects.

Test Plan: Ran the `save_lint.php` script and queried results using the `diffusion.getlintmessages` Conduit endpoint.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12893
2015-05-19 00:07:47 +10:00
Aviv Eyal
898ce6bace Search Symbols by Repository, not Project
Summary:
Fixes T7977.
- Move Indexed Languages and See Symbols From config to Repository
- Make symbol search skip projects

This also makes the default languages to Everything instead of Nothing.

Test Plan:
- Browse files, click symbols.
- Use quick search to find symbols
- Browse revision, click symbols

Reviewers: joshuaspence, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7977

Differential Revision: https://secure.phabricator.com/D12687
2015-05-18 06:41:19 -07:00
Joshua Spence
61b178f44e Use PhutilInvalidStateException
Summary: Use `PhutilInvalidStateException`. Depends on D12803.

Test Plan: Unit tests pass.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12829
2015-05-14 07:53:52 +10:00
Joshua Spence
acb45968d8 Use __CLASS__ instead of hard-coding class names
Summary: Use `__CLASS__` instead of hard-coding class names. Depends on D12605.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12806
2015-05-14 07:21:13 +10:00
epriestley
7f8ee4e329 Fix a bug with DiffusionMergedCommitsQueryConduitAPIMethod
Summary: We were adding "+1" to PHP_INT_MAX, which could cause us to wrap and run `git log -n -9223372036854775808 ...`. Git, unable to give us negative nine trillion zillion results, would then exit without producing results.

Test Plan: Got results out of this method.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12726
2015-05-05 15:44:34 -07:00
Joshua Spence
70c8649142 Use phutil_json_decode instead of json_decode
Summary: Generally, `phutil_json_decode` should be preferred over `json_decode`.

Test Plan: Eyellballed.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12680
2015-05-05 20:48:55 +10:00
Joshua Spence
2cc898a63f Add repository parameter to diffusion.findsymbols method
Summary: Ref T7977. Add a repository parameter to the `diffusion.findsymbols` Conduit method to allow restricting search results to the specified repository.

Test Plan: Queried the Conduit endpoint.

Reviewers: avivey, epriestley, #blessed_reviewers

Reviewed By: avivey, epriestley, #blessed_reviewers

Subscribers: eadler, avivey, Korvin, epriestley

Maniphest Tasks: T7977

Differential Revision: https://secure.phabricator.com/D12663
2015-05-05 08:27:08 +10:00
Joshua Spence
791de0886f Change default value for bugtraq.url
Summary: `null` is a more-intuitive default value than `''`, methinks.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12671
2015-05-04 12:19:48 +10:00
Joshua Spence
2483f6f120 Move symbols to be repository-based
Summary: Fixes T7220. Ref T7977. Changes symbols from being bound to an Arcanist project to being bound to a repository.

Test Plan:
- Added symbols and then applied migrations, symbols seemed to be migrated successfully.
- Tested the `/diffusion/symbol/$SYMBOL_NAME` endpoint.
- Tested the `/diffusion/symbol/$SYMBOL_NAME` endpoint with the `?repositories=$REPOSITORY_PHID` parameter.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: avivey, Korvin, epriestley

Maniphest Tasks: T7977, T7220

Differential Revision: https://secure.phabricator.com/D12608
2015-05-03 13:23:07 +10:00
epriestley
28d0094856 Improve ref resolution for Git branches and tags
Summary:
Fixes T7982.

  - When resolving branches, make sure they get type `'branch'`.
  - Correctly resolve refs when a repository has a branch and tag with the same name.

Test Plan: Disabled ref cache and resolved refs in a Git repository with a 'master' tag and a 'master' branch. Saw refs resolve accurately.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7982

Differential Revision: https://secure.phabricator.com/D12609
2015-04-29 13:21:12 -07:00
epriestley
ced20d48ea Improve handling of bad branches in Diffusion
Summary:
Fixes T7972.

  - Trap the RefNotFound error which may occur in `getAlternatives()`.
  - Improve error handling in Mercurial.

Test Plan: {F387611}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7972

Differential Revision: https://secure.phabricator.com/D12590
2015-04-28 08:56:16 -07:00
epriestley
2a37459a5f Only resolve branch names to branches
Summary: Fixes T7100. In the bizarre case that a Git repository has a branch and tag with the same name, don't resolve branch names into tag names.

Test Plan: Test repo with branch and tag both named "git" no longer reports ambiguity.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7100

Differential Revision: https://secure.phabricator.com/D12553
2015-04-27 03:51:53 -07:00