Skip to content
Snippets Groups Projects

Disallow reopening of locked merge request

Merged Jan Beckmann requested to merge kingjan1999/gitlab-ce:56864-reopen-locked-mr into master
All threads resolved!

What does this MR do?

It prevents reopening of locked merge requests. This MR is basically a clone of !21299 (merged) for merge requests.

Note: I did not add specific tests as !21299 (merged) did, because app/policies/merge_request_policy is generally not tested at the moment (as opposed to issues_policy). I can add this if necessary.

What are the relevant issue numbers?

#56864 (closed)

Does this MR meet the acceptance criteria?

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Thanks @kingjan1999. Added some comments to your changes.

    Feel free to let me know if you have any questions.

  • Jan Beckmann added 1 commit

    added 1 commit

    Compare with previous version

  • Thanks @kingjan1999, changes look good to me. Just added a docs suggestion.

    I think we need to add some specs though. Specially for cases like the user is the author or assignee, since we added enable for the author we need to verify it's superseded by the prevent with the locked condition.

    The hierarchy of rules in the policy isn't explicit so it's better to have a test.

  • Contributor

    @kingjan1999 not sure if you can complete this for the 11.9 release?

  • Ray Paik changed milestone to %11.9

    changed milestone to %11.9

  • Author Contributor

    Sorry for the delay, I'll tackle this by the weekend at the latest.

  • Jan Beckmann added 1732 commits

    added 1732 commits

    Compare with previous version

  • Author Contributor

    @engwan I've added some tests with b313f659, can you give me a short feedback whether this is sufficient (I feel like it isn't) or what specs to add?

  • Heinrich Lee Yu resolved all discussions

    resolved all discussions

  • Heinrich Lee Yu
  • Heinrich Lee Yu
  • Thanks @kingjan1999. Added a few comments above.

  • Jan Beckmann added 1 commit

    added 1 commit

    Compare with previous version

  • Jan Beckmann resolved all discussions

    resolved all discussions

  • Author Contributor

    Thank you very much @engwan! I've implemented your suggested test structure with a6646f95

  • Thanks @kingjan1999. Looks good to me.

    @smcgivern Can you review / merge? I think we can skip frontend review here since it's a very minor change in the HAML?

  • Thanks @kingjan1999, LGTM. Unfortunately this missed the feature freeze for 11.9 by a small amount, so it will go into 11.10.

  • Sean McGivern changed milestone to %11.10

    changed milestone to %11.10

  • Sean McGivern approved this merge request

    approved this merge request

  • merged

  • Sean McGivern mentioned in commit e61cce36

    mentioned in commit e61cce36

  • Please register or sign in to reply
    Loading