diff options
54 files changed, 576 insertions, 290 deletions
diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 5b29a76e886..7d5932886d3 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -163,7 +163,7 @@ frontend-fixtures: frontend-fixtures-as-if-foss: extends: - .frontend-fixtures-base - - .frontend:rules:default-frontend-jobs-as-if-foss + - .frontend:rules:default-frontend-jobs-no-foss - .as-if-foss .frontend-job-base: @@ -206,7 +206,7 @@ karma: karma-as-if-foss: extends: - .karma-base - - .frontend:rules:default-frontend-jobs-as-if-foss + - .frontend:rules:default-frontend-jobs-no-foss - .as-if-foss needs: ["frontend-fixtures-as-if-foss"] @@ -241,7 +241,7 @@ jest: jest-as-if-foss: extends: - .jest-base - - .frontend:rules:default-frontend-jobs-as-if-foss + - .frontend:rules:default-frontend-jobs-no-foss - .as-if-foss needs: ["frontend-fixtures-as-if-foss"] cache: @@ -250,7 +250,7 @@ jest-as-if-foss: coverage-frontend: extends: - .default-retry - - .frontend:rules:default-frontend-jobs + - .frontend:rules:default-frontend-jobs-no-foss needs: ["jest"] stage: post-test before_script: diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index aae6f5a1e8e..6420e951a78 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -273,7 +273,7 @@ changes: *code-backstage-patterns when: on_success -.frontend:rules:default-frontend-jobs-as-if-foss: +.frontend:rules:default-frontend-jobs-no-foss: rules: - <<: *if-not-ee when: never diff --git a/.rubocop.yml b/.rubocop.yml index a05fffe8740..9251c961289 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -200,6 +200,35 @@ GitlabSecurity/PublicSend: - 'ee/lib/**/*.rake' - 'ee/spec/**/*' +Gitlab/DuplicateSpecLocation: + Exclude: + - ee/spec/controllers/groups_controller_spec.rb + - ee/spec/controllers/projects/jobs_controller_spec.rb + - ee/spec/helpers/auth_helper_spec.rb + - ee/spec/lib/gitlab/gl_repository_spec.rb + - ee/spec/lib/gitlab/usage_data_spec.rb + - ee/spec/models/namespace_spec.rb + - ee/spec/models/note_spec.rb + - ee/spec/serializers/environment_entity_spec.rb + - ee/spec/services/issues/create_service_spec.rb + - ee/spec/services/merge_requests/create_service_spec.rb + - ee/spec/services/merge_requests/refresh_service_spec.rb + - ee/spec/services/merge_requests/update_service_spec.rb + - ee/spec/services/system_hooks_service_spec.rb + - ee/spec/controllers/ee/groups_controller_spec.rb + - ee/spec/controllers/ee/projects/jobs_controller_spec.rb + - ee/spec/helpers/ee/auth_helper_spec.rb + - ee/spec/lib/ee/gitlab/gl_repository_spec.rb + - ee/spec/lib/ee/gitlab/usage_data_spec.rb + - ee/spec/models/ee/namespace_spec.rb + - ee/spec/models/ee/note_spec.rb + - ee/spec/serializers/ee/environment_entity_spec.rb + - ee/spec/services/ee/issues/create_service_spec.rb + - ee/spec/services/ee/merge_requests/create_service_spec.rb + - ee/spec/services/ee/merge_requests/refresh_service_spec.rb + - ee/spec/services/ee/merge_requests/update_service_spec.rb + - ee/spec/services/ee/system_hooks_service_spec.rb + Cop/InjectEnterpriseEditionModule: Enabled: true Exclude: diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index 88a7e3e3a68..9cf42b34974 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -1,7 +1,7 @@ <script> import { escapeRegExp } from 'lodash'; import { GlBadge, GlLink, GlSkeletonLoading, GlTooltipDirective, GlLoadingIcon } from '@gitlab/ui'; -import { visitUrl, escapeFileUrl } from '~/lib/utils/url_utility'; +import { escapeFileUrl } from '~/lib/utils/url_utility'; import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import Icon from '~/vue_shared/components/icon.vue'; import { getIconName } from '../../utils/icon'; @@ -117,39 +117,37 @@ export default { return this.commit && this.commit.lockLabel; }, }, - methods: { - openRow(e) { - if (e.target.tagName === 'A') return; - - if (this.isFolder && !e.metaKey) { - this.$router.push(this.routerLinkTo); - } else { - visitUrl(this.url, e.metaKey); - } - }, - }, }; </script> <template> - <tr :class="`file_${id}`" class="tree-item" @click="openRow"> - <td class="tree-item-file-name"> + <tr class="tree-item"> + <td class="tree-item-file-name cursor-default position-relative"> <gl-loading-icon v-if="path === loadingPath" size="sm" inline class="d-inline-block align-text-bottom fa-fw" /> - <i v-else :aria-label="type" role="img" :class="iconName" class="fa fa-fw"></i> <component :is="linkComponent" ref="link" :to="routerLinkTo" :href="url" - class="str-truncated" + :class="{ + 'is-submodule': isSubmodule, + }" + class="tree-item-link str-truncated" data-qa-selector="file_name_link" > - {{ fullPath }} + <i + v-if="!loadingPath" + :aria-label="type" + role="img" + :class="iconName" + class="fa fa-fw mr-1" + ></i + ><span class="position-relative">{{ fullPath }}</span> </component> <!-- eslint-disable-next-line @gitlab/vue-require-i18n-strings --> <gl-badge v-if="lfsOid" variant="default" class="label-lfs ml-1">LFS</gl-badge> @@ -165,7 +163,7 @@ export default { class="ml-2 vertical-align-middle" /> </td> - <td class="d-none d-sm-table-cell tree-commit"> + <td class="d-none d-sm-table-cell tree-commit cursor-default"> <gl-link v-if="commit" :href="commit.commitPath" @@ -176,7 +174,7 @@ export default { </gl-link> <gl-skeleton-loading v-else :lines="1" class="h-auto" /> </td> - <td class="tree-time-ago text-right"> + <td class="tree-time-ago text-right cursor-default"> <timeago-tooltip v-if="commit" :time="commit.committedDate" /> <gl-skeleton-loading v-else :lines="1" class="ml-auto h-auto w-50" /> </td> diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index bb1c304b9fe..9d21b67c95e 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -511,3 +511,21 @@ span.idiff { .code-navigation-popover { max-width: 450px; } + +.tree-item-link { + &:not(.is-submodule) { + span { + z-index: 2; + } + + &::before { + content: ''; + position: absolute; + left: 0; + top: 0; + width: 100%; + height: 100%; + z-index: 1; + } + } +} diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 953b2ffeb0b..1fb5e333aaa 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -65,7 +65,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic options = additional_attributes.merge(diff_view: diff_view) if @merge_request.project.context_commits_enabled? - options[:context_commits] = @merge_request.context_commits + options[:context_commits] = @merge_request.recent_context_commits end render json: DiffsSerializer.new(request).represent(diffs, options) diff --git a/app/graphql/resolvers/error_tracking/sentry_detailed_error_resolver.rb b/app/graphql/resolvers/error_tracking/sentry_detailed_error_resolver.rb index 72c5c19c25c..5027403e95c 100644 --- a/app/graphql/resolvers/error_tracking/sentry_detailed_error_resolver.rb +++ b/app/graphql/resolvers/error_tracking/sentry_detailed_error_resolver.rb @@ -9,7 +9,7 @@ module Resolvers def resolve(**args) current_user = context[:current_user] - issue_id = GlobalID.parse(args[:id]).model_id + issue_id = GlobalID.parse(args[:id])&.model_id # Get data from Sentry response = ::ErrorTracking::IssueDetailsService.new( diff --git a/app/graphql/resolvers/error_tracking/sentry_error_stack_trace_resolver.rb b/app/graphql/resolvers/error_tracking/sentry_error_stack_trace_resolver.rb index f5356660569..c365baaf475 100644 --- a/app/graphql/resolvers/error_tracking/sentry_error_stack_trace_resolver.rb +++ b/app/graphql/resolvers/error_tracking/sentry_error_stack_trace_resolver.rb @@ -8,7 +8,7 @@ module Resolvers description: 'ID of the Sentry issue' def resolve(**args) - issue_id = GlobalID.parse(args[:id]).model_id + issue_id = GlobalID.parse(args[:id])&.model_id # Get data from Sentry response = ::ErrorTracking::IssueLatestEventService.new( diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 412d0fa4ec8..bb7afc49cd8 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -410,8 +410,16 @@ class MergeRequest < ApplicationRecord "#{project.to_reference_base(from, full: full)}#{reference}" end - def context_commits - @context_commits ||= merge_request_context_commits.map(&:to_commit) + def context_commits(limit: nil) + @context_commits ||= merge_request_context_commits.limit(limit).map(&:to_commit) + end + + def recent_context_commits + context_commits(limit: MergeRequestDiff::COMMITS_SAFE_SIZE) + end + + def context_commits_count + context_commits.count end def commits(limit: nil) diff --git a/app/models/resource_milestone_event.rb b/app/models/resource_milestone_event.rb index d362ebc307a..b97c02f1713 100644 --- a/app/models/resource_milestone_event.rb +++ b/app/models/resource_milestone_event.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ResourceMilestoneEvent < ResourceEvent + include IgnorableColumns + belongs_to :issue belongs_to :merge_request belongs_to :milestone @@ -18,6 +20,8 @@ class ResourceMilestoneEvent < ResourceEvent # state is used for issue and merge request states. enum state: Issue.available_states.merge(MergeRequest.available_states) + ignore_columns %i[reference reference_html cached_markdown_version], remove_with: '13.1', remove_after: '2020-06-22' + def self.issuable_attrs %i(issue merge_request).freeze end diff --git a/app/serializers/commit_entity.rb b/app/serializers/commit_entity.rb index ae3f1c6bbf5..c9c4981682a 100644 --- a/app/serializers/commit_entity.rb +++ b/app/serializers/commit_entity.rb @@ -1,49 +1,4 @@ # frozen_string_literal: true -class CommitEntity < API::Entities::Commit - include MarkupHelper - include RequestAwareEntity - - expose :author, using: UserEntity - - expose :author_gravatar_url do |commit| - GravatarService.new.execute(commit.author_email) # rubocop: disable CodeReuse/ServiceClass - end - - expose :commit_url do |commit, options| - project_commit_url(request.project, commit, params: options.fetch(:commit_url_params, {})) - end - - expose :commit_path do |commit, options| - project_commit_path(request.project, commit, params: options.fetch(:commit_url_params, {})) - end - - expose :description_html, if: { type: :full } do |commit| - markdown_field(commit, :description) - end - - expose :title_html, if: { type: :full } do |commit| - markdown_field(commit, :title) - end - - expose :signature_html, if: { type: :full } do |commit| - render('projects/commit/_signature', signature: commit.signature) if commit.has_signature? - end - - expose :pipeline_status_path, if: { type: :full } do |commit, options| - pipeline_ref = options[:pipeline_ref] - pipeline_project = options[:pipeline_project] || commit.project - next unless pipeline_ref && pipeline_project - - pipeline = commit.latest_pipeline_for_project(pipeline_ref, pipeline_project) - next unless pipeline&.status - - pipelines_project_commit_path(pipeline_project, commit.id, ref: pipeline_ref) - end - - def render(*args) - return unless request.respond_to?(:render) && request.render.respond_to?(:call) - - request.render.call(*args) - end +class CommitEntity < API::Entities::CommitWithLink end diff --git a/app/serializers/user_entity.rb b/app/serializers/user_entity.rb index 656900bb8af..8909ae8df2c 100644 --- a/app/serializers/user_entity.rb +++ b/app/serializers/user_entity.rb @@ -1,10 +1,4 @@ # frozen_string_literal: true -class UserEntity < API::Entities::UserBasic - include RequestAwareEntity - include UserStatusTooltip - - expose :path do |user| - user_path(user) - end +class UserEntity < API::Entities::UserPath end diff --git a/app/services/issuable/clone/attributes_rewriter.rb b/app/services/issuable/clone/attributes_rewriter.rb index 135ab011d69..55f5629baac 100644 --- a/app/services/issuable/clone/attributes_rewriter.rb +++ b/app/services/issuable/clone/attributes_rewriter.rb @@ -74,7 +74,7 @@ module Issuable if matching_destination_milestone.present? event.attributes - .except('id', 'reference', 'reference_html') + .except('id') .merge(entity_key => new_entity.id, 'milestone_id' => matching_destination_milestone.id, 'action' => ResourceMilestoneEvent.actions[event.action], diff --git a/app/services/metrics/dashboard/clone_dashboard_service.rb b/app/services/metrics/dashboard/clone_dashboard_service.rb index c31d6d326e3..ee5b50aefc3 100644 --- a/app/services/metrics/dashboard/clone_dashboard_service.rb +++ b/app/services/metrics/dashboard/clone_dashboard_service.rb @@ -6,7 +6,7 @@ module Metrics module Dashboard class CloneDashboardService < ::BaseService ALLOWED_FILE_TYPE = '.yml' - USER_DASHBOARDS_DIR = ::Metrics::Dashboard::ProjectDashboardService::DASHBOARD_ROOT + USER_DASHBOARDS_DIR = ::Metrics::Dashboard::CustomDashboardService::DASHBOARD_ROOT class << self def allowed_dashboard_templates @@ -52,7 +52,7 @@ module Metrics def dashboard_details { path: new_dashboard_path, - display_name: ::Metrics::Dashboard::ProjectDashboardService.name_for_path(new_dashboard_path), + display_name: ::Metrics::Dashboard::CustomDashboardService.name_for_path(new_dashboard_path), default: false, system_dashboard: false } diff --git a/app/services/metrics/dashboard/project_dashboard_service.rb b/app/services/metrics/dashboard/custom_dashboard_service.rb index fadbe0fae01..77173813a4f 100644 --- a/app/services/metrics/dashboard/project_dashboard_service.rb +++ b/app/services/metrics/dashboard/custom_dashboard_service.rb @@ -5,7 +5,7 @@ # Use Gitlab::Metrics::Dashboard::Finder to retrive dashboards. module Metrics module Dashboard - class ProjectDashboardService < ::Metrics::Dashboard::BaseService + class CustomDashboardService < ::Metrics::Dashboard::BaseService DASHBOARD_ROOT = ".gitlab/dashboards" class << self diff --git a/app/services/metrics/dashboard/update_dashboard_service.rb b/app/services/metrics/dashboard/update_dashboard_service.rb index 25a727ad44c..d37d06a0222 100644 --- a/app/services/metrics/dashboard/update_dashboard_service.rb +++ b/app/services/metrics/dashboard/update_dashboard_service.rb @@ -7,7 +7,7 @@ module Metrics include Stepable ALLOWED_FILE_TYPE = '.yml' - USER_DASHBOARDS_DIR = ::Metrics::Dashboard::ProjectDashboardService::DASHBOARD_ROOT + USER_DASHBOARDS_DIR = ::Metrics::Dashboard::CustomDashboardService::DASHBOARD_ROOT steps :check_push_authorized, :check_branch_name, @@ -117,7 +117,7 @@ module Metrics def dashboard_details { path: update_dashboard_path, - display_name: ::Metrics::Dashboard::ProjectDashboardService.name_for_path(update_dashboard_path), + display_name: ::Metrics::Dashboard::CustomDashboardService.name_for_path(update_dashboard_path), default: false, system_dashboard: false } diff --git a/app/views/projects/wikis/_sidebar.html.haml b/app/views/projects/wikis/_sidebar.html.haml index 83d145444d8..0f2938686cc 100644 --- a/app/views/projects/wikis/_sidebar.html.haml +++ b/app/views/projects/wikis/_sidebar.html.haml @@ -17,5 +17,6 @@ %ul.wiki-pages = render @sidebar_wiki_entries, context: 'sidebar' .block.w-100 - = link_to project_wikis_pages_path(@project), class: 'btn btn-block' do - = s_("Wiki|More Pages") + - if @sidebar_wiki_entries&.length.to_i >= 15 + = link_to project_wikis_pages_path(@project), class: 'btn btn-block' do + = s_("Wiki|View All Pages") diff --git a/app/views/shared/web_hooks/_index.html.haml b/app/views/shared/web_hooks/_index.html.haml index b22d51a101a..149f4baeb21 100644 --- a/app/views/shared/web_hooks/_index.html.haml +++ b/app/views/shared/web_hooks/_index.html.haml @@ -3,7 +3,7 @@ .card-header %h5 = hook_class.underscore.humanize.titleize.pluralize - (#{hooks.count}) + (#{hooks.load.size}) - if hooks.any? %ul.content-list diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 19158e7173c..f4d8483db84 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -891,7 +891,7 @@ :urgency: :low :resource_boundary: :unknown :weight: 1 - :idempotent: + :idempotent: true - :name: update_namespace_statistics:namespaces_schedule_aggregation :feature_category: :source_code_management :has_external_dependencies: diff --git a/app/workers/namespaces/root_statistics_worker.rb b/app/workers/namespaces/root_statistics_worker.rb index 70b2510488b..5d4b510ceb7 100644 --- a/app/workers/namespaces/root_statistics_worker.rb +++ b/app/workers/namespaces/root_statistics_worker.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true module Namespaces - class RootStatisticsWorker # rubocop:disable Scalability/IdempotentWorker + class RootStatisticsWorker include ApplicationWorker queue_namespace :update_namespace_statistics feature_category :source_code_management + idempotent! def perform(namespace_id) namespace = Namespace.find(namespace_id) diff --git a/changelogs/unreleased/211408-update-more-pages-button-on-wiki-page.yml b/changelogs/unreleased/211408-update-more-pages-button-on-wiki-page.yml new file mode 100644 index 00000000000..b59982583af --- /dev/null +++ b/changelogs/unreleased/211408-update-more-pages-button-on-wiki-page.yml @@ -0,0 +1,5 @@ +--- +title: Update More Pages button on Wiki Page +merge_request: 27499 +author: +type: changed diff --git a/changelogs/unreleased/fix-openapi-file-detector.yml b/changelogs/unreleased/fix-openapi-file-detector.yml new file mode 100644 index 00000000000..7793df62e77 --- /dev/null +++ b/changelogs/unreleased/fix-openapi-file-detector.yml @@ -0,0 +1,5 @@ +--- +title: Fix OpenAPI file detector +merge_request: 27321 +author: Roger Meier +type: fixed diff --git a/changelogs/unreleased/improve_webhooks_view_performance.yml b/changelogs/unreleased/improve_webhooks_view_performance.yml new file mode 100644 index 00000000000..8760d87ef54 --- /dev/null +++ b/changelogs/unreleased/improve_webhooks_view_performance.yml @@ -0,0 +1,5 @@ +--- +title: Reduce SQL queries when rendering webhook settings +merge_request: 27359 +author: +type: performance diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index 1c4647121fb..3c9030e7dbc 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -3,9 +3,9 @@ require 'yaml' -SEE_DOC = "See [the documentation](https://docs.gitlab.com/ce/development/changelog.html)." +SEE_DOC = "See [the documentation](https://docs.gitlab.com/ee/development/changelog.html)." CREATE_CHANGELOG_MESSAGE = <<~MSG -You can create one with: +If you want to create a changelog entry for GitLab FOSS, run the following: ``` bin/changelog -m %<mr_iid>s "%<mr_title>s" @@ -20,7 +20,7 @@ bin/changelog --ee -m %<mr_iid>s "%<mr_title>s" Note: Merge requests with %<labels>s do not trigger this check. MSG -def check_changelog(path) +def check_changelog_yaml(path) yaml = YAML.safe_load(File.read(path)) fail "`title` should be set, in #{gitlab.html_link(path)}! #{SEE_DOC}" if yaml["title"].nil? @@ -28,8 +28,6 @@ def check_changelog(path) if yaml["merge_request"].nil? && !helper.security_mr? message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{gitlab.html_link(path)}. #{SEE_DOC}" - elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !changelog.ce_port_changelog?(path) - fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}" end rescue Psych::SyntaxError, Psych::DisallowedClass, Psych::BadAlias # YAML could not be parsed, fail the build. @@ -38,6 +36,19 @@ rescue StandardError => e warn "There was a problem trying to check the Changelog. Exception: #{e.name} - #{e.message}" end +def check_changelog_path(path) + ee_changes = helper.all_ee_changes.dup + ee_changes.delete(path) + + if ee_changes.any? && !changelog.ee_changelog? + warn "This MR has a Changelog file outside `ee/`, but code changes in `ee/`. Consider moving the Changelog file into `ee/`." + end + + if ee_changes.empty? && changelog.ee_changelog? + warn "This MR has a Changelog file in `ee/`, but no code changes in `ee/`. Consider moving the Changelog file outside `ee/`." + end +end + def sanitized_mr_title helper.sanitize_mr_title(gitlab.mr_json["title"]) end @@ -49,11 +60,10 @@ end changelog_found = changelog.found -if changelog.needed? - if changelog_found - check_changelog(changelog_found) - else - message "**[CHANGELOG missing](https://docs.gitlab.com/ce/development/changelog.html)**: If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message.\n\n" + - format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: changelog.presented_no_changelog_labels) - end +if changelog_found + check_changelog_yaml(changelog_found) + check_changelog_path(changelog_found) +elsif changelog.needed? + message "**[CHANGELOG missing](https://docs.gitlab.com/ee/development/changelog.html)**: If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message.\n\n" + + format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: changelog.presented_no_changelog_labels) end diff --git a/lib/api/entities/commit_with_link.rb b/lib/api/entities/commit_with_link.rb new file mode 100644 index 00000000000..31a9efed9bc --- /dev/null +++ b/lib/api/entities/commit_with_link.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module API + module Entities + class CommitWithLink < Commit + include MarkupHelper + include RequestAwareEntity + + expose :author, using: Entities::UserPath + + expose :author_gravatar_url do |commit| + GravatarService.new.execute(commit.author_email) + end + + expose :commit_url do |commit, options| + project_commit_url(request.project, commit, params: options.fetch(:commit_url_params, {})) + end + + expose :commit_path do |commit, options| + project_commit_path(request.project, commit, params: options.fetch(:commit_url_params, {})) + end + + expose :description_html, if: { type: :full } do |commit| + markdown_field(commit, :description) + end + + expose :title_html, if: { type: :full } do |commit| + markdown_field(commit, :title) + end + + expose :signature_html, if: { type: :full } do |commit| + render('projects/commit/_signature', signature: commit.signature) if commit.has_signature? + end + + expose :pipeline_status_path, if: { type: :full } do |commit, options| + pipeline_ref = options[:pipeline_ref] + pipeline_project = options[:pipeline_project] || commit.project + next unless pipeline_ref && pipeline_project + + pipeline = commit.latest_pipeline_for_project(pipeline_ref, pipeline_project) + next unless pipeline&.status + + pipelines_project_commit_path(pipeline_project, commit.id, ref: pipeline_ref) + end + + def render(*args) + return unless request.respond_to?(:render) && request.render.respond_to?(:call) + + request.render.call(*args) + end + end + end +end diff --git a/lib/api/entities/user_path.rb b/lib/api/entities/user_path.rb new file mode 100644 index 00000000000..7d922b39654 --- /dev/null +++ b/lib/api/entities/user_path.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module API + module Entities + class UserPath < UserBasic + include RequestAwareEntity + include UserStatusTooltip + + expose :path do |user| + user_path(user) + end + end + end +end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 2b1bcc855d2..4d9f035e2cd 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -306,7 +306,7 @@ module API context_commits = paginate(merge_request.merge_request_context_commits).map(&:to_commit) - present context_commits, with: Entities::Commit + present context_commits, with: Entities::CommitWithLink, type: :full, request: merge_request end params do diff --git a/lib/gitlab/danger/changelog.rb b/lib/gitlab/danger/changelog.rb index b53516081be..d64177f9565 100644 --- a/lib/gitlab/danger/changelog.rb +++ b/lib/gitlab/danger/changelog.rb @@ -11,7 +11,7 @@ module Gitlab end def found - git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} } + @found ||= git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} } end def presented_no_changelog_labels @@ -22,12 +22,8 @@ module Gitlab gitlab.mr_json["title"].gsub(/^WIP: */, '').gsub(/`/, '\\\`') end - def ee_changelog?(changelog_path) - changelog_path =~ /unreleased-ee/ - end - - def ce_port_changelog?(changelog_path) - helper.ee? && !ee_changelog?(changelog_path) + def ee_changelog? + found.start_with?('ee/') end private diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index 77a84634815..6bb46b1730f 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -34,6 +34,10 @@ module Gitlab .sort end + def all_ee_changes + all_changed_files.grep(%r{\Aee/}) + end + def ee? # Support former project name for `dev` and support local Danger run %w[gitlab gitlab-ee].include?(ENV['CI_PROJECT_NAME']) || Dir.exist?('../../ee') diff --git a/lib/gitlab/file_detector.rb b/lib/gitlab/file_detector.rb index 305fbeecce1..a9e9261cd3c 100644 --- a/lib/gitlab/file_detector.rb +++ b/lib/gitlab/file_detector.rb @@ -40,7 +40,7 @@ module Gitlab yarn_lock: 'yarn.lock', # OpenAPI Specification files - openapi: %r{.*(openapi|swagger).*\.(yaml|yml|json)\z}i + openapi: %r{[^/]*(openapi|swagger)[^/]*\.(yaml|yml|json)\z}i }.freeze # Returns an Array of file types based on the given paths. diff --git a/lib/gitlab/metrics/dashboard/finder.rb b/lib/gitlab/metrics/dashboard/finder.rb index 990fd57bf41..d80985e0a0e 100644 --- a/lib/gitlab/metrics/dashboard/finder.rb +++ b/lib/gitlab/metrics/dashboard/finder.rb @@ -78,7 +78,7 @@ module Gitlab end def project_service - ::Metrics::Dashboard::ProjectDashboardService + ::Metrics::Dashboard::CustomDashboardService end def self_monitoring_service diff --git a/lib/gitlab/metrics/dashboard/service_selector.rb b/lib/gitlab/metrics/dashboard/service_selector.rb index 993e508cbc6..281538de502 100644 --- a/lib/gitlab/metrics/dashboard/service_selector.rb +++ b/lib/gitlab/metrics/dashboard/service_selector.rb @@ -20,7 +20,7 @@ module Gitlab ::Metrics::Dashboard::SystemDashboardService, ::Metrics::Dashboard::PodDashboardService, ::Metrics::Dashboard::SelfMonitoringDashboardService, - ::Metrics::Dashboard::ProjectDashboardService + ::Metrics::Dashboard::CustomDashboardService ].freeze # Returns a class which inherits from the BaseService diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs.rb index 23222430902..f0e26f99c2c 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs.rb @@ -5,8 +5,18 @@ require 'digest' module Gitlab module SidekiqMiddleware module DuplicateJobs - def self.drop_duplicates? - Feature.enabled?(:drop_duplicate_sidekiq_jobs) + DROPPABLE_QUEUES = Set.new([ + Namespaces::RootStatisticsWorker.queue + ]).freeze + + def self.drop_duplicates?(queue_name) + Feature.enabled?(:drop_duplicate_sidekiq_jobs) || + drop_duplicates_for_queue?(queue_name) + end + + private_class_method def self.drop_duplicates_for_queue?(queue_name) + DROPPABLE_QUEUES.include?(queue_name) && + Feature.enabled?(:drop_duplicate_sidekiq_jobs_for_queue) end end end diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb index c6fb50b4610..a9007039334 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb @@ -67,7 +67,7 @@ module Gitlab end def droppable? - idempotent? && duplicate? && DuplicateJobs.drop_duplicates? + idempotent? && duplicate? && DuplicateJobs.drop_duplicates?(queue_name) end private diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f22e415b51f..8e96296730e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9166,18 +9166,6 @@ msgstr "" msgid "GeoNodes|Last event ID seen from primary" msgstr "" -msgid "GeoNodes|Learn more about Repository checksum progress" -msgstr "" - -msgid "GeoNodes|Learn more about Repository verification" -msgstr "" - -msgid "GeoNodes|Learn more about Wiki checksum progress" -msgstr "" - -msgid "GeoNodes|Learn more about Wiki verification" -msgstr "" - msgid "GeoNodes|Loading nodes" msgstr "" @@ -9208,6 +9196,9 @@ msgstr "" msgid "GeoNodes|Removing a Geo secondary node stops the synchronization to that node. Are you sure?" msgstr "" +msgid "GeoNodes|Replicated data is verified with the %{nodeText} using checksums" +msgstr "" + msgid "GeoNodes|Replication slot WAL" msgstr "" @@ -9217,12 +9208,6 @@ msgstr "" msgid "GeoNodes|Repositories" msgstr "" -msgid "GeoNodes|Repositories checksummed for verification with their counterparts on Secondary nodes" -msgstr "" - -msgid "GeoNodes|Repositories verified with their counterparts on the Primary node" -msgstr "" - msgid "GeoNodes|Repository checksum progress" msgstr "" @@ -9274,16 +9259,16 @@ msgstr "" msgid "GeoNodes|Wikis" msgstr "" -msgid "GeoNodes|Wikis checksummed for verification with their counterparts on Secondary nodes" +msgid "GeoNodes|With %{geo} you can install a special read-only and replicated instance anywhere. Before you add nodes, follow the %{instructions} in the exact order they appear." msgstr "" -msgid "GeoNodes|Wikis verified with their counterparts on the Primary node" +msgid "GeoNodes|You have configured Geo nodes using an insecure HTTP connection. We recommend the use of HTTPS." msgstr "" -msgid "GeoNodes|With %{geo} you can install a special read-only and replicated instance anywhere. Before you add nodes, follow the %{instructions} in the exact order they appear." +msgid "GeoNodes|primary node" msgstr "" -msgid "GeoNodes|You have configured Geo nodes using an insecure HTTP connection. We recommend the use of HTTPS." +msgid "GeoNodes|secondary nodes" msgstr "" msgid "Geo|%{name} is scheduled for forced re-download" @@ -22762,9 +22747,6 @@ msgstr "" msgid "Wiki|Edit Page" msgstr "" -msgid "Wiki|More Pages" -msgstr "" - msgid "Wiki|New page" msgstr "" @@ -22783,6 +22765,9 @@ msgstr "" msgid "Wiki|Title" msgstr "" +msgid "Wiki|View All Pages" +msgstr "" + msgid "Wiki|Wiki Pages" msgstr "" diff --git a/qa/qa/tools/generate_perf_testdata.rb b/qa/qa/tools/generate_perf_testdata.rb index 26bcb2fe958..e77678a1527 100644 --- a/qa/qa/tools/generate_perf_testdata.rb +++ b/qa/qa/tools/generate_perf_testdata.rb @@ -33,9 +33,9 @@ module QA create_many_branches create_many_new_files create_mr_with_many_commits + create_many_issues methods_arr = [ - method(:create_many_issues), method(:create_many_labels), method(:create_many_todos), method(:create_many_merge_requests), @@ -104,6 +104,7 @@ module QA def create_many_new_files create_a_new_file_api_req("hello.txt", "master", "#{@group_name}%2F#{@project_name}", "hello", "my new content") 30.times do |i| + create_a_new_file_api_req("hello#{i}.txt", "master", "#{@group_name}%2F#{@project_name}", "hello", "my new content") create_a_new_file_api_req("hello#{i}.txt", "branch#{i}", "#{@group_name}%2F#{@project_name}", "hello", "my new content") end @@ -137,7 +138,7 @@ module QA 16.times do |i| faker_line_arr = Faker::Lorem.sentences(1500) content = faker_line_arr.join("\n\r") - create_a_new_file_api_req("hello#{i}.txt", "master", "#{@group_name}%2F#{@project_name}", "Add hello#{i}.txt", content) + create_a_new_file_api_req("hello#{i + 100}.txt", "master", "#{@group_name}%2F#{@project_name}", "Add hello#{i + 100}.txt", content) content_arr[i] = faker_line_arr end @@ -147,7 +148,7 @@ module QA missed_line_array = content_arr[i].each_slice(2).map(&:first) content = missed_line_array.join("\n\rIm new!:D \n\r ") - update_file_api_req("hello#{i}.txt", "performance", "#{@group_name}%2F#{@project_name}", "Update hello#{i}.txt", content) + update_file_api_req("hello#{i + 100}.txt", "performance", "#{@group_name}%2F#{@project_name}", "Update hello#{i + 100}.txt", content) end create_mr_response = create_a_merge_request_api_req("#{@group_name}%2F#{@project_name}", "performance", "master", "Large_MR") @@ -182,7 +183,7 @@ module QA def create_diff_note(iid, file_count, line_count, head_sha, start_sha, base_sha, line_type) post Runtime::API::Request.new(@api_client, "/projects/#{@group_name}%2F#{@project_name}/merge_requests/#{iid}/discussions").url, - "" "body=\"Let us discuss\"& + "" "body=\"Let us discuss\"& position[position_type]=text& position[new_path]=hello#{file_count}.txt& position[old_path]=hello#{file_count}.txt& diff --git a/rubocop/cop/gitlab/duplicate_spec_location.rb b/rubocop/cop/gitlab/duplicate_spec_location.rb new file mode 100644 index 00000000000..025f75d644f --- /dev/null +++ b/rubocop/cop/gitlab/duplicate_spec_location.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'rubocop/rspec/top_level_describe' + +module RuboCop + module Cop + module Gitlab + # Cop that detects duplicate EE spec files + # + # There should not be files in both ee/spec/*/ee/my_spec.rb and ee/spec/*/my_spec.rb + # + # # bad + # ee/spec/controllers/my_spec.rb # describe MyClass + # ee/spec/controllers/ee/my_spec.rb # describe MyClass + # + # # good, spec for EE extension code + # ee/spec/controllers/ee/my_spec.rb # describe MyClass + # + # # good, spec for EE only code + # ee/spec/controllers/my_spec.rb # describe MyClass + # + class DuplicateSpecLocation < RuboCop::Cop::Cop + include RuboCop::RSpec::TopLevelDescribe + + MSG = 'Duplicate spec location in `%<path>s`.' + + def on_top_level_describe(node, _args) + path = file_path_for_node(node).sub(/\A#{rails_root}\//, '') + duplicate_path = find_duplicate_path(path) + + if duplicate_path && File.exist?(File.join(rails_root, duplicate_path)) + add_offense(node, message: format(MSG, path: duplicate_path)) + end + end + + private + + def ee_spec?(path) + File.fnmatch?('ee/spec/**/*.rb', path, File::FNM_PATHNAME) + end + + def find_duplicate_path(path) + return unless ee_spec?(path) + + if File.fnmatch?('ee/spec/**/ee/**', path) + path.match('\A(ee/spec/[^/]+)/ee/(.+)') do |match| + File.join(match[1], match[2]) + end + else + path.match('\A(ee/spec/[^/]+)/(.+)') do |match| + File.join(match[1], 'ee', match[2]) + end + end + end + + def file_path_for_node(node) + node.location.expression.source_buffer.name + end + + def rails_root + File.expand_path('../../..', __dir__) + end + end + end + end +end diff --git a/scripts/trigger-build-docs b/scripts/trigger-build-docs index ea717d9ccbf..70e7150d475 100755 --- a/scripts/trigger-build-docs +++ b/scripts/trigger-build-docs @@ -21,7 +21,13 @@ GITLAB_DOCS_REPO = 'gitlab-org/gitlab-docs'.freeze # kicked the review app. # def docs_branch - "docs-preview-#{slug}-#{ENV["CI_MERGE_REQUEST_IID"]}" + # Check if CI_MERGE_REQUEST_IID is present. This requires pipelines + # for merge requests to be enabled. + if ENV["CI_MERGE_REQUEST_IID"].nil? + "docs-preview-#{slug}-#{ENV["CI_COMMIT_REF_SLUG"]}" + else + "docs-preview-#{slug}-#{ENV["CI_MERGE_REQUEST_IID"]}" + end end # diff --git a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb index e67982bbd31..67996cc3e5d 100644 --- a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb @@ -312,7 +312,6 @@ describe "User creates wiki page" do visit(project_wikis_path(project)) expect(page).to have_content('another') - expect(page).to have_content('More Pages') end context 'when there is a customized sidebar' do @@ -324,10 +323,23 @@ describe "User creates wiki page" do visit(project_wikis_path(project)) expect(page).to have_content('My customized sidebar') - expect(page).to have_content('More Pages') expect(page).not_to have_content('Another') end end end + + context 'when there are more than 15 existing pages' do + before do + create(:wiki_page, wiki: wiki, attrs: { title: 'home', content: 'home' }) + (1..14).each { |i| create(:wiki_page, wiki: wiki, attrs: { title: "page-#{i}", content: "page #{i}" }) } + end + + it 'renders a default sidebar when there is no customized sidebar' do + visit(project_wikis_path(project)) + + expect(page).to have_content('View All Pages') + expect(page).to have_content('page 1') + end + end end end diff --git a/spec/frontend/helpers/dom_shims/element_scroll_by.js b/spec/frontend/helpers/dom_shims/element_scroll_by.js new file mode 100644 index 00000000000..7d91279e4aa --- /dev/null +++ b/spec/frontend/helpers/dom_shims/element_scroll_by.js @@ -0,0 +1 @@ +Element.prototype.scrollBy = jest.fn(); diff --git a/spec/frontend/helpers/dom_shims/index.js b/spec/frontend/helpers/dom_shims/index.js index bcd5da0ce48..17a2090d2f1 100644 --- a/spec/frontend/helpers/dom_shims/index.js +++ b/spec/frontend/helpers/dom_shims/index.js @@ -1,4 +1,5 @@ import './element_scroll_into_view'; +import './element_scroll_by'; import './form_element'; import './get_client_rects'; import './inner_text'; diff --git a/spec/frontend/repository/components/table/__snapshots__/row_spec.js.snap b/spec/frontend/repository/components/table/__snapshots__/row_spec.js.snap index 2d411fcce79..5b5c9fd714e 100644 --- a/spec/frontend/repository/components/table/__snapshots__/row_spec.js.snap +++ b/spec/frontend/repository/components/table/__snapshots__/row_spec.js.snap @@ -2,25 +2,28 @@ exports[`Repository table row component renders table row 1`] = ` <tr - class="tree-item file_1" + class="tree-item" > <td - class="tree-item-file-name" + class="tree-item-file-name cursor-default position-relative" > - <i - aria-label="file" - class="fa fa-fw fa-file-text-o" - role="img" - /> + <!----> <a - class="str-truncated" + class="tree-item-link str-truncated" data-qa-selector="file_name_link" href="https://test.com" > - - test - + <i + aria-label="file" + class="fa fa-fw mr-1 fa-file-text-o" + role="img" + /> + <span + class="position-relative" + > + test + </span> </a> <!----> @@ -31,7 +34,7 @@ exports[`Repository table row component renders table row 1`] = ` </td> <td - class="d-none d-sm-table-cell tree-commit" + class="d-none d-sm-table-cell tree-commit cursor-default" > <gl-skeleton-loading-stub class="h-auto" @@ -40,7 +43,7 @@ exports[`Repository table row component renders table row 1`] = ` </td> <td - class="tree-time-ago text-right" + class="tree-time-ago text-right cursor-default" > <gl-skeleton-loading-stub class="ml-auto h-auto w-50" @@ -52,25 +55,28 @@ exports[`Repository table row component renders table row 1`] = ` exports[`Repository table row component renders table row for path with special character 1`] = ` <tr - class="tree-item file_1" + class="tree-item" > <td - class="tree-item-file-name" + class="tree-item-file-name cursor-default position-relative" > - <i - aria-label="file" - class="fa fa-fw fa-file-text-o" - role="img" - /> + <!----> <a - class="str-truncated" + class="tree-item-link str-truncated" data-qa-selector="file_name_link" href="https://test.com" > - - test - + <i + aria-label="file" + class="fa fa-fw mr-1 fa-file-text-o" + role="img" + /> + <span + class="position-relative" + > + test + </span> </a> <!----> @@ -81,7 +87,7 @@ exports[`Repository table row component renders table row for path with special </td> <td - class="d-none d-sm-table-cell tree-commit" + class="d-none d-sm-table-cell tree-commit cursor-default" > <gl-skeleton-loading-stub class="h-auto" @@ -90,7 +96,7 @@ exports[`Repository table row component renders table row for path with special </td> <td - class="tree-time-ago text-right" + class="tree-time-ago text-right cursor-default" > <gl-skeleton-loading-stub class="ml-auto h-auto w-50" diff --git a/spec/frontend/repository/components/table/row_spec.js b/spec/frontend/repository/components/table/row_spec.js index 22aacdc735c..7bb7ad6e5dd 100644 --- a/spec/frontend/repository/components/table/row_spec.js +++ b/spec/frontend/repository/components/table/row_spec.js @@ -1,14 +1,8 @@ import { shallowMount, RouterLinkStub } from '@vue/test-utils'; import { GlBadge, GlLink, GlLoadingIcon } from '@gitlab/ui'; -import { visitUrl } from '~/lib/utils/url_utility'; import TableRow from '~/repository/components/table/row.vue'; import Icon from '~/vue_shared/components/icon.vue'; -jest.mock('~/lib/utils/url_utility', () => ({ - ...jest.requireActual('~/lib/utils/url_utility'), - visitUrl: jest.fn(), -})); - let vm; let $router; @@ -88,31 +82,6 @@ describe('Repository table row component', () => { }); it.each` - type | pushes - ${'tree'} | ${true} - ${'file'} | ${false} - ${'commit'} | ${false} - `('pushes new router if type $type is tree', ({ type, pushes }) => { - factory({ - id: '1', - sha: '123', - path: 'test', - type, - currentPath: '/', - }); - - return vm.vm.$nextTick().then(() => { - vm.trigger('click'); - - if (pushes) { - expect($router.push).toHaveBeenCalledWith({ path: '/-/tree/master/test' }); - } else { - expect($router.push).not.toHaveBeenCalled(); - } - }); - }); - - it.each` path ${'test#'} ${'Änderungen'} @@ -132,7 +101,7 @@ describe('Repository table row component', () => { }); }); - it('pushes new route for directory with hash', () => { + it('renders link for directory with hash', () => { factory({ id: '1', sha: '123', @@ -142,36 +111,7 @@ describe('Repository table row component', () => { }); return vm.vm.$nextTick().then(() => { - vm.trigger('click'); - - expect($router.push).toHaveBeenCalledWith({ path: '/-/tree/master/test%23' }); - }); - }); - - it.each` - type | pushes - ${'tree'} | ${true} - ${'file'} | ${false} - ${'commit'} | ${false} - `('calls visitUrl if $type is not tree', ({ type, pushes }) => { - factory({ - id: '1', - sha: '123', - path: 'test', - type, - currentPath: '/', - }); - - return vm.vm.$nextTick().then(() => { - vm.trigger('click'); - - if (pushes) { - expect(visitUrl).not.toHaveBeenCalled(); - } else { - const [url, external] = visitUrl.mock.calls[0]; - expect(url).toBe('https://test.com'); - expect(external).toBeFalsy(); - } + expect(vm.find('.tree-item-link').props('to')).toEqual({ path: '/-/tree/master/test%23' }); }); }); diff --git a/spec/graphql/resolvers/error_tracking/sentry_detailed_error_resolver_spec.rb b/spec/graphql/resolvers/error_tracking/sentry_detailed_error_resolver_spec.rb index 4490b54d1f7..8b2e33cdfda 100644 --- a/spec/graphql/resolvers/error_tracking/sentry_detailed_error_resolver_spec.rb +++ b/spec/graphql/resolvers/error_tracking/sentry_detailed_error_resolver_spec.rb @@ -18,6 +18,16 @@ describe Resolvers::ErrorTracking::SentryDetailedErrorResolver do .and_return issue_details_service end + shared_examples 'it resolves to nil' do + it 'resolves to nil' do + allow(issue_details_service).to receive(:execute) + .and_return(issue: nil) + + result = resolve_error(args) + expect(result).to be_nil + end + end + describe '#resolve' do let(:args) { { id: issue_global_id(1234) } } @@ -32,7 +42,7 @@ describe Resolvers::ErrorTracking::SentryDetailedErrorResolver do before do allow(issue_details_service).to receive(:execute) - .and_return({ issue: detailed_error }) + .and_return(issue: detailed_error) end it 'resolves to a detailed error' do @@ -44,12 +54,14 @@ describe Resolvers::ErrorTracking::SentryDetailedErrorResolver do end end - it 'resolves to nil if no match' do - allow(issue_details_service).to receive(:execute) - .and_return({ issue: nil }) + context 'if id does not match issue' do + it_behaves_like 'it resolves to nil' + end - result = resolve_error(args) - expect(result).to eq nil + context 'blank id' do + let(:args) { { id: '' } } + + it_behaves_like 'it resolves to nil' end end diff --git a/spec/lib/gitlab/danger/changelog_spec.rb b/spec/lib/gitlab/danger/changelog_spec.rb index 64f87ec8cd3..c61e47f80d9 100644 --- a/spec/lib/gitlab/danger/changelog_spec.rb +++ b/spec/lib/gitlab/danger/changelog_spec.rb @@ -76,10 +76,10 @@ describe Gitlab::Danger::Changelog do context 'added files contain a changelog' do [ - 'changelogs/unreleased/entry.md', - 'ee/changelogs/unreleased/entry.md', - 'changelogs/unreleased-ee/entry.md', - 'ee/changelogs/unreleased-ee/entry.md' + 'changelogs/unreleased/entry.yml', + 'ee/changelogs/unreleased/entry.yml', + 'changelogs/unreleased-ee/entry.yml', + 'ee/changelogs/unreleased-ee/entry.yml' ].each do |file_path| let(:added_files) { [file_path] } @@ -107,46 +107,22 @@ describe Gitlab::Danger::Changelog do end describe '#ee_changelog?' do - context 'is ee changelog' do - [ - 'changelogs/unreleased-ee/entry.md', - 'ee/changelogs/unreleased-ee/entry.md' - ].each do |file_path| - subject { changelog.ee_changelog?(file_path) } + subject { changelog.ee_changelog? } - it { is_expected.to be_truthy } - end + before do + allow(changelog).to receive(:found).and_return(file_path) end - context 'is not ee changelog' do - [ - 'changelogs/unreleased/entry.md', - 'ee/changelogs/unreleased/entry.md' - ].each do |file_path| - subject { changelog.ee_changelog?(file_path) } - - it { is_expected.to be_falsy } - end - end - end + context 'is ee changelog' do + let(:file_path) { 'ee/changelogs/unreleased/entry.yml' } - describe '#ce_port_changelog?' do - where(:helper_ee?, :file_path, :expected) do - true | 'changelogs/unreleased-ee/entry.md' | false - true | 'ee/changelogs/unreleased-ee/entry.md' | false - false | 'changelogs/unreleased-ee/entry.md' | false - false | 'ee/changelogs/unreleased-ee/entry.md' | false - true | 'changelogs/unreleased/entry.md' | true - true | 'ee/changelogs/unreleased/entry.md' | true - false | 'changelogs/unreleased/entry.md' | false - false | 'ee/changelogs/unreleased/entry.md' | false + it { is_expected.to be_truthy } end - with_them do - let(:ee?) { helper_ee? } - subject { changelog.ce_port_changelog?(file_path) } + context 'is not ee changelog' do + let(:file_path) { 'changelogs/unreleased/entry.yml' } - it { is_expected.to eq(expected) } + it { is_expected.to be_falsy } end end end diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index 4cb957f378c..d5d582d7d6c 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -76,6 +76,16 @@ describe Gitlab::Danger::Helper do end end + describe '#all_ee_changes' do + subject { helper.all_ee_changes } + + it 'returns all changed files starting with ee/' do + expect(helper).to receive(:all_changed_files).and_return(%w[fr/ee/beer.rb ee/wine.rb ee/lib/ido.rb ee.k]) + + is_expected.to match_array(%w[ee/wine.rb ee/lib/ido.rb]) + end + end + describe '#ee?' do subject { helper.ee? } diff --git a/spec/lib/gitlab/file_detector_spec.rb b/spec/lib/gitlab/file_detector_spec.rb index 3972bd24e80..5bf70ef898a 100644 --- a/spec/lib/gitlab/file_detector_spec.rb +++ b/spec/lib/gitlab/file_detector_spec.rb @@ -96,14 +96,25 @@ describe Gitlab::FileDetector do 'swagger.yml', 'swagger.yaml', 'swagger.json', 'gitlab_swagger.yml', 'openapi_gitlab.yml', 'OpenAPI.YML', 'openapi.Yaml', 'openapi.JSON', - 'openapi.gitlab.yml', 'gitlab.openapi.yml' + 'openapi.gitlab.yml', 'gitlab.openapi.yml', + 'attention/openapi.yml', 'attention/swagger.yml', + 'attention/gitlab_swagger.yml', 'attention/openapi_gitlab.yml', + 'openapi/openapi.yml', 'openapi/swagger.yml', + 'openapi/my_openapi.yml', 'openapi/swagger_one.yml' ] openapi_types.each do |type_name| expect(described_class.type_of(type_name)).to eq(:openapi) end - expect(described_class.type_of('openapiyml')).to be_nil + openapi_bad_types = [ + 'openapiyml', + 'openapi/attention.yaml', 'swagger/attention.yaml' + ] + + openapi_bad_types.each do |type_name| + expect(described_class.type_of(type_name)).to be_nil + end end end end diff --git a/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb index c0d71bfe5d0..387baf1ee53 100644 --- a/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::Metrics::Dashboard::ServiceSelector do context 'when just the dashboard path is provided' do let(:arguments) { { dashboard_path: '.gitlab/dashboards/test.yml' } } - it { is_expected.to be Metrics::Dashboard::ProjectDashboardService } + it { is_expected.to be Metrics::Dashboard::CustomDashboardService } context 'when the path is for the system dashboard' do let(:arguments) { { dashboard_path: system_dashboard_path } } diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb index 058e0737a25..e11613b202d 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb @@ -129,7 +129,8 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gitlab_r before do allow(AuthorizedProjectsWorker).to receive(:idempotent?).and_return(idempotent) allow(duplicate_job).to receive(:duplicate?).and_return(duplicate) - stub_feature_flags(drop_duplicate_sidekiq_jobs: feature_enabled) + allow(Gitlab::SidekiqMiddleware::DuplicateJobs) + .to receive(:drop_duplicates?).with(queue).and_return(feature_enabled) end it 'is droppable when all conditions are met' do diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs_spec.rb new file mode 100644 index 00000000000..fa5938f470b --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::SidekiqMiddleware::DuplicateJobs do + using RSpec::Parameterized::TableSyntax + + describe '.drop_duplicates?' do + where(:global_feature_enabled, :selected_queue_enabled, :queue, :expected) do + true | true | described_class::DROPPABLE_QUEUES.first | true + true | true | "other_queue" | true + true | false | described_class::DROPPABLE_QUEUES.first | true + true | false | "other_queue" | true + false | true | described_class::DROPPABLE_QUEUES.first | true + false | true | "other_queue" | false + false | false | described_class::DROPPABLE_QUEUES.first | false + false | false | "other_queue" | false + end + + with_them do + before do + stub_feature_flags(drop_duplicate_sidekiq_jobs: global_feature_enabled, + drop_duplicate_sidekiq_jobs_for_queue: selected_queue_enabled) + end + + it "allows dropping jobs when expected" do + expect(described_class.drop_duplicates?(queue)).to be(expected) + end + end + end +end diff --git a/spec/rubocop/cop/gitlab/duplicate_spec_location_spec.rb b/spec/rubocop/cop/gitlab/duplicate_spec_location_spec.rb new file mode 100644 index 00000000000..87dd2f14b31 --- /dev/null +++ b/spec/rubocop/cop/gitlab/duplicate_spec_location_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/gitlab/duplicate_spec_location' + +describe RuboCop::Cop::Gitlab::DuplicateSpecLocation do + include RuboCop::RSpec::ExpectOffense + include CopHelper + + subject(:cop) { described_class.new } + + let(:rails_root) { '../../../../' } + + def full_path(path) + File.expand_path(File.join(rails_root, path), __dir__) + end + + context 'Non-EE spec file' do + it 'registers no offenses' do + expect_no_offenses(<<~SOURCE.strip_indent, full_path('spec/foo_spec.rb')) + describe 'Foo' do + end + SOURCE + end + end + + context 'Non-EE application file' do + it 'registers no offenses' do + expect_no_offenses(<<~SOURCE.strip_indent, full_path('app/models/blog_post.rb')) + class BlogPost + end + SOURCE + end + end + + context 'EE application file' do + it 'registers no offenses' do + expect_no_offenses(<<~SOURCE.strip_indent, full_path('ee/app/models/blog_post.rb')) + class BlogPost + end + SOURCE + end + end + + context 'EE spec file for EE only code' do + let(:spec_file_path) { full_path('ee/spec/controllers/foo_spec.rb') } + + it 'registers no offenses' do + expect_no_offenses(<<~SOURCE.strip_indent, spec_file_path) + describe 'Foo' do + end + SOURCE + end + + context 'there is a duplicate file' do + before do + allow(File).to receive(:exist?).and_call_original + + allow(File).to receive(:exist?) + .with(full_path('ee/spec/controllers/ee/foo_spec.rb')) + .and_return(true) + end + + it 'marks the describe as offending' do + expect_offense(<<~SOURCE.strip_indent, spec_file_path) + describe 'Foo' do + ^^^^^^^^^^^^^^ Duplicate spec location in `ee/spec/controllers/ee/foo_spec.rb`. + end + SOURCE + end + end + end + + context 'EE spec file for EE extension' do + let(:spec_file_path) { full_path('ee/spec/controllers/ee/foo_spec.rb') } + + it 'registers no offenses' do + expect_no_offenses(<<~SOURCE.strip_indent, spec_file_path) + describe 'Foo' do + end + SOURCE + end + + context 'there is a duplicate file' do + before do + allow(File).to receive(:exist?).and_call_original + + allow(File).to receive(:exist?) + .with(full_path('ee/spec/controllers/foo_spec.rb')) + .and_return(true) + end + + it 'marks the describe as offending' do + expect_offense(<<~SOURCE.strip_indent, spec_file_path) + describe 'Foo' do + ^^^^^^^^^^^^^^ Duplicate spec location in `ee/spec/controllers/foo_spec.rb`. + end + SOURCE + end + end + end +end diff --git a/spec/services/issuable/clone/attributes_rewriter_spec.rb b/spec/services/issuable/clone/attributes_rewriter_spec.rb index 6bc0df8260b..9111b19d7b7 100644 --- a/spec/services/issuable/clone/attributes_rewriter_spec.rb +++ b/spec/services/issuable/clone/attributes_rewriter_spec.rb @@ -112,9 +112,6 @@ describe Issuable::Clone::AttributesRewriter do expect(event.milestone_id).to eq(expected_attrs[:milestone].id) expect(event.action).to eq(expected_attrs[:action]) expect(event.state).to eq(expected_attrs[:state]) - - expect(event.reference).to be_nil - expect(event.reference_html).to be_nil end end end diff --git a/spec/services/metrics/dashboard/project_dashboard_service_spec.rb b/spec/services/metrics/dashboard/custom_dashboard_service_spec.rb index 829e750d438..9458df3dca0 100644 --- a/spec/services/metrics/dashboard/project_dashboard_service_spec.rb +++ b/spec/services/metrics/dashboard/custom_dashboard_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Metrics::Dashboard::ProjectDashboardService, :use_clean_rails_memory_store_caching do +describe Metrics::Dashboard::CustomDashboardService, :use_clean_rails_memory_store_caching do include MetricsDashboardHelpers let_it_be(:user) { create(:user) } diff --git a/spec/workers/namespaces/root_statistics_worker_spec.rb b/spec/workers/namespaces/root_statistics_worker_spec.rb index 45e75c9b0da..910a5b23e17 100644 --- a/spec/workers/namespaces/root_statistics_worker_spec.rb +++ b/spec/workers/namespaces/root_statistics_worker_spec.rb @@ -74,4 +74,19 @@ describe Namespaces::RootStatisticsWorker, '#perform' do worker.perform(group.id) end end + + it_behaves_like 'an idempotent worker' do + let(:job_args) { [group.id] } + + it 'deletes one aggregation schedule' do + # Make sure the group and it's aggregation schedule are created before + # counting + group + + expect { worker.perform(*job_args) } + .to change { Namespace::AggregationSchedule.count }.by(-1) + expect { worker.perform(*job_args) } + .not_to change { Namespace::AggregationSchedule.count } + end + end end |