Build separate comments for each inline comment in Audit

Summary:
Ref T4896. Moves us closer to migrating comments to transactions by building a transaction per inline.

This makes the UI a little wonky, and it will get slightly worse until we swap to the new UI and grouping/collapsing starts working. It's still usable, there's just a box per inline.

Test Plan:
  - Added a comment.
  - Added an inline comment.
  - Added a comment and an inline comment.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10023
Write separate comments for every action in Audit

Summary:
Ref T4896. Depends on D10023. Prepares the code for the final migration.

The transaction table stores one row per distinct effect (e.g., add CCs) rather than one row per user action (e.g., "add CCs + comment"). We can double-read that table as long as the code doesn't expect transactions/comments to have multiple different effects, and doesn't try to write any such rows.

Everywhere that we were writing a big "X + Y" comment, write two separate "X" and "Y" comments instead. Like D10023, this disrupts the UI a little (you get more boxes), but that will be resolved once the rendering code swaps over. Otherwise, this retains the existing behavior.

Test Plan:
- Used `diffusion.createcomment` to add comments, raise concern, and accept.
- Previewed commenting, adding auditors/ccs, accepting, raising concern.
- Actually performed commenting, adding auditors/ccs, accepting, raising concern.
- Added a user with mentions.
- Added an explicit CC and a mention user.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10052
Migrate audit comments to transactions

Summary:
Ref T4896. Depends on D10052. This is the major/scary migration, but not really so bad. It is substantially similar to D8210, but less complex because there are fewer actions here.

This moves `PhabricatorAuditComment` storage to `PhabricatorAuditTransaction`, then reads `PhabricatorAuditComment`s as a proxy around the new objects.

Test Plan:
  - Before migrating, browsed around. Nothing appeared broken.
  - Migrated cleanly.
  - Viewed old transactions (inlines, comments, accept/reject/etc, add auditors, add ccs, implicit CCs).
  - Added all of those comment types.
  - Edited a draft.
  - Deleted a draft.
  - Spot checked the database for sanity.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10055
Convert Audit comment rendering to standard infrastructure

Summary: Ref T4896. Depends on D10055. This uses core rendering stuff for audit comments, and fixes all the wonkiness with inlines so we can actually land the migration.

Test Plan: Viewed, previewed and edited various types of comments in Diffusion.

Reviewers: chad, btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10056
Use ApplicationTransactions when indexing commit/audit comments

Summary:
Ref T4896. Depends on D10056. Moves search indexing to standard infrastructure.

Also, fixes a bug where inline comments would not be indexed.

Test Plan: Used `bin/search index ... --trace` to view index construction of a commit, saw all the comments and inlines get indexed.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10057
Render Phame templates in the expected order

Summary: Currently, content runs before header. Instead, run header, content, footer. This makes it easier to use `header.php` for includes, for Phacility.

Test Plan: Made Phacility blog work better, see next diff.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10067
Fix Legalpad Preamble

Summary: Fixes T5740, scopes sections

Test Plan: Reload Legalpad with Preamble, Diviner, and Phriction

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5740

Differential Revision: https://secure.phabricator.com/D10077
Fix audit commenting where there are only inlines

Summary: been some changes here and this code was broked. turns out we re-assign $action like two lines later and never used the initial value, so we can simply delete the offending line. Fixes T5745.

Test Plan: submitted inline comment pre-patch and fatal. re-submitted post patch and great success!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5745

Differential Revision: https://secure.phabricator.com/D10078
Update a panel location in DarkConsole documentation

Summary: This moved.

Test Plan: Reading.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10079
Spell "dependendency" less incorrecter

Summary: Diction-

Test Plan: -ary!

Reviewers: btrahan, chad

Reviewed By: btrahan, chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10080
Workboards - fix adding new tasks and sorting

Summary: At least on my install, sorting was pretty borked from a type issue. (e.g. "unbreak now" of 100 sorting as less than "High" of 90). Fix this with some parseInt action. Also support adding new cards with the new colsort stuff. The clever bit here is to include the task ID in the sorting vector because the task ID wins ties at the moment I think / new tasks need to show up before older tasks when they are initially created. Fixes T5716.

Test Plan: added many "normal" priority cards and saw them fly in correctly. changed priority and moved correctly. made no edits and no moves were made correctly.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5716

Differential Revision: https://secure.phabricator.com/D10081
Make the 404 page device friendly.

Summary: Fixes T5686.

Test Plan: Viewed in iOS simulator and Chrome with various viewport sizes - looked good!

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5686

Differential Revision: https://secure.phabricator.com/D10082
Allow searching diffs by more status options

Summary:
This Fixes T5737. Apparently the functionality to search by different
statuses in differential was already there, but the options weren't
exposed in the frontend. I can't think of any reason why this should've
been the case, so I just added the other options.

Test Plan: Tested against some local diffs to match new query option.

Reviewers: joshuaspence, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5737

Differential Revision: https://secure.phabricator.com/D10076
Macros - fix audio disabling not actually disabling audio

Summary: Fixes T5653.

Test Plan: made a macro with sound and it played. changed it to not play and it did not play. changed it back and it played once more.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5653

Differential Revision: https://secure.phabricator.com/D10085
update celerity map

Summary: forgot to do this today; its like i'm rusty or something. :/

Test Plan: na

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10086
Prevent crash caused by empty array when rendering Pholio mocks

Summary: This was causing a crash according to a conversation with @rfergu in IRC.  The issue is that if `$images_to_show` is empty, then `$image` never gets set, and it attempts to call `getID` on a null / unset variable.

Test Plan: @rfergu confirmed this fixed the crash.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, rfergu, epriestley

Differential Revision: https://secure.phabricator.com/D10073
Fix email for closing a blocking task

Summary: Fixes T5751. Currently, we incorrectly overwrite `$xactions`.

Test Plan: Closed a blocking task, got an email about the correct transaction set ("closed task; added comment") instead of an overwritten transaction set ("closed blocking task").

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5751

Differential Revision: https://secure.phabricator.com/D10088
Implement build simulation; convert Harbormaster to be purely dependency based

Summary:
Depends on D9806.  This implements the build simulator, which is used to calculate the order of build steps in the plan editor.  This includes a migration script to convert existing plans from sequential based to dependency based, and then drops the sequence column.

Because build plans are now dependency based, the grippable and re-order behaviour has been removed.

Test Plan: Tested the migration, saw the dependencies appear correctly.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9847
Maybe fix excluding to / cc folks on differential emails

Summary: Ref T5185. By code inspection, I am pretty sure before this patch it was doing a set of a get on itself which does nothing. Now, being careful not to break Facebook we get the proper exclusion phids. I am pretty sure the folks in T5185 are experiencing this in Differential only.

Test Plan: Get some folks on T5185 to play with this

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5185

Differential Revision: https://secure.phabricator.com/D10087
Make artifacts imply dependencies on build steps

Summary: This makes input artifacts imply the appropriate build step dependencies in the build plan.  That is, if you use a host artifact in a build step, it will then implicitly depend on the 'Lease Host' step.

Test Plan: Viewed the build plan with the artifacts, saw the dependencies.  Ran a build, saw everything execute in the correct order.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10089