Skip to content

Support getting a todo for an alert in graphql api

Allison Browne requested to merge graphql-get-alert-todo into master

What does this MR do?

Adds a todos field for alerts in graphql, which can be used to obtain pending todos for the current user on the alert.

Uses the TodoResolver, so accepts the same parameters as user todos (action, state, author_id, etc; type argument is ignored).

Sample request:

{
  project(fullPath: "root/autodevops") {
    id
    fullPath
    alertManagementAlerts(iid: "277") {
      nodes {
        iid
        status
	title
        todos {
          id
          targetType
          body
        }
      }
    }
  }
}

Alternate approaches considered:

  • Adding support for a target_id as part of the TodoResolver. I ruled this out as the FE is only aware of the iid for an alert, and fetching the id from either the TodoResolver or TodosFinder by using the iid is on the messy side, as there are several target_types for Todos. Not all have iids, and there isn't a consistent API across these classes.
  • Exposing a todos field on the alerts (rather than the singular todo) which would include pending and done todos. I opted against this mostly for performance - filtering on the FE could be kind of a bummer here.
  • Preloading to avoid N+1 queries. This field is intended to be used on the FE for a single alert - not the whole batch. I'm definitely open to improving this in future, but I don't think it needs to happen now.
  • Separate resolver for a singular pending_todo field on alerts, using the PendingTodosFinder.

Database overview

Example query:

SELECT todos.* FROM todos 
WHERE todos.user_id = 3163848 
AND (todos.state IN ('pending')) 
AND todos.target_id = 277 
AND todos.target_type = 'AlertManagement::Alert' 
ORDER BY todos.id DESC 
LIMIT 100

Query plan visualization: https://explain.depesz.com/s/hNyL

Query plan:

Limit  (cost=3.25..3.26 rows=1 width=106) (actual time=0.038..0.039 rows=1 loops=1)
  Buffers: shared hit=5
  ->  Sort  (cost=3.25..3.26 rows=1 width=106) (actual time=0.038..0.038 rows=1 loops=1)
        Sort Key: id DESC
        Sort Method: quicksort  Memory: 25kB
        Buffers: shared hit=5
        ->  Index Scan using index_todos_on_target_type_and_target_id on todos  (cost=0.56..3.24 rows=1 width=106) (actual time=0.032..0.033 rows=1 loops=1)
              Index Cond: (((target_type)::text = 'AlertManagement::Alert'::text) AND (target_id = 278))
              Filter: ((user_id = 3163848) AND ((state)::text = 'pending'::text))
              Buffers: shared hit=5
Planning Time: 0.189 ms
Execution Time: 0.060 ms

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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 Sarah Yasonik

Merge request reports