To-Do counts might be off due to the inclusion of banned users

We sometimes have cases of "Phantom" To-Dos, where the todo count in the sidebar is higher than the actual todos in the list:

sidebar to-do list
image image__1_

Root cause

The background is that this method: https://gitlab.com/gitlab-org/gitlab/-/blob/520679cf7d430130c9363ea3e1d07670a90b511a/app/models/todo.rb#L233-245 is considering todos from banned users, when it should ignore them.

Every time a new todo is created the method is called via UpdateTodoCountCacheService, setting the Rails cache to a count that includes todos from banned users. As far as I can tell, other counting methods do filter out the banned todos. Especially the fallback for todos_done_count in the user model and graphql.

What makes this hard to reproduce: Marking a todo as done in the list => resets the cache and repopulates it via todos_count correctly. Creating new todos via the todo_service uses the updating method. See above.

Proposed fix:

  1. Make count_grouped_by_user_id_and_state function filter out banned users. This will make sure that the count match what's in the list: https://gitlab.com/gitlab-org/gitlab/-/blob/520679cf7d430130c9363ea3e1d07670a90b511a/app/models/todo.rb#L233-244
  2. Make todos_done_count and todos_pending_count in the user model use Todo.for_user(id).count_grouped_by_user_id_and_state, this will ensure that the UpdateTodoCountCacheService and the fallback in case the cache had been evicted use the same code: https://gitlab.com/gitlab-org/gitlab/-/blob/520679cf7d430130c9363ea3e1d07670a90b511a/app/models/user.rb#L2146-2156