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

11888 commits

Author SHA1 Message Date
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
55373030df Fix a bad copy/paste in Conduit documentation
Summary: This is linking to the wrong article.

Test Plan: O.O

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14885
2015-12-26 07:29:24 -08:00
epriestley
854e27d1c9 End of limited time only
This feature was only available for a limited time! Looks like you missed out.

This reverts commit 53fac2f89a.
2015-12-26 02:37:42 -08:00
epriestley
53fac2f89a Limited Time Only!
Summary: I'll revert this within 24 hours, before I promote.

Test Plan: Had completely functional but greatly enhanced Phabricator experience in Firefox, Safari and Chrome.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14884
2015-12-25 08:16:12 -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
e0ad791247 Fix hovercard behavior for multiple copies of the same object
Summary: Ref T8980. Previously, if you had like `T123 T123 T123` and waved your mouse over them, we wouldn't move the card. Now, move the card.

Test Plan: Waved mouse. Saw the card move.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8980

Differential Revision: https://secure.phabricator.com/D14881
2015-12-24 13:24:00 -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
e88dbbe1b1 Keep hovercards mostly on screen
Summary: Ref T8980.

Test Plan: {F1043268}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8980

Differential Revision: https://secure.phabricator.com/D14879
2015-12-24 12:18:41 -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
37f1f55557 Fix a possible deadlock in unit tests after an error
Summary:
After certain types of errors, we may deadlock when trying to destroy test databases.

Specifically, we still have connections open to, say, `phabricator_unittest_abasonaknlbaklnasb_herald` (or whatever) and MySQL sometimes (not sure exactly when?) waits for them before destorying the database.

Test Plan:
  - Added `$m = null; $m->method()` to a fixture test to force a fatal.
  - Saw consistent deadlock, with `storage destroy` never exiting.
  - Added `--trace` to the `storage destroy` command and made it use `phutil_passthru()` so I could see what was happening.
  - Saw it hang on some arbitrary database.
  - Conneced to MySQL, used `show full processlist;` to see what was wrong.
  - Saw the `DROP DATABASE ...` command waiting for locks to release on the database, and other connections still open.
  - Applied patch.
  - Saw consistent success.
  - Used `storage destroy --unittest-fixtures` to clean up extra databases.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14875
2015-12-24 09:11:47 -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
c4df80b39e Remove various unused beahviors from PHUIWorkboardView
Summary: Ref T5240. We don't currently use any of these options and I don't think we have any plans to use them. Strip them out for now to make fixing drag-and-drop stuff easier.

Test Plan: Grepped for removed stuff, no hits.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5240

Differential Revision: https://secure.phabricator.com/D14864
2015-12-23 14:01:14 -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
epriestley
f422c9b955 Allow users to swipe away stacked actions on mobile
Summary:
Ref T10004. This might be a little too funky or magical, but is probably not necessary too often and easier than doing new layout with the CSS.

Also make the desktop icons a little meatier.

Test Plan: Swiped things away on mobile. Clicked things away on desktop.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10004

Differential Revision: https://secure.phabricator.com/D14857
2015-12-22 19:17:11 -08:00
epriestley
bb8cf8eca8 Fix incorrect rendering of "Spaces" dropdown control when configuring EditEngine defaults
Summary:
Ref T10004. This could sometimes pass `false`, which counts as disabled.

Instead, pass `null` explicitly.

Test Plan: Edited default space on an EditEngine form.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10004

Differential Revision: https://secure.phabricator.com/D14858
2015-12-22 18:39:20 -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
8752bd4966 Disable live previews on mobile
Summary:
Fixes T1895. Now that we have on-demand prviews, we can use them on mobile. On mobile:

  - don't show live previews;
  - only save drafts every 10 seconds.

Also, show fewer remarkup buttons on mobile to try to make sure the more important ones (preview, e.g.) fit.

Test Plan:
  - Made window narrower and wider to trigger preview/no-preview behavior.
  - Used DarkConsole to verify request rate.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T1895

Differential Revision: https://secure.phabricator.com/D14856
2015-12-22 12:52:51 -08: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
af8cc2a5fc Don't select {F...} text after adding it to textareas
Summary:
Fixes T10039. We add text to textareas in two cases:

  - Users clicking assitance buttons in Remarkup text areas.
  - Drag-and-drop file uploads.

In case (1), it makes sense to highlight the text (it shows the user what we inserted, and lets them undo the action easily if it isn't what they wanted).

In case (2), it does not. Users almost never want to delete or edit a file reference. It is slightly nice to have the reference as a visual callout, but I don't think this is a big deal.

Change the behavior so that we highlight only for remarkup buttons, not for drag-and-drop files.

Test Plan:
  - Clicked an "isnert quote" button on remarkup assist area, got highlighted example text.
  - Dragged and dropped a file, got text inserted with no highlight.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10039

Differential Revision: https://secure.phabricator.com/D14851
2015-12-22 08:21:06 -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