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

9116 commits

Author SHA1 Message Date
epriestley
501630d931 Make adding projects a standard Herald effect
Summary: Ref T7849. Lift project actions into the base class. Some day they'll be fully modular.

Test Plan:
  - Wrote an "add projects" Herald rule.
  - Edited Maniphest tasks.
  - Got projects added.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7849

Differential Revision: https://secure.phabricator.com/D12503
2015-04-22 14:01:21 -07:00
epriestley
4e47e3a116 Merge redundant Herald effect code
Summary: Ref T7849. Lift more action handling out of adapters. In theory, adapters will some day do no action handling. That day is not today, but it is now a step closer.

Test Plan:
  - Wrote a rule using the email and flag actions.
  - Ran that rule.
  - Got an email and flag.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7849

Differential Revision: https://secure.phabricator.com/D12502
2015-04-22 14:00:44 -07:00
Bob Trahan
2e8649c292 Conpherence - sort participant list in a case insensitive way
Summary: Fixes T7857. Some sorting gymnastics - make a nice list for natcasesort and then array_select_key to sort - but nothing too crazy.

Test Plan: made a room with user "btrahan" and other users "BTRAHA", "BTRAHANCAPS", and "xerxes". chat room correctly ordered "btrahan" (since logged in user always at top), "BTRAHA", "BTRAHANCAPS", "xerxes". Viewed room as "xerxes" and saw ordering "xerxes", "BTRAHA", "btrahan", "BTRAHANCAPS".

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7857

Differential Revision: https://secure.phabricator.com/D12507
2015-04-22 12:19:24 -07:00
Chad Little
306ab29259 Add documentation for customizing fonts
Summary: Fixes T7888, adds clarification of property and an example.

Test Plan: Read the text, copy/paste example.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7888

Differential Revision: https://secure.phabricator.com/D12508
2015-04-22 12:13:11 -07:00
Bob Trahan
688c506ecf Quicksand - update help appropriately as you navigate about
Summary: Fixes T7724. Things are a bit tricky here as sometimes we don't want to display the help icon at all...! Change things such that we always render at least the icon, though it may have the style set to hidden if appropriate. Instrument quicksand javascript to then load the proper help dropdown if it can. Do this generally so other aphlict dropdowns could work pretty easily.

Test Plan: started on home and noted no help. clicked maniphest and saw maniphest help. clicked home and saw no help again. clicked differential and saw differential help.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7724

Differential Revision: https://secure.phabricator.com/D12499
2015-04-22 12:05:27 -07:00
epriestley
e7702acdc6 Don't escape quotation marks when printing the monospaced CSS rule
Summary:
Fixes T7888. This is currently safe, but double quotes are incorrectly escaped.

To keep them unescaped, we have to punch through PhutilSafeHTML a bit. Since the allowable characters are strictly filtered this is still safe in practice, just not as theoretically-safe.

Test Plan: Set font to `32px "impact"` (with quotes), saw impact font.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7888

Differential Revision: https://secure.phabricator.com/D12506
2015-04-22 09:28:35 -07:00
epriestley
4dea152215 Fix a possible undefined variable
Summary: Fixes T7881.

Test Plan: Inspection.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T7881

Differential Revision: https://secure.phabricator.com/D12500
2015-04-21 19:28:43 -07:00
Bob Trahan
dd9ec255ec Quicksand - make notification and message counts update as you navigate around
Summary: Ref T7573. Unify code to fetch these counts and do some light formatting since we're going to need to do the same thing for some conpherence-specific ajax in the durable column (See T7708).

Test Plan: loaded up two tabs, one with a durable column on and one without. in the without browser, i read some messages, decrementing my unread count. when i navigated again in the durable column browser, the count updated correctly. with no notifications, commented on a task with another user to get a notification and it showed up properly. visited the task by clicking not the notification and the bubble count decremented correctly

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7573

Differential Revision: https://secure.phabricator.com/D12498
2015-04-21 15:46:36 -07:00
epriestley
c509e80a74 Fix an issue where versus diffs tried to load invalid changeset IDs
Auditors: btrahan
2015-04-21 15:38:52 -07:00
epriestley
1858d2aa66 Make Differential timeline aware of ghostly inlines
Summary:
Ref T7447. Ref T7870.

When a ghostly inline appears on the page, the timeline isn't currently aware that it's present, so it links elsewhere.

Instead, apply adjustments before rendering the timeline.

Ref T5030. This makes the behaviors in T5030 irrelevant most of the time.

Test Plan:
Before:

{F378106}

After:

  - Inlines visible on page are linked directly.
  - Inlines which we can't port (e.g., on files not present on current page) are still linked off-page.
  - Used "Show Older" to page through and verify consistent rendering.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: yelirekim, epriestley

Maniphest Tasks: T5030, T7870, T7447

Differential Revision: https://secure.phabricator.com/D12497
2015-04-21 15:32:23 -07:00
epriestley
d8bd3efa2c Simplify timeline rendering on Differential revisions
Summary: Ref T7447. Remove the `loadInlineComments()` by-ref side-effect mess and make this load more direct.

Test Plan:
  - Viewed a revision.
  - Clicked "Show Older".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7447

Differential Revision: https://secure.phabricator.com/D12496
2015-04-21 15:31:43 -07:00
epriestley
a4a65bd5f1 Let ghostly inlines automatically expand changesets in enormous diffs
Summary:
Ref T7447. Ref T7870. When a diff affects more than 100 files, we collapse files by default, then expand files with inlines.

Ghostly inlines currently don't cause files to expand, but reasonably should.

Test Plan:
Before:

{F378065}

After:

{F378066}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: yelirekim, epriestley

Maniphest Tasks: T7447, T7870

Differential Revision: https://secure.phabricator.com/D12495
2015-04-21 15:31:18 -07:00
epriestley
8c39885076 Track ghostly inlines across file moves
Summary:
Ref T7447. Ref T7870.

  - Forward: When rendering some file "B" which was moved from "A", port ghosts for both "A" and "B" to it.
  - Backward: When porting a comment on "X" which was moved from some "Y", allow it to port to a file named "Y" if it can't find a file named "X".

Test Plan:
Before:

{F377809}

After:

{F377806}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7447, T7870

Differential Revision: https://secure.phabricator.com/D12493
2015-04-21 11:06:44 -07:00
epriestley
c596c358db Make "Reply" on ghost inlines work correctly (or, at least, consistently)
Summary:
Ref T7447. Ref T7870. When you "reply" to a ghost inline, make it work properly.

This exact behavior is arguable. In particular, when you reply to a ghost inline, we //could// put the reply on the same diff as the original.

I suspect it aligns better with user exepectation to put the new inline on the current (visible) diff instead, and generally for inlines to flow forward through time and all of the ghosts to pretty much be older than all of the non-ghosts in most cases. We can see how it feels and adjust things if this turns out to not make sense.

Test Plan:
  - Replied to ghost inlines, got new inlines on the proper display line.
  - Replied to normal inlines, got normal behavior.
  - Made some new inlines.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7870, T7447

Differential Revision: https://secure.phabricator.com/D12492
2015-04-21 11:06:44 -07:00
epriestley
84f62bdf9a Move ported inlines which appear on nonvisible lines backward so they show up
Summary:
Ref T7447. Ref T7870. See T7870 for a detailed description of this issue.

NOTE: Replying to these inlines from the UI still does the wrong thing, because we use the database line number, not the UI line number.

Test Plan:
Before:

{F377732}

After:

{F377733}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7447, T7870

Differential Revision: https://secure.phabricator.com/D12491
2015-04-21 11:06:44 -07:00
epriestley
fe774e68e2 Distinguish between ported-forward and ported-backward comments
Summary:
Ref T7447. This might be overkill, but I want to over-explain things until we have more confidence that this is rarely confusing.

NOTE: I'm playing it a bit fast and loose with `setIsGhost()` (passing a dictionary) because making API changes requires changing the interface and Diffusion, which is a pain. I'll clean this up at the end once the interface is more final. This is well-contained for now.

Test Plan:
  - Viewed "base vs 2" in a diff with 3 diffs, saw some "older comments" and some "newer comments".
  - Hovered the tags for an explanation of comment spookiness.

{F377703}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7447

Differential Revision: https://secure.phabricator.com/D12490
2015-04-21 11:06:44 -07:00
epriestley
aa04e97de7 Prefer left-side for old comments in new-vs-new diff of diffs
Summary: Ref T7447. Ref T7870.

Test Plan:
Before:

{F377691}

After:

{F377692}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7870, T7447

Differential Revision: https://secure.phabricator.com/D12489
2015-04-21 11:06:44 -07:00
epriestley
b74bb0019e Filter nonvisible inlines out of changeset inline result set
Summary:
Ref T7447. After compiling inlines which will appear on the changeset, remove inlines which

Later stages remove these anyway, so it doesn't change anything to keep them around, but we can filter them out here cheaply.

This will also let us drive the Differential timeline view with the same logic a few diffs from now, to improve how it renders inlines. Generalize things a little bit.

Test Plan:
  - Made a comment on the left of diff 1.
  - Made diff 2.
  - Viewed diff 2 vs diff 1.
  - Verified old-left comment was filtered out by the new loop.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7447

Differential Revision: https://secure.phabricator.com/D12488
2015-04-21 11:06:44 -07:00
epriestley
b2d280ff51 Port comments through time and space in the common/best case
Summary:
Ref T7447. This ports comments forward and backward in the best case:

  - The old comment is on a changeset with the same filename.
  - The old and new files are pretty much the same, line-for-line.

This will fail to port a lot of comments around and probably port a lot of comments into goofy places. We can see how bad it is in practice.

Errata:

  - Design is me cobbling something together, probably worth tweaking.
  - "Old Comment" should, at a minimum, say "Newer Comment" sometimes, or we should come up with some better name for this stuff.

Test Plan: {F377214}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: johnny-bit, yelirekim, epriestley

Maniphest Tasks: T7447

Differential Revision: https://secure.phabricator.com/D12484
2015-04-21 11:06:44 -07:00
epriestley
5645a07d99 Modernize DifferentialInlineCommentQuery
Summary:
Ref T7447. This class is currently a big mess with a lot of `withWeirdSpecialThingUsedInOnePlace()` type qualifiers.

Try to generalize/normalize it a bit.

Test Plan:
- Viewed inline comments.
- Created a new inline comment.
- Edited an inline comment.
- Marked an inline comment complete.
- Deleted, then undeleted an inline comment.
- Previewed inline comments.
- Viewed drafts as another user, verified they don't show up.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: yelirekim, epriestley

Maniphest Tasks: T7447

Differential Revision: https://secure.phabricator.com/D12483
2015-04-21 11:06:44 -07:00
Bob Trahan
2fab72d43b Quicksand - update title while navigating about
Summary: Fixes T7744. Also fixes a bug where we were copying the response object erroneously; that's not necessary to move around since we cleanly initialize it for each load

Test Plan: from user profile, clicked feed tab and saw new title. clicked calendar tab and saw new title. clicked back and saw feed title and page render.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7744

Differential Revision: https://secure.phabricator.com/D12487
2015-04-21 11:01:05 -07:00
Bob Trahan
d8ab5f594c Quicksand - make page object notifications work properly with quicksand
Summary: Fixes T7680. Make it so the listen behavior can be initialized multiple times from the server by having the behavior only update a few static data variables on subsequent initializations.

Test Plan:
visited TX with user A and left a comment with user B and got the "reload" and "TX updated" bubbles.
Reloaded and navigated to /maniphest/ with user A and had user B leave another comment on TX - no "reload" bubble and correct "TX updated" bubble.
Navigated to TX again with user A and had user B leave a comment and got the "reload" and "TX updated" bubbles.
visited TX with user A and left a comment with user B and got the "reload" and "TX updated" bubbles. navigated away with user A and the "reload" bubble was automagically closed.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7680

Differential Revision: https://secure.phabricator.com/D12448
2015-04-20 16:43:32 -07:00
Bob Trahan
4d5badeec0 Quicksand - provide some plumbing for fancier updates
Summary: Ref D12448. Ref T7573. This changes quicksand up a bit so rather than caching just rendered HTML we also cache the initial response from the server. We also fire off a quicksand-redraw event which will let things like the page objects for notifications update correctly while using Quicksand (see D12448).

Test Plan: loaded up /p/btrahan/  Clicked the UI elements to navigate to various profile views up to maniphest. clicked back until back at /p/btrahan/ and it worked. clicked forward until all the way back to maniphest and it worked. clicked back 2x, then clicked new links, then back and it worked

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7573

Differential Revision: https://secure.phabricator.com/D12449
2015-04-20 15:44:14 -07:00
epriestley
bae0a85dbc Add contextual typeahead function documentation
Summary: Ref T4100. This integration into the "Browse" dialog is probably a little more heavy-handed than we should shoot for.

Test Plan:
{F377131}

{F377132}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12482
2015-04-20 13:43:07 -07:00
epriestley
6fa9b10914 Collapse "Show unassigned tasks" in Maniphest into a parameterized owners typeahead
Summary: Ref T4100. Support viewer(), members(), and add a new none().

Test Plan:
  - Used all new functions.
  - Batch edited tasks with unassign action.
  - Saved a query from master, upgrade it to this patch, checkbox migrated cleanly into a "no one" token.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12470
2015-04-20 10:07:30 -07:00
epriestley
e5e5974d9f Give typeahead browse dialogs sensible titles
Summary: Ref T4100. Let datasources specify a more meaningful title than the class name.

Test Plan: Browsed some sources.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: chad, epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12469
2015-04-20 10:06:23 -07:00
epriestley
b85f8b91de Implement readProjectsFromRequest() helper for SearchEngines
Summary:
Ref T4100. This just makes the "specify stuff in query parameters" workflow a little better:

  - You can now do `?projects=differential,diffusion`.
  - You can now do `?projects=projects(alincoln)`.

Test Plan: Did that stuff ^^^^

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12468
2015-04-20 10:06:22 -07:00
epriestley
4005bff567 Implement function-driven logical project queries in Differential
Summary:
Ref T4100. Ref T5595. This implements these fields in one mega-field:

  - Projects
  - Not in projects
  - In any project
  - Include results in no projects
  - In users' projects

Hopefully, this is a step in the right direction.

Test Plan: {F375555}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, chad, epriestley

Maniphest Tasks: T4100, T5595

Differential Revision: https://secure.phabricator.com/D12463
2015-04-20 10:06:21 -07:00
epriestley
f331463d50 Make "user's projects" a logical datasource and add viewerprojects()
Summary:
Ref T4100. Ref T5595. This converts `projects(user)` into a logical datasource.

Also adds a viewerprojects() logical datasource.

Test Plan: {F375546}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100, T5595

Differential Revision: https://secure.phabricator.com/D12462
2015-04-20 10:06:20 -07:00
epriestley
c2fc065bfb Implement a project logic datasource
Summary:
Ref T4100. Ref T5595. This allows you to execute all "Projects", "Any Project", "Not in projects" and "include no projects" operations in one field.

It doesn't actually implement such a field yet.

Test Plan: {F375516}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5595, T4100

Differential Revision: https://secure.phabricator.com/D12460
2015-04-20 10:06:18 -07:00
epriestley
eb69b115a7 Simplify members() datasource function
Summary: Ref T4100. Implement members() in terms of the real projects datasource.

Test Plan: Used members() functions in Differential, browsed members() functions.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12459
2015-04-20 10:06:17 -07:00
epriestley
c246aa2638 Implement a projects() datasource function
Summary:
Ref T4100. This is like members(), but is implemented on top of the raw datasource. This is a lot simpler and involves way less code duplication.

I'll go back and implement members() like this, too.

Nothing actually uses this yet.

Test Plan:
  - Used browse view to browse datasource.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12458
2015-04-20 10:06:16 -07:00
epriestley
2bbe3b0cbf Move token rendering into Datasources
Summary:
Ref T4100. I want to reduce the amount of code duplication that function datasources currently need to wrap some parameter datasource.

For example, `ProjectMembersDatasource` should really just be a little bit of logic on top of `ProjectsDatasource`, which should do most of the heavy lifting.

Moving rendering into datasources brings us a step closer to being able to do this.

Test Plan:
  - Rendered normal, function, and invalid tokens.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12457
2015-04-20 10:06:14 -07:00
epriestley
a11dab59b0 Require TokenizerControl to always have a datasource
Summary:
Ref T4100. I can simplify the logic a bit here by moving some rendering into the datasources, but a few TokenizerControls currently don't have datasources.

Require datasources and always provide datasources.

Test Plan:
  - Used previously-datasourceless controls (e.g., "Add Reviewers").
  - Used normal controls.
  - Manually verified that no other controls are missing datasources.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12456
2015-04-20 10:06:13 -07:00
epriestley
e27c0b416d Add "Edge Logic" support to PolicyAwareQuery
Summary:
Ref T4100. Ref T5595. This allows PolicyAwareQuery to write all the logic for AND, OR, NOT, and NULL (i.e., "not in any projects") queries against any edge type.

It accepts an edge type and a list of constraints (which are basically just operator-value pairs, like `<NOT, PHID-X-Y>`, meaning the results must not have an edge connecting them to `PHID-X-Y`).

This doesn't actually do anything yet; see future diffs.

Test Plan: `arc unit --everything`

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100, T5595

Differential Revision: https://secure.phabricator.com/D12455
2015-04-20 10:06:12 -07:00
epriestley
55e49d7e31 Provide more buildXClause() and buildXClauseParts() on PolicyAwareQuery
Summary:
Ref T4100. Ref T5595. These functions are trivial for now, but move us toward being able to define more default query behavior by default.

Future changes will give these methods meaningful, nontrivial behaviors.

Test Plan: `arc unit --everything`

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5595, T4100

Differential Revision: https://secure.phabricator.com/D12454
2015-04-20 10:06:10 -07:00
epriestley
f5580c7a08 Make buildWhereClause() a method of AphrontCursorPagedPolicyAwareQuery
Summary:
Ref T4100. Ref T5595.

To support a unified "Projects:" query across all applications, a future diff is going to add a set of "Edge Logic" capabilities to `PolicyAwareQuery` which write the required SELECT, JOIN, WHERE, HAVING and GROUP clauses for you.

With the addition of "Edge Logic", we'll have three systems which may need to build components of query claues: ordering/paging, customfields/applicationsearch, and edge logic.

For most clauses, queries don't currently call into the parent explicitly to get default components. I want to move more query construction logic up the class tree so it can be shared.

For most methods, this isn't a problem, but many subclasses define a `buildWhereClause()`. Make all such definitions protected and consistent.

This causes no behavioral changes.

Test Plan: Ran `arc unit --everything`, which does a pretty through job of verifying this statically.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: yelirekim, hach-que, epriestley

Maniphest Tasks: T4100, T5595

Differential Revision: https://secure.phabricator.com/D12453
2015-04-20 10:06:09 -07:00
epriestley
01c99335fd Give Phortune carts a proper isInvoice flag
Summary:
See some earlier discussion in D11593:

> One thing I'm vaguely thinking about is the possibilty that users may be able to invoice one another directly, eventually. For example, we might invoice a contracting client.
> We might need an `isInvoice` flag eventually, but `subscriptionPHID` is a reasonable stand-in for now.

This adds such a flag.

Test Plan:
  - Generated an ad-hoc invoice and verified it showed up in the right place.
  - Used `bin/phortune invoice` to invoice a subscription and verified it worked correctly.
  - Paid an invoice and saw it leave "pending invoices" status.

{F377029}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12480
2015-04-20 10:05:44 -07:00
epriestley
0195e751c6 Support basic ad-hoc invoices in Phortune
Summary:
This allows a merchant to send a user an invoice for something arbitrary, like services rendered.

Two major missing parts:

  - These don't actually get marked as invoices. I'll fix that in the next diff, but it's not entirely trivial because `subscriptionPHID` is currently overloaded to also mean "is invoice".
  - We don't send email automatically. I don't plan to fix that for now, since all our invoicing needs are covered by personal email.

Test Plan:
Merchants have a new "new invoice" option:

{F376999}

This leads to selecting a user and account, and then you can generate the invoice (only one actual "purchase" / line item for the moment). You can add a longer-form remarkup description to contextualize the billable items:

{F377001}

This sends the invoice and takes you to the merchant order overview screen:

{F377002}

For now, you copy/paste that link into a nice personal enterprisey business-to-business email; the recipient sees this:

{F377003}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12478
2015-04-20 10:05:22 -07:00
epriestley
e0c95bca86 Add a "Batch Edit Tasks..." action to workboard columns
Summary:
Ref T5523. Adds a new workflow to make some kinds of bulk workboard operations easier.

New dropdown action:

{F376848}

This brings you into the existing bulk edit flow:

{F376849}

When you save an edit, you're taken back to the board:

{F376850}

If you try to edit a column with nothing in it, you get an error:

{F376851}

Note that the selected workboard filter is applied before choosing tasks, so if your filter is set to "open tasks" we only batch edit the open (i.e., currently visible) tasks in the column. I think this is more powerful (it lets you use filtering to select task subsets) but might not be completely obvious in all cases (although I do think it's more obvious than the alternative rule -- just an issue of neither rule being completely obvious).

Test Plan:
  - Batch edited tasks in a column.
  - Used "Batch Edit Tasks..." to move tasks to a different workboard by removing + adding a project.
  - Batch edited a column with filtered-out tasks, verified only visible tasks were edited.
  - Batch edited a column with no visible tasks, received error.
  - Used the batch editor normally (Maniphest -> Maniphest, no boards).

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: johnny-bit, cburroughs, epriestley

Projects: #prioritized

Maniphest Tasks: T5523

Differential Revision: https://secure.phabricator.com/D12475
2015-04-20 10:05:08 -07:00
epriestley
8f61eb45ab Give tokenizer tokens CSS color classes on the container instead of the icon
Summary:
Ref T4100. See D12465.

  - Instead of putting CSS color classes on the tokenizer icons, put them on the container tags.
  - Note that this removes the "bluegrey" default classes.
  - This doesn't actually add CSS for the classes so, e.g., "green" doesn't make things green yet. This just supports D12465.

Test Plan: Viewed markup, saw classes.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12467
2015-04-19 07:17:54 -07:00
Chad Little
2106a553e4 Move setup issue callout to under the header
Summary: Fixes T7830. Moves the callout to sit under the main header, which resolved Differential issues and Durable Column issues, but still some extra CSS shenanigans.

Test Plan:
Test lots of pages, Home, Dashboards, Conpherence, Durable Column, Inline Diff Replies, Mobile, Workboards, etc.

{F375821}

{F375822}

{F375823}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7830

Differential Revision: https://secure.phabricator.com/D12466
2015-04-19 07:15:59 -07:00
epriestley
e5b97cd17c Fix typeahead browse dialog paging
Summary: This accidentally got disabled; we need to pull one more result that we'll use to tell if there's a next page.

Test Plan: Saw "more results" again.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12461
2015-04-18 11:09:44 -07:00
Chad Little
ca388d95fd Make the browse button on the tokenizer, be on the tokenizer
Summary: Moves the Browse... button into a Search Icon on the actual tokenizer. I played with a number of icon treatments, and Search seems to convey the right attribute, other things like lists and menus didn't quite feel right to me, but feel free to push back if you hate search.

Test Plan:
Tested lots of tokens, little tokens, small screens, etc.

{F375467}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12452
2015-04-18 10:58:50 -07:00
Chad Little
50e2d5174f Remove Send Message Button on Profile w/ no Conpherence
Summary: Fixes T7847

Test Plan: Uninstall Conpherence

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7847

Differential Revision: https://secure.phabricator.com/D12450
2015-04-17 13:25:05 -07:00
epriestley
52c6a9707f Force typeahead datasource response into expected array format
Ref T4100. Without this, we sometimes get a sparse array from composite sources and the client isn't sure what to do with it.
2015-04-17 11:55:22 -07:00
epriestley
e558a2e729 Always set a typeahead result limit, even for non-browse queries
Ref T4100. The limit on People queries was inadvertently removed.
2015-04-17 11:18:58 -07:00
epriestley
ea97d75a67 Make Differential use viewer's token instead of viewer() token again
Summary: Ref T4100. This restores the simpler behavior. See discussion in T4100#107445

Test Plan: Used Differential search, saw my token.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12447
2015-04-17 11:06:58 -07:00
epriestley
76448a75de Make token UI stronger and more consistent
Summary:
Ref T4100. Overall:

  - Use token background color to communicate token type (blue = object, yellow = function, grey = disabled/closed, red = invalid).
  - Use token icon color to make color choices consistent (specifically, use project icon colors in project tokens).
  - For functions, use token icon to communicate function result type (e.g., viewer() has a user icon; members(...) has a group icon), since we don't need the icon to indicate "this is a function" anymore.

Test Plan:
{F374615}
{F374616}
{F374617}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12446
2015-04-17 11:06:58 -07:00
epriestley
845466b49b Implement viewer() and members(project) typeahead functions
Summary:
Ref T4100. This is still a bit rough around the edges, but mostly does what we're after.

  - Implements viewer() and members(...) functions.
  - The new browse workflow makes these discoverable.

Test Plan: {F374201}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: chad, epriestley

Maniphest Tasks: T4100

Differential Revision: https://secure.phabricator.com/D12444
2015-04-17 11:06:58 -07:00
epriestley
a4261f41c2 Make browse action available for dynamic/JS-driven tokenizers
Summary: Ref T5750. This makes browse work for all of the dynamic tokenizers in Herald, Policies, batch editor, etc.

Test Plan: Used tokenizers in Herald, Policies, Batch editor.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12442
2015-04-17 11:06:58 -07:00
epriestley
a641601407 Implement a rough browse view for tokenizers
Summary: Ref T5750. This adds a basic browse view. Design is a bit rough, see T7841 for some screenshots.

Test Plan: Used browse view to add tokens to tokenizers.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12441
2015-04-17 11:06:58 -07:00
epriestley
ba48e05964 Uncheat Macro datasource browse support
Summary: Ref T5750. Macro browse support is actually useful (in the meme dialog) and fairly straightforward to implement.

Test Plan:
  - Browsed macros.
  - Queired macros.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12439
2015-04-17 11:06:58 -07:00
epriestley
7db362a4b6 Cheat my way through the rest of the typeahead datasources
Summary:
Ref T5750. These are a pain to modernize and most don't matter, so cheat:

  - Mark a bunch nonbrowsable, including some that probably should be browsable but which I don't want to deal with for now.
  - For static datasources, add an easy server-side filter (this isn't really cheating, and is appropriate for the status/priority/application datasources).
  - Make composite sources browsable if their components are browsable.

Test Plan:

  - Tried to browse an unbrowsable source, got a 404.
  - Browsed a composite source.
  - Browsed static sources (priority/status/applications).
  - Browsed normal sources (people/projects).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12438
2015-04-17 11:06:58 -07:00
epriestley
5eb496bb3e Make Harbormaster build plan datasource more browsable
Summary: Ref T5750.

Test Plan:
  - Browsed plans in Harbormaster.
  - Browsed plans in typeahead browser.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12435
2015-04-17 11:06:58 -07:00
epriestley
4fc5f8e297 Improve browsability of Diffusion repository datasource
Summary: Ref T5750. Make browsing work and improve filtering.

Test Plan:
  - Used browse UI.
  - Used normal search UI.

{F373752}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12434
2015-04-17 11:06:57 -07:00
epriestley
9437414f17 Improve browsability of Almanac service datasource query
Summary: Ref T5750. Update the Almanac service query to be browsable.

Test Plan:
  - Browsed and reordered Diffusion.
  - Browsed and reordered services in Almanac.

{F373735}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12433
2015-04-17 11:06:57 -07:00
epriestley
c8dc11d81a Improve browsability of Projects query
Summary:
Ref T5750. Same deal as D12427, but for projects.

(As before, mostly making the empty/browse view work instead of return nothing.)

Test Plan: {F372353}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12428
2015-04-17 11:06:57 -07:00
epriestley
fd60a739c1 Improve browsability of People query
Summary: Ref T5750. Fix this query so it works better with the browse view. In particular, make it return results before the user types a query.

Test Plan:
  - Used a people tokenizer.
  - Used global search tokenizer to search for people.
  - Ran new version of query in production to make sure it wasn't crazy.
  - Browsed through results in browse view.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12427
2015-04-17 11:06:57 -07:00
epriestley
f688181eaf Add a typehaead filter to tokenizer browse views
Summary: Ref T5750. This adds a search filter which filters results (kind of: a lot of datasources don't do a great job with this right now -- but the correct data is sent to the server).

Test Plan: {F372313}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12426
2015-04-17 11:06:57 -07:00
epriestley
06d79703d6 Make tokenizer browse view paging work correctly
Summary: Ref T5750. Adds a working "more results". Hard limits at 1000 results to mitigate the amount of trouble offset paging can get us into.

Test Plan:
Artificially set hard limit down; clicked through results.

{F372284}

{F372285}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12425
2015-04-17 11:06:57 -07:00
epriestley
6a4de406b8 Make browse view show tokens instead of raw data
Summary: Ref T5750. Move the server-side token rendering code to a shared location and use it in the browse view.

Test Plan: {F372252}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12424
2015-04-17 11:06:57 -07:00
epriestley
64182dc94d Implement an extremely primitive typeahead browse view with offset paging
Summary:
Ref T5750. Give up and use offset paging.

Each page will become progressively slower, but the amount of time I was sinking into T7803 without making real headway was not justified by fixing this. In comparison, this took 10 minutes. It isn't really useful to browse through tens of thousands of results anyway, and no one is realistically going to try to do that (later in this sequence, I'll explicitly prevent it).

Test Plan:
This is completely primitive right now:

{F372224}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

Differential Revision: https://secure.phabricator.com/D12423
2015-04-17 11:06:57 -07:00
Bob Trahan
a81072046a Conpherence - fix policy bug on comments with files in rooms
Summary: Fixes T7840. Add some unit tests. One explicitly covers the case in T7840. Other cases are "base" cases for rooms and threads, plus the version if T7840 for a thread, where a policy exception should be thrown.

Test Plan: `arc unit` and was able to successfully add a file to a conpherence room i did not have edit permissions on

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7840

Differential Revision: https://secure.phabricator.com/D12443
2015-04-16 16:15:36 -07:00
Povilas Balzaravicius Pawka
3a17220ce3 T7827: Boost Elasticsearch results on title match.
Summary: If match is found on document title, boost result.

Test Plan:
- Setup phabricator with elasticsearch.
- Index documents with `bin/search index --all`
- Use search and expect query matching titles appear on the top of results list.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: WikiChad, Korvin, epriestley

Maniphest Tasks: T7827

Differential Revision: https://secure.phabricator.com/D12432
2015-04-16 07:43:40 -07:00
Chad Little
9796515b70 Use white for active state on mobile menus
Summary: Perviously we used {$sky} but we've since change the icons to .8 opacity on white, allowing use of white on select.

Test Plan: Click on the mobile menu, see icon turn white instead of blue.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12430
2015-04-15 16:04:44 -07:00
Chad Little
9537970a42 Move ToC in Phriction to it's own column
Summary: Adds additional CSS to make the Table of Contents into it's own column (if one exists).

Test Plan:
Tested a page with and without a table of contents. Tested tablet, mobile, and desktop breakpoints. Tested Conpherence, scrolling seemed fine still on trackpad and mouse.

{F370973}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12417
2015-04-14 16:02:30 -07:00
epriestley
48b6120ee2 Fix nonstandard specification of a constant
Summary: See <648fa2e1bc (commitcomment-10724671)>.

Test Plan: peered deep into its soul

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12415
2015-04-14 14:44:21 -07:00
Bob Trahan
a91a3cd22f Conpherence - refactor CSS a bit
Summary: Follow on to D12410. This kills the general CSS file that once powered application transaction in favor of a specific CSS file for the shared transaction styles for both full view and durable column view. I was able to delete some stuff and identify some shared stuff but there is probably more to go here. Also, rename "phabricator-x" to "conpherence-x" for class names.

Test Plan: viewed full conpherence and durable column conpherence and things looked nice

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12412
2015-04-14 12:33:02 -07:00
Bob Trahan
87d360bb1b Conpherence - refactor display classes a bit
Summary:
D12409 made me realize this was a bit janky. `PhabricatorTransactionView` was only being used by Conpherence, so move and rename that class to `ConpherenceTransactionView`. Also, rename the existing `ConpherenceTransactionView` to `ConpherenceTransactionRenderer`, moving the actual view bits into the new `ConpherenceTransactionView`. Resulting code is a bit cleaner IMO.

Diff 1 of 2 (second diff has to be written. =D). Diff 2 will take care of the CSS and possibly clean things up further.

Test Plan: played around in conpherence full and conpherence column and things looked nice

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12410
2015-04-14 12:25:35 -07:00
Bob Trahan
d90496a719 Conpherence - make timestamps in durable column link to full message in conpherence
Summary: Ref T7757. This diff made me realize that `PhabricatorTransactionView` is only used in Conpherence, as well as that `ConpherenceTransactionView` is more like a rendering class with a few static methods. Going to take a diff after this to clean all that up.

Test Plan: opened up durable column, clicked on timestamp, voila!

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7757

Differential Revision: https://secure.phabricator.com/D12409
2015-04-14 11:28:33 -07:00
Chad Little
e66d6448b6 Move base drop shadow into CelerityResourceTransformer
Summary: Simplifies some common CSS rules.

Test Plan: Test some dialogs and menus.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12406
2015-04-14 09:48:59 -07:00
Chad Little
2697dd53b6 Use standard icon size, color on mobile headers
Summary: This just matches the UI of icons on mobile with the icons size, color on desktop.

Test Plan:
test tablet and phone breakpoints.

{F369688}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12407
2015-04-14 09:45:58 -07:00
Bob Trahan
295308de5b Conpherence - turn on mentions interface for Conpherence rooms / messages
Summary: Fixes T7756. This is the last little stray bit, though finishing T7757 also helps this feature IMO.

Test Plan: said "ZXX is the best" in comment on DXX and saw proper mention transaction on ZXX

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7756

Differential Revision: https://secure.phabricator.com/D12405
2015-04-13 18:12:48 -07:00
Chad Little
ca5909cac6 Add Mailtags to Paste
Summary: Fixes T7786. Adds very basic mailtag support.

Test Plan: Tested changing language, title to a Paste, didn't get notifications on my test account.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7786

Differential Revision: https://secure.phabricator.com/D12404
2015-04-13 15:30:59 -07:00
Bob Trahan
0acdf256fc Conpherence - get #anchor links to messages working
Summary:
Ref T7757. Oddities include:

 - not working in column view, since the generic anchor technology conflicts once you navigate to a page with a transaction timeline view
 - not working if you are linking to a message not included in initial load

Remaining work is addressing these oddities.

 - make column view timestamp link to full conpherence correctly?
 - make back end load from hyperlinked transaction forward? or do it more like application transactions and have the client keep requesting stuff until it gets it?

Open to suggestions! :D

Test Plan: played around in conpherence full and stuff looked okay. noted no changes as intended in column view.

Reviewers: epriestley, chad

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7757

Differential Revision: https://secure.phabricator.com/D12402
2015-04-13 15:24:48 -07:00
epriestley
ff7215f3d6 Fix a fatal on the application list
Summary:
Fixes T7816. This just punts adding proper cursor-based paging to Applications for now, since they don't have a handy order.

If we get to 101, we can either fix this properly or change the SearchEngine to return 200 results.

(Previously, we generated a cursor only if we absolutely needed to, so this code wasn't called. We generate cursors in some cases where we may not need them now, but the code is simpler this way.)

Test Plan: `/applications/` no longer fatals.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7816

Differential Revision: https://secure.phabricator.com/D12403
2015-04-13 14:51:00 -07:00
epriestley
281bdf87cf Fix a fatal for 404s in Phriction
Summary:
Fixes T7817. Not all documents have `$content`, so don't try to set a timestamp if there's no content.

An example is `/w/asdlkfnalskdnfaslk/`, i.e. the 404 document.

Test Plan: Hit `/w/junk/` and got a nice 404 instead of a fatal.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7817

Differential Revision: https://secure.phabricator.com/D12401
2015-04-13 14:44:26 -07:00
Bob Trahan
dd22db18ff Conpherence - make ZXXX monograms a bit more useful.
Summary: Ref T7756. Now viewing individual threads in Conpherence is `ZXXX` driven. Also adds remarkup support.

Test Plan: clicked around on list of conpherences in full view and it worked. selected 'view in conpherence' action from column and loaded correct `ZXXX` uri. Typed `ZXXX` in Maniphest and saw it link to Conpherence room.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7756

Differential Revision: https://secure.phabricator.com/D12397
2015-04-13 12:45:55 -07:00
Bob Trahan
f12839ffd4 Conpherenece - improve performance by not fetching data as often from D12347
Summary: Turns out the pertinent views don't use this data anyway. Additonally, change the remaining pathway to fetch 15% of the rows it fetched in D12347. Fixes T7815.

Test Plan: clicked around and things worked. my instance always feels snappy though

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7815

Differential Revision: https://secure.phabricator.com/D12399
2015-04-13 12:43:40 -07:00
epriestley
d096b9c151 Fix Conpherence transaction ordering
Summary: Ref T7815. Pretty sure this is right? Seems OK locally. Also: simpler!

Test Plan:
  - Conpherence is right-side-up.
  - Transactions on other objects are also still right-side-up.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7815

Differential Revision: https://secure.phabricator.com/D12398
2015-04-13 12:34:37 -07:00
epriestley
8efdc4aabf Replace getPagingValue() with cursor methods
Summary:
Ref T7803. Prior to this change sequence, Query classes conflated paging values (the actual thing that goes in a "x > 3" clause) with cursor values (arbitrary identifiers which track where the user is in a result list).

Although the two can sometimes be the same, the vast majority of implementations are simpler and better when object IDs are used as cursors and paging values are derived from them.

The new stuff handles this in a consistent way, so we're free to separate getPagingValue() from paging. The new method is essentially getResultCursor().

This also implements getPageCursors(), which allows queries to return directional cursors. The inability to do this was a practical limitation blocking the implementation of T7803.

Test Plan:
  - Browsed a bunch of results and paged through queries.
  - Grepped for removed methods.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12383
2015-04-13 11:58:38 -07:00
epriestley
09ad69238e Drive conduit result ordering through Query order specifications
Summary:
Ref T7803. Ref T5873. Allows Query methods to expose orderings from the underlying Query class nearly-for-free.

Callers can specify a string to use a builtin ordering, or an array to use a low-level column ordering.

Test Plan: {F368236}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5873, T7803

Differential Revision: https://secure.phabricator.com/D12381
2015-04-13 11:58:37 -07:00
epriestley
156b156e77 Give Conduit params/return/errors protected visibility
Summary:
Ref T7803. Ref T5873. I want to drive Conduit through more shared infrastructure, but can't currently add parameters automatically.

Put a `getX()` around the `defineX()` methods so the parent can provide default behaviors.

Also like 60% of methods don't define any special error types; don't require them to implement this method. I want to move away from this in general.

Test Plan:
  - Ran `arc unit --everything`.
  - Called `conduit.query`.
  - Browsed Conduit UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: hach-que, epriestley

Maniphest Tasks: T5873, T7803

Differential Revision: https://secure.phabricator.com/D12380
2015-04-13 11:58:35 -07:00
epriestley
6e4f508beb Provide "builtin" high-level result orders
Summary:
Ref T7803. Currently, available high-level orders are spread across Query and SearchEngine classes and implemented separately for each application.

Lift the concept of "builtin" (high-level, user-facing, named) orders (similar to "builtin" queries in ApplicationSearch) into the root Query class, and let it drive the SearchEngine implementation. This allows you to define a new order in one place and have it automatically work across the entire stack.

This will also let Conduit expose this information in a straightforward way.

Test Plan:
  - Used ApplicationSearch in Diffusion.
  - Used all result orderings.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12379
2015-04-13 11:58:34 -07:00
epriestley
2794c69db5 Remove getPagingColumn() / getReversePaging()
Summary: Ref T7803. Remove these in favor of more generalized paging and ordering.

Test Plan: Sorted and paged results in various applications.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12378
2015-04-13 11:58:32 -07:00
epriestley
bdd1edea7a Modernize ManiphestTask paging and ordering
Summary:
Ref T7803. The ApplicationSearch integration is still a little rough here, but it seems to have the correct behavior.

The rest of this is now at least relatively sane, cohesive, and properly behaved.

Test Plan:
  - Used all grouping and ordering queries in Maniphest. Pagingated results.
  - Used custom field ordering in Maniphest. Paginated results.
  - Paginated through the `null` section of "Assigned" and "Projects" group-by queries. Pagingation now works correctly (it does not work at HEAD).
  - Ran unit tests covering priority changes.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12372
2015-04-13 11:58:31 -07:00
epriestley
4114560844 Modernize more paging/order queries
Summary:
Ref T7803. Removes some getReversePaging().

This also fixes `null` column handling, by adding an explicit `'null'` key with possible values "head" (put NULL before other values) or "tail" (put NULL after other values).

Maniphest has some glitchiness in paging through NULLs right now, but I believe it's all pre-existing and will be resolved when it fully converts. Diffusion is fully converted and pages through NULL correctly.

Test Plan:
  - Failed to identify any reason for ChangesetQuery to reverse paging.
  - Paged thorugh Diffusion.
  - Paged through Maniphest.
    - Maniphest has some issues when paging inside a NULL section, but these issues are preexisting and will be resolved later in this change sequence.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12371
2015-04-13 11:58:30 -07:00
epriestley
9c7c13ffc8 Modernize Phrequent and Commit query ordering/paging
Summary: Ref T7803. Fixes T3870. Move these away from pagingColumn / reversePaging.

Test Plan:
  - Tested/paged audit query.
  - Poked at Phrequent. Didn't seem any more broken than before.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3870, T7803

Differential Revision: https://secure.phabricator.com/D12363
2015-04-13 11:58:29 -07:00
epriestley
51dabc5007 Modernize Differential paging/ordering
Summary: Ref T7803. Move Differential off getPagingColumn() / getReversePaging().

Test Plan: Paged Differential results.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12362
2015-04-13 11:58:28 -07:00
epriestley
9b5198f463 Remove ORDER_PATH_MODIFIED from Differential
Summary:
Ref T7803. This is a performance hack, not a real order, and isn't really meaningful or pageable.

After D12158, we constraint his query on `dateModified` anyway, which should generally give the database a relatively small result set to examine.

Test Plan: Browsed Differential and Diffusion. Checked query plan, it didn't look too crazy.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12361
2015-04-13 11:58:26 -07:00
epriestley
e0fa0fbdee Modernize Phriction ordering/paging
Summary: Ref T7803. Fixes T7809. Move Phriction away from getReversePaging() / getPagingColumn().

Test Plan: Paged "All Documents", "Updated", and viewed document hierarchy.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: ite-klass, epriestley

Maniphest Tasks: T7809, T7803

Differential Revision: https://secure.phabricator.com/D12360
2015-04-13 11:58:25 -07:00
epriestley
8bd1ab9d13 Modernize Feed and Phlux ordering/paging
Summary: Ref T7803. Move these off getReversePaging() / getPagingColumn().

Test Plan: Paged through Phlux and Feed.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12359
2015-04-13 11:58:22 -07:00
epriestley
a4a198342e Modernize ReleephProjectQuery ordering/paging
Summary: Ref T7803. Continue removing implementations of getPagingColumn() and getReversePaging().

Test Plan: Browsed and paged through Releeph projects, Maniphest tasks, Diffusion repositories.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12358
2015-04-13 11:58:21 -07:00
epriestley
d496f4d28c Modernize ProjectQuery paging/ordering
Summary: Ref T7803. Move ProjectQuery off getReversePaging() / getPagingColumn() and onto order vectors.

Test Plan: Set project page size to 3 and paged back and forth.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12357
2015-04-13 11:58:20 -07:00
epriestley
4fba6e7730 Remove trivial implementations of getPagingColumn()
Summary:
Ref T7803. Some Query subclasses implement getPagingColumn() in a trivial way, usually to provide a table alias.

Formalize the concept of a primary table alias, and remove obsoleted getPagingColumn() implementations.

Test Plan: Issued affected queries.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12356
2015-04-13 11:58:19 -07:00
epriestley
a40c40fade Drive query ordering and paging more cohesively
Summary:
Ref T7803. Ordering and paging are inherently intertwined, but they often aren't driven by the same data right now.

Start driving them through the same data:

  - `getOrderableColumns()` defines orderable and pageable columns.
  - `getPagingValueMap()` reads values from a cursor.

This is generally sufficient to implement both paging and ordering.

Also, add some more sanity checks to try to curtail the number of ambiguous/invalid orderings applications produce, since these cause subtle/messy bugs.

Test Plan:
  - Paged through pastes and a few other object types.
  - Intentionally changed defaults to be invalid and hit some of the errors.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12355
2015-04-13 11:58:18 -07:00
epriestley
e6174ed45c Fix an issue where pastes could be reordered as a side effect of cache fills
Summary: Ref T7803. Pastes which needed a cache fill would incorrectly be dropped to the bottom of the list. Stop doing that.

Test Plan: Loaded a list of pastes with some that needed cache fills, saw them appear in the correct order.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12354
2015-04-13 11:58:16 -07:00
epriestley
604d1409f1 Make buildPagingClauseFromMultipleColumns() safer
Summary: Ref T7803. Reduce the amount of code we're trusting to build SQL queries.

Test Plan:
  - Paged through results in Maniphest, Differential and Diffusion.
  - Some of the NULLable groups in Maniphest are a bit funky but this was preexisting.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12353
2015-04-13 11:58:15 -07:00
epriestley
a43473c4b6 Begin formalizing query orders
Summary:
Ref T7803. Queries currently have a single `getPagingColumn()`, which is oversimplified and insufficient to describe many ordering operations. Frequently, orders must span multiple columns.

Move toward an "order vector", which is a list of orderable values like "name, id". These map directly to columns, and are sufficient to actually describe orders. The more modern Query classes (Maniphest, Repository) essentially do this manually anyway.

Test Plan:
  - Added and executed unit tests.
  - Browsed around, verified the correct ORDER BY clauses were generated.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12352
2015-04-13 11:58:14 -07:00
epriestley
9dc114d115 Make formatOrderClause() safer
Summary:
Ref T7803. Instead of trusting subqueries to provide safe values, escape them explicitly.

(We'll probably have a few cases somewhere where this doesn't work, but can make them the exception rather than the rule.)

Test Plan: Issued all "order" queries in Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12351
2015-04-13 11:58:13 -07:00
Bob Trahan
78bfcc3221 Conpherence - change "A, B, C..." subtitle to "A: what most recent person said" when we can
Summary:
For the price of loading transactions more consistently, we get a better subtitle. We do this in all cases EXCEPT for when we're grabbing handles, because that makes the handles pretty heavy weight and I could even feel the perf hit on my development machine and we don't use subtitle there anyway. We may want to cache the latest message on the conpherence thread object to improve performance here as well as consider falling back to "A, B, C..." more often. Code is written such that no transactions means an automagical fallback.

Fixes T7795. (Technically, there's still a note about handle code conversion work on T7795 but we'll get that generally later.)

Test Plan:
played around with conpherence in both views and things seemed to work nicely.
made sure to try the original repro in T7795 and couldn't get that to go either
posted a long comment and verified that the CSS / string truncation both make it display nicely. Note that without the CSS the chosen glyph value can be too high to fit nicely at times.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7795

Differential Revision: https://secure.phabricator.com/D12347
2015-04-13 11:31:34 -07:00
epriestley
b467e58358 Fix two minor issues
Summary:
Ref T7811. Fixes two minor issues I observed in the cluster:

  - Sometimes APC doesn't give us key names. Not sure exactly what's up here, but we can do a better job with this.
  - The `%` in `25%` actually needs more escaping, since it's interpreted by both `pht()` (immediately) and `console_format()` (later).

Test Plan:
  - First one is just from an error log, not sure how to repro offhand.
  - Ran `bin/phd help start` for the second one.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7814, T7811

Differential Revision: https://secure.phabricator.com/D12395
2015-04-13 11:11:18 -07:00
Chad Little
d56012ce26 Move Conpherence when warning is present
Summary: Turns out only Conpherence/Durable Column still have issues. Workboards is OK. Simple 2 new CSS classes and punt the issue down the road.

Test Plan: Test Workboards, Conpherence, Durable Column with a setup check issue.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12394
2015-04-13 10:30:13 -07:00
Chad Little
4ab9ebd901 Add setEpoch for PHUIHeaderView, use in all Documents
Summary: Sets a consistent last update time in the header of PHUIDocuments, Legalpad, Diviner, Phriction. I'm not set on the exact language, just that there is consistency, feel free to suggest changes.

Test Plan:
Test Legalpad, Diviner, Phriction.

{F368270}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12384
2015-04-12 18:08:09 -07:00
Chad Little
a912e3bf83 Make ActionList on PHUIDocumentView always a dropdown menu
Summary: Gives back 160px of document space, makes Phriction easier to read. Moves ActionList into menu

Test Plan: Review Phriction Actions Menu, Edit Document, etc. Test mobile, tablet, desktop breakpoints.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12375
2015-04-12 13:57:17 -07:00
Bob Trahan
9a49c81393 Audit - fix a bug with unloaded repositories
Summary:
...because its always at least the string <ATTACHABLE>...  Not sure when we'd hit this / see the TODO about making it better, but its definitely a logic bug right now.

(an update to D12347 helped me notice that this conditional is always hit and may fatal later)

Test Plan: NA

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12348
2015-04-11 21:50:30 -07:00
epriestley
3c6c6552d3 Strip more Mailbox signatures
Summary: Saw this variant in a thread.

Test Plan: Unit tests.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12349
2015-04-11 08:52:33 -07:00
epriestley
15b41f5639 Remove Herald rule edit log
Summary:
Fixes T7601. Ref T7803, weakly (this removes a Query subclass with ad-hoc paging). Herald has a very old edit log which predates transactions and is essentially useless and not really policy-aware. I think it's doing more harm than good; remove it.

Herald rules have proper transactions, but rule edits don't currently render something nice into the transaction log. This is definitely the way forward, but we haven't seen requests for this so don't bother building it for now.

I did put a nice end-cap on the transaction log, though.

Test Plan:
  - Viewed Herald UI.
  - Grepped for removed classes and methods.
  - Edited a rule.
  - Viewed rule transaction log.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: cburroughs, chad, epriestley

Maniphest Tasks: T7601, T7803

Differential Revision: https://secure.phabricator.com/D12346
2015-04-11 08:50:50 -07:00
Sébastien Santoro
bf68ab2562 Fixed typo in PhabricatorPasteTransaction
Summary: update -> updated

Test Plan: read again the sentence

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12350
2015-04-11 08:39:39 -07:00
Bob Trahan
238381a623 Conpherence - Fix missing method from D12336; bad branch management. =/
Summary: Ref T7795.

Test Plan: updating a conpherence works once more

Reviewers: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7795

Differential Revision: https://secure.phabricator.com/D12345
2015-04-10 10:18:48 -07:00
epriestley
3c254469c9 Document just-in-time methods for handles
Summary: Fixes T7689. I'm not going to go clean up all the rest of the `loadViewerHandles()` calls right now since a lot of them are kind of a pain and they aren't really hurting anything so it doesn't feel very leveraged, but at least deprecate it and document the new hotness.

Test Plan:
have a look
in a book
reading rainbow

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12343
2015-04-10 10:17:53 -07:00
Bob Trahan
26f7b69ab2 Conpherence - fix a fatal
Summary:
Ref T7795.

I can't get this to reproduce and its confusing to me how its possible. The trace in T7795 uses the "LOAD" pathway on the update controller. Under the hood, this issues a ThreadQuery with needTransactions to true. With needTransactions to true, the transactions and pertinent handles are all loaded nicely.

So... best guess is there has been some LIMIT of transactions since the offending person participated...? Alternative fix which would probably work is to specify needParticipantCache to true.

More on T7795 - the user report found the "a, b, c..." subtitle thing in the messages dropdown confusing. Yet another fix here would be to change that to be something like "a: snippet of what a said...". I'll discuss that on the task.

Test Plan: iiam

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: nevogd, Korvin, epriestley

Maniphest Tasks: T7795

Differential Revision: https://secure.phabricator.com/D12336
2015-04-10 09:08:38 -07:00
epriestley
d44f05eead Clean up some more handle callsites
Summary: Ref T7689.

Test Plan:
- Viewed Calendar browse and detail views.
- Went through Differential update workflow.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12341
2015-04-09 16:15:13 -07:00
epriestley
b717298c9e Remove some callsites to loadViewerHandles()
Summary: Ref T7689. Use the more modern handle load mechanisms in Almanac.

Test Plan:
- Viewed Almanac binding detail page.
- Viewed Almanac device interface list.
  - Grepped for other callsites to InterfaceTableView.
- Viewed Almanac binding table.
  - Grepped for other callsites to BindingTableView.
- Viewed Alamanc service table.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12340
2015-04-09 16:15:02 -07:00
root
75c03986ad typo fix in user doc for events
Summary: Typo fix in user documentation for events

Test Plan: None

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12342
2015-04-09 16:14:44 -07:00
Bob Trahan
b14ca38940 Conpherence - improve stack re: non-update updates
Summary:
Fixes T7761. Fixes T7318.

When we send an empty message to the server, pretend its just a request to load the page. Make load a bit smarter such that if we don't get back any transactions, rather than error like the fool, just send down to the client the notion of a 'non_update'. Instrument the client to just turn off the appropriate loading state, etc for a non update.

T7318 is a tricky beast since we don't know exactly how to reproduce it but if / when it occurs again it would be some other bizarre application behavior maybe? We won't be getting the execption anymore, that's for sure.

Test Plan: removed code in `ConpherenceThreadManager.sendMessage` that protects against sending empty messages. sent empty messages (non updates) like whoa and everything worked on both durable column and main column view. re-added the code in `ConpherenceThreadManager.sendMessage` and noted empty messages did not send while any text including a space sent up nicely

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7318, T7761

Differential Revision: https://secure.phabricator.com/D12339
2015-04-09 15:07:12 -07:00
Bob Trahan
540e38d20e Conpherence - fix recent participant cache
Summary: Ref T7795. This fixes the behavior where you end up with a "a, b, c..." as the list of participants, and yet user a just left.

Test Plan: joined and left a thread. verified database had correct values. observed correct behavior in messages dropdown

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7795

Differential Revision: https://secure.phabricator.com/D12338
2015-04-09 14:02:35 -07:00
epriestley
dba984bd87 Fix bad rendering for custom policy descriptions
Summary: This call got renamed but I missed the callsite.

Test Plan: No more fatal when viewing a custom policy from a transaction history page.

Reviewers: btrahan, chad, Mnkras

Reviewed By: chad, Mnkras

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12337
2015-04-09 13:10:19 -07:00
Bob Trahan
e0473aa702 Cache - fix method call typo
Summary: Fixes T7791.

Test Plan: grep'd for the typo and only the typo declaration had that functon name.

Reviewers: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7791

Differential Revision: https://secure.phabricator.com/D12334
2015-04-08 18:01:58 -07:00
Bob Trahan
76d360740e Conpherence - don't lose title glyph when switching between messages
Summary: Fixes T7735

Test Plan: switched threads in main conpherence view and observed working title glyph.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7735

Differential Revision: https://secure.phabricator.com/D12305
2015-04-08 13:46:41 -07:00
Bob Trahan
d24b3dcb7d Conpherence - implement PhabricatorDestructibleInterface so threads can be deleted
Summary: Fixes T7694. I had to complicate the `ConpherenceThreadQuery` code slightly so that if we specify id(s) or phid(s) then we don't bother with all that join stuff we need to make sure we have a reasonable query in production.

Test Plan: `bin/remove destroy ZXX` worked! tried to visit `ZXX` and got a nice 404. Clicked around and couldn't find anything broken because of the deletion

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7756, T7694

Differential Revision: https://secure.phabricator.com/D12304
2015-04-08 12:19:40 -07:00
Bob Trahan
8fc45e1774 Diffusion - further fix translation of revert commit stories
Summary: didn't quite get there in D12309

Test Plan: made a revert commit and inspected my feed and it was correct (screenshot shortly)

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12333
2015-04-08 12:12:41 -07:00
epriestley
2f0e2544d3 Fix a bad method call in cache startup. 2015-04-08 11:32:30 -07:00
epriestley
2a3783cd2c Suggest "apc.slam_defense" off and "apc.write_lock" on
Summary: Ref T5501. These settings reduce error log noise.

Test Plan: Faked into this branch and hit the warning.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5501

Differential Revision: https://secure.phabricator.com/D12319
2015-04-08 11:31:19 -07:00
epriestley
ffe9c26b00 Emit cache setup issues from cache specs
Summary:
Ref T5501. Currently, we emit some bad warnings about, e.g., "apc.stat" on PHP 5.5+ systems with OPcache, where the warnings are not relevant.

Generate and raise warnings out of the CacheSpec pipeline so we only run relevant code.

Test Plan: Faked various warnings and saw them render correctly.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5501

Differential Revision: https://secure.phabricator.com/D12318
2015-04-08 11:31:01 -07:00
Chad Little
ff59f4f16b Send Markdown in Diffusion through SourceSans
Summary: This moves Markdown rendering from normal fonts to PHUIDocumentView with Source Sans improving readability of this longer form text.

Test Plan:
Test libphutil and Phabricator readmes in my sandbox.

{F363483}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12330
2015-04-08 09:41:56 -07:00
Chad Little
21f0ce7362 Make taller tables the default for AphrontTableView
Summary: I considered at the time just making all tables taller. This removes the special casing and adds the space universally. On first glance all smaller tables look great, but Diffusion seems a little bloated. After a short time period though that went away for me. I do think Diffusion overall needs a UI refresh.

Test Plan: Tested numerous tables in Phortune, Diffusion, etc. Spacing feels more readable.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12328
2015-04-08 08:38:15 -07:00
epriestley
13c0c3b850 Fix "To: Unknown Object" on outbound Maniphest mail with no owner
Summary: Fixes T7778. This was likely caused by removing an `array_filter()` somewhere in the course of T7731, but I'd rather have the code be more correct.

Test Plan:
Sent mail on a task with no owner.

  - Before patch: unknown recipient.
  - After patch: expected recipients.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T7778

Differential Revision: https://secure.phabricator.com/D12320
2015-04-08 05:22:16 -07:00
epriestley
4783c3940d Summarize data cache usage and allocation information
Summary: Ref T5501. Group cache data by key pattern.

Test Plan: {F362994}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5501

Differential Revision: https://secure.phabricator.com/D12317
2015-04-07 16:00:18 -07:00
Michael Krasnow
ac27b93c9f Wowify translations
Summary: Add some more translations to make the interface very wow (Thanks to Robert Calaceto)

Test Plan: Squinted my eyes and stared at the UI until stuff made sense.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, chad, epriestley

Differential Revision: https://secure.phabricator.com/D12312
2015-04-07 15:53:59 -07:00
epriestley
c5d38c6e08 Provide more information about cache sizing in cache diagnostics
Summary: Ref T5501. This expands cache information a little more.

Test Plan: {F362975}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5501

Differential Revision: https://secure.phabricator.com/D12316
2015-04-07 15:08:47 -07:00
epriestley
0880788bd4 Restructure cache checks to improve modularity
Summary:
Ref T5501. This code was headed down a bad road; dump an indirection layer between rendering and data gatehring.

In particular, this will make it much easier to lift these issues into setup warnings eventually.

Test Plan: Viewed cache status page.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5501

Differential Revision: https://secure.phabricator.com/D12315
2015-04-07 14:38:03 -07:00
epriestley
c6b05dbb63 Add a very basic cache status page
Summary:
Ref T5501. This is just getting version detection and availability right, probably.

Eventually, this will get lifted up a bit and "$remedy" will turn into setup issues (or maybe one setup issue saying "your cache setup is messed up, click here to understand why").

Test Plan:
{F362935}

I intend to shove these up to production one-by-one since production is APC and local is Opcache + APCu.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5501

Differential Revision: https://secure.phabricator.com/D12314
2015-04-07 14:28:20 -07:00
Chad Little
f4baa3bd15 New Search UI
Summary: This is the new search UI as seen in Pholio. Unrounds it (mostly, and makes the eyeglass more button like.

Test Plan:
Review in all header colors, check photoshop for pixel layout consistency. Review in FF, Chrome, Safari, IE. Make sure I didn't mess up mobile.

{F359366}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12277
2015-04-07 12:03:40 -07:00
Joshua Spence
ec12b710aa Re-enable the deprecated calls setup check
Summary: This reverts commit 55c00ebfa1. T7111, T7112 and T7113 have all been resolved now.

Test Plan: Maybe wait a few more weeks... I just wanted to ensure that this wasn't forgotten.

Reviewers: btrahan, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11910
2015-04-07 18:08:49 +10:00
Bob Trahan
3250efa2af translations - add missing translations for revert commits
Summary: I think D11212 missed the feed variants. add 'em.

Test Plan: hope and pray and push to prod and see

Reviewers: joshuaspence, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12309
2015-04-06 21:04:14 -07:00
Joshua Spence
14507dc64b Revert "Minor change to suppress linter warning"
This reverts commit 48569f3629. See P1753.
2015-04-07 09:05:31 +10:00
Bob Trahan
30398b6371 Typeahead - filter typeaheads that the viewer can't see in typeahead debug tool
Summary:
Fixes T7255.

Note however that some datasources - notably user or project - don't implement the class thing in a clean way since multiple classes apply. For now, we just show these datasources to the user.

Also, I guess this could be done more efficiently by querying for all the applications at once via an application query? LMK if you want me to make that change.

Test Plan: loaded /typeahead/class/ and played with it a bit with no issues

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7255

Differential Revision: https://secure.phabricator.com/D12307
2015-04-06 15:24:58 -07:00
Joshua Spence
b2c23d88e8 Implement HTTP response messages
Summary: Fixes T7486. Implement HTTP response messages such as `200 OK` and `404 Not Found`. The status codes were taken from http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html.

Test Plan: Navigated to `/foo` and saw the response showing `404 Not Found` in the Network tab of Chrome.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7486

Differential Revision: https://secure.phabricator.com/D12299
2015-04-07 07:41:32 +10:00
Joshua Spence
ed3c5f91e9 Mark ChatLog as deprecated
Summary: Ref T6875. #chatlog is deprecated and will eventually be merged into #conpherence.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: cburroughs, epriestley

Maniphest Tasks: T6875

Differential Revision: https://secure.phabricator.com/D12097
2015-04-07 07:24:02 +10:00
Joshua Spence
cbaa2a508d Add doclink for Diviner documentation
Summary: Fixes T7502.

Test Plan: Went to `/diviner/` and saw a link to the documentation at `/help/documentation/PhabricatorDivinerApplication/`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7502

Differential Revision: https://secure.phabricator.com/D12094
2015-04-07 07:23:50 +10:00
Joshua Spence
48569f3629 Minor change to suppress linter warning
Summary: Explicitly declare the delimiter for `preg_quote`.

Test Plan: `arc lint`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11914
2015-04-07 07:23:41 +10:00
Bob Trahan
a55fb95bd5 Conpherence - only validate **new** participants when validating TYPE_PARTICIPANT transactions
Summary: Fixes T7693.

Test Plan: made a conpherence with my main test account and a throwaway test account. deleted the test account. removed myself from the thread. added myself back to the thread.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7693

Differential Revision: https://secure.phabricator.com/D12306
2015-04-06 11:45:43 -07:00
epriestley
a940e6a32d Fix issue with transcripts that do not have associated rules
Some effects are synthetic and do not directly result from rules.

Auditors: btrahan
2015-04-06 10:08:33 -07:00
epriestley
b16db61a87 Allow "send me an email" in personal rules to punch through settings
Summary:
Fixes T7731. When a user writes a "Send me an email" rule, always try send them an email, even if their notification settings would normally downgrade it to a notification.

In particular, this is stronger than these downgrades:

  - Downgrades due to "self actions";
  - downgrades due to "mail tags".

Test Plan:
  - Wrote various Herald rules with "Send me an email" rules.
  - Used `bin/mail list-outbound` / `show-outbound` to vet generated mail.
  - Mail reacted properly to a variety of conditions (disabled accounts, settings, "send me an email" rule, forced delivery).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7731

Differential Revision: https://secure.phabricator.com/D12300
2015-04-06 10:01:32 -07:00
epriestley
c0e26c65e0 Make mail delivery reasons code-based; include positive and negative reasons
Summary:
Ref T7731. Looking forward to T5791, I eventually anticipate writing an interface which looks like a webmail UI where users can review mail they've been sent and understand why they recieved (or did not receive) the mail. Roughly like `bin/mail list-outbound` / `bin/mail show-outbound` work today, but policy-aware (so you can only see messages where delivery was attempted to you).

We currently record a list of "reasons" why a mail is undeliverable, but this list is string-based (so it can not be translated once we start persisting it) and has only negative reasons (so it can not be used to fully understand reasons for delivery or nondelivery).

Make it code-based (so it can be translated) and allow both positive and negative reasons to be listed (so positive reasons can be understood).

Test Plan: Used `bin/mail show-outbound` to review mail delivery reasons, including the positive reason we currently have (forced delivery of authentication mail).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7731

Differential Revision: https://secure.phabricator.com/D12297
2015-04-06 10:01:11 -07:00
epriestley
63f4e66b11 Attach HeraldRules to HeraldEffects
Summary:
Ref T7731. For no particular reason, we currently put `ruleID` and `rulePHID` on `HeraldEffect` objects.

Pretty much all callers need the `HeraldRule` objects instead, and some go to great lengths to get them.

Just attach the `Rule` objects.

Test Plan: Will test thoroughly after next-ish changeset.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7731

Differential Revision: https://secure.phabricator.com/D12269
2015-04-06 10:00:45 -07:00
epriestley
a40748a46c Lift handling of Herald "email" effect to Adapter
Summary:
Ref T7731. Every adapter subclass currently implements this effect in an essentially identical way.

Some day far from now the effects will be modular and this mess will vanish completely, but reduce its sprawl for now.

Test Plan: I'll test this thoroughly at the end of the change sequence since writing rules is a pain.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7731

Differential Revision: https://secure.phabricator.com/D12268
2015-04-06 10:00:35 -07:00
epriestley
e0edb0797c Remove "effector" from HeraldAdapter
Summary:
Ref T7731.

  - This does nothing.
  - I don't know what this was supposed to do.
  - It didn't do anything when it was introduced in rP084c79d85a in 2011, either.

iiam

Test Plan:
  - `grep`
  - ???

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7731

Differential Revision: https://secure.phabricator.com/D12267
2015-04-06 10:00:18 -07:00
Bob Trahan
3a72e53795 Conpherence - sort participant list in main conpherence with an actual algorithm
Summary: Fixes T7765. The existing sort was based on when people were added to the conpherence and basically feels random in time. Instead, sort current logged in user to the top and make the rest of the list alphabetical.

Test Plan:
viewed a conpherenece and noted my logged in user at top and the rest was sorted alphabetically
viewed a room that i was not  a participant in and participant list rendered correctly

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7765

Differential Revision: https://secure.phabricator.com/D12302
2015-04-06 09:55:03 -07:00
epriestley
7aebd2388f Remove monospaced font preference global defaults
Summary: Fixes T7764. These settings have low utility, are no longer used by default, have become less useful on modern Windows which has a better selection of available fonts, and will eventually be subsumed (at least, for the most part) by T4103.

Test Plan:
  - Grepped for strings.
  - Viewed settings.
  - Changed font to "24px impact".
  - Viewed diffs with default and custom font.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: chad, epriestley

Maniphest Tasks: T7764

Differential Revision: https://secure.phabricator.com/D12301
2015-04-06 08:08:53 -07:00
Joshua Spence
75f081aaf2 Separate @nolint annotation
Summary: In its current form, this file is not being linted. This doesn't seem to be intentional.

Test Plan: Introduced a linter error and ran `arc lint`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12282
2015-04-05 22:30:15 +10:00
Joshua Spence
ea376685ae Fix some odd looking arrays
Summary: These arrays looks a little odd, most likely due to the autofix applied by `ArcanistXHPASTLinter::LINT_ARRAY_SEPARATOR`. See D12296 in which I attempt to improve the autocorrection from this linter rule.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12281
2015-04-05 22:29:39 +10:00
epriestley
5b1d0f9ed7 Remove "metamta.precedence-bulk" option (always enable it)
Summary: Ref T7746. This might possibly improve deliverability. Or might make it worse. Who knows?

Test Plan: `grep`

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7746

Differential Revision: https://secure.phabricator.com/D12266
2015-04-03 16:38:27 -07:00
epriestley
d8ab19a166 Tune project hashtags a little more
Summary: Fixes T7738. Improves handling of question marks and quotation marks.

Test Plan: Unit tests.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7738

Differential Revision: https://secure.phabricator.com/D12265
2015-04-03 16:38:19 -07:00
Bob Trahan
7e0c516276 Conpherence - add edit control for rooms
Summary: Fixes T7582. Basically if its a room we should be able to change title + policy and if its a thread just the title. T7582 had ideas to do a dropdown but "view in column" doesn't make sense from conpherence afaik - what would the page you'd end up with the column be? (maybe home?) Anyway, that is iteration we can add laters

Test Plan: edited room metadata successfully from main and column view. edtied thread title from main and column view.

Reviewers: epriestley, chad

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7582

Differential Revision: https://secure.phabricator.com/D12252
2015-04-03 10:12:10 -07:00
epriestley
2b3d3cf7e4 Enforce that global locks have keys shorter than 64 characters
Summary:
Fixes T7484. There's a bunch of spooky mystery here but the current behavior can probably cause problems in at least some situations.

Also moves a couple callsigns to monograms (see T4245).

Test Plan:
  - Faked a short lock length to hit the exception.
  - Updated normally.
  - Grepped for other use sites, none seemed suspicious or likely to overflow the lock length.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7484

Differential Revision: https://secure.phabricator.com/D12263
2015-04-02 13:42:22 -07:00
epriestley
c39b64d963 Improve handle loading semantics for tokenizers
Summary:
Ref T7689. Fixes T7688. When there are a bunch of tokenizers with different values of the same object type (e.g. 5 tokenizers with users a, b, c, d, e) we currently issue 5 separate queries to load their handles.

Improve this behavior in the common case so we can usually batch these loads.

Test Plan:
  - In Maniphest, set the various search fields to different values.
  - Used DarkConsole to examine query plan.
  - Saw fewer queries after patch than before.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7688, T7689

Differential Revision: https://secure.phabricator.com/D12261
2015-04-02 13:42:01 -07:00
epriestley
a804f0ab93 Make file policies for emailed files more consistent
Summary:
Fixes T7712. Currently, files sent via email get default policies, like they were dragged and dropped onto the home page.

User expectation is better aligned with giving files more restrictive policies, like they were draggged and dropped directly onto an object.

Make files sent via email have restricted default visibility. Once we identify the sender, set them as the file author. Later, the file will become visible to other users via attachment to a task, revision, etc.

Test Plan: Sent some files via email; verified they got restrictive policies, correct authorship, and appropriate object attachment.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7712

Differential Revision: https://secure.phabricator.com/D12255
2015-04-02 13:41:39 -07:00
epriestley
e74c386d56 Fix an issue where RepositoryQuery may fail for viewers who can't use Diffusion
Summary:
See IRC. We don't always reach `loadPage()`, particularly if `canViewerUseQueryApplication()` fails.

Perform initialization steps in `willExecute()` instead.

Trace from IRC:

```
2015/04/02 11:48:32 [error] 26979#0: *48012 FastCGI sent in stderr: "PHP message: [2015-04-02 11:48:32] EXCEPTION: (Exception) You must execute() the query before accessing the identifier map. at [<phabricator>/src/applications/repository/query/PhabricatorRepositoryQuery.php:134]
PHP message:   #0 PhabricatorRepositoryQuery::getIdentifierMap() called at [<phabricator>/src/applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php:26]
PHP message:   #1 DiffusionRepositoryRemarkupRule::loadObjects(array) called at [<phabricator>/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php:309]
PHP message:   #2 PhabricatorObjectRemarkupRule::didMarkupText() called at [<phutil>/src/markup/engine/PhutilRemarkupEngine.php:292]
PHP message:   #3 PhutilRemarkupEngine::postprocessText(array) called at [<phabricator>/src/infrastructure/markup/PhabricatorMarkupEngine.php:138]
PHP message:   #4 PhabricatorMarkupEngine::process() called at [<phabricator>/src/infrastructure/markup/PhabricatorMarkupEngine.php:71]
PHP message:   #5 PhabricatorMarkupEngine::renderOneObject(PhabricatorMarkupOneOff, string, PhabricatorUser) called at [<phabricator>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldRemarkup.php:47]
PHP message:   #6 PhabricatorStandardCustomFieldRemarkup::renderPropertyViewValue(array) called at [<phabricator>/src/infrastructure/customfield/field/PhabricatorCustomField.php:1191]
PHP message:   #7 PhabricatorCustomField::renderPropertyViewValue(array) called at [<phabricator>/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php:176]
PHP message:   #8 PhabricatorCustomFieldList::appendFieldsToPropertyList(PhabricatorProject, PhabricatorUser, PHUIPropertyListView) called at [<phabricator>/src/applications/project/controller/PhabricatorProjectProfileController.php:220]
PHP message:   #9 PhabricatorProjectProfileController::buildPropertyListView(PhabricatorProject, PhabricatorActionListView) called at [<phabricator>/src/applications/project/controller/PhabricatorProjectPr
```

Test Plan:
  - Used `rX` in remarkup.
  - Used `rX` in search.
  - Browed Diffusion, etc.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12264
2015-04-02 13:41:30 -07:00
epriestley
638cdb8d57 Don't show comment draft state to non-object-owners
Summary: Fixes T7720. We currently leak the "draft" state of checkboxes; never treat checkboxes as drafts if you can't mark them.

Test Plan: Checked a box, reloaded page in other browser. Previously, the draft state partially propagated. Now, it no longer does.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7720

Differential Revision: https://secure.phabricator.com/D12262
2015-04-02 07:27:53 -07:00
Elan Kugelmass
1ae1cdd7ba Herald queries correctly label Object Rules
Summary: Fixes T7730. Herald queries used to incorrectly label object rules as global rules. An object rule is now labeled as such.

Test Plan: Made a few rules and looked at the herald query page.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7730

Differential Revision: https://secure.phabricator.com/D12259
2015-04-02 05:00:51 -07:00
epriestley
e9886c4353 Fix an issue where we would try to release an unheld lock
Summary: Fixes T7484. If the lock failed, we'd still try to unlock it, which is incorrect.

Test Plan: Ran two `bin/repository update X` in different windows, got proper LockException instead of indirect symptomatic "not locked by this process" exception.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7484

Differential Revision: https://secure.phabricator.com/D12253
2015-04-01 17:37:46 -07:00
Michael Krasnow
898ccd7552 Add translation for the string "Submit" for the Very Wow translation
Summary: Added Submit translation for Very Wow Locale

Test Plan: Look at the code and hope it works

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12254
2015-04-01 16:41:53 -07:00
Chad Little
de737e8039 Update Participants UI in Conpherence Full
Summary: This shrinks the UI to fit more people in the participants list, useful for rooms. Also update the remove icon.

Test Plan:
Review a lot of people in a room, so so many.

{F354233}

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12213
2015-04-01 15:49:25 -07:00
Bob Trahan
fa673dde35 Conpherence - finish basic application search
Summary:
Fixes T7584. Adds the ability to specify rooms, messages, or both. Adds policy icon to rooms result view and envelope icon to messages result view. Fixes a missing group by clause in thread query. Enforces having participant phid if the query isn't looking at rooms and doesn't have other particpant phids.

This last bit has a small UI quirk if the user searches for "messages" or "both" with no participant phids as we don't give them the feedback that they were included in the query. We could just slap the viewer in the particpants list in this case but it seemed like a buggier feeling experience to have the viewer appear up there? (Especially so in messages case, where we are basically being smart about policy filtering to come.)

Test Plan: clicked around and got sensible results

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7584

Differential Revision: https://secure.phabricator.com/D12232
2015-04-01 15:48:47 -07:00
Chad Little
410380be3f Update Conpherence full message column UI
Summary:
Minor things

 - Use radiused avatars like timeline
 - Fix edge case with conpherence edited and date markers

Test Plan: Review a number of odd states in Conpherence full.

Reviewers: epriestley, btrahan

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12251
2015-04-01 13:15:53 -07:00
epriestley
983c6f9d4b Fix a tokenizer issue in a rare edge case
Summary:
Ref T7689. We may not propgate the viewer down to the controls in some rendering pathways. An example is "Create Task..." on workboard columns.

Instead, move the propagation a little deeper.

Test Plan: "Create Task..." on a workboard column.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12250
2015-04-01 12:41:23 -07:00
epriestley
eb81fd1562 Expose all application mail receivers
Summary:
Fixes T7199. This still isn't a shining example of perfect code, but the raw amount of copy/paste is much lower than it used to be.

  - Reduce code duplication between existing receivers.
  - Expose receiving objects in help menus where appropriate.
  - Connect some "TODO" receivers.

Test Plan:
  - Sent mail to every supported object type.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12249
2015-04-01 11:52:02 -07:00
epriestley
52eab87608 Implement the "!priority" and "!status" mail commands
Summary: Ref T7199. Adds "!priority" and "!status".

Test Plan:
  - Used `!priority` and `!status` to adjust tasks.
  - Changed config and provided keywords.

{F355976}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12248
2015-04-01 11:51:53 -07:00
epriestley
be1fbba698 Implement a "!subscribe" mail command
Summary: Ref T7199. Implements "!subscribe".

Test Plan: Used `bin/mail receive-test` to apply the command to objects.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12247
2015-04-01 11:51:52 -07:00
epriestley
cb6349b88c Implement a "!projects" mail command
Summary: Ref T7199. Implements `!projects` for all objects which implement `PhabricatorProjectInterface`.

Test Plan: Added projects to a task via email.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12246
2015-04-01 11:51:51 -07:00
epriestley
25f8a37f85 Provide better general high-level documentation on mail commands
Summary: Ref T7199. This makes the page look less janky and provides more context about how mail commands work and how to use them.

Test Plan: {F355959}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12245
2015-04-01 11:51:49 -07:00
epriestley
c169199e64 Allow applications to have multiple "help" menu items
Summary:
Ref T7199. Convert the single help menu item into a dropdown and allow applications to list multiple items there.

When an application has mail command objects, link them in the menu.

Test Plan:
{F355925}

{F355926}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12244
2015-04-01 11:51:48 -07:00
epriestley
6f95b325c6 Generate mail command documentation from active commands
Summary: Ref T7199. This needs some polish and isn't reachable from the UI, but technically has all of the information.

Test Plan:
{F355899}

{F355900}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12241
2015-04-01 11:51:47 -07:00
epriestley
6f59b2ab87 Move Maniphest to modular mail commands
Summary:
Ref T7199. This fully modularizes mail command handling in Maniphest.

I had to add a couple of minor not-totally-solid-feeling tricks to deal with the "create" case, but they feel not-too-bad, and a million times better than what came before.

Test Plan: Used all commands with `receive-test`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12240
2015-04-01 08:40:42 -07:00
epriestley
d4bfaa2feb Move Differential to modular mail commands
Summary: Ref T7199. Convert Differential to modern modular commands.

Test Plan: Used `bin/mail receive-test` to send command and comment mail to Differential.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12239
2015-04-01 08:40:12 -07:00
epriestley
c32fee0e48 Fully modularize mail commands
Summary: Ref T7199. Everyone can have a mail command! You can have a mail command! You can have a mail command! Mail commands for everyone!

Test Plan: Used `bin/mail receive-test` to issue commands against files and pastes.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12238
2015-04-01 08:40:00 -07:00
epriestley
7c5f71b691 Subclass most ReplyHandlers from TransactionReplyHandler
Summary: Ref T7199. Half of these aren't even reachable, but make some progress toward reducing the amount of nonsense and garbage in mail handling.

Test Plan: Tested all reachable handlers with `bin/mail receive-test`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12237
2015-04-01 08:39:50 -07:00
epriestley
161f936871 Lift common code for transaction-based reply handlers into parent class
Summary:
Ref T7199. Essentially all of the reply handlers now apply transactions to something which implements PhabricatorApplicationTransactionInterface.

We can share code between them by lifting this stuff into a superclass.

First, convert paste. Also rename `PasteMockMailReceiver` to `PasteMailReceiver` (this got mis-copied from Pholio at some point, I think).

Test Plan: Used `bin/mail receive-test` to send comments + `!unsubscribe` to pastes.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12236
2015-04-01 08:39:21 -07:00
epriestley
afd86a0420 Improve rules for embedding files received via email
Summary:
Ref T7199. Ref T7712. This improves the file rules for email:

  - Embed visible images as thumbnails.
  - Put all other file types in a nice list.

This "fixes" an issue caused by the opposite of the problem described in T7712 -- files being dropped if the default ruleset is too restrictive. T7712 is the real solution here, but use a half-measure for now.

Test Plan:
  - Sent mail with two non-images and two images.
  - Got a nice list of non-images and embeds of images.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7712, T7199

Differential Revision: https://secure.phabricator.com/D12235
2015-04-01 08:39:03 -07:00
Bob Trahan
6f028e16e8 Conpherence - fix fatal if you don't have conpherences
Summary: Fixes T7721. Looks like this got renamed at some point but skipped. Also, change the true / false idx look ups to correct integers here since there's a warning in the error logs in this case without it.

Test Plan: made  new user. loaded up conpherence and it worked. tried all the links too for good measure and they worked

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7717, T7721

Differential Revision: https://secure.phabricator.com/D12243
2015-04-01 07:59:55 -07:00
epriestley
77b23035c3 Allow Maniphest to process multiple commands via email
Summary:
Ref T7199. Two notable changes:

  - Process multiple commands.
  - Process commands when creating //or// updating a task.

And generally clean things up a bit.

Test Plan:
  - Used `receive-test` to execute all commands for new tasks.
  - Used `receive-test` to execute all commands for existing tasks.
  - Used a combination of commands to produce varied effects.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12234
2015-03-31 17:25:19 -07:00
epriestley
34db543d27 Remove all application-specific reply handler class overrides
Summary:
Ref T7199. In the vein of D12231, these options were a bad idea.

  - They once served a very narrow, Facebook-specific need (see T1992), except even Facebook only used the Differential setting AFAIK.
  - Outside of that special case, they are unused and essentially unusable (generally speaking, they do not meaningfully implement anything modular or replaceable).
  - I have no knowledge of any install ever changing these settings, and can imagine no reason why they would.

Moving forward:

  - If they really need to, they can fork locally and chagne one line.
  - I expect "!actions" to make mail at least somewhat more modular soon, anyway.
  - Any derived handlers would break after T7199 and need to be rewritten anyway, so this is just taking advantage of a BC break to do cleanup.

Test Plan:
  - Grepped for removed configuration.
  - Sent some mail from applications, verified the reply handlers set proper reply addresses.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12233
2015-03-31 17:22:01 -07:00
epriestley
bad645f1ec Remove all application-specific reply handler domains
Summary:
Ref T7199. These were a bad idea which got copy-pasted a bunch.

  - There is zero reason to ever set these to different things.
  - Unsurprisingly, I don't know of any install which has them set to different things.

Unless I've completely forgotten about it, this option was not motivated by some obscure business need, it was just a bad decision which didn't catch anyone's attention at the time.

We partially remedied the mistake at some point by introducing `metamta.reply-handler-domain`, which works as a default for all applications, but never cleaned this mess up.

Test Plan: Sent some mail from applications, verified it picked up appropraite reply handler domains.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12231
2015-03-31 16:48:40 -07:00
epriestley
0d99c84bd7 Modernize email command parsing
Summary:
Ref T7199. This prepares for an exciting new world of more powerful "!action" commands. In particular:

  - We parse multiple commands per mail.
  - We parse command arguments (these are currently not used).
  - We parse commands at the beginning or end of mail.

Additionally:

  - Do a quick modernization pass on all handlers.
  - Break legacy compatibility with really hacky Facebook stuff (see T1992). They've theoretically been on notice for a year and a half, and their setup relies on calling very old reply handler APIs directly.
  - Some of these handlers had some copy/paste fluff.
  - The Releeph handler is unreachable, but fix it //in theory//.

Test Plan:
- Sent mail to a file; used "!unsubscribe".
- Sent mail to a legalpad document; used "!unsubscribe".
- Sent mail to a task; used various "!close", "!claim", "!assign", etc.
- Sent mail to a paste.
- Sent mail to a revision; used various "!reject", "!claim", etc.
- Tried to send mail to a pull request but it's not actually reachable.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12230
2015-03-31 16:48:27 -07:00
epriestley
030e05aa4c Remove reply handler instructions from email
Summary:
Ref T7199. Although this is useful for discovery, it's un-useful enough that we already have an option to disable it, and most applications do not provide any meaningful instructions.

Throwing it away makes it easier to move forward and lets us get rid of a config option.

This is becoming a more advanced/power-user feature anyway, and the new syntax will be significantly more complex and hard to explain with a one-liner. I'm currently thinking that I'll maybe make the "help" menu a dropdown and give it some options like:

  +---+
  | O |
  +---+---------------------+
  | Maniphest Documentation |
  | Maniphest Email Actions |
  +-------------------------+

Then you click the "Email Actions" thing and get a runtime-derived list of available options. Not sure if I'll actually build that, but I think we can fairly throw the in-mail instructions away even if we don't go in that specific direction.

Test Plan: Grepped for `replyHandlerInstructions`, got no hits.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12229
2015-03-31 16:48:17 -07:00
epriestley
7cf726c7f7 Fix an issue with FileMailReceiver not working well
Summary: Ref T7199. Guess no one has ever tried to reply to file mail.

Test Plan: Used `bin/mail receive-test` to send mail to files.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12228
2015-03-31 16:47:58 -07:00
Bob Trahan
e0acb75be9 Conpherence - fix possible fatal
Summary: if users have no conpherences I think this is possible? just init it with the rest of the variables we init when we have no conpherence and we should be good to go. fixes T7671.

Test Plan: logicypoo

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7671

Differential Revision: https://secure.phabricator.com/D12227
2015-03-31 14:13:41 -07:00
epriestley
d403700e1f Convert all tokenizers to take token/scalar inputs
Summary: Ref T7689. Ref T4100. This advances the goals of removing `loadViewerHandles()` (only 67 callsites remain!) and letting tokenizers some day take token functions like `viewer()` and `members(differential)`.

Test Plan:
- Sent a new message; used "To".
  - I simplified the cancel URI construction slightly because it's moot in all normal cases.
- Edited a thread; used "Add Participants".
- Searched rooms; used "Participants".
- Searched countdowns; used "Authors".
- Created a diff; used "Repository".
- Edited a revision; edited "Projects"; edited "Reveiwers"; edited "Subscribers".
- Searched for revisions; edited "responsible users"; "authors"; "reviwers"; "subscribers"; "repositories".
- Added revision comments; edited "Add Reveiwers"; "Add Subscribers".
- Commented on a commit; edited "Add Auditors"; "Add subscribers".
- Edited a commit; edited "Projects".
- Edited a repository; edited "Projects".
- Searched feed, used "include Users"; "include Proejcts".
- Searched files, used "authors".
- Edited initiative; edited "Projects".
- Searched backers; used "Backers".
- Searched initiatives; used "Owners".
- Edited build plans; edited "Run Command".
- Searched Herald; used "Authors".
- Added signature exemption in Legalpad.
- Searhced legalpad; used "creators"; used "contributors".
- Searched signatures; used "documents"; used "signers".
- Created meme.
- Searched macros; used "Authors".
- Used "Projects" in Maniphest reports.
- Used Maniphest comment actions.
- Edited Maniphest tasks; edited "Assigned To"; edited "CC"; edited "projects".
- Used "parent" in Maniphest task creation workflow.
- Searched for projects; used "assigned to"; "in any projec"; "in all projects"; "not in projects"; "in users' projects"; "authors"; "subscribers".
- Edited Maniphest bug filing domains, used "Default Author".
- Searched for OAuth applications, used "Creators".
- Edited Owners pacakge; edited "Primary Owner"; edited "Owners".
- Searched for Owners packages; used "Owner".
  - OMG this UI is OLD
- Edited a paste; edited "Projects".
- Searched for paste; used "Authors".
- Searched user activity log; used "Actors"; used "Users".
- Edited a mock; edited "Projects"; edited "CC".
- Searched for mocks; used "Authors".
- Edited Phortune account; edited "Members".
- Edited Phortune merchant account; edited "Members".
- Searched Phrequent; used "Users".
- Edited Ponder question; sued "projects".
- Searched Ponder; used "Authors"; used "Answered By".
- Added project members.
- Searched for projects; used "Members".
- Edited a Releeph product; edited "Pushers".
- Searched pull requests; searched "Requestors".
- Edited an arcanist project; used "Uses Symbols From".
- Searhced push logs; used "Repositories"; used "Pushers".
- Searched repositories; used "In nay project".
- Used global search; used Authors/owners/Subscribers/In Any Project.
- Edited a slowvote; used "Projects".
- Searched slovotes; used "Authors".
- Created a custom "Users" field; edited and searched for it.
- Made a whole lot of typos in this list. ^^^^^^

Did not test:

- Lint is nontrivial to test locally, I'll test it in production.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100, T7689

Differential Revision: https://secure.phabricator.com/D12224
2015-03-31 14:10:55 -07:00
epriestley
7711ea9855 Move handle fetching into tokenizer Datasources
Summary:
Ref T7689. This serves two goals:

  - I want to remove Controller->loadViewerHandles(). A nontrivial number of these callsites are loading handles to pass to tokenizers. Since tokenizers need to take strings eventually anyway, we can do less work by letting them take PHIDs now.
  - A few changes out, I want tokenizers to accept parameterized tokens (like `viewer()`, `members(differential)`, etc.), so the `setValues()` signature needs to change eventually anyway.

I made this work and converted a handful of callsites as an example; upcoming changes will convert more.

Test Plan:
- Viewed Almanac binding editor; used "Interface".
- Edited Almanac services; used "Projects".
- Edited Almanac devices; used "Projects".
- Searched for commits; used "Auditors"; "Commit Authors", "Repositories".
- Searched for calendar events; used "Created By"; used "Invited".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12218
2015-03-31 14:10:32 -07:00
Bob Trahan
3cb5cac018 Conpherence - fix bad query from D12222
Summary: D12222 did an evil thing. This correctly scopes this to just the threads you are participating in and not all threads which then get filtered from policy later, super slowly...

Test Plan: still 'works' and probably wont kill prod

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12223
2015-03-31 13:02:45 -07:00
Bob Trahan
f75248de6f Conpherence - refine menu interactions with respect to "Show More"
Summary:
Ref T7566. Prior to this diff, we had a broken mess in the "Messages" section. Now, "Messages" behave like rooms in that whatever is loaded at page load time is at the top of the list.

Additionally, refine "show more" behavior such that it simply shows the next X, but if there exists X + 1 then we have another "show more" that kicks you to application search. Theoretically, there are still corner cases where users are in a ton of rooms or a ton of messages respectively, but this feels pretty good.

Consolidates title rendering code so we always render the list of participants and no more "No Title".

Also remove the policy icons for messages consistently, helping to differentiate them from rooms at a glance.

Test Plan: clicked around in conpherence main - looked good. tried "show more" and it worked! played around in durable column and things seemed reasonable there too.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7566

Differential Revision: https://secure.phabricator.com/D12222
2015-03-31 12:45:32 -07:00
Chad Little
d81d8caa93 Hide Search text better
Summary: Previously, this text was transparent, moved it to a new span and set visibility on it.

Test Plan: review search in FF, Chrome. Text no longer displays.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7710

Differential Revision: https://secure.phabricator.com/D12219
2015-03-31 09:59:09 -07:00
epriestley
64dddc76c5 Remove Controller->getHandle() and Controller->loadHandles()
Summary: Ref T7689. Modernize all callsites of these methods.

Test Plan:
- Poked at dashboards.
  - Pretty sure this code is technically unreachable right now.
- Viewed commit; viewed "Audit Status".
- Viewed a fund; viewed "Payable to"; viewed "Owner".
- Viewed herald rules; viewed "Author"; viewed "Applies To".
- Viewed a Legalpad document; viewed "Contributors".
- Viewed Phame post list; viewed blog; viewed post (viewed "Blog", viewed "Blogger").
- Viewed a macro; viewed "Audio".
- Viewed a Phriction page; viewed "Last Author".
- Viewed a Ponder question; viewed "Author".
- Viewed a Ponder answer; viewed header.
  - Behavior changed very slightly here; whatevs.
- Viewed a Countdown; viewed "Author".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12210
2015-03-31 05:48:20 -07:00
epriestley
0f52fc771d Remove implode_selected_handle_links()
Summary: Ref T7689. Use the newer, less-janky stuff for rendering handles.

Test Plan:
- Viewed a revision hovercard; viewed "Author", viewed "Reviewers", viewed "Tasks".
- Viewed a task hovercard; viewed "Assigned To"; viewed "Projects"; viewed other edge fields.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12209
2015-03-31 05:48:20 -07:00
epriestley
e1eafd784e Remove Controller->renderHandlesForPHIDs()
Summary: Ref T7689. Remove all remaining callsites for this method.

Test Plan:
- Viewed a custom policy; viewed handles in the policy rules.
- Viewed a Releeph product; viewed "Pushers".
- Viewed a project; viewed "Watchers"; viewed "Members"; viewed "Looks Like".
- Viewed repository edit; viewed "Credential"; viewed "Storage Service"; viewed "Projects".
- Viewed repository detail; viewed "Projects".
- Viewed commit; viewed (faked) "Reverts"; viewed (faked) "Reverted By".
  - These are kind of a pain to generate so I faked 'em.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12208
2015-03-31 05:48:19 -07:00
epriestley
a8271ecd40 Remove most callsites to Controller->renderHandlesForPHIDs()
Summary: Ref T7689. This moves most of the easy/testable callsites off `Controller->renderHandlesForPHIDs()`.

Test Plan:
- Viewed a file; viewed author; viewed "attached" tab.
- Viewed a mock; viewed attached tasks.
- Viewed a credential; viewed "Used By".
- Viewed a paste; viewed author; viewed forks; viewed forked from.
- Viewed a dashboard; viewed panel list.
- Viewed a dashboard panel; viewed "Appears On".
- Viewed a Phortune account; viewed "Members"; viewed payment methods.
- Viewed a Phortune merchant account; viewed "Members".
- Viewed Phortune account switcher; viewed "Accounts".
  - I just removed "Members:" here since it felt kind of out-of-place anyway.
- Viewed a Phragment fragment, viewed "Latest Version", viewed "Snapshots".
- Viewed a Phargment snapshot, viewed "Fragment".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: hach-que, epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12207
2015-03-31 05:48:19 -07:00
epriestley
dec03cf076 Prepare a replacement for Controller->renderHandlesForPHIDs()
Summary:
Ref T7689. This gives HandleLists `renderList()` and `renderHandle()` methods, which return views that can perform just-in-time data fetching and generally look and feel like other rendering code, instead of being odd pseudo-functional methods on `Controller`.

Also converts callsites on the Maniphest detail page to use these methods.

Next changes will wipe out more of the callsites.

Test Plan:
  - Viewed Maniphest detail page with many relevant handles.
  - Created a new subtask.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12205
2015-03-31 05:48:19 -07:00
epriestley
580590fcc9 Remove Controller->getLoadedHandles()
Summary: Ref T7689. Removes this part of the `Controller->loadHandles()` + `Controller->getLoadedHandles()` mechanism.

Test Plan:
  - Viewed Herald transcripts.
  - Viewed Maniphest tasks with attached revisions and commits.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12204
2015-03-31 05:48:19 -07:00
epriestley
1752be630c Improve handle semantics with HandlePool / HandleList
Summary:
Ref T7689, which discusses some of the motivation here. Briefly, these methods are awkward:

  - Controller->loadHandles()
  - Controller->loadViewerHandles()
  - Controller->renderHandlesForPHIDs()

This moves us toward better semantics, less awkwardness, and a more reasonable attack on T7688 which won't double-fetch a bunch of data.

Test Plan:
  - Added unit tests.
  - Converted one controller to the new stuff.
    - Viewed countdown lists, saw handles render.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12202
2015-03-31 05:48:19 -07:00
Bob Trahan
3f738e1935 Conpherence - fix see more link
Summary: Fixes T7705.

Test Plan: link works

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7705

Differential Revision: https://secure.phabricator.com/D12217
2015-03-30 16:53:54 -07:00
Bob Trahan
b200c3bd78 Conpherence - actually remove route from feedback in D12215
Summary: pebkac issue of some sort and I didn't actually commit removing the defunct /conpherence/room/ uri route

Test Plan: made a new room and it worked

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12216
2015-03-30 16:29:08 -07:00
Bob Trahan
89fe35cb0e Conpherence - add application search in a few more places.
Summary: Ref T7584. In Conpherence main view, this adds a "search" link right in the "Rooms" header. This piece addresses an outstanding item on T7584. This diff also adds a search button in the durable column that takes you to the application search. This kind of a big product bet that rooms are going to be dominating things and its most useful to find another room quickly from this view. That said, I think the application search should get massaged slightly to allow searching threads and this won't be much of a trade off at all.

Test Plan: verified new search links took me to correct place and displayed reasonably.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7584

Differential Revision: https://secure.phabricator.com/D12215
2015-03-30 16:25:24 -07:00
epriestley
a03527f440 Fix an issue where redirects would work incorrectly in Quicksand
Summary: Ref T7061. Quicksand still needs an ajax-style response here.

Test Plan: Clicked a file detail page (this redirects) with column open, ended up in the right place.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7061

Differential Revision: https://secure.phabricator.com/D12206
2015-03-30 13:02:51 -07:00
epriestley
c8529787f3 Provide TERM=dumb for Mercurial commit hooks
Summary: Fixes T7119.

Test Plan: Will make someone test.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: cspeckmim, epriestley

Maniphest Tasks: T7119

Differential Revision: https://secure.phabricator.com/D12182
2015-03-30 13:02:43 -07:00
Chad Little
97cc3b1588 Use FontAwesome for search icon
Summary: Swaps out search icon for font-awesome. Tweaks colors a bit for different color headers.

Test Plan: Test a number of colors in the search box. Click on button and still go to advanced search.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12200
2015-03-29 16:20:50 -07:00
Chad Little
12b2257371 Grid spacing for full Conpherence thread list
Summary: Moves to 4px grid / alignment.

Test Plan: Tested per photoshop gridlines, spacing measurements.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12197
2015-03-28 19:28:02 -07:00
epriestley
29beb174d3 Don't treat Quicksand requests as isWorkflow() or isAjax()
Summary:
Fixes T7061. Although it's very simple, I think this is a complete fix.

Quicksand technically is Ajax and uses Workflow as a transport mechanism, but the server should always pretend the user clicked a normal link when rendering.

Test Plan: Links that were autoconverting into dialogs (like "Edit Task") or otherwise making the wrong behavioral choices now work as expected.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7061

Differential Revision: https://secure.phabricator.com/D12194
2015-03-28 15:45:52 -07:00
epriestley
21454842bf Fix comment previews in Quicksand
Summary:
Fixes T7063. Fixes T7059. We were running some extra unnecessary behaviors. In particular, we would render the menu but not actually send it over the wire, so some behaviors would execute, fail to find the nodes they expected, and throw. This cascaded into some other failures.

Do less work and don't activate behaviors which won't be able to run.

Test Plan: Clicked around, no more errors on page transition. Comment previews work.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7059, T7063

Differential Revision: https://secure.phabricator.com/D12188
2015-03-28 06:54:04 -07:00
Chad Little
637974a190 Polish up Done Button UI States
Summary:
Improves the UI quite a bit.

 - `dashed` borders //everywhere// to denote **Unsubmitted**
 - `$sky` sprinkled everywhere to denote **Done**
 - Consilidate `inline-state-is-draft` to simply styles.

Test Plan:
Sent myself test comments, logged out, read comments on new account. Marked as done, submitted.

{F352240}

{F352242}

{F352245}

{F352246}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12187
2015-03-27 18:30:09 -07:00
epriestley
a17542ab28 Touch up PHP/JS interactions for inline comments
Summary:
Ref T1460. Overall:

  - Pass `objectOwnerPHID` consistently.
  - Pass viewer consistently.
  - Set the correct draft state for checkboxes on the client.

Test Plan:
  - Made inline comments in Differential.
  - Made inline comments in Diffusion.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T1460

Differential Revision: https://secure.phabricator.com/D12186
2015-03-27 17:08:31 -07:00
Chad Little
b560014577 Revamp inline commenting UI
Summary:
Rebuilds the UI in Differential commenting. Specifically we look at the following design patterns:

**To the author:**
 - The author of the diff should be able to easily identify what comments are done and not done.
 - We keep undone comments yellow
 - Clicking done turns comment block into 'unsubmitted state'

**To the reviewer:**
 - Easier understanding of unsubmitted states
 - All conversations to be yellow/important

**Todo**
 - Not all color CSS states correct
 - Unpulished checkbox support

Test Plan:
Test leaving comments, published and unpublished. Checking Done, unpublished and published. Check delete states.

From the Diff Author's perspective:
{F352094}

For a Diff commenter's perspective:
{F352095}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T1460, T7660, T7503

Differential Revision: https://secure.phabricator.com/D12171
2015-03-27 16:00:09 -07:00
epriestley
174cf82398 Provide getObjectOwnerPHID() on inline comment views
Summary:
This returns the PHID of the current revision owner, or the commit author, if one exists.

NOTE: For drafts, we currently return `null`; I'll fix that in a future change. Should be correct for submitted comments.

Test Plan: Added an inline, nothing seemed broken.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12185
2015-03-27 11:23:10 -07:00
epriestley
52a4c821f5 Specifically describe "local.json" in backup/migration config documentation
Summary: Fixes T7648. The existence of this file is not necessarily obvious and is important to backup, transfer, and restore.

Test Plan: Read documentation.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7648

Differential Revision: https://secure.phabricator.com/D12183
2015-03-27 09:20:51 -07:00
Bob Trahan
a428bb8ad4 Conpherence - fix bug with possible null value
Summary: Fixes T7675. epriestley caught this in code review and I didn't implement it all the way

Test Plan: logicypoo

Reviewers: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7675

Differential Revision: https://secure.phabricator.com/D12181
2015-03-27 06:32:23 -07:00
epriestley
0dda809da6 Fix a translation string
Summary: Fixes T7672. This had two `%d` conversions but only one parameter.

Test Plan: Adjusted limit to 0, viewed a merge, saw proper message.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7672

Differential Revision: https://secure.phabricator.com/D12180
2015-03-26 17:12:00 -07:00
Bob Trahan
22501ab31e Conpherence - unit tests for T7670
Summary: Ref T7670. Add a few unit tests to make sure deleting everyone works. Also change remaining processRequest to handleRequest while in there.

Test Plan: `arc unit` passed

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7670

Differential Revision: https://secure.phabricator.com/D12179
2015-03-26 16:46:47 -07:00
Bob Trahan
39fa190c15 Conpherence - get lots of rooms stuff hooked up nicely
Summary:
Ref T7566. This does a big chunk of what's left

 - Main view
  - "Rooms" sub header
    - 5 Rooms shown at a time, with room you're looking at in the top on page load
      - e.g. viewing /conpherence/x/ the room x is at top always
      - solves corner case of when you have yet to "join" the room
    - "See More" link takes you to application search for rooms you have participated in
    - if no rooms, there is a "Create Room" and "Find Rooms" links.
  - "Messages" sub header
    - same as before
  - policy icons showing up in the menu
 - Durable column view - still just the latest N, no changes really there
 - Transactions - special cased rendering to try to say room vs thread as appropos
 - Bug fix - we weren't recording the initial participants transaction post D12177 / D12163. This fixes that.

Should probably test pagination, and if you want to show more than 5 rooms of have it behave more like messages (where you can wind up in the middle of a paginated list) that will be more work. Also, if lots of messages / rooms (100 is the limit) we might not display rooms if we're supposed to. Yay whale usage! :D

Test Plan: made a new room - success. made a new message - success.  viewed a room from /conpherenece/room/ i wasn't a participant in and noted it showed up at the top of the five rooms. clicked around rooms and stuff loaded nicely.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7566

Differential Revision: https://secure.phabricator.com/D12178
2015-03-26 16:37:32 -07:00
Bob Trahan
d2e59b2289 Conpherence - fix missing method
Summary: Fixes T7669. Broken by D12163 re-factoring and foolihardiness of test coverage. Notably / interestingly, this was broken before D12163 from not implementing policy correctly, so Conpherence has been broken for a bit with few reports.

Test Plan: had user send himself a message

Reviewers: chad, epriestley

Reviewed By: chad, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7669

Differential Revision: https://secure.phabricator.com/D12177
2015-03-26 13:35:48 -07:00
Chad Little
6b74873358 Update icon color in Durable Column header-text
Summary: Moving to an rgba color here to work better with all the various header colors.

Test Plan: Reload sandbox, see new icon color.

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12176
2015-03-26 13:34:34 -07:00
Chad Little
c4799b2c45 Basic touchups to Phortune UI
Summary: This is just a quick pass to fix a few bugs and spacing issues, Phortune itself could probably use some more custom UI, but that'll require some thought and abstraction. This also adds a new taller table CSS, which I mayyyy make automatic on tables with few rows, we'll see.

Test Plan: Browsed my Phortune account, tested new spacing on `admin` for 'full effect'

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12115
2015-03-26 13:16:09 -07:00
Bob Trahan
871c06ab4e Calendar - error better with invalid time values
Summary: Fixes T7665. Unfortunately when the user messes this up its not easy to partially recover so we just reset that time to the default.

Test Plan: set time to "00:00 AM" and got a sensible error.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7665

Differential Revision: https://secure.phabricator.com/D12174
2015-03-26 13:02:49 -07:00
Chad Little
4ae28837fd Update Conpherence CSS to handle multiple edits better
Summary: Fixes T7655. We'll set tighter spacing around edit clusters. Also darkened up the date marker and remove unused `phabricator-transaction-view` CSS that was still scattered around the site.

Test Plan: Test a full and column multi-edit spam. Visited Ponder and Diffusion, noticed no issues using those apps. Grepped for other users of `phabricator-transaction-view`

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: Korvin, epriestley

Maniphest Tasks: T7655

Differential Revision: https://secure.phabricator.com/D12148
2015-03-26 12:56:58 -07:00
Bob Trahan
e4b7263bf8 Conpherence - Differentiate audience of Threads/Rooms with icon
Summary:
Fixes T7629 plus an un filed bug that's breaking creating new threads since we need to add participants EVEN EARLIER than we were doing it now that policy is actually enforced.

Back to the main thrust of this, there is one UI corner case - in the main view if you go from 1:1 to 1:1:1 (i.e. add a 3rd recipient, or Nth in a row) the icon only updates on page reload. I figure this will get sorted out at a later refactor as we make the client better / share more code with durable column.

One other small behavioral oddity is in the main view sometime we start loading with no conpherence. in that case, rather than show some incorrect icon, we show no icon (and "no title") and then things change at load. Seems okay-ish.

Finally, @chad - the CSS is a very work-man-like "use the built in stuff you can specify from PHP" so I'm sure it needs some love.

Test Plan: made all sorts of rooms and threads and liked the icons. noted smooth loading action as i switched around

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, chad, epriestley

Maniphest Tasks: T7629

Differential Revision: https://secure.phabricator.com/D12163
2015-03-26 12:24:29 -07:00
epriestley
ffe654e5e3 Fix directory moves and copies in Subversion hosted repositories
Summary: Fixes T6490.

Test Plan:
```
$ svn mv dir/ dir2
A         dir2
D         dir
D         dir/list.txt

$ svn commit -m 'Move dir/ to dir2/'
Deleting       dir
Adding         dir2

Committed revision 3.

$ svn cp dir2/ dir3
A         dir3

$ svn commit -m 'Copy dir2/ to dir3/'
Adding         dir3

Committed revision 4.
```

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6490

Differential Revision: https://secure.phabricator.com/D12173
2015-03-26 11:13:41 -07:00
epriestley
a4bfed8415 Censor response bodies from Mercurial error messages
Summary:
Ref T6755. In Git and Subversion, running `git clone http://google.com/` or `svn checkout http://google.com/` does not echo the response body.

In Mercurial, it does. Censor it from the output of `hg pull` and `hg clone`. This prevents an attacker from:

  - Creating a Mercurial remote repository with URI `http://10.0.0.1/secrets/`; and
  - reading the secrets out of the error message after the clone fails.

Test Plan: Set a Mercurial remote URI to a non-Mercurial repository, ran `repository update`, saw censored error message.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6755

Differential Revision: https://secure.phabricator.com/D12170
2015-03-26 11:13:17 -07:00
epriestley
40fb0f98df Mostly defuse DNS rebinding attack for outbound requests
Summary: Ref T6755. I'll add some notes there about specifics.

Test Plan:
  - Made connections to HTTP and HTTPS URIs.
  - Added some debugging code to verify that HTTP URIs were pre-resolved.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6755

Differential Revision: https://secure.phabricator.com/D12169
2015-03-26 11:12:22 -07:00
epriestley
2e72e9ff31 Rate limit outbound requests in Macros
Summary:
Ref T6755. Although we do not return response bodies, it is possible to perform crude portscanning if you can execute a DNS rebinding attack (which, for now, remains theoretical).

Limit users to 60 requests / hour to make it less feasible. This would require ~30 years to portscan all ports on a `/32` netblock.

Users who can guess that services may exist can confirm their existence more quickly than this, but if the attacker already had a very small set of candidate services it seems unlikely that portscanning would be of much use in executing the attack.

This protection should eventually be applied to T4190, too (that task also has other considerations).

Test Plan: Set rate limit very low, hit rate limit.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6755

Differential Revision: https://secure.phabricator.com/D12168
2015-03-26 11:11:52 -07:00
epriestley
cce6d06fa5 Move abandoned revisions to "needs review" when updated
Summary:
Fixes T7602. This is similar to the existing behavior for "changes planned" and "needs revision" revisions.

Also fix the "Update Diff" workflow so it correctly selects closed revisions as attachable.

Test Plan: Updated an abandoned revision, saw it change to "Needs Review".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7602

Differential Revision: https://secure.phabricator.com/D12167
2015-03-26 11:11:33 -07:00
epriestley
731404445f Improve task subpriority movement algorithm for homogenous blocks
Summary:
Fixes T7664. When there are a large number of tasks (400+) with the same subpriority (which can happen if the subpriority features are rarely used), it may take more than 30 seconds to rebalance them.

Make the algorithm more aggressive about rebalancing homogenous blocks of tasks.

This may need to get even fancier, but I'd guess it can process blocks 1-2 orders of magnitude larger, which should be ~all installs.

(If someone still hits issues with this, I'll make it fancier.)

Once a block is rebalanced, it doesn't need to be rebalanced again (at least, not as a whole block) so we basically just need to get over the initial hurdle here and then we're good.

In the worst case, we can provide `bin/maniphest rebalance` or similar and do the rebalance step offline.

And, in any case, we have more test coverage here now.

Test Plan:
  - Existing tests.
  - New tests.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7664

Differential Revision: https://secure.phabricator.com/D12166
2015-03-26 11:11:23 -07:00
Chad Little
4bdc51237a Add ability to have tooltips on buttons
Summary: Enables a basic tooltip when using icon buttons and a convenience method for setting an icon.

Test Plan: Built a UIExample.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12172
2015-03-26 11:09:20 -07:00
Elan Kugelmass
fe89d67663 Fixes spelling error in settings log on auth provider pages
Summary: The settings logs on auth provider pages shows "enabled accont linking" instead of "enabled account linking."

Test Plan: Checked the copy on the settings log.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12164
2015-03-26 03:49:58 -07:00
Chad Little
47114513b0 More SIMPLE button styles for buttons and button bars
Summary: Ref T1460, this adds additional buttons colors and styles for use in inline comments. Will also backport to Calendar and PHUIInfoView

Test Plan:
Review new buttons and hover states in UI Examples.

{F350549}

{F350550}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T1460

Differential Revision: https://secure.phabricator.com/D12162
2015-03-25 12:51:54 -07:00
Bob Trahan
42a0229a52 Conpherence - Implement edit rules for rooms
Summary: Fixes T7586. If you can't edit a room, the pertinent UI is greyed out. One exception is the title of the room in the full viewer; this crumb is not disabled as it would be hard to read. Otherwise though, everything is disabled nicely.

Test Plan: tried to add participants when I wasn't allowed to and got an error. added participants otherwise okay. tried to edit title when i wasn't allowed and got an error. otherwise okay. left conpherence threads / rooms successfully.

Reviewers: epriestley, chad

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7586

Differential Revision: https://secure.phabricator.com/D12161
2015-03-25 11:48:22 -07:00
epriestley
1fd163d097 Mostly provide CSS for "done" states
Summary: Ref T7660. I'm not toggling "inline-state-is-draft" correctly in JS yet since it's a little tricky (you can reload to see it) but the main state should work.

Test Plan:
  - Clicked "done", saw comment opacity fade with placeholder style.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7660

Differential Revision: https://secure.phabricator.com/D12160
2015-03-25 10:57:08 -07:00
epriestley
e5445de163 Show only recent open revisions affecting the same files
Summary: Fixes T5658. Over a long period of time, some cruft can build up here. Only show revisions which have been updated in the last 30 days.

Test Plan:
  - Viewed panel in Differential and Diffusion.
  - Changed limit from 30 days to 30 seconds and saw no revisions.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5658

Differential Revision: https://secure.phabricator.com/D12158
2015-03-25 10:21:56 -07:00
epriestley
6ce4044bfa Lock MIME type configuration
Summary:
Ref T6755. This mitigates an attack where you:

  - compromise an administrative account;
  - configure "text/plain" as an "image" MIME type; and
  - create a new macro sourced from a sensitive resource which is locally accessible over HTTP GET, using DNS rebinding.

You can then view the content of the resource in Files. By preventing the compromised account from reconfiguring the MIME types, the server will instead destroy the response and prevent the attacker from seeing it.

In general, these options should change very rarely, and they often sit just beyond the edge of security vulnerabilities anyway.

For example, if you ignore the warnings about an alternate file domain and elect to serve content from the primary domain, it's still somewhat difficult for an attacker to exploit the vulnerability. If they can add "text/html" or "image/svg+xml" as image MIME types, it becomes trivial. In this case not having an alternate domain is the main issue, but easy modification of this config increases risk/exposure.

Test Plan: Viewed affected config and saw that it is locked.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6755

Differential Revision: https://secure.phabricator.com/D12154
2015-03-25 10:16:22 -07:00
epriestley
17e1e7a65a Document the need to purge caches after updating differential.generated-paths
Summary: Fixes T6378.

Test Plan: Set config to `/.*/`, created a new diff, everything was collapsed as generated.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T6378

Differential Revision: https://secure.phabricator.com/D12159
2015-03-25 07:29:09 -07:00
epriestley
4f8147dbb8 Improve protection against SSRF attacks
Summary:
Ref T6755. This improves our resistance to SSRF attacks:

  - Follow redirects manually and verify each component of the redirect chain.
  - Handle authentication provider profile picture fetches more strictly.

Test Plan:
  - Tried to download macros from various URIs which issued redirects, etc.
  - Downloaded an actual macro.
  - Went through external account workflow.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6755

Differential Revision: https://secure.phabricator.com/D12151
2015-03-24 18:49:01 -07:00
epriestley
22b2b8eb89 Fix a bad call in file chunk destruction
Summary: This signature changed at some point after I tested things and I didn't catch it.

Test Plan: Destroyed a chunked large file with `bin/remove`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12152
2015-03-24 18:48:51 -07:00
Bob Trahan
25767096c9 Conpherence - implement join / view rules for rooms
Summary:
Ref T7585. This implements everything specified, with a few caveats

- since rooms you have yet to join can't be viewed in the column yet, the column view has some bugs and isn't expected to work.
- the room you're looking at is just pre-pending to the top of the "recent" list

Test Plan: made a room that no one could join. verified when viewing that there was no comment ui. made a room that others could join. verified folks who had yet to join had a "join" button with an area for text. tried joining with / without message text and it worked in both cases

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7585

Differential Revision: https://secure.phabricator.com/D12149
2015-03-24 18:38:16 -07:00
epriestley
aa310230b6 Detect moves and copies with some unchanged lines as moves or copies
Summary:
Ref T1266. We won't detect a move/copy if fewer than 3 lines are changed.

However, you may move a block like:

  Complicated Line A
  Trivial Line B
  Complicated Line C

...where "Trivial Line B" is something like a curly brace. If you move this block somewhere that happened to previously have a similar trivial curly brace line, we won't be able to find 3 contiguous added lines in order to detect the copy/move.

Instead, consider both changed and unchanged lines when trying to find contiguous blocks. This allows us to detect across gaps where lines were not actually changed.

This new algorithm may be too liberal (for example, we may end up incorrectly identifying moved/copied code before or after changed lines, not just between changed lines), but we can keep an eye on it and tweak it. The algorithm is better factored and better covered, now.

Test Plan:
  - Added a unit test for this case.
  - Spot-checked a handful of diffs and generally saw behavior that made sense and looked better than before.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1266

Differential Revision: https://secure.phabricator.com/D12146
2015-03-24 13:12:24 -07:00
epriestley
373aaa643a Clean up copy detection code a bit
Summary:
Ref T1266. This doesn't change any behaviors, but some of this code has a lot of really complicated conditionals and I tried to break that up a bit.

Also, reexpress this stuff in terms of the "structured" parser in D12144.

Test Plan: Unit tests still pass. They aren't hugely comprehensive but did reliably fail when I screwed stuff up.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1266

Differential Revision: https://secure.phabricator.com/D12145
2015-03-24 13:12:09 -07:00
epriestley
74a4c2cf0b Provide better parsing primitives for hunks
Summary:
Ref T1266. This prepares to fix case (2) on T1266 by improving the robustness of hunk parsing.

In particular, the copy detection code abuses this API because it isn't currently expressive or flexible enough.

Make it more flexible and cover it exhaustively.

I'll move callsites to the new stuff in upcoming revisions.

Test Plan: Unit tests.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1266

Differential Revision: https://secure.phabricator.com/D12144
2015-03-24 13:11:37 -07:00
Bob Trahan
dcaafd6159 Conpherence - grey out username mentions if they aren't in the conpherence
Summary: Fixes T7578. This was pretty easy because conpherence funnels all transacton stuff through this spot

Test Plan: made a new room so only my user was a participant. wrote "@myself will work and @anotherguy will be greyed out" and so it was as expected

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7578

Differential Revision: https://secure.phabricator.com/D12114
2015-03-24 13:08:53 -07:00
Bob Trahan
014bb72050 Conpherence - add "room" search UI and create UI
Summary: Ref T7584. This hits all the major bullets there. Next step on T7584 is figuring out how it integrates into the full UI and column UI. That said, this is a bit buggy feeling right now since Conpherence as is assumes you are a participant all over the place and rooms make no such assumption. I'll probably this bit up next.

Test Plan:
viewed /conpherence/room/ and saw stuff. viewed the "participant" query as two different users and saw different correct result sets. made a room via the button and it worked. tried to view a room I wasn't a participant in and it failed horribly, which is something to fix in a future diff

created a thread via "send message" on a user profile and it worked

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7584

Differential Revision: https://secure.phabricator.com/D12113
2015-03-24 13:04:33 -07:00
epriestley
86404a1a18 Fix handling of notifications with project members
Summary: Fixes T7377. We don't expand projects into members when sending notifications right now. Instead, expand them.

Test Plan:
  - Added a project as a reviewer to a revision, made a comment, saw project members receive a read notification + email (with appropriate preferences).
  - There's meaningful test coverage on the core mail stuff.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7377

Differential Revision: https://secure.phabricator.com/D12142
2015-03-24 12:47:38 -07:00
epriestley
0efae2858e Don't syntax highlight codebase pattern search results
Summary:
Ref T5644. Ref T7472. Currently, we highlight each line of pattern search results in Diffusion.

  - This is incredibly slow for non-PHP languages which need to shell out to Pygments.
  - A lot of this highlighting isn't very useful anyway, because it doesn't have any context.

Instead, try to highlight pattern matches but don't highlight the source itself.

Test Plan: {F349637}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7472, T5644

Differential Revision: https://secure.phabricator.com/D12141
2015-03-24 12:47:28 -07:00