From ca52f848411efaa5ccfb364c2169b38a8e9644b4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Date: Thu, 8 Aug 2013 15:14:59 +0300 Subject: [PATCH] Update chosen, improve ui, fix MR fork tests --- Gemfile | 2 +- Gemfile.lock | 18 +++-- app/assets/stylesheets/selects.scss | 24 +++---- .../projects/merge_requests_controller.rb | 4 +- .../projects/merge_requests/_form.html.haml | 18 +++-- .../project/forked_merge_requests.feature | 10 +-- .../project/project_forked_merge_requests.rb | 72 ++++++++----------- features/support/env.rb | 2 +- spec/support/chosen_helper.rb | 21 ++++++ spec/support/test_env.rb | 10 +-- 10 files changed, 99 insertions(+), 82 deletions(-) create mode 100644 spec/support/chosen_helper.rb diff --git a/Gemfile b/Gemfile index c865e9f6dd1..ca1b593013d 100644 --- a/Gemfile +++ b/Gemfile @@ -129,7 +129,7 @@ group :assets do gem 'turbolinks' gem 'jquery-turbolinks' - gem 'chosen-rails', "0.9.8" + gem 'chosen-rails', "1.0.0" gem 'select2-rails' gem 'jquery-atwho-rails', "0.3.0" gem "jquery-rails", "2.1.3" diff --git a/Gemfile.lock b/Gemfile.lock index b4a2626149f..d0ee20f34e7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -72,9 +72,12 @@ GEM charlock_holmes (0.6.9.4) childprocess (0.3.9) ffi (~> 1.0, >= 1.0.11) - chosen-rails (0.9.8) - railties (~> 3.0) - thor (~> 0.14) + chosen-rails (1.0.0) + coffee-rails (>= 3.2) + compass-rails (>= 1.0) + railties (>= 3.0) + sass-rails (>= 3.2) + chunky_png (1.2.8) code_analyzer (0.3.2) sexp_processor coderay (1.0.9) @@ -87,6 +90,12 @@ GEM coffee-script-source (1.6.2) colored (1.2) colorize (0.5.8) + compass (0.12.2) + chunky_png (~> 1.2) + fssm (>= 0.2.7) + sass (~> 3.1) + compass-rails (1.0.3) + compass (>= 0.12.2, < 0.14) connection_pool (1.1.0) coveralls (0.6.7) colorize @@ -149,6 +158,7 @@ GEM dotenv (>= 0.7) thor (>= 0.13.6) formatador (0.2.4) + fssm (0.2.10) gemoji (1.2.1) gherkin-ruby (0.3.0) github-linguist (2.3.4) @@ -548,7 +558,7 @@ DEPENDENCIES bootstrap-sass capybara carrierwave - chosen-rails (= 0.9.8) + chosen-rails (= 1.0.0) coffee-rails colored coveralls diff --git a/app/assets/stylesheets/selects.scss b/app/assets/stylesheets/selects.scss index 9b7b6ad583c..589d34672e7 100644 --- a/app/assets/stylesheets/selects.scss +++ b/app/assets/stylesheets/selects.scss @@ -1,10 +1,10 @@ /* CHZN reset few styles */ -.chzn-container-single .chzn-single { +.chosen-container-single .chosen-single { background: #FFF; border: 1px solid #bbb; box-shadow: none; } -.chzn-container-active .chzn-single { +.chosen-container-active .chosen-single { background: #fff; } @@ -41,38 +41,38 @@ width: 120px; } -.project-refs-form .chzn-container { +.project-refs-form .chosen-container { position: relative; top: 0; left: 0; margin-right: 10px; - .chzn-drop { + .chosen-drop { min-width: 400px; - .chzn-results { + .chosen-results { max-height: 300px; } - .chzn-search input { + .chosen-search input { min-width: 365px; } } } /** Fix for Search Dropdown Border **/ -.chzn-container { - .chzn-search { +.chosen-container { + .chosen-search { input:focus { @include box-shadow(none); } } - .chzn-drop { + .chosen-drop { margin: 7px 0; min-width: 200px; border: 1px solid #bbb; @include border-radius(0); - .chzn-results { + .chosen-results { margin-top: 5px; max-height: 300px; @@ -95,7 +95,7 @@ } } - .chzn-search { + .chosen-search { @include bg-gray-gradient; input { min-width: 165px; @@ -104,7 +104,7 @@ } } - .chzn-single { + .chosen-single { @include bg-light-gray-gradient; div { diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index dd1f8cee873..aa244143438 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -101,12 +101,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController def branch_from #This is always source @source_project = @merge_request.nil? ? @project : @merge_request.source_project - @commit = @repository.commit(params[:ref]) + @commit = @repository.commit(params[:ref]) if params[:ref].present? end def branch_to @target_project = selected_target_project - @commit = @target_project.repository.commit(params[:ref]) + @commit = @target_project.repository.commit(params[:ref]) if params[:ref].present? end def update_branches diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml index c8a69800500..0589bbb02f0 100644 --- a/app/views/projects/merge_requests/_form.html.haml +++ b/app/views/projects/merge_requests/_form.html.haml @@ -13,8 +13,12 @@ .span5 .light-well %h5.cgray From - .padded= f.select(:source_project_id,[[@merge_request.source_project.path_with_namespace,@merge_request.source_project.id]] , {}, {class: 'source_project chosen span4'}) - .padded= f.select(:source_branch, @merge_request.source_project.repository.branch_names, { include_blank: "Select branch" }, {class: 'source_branch chosen span4'}) + .padded + = f.select(:source_project_id,[[@merge_request.source_project.path_with_namespace,@merge_request.source_project.id]] , {}, {class: 'source_project chosen span4'}) + .prepend-top-10 + %i.icon-code-fork + + = f.select(:source_branch, @merge_request.source_project.repository.branch_names, { include_blank: "Select branch" }, {class: 'source_branch chosen span3'}) .mr_source_commit.prepend-top-10 .span2 %h1.merge-request-angle @@ -23,9 +27,13 @@ .light-well %h5.cgray To - projects = @project.forked_from_project.nil? ? [@project] : [ @project,@project.forked_from_project] - .padded= f.select(:target_project_id, projects.map { |proj| [proj.path_with_namespace,proj.id] }, {include_blank: "Select Target Project" }, {class: 'target_project chosen span4'}) - .padded= f.select(:target_branch, @target_branches, { include_blank: "Select branch" }, {class: 'target_branch chosen span4'}) - .mr_target_commit.prepend-top-10 + .padded + = f.select(:target_project_id, projects.map { |proj| [proj.path_with_namespace,proj.id] }, {include_blank: "Select Target Project" }, {class: 'target_project chosen span4'}) + .prepend-top-10 + %i.icon-code-fork + + = f.select(:target_branch, @target_branches, { include_blank: "Select branch" }, {class: 'target_branch chosen span3'}) + .mr_target_commit.prepend-top-10 %hr diff --git a/features/project/forked_merge_requests.feature b/features/project/forked_merge_requests.feature index 245515c33ad..2e81b71c43c 100644 --- a/features/project/forked_merge_requests.feature +++ b/features/project/forked_merge_requests.feature @@ -20,14 +20,6 @@ Feature: Project Forked Merge Requests And I submit the merge request Then I should see merge request "Merge Request On Forked Project" - @javascript - Scenario: I should see a push widget for forked merge requests - Given project "Forked Shop" has push event - And I visit dashboard page - Then I should see last push widget - And I click "Create Merge Request on fork" link - Then I see prefilled new Merge Request page for the forked project - @javascript Scenario: I can edit a forked merge request Given I visit project "Forked Shop" merge requests page @@ -47,4 +39,4 @@ Feature: Project Forked Merge Requests And I click link "New Merge Request" And I fill out an invalid "Merge Request On Forked Project" merge request And I submit the merge request - Then I should see validation errors \ No newline at end of file + Then I should see validation errors diff --git a/features/steps/project/project_forked_merge_requests.rb b/features/steps/project/project_forked_merge_requests.rb index d6cad1ba2ec..86f16d3bac5 100644 --- a/features/steps/project/project_forked_merge_requests.rb +++ b/features/steps/project/project_forked_merge_requests.rb @@ -3,30 +3,31 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps include SharedProject include SharedNote include SharedPaths + include ChosenHelper - Given 'I am a member of project "Shop"' do + step 'I am a member of project "Shop"' do @project = Project.find_by_name "Shop" @project ||= create(:project_with_code, name: "Shop") @project.team << [@user, :reporter] end - And 'I have a project forked off of "Shop" called "Forked Shop"' do + step 'I have a project forked off of "Shop" called "Forked Shop"' do @forking_user = @user forked_project_link = build(:forked_project_link) @forked_project = Project.find_by_name "Forked Shop" @forked_project ||= create(:source_project_with_code, name: "Forked Shop", forked_project_link: forked_project_link, creator_id: @forking_user.id , namespace: @forking_user.namespace) + forked_project_link.forked_from_project = @project forked_project_link.forked_to_project = @forked_project @forked_project.team << [@forking_user , :master] forked_project_link.save! end - Given 'I click link "New Merge Request"' do + step 'I click link "New Merge Request"' do click_link "New Merge Request" end - Then 'I should see merge request "Merge Request On Forked Project"' do - page.should have_content "Merge Request On Forked Project" + step 'I should see merge request "Merge Request On Forked Project"' do @project.merge_requests.size.should >= 1 @merge_request = @project.merge_requests.last current_path.should == project_merge_request_path(@project, @merge_request) @@ -40,56 +41,41 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps page.should have_content @merge_request.target_branch end - And 'I fill out a "Merge Request On Forked Project" merge request' do - #The ordering here is a bit whacky on purpose: - #Select the target right away, to give update_branches time to run and clean up the target_branches - find(:select, "merge_request_target_project_id", {}).value.should == @forked_project.id.to_s - select @project.path_with_namespace, from: "merge_request_target_project_id" - + step 'I fill out a "Merge Request On Forked Project" merge request' do + chosen @forked_project.id, from: "#merge_request_source_project_id" + chosen @project.id, from: "#merge_request_target_project_id" - fill_in "merge_request_title", with: "Merge Request On Forked Project" find(:select, "merge_request_source_project_id", {}).value.should == @forked_project.id.to_s - find(:select, "merge_request_target_project_id", {}).value.should == @project.id.to_s - #Ensure the option exists in the select - find(:select, "merge_request_source_branch", {}).should have_content "master" - select "master", from: "merge_request_source_branch" - #Ensure the option is selected - find(:select, "merge_request_source_branch", {}).value.should have_content "master" - verify_commit_link(".mr_source_commit",@forked_project) - + chosen "master", from: "#merge_request_source_branch" + chosen "stable", from: "#merge_request_target_branch" - #This could fail if the javascript hasn't run yet, there is a timing issue here -- this is why we do the select at the top - #Ensure the option exists in the select - find(:select, "merge_request_target_branch", {}).should have_content "stable" - #We must give apparently lots of time for update branches to finish + find(:select, "merge_request_source_branch", {}).value.should == 'master' + find(:select, "merge_request_target_branch", {}).value.should == 'stable' - (find(:select, "merge_request_target_branch", {}).find(:option, "stable",{}).select_option).should be_true - #Ensure the option is selected - find(:select, "merge_request_target_branch", {}).value.should have_content "stable" - verify_commit_link(".mr_target_commit",@project) + fill_in "merge_request_title", with: "Merge Request On Forked Project" end - And 'I submit the merge request' do + step 'I submit the merge request' do click_button "Submit merge request" end - And 'I follow the target commit link' do + step 'I follow the target commit link' do commit = @project.repository.commit click_link commit.short_id(8) end - Then 'I should see the commit under the forked from project' do + step 'I should see the commit under the forked from project' do commit = @project.repository.commit page.should have_content(commit.message) end - And 'I click "Create Merge Request on fork" link' do + step 'I click "Create Merge Request on fork" link' do click_link "Create Merge Request on fork" end - Then 'I see prefilled new Merge Request page for the forked project' do + step 'I see prefilled new Merge Request page for the forked project' do current_path.should == new_project_merge_request_path(@forked_project) find("#merge_request_source_project_id").value.should == @forked_project.id.to_s find("#merge_request_target_project_id").value.should == @project.id.to_s @@ -100,15 +86,15 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps verify_commit_link(".mr_source_commit",@forked_project) end - And 'I update the merge request title' do + step 'I update the merge request title' do fill_in "merge_request_title", with: "An Edited Forked Merge Request" end - And 'I save the merge request' do + step 'I save the merge request' do click_button "Save changes" end - Then 'I should see the edited merge request' do + step 'I should see the edited merge request' do page.should have_content "An Edited Forked Merge Request" @project.merge_requests.size.should >= 1 @merge_request = @project.merge_requests.last @@ -122,12 +108,12 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps page.should have_content @merge_request.target_branch end - Then 'I should see last push widget' do + step 'I should see last push widget' do page.should have_content "You pushed to new_design" page.should have_link "Create Merge Request" end - Given 'project "Forked Shop" has push event' do + step 'project "Forked Shop" has push event' do @forked_project = Project.find_by_name("Forked Shop") data = { @@ -154,13 +140,13 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps end - Then 'I click link edit "Merge Request On Forked Project"' do + step 'I click link edit "Merge Request On Forked Project"' do find("#edit_merge_request").click end - Then 'I see the edit page prefilled for "Merge Request On Forked Project"' do + step 'I see the edit page prefilled for "Merge Request On Forked Project"' do current_path.should == edit_project_merge_request_path(@project, @merge_request) - page.should have_content "Edit merge request #{@merge_request.id}" + page.should have_content "Edit merge request ##{@merge_request.id}" find("#merge_request_title").value.should == "Merge Request On Forked Project" find("#merge_request_source_project_id").value.should == @forked_project.id.to_s find("#merge_request_target_project_id").value.should == @project.id.to_s @@ -170,7 +156,7 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps verify_commit_link(".mr_target_commit",@project) end - And 'I fill out an invalid "Merge Request On Forked Project" merge request' do + step 'I fill out an invalid "Merge Request On Forked Project" merge request' do #If this isn't filled in the rest of the validations won't be triggered fill_in "merge_request_title", with: "Merge Request On Forked Project" find(:select, "merge_request_source_project_id", {}).value.should == @forked_project.id.to_s @@ -179,7 +165,7 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps find(:select, "merge_request_target_branch", {}).value.should == "" end - Then 'I should see validation errors' do + step 'I should see validation errors' do page.should have_content "Source branch can't be blank" page.should have_content "Target branch can't be blank" page.should have_content "Branch conflict You can not use same project/branch for source and target" diff --git a/features/support/env.rb b/features/support/env.rb index 5dce3402083..0cc7d8d2fe9 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -14,7 +14,7 @@ require 'spinach/capybara' require 'sidekiq/testing/inline' -%w(valid_commit select2_helper test_env).each do |f| +%w(valid_commit select2_helper chosen_helper test_env).each do |f| require Rails.root.join('spec', 'support', f) end diff --git a/spec/support/chosen_helper.rb b/spec/support/chosen_helper.rb new file mode 100644 index 00000000000..42c9342c77a --- /dev/null +++ b/spec/support/chosen_helper.rb @@ -0,0 +1,21 @@ +# Chosen programmatic helper +# It allows you to select value from chosen select +# +# Params +# value - real value of selected item +# opts - options containing css selector +# +# Usage: +# +# chosen(2, from: '#user_ids') +# + +module ChosenHelper + def chosen(value, options={}) + raise "Must pass a hash containing 'from'" if not options.is_a?(Hash) or not options.has_key?(:from) + + selector = options[:from] + + page.execute_script("$('#{selector}').val('#{value}').trigger('chosen:updated');") + end +end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index cae7ff88513..203e661f514 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -88,11 +88,11 @@ module TestEnv def clear_repo_dir(namespace, name) setup_stubs - #Clean any .wiki.git that may have been created + # Clean any .wiki.git that may have been created FileUtils.rm_rf File.join(testing_path(), "#{name}.wiki.git") end - #Create a repo and it's satellite + # Create a repo and it's satellite def create_repo(namespace, name) setup_stubs repo = repo(namespace, name) @@ -152,7 +152,7 @@ module TestEnv # Recreate tmp/test-git-base-path FileUtils.mkdir_p Gitlab.config.gitlab_shell.repos_path - #Since much more is happening in satellites + # Since much more is happening in satellites FileUtils.mkdir_p Gitlab.config.satellites.path end @@ -161,8 +161,8 @@ module TestEnv satellite_repo = satellite(namespace, satellite_name) # Symlink tmp/satellite/gitlabhq to tmp/test-git-base-path/satellite/gitlabhq, create the directory if it doesn't exist already satellite_dir = File.dirname(satellite_repo) - FileUtils.mkdir_p satellite_dir unless File.exists?(satellite_dir) - system("ln -s -f #{seed_satellite_path()} #{satellite_repo}") + FileUtils.mkdir_p(satellite_dir) unless File.exists?(satellite_dir) + system("ln -s -f #{seed_satellite_path} #{satellite_repo}") end def create_temp_repo(path) -- GitLab