1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-05 05:02:44 +01:00
Commit graph

15513 commits

Author SHA1 Message Date
Chad Little
af7c92f2c6 Config re-design
Summary:
This is a full UI pass at a cleaner "Config" application. The main idea is to simplify the UI, center it, and have a different feel than other UI, a sort of "manage" UI theme for objects with loads of settings. Also adds a new minimalistic "WHITE_CONFIG" box type which may get re-used in Diffusion settings. This is a 90% pass, I'll have a few follow up diffs. Specifically:

 - Build breadcrumbs as a flexible UI to go into headers.
 - One click ObjectItemView option, for hover states.
 - Sidenav doesn't always select (AphrontFilter issue)
 - Mobile touchups, though it's pretty reasonable.

Test Plan:
Click through every page here, edit options, see new navigation UI. Test a few various setup issue layouts including fatals.

{F5163228}

{F5163229}

{F5163230}

{F5163231}

{F5163232}

{F5163233}

{F5163234}

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D18519
2017-09-05 15:24:15 -07:00
epriestley
20aad35e60 Move Ferret engine "title:..." field definitions to the engine itself
Summary: Ref T12819. Move these out of the core engine into the Ferret engine. In the future different applications can define different functions, like "summary:..." or whatever. This may get more formalization when I possibly do "author:" and such some time down the road.

Test Plan: Searched for "title:...". Searched for "dog:...", got a useful error.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18536
2017-09-05 11:57:51 -07:00
epriestley
46abc11114 Reduce the number of magic strings in the Ferret implementation
Summary:
Ref T12819. Push more of the magic `' '` stuff into the engine and simplify calls to ngram construction.

Also fixes a bug where a task with title "apple banana" and description "cherry doughnut" could match query "banana cherry" by separating separate term segments with newlines instead of spaces.

Test Plan:
  - Indexed some objects.
  - Searched (term, substring, quoted terms).
  - Viewed index in database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18534
2017-09-05 11:57:35 -07:00
epriestley
4a7593f47f Consolidate more Ferret engine code into FerretEngine
Summary: Ref T12819. Earlier I separated some ngram code into an "ngram engine" hoping to share it across the simple Ngrams stuff and the full Ferret stuff, but they actually use slightly different rules. Just pull more of this stuff into FerretEngine to reduce the number of moving pieces and the amount of code duplication.

Test Plan: Searched for terms, rebuilt indexes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18533
2017-09-05 11:57:18 -07:00
epriestley
d5ba30c777 Fix credential control logic for restricted credentials
Summary: Fixes T12975. This logic didn't deal with PolicyException correctly.

Test Plan: {F5167549}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12975

Differential Revision: https://secure.phabricator.com/D18537
2017-09-05 11:56:46 -07:00
Chad Little
5ceca721cc Minor bug fix with PHUIInfoView
Summary: Custom icons here aren't being set. Also use more standard `tt` UI.

Test Plan: Set an icon, see set Icon.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18522
2017-09-05 10:42:31 -07:00
Chad Little
33b4de9acf Update Setup Issue UI
Summary: Slightly cleaner layout

Test Plan: review setup issues resolved and unresolved in local config. fake a fatal.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18521
2017-09-05 10:40:48 -07:00
Chad Little
0efad0276a Vertical align header buttons
Summary: I can't seem to blame any rational for this, these should vertical align.

Test Plan: Project headers with watching buttons.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18520
2017-09-05 10:40:35 -07:00
epriestley
577d498033 Create a virtual "core" field in the Ferret engine for "title and body together"
Summary: See PHI46. The `core:` function means "find results in either the title or body, but not other auxiliary fields like comments".

Test Plan: Searched for text present in the title (yes), body (yes), and comments (no) with the `core:...` prefix.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18514
2017-09-01 09:40:56 -07:00
epriestley
f4f73e0a7e Separate fulltext engine extensions into "enrich" and "index" phases
Summary:
Ref T12819. Some of the extensions "enrich" the document (adding more fields or relationships), while others "index" it (insert it into some kind of index for later searching).

Currently, these are all muddled under a single "index" phase. However, the Ferret extension cares about fields and relationships which other extensions may add.

Split this into two phases: "enrich" adds fields and relationships so other extensions can read them later if they want. "Index" happens after the document is built and has all the fields and relationships.

The specific problem this solves is that comments may not have been added to the document when the Ferret extension runs. By moving them to the "enrich" phase, the Ferret engine will be able to see and index comments.

Test Plan: Ran `bin/search index ...`, grepped for `indexFulltextDocument`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18513
2017-09-01 09:40:11 -07:00
Chad Little
ab99222f28 Fix harbormaster ui on history view / mobile
Summary: This is an older CSS rule that's no longer needed on mobile.

Test Plan: view history view with harbormaster results

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18507
2017-08-30 12:58:02 -07:00
Chad Little
2ba5968b76 Mobile layouts for Diffusion
Summary: Implements a new mobile view thats more fullscreen, not boxed, so more space. Fixes issues with mobile tables when scrolling overflowed content.

Test Plan: Test home, branch, tags, code, file browse, graph, compare, history, readme, open revisions, owners.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18505
2017-08-30 12:28:00 -07:00
Chad Little
67c658a7ed Use selected button state on blame button
Summary: Visually selects the button if blame is on.

Test Plan: Turn blame on and off in Diffusion on a file.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18504
2017-08-30 18:33:42 +00:00
epriestley
3b43a70773 Add "title:..." support to the Ferret engine
Summary:
Ref T12819. Adds (hacky, hard-coded) field support (for now, only for "title").

I've written this so `title:quick ferret` is the same as `title:quick title:ferret`. I think this is what users probably mean.

You can do the other thing as `ferret title:quick`, or `title:quick all:ferret`.

Test Plan: Searched for `title:x`, `title:"x"`, `title:~"x"`, etc. Searched for "garbage:y", got an exception since that's not a recognized function. Searched for `title:x y`, saw both do title search.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18503
2017-08-30 11:30:42 -07:00
epriestley
048aa36c23 Support "-term" in Ferret engine queries
Summary:
Ref T12819. Supports negating search terms, e.g. "apple -honeycrisp".

When negating a term, we're a little more strict about what can match (that is, what can //prevent// a document from being returned) since it's easy for a user to type "apple -honeycrisp -honey -crisp -crispies -olcrispers -honeyyums" to keep refining their search, but hard/impossible to split apart an overboard term.

Test Plan:
  - Ran `apple -smith`, `apple -"granny smith"`, etc.
  - Verified `phone -tact` does not exclude `phone contact`.
  - (In theory, `phone -~tact` would, but the parser currently doesn't support this, and I'm not champing at the bit to add support.)

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18502
2017-08-30 11:30:24 -07:00
epriestley
df9c24e750 Provide some "term vs substring" support for the Ferret engine
Summary:
Ref T12819. Distinguishes between "term" queries and "substring" queries, and tries to match them correctly most of the time. For example:

  - `example` matches "example", obviously.
  - `~amp` matches "example", but `amp` does not.
  - `examples` matches "example" through stemming.
  - `"examples"` does not match "example" (quoted text does not stem).
  - `"an examp"` does not match "an example" (quoted text is still term text).
  - `~"an examp"` matches "an example" (quoted, substring-operator text uses substring search).

Test Plan: Ran searches similar to the above, they seemed to do what they should.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18500
2017-08-30 11:30:04 -07:00
epriestley
e5a495f435 Parse raw Ferret queries into tokens before processing them
Summary:
Ref T12819. Depends on D18492. Instead of passing a raw query into the Query layer, parse it first.

This allows the query layer to figure out which parts should be substring vs term match, and would allow the SearchEngine layer to do `author:...` eventually by picking it out before sending it to the Ferret engine.

Test Plan: Ran some Ferret queries. They work like before, except that nonsense like `-+"quack"` raises an exception now.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18499
2017-08-30 11:29:46 -07:00
epriestley
0e2e525bb4 Add a "terms" corpus to Ferret fields
Summary:
Ref T12819. Ferret currently does substring search, but this is not the default mode users expect: when you search for the "RICO" act, you do not expect to find documents containing "apRICOt" even though "RICO" is a substring.

To support term search, index the corpus as a list of terms with puncutation removed and whitespace normalized so the engine can match against it.

Test Plan:
Ran `storage upgrade`, ran `search index`, saw sensible database results:

```
   rawCorpus: This is the task description.

Hark! Whom'st'dve eaten this "food" shall surely ~perish~?? #blessed
normalCorpus: thi the task descript hark whom dve eaten food shall sure perish bless
  termCorpus:  This is the task description Hark Whom'st'dve eaten this food shall surely perish blessed
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18498
2017-08-30 11:29:14 -07:00
epriestley
77ef38f9a8 Aggregate corpus data in Ferret field rows
Summary:
Ref T12819. This addresses two issues:

  - One practical issue is that right now, if you search for "dog cat", and they appear in different fields (for example, "dog" appears ONLY in the title, while "cat" appears ONLY in a comment) we won't find the document. This is somewhat rare -- usually, if "dog" appears in the title, it's also repeated in the description -- but I think clearly a bug. To attack this, start automatically creating a virtual "ALL" field with the full document text which we'll use as the primary thing we match against.
  - For fields which may occur more than once -- today, only comments -- aggregate them all into one big "all of the text" row instead of writing one row per comment. This partly addresses the first point ("dog" in one comment and "cat" in a different comment won't be found) and partly makes some of the query gymnastics easier.

Test Plan:
Ran `bin/storage upgrade`, ran `bin/search index <Txxx>`, saw sensible corpus values in the database:

```
mysql> select * from maniphest_task_ffield\G
*************************** 1. row ***************************
          id: 3
  documentID: 1981
    fieldKey: full
   rawCorpus: This is the task title
This is the task description.
normalCorpus: thi the task titl
thi the task descript
*************************** 2. row ***************************
          id: 4
  documentID: 1981
    fieldKey: titl
   rawCorpus: This is the task title
normalCorpus: thi the task titl
*************************** 3. row ***************************
          id: 5
  documentID: 1981
    fieldKey: body
   rawCorpus: This is the task description.
normalCorpus: thi the task descript
3 rows in set (0.00 sec)
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18497
2017-08-30 11:28:30 -07:00
epriestley
72cb3d3c84 Limit the damage that degenerate project name typeahead queries can cause
Summary:
See PHI47. When users copy/paste a wall of text into a project tokenizer, we can end up performing a very large number of JOINs.

These JOINs seem okay locally and on `secure`, but the install in PHI47 reports hitting issues.

Since these queries are almost certainly illegitimate (I think no one uses 5+ words to find a project), just limit the search to the 5 longest tokens.

Note that typing 6 tokens will still almost always work, since the UI does additional filtering. However, if you have 100+ projects named "a b c d e ..." and search for "a b c d e z", you may not hit it. This is so degenerate that it's hard to imagine any users encountering it.

This is a stopgap fix, I'll file something longer-term as a followup.

Test Plan: Used `/typeahead/class/PhabricatorProjectDatasource/` to run queries. Saw the same results with shorter query plans for all reasonable queries.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18506
2017-08-30 11:23:38 -07:00
Chad Little
11046d495d Add a selected button ui state
Summary: Only for grey buttons, but can expand. Sets a selected class.

Test Plan: Review new changes in UIExamples.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18501
2017-08-30 10:14:29 -07:00
epriestley
b4cbea9018 Make legacy revision statuses from "differential.query" have type "string" again
Summary:
Ref T2543. The type on these got changed by accident, it should be "string" (crazy nonsense, compatible) not "int" (sensible, not compatible).

(New API uses sensible strings like "accepted" only.)

Test Plan: Called `differential.query` from web UI, saw `"2"` and similar statuses.

Reviewers: chad, jmeador, lvital

Reviewed By: jmeador, lvital

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18493
2017-08-29 13:05:02 -07:00
epriestley
f49d103af5 Fix an issue where "Close Revision" did not appear in the UI
Summary:
Ref T2543. When called from the UI to build the dropdown, there's no Editor, since we aren't actually in an edit flow.

This logic worked for actually performing the edits, just not for getting the option into the dropdown.

Test Plan: Used the dropdown to close an "Accepted" revision which I authored.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18490
2017-08-29 09:58:48 -07:00
Chad Little
f3f671aa90 Align first nav item in settings
Summary: This removes the redundant "Account" label and item, and just keeps the page better aligned.

Test Plan: Review personal settings

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18489
2017-08-29 09:40:49 -07:00
Chad Little
81f42b8343 Align sidenavs better, use sky to highlight
Summary: Simplifies the UI here by removing various borders, instead using just background colors and better alignment.

Test Plan: Test instances, settings, home, projects, workboards.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18488
2017-08-28 16:36:41 -07:00
epriestley
4005a465f7 Make Ferret indexing more robust (UTF8, exception handling)
Summary:
Ref T12819. Two minor improvements from live data:

  - Tokenize in a UTF8-aware way.
  - When one document fails to index, kill the transaction explicitly (rather than leaving it hanging) so we don't cause other failures later.

Test Plan: Created some UTF8 documents locally, indexed them, got clean results.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18487
2017-08-28 15:49:57 -07:00
Chad Little
0609133f45 Limit notifications to latest 10, instead of 15
Summary: This panel just gets super tall at 15 now that date is on it's own line.

Test Plan: Reload panel, count to 10.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18486
2017-08-28 15:15:06 -07:00
epriestley
f97157e7ed Build a prototype fulltext engine ("Ferret") using only basic MySQL primitives
Summary:
Ref T12819. I gave this stuff a sweet code name because all the terms related to "fulltext" and "search" already mean 5 different things. It, uh, ferrets out documents for you?

I'm building this to work a lot like the existing ngram index, which seems to work pretty well. If this sticks, it will auto-resolve the join issue (in T12443) by letting us do the entire thing locally in a JOIN and thus dodge a lot of mess.

This index gets built alongside other indexes, but only shows up in the UI if you have prototypes enabled. If you do, it appears under the existing fulltext field in Maniphest. No existing functionality is affected or disrupted.

NOTE: The query engine half of this is still EXTREMELY primitive, and this probably performs worse than the existing field for now. If this doesn't show obvious signs of being awful on `secure` I'll improve that in followup changes.

Test Plan:
Indexed my tasks, ran some simple queries, got the results I wanted, even for queries "ko", "k", "v0.1".

{F5147746}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819, T12443

Differential Revision: https://secure.phabricator.com/D18484
2017-08-28 14:52:59 -07:00
Chad Little
ed75250f1a Update notification UI a little
Summary: Fixes T8944. Adds a small dot if notification is new along with color. Goes away when clicked. Increased font and padding for readability.

Test Plan: Send notifications from test account, review them in menu, application search, and in real-time display.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Maniphest Tasks: T8944

Differential Revision: https://secure.phabricator.com/D18485
2017-08-28 14:52:52 -07:00
epriestley
643877b467 Don't prompt to mark notifications as read if we don't need to
Summary: Fixes whatever task is tracking this junk, if one exists. Don't prompt unless there's a security issue.

Test Plan:
  - Generated notifications from a test account.
  - Clicked "Mark All" from dropdown menu, no prompt.
  - Clicked "Mark All" from notifications screen, no prompt.
  - Command-Clicked "Mark All" from dropdown menu to open in new window, got normal prompt.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18483
2017-08-28 13:05:08 -07:00
Chad Little
b8b701faf7 Clarify language when Autoclose is disabled for a repository
Summary: Fixes T12051, adds additional language.

Test Plan:
Disable Autoclose in Actions, see updated language under Branches.

{F5147291}

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Maniphest Tasks: T12051

Differential Revision: https://secure.phabricator.com/D18482
2017-08-28 12:00:46 -07:00
Chad Little
37843127e9 Widen blame age line in blame view
Summary: 50% more line, no additional cost! Order Now! Operators are standing by.

Test Plan: Blame a file

Reviewers: epriestley, avivey

Reviewed By: avivey

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18481
2017-08-28 11:32:06 -07:00
Chad Little
750be1c92a Minor spacing clean up on search button
Summary: Gives the search box a small amount of space, smaller button

Test Plan: look closely.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18477
2017-08-25 09:41:19 -07:00
Chad Little
79c6b50049 Fix fatal on logged out Phame Post
Summary: Just deletes the view code until I have time to better plan this out, or just not ship.

Test Plan: Visit Phame post on public logged out page, view count doesnt cause transaction fatal.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18475
2017-08-25 08:47:59 -07:00
epriestley
213e4ec9b5 Add a missing (int) cast to diff IDs for new "transaction.search" method
Summary: These come out of the database as strings (see T12678), force them to integers for the API.

Test Plan: Called `transaction.search`, got integers in JSON instead of strings.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18476
2017-08-25 07:31:22 -07:00
Chad Little
94cad30ac3 Fix bad tables in diffusion blame
Summary: My fake data was 100%, and not all tables have full revision history. This leads to a broken table. Instead check if we have //any// revisions at all, then always show the column, with or without a link inside.

Test Plan: going on a limb this is the correct fix and test on secure... again ...

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18474
2017-08-24 20:02:35 -07:00
Chad Little
12ae08b6b1 Move differential revision to its own table column in blame view
Summary: There is still some layout issues with revisions, so I've tested it better and moved it to it's own column

Test Plan: Fake in some revision data, test various sizes and shapes.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18473
2017-08-24 19:36:42 -07:00
Chad Little
c49896f7c5 Some header tag icons are too small
Summary: Ref D17991, this rule got more specific with shade tag re-write, so needs updating for headers.

Test Plan: Review a differential header, first icon is now 15px instead of 12px

Reviewers: epriestley, avivey

Reviewed By: avivey

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18472
2017-08-24 19:30:04 -07:00
Chad Little
336fe5cdc5 Dont send an email when someone views a Phame post
Summary: lulz. :(

Test Plan: Load PhamePost, get email. Fix. Reload PhamePost, no email.

Reviewers: epriestley, avivey

Reviewed By: avivey

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18471
2017-08-24 19:00:20 -07:00
epriestley
fa5bcf5d94 Provide some more detailed information about inline comments in "transaction.search"
Summary:
Ref T5873. This provides paths and line numbers for inline comments.

This is a touch hacky but I was able to keep it mostly under control.

Test Plan:
  - Made inline comments.
  - Called API, got path/line information.

{F5120157}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5873

Differential Revision: https://secure.phabricator.com/D18469
2017-08-24 15:26:50 -07:00
epriestley
9639ec0dfa Slightly simplify logic for determining if an inline comment has an effect
Summary: Minor cleanup, this logic can be simpler. Instead of special-casing inlines as having an effect if the have a comment, just consider any transaction with a comment to have an effect. I'm fairly certain this is always true.

Test Plan: Made inlines, tried to submit empty comments. Behavior unchanged.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18468
2017-08-24 15:26:32 -07:00
epriestley
6c9026c33a Allow ModularTransactions to opt in to providing data to Conduit
Summary:
Ref T5873. See PHI14. I don't want to just expose internal transaction data to Conduit by default, since it's often: unstable, unusable, sensitive, or some combination of the three.

Instead, let ModularTransactions opt in to providing additional data to Conduit, similar to other infrastructure. If a transaction doesn't, the API returns an empty skeleton for it. This is generally fine since most transactions have no real use cases, and I think we can fill them in as we go.

This also probably builds toward T5726, which would likely use the same format, and perhaps simply not publish stuff which did not opt in.

This doesn't actually cover "comment" or "inline comment", which are presumably what PHI14 is after, since neither is modular. I'll probably just put a hack in place for this until they can modularize since I suspect modularizing them here is difficult.

Test Plan: Ran `transaction.search` on a revision, saw some transactions (title and status transactions) populate with values.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5873

Differential Revision: https://secure.phabricator.com/D18467
2017-08-24 15:25:55 -07:00
epriestley
2722c167d8 Add the skeleton for a "transaction.search" Conduit API method
Summary:
Ref T5873. See PHI14. This does the basics that are shared across everything (IDs, PHIDs, dates, comments).

It doesn't do types (I think I don't necessarily want to expose internal types over the API?) or transaction-specific data.

In the next change, I'm going to add ways to let ModularTransactions "opt-in" to providing more data to Conduit. I'll use this to flesh out the actual desired transaction types (comments, presumably inline comments) and likely leave the rest as skeletons for now until use cases arise so we don't create a backward compatibility issue (or a security issue!) by exposing tons of internal stuff as public-facing API.

Test Plan:
Ran queries, used paging. Retrieved an edited, deleted, and normal comment.

{F5120060}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5873

Differential Revision: https://secure.phabricator.com/D18466
2017-08-24 15:25:34 -07:00
epriestley
ba1925b155 Prevent Differential changeset HTML anchors from colliding with comment anchors
Summary:
Fixes T12970. This is easier than I expected, and appears to occur in only one place.

This prevents a change from ever generating with an anchor like `#12345678`, which is ambiguous because it may be a comment anchor.

Test Plan: Viewed a revision, saw new `change-xxxyyyzzz` anchors, clicked one, got jumped to the right place.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12970

Differential Revision: https://secure.phabricator.com/D18465
2017-08-24 15:25:17 -07:00
epriestley
bc0963d54b Remove rows for personal saved builtin queries
Summary:
Ref T12956. After this change, individual users will no longer be able to modify builtin queries on a user-by-user basis: they will always appear at the bottom of the list, under their personal queries, and can only be managed by administrators.

To support this, clean up the old rows which could be hanging around from before: delete any personal saved queries where the saved query is a builtin query.

To ease this transition, try to pin the query we're deleting //if// the user had reordered things to put it on top.

Test Plan:
  - Ran the migration, saw no changes in the UI but fewer rows.
  - Went back to `master`, reordered queries to put a builtin one on top.
  - Ran the migration.
  - Saw that builtin one drop to the bottom (since it can't be on top anymore) but be pinned, preserving the behavior of `/maniphest/`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12956

Differential Revision: https://secure.phabricator.com/D18464
2017-08-24 15:25:00 -07:00
epriestley
47da632a22 Separate saved queries in applications into "personal" and "global" queries
Summary:
Ref T12956. UI changes:

  - Administrators get a new `[X] Save as global query` option when saving a query.
  - "Edit Queries..." is split into "Personal" and "Global" sections. For administrators, each section can be edited. For non-admins, only the top section can be edited, but any query can be pinned.

A couple notes:

  - This doesn't support "pin for everyone by default". New users just get the first query from the bottom set. That seems reasonable for now.
  - Reordering is currently a little buggy (it works if you've reordered before, but not if you're reordering for the first time), but I need to migrate before I can fix / test that properly. So that'll get cleaned up in the next change or two.

Test Plan:
  - As an admin and non-admin, viewed, edited, disabled, saved-as-personal and saved-as-global various queries.

{F5098581}

{F5098582}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12956

Differential Revision: https://secure.phabricator.com/D18426
2017-08-24 15:24:34 -07:00
epriestley
58b889c5b0 Make the default ApplicationSearch query explicit, not just the first item in the list
Summary:
Ref T12956. Currently, when you visit `/maniphest/` (or any other ApplicationSearch application) we execute the first query in the list by default.

In T12956, I plan to make changes so that personal queries are always first, then global/builtin queries. Without changing the "default query" rule, this will make it harder to have, for example, some custom queries in Differential but still run a global query like "Active" by default. To make this work, you'd have to save a personal copy of the "Active" query, then put it at the top.

This feels a bit cumbersome and this rule is kind of implicit and a little weird anyway. To make this work a little better as we make changes here, add an explicit pinning action, like the one we have in Project ProfileMenus.

You can now explicitly choose a query to make default.

Test Plan:
  - Browsed without pinning anything, saw normal behavior.
  - Pinned queries, viewed `/maniphest/`, saw a non-initial query selected by default.
  - Pinned a query, deleted it, nothing exploded.

{F5098484}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12956

Differential Revision: https://secure.phabricator.com/D18422
2017-08-24 15:21:00 -07:00
Chad Little
ec88917dd7 Remove hover state from labels in action list
Summary: This is wierd and I can't think of a use for it? Causing issues on hover states.

Test Plan: Action list, menus, dropdowns, etc. Labels shouldn't have hover states.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18463
2017-08-24 13:00:09 -07:00
epriestley
d6e47eef19 Don't set a default "group by priority" in the task search engine
Summary:
See PHI42. Currently, `maniphest.search` incorrectly applies this default (group by priority) to all queries via Conduit.

The correct behavior is to apply no grouping constraint.

I think this is also a reasonable general behavior, and the current code seems to date from D6960 in 2013 and didn't seem particularly carefully considered.

This is a minor compatibility break -- saved queries which are more than 4 years old might change their group behavior. I'll note this in the change logs but expect essentially no one to be affected.

Test Plan: Ran a `maniphest.search` Conduit call and observed the underlying query. Before this change, it executed `ORDER BY priority, id`. After this change, it correctly executed `ORDER BY id` only.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18459
2017-08-24 12:37:44 -07:00
Chad Little
66613240fa Have text-less dropdown buttons look better
Summary: Using icons and dropdown buttons without text looks a little wonky, this resets the CSS a bit.

Test Plan: Review button with icon and text, just icon, just test, and dropdowns.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18461
2017-08-24 12:36:56 -07:00