Maniphest search

[jakobw]

Task: https://phabricator.wikimedia.org/T127179

Things this patch includes:

• use maniphest.search instead of `maniphest.query´
• introduce a TaskDataFetcher that replaces what were previously API calls to fetch tasks
• introduce a TaskDataProcessor (anyone got an idea for a better name?) that creates a list of Task objects from the output of TaskDataFetcher
• introduce Task which is a value object for tasks
• remove a bunch of stuff from TaskList and TaskListTest and put it where it belongs (mostly TaskDataProcessor, StatusByWorkboardDispatcher and their tests)
• test some more things
• StatusByStatusFieldDispatcher got a little bit uglier since information whether a task is closed is not included in maniphest.search so it now checks for a list of hardcoded statuses which might be silly

... whew, did I just rewrite Phragile?

These commits should probably be squashed before merging.

[wmde-manicki]

I've just started to look how it works (on local instance) and seems it works fine for new sprints! Might be just my impression but fetching data from Phabricator seems to be slower (it takes some time before graphs are plotted etc). Have you also noticed something similar?

When selecting sprint that has ended (thus snapshot data is being fetch) I am getting the following error message: ErrorException in TaskDataProcessor.php line 24: Undefined index: fields. Or at least my guess it is related to snapshots. I believe this is expected and it is going to be fixed in https://phabricator.wikimedia.org/T127180, right?

I haven't really looked at the code yet.

[jakobw]

Yes, it seemed slower for projects with >100 tasks which I guess makes sense since it now does multiple HTTP requests for those.

Correct, snapshots aren't supposed to work yet since they don't have the JSON structure that the TaskDataProcessor is expecting.

[wmde-manicki]

it is a string, right?

[jakobw]

Yes! Fixed

[wmde-manicki]

So I had a look at the code and:

• I really like Task class being introduced, and moving out to separate classes stuff that is doing the magic related to processing data from Phabricator,
• found couple of small things that @jakobw already managed to fix before I even finished reviewing the patch!
• I agree that having "closed statuses" hard-coded in StatusByStatusFieldDispatcher is no good but @jakobw has already idea how to make it clean and generic (see https://phabricator.wikimedia.org/T127433). I am not sure if this must be included in this patch, or added later. I'd be fine if it is changed later,
• having looked at the patch I started to think that maybe some parts of the code dealing with Phabricator could be moved out from the Sprint class. But this is definitely beyond the scope here.

So basically I'd consider this PR as ready to merge if @jakobw didn't mention cleaning up the commit list a bit. At least "Fix foo" commits could be probably joined with relevant commits. Then I'd be happy to merge. Very nice job!

[jakobw]

@wmde-manicki Squashed the commits!

I also think it's fine to fix the status issue later and I agree that some of the stuff in the Sprint class should probably go elsewhere, especially since it's pretty redundant with some of the code in SprintDataFactory.