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

60 commits

Author SHA1 Message Date
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
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
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
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
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
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
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
epriestley
5aca529980 Fix literally thousands of drag-to-reorder priority bugs
Summary:
Fixes T7563. Fixes T5201. Reframe this as two separate operations:

  - Move before or after a task.
  - Move to the beginning or end of a priority.

Then:

  - Make all the order queries unambiguous and properly reversible, with an explicit `id` order.
  - Just reuse `ManiphestTask` to get results in the correct order.
  - Simplify the actual transaction apply logic.
  - Detect and recover from cases where tasks have identical or similar subpriorities.

Test Plan:
  - Wrote and executed unit tests.
  - Dragged and dropped tasks within priorities and between priorities in the main Maniphest view.
  - Dragged and dropped tasks within priorities in the workboard view, when ordered by priority.
  - Also poked at the "natural" order, but that shouldn't be affected.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: chad, epriestley

Maniphest Tasks: T5201, T7563

Differential Revision: https://secure.phabricator.com/D12121
2015-03-20 17:38:25 -07:00
epriestley
106ca70acb Fix an issue where subpriority paging could be truncated
Ref T7548. Subpriority is a float, but we're truncating it to an int, which can cause reselection of the same row while paging.
2015-03-14 13:42:06 -07:00
epriestley
77e0a4abba Guarantee that Maniphest paging clauses strictly progress
Ref T7548. Some of these clauses are not guaranteed to select only rows following the cursor.
2015-03-14 13:29:05 -07:00
epriestley
c431d91f58 Fail explicitly instead of looping if we can't build a paging clause
Ref T7548. This isn't a complete fix but hopefully will give me more debugging information.
2015-03-14 13:23:48 -07:00
Bob Trahan
05510aa41f Maniphest - fix blocking / blocked task queries
Summary: Fixes T7392. I kind of stink at SQL so my approach here was to "start over" conceptually and this way makes the most sense to me - we basically do one join on the dependency table and then a second join back from the dependency table to the main task table. In the where clause we filter the resulting rows, first checking the data from dependency join for existence as appropros and then checking the second join for main task table for the proper "open" task values.

Test Plan: made a task X be blocked by task Y. closed task y. search for "not blocked" tasks and saw task X.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7392

Differential Revision: https://secure.phabricator.com/D11962
2015-03-03 15:53:08 -08:00
epriestley
a76049abd4 Fix not blocking / not blocked queries
Summary: Fixes T7434. We need to LEFT JOIN, not JOIN here, because we still want result rows where the value is `null`.

Test Plan: Issued blocked/not-blocked + blocking/not-blocking queries, got results in all cases.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7434

Differential Revision: https://secure.phabricator.com/D11939
2015-03-02 17:00:20 -08:00
epriestley
bb6eb9bfcf Fix ambiguous column names in Maniphest blocking queries
Summary: Fixes T7421. Now that we join the task table again to ignore //closed// blockers, all the column names are ambiguous. Make them unambiguous.

Test Plan: Issued some searches with various different parameters.

Reviewers: btrahan, joshuaspence, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7421

Differential Revision: https://secure.phabricator.com/D11922
2015-03-02 06:43:52 -08:00
Joshua Spence
130aa755bb Exclude closed tasks from blocked/blocking filter
Summary: Fixes T7392. When filtering blocked/blocking Maniphest tasks, don't consider closed tasks.

Test Plan:
# Created `T1` and `T2` with `T2 depends on T1`.
# Marked `T1` as resolved.
# Searched for tasks "blocked by other tasks" and noted that `T2` wasn't in the result set.

Reviewers: btrahan, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7392

Differential Revision: https://secure.phabricator.com/D11911
2015-03-01 09:39:34 +11:00
Bob Trahan
da1531f219 Policy - make ManiphestTaskQuery verify project visibility first thing
Summary: Fixes T7094 (last of many revisions). Its important to do this filtering ASAP so that users can't deduce the identify of an unknown / invisible project.

Test Plan: executed a query for tasks in project foo using user bar. using user foo, lock user bar out of project foo. reissued the query and saw "no data" as well as "restricted project" in the project typeahead.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7094

Differential Revision: https://secure.phabricator.com/D11660
2015-02-03 13:53:35 -08:00
Joshua Spence
463d094f96 Fix method visibility for PhabricatorPolicyAwareQuery subclasses
Summary: Ref T6822.

Test Plan:
`grep` for the following:

  - `->willFilterPage(`
  - `->loadPage(`
  - `->didFilterPage(`
  - `->getReversePaging(`
  - `->didFilterPage(`
  - `->willExecute(`
  - `->nextPage(`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11367
2015-01-14 07:01:16 +11:00
Bob Trahan
1eb8b6a1b8 Maniphest - allow for searching for tasks based on dependency relationships
Summary:
Fixes T5352. This is very useful for finding things that should be easy to do ("not blocked") as well as things that are important to do ("blocking"). I have wanted to check out the latter case in our installation, though no promises on what I would end up actually doing from that search result list. =D

I also think supporting something like T6638 is reasonable but the UI seems trickier to me; its some sort of task tokenizer, which I don't think we've done before?

Test Plan: toggled various search options and got reasonable results. When i clicked conflicting things like "blocking" and "not blocking" verified it was like I had not clicked anything at all.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5352

Differential Revision: https://secure.phabricator.com/D11306
2015-01-12 13:42:37 -08:00
Joshua Spence
7c2a7d0365 Modernize remaining edge types
Summary: Modernize remaining edges to subclass `PhabricatorEdgeType`. Largely based on D11045.

Test Plan: Browsed around and performed various actions include subscribing, unsubscribing and watching.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11116
2015-01-03 10:58:20 +11:00
Bob Trahan
83db5965ab Maniphest - introduce needProjectPHIDs
Summary: Ref T5245. This is some of the associated cleanup there.

Test Plan:
foreach ManiphestTaskQuery site, I made the change (or not) and tested as follows:

=== Call sites where added needProjectPHIDs ===

- PhabricatorHomeMainController - loaded the home page
- ManiphestBatchEditController - batch edited some tasks (added a project)
- ManiphestConduitAPIMethod - tested implicitly when tested ManiphestUpdateConduitAPIMethod
- ManiphestInfoConduitAPIMethod - used the method via conduit console with input id : 1
- ManiphestQueryConduitAPIMethod - used the method via conduit console with input ids : [1, 2]
- ManiphestUpdateConduitAPIMethod - used the method via conduit with input id : 1 and comment : “asdasds"
- ManiphestReportController - viewed “By User” and “By Project”
- ManiphestSubpriorityController - changed the priority of a task via a drag on manphest home
- ManiphestTaskMailReceiver - updated Task 1 via bin/mail receive-test with a comment that is the README
- ManiphestTaskSearchEngine - loaded Manifest home page
- ManiphestTaskEditController - edited a task
- ManiphestTransactionEditor - closed a blocking task
- ManiphestTransactionSaveController - commented on a task
- PhabricatorProjectProfileController - viewed project with id of 1 that has a few tasks in it
- PhabricatorSearchAttachController - merged tasks together
- DifferentialTransactionEditor - submit a diff that references a task; commit the diff (thus closing the diff) and the task gets updated
- PhabricatorRepositoryCommitMessageParserWorker - submit a diff that references a task; commit the diff (thus closing the diff) and the task gets updated

=== Calls sites where *did not* add needProjectPHIDs (they do not appear in this revision) ===

- PhabricatorManiphestApplication - loaded the home page
- ManiphestGetTaskTransactionsConduitAPIMethod - used the method via conduit console with input ids : [1, 2] ManiphestTaskDetailController - viewed a task with and without associated projects; finished workflow creating a task with a parent
- ManiphestTransactionPreviewController - verified transaction preview showed up properly
- PhabricatorProjectBoardViewController - viewed a board
- PhabricatorProjectMoveController - moved a task around
- ManiphestRemarkupRule - made a task reference like {T123}
- ManiphestTaskQuery - executed a custom query for all tasks with page size of 2 and paginated through some tasks
- ManiphestTaskPHIDType - nothing random seems broken? =D

=== Call sites where had to do something funky ===

- ManiphestHovercardEventListener - loaded hover cards from task mentions

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5245

Differential Revision: https://secure.phabricator.com/D11004
2014-12-18 13:53:45 -08:00
Bob Trahan
7d96870570 Maniphest - use subscribers framework properly
Summary: Fixes T5604. This should fix some random bugs, lets us move forward more easily, and all that good stuff about killing code debt.

Test Plan:
- Conduit method maniphest.createtask
  - verified creating user subscribed
  - verified subscription transaction
- Conduit method maniphest.update
  - verified subscribers set as specified to ccPHIDs parameter
  - verified subscription transaction
- Herald
  - verified herald rule to add subscriber worked
  - verified no subscribers removed accidentally
- edit controller
  - test create and verify author gets added IFF they put themselves in subscribers control box
  - test update gets set to exactly what user enters
- lipsum generator'd tasks work
- bulk add subscribers works
- bulk remove subscriber works
- detail controller
  - added myself by leaving a comment
  - added another user via explicit action
  - added another user via implicit mention
- task merge via search attach controller
- mail reply handler
  - add subscriber via ./bin/mail receive-test
  - unsubscribe via ./bin/mail receive-test

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5604

Differential Revision: https://secure.phabricator.com/D10965
2014-12-10 16:27:30 -08:00
James Rhodes
46b4fa85d0 Support custom fields in "Order By" for Maniphest
Summary:
Resolves T4659.  This implements support for sorting tasks by custom fields.

Some of this feels hacky in the way it's hooked up to the Maniphest search engine and task query.

Test Plan: Queryed on a custom date field, with a small page size, and moved back and forth through the result set.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4659

Differential Revision: https://secure.phabricator.com/D10106
2014-08-02 18:22:16 +10:00
Joshua Spence
97a8700e45 Rename PHIDType classes
Summary: Ref T5655. Rename `PhabricatorPHIDType` subclasses for clarity (see discussion in D9839). I'm not too keen on some of the resulting class names, so feel free to suggest alternatives.

Test Plan: Ran unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin, hach-que

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D9986
2014-07-24 08:05:46 +10:00
Joshua Spence
86c399b657 Rename PhabricatorApplication subclasses
Summary: Ref T5655. Some discussion in D9839. Generally speaking, `Phabricator{$name}Application` is clearer than `PhabricatorApplication{$name}`.

Test Plan:
# Pinned and uninstalled some applications.
# Applied patch and performed migrations.
# Verified that the pinned applications were still pinned and that the uninstalled applications were still uninstalled.
# Performed a sanity check on the database contents.

Reviewers: btrahan, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: hach-que, epriestley, Korvin

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D9982
2014-07-23 10:03:09 +10:00
epriestley
9d64beeaa3 Namespace dateCreated in Maniphest query construction
Summary: Fixes T5661. We may now pick up a conflicting `dateCreated` field from an edge table join.

Test Plan: Ran a project + dateCreated filtering query, no longer got an exception.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5661

Differential Revision: https://secure.phabricator.com/D9997
2014-07-21 06:44:35 -07:00
epriestley
b8b59895ee Stop reads and writes to projectPHIDs property on ManiphestTask
Summary: Ref T5245. This property predates edges and is unusual in modern applications. Stop writes to it and populate it implicitly from edges when querying.

Test Plan:
  - Viewed task list.
  - Created a task.
  - Added and removed projects from tasks.

Reviewers: joshuaspence, chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5245

Differential Revision: https://secure.phabricator.com/D9851
2014-07-17 15:42:53 -07:00
epriestley
aa79539789 Move task/project storage to edges
Summary: Ref T5245. This moves the actual storage over and stops reads and writes to the old table.

Test Plan:
  - Verified tasks retained projects across the migration.
  - Added and removed projects from tasks.
  - Searched for: all, any, users' projects, not-in-projects, no-projects.

Reviewers: chad, btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5245

Differential Revision: https://secure.phabricator.com/D9850
2014-07-17 15:42:30 -07:00
Joshua Spence
8756d82cf6 Remove @group annotations
Summary: I'm pretty sure that `@group` annotations are useless now... see D9855. Also fixed various other minor issues.

Test Plan: Eye-ball it.

Reviewers: #blessed_reviewers, epriestley, chad

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9859
2014-07-10 08:12:48 +10:00
epriestley
f0147fd8ad Allow workboards to be filtered with ApplicationSearch
Summary:
Ref T4673.

IMPORTANT: I had to break one thing (see TODO) to get this working. Not sure how you want to deal with that. I might be able to put the element //inside// the workboard, or I could write some JS. But I figured I'd get feedback first.

General areas for improvement:

  - It would be nice to give you some feedback that you have a filter applied.
  - It would be nice to let you save and quickly select common filters.
  - These would probably both be covered by a dropdown menu instead of a button, but that's more JS than I want to sign up for right now.
  - Managing custom filters is also a significant amount of extra UI to build.
  - Also, maybe these filters should be sticky per-board? Or across all boards? Or have a "make this my default view"? I tend to dislike implicit stickiness.

Test Plan:
Before:

{F157543}

Apply Filter:

{F157544}

Filtered:

{F157545}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: qgil, swisspol, epriestley

Maniphest Tasks: T4673

Differential Revision: https://secure.phabricator.com/D9211
2014-05-20 11:42:05 -07:00
epriestley
cf6353e516 Fix a typo in ManiphestTaskQuery
Summary: This doesn't have any impact on anything right now (we don't persist the query) but could in the future, so I just left it as-is but fixed the typo.

Test Plan: looked at it carefully

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: WikiChad, epriestley

Differential Revision: https://secure.phabricator.com/D9155
2014-05-16 12:20:13 -07:00
epriestley
3e9a988cd7 Fix an incorrect filtering of groupless results from Maniphest "Group By" query
Summary:
Fixes T5081. This block enforces policies and prevents you from seeing groups associated with projects you can't see. However, it incorrectly removes tasks which end up with no group key. This can happen in two cases:

  - The task isn't in a project.
  - The task is in a project, but the query includes an "In All Projects: <that project>" constraint. In that case, we don't show the group becuase it would always contain every task.

Test Plan:
Replicated the setup in T5081, saw an "Ungrouped" group with "Task A":

{F155766}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5081

Differential Revision: https://secure.phabricator.com/D9152
2014-05-16 08:47:06 -07:00
epriestley
0a76d82a7c Use string constants, not integer constants, to represent task status internally
Summary:
Ref T1812. I think integer constants are going to be confusing and error prone for users to interact with. For example, because we use 0-5, adding a second "open" status like "needs verification" without disrupting the existing statuses would require users to define a status with, e.g., constant `6`, but order it between constants `0` and `1`. And if they later remove statuses, they need to avoid reusing existing constants.

Instead, use more manageable string constants like "open", "resolved", etc.

We must migrate three tables:

  - The task table itself, to update task status.
  - The transaction table, to update historic status changes.
  - The saved query table, to update saved queries which specify status sets.

Test Plan:
  - Saved a query with complicated status filters.
  - Ran migrations.
  - Looked at the query, at existing tasks, and at task transactions.
  - Forced migrations to run again to verify idempotentcy/safety.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1812

Differential Revision: https://secure.phabricator.com/D8583
2014-03-25 13:58:14 -07:00
epriestley
33bda2d590 Despecialize most task status handling
Summary: Ref T1812. Moves most specialized status handling into `ManiphestTaskStatus`. The only real missing case is reports.

Test Plan:
Browsed most of the affected interfaces. Changed task status:

{F132697}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1812

Differential Revision: https://secure.phabricator.com/D8579
2014-03-25 13:47:42 -07:00
epriestley
5b2887b69b Add "Date Updated" query fields for Maniphest
Summary:
Fixes T4637.

  - We already allow you to order by this column but don't have a key on it. Add one.
  - Expose UI for querying on ranges.

Test Plan:
  - Ran some queries, got reasonable-looking results and no table scans.

Reviewers: btrahan, bigo

Reviewed By: bigo

Subscribers: bigo, epriestley

Maniphest Tasks: T4637

Differential Revision: https://secure.phabricator.com/D8557
2014-03-17 15:53:07 -07:00
epriestley
a48128d36f Partially use ApplicationSearch in main search
Summary:
Ref T4365. Primary search currently uses `PhabricatorSearchQuery` for storage, which is pretty much the same as `PhabricatorSavedQuery`, except that it's old and not used anywhere else anymore.

Maniphest used to also use this table, but no longer does after Septmeber, 2013. We need to retain the class so the migration can work.

This introduces `PhabricatorSearchApplicationSearchEngine` and `PhabricatorSearchDocumentQuery`, but they're both stubs that I just needed for technical reasons and/or to pass lint. The next couple patches will move logic into them and use ApplicationSearch properly.

Test Plan:
  - Searched for stuff.
  - Searched for stuff with filters.
  - Searched for fulltext in Maniphest.
  - Grepped for `PhabricatorSearchQuery`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4365

Differential Revision: https://secure.phabricator.com/D8120
2014-02-03 12:51:08 -08:00
epriestley
2a5c987c71 Lock policy queries to their applications
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.

This has several parts:

  - For PolicyAware queries, provide an application class name method.
  - If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
  - For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.

Test Plan:
  - Added a unit test to verify I got all the class names right.
  - Browsed around, logged in/out as a normal user with public policies on and off.
  - Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7367
2013-10-21 17:20:27 -07:00
epriestley
36343600c5 Remove obsolete code from ManiphestTaskQuery
Summary:
Ref T603. Cleans up some obsolete stuff here:

  - We no longer ever query by min/max priority (instead, `withPriorities(...)`).
  - A parent class provides limit/offset.
  - Result count is no longer reliable with policies. We could do "about X tasks" or something, but just drop it for now. There's only one remaining callsite anyway.

Test Plan:
  - `grep`
  - Viewed task list.
  - Viewed a project page.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7121
2013-09-25 13:44:36 -07:00
epriestley
79ec5222a5 Reduce Maniphest fulltext limit from PHP_INT_MAX to 10k
Summary:
PHP_INT_MAX is rejected by ElasticSearch since it's outside of the representable integer range (see: <https://gist.github.com/JustinTulloss/c4ac0e1c93d6d1e91744>).

Just use 10K, as matching more than 10K results probably isn't useful to anyone.

Test Plan: Confirmed this fixes the issue in IRC. Ran a fulltext search.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7097
2013-09-24 10:50:56 -07:00
epriestley
e7a7e43104 Fix a bug where policy queries with cursor-based pagers and non-ID orders can go into infinite loops
Summary:
Ref T603. See inlines for an explanation. The case where I hit this was loading the "Pending Differential Revisions" panel in Diffusion when logged out, after making a repository public.

What happens is that we load 10 revisions (say, D1 .. D10) but the user can't see any of them. We then try to load the next 10, but since the pagination is ordered by date modified, we need to base the next query on the modified date of the last thing we loaded (D10). However, since we use the viewer's policies to load that cursor object, it fails to load, and then we just issue the same query over and over again, loading D1 .. D10 until we run out of execution time.

Test Plan: Interface now loads correctly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7059
2013-09-21 16:23:44 -07:00
epriestley
ed126cd47e Provide ApplicationSearch hooks in Maniphest
Summary: Ref T418. Adds hooks to support customized ApplicationSearch (you can't currently add indexable fields without writing custom code).

Test Plan: Wrote custom code to add an indexable field.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T418

Differential Revision: https://secure.phabricator.com/D7002
2013-09-16 16:03:09 -07:00
epriestley
62e66e3cf6 Completely implement cursor paging in Maniphest
Summary: Ref T2625. Fixes user and project paging. Adds visibility-aware project group filtering.

Test Plan: Set page size very small and paged forward and backward in Maniphest, particularly with "Assigned" and "Project" group-by filters.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6973
2013-09-13 11:49:43 -07:00
epriestley
5062abc534 Most implement cursor paging in Maniphest
Summary:
Ref T2625. Depends on D6971. Maniphest is complicated to implement cursor paging for. Builds on D6971 to do so.

This is //almost// complete. Paging on projects and authors doesn't quite work, I'll clean that up shortly. Left some TODOs.

Test Plan: Set page size to `3`, paged forward and backward in a bunch of group/order modes. Results seemed to be as expected.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6972
2013-09-13 11:49:42 -07:00
epriestley
ddfc6bbc9e Service "Group by: Project" in Maniphest out of a local index
Summary:
See discussion in D6955. Currently, the logic for "Group by: Project" is roughly:

  - Load every possible result.
  - Lots of in-process garbage.

Instead, use the new local project name index (from D6957) to service this query more reasonably. Basically:

  - Join a table which has keyed project names.
  - Order by that table.

Test Plan: {F58033}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6958
2013-09-12 13:08:25 -07:00
epriestley
df86f87289 Add a "dateCreated" key to Maniphest
Summary: Depends on D6952. Unpunts there since I'm rolling into a swamp full of schema changes.

Test Plan: Issued date-constrained query and saw key as a candidate.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6954
2013-09-12 13:04:31 -07:00
epriestley
1c43fceffb Add date filtering to Maniphest "pro" search
Summary: Adds date created filtering. There's a task for this somewhere that I can't immediately find.

Test Plan: Filtered tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6952
2013-09-12 13:03:39 -07:00
epriestley
bdef58216e Restore project filtering to Maniphest "pro" search
Summary: Restores any/all/user/exclude project filters to the new search.

Test Plan: Filtered stuff by projects.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6951
2013-09-12 13:03:14 -07:00
epriestley
f679ea7d7e Add "fulltext" to Maniphest pro search
Summary: Restores this field to the new ApplicationSearch-based search.

Test Plan: Used fulltext search to find tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6950
2013-09-12 13:03:05 -07:00
epriestley
a3c6e9aebf Move ManiphestTaskQuery into query/
Summary: Move this into a more consistent location.

Test Plan: Loaded Maniphest.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6949
2013-09-12 13:02:55 -07:00
vrana
6cc196a2e5 Move files in Phabricator one level up
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.

NOTE: `arc diff` timed out so I'm pushing it without review.

Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.

Auditors: epriestley

Maniphest Tasks: T1103
2012-06-01 12:32:44 -07:00