diff options
112 files changed, 2151 insertions, 305 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 049340f90d4..fc29bf071c5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -56,7 +56,7 @@ Style/FrozenStringLiteralComment: - 'qa/**/*' - 'rubocop/**/*' - 'scripts/**/*' - - 'spec/**/*' + - 'spec/lib/**/*' RSpec/FilePath: Exclude: @@ -387,7 +387,6 @@ group :development, :test do gem 'benchmark-ips', '~> 2.3.0', require: false - gem 'license_finder', '~> 5.4', require: false gem 'knapsack', '~> 1.17' gem 'stackprof', '~> 0.2.10', require: false @@ -397,6 +396,11 @@ group :development, :test do gem 'timecop', '~> 0.8.0' end +# Gems required in omnibus-gitlab pipeline +group :development, :test, :omnibus do + gem 'license_finder', '~> 5.4', require: false +end + group :test do gem 'shoulda-matchers', '~> 4.0.1', require: false gem 'email_spec', '~> 2.2.0' diff --git a/app/assets/javascripts/pages/projects/show/index.js b/app/assets/javascripts/pages/projects/show/index.js index 6aa41d0825b..370f3c6e7a2 100644 --- a/app/assets/javascripts/pages/projects/show/index.js +++ b/app/assets/javascripts/pages/projects/show/index.js @@ -48,7 +48,7 @@ document.addEventListener('DOMContentLoaded', () => { leaveByUrl('project'); if (document.getElementById('js-tree-list')) { - import('~/repository') + import('ee_else_ce/repository') .then(m => m.default()) .catch(e => { throw e; diff --git a/app/assets/javascripts/pages/projects/tree/show/index.js b/app/assets/javascripts/pages/projects/tree/show/index.js index 7b90a3a4f6e..16d71379e31 100644 --- a/app/assets/javascripts/pages/projects/tree/show/index.js +++ b/app/assets/javascripts/pages/projects/tree/show/index.js @@ -42,7 +42,7 @@ document.addEventListener('DOMContentLoaded', () => { GpgBadges.fetch(); if (document.getElementById('js-tree-list')) { - import('~/repository') + import('ee_else_ce/repository') .then(m => m.default()) .catch(e => { throw e; diff --git a/app/assets/javascripts/repository/index.js b/app/assets/javascripts/repository/index.js index f9727960040..6a6e7f73188 100644 --- a/app/assets/javascripts/repository/index.js +++ b/app/assets/javascripts/repository/index.js @@ -9,8 +9,10 @@ import { parseBoolean } from '../lib/utils/common_utils'; export default function setupVueRepositoryList() { const el = document.getElementById('js-tree-list'); - const { projectPath, projectShortPath, ref, fullName } = el.dataset; + const { dataset } = el; + const { projectPath, projectShortPath, ref, fullName } = dataset; const router = createRouter(projectPath, ref); + const hideOnRootEls = document.querySelectorAll('.js-hide-on-root'); apolloProvider.clients.defaultClient.cache.writeData({ data: { @@ -35,6 +37,7 @@ export default function setupVueRepositoryList() { document .querySelectorAll('.js-hide-on-navigation') .forEach(elem => elem.classList.toggle('hidden', !isRoot)); + hideOnRootEls.forEach(elem => elem.classList.toggle('hidden', isRoot)); }); const breadcrumbEl = document.getElementById('js-repo-breadcrumb'); @@ -88,7 +91,8 @@ export default function setupVueRepositoryList() { }, }); - return new Vue({ + // eslint-disable-next-line no-new + new Vue({ el, router, apolloProvider, @@ -96,4 +100,6 @@ export default function setupVueRepositoryList() { return h(App); }, }); + + return { router, data: dataset }; } diff --git a/app/controllers/concerns/preview_markdown.rb b/app/controllers/concerns/preview_markdown.rb index a3938ea3652..4189b8dcf96 100644 --- a/app/controllers/concerns/preview_markdown.rb +++ b/app/controllers/concerns/preview_markdown.rb @@ -5,7 +5,7 @@ module PreviewMarkdown # rubocop:disable Gitlab/ModuleWithInstanceVariables def preview_markdown - result = PreviewMarkdownService.new(@project, current_user, params).execute + result = PreviewMarkdownService.new(@project, current_user, markdown_service_params).execute markdown_params = case controller_name @@ -26,6 +26,8 @@ module PreviewMarkdown end # rubocop:enable Gitlab/ModuleWithInstanceVariables + private + def projects_filter_params { issuable_state_filter_enabled: true, @@ -33,10 +35,12 @@ module PreviewMarkdown } end - private - # Override this method to customise the markdown for your controller def preview_markdown_params {} end + + def markdown_service_params + params + end end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 35e364abba3..31c25250745 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -6,6 +6,7 @@ class GroupsController < Groups::ApplicationController include ParamsBackwardCompatibility include PreviewMarkdown include RecordUserLastActivity + extend ::Gitlab::Utils::Override respond_to :html @@ -233,6 +234,11 @@ class GroupsController < Groups::ApplicationController @group.self_and_descendants.public_or_visible_to_user(current_user) end end + + override :markdown_service_params + def markdown_service_params + params.merge(group: group) + end end GroupsController.prepend_if_ee('EE::GroupsController') diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index 4f31cc67ccc..404ea7b00d4 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -66,7 +66,7 @@ module GitlabRoutingHelper end def preview_markdown_path(parent, *args) - return group_preview_markdown_path(parent) if parent.is_a?(Group) + return group_preview_markdown_path(parent, *args) if parent.is_a?(Group) if @snippet.is_a?(PersonalSnippet) preview_markdown_snippets_path diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb index ea7c7af72d3..19a27ba3499 100644 --- a/app/helpers/services_helper.rb +++ b/app/helpers/services_helper.rb @@ -32,7 +32,7 @@ module ServicesHelper end def service_save_button(service) - button_tag(class: 'btn btn-success', type: 'submit', disabled: service.deprecated?) do + button_tag(class: 'btn btn-success', type: 'submit', disabled: service.deprecated?, data: { qa_selector: 'save_changes_button' }) do icon('spinner spin', class: 'hidden js-btn-spinner') + content_tag(:span, 'Save changes', class: 'js-btn-label') end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index afa057421e0..1020c91b245 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -186,6 +186,15 @@ module TreeHelper attrs end + + def vue_file_list_data(project, ref) + { + project_path: project.full_path, + project_short_path: project.path, + ref: ref, + full_name: project.name_with_namespace + } + end end TreeHelper.prepend_if_ee('::EE::TreeHelper') diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index acd744bfaf5..0db1fe9d6dc 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -6,6 +6,7 @@ module Clusters include Gitlab::Utils::StrongMemoize include FromUnion include ReactiveCaching + include AfterCommitQueue self.table_name = 'clusters' @@ -126,7 +127,55 @@ module Clusters hierarchy_groups.flat_map(&:clusters) + Instance.new.clusters end + state_machine :cleanup_status, initial: :cleanup_not_started do + state :cleanup_not_started, value: 1 + state :cleanup_uninstalling_applications, value: 2 + state :cleanup_removing_project_namespaces, value: 3 + state :cleanup_removing_service_account, value: 4 + state :cleanup_errored, value: 5 + + event :start_cleanup do |cluster| + transition [:cleanup_not_started, :cleanup_errored] => :cleanup_uninstalling_applications + end + + event :continue_cleanup do + transition( + cleanup_uninstalling_applications: :cleanup_removing_project_namespaces, + cleanup_removing_project_namespaces: :cleanup_removing_service_account) + end + + event :make_cleanup_errored do + transition any => :cleanup_errored + end + + before_transition any => [:cleanup_errored] do |cluster, transition| + status_reason = transition.args.first + cluster.cleanup_status_reason = status_reason if status_reason + end + + after_transition [:cleanup_not_started, :cleanup_errored] => :cleanup_uninstalling_applications do |cluster| + cluster.run_after_commit do + Clusters::Cleanup::AppWorker.perform_async(cluster.id) + end + end + + after_transition cleanup_uninstalling_applications: :cleanup_removing_project_namespaces do |cluster| + cluster.run_after_commit do + Clusters::Cleanup::ProjectNamespaceWorker.perform_async(cluster.id) + end + end + + after_transition cleanup_removing_project_namespaces: :cleanup_removing_service_account do |cluster| + cluster.run_after_commit do + Clusters::Cleanup::ServiceAccountWorker.perform_async(cluster.id) + end + end + end + def status_name + return cleanup_status_name if cleanup_errored? + return :cleanup_ongoing unless cleanup_not_started? + provider&.status_name || connection_status.presence || :created end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index cd8ede3905a..250bac95b82 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -209,14 +209,20 @@ class MergeRequest < ApplicationRecord scope :by_target_branch, ->(branch_name) { where(target_branch: branch_name) } scope :preload_source_project, -> { preload(:source_project) } - scope :with_open_merge_when_pipeline_succeeds, -> do - with_state(:opened).where(merge_when_pipeline_succeeds: true) + scope :with_auto_merge_enabled, -> do + with_state(:opened).where(auto_merge_enabled: true) end after_save :keep_around_commit alias_attribute :project, :target_project alias_attribute :project_id, :target_project_id + + # Currently, `merge_when_pipeline_succeeds` column is used as a flag + # to check if _any_ auto merge strategy is activated on the merge request. + # Today, we have multiple strategies and MWPS is one of them. + # we'd eventually rename the column for avoiding confusions, but in the mean time + # please use `auto_merge_enabled` alias instead of `merge_when_pipeline_succeeds`. alias_attribute :auto_merge_enabled, :merge_when_pipeline_succeeds alias_method :issuing_parent, :target_project diff --git a/app/presenters/todo_presenter.rb b/app/presenters/todo_presenter.rb index b57fc712c5a..291be7848e2 100644 --- a/app/presenters/todo_presenter.rb +++ b/app/presenters/todo_presenter.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true class TodoPresenter < Gitlab::View::Presenter::Delegated - include GlobalID::Identification - presents :todo end diff --git a/app/serializers/diff_file_base_entity.rb b/app/serializers/diff_file_base_entity.rb index ee68b4b98e0..302fe3d7c67 100644 --- a/app/serializers/diff_file_base_entity.rb +++ b/app/serializers/diff_file_base_entity.rb @@ -89,6 +89,14 @@ class DiffFileBaseEntity < Grape::Entity expose :viewer, using: DiffViewerEntity + expose :old_size do |diff_file| + diff_file.old_blob&.raw_size + end + + expose :new_size do |diff_file| + diff_file.new_blob&.raw_size + end + private def memoized_submodule_links(diff_file, options) diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index b32499629ff..bd3fcf85a62 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -152,7 +152,8 @@ module MergeRequests def abort_ff_merge_requests_with_when_pipeline_succeeds return unless @project.ff_merge_must_be_possible? - requests_with_auto_merge_enabled_to(@push.branch_name).each do |merge_request| + merge_requests_with_auto_merge_enabled_to(@push.branch_name).each do |merge_request| + next unless merge_request.auto_merge_strategy == AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS next unless merge_request.should_be_rebased? abort_auto_merge_with_todo(merge_request, 'target branch was updated') @@ -167,11 +168,11 @@ module MergeRequests todo_service.merge_request_became_unmergeable(merge_request) end - def requests_with_auto_merge_enabled_to(target_branch) + def merge_requests_with_auto_merge_enabled_to(target_branch) @project .merge_requests .by_target_branch(target_branch) - .with_open_merge_when_pipeline_succeeds + .with_auto_merge_enabled end def mark_pending_todos_done diff --git a/app/services/preview_markdown_service.rb b/app/services/preview_markdown_service.rb index 2b4c4ae68e2..afe2651b11a 100644 --- a/app/services/preview_markdown_service.rb +++ b/app/services/preview_markdown_service.rb @@ -16,8 +16,12 @@ class PreviewMarkdownService < BaseService private + def quick_action_types + %w(Issue MergeRequest Commit) + end + def explain_quick_actions(text) - return text, [] unless %w(Issue MergeRequest Commit).include?(target_type) + return text, [] unless quick_action_types.include?(target_type) quick_actions_service = QuickActions::InterpretService.new(project, current_user) quick_actions_service.explain(text, find_commands_target) @@ -51,7 +55,7 @@ class PreviewMarkdownService < BaseService def find_commands_target QuickActions::TargetService - .new(project, current_user) + .new(project, current_user, group: params[:group]) .execute(target_type, target_id) end @@ -63,3 +67,5 @@ class PreviewMarkdownService < BaseService params[:target_id] end end + +PreviewMarkdownService.prepend_if_ee('EE::PreviewMarkdownService') diff --git a/app/services/quick_actions/target_service.rb b/app/services/quick_actions/target_service.rb index 69464c3c1ae..4273acfbf8b 100644 --- a/app/services/quick_actions/target_service.rb +++ b/app/services/quick_actions/target_service.rb @@ -32,3 +32,5 @@ module QuickActions end end end + +QuickActions::TargetService.prepend_if_ee('EE::QuickActions::TargetService') diff --git a/app/views/admin/application_settings/_outbound.html.haml b/app/views/admin/application_settings/_outbound.html.haml index ad26f52aea7..42528f40123 100644 --- a/app/views/admin/application_settings/_outbound.html.haml +++ b/app/views/admin/application_settings/_outbound.html.haml @@ -4,7 +4,7 @@ %fieldset .form-group .form-check - = f.check_box :allow_local_requests_from_web_hooks_and_services, class: 'form-check-input' + = f.check_box :allow_local_requests_from_web_hooks_and_services, class: 'form-check-input', data: { qa_selector: 'allow_requests_from_services_checkbox' } = f.label :allow_local_requests_from_web_hooks_and_services, class: 'form-check-label' do = _('Allow requests to the local network from web hooks and services') .form-check @@ -27,4 +27,4 @@ %span.form-text.text-muted = _('Resolves IP addresses once and uses them to submit requests') - = f.submit 'Save changes', class: "btn btn-success" + = f.submit 'Save changes', class: "btn btn-success", data: { qa_selector: 'save_changes_button' } diff --git a/app/views/admin/application_settings/network.html.haml b/app/views/admin/application_settings/network.html.haml index 092834b993c..7bd51172195 100644 --- a/app/views/admin/application_settings/network.html.haml +++ b/app/views/admin/application_settings/network.html.haml @@ -24,7 +24,7 @@ .settings-content = render 'ip_limits' -%section.settings.as-outbound.no-animate#js-outbound-settings{ class: ('expanded' if expanded_by_default?) } +%section.settings.as-outbound.no-animate#js-outbound-settings{ class: ('expanded' if expanded_by_default?), data: { qa_selector: 'outbound_requests_section' } } .settings-header %h4 = _('Outbound requests') diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index ec27f3c24df..247fbfefde9 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -163,7 +163,7 @@ - if project_nav_tab? :pipelines = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :artifacts]) do - = link_to project_pipelines_path(@project), class: 'shortcuts-pipelines qa-link-pipelines rspec-link-pipelines' do + = link_to project_pipelines_path(@project), class: 'shortcuts-pipelines qa-link-pipelines rspec-link-pipelines', data: { qa_selector: 'ci_cd_link' } do .nav-icon-container = sprite_icon('rocket') %span.nav-item-name#js-onboarding-pipelines-link @@ -347,7 +347,7 @@ = _('Members') - if can_edit = nav_link(controller: [:integrations, :services, :hooks, :hook_logs]) do - = link_to project_settings_integrations_path(@project), title: _('Integrations') do + = link_to project_settings_integrations_path(@project), title: _('Integrations'), data: { qa_selector: 'integrations_settings_link' } do %span = _('Integrations') = nav_link(controller: :repository) do diff --git a/app/views/projects/_files.html.haml b/app/views/projects/_files.html.haml index 95fdad125a7..3c0dfd4c029 100644 --- a/app/views/projects/_files.html.haml +++ b/app/views/projects/_files.html.haml @@ -19,7 +19,7 @@ = render 'stat_anchor_list', anchors: @project.statistics_buttons(show_auto_devops_callout: show_auto_devops_callout) - if vue_file_list_enabled? - #js-tree-list{ data: { project_path: @project.full_path, project_short_path: @project.path, ref: ref, full_name: @project.name_with_namespace } } + #js-tree-list{ data: vue_file_list_data(project, ref) } - if can_edit_tree? = render 'projects/blob/upload', title: _('Upload New File'), placeholder: _('Upload New File'), button_title: _('Upload file'), form_path: project_create_blob_path(@project, @id), method: :post = render 'projects/blob/new_dir' diff --git a/app/views/projects/services/_index.html.haml b/app/views/projects/services/_index.html.haml index 7748a7a6a8e..3f33d72d3ec 100644 --- a/app/views/projects/services/_index.html.haml +++ b/app/views/projects/services/_index.html.haml @@ -21,7 +21,7 @@ %td{ "aria-label" => (service.activated? ? s_("ProjectService|%{service_title}: status on") : s_("ProjectService|%{service_title}: status off")) % { service_title: service.title } } = boolean_to_icon service.activated? %td - = link_to edit_project_service_path(@project, service.to_param) do + = link_to edit_project_service_path(@project, service.to_param), { data: { qa_selector: "#{service.title.downcase.gsub(/[\s\(\)]/,'_')}_link" } } do %strong= service.title %td.d-none.d-sm-block = service.description diff --git a/app/views/shared/_field.html.haml b/app/views/shared/_field.html.haml index 606d0f241aa..a7ad6d6f2c4 100644 --- a/app/views/shared/_field.html.haml +++ b/app/views/shared/_field.html.haml @@ -16,7 +16,7 @@ = form.label name, title, class: "col-form-label col-sm-2" .col-sm-10 - if type == 'text' - = form.text_field name, class: "form-control", placeholder: placeholder, required: required, disabled: disabled + = form.text_field name, class: "form-control", placeholder: placeholder, required: required, disabled: disabled, data: { qa_selector: "#{name.downcase.gsub('\s', '')}_field" } - elsif type == 'textarea' = form.text_area name, rows: 5, class: "form-control", placeholder: placeholder, required: required, disabled: disabled - elsif type == 'checkbox' @@ -24,6 +24,6 @@ - elsif type == 'select' = form.select name, options_for_select(choices, value ? value : default_choice), {}, { class: "form-control", disabled: disabled} - elsif type == 'password' - = form.password_field name, autocomplete: "new-password", placeholder: placeholder, class: "form-control", required: value.blank? && required, disabled: disabled + = form.password_field name, autocomplete: "new-password", placeholder: placeholder, class: "form-control", required: value.blank? && required, disabled: disabled, data: { qa_selector: "#{name.downcase.gsub('\s', '')}_field" } - if help %span.form-text.text-muted= help diff --git a/app/views/shared/_service_settings.html.haml b/app/views/shared/_service_settings.html.haml index 6fa61c15493..627a1eb6eae 100644 --- a/app/views/shared/_service_settings.html.haml +++ b/app/views/shared/_service_settings.html.haml @@ -12,7 +12,7 @@ .form-group.row = form.label :active, "Active", class: "col-form-label col-sm-2" .col-sm-10 - = form.check_box :active, disabled: disable_fields_service?(@service) + = form.check_box :active, disabled: disable_fields_service?(@service), data: { qa_selector: 'active_checkbox' } - if @service.configurable_events.present? .form-group.row diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index b161cc65602..10081840305 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -45,6 +45,9 @@ - gcp_cluster:cluster_project_configure - gcp_cluster:clusters_applications_wait_for_uninstall_app - gcp_cluster:clusters_applications_uninstall +- gcp_cluster:clusters_cleanup_app +- gcp_cluster:clusters_cleanup_project_namespace +- gcp_cluster:clusters_cleanup_service_account - github_import_advance_stage - github_importer:github_import_import_diff_note diff --git a/app/workers/clusters/cleanup/app_worker.rb b/app/workers/clusters/cleanup/app_worker.rb new file mode 100644 index 00000000000..1eedf510ba1 --- /dev/null +++ b/app/workers/clusters/cleanup/app_worker.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Clusters + module Cleanup + class AppWorker + include ApplicationWorker + include ClusterQueue + include ClusterApplications + + # TODO: Merge with https://gitlab.com/gitlab-org/gitlab/merge_requests/16954 + # We're splitting the above MR in smaller chunks to facilitate reviews + def perform + end + end + end +end diff --git a/app/workers/clusters/cleanup/project_namespace_worker.rb b/app/workers/clusters/cleanup/project_namespace_worker.rb new file mode 100644 index 00000000000..09f2abf5d8a --- /dev/null +++ b/app/workers/clusters/cleanup/project_namespace_worker.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Clusters + module Cleanup + class ProjectNamespaceWorker + include ApplicationWorker + include ClusterQueue + include ClusterApplications + + # TODO: Merge with https://gitlab.com/gitlab-org/gitlab/merge_requests/16954 + # We're splitting the above MR in smaller chunks to facilitate reviews + def perform + end + end + end +end diff --git a/app/workers/clusters/cleanup/service_account_worker.rb b/app/workers/clusters/cleanup/service_account_worker.rb new file mode 100644 index 00000000000..fab6318a807 --- /dev/null +++ b/app/workers/clusters/cleanup/service_account_worker.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Clusters + module Cleanup + class ServiceAccountWorker + include ApplicationWorker + include ClusterQueue + include ClusterApplications + + # TODO: Merge with https://gitlab.com/gitlab-org/gitlab/merge_requests/16954 + # We're splitting the above MR in smaller chunks to facilitate reviews + def perform + end + end + end +end diff --git a/changelogs/unreleased/34850-fix-graphql-todo-ids.yml b/changelogs/unreleased/34850-fix-graphql-todo-ids.yml new file mode 100644 index 00000000000..ba3d63a2ee5 --- /dev/null +++ b/changelogs/unreleased/34850-fix-graphql-todo-ids.yml @@ -0,0 +1,5 @@ +--- +title: Fix Todo IDs in GraphQL API +merge_request: 19068 +author: +type: fixed diff --git a/changelogs/unreleased/8199-epic-quick-actions-preview.yml b/changelogs/unreleased/8199-epic-quick-actions-preview.yml new file mode 100644 index 00000000000..640c2b47c6f --- /dev/null +++ b/changelogs/unreleased/8199-epic-quick-actions-preview.yml @@ -0,0 +1,5 @@ +--- +title: Fix previewing quick actions for epics +merge_request: 19042 +author: +type: fixed diff --git a/changelogs/unreleased/do-not-abort-merge-trains-on-ff.yml b/changelogs/unreleased/do-not-abort-merge-trains-on-ff.yml new file mode 100644 index 00000000000..cdaf004c553 --- /dev/null +++ b/changelogs/unreleased/do-not-abort-merge-trains-on-ff.yml @@ -0,0 +1,5 @@ +--- +title: Abort only MWPS when FF only merge is impossible +merge_request: 18591 +author: +type: fixed diff --git a/changelogs/unreleased/feature-cluster-cleanup-state-machine.yml b/changelogs/unreleased/feature-cluster-cleanup-state-machine.yml new file mode 100644 index 00000000000..0de86de090d --- /dev/null +++ b/changelogs/unreleased/feature-cluster-cleanup-state-machine.yml @@ -0,0 +1,5 @@ +--- +title: Add cleanup status to clusters +merge_request: 18144 +author: +type: added diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index de396ea1f32..7f8ba35bf41 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -100,6 +100,7 @@ - [create_evidence, 2] # EE-specific queues + - [analytics, 1] - [ldap_group_sync, 2] - [create_github_webhook, 2] - [geo, 1] diff --git a/db/migrate/20190918104731_add_cleanup_status_to_cluster.rb b/db/migrate/20190918104731_add_cleanup_status_to_cluster.rb new file mode 100644 index 00000000000..0ba9d8e6c89 --- /dev/null +++ b/db/migrate/20190918104731_add_cleanup_status_to_cluster.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddCleanupStatusToCluster < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:clusters, :cleanup_status, + :smallint, + default: 1, + allow_null: false) + end + + def down + remove_column(:clusters, :cleanup_status) + end +end diff --git a/db/migrate/20190918121135_add_cleanup_status_reason_to_cluster.rb b/db/migrate/20190918121135_add_cleanup_status_reason_to_cluster.rb new file mode 100644 index 00000000000..4e71905e3a3 --- /dev/null +++ b/db/migrate/20190918121135_add_cleanup_status_reason_to_cluster.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class AddCleanupStatusReasonToCluster < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def change + add_column :clusters, :cleanup_status_reason, :text + end +end diff --git a/db/migrate/20191023132005_add_merge_requests_index_on_target_project_and_branch.rb b/db/migrate/20191023132005_add_merge_requests_index_on_target_project_and_branch.rb new file mode 100644 index 00000000000..a3de3f34c44 --- /dev/null +++ b/db/migrate/20191023132005_add_merge_requests_index_on_target_project_and_branch.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddMergeRequestsIndexOnTargetProjectAndBranch < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :merge_requests, [:target_project_id, :target_branch], + where: "state_id = 1 AND merge_when_pipeline_succeeds = true" + end + + def down + remove_concurrent_index :merge_requests, [:target_project_id, :target_branch] + end +end diff --git a/db/schema.rb b/db/schema.rb index 39c4f3005be..09149cfbcfe 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1041,6 +1041,8 @@ ActiveRecord::Schema.define(version: 2019_10_26_041447) do t.boolean "managed", default: true, null: false t.boolean "namespace_per_environment", default: true, null: false t.integer "management_project_id" + t.integer "cleanup_status", limit: 2, default: 1, null: false + t.text "cleanup_status_reason" t.index ["enabled"], name: "index_clusters_on_enabled" t.index ["management_project_id"], name: "index_clusters_on_management_project_id", where: "(management_project_id IS NOT NULL)" t.index ["user_id"], name: "index_clusters_on_user_id" @@ -2340,6 +2342,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_041447) do t.index ["target_project_id", "iid"], name: "index_merge_requests_on_target_project_id_and_iid", unique: true t.index ["target_project_id", "iid"], name: "index_merge_requests_on_target_project_id_and_iid_opened", where: "((state)::text = 'opened'::text)" t.index ["target_project_id", "merge_commit_sha", "id"], name: "index_merge_requests_on_tp_id_and_merge_commit_sha_and_id" + t.index ["target_project_id", "target_branch"], name: "index_merge_requests_on_target_project_id_and_target_branch", where: "((state_id = 1) AND (merge_when_pipeline_succeeds = true))" t.index ["title"], name: "index_merge_requests_on_title" t.index ["title"], name: "index_merge_requests_on_title_trigram", opclass: :gin_trgm_ops, using: :gin t.index ["updated_by_id"], name: "index_merge_requests_on_updated_by_id", where: "(updated_by_id IS NOT NULL)" diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 05786319d96..f7d45b3882a 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -146,6 +146,10 @@ query($project_path: ID!) { } ``` +To ensure that we get consistent ordering, we will append an ordering on the primary +key, in descending order. This is usually `id`, so basically we will add `order(id: :desc)` +to the end of the relation. A primary key _must_ be available on the underlying table. + ### Exposing permissions for a type To expose permissions the current user has on a resource, you can call diff --git a/doc/development/testing_guide/end_to_end/feature_flags.md b/doc/development/testing_guide/end_to_end/feature_flags.md new file mode 100644 index 00000000000..3238ec716bf --- /dev/null +++ b/doc/development/testing_guide/end_to_end/feature_flags.md @@ -0,0 +1,25 @@ +# Testing with feature flags + +To run a specific test with a feature flag enabled you can use the `QA::Runtime::Feature` class to enabled and disable feature flags ([via the API](../../../api/features.md)). + +```ruby +context "with feature flag enabled" do + before do + Runtime::Feature.enable('feature_flag_name') + end + + it "feature flag test" do + # Execute a test with a feature flag enabled + end + + after do + Runtime::Feature.disable('feature_flag_name') + end +end +``` + +## Running a scenario with a feature flag enabled + +It's also possible to run an entire scenario with a feature flag enabled, without having to edit existing tests or write new ones. + +Please see the [QA readme](https://gitlab.com/gitlab-org/gitlab/tree/master/qa#running-tests-with-a-feature-flag-enabled) for details. diff --git a/doc/development/testing_guide/end_to_end/index.md b/doc/development/testing_guide/end_to_end/index.md index a9fb4be284e..27470eb2752 100644 --- a/doc/development/testing_guide/end_to_end/index.md +++ b/doc/development/testing_guide/end_to_end/index.md @@ -130,6 +130,7 @@ Continued reading: - [Quick Start Guide](quick_start_guide.md) - [Style Guide](style_guide.md) - [Best Practices](best_practices.md) +- [Testing with feature flags](feature_flags.md) ## Where can I ask for help? diff --git a/lib/gitlab/graphql/connections.rb b/lib/gitlab/graphql/connections.rb index fbccdfa7b08..64f7a268b7e 100644 --- a/lib/gitlab/graphql/connections.rb +++ b/lib/gitlab/graphql/connections.rb @@ -6,7 +6,7 @@ module Gitlab def self.use(_schema) GraphQL::Relay::BaseConnection.register_connection_implementation( ActiveRecord::Relation, - Gitlab::Graphql::Connections::KeysetConnection + Gitlab::Graphql::Connections::Keyset::Connection ) end end diff --git a/lib/gitlab/graphql/connections/keyset/conditions/base_condition.rb b/lib/gitlab/graphql/connections/keyset/conditions/base_condition.rb new file mode 100644 index 00000000000..22728cc0b65 --- /dev/null +++ b/lib/gitlab/graphql/connections/keyset/conditions/base_condition.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Connections + module Keyset + module Conditions + class BaseCondition + def initialize(arel_table, names, values, operator, before_or_after) + @arel_table, @names, @values, @operator, @before_or_after = arel_table, names, values, operator, before_or_after + end + + def build + raise NotImplementedError + end + + private + + attr_reader :arel_table, :names, :values, :operator, :before_or_after + + def table_condition(attribute, value, operator) + case operator + when '>' + arel_table[attribute].gt(value) + when '<' + arel_table[attribute].lt(value) + when '=' + arel_table[attribute].eq(value) + when 'is_null' + arel_table[attribute].eq(nil) + when 'is_not_null' + arel_table[attribute].not_eq(nil) + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/graphql/connections/keyset/conditions/not_null_condition.rb b/lib/gitlab/graphql/connections/keyset/conditions/not_null_condition.rb new file mode 100644 index 00000000000..3b56ddb996d --- /dev/null +++ b/lib/gitlab/graphql/connections/keyset/conditions/not_null_condition.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Connections + module Keyset + module Conditions + class NotNullCondition < BaseCondition + def build + conditions = [first_attribute_condition] + + # If there is only one order field, we can assume it + # does not contain NULLs, and don't need additional + # conditions + unless names.count == 1 + conditions << [second_attribute_condition, final_condition] + end + + conditions.join + end + + private + + # ex: "(relative_position > 23)" + def first_attribute_condition + <<~SQL + (#{table_condition(names.first, values.first, operator.first).to_sql}) + SQL + end + + # ex: " OR (relative_position = 23 AND id > 500)" + def second_attribute_condition + condition = <<~SQL + OR ( + #{table_condition(names.first, values.first, '=').to_sql} + AND + #{table_condition(names[1], values[1], operator[1]).to_sql} + ) + SQL + + condition + end + + # ex: " OR (relative_position IS NULL)" + def final_condition + if before_or_after == :after + <<~SQL + OR (#{table_condition(names.first, nil, 'is_null').to_sql}) + SQL + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/graphql/connections/keyset/conditions/null_condition.rb b/lib/gitlab/graphql/connections/keyset/conditions/null_condition.rb new file mode 100644 index 00000000000..71a74936d5d --- /dev/null +++ b/lib/gitlab/graphql/connections/keyset/conditions/null_condition.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Connections + module Keyset + module Conditions + class NullCondition < BaseCondition + def build + [first_attribute_condition, final_condition].join + end + + private + + # ex: "(relative_position IS NULL AND id > 500)" + def first_attribute_condition + condition = <<~SQL + ( + #{table_condition(names.first, nil, 'is_null').to_sql} + AND + #{table_condition(names[1], values[1], operator[1]).to_sql} + ) + SQL + + condition + end + + # ex: " OR (relative_position IS NOT NULL)" + def final_condition + if before_or_after == :before + <<~SQL + OR (#{table_condition(names.first, nil, 'is_not_null').to_sql}) + SQL + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/graphql/connections/keyset/connection.rb b/lib/gitlab/graphql/connections/keyset/connection.rb new file mode 100644 index 00000000000..0daf726c005 --- /dev/null +++ b/lib/gitlab/graphql/connections/keyset/connection.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true + +# Keyset::Connection provides cursor based pagination, to avoid using OFFSET. +# It basically sorts / filters using WHERE sorting_value > cursor. +# We do this for performance reasons (https://gitlab.com/gitlab-org/gitlab-foss/issues/45756), +# as well as for having stable pagination +# https://graphql-ruby.org/pro/cursors.html#whats-the-difference +# https://coderwall.com/p/lkcaag/pagination-you-re-probably-doing-it-wrong +# +# It currently supports sorting on two columns, but the last column must +# be the primary key. For example +# +# Issue.order(created_at: :asc).order(:id) +# Issue.order(due_date: :asc).order(:id) +# +# It will tolerate non-attribute ordering, but only attributes determine the cursor. +# For example, this is legitimate: +# +# Issue.order('issues.due_date IS NULL').order(due_date: :asc).order(:id) +# +# but anything more complex has a chance of not working. +# +module Gitlab + module Graphql + module Connections + module Keyset + class Connection < GraphQL::Relay::BaseConnection + include Gitlab::Utils::StrongMemoize + + # TODO https://gitlab.com/gitlab-org/gitlab/issues/35104 + include Gitlab::Graphql::Connections::Keyset::LegacyKeysetConnection + + def cursor_from_node(node) + return legacy_cursor_from_node(node) if use_legacy_pagination? + + encoded_json_from_ordering(node) + end + + def sliced_nodes + return legacy_sliced_nodes if use_legacy_pagination? + + @sliced_nodes ||= + begin + OrderInfo.validate_ordering(ordered_nodes, order_list) + + sliced = ordered_nodes + sliced = slice_nodes(sliced, before, :before) if before.present? + sliced = slice_nodes(sliced, after, :after) if after.present? + + sliced + end + end + + def paged_nodes + # These are the nodes that will be loaded into memory for rendering + # So we're ok loading them into memory here as that's bound to happen + # anyway. Having them ready means we can modify the result while + # rendering the fields. + @paged_nodes ||= load_paged_nodes.to_a + end + + private + + def load_paged_nodes + if first && last + raise Gitlab::Graphql::Errors::ArgumentError.new("Can only provide either `first` or `last`, not both") + end + + if last + sliced_nodes.last(limit_value) + else + sliced_nodes.limit(limit_value) # rubocop: disable CodeReuse/ActiveRecord + end + end + + # rubocop: disable CodeReuse/ActiveRecord + def slice_nodes(sliced, encoded_cursor, before_or_after) + decoded_cursor = ordering_from_encoded_json(encoded_cursor) + builder = QueryBuilder.new(arel_table, order_list, decoded_cursor, before_or_after) + ordering = builder.conditions + + sliced.where(*ordering).where.not(id: decoded_cursor['id']) + end + # rubocop: enable CodeReuse/ActiveRecord + + def limit_value + @limit_value ||= [first, last, max_page_size].compact.min + end + + def ordered_nodes + strong_memoize(:order_nodes) do + unless nodes.primary_key.present? + raise ArgumentError.new('Relation must have a primary key') + end + + list = OrderInfo.build_order_list(nodes) + + # ensure there is a primary key ordering + if list&.last&.attribute_name != nodes.primary_key + nodes.order(arel_table[nodes.primary_key].desc) # rubocop: disable CodeReuse/ActiveRecord + else + nodes + end + end + end + + def order_list + strong_memoize(:order_list) do + OrderInfo.build_order_list(ordered_nodes) + end + end + + def arel_table + nodes.arel_table + end + + # Storing the current order values in the cursor allows us to + # make an intelligent decision on handling NULL values. + # Otherwise we would either need to fetch the record first, + # or fetch it in the SQL, significantly complicating it. + def encoded_json_from_ordering(node) + ordering = { 'id' => node[:id].to_s } + + order_list.each do |field| + field_name = field.attribute_name + ordering[field_name] = node[field_name].to_s + end + + encode(ordering.to_json) + end + + def ordering_from_encoded_json(cursor) + JSON.parse(decode(cursor)) + rescue JSON::ParserError + # for the transition period where a client might request using an + # old style cursor. Once removed, make it an error: + # raise Gitlab::Graphql::Errors::ArgumentError, "Please provide a valid cursor" + # TODO can be removed in next release + # https://gitlab.com/gitlab-org/gitlab/issues/32933 + field_name = order_list.first.attribute_name + + { field_name => decode(cursor) } + end + end + end + end + end +end diff --git a/lib/gitlab/graphql/connections/keyset/legacy_keyset_connection.rb b/lib/gitlab/graphql/connections/keyset/legacy_keyset_connection.rb new file mode 100644 index 00000000000..baf900d1048 --- /dev/null +++ b/lib/gitlab/graphql/connections/keyset/legacy_keyset_connection.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +# TODO https://gitlab.com/gitlab-org/gitlab/issues/35104 +module Gitlab + module Graphql + module Connections + module Keyset + module LegacyKeysetConnection + def legacy_cursor_from_node(node) + encode(node[legacy_order_field].to_s) + end + + # rubocop: disable CodeReuse/ActiveRecord + def legacy_sliced_nodes + @sliced_nodes ||= + begin + sliced = nodes + + sliced = sliced.where(legacy_before_slice) if before.present? + sliced = sliced.where(legacy_after_slice) if after.present? + + sliced + end + end + # rubocop: enable CodeReuse/ActiveRecord + + private + + def use_legacy_pagination? + strong_memoize(:feature_disabled) do + Feature.disabled?(:graphql_keyset_pagination, default_enabled: true) + end + end + + def legacy_before_slice + if legacy_sort_direction == :asc + arel_table[legacy_order_field].lt(decode(before)) + else + arel_table[legacy_order_field].gt(decode(before)) + end + end + + def legacy_after_slice + if legacy_sort_direction == :asc + arel_table[legacy_order_field].gt(decode(after)) + else + arel_table[legacy_order_field].lt(decode(after)) + end + end + + def legacy_order_info + @legacy_order_info ||= nodes.order_values.first + end + + def legacy_order_field + @legacy_order_field ||= legacy_order_info&.expr&.name || nodes.primary_key + end + + def legacy_sort_direction + @legacy_order_direction ||= legacy_order_info&.direction || :desc + end + end + end + end + end +end diff --git a/lib/gitlab/graphql/connections/keyset/order_info.rb b/lib/gitlab/graphql/connections/keyset/order_info.rb new file mode 100644 index 00000000000..6c4be93bfee --- /dev/null +++ b/lib/gitlab/graphql/connections/keyset/order_info.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Connections + module Keyset + class OrderInfo + def initialize(order_value) + @order_value = order_value + end + + def attribute_name + order_value.expr.name + end + + def operator_for(before_or_after) + case before_or_after + when :before + sort_direction == :asc ? '<' : '>' + when :after + sort_direction == :asc ? '>' : '<' + end + end + + # Only allow specific node types. For example ignore String nodes + def self.build_order_list(relation) + order_list = relation.order_values.select do |value| + value.is_a?(Arel::Nodes::Ascending) || value.is_a?(Arel::Nodes::Descending) + end + + order_list.map { |info| OrderInfo.new(info) } + end + + def self.validate_ordering(relation, order_list) + if order_list.empty? + raise ArgumentError.new('A minimum of 1 ordering field is required') + end + + if order_list.count > 2 + raise ArgumentError.new('A maximum of 2 ordering fields are allowed') + end + + # make sure the last ordering field is non-nullable + attribute_name = order_list.last&.attribute_name + + if relation.columns_hash[attribute_name].null + raise ArgumentError.new("Column `#{attribute_name}` must not allow NULL") + end + + if order_list.last.attribute_name != relation.primary_key + raise ArgumentError.new("Last ordering field must be the primary key, `#{relation.primary_key}`") + end + end + + private + + attr_reader :order_value + + def sort_direction + order_value.direction + end + end + end + end + end +end diff --git a/lib/gitlab/graphql/connections/keyset/query_builder.rb b/lib/gitlab/graphql/connections/keyset/query_builder.rb new file mode 100644 index 00000000000..e93c25d85fc --- /dev/null +++ b/lib/gitlab/graphql/connections/keyset/query_builder.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Connections + module Keyset + class QueryBuilder + def initialize(arel_table, order_list, decoded_cursor, before_or_after) + @arel_table, @order_list, @decoded_cursor, @before_or_after = arel_table, order_list, decoded_cursor, before_or_after + + if order_list.empty? + raise ArgumentError.new('No ordering scopes have been supplied') + end + end + + # Based on whether the main field we're ordering on is NULL in the + # cursor, we can more easily target our query condition. + # We assume that the last ordering field is unique, meaning + # it will not contain NULLs. + # We currently only support two ordering fields. + # + # Example of the conditions for + # relation: Issue.order(relative_position: :asc).order(id: :asc) + # after cursor: relative_position: 1500, id: 500 + # + # when cursor[relative_position] is not NULL + # + # ("issues"."relative_position" > 1500) + # OR ( + # "issues"."relative_position" = 1500 + # AND + # "issues"."id" > 500 + # ) + # OR ("issues"."relative_position" IS NULL) + # + # when cursor[relative_position] is NULL + # + # "issues"."relative_position" IS NULL + # AND + # "issues"."id" > 500 + # + def conditions + attr_names = order_list.map { |field| field.attribute_name } + attr_values = attr_names.map { |name| decoded_cursor[name] } + + if attr_names.count == 1 && attr_values.first.nil? + raise Gitlab::Graphql::Errors::ArgumentError.new('Before/after cursor invalid: `nil` was provided as only sortable value') + end + + if attr_names.count == 1 || attr_values.first.present? + Keyset::Conditions::NotNullCondition.new(arel_table, attr_names, attr_values, operators, before_or_after).build + else + Keyset::Conditions::NullCondition.new(arel_table, attr_names, attr_values, operators, before_or_after).build + end + end + + private + + attr_reader :arel_table, :order_list, :decoded_cursor, :before_or_after + + def operators + order_list.map { |field| field.operator_for(before_or_after) } + end + end + end + end + end +end diff --git a/lib/gitlab/graphql/connections/keyset_connection.rb b/lib/gitlab/graphql/connections/keyset_connection.rb deleted file mode 100644 index 715963a44c1..00000000000 --- a/lib/gitlab/graphql/connections/keyset_connection.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Graphql - module Connections - class KeysetConnection < GraphQL::Relay::BaseConnection - def cursor_from_node(node) - encode(node[order_field].to_s) - end - - # rubocop: disable CodeReuse/ActiveRecord - def sliced_nodes - @sliced_nodes ||= - begin - sliced = nodes - - sliced = sliced.where(before_slice) if before.present? - sliced = sliced.where(after_slice) if after.present? - - sliced - end - end - # rubocop: enable CodeReuse/ActiveRecord - - def paged_nodes - # These are the nodes that will be loaded into memory for rendering - # So we're ok loading them into memory here as that's bound to happen - # anyway. Having them ready means we can modify the result while - # rendering the fields. - @paged_nodes ||= load_paged_nodes.to_a - end - - private - - def load_paged_nodes - if first && last - raise Gitlab::Graphql::Errors::ArgumentError.new("Can only provide either `first` or `last`, not both") - end - - if last - sliced_nodes.last(limit_value) - else - sliced_nodes.limit(limit_value) # rubocop: disable CodeReuse/ActiveRecord - end - end - - def before_slice - if sort_direction == :asc - table[order_field].lt(decode(before)) - else - table[order_field].gt(decode(before)) - end - end - - def after_slice - if sort_direction == :asc - table[order_field].gt(decode(after)) - else - table[order_field].lt(decode(after)) - end - end - - def limit_value - @limit_value ||= [first, last, max_page_size].compact.min - end - - def table - nodes.arel_table - end - - def order_info - @order_info ||= nodes.order_values.first - end - - def order_field - @order_field ||= order_info&.expr&.name || nodes.primary_key - end - - def sort_direction - @order_direction ||= order_info&.direction || :desc - end - end - end - end -end @@ -331,6 +331,7 @@ module QA module Component autoload :IpLimits, 'qa/page/admin/settings/component/ip_limits' + autoload :OutboundRequests, 'qa/page/admin/settings/component/outbound_requests' autoload :RepositoryStorage, 'qa/page/admin/settings/component/repository_storage' autoload :AccountAndLimit, 'qa/page/admin/settings/component/account_and_limit' autoload :PerformanceBar, 'qa/page/admin/settings/component/performance_bar' @@ -406,6 +407,7 @@ module QA module DockerRun autoload :Base, 'qa/service/docker_run/base' + autoload :Jenkins, 'qa/service/docker_run/jenkins' autoload :LDAP, 'qa/service/docker_run/ldap' autoload :Maven, 'qa/service/docker_run/maven' autoload :NodeJs, 'qa/service/docker_run/node_js' @@ -438,6 +440,17 @@ module QA end end + module Jenkins + module Page + autoload :Base, 'qa/vendor/jenkins/page/base' + autoload :Login, 'qa/vendor/jenkins/page/login' + autoload :Configure, 'qa/vendor/jenkins/page/configure' + autoload :NewCredentials, 'qa/vendor/jenkins/page/new_credentials' + autoload :NewJob, 'qa/vendor/jenkins/page/new_job' + autoload :ConfigureJob, 'qa/vendor/jenkins/page/configure_job' + end + end + module Github module Page autoload :Base, 'qa/vendor/github/page/base' diff --git a/qa/qa/page/admin/settings/component/outbound_requests.rb b/qa/qa/page/admin/settings/component/outbound_requests.rb new file mode 100644 index 00000000000..248ea5b6715 --- /dev/null +++ b/qa/qa/page/admin/settings/component/outbound_requests.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module QA + module Page + module Admin + module Settings + module Component + class OutboundRequests < Page::Base + view 'app/views/admin/application_settings/_outbound.html.haml' do + element :allow_requests_from_services_checkbox + element :save_changes_button + end + + def allow_requests_to_local_network_from_services + check_allow_requests_to_local_network_from_services_checkbox + click_save_changes_button + end + + private + + def check_allow_requests_to_local_network_from_services_checkbox + check_element :allow_requests_from_services_checkbox + end + + def click_save_changes_button + click_element :save_changes_button + end + end + end + end + end + end +end diff --git a/qa/qa/page/admin/settings/network.rb b/qa/qa/page/admin/settings/network.rb index fdb8fcda281..83566d3d1ca 100644 --- a/qa/qa/page/admin/settings/network.rb +++ b/qa/qa/page/admin/settings/network.rb @@ -9,6 +9,7 @@ module QA view 'app/views/admin/application_settings/network.html.haml' do element :ip_limits_section + element :outbound_requests_section end def expand_ip_limits(&block) @@ -16,6 +17,12 @@ module QA Component::IpLimits.perform(&block) end end + + def expand_outbound_requests(&block) + expand_section(:outbound_requests_section) do + Component::OutboundRequests.perform(&block) + end + end end end end diff --git a/qa/qa/page/main/menu.rb b/qa/qa/page/main/menu.rb index 024f56db8e2..49c48568e68 100644 --- a/qa/qa/page/main/menu.rb +++ b/qa/qa/page/main/menu.rb @@ -20,7 +20,7 @@ module QA element :admin_area_link element :projects_dropdown, required: true element :groups_dropdown, required: true - element :more_dropdown, required: true + element :more_dropdown element :snippets_link end diff --git a/qa/qa/page/project/sub_menus/settings.rb b/qa/qa/page/project/sub_menus/settings.rb index 1cd39fcff58..8be442ba35d 100644 --- a/qa/qa/page/project/sub_menus/settings.rb +++ b/qa/qa/page/project/sub_menus/settings.rb @@ -13,6 +13,7 @@ module QA element :settings_item element :link_members_settings element :general_settings_link + element :integrations_settings_link end end end @@ -55,6 +56,14 @@ module QA end end + def go_to_integrations_settings + hover_settings do + within_submenu do + click_element :integrations_settings_link + end + end + end + private def hover_settings diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index caaa766e982..3bebe2aaeda 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -9,6 +9,7 @@ module QA include Members attr_writer :initialize_with_readme + attr_writer :auto_devops_enabled attr_writer :visibility attribute :id @@ -47,6 +48,7 @@ module QA @standalone = false @description = 'My awesome project' @initialize_with_readme = false + @auto_devops_enabled = true @visibility = 'public' end @@ -101,7 +103,8 @@ module QA name: name, description: description, visibility: @visibility, - initialize_with_readme: @initialize_with_readme + initialize_with_readme: @initialize_with_readme, + auto_devops_enabled: @auto_devops_enabled } unless @standalone diff --git a/qa/qa/service/docker_run/jenkins.rb b/qa/qa/service/docker_run/jenkins.rb new file mode 100644 index 00000000000..00b63282484 --- /dev/null +++ b/qa/qa/service/docker_run/jenkins.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module QA + module Service + module DockerRun + class Jenkins < Base + def initialize + @image = 'registry.gitlab.com/gitlab-org/gitlab-qa/jenkins-gitlab:version1' + @name = 'jenkins-server' + @port = '8080' + super() + end + + def host_address + "http://#{host_name}:#{@port}" + end + + def host_name + return 'localhost' unless QA::Runtime::Env.running_in_ci? + + super + end + + def register! + command = <<~CMD.tr("\n", ' ') + docker run -d --rm + --network #{network} + --hostname #{host_name} + --name #{@name} + --env JENKINS_HOME=jenkins_home + --publish #{@port}:8080 + --publish 50000:50000 + #{@image} + CMD + + command.gsub!("--network #{network} ", '') unless QA::Runtime::Env.running_in_ci? + + shell command + end + end + end + end +end diff --git a/qa/qa/vendor/jenkins/page/base.rb b/qa/qa/vendor/jenkins/page/base.rb new file mode 100644 index 00000000000..8dfbe7570f8 --- /dev/null +++ b/qa/qa/vendor/jenkins/page/base.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module QA + module Vendor + module Jenkins + module Page + class Base + include Capybara::DSL + include Scenario::Actable + + attr_reader :path + + class << self + attr_accessor :host + end + + def visit! + page.visit URI.join(Base.host, path).to_s + end + end + end + end + end +end diff --git a/qa/qa/vendor/jenkins/page/configure.rb b/qa/qa/vendor/jenkins/page/configure.rb new file mode 100644 index 00000000000..8851a2564fd --- /dev/null +++ b/qa/qa/vendor/jenkins/page/configure.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'capybara/dsl' + +module QA + module Vendor + module Jenkins + module Page + class Configure < Page::Base + def initialize + @path = 'configure' + end + + def visit_and_setup_gitlab_connection(gitlab_host, token_description) + visit! + fill_in '_.name', with: 'GitLab' + find('.setting-name', text: "Gitlab host URL").find(:xpath, "..").find('input').set gitlab_host + + dropdown_element = find('.setting-name', text: "Credentials").find(:xpath, "..").find('select') + + QA::Support::Retrier.retry_until(exit_on_failure: true) do + dropdown_element.select "GitLab API token (#{token_description})" + dropdown_element.value != '' + end + + yield if block_given? + + click_save + end + + def click_test_connection + click_on 'Test Connection' + end + + def has_success? + has_css?('div.ok', text: "Success") + end + + private + + def click_save + click_on 'Save' + end + end + end + end + end +end diff --git a/qa/qa/vendor/jenkins/page/configure_job.rb b/qa/qa/vendor/jenkins/page/configure_job.rb new file mode 100644 index 00000000000..ab16e895fa9 --- /dev/null +++ b/qa/qa/vendor/jenkins/page/configure_job.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'capybara/dsl' + +module QA + module Vendor + module Jenkins + module Page + class ConfigureJob < Page::Base + attr_accessor :job_name + + def initialize + @path = "/job/#{@job_name}/configure" + end + + def configure(scm_url:) + set_git_source_code_management_url(scm_url) + click_build_when_change_is_pushed_to_gitlab + set_publish_status_to_gitlab + click_save + end + + private + + def set_git_source_code_management_url(repository_url) + select_git_source_code_management + set_repository_url(repository_url) + end + + def click_build_when_change_is_pushed_to_gitlab + find('label', text: 'Build when a change is pushed to GitLab').find(:xpath, "..").find('input').click + end + + def set_publish_status_to_gitlab + click_add_post_build_action + select_publish_build_status_to_gitlab + end + + def click_save + click_on 'Save' + end + + def select_git_source_code_management + find('#radio-block-1').click + end + + def set_repository_url(repository_url) + find('.setting-name', text: "Repository URL").find(:xpath, "..").find('input').set repository_url + end + + def click_add_post_build_action + click_on "Add post-build action" + end + + def select_publish_build_status_to_gitlab + click_link "Publish build status to GitLab" + end + end + end + end + end +end diff --git a/qa/qa/vendor/jenkins/page/login.rb b/qa/qa/vendor/jenkins/page/login.rb new file mode 100644 index 00000000000..7b3558b25e2 --- /dev/null +++ b/qa/qa/vendor/jenkins/page/login.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'capybara/dsl' + +module QA + module Vendor + module Jenkins + module Page + class Login < Page::Base + def initialize + @path = 'login' + end + + def visit! + super + + QA::Support::Retrier.retry_until(sleep_interval: 3, reload_page: page, max_attempts: 20, exit_on_failure: true) do + page.has_text? 'Welcome to Jenkins!' + end + end + + def login + fill_in 'j_username', with: 'admin' + fill_in 'j_password', with: 'password' + click_on 'Sign in' + end + end + end + end + end +end diff --git a/qa/qa/vendor/jenkins/page/new_credentials.rb b/qa/qa/vendor/jenkins/page/new_credentials.rb new file mode 100644 index 00000000000..bdef1a13fd4 --- /dev/null +++ b/qa/qa/vendor/jenkins/page/new_credentials.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'capybara/dsl' + +module QA + module Vendor + module Jenkins + module Page + class NewCredentials < Page::Base + def initialize + @path = 'credentials/store/system/domain/_/newCredentials' + end + + def visit_and_set_gitlab_api_token(api_token, description) + visit! + wait_for_page_to_load + select_gitlab_api_token + set_api_token(api_token) + set_description(description) + click_ok + end + + private + + def select_gitlab_api_token + find('.setting-name', text: "Kind").find(:xpath, "..").find('select').select "GitLab API token" + end + + def set_api_token(api_token) + fill_in '_.apiToken', with: api_token + end + + def set_description(description) + fill_in '_.description', with: description + end + + def click_ok + click_on 'OK' + end + + def wait_for_page_to_load + QA::Support::Waiter.wait(interval: 1.0) do + page.has_css?('.setting-name', text: "Description") + end + end + end + end + end + end +end diff --git a/qa/qa/vendor/jenkins/page/new_job.rb b/qa/qa/vendor/jenkins/page/new_job.rb new file mode 100644 index 00000000000..11fa4ca8a53 --- /dev/null +++ b/qa/qa/vendor/jenkins/page/new_job.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'capybara/dsl' + +module QA + module Vendor + module Jenkins + module Page + class NewJob < Page::Base + def initialize + @path = 'newJob' + end + + def visit_and_create_new_job_with_name(new_job_name) + visit! + set_new_job_name(new_job_name) + click_free_style_project + click_ok + end + + private + + def set_new_job_name(new_job_name) + fill_in 'name', with: new_job_name + end + + def click_free_style_project + find('.hudson_model_FreeStyleProject').click + end + + def click_ok + click_on 'OK' + end + end + end + end + end +end diff --git a/spec/factories/clusters/clusters.rb b/spec/factories/clusters/clusters.rb index 63f33633a3c..609e7e20187 100644 --- a/spec/factories/clusters/clusters.rb +++ b/spec/factories/clusters/clusters.rb @@ -93,5 +93,25 @@ FactoryBot.define do trait :not_managed do managed { false } end + + trait :cleanup_not_started do + cleanup_status { 1 } + end + + trait :cleanup_uninstalling_applications do + cleanup_status { 2 } + end + + trait :cleanup_removing_project_namespaces do + cleanup_status { 3 } + end + + trait :cleanup_removing_service_account do + cleanup_status { 4 } + end + + trait :cleanup_errored do + cleanup_status { 5 } + end end end diff --git a/spec/graphql/features/authorization_spec.rb b/spec/graphql/features/authorization_spec.rb index 9a60ff3b78c..7ad6a622b4b 100644 --- a/spec/graphql/features/authorization_spec.rb +++ b/spec/graphql/features/authorization_spec.rb @@ -259,7 +259,8 @@ describe 'Gitlab::Graphql::Authorization' do let(:project_type) do |type| type_factory do |type| type.graphql_name 'FakeProjectType' - type.field :test_issues, issue_type.connection_type, null: false, resolve: -> (_, _, _) { Issue.where(project: [visible_project, other_project]) } + type.field :test_issues, issue_type.connection_type, null: false, + resolve: -> (_, _, _) { Issue.where(project: [visible_project, other_project]).order(id: :asc) } end end let(:query_type) do diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 0a27bbecfef..dcf3c989047 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -36,7 +36,7 @@ describe GitlabSchema do it 'paginates active record relations using `Gitlab::Graphql::Connections::KeysetConnection`' do connection = GraphQL::Relay::BaseConnection::CONNECTION_IMPLEMENTATIONS[ActiveRecord::Relation.name] - expect(connection).to eq(Gitlab::Graphql::Connections::KeysetConnection) + expect(connection).to eq(Gitlab::Graphql::Connections::Keyset::Connection) end describe '.execute' do diff --git a/spec/helpers/gitlab_routing_helper_spec.rb b/spec/helpers/gitlab_routing_helper_spec.rb index bf043f3f013..38699108b06 100644 --- a/spec/helpers/gitlab_routing_helper_spec.rb +++ b/spec/helpers/gitlab_routing_helper_spec.rb @@ -75,6 +75,12 @@ describe GitlabRoutingHelper do expect(preview_markdown_path(group)).to eq("/groups/#{group.path}/preview_markdown") end + it 'returns group preview markdown path for a group parent with args' do + group = create(:group) + + expect(preview_markdown_path(group, { type_id: 5 })).to eq("/groups/#{group.path}/preview_markdown?type_id=5") + end + it 'returns project preview markdown path for a project parent' do expect(preview_markdown_path(project)).to eq("/#{project.full_path}/preview_markdown") end diff --git a/spec/lib/gitlab/graphql/connections/keyset/conditions/not_null_condition_spec.rb b/spec/lib/gitlab/graphql/connections/keyset/conditions/not_null_condition_spec.rb new file mode 100644 index 00000000000..d943540fe1f --- /dev/null +++ b/spec/lib/gitlab/graphql/connections/keyset/conditions/not_null_condition_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Graphql::Connections::Keyset::Conditions::NotNullCondition do + describe '#build' do + let(:condition) { described_class.new(Issue.arel_table, %w(relative_position id), [1500, 500], ['>', '>'], before_or_after) } + + context 'when there is only one ordering field' do + let(:condition) { described_class.new(Issue.arel_table, ['id'], [500], ['>'], :after) } + + it 'generates a single condition sql' do + expected_sql = <<~SQL + ("issues"."id" > 500) + SQL + + expect(condition.build.squish).to eq expected_sql.squish + end + end + + context 'when :after' do + let(:before_or_after) { :after } + + it 'generates :after sql' do + expected_sql = <<~SQL + ("issues"."relative_position" > 1500) + OR ( + "issues"."relative_position" = 1500 + AND + "issues"."id" > 500 + ) + OR ("issues"."relative_position" IS NULL) + SQL + + expect(condition.build.squish).to eq expected_sql.squish + end + end + + context 'when :before' do + let(:before_or_after) { :before } + + it 'generates :before sql' do + expected_sql = <<~SQL + ("issues"."relative_position" > 1500) + OR ( + "issues"."relative_position" = 1500 + AND + "issues"."id" > 500 + ) + SQL + + expect(condition.build.squish).to eq expected_sql.squish + end + end + end +end diff --git a/spec/lib/gitlab/graphql/connections/keyset/conditions/null_condition_spec.rb b/spec/lib/gitlab/graphql/connections/keyset/conditions/null_condition_spec.rb new file mode 100644 index 00000000000..7fce94adb81 --- /dev/null +++ b/spec/lib/gitlab/graphql/connections/keyset/conditions/null_condition_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Graphql::Connections::Keyset::Conditions::NullCondition do + describe '#build' do + let(:condition) { described_class.new(Issue.arel_table, %w(relative_position id), [nil, 500], [nil, '>'], before_or_after) } + + context 'when :after' do + let(:before_or_after) { :after } + + it 'generates sql' do + expected_sql = <<~SQL + ( + "issues"."relative_position" IS NULL + AND + "issues"."id" > 500 + ) + SQL + + expect(condition.build.squish).to eq expected_sql.squish + end + end + + context 'when :before' do + let(:before_or_after) { :before } + + it 'generates :before sql' do + expected_sql = <<~SQL + ( + "issues"."relative_position" IS NULL + AND + "issues"."id" > 500 + ) + OR ("issues"."relative_position" IS NOT NULL) + SQL + + expect(condition.build.squish).to eq expected_sql.squish + end + end + end +end diff --git a/spec/lib/gitlab/graphql/connections/keyset/connection_spec.rb b/spec/lib/gitlab/graphql/connections/keyset/connection_spec.rb new file mode 100644 index 00000000000..ba1addadb5a --- /dev/null +++ b/spec/lib/gitlab/graphql/connections/keyset/connection_spec.rb @@ -0,0 +1,303 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Graphql::Connections::Keyset::Connection do + let(:nodes) { Project.all.order(id: :asc) } + let(:arguments) { {} } + subject(:connection) do + described_class.new(nodes, arguments, max_page_size: 3) + end + + def encoded_cursor(node) + described_class.new(nodes, {}).cursor_from_node(node) + end + + def decoded_cursor(cursor) + JSON.parse(Base64Bp.urlsafe_decode64(cursor)) + end + + describe '#cursor_from_nodes' do + let(:project) { create(:project) } + let(:cursor) { connection.cursor_from_node(project) } + + it 'returns an encoded ID' do + expect(decoded_cursor(cursor)).to eq('id' => project.id.to_s) + end + + context 'when an order is specified' do + let(:nodes) { Project.order(:updated_at) } + + it 'returns the encoded value of the order' do + expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s) + end + + it 'includes the :id even when not specified in the order' do + expect(decoded_cursor(cursor)).to include('id' => project.id.to_s) + end + end + + context 'when multiple orders are specified' do + let(:nodes) { Project.order(:updated_at).order(:created_at) } + + it 'returns the encoded value of the order' do + expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s) + end + end + + context 'when multiple orders with SQL are specified' do + let(:nodes) { Project.order(Arel.sql('projects.updated_at IS NULL')).order(:updated_at).order(:id) } + + it 'returns the encoded value of the order' do + expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s) + end + end + end + + describe '#sliced_nodes' do + let(:projects) { create_list(:project, 4) } + + context 'when before is passed' do + let(:arguments) { { before: encoded_cursor(projects[1]) } } + + it 'only returns the project before the selected one' do + expect(subject.sliced_nodes).to contain_exactly(projects.first) + end + + context 'when the sort order is descending' do + let(:nodes) { Project.all.order(id: :desc) } + + it 'returns the correct nodes' do + expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1]) + end + end + end + + context 'when after is passed' do + let(:arguments) { { after: encoded_cursor(projects[1]) } } + + it 'only returns the project before the selected one' do + expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1]) + end + + context 'when the sort order is descending' do + let(:nodes) { Project.all.order(id: :desc) } + + it 'returns the correct nodes' do + expect(subject.sliced_nodes).to contain_exactly(projects.first) + end + end + end + + context 'when both before and after are passed' do + let(:arguments) do + { + after: encoded_cursor(projects[1]), + before: encoded_cursor(projects[3]) + } + end + + it 'returns the expected set' do + expect(subject.sliced_nodes).to contain_exactly(projects[2]) + end + end + + context 'when multiple orders are defined' do + let!(:project1) { create(:project, last_repository_check_at: 10.days.ago) } # Asc: project5 Desc: project3 + let!(:project2) { create(:project, last_repository_check_at: nil) } # Asc: project1 Desc: project1 + let!(:project3) { create(:project, last_repository_check_at: 5.days.ago) } # Asc: project3 Desc: project5 + let!(:project4) { create(:project, last_repository_check_at: nil) } # Asc: project2 Desc: project2 + let!(:project5) { create(:project, last_repository_check_at: 20.days.ago) } # Asc: project4 Desc: project4 + + context 'when ascending' do + let(:nodes) do + Project.order(Arel.sql('projects.last_repository_check_at IS NULL')).order(last_repository_check_at: :asc).order(id: :asc) + end + + context 'when no cursor is passed' do + let(:arguments) { {} } + + it 'returns projects in ascending order' do + expect(subject.sliced_nodes).to eq([project5, project1, project3, project2, project4]) + end + end + + context 'when before cursor value is NULL' do + let(:arguments) { { before: encoded_cursor(project4) } } + + it 'returns all projects before the cursor' do + expect(subject.sliced_nodes).to eq([project5, project1, project3, project2]) + end + end + + context 'when before cursor value is not NULL' do + let(:arguments) { { before: encoded_cursor(project3) } } + + it 'returns all projects before the cursor' do + expect(subject.sliced_nodes).to eq([project5, project1]) + end + end + + context 'when after cursor value is NULL' do + let(:arguments) { { after: encoded_cursor(project2) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq([project4]) + end + end + + context 'when after cursor value is not NULL' do + let(:arguments) { { after: encoded_cursor(project1) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq([project3, project2, project4]) + end + end + + context 'when before and after cursor' do + let(:arguments) { { before: encoded_cursor(project4), after: encoded_cursor(project5) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq([project1, project3, project2]) + end + end + end + + context 'when descending' do + let(:nodes) do + Project.order(Arel.sql('projects.last_repository_check_at IS NULL')).order(last_repository_check_at: :desc).order(id: :asc) + end + + context 'when no cursor is passed' do + let(:arguments) { {} } + + it 'only returns projects in descending order' do + expect(subject.sliced_nodes).to eq([project3, project1, project5, project2, project4]) + end + end + + context 'when before cursor value is NULL' do + let(:arguments) { { before: encoded_cursor(project4) } } + + it 'returns all projects before the cursor' do + expect(subject.sliced_nodes).to eq([project3, project1, project5, project2]) + end + end + + context 'when before cursor value is not NULL' do + let(:arguments) { { before: encoded_cursor(project5) } } + + it 'returns all projects before the cursor' do + expect(subject.sliced_nodes).to eq([project3, project1]) + end + end + + context 'when after cursor value is NULL' do + let(:arguments) { { after: encoded_cursor(project2) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq([project4]) + end + end + + context 'when after cursor value is not NULL' do + let(:arguments) { { after: encoded_cursor(project1) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq([project5, project2, project4]) + end + end + + context 'when before and after cursor' do + let(:arguments) { { before: encoded_cursor(project4), after: encoded_cursor(project3) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq([project1, project5, project2]) + end + end + end + end + + # TODO Enable this as part of below issue + # https://gitlab.com/gitlab-org/gitlab/issues/32933 + # context 'when an invalid cursor is provided' do + # let(:arguments) { { before: 'invalidcursor' } } + # + # it 'raises an error' do + # expect { expect(subject.sliced_nodes) }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) + # end + # end + + # TODO Remove this as part of below issue + # https://gitlab.com/gitlab-org/gitlab/issues/32933 + context 'when an old style cursor is provided' do + let(:arguments) { { before: Base64Bp.urlsafe_encode64(projects[1].id.to_s, padding: false) } } + + it 'only returns the project before the selected one' do + expect(subject.sliced_nodes).to contain_exactly(projects.first) + end + end + end + + describe '#paged_nodes' do + let!(:projects) { create_list(:project, 5) } + + it 'returns the collection limited to max page size' do + expect(subject.paged_nodes.size).to eq(3) + end + + it 'is a loaded memoized array' do + expect(subject.paged_nodes).to be_an(Array) + expect(subject.paged_nodes.object_id).to eq(subject.paged_nodes.object_id) + end + + context 'when `first` is passed' do + let(:arguments) { { first: 2 } } + + it 'returns only the first elements' do + expect(subject.paged_nodes).to contain_exactly(projects.first, projects.second) + end + end + + context 'when `last` is passed' do + let(:arguments) { { last: 2 } } + + it 'returns only the last elements' do + expect(subject.paged_nodes).to contain_exactly(projects[3], projects[4]) + end + end + + context 'when both are passed' do + let(:arguments) { { first: 2, last: 2 } } + + it 'raises an error' do + expect { subject.paged_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) + end + end + + context 'when primary key is not in original order' do + let(:nodes) { Project.order(last_repository_check_at: :desc) } + + it 'is added to end' do + sliced = subject.sliced_nodes + last_order_name = sliced.order_values.last.expr.name + + expect(last_order_name).to eq sliced.primary_key + end + end + + context 'when there is no primary key' do + let(:nodes) { NoPrimaryKey.all } + + it 'raises an error' do + expect(NoPrimaryKey.primary_key).to be_nil + expect { subject.sliced_nodes }.to raise_error(ArgumentError, 'Relation must have a primary key') + end + end + end + + class NoPrimaryKey < ActiveRecord::Base + self.table_name = 'no_primary_key' + self.primary_key = nil + end +end diff --git a/spec/lib/gitlab/graphql/connections/keyset/legacy_keyset_connection_spec.rb b/spec/lib/gitlab/graphql/connections/keyset/legacy_keyset_connection_spec.rb new file mode 100644 index 00000000000..aaf28fed684 --- /dev/null +++ b/spec/lib/gitlab/graphql/connections/keyset/legacy_keyset_connection_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +# TODO https://gitlab.com/gitlab-org/gitlab/issues/35104 +require 'spec_helper' + +describe Gitlab::Graphql::Connections::Keyset::LegacyKeysetConnection do + describe 'old keyset_connection' do + let(:described_class) { Gitlab::Graphql::Connections::Keyset::Connection } + let(:nodes) { Project.all.order(id: :asc) } + let(:arguments) { {} } + subject(:connection) do + described_class.new(nodes, arguments, max_page_size: 3) + end + + before do + stub_feature_flags(graphql_keyset_pagination: false) + end + + def encoded_property(value) + Base64Bp.urlsafe_encode64(value.to_s, padding: false) + end + + describe '#cursor_from_nodes' do + let(:project) { create(:project) } + + it 'returns an encoded ID' do + expect(connection.cursor_from_node(project)) + .to eq(encoded_property(project.id)) + end + + context 'when an order was specified' do + let(:nodes) { Project.order(:updated_at) } + + it 'returns the encoded value of the order' do + expect(connection.cursor_from_node(project)) + .to eq(encoded_property(project.updated_at)) + end + end + end + + describe '#sliced_nodes' do + let(:projects) { create_list(:project, 4) } + + context 'when before is passed' do + let(:arguments) { { before: encoded_property(projects[1].id) } } + + it 'only returns the project before the selected one' do + expect(subject.sliced_nodes).to contain_exactly(projects.first) + end + + context 'when the sort order is descending' do + let(:nodes) { Project.all.order(id: :desc) } + + it 'returns the correct nodes' do + expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1]) + end + end + end + + context 'when after is passed' do + let(:arguments) { { after: encoded_property(projects[1].id) } } + + it 'only returns the project before the selected one' do + expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1]) + end + + context 'when the sort order is descending' do + let(:nodes) { Project.all.order(id: :desc) } + + it 'returns the correct nodes' do + expect(subject.sliced_nodes).to contain_exactly(projects.first) + end + end + end + + context 'when both before and after are passed' do + let(:arguments) do + { + after: encoded_property(projects[1].id), + before: encoded_property(projects[3].id) + } + end + + it 'returns the expected set' do + expect(subject.sliced_nodes).to contain_exactly(projects[2]) + end + end + end + + describe '#paged_nodes' do + let!(:projects) { create_list(:project, 5) } + + it 'returns the collection limited to max page size' do + expect(subject.paged_nodes.size).to eq(3) + end + + it 'is a loaded memoized array' do + expect(subject.paged_nodes).to be_an(Array) + expect(subject.paged_nodes.object_id).to eq(subject.paged_nodes.object_id) + end + + context 'when `first` is passed' do + let(:arguments) { { first: 2 } } + + it 'returns only the first elements' do + expect(subject.paged_nodes).to contain_exactly(projects.first, projects.second) + end + end + + context 'when `last` is passed' do + let(:arguments) { { last: 2 } } + + it 'returns only the last elements' do + expect(subject.paged_nodes).to contain_exactly(projects[3], projects[4]) + end + end + + context 'when both are passed' do + let(:arguments) { { first: 2, last: 2 } } + + it 'raises an error' do + expect { subject.paged_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) + end + end + end + end +end diff --git a/spec/lib/gitlab/graphql/connections/keyset/order_info_spec.rb b/spec/lib/gitlab/graphql/connections/keyset/order_info_spec.rb new file mode 100644 index 00000000000..608a9ed1d85 --- /dev/null +++ b/spec/lib/gitlab/graphql/connections/keyset/order_info_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Graphql::Connections::Keyset::OrderInfo do + describe '#build_order_list' do + let(:order_list) { described_class.build_order_list(relation) } + + context 'when multiple orders with SQL is specified' do + let(:relation) { Project.order(Arel.sql('projects.updated_at IS NULL')).order(:updated_at).order(:id) } + + it 'ignores the SQL order' do + expect(order_list.count).to eq 2 + expect(order_list.first.attribute_name).to eq 'updated_at' + expect(order_list.first.operator_for(:after)).to eq '>' + expect(order_list.last.attribute_name).to eq 'id' + expect(order_list.last.operator_for(:after)).to eq '>' + end + end + end + + describe '#validate_ordering' do + let(:order_list) { described_class.build_order_list(relation) } + + context 'when number of ordering fields is 0' do + let(:relation) { Project.all } + + it 'raises an error' do + expect { described_class.validate_ordering(relation, order_list) } + .to raise_error(ArgumentError, 'A minimum of 1 ordering field is required') + end + end + + context 'when number of ordering fields is over 2' do + let(:relation) { Project.order(last_repository_check_at: :desc).order(updated_at: :desc).order(:id) } + + it 'raises an error' do + expect { described_class.validate_ordering(relation, order_list) } + .to raise_error(ArgumentError, 'A maximum of 2 ordering fields are allowed') + end + end + + context 'when the second (or first) column is nullable' do + let(:relation) { Project.order(last_repository_check_at: :desc).order(updated_at: :desc) } + + it 'raises an error' do + expect { described_class.validate_ordering(relation, order_list) } + .to raise_error(ArgumentError, "Column `updated_at` must not allow NULL") + end + end + + context 'for last ordering field' do + let(:relation) { Project.order(namespace_id: :desc) } + + it 'raises error if primary key is not last field' do + expect { described_class.validate_ordering(relation, order_list) } + .to raise_error(ArgumentError, "Last ordering field must be the primary key, `#{relation.primary_key}`") + end + end + end +end diff --git a/spec/lib/gitlab/graphql/connections/keyset/query_builder_spec.rb b/spec/lib/gitlab/graphql/connections/keyset/query_builder_spec.rb new file mode 100644 index 00000000000..59e153d9e07 --- /dev/null +++ b/spec/lib/gitlab/graphql/connections/keyset/query_builder_spec.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Graphql::Connections::Keyset::QueryBuilder do + context 'when number of ordering fields is 0' do + it 'raises an error' do + expect { described_class.new(Issue.arel_table, [], {}, :after) } + .to raise_error(ArgumentError, 'No ordering scopes have been supplied') + end + end + + describe '#conditions' do + let(:relation) { Issue.order(relative_position: :desc).order(:id) } + let(:order_list) { Gitlab::Graphql::Connections::Keyset::OrderInfo.build_order_list(relation) } + let(:builder) { described_class.new(arel_table, order_list, decoded_cursor, before_or_after) } + let(:before_or_after) { :after } + + context 'when only a single ordering' do + let(:relation) { Issue.order(id: :desc) } + + context 'when the value is nil' do + let(:decoded_cursor) { { 'id' => nil } } + + it 'raises an error' do + expect { builder.conditions } + .to raise_error(Gitlab::Graphql::Errors::ArgumentError, 'Before/after cursor invalid: `nil` was provided as only sortable value') + end + end + + context 'when value is not nil' do + let(:decoded_cursor) { { 'id' => 100 } } + let(:conditions) { builder.conditions } + + context 'when :after' do + it 'generates the correct condition' do + expect(conditions.strip).to eq '("issues"."id" < 100)' + end + end + + context 'when :before' do + let(:before_or_after) { :before } + + it 'generates the correct condition' do + expect(conditions.strip).to eq '("issues"."id" > 100)' + end + end + end + end + + context 'when two orderings' do + let(:decoded_cursor) { { 'relative_position' => 1500, 'id' => 100 } } + + context 'when no values are nil' do + context 'when :after' do + it 'generates the correct condition' do + conditions = builder.conditions + + expect(conditions).to include '"issues"."relative_position" < 1500' + expect(conditions).to include '"issues"."id" > 100' + expect(conditions).to include 'OR ("issues"."relative_position" IS NULL)' + end + end + + context 'when :before' do + let(:before_or_after) { :before } + + it 'generates the correct condition' do + conditions = builder.conditions + + expect(conditions).to include '("issues"."relative_position" > 1500)' + expect(conditions).to include '"issues"."id" < 100' + expect(conditions).to include '"issues"."relative_position" = 1500' + end + end + end + + context 'when first value is nil' do + let(:decoded_cursor) { { 'relative_position' => nil, 'id' => 100 } } + + context 'when :after' do + it 'generates the correct condition' do + conditions = builder.conditions + + expect(conditions).to include '"issues"."relative_position" IS NULL' + expect(conditions).to include '"issues"."id" > 100' + end + end + + context 'when :before' do + let(:before_or_after) { :before } + + it 'generates the correct condition' do + conditions = builder.conditions + + expect(conditions).to include '"issues"."relative_position" IS NULL' + expect(conditions).to include '"issues"."id" < 100' + expect(conditions).to include 'OR ("issues"."relative_position" IS NOT NULL)' + end + end + end + end + end + + def arel_table + Issue.arel_table + end +end diff --git a/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb b/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb deleted file mode 100644 index 4eb121794e1..00000000000 --- a/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb +++ /dev/null @@ -1,117 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Graphql::Connections::KeysetConnection do - let(:nodes) { Project.all.order(id: :asc) } - let(:arguments) { {} } - subject(:connection) do - described_class.new(nodes, arguments, max_page_size: 3) - end - - def encoded_property(value) - Base64Bp.urlsafe_encode64(value.to_s, padding: false) - end - - describe '#cursor_from_nodes' do - let(:project) { create(:project) } - - it 'returns an encoded ID' do - expect(connection.cursor_from_node(project)) - .to eq(encoded_property(project.id)) - end - - context 'when an order was specified' do - let(:nodes) { Project.order(:updated_at) } - - it 'returns the encoded value of the order' do - expect(connection.cursor_from_node(project)) - .to eq(encoded_property(project.updated_at)) - end - end - end - - describe '#sliced_nodes' do - let(:projects) { create_list(:project, 4) } - - context 'when before is passed' do - let(:arguments) { { before: encoded_property(projects[1].id) } } - - it 'only returns the project before the selected one' do - expect(subject.sliced_nodes).to contain_exactly(projects.first) - end - - context 'when the sort order is descending' do - let(:nodes) { Project.all.order(id: :desc) } - - it 'returns the correct nodes' do - expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1]) - end - end - end - - context 'when after is passed' do - let(:arguments) { { after: encoded_property(projects[1].id) } } - - it 'only returns the project before the selected one' do - expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1]) - end - - context 'when the sort order is descending' do - let(:nodes) { Project.all.order(id: :desc) } - - it 'returns the correct nodes' do - expect(subject.sliced_nodes).to contain_exactly(projects.first) - end - end - end - - context 'when both before and after are passed' do - let(:arguments) do - { - after: encoded_property(projects[1].id), - before: encoded_property(projects[3].id) - } - end - - it 'returns the expected set' do - expect(subject.sliced_nodes).to contain_exactly(projects[2]) - end - end - end - - describe '#paged_nodes' do - let!(:projects) { create_list(:project, 5) } - - it 'returns the collection limited to max page size' do - expect(subject.paged_nodes.size).to eq(3) - end - - it 'is a loaded memoized array' do - expect(subject.paged_nodes).to be_an(Array) - expect(subject.paged_nodes.object_id).to eq(subject.paged_nodes.object_id) - end - - context 'when `first` is passed' do - let(:arguments) { { first: 2 } } - - it 'returns only the first elements' do - expect(subject.paged_nodes).to contain_exactly(projects.first, projects.second) - end - end - - context 'when `last` is passed' do - let(:arguments) { { last: 2 } } - - it 'returns only the last elements' do - expect(subject.paged_nodes).to contain_exactly(projects[3], projects[4]) - end - end - - context 'when both are passed' do - let(:arguments) { { first: 2, last: 2 } } - - it 'raises an error' do - expect { subject.paged_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) - end - end - end -end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 8a3a7eee25d..47530025620 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -686,12 +686,36 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do context 'the cluster has a provider' do let(:cluster) { create(:cluster, :provided_by_gcp) } + let(:provider_status) { :errored } before do cluster.provider.make_errored! end - it { is_expected.to eq :errored } + it { is_expected.to eq provider_status } + + context 'when cluster cleanup is ongoing' do + using RSpec::Parameterized::TableSyntax + + where(:status_name, :cleanup_status) do + provider_status | :cleanup_not_started + :cleanup_ongoing | :cleanup_uninstalling_applications + :cleanup_ongoing | :cleanup_removing_project_namespaces + :cleanup_ongoing | :cleanup_removing_service_account + :cleanup_errored | :cleanup_errored + end + + with_them do + it 'returns cleanup_ongoing when uninstalling applications' do + cluster.cleanup_status = described_class + .state_machines[:cleanup_status] + .states[cleanup_status] + .value + + is_expected.to eq status_name + end + end + end end context 'there is a cached connection status' do @@ -715,6 +739,83 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end + describe 'cleanup_status state_machine' do + shared_examples 'cleanup_status transition' do + let(:cluster) { create(:cluster, from_state) } + + it 'transitions cleanup_status correctly' do + expect { subject }.to change { cluster.cleanup_status_name } + .from(from_state).to(to_state) + end + + it 'schedules a Clusters::Cleanup::*Worker' do + expect(expected_worker_class).to receive(:perform_async).with(cluster.id) + subject + end + end + + describe '#start_cleanup!' do + let(:expected_worker_class) { Clusters::Cleanup::AppWorker } + let(:to_state) { :cleanup_uninstalling_applications } + + subject { cluster.start_cleanup! } + + context 'when cleanup_status is cleanup_not_started' do + let(:from_state) { :cleanup_not_started } + + it_behaves_like 'cleanup_status transition' + end + + context 'when cleanup_status is errored' do + let(:from_state) { :cleanup_errored } + + it_behaves_like 'cleanup_status transition' + end + end + + describe '#make_cleanup_errored!' do + NON_ERRORED_STATES = Clusters::Cluster.state_machines[:cleanup_status].states.keys - [:cleanup_errored] + + NON_ERRORED_STATES.each do |state| + it "transitions cleanup_status from #{state} to cleanup_errored" do + cluster = create(:cluster, state) + + expect { cluster.make_cleanup_errored! }.to change { cluster.cleanup_status_name } + .from(state).to(:cleanup_errored) + end + + it "sets error message" do + cluster = create(:cluster, state) + + expect { cluster.make_cleanup_errored!("Error Message") }.to change { cluster.cleanup_status_reason } + .from(nil).to("Error Message") + end + end + end + + describe '#continue_cleanup!' do + context 'when cleanup_status is cleanup_uninstalling_applications' do + let(:expected_worker_class) { Clusters::Cleanup::ProjectNamespaceWorker } + let(:from_state) { :cleanup_uninstalling_applications } + let(:to_state) { :cleanup_removing_project_namespaces } + + subject { cluster.continue_cleanup! } + + it_behaves_like 'cleanup_status transition' + end + + context 'when cleanup_status is cleanup_removing_project_namespaces' do + let(:expected_worker_class) { Clusters::Cleanup::ServiceAccountWorker } + let(:from_state) { :cleanup_removing_project_namespaces } + let(:to_state) { :cleanup_removing_service_account } + + subject { cluster.continue_cleanup! } + + it_behaves_like 'cleanup_status transition' + end + end + end + describe '#connection_status' do let(:cluster) { create(:cluster) } let(:status) { :connected } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 91a743c4377..62f73c3867b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3368,7 +3368,7 @@ describe MergeRequest do end end - describe '.with_open_merge_when_pipeline_succeeds' do + describe '.with_auto_merge_enabled' do let!(:project) { create(:project) } let!(:fork) { fork_project(project) } let!(:merge_request1) do @@ -3380,15 +3380,6 @@ describe MergeRequest do source_branch: 'feature-1') end - let!(:merge_request2) do - create(:merge_request, - :merge_when_pipeline_succeeds, - target_project: project, - target_branch: 'master', - source_project: fork, - source_branch: 'fork-feature-1') - end - let!(:merge_request4) do create(:merge_request, target_project: project, @@ -3397,9 +3388,9 @@ describe MergeRequest do source_branch: 'fork-feature-2') end - let(:query) { described_class.with_open_merge_when_pipeline_succeeds } + let(:query) { described_class.with_auto_merge_enabled } - it { expect(query).to contain_exactly(merge_request1, merge_request2) } + it { expect(query).to contain_exactly(merge_request1) } end it_behaves_like 'versioned description' diff --git a/spec/requests/api/graphql/current_user/todos_query_spec.rb b/spec/requests/api/graphql/current_user/todos_query_spec.rb new file mode 100644 index 00000000000..6817e37e64b --- /dev/null +++ b/spec/requests/api/graphql/current_user/todos_query_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Query current user todos' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:commit_todo) { create(:on_commit_todo, user: current_user, project: create(:project, :repository)) } + let_it_be(:issue_todo) { create(:todo, user: current_user, target: create(:issue)) } + let_it_be(:merge_request_todo) { create(:todo, user: current_user, target: create(:merge_request)) } + + let(:fields) do + <<~QUERY + nodes { + id + } + QUERY + end + + let(:query) do + graphql_query_for('currentUser', {}, query_graphql_field('todos', {}, fields)) + end + + subject { graphql_data.dig('currentUser', 'todos', 'nodes') } + + before do + post_graphql(query, current_user: current_user) + end + + it 'contains the expected ids' do + is_expected.to include( + a_hash_including('id' => commit_todo.to_global_id.to_s), + a_hash_including('id' => issue_todo.to_global_id.to_s), + a_hash_including('id' => merge_request_todo.to_global_id.to_s) + ) + end +end diff --git a/spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb b/spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb index ac7b1575ec0..62f6c7a3414 100644 --- a/spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb +++ b/spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' diff --git a/spec/rubocop/cop/avoid_return_from_blocks_spec.rb b/spec/rubocop/cop/avoid_return_from_blocks_spec.rb index a5c280a7adc..133d286ccd2 100644 --- a/spec/rubocop/cop/avoid_return_from_blocks_spec.rb +++ b/spec/rubocop/cop/avoid_return_from_blocks_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' diff --git a/spec/rubocop/cop/destroy_all_spec.rb b/spec/rubocop/cop/destroy_all_spec.rb index b0bc40552b3..ac8aa56e040 100644 --- a/spec/rubocop/cop/destroy_all_spec.rb +++ b/spec/rubocop/cop/destroy_all_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' diff --git a/spec/rubocop/cop/gitlab/finder_with_find_by_spec.rb b/spec/rubocop/cop/gitlab/finder_with_find_by_spec.rb index 7f689b196c5..7af98b66218 100644 --- a/spec/rubocop/cop/gitlab/finder_with_find_by_spec.rb +++ b/spec/rubocop/cop/gitlab/finder_with_find_by_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/gitlab/httparty_spec.rb b/spec/rubocop/cop/gitlab/httparty_spec.rb index 510839a21d7..42da97679ec 100644 --- a/spec/rubocop/cop/gitlab/httparty_spec.rb +++ b/spec/rubocop/cop/gitlab/httparty_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' diff --git a/spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb b/spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb index 8e2d5f70353..9cb55ced1fa 100644 --- a/spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb +++ b/spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' diff --git a/spec/rubocop/cop/gitlab/predicate_memoization_spec.rb b/spec/rubocop/cop/gitlab/predicate_memoization_spec.rb index 21fc4584654..ae9466368d2 100644 --- a/spec/rubocop/cop/gitlab/predicate_memoization_spec.rb +++ b/spec/rubocop/cop/gitlab/predicate_memoization_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' diff --git a/spec/rubocop/cop/group_public_or_visible_to_user_spec.rb b/spec/rubocop/cop/group_public_or_visible_to_user_spec.rb index 7b5235a3da7..8e027ad59f7 100644 --- a/spec/rubocop/cop/group_public_or_visible_to_user_spec.rb +++ b/spec/rubocop/cop/group_public_or_visible_to_user_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' diff --git a/spec/rubocop/cop/include_sidekiq_worker_spec.rb b/spec/rubocop/cop/include_sidekiq_worker_spec.rb index f5109287876..39965646aff 100644 --- a/spec/rubocop/cop/include_sidekiq_worker_spec.rb +++ b/spec/rubocop/cop/include_sidekiq_worker_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb index cc933ce12c8..d09de4c6614 100644 --- a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb +++ b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' diff --git a/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb index 1df1fffb94e..419d74c298a 100644 --- a/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb +++ b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/add_concurrent_index_spec.rb b/spec/rubocop/cop/migration/add_concurrent_index_spec.rb index 9c1ebcc0ced..9812e64216f 100644 --- a/spec/rubocop/cop/migration/add_concurrent_index_spec.rb +++ b/spec/rubocop/cop/migration/add_concurrent_index_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/add_reference_spec.rb b/spec/rubocop/cop/migration/add_reference_spec.rb index 0b56fe8ed83..03348ecc744 100644 --- a/spec/rubocop/cop/migration/add_reference_spec.rb +++ b/spec/rubocop/cop/migration/add_reference_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/add_timestamps_spec.rb b/spec/rubocop/cop/migration/add_timestamps_spec.rb index 33f1bb85af8..a3314d878e5 100644 --- a/spec/rubocop/cop/migration/add_timestamps_spec.rb +++ b/spec/rubocop/cop/migration/add_timestamps_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/datetime_spec.rb b/spec/rubocop/cop/migration/datetime_spec.rb index f2d9483d8d3..0a771003100 100644 --- a/spec/rubocop/cop/migration/datetime_spec.rb +++ b/spec/rubocop/cop/migration/datetime_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/hash_index_spec.rb b/spec/rubocop/cop/migration/hash_index_spec.rb index 5d53dde9a79..e8b05a94653 100644 --- a/spec/rubocop/cop/migration/hash_index_spec.rb +++ b/spec/rubocop/cop/migration/hash_index_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/remove_column_spec.rb b/spec/rubocop/cop/migration/remove_column_spec.rb index f1a64f431bd..bc2fa04ce64 100644 --- a/spec/rubocop/cop/migration/remove_column_spec.rb +++ b/spec/rubocop/cop/migration/remove_column_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb b/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb index a23d5d022e3..9de4c756f12 100644 --- a/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb +++ b/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/remove_index_spec.rb b/spec/rubocop/cop/migration/remove_index_spec.rb index bbf2227e512..d343d27484a 100644 --- a/spec/rubocop/cop/migration/remove_index_spec.rb +++ b/spec/rubocop/cop/migration/remove_index_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb b/spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb index ba8cd2c6c4a..b3c5b855004 100644 --- a/spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb +++ b/spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/safer_boolean_column_spec.rb b/spec/rubocop/cop/migration/safer_boolean_column_spec.rb index 1c4f18fbcc3..915b73ed5a7 100644 --- a/spec/rubocop/cop/migration/safer_boolean_column_spec.rb +++ b/spec/rubocop/cop/migration/safer_boolean_column_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/timestamps_spec.rb b/spec/rubocop/cop/migration/timestamps_spec.rb index cafe255dc9a..d03c75e7cfc 100644 --- a/spec/rubocop/cop/migration/timestamps_spec.rb +++ b/spec/rubocop/cop/migration/timestamps_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/update_column_in_batches_spec.rb b/spec/rubocop/cop/migration/update_column_in_batches_spec.rb index cba01400d85..f72efaf2eb2 100644 --- a/spec/rubocop/cop/migration/update_column_in_batches_spec.rb +++ b/spec/rubocop/cop/migration/update_column_in_batches_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/migration/update_large_table_spec.rb b/spec/rubocop/cop/migration/update_large_table_spec.rb index 5e08eb4f772..0463b6550a8 100644 --- a/spec/rubocop/cop/migration/update_large_table_spec.rb +++ b/spec/rubocop/cop/migration/update_large_table_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/project_path_helper_spec.rb b/spec/rubocop/cop/project_path_helper_spec.rb index 84e6eb7d87f..1b69030c798 100644 --- a/spec/rubocop/cop/project_path_helper_spec.rb +++ b/spec/rubocop/cop/project_path_helper_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/rspec/env_assignment_spec.rb b/spec/rubocop/cop/rspec/env_assignment_spec.rb index 621afbad3ba..2a2bd1434d6 100644 --- a/spec/rubocop/cop/rspec/env_assignment_spec.rb +++ b/spec/rubocop/cop/rspec/env_assignment_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb b/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb index 94324bc615d..20013519db4 100644 --- a/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb +++ b/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/rubocop/cop/sidekiq_options_queue_spec.rb b/spec/rubocop/cop/sidekiq_options_queue_spec.rb index 7f237d5ffbb..c10fd7bd32b 100644 --- a/spec/rubocop/cop/sidekiq_options_queue_spec.rb +++ b/spec/rubocop/cop/sidekiq_options_queue_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' diff --git a/spec/serializers/blob_entity_spec.rb b/spec/serializers/blob_entity_spec.rb index c0687d0232e..7e3a0a87bd5 100644 --- a/spec/serializers/blob_entity_spec.rb +++ b/spec/serializers/blob_entity_spec.rb @@ -15,8 +15,16 @@ describe BlobEntity do context 'as json' do subject { entity.as_json } - it 'exposes needed attributes' do - expect(subject).to include(:readable_text, :url) + it 'contains needed attributes' do + expect(subject).to include({ + id: blob.id, + path: blob.path, + name: blob.name, + mode: "100644", + readable_text: true, + icon: "file-text-o", + url: "/#{project.full_path}/blob/master/bar/branch-test.txt" + }) end end end diff --git a/spec/serializers/diff_file_base_entity_spec.rb b/spec/serializers/diff_file_base_entity_spec.rb index 68c5c665ed6..80f5bc8f159 100644 --- a/spec/serializers/diff_file_base_entity_spec.rb +++ b/spec/serializers/diff_file_base_entity_spec.rb @@ -5,15 +5,15 @@ require 'spec_helper' describe DiffFileBaseEntity do let(:project) { create(:project, :repository) } let(:repository) { project.repository } + let(:entity) { described_class.new(diff_file, options).as_json } context 'diff for a changed submodule' do let(:commit_sha_with_changed_submodule) do "cfe32cf61b73a0d5e9f13e774abde7ff789b1660" end let(:commit) { project.commit(commit_sha_with_changed_submodule) } - let(:diff_file) { commit.diffs.diff_files.to_a.last } let(:options) { { request: {}, submodule_links: Gitlab::SubmoduleLinks.new(repository) } } - let(:entity) { described_class.new(diff_file, options).as_json } + let(:diff_file) { commit.diffs.diff_files.to_a.last } it do expect(entity[:submodule]).to eq(true) @@ -23,4 +23,15 @@ describe DiffFileBaseEntity do ) end end + + context 'contains raw sizes for the blob' do + let(:commit) { project.commit('png-lfs') } + let(:options) { { request: {} } } + let(:diff_file) { commit.diffs.diff_files.to_a.second } + + it do + expect(entity[:old_size]).to eq(1219696) + expect(entity[:new_size]).to eq(132) + end + end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 58302ce14ba..9d0ad60a624 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -769,7 +769,7 @@ describe MergeRequests::RefreshService do fork_project(target_project, author, repository: true) end - let_it_be(:merge_request) do + let_it_be(:merge_request, refind: true) do create(:merge_request, author: author, source_project: source_project, @@ -795,88 +795,58 @@ describe MergeRequests::RefreshService do .parent_id end + let(:auto_merge_strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS } let(:refresh_service) { service.new(project, user) } before do target_project.merge_method = merge_method target_project.save! + merge_request.auto_merge_strategy = auto_merge_strategy + merge_request.save! refresh_service.execute(oldrev, newrev, 'refs/heads/master') merge_request.reload end - let(:aborted_message) do - /aborted the automatic merge because target branch was updated/ - end - - shared_examples 'aborted MWPS' do - it 'aborts auto_merge' do - expect(merge_request.auto_merge_enabled?).to be_falsey - expect(merge_request.notes.last.note).to match(aborted_message) - end - - it 'removes merge_user' do - expect(merge_request.merge_user).to be_nil - end - - it 'does not add todos for merge user' do - expect(user.todos.for_target(merge_request)).to be_empty - end - - it 'adds todos for merge author' do - expect(author.todos.for_target(merge_request)).to be_present.and be_all(&:pending?) - end - end - context 'when Project#merge_method is set to FF' do let(:merge_method) { :ff } - it_behaves_like 'aborted MWPS' + it_behaves_like 'aborted merge requests for MWPS' context 'with forked project' do let(:source_project) { forked_project } - it_behaves_like 'aborted MWPS' + it_behaves_like 'aborted merge requests for MWPS' + end + + context 'with bogus auto merge strategy' do + let(:auto_merge_strategy) { 'bogus' } + + it_behaves_like 'maintained merge requests for MWPS' end end context 'when Project#merge_method is set to rebase_merge' do let(:merge_method) { :rebase_merge } - it_behaves_like 'aborted MWPS' + it_behaves_like 'aborted merge requests for MWPS' context 'with forked project' do let(:source_project) { forked_project } - it_behaves_like 'aborted MWPS' + it_behaves_like 'aborted merge requests for MWPS' end end context 'when Project#merge_method is set to merge' do let(:merge_method) { :merge } - shared_examples 'maintained MWPS' do - it 'does not cancel auto merge' do - expect(merge_request.auto_merge_enabled?).to be_truthy - expect(merge_request.notes).to be_empty - end - - it 'does not change merge_user' do - expect(merge_request.merge_user).to eq(user) - end - - it 'does not add todos' do - expect(author.todos.for_target(merge_request)).to be_empty - expect(user.todos.for_target(merge_request)).to be_empty - end - end - - it_behaves_like 'maintained MWPS' + it_behaves_like 'maintained merge requests for MWPS' context 'with forked project' do let(:source_project) { forked_project } - it_behaves_like 'maintained MWPS' + it_behaves_like 'maintained merge requests for MWPS' end end end diff --git a/spec/support/generate-seed-repo-rb b/spec/support/generate-seed-repo-rb index bee9d419376..b63ff7147ec 100755 --- a/spec/support/generate-seed-repo-rb +++ b/spec/support/generate-seed-repo-rb @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true # # # generate-seed-repo-rb # @@ -15,9 +16,9 @@ require 'erb' require 'tempfile' -SOURCE = File.expand_path('gitlab-git-test.git', __dir__).freeze -SCRIPT_NAME = 'generate-seed-repo-rb'.freeze -REPO_NAME = 'gitlab-git-test.git'.freeze +SOURCE = File.expand_path('gitlab-git-test.git', __dir__) +SCRIPT_NAME = 'generate-seed-repo-rb' +REPO_NAME = 'gitlab-git-test.git' def main Dir.mktmpdir do |dir| diff --git a/spec/support/prepare-gitlab-git-test-for-commit b/spec/support/prepare-gitlab-git-test-for-commit index d08e3ba5481..77c7f309312 100755 --- a/spec/support/prepare-gitlab-git-test-for-commit +++ b/spec/support/prepare-gitlab-git-test-for-commit @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true abort unless [ system('spec/support/generate-seed-repo-rb', out: 'spec/support/helpers/seed_repo.rb'), diff --git a/spec/support/shared_examples/ci/auto_merge_merge_requests_examples.rb b/spec/support/shared_examples/ci/auto_merge_merge_requests_examples.rb new file mode 100644 index 00000000000..c11448ffe0f --- /dev/null +++ b/spec/support/shared_examples/ci/auto_merge_merge_requests_examples.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +shared_examples 'aborted merge requests for MWPS' do + let(:aborted_message) do + /aborted the automatic merge because target branch was updated/ + end + + it 'aborts auto_merge' do + expect(merge_request.auto_merge_enabled?).to be_falsey + expect(merge_request.notes.last.note).to match(aborted_message) + end + + it 'removes merge_user' do + expect(merge_request.merge_user).to be_nil + end + + it 'does not add todos for merge user' do + expect(user.todos.for_target(merge_request)).to be_empty + end + + it 'adds todos for merge author' do + expect(author.todos.for_target(merge_request)).to be_present.and be_all(&:pending?) + end +end + +shared_examples 'maintained merge requests for MWPS' do + it 'does not cancel auto merge' do + expect(merge_request.auto_merge_enabled?).to be_truthy + expect(merge_request.notes).to be_empty + end + + it 'does not change merge_user' do + expect(merge_request.merge_user).to eq(user) + end + + it 'does not add todos' do + expect(author.todos.for_target(merge_request)).to be_empty + expect(user.todos.for_target(merge_request)).to be_empty + end +end diff --git a/spec/support/unpack-gitlab-git-test b/spec/support/unpack-gitlab-git-test index d5b4912457d..5d5f1b7d082 100755 --- a/spec/support/unpack-gitlab-git-test +++ b/spec/support/unpack-gitlab-git-test @@ -1,10 +1,12 @@ #!/usr/bin/env ruby +# frozen_string_literal: true + require 'fileutils' -REPO = 'spec/support/gitlab-git-test.git'.freeze +REPO = 'spec/support/gitlab-git-test.git' PACK_DIR = REPO + '/objects/pack' GIT = %W[git --git-dir=#{REPO}].freeze -BASE_PACK = 'pack-691247af2a6acb0b63b73ac0cb90540e93614043'.freeze +BASE_PACK = 'pack-691247af2a6acb0b63b73ac0cb90540e93614043' def main unpack diff --git a/spec/views/projects/tree/_tree_header.html.haml_spec.rb b/spec/views/projects/tree/_tree_header.html.haml_spec.rb index 4b71ea9ffe3..caf8c4d1969 100644 --- a/spec/views/projects/tree/_tree_header.html.haml_spec.rb +++ b/spec/views/projects/tree/_tree_header.html.haml_spec.rb @@ -8,6 +8,8 @@ describe 'projects/tree/_tree_header' do let(:repository) { project.repository } before do + stub_feature_flags(vue_file_list: false) + assign(:project, project) assign(:repository, repository) assign(:id, File.join('master', '')) |