diff options
36 files changed, 323 insertions, 307 deletions
diff --git a/app/controllers/concerns/render_access_tokens.rb b/app/controllers/concerns/render_access_tokens.rb index b0bbad7e37f..43e4686e66f 100644 --- a/app/controllers/concerns/render_access_tokens.rb +++ b/app/controllers/concerns/render_access_tokens.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module RenderAccessTokens extend ActiveSupport::Concern diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb index c1e61e9d672..8bce1ccd778 100644 --- a/app/helpers/ci/status_helper.rb +++ b/app/helpers/ci/status_helper.rb @@ -16,41 +16,41 @@ module Ci # rubocop:disable Metrics/CyclomaticComplexity def ci_icon_for_status(status, size: 16) - if detailed_status?(status) - return sprite_icon(status.icon, size: size) - end - icon_name = - case status - when 'success' - 'status_success' - when 'success-with-warnings' - 'status_warning' - when 'failed' - 'status_failed' - when 'pending' - 'status_pending' - when 'waiting_for_resource' - 'status_pending' - when 'preparing' - 'status_preparing' - when 'running' - 'status_running' - when 'play' - 'play' - when 'created' - 'status_created' - when 'skipped' - 'status_skipped' - when 'manual' - 'status_manual' - when 'scheduled' - 'status_scheduled' + if detailed_status?(status) + status.icon else - 'status_canceled' + case status + when 'success' + 'status_success' + when 'success-with-warnings' + 'status_warning' + when 'failed' + 'status_failed' + when 'pending' + 'status_pending' + when 'waiting-for-resource' + 'status_pending' + when 'preparing' + 'status_preparing' + when 'running' + 'status_running' + when 'play' + 'play' + when 'created' + 'status_created' + when 'skipped' + 'status_skipped' + when 'manual' + 'status_manual' + when 'scheduled' + 'status_scheduled' + else + 'status_canceled' + end end - sprite_icon(icon_name, size: size) + sprite_icon(icon_name, size: size, css_class: 'gl-icon') end # rubocop:enable Metrics/CyclomaticComplexity @@ -68,23 +68,34 @@ module Ci project = commit.project path = pipelines_project_commit_path(project, commit, ref: ref) - render_status_with_link( + render_ci_icon( status, path, tooltip_placement: tooltip_placement, - icon_size: 16) + option_css_classes: 'gl-ml-3' + ) end - def render_status_with_link(status, path = nil, type: _('pipeline'), tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16) + def render_ci_icon( + status, + path = nil, + tooltip_placement: 'left', + option_css_classes: '', + container: 'body', + show_status_text: false + ) variant = badge_variant(status) - klass = "js-ci-status-badge-legacy #{ci_icon_class_for_status(status)} d-inline-flex gl-line-height-1 #{cssclass}" - title = "#{type.titleize}: #{ci_label_for_status(status)}" - data = { toggle: 'tooltip', placement: tooltip_placement, container: container, testid: 'ci-status-badge-legacy' } - badge_classes = 'gl-px-2 gl-ml-3' + klass = "js-ci-status-badge-legacy ci-status-icon #{ci_icon_class_for_status(status)} gl-rounded-full gl-justify-content-center gl-line-height-0" + title = "#{_('Pipeline')}: #{ci_label_for_status(status)}" + data = { toggle: 'tooltip', placement: tooltip_placement, container: container, testid: 'ci-icon' } + badge_classes = "ci-icon gl-p-2 #{option_css_classes}" gl_badge_tag(variant: variant, size: :md, href: path, class: badge_classes, title: title, data: data) do - content_tag :span, ci_icon_for_status(status, size: icon_size), - class: klass + if show_status_text + content_tag(:span, ci_icon_for_status(status), { class: klass }) + content_tag(:span, status.label, { class: 'gl-mx-2 gl-white-space-nowrap' }) + else + content_tag(:span, ci_icon_for_status(status), { class: klass }) + end end end diff --git a/app/models/concerns/analytics/cycle_analytics/stage_event_model.rb b/app/models/concerns/analytics/cycle_analytics/stage_event_model.rb index 1d9cf5729cd..dfcc905b3c3 100644 --- a/app/models/concerns/analytics/cycle_analytics/stage_event_model.rb +++ b/app/models/concerns/analytics/cycle_analytics/stage_event_model.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Analytics module CycleAnalytics module StageEventModel diff --git a/app/models/concerns/commit_signature.rb b/app/models/concerns/commit_signature.rb index 5bdf6bb31bf..201994cb321 100644 --- a/app/models/concerns/commit_signature.rb +++ b/app/models/concerns/commit_signature.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module CommitSignature extend ActiveSupport::Concern diff --git a/app/models/concerns/diff_positionable_note.rb b/app/models/concerns/diff_positionable_note.rb index 2f64129b65f..e799127d69a 100644 --- a/app/models/concerns/diff_positionable_note.rb +++ b/app/models/concerns/diff_positionable_note.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module DiffPositionableNote extend ActiveSupport::Concern diff --git a/app/models/concerns/restricted_signup.rb b/app/models/concerns/restricted_signup.rb index 6af9ede5e8b..87b62214529 100644 --- a/app/models/concerns/restricted_signup.rb +++ b/app/models/concerns/restricted_signup.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module RestrictedSignup extend ActiveSupport::Concern diff --git a/app/presenters/member_presenter.rb b/app/presenters/member_presenter.rb index 4cdaca3c39e..73aae2d85ad 100644 --- a/app/presenters/member_presenter.rb +++ b/app/presenters/member_presenter.rb @@ -15,6 +15,10 @@ class MemberPresenter < Gitlab::View::Presenter::Delegated end end + def valid_member_roles + [] + end + def can_resend_invite? invite? && can?(current_user, admin_member_permission, source) diff --git a/app/serializers/member_entity.rb b/app/serializers/member_entity.rb index 8e5d352e413..e481abf22e3 100644 --- a/app/serializers/member_entity.rb +++ b/app/serializers/member_entity.rb @@ -32,6 +32,7 @@ class MemberEntity < Grape::Entity expose :access_level do expose :human_access, as: :string_value expose :access_level, as: :integer_value + expose :member_role_id end expose :source do |member| @@ -42,6 +43,8 @@ class MemberEntity < Grape::Entity expose :valid_level_roles, as: :valid_roles + expose :valid_member_roles, as: :custom_roles + expose :user, if: -> (member) { member.user.present? } do |member, options| MemberUserEntity.represent(member.user, options) end diff --git a/app/views/ci/status/_badge.html.haml b/app/views/ci/status/_badge.html.haml deleted file mode 100644 index e3b409dea76..00000000000 --- a/app/views/ci/status/_badge.html.haml +++ /dev/null @@ -1,13 +0,0 @@ -- status = local_assigns.fetch(:status) -- link = local_assigns.fetch(:link, true) -- title = local_assigns.fetch(:title, nil) -- css_classes = "gl-display-inline-flex gl-align-items-center gl-gap-2 gl-line-height-0 gl-px-3 gl-py-2 gl-rounded-base ci-status ci-#{status.group} #{'has-tooltip' if title.present?}" - -- if link && status.has_details? - = link_to status.details_path, class: css_classes, title: title, data: { html: title.present? } do - = sprite_icon(status.icon) - = status.text -- else - %span{ class: css_classes, title: title, data: { html: title.present? } } - = sprite_icon(status.icon) - = status.text diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index 48e4472c117..bcb83874bfb 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -1,10 +1,7 @@ - status = local_assigns.fetch(:status) -- tooltip_placement = local_assigns.fetch(:tooltip_placement, "left") - path = local_assigns.fetch(:path, status.has_details? ? status.details_path : nil) -- option_css_classes = local_assigns.fetch(:option_css_classes, '') -- css_classes = "gl-px-2 #{option_css_classes}" -- ci_css_classes = "ci-status-icon ci-status-icon-#{status.group} gl-line-height-1" -- title = s_("PipelineStatusTooltip|Pipeline: %{ci_status}") % {ci_status: status.label} +- tooltip_placement = local_assigns.fetch(:tooltip_placement, "left") +- option_css_classes = local_assigns.fetch(:option_css_classes, nil) +- show_status_text = local_assigns.fetch(:show_status_text, false) -= gl_badge_tag(variant: badge_variant(status), size: :md, href: path, class: css_classes, title: title, data: { toggle: 'tooltip', placement: tooltip_placement, testid: "ci-status-badge" }) do - = content_tag :span, sprite_icon(status.icon, size: 16), class: ci_css_classes += render_ci_icon(status, path, tooltip_placement: tooltip_placement, option_css_classes: option_css_classes, show_status_text: show_status_text) diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 61961172eb2..360e38afb5d 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -28,7 +28,7 @@ .pipeline-status.d-none.d-md-block< - if commit_status - = render 'ci/status/icon', size: 16, status: commit_status + = render 'ci/status/icon', status: commit_status - elsif show_commit_status .gl-display-inline-flex.gl-vertical-align-middle.gl-mr-3 %svg.s16 diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 4017db459a9..a622895b4de 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -14,7 +14,7 @@ %td.status -# Sending 'status' prevents calling the user relation inside the presenter, generating N+1, -# see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68743 - = render "ci/status/badge", status: status, title: job.status_title(status) + = render "ci/status/icon", status: status, show_status_text: true %td - if can?(current_user, :read_build, job) diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml index a3569d41714..e766536f12b 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml @@ -7,7 +7,7 @@ %tr.generic-commit-status{ class: ('retried' if retried) } %td.status - = render 'ci/status/badge', status: generic_commit_status.detailed_status(current_user) + = render 'ci/status/icon', status: generic_commit_status.detailed_status(current_user), show_status_text: true %td = generic_commit_status.name diff --git a/app/views/projects/issues/_related_branches.html.haml b/app/views/projects/issues/_related_branches.html.haml index ba5c41be7c6..1a6edb288b5 100644 --- a/app/views/projects/issues/_related_branches.html.haml +++ b/app/views/projects/issues/_related_branches.html.haml @@ -18,8 +18,8 @@ .item-contents.gl-display-flex.gl-align-items-center.gl-flex-wrap.gl-flex-grow-1.gl-min-h-7 .item-title.gl-display-flex.mb-xl-0.gl-min-w-0.gl-align-items-center - if branch[:pipeline_status].present? - %span.related-branch-ci-status - = render 'ci/status/icon', status: branch[:pipeline_status], option_css_classes: 'gl-display-block gl-mr-3' + %span.gl-mt-n2.gl-mb-n2.gl-mr-3 + = render 'ci/status/icon', status: branch[:pipeline_status] %span.related-branch-info %strong = link_to branch[:name], branch[:link], class: "ref-name" diff --git a/app/views/projects/jobs/_header.html.haml b/app/views/projects/jobs/_header.html.haml index 018ff093475..a77e8f2d0b4 100644 --- a/app/views/projects/jobs/_header.html.haml +++ b/app/views/projects/jobs/_header.html.haml @@ -2,7 +2,7 @@ .content-block.build-header.top-area.page-content-header .header-content - = render 'ci/status/badge', status: @build.detailed_status(current_user), link: false, title: @build.status_title + = render 'ci/status/icon', status: @build.detailed_status(current_user), show_status_text: true %strong Job = link_to "##{@build.id}", project_job_path(@project, @build), class: 'js-build-id' diff --git a/app/views/projects/tags/_tag.html.haml b/app/views/projects/tags/_tag.html.haml index cc49ff9e293..f04d6ab341f 100644 --- a/app/views/projects/tags/_tag.html.haml +++ b/app/views/projects/tags/_tag.html.haml @@ -30,7 +30,7 @@ = render partial: 'projects/commit/signature', object: tag.signature - if commit_status - = render 'ci/status/icon', size: 24, status: commit_status, option_css_classes: 'gl-display-inline-flex gl-vertical-align-middle gl-mr-5' + = render 'ci/status/icon', status: commit_status, option_css_classes: 'gl-display-inline-flex gl-vertical-align-middle gl-mr-5' - elsif @tag_pipeline_statuses && @tag_pipeline_statuses.any? .gl-display-inline-flex.gl-vertical-align-middle.gl-mr-5 %svg.s24 diff --git a/db/post_migrate/20231019084731_swap_columns_for_ci_stages_pipeline_id_bigint_v2.rb b/db/post_migrate/20231019084731_swap_columns_for_ci_stages_pipeline_id_bigint_v2.rb new file mode 100644 index 00000000000..d64d2ea737d --- /dev/null +++ b/db/post_migrate/20231019084731_swap_columns_for_ci_stages_pipeline_id_bigint_v2.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +class SwapColumnsForCiStagesPipelineIdBigintV2 < Gitlab::Database::Migration[2.1] + include Gitlab::Database::MigrationHelpers::WraparoundAutovacuum + include Gitlab::Database::MigrationHelpers::Swapping + + disable_ddl_transaction! + + TABLE_NAME = :ci_stages + TRIGGER_FUNCTION_NAME = :trigger_07bc3c48f407 + COLUMN_NAME = :pipeline_id + BIGINT_COLUMN_NAME = :pipeline_id_convert_to_bigint + FK_NAME = :fk_fb57e6cc56 + BIGINT_FK_NAME = :fk_c5ddde695f + INDEX_NAMES = %i[ + index_ci_stages_on_pipeline_id + index_ci_stages_on_pipeline_id_and_id + index_ci_stages_on_pipeline_id_and_name + index_ci_stages_on_pipeline_id_and_position + ] + BIGINT_INDEX_NAMES = %i[ + index_ci_stages_on_pipeline_id_convert_to_bigint + index_ci_stages_on_pipeline_id_convert_to_bigint_and_id + index_ci_stages_on_pipeline_id_convert_to_bigint_and_name + index_ci_stages_on_pipeline_id_convert_to_bigint_and_position + ] + + def up + return if should_skip? || column_type_of?(:bigint) + + swap + end + + def down + return if should_skip? || column_type_of?(:integer) + + swap + end + + private + + def should_skip? + !can_execute_on?(:ci_pipelines, :ci_stages) + end + + def column_type_of?(type) + column_for(TABLE_NAME, COLUMN_NAME).sql_type.to_s == type.to_s + end + + def swap + # rubocop:disable Migration/WithLockRetriesDisallowedMethod + with_lock_retries(raise_on_exhaustion: true) do + # Lock the tables involved. + lock_tables(:ci_pipelines, :ci_stages) + + # Rename the columns to swap names + swap_columns(TABLE_NAME, COLUMN_NAME, BIGINT_COLUMN_NAME) + + # Reset the trigger function + reset_trigger_function(TRIGGER_FUNCTION_NAME) + + # Swap fkey constraint + swap_foreign_keys(TABLE_NAME, FK_NAME, BIGINT_FK_NAME) + + # Swap index + INDEX_NAMES.each_with_index do |index_name, i| + swap_indexes(TABLE_NAME, index_name, BIGINT_INDEX_NAMES[i]) + end + end + # rubocop:enable Migration/WithLockRetriesDisallowedMethod + end +end diff --git a/db/schema_migrations/20231019084731 b/db/schema_migrations/20231019084731 new file mode 100644 index 00000000000..5c7275172d6 --- /dev/null +++ b/db/schema_migrations/20231019084731 @@ -0,0 +1 @@ +dd4575590153280219bff3b7f37047c67c1a16219c2b6ba18322cb7e295665f7
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index d5c455c5423..c9ad3e81d54 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14299,7 +14299,7 @@ ALTER SEQUENCE ci_sources_projects_id_seq OWNED BY ci_sources_projects.id; CREATE TABLE ci_stages ( project_id integer, - pipeline_id integer, + pipeline_id_convert_to_bigint integer, created_at timestamp without time zone, updated_at timestamp without time zone, name character varying, @@ -14308,7 +14308,7 @@ CREATE TABLE ci_stages ( "position" integer, id bigint NOT NULL, partition_id bigint NOT NULL, - pipeline_id_convert_to_bigint bigint, + pipeline_id bigint, CONSTRAINT check_81b431e49b CHECK ((lock_version IS NOT NULL)) ); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index df4ab7f9c71..f5353530203 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -29298,9 +29298,6 @@ msgstr "" msgid "MemberRole|%{requirement} has to be enabled in order to enable %{permission}." msgstr "" -msgid "MemberRole|%{role} - custom" -msgstr "" - msgid "MemberRole|can't be changed" msgstr "" @@ -34805,9 +34802,6 @@ msgstr "" msgid "PipelineStatusTooltip|Pipeline: %{ciStatus}" msgstr "" -msgid "PipelineStatusTooltip|Pipeline: %{ci_status}" -msgstr "" - msgid "PipelineWizardDefaultCommitMessage|Add %{filename}" msgstr "" @@ -57358,9 +57352,6 @@ msgstr "" msgid "personal access tokens" msgstr "" -msgid "pipeline" -msgstr "" - msgid "pipelineEditorWalkthrough|Let's do this!" msgstr "" diff --git a/spec/features/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb index a5acba1fe4a..2aec5baf351 100644 --- a/spec/features/admin/admin_hooks_spec.rb +++ b/spec/features/admin/admin_hooks_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe 'Admin::Hooks', feature_category: :webhooks do include Spec::Support::Helpers::ModalHelpers - let_it_be(:user) { create(:admin, :no_super_sidebar) } + let_it_be(:user) { create(:admin) } before do sign_in(user) @@ -13,10 +13,10 @@ RSpec.describe 'Admin::Hooks', feature_category: :webhooks do end describe 'GET /admin/hooks' do - it 'is ok' do + it 'is ok', :js do visit admin_root_path - page.within '.nav-sidebar' do + within_testid('super-sidebar') do click_on 'System Hooks', match: :first end diff --git a/spec/features/admin/admin_mode/logout_spec.rb b/spec/features/admin/admin_mode/logout_spec.rb index 5d9106fea02..7a33256e7a8 100644 --- a/spec/features/admin/admin_mode/logout_spec.rb +++ b/spec/features/admin/admin_mode/logout_spec.rb @@ -5,9 +5,8 @@ require 'spec_helper' RSpec.describe 'Admin Mode Logout', :js, feature_category: :system_access do include TermsHelper include UserLoginHelper - include Features::TopNavSpecHelpers - let(:user) { create(:admin, :no_super_sidebar) } + let(:user) { create(:admin) } before do # TODO: This used to use gitlab_sign_in, instead of sign_in, but that is buggy. See @@ -22,11 +21,9 @@ RSpec.describe 'Admin Mode Logout', :js, feature_category: :system_access do expect(page).to have_current_path root_path, ignore_query: true - open_top_nav + click_button 'Search or go to…' - within_top_nav do - expect(page).to have_link(href: new_admin_session_path) - end + expect(page).to have_link(href: new_admin_session_path) end it 'disable shows flash notice' do @@ -45,11 +42,9 @@ RSpec.describe 'Admin Mode Logout', :js, feature_category: :system_access do expect(page).to have_current_path root_path, ignore_query: true - open_top_nav + click_button 'Search or go to…' - within_top_nav do - expect(page).to have_link(href: new_admin_session_path) - end + expect(page).to have_link(href: new_admin_session_path) end end end diff --git a/spec/features/admin/admin_mode/workers_spec.rb b/spec/features/admin/admin_mode/workers_spec.rb index 2a862c750d7..124c43eef9d 100644 --- a/spec/features/admin/admin_mode/workers_spec.rb +++ b/spec/features/admin/admin_mode/workers_spec.rb @@ -6,8 +6,8 @@ require 'spec_helper' RSpec.describe 'Admin mode for workers', :request_store, feature_category: :system_access do include Features::AdminUsersHelpers - let(:user) { create(:user, :no_super_sidebar) } - let(:user_to_delete) { create(:user, :no_super_sidebar) } + let(:user) { create(:user) } + let(:user_to_delete) { create(:user) } before do sign_in(user) @@ -22,7 +22,7 @@ RSpec.describe 'Admin mode for workers', :request_store, feature_category: :syst end context 'as an admin user' do - let(:user) { create(:admin, :no_super_sidebar) } + let(:user) { create(:admin) } context 'when admin mode disabled' do it 'cannot delete user', :js do diff --git a/spec/features/admin/admin_mode_spec.rb b/spec/features/admin/admin_mode_spec.rb index edfa58567ad..b1b44ce143f 100644 --- a/spec/features/admin/admin_mode_spec.rb +++ b/spec/features/admin/admin_mode_spec.rb @@ -4,10 +4,9 @@ require 'spec_helper' RSpec.describe 'Admin mode', :js, feature_category: :shared do include MobileHelpers - include Features::TopNavSpecHelpers include StubENV - let(:admin) { create(:admin, :no_super_sidebar) } + let(:admin) { create(:admin) } before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') @@ -21,20 +20,16 @@ RSpec.describe 'Admin mode', :js, feature_category: :shared do context 'when not in admin mode' do it 'has no leave admin mode button' do visit new_admin_session_path - open_top_nav + open_search_modal - page.within('.navbar-sub-nav') do - expect(page).not_to have_link(href: destroy_admin_session_path) - end + expect(page).not_to have_link(href: destroy_admin_session_path) end it 'can open pages not in admin scope' do visit new_admin_session_path - open_top_nav_projects + open_search_modal - within_top_nav do - click_link('View all projects') - end + click_link('View all my projects') expect(page).to have_current_path(dashboard_projects_path) end @@ -78,29 +73,23 @@ RSpec.describe 'Admin mode', :js, feature_category: :shared do end it 'contains link to leave admin mode' do - open_top_nav + open_search_modal - within_top_nav do - expect(page).to have_link(href: destroy_admin_session_path) - end + expect(page).to have_link(href: destroy_admin_session_path) end it 'can leave admin mode using main dashboard link' do gitlab_disable_admin_mode - open_top_nav + open_search_modal - within_top_nav do - expect(page).to have_link(href: new_admin_session_path) - end + expect(page).to have_link(href: new_admin_session_path) end it 'can open pages not in admin scope' do - open_top_nav_projects + open_search_modal - within_top_nav do - click_link('View all projects') - end + click_link('View all my projects') expect(page).to have_current_path(dashboard_projects_path) end @@ -108,7 +97,7 @@ RSpec.describe 'Admin mode', :js, feature_category: :shared do context 'nav bar' do it 'shows admin dashboard links on bigger screen' do visit root_dashboard_path - open_top_nav + open_search_modal expect(page).to have_link(text: 'Admin', href: admin_root_path, visible: true) expect(page).to have_link(text: 'Leave admin mode', href: destroy_admin_session_path, visible: true) @@ -123,11 +112,9 @@ RSpec.describe 'Admin mode', :js, feature_category: :shared do it 'can leave admin mode' do gitlab_disable_admin_mode - open_top_nav + open_search_modal - within_top_nav do - expect(page).to have_link(href: new_admin_session_path) - end + expect(page).to have_link(href: new_admin_session_path) end end end @@ -141,10 +128,14 @@ RSpec.describe 'Admin mode', :js, feature_category: :shared do it 'shows no admin mode buttons in navbar' do visit admin_root_path - open_top_nav + open_search_modal expect(page).not_to have_link(href: new_admin_session_path) expect(page).not_to have_link(href: destroy_admin_session_path) end end + + def open_search_modal + click_button 'Search or go to…' + end end diff --git a/spec/features/admin/admin_sees_background_migrations_spec.rb b/spec/features/admin/admin_sees_background_migrations_spec.rb index 7423e74bf3a..ae307b8038c 100644 --- a/spec/features/admin/admin_sees_background_migrations_spec.rb +++ b/spec/features/admin/admin_sees_background_migrations_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe "Admin > Admin sees background migrations", feature_category: :database do include ListboxHelpers - let_it_be(:admin) { create(:admin, :no_super_sidebar) } + let_it_be(:admin) { create(:admin) } let(:job_class) { Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJob } let_it_be(:active_migration) { create(:batched_background_migration, :active, table_name: 'active') } @@ -21,16 +21,18 @@ RSpec.describe "Admin > Admin sees background migrations", feature_category: :da gitlab_enable_admin_mode_sign_in(admin) end - it 'can navigate to background migrations' do + it 'can navigate to background migrations', :js do visit admin_root_path - within '.nav-sidebar' do - link = find_link 'Background Migrations' + within_testid('super-sidebar') do + click_on 'Monitoring' + click_on 'Background Migrations' + end - link.click + expect(page).to have_current_path(admin_background_migrations_path) - expect(page).to have_current_path(admin_background_migrations_path) - expect(link).to have_ancestor(:css, 'li.active') + within_testid('super-sidebar') do + expect(page).to have_css('a[aria-current="page"]', text: 'Background Migrations') end end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index aae41cab21f..dbdc0a03404 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do include TermsHelper include UsageDataHelpers - let_it_be(:admin) { create(:admin, :no_super_sidebar) } + let_it_be(:admin) { create(:admin) } context 'application setting :admin_mode is enabled', :request_store do before do @@ -990,15 +990,14 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do end end - context 'Nav bar' do + context 'Nav bar', :js do it 'shows default help links in nav' do default_support_url = "https://#{ApplicationHelper.promo_host}/get-help/" visit root_dashboard_path - find('.header-help-dropdown-toggle').click - - page.within '.header-help' do + within_testid('super-sidebar') do + click_on 'Help' expect(page).to have_link(text: 'Help', href: help_path) expect(page).to have_link(text: 'Support', href: default_support_url) end @@ -1010,9 +1009,8 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do visit root_dashboard_path - find('.header-help-dropdown-toggle').click - - page.within '.header-help' do + within_testid('super-sidebar') do + click_on 'Help' expect(page).to have_link(text: 'Support', href: new_support_url) end end diff --git a/spec/features/admin/users/user_spec.rb b/spec/features/admin/users/user_spec.rb index 7dc329e6909..fbf11063f7c 100644 --- a/spec/features/admin/users/user_spec.rb +++ b/spec/features/admin/users/user_spec.rb @@ -6,8 +6,8 @@ RSpec.describe 'Admin::Users::User', feature_category: :user_management do include Features::AdminUsersHelpers include Spec::Support::Helpers::ModalHelpers - let_it_be(:user) { create(:omniauth_user, :no_super_sidebar, provider: 'twitter', extern_uid: '123456') } - let_it_be(:current_user) { create(:admin, :no_super_sidebar) } + let_it_be(:user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') } + let_it_be(:current_user) { create(:admin) } before do sign_in(current_user) @@ -145,7 +145,7 @@ RSpec.describe 'Admin::Users::User', feature_category: :user_management do end describe 'Impersonation' do - let_it_be(:another_user) { create(:user, :no_super_sidebar) } + let_it_be(:another_user) { create(:user) } context 'before impersonating' do subject { visit admin_user_path(user_to_visit) } @@ -257,15 +257,13 @@ RSpec.describe 'Admin::Users::User', feature_category: :user_management do visit admin_user_path(another_user) end - it 'logs in as the user when impersonate is clicked' do + it 'logs in as the user when impersonate is clicked', :js do subject - find('[data-testid="user-dropdown"]').click - - expect(page.find(:css, '[data-testid="user-profile-link"]')['data-user']).to eql(another_user.username) + expect(page).to have_button("#{another_user.name} user’s menu") end - it 'sees impersonation log out icon' do + it 'sees impersonation log out icon', :js do subject icon = first('[data-testid="incognito-icon"]') @@ -306,8 +304,8 @@ RSpec.describe 'Admin::Users::User', feature_category: :user_management do end end - context 'ending impersonation' do - subject { find(:css, 'li.impersonation a').click } + context 'ending impersonation', :js do + subject { click_on 'Stop impersonating' } before do visit admin_user_path(another_user) @@ -317,9 +315,7 @@ RSpec.describe 'Admin::Users::User', feature_category: :user_management do it 'logs out of impersonated user back to original user' do subject - find('[data-testid="user-dropdown"]').click - - expect(page.find(:css, '[data-testid="user-profile-link"]')['data-user']).to eq(current_user.username) + expect(page).to have_button("#{current_user.name} user’s menu") end it 'is redirected back to the impersonated users page in the admin after stopping' do diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 5f880af37dc..1df1b7373d9 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -81,7 +81,7 @@ RSpec.describe 'Commits', feature_category: :source_code_management do it 'shows correct build status from default branch' do page.within("//li[@id='commit-#{pipeline.short_sha}']") do - expect(page).to have_css("[data-testid='ci-status-badge-legacy']") + expect(page).to have_css("[data-testid='ci-icon']") expect(page).to have_css('.ci-status-icon-success') end end diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb index 90ad6fcea25..65436b35cd7 100644 --- a/spec/features/dashboard/projects_spec.rb +++ b/spec/features/dashboard/projects_spec.rb @@ -153,7 +153,7 @@ RSpec.describe 'Dashboard Projects', feature_category: :groups_and_projects do page.within('[data-testid="project_controls"]') do expect(page).to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit, ref: pipeline.ref)}']") - expect(page).to have_css("[data-testid='ci-status-badge']") + expect(page).to have_css("[data-testid='ci-icon']") expect(page).to have_css('.ci-status-icon-success') expect(page).to have_link('Pipeline: passed') end @@ -165,7 +165,7 @@ RSpec.describe 'Dashboard Projects', feature_category: :groups_and_projects do page.within('[data-testid="project_controls"]') do expect(page).not_to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit, ref: pipeline.ref)}']") - expect(page).not_to have_css("[data-testid='ci-status-badge']") + expect(page).not_to have_css("[data-testid='ci-icon']") expect(page).not_to have_css('.ci-status-icon-success') expect(page).not_to have_link('Pipeline: passed') end diff --git a/spec/fixtures/api/schemas/entities/member.json b/spec/fixtures/api/schemas/entities/member.json index cd8a4e0519b..38f8a245b49 100644 --- a/spec/fixtures/api/schemas/entities/member.json +++ b/spec/fixtures/api/schemas/entities/member.json @@ -11,7 +11,8 @@ "type", "can_update", "can_remove", - "is_direct_member" + "is_direct_member", + "custom_roles" ], "properties": { "id": { @@ -48,7 +49,8 @@ "type": "object", "required": [ "integer_value", - "string_value" + "string_value", + "member_role_id" ], "properties": { "integer_value": { @@ -56,6 +58,12 @@ }, "string_value": { "type": "string" + }, + "member_role_id": { + "type": [ + "integer", + "null" + ] } }, "additionalProperties": false @@ -138,6 +146,26 @@ } }, "additionalProperties": false + }, + "custom_roles": { + "type": "array", + "items": [ + { + "type": "object", + "properties": { + "base_access_level": { + "type": "integer" + }, + "member_role_id": { + "type": "integer" + }, + "name": { + "type": "string" + } + }, + "additionalProperties": false + } + ] } } -}
\ No newline at end of file +} diff --git a/spec/helpers/ci/status_helper_spec.rb b/spec/helpers/ci/status_helper_spec.rb index dc56ec96e3b..18983c83262 100644 --- a/spec/helpers/ci/status_helper_spec.rb +++ b/spec/helpers/ci/status_helper_spec.rb @@ -8,18 +8,6 @@ RSpec.describe Ci::StatusHelper do let(:success_commit) { double("Ci::Pipeline", status: 'success') } let(:failed_commit) { double("Ci::Pipeline", status: 'failed') } - describe '#ci_icon_for_status' do - it 'renders to correct svg on success' do - expect(helper.ci_icon_for_status('success').to_s) - .to include 'status_success' - end - - it 'renders the correct svg on failure' do - expect(helper.ci_icon_for_status('failed').to_s) - .to include 'status_failed' - end - end - describe "#pipeline_status_cache_key" do it "builds a cache key for pipeline status" do pipeline_status = Gitlab::Cache::Ci::ProjectPipelineStatus.new( @@ -33,12 +21,8 @@ RSpec.describe Ci::StatusHelper do end end - describe "#render_status_with_link" do - subject { helper.render_status_with_link("success") } - - it "renders a passed status icon" do - is_expected.to include("<span class=\"js-ci-status-badge-legacy ci-status-icon-success d-inline-flex") - end + describe "#render_ci_icon" do + subject { helper.render_ci_icon("success") } it "has 'Pipeline' as the status type in the title" do is_expected.to include("title=\"Pipeline: passed\"") @@ -49,7 +33,7 @@ RSpec.describe Ci::StatusHelper do end context "when pipeline has commit path" do - subject { helper.render_status_with_link("success", "/commit-path") } + subject { helper.render_ci_icon("success", "/commit-path") } it "links to commit" do is_expected.to include("href=\"/commit-path\"") @@ -64,49 +48,24 @@ RSpec.describe Ci::StatusHelper do end end - context "when different type than pipeline is provided" do - subject { helper.render_status_with_link("success", type: "commit") } - - it "has the provided type in the title" do - is_expected.to include("title=\"Commit: passed\"") - end - end - context "when tooltip_placement is provided" do - subject { helper.render_status_with_link("success", tooltip_placement: "right") } + subject { helper.render_ci_icon("success", tooltip_placement: "right") } it "has the provided tooltip placement" do is_expected.to include("data-placement=\"right\"") end end - context "when additional CSS classes are provided" do - subject { helper.render_status_with_link("success", cssclass: "extra-class") } - - it "has appended extra class to icon classes" do - is_expected.to include('class="js-ci-status-badge-legacy ci-status-icon-success d-inline-flex ' \ - 'gl-line-height-1 extra-class"') - end - end - context "when container is provided" do - subject { helper.render_status_with_link("success", container: "my-container") } + subject { helper.render_ci_icon("success", container: "my-container") } it "has the provided container in data" do is_expected.to include("data-container=\"my-container\"") end end - context "when icon_size is provided" do - subject { helper.render_status_with_link("success", icon_size: 24) } - - it "has the svg class to change size" do - is_expected.to include("<svg class=\"s24\"") - end - end - context "when status is success-with-warnings" do - subject { helper.render_status_with_link("success-with-warnings") } + subject { helper.render_ci_icon("success-with-warnings") } it "renders warning variant of gl-badge" do is_expected.to include('gl-badge badge badge-pill badge-warning') @@ -114,7 +73,7 @@ RSpec.describe Ci::StatusHelper do end context "when status is manual" do - subject { helper.render_status_with_link("manual") } + subject { helper.render_ci_icon("manual") } it "renders neutral variant of gl-badge" do is_expected.to include('gl-badge badge badge-pill badge-neutral') @@ -137,11 +96,40 @@ RSpec.describe Ci::StatusHelper do end with_them do - subject { helper.render_status_with_link(status) } + subject { helper.render_ci_icon(status) } it 'uses the correct badge variant classes for gl-badge' do is_expected.to include("gl-badge badge badge-pill #{expected_badge_variant_class}") end end end + + describe '#ci_icon_for_status' do + using RSpec::Parameterized::TableSyntax + + where(:status, :icon_variant) do + 'success' | 'status_success' + 'success-with-warnings' | 'status_warning' + 'preparing' | 'status_preparing' + 'pending' | 'status_pending' + 'waiting-for-resource' | 'status_pending' + 'failed' | 'status_failed' + 'running' | 'status_running' + 'canceled' | 'status_canceled' + 'created' | 'status_created' + 'scheduled' | 'status_scheduled' + 'play' | 'play' + 'skipped' | 'status_skipped' + 'manual' | 'status_manual' + end + + with_them do + subject { helper.render_ci_icon(status).to_s } + + it 'uses the correct icon variant for status' do + is_expected.to include("ci-status-icon-#{status}") + is_expected.to include(icon_variant) + end + end + end end diff --git a/spec/migrations/20231019084731_swap_columns_for_ci_stages_pipeline_id_bigint_v2_spec.rb b/spec/migrations/20231019084731_swap_columns_for_ci_stages_pipeline_id_bigint_v2_spec.rb new file mode 100644 index 00000000000..266786dda3a --- /dev/null +++ b/spec/migrations/20231019084731_swap_columns_for_ci_stages_pipeline_id_bigint_v2_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe SwapColumnsForCiStagesPipelineIdBigintV2, feature_category: :continuous_integration do + context 'when pipeline_id sql type is integer' do + before do + active_record_base.connection.execute(<<~SQL) + ALTER TABLE ci_stages ALTER COLUMN pipeline_id TYPE integer; + ALTER TABLE ci_stages ALTER COLUMN pipeline_id_convert_to_bigint TYPE bigint; + SQL + end + + it_behaves_like( + 'swap conversion columns', + table_name: :ci_stages, + from: :pipeline_id, + to: :pipeline_id_convert_to_bigint + ) + end + + context 'when pipeline_id sql type is bigint' do + before do + active_record_base.connection.execute(<<~SQL) + ALTER TABLE ci_stages ALTER COLUMN pipeline_id TYPE bigint; + ALTER TABLE ci_stages ALTER COLUMN pipeline_id_convert_to_bigint TYPE integer; + SQL + end + + it 'does nothing' do + recorder = ActiveRecord::QueryRecorder.new { migrate! } + expect(recorder.log).not_to include(/LOCK TABLE/) + expect(recorder.log).not_to include(/ALTER TABLE/) + end + end +end diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index abe21d2b74c..873e4722c14 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -70,7 +70,13 @@ module LoginHelpers # Requires Javascript driver. def gitlab_sign_out - find(".header-user-dropdown-toggle").click + if has_testid?('super-sidebar') + click_on "#{@current_user.name} user’s menu" + else + # This can be removed once https://gitlab.com/gitlab-org/gitlab/-/issues/420121 is complete. + find(".header-user-dropdown-toggle").click + end + click_link "Sign out" @current_user = nil @@ -79,11 +85,8 @@ module LoginHelpers # Requires Javascript driver. def gitlab_disable_admin_mode - open_top_nav - - within_top_nav do - click_on 'Leave admin mode' - end + click_on 'Search or go to…' + click_on 'Leave admin mode' end private diff --git a/spec/views/ci/status/_badge.html.haml_spec.rb b/spec/views/ci/status/_badge.html.haml_spec.rb deleted file mode 100644 index 65497de1608..00000000000 --- a/spec/views/ci/status/_badge.html.haml_spec.rb +++ /dev/null @@ -1,92 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'ci/status/_badge' do - let(:user) { create(:user) } - let(:project) { create(:project, :private) } - let(:pipeline) { create(:ci_pipeline, project: project) } - - context 'when rendering status for build' do - let(:build) do - create(:ci_build, :success, pipeline: pipeline) - end - - context 'when user has ability to see details' do - before do - project.add_developer(user) - end - - it 'has link to build details page' do - details_path = project_job_path(project, build) - - render_status(build) - - expect(rendered).to have_link 'Passed', href: details_path - end - end - - context 'when user do not have ability to see build details' do - before do - render_status(build) - end - - it 'contains build status text' do - expect(rendered).to have_content 'Passed' - end - - it 'does not contain links' do - expect(rendered).not_to have_link 'Passed' - end - end - end - - context 'when rendering status for external job' do - context 'when user has ability to see commit status details' do - before do - project.add_developer(user) - end - - context 'status has external target url' do - before do - external_job = create( - :generic_commit_status, - status: :running, - pipeline: pipeline, - target_url: 'http://gitlab.com' - ) - - render_status(external_job) - end - - it 'contains valid commit status text' do - expect(rendered).to have_content 'Running' - end - - it 'has link to external status page' do - expect(rendered).to have_link 'Running', href: 'http://gitlab.com' - end - end - - context 'status do not have external target url' do - before do - external_job = create(:generic_commit_status, status: :canceled) - - render_status(external_job) - end - - it 'contains valid commit status text' do - expect(rendered).to have_content 'Canceled' - end - - it 'has link to external status page' do - expect(rendered).not_to have_link 'Canceled' - end - end - end - end - - def render_status(resource) - render 'ci/status/badge', status: resource.detailed_status(user) - end -end diff --git a/spec/views/projects/commits/_commit.html.haml_spec.rb b/spec/views/projects/commits/_commit.html.haml_spec.rb index d45f1da86e8..cc73418ea1e 100644 --- a/spec/views/projects/commits/_commit.html.haml_spec.rb +++ b/spec/views/projects/commits/_commit.html.haml_spec.rb @@ -74,7 +74,7 @@ RSpec.describe 'projects/commits/_commit.html.haml' do commit: commit } - expect(rendered).not_to have_css("[data-testid='ci-status-badge-legacy']") + expect(rendered).not_to have_css("[data-testid='ci-icon']") end end @@ -91,7 +91,7 @@ RSpec.describe 'projects/commits/_commit.html.haml' do commit: commit } - expect(rendered).to have_css("[data-testid='ci-status-badge-legacy']") + expect(rendered).to have_css("[data-testid='ci-icon']") end end @@ -103,7 +103,7 @@ RSpec.describe 'projects/commits/_commit.html.haml' do commit: commit } - expect(rendered).not_to have_css("[data-testid='ci-status-badge-legacy']") + expect(rendered).not_to have_css("[data-testid='ci-icon']") end end end diff --git a/spec/views/projects/issues/_related_branches.html.haml_spec.rb b/spec/views/projects/issues/_related_branches.html.haml_spec.rb index deec2db6865..11c398f4e3b 100644 --- a/spec/views/projects/issues/_related_branches.html.haml_spec.rb +++ b/spec/views/projects/issues/_related_branches.html.haml_spec.rb @@ -25,7 +25,6 @@ RSpec.describe 'projects/issues/_related_branches' do expect(rendered).to have_text('other') expect(rendered).to have_link(href: 'link-to-feature') expect(rendered).to have_link(href: 'link-to-other') - expect(rendered).to have_css('.related-branch-ci-status') expect(rendered).to have_css('.ci-status-icon') expect(rendered).to have_css('.related-branch-info') end |