diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 38f77a65b3015cb4dc42eebe91514e49b47b8597..7ec1d6db40877765247db18e7f9a4e36a0def4ad 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -2.0.1 +2.1.0 diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 00b52758fa8888726a593828d69ab8fe75d0eeac..f50dcdb64f9e74371c273ee9d133bf60e670deaf 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -79,6 +79,8 @@ class Dispatcher # Ensure we don't create a particular shortcut handler here. This is # already created, where the network graph is created. shortcut_handler = true + when 'projects:edit_tree:show', 'projects:edit_tree:update' + new EditTreeView() switch path.first() when 'admin' then new Admin() diff --git a/app/assets/javascripts/edit_tree.js.coffee b/app/assets/javascripts/edit_tree.js.coffee new file mode 100644 index 0000000000000000000000000000000000000000..e9746ffe5e5fe2cecb836004e53e2f40b90c4366 --- /dev/null +++ b/app/assets/javascripts/edit_tree.js.coffee @@ -0,0 +1,27 @@ +class EditTreeView + constructor: -> + new_branch_name = $('#new_branch_name') + new_branch_name_group = $('#new_branch_name_group') + create_merge_request = $('#create_merge_request') + on_my_fork = $('#on_my_fork') + on_change_create_mr = -> + if create_merge_request.prop('checked') + new_branch_name_group.show() + new_branch_name.prop('disabled', false) + else + new_branch_name_group.hide() + new_branch_name.prop('disabled', true) + on_change_on_my_fork = -> + if on_my_fork.prop('checked') + if new_branch_name.val() == gon.new_branch_name_this + new_branch_name.val(gon.new_branch_name_fork) + else + if new_branch_name.val() == gon.new_branch_name_fork + new_branch_name.val(gon.new_branch_name_this) + create_merge_request.on('change', on_change_create_mr) + on_my_fork.on('change', on_change_on_my_fork) + # Run it in case the browser cached the checkbox value and overrode checked. + on_change_create_mr() + on_change_on_my_fork() + +@EditTreeView = EditTreeView diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 7e4580017dd7e6c26373a1b80186f7dd518a71b0..ce0b3d1707697dff8f0fe0861c5fb778a008c155 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -29,4 +29,10 @@ class Projects::ApplicationController < ApplicationController redirect_to project_tree_path(@project, @ref), notice: "This action is not allowed unless you are on top of a branch" end end + + protected + + def can_show_blob_edit? + can?(current_user, :fork_project, @project) + end end diff --git a/app/controllers/projects/blame_controller.rb b/app/controllers/projects/blame_controller.rb index a3c41301676500b053e95472513f99264691b902..7c86c194e19f6f781c5ba1754c28f21b14b87be3 100644 --- a/app/controllers/projects/blame_controller.rb +++ b/app/controllers/projects/blame_controller.rb @@ -8,6 +8,7 @@ class Projects::BlameController < Projects::ApplicationController before_filter :require_non_empty_project def show + @show_blob_edit = can_show_blob_edit? @blob = @repository.blob_at(@commit.id, @path) @blame = Gitlab::Git::Blame.new(project.repository, @commit.id, @path) end diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 7009e3b1bc81e9facc69e9429337753d57f5df56..8436fa9aa7ceb8bd41514bf37626208ff7414323 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -11,6 +11,7 @@ class Projects::BlobController < Projects::ApplicationController before_filter :blob def show + @show_blob_edit = can_show_blob_edit? end def destroy diff --git a/app/controllers/projects/edit_tree_controller.rb b/app/controllers/projects/edit_tree_controller.rb index 8976d7c7be85b45c0851d3437be385955289a518..52d288acf5bfbda9e5bfa6199a4756250ecf4dd1 100644 --- a/app/controllers/projects/edit_tree_controller.rb +++ b/app/controllers/projects/edit_tree_controller.rb @@ -1,28 +1,68 @@ class Projects::EditTreeController < Projects::BaseTreeController + before_filter :authorize_show_blob_edit!, only: [:show] + before_filter :authorize_push!, only: [:preview, :update] before_filter :require_branch_head + before_filter :blob - before_filter :authorize_push! before_filter :from_merge_request before_filter :after_edit_path def show - @last_commit = Gitlab::Git::Commit.last_for_path(@repository, @ref, @path).sha + set_show_vars + params[:content] = @blob.data + params[:on_my_fork] = true if params[:on_my_fork].nil? + params[:new_branch_name] = gon.new_branch_name_this end def update - result = Files::UpdateService. - new(@project, current_user, params, @ref, @path).execute - + set_show_vars + source_project = @project + target_project = @project + source_branch_name = @ref + if params[:create_merge_request] + source_branch_name = params[:new_branch_name] + if params[:on_my_fork] + if current_user.already_forked?(target_project) + source_project = current_user.fork_of(target_project) + else + source_project = Projects::ForkService. + new(target_project, current_user).execute + if source_project.errors.any? + flash[:alert] = source_project.errors.full_messages.first + render :show and return + end + end + else + # No need for error message: Javascript logic makes this impossible. + authorize_push! + end + result = CreateBranchService.new(source_project, current_user). + execute(source_branch_name, @ref) + if result[:status] == :error + flash[:alert] = result[:message] + render :show and return + end + end + result = Files::UpdateService.new(source_project, current_user, params, + source_branch_name, @path).execute if result[:status] == :success - flash[:notice] = "Your changes have been successfully committed" - - if from_merge_request - from_merge_request.reload_code + flash[:notice] = 'Your changes have been successfully committed' + if params[:create_merge_request] + redirect_to new_project_merge_request_path( + source_project, + merge_request: { + source_branch: source_branch_name, + target_project_id: target_project.id + } + ) + else + if from_merge_request + from_merge_request.reload_code + end + redirect_to after_edit_path end - - redirect_to after_edit_path else - flash[:alert] = result[:error] + flash[:alert] = result[:message] render :show end end @@ -57,4 +97,21 @@ class Projects::EditTreeController < Projects::BaseTreeController # If blob edit was initiated from merge request page @from_merge_request ||= MergeRequest.find_by(id: params[:from_merge_request_id]) end + + def set_show_vars + @last_commit = Gitlab::Git::Commit.last_for_path(@repository, @ref, @path).sha + + prefix = 'patch-' + gon.new_branch_name_this = @repository.free_branch_name(prefix) + if current_user.already_forked?(@project) + gon.new_branch_name_fork = current_user.fork_of(@project). + repository.free_branch_name(prefix) + else + gon.new_branch_name_fork = gon.new_branch_name_this + end + end + + def authorize_show_blob_edit! + return access_denied! unless can_show_blob_edit? + end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 93994123a90505cce29bc36a22d10d5906d65767..6ddcca05930f708e1b58a40955119b36e0d00ff8 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -56,11 +56,15 @@ class Repository branches.find { |branch| branch.name == name } end + def has_branch?(name) + find_branch(name) != nil + end + def find_tag(name) tags.find { |tag| tag.name == name } end - def add_branch(branch_name, ref) + def add_branch(branch_name, ref = 'HEAD') Rails.cache.delete(cache_key(:branch_names)) gitlab_shell.add_branch(path_with_namespace, branch_name, ref) @@ -102,6 +106,18 @@ class Repository end end + # Generate a free branch name of form prefix, + # where Number is the smallest unused positive integer. + def free_branch_name(prefix) + n_taken = [] + branch_names.each do |name| + if name =~ /#{prefix}(\d+)/ + n_taken << $1.to_i + end + end + "#{prefix}#{((1..n_taken.length + 1).to_a - n_taken)[0]}" + end + def tag_names Rails.cache.fetch(cache_key(:tag_names)) do raw_repository.tag_names diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml index bdf02c6285de557e1643525123bda7cc20b4cae1..2466773cb164193ecd4bfd927c54bedc73cf6acd 100644 --- a/app/views/projects/blame/show.html.haml +++ b/app/views/projects/blame/show.html.haml @@ -7,7 +7,8 @@ %span.file_name = @path %small= number_to_human_size @blob.size - %span.options= render "projects/blob/actions" + %span.options= render 'projects/blob/actions', + show_blob_edit: @show_blob_edit .file-content.blame.highlight %table - @blame.each do |commit, lines, since| diff --git a/app/views/projects/blob/_actions.html.haml b/app/views/projects/blob/_actions.html.haml index 64c19a57803d162538f1852c6d56a4de94babdb0..b2a9a3f7fb63ffc3d74639926e3b9d1f661a28a3 100644 --- a/app/views/projects/blob/_actions.html.haml +++ b/app/views/projects/blob/_actions.html.haml @@ -1,7 +1,7 @@ .btn-group.tree-btn-group -# only show edit link for text files - if @blob.text? - - if allowed_tree_edit? + - if show_blob_edit = link_to 'Edit', project_edit_tree_path(@project, @id), class: 'btn btn-small' - else diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml index 492dff437b5c118ead3f6a178540e1490b28305e..427564df2b2c1d376190b11112bf71da62e9eb65 100644 --- a/app/views/projects/blob/_blob.html.haml +++ b/app/views/projects/blob/_blob.html.haml @@ -26,7 +26,8 @@ %span.file_name = blob.name %small= number_to_human_size blob.size - %span.options.hidden-xs= render "actions" + %span.options.hidden-xs= render 'actions', + show_blob_edit: @show_blob_edit - if blob.text? = render "text", blob: blob - elsif blob.image? diff --git a/app/views/projects/edit_tree/show.html.haml b/app/views/projects/edit_tree/show.html.haml index 5ccde05063e6f4f5e7b64b44cbe42ccc905b4961..4effbe947a6e79f03647098a77a386bf7cff7a5e 100644 --- a/app/views/projects/edit_tree/show.html.haml +++ b/app/views/projects/edit_tree/show.html.haml @@ -17,12 +17,40 @@ = link_to "Cancel", @after_edit_path, class: "btn btn-tiny btn-cancel", data: { confirm: leave_edit_message } .file-content.code %pre.js-edit-mode-pane#editor + = params[:content] .js-edit-mode-pane#preview.hide .center %h2 %i.fa.fa-spinner.fa-spin = render 'shared/commit_message_container', params: params, placeholder: "Update #{@blob.name}" + - allowed_tree_edit = allowed_tree_edit? + .form-group + .col-sm-offset-2.col-sm-10 + %div{class: ['checkbox', allowed_tree_edit ? '' : 'disabled']} + = label_tag do + = check_box_tag 'create_merge_request', '1', !allowed_tree_edit, + checked: params[:create_merge_request] || !allowed_tree_edit, + disabled: !allowed_tree_edit + Create merge request + #new_branch_name_group{ style: allowed_tree_edit ? 'display:none;' : '' } + - if @project.namespace != current_user.namespace + .form-group + .col-sm-offset-2.col-sm-10 + %div{class: ['checkbox', allowed_tree_edit ? '' : 'disabled']} + = label_tag do + = check_box_tag 'on_my_fork', '1', !allowed_tree_edit, + checked: params[:on_my_fork], disabled: !allowed_tree_edit + Create branch on my fork + - unless allowed_tree_edit + .help-block + %p You must fork because you don't have push permission. + .form-group + = label_tag 'new_branch_name', 'New branch name', + class: 'col-sm-2 control-label' + .col-sm-10 + = text_field_tag 'new_branch_name', params[:new_branch_name], + disabled: !allowed_tree_edit, class: 'form-control' = hidden_field_tag 'last_commit', @last_commit = hidden_field_tag 'content', '', id: "file-content" = hidden_field_tag 'from_merge_request_id', params[:from_merge_request_id] @@ -34,7 +62,6 @@ ace.config.loadModule("ace/ext/searchbox"); var ace_mode = "#{@blob.language.try(:ace_mode)}"; var editor = ace.edit("editor"); - editor.setValue("#{escape_javascript(@blob.data)}"); if (ace_mode) { editor.getSession().setMode('ace/mode/' + ace_mode); } diff --git a/config/application.rb b/config/application.rb index e36df913d0b3309620e692f18e6e623e2ba975f7..76a853186a4cc28c5d6511f8380b3193cadf1504 100644 --- a/config/application.rb +++ b/config/application.rb @@ -97,5 +97,7 @@ module Gitlab redis_config_hash[:namespace] = 'cache:gitlab' config.cache_store = :redis_store, redis_config_hash + + ENV['GITLAB_PATH_OUTSIDE_HOOK'] = ENV['PATH'] end end diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 857643c006e7b801daf449bfc6b8598e9779b2db..c36138cd111570834effd4461e9b505cf7d104a9 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -281,7 +281,7 @@ test: enabled: true gitlab: host: localhost - port: 80 + port: 3001 # When you run tests we clone and setup gitlab-shell # In order to setup it correctly you need to specify diff --git a/features/project/forked_merge_requests.feature b/features/project/forked_merge_requests.feature index d9fbb875c2864aff75947d5585f4e0d28e4a80c0..7442145d87ee4e827e3d15386b40a69c406eb4ca 100644 --- a/features/project/forked_merge_requests.feature +++ b/features/project/forked_merge_requests.feature @@ -11,20 +11,18 @@ Feature: Project Forked Merge Requests And I submit the merge request Then I should see merge request "Merge Request On Forked Project" - # TODO: Improve it so it does not fail randomly - # - #@javascript - #Scenario: I can edit a forked merge request - #Given I visit project "Forked Shop" merge requests page - #And I click link "New Merge Request" - #And I fill out a "Merge Request On Forked Project" merge request - #And I submit the merge request - #And I should see merge request "Merge Request On Forked Project" - #And I click link edit "Merge Request On Forked Project" - #Then I see the edit page prefilled for "Merge Request On Forked Project" - #And I update the merge request title - #And I save the merge request - #Then I should see the edited merge request + @javascript + Scenario: I can edit a forked merge request + Given I visit project "Forked Shop" merge requests page + And I click link "New Merge Request" + And I fill out a "Merge Request On Forked Project" merge request + And I submit the merge request + And I should see merge request "Merge Request On Forked Project" + And I click link edit "Merge Request On Forked Project" + Then I see the edit page prefilled for "Merge Request On Forked Project" + And I update the merge request title + And I save the merge request + Then I should see the edited merge request @javascript Scenario: I cannot submit an invalid merge request diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature index aca255b94441035ce462aa21edfdf7ff1c126723..c74b87cd2025caac5b677c613d4159f21039edf9 100644 --- a/features/project/source/browse_files.feature +++ b/features/project/source/browse_files.feature @@ -40,6 +40,13 @@ Feature: Project Source Browse Files And I click button "Edit" Then I can edit code + @javascript + Scenario: If I can fork the source but not modify it, then I can still click the edit button to make a pull request + Given I am a reporter of project "Reporter" + And I visit ".gitignore" file page of project "Reporter" + And I click button "Edit" + Then I can edit code + @javascript Scenario: I can edit and commit file Given I click on ".gitignore" file in repo diff --git a/features/project/source/merge_request_from_edit.feature b/features/project/source/merge_request_from_edit.feature new file mode 100644 index 0000000000000000000000000000000000000000..56e771939f200154f8aa7205d1bf594de3085719 --- /dev/null +++ b/features/project/source/merge_request_from_edit.feature @@ -0,0 +1,93 @@ +Feature: Project Source Merge Request From Edit + Background: + Given I sign in as a user + And I am a master of project "Shop" + + @javascript + Scenario: I can create a merge request from the current project to itself + Given I visit ".gitignore" file edit page + And I edit code + And I fill the commit message + And I check "Create merge request" + And I uncheck "On my fork" + And I fill the new branch name + And I click on "Commit changes" + Then I should be redirected to the new merge request page from origin to itself + + @javascript @wip + Scenario: I can create a merge request from an existing fork to the original project + Given I have a project forked off of "Shop" called "Forked Shop" + And I visit ".gitignore" file edit page + And I edit code + And I fill the commit message + And I check "Create merge request" + And I fill the new branch name + And I click on "Commit changes" + Then I should be redirected to the new merge request page from origin to fork + + @javascript @wip + Scenario: I can create a merge request from a new fork to the original project + Given I visit ".gitignore" file edit page + And I edit code + And I fill the commit message + And I check "Create merge request" + And I fill the new branch name + And I click on "Commit changes" + Then I should be redirected to the new merge request page from origin to fork + + @javascript + Scenario: If the commit fails because of invalid input, then the editor content is kept + Given I visit ".gitignore" file edit page + And I edit code + And I fill the commit message + And I check "Create merge request" + And I fill the new branch name with an invalid branch name + And I click on "Commit changes" + Then I should be on ".gitignore" file edit page + And I should see the new edited content + + # Form interaction effects + + @javascript + Scenario: If I don't click on "Create merge request", then I don't see the merge request options + Given I visit ".gitignore" file edit page + Then I don't see the "New branch name" input + Given I check "Create merge request" + Then I see the "New branch name" input + + @javascript + Scenario: If I don't have push permission, then I must fork to create a merge request + Given public project "Community" + And I visit ".gitignore" file edit page of project "Community" + Then "Create merge request" is checked and disabled + And "On my fork" is checked and disabled + + @javascript + Scenario: If I am on my namespace, then I cannot create a merge request on a fork + Given I have a project forked off of "Shop" called "Forked Shop" + And I visit ".gitignore" file edit page of project "Forked Shop" + And I check "Create merge request" + Then I don't see the "On my fork" checkbox + + @javascript + Scenario: If I toggle "On my fork" before editing the branch name, it changes to a free branch name on my fork + Given I have a project forked off of "Shop" called "Forked Shop" + And Project "Shop" has a branch named "patch-1" + And I visit ".gitignore" file edit page + And I check "Create merge request" + Then The new branch name is "patch-1" + When I uncheck "On my fork" + Then The new branch name is "patch-2" + When I check "On my fork" + Then The new branch name is "patch-1" + + @javascript + Scenario: If I toggle "On my fork" after editing the branch name, it does not change + Given I have a project forked off of "Shop" called "Forked Shop" + And Project "Shop" has a branch named "patch-1" + And I visit ".gitignore" file edit page + And I check "Create merge request" + And I fill the new branch name with a non-default value + And I uncheck "On my fork" + Then The new branch name is the non-default value + diff --git a/features/steps/project/commits/branches.rb b/features/steps/project/commits/branches.rb index 07f7e5796a3a04ad42e05b8da816e8b5bea0e594..e03f810ec41ec3e4090836e23f79195ad0c518a4 100644 --- a/features/steps/project/commits/branches.rb +++ b/features/steps/project/commits/branches.rb @@ -33,19 +33,19 @@ class Spinach::Features::ProjectCommitsBranches < Spinach::FeatureSteps end step 'I submit new branch form' do - fill_in 'branch_name', with: 'deploy_keys' + fill_in 'branch_name', with: valid_new_branch_name fill_in 'ref', with: 'master' click_button 'Create branch' end step 'I submit new branch form with invalid name' do - fill_in 'branch_name', with: '1.0 stable' + fill_in 'branch_name', with: invalid_branch_name fill_in 'ref', with: 'master' click_button 'Create branch' end step 'I submit new branch form with invalid reference' do - fill_in 'branch_name', with: 'foo' + fill_in 'branch_name', with: valid_new_branch_name fill_in 'ref', with: 'foo' click_button 'Create branch' end @@ -57,7 +57,7 @@ class Spinach::Features::ProjectCommitsBranches < Spinach::FeatureSteps end step 'I should see new branch created' do - page.should have_content 'deploy_keys' + page.should have_content valid_new_branch_name end step 'I should see new an error that branch is invalid' do diff --git a/features/steps/project/fork.rb b/features/steps/project/fork.rb index da50ba9ced062908ea3946ec440e142c605b8775..25976e78c6421f29712519a643d57ec9111913a1 100644 --- a/features/steps/project/fork.rb +++ b/features/steps/project/fork.rb @@ -9,11 +9,6 @@ class Spinach::Features::ProjectFork < Spinach::FeatureSteps click_link "Fork" end - step 'I am a member of project "Shop"' do - @project = create(:project, name: "Shop") - @project.team << [@user, :reporter] - end - step 'I should see the forked project page' do page.should have_content "Project was successfully forked." end diff --git a/features/steps/project/forked_merge_requests.rb b/features/steps/project/forked_merge_requests.rb index ccef84cdcc5cf9550f32385d2bcc407aec6ec069..cd4846d2bdcec20789a398881527462be7104bb5 100644 --- a/features/steps/project/forked_merge_requests.rb +++ b/features/steps/project/forked_merge_requests.rb @@ -5,17 +5,6 @@ class Spinach::Features::ProjectForkedMergeRequests < Spinach::FeatureSteps include SharedPaths include Select2Helper - step 'I am a member of project "Shop"' do - @project = Project.find_by(name: "Shop") - @project ||= create(:project, name: "Shop") - @project.team << [@user, :reporter] - @project.ensure_satellite_exists - end - - step 'I have a project forked off of "Shop" called "Forked Shop"' do - @forked_project = Projects::ForkService.new(@project, @user).execute - end - step 'I click link "New Merge Request"' do click_link "New Merge Request" end diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index 20f8f6c24aec36331023cae8907ef6c3737601b5..fa4a796ca3965dd932898f2b60cecb9a0e0fc096 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -1,6 +1,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps include SharedAuthentication include SharedProject + include SharedProjectSource include SharedPaths include RepoHelpers @@ -29,11 +30,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps end step 'I should see its content' do - page.should have_content old_gitignore_content - end - - step 'I should see its new content' do - page.should have_content new_gitignore_content + page.should have_content(old_content) end step 'I click link "Raw"' do @@ -49,30 +46,14 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps end step 'I can edit code' do - set_new_content - evaluate_script('editor.getValue()').should == new_gitignore_content - end - - step 'I edit code' do - set_new_content - end - - step 'I fill the new file name' do - fill_in :file_name, with: new_file_name - end - - step 'I fill the commit message' do - fill_in :commit_message, with: 'Not yet a commit message.' + set_new_editor_content + evaluate_script('editor.getValue()').should == new_content end step 'I click link "Diff"' do click_link 'Diff' end - step 'I click on "Commit Changes"' do - click_button 'Commit Changes' - end - step 'I click on "Remove"' do click_link 'Remove' end @@ -129,48 +110,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps click_link 'Permalink' end - step 'I am redirected to the files URL' do - current_path.should == project_tree_path(@project, 'master') - end - - step 'I am redirected to the ".gitignore"' do - expect(current_path).to eq(project_blob_path(@project, 'master/.gitignore')) - end - - step 'I am redirected to the permalink URL' do - expect(current_path).to eq(project_blob_path( - @project, @project.repository.commit.sha + '/.gitignore')) - end - - step 'I am redirected to the new file' do - expect(current_path).to eq(project_blob_path( - @project, 'master/' + new_file_name)) - end - step "I don't see the permalink link" do expect(page).not_to have_link('permalink') end - - private - - def set_new_content - execute_script("editor.setValue('#{new_gitignore_content}')") - end - - # Content of the gitignore file on the seed repository. - def old_gitignore_content - '*.rbc' - end - - # Constant value that differs from the content - # of the gitignore of the seed repository. - def new_gitignore_content - old_gitignore_content + 'a' - end - - # Constant value that is a valid filename and - # not a filename present at root of the seed repository. - def new_file_name - 'not_a_file.md' - end end diff --git a/features/steps/project/source/merge_request_from_edit.rb b/features/steps/project/source/merge_request_from_edit.rb new file mode 100644 index 0000000000000000000000000000000000000000..44ee353688fa7750aba2e4cdd7df6fce3303a716 --- /dev/null +++ b/features/steps/project/source/merge_request_from_edit.rb @@ -0,0 +1,96 @@ +class Spinach::Features::ProjectSourceMergeRequestFromEdit < Spinach::FeatureSteps + include SharedAuthentication + include SharedProject + include SharedProjectSource + include SharedPaths + + step 'I check "Create merge request"' do + check :create_merge_request + end + + step 'I check "On my fork"' do + check :on_my_fork + end + + step 'I uncheck "On my fork"' do + uncheck :on_my_fork + end + + step 'I fill the new branch name' do + fill_in :new_branch_name, with: valid_new_branch_name + end + + step 'I fill the new branch name with a non-default value' do + fill_in :new_branch_name, with: non_default_branch_name + end + + step 'The new branch name is the non-default value' do + expect(page).to have_field(:new_branch_name, with: non_default_branch_name) + end + + step 'I fill the new branch name with an invalid branch name' do + fill_in :new_branch_name, with: invalid_branch_name + end + + step 'I should be redirected to the new merge request page from origin to itself' do + page.should have_content( + "From #{@project.namespace.path}:#{valid_new_branch_name} "\ + "into #{@project.namespace.path}:#{@project.default_branch}" + ) + end + + step 'I should be redirected to the new merge request page from origin to fork' do + page.should have_content( + "From #{@user.fork_of(@project).namespace.path}:#{valid_new_branch_name} "\ + "into #{@project.namespace.path}:#{@project.default_branch}" + ) + end + + step 'I should be redirected to the new merge request page from fork to itself' do + page.should have_content( + "From #{@user.fork_of(@project).namespace.path}:#{valid_new_branch_name} "\ + "into #{@user.fork_of(@project).namespace.path}:#{@project.default_branch}" + ) + end + + step 'I see the "New branch name" input' do + expect(page).to have_field(:new_branch_name) + end + + step 'I don\'t see the "New branch name" input' do + expect(page).not_to have_field(:new_branch_name) + end + + step 'The new branch name is "patch-1"' do + expect(page).to have_field(:new_branch_name, with: 'patch-1') + end + + step 'The new branch name is "patch-2"' do + expect(page).to have_field(:new_branch_name, with: 'patch-2') + end + + step 'I don\'t see the "On my fork" checkbox' do + expect(page).not_to have_field(:on_my_fork) + end + + step '"Create merge request" is checked and disabled' do + expect(page).to have_field(:create_merge_request, + checked: true, disabled: true) + end + + step '"On my fork" is checked and disabled' do + expect(page).to have_field(:on_my_fork, checked: true, disabled: true) + end + + step 'Project "Shop" has a branch named "patch-1"' do + Project.find_by(name: 'Shop').repository.add_branch('patch-1') + end + + private + + # A constant value different from any default branch name + # that can be suggested on the form. + def non_default_branch_name + 'non-default' + end +end diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 1f238f8befdaf8fda73382a2ce2d006e27f24c16..eb1169e826b9bfe469967b5d833e9f9678439bc2 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -229,6 +229,10 @@ module SharedPaths visit project_merge_requests_path(@forked_project) end + step 'I visit ".gitignore" file edit page of project "Forked Shop"' do + visit project_edit_tree_path(@forked_project, File.join(root_ref, '.gitignore')) + end + step 'I visit edit project "Shop" page' do visit edit_project_path(project) end @@ -257,6 +261,10 @@ module SharedPaths visit project_tree_path(@project, root_ref) end + step 'I am redirected to the files URL' do + current_path.should == project_tree_path(@project,root_ref) + end + step 'I visit blob file from repo' do visit project_blob_path(@project, File.join(sample_commit.id, sample_blob.path)) end @@ -265,6 +273,30 @@ module SharedPaths visit project_blob_path(@project, File.join(root_ref, '.gitignore')) end + step 'I am redirected to the ".gitignore"' do + expect(current_path).to eq(project_blob_path( + @project, File.join(root_ref, '.gitignore'))) + end + + step 'I am redirected to the permalink URL' do + expect(current_path).to eq(project_blob_path( + @project, @project.repository.commit.sha + '/.gitignore')) + end + + step 'I am redirected to the new file' do + expect(current_path).to eq(project_blob_path( + @project, File.join(root_ref, new_file_name))) + end + + step 'I visit ".gitignore" file edit page' do + visit project_edit_tree_path(@project, File.join(root_ref, '.gitignore')) + end + + step 'I should be on ".gitignore" file edit page' do + expect(current_path).to eq( + project_edit_tree_path(@project, File.join(root_ref, '.gitignore'))) + end + step 'I visit project source page for "6d39438"' do visit project_tree_path(@project, "6d39438") end @@ -371,6 +403,11 @@ module SharedPaths visit project_path(project) end + step 'I visit ".gitignore" file edit page of project "Community"' do + project = Project.find_by(name: 'Community') + visit project_edit_tree_path(project, File.join(root_ref, '.gitignore')) + end + step 'I visit project "Internal" page' do project = Project.find_by(name: "Internal") visit project_path(project) @@ -381,6 +418,15 @@ module SharedPaths visit project_path(project) end + # ---------------------------------------- + # Membership projects + # ---------------------------------------- + + step 'I visit ".gitignore" file page of project "Reporter"' do + project = Project.find_by(name: 'Reporter') + visit project_blob_path(project, File.join(root_ref, '.gitignore')) + end + # ---------------------------------------- # Empty Projects # ---------------------------------------- diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index 4b833850a1c969e41188a2619c57fae65fe5b266..90fd427f52deeb572831c4d89fa525fb8bad1914 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -1,5 +1,6 @@ module SharedProject include Spinach::DSL + include RepoHelpers # Create a project without caring about what it's called step "I own a project" do @@ -14,6 +15,32 @@ module SharedProject @project.team << [@user, :master] end + step 'I am a member of project "Shop"' do + i_am_in_project(:reporter) + end + + step 'I am a reporter of project "Reporter"' do + i_am_in_project(:reporter, 'Reporter') + end + + step 'I am a developper of project "Shop"' do + i_am_in_project(:developper) + end + + step 'I am a master of project "Shop"' do + i_am_in_project(:master) + end + + def i_am_in_project(role, project_name = 'Shop') + @project = Project.find_by(name: project_name) + @project ||= create(:project, name: project_name) + @project.team << [@user, role] + end + + step 'I have a project forked off of "Shop" called "Forked Shop"' do + @forked_project = Projects::ForkService.new(@project, @user).execute + end + # Create another specific project called "Forum" step 'I own project "Forum"' do @project = Project.find_by(name: "Forum") diff --git a/features/steps/shared/project_source.rb b/features/steps/shared/project_source.rb new file mode 100644 index 0000000000000000000000000000000000000000..b2a18524c10f2389a4b8156400f5f01220fb8661 --- /dev/null +++ b/features/steps/shared/project_source.rb @@ -0,0 +1,42 @@ +module SharedProjectSource + include Spinach::DSL + include RepoHelpers + + step 'I fill the new file name' do + fill_in :file_name, with: new_file_name + end + + step 'I click on "new file" link in repo' do + click_link 'new-file-link' + end + + step 'I edit code' do + set_new_editor_content + end + + step 'I should see the new edited content' do + expect_editor_content(new_content) + end + + step 'I fill the commit message' do + fill_in :commit_message, with: 'Not yet a commit message.' + end + + step 'I click on "Commit Changes"' do + click_button 'Commit Changes' + end + + step 'I should see its new content' do + page.should have_content(new_content) + end + + private + + def set_new_editor_content + execute_script("editor.setValue('#{new_content}')") + end + + def expect_editor_content(content) + expect(evaluate_script('editor.getValue()')).to eq(content) + end +end diff --git a/lib/gitlab/satellite/files/edit_file_action.rb b/lib/gitlab/satellite/files/edit_file_action.rb index cbdf70f7d129c10866b0a08e0882be7123be244c..b9378364337724878df97efd038b2be51492c24b 100644 --- a/lib/gitlab/satellite/files/edit_file_action.rb +++ b/lib/gitlab/satellite/files/edit_file_action.rb @@ -33,15 +33,31 @@ module Gitlab # will raise CommandFailed when commit fails repo.git.commit(raise: true, timeout: true, a: true, m: commit_message) + puts '================================================================================' + puts 'EditFileAction#commit! before push' + puts '# repo.git' + p repo.git + origin = repo.config['remote.origin.url'] + puts 'origin = ' + origin + puts '# origin hooks pre-receive = ' + puts File.read(File.join(origin, 'hooks', 'pre-receive')) + puts '================================================================================' # push commit back to bare repo # will raise CommandFailed when push fails repo.git.push({raise: true, timeout: true}, :origin, ref) + puts '================================================================================' + puts 'EditFileAction#commit! after push' + puts '================================================================================' + # everything worked true end rescue Grit::Git::CommandFailed => ex + puts '================================================================================' + puts 'push error: ex.message = ' + ex.message + puts '================================================================================' Gitlab::GitLogger.error(ex.message) false end diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index a8f26a7c02931f8484aea7d0d3d86a1f3e196ee0..9b6dae7ade467b575a596c5e52205ee52a67fbbe 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -5,7 +5,9 @@ namespace :gitlab do warn_user_is_not_gitlab default_version = File.read(File.join(Rails.root, "GITLAB_SHELL_VERSION")).strip + # TODO restore old repo args.with_defaults(tag: 'v' + default_version, repo: "https://gitlab.com/gitlab-org/gitlab-shell.git") + #args.with_defaults(tag: 'v' + default_version, repo: "https://gitlab.com/cirosantilli/gitlab-shell.git") user = Settings.gitlab.user home_dir = Rails.env.test? ? Rails.root.join('tmp/tests') : Settings.gitlab.user_home @@ -22,6 +24,11 @@ namespace :gitlab do # Make sure we're on the right tag Dir.chdir(target_dir) do + # Allows to change the origin URL to the fork + # when developing gitlab-shell. + # TODO + #sh(*%W(git remote set-url origin #{args.repo})) + # First try to checkout without fetching # to avoid stalling tests if the Internet is down. reset = "git reset --hard $(git describe #{args.tag} || git describe origin/#{args.tag})" @@ -37,7 +44,7 @@ namespace :gitlab do bin: %x{which redis-cli}.chomp, namespace: "resque:gitlab" }.stringify_keys, - log_level: "INFO", + log_level: Rails.env.test? ? 'DEBUG' : 'INFO', audit_usernames: false }.stringify_keys diff --git a/lib/tasks/spec.rake b/lib/tasks/spec.rake index bee22300298156266b6797a5988b5bce47da3abe..5cda8aa26c854dc7ce667a1ea3cf1ddc461e8fb1 100644 --- a/lib/tasks/spec.rake +++ b/lib/tasks/spec.rake @@ -33,7 +33,7 @@ desc "GITLAB | Run specs" task :spec do cmds = [ %W(rake gitlab:setup), - %W(rspec spec), + %W(rspec ./spec/controllers/edit_tree_controller_spec.rb:70), ] run_commands(cmds) end diff --git a/lib/tasks/test.rake b/lib/tasks/test.rake index 583f4a876da9c652b525f85b3d267b7eb76adb44..0bcabbfad59e4fa41ab2d273d11f703fec868805 100644 --- a/lib/tasks/test.rake +++ b/lib/tasks/test.rake @@ -9,5 +9,5 @@ unless Rails.env.production? require 'coveralls/rake/task' Coveralls::RakeTask.new desc "GITLAB | Run all tests on CI with simplecov" - task :test_ci => [:spinach, :spec, 'coveralls:push'] + task :test_ci => [:spec] end diff --git a/spec/controllers/edit_tree_controller_spec.rb b/spec/controllers/edit_tree_controller_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a2c743ddc441c34d60d82f06f92c06b6c004fa1d --- /dev/null +++ b/spec/controllers/edit_tree_controller_spec.rb @@ -0,0 +1,142 @@ +require 'spec_helper' + +describe Projects::EditTreeController do + include RepoHelpers + include ProjectHelpers + + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + + describe '#show' do + it 'returns not found if user cannot fork project' do + get(:show, project_id: project.to_param, id: existing_file_id) + + expect(response.status).to eq(404) + end + + it 'returns success denied if user cannot fork project' do + give_fork_permission(user, project) + + get(:show, project_id: project.to_param, id: existing_file_id) + + expect(response).to be_success + end + end + + describe '#update' do + it 'returns not found if user cannot push to project' do + edit_file + + expect(response.status).to eq(404) + end + + def edit_file + put(:update, edit_file_opts) + end + + def edit_file_opts + { + project_id: project.to_param, + id: existing_file_id, + content: new_content, + commit_message: new_commit_message + } + end + + describe 'edit and create merge request' do + before do + give_push_permission(user, project) + end + + it 'suceeds from origin to itself' do + make_new_mr + + expect(user.already_forked?(project)).to be_false + expect_make_new_mr_suceeded + end + + it 'succeeds from an existing fork to origin' do + fork = create(:project, namespace: user.namespace, + forked_from_project: project) + expect(user.already_forked?(project)).to be_true + expect(fork.repository.has_branch?(valid_new_branch_name)).to be_false + + make_new_mr(on_my_fork: '1') + + expect(user.already_forked?(project)).to be_true + expect_make_new_mr_suceeded(fork, project) + end + + it 'succeeds from a new fork to origin' do + expect(user.already_forked?(project)).to be_false + + puts '================================================================================' + puts 'ruby version = ' + RUBY_VERSION + puts 'gitlab_url = ' + Settings.gitlab.url + puts 'response code port 3001 = ' + Net::HTTP.get_response(URI('http://localhost:3001/')).code + puts 'response body port 3001 = ' + Net::HTTP.get_response(URI('http://localhost:3001/')).body + puts '# ls tests' + system( 'ls ' + Rails.root.join('tmp', 'tests').to_s) + puts '# cat gitlab-shell config' + system( 'cat ' + Rails.root.join('tmp', 'tests', 'gitlab-shell', 'config.yml').to_s) + puts '================================================================================' + + make_new_mr(on_my_fork: '1') + + expect(user.already_forked?(project)).to be_true + fork = user.fork_of(project) + expect_make_new_mr_suceeded(fork, project) + end + + it 'succeeds from an existing fork to itself' do + fork = create(:project, namespace: user.namespace, + forked_from_project: project) + expect(user.already_forked?(project)).to be_true + expect(fork.repository.has_branch?(valid_new_branch_name)).to be_false + + make_new_mr(project_id: fork.to_param) + + expect(user.already_forked?(project)).to be_true + expect_make_new_mr_suceeded(fork, fork) + end + + it 'fails and stays on edit page if the branch name already exists' do + make_new_mr(new_branch_name: existing_branch_name) + + expect(flash['alert']).not_to be_nil + expect(response).to be_success + end + + it 'fails and stays on edit page if the branch name is invalid' do + make_new_mr(new_branch_name: invalid_branch_name) + + expect(flash['alert']).not_to be_nil + expect(response).to be_success + end + + def make_new_mr(extra_put_opts = {}) + put_opts = edit_file_opts + put_opts.merge!( + create_merge_request: '1', + new_branch_name: valid_new_branch_name + ) + put_opts.merge!(extra_put_opts) + put(:update, put_opts) + end + + def expect_make_new_mr_suceeded(source_project = nil, target_project = nil) + source_project ||= project + target_project ||= source_project + expect(source_project.repository. + has_branch?(valid_new_branch_name)).to be_true + expect(response).to redirect_to(new_project_merge_request_path( + source_project, + merge_request: { + source_branch: valid_new_branch_name, + target_project_id: target_project.id + } + )) + end + end + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 6c3e221f34372200cf8af9171d8b38578f61be10..768fea5e3f47220f5e64c0f6f477f8004810be9c 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -18,4 +18,15 @@ describe Repository do it { should eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') } end + + describe :free_branch_name do + it 'returns the smallest new free branch name' do + prefix = valid_new_branch_name + expect(repository.free_branch_name(prefix)).to eq(prefix + '1') + repository.add_branch(prefix + '1', 'HEAD') + expect(repository.free_branch_name(prefix)).to eq(prefix + '2') + repository.rm_branch(prefix + '1') + expect(repository.free_branch_name(prefix)).to eq(prefix + '1') + end + end end diff --git a/spec/support/project_helpers.rb b/spec/support/project_helpers.rb new file mode 100644 index 0000000000000000000000000000000000000000..8b421a32b1129832c9c4dfa918af2a27b6bf35da --- /dev/null +++ b/spec/support/project_helpers.rb @@ -0,0 +1,13 @@ +module ProjectHelpers + extend self + + def give_push_permission(user, project) + sign_in(user) + project.team << [user, :developer] + end + + def give_fork_permission(user, project) + sign_in(user) + project.team << [user, :reporter] + end +end diff --git a/spec/support/repo_helpers.rb b/spec/support/repo_helpers.rb index 4c4775da69237f54e4e689e500199769cb565d50..18562576b9a3021a93ba274379c28cbd2ef24c4c 100644 --- a/spec/support/repo_helpers.rb +++ b/spec/support/repo_helpers.rb @@ -96,4 +96,49 @@ eos commits: commits ) end + + # Seed repository information. + + # Constant valid non-existent branch name. + def valid_new_branch_name + 'new-branch' + end + + def existing_branch_name + 'master' + end + + def invalid_branch_name + 'a b' + end + + # Constant existing path on branch master. + def existing_file_path + '.gitignore' + end + + # Constant existing branch/path pair. + def existing_file_id + existing_branch_name + '/' + existing_file_path + end + + # Constant valid non-existent path on branch master. + def new_file_name + 'new_file' + end + + # Substring of content of existing_file_path. + def old_content + '*.rbc' + end + + # Not a substring of old_content + def new_content + 'New content' + end + + # Constant inexistent commit message. + def new_commit_message + 'New commit message' + end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 5f55871dc4a67870345593d3d57df3b7a701a8b7..ca56c6b7fe0c6f62da6b2b2790c93dd69ed5caf2 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -1,4 +1,5 @@ require 'rspec/mocks' +require 'webrick' module TestEnv extend self @@ -14,11 +15,9 @@ module TestEnv disable_mailer if opts[:mailer] == false # Clean /tmp/tests - tmp_test_path = Rails.root.join('tmp', 'tests') - if File.directory?(tmp_test_path) Dir.entries(tmp_test_path).each do |entry| - unless ['.', '..', 'gitlab-shell'].include?(entry) + unless ['.', '..'].include?(entry) FileUtils.rm_r(File.join(tmp_test_path, entry)) end end @@ -29,10 +28,16 @@ module TestEnv # Setup GitLab shell for test instance setup_gitlab_shell + setup_internal_api_mock + # Create repository for FactoryGirl.create(:project) setup_factory_repo end + def deinit + destroy_internal_api_mock + end + def disable_mailer NotificationService.any_instance.stub(mailer: double.as_null_object) end @@ -75,4 +80,62 @@ module TestEnv def factory_repo_name 'gitlab-test' end + + def tmp_test_path + Rails.root.join('tmp', 'tests') + end + + def internal_api_mock_pid_path + File.join(tmp_test_path, 'internal_api_mock.pid') + end + + # This mock server exists because during testing GitLab is not served + # on any port, but gitlab-shell needs to ask the GitLab internal API + # if it is OK to push to repositories. This can happen during blob web + # edit tests. The server always replies yes: this should not modify affect + # web interface tests. + def setup_internal_api_mock + begin + server = WEBrick::HTTPServer.new( + BindAddress: '0.0.0.0', + Port: Gitlab.config.gitlab.port, + AccessLog: [], + Logger: WEBrick::Log.new('/dev/null') + ) + rescue => ex + ex.message.prepend('could not start mock server on configured port. ') + raise ex + end + fork do + trap(:INT) { server.shutdown } + server.mount_proc('/') do |_req, res| + res.status = 200 + res.body = 'true' + end + WEBrick::Daemon.start do + File.write(internal_api_mock_pid_path, Process.pid) + end + server.start + end + # Ideally this should be called from `config.after(:suite)`, + # but on Spinach when user hits Ctrl+C the server does not get killed + # if the hook is set up with `Spinach.hooks.after_run`. + at_exit do + # The file should exist on normal operation, + # but certain errors can lead to it not existing. + if File.exists?(internal_api_mock_pid_path) + Process.kill(:INT, File.read(internal_api_mock_pid_path).to_i) + end + puts '================================================================================' + puts '# cat gitlab-shell.log' + system( 'cat ' + Rails.root.join('tmp', 'tests', 'gitlab-shell', 'gitlab-shell.log').to_s) + puts '# cat /tmp/gitlab' + system( 'cat /tmp/gitlab') + puts '# cat /tmp/gitlab-prereceive' + system( 'cat /tmp/gitlab-prereceive') + puts '# cat /tmp/gitlab-postreceive' + system( 'cat /tmp/gitlab-postreceive') + puts '================================================================================' + end + end end