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

9000 commits

Author SHA1 Message Date
epriestley
f24318f308 Make "profile menu" configuration mostly work
Summary:
Ref T10054. This does a big chunk of the legwork to let users reconfigure profile menus (currently, just project menus).

This includes:

  - Editing builtin items (e.g., you can rename the default items).
  - Creating new items (for now, only links are available).

This does not yet include:

  - Hiding items.
  - Reordering items.
  - Lots of fancy types of items (dashboards, etc).
  - Any UI changes.
  - Documentation (does feature: TODO link for documentation).

Test Plan:
{F1060695}

{F1060696}

{F1060697}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10054

Differential Revision: https://secure.phabricator.com/D15010
2016-01-13 11:45:31 -08:00
epriestley
bb6df2e5f3 Hacks on Hacks
Summary:
hack hack hack

(`class_exists()` no longer throws in a libphutil environment.)

Test Plan: derpaderp

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15013
2016-01-13 11:09:13 -08:00
epriestley
fdd5500fec Change wording of "Show Entire File" to "Show All Context"
Summary: Fixes T10139. This clarification seems reasonable to me.

Test Plan:
  - Viewed a revision with context.
  - Clicked "Show All Context".
  - Saw "All Context Shown".

{F1060624}

{F1060625}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10139

Differential Revision: https://secure.phabricator.com/D15009
2016-01-13 11:09:01 -08:00
epriestley
102eacbfb7 Make project page/navigation construction more consistent, particularly on mobile
Summary: Ref T10054. This mostly makes sure that mobile gets to have the same profile menu that Desktop does.

Test Plan:
  - Viewed menus on mobile, saw all profile menu actions available.
  - Viewed/used changed pages (column detail, edit column, edit picture).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10054

Differential Revision: https://secure.phabricator.com/D15008
2016-01-13 09:34:57 -08:00
epriestley
7bde92b9c9 Begin modularizing profile panel/link construction
Summary: Ref T10054. This has no product impact, but prepares us for customizable side nav on "profiles" (today, projects; probably users some day; and maybe other stuff down the road).

Test Plan: Clicked all links on a profile, everything was exactly the same as before.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10054

Differential Revision: https://secure.phabricator.com/D15007
2016-01-13 09:34:41 -08: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
d57c1d6ce2 Make hashtags at the end of bolded text render properly
Summary: Fixes T10096. The `**` was being parsed as part of the hashtag, so `**#asdf**` interpreted `#asdf**` as a hashtag.

Test Plan: Unit test; bolded stuff with hashy contents.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10096

Differential Revision: https://secure.phabricator.com/D15006
2016-01-12 10:03:20 -08:00
epriestley
dd21761df9 Don't return any results for viewerprojects() if the viewer is in no projects
Summary:
Fixes T10135. When the viewer is a member of no projects, specify the constraint type as a new "EMPTY" type.

When a query has an "EMPTY" constraint, fail fast with no results.

Test Plan:
  - Viewed a viewerprojects() query result set as a user in no projects.
    - Before patch: got a lot of hits. After patch: no hits.
  - Viewed a normal result set, no changes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10135

Differential Revision: https://secure.phabricator.com/D15003
2016-01-12 07:11:14 -08:00
epriestley
39a87f6000 Turn off subprojects and milestones in the UI completely for now
Summary: A lot of confusing stuff happens when you create a milestone or subproject which isn't explained clearly in the UI. This is causing more harm than good on the balance since we're still figuring out how to move forward here. Just turn it off for now until we're closer to pushing it forward.

Test Plan: Viewed a project, no more UI for subprojects or milestones.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14999
2016-01-11 14:54:23 -08:00
epriestley
7e0558612b Fix excessively harsh validation of certain complex configuration
Summary:
See IRC. We're supposed to repair configuration, but if custom validators throw a generic `Exception` or use `PhutilTypeSpec` to do a check, we may explode way harder than we intend to.

Instead, soften these exceptions into validation exceptions so we repair configuration, raise a setup issue, and continue.

Test Plan: {F1059609}

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14998
2016-01-11 14:20:11 -08:00
epriestley
8e1b2f9861 Remove "username@phabricator.mycompany.com" creating a Conpherence
Summary:
Ref T10121. This doesn't work at all at HEAD, and even when it did it was mostly just confusing to installs with unexpected setups where Phabricator is receiving mail at `@mycompany.com` and this is colliding with real addresses.

It might make sense to restore it some day after the next Conphernece update, but just strip it out for now. Since it doesn't work anyway, I'm pretty confident no one is using it.

Test Plan:
  - Before patch: send mail to `dog@local.phacility.com`, got a policy error from Conpherece.
  - After patch: sent mail to `dog@local.phacility.com`, got a correct "no routable recipients" error.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10121

Differential Revision: https://secure.phabricator.com/D14997
2016-01-11 10:32:23 -08:00
epriestley
516ba5e6c5 Fix an issue where generation of mail processing error email could fail in the presence of duplicate headers
Summary:
Ref T10121. If a user sends mail with duplicate headers, like:

```
X-Duplicate: A
X-Duplicate: B
```

...and we process it with `mail_handler.php`, we may end up with `array('A', 'B')` as the header value. When we try to write this back into an error response mail, it fails.

Test Plan:
  - Generated a message with duplicate headers.
  - Piped it into `mail_handler.php` with `--process-duplicates` and `--trace` to get a look at it.
  - Faked an exception.
  - Before patch: bad error email.
  - After patch: clean error email showing multiple header values.

```
$ ./bin/mail show-outbound --id 12386
PROPERTIES
ID: 12386
Status: queued
Related PHID:
Message:

PARAMETERS
sensitive: 1
is-error: 1
force: 1
subject: Error Processing Mail (Exception)
raw-to: ["epriestley@phacility.com"]

HEADERS
X-Phabricator-Sent-This-Message: Yes
X-Mail-Transport-Agent: MetaMTA
X-Auto-Response-Suppress: All

TEXT BODY
Your email to Phabricator was not processed, because an error occurred while
trying to handle it:

Exception: TEST

-- Original Message Body -----------------------------------------------------

testy testy

-- Original Message Headers --------------------------------------------------

from: Evan Priestley <epriestley@phacility.com>
content-type: text/plain; charset=us-ascii
content-transfer-encoding: 7bit
x-smtp-server: smtp.gmail.com:epriestley@phacility.com
subject: test outbound mail
message-id: 7isvptmllqvdvtdxthvdwzg3woj5au7csyuh3hopypjv6y6hqb32qm4bcrd4jtid
x-universally-unique-identifier: 4E489E20-F674-49B2-94BA-0DE44F504EAA
date: Mon, 11 Jan 2016 09:50:12 -0800
date: Mon, 11 Jan 2016 09:50:13 -0800
date: Mon, 11 Jan 2016 09:50:14 -0800
date: Mon, 11 Jan 2016 09:50:15 -0800
to: epriestley@yghe.net
mime-version: 1.0 (Mac OS X Mail 8.2 \(2104\))

HTML BODY
(This message has no HTML body.)
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10121

Differential Revision: https://secure.phabricator.com/D14996
2016-01-11 10:16:57 -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
add8333b98 Improve behavior of "owner" transaction in "maniphest.edit" endpoint
Summary:
Fixes T10117.

  - I accidentally broke setting `null` to unassign tasks at some point when I added richer validation.
  - Raise a better error if the user passes junk.

Test Plan:
  - Unassigned a task via API and web UI.
  - Reassigned a task via API and web UI.
  - Tried to do an invalid assign via API, got a sensible error.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10117

Differential Revision: https://secure.phabricator.com/D14992
2016-01-11 09:19:18 -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
a06715ccdd Allow repository short names to be used as identifiers
Summary:
Ref T4245. This allows `bin/repository update bread` to work, in addition to `rBREAD`, `R123`, `123`, `BREAD`, etc., if a repository has a short name set.

This primarily affects CLI commands (like `bin/repository`) and Conduit API calls. It has no normal user-facing impact.

Test Plan: Ran `bin/repository update bread` and such.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14988
2016-01-11 09:17:13 -08:00
epriestley
f59ebf4c09 Fix incorrect key handling in extended policy filtering
Summary:
Via HackerOne. The use of `$key` here should be `$extended_key`.

Exploiting this requires a very unusual group of objects to be subjected to extended policy checks. I believe there is no way to actually get anything bad through the policy filter today, but this could have been an issue in the future.

Test Plan:
  - Added a unit test which snuck something through the policy filter.
  - Fixed use of `$extended_key`.
  - Test now passes.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14993
2016-01-11 07:04:47 -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
e84693f589 Fix a bad lookup in hovercards
Summary:
If you try to pull the hovercard of something you can no longer see (maybe you loaded the page, then the policy changed) there won't be a value in the array here.

(The rest of the code anticipates this possibility.)

Test Plan: Hovered some stuff.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14987
2016-01-10 12:12:12 -08:00
epriestley
96b1665eaa Link "continue" action to confirm dialog in bulk jobs that are unconfirmed
Summary: See Q266.

Test Plan: Created a bulk job, clicked "Details" instead of "Confirm", clicked "Continue" to get back to confirmation dialog.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14985
2016-01-10 10:55:58 -08:00
epriestley
cb08757032 Swap S3 to first-party client
Summary:
Ref T5155. Swaps Phabricator over to the new first-party S3 client using the v4 authentication API so it works in all regions.

The API requires an explicit region, so the new `amazon-s3.region` is now required. I'll write guidance about this.

Test Plan:
  - Uploaded files to S3.
  - Migrated ~1GB of files to S3.
  - Loaded a bunch of files off S3.
  - Browsed around the S3 bucket.
  - Deleted a file, verified the data on S3 was destroyed.
  - Hit new setup warning.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5155

Differential Revision: https://secure.phabricator.com/D14982
2016-01-10 07:55:27 -08:00
epriestley
ff6bfe387d Don't drop "phabricator-remarkup-embed-image" class from Remarkup images with width or height
Summary: Ref T10110. If an image had `width` or `height`, we would accidentally not give it an `$image_class`.

Test Plan:
{F1057988}

{F1057989}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10110

Differential Revision: https://secure.phabricator.com/D14983
2016-01-09 15:40:45 -08:00
epriestley
67ac356b03 Modernize bin/files migrate and fix behavior on chunk engines
Summary: Ref T9828. Mostly just does a minor modernization pass, but also doesn't migrate chunked files since it's not meaningful (they don't have data, directly).

Test Plan: Ran `bin/files migrate` with various flags. Migrated S3 -> Blob and Blob -> S3.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9828

Differential Revision: https://secure.phabricator.com/D14981
2016-01-09 11:46:23 -08:00
epriestley
efba69a440 Give users an explicit error if they try to upload a too-large diff to Differential
Summary:
Ref T8612. If a change affects more than 10K paths + hunks, tell the user it's too big and don't bother trying to write it. We're mostly bounded by INSERTs here.

Also, fix an issue with file upload errors. The keys are real PHP constants, but were accidentally converted to strings in D12797, causing every error to show as "unknown error".

Test Plan: {F1057509}

Reviewers: joshuaspence

Reviewed By: joshuaspence

Maniphest Tasks: T8612

Differential Revision: https://secure.phabricator.com/D14977
2016-01-08 18:53:33 -08:00
epriestley
2c293bdca8 Fix a missed use of project icons in typehaeads
Summary: I missed this in the recent icon customziation thing.

Test Plan: Typehaead'ed some projects, saw icons properly.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14975
2016-01-08 14:41:11 -08:00
epriestley
9ab22e21b3 Allow installs to customize project icons
Summary:
Ref T10010. Ref T5819. General alignment of the stars:

  - There were some hacks in Conduit around stripping `fa-...` off icons when reading and writing that I wanted to get rid of.
  - We probably have room for a subtitle in the new heavy nav, and using the icon name is a good starting point (and maybe good enough on its own?)
  - The project list was real bad looking with redundant tag/names, now it is very slightly less bad looking with non-redundant types?
  - Some installs will want to call Milestones something else, and this gets us a big part of the way there.
  - This may slightly help to reinforce "tag" vs "policy" vs "group" stuff?

---

I'm letting installs have enough rope to shoot themselves in the foot (e.g., define 100 icons). It isn't the end of the world if they reuse icons, and is clearly their fault.

I think the cases where 100 icons will break down are:

  - Icon selector dialog may get very unwieldy.
  - Query UI will be pretty iffy/huge with 100 icons.

We could improve these fairly easily if an install comes up with a reasonable use case for having 100 icons.

---

The UI on the icon itself in the list views is a little iffy -- mostly, it's too saturated/bold.

I'd ideally like to try either:

  - rendering a "shade" version (i.e. lighter, less-saturated color); or
  - rendering a "shade" tag with just the icon in it.

However, there didn't seem to be a way to do the first one right now (`fa-example sh-blue` doesn't work) and the second one had weird margins/padding, so I left it like this for now. I figure we can clean it up once we build the thick nav, since that will probably also want an identical element.

(I don't want to render a full tag with the icon + name since I think that's confusing -- it looks like a project/object tag, but is not.)

Test Plan:
{F1049905}

{F1049906}

Reviewers: chad

Reviewed By: chad

Subscribers: 20after4, Luke081515.2

Maniphest Tasks: T5819, T10010

Differential Revision: https://secure.phabricator.com/D14918
2016-01-08 14:01:53 -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
da3963b009 Convert a low-level VCS query in Diff extraction to a Conduit call
Summary: Ref T9319. Ref T2783. This won't currently work in a future environment where daemons and repositories are not on the same host. Send it over Conduit instead.

Test Plan: Used `bin/differential attach-commit rX Dy` to force attachment, saw valid content pull over Conduit.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2783, T9319

Differential Revision: https://secure.phabricator.com/D14969
2016-01-08 09:28:15 -08:00
epriestley
413ca12fda Move commit attachment to a separate CLI command
Summary:
Ref T9319. See D14967. As before, this is making a deeply-buried, complex operation easier to test by providing a CLI command.

This adds `bin/differential attach-commit rXnnnn Dnnnn` to pretend that `rXnnnn` was just committed and matched `Dnnnn`.

Test Plan:
  - Ran `bin/differential attach-commit X Y` for several different values, saw updates in the UI.
  - Faked the message parser to make sure stuff still worked there.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9319

Differential Revision: https://secure.phabricator.com/D14968
2016-01-08 09:27:57 -08:00
epriestley
5592e59f92 Improve error message if local Git working copy directory exists but isn't a working copy
Summary: Fixes T9701. I don't want to try to autofix this because destroying the directory could destroy important files, but we can improve the error message.

Test Plan: Faked a failure, ran `repository update X`, got a more tailored error message.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9701

Differential Revision: https://secure.phabricator.com/D14971
2016-01-08 09:26:40 -08:00
epriestley
2fcf571bfd Use more reassuring UI and copy for removing payment methods
Summary:
The old treatment was fairly technical. Give this UI a more human-friendly flow:

  - Use language "remove" instead of "disable". We keep the record that the card existed around for auditing/historical purposes, but it is no longer a valid payment method going forward and can not be undone. I think this aligns with user expectation and actual behavior better than "disable".
  - Only show active methods on the profile screen.

Test Plan: {F1057153}

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14973
2016-01-08 09:25:36 -08:00
epriestley
0dd947cced Move diff extraction from commits to a separate test with a CLI command
Summary:
Ref T9319. When we discover a commit, we sometimes update the corresponding revision with a "this is the actual committed change" diff and send out a link to the changes between review and commit.

This is currently very difficult to test, because it only happens the first time and you have to either go set up a bunch of objects or add a bunch of special casing to the parser to hit the workflow.

I'm making some changes to how it pulls file content. To make those changes easier to test, first start extracting this stuff so the code can be run with `bin/differential extract ...` instead of needing to do a bunch of more complicated setup steps.

Test Plan:
  - Ran `bin/differential extract ...` to extract diffs from commits.
  - Forced my way through the daemon workflow by faking out a bunch of flags, got a clean extract + attach + update. After this patch, this should rarely be necessary.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9319

Differential Revision: https://secure.phabricator.com/D14967
2016-01-08 09:22:37 -08:00
epriestley
7ba13edc2e Fix an issue with the Phortune card disable route
I think this got clipped in modernization at some point.

Auditors: chad
2016-01-08 09:08:43 -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
Chad Little
ab27af9fc6 Fix badges edit form
Summary: Make sure to subclass the right controller on badges.

Test Plan: arc liberate, make a custom badges edit form.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14961
2016-01-06 21:13:52 +00: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
d326c6096e Make unsubscribing from a project have an effect
Summary:
Fixes T10089. This did work at one point, but was broken by D12868, which got too aggressive about mailing members.

We don't want to send mail to all members by default, only those who are subscribed. The parent implementation of `getMailCC()` handles this for us.

Test Plan:
Joined a project as users A and B. Unsubscribed with B. Made an edit.

Before patch: both A and B got mail. After patch: only A got mail.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10089

Differential Revision: https://secure.phabricator.com/D14955
2016-01-05 18:16:45 -08:00
epriestley
e068188ea1 Mention !status explicitly in the documentation for !close
Summary: Ref T10088.

Test Plan: {F1055107}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10088

Differential Revision: https://secure.phabricator.com/D14953
2016-01-05 15:08:52 -08:00
Chad Little
744215d5ff Add Herald Adapters to Phame
Summary: Adds a basic HeraldAdapter to Phame Blogs and Posts.

Test Plan: Make a Herald rule to CC me on new posts or blogs automatically.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14897
2016-01-05 14:10:43 -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
d0cdf1efdb When a repository is importing, show it on the list view
Summary: Fixes T9191. This is pretty fluff but doesn't hurt anything, I guess.

Test Plan: Viewed repository list, saw an importing repository get a little icon.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9191

Differential Revision: https://secure.phabricator.com/D14950
2016-01-05 14:02:30 -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
08de131da5 Begin modularizing main menu items
Summary:
Ref T10077. Ref T8918. The way the main menu is built is not very modular and fairly hacky.

It assumes menus are provided by applications, but this isn't exactly true. Notably, the "Quick Create" menu is not per-application.

The current method of building this menu is very inefficient (see T10077). Particularly, we have to build it //twice// because we need to build it once to render the item and then again to render the dropdown options.

Start cleaning this up. This diff doesn't actually have any behavioral changes, since I can't swap the menu over until we get rid of all the other items and I haven't extended this to Notifications/Conpherence yet so it doesn't actually fix T8918.

Test Plan: Viewed menus while logged in, logged out, in different applications, in desktop/mobile. Nothing appeared different.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8918, T10077

Differential Revision: https://secure.phabricator.com/D14922
2016-01-04 06:57:09 -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
d9e034f02c Remove calls to getCallsign() from repository daemons
Summary: Ref T4245. These are all descriptive or UI-facing.

Test Plan:
- Ran `bin/repository pull ...` with various identifiers.
- Ran `bin/repository mirror ...` with various identifiers.
- Ran `bin/repository discover ...` with various identifiers.
- Ran `bin/phd debug pull X Y --not Z` with various identifiers.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14926
2016-01-02 11:03:51 -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
2328e739b7 Fix an issue where Phame could post to the wrong blog
When you `getInt()` an array, PHP decides the array has value `1`. This would
cause us to post to blog #1 incorrectly. I didn't catch this locally because
I happened to be posting to blog #1.

Stop us from interpreting array values as `1`, and fix blog interpretation.

This approach is a little messy (projects has the same issue) but I'll see
if I can clean it up in some future change.

Auditors: chad
2016-01-02 05:20:41 -08:00
epriestley
edcc3232aa Remove calls to getCallsign() in bin/repository scripts
Summary:
Ref T4245. Prepare these scripts for a callsign-free world. This also makes them more flexible and easier to use.

The following are now valid ways to identify a repository for these scripts: ID (`3`), PHID (`PHID-REPO-wxyz`), R<ID> (`R3`), r<CALLSIGN> (`rSKYNET`), CALLSIGN (`SKYNET`).

In the future, a human-readable label (`skynet`) may also become valid.

Test Plan:
- Ran `bin/repository reparse --all ...` with `rX`, `X`, `3`, `R3`.
- Ran `bin/repository reparse --change ...` with `rXaaa`, including short versions.
- Ran `bin/repository update ...` with `rX`, `X`, `3`, `R3`.
- Ran `bin/repository refs ...` with various identifiers.
- Ran `bin/repository pull ...` with various identifiers.
- Ran `bin/repository mirror ...` with various identifiers.
- Ran `bin/repository mark-imported ...` with various identifiers.
- Ran `bin/repository list`.
- Ran `bin/repository importing ...` with various identifiers and examined output.
- Ran `bin/repository edit ...` with various identifiers.
- Ran `bin/repository discover ...` with various identifiers.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14924
2016-01-02 04:23:26 -08:00
epriestley
ff1bfb64dd Reduce the total number of calls to getCallsign()
Summary: Ref T4245. Before doing any hard work here, we can dramatically reduce the number of things that make calls to `getCallsign()` to make navigating things easier. Almost all of them only care about a monogram, URI, or display name.

Test Plan:
- Searched for `r uniquename` in jump nav.
- Ran `bin/repository reparse --change rXXXyyyyy --trace`, observed query against bad commit table.
- Ran `bin/search index rXXXyyyy --trace --force`, observed proper title when indexing commit.
- Browed repository list, saw proper `rXXX` and appropriate link targets.
- Mentioned `rXXX` in Remarkup, got a link to the right place.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14923
2016-01-02 04:23:06 -08:00
Chad Little
45ccc930ec Convert Badges to use EditEngine
Summary: Moves Badges over to EditEngine

Test Plan: Create a new Badge, Edit a Badge

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14771
2016-01-01 16:12:13 +00:00
Chad Little
fe6224f505 Add Next and Previous UI to PhamePostView
Summary: Creates a new next/previous UI for PhamePosts, and adds a setFoot to PHUIDocumentViewPro for future use in other apps.

Test Plan:
Test first, next, last posts on Phame in mobile, desktop, and tablet breakpoints.

{F1050152}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14919
2015-12-31 13:09:59 -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
389e4d1b1f Lock milestone projects to an automatic color/icon
Summary:
Ref T10010.

Currently, milestone subproject have editable icons/colors, but I don't think this is likely to be used much (the expectation is that milestones will be common and homogenous, and it doesn't make much sense to pick different icons for "Sprint 32" vs "Sprint 33", I think).

Locking the icon and color lets us simplify the form, make milestones more distinct, and potentially reuse the color later for other things (e.g., active/future/past or on time / overdue or whatever else) or just give them a special color to make them more visible.

The best argument for retaining this that I can come up with is that certain milestones may be special (e.g., Sprint 19 is a major release?), but you can just name it "Sprint 19 (v3.0!)" or something, which seems pretty good for now.

Also don't show milestones on task browse/list view.

Test Plan: {F1048532}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14912
2015-12-30 14:42:50 -08:00
epriestley
972788b8b5 Give IconSetControl a meaningful disabled state
Summary: Ref T10004. This control doesn't disable visually or behaviorally, e.g. when locked in an EditEngine configuration.

Test Plan:
  - Locked field for Projects.
  - Reviewed form in EditEngine.
  - Created/edited a project.
  - Swapped default.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10004

Differential Revision: https://secure.phabricator.com/D14911
2015-12-30 14:42:27 -08:00
Chad Little
5ea5b0c41c Update some PhamePost transactions
Summary: Cleans up some language, colors, etc.

Test Plan: Write lots of new posts, hide them, edit them, check history.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9360

Differential Revision: https://secure.phabricator.com/D14914
2015-12-30 12:57:00 -08:00
Chad Little
4acb7f63e8 Drop domain key on PhameBlog
Summary: Right now you can't create two blogs without a domain name, since it has a unique key on the column. Removing the key.

Test Plan: Create two blogs with no domain name, works as expected. Create two blogs with `cat.dog` as domain name, get duplicate domain error.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9360

Differential Revision: https://secure.phabricator.com/D14915
2015-12-30 12:55:43 -08:00
epriestley
14ae3c099c Make queries for Project "X" mean "X, or any subproject of X"
Summary:
Ref T10010. I think this is the desired/expected default behavior (e.g., searching for "Maniphest" should find tasks in any subproject or sprint of that project).

I'll probably add an "exact(...)" function later to mean "only the Maniphest superproject, exactly, not any of its children".

Test Plan:
  - Added and executed unit tests.
  - Ran various queries from the web UI.
  - Got sensible-seeming results.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14910
2015-12-29 10:41:13 -08:00
epriestley
70053beeed Smooth out milestone creation workflow
Summary:
Ref T10010.

  - Default name to "Milestone X".
  - Remove policy controls, which have no effect.
  - Don't generate slugs for milestones since this is a big pain where they all generate as `#milestone_1` by default (you can add one if you want). I plan to add some kind of syntax like `#parent/32` to mean "Milestone 32 in Parent" later.
  - Don't require projects to have unique names (again, 900 copies of "Milestone X"). I think we can trust users to sort this out for themselves since modern Phabricator has "Can Create Projects" permission, etc.

Test Plan: Created some milestones, had a less awful experience.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14909
2015-12-29 10:40:28 -08:00
epriestley
7c5ad63fd1 Add very basic UI for creating milestones and subprojects
Summary:
Ref T10010. This has a lot of UI/UX problems but I think it:

  - technically allows subproject creation;
  - technically allows milestone creation;
  - doesn't let users unwittingly destroy their installs (probably).

Test Plan:
  - Created milestones.
  - Created subprojects.
  - Created and edited normal projects.
  - Observed some reasonable interactions (e.g., you can't create milestones for a milestone or edit a superproject's members).
  - Observed plenty of silly/confusing interactions that need additional work.

{F1046657}

{F1046658}

{F1046655}

{F1046656}

{F1046654}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14904
2015-12-29 10:40:16 -08:00
Chad Little
7732f9c03c Fix bad query on PhameHome with no Blogs
Summary: We're checking for drafts even though we already know there are no blogs, just skip the query.

Test Plan: trucate phame_blogs; See proper blank state.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14908
2015-12-29 15:24:06 +00:00
epriestley
1443e4b13d Fix an excessively aggressive transaction check in Owners
Summary: Fixes T10058. We don't need to continue on this check if no path changes are being applied.

Test Plan: Archived an owners package.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10058

Differential Revision: https://secure.phabricator.com/D14906
2015-12-28 14:10:43 -08:00
epriestley
33384abff7 Fix an exception in Tokens if a bad object was given a token
Summary:
Fixes T10057. Root issue is:

  - In the past, you could give tokens to objects of type X (here, Ponder answers).
  - Now, you can't.
  - If you try to load a token on an object of type X, we do a bad call to attach it and fatal.

Instead, make sure objects implement the proper interface before we attach them, and just pretend the token does not exist otherwise.

Test Plan: Faked the exception in T10057, applied patch, got clean tokens page.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10057

Differential Revision: https://secure.phabricator.com/D14905
2015-12-28 13:28:25 -08:00
epriestley
abd60eeee0 Rough data fetch for previous/next posts on a blog
Summary: Ref T9897. Not pretty, but pulls data.

Test Plan: {F1046464}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D14903
2015-12-28 07:13:23 -08:00
epriestley
e0a97c88db Provide phame.post.edit Conduit API method
Summary:
Ref T9897. This one is a little more involved because of how getting a post on a blog works.

I also changed moving posts to be a real transaction (which shows up in history, now).

Test Plan: Created posts from web UI and conduit.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D14902
2015-12-28 06:55:35 -08:00
epriestley
00f1389f72 Add phame.post.search Conduit API endpoint
Summary: Ref T9897. Mostly straightforward, but also modernize/fixup the Query a little so that posts never load with no blog.

Test Plan: Queried posts via API.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D14901
2015-12-28 06:49:43 -08:00
epriestley
b74f93f229 Add phame.blog.search Conduit API endpoint
Summary: Ref T9897. Adds basic blog query support.

Test Plan: Ran some queries.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D14900
2015-12-28 06:49:28 -08:00
epriestley
3335bcbfc9 Add a phame.blog.edit Conduit API endpoint
Summary: Ref T9897.

Test Plan: Used API to make a few changes to a blog.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D14899
2015-12-28 06:49:16 -08:00
epriestley
6cb01374a5 Remove previous-generation Phame Conduit API methods
Summary: Ref T9897. We can now provide modern `search` and `edit` endpoints (I'll do this next).

Test Plan: Grepped for removed methods.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D14898
2015-12-28 06:48:58 -08:00
Chad Little
10ed330523 Update PhamePost to EditEngine
Summary: Allows create and edit workflows through EditEngine. Not sure I did the 'blog' stuff correct.

Test Plan: Create a new post, edit a post, move a post.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14802
2015-12-27 17:40:37 -08:00
epriestley
6fe882e50a Convert projects to EditEngine
Summary: Ref T10010. This is pretty straightforward with a couple of very minor new behaviors, like the icon selector edit field.

Test Plan:
  - Created projects.
  - Edited projects.
  - Saw "Create Project" in quick create menu.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14896
2015-12-27 15:42:50 -08:00
epriestley
e8ddfad6db Move "Lock Project" to a separate action
Summary:
Ref T10010. Three motivations:

  - Primarily: this makes conversion to EditEngine easier since I don't have to convert this weird control.
  - This probably needs to have "Lock", "Unlock" and "Use Parent Project Setting" values after subprojects? But maybe just locking any parent locks all the children? Anyway, doesn't make sense to put it on the main edit form if it's weird like this, I think, since we'll want some kind of explanatory text.
  - I probably want to move this to the "Members" tab anyway, and this won't be available on milestone projects at all.

Test Plan: Locked, unlocked, edited projects.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14895
2015-12-27 09:28:34 -08:00
epriestley
11e53f2948 Add empty subproject/milestone controllers
Summary: Ref T10010. These do nothing yet.

Test Plan: Clicked 'em.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14894
2015-12-27 09:26:47 -08:00
epriestley
373ff7f9d4 Read materialized project members instead of real members
Summary:
Ref T10010. This will allow us to find superprojects with `withMemberPHIDs(...)` queries.

  - Copy all the current real member edges to materialized member edges.
  - Redirect all reads to look at materialized members.
  - This table is already kept in sync by earlier work with indexing.

Basically, flow is:

  - Writes (joining, leaving, adding/removing members) write to the real member edge type.
  - After a project's members change, they're copied to the materialized member edge type for that project and all of its superprojects.
  - Reads look at materialized members, so "Parent" sees the members of "Child" and "Grandchild" as its own members, but we still have the "real members" edge type to keep track of "natural" or "direct" members.

Test Plan:
  - Ran migration.
  - Ran unit tests.
  - Saw the same projects as projects I was a member of.
  - Added some `var_dump()` stuff to verify the Owners changed.
  - Used `grep` to look for other readers of this edge type.
  - Made some project updates.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14893
2015-12-27 09:26:27 -08:00
epriestley
77897ce862 Clean up ProjectQuery when viewer is logged-out or omnipotent
Summary:
Ref T10010. When the viewer is logged-out or omnipotent, we can skip this query.

(Currently we issue a silly query like `src = X AND type = Y AND dst = ''`, which will never return results.)

Test Plan:
  - Viewed projects as normal user and logged-out user.
  - Ran unit tests.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14892
2015-12-27 09:26:11 -08:00
epriestley
2b5d4bca8a Put some crumbs on some project pages
Summary: Ref T10010. This is primarily to make "Parent > Child > Grandchild" navigation more manageable for subprojects, at least for now.

Test Plan: Viewed profile, members, feed; saw crumbs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14891
2015-12-27 09:21:50 -08:00
epriestley
5e715c1aca Simplify some logic in project controllers
Summary: Ref T10010. Several controlers currently have similar logic for handling tags and slugs, loading projects, and canonicalizing URIs. Clean it up a bit.

Test Plan:
  - Visited profile, boards, feed.
  - Visited by ID and by tag.
  - Visited by non-normal tag (redircted).
  - Visited by alternate tag (redirected).
  - Visited non-policy project by non-normal tag (redirected into policy error).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14890
2015-12-27 09:21:31 -08:00
epriestley
d1f1d3ec33 Implement a basic project.search third-generation API method
Summary: Ref T10010. This still needs support for attachments (to get members) and more constraints (like slugs), but mostly works.

Test Plan: Ran query, saw basically sensible results.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14889
2015-12-27 09:21:13 -08:00
epriestley
211a6c0d55 Move project slug normalization inside project Query
Summary:
Ref T10010. We currently require `withSlugs()` to have properly formatted slugs, but this leads to similar code in several places.

Instead: accept any slug, normalize slugs in the query, return a map so callers can figure out what happened if they want.

This tends to do the right thing by default, while keeping enough information around to do more complex things if necessary. A similar approach for querying commits has worked well in Diffusion.

Test Plan: Added and executed unit tests.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14888
2015-12-27 09:20:41 -08:00
epriestley
aa2089ba68 Support field previews in EditEngine
Summary: Ref T10004. This primarily supports moving Phame to EditEngine.

Test Plan: {F1045166}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10004

Differential Revision: https://secure.phabricator.com/D14887
2015-12-27 08:17:18 -08:00
epriestley
367955f3fd Improve UX and messaging for certain errors when landing revisions
Summary:
Ref T9994.

  - Allow errors to be dismissed.
  - Tailor messaging for closed/abandoned revisions.
  - Reduce scare messaging on land dialog, since it's not really that scary anymore.

Test Plan:
  - Dismissed errors.
  - Hit new warnings.
  - Wasn't as scared when landing.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9994

Differential Revision: https://secure.phabricator.com/D14886
2015-12-27 08:01:20 -08:00
epriestley
8f81b34ea1 Improve lispum generation of pastes
Summary: Fixes T8482, or something. I can't actually repro that but I think it should be fixed either here or earlier

Test Plan:
```
$ ./bin/lipsum generate paste
 GENERATORS  Selected generators: Pastes.
 WARNING  This command generates synthetic test data, including user accounts. It is intended for use in development environments so you can test features more easily. There is no easy way to delete this data or undo the effects of this command. If you run it in a production environment, it will pollute your data with large amounts of meaningless garbage that you can not get rid of.

    Are you sure you want to generate piles of garbage? [y/N] y

 LIPSUM  Generating synthetic test objects forever. Use ^C to stop when satisfied.
Generated "Paste": P223 forgotten_memory_disks_backup.java
Generated "Paste": P224 backup_disk_tables_and_administrate_backup_memory_account.java
Generated "Paste": P225 sync_backup_disk_and_undo_memory.php
Generated "Paste": P226 administrate_memory_shard_helper.php
Generated "Paste": P227 cancel_disk_users
Generated "Paste": P228 backups_pro.txt
Generated "Paste": P229 undo_host.txt
Generated "Paste": P230 accelerate_database_accounts.java
Generated "Paste": P231 entomb_accounts.java
Generated "Paste": P232 legendary_legendary_shards_helper.java
Generated "Paste": P233 compact_backup_and_user_and_purge_memory
Generated "Paste": P234 account_script_script_backup_helper_helper.java
Generated "Paste": P235 purge_disk.php
Generated "Paste": P236 forgotten_elder_account.txt
Generated "Paste": P237 ancient_ancient_disks.txt
Generated "Paste": P238 disk_user.php
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8482

Differential Revision: https://secure.phabricator.com/D14883
2015-12-25 07:41:38 -08:00
epriestley
8025bc6432 Keep hovercards on screen a little harder
Summary: Ref T8980. This calculation was not quite right and you could sneak one off screen if you tried carefully.

Test Plan: Couldn't sneak one off screen anymore.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8980

Differential Revision: https://secure.phabricator.com/D14880
2015-12-24 12:38:36 -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
3ec07c4987 Show hovercards for most links in object property views
Summary:
Ref T8980. This isn't 100% coverage but should be pretty much all of the common ones.

These feel a touch iffy to me at first glance so I didn't go crazy trying to hunt all of them down. I have some other plans for them so maybe they'll feel better by the end of it.

Test Plan: Hovered over author, reviewers, blocked tasks, projects, etc.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8980

Differential Revision: https://secure.phabricator.com/D14877
2015-12-24 12:10:56 -08:00
epriestley
992dedcadd Modernize Differential SearchEngine just enough to get NUX
Summary: Ref T10032. This is sufficent to hit NUX without doing anything bad.

Test Plan:
  - Visited NUX.
  - Browsed normally.

{F1043191}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10032

Differential Revision: https://secure.phabricator.com/D14876
2015-12-24 10:47:52 -08:00
epriestley
19b2eb57a9 Improve lipsum generation of projects
Summary:
Fixes T9156.

  - Fix hashtag generation.
  - Fix various badnesses.
  - Improve project name generator.

Test Plan:
```
$ ./bin/lipsum generate projects
 GENERATORS  Selected generators: Projects.
 WARNING  This command generates synthetic test data, including user accounts. It is intended for use in development environments so you can test features more easily. There is no easy way to delete this data or undo the effects of this command. If you run it in a production environment, it will pollute your data with large amounts of meaningless garbage that you can not get rid of.

    Are you sure you want to generate piles of garbage? [y/N] y

 LIPSUM  Generating synthetic test objects forever. Use ^C to stop when satisfied.
Generated "Project": Self-Flying Data Center Swag Performance
Generated "Project": Optimize Cars
Generated "Project": Triaging Culture Optimization
Generated "Project": Automating Experience
Generated "Project": Accelerating NUX Performance
Generated "Project": Optimizing Culture Optimization
Generated "Project": Optimize Hardware
```

{F1042949}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9156

Differential Revision: https://secure.phabricator.com/D14874
2015-12-24 09:06:56 -08:00
epriestley
ba37149bf9 Improve bin/lipsum UX
Summary: Ref T9156. This makes the UX a little more modern/standard/safe.

Test Plan:
```
epriestley@orbital ~/dev/phabricator $ ./bin/lipsum generate
Choose which type or types of test data you want to generate, or select "all".

      - Differential Revisions
      - Files
      - Maniphest Tasks
      - Pastes
      - Pholio Mocks
      - Projects
      - User Accounts
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9156

Differential Revision: https://secure.phabricator.com/D14873
2015-12-24 09:06:35 -08:00
epriestley
1c572d1da5 Implement a "Project Members" object policy rule
Summary:
Fixes T9019. Pretty much ripped from D14467. I added the "policy hint" stuff so that you can create a project with this policy immediately.

I really dislike how the "hint" code works, but we //almost// never need to use it and the badness feels fairly well-contained.

Also pick up a quick feedback fix from D14863.

Test Plan:
  - Added test coverage, got it to pass.
  - Created a project with "Visible To: Project Members".

Reviewers: joshuaspence, chad

Reviewed By: chad

Maniphest Tasks: T9019

Differential Revision: https://secure.phabricator.com/D14869
2015-12-24 08:16:27 -08:00
epriestley
e2edb1577c Improve error messages for bad hashtags and project names
Summary: Ref T8509. We currently give you a fairly obtuse error when trying to name a project something like "!!". The error is correct, but not as helpful as it could be. Give users a more specific, more helpful error.

Test Plan: {F1042883}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8509

Differential Revision: https://secure.phabricator.com/D14872
2015-12-24 08:11:02 -08:00
epriestley
33f55a85b0 Fix project hashtag bugs: allow simultaneously changing name and adding same name as a tag
Summary:
Fixes T8509. Changes these behaviors:

  - If you create a project named "QQQ" and add "qqq" as a hashtag at the same time, it fails in an unhelpful way. (Now: succeeds.)
  - If you add "qqq" as a hashtag to a project with primary hashtag "qqq", it fails in a correct but probably unnecessary way (Now: just works).

We could make one or both of these behaviors show the user an error instead, but I think it's likely that this behavior is just what they always want.

Test Plan:
  - Added failing tests and made them pass.
  - Executed both scenarios described above from the web UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8509

Differential Revision: https://secure.phabricator.com/D14871
2015-12-24 08:10:37 -08:00
epriestley
92912a6072 Fix project hashtag bugs: duplicate tags, uppercase tags
Summary:
Ref T8509. This fixes three issues:

  - Adding a slug like `UPPERCASE` would not give you a normalized slug. (Now: normalizes as `uppercase`.)
  - Adding a slug like `UPPERCASE` would allow you to give two different projects the different tags `UPPERCASE` and `uppercase` (and `UpPeRcAsE`, etc). (Now: second tag is rejected as a duplicate.)
  - Adding multiple identical or similar slugs would produce a duplicate key exception. (Now: ignores the duplicates.)

Test Plan:
  - Added test coverage.
  - Made tests pass.
  - Hit these cases in the UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8509

Differential Revision: https://secure.phabricator.com/D14870
2015-12-24 08:10:03 -08:00
epriestley
dc5397b2db Trivial fixes from D14467
Summary: See D14467. Just teasing this apart so I can be a little more confident in my commandeering. These are the unambiguous cleanup changes from D14467.

Test Plan: inspection / clicked stuff / no impact

Reviewers: chad, joshuaspence

Reviewed By: joshuaspence

Differential Revision: https://secure.phabricator.com/D14868
2015-12-23 17:19:33 -08:00
Joshua Spence
8bacb3da23 Lock daemon configuration
Summary: I feel like the daemon configuration should be locked from editing from the web UI, given that much of it won't work unless the daemons are restarted anyway.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14867
2015-12-24 11:15:07 +11:00
epriestley
26ba4e8717 Materialize parent project memberships
Summary:
Ref T10010. Subprojects have the following general membership rule: if you are a member of a subproject ("Engineering > Backend"), you are also a member of the parent project.

It would be unreasonably difficult to implement this rule directly in SQL when querying `withMemberPHIDs()`, because we'd have to do an arbitrarily large number of arbitrarily deep joins, or fetch and then requery a lot of data.

Instead, introduce "materailized members", which are just a copy of all the effective members of a project. When a subproject has a membership change, we go recompute the effective membership of all the parent projects. Then we can just JOIN to satisfy `withMemberPHIDs()`.

Having this process avialable will also be useful in the future, when a project's membership might be defined by some external source.

Also make milestones mostly work like we'd expect them to with respect to membership and visibility.

Test Plan:
  - Added and executed unit tests.
  - Changed project members, verified materialized members populated correctly in the database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14863
2015-12-23 14:39:09 -08:00
epriestley
70f6bf306f Implement child/descendant query rules in Projects
Summary:
Ref T10010. This adds infrastructure for querying projects by type, depth, parent and ancestor.

I needed to revise the "extended policy check" cycle detection rules. When, e.g., querying a grandchild, they incorrectly detected a cycle because both the child and grandchild needed to check the policy of the grandparent.

Instead, simplify it to just do a basic runaway calldepth check. There are many other safety mechanisms to make it so this can't ever occur.

(Cycle detection does have existing test coverage, and those tests still pass, it just takes a little longer to detect the cycle internally.)

There is still no way to create subprojects in the UI.

Test Plan: Added and executed unit tests.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14862
2015-12-23 14:38:51 -08:00
epriestley
3068639ccf Implement query and policy rules for subprojects
Summary:
Ref T10010. This implements technical groundwork for subprojects. Specifically, it implements policy rules like Phriction:

  - to see a project, you must be able to see all of its parents (and the project itself).
  - you can edit a project if you can edit any of its parents (or the project itself).

To facilitiate this, we load all project ancestors when querying projects so we can do the view/edit checks.

This does NOT yet implement:

  - proper membership rules for these projects (up next);
  - any kind of UI to let users create subprojects.

Test Plan:
  - Added unit tests.
  - Executed unit tests.
  - Browsed Projects (no change in behavior is expected).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14861
2015-12-23 14:38:35 -08:00
epriestley
16d8e806a0 Simplify ProjectQuery handling of viewer membership
Summary:
Ref T10010. Currently, we do an unusual JOIN to make testing for viewer membership in projects a little cheaper.

This won't work as-is once we have subprojects, so standardize, simplify, and cover it with more tests for now. (I may be able to get a similar optimization later, but want a correct implementation first.)

Test Plan:
This change should create no behavioral differences.

  - Added tests.
  - Ran tests.
  - Viewed projects.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14859
2015-12-23 14:38:21 -08:00
epriestley
9a99c0fbde Always show "Change Priority" Maniphest stacked action, even for closed tasks
Summary: Ref T10004.

Test Plan: Changed priority of closed and open tasks.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10004

Differential Revision: https://secure.phabricator.com/D14865
2015-12-23 13:43:51 -08:00
Joshua Spence
15550b5582 Show node IDs in XHPAST tree view
Summary: Currently we do not show node IDs in this view, but do show token IDs in the stream view. Given that this view facilitates testing various XHPAST functionality, it would be useful to add this information.

Test Plan: Saw node IDs in XHPAST.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14697
2015-12-23 08:39:24 +11:00
epriestley
f5ff10fe28 Put inline previews in remarkup textareas
Summary:
Ref T3967. This gives us a reasonable baseline for doing remarkup previews inline in all contexts, and works in weird/constrained context including:

  - inline comments;
  - conpherence; and
  - custom fields.

It would be nicer to go beyond this in contexts like Phame posts, but this is a start, at least.

Test Plan:
{F1040877}

{F1040878}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T3967

Differential Revision: https://secure.phabricator.com/D14855
2015-12-22 12:18:28 -08:00
Chad Little
551732b962 Basic NUX states for Phriction
Summary: Adds a basic nux for `/` and not found documents. Ref T10023

Test Plan: Visit a clean install, see Welcome page. Visit a non built page, see not found UX

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10023

Differential Revision: https://secure.phabricator.com/D14854
2015-12-22 12:14:44 -08:00
epriestley
536d3a2185 Don't show self-subscribes in feed or mail
Summary: These transactions (when a user subscribes or unsubscribes only themselves) are universally uninteresting.

Test Plan:
  - Subscribed/unsubscribed, saw transactions but no feed/mail.
  - Commented, got implicitly subscribed, saw only comment in feed/mail, saw both transasctions on task.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14853
2015-12-22 10:45:51 -08:00
epriestley
61a92df66e Fix two issues with Phurl / Badges mail generation
Summary:
  - Phurl is missing a ReplyHandler / MailReceiver (all of this code should get cleaned up eventually, but I don't plan to get to it for a while).
  - Badges has a bad call.

This should clean up some bad daemon tasks.

Test Plan: Saw fewer daemon errors after these changes.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14852
2015-12-22 08:19:53 -08:00
epriestley
96fe8c0b83 Implement basic ngram search for Owners Package names
Summary:
Ref T9979. This uses ngrams (specifically, trigrams) to build a reasonably efficient index for substring matching. Specifically, for a package like "Example", with ID 123, we store rows like this:

```
< ex, 123>
<exa, 123>
<xam, 123>
<amp, 123>
<mpl, 123>
<ple, 123>
<le , 123>
```

When the user searches for `exam`, we join this table for packages with tokens `exa` and `xam`. MySQL can do this a lot more efficiently than it can process a `LIKE "%exam%"` query against a huge table.

When the user searches for a one-letter or two-letter string, we only search the beginnings of words. This is probably what they want, the only thing we can do quickly, and a reasonable/expected behavior for typeaheads.

Test Plan:
  - Ran storage upgrades and search indexer.
  - Searched for stuff with "name contains".
  - Used typehaead and got sensible results.
  - Searched for `aabbccddeeffgghhiijjkkllmmnnooppqqrrssttuuvvwwxxyyzz` and saw only 16 joins.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9979

Differential Revision: https://secure.phabricator.com/D14846
2015-12-22 08:00:33 -08:00
Chad Little
5c8025c41d Add some more consistant NUX to Phame
Summary: Adds a no visible blogs and no posts nux state using new UI. Ref T10032

Test Plan: Archived all my blogs, got no posts fallback. Test a New Blog, got create a post, logged out, saw no create button.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10032

Differential Revision: https://secure.phabricator.com/D14848
2015-12-22 07:12:17 -08:00
epriestley
426d648681 In Drydock, don't reset current branch to point at unrelated commit
Summary:
Fixes T10037. When we're building commit `aabbccdd`, we currently do this to check it out:

  git reset --hard aabbccdd

However, this has an undesirable side effect of moving the current branch pointer to point at `aabbccdd`. The current branch pointer may be some totally different branch which `aabbccdd` is not part of, so this is confusing and misleading.

Instead, use `git reset --hard HEAD` to get the primary effect we want (destroying staged changes) and then `git checkout aabbccdd` to checkout the commit in a detached HEAD state.

Test Plan:
  - Ran a build (a commit-focused operation) successfully.
  - Verified working copy was pointed at a detached HEAD afterward:

```
builder@sbuild001:/var/drydock/workingcopy-167/repo/git-test-ii$ git status
HEAD detached at ffc7635
nothing to commit, working directory clean
```

  - Ran a land (a branch-foused operation) successfully.
  - Verified working copy was pointed at a branch afterward:

```
builder@sbuild001:/core/data/drydock/workingcopy-168/repo/git-test$ git status
On branch master
Your branch is up-to-date with 'origin/master'.

nothing to commit, working directory clean
```

Reviewers: chad

Reviewed By: chad

Subscribers: yelirekim

Maniphest Tasks: T10037

Differential Revision: https://secure.phabricator.com/D14850
2015-12-22 06:47:47 -08:00
epriestley
57909a705c Improve strings for creating blocking subtasks
Summary:
Ref T6884. Ref T10004. For various reasons we previously didn't publish these transactions, but now do. This is probably a better behavior overall, but we didn't have reasonable strings for them.

Parent tasks now show "alice created blocking task Txxx.".

Feed now shows nothing, since "alice created task Txxx." is right next to any story we would show and showing them both seems silly.

Test Plan:
  - Created subtasks.
  - Viewed parent tasks.
  - Viewed feed.
  - Saw pretty reasonable strings/stories.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6884, T10004

Differential Revision: https://secure.phabricator.com/D14849
2015-12-21 17:35:09 -08:00
epriestley
a761f73384 Allow index extensions to skip indexing if the object has not changed
Summary:
Fixes T9890. This allows IndexExtensions to emit an object version.

Before we build indexes, we check if the indexed version is the same as the current version. If it is, we just don't call that extension.

T9890 has a case where this is useful: a script went crazy and posted thousands of comments to a single task.

Without versioning, that results in the same comments being indexed over and over again. With versioning, most of the queue could just exit without doing any work.

Test Plan:
  - Added a `sleep(1)` to the actual indexing, used `bin/search index --background` to queue up a lot of tasks, ran them with `bin/phd debug task`, saw them complete very quickly with only one actual index operation performed.
  - Used `bin/search index --trace` and `bin/search index --trace --background` to observe the behavior of queries against the index version store, which looked sensible.
  - Made comments/transactions, saw versions update.
  - Used `bin/remove destroy`, verified index versions were purged.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9890

Differential Revision: https://secure.phabricator.com/D14845
2015-12-21 17:27:14 -08:00
epriestley
23c42486e4 Rename "SearchEngine" to "FulltextStorageEngine"
Summary:
Ref T9979. I picked this name long before the advent of modern "Engine" architecture and it ended up being pretty confusing.

Rename "SearchEngine" (currently: mysql or elasticsearch, used to store and query fulltext indexes) to "FulltextStorageEngine" to make it more clear what it does and disambituate it from ApplicationSearch, which also has a bunch of stuff called "SearchEngine", "SearchEngineExtension", etc.

Test Plan: Grepped for `phabricatorsearchengine`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9979

Differential Revision: https://secure.phabricator.com/D14843
2015-12-21 17:26:19 -08:00
epriestley
99c9df96b4 Convert all "DocumentIndexers" into "FulltextEngines"
Summary: Ref T9979. This simplifies/standardizes the code a bit, but mostly gives us more consistent class names and structure.

Test Plan:
  - Used `bin/search index --type ...` to index documents of every indexable type.
  - Searched for documents by unique text, found them.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9979

Differential Revision: https://secure.phabricator.com/D14842
2015-12-21 17:25:23 -08:00
epriestley
99bd12b98d Lift Conpherence indexing up out of the Fulltext index
Summary:
Ref T9979. There are currently some hacks around Conpherence indexing: it does not really use the fulltext index, but its own specialized index. However, it's kind of hacked up so it can get reindexed by the normal indexing pipeline.

Lift it up into IndexEngine, instead of FulltextEngine. Specifically, the new stuff is going to look like this:

  - IndexEngine: Rebuild all indexes.
    - ConpherenceIndexExtension: Rebuild thread indexes.
    - ProjectMemberIndexExtension: Rebuild project membership views.
    - NgramIndexExtension: Rebuild ngram indexes.
    - FulltextIndexExtension / FulltextEngine: Rebuild fulltext indexes, a special type of index.
      - FulltextCommentExtension: Rebuild comment fulltext indexes.
      - FulltextProjectExtension: Rebuild project fulltext indexes.
      - etc.

Most of this is at least sort-of-in-place as of this diff, although some of the part in the middle is still pretty rough.

Test Plan:
  - Made a unique comment in a Conpherence thread.
  - Used `bin/search index --force` to rebuild the index.
  - Searched for the comment.
  - Found the thread.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9979

Differential Revision: https://secure.phabricator.com/D14841
2015-12-21 17:25:05 -08:00
epriestley
ecc3314a25 Modularize transaction/comment indexing in the FulltextEngine
Summary:
Ref T9979. This is currently hard-coded but can be done in a generic way.

This has one minor behavioral changes: answer text is no longer included in the question text index in Ponder. I'm not planning to accommodate that for now since I don't want to dig this hole any deeper than I already have. This behavior should be different anyway (e.g., index the answer, then show the question in the results or something).

Test Plan:
  - Put a unique word in a Maniphest comment.
  - Searched for the word.
  - Found the task.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9979

Differential Revision: https://secure.phabricator.com/D14837
2015-12-21 17:24:40 -08:00
epriestley
aab1574e33 Remove TYPE_SEARCH_DIDUPDATEINDEX event
Summary:
Ref T9979. This event had one weird callsite and no known third-party callers. It can be done more cleanly as an extension, now.

This index is used to allow us to "Group By: Project" in Maniphest without joining into the Projects database.

Test Plan:
  - Ran a query with "Group By: Project" in Maniphest.
  - Renamed project "Apples" to "Zebras".
  - Reloaded page.
  - UI properly moved "Zebras" tasks to the bottom of the list.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9979

Differential Revision: https://secure.phabricator.com/D14836
2015-12-21 17:23:59 -08:00
epriestley
02f82c2af5 Modularize fulltext indexing of Projects, Subscriptions and Custom Fields
Summary: Ref T9979. This is going to become `FulltextEngine`, but pave the way for that by pulling extensions out of it.

Test Plan:
{F1036624}

  - Used `bin/search index Txxx`, saw projects, subscribers and custom fields rebuild in the index.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9979

Differential Revision: https://secure.phabricator.com/D14835
2015-12-21 17:04:25 -08:00
epriestley
2447d9bdf2 Begin improving modularity of IndexEngine, add locks
Summary:
Ref T9890. Ref T9979. Several adjacent goals:

  - The `SearchEngine` vs `ApplicationSearchEngine` thing is really confusing. There are also a bunch of confusing class names and class relationships within the fulltext indexing. I want to rename these classes to be more standard (`IndexEngine`, `IndexEngineExtension`, etc). Rename `SearchIndexer` to `IndexEngine`. A future change will rename `SearchEngine`.
  - Add the index locks described in T9890.
  - Structure things a little more normally so future diffs can do the "EngineExtension" thing more cleanly.

Test Plan:
Indexing:

  - Renamed a task to have a unique word in the title.
  - Ran `bin/search index Txxx`.
  - Searched for unique word.
  - Found task.

Locking:

  - Added a `sleep(10)` after the `lock()` call.
  - Ran `bin/search index Txxx` in two windows.
  - Saw first one lock, sleep 10 seconds, index.
  - Saw second one give up temporarily after failing to grab the lock.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9890, T9979

Differential Revision: https://secure.phabricator.com/D14834
2015-12-21 17:04:10 -08:00
epriestley
4bba3fd4c1 Fully modularize DestructionEngine
Summary: Ref T9979. Convert all DestructionEngine behaviors to extensions.

Test Plan:
{F1033244}

Destroyed an object, verifying:

  - Herald transcripts were destroyed;
  - edges were destroyed;
  - flags were destroyed;
  - tokens were destroyed;
  - transactions were destroyed;
  - worker tasks were cancelled.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9979

Differential Revision: https://secure.phabricator.com/D14832
2015-12-21 17:03:44 -08:00
epriestley
674388ce6a Prepare DestructionEngine to be modularized
Summary:
Ref T9979. The general shape of "engine" code feels pretty good, and I plan to move indexing to be more in line with other modern engines, with the ultimate goal of supporting subprojects (T10010) and several intermediate goals.

Before moving indexing, clean up Destruction, since some of the new indexes will need destruction hooks and destruction currently has a lot of `instanceof` stuff that should be easy to fix by applying more modern approaches.

Test Plan:
  - Used `bin/remove destroy` to destory an Almanac device.
  - Verified that properties for the device were destroyed.
  - Viewed module panel in UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9979

Differential Revision: https://secure.phabricator.com/D14831
2015-12-21 17:03:32 -08:00
Chad Little
5fecd55d6e More NUX states
Summary: Ref T10032, adds "Basic" NUX to more applications.

Test Plan: Visit each with ?nux=true and click on the create link. T10032 is tracking which apps need general modernization to pick up these changes.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10032

Differential Revision: https://secure.phabricator.com/D14847
2015-12-21 13:13:44 -08:00
Chad Little
675be8efc5 Add more NUX states
Summary: Adds basic NUX to Dashboards, Herald, Repositories, Maniphest. Note Herald and Dashboard Panels don't fine the nux for some reason, assume they will when modernized?

Test Plan: Read text, click buttons.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14844
2015-12-21 11:15:54 -08:00
Chad Little
bde9ac43e7 Add various NUX states
Summary: Adds basic NUX UI to Countdown, Paste, Phurl, Ponder, Slowvote, Macro, and Pholio.

Test Plan: Review each with ?nux=true. Click on Create Button.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14840
2015-12-21 09:55:54 -08:00
Chad Little
c6f3a03209 Basic NUX blank states
Summary: Implement in Badges

Test Plan:
Test with nux=true.

{F1033431}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: johnny-bit, Korvin

Differential Revision: https://secure.phabricator.com/D14833
2015-12-21 07:39:46 -08:00
epriestley
8a81d208c4 Don't show "master (branched from master)" in UI
Summary:
Ref T3462. If someone works directly on `master`, we currently show "Branch: master (branched from master)" in the UI.

Although this is sort of technically accurate, it is confusing.

Instead, just show "Branch: master" in this situation.

Test Plan: Saw "master" instead of "master (branched from master)".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T3462

Differential Revision: https://secure.phabricator.com/D14829
2015-12-19 12:57:59 -08:00
epriestley
dbb84f1ddc Add basic NUX support to SearchEngines
Summary:
This is just putting a hook in that pretty much works. Behavior:

  - If you visit `/maniphest/?nux=true`, it always shows NUX for testing.
  - Otherwise, it shows NUX if there are no objects in the application yet.

Test Plan: {F1031846}

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14828
2015-12-19 12:57:13 -08:00
Nick Zheng
8eec9e2c0e Provide a more straightforward way to revoke SSH keys by finding and destroying the objects
Summary: Ref T9967

Test Plan:
Ran migrations.
Verified database populated properly with PHIDs (SELECT * FROM auth_sshkey;).
Ran auth.querypublickeys conduit method to see phids show up
Ran bin/remove destroy <phid>.
Viewed the test key was gone.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T9967

Differential Revision: https://secure.phabricator.com/D14823
2015-12-19 11:52:26 -08:00