findInboxTopLevel() returns tasks without a parent (neither a
parent task nor a project), as expected.
findTopLevel() was doing exactly the same, and isn't used anywhere
except in unittests, currently.
But I'm going to use it for an "All Tasks" page, where we do want tasks
that belong to a project, but where child tasks (of other tasks)
shouldn't be returned since they'll be visible under their parent task.
Port most unittests to findInboxTopLevel so they don't require a cache.
test passes. Also tested in the app together with a WIP commit
for adding an "All Tasks" page.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
Makes me wonder if we shouldn't also fix findInboxTopLevel() indeed your improved findTopLevel() makes a better job at verifying the related-to field doesn't contain a broken reference to a parent... which the inbox doesn't (and should). Would be a separate patch I guess.
| src/akonadi/akonaditaskqueries.cpp | ||
|---|---|---|
| 111 | Why is it needed, we don't do it anywhere else AFAIK | |
| 114–135 | Are you sure of this comment? Because AFAICT you (rightfully) reject tasks whose parents are tasks. IIUC the intent, you should indeed keep either tasks with no parents or tasks with a project as parent. | |
| 118 | I guess because of this, but then why not do a fetch through the storage? This way it is transparent if there's a cache behind it or not (with different performances obviously). Because of the sync vs async? Maybe time to allow an exec() for once? Just thinking aloud there. | |
| src/akonadi/akonaditaskqueries.cpp | ||
|---|---|---|
| 111 | We do, see below (your code, https://phabricator.kde.org/D6922) | |
| 114–135 | You're right, I got this the wrong way around. | |
| 118 | See your own impl further below. IMHO requiring a cache is the right thing to do. It's fast, it's sync, it's exactly what we need to be able to implement such things. I see no benefit in going through the storage here. Without a cache, that code would be horribly slow, and the async API requires awful hacks like exec. | |
| 184 | We do require m_cache here as well. Just the assert is missing, but IMHO we should add it, it's slightly easier to debug an assert than a null pointer crash. | |
| src/akonadi/akonaditaskqueries.cpp | ||
|---|---|---|
| 118 | Fair enough | |