Skip to content

Respect merge request block validations

Nick Thomas requested to merge (removed):13376-fix-mr-block-validation-checks into master

What does this MR do?

Using Gitlab::Database.bulk_insert bypasses rails-level validations, which are critical for merge request blocks, so replace the super-efficient insert with a looped insert instead.

It's less efficient, but not too inefficient - the total number of configured blocks will typically be in single digits, rarely in double digits. The change is done transactionally, so the multiple INSERT statements will be executed efficiently.

Since the validity of block N is dependent on the presence of block N-1, I think this is really the best way to do this.

As more complex merge order dependencies start to be supported, the list of restrictions becomes less comprehensive. At some point, we may be able to express them all at database level, but that point is not now.

Adding blocks, just like adding related issues, is "best-effort". If a requested block is invalid, we silently don't add it.

I don't think we need to tackle bad data added to the database. This is a regression in %12.2 only, which isn't released yet, so the scope is only GitLab.com. An invalid block will prevent the MR from being merged, but won't cause any other problems, since we don't try to traverse the tree of blocks at any point. The user can simply remove any invalid block they've happened to create to unblock themselves.

Does this MR meet the acceptance criteria?

Conformity

The feature, and this fix, both go into %12.2 (.0) which will be released on the 22nd, so no need for a changelog entry.

Implements #13376 (closed)

Edited by 🤖 GitLab Bot 🤖

Merge request reports