Skip to content

Backend for blocking merge requests MVC

Nick Thomas requested to merge 9688-mr-merge-order into master

What does this MR do?

Introduces the idea of a merge request block. An MR blocks another MR, or can be blocked by another MR.

In this MVC, An MR that blocks another MR cannot itself be blocked, and vice-versa. This simple rule prevents the formation of long chains of blocking MRs; we can add this functionality in a follow-up.

This MR doesn't include any frontend functionality, and that includes API endpoints. Blocks can be created or removed in the rails console to verify functionality. If a block exists, it will be respected by the merge service.

CE backport: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/27323

All EE-specific schema changes are now to be in CE as well, as part of the unification effort, but the models, etc, remain EE-only.

Does this MR meet the acceptance criteria?

Conformity

Performance and testing

  • An MR should not be allowed to block itself
  • No N+1 when looking up a list of MRs
  • No N+1 when merging an MR
  • Batch merge isn't a thing (yet), so we don't need to worry about that
  • Ensure blocks are respected and cannot be bypassed under any circumstances
  • [-] Once a block has been obsoleted by the merge of an MR, its database row should be removed to avoid bloat
  • Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
  • Licensed feature should be respected. Blocks should be invisible and not enforced when the feature is unavailable, but we can still remove them from the DB when an MR that is party to them is removed

Security

  • The existence of confidential MRs should not be disclosed (this is probably not an issue in this MR, but will need to be considered at the API level. Blocking approval rules + private groups is an instructive example)
  • Who can create or remove a block? Just maintainers, or should the author of an MR be able to state it is blocked by another MR? How about other developers who can touch on both MRs that are parties to the block?

Related to #9688 (closed)

Edited by 🤖 GitLab Bot 🤖

Merge request reports