Skip to content

[WIP] Create merge request when editing on web UI v2

Ramya Authappan requested to merge github/fork/cirosantilli/edit-mr-v2 into master

Created by: cirosantilli

Fixes ACCEPTING MR at: http://feedback.gitlab.com/forums/176466-general/suggestions/5350102-option-to-make-a-merge-request-when-editing-via-we

Also fixes the bugs:

And superseeds: https://github.com/gitlabhq/gitlabhq/pull/8056

Dependencies:

TODO on future MR: Allow MR creation from new file.

Blob edit page UI changes

If user has push permission on the current branch, when the page loads it looks like:

screenshot from 2014-10-15 21 20 53 user has permission initial

If user clicks on "Create merge request", more options appear:

screenshot from 2014-10-15 21 21 24 user has permission expand

By default, branch name is a free branch name on both repositories. When user toggles "On my fork", and he has not yet modified it, it toggles the branch name to a good branch name on given repo:

screenshot from 2014-10-15 21 21 42 toggle on my fork before edit

toggle again:

screenshot from 2014-10-15 21 22 06 toggle again

If user edits the branch name to a new one, the toggle stops. After editing and toggle:

screenshot from 2014-10-15 21 22 24 edit and toggle

User does not have push permission, he must fork. "Create merge request" and "On my fork" are checked and disabled:

screenshot from 2014-10-15 21 23 22 user does not have push permission on current branch

If user is already on his fork, he does not see "On my fork":

screenshot from 2014-10-15 21 24 20 user is on his own fork already

The merge request would be generated between the current repo and itself.

There is also a minor UI behavior change at blob#show: the edit button is enabled even if you don't have push permission since you can edit to start a merge request:

screenshot from 2014-10-15 21 45 23 blob show edit

Are there points in the code the reviewer needs to double check?

The hard thing of this MR was the testing.

There are two problems.

GitLab test port

The hardest part is that it is not possible to run the test server on a port: http://stackoverflow.com/questions/6807268/rails-port-of-testing-environment

The problem is that GitLab shell needs to make an API call to /internal/allowed after the push hooks to check if the push is OK, but there is no such port.

On the current testing environment, things work because of a workaround: spec/factories/projects.rb does an explicit repository copy without setting up the hooks.

But now this is not possible anymore, because I'm cloning and modifying the clone on a single test!

This is also the cause of: https://github.com/gitlabhq/gitlabhq/commit/71fce0b2

I have considered tons of alternatives, and the best I found was: start a mock server on port 3001 which always returns true to gitlab shell.

This is a good option because:

  • it is very little to mock, only a single place with constant simple response. Mocking into GitLab would be much harder.
  • it is the mock point which tests the most of the system since the limit is not being able to serve gitlab at the port
  • does not cause any behavior change since controllers currently deal with all web UI permission questions, and direct HTTP pushes are not tested.

Downsides:

  • it takes up a port
  • it will cause current developer's test environment to break until they update config/gitlab.yml and set test -> gitlab -> port to something accessible (the current value 80 will fail because requires root)

However:

  • I have added a clear error message that should allow any dev to quickly find what is going wrong
  • if someday someone is able to run gitlab tests on a fixed port (the perfect solution), they will need an accessible port anyways. So let's do it now and be done with it.

git hooks PATH prepending

Cause described in detail at: https://github.com/gitlabhq/gitlabhq/issues/8045

Merge request reports