Consider omitting users/groups for regular approval rules when finalizing approval state

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Work on this issue
• Close this issue

The following discussion from !17466 (merged) should be addressed:

• @stanhu started a discussion: (+2 comments)

  I'm not completely sure I understand why we need to do this. If you have an any_approval rule, this could potentially create hundreds of rows for every merge request. Do we want to limit this to regular approval rules (in another MR)?

Igor: actually for any_approver rule users and groups are empty, so it doesn't create any rows. but since we're running this code only when merge_request.approval_rules.regular.exists?, then it probably makes sense to limit it only to regular rules. @lulalala wdyt?

Mark: This code is to keep a finalized state so future group changes will not affect past approval state. It's only for log keeping. In that sense I would prefer doing it to all types of rules except not any_approver. But there is no perceived concern yet if we don't do log keeping like this, I did this as a safety net. So it's open for debate if performance is a concern.