What is the purpose of the merge_request.review.comments GitLab hook config option?

[zecakeh]

Currently it's not displayed in the widget, and it doesn't actually do anything, but there is some code around it.

The purpose of the current code seems to be to only send new comments if there is an approval change in the same batch as a comment. That would mean that the user can chose if they only want to see reviews with approvals or all comments. It just lacks the last step to actually skip those comments. See this line of code that sets skip, but skip is never used after:

matrix-hookshot/src/Connections/GitlabRepo.ts

Line 914 in 2763801

skip: this.hookFilter.shouldSkip('merge_request.review.comments'),

The issue I see with this is that the wording of the widget says "Completed review". However, it is possible to make a review without changing the approval of the MR. So either the wording needs to be fixed to something like "Approved", or the code needs to be changed to actually detect if a review took place.

The former is simple and fixing the code to use what's in place should be easy.

The latter can be done with the data received by the webhook. With GitLab's interface, outside of approvals, a review is only possible if there is at least one comment on the code that goes with it so we just need to check if there is one of those in the batch. They can be identified with different fields: they have a position or a line_id, or their type is DiffNote.