Skip to content

Stop polling when checking task lists on an issue

What does this MR do and why?

This fixes a race condition when something like this happens:

  1. Polling request starts (DB query returns unchecked task)
  2. User checks a task
  3. Polling returns the stale response which reverses the user's action
  4. User tries to check the task again. This now fails with a 409 because it's sending a "check" request for a task list that's already checked on the backend.

This is easier to understand with the screen recordings below.

Screenshots or screen recordings

Before After
Screen_Recording_2021-10-20_at_12.11.23_AM Screen_Recording_2021-10-20_at_12.08.22_AM
  • Before this MR, you can also see the description flashing multiple times as realtime_changes updates the description.

  • This is easier to reproduce with this patch:

    diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
    index e1e662a1968..876619277be 100644
    --- a/app/controllers/concerns/issuable_actions.rb
    +++ b/app/controllers/concerns/issuable_actions.rb
    @@ -86,6 +86,8 @@ def realtime_changes
           response[:updated_by_path] = user_path(issuable.last_edited_by)
         end
    
    +    sleep 2
    +
         render json: response
       end

    But even without that change, it's not that hard to reproduce.

  • Tip to reproduce: Just check / uncheck the tasks sequentially until you notice the poller reversed your previous change. Then click on the task that was reversed to see the error banner.

  • You can't test this on MRs because MRs don't poll for description changes.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Heinrich Lee Yu

Merge request reports

Loading