Skip to content
Snippets Groups Projects

Disable "Remove source branch" in MR Widget for users who can't remove, and re-add checkbox to MR form

What does this MR do?

This MR does three things:

  1. Re-adds the "Remove source branch" checkbox to the Create/Edit MR page
  2. Ensures the "Remove source branch" checkbox in the MR widget is disabled for users who do not have rights to delete the source branch
  3. Makes the "Remove source branch" checkbox in the MR Widget dependent on initial MR form settings, rather than defaulting to removing.

Closes #32447 (closed)

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/32907

Edited by Luke Bennett

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
  • Just FYI, looks like there are a few build failures I still need to fix up. I'll ping you for final review when they're :green_apple:.

    I was already reviewing meanwhile :P - Thanks, LGTM, I've left just one comment.

  • Bryce Johnson added 2 commits

    added 2 commits

    Compare with previous version

  • Bryce Johnson added 2 commits

    added 2 commits

    • 21fe8f36 - Shush rubocop.
    • ff5044d8 - Update spinach specs to explicitly check Remove Source Branch.

    Compare with previous version

  • Bryce Johnson added 1 commit

    added 1 commit

    • 92d290c9 - Set merge params to force_remove_source_branch by default.

    Compare with previous version

  • Bryce Johnson added 1 commit

    added 1 commit

    • 0eaa65d5 - Add spec for disabled Remove source branch checkbox.

    Compare with previous version

  • assigned to @oswaldo

  • Oswaldo Ferreira
    Oswaldo Ferreira @oswaldo started a thread on commit 0eaa65d5
  • 63 63 end
    64 64 end
    65 65
    66 context 'user can merge into source project but cannot push to fork' do
  • Oswaldo Ferreira
    Oswaldo Ferreira @oswaldo started a thread on commit 0eaa65d5
  • 63 63 end
    64 64 end
    65 65
    66 context 'user can merge into source project but cannot push to fork' do
    67 given(:user2) { create(:user) }
    68
    69 background do
    70 project.team << [user2, :master]
    71 logout
    72 login_as user2
    73 visit_merge_request(merge_request)
    74 end
    75
    76 scenario 'user cannot remove source branch', js: true do
  • Thanks for adding this test @brycepj, I've left 2 questions. Let me know if you need any help on the test setup.

  • Bryce Johnson unmarked as a Work In Progress

    unmarked as a Work In Progress

  • Bryce Johnson changed title from WIP: Resolve "'Remove source branch' is displayed to mergers who cannot actually delete the source branch from a forked project" to Disable "Remove source branch" in MR Widget for users who can't remove, and re-add checkbox to MR form

    changed title from WIP: Resolve "'Remove source branch' is displayed to mergers who cannot actually delete the source branch from a forked project" to Disable "Remove source branch" in MR Widget for users who can't remove, and re-add checkbox to MR form

  • mentioned in issue #32885 (closed)

  • Bryce Johnson marked the checklist item Get the build passing as completed

    marked the checklist item Get the build passing as completed

  • Bryce Johnson marked the checklist item Ensure this works in EE, open an MR there if neccessary as completed

    marked the checklist item Ensure this works in EE, open an MR there if neccessary as completed

  • Bryce Johnson marked the checklist item Solicit review from @fatihacet and @oswaldo as completed

    marked the checklist item Solicit review from @fatihacet and @oswaldo as completed

  • Bryce Johnson marked the checklist item Get the build passing as incomplete

    marked the checklist item Get the build passing as incomplete

  • Bryce Johnson added 1 commit

    added 1 commit

    • 41e0029c - Move spec and add one for merge button not disabled.

    Compare with previous version

  • Bryce Johnson added 1 commit

    added 1 commit

    • 7e1e1389 - Remove spec from created_from_fork_spec.

    Compare with previous version

  • Bryce Johnson resolved all discussions

    resolved all discussions

  • Author Contributor

    @oswaldo I just pushed a couple commits that address your feedback -- can you take a look at the diff?

  • Bryce Johnson changed the description

    changed the description

  • Bryce Johnson changed the description

    changed the description

  • Bryce Johnson mentioned in commit 0eaa65d5

    mentioned in commit 0eaa65d5

  • assigned to @oswaldo

  • Oswaldo Ferreira
  • Oswaldo Ferreira resolved all discussions

    resolved all discussions

  • Bryce Johnson added 1 commit

    added 1 commit

    • 9b479f43 - Set up merge request from fork more simply.

    Compare with previous version

  • Bryce Johnson resolved all discussions

    resolved all discussions

  • Author Contributor

    @fatihacet I've addressed all of @oswaldo's feedback -- assigning back to you :star:

  • Bryce Johnson changed the description

    changed the description

  • Bryce Johnson resolved all discussions

    resolved all discussions

  • mentioned in issue #32823 (closed)

  • added 1 commit

    • 33923a49 - Expose remove_source_branch boolean/null

    Compare with previous version

  • mentioned in issue #32907 (closed)

  • Luke Bennett changed the description

    changed the description

  • Bryce Johnson added 3 commits

    added 3 commits

    • 8d2a1591 - Update shouldRemoveSourceBranch against API entity changes.
    • 8b746c22 - Merge branch '32447-remove-source-branch-is-displayed-to-mergers-who-cannot-actu…
    • 08a7073e - Fallback to false when remove_source_branch is null.

    Compare with previous version

  • Bryce Johnson resolved all discussions

    resolved all discussions

  • Author Contributor

    @oswaldo looks like they are :frowning2:

  • Author Contributor

    @oswaldo was a quick one -- fixed 099a9e01

  • Bryce Johnson added 1 commit

    added 1 commit

    • 099a9e01 - Update MWPS feature spec per change to remove source branch default.

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading