Skip to content

Duplicate todo is created for already mentioned user

Summary

Duplicate Todos are created on updating an issue's description for its mentioned users.

Initially reported in Slack internal link.

Workaround

To stop any spam caused by the bug, please remove either the trailing or leading HTML comment from your issue's description.

- <!-- some comment at the head -->
Some description content
- <!-- some comment at the tail -->

Steps to reproduce

  1. Enable the FF :multiple_todos

  2. Create an issue with a user mention in the description that starts and ends with <!-- -->.

<!-- foobar -->

@root 

- [ ] Hello!

<!-- foobar -->
  1. Toggle on and off the task and observe the multiple todos.

Relevant logs and/or screenshots

demo

Possible fixes

Why and how does the bug occur?

!110805 (merged) introduced a change that ignores a commented out tasklist and the introduced regex in the MR likely had a bug. When a tasklist falls in-between two HTML comments, the task is ignored by Taskable.get_tasks:

<!-- some comment at the head -->
- [ ] This task is ignored
<!-- some comment at the tail -->

Now consider the following user interaction and subsequent backend update:

  1. User toggles the task sandwiched between HTML comments and an issue update is triggered.

  2. While updating the issue, TodoService should skip execution when the user is toggling the task but the task's been ignored and toggling_tasks? returns false:

# https://gitlab.com/gitlab-org/gitlab/-/blob/14760f5ae467b9456ff1e5a6749e88d67a977a37/app/services/todo_service.rb#L302

  def update_issuable(issuable, author, skip_users = [])
    # Skip toggling a task list item in a description
    return if toggling_tasks?(issuable)

    create_mention_todos(issuable.project, issuable, author, nil, skip_users)
  end

toggling_tasks? is checking issuable.tasks?:

# https://gitlab.com/gitlab-org/gitlab/-/blob/5e3c334116178eec5f50fc5fee2ec0b3841a2504/app/models/concerns/taskable.rb#L75

  # Return true if this object's description has any task list items.
  def tasks?
    tasks.summary.items?
  end

Since get_tasks fails to pick up - [ ] This task is ignored, an empty array is returned.

  def self.get_tasks(content)
    items = []

    content.to_s.scan(REGEX) do
      next unless $~[:task_item]

      $~[:task_item].scan(ITEM_PATTERN) do |prefix, checkbox, label|
        items << TaskList::Item.new("#{prefix.strip} #{checkbox}", label.strip)
      end
    end

    items # an empty array
  end

Subsequently toggling_tasks? returns false and TodoService#update_issuable is executed creating a new mention.

Edited by euko