Fix broken todo GraphQL API filtering when filtering by type
What does this MR do?
Graphql todo fetching was not filtering properly by a list of target_type. The TodoFinder support for an array of types was broken. The finder would always return all types.
The spec was working because there was no data that should have been filtered out by the given example in both the graphql resolver and the TodoFinder. Adding an Design todo to the example caused it to fail:
Failures:
1) TodosFinder#execute #execute filtering returns correct todos when filtered by a group
Failure/Error: expect(todos).to match_array([todo1, todo2])
expected collection contained: [#<Todo id: 1, user_id: 1, project_id: 1, target_id: 1, target_type: "Issue", author_id: 2, action: 1...ated_at: "2020-06-18 11:08:19", note_id: nil, commit_id: nil, group_id: 1, resolved_by_action: nil>]
actual collection contained: [#<Todo id: 1, user_id: 1, project_id: 1, target_id: 1, target_type: "Issue", author_id: 2, action: 1...ated_at: "2020-06-18 11:08:19", note_id: nil, commit_id: nil, group_id: 1, resolved_by_action: nil>]
the extra elements were: [#<Todo id: 3, user_id: 1, project_id: 3, target_id: 1, target_type: "DesignManagement::Design", auth...ated_at: "2020-06-18 11:08:19", note_id: nil, commit_id: nil, group_id: 1, resolved_by_action: nil>]
The todo finder now properly supports passing either a single target_type or an array.
See example query:
{
currentUser {
todos(type: [MERGEREQUEST, ISSUE]) {
nodes {
id,
targetType
}
}
}
}
Explain plans
For a user with 62141 todos:
explain SELECT "todos".*
FROM "todos"
WHERE "todos"."user_id" = 2503633
AND ("todos"."state" IN ('pending'))
AND "todos"."target_type" = 'Issue'
ORDER BY "todos"."id" DESC
LIMIT 100
Time: 0.558 ms
- planning: 0.242 ms
- execution: 0.316 ms
- I/O read: 0.000 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 103 (~824.00 KiB) from the buffer pool
- reads: 0 from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0
mit (cost=0.43..212.04 rows=100 width=106)
-> Index Scan Backward using index_todos_on_user_id_and_id_pending on todos (cost=0.43..17303.61 rows=8177 width=106)
Index Cond: (user_id = 2503633)
Filter: ((target_type)::text = ''::text)
Limit (cost=0.43..355.30 rows=100 width=106) (actual time=0.039..0.277 rows=100 loops=1)
Buffers: shared hit=103
-> Index Scan using index_todos_on_user_id_and_id_pending on public.todos (cost=0.43..17303.61 rows=4876 width=106) (actual time=0.037..0.261 rows=100 loops=1)
Index Cond: (todos.user_id = 2503633)
Filter: ((todos.target_type)::text = 'Issue'::text)
Rows Removed by Filter: 0
Buffers: shared hit=103
Screenshots
description | screenshot |
---|---|
todo ui | ![]() |
qraphql explorer | ![]() |
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry - [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides - [-] Database guides
- [-] Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. - [-] Tested in all supported browsers
- [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
- [-] Label as security and @ mention
@gitlab-com/gl-security/appsec
- [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
- [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Allison Browne