From 349e3622f92cc4c6ea51ecaac76ef7c4ea8999e3 Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Wed, 15 Nov 2017 13:47:22 -0600 Subject: [PATCH] Added ssh_button helper specs and addressed ruby code observations --- app/assets/stylesheets/pages/projects.scss | 4 ++ app/helpers/button_helper.rb | 44 ++++++++--------- spec/helpers/button_helper_spec.rb | 55 ++++++++++++++++------ 3 files changed, 67 insertions(+), 36 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 26701a97ec9..3799a3d4cc8 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -409,6 +409,10 @@ .clone-options-dropdown { min-width: 240px; + + .dropdown-menu-inner-content { + min-width: 320px; + } } } diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb index 4f945b5ad9e..d06cf2de2c3 100644 --- a/app/helpers/button_helper.rb +++ b/app/helpers/button_helper.rb @@ -58,34 +58,34 @@ module ButtonHelper def http_clone_button(project, append_link: true) protocol = gitlab_config.protocol.upcase + dropdown_description = http_dropdown_description(protocol) + append_url = project.http_url_to_repo if append_link - protocol_description = - if current_user.try(:require_password_creation?) - _("Set a password on your account to pull or push via %{protocol}.") % { protocol: protocol } - else - _("Create a personal access token on your account to pull or push via %{protocol}.") % { protocol: protocol } - end + dropdown_item_with_description(protocol, dropdown_description, href: append_url) + end - protocol_element_output = content_tag(:strong, protocol, class: 'dropdown-menu-inner-title') - - if current_user.try(:require_password_creation?) || current_user.try(:require_personal_access_token_creation_for_git_auth?) - protocol_element_output << content_tag(:span, protocol_description, class: 'dropdown-menu-inner-content') + def http_dropdown_description(protocol) + if current_user.try(:require_password_creation_for_git?) + _("Set a password on your account to pull or push via %{protocol}.") % { protocol: protocol } + else + _("Create a personal access token on your account to pull or push via %{protocol}.") % { protocol: protocol } end - - content_tag (append_link ? :a : :span), - protocol_element_output, - class: 'http-selector', - href: (project.http_url_to_repo if append_link) end def ssh_clone_button(project, append_link: true) - ssh_description = _("You won't be able to pull or push project code via SSH until you add an SSH key to your profile") - ssh_element_output = content_tag(:strong, 'SSH', class: 'dropdown-menu-inner-title') - ssh_element_output << content_tag(:span, ssh_description, class: 'dropdown-menu-inner-content') if current_user.try(:require_ssh_key?) + dropdown_description = _("You won't be able to pull or push project code via SSH until you add an SSH key to your profile") if current_user.try(:require_ssh_key?) + append_url = project.ssh_url_to_repo if append_link - content_tag (append_link ? :a : :span), - ssh_element_output, - class: 'ssh-selector', - href: (project.ssh_url_to_repo if append_link) + dropdown_item_with_description('SSH', dropdown_description, href: append_url) + end + + def dropdown_item_with_description(title, description, href: nil) + button_content = content_tag(:strong, title, class: 'dropdown-menu-inner-title') + button_content << content_tag(:span, description, class: 'dropdown-menu-inner-content') if description + + content_tag (href ? :a : :span), + button_content, + class: "#{title.downcase}-selector", + href: (href if href) end end diff --git a/spec/helpers/button_helper_spec.rb b/spec/helpers/button_helper_spec.rb index b5222f301ba..d2c7867febb 100644 --- a/spec/helpers/button_helper_spec.rb +++ b/spec/helpers/button_helper_spec.rb @@ -27,9 +27,9 @@ describe ButtonHelper do let(:user) { create(:user, password_automatically_set: true) } it 'shows the password text on the dropdown' do - expect(element.children.length).to eq(2) - expect(element.children[1].name).to eq('span') - expect(element.children[1].children[0].text).to eq('Set a password on your account to pull or push via HTTP.') + description = element.search('.dropdown-menu-inner-content').first + + expect(description.inner_text).to eq 'Set a password on your account to pull or push via HTTP.' end end end @@ -40,18 +40,10 @@ describe ButtonHelper do end context 'when user has no personal access tokens' do - it 'has a personal access token tooltip ' do - expect(element.children.length).to eq(2) - expect(element.children[1].name).to eq('span') - expect(element.children[1].children[0].text).to eq('Create a personal access token on your account to pull or push via HTTP.') - end - end + it 'has a personal access token text on the dropdown description ' do + description = element.search('.dropdown-menu-inner-content').first - context 'when user has a personal access token' do - it 'shows no tooltip' do - create(:personal_access_token, user: user) - - expect(element.attr('class')).not_to include(has_tooltip_class) + expect(description.inner_text).to eq 'Create a personal access token on your account to pull or push via HTTP.' end end end @@ -65,6 +57,41 @@ describe ButtonHelper do end end + describe 'ssh_button' do + let(:user) { create(:user) } + let(:project) { build_stubbed(:project) } + + def element + element = helper.ssh_clone_button(project) + + Nokogiri::HTML::DocumentFragment.parse(element).first_element_child + end + + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'without an ssh key on the user' do + it 'shows a warning on the dropdown description' do + description = element.search('.dropdown-menu-inner-content').first + + expect(description.inner_text).to eq "You won't be able to pull or push project code via SSH until you add an SSH key to your profile" + end + end + + context 'with an ssh key on the user' do + before do + create(:key, user: user) + end + + it 'there is no warning on the dropdown description' do + description = element.search('.dropdown-menu-inner-content').first + + expect(description).to eq nil + end + end + end + describe 'clipboard_button' do let(:user) { create(:user) } let(:project) { build_stubbed(:project) }