Skip to content

Handle dismissed GitHub review events correctly

Administrator requested to merge campaigns/github-dismissed into master

Created by: mrnugget

This fixes #9010.

Previously we wrongly deleted the last review by the same author if we found a "ReviewDismissed" event and, doubly wrong, we also deleted the review when we found a review in state "dismissed".

What GitHub does when a review is dismissed is to:

  • update the original review and set its state to "dismissed"
  • emit an "review dismissed" event

Attention: The downside of this fix is that we don't update the burndown chart when a single "review dismissed" event comes in via webhooks. We need to wait until the original "review" event has been updated.

But I think (!) that GitHub also sends an updated "review" event via webhooks with state "dismissed"

Update

GitHub only sends an updated "review" event via webhook. It doesn't send the "review dismissed" event. That's a GraphQL timeline event.

so the fix here is correct.

Update 2

Together with @eseliger I found out that GitHub dismissed all previous reviews by an author if one of the reviews was dismissed. It still shows the "undismissed" reviews in the timeline, but removes the author from the overall "review state", in other words: GitHub doesn't fall back to the previous review.

Merge request reports