diff --git a/app/assets/javascripts/pages/profiles/keys/index.js b/app/assets/javascripts/pages/profiles/keys/index.js index 28b1aa02dfad0e6c10a641a35a15591c63c4c306..b79acfd5c57950931606c47446a9033004deec1f 100644 --- a/app/assets/javascripts/pages/profiles/keys/index.js +++ b/app/assets/javascripts/pages/profiles/keys/index.js @@ -12,6 +12,7 @@ function initSshKeyValidation() { const warning = document.querySelector('.js-add-ssh-key-validation-warning'); const originalSubmit = input.form.querySelector('.js-add-ssh-key-validation-original-submit'); const confirmSubmit = warning.querySelector('.js-add-ssh-key-validation-confirm-submit'); + const cancelButton = input.form.querySelector('.js-add-ssh-key-validation-cancel'); const addSshKeyValidation = new AddSshKeyValidation( supportedAlgorithms, @@ -19,6 +20,7 @@ function initSshKeyValidation() { warning, originalSubmit, confirmSubmit, + cancelButton, ); addSshKeyValidation.register(); } diff --git a/app/assets/javascripts/profile/add_ssh_key_validation.js b/app/assets/javascripts/profile/add_ssh_key_validation.js index 628dd159db8c49299c4ff66d88effa6e841d9f8b..8e395a1c6e91dfdb2430dbd761a3ff631d95fe86 100644 --- a/app/assets/javascripts/profile/add_ssh_key_validation.js +++ b/app/assets/javascripts/profile/add_ssh_key_validation.js @@ -5,6 +5,7 @@ export default class AddSshKeyValidation { warningElement, originalSubmitElement, confirmSubmitElement, + cancelButtonElement, ) { this.inputElement = inputElement; this.form = inputElement.form; @@ -16,6 +17,7 @@ export default class AddSshKeyValidation { this.originalSubmitElement = originalSubmitElement; this.confirmSubmitElement = confirmSubmitElement; + this.cancelButtonElement = cancelButtonElement; this.isValid = false; } @@ -44,6 +46,7 @@ export default class AddSshKeyValidation { toggleWarning(isVisible) { this.warningElement.classList.toggle('hide', !isVisible); this.originalSubmitElement.classList.toggle('hide', isVisible); + this.cancelButtonElement?.classList.toggle('hide', isVisible); } isPublicKey(value) { diff --git a/app/views/profiles/keys/_form.html.haml b/app/views/profiles/keys/_form.html.haml index 5c4ea7b2ecb78a7e0efd6b226f958c02f229f1e3..b1df63a72ab7a0ed9a22695e2d2c7673463afe20 100644 --- a/app/views/profiles/keys/_form.html.haml +++ b/app/views/profiles/keys/_form.html.haml @@ -25,12 +25,13 @@ %p.form-text.text-muted= ssh_key_expires_field_description .js-add-ssh-key-validation-warning.hide - .bs-callout.bs-callout-warning{ role: 'alert', aria_live: 'assertive' } + .bs-callout.bs-callout-warning.gl-mt-0{ role: 'alert', aria_live: 'assertive' } %strong= _('Oops, are you sure?') %p= s_("Profiles|Publicly visible private SSH keys can compromise your system.") - = render Pajamas::ButtonComponent.new(variant: :confirm, button_options: { class: 'js-add-ssh-key-validation-confirm-submit' }) do = _("Yes, add it") - .gl-mt-3 - = f.submit s_('Profiles|Add key'), class: "js-add-ssh-key-validation-original-submit", pajamas_button: true, data: { qa_selector: 'add_key_button' } + + = f.submit s_('Profiles|Add key'), class: "js-add-ssh-key-validation-original-submit", pajamas_button: true, data: { qa_selector: 'add_key_button' } + = render Pajamas::ButtonComponent.new(button_options: { type: 'reset', class: 'js-add-ssh-key-validation-cancel gl-ml-2 js-toggle-button' }) do + = _('Cancel') diff --git a/app/views/profiles/keys/_key.html.haml b/app/views/profiles/keys/_key.html.haml index 288007ec8061cd58a563cc443aba55aeb51cea7a..7ba42274f88964f568ad7a6414b46f9b120d5407 100644 --- a/app/views/profiles/keys/_key.html.haml +++ b/app/views/profiles/keys/_key.html.haml @@ -1,43 +1,49 @@ - icon_classes = 'settings-list-icon gl-display-none gl-sm-display-block' -%li.key-list-item - .gl-display-flex.gl-align-items-flex-start - .key-list-item-info.gl-w-full.float-none - = link_to path_to_key(key, is_admin), class: "title text-3" do - = key.title +%tr.key-list-item + %td{ data: { label: _('Title'), testid: 'title' } } + = link_to path_to_key(key, is_admin) do + = key.title - .gl-display-flex.gl-align-items-center.gl-mt-2 - - if key.valid? && !key.expired? - = sprite_icon('key', css_class: icon_classes) - - else - %span.gl-display-inline-block.has-tooltip{ title: ssh_key_expiration_tooltip(key) } - = sprite_icon('warning-solid', css_class: icon_classes) + %td{ data: { label: s_('Profiles|Key'), testid: 'key' } } + .gl-align-items-center{ class: 'gl-display-flex! gl-pl-0!' } + - if key.valid? && !key.expired? + = sprite_icon('key', css_class: icon_classes) + - else + %span.gl-display-inline-block.has-tooltip{ title: ssh_key_expiration_tooltip(key) } + = sprite_icon('warning-solid', css_class: icon_classes) + %span.gl-text-truncate.gl-sm-ml-3 + = key.fingerprint - %span.gl-text-truncate.gl-sm-ml-3 - = key.fingerprint + %td{ data: { label: s_('Profiles|Usage type'), testid: 'usage-type' } } + = ssh_key_usage_types.invert[key.usage_type] - .gl-mt-3= html_escape(s_('Profiles|Created%{time_ago}')) % { time_ago: time_ago_with_tooltip(key.created_at, html_class: 'gl-ml-2').html_safe} + %td{ data: { label: s_('Profiles|Created'), testid: 'created' } } + = html_escape(s_('%{time_ago}')) % { time_ago: time_ago_with_tooltip(key.created_at).html_safe} - .key-list-item-dates - %span.last-used-at.gl-mr-3 - = s_('Profiles|Last used:') - -# TODO: Remove this conditional when https://gitlab.com/gitlab-org/gitlab/-/issues/324764 is resolved. - - if Feature.enabled?(:disable_ssh_key_used_tracking) - = _('Unavailable') - = link_to sprite_icon('question-o'), help_page_path('user/ssh.md', anchor: 'view-your-accounts-ssh-keys') - - else - = key.last_used_at ? time_ago_with_tooltip(key.last_used_at) : _('Never') - %span.expires.gl-mr-3 - = key.expired? ? s_('Profiles|Expired:') : s_('Profiles|Expires:') - = key.expires_at ? key.expires_at.to_date : _('Never') - %span.last-used-at.gl-mr-3 - = s_('Profiles|Usage type:') - = ssh_key_usage_types.invert[key.usage_type] - .gl-display-flex.gl-float-right - - if key.can_delete? - - if key.signing? && !is_admin - = render Pajamas::ButtonComponent.new(size: :small, button_options: { class: 'js-confirm-modal-button', data: ssh_key_revoke_modal_data(key, revoke_profile_key_path(key)) }) do - = _('Revoke') - .gl-pl-3 - = render Pajamas::ButtonComponent.new(size: :small, button_options: { class: 'js-confirm-modal-button', data: ssh_key_delete_modal_data(key, path_to_key(key, is_admin)) }) do - = _('Remove') + %td{ data: { label: s_('Profiles|Last used'), testid: 'last-used' } } + -# TODO: Remove this conditional when https://gitlab.com/gitlab-org/gitlab/-/issues/324764 is resolved. + - if Feature.enabled?(:disable_ssh_key_used_tracking) + = _('Unavailable') + = link_to sprite_icon('question-o'), help_page_path('user/ssh.md', anchor: 'view-your-accounts-ssh-keys') + - else + = key.last_used_at ? time_ago_with_tooltip(key.last_used_at) : _('Never') + + %td{ data: { label: s_('Profiles|Expires'), testid: 'expires' } } + - if key.expired? + %span.gl-text-red-500 + = s_('Profiles|Expired') + = key.expires_at.to_date + - elsif key.expires_at + = key.expires_at.to_date + - else + = _('Never') + + %td{ data: { label: _('Actions'), testid: 'actions' } } + %div{ class: 'gl-display-flex! gl-pl-0!' } + - if key.can_delete? + - if key.signing? && !is_admin + = render Pajamas::ButtonComponent.new(size: :small, button_options: { class: 'js-confirm-modal-button', 'aria-label' => _('Revoke'), data: ssh_key_revoke_modal_data(key, revoke_profile_key_path(key)) }) do + = _('Revoke') + .gl-pl-3 + = render Pajamas::ButtonComponent.new(size: :small, icon: 'remove', button_options: { title: _('Remove'), 'aria-label' => _('Remove'), class: 'js-confirm-modal-button', data: ssh_key_delete_modal_data(key, path_to_key(key, is_admin)) }) diff --git a/app/views/profiles/keys/_key_table.html.haml b/app/views/profiles/keys/_key_table.html.haml index 176d7a42002dffaea879e4bfa3636bb0be61a690..cfe507ad65d3e8990076482407b4139db80ee95d 100644 --- a/app/views/profiles/keys/_key_table.html.haml +++ b/app/views/profiles/keys/_key_table.html.haml @@ -1,10 +1,21 @@ - is_admin = local_assigns.fetch(:admin, false) +- hide_class = local_assigns.fetch(:hide_class, false) - if @keys.any? - %ul.content-list.ssh-keys-list{ data: { qa_selector: 'ssh_keys_list' } } - = render partial: 'profiles/keys/key', collection: @keys, locals: { is_admin: is_admin } + .table-holder + %table.table.b-table.gl-table.b-table-stacked-md.gl-mt-n1.gl-mb-n2.ssh-keys-list{ data: { qa_selector: 'ssh_keys_list' } } + %thead.d-none.d-md-table-header-group + %tr + %th= _('Title') + %th= s_('Profiles|Key') + %th= s_('Profiles|Usage type') + %th= s_('Profiles|Created') + %th= s_('Profiles|Last used') + %th= s_('Profiles|Expires') + %th= _('Actions') + = render partial: 'profiles/keys/key', collection: @keys, locals: { is_admin: is_admin } - else - %p.settings-message.text-center + %p.gl-new-card-empty.gl-px-5.gl-py-4.js-toggle-content{ class: hide_class } - if is_admin = _('There are no SSH keys associated with this account.') - else diff --git a/app/views/profiles/keys/index.html.haml b/app/views/profiles/keys/index.html.haml index c2e65dcc8efb575417f8300f1a8de283d3280302..0cd41788a53cdc2d78d7c779a632efef6a9b452d 100644 --- a/app/views/profiles/keys/index.html.haml +++ b/app/views/profiles/keys/index.html.haml @@ -1,6 +1,8 @@ - page_title _('SSH Keys') - add_page_specific_style 'page_bundles/profile' - @force_desktop_expanded_sidebar = true +- add_form_class = 'gl-display-none' if !form_errors(@key) +- hide_class = 'gl-display-none' if form_errors(@key) .settings-section.js-search-settings-section .settings-sticky-header @@ -12,17 +14,24 @@ - config_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_instance_configuration_url } = html_escape(s_('SSH fingerprints verify that the client is connecting to the correct host. Check the %{config_link_start}current instance configuration%{config_link_end}.')) % { config_link_start: config_link_start, config_link_end: '</a>'.html_safe } - %h5.gl-font-lg.gl-mt-0 - = _('Add an SSH key') - %p - - help_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_page_path('user/ssh.md') } - = _('Add an SSH key for secure access to GitLab. %{help_link_start}Learn more%{help_link_end}.').html_safe % {help_link_start: help_link_start, help_link_end: '</a>'.html_safe } - = render 'form' + = render Pajamas::CardComponent.new(card_options: { class: 'gl-new-card js-toggle-container' }, header_options: { class: 'gl-new-card-header' }, body_options: { class: 'gl-new-card-body gl-px-0' }) do |c| + - c.with_header do + .gl-new-card-title-wrapper + %h3.gl-new-card-title + = _('Your SSH keys') + .gl-new-card-count + = sprite_icon('key', css_class: 'gl-mr-2') + = @keys.count + .gl-new-card-actions + = render Pajamas::ButtonComponent.new(size: :small, button_options: { class: "js-toggle-button js-toggle-content #{hide_class}" }) do + = _('Add new key') + - c.with_body do + .gl-new-card-add-form.gl-m-3.js-toggle-content{ class: add_form_class } + %h4.gl-mt-0 + = _('Add an SSH key') + %p + - help_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_page_path('user/ssh.md') } + = _('Add an SSH key for secure access to GitLab. %{help_link_start}Learn more%{help_link_end}.').html_safe % {help_link_start: help_link_start, help_link_end: '</a>'.html_safe } + = render 'form' -.settings-section.js-search-settings-section - .settings-sticky-header - .settings-sticky-header-inner - %h4.gl-my-0 - = _('Your SSH keys (%{count})') % { count: @keys.count } - .gl-mb-3 - = render 'key_table' + = render 'key_table', hide_class: hide_class diff --git a/ee/spec/features/admin/admin_users_spec.rb b/ee/spec/features/admin/admin_users_spec.rb index 4c19c79e3aeb89a4c0431f44c776951371e5e171..ceb327eee678b0a4a5956522c48e61d9c8f93a5d 100644 --- a/ee/spec/features/admin/admin_users_spec.rb +++ b/ee/spec/features/admin/admin_users_spec.rb @@ -193,16 +193,16 @@ def visit_edit_user(user_id) # Check that the regular Key shows the delete icon and the LDAPKey does not # SSH key should be the first in the list - within('ul.content-list li.key-list-item:nth-of-type(1)') do + within('.ssh-keys-list .key-list-item:nth-of-type(1)') do expect(page).to have_content(key2.title) - expect(page).to have_content('Remove') + expect(page).to have_css('[aria-label="Remove"]') expect(page).not_to have_content('Revoke') end # Next, LDAP key - within('ul.content-list li.key-list-item:nth-of-type(2)') do + within('.ssh-keys-list .key-list-item:nth-of-type(2)') do expect(page).to have_content(key1.title) - expect(page).not_to have_content('Remove') + expect(page).not_to have_css('[aria-label="Remove"]') expect(page).not_to have_content('Revoke') end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2d39cc82515435120581687d81eef39f7dd90ad4..867cb7772fee42be6a39e0f6c6f4de50d3c3e43c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1214,6 +1214,9 @@ msgstr "" msgid "%{text} is available" msgstr "" +msgid "%{time_ago}" +msgstr "" + msgid "%{timebox_type} does not support burnup charts" msgstr "" @@ -2860,6 +2863,9 @@ msgstr "" msgid "Add new emoji" msgstr "" +msgid "Add new key" +msgstr "" + msgid "Add new webhook" msgstr "" @@ -35486,9 +35492,6 @@ msgstr "" msgid "Profiles|Created" msgstr "" -msgid "Profiles|Created%{time_ago}" -msgstr "" - msgid "Profiles|Current path: %{path}" msgstr "" @@ -35558,15 +35561,9 @@ msgstr "" msgid "Profiles|Expired" msgstr "" -msgid "Profiles|Expired:" -msgstr "" - msgid "Profiles|Expires" msgstr "" -msgid "Profiles|Expires:" -msgstr "" - msgid "Profiles|Feed token was successfully reset" msgstr "" @@ -35609,9 +35606,6 @@ msgstr "" msgid "Profiles|Last used" msgstr "" -msgid "Profiles|Last used:" -msgstr "" - msgid "Profiles|Linked emails" msgstr "" @@ -35741,9 +35735,6 @@ msgstr "" msgid "Profiles|Usage type" msgstr "" -msgid "Profiles|Usage type:" -msgstr "" - msgid "Profiles|Use a private email - %{email}" msgstr "" @@ -53707,7 +53698,7 @@ msgstr "" msgid "Your SSH key was deleted" msgstr "" -msgid "Your SSH keys (%{count})" +msgid "Your SSH keys" msgstr "" msgid "Your Time-based OTP device was registered!" diff --git a/qa/qa/page/profile/ssh_keys.rb b/qa/qa/page/profile/ssh_keys.rb index 0653df62560e09690f88e412dbc665e0214ddc52..2990ba6a4ac7255be3eebb1056df2389a3aa976b 100644 --- a/qa/qa/page/profile/ssh_keys.rb +++ b/qa/qa/page/profile/ssh_keys.rb @@ -24,6 +24,8 @@ class SSHKeys < Page::Base end def add_key(public_key, title) + click_button('Add new key') + fill_element(:key_public_key_field, public_key) fill_element(:key_title_field, title) # Expire in 2 days just in case the key is created just before midnight diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/ssh_key_support_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/ssh_key_support_spec.rb index 64858287285adc371bff529b7d485f20f01a534c..a2fe79ae65df9ecafe541134569e4e82cf314ba0 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/ssh_key_support_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/ssh_key_support_spec.rb @@ -29,7 +29,7 @@ module QA ssh_keys.remove_key(key.title) end - expect(page).not_to have_content("Title: #{key.title}") + expect(page).not_to have_content(key.title) expect(page).not_to have_content(key.sha256_fingerprint) end end diff --git a/spec/features/profiles/keys_spec.rb b/spec/features/profiles/keys_spec.rb index b49d16603b249c26db030ade42d47cb1a6562916..f54ee96befa7611583d8d02e1a47a08b6d1d3b9e 100644 --- a/spec/features/profiles/keys_spec.rb +++ b/spec/features/profiles/keys_spec.rb @@ -15,6 +15,7 @@ end it 'auto-populates the title', :js do + click_button('Add new key') fill_in('Key', with: attributes_for(:key).fetch(:key)) expect(page).to have_field("Title", with: "dummy@gitlab.com") @@ -23,6 +24,7 @@ it 'saves the new key' do attrs = attributes_for(:key) + click_button('Add new key') fill_in('Key', with: attrs[:key]) fill_in('Title', with: attrs[:title]) click_button('Add key') @@ -35,6 +37,7 @@ it 'shows a confirmable warning if the key begins with an algorithm name that is unsupported' do attrs = attributes_for(:key) + click_button('Add new key') fill_in('Key', with: 'unsupported-ssh-rsa key') fill_in('Title', with: attrs[:title]) click_button('Add key') @@ -60,6 +63,7 @@ it 'shows a validation error' do attrs = attributes_for(:key) + click_button('Add new key') fill_in('Key', with: attrs[:key]) fill_in('Title', with: attrs[:title]) click_button('Add key') @@ -79,13 +83,16 @@ def destroy_key(path, action, confirmation_button) visit path - page.click_button(action) + page.find("button[aria-label=\"#{action}\"]").click page.within('.modal') do page.click_button(confirmation_button) end - expect(page).to have_content('Your SSH keys (0)') + expect(page).to have_content('Your SSH keys') + page.within('.gl-new-card-count') do + expect(page).to have_content('0') + end end describe 'User removes a key', :js do diff --git a/spec/views/profiles/keys/_key.html.haml_spec.rb b/spec/views/profiles/keys/_key.html.haml_spec.rb index 4d14ce7c9090cf5490d8fa74dc550f296dc95c11..9ce6779d3aee54949d79fd48acdff3b0c6c357ed 100644 --- a/spec/views/profiles/keys/_key.html.haml_spec.rb +++ b/spec/views/profiles/keys/_key.html.haml_spec.rb @@ -39,7 +39,7 @@ it 'renders "Unavailable" for last used' do render - expect(rendered).to have_text('Last used: Unavailable') + expect(rendered).to have_text('Unavailable') end end @@ -62,7 +62,7 @@ it 'renders "Never" for last used' do render - expect(rendered).to have_text('Last used: Never') + expect(rendered).to have_text('Never') end end end @@ -85,11 +85,11 @@ expect(rendered).to have_text(usage_type_text) displayed_buttons.each do |button| - expect(rendered).to have_text(button) + expect(rendered).to have_css("button[aria-label=#{button}]") end hidden_buttons.each do |button| - expect(rendered).not_to have_text(button) + expect(rendered).not_to have_css("button[aria-label=#{button}]") end end end @@ -103,17 +103,17 @@ it 'renders "Never" for expires' do render - expect(rendered).to have_text('Expires: Never') + expect(rendered).to have_text('Never') end end context 'when the key has expired' do let_it_be(:key) { create(:personal_key, :expired, user: user) } - it 'renders "Expired:" as the expiration date label' do + it 'renders "Expired" as the expiration date label' do render - expect(rendered).to have_text('Expired:') + expect(rendered).to have_text('Expired') end end