Page MenuHomePhabricator

Add a `title` field to the `maniphest.query` conduit method
AbandonedPublic

Authored by jcox on Dec 9 2016, 8:17 PM.

Details

Reviewers
None
Group Reviewers
Blessed Reviewers
Summary

We've had several requests from API users to be able to query for exact task titles. Using fullText works, but in many cases it includes extra tasks which aren't desired. This adds an optional title parameter to allow people to search for tasks with an exact title.

Test Plan

Went to /conduit/method/maniphest.query/, filled in the title field and saw that conduit brought me the task with that exact title! What joy!

Diff Detail

Repository
rP Phabricator
Branch
AddTitleConstraint
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 14829
Build 19402: Run Core Tests
Build 19401: arc lint + arc unit

Event Timeline

jcox updated this revision to Diff 40943.Dec 9 2016, 8:17 PM
jcox retitled this revision from to Add a `title` field to the `maniphest.query` conduit method.
jcox updated this object.
jcox edited the test plan for this revision. (Show Details)

uh what

how could searching for a task's entire exact title possibly be useful

(See also T9979.)

jcox added a comment.Dec 9 2016, 8:30 PM

haha I had a feeling this probably fell into the category of things that are in the process of being done a better/holistic way

Basically there's T6721, which is "allow users to do fulltext search on the title". I hope that's not really necessary now that we boost title matches in the normal results. I plan to maybe make fulltext search in Maniphest have a 'query relevance' order.

There's also T9979 and some related "maybe do exact substring search on titles", but I'm not sure this is useful with fulltext ordering.

This is a step beyond that, full matching for exact full titles. I don't know why anyone would ever know the exact, full title of a task without also knowing its task ID or PHID?

(Whoever asked for this definitely wanted a query for "browser" to return only tasks titled "browser" exactly, not tasks where "browser" appears somewhere in the title? Why do they know the entire exact task title but not its ID/PHID?)

Because people are filing tasks from external systems in some systematic way (consistent titles) not saving the ID, and then wanting to reference the task later.

The title has an ID in it already:

I think invariably this discussion will descend into "no you're doing it wrong" and I don't actually know which side I support anymore.

no no no no no no no no no no

why

why why

no no

Is Phabricator not a suitable replacement for a Jiffy Lube inventory terminal?

What is the plan for when I, purely out of spite, move to the east coast and get myself hired so I can create tasks with duplicate names filled with unrelated frivolous content?

Also why can't that asset have more than one "IMPI problem"? That title doesn't even look like it's guaranteed to be unique?

I assume everyone is fully aware of the caveats and just being lazy and not wanting to store the identifier they get back when they create the task? I will inquire further.

Materially, here are my concerns about this change:

  • There is no legitimate reason to possibly ever do this.
  • This field actually has a key on it, so query performance shouldn't be a problem (it would be for the LIKE %~ version of this).
  • This field uses sort collation, so titles will be matched case-insenstively. This is probably not desirable when treating titles as unique keys, because it means that IDs "Asset 3ac" and "Asset 3AC" will be considered the same. It is reasonable to imagine that a system based on task titles as global unique identifiers also considers "3ac" and "3AC" to be reasonable, distinct unique identifiers.
  • The most natural interpretation of title as someone learning this API for the first time is probably substring or fulltext match, not complete exact match. As a naive API consumer, I'd be surprised by this behavior. A better name might be entireExactTitle, maybe, hypothetically.
  • Testing !$title will prevent users from querying for tasks with the title "0" (literal zero, no quotes). Use !strlen($title) instead.
  • maniphest.search is the modern preferred method and maniphest.query will vanish at some point.
  • You can implement the constraint into maniphest.search without upstreaming anything by writing a SearchEngineExtension. A reasonable example to follow might be PhabricatorSubscriptionsSearchEngineExtension. (You can't actually execute the query without modifying TaskQuery though.) But this would let you get halfway there without any upstream code.
  • Saving the ID when the remote system creates the task is the correct way to do this.
  • Using a custom field to store the remote object ID is also probably a reasonable way to do this, if some of this mess is because users are being bounced around between forms or something?

I think if this change comes upstream, it will be in the form of fulltext getting a smarter grammar, so you can search for something like title:~"exact full title" to mean "fulltext search of only the title field, performing a substring search which must match the entire field against the query". We're 3-4 steps away from this today, but a step or two closer than we were a couple weeks ago after stuff like D16938.

jcox added a comment.Dec 9 2016, 9:04 PM

Oh god. I step away for 5 minutes and I'm now on the phacility hit-list.

Because people are filing tasks from external systems in some systematic way (consistent titles) not saving the ID, and then wanting to reference the task later.

The "systematic way" in at least one case just consists of giving the user a phab link with URL parameters to pre-fill in the fields. So the system doesn't get the ID returned to it. Yes, this is gross.

I assume everyone is fully aware of the caveats and just being lazy and not wanting to store the identifier they get back when they create the task

This is true.

jcox abandoned this revision.Dec 9 2016, 9:06 PM

Choosing "Abandon Revision" because "Abandon Revision, Set it on Fire, and Pretend it Never Existed" isn't an option. I'll get a diff ready that adds that as an option.

I think we are doing this all downstream.

At worst we can just add another API method to our install, like, maniphest.reallysearchtitle