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

538 commits

Author SHA1 Message Date
Chad Little
36e53fd5d0 Remove collapsable option from ProfileMenu
Summary: Never really used this to full potential and takes up a lot of code and space. Remove option for now and make all profile nav menus small by default.

Test Plan: Review user, project, workboard. Set new menus.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T5867

Differential Revision: https://secure.phabricator.com/D17206
2017-01-13 15:03:31 -08:00
epriestley
a635da68d4 Provide bucketing for commits in Audit
Summary:
Fixes T9430. Fixes T9362. Fixes T9544. This changes the default view of Audit to work like Differential, where commits you need to audit or respond to are shown in buckets.

This is a bit messy and probably needs some followups. This stuff has changed from a compatibility viewpoint:

  - The query works differently now (but in a better, modern way), so existing saved queries will need to be updated.
  - I've removed the counters from the home page instead of updating them, since they're going to get wiped out by ProfileMenu soon anyway.
  - When bucketed queries return too many results (more than 1,000) we now show a warning about it. This isn't greaaaat but it seems good enough for now.

Test Plan: {F2351123}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9430, T9362, T9544

Differential Revision: https://secure.phabricator.com/D17192
2017-01-12 12:04:05 -08:00
epriestley
11861265fe Merge "Audit" more completely into "Diffusion"
Summary:
Fixes T6630. Long ago, "Audit", "Diffusion" and "Repositories" were three totally separate applications.

This separation isn't useful and the three rapidly became intertwined. Ideally, they would all be one application.

This doesn't take us quite that far, but Audit no longer has any controllers and has little actual behavior.

The "Audit" screen has always just been a SearchEngine view of commits with some filters on it, and this formalizes that and puts a link to it in Diffusion. (This view has other uses, too.)

Test Plan:
  - Accessed audit from home page.
  - Accessed audit/commits from Diffusion.
  - Could no longer uninstall Audit on its own.
  - Grepped for `/audit/` and `AuditApplication`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6630

Differential Revision: https://secure.phabricator.com/D17186
2017-01-11 16:28:42 -08:00
Chad Little
6816974d57 Basic Favorites application
Summary: Ref T5867. Rough in a Favorites application, not wired to anything.

Test Plan: tbd. currently 404s so... I messed up something. Tossing up to read.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T5867

Differential Revision: https://secure.phabricator.com/D17160
2017-01-10 11:20:44 -08:00
epriestley
0e1388340c Make profile menu /edit/ requests explicitly 404
Summary:
See D17160. Previously, the `/edit/` route was never linked, but fataled when accessed. Make it 404 instead.

Also, fix an issue where editing "Application" menu items would fail because they didn't have a viewer.

Test Plan:
  - Hit `/edit/`, got a 404.
  - Edited an "Application" item.
  - Moved, added, deleted, and edited other items.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17165
2017-01-09 12:13:57 -08:00
epriestley
aa6e788f36 Mark "v3" API methods as stable; mark obsoleted methods as "Frozen"
Summary:
Ref T12074. The "v3" API methods (`*.search`, `*.edit`) are currently marked as "unstable", but they're pretty stable and essentially all new code should be using them.

Although these methods are seeing some changes, almost all changes are additive (support for new constraints or attachemnts) and do not break backward compatibility. We have no major, compatibility-breaking changes planned.

I don't want to mark the older methods "deprecated" yet since `arc` still uses a lot of them and there are some capabilities not yet available on the v3 methods, but introduce a new "frozen" status with pointers to the new methods.

Overall, this should gently push users toward the newer methods.

Test Plan: {F2325323}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12074

Differential Revision: https://secure.phabricator.com/D17158
2017-01-09 07:16:27 -08:00
Chad Little
8a85ee7c15 Add CustomPHID to PhabricatorProfileMenuEngineConfiguration
Summary: Ref T5867, adds a customPHID field, nullable, and lets you query by it... i think? Not fully able to grok all the EditEngine stuff, but I think this is the right place for the query.

Test Plan: Not wired to anything, but pulling up project menu, editing, all still works.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T5867

Differential Revision: https://secure.phabricator.com/D17149
2017-01-07 10:49:54 -08:00
Chad Little
e9243f22b9 Add Form MenuItem, Fix EditEngine Typeahead
Summary: Adds a FormEditEngine MenuItem for adding forms to Projects, Home, QuickCreate. Also adds an EditEngine typeahead that has token rendering issues currently.

Test Plan: Set a normal form as a menu item, edit it, set the name. Set a custom form as a menu item, edit it, set a name.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17098
2017-01-04 13:12:32 -08:00
Chad Little
5e6afa97bc Add a Dashboard MenuItem
Summary: Built similar to Projects, allows setting of a Dashboard to MenuItem.

Test Plan: Add a dashboard with and without a name / icon to a Project.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17092
2016-12-16 13:33:03 -08:00
Chad Little
f277de1d02 Add a basic ProjectProfileMenuItem
Summary: Allows you to name and set a project as a menu item navigation element.

Test Plan: Add a project, no name, see project. Remove. Add a project and give it a short name (bugs) and see project link.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17021
2016-12-15 15:26:29 -08:00
Chad Little
d8b028b51b Clean up Profile Menu Item page
Summary: Cleans up the UI on the page here, uses two column layout, places actions as actionlist instead of dropdown. Changes edit pages to dialogs.

Test Plan: Add an application, divider, link, and facts to a menu page.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17030
2016-12-12 08:38:23 -08:00
epriestley
42896f9f90 Rename all ProfilePanels into ProfileMenuItems
Summary: Ref T11957.

Test Plan:
  - Viewed an existing project profile.
  - Viewed a user profile.
  - Created a new project.
  - Edited a profile menu.
  - Added new profile items.
  - Grepped for renamed symbols.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11957

Differential Revision: https://secure.phabricator.com/D17028
2016-12-11 11:44:38 -08:00
epriestley
8480776ccd Rename "ProfilePanelConfiguration" to "ProfileMenuItemConfiguration"
Summary:
Ref T11957. This renames the Configuration storage, transaction, query, and PHID type.

No rename on the actual menu item types yet, that's next (and should be the end of this, I think).

Test Plan:
  - Viewed projects.
  - Viewed profiles.
  - Edited a project menu.
  - Grepped for all renamed symbols, I think?

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11957

Differential Revision: https://secure.phabricator.com/D17027
2016-12-11 11:44:22 -08:00
epriestley
d6704705a7 Rename "ProfilePanelEditEngine" to "ProfileMenuEditEngine"
Summary: Ref T11957.

Test Plan: Edited profile menus, grepped for renamed symbol.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11957

Differential Revision: https://secure.phabricator.com/D17026
2016-12-11 11:44:01 -08:00
epriestley
923d3d3060 Rename "PanelEngine" to "MenuEngine"
Summary: Ref T11957.

Test Plan:
Grepped for "PanelEngine", renamed everything except "PanelEditEngine".

Grepped for these changed symbols:

```
ispanelengineconfigurable
getprofilepanelengine
setprofilepanelengine
setpanelengine
getpanelengine
PhabricatorProfilePanelEditEngine
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11957

Differential Revision: https://secure.phabricator.com/D17025
2016-12-11 11:43:42 -08:00
Chad Little
f0b6952391 Add an ApplicationProfilePanel
Summary: Allows applications to be added as profile menu items

Test Plan: Add an application to a project, see menu item, click on menu. Uninstall application, see menu without application.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17016
2016-12-09 13:35:17 -08:00
epriestley
23a202866a When running a fulltext query with no query, enforce order by document creation date
Summary:
Fixes T11929. When running with a query, we no longer enforce an order on the subquery join to produce results more quickly when searching for common strings.

However, this means that empty queries (like those issued by "Close as Duplicate") don't order subquery results.

Restore a `dateCreated` order if there is no query text.

Test Plan: Artificially set limit to 10, still saw 10 most recent tasks.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11929

Differential Revision: https://secure.phabricator.com/D16960
2016-11-29 12:14:57 -08:00
epriestley
b2cdebefea Fix two errors from the error logs
Summary: Found these in the `secure` error logs: one bad call, one bad column.

Test Plan: Searched for empty string. Double-checked method name.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16948
2016-11-26 07:50:57 -08:00
epriestley
7c5b5327c8 Use stemming in the MySQL fulltext search engine
Summary:
Ref T6740. When we index a document, also save a copy of the stemmed version.

When querying, search the combined corpus for the terms.

(We may need to tune this a bit later since it's possible for literal, quoted terms to match in the stemmed section, but I think this wil rarely cause issues in practice.)

A downside here is that search sort of breaks if you upgrade into this and don't reindex. I wasn't able to find a way to issue the query that remained compatible with older indexes and didn't have awful performance, so my plan is:

  - Put this on `secure`.
  - Rebuild the index.
  - If things look good after a couple of days, add a way that we can tell people they need to rebuild the search index with a setup warning.

We might get some reports between now and then, but if this is super awful we should know by the end of the weekend.

Test Plan:
WOW AMAZING

{F2021466}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6740

Differential Revision: https://secure.phabricator.com/D16947
2016-11-25 15:30:50 -08:00
epriestley
d54c14c644 If InnoDB FULLTEXT is available, use it for for fulltext indexes
Summary: Ref T11741. I'll wait until the release cut to land this; it just adds a test for InnoDB FULLTEXT being available instead of always returning `false`.

Test Plan:
  - Ran with InnoDB fulltext locally for a day and a half without issues.
  - Ran `bin/storage upgrade`, saw it detect InnoDB fulltext.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11741

Differential Revision: https://secure.phabricator.com/D16946
2016-11-25 15:29:14 -08:00
epriestley
2b7ec1deea Boost search result title matches
Summary: Ref T6740. When a query matches a document title, boost results.

Test Plan:
  - Searched for `button bar`.
  - Before:

{F2019463}

  - After:

{F2019470}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6740

Differential Revision: https://secure.phabricator.com/D16945
2016-11-25 15:25:49 -08:00
epriestley
54470a12d4 Execute fulltext queries using a subquery instead of by ordering the entire result set
Summary:
Ref T6740. Currently, we issue fulltext queries with an "ORDER BY <score>" on the entire result set.

For very large result sets, this can require MySQL to do a lot of work. However, this work is generally useless: if you search for some common word like "diff" or "internet" or whatever and match 4,000 documents, the chance that we can score whatever thing you were thinking of at the top of the result set is nearly nothing. It's more useful to return quickly, and let the user see that they need to narrow their query to get useful results.

Instead of doing all that work, let MySQL find up to 1,000 results, then pick the best ones out of those.

This actual change is a little flimsy, since our index isn't really big enough to suffer indexing issues. However, searching for common terms on my local install (where I have some large repositories imported and indexed) drops from ~40ms to ~10ms.

My hope is to improve downstream performance for queries like "translatewiki" here, particularly:

<https://phabricator.wikimedia.org/T143863>

That query matches about 300 trillion documents but there's a ~0% chance that the one the user wants is at the top. It takes a couple of seconds to execute, for me. Better to return quickly and let the user refine their results.

I think this will also make some other changes related to stemming easier.

This also removes the "list users first" ordering on the query, which made performance more complicated and seems irrelevant now that we have the typeahead.

Test Plan:
  - Searched for some common terms like "code" locally, saw similar results with better performance.
  - Searched for useful queries (e.g., small result set), got identical results.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6740

Differential Revision: https://secure.phabricator.com/D16944
2016-11-25 15:19:54 -08:00
epriestley
48a34eced2 Prepare for InnoDB FULLTEXT support
Summary:
Ref T11741. This makes everything work if we switch to InnoDB, but never actually switches yet.

Since the default minimum word length (3) and stopword list (36 common English words) in InnoDB are generally pretty reasonable, I just didn't add any setup advice for them. I figure we're better off with simpler setup until we identify some real problem that the builtin stopwords create.

Test Plan: Swapped the `false` to `true`, ran `storage adjust`, got InnoDB fulltext indexes, searched for stuff, got default "AND" behavior.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11741

Differential Revision: https://secure.phabricator.com/D16942
2016-11-25 15:18:26 -08:00
epriestley
ff3333548f Create and populate a stopwords table for InnoDB fulltext indexes to use in the future
Summary:
Ref T11741. InnoDB uses a stopwords table instead of a stopwords file.

During `storage upgrade`, synchronize the table from the stopwords file on disk.

Test Plan:
  - Ran `storage upgrade`.
  - Ran `select * from stopwords`, saw stopwords.
  - Added some garbage to the table.
  - Ran `storage upgrade`, saw it remove it.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11741

Differential Revision: https://secure.phabricator.com/D16940
2016-11-25 15:13:08 -08:00
epriestley
a956047989 Use PhutilQueryCompiler in Phabricator fulltext search
Summary:
Ref T11741. Fixes T10642. Parse and compile user queries with a consistent ruleset, then submit queries to the backend using whatever ruleset MySQL is configured with.

This means that `ft_boolean_syntax` no longer needs to be configured (we'll just do the right thing in all cases).

This should improve behavior with RDS immediately (T10642), and allow us to improve behavior with InnoDB in the future (T11741).

Test Plan:
  - Ran various queries in the UI, saw the expected results.
  - Ran bad queries, got useful errors.
  - Searched threads in Conpherence.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10642, T11741

Differential Revision: https://secure.phabricator.com/D16939
2016-11-25 14:46:10 -08:00
epriestley
706c21375e Remove empty implementations of describeAutomaticCapabilities()
Summary:
This has been replaced by `PolicyCodex` after D16830. Also:

  - Rebuild Celerity map to fix grumpy unit test.
  - Fix one issue on the policy exception workflow to accommodate the new code.

Test Plan:
  - `arc unit --everything`
  - Viewed policy explanations.
  - Viewed policy errors.

Reviewers: chad

Reviewed By: chad

Subscribers: hach-que, PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D16831
2016-11-09 15:24:22 -08:00
epriestley
c3644216bf Add developer UI for accessing NUX and "Overheated" query states
Summary: Ref T11773. Not committed to this implementation, but adds some "Developer" query actions to jump to the nux/overheated states without needing to know secret magic URL variables.

Test Plan: {F1878984}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11773

Differential Revision: https://secure.phabricator.com/D16736
2016-10-20 13:38:26 -07:00
epriestley
a3253f78ce Make query engines "overheat" instead of stalling when filtering too many results
Summary: Ref T11773. This is an initial first step toward a more complete solution, but should make the worst case much less bad: prior to this change, the worst case was "30 second exeuction timeout". After this patch, the worst case is "no results + explanatory message", which is strictly better.

Test Plan:
Made all feed stories fail policy checks, loaded home page.

  - Before adding overheating: 9,600 queries / 20 seconds
  - After adding overheating: 376 queries / 800ms

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11773

Differential Revision: https://secure.phabricator.com/D16735
2016-10-20 09:31:37 -07:00
epriestley
f9f25c1e4d Allow users to drop .ics files on calendar views to import them
Summary:
Ref T10747. When a user drops a ".ics" file or a bunch of ".ics" files into a calendar view, import the events.

(Possibly we should just do this if you drop ".ics" files into any application, but we can look at that later.)

Test Plan: Dropped some .ics files into calendar views, got imports.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16722
2016-10-18 15:26:44 -07:00
Giedrius Dubinskas
c71bb0550c Conduit accept int/bool parameters as strings
Summary: Accept Conduit parameter values as strings (e.g. from `curl`) and convert to required type.

Test Plan:
Call conduit method with int/bool parameter iusing `curl` and make sure it does not result in validation error, e.g.
```
$ curl http://$PHABRICATOR_HOST/api/maniphest.search -d api.token=$CONDUIT_TOKEN -d constraints[modifiedEnd]=$(date +%s) -d constraints[hasParents]=true -d limit=1
```

Fixes T10456.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T10456

Differential Revision: https://secure.phabricator.com/D16694
2016-10-14 14:45:57 +00:00
Chad Little
754397c4e7 Fix some minor UI issues with mobile application search
Summary: Background is now always white, spacing in header is more consistent

Test Plan: test mobile, table, desktop application search apps.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16691
2016-10-11 15:52:29 -07:00
epriestley
fa6a5a46ba Make more of the Calendar export workflow work
Summary:
Ref T10747.

  - Adds a "Use Results..." dropdown to query result pages, with actions you can take with search results (today: create export; in future: bulk edit, export as excel, make dashboard panel, etc).
  - Allows you to create an export against a query key.
    - I'm just using a text edit field for this for now.
  - Fleshes out export modes. I plan to support: public (as though you were logged out), privileged (as though you were logged in) and availability (event times, but not details).

This does not actually export stuff yet.

Test Plan: Created some exports. Viewed and listed exports.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16676
2016-10-06 04:14:29 -07:00
Chad Little
87ebb80059 Revert "Clean up more Quicksand"
Summary: This reverts commit 5eb4bc6ca9.

Test Plan: Reload homepage, no scrollbars

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16645
2016-10-01 12:58:30 -07:00
Chad Little
5eb4bc6ca9 Clean up more Quicksand
Summary: Creates a background that renders inside the Quicksand frame, through sorcery.

Test Plan: Turn on Quicksand, visit lots of pages. See correct background colors. This probably blows something up I'm not testing.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16642
2016-10-01 11:22:42 -07:00
Chad Little
e498d4476d Fix some Quicksand bugs
Summary: Packages AppSearch, fixes body color, moves Differential filetree into differential package.

Test Plan: Enable quicksand. Navigate home -> differential -> diff.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16641
2016-09-30 14:59:53 -07:00
epriestley
38ae81fb39 Throw when callers pass an invalid constraint to a "*.search" method
Summary:
Ref T11593. When you call a `*.search` method like `maniphest.search`, we don't currently validate that all the constraints you pass are recognized.

I think there were two very weak arguments for not doing this:

  - It makes compatibility in `arc` across versions slightly easier: if we add a new constraint, we could add it to `arc` but also do client-side filtering for a while.
  - Conduit parameter types //could//, in theory, accept multiple inputs or optional/alias inputs.

These reasons are pretty fluff and T11593 is a concrete issue caused by not validating. Just validate instead.

Test Plan:
  - Made a `maniphest.search` call with a bogus constraint, got an explicit error about the bad constraint.
  - Made a `maniphest.search` call with a valid constraint (`"ids"`).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11593

Differential Revision: https://secure.phabricator.com/D16507
2016-09-07 09:07:53 -07:00
Chad Little
b75cea55a7 Fix search results with tables, fatals in Phortune
Summary: Previously we collapsed all table search results, but the new UI doesn't need it. Remove unused methods and fix CSS.

Test Plan: Legalpad Signatures, Phortune Accounts.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16469
2016-08-29 20:39:53 -07:00
Chad Little
60d1762a85 Redesign Config Application
Summary: Ref T11132, significantly cleans up the Config app, new layout, icons, spacing, etc. Some minor todos around re-designing "issues", mobile support, and maybe another pass at actual Group pages.

Test Plan: Visit and test every page in the config app, set new items, resolve setup issues, etc.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam, Korvin

Maniphest Tasks: T11132

Differential Revision: https://secure.phabricator.com/D16468
2016-08-29 15:49:49 -07:00
Josh Cox
d135b3f2d5 Added application name to the typeahead results for doc type search
Summary: Ref T10951. This adds the application name as an attribute below the document type in the UI for doc type search.

Test Plan: Verify that the application name appears as an attribute on the document type results.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T10951

Differential Revision: https://secure.phabricator.com/D16446
2016-08-25 11:26:49 -04:00
Josh Cox
7f7c3acfac Remove unused apps from the DocumentType typeahead
Summary: Ref T10951. This diff removes uninstalled applications from the result set for DocumentType restults

Test Plan: Uninstall an application (diviner for example), then go to the document type search menu and ensure that the uninstalled application doesn't show up.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T10951

Differential Revision: https://secure.phabricator.com/D16445
2016-08-25 11:05:35 -04:00
Josh Cox
b521f2349e Explain how cats use their time
Summary: Added a brand new shiny cat fact

Test Plan: Pulled up a project with motivator installed and nothing broke

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D16430
2016-08-22 14:42:56 -05:00
Chad Little
15ed2b936c Update Config Application UI
Summary: Switches over to new property UI boxes, splits core and apps into separate pages. Move Versions into "All Settings". I think there is some docs I likely need to update here as well.

Test Plan: Click on each item in the sidebar, see new headers.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16429
2016-08-22 10:40:24 -07:00
epriestley
07082d2867 Don't allow empty list constraints in Conduit calls
Summary:
Ref T11473. If you write a method like `get_stuff(ids)` and then call it with an empty list of IDs, you can end up passing an empty constraint to Conduit.

If you run a `*.search` method with such a constraint, like this one:

```
{
  "ids": []
}
```

...we have three possible beahviors:

  # Treat it like the user passed no constraint (basically, ignore the constraint).
  # Respect the constraint (return no results).
  # Error.

Currently, we do (1). However, this is pretty confusing and I think clearly the worst option, since it means `get_stuff(array())` in client code will often tend to return a ton of results.

We could do (2) instead, but this is also sort of confusing (it may not be obvious why nothing matched, even though it's an application bug) and I think most reasonable client code should be doing an `if ($ids)` test: this test makes clients a little more complicated, but they can save a network call, and I think they often need to do this test anyway (for example, to show the user a different message).

This implements (3), and just considers these to be errors: this is the least tricky behavior, it's consistent with what we do in PHP, makes fairly good sense, and the only cost for this is that client code may need to be slightly more complex, but this slightly more complex code is usually better code.

Test Plan: Ran Conduit `*.search` queries with `"ids":[]` and `"phids":[]`, got sensible errors instead of runaway result sets.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11473

Differential Revision: https://secure.phabricator.com/D16396
2016-08-14 08:31:13 -07:00
Evaldas Alexander
0cb9ca5500 Explain reasoning behind kitty gifts
Summary: I find this fact very useful for understanding my feline companion

Test Plan: Added "Motivator: Cat Facts" to the project, nothing broke

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D16374
2016-08-06 06:26:15 -07:00
Chad Little
11e84c166a Redesign Application Search
Summary: This moves aphront-side-nav to use same table css display as profile nav. Slightly less code to support. Cleans up AppSearch UI, think I've gotten all the edge cases here, but bang on it, can hold until after release cut.

Test Plan: Config, Maniphest, Differential, Diffusion, Home.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16346
2016-08-01 12:23:36 -07:00
epriestley
6e57582aff Allow *.search Conduit API methods to have data bulk-loaded by extensions
Summary:
Ref T11404. Currently, SearchEngineAttachments can bulk-load data but SearchEngineExtensions can not.

This leads to poor performance of custom fields. See T11404 for discussion.

This changes the API to support a bulk load + format pattern like the one Attachments use. The next change will use it to bulk-load custom field data.

Test Plan:

  - Ran `differential.query`, `differential.revision.search` as a sanity check.
  - No behavioral changes are expected
  - See next revision.

Reviewers: yelirekim, chad

Reviewed By: chad

Maniphest Tasks: T11404

Differential Revision: https://secure.phabricator.com/D16350
2016-07-31 11:15:18 -07:00
epriestley
eab74a9d7c Provide better headers and crumbs for Calendar result views
Summary:
Ref T11326. This isn't perfect, but should be a little easier to use and less weird/confusing.

Generally, provide a "Query > Month > Day" crumb on day views, and a "Wed, July 3" header.

Generally, provide a "Query > Month" crumb on month views, and a "July 2019" header.

Also try to fix a bit of padding/spacing on the day view.

Test Plan: {F1739128}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11326

Differential Revision: https://secure.phabricator.com/D16338
2016-07-27 09:44:56 -07:00
epriestley
ba00022730 Remove extra margins on Calendar month view
Summary:
Ref T11326. This doesn't go quite as far as the mock in T11326#185932, but gets rid of the easy margins.

Also cleans up some of the border rules so they're simpler and more consistent (no weird ragged edges on the far right).

Test Plan: {F1738951}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11326

Differential Revision: https://secure.phabricator.com/D16335
2016-07-27 09:36:40 -07:00
epriestley
637b58c7c8 Correct an issue with epoch timestamps in Conduit
Summary:
Fixes T11375. Some validation code was mishandling raw epoch timestamps.

For numeric values larger than 29999999 (e.g., 2999-12-25, christmas 2999), assume the value is a timestamp.

Test Plan: Used `maniphest.search` to query for `modifiedStart`, got a better result set and saw the `dateModified` constraint in the query.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11375

Differential Revision: https://secure.phabricator.com/D16326
2016-07-26 11:02:46 -07:00
epriestley
1c33b70c66 Remove two unused SearchEngine methods
Summary:
Ref T11326. These are last-generation and neither of these have callsites anymore.

(I nuked these since I'm trying to simplify date handling.)

Test Plan: Grepped for callsites.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11326

Differential Revision: https://secure.phabricator.com/D16301
2016-07-14 13:05:33 -07:00