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:
- Re-adds the "Remove source branch" checkbox to the Create/Edit MR page
- Ensures the "Remove source branch" checkbox in the MR widget is disabled for users who do not have rights to delete the source branch
- Makes the "Remove source branch" checkbox in the MR Widget dependent on initial MR form settings, rather than defaulting to removing.
Closes #32447 (closed)
Merge request reports
Activity
added 1 commit
- 53919b60 - Disable removeSourceBranch button when user doesn't have permission.
@oswaldo I need to re-add the "Remove source branch" button to the Create/Edit MR page to fix #32447 (closed). Do you what's required on the server side for that?
added 1 commit
- e09ad639 - Use should_remove_source_branch to set initial value.
added 1 commit
- 3adda594 - Check for force_remove_source_branch in mr widget store.
assigned to @brycepj
changed milestone to %9.2
assigned to @ClemMakesApps
assigned to @jivanvl
I spoke with @jivanvl and we are not really sure if this MR causes any other side effects/is the best solution for this problem.
In addition to that, we are also not sure we can finish up this MR before we do our %9.2 release since @selfup is already tagging the last RC. As a result, we think it would be best for you to collaborate with @fatihacet so that we can get this into 9.2.1
Edited by Clement Hoassigned to @brycepj
@ClemMakesApps I had similar reservations. I agree, best to play it safe here.
@fatihacet can you help?
added 89 commits
-
dbe92439...18a6d9c5 - 88 commits from branch
master
- a25fb529 - Upgrade Remove Source Branch checkbox UX.
-
dbe92439...18a6d9c5 - 88 commits from branch
@ClemMakesApps it makes sense this to go with 9.2.1. What help do you need?
@fatihacet I was just about to ask if you could review this. I think that's the only 'help' I'll need.
assigned to @fatihacet
@fatihacet @oswaldo 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
.- Resolved by Bryce Johnson
assigned to @brycepj
added 2 commits
added 2 commits
added 1 commit
- 92d290c9 - Set merge params to force_remove_source_branch by default.
added 1 commit
- 0eaa65d5 - Add spec for disabled Remove source branch checkbox.
assigned to @oswaldo
63 63 end 64 64 end 65 65 66 context 'user can merge into source project but cannot push to fork' do 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 Can we have a new scenario like
user can merge the merge request
, by checking we can see a non-disabledMerge
button?Edited by Oswaldo FerreiraDiscussion moved here and resolved
Thanks for adding this test @brycepj, I've left 2 questions. Let me know if you need any help on the test setup.
assigned to @brycepj
mentioned in issue #32885 (closed)
- Resolved by Bryce Johnson
- Resolved by Bryce Johnson
marked the checklist item Solicit review from @fatihacet and @oswaldo as completed
added 1 commit
- 41e0029c - Move spec and add one for merge button not disabled.
mentioned in commit 0eaa65d5
assigned to @oswaldo
- Resolved by Oswaldo Ferreira
- Resolved by Bryce Johnson
assigned to @brycepj
@fatihacet I've addressed all of @oswaldo's feedback -- assigning back to you
assigned to @fatihacet
mentioned in issue #32823 (closed)
added Next Patch Release label
- Resolved by Bryce Johnson
mentioned in issue #32907 (closed)
@brycepj Could you check if those are legit please? https://gitlab.com/gitlab-org/gitlab-ce/builds/17193135
@oswaldo looks like they are
added 1 commit
- 099a9e01 - Update MWPS feature spec per change to remove source branch default.