diff options
115 files changed, 1319 insertions, 555 deletions
diff --git a/.rubocop_todo/cop/redirect_with_status.yml b/.rubocop_todo/cop/redirect_with_status.yml deleted file mode 100644 index 3b4d9d2681e..00000000000 --- a/.rubocop_todo/cop/redirect_with_status.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -Cop/RedirectWithStatus: - Details: grace period - Exclude: - - 'app/controllers/concerns/issuable_actions.rb' - - 'app/controllers/concerns/membership_actions.rb' diff --git a/.rubocop_todo/style/class_and_module_children.yml b/.rubocop_todo/style/class_and_module_children.yml index 2c5aeba13b3..4257adbdc6f 100644 --- a/.rubocop_todo/style/class_and_module_children.yml +++ b/.rubocop_todo/style/class_and_module_children.yml @@ -98,7 +98,6 @@ Style/ClassAndModuleChildren: - 'app/controllers/jira_connect/installations_controller.rb' - 'app/controllers/jira_connect/oauth_callbacks_controller.rb' - 'app/controllers/jira_connect/subscriptions_controller.rb' - - 'app/controllers/jira_connect/users_controller.rb' - 'app/controllers/ldap/omniauth_callbacks_controller.rb' - 'app/controllers/oauth/applications_controller.rb' - 'app/controllers/oauth/authorizations_controller.rb' diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 6c0cb4b5b85..3cc5d640565 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -189338d40ba6351529c7bd7d39a9018e66f7b64e +91b69d050acf344c09a9238f24a75c4938001113 diff --git a/app/assets/javascripts/behaviors/shortcuts/shortcuts.js b/app/assets/javascripts/behaviors/shortcuts/shortcuts.js index efb462f4778..9514ad853b0 100644 --- a/app/assets/javascripts/behaviors/shortcuts/shortcuts.js +++ b/app/assets/javascripts/behaviors/shortcuts/shortcuts.js @@ -186,14 +186,6 @@ export default class Shortcuts { } static toggleMarkdownPreview(e) { - const $form = $(e.target).closest('form'); - const toggle = $('.js-md-preview-button', $form).get(0); - - if (!toggle) return; - - toggle.focus(); - toggle.click(); - $(document).triggerHandler('markdown-preview:toggle', [e]); } diff --git a/app/assets/javascripts/content_editor/components/formatting_toolbar.vue b/app/assets/javascripts/content_editor/components/formatting_toolbar.vue index e7e520a55da..1ef38df0f78 100644 --- a/app/assets/javascripts/content_editor/components/formatting_toolbar.vue +++ b/app/assets/javascripts/content_editor/components/formatting_toolbar.vue @@ -32,7 +32,7 @@ export default { class="gl-w-full gl-display-flex gl-align-items-center gl-flex-wrap gl-bg-gray-50 gl-px-2 gl-rounded-base gl-justify-content-space-between" data-testid="formatting-toolbar" > - <div class="gl-py-2 gl-display-flex gl-flex-wrap-wrap"> + <div class="gl-py-2 gl-display-flex gl-flex-wrap"> <toolbar-text-style-dropdown data-testid="text-styles" @execute="trackToolbarControlExecution" diff --git a/app/assets/javascripts/lib/utils/vue3compat/vue_router.js b/app/assets/javascripts/lib/utils/vue3compat/vue_router.js index 1a2f958fb78..aa2963ece31 100644 --- a/app/assets/javascripts/lib/utils/vue3compat/vue_router.js +++ b/app/assets/javascripts/lib/utils/vue3compat/vue_router.js @@ -84,7 +84,17 @@ const installed = new WeakMap(); export default class VueRouterCompat { constructor(options) { // eslint-disable-next-line no-constructor-return - return createRouter(transformOptions(options)); + return new Proxy(createRouter(transformOptions(options)), { + get(target, prop) { + const result = target[prop]; + // eslint-disable-next-line no-underscore-dangle + if (result?.__v_isRef) { + return result.value; + } + + return result; + }, + }); } static install() { diff --git a/app/assets/javascripts/ml/experiment_tracking/components/model_experiments_header.vue b/app/assets/javascripts/ml/experiment_tracking/components/model_experiments_header.vue new file mode 100644 index 00000000000..02869bacb66 --- /dev/null +++ b/app/assets/javascripts/ml/experiment_tracking/components/model_experiments_header.vue @@ -0,0 +1,35 @@ +<script> +import { GlBadge } from '@gitlab/ui'; +import { __ } from '~/locale'; +import { helpPagePath } from '~/helpers/help_page_helper'; + +export default { + components: { + GlBadge, + }, + props: { + pageTitle: { + type: String, + required: true, + }, + }, + i18n: { + experimentBadgeLabel: __('Experiment'), + }, + experimentDocHref: helpPagePath('user/project/ml/experiment_tracking/index.md'), +}; +</script> + +<template> + <div class="detail-page-header gl-flex-wrap"> + <div class="detail-page-header-body"> + <div class="page-title gl-flex-grow-1 gl-display-flex gl-align-items-center"> + <h3 class="gl-font-size-h-display gl-my-0">{{ pageTitle }}</h3> + <gl-badge class="gl-mx-4" variant="info" size="lg" :href="$options.experimentDocHref"> + {{ $options.i18n.experimentBadgeLabel }} + </gl-badge> + </div> + <slot></slot> + </div> + </div> +</template> diff --git a/app/assets/javascripts/ml/experiment_tracking/routes/candidates/show/ml_candidates_show.vue b/app/assets/javascripts/ml/experiment_tracking/routes/candidates/show/ml_candidates_show.vue index b671bdc47a6..cef5da726f3 100644 --- a/app/assets/javascripts/ml/experiment_tracking/routes/candidates/show/ml_candidates_show.vue +++ b/app/assets/javascripts/ml/experiment_tracking/routes/candidates/show/ml_candidates_show.vue @@ -1,7 +1,6 @@ <script> import { GlLink } from '@gitlab/ui'; -import { FEATURE_NAME, FEATURE_FEEDBACK_ISSUE } from '~/ml/experiment_tracking/constants'; -import IncubationAlert from '~/vue_shared/components/incubation/incubation_alert.vue'; +import ModelExperimentsHeader from '~/ml/experiment_tracking/components/model_experiments_header.vue'; import DeleteButton from '~/ml/experiment_tracking/components/delete_button.vue'; import { TITLE_LABEL, @@ -22,7 +21,7 @@ import { export default { name: 'MlCandidatesShow', components: { - IncubationAlert, + ModelExperimentsHeader, DeleteButton, GlLink, }, @@ -65,32 +64,19 @@ export default { ]; }, }, - FEATURE_NAME, - FEATURE_FEEDBACK_ISSUE, }; </script> <template> <div> - <incubation-alert - :feature-name="$options.FEATURE_NAME" - :link-to-feedback-issue="$options.FEATURE_FEEDBACK_ISSUE" - /> - - <div class="detail-page-header gl-flex-wrap"> - <div class="detail-page-header-body"> - <h1 class="page-title gl-font-size-h-display flex-fill"> - {{ $options.i18n.TITLE_LABEL }} - </h1> - - <delete-button - :delete-path="candidate.info.path" - :delete-confirmation-text="$options.i18n.DELETE_CANDIDATE_CONFIRMATION_MESSAGE" - :action-primary-text="$options.i18n.DELETE_CANDIDATE_PRIMARY_ACTION_LABEL" - :modal-title="$options.i18n.DELETE_CANDIDATE_MODAL_TITLE" - /> - </div> - </div> + <model-experiments-header :page-title="$options.i18n.TITLE_LABEL"> + <delete-button + :delete-path="candidate.info.path" + :delete-confirmation-text="$options.i18n.DELETE_CANDIDATE_CONFIRMATION_MESSAGE" + :action-primary-text="$options.i18n.DELETE_CANDIDATE_PRIMARY_ACTION_LABEL" + :modal-title="$options.i18n.DELETE_CANDIDATE_MODAL_TITLE" + /> + </model-experiments-header> <table class="candidate-details gl-w-full"> <tbody> diff --git a/app/assets/stylesheets/page_bundles/jira_connect_users.scss b/app/assets/stylesheets/page_bundles/jira_connect_users.scss deleted file mode 100644 index 602910adad9..00000000000 --- a/app/assets/stylesheets/page_bundles/jira_connect_users.scss +++ /dev/null @@ -1 +0,0 @@ -@import '../themes/theme_indigo'; diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index a86a8a0415a..0ad8a08960a 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -97,7 +97,7 @@ module IssuableActions index_path = polymorphic_path([parent, issuable.class]) respond_to do |format| - format.html { redirect_to index_path } + format.html { redirect_to index_path, status: :see_other } format.json do render json: { web_url: index_path diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index a202808e2c3..b02a636ff74 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -141,7 +141,7 @@ module IssuableCollections common_attributes = [:author, :assignees, :labels, :milestone] @preload_for_collection ||= case collection_type when 'Issue' - common_attributes + [:project, project: :namespace] + common_attributes + [:work_item_type, :project, project: :namespace] when 'MergeRequest' common_attributes + [ :target_project, :latest_merge_request_diff, :approvals, :approved_by_users, :reviewers, diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index da2ed9d62e7..09b82e36b1a 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -51,7 +51,7 @@ module MembershipActions _("User was successfully removed from project.") end - redirect_to members_page_url, notice: message + redirect_to members_page_url, notice: message, status: :see_other end format.js { head :ok } diff --git a/app/controllers/jira_connect/subscriptions_controller.rb b/app/controllers/jira_connect/subscriptions_controller.rb index 76f0946762a..773ef2bddca 100644 --- a/app/controllers/jira_connect/subscriptions_controller.rb +++ b/app/controllers/jira_connect/subscriptions_controller.rb @@ -30,7 +30,7 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController respond_to do |format| format.html format.json do - render json: JiraConnect::AppDataSerializer.new(@subscriptions, !!current_user).as_json + render json: JiraConnect::AppDataSerializer.new(@subscriptions).as_json end end end diff --git a/app/controllers/jira_connect/users_controller.rb b/app/controllers/jira_connect/users_controller.rb deleted file mode 100644 index a37c68de299..00000000000 --- a/app/controllers/jira_connect/users_controller.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -class JiraConnect::UsersController < ApplicationController - feature_category :integrations - - layout 'signup_onboarding' - - before_action :verify_return_to_url, only: [:show] - - def show - @jira_app_link = params.delete(:return_to) - end - - private - - def verify_return_to_url - return unless params[:return_to].present? - - params.delete(:return_to) unless Integrations::Jira.valid_jira_cloud_url?(params[:return_to]) - end -end diff --git a/app/controllers/projects/analytics/cycle_analytics/stages_controller.rb b/app/controllers/projects/analytics/cycle_analytics/stages_controller.rb index a61b774f9c8..6d48aa765fb 100644 --- a/app/controllers/projects/analytics/cycle_analytics/stages_controller.rb +++ b/app/controllers/projects/analytics/cycle_analytics/stages_controller.rb @@ -6,7 +6,7 @@ class Projects::Analytics::CycleAnalytics::StagesController < Projects::Applicat respond_to :json - feature_category :planning_analytics + feature_category :team_planning before_action :authorize_read_cycle_analytics! before_action :only_default_value_stream_is_allowed! diff --git a/app/controllers/projects/analytics/cycle_analytics/summary_controller.rb b/app/controllers/projects/analytics/cycle_analytics/summary_controller.rb index 4ca59d9f6e7..1f58b97867f 100644 --- a/app/controllers/projects/analytics/cycle_analytics/summary_controller.rb +++ b/app/controllers/projects/analytics/cycle_analytics/summary_controller.rb @@ -6,7 +6,7 @@ class Projects::Analytics::CycleAnalytics::SummaryController < Projects::Applica respond_to :json - feature_category :planning_analytics + feature_category :team_planning before_action :authorize_read_cycle_analytics! diff --git a/app/controllers/projects/analytics/cycle_analytics/value_streams_controller.rb b/app/controllers/projects/analytics/cycle_analytics/value_streams_controller.rb index f58730f1d33..bf6c667b87b 100644 --- a/app/controllers/projects/analytics/cycle_analytics/value_streams_controller.rb +++ b/app/controllers/projects/analytics/cycle_analytics/value_streams_controller.rb @@ -5,7 +5,7 @@ class Projects::Analytics::CycleAnalytics::ValueStreamsController < Projects::Ap respond_to :json - feature_category :planning_analytics + feature_category :team_planning urgency :low private diff --git a/app/controllers/projects/cycle_analytics/events_controller.rb b/app/controllers/projects/cycle_analytics/events_controller.rb index 43b4cdbe9a8..d41a7e4f2df 100644 --- a/app/controllers/projects/cycle_analytics/events_controller.rb +++ b/app/controllers/projects/cycle_analytics/events_controller.rb @@ -11,7 +11,7 @@ module Projects before_action :authorize_read_issue!, only: [:issue, :production] before_action :authorize_read_merge_request!, only: [:code, :review] - feature_category :planning_analytics + feature_category :team_planning urgency :low def issue diff --git a/app/controllers/projects/cycle_analytics_controller.rb b/app/controllers/projects/cycle_analytics_controller.rb index 993b9d3b913..10dd18c0c86 100644 --- a/app/controllers/projects/cycle_analytics_controller.rb +++ b/app/controllers/projects/cycle_analytics_controller.rb @@ -17,7 +17,7 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController label: 'redis_hll_counters.analytics.analytics_total_unique_counts_monthly', destinations: %i[redis_hll snowplow] - feature_category :planning_analytics + feature_category :team_planning urgency :low before_action do diff --git a/app/graphql/mutations/work_items/update.rb b/app/graphql/mutations/work_items/update.rb index 3bcec7ebb1c..3fd0f5aab62 100644 --- a/app/graphql/mutations/work_items/update.rb +++ b/app/graphql/mutations/work_items/update.rb @@ -17,6 +17,8 @@ module Mutations description: 'Updated work item.' def resolve(id:, **attributes) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/408575') + work_item = authorized_find!(id: id) spam_params = ::Spam::SpamParams.new_from_request(request: context[:request]) diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb index 1e5833a5cf0..488e4d10cbc 100644 --- a/app/graphql/types/issue_type.rb +++ b/app/graphql/types/issue_type.rb @@ -122,6 +122,7 @@ module Types description: 'Collection of design images associated with this issue.' field :type, Types::IssueTypeEnum, null: true, + method: :issue_type, description: 'Type of the issue.' field :alert_management_alert, @@ -209,14 +210,6 @@ module Types def escalation_status object.supports_escalation? ? object.escalation_status&.status_name : nil end - - def type - if Feature.enabled?(:issue_type_uses_work_item_types_table) - object.work_item_type.base_type - else - object.issue_type - end - end end end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 45b231ebdbe..d058d0f697c 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -144,9 +144,9 @@ module IssuablesHelper def issuable_meta(issuable, project) output = [] - if issuable.respond_to?(:work_item_type) && WorkItems::Type::WI_TYPES_WITH_CREATED_HEADER.include?(issue_type_for(issuable)) + if issuable.respond_to?(:work_item_type) && WorkItems::Type::WI_TYPES_WITH_CREATED_HEADER.include?(issuable.issue_type) output << content_tag(:span, sprite_icon(issuable.work_item_type.icon_name.to_s, css_class: 'gl-icon gl-vertical-align-middle gl-text-gray-500'), class: 'gl-mr-2', aria: { hidden: 'true' }) - output << content_tag(:span, s_('IssuableStatus|%{wi_type} created %{created_at} by ').html_safe % { wi_type: IntegrationsHelper.integration_issue_type(issue_type_for(issuable)), created_at: time_ago_with_tooltip(issuable.created_at) }, class: 'gl-mr-2') + output << content_tag(:span, s_('IssuableStatus|%{wi_type} created %{created_at} by ').html_safe % { wi_type: IntegrationsHelper.integration_issue_type(issuable.issue_type), created_at: time_ago_with_tooltip(issuable.created_at) }, class: 'gl-mr-2') else output << content_tag(:span, s_('IssuableStatus|Created %{created_at} by').html_safe % { created_at: time_ago_with_tooltip(issuable.created_at) }, class: 'gl-mr-2') end @@ -263,7 +263,7 @@ module IssuablesHelper { hasClosingMergeRequest: issuable.merge_requests_count(current_user) != 0, - issueType: issue_type_for(issuable), + issueType: issuable.issue_type, zoomMeetingUrl: ZoomMeeting.canonical_meeting_url(issuable), sentryIssueIdentifier: SentryIssue.find_by(issue: issuable)&.sentry_issue_identifier, # rubocop:disable CodeReuse/ActiveRecord iid: issuable.iid.to_s, @@ -329,7 +329,7 @@ module IssuablesHelper def issuable_display_type(issuable) case issuable when Issue - issue_type_for(issuable).downcase + issuable.issue_type.downcase when MergeRequest issuable.model_name.human.downcase end @@ -388,7 +388,7 @@ module IssuablesHelper def issuable_type_selector_data(issuable) { - selected_type: issue_type_for(issuable), + selected_type: issuable.issue_type, is_issue_allowed: create_issue_type_allowed?(@project, :issue).to_s, is_incident_allowed: create_issue_type_allowed?(@project, :incident).to_s, issue_path: new_project_issue_path(@project), diff --git a/app/helpers/issue_type_helper.rb b/app/helpers/issue_type_helper.rb deleted file mode 100644 index 705cbe5c07a..00000000000 --- a/app/helpers/issue_type_helper.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -module IssueTypeHelper - def issue_type_for(issue) - return if issue.blank? - - if Feature.enabled?(:issue_type_uses_work_item_types_table) - issue.work_item_type.base_type - else - issue.issue_type - end - end -end diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index feafe35a3d5..9b0810f3d17 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -76,9 +76,7 @@ module TodosHelper elsif todo.member_access_requested? _('Group') elsif todo.for_issue_or_work_item? - IntegrationsHelper.integration_issue_type( - issue_type_for(todo.target) - ) + IntegrationsHelper.integration_issue_type(todo.target.issue_type) else IntegrationsHelper.integration_todo_target_type(todo.target_type) end diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 59234028c49..766155c6a99 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -14,9 +14,6 @@ module Ci include EachBatch include Gitlab::Utils::StrongMemoize - # NOTE: Temporarily ignore. This will will be used in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106740 - ignore_column :file_final_path, remove_with: '16.1', remove_after: '2023-05-23' - enum accessibility: { public: 0, private: 1 }, _suffix: true NON_ERASABLE_FILE_TYPES = %w[trace].freeze diff --git a/app/models/concerns/issue_available_features.rb b/app/models/concerns/issue_available_features.rb index c5d194a93e7..209456f8b67 100644 --- a/app/models/concerns/issue_available_features.rb +++ b/app/models/concerns/issue_available_features.rb @@ -27,14 +27,7 @@ module IssueAvailableFeatures raise ArgumentError, 'invalid feature' end - type_for_issue = if Feature.enabled?(:issue_type_uses_work_item_types_table) - # The default will only be used in places where an issue is only build and not saved - work_item_type_with_default.base_type - else - issue_type - end - - self.class.available_features_for_issue_types[feature].include?(type_for_issue) + self.class.available_features_for_issue_types[feature].include?(issue_type) end end diff --git a/app/models/event.rb b/app/models/event.rb index 76a34bf7810..9345776c32b 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -69,7 +69,7 @@ class Event < ApplicationRecord # If the association for "target" defines an "author" association we want to # eager-load this so Banzai & friends don't end up performing N+1 queries to # get the authors of notes, issues, etc. (likewise for "noteable"). - incs = %i(author noteable).select do |a| + incs = %i(author noteable work_item_type).select do |a| reflections['events'].active_record.reflect_on_association(a) end diff --git a/app/models/issue.rb b/app/models/issue.rb index 8c8b7b6e9fa..b0535a28640 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -127,7 +127,6 @@ class Issue < ApplicationRecord accepts_nested_attributes_for :incident_management_issuable_escalation_status, update_only: true validates :project, presence: true, if: -> { !namespace || namespace.is_a?(Namespaces::ProjectNamespace) } - validates :issue_type, presence: true validates :namespace, presence: true validates :work_item_type, presence: true validates :confidential, inclusion: { in: [true, false], message: 'must be a boolean' } @@ -135,6 +134,8 @@ class Issue < ApplicationRecord validate :allowed_work_item_type_change, on: :update, if: :work_item_type_id_changed? validate :due_date_after_start_date validate :parent_link_confidentiality + # using a custom validation since we are overwriting the `issue_type` method to use the work_item_types table + validate :issue_type_attribute_present enum issue_type: WorkItems::Type.base_types @@ -191,8 +192,8 @@ class Issue < ApplicationRecord scope :with_prometheus_alert_events, -> { joins(:issues_prometheus_alert_events) } scope :with_self_managed_prometheus_alert_events, -> { joins(:issues_self_managed_prometheus_alert_events) } scope :with_api_entity_associations, -> { - preload(:timelogs, :closed_by, :assignees, :author, :labels, :issuable_severity, namespace: [{ parent: :route }, :route], - milestone: { project: [:route, { namespace: :route }] }, + preload(:work_item_type, :timelogs, :closed_by, :assignees, :author, :labels, :issuable_severity, + namespace: [{ parent: :route }, :route], milestone: { project: [:route, { namespace: :route }] }, project: [:project_namespace, :project_feature, :route, { group: :route }, { namespace: :route }], duplicated_to: { project: [:project_feature] }) } @@ -726,6 +727,14 @@ class Issue < ApplicationRecord work_item_type || WorkItems::Type.default_by_type(DEFAULT_ISSUE_TYPE) end + def issue_type + if ::Feature.enabled?(:issue_type_uses_work_item_types_table) + work_item_type_with_default.base_type + else + super + end + end + private def check_issue_type_in_sync! @@ -734,7 +743,8 @@ class Issue < ApplicationRecord # https://gitlab.com/gitlab-org/gitlab/-/work_items/403158 return unless (changes.keys & %w[issue_type work_item_type_id]).any? - if issue_type != work_item_type.base_type + # Do not replace the use of attributes with `issue_type` here + if attributes['issue_type'] != work_item_type.base_type error = IssueTypeOutOfSyncError.new( <<~ERROR Issue `issue_type` out of sync with `work_item_type_id` column. @@ -749,12 +759,18 @@ class Issue < ApplicationRecord Gitlab::ErrorTracking.track_and_raise_for_dev_exception( error, - issue_type: issue_type, + issue_type: attributes['issue_type'], work_item_type_id: work_item_type_id ) end end + def issue_type_attribute_present + return if attributes['issue_type'].present? + + errors.add(:issue_type, 'Must be present') + end + def due_date_after_start_date return unless start_date.present? && due_date.present? @@ -825,7 +841,7 @@ class Issue < ApplicationRecord # TODO: We should switch to DEFAULT_ISSUE_TYPE here when the issue_type column is dropped # https://gitlab.com/gitlab-org/gitlab/-/work_items/402700 - self.work_item_type = WorkItems::Type.default_by_type(issue_type) + self.work_item_type = WorkItems::Type.default_by_type(attributes['issue_type']) end def allowed_work_item_type_change diff --git a/app/serializers/jira_connect/app_data_serializer.rb b/app/serializers/jira_connect/app_data_serializer.rb index 994ff19f96e..5e275f35beb 100644 --- a/app/serializers/jira_connect/app_data_serializer.rb +++ b/app/serializers/jira_connect/app_data_serializer.rb @@ -4,9 +4,8 @@ class JiraConnect::AppDataSerializer include Gitlab::Routing include ::API::Helpers::RelatedResourcesHelpers - def initialize(subscriptions, signed_in) + def initialize(subscriptions) @subscriptions = subscriptions - @signed_in = signed_in end def as_json @@ -15,14 +14,7 @@ class JiraConnect::AppDataSerializer { groups_path: api_v4_groups_path(params: { min_access_level: Gitlab::Access::MAINTAINER, skip_groups: skip_groups }), subscriptions: JiraConnect::SubscriptionEntity.represent(@subscriptions).as_json, - subscriptions_path: jira_connect_subscriptions_path, - login_path: signed_in? ? nil : jira_connect_users_path + subscriptions_path: jira_connect_subscriptions_path } end - - private - - def signed_in? - !!@signed_in - end end diff --git a/app/services/ci/job_artifacts/create_service.rb b/app/services/ci/job_artifacts/create_service.rb index 1de2424924a..f7e04c59463 100644 --- a/app/services/ci/job_artifacts/create_service.rb +++ b/app/services/ci/job_artifacts/create_service.rb @@ -23,7 +23,11 @@ module Ci result = validate_requirements(artifact_type: artifact_type, filesize: filesize) return result unless result[:status] == :success - headers = JobArtifactUploader.workhorse_authorize(has_length: false, maximum_size: max_size(artifact_type)) + headers = JobArtifactUploader.workhorse_authorize( + has_length: false, + maximum_size: max_size(artifact_type), + use_final_store_path: Feature.enabled?(:ci_artifacts_upload_to_final_location, project) + ) if lsif?(artifact_type) headers[:ProcessLsif] = true diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb index d8c7c5d4455..a65fc0c7c87 100644 --- a/app/services/issues/build_service.rb +++ b/app/services/issues/build_service.rb @@ -84,15 +84,17 @@ module Issues end base_type = work_item_type&.base_type + if create_issue_type_allowed?(container, base_type) issue.work_item_type = work_item_type # Up to this point issue_type might be set to the default, so we need to sync if a work item type is provided - issue.issue_type = work_item_type.base_type + issue.issue_type = base_type + else + # If no work item type was provided or not allowed, we need to set it to issue_type, + # and that includes the column default + issue_type = issue_params[:issue_type] || ::Issue::DEFAULT_ISSUE_TYPE + issue.work_item_type = WorkItems::Type.default_by_type(issue_type) end - - # If no work item type was provided, we need to set it to whatever issue_type was up to this point, - # and that includes the column default - issue.work_item_type = WorkItems::Type.default_by_type(issue.issue_type) end def model_klass diff --git a/app/services/issues/export_csv_service.rb b/app/services/issues/export_csv_service.rb index d7c1ea276de..9e524d90505 100644 --- a/app/services/issues/export_csv_service.rb +++ b/app/services/issues/export_csv_service.rb @@ -18,7 +18,7 @@ module Issues private def associations_to_preload - [:author, :assignees, :timelogs, :milestone, { project: { namespace: :route } }] + [:work_item_type, :author, :assignees, :timelogs, :milestone, { project: { namespace: :route } }] end def header_to_value_hash diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 2cf3f36eef1..201bf19b535 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -39,7 +39,8 @@ module Issues def change_work_item_type(issue) return unless issue.changed_attributes['issue_type'] - type_id = find_work_item_type_id(issue.issue_type) + issue_type = params[:issue_type] || ::Issue::DEFAULT_ISSUE_TYPE + type_id = find_work_item_type_id(issue_type) issue.work_item_type_id = type_id end diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index ab5c46d14f9..4188e0caa8e 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -23,6 +23,19 @@ module ObjectStorage end end + class DirectUploadStorage < ::CarrierWave::Storage::Fog + def store!(file) + return super unless @uploader.direct_upload_final_path.present? + + # The direct_upload_final_path is defined which means + # file was uploaded to its final location so no need to move it. + # Now we delete the pending upload entry as the upload is considered complete. + ObjectStorage::PendingDirectUpload.complete(@uploader.class.storage_location_identifier, file.path) + + file + end + end + TMP_UPLOAD_PATH = 'tmp/uploads' module Store @@ -91,6 +104,7 @@ module ObjectStorage module Concern extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override included do |base| base.include(ObjectStorage) @@ -111,6 +125,10 @@ module ObjectStorage object_store_options&.direct_upload end + def direct_upload_to_object_store? + object_store_enabled? && direct_upload_enabled? + end + def proxy_download_enabled? object_store_options.proxy_download end @@ -131,10 +149,26 @@ module ObjectStorage model_class.uploader_options.dig(mount_point, :mount_on) || mount_point end - def workhorse_authorize(has_length:, maximum_size: nil) + def generate_remote_id + [CarrierWave.generate_cache_id, SecureRandom.hex].join('-') + end + + def generate_final_store_path + hash = Digest::SHA2.hexdigest(SecureRandom.uuid) + + # We prefix '@final' to prevent clashes and make the files easily recognizable + # as having been created by this code. + File.join('@final', hash[0..1], hash[2..3], hash[4..]) + end + + def workhorse_authorize(has_length:, maximum_size: nil, use_final_store_path: false) {}.tap do |hash| - if self.object_store_enabled? && self.direct_upload_enabled? - hash[:RemoteObject] = workhorse_remote_upload_options(has_length: has_length, maximum_size: maximum_size) + if self.direct_upload_to_object_store? + hash[:RemoteObject] = workhorse_remote_upload_options( + has_length: has_length, + maximum_size: maximum_size, + use_final_store_path: use_final_store_path + ) else hash[:TempPath] = workhorse_local_upload_path end @@ -148,21 +182,38 @@ module ObjectStorage File.join(self.root, TMP_UPLOAD_PATH) end + def with_bucket_prefix(path) + File.join([object_store_options.bucket_prefix, path].compact) + end + def object_store_config ObjectStorage::Config.new(object_store_options) end - def workhorse_remote_upload_options(has_length:, maximum_size: nil) - return unless self.object_store_enabled? - return unless self.direct_upload_enabled? + def workhorse_remote_upload_options(has_length:, maximum_size: nil, use_final_store_path: false) + return unless direct_upload_to_object_store? + + if use_final_store_path + id = generate_final_store_path + upload_path = with_bucket_prefix(id) + prepare_pending_direct_upload(id) + else + id = generate_remote_id + upload_path = File.join(TMP_UPLOAD_PATH, id) + end - id = [CarrierWave.generate_cache_id, SecureRandom.hex].join('-') - upload_path = File.join(TMP_UPLOAD_PATH, id) direct_upload = ObjectStorage::DirectUpload.new(self.object_store_config, upload_path, - has_length: has_length, maximum_size: maximum_size) + has_length: has_length, maximum_size: maximum_size, skip_delete: use_final_store_path) direct_upload.to_hash.merge(ID: id) end + + def prepare_pending_direct_upload(path) + ObjectStorage::PendingDirectUpload.prepare( + storage_location_identifier, + path + ) + end end class OpenFile @@ -304,13 +355,16 @@ module ObjectStorage def store_path(*args) if self.object_store == Store::REMOTE + path = direct_upload_final_path + path ||= super + # We allow administrators to create "sub buckets" by setting a prefix. # This makes it possible to deploy GitLab with only one object storage # bucket. Because the prefix is configuration data we do not want to # store it in the uploads table via RecordsUploads. That means that the # prefix cannot be part of store_dir. This is why we chose to implement # the prefix support here in store_path. - File.join([self.class.object_store_options.bucket_prefix, super].compact) + self.class.with_bucket_prefix(path) else super end @@ -335,7 +389,7 @@ module ObjectStorage def store!(new_file = nil) # when direct upload is enabled, always store on remote storage - if self.class.object_store_enabled? && self.class.direct_upload_enabled? + if self.class.direct_upload_to_object_store? self.object_store = Store::REMOTE end @@ -346,12 +400,44 @@ module ObjectStorage "object_storage_migrate:#{model.class}:#{model.id}" end + override :delete_tmp_file_after_storage + def delete_tmp_file_after_storage + # If final path is present then the file is not on temporary location + # so we don't want carrierwave to delete it. + return false if direct_upload_final_path.present? + + super + end + + def retrieve_from_store!(identifier) + # We need to force assign the value of @filename so that we will still + # get the original_filename in cases wherein the file points to a random generated + # path format. This happens for direct uploaded files to final location. + # + # If we don't set @filename value here, the result of uploader.filename (see ObjectStorage#filename) will result + # to the value of uploader.file.filename which will then contain the random generated path. + # The `identifier` variable contains the value of the `file` column which is the original_filename. + # + # In cases wherein we are not uploading to final location, it is still fine to set the + # @filename with the `identifier` value because it still contains the original filename from the `file` column, + # which is what we want in either case. + @filename = identifier # rubocop: disable Gitlab/ModuleWithInstanceVariables + + super + end + private def cache_remote_file!(remote_object_id, original_filename) - file_path = File.join(TMP_UPLOAD_PATH, remote_object_id) - file_path = Pathname.new(file_path).cleanpath.to_s - raise RemoteStoreError, 'Bad file path' unless file_path.start_with?(TMP_UPLOAD_PATH + '/') + if ObjectStorage::PendingDirectUpload.exists?(self.class.storage_location_identifier, remote_object_id) # rubocop:disable CodeReuse/ActiveRecord + # This is an assumption that a model with matching pending direct upload will have this attribute + model.write_attribute(direct_upload_final_path_attribute_name, remote_object_id) + file_path = self.class.with_bucket_prefix(remote_object_id) + else + file_path = File.join(TMP_UPLOAD_PATH, remote_object_id) + file_path = Pathname.new(file_path).cleanpath.to_s + raise RemoteStoreError, 'Bad file path' unless file_path.start_with?(TMP_UPLOAD_PATH + '/') + end # TODO: # This should be changed to make use of `tmp/cache` mechanism @@ -398,7 +484,7 @@ module ObjectStorage when Store::REMOTE raise "Object Storage is not enabled for #{self.class}" unless self.class.object_store_enabled? - CarrierWave::Storage::Fog.new(self) + DirectUploadStorage.new(self) when Store::LOCAL CarrierWave::Storage::File.new(self) else @@ -464,6 +550,14 @@ module ObjectStorage cache_storage.delete_dir!(cache_path(nil)) end end + + def direct_upload_final_path_attribute_name + "#{mounted_as}_final_path" + end + + def direct_upload_final_path + model.try(direct_upload_final_path_attribute_name) + end end ObjectStorage::Concern.include_mod_with('ObjectStorage::Concern') diff --git a/app/views/jira_connect/users/show.html.haml b/app/views/jira_connect/users/show.html.haml deleted file mode 100644 index 5db6cb44ff6..00000000000 --- a/app/views/jira_connect/users/show.html.haml +++ /dev/null @@ -1,21 +0,0 @@ -.gl-text-center.gl-mx-auto.gl-pt-6 - %h3.gl-mb-4 - = _('You are signed in to GitLab as:') - - .gl-display-flex.gl-flex-direction-column.gl-align-items-center.gl-mb-4 - = link_to user_path(current_user), target: '_blank', rel: 'noopener noreferrer' do - = user_avatar_without_link(user: current_user, size: 60, css_class: 'gl-mr-0! gl-mb-2', has_tooltip: false) - = link_to current_user.to_reference, user_path(current_user), target: '_blank', rel: 'noopener noreferrer' - - %p.gl-mb-6 - = s_('JiraService|You can now close this window and%{br}return to the GitLab for Jira application.').html_safe % { br: '<br>'.html_safe } - - - if @jira_app_link - %p - = render Pajamas::ButtonComponent.new(href: @jira_app_link, variant: :confirm) do - = s_('Integrations|Return to GitLab for Jira') - - - %p= link_to _('Sign out'), destroy_user_session_path, method: :post - -- add_page_specific_style 'page_bundles/jira_connect_users' diff --git a/config/application.rb b/config/application.rb index 2fd6f5c72c4..9d8d285b6bc 100644 --- a/config/application.rb +++ b/config/application.rb @@ -292,7 +292,6 @@ module Gitlab config.assets.precompile << "page_bundles/issues_list.css" config.assets.precompile << "page_bundles/issues_show.css" config.assets.precompile << "page_bundles/jira_connect.css" - config.assets.precompile << "page_bundles/jira_connect_users.css" config.assets.precompile << "page_bundles/learn_gitlab.css" config.assets.precompile << "page_bundles/login.css" config.assets.precompile << "page_bundles/marketing_popover.css" diff --git a/config/feature_categories.yml b/config/feature_categories.yml index 33044cfff8e..804ab08ad6a 100644 --- a/config/feature_categories.yml +++ b/config/feature_categories.yml @@ -91,7 +91,6 @@ - package_registry - pages - pipeline_composition -- planning_analytics - portfolio_management - product_analytics - projects diff --git a/config/feature_flags/development/ci_artifacts_upload_to_final_location.yml b/config/feature_flags/development/ci_artifacts_upload_to_final_location.yml new file mode 100644 index 00000000000..452c6ed8331 --- /dev/null +++ b/config/feature_flags/development/ci_artifacts_upload_to_final_location.yml @@ -0,0 +1,8 @@ +--- +name: ci_artifacts_upload_to_final_location +introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106740' +rollout_issue_url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/385447' +milestone: '16.0' +type: development +group: group::pipeline security +default_enabled: false diff --git a/config/feature_flags/ops/runner_migrations_backoff.yml b/config/feature_flags/ops/runner_migrations_backoff.yml new file mode 100644 index 00000000000..5b8dc2bd420 --- /dev/null +++ b/config/feature_flags/ops/runner_migrations_backoff.yml @@ -0,0 +1,8 @@ +--- +name: runner_migrations_backoff +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115679 +rollout_issue_url: +milestone: '16.0' +type: ops +group: group::pipeline execution +default_enabled: false diff --git a/config/initializers/active_record_migrations.rb b/config/initializers/active_record_migrations.rb index b83b394d8d3..6e5d519e11b 100644 --- a/config/initializers/active_record_migrations.rb +++ b/config/initializers/active_record_migrations.rb @@ -2,3 +2,4 @@ Gitlab::Database::Migrations::LockRetryMixin.patch! Gitlab::Database::Migrations::PgBackendPid.patch! +Gitlab::Database::Migrations::RunnerBackoff::ActiveRecordMixin.patch! diff --git a/config/metrics/counts_28d/20210216182102_wiki_action_monthly.yml b/config/metrics/counts_28d/20210216182102_wiki_action_monthly.yml index fc821f8c83f..56052ecb81a 100644 --- a/config/metrics/counts_28d/20210216182102_wiki_action_monthly.yml +++ b/config/metrics/counts_28d/20210216182102_wiki_action_monthly.yml @@ -3,8 +3,8 @@ data_category: optional key_path: redis_hll_counters.source_code.wiki_action_monthly description: Count of unique actions done on a wiki (create, edit, delete) product_section: dev -product_stage: create -product_group: source_code +product_stage: plan +product_group: knowledge value_type: number status: active time_frame: 28d diff --git a/config/metrics/counts_all/20210216183023_wiki_pages_view.yml b/config/metrics/counts_all/20210216183023_wiki_pages_view.yml index cf56b7d8a86..5e54f9d806d 100644 --- a/config/metrics/counts_all/20210216183023_wiki_pages_view.yml +++ b/config/metrics/counts_all/20210216183023_wiki_pages_view.yml @@ -3,8 +3,8 @@ data_category: optional key_path: counts.wiki_pages_view description: Total number of wiki page views product_section: dev -product_stage: create -product_group: editor +product_stage: plan +product_group: knowledge value_type: number status: active milestone: "13.3" diff --git a/config/routes/jira_connect.rb b/config/routes/jira_connect.rb index f45f524935a..91f2a313c4e 100644 --- a/config/routes/jira_connect.rb +++ b/config/routes/jira_connect.rb @@ -5,7 +5,6 @@ namespace :jira_connect do root to: proc { [404, {}, ['']] }, as: 'base' get 'app_descriptor' => 'app_descriptor#show' - get :users, to: 'users#show' namespace :events do post 'installed' diff --git a/db/docs/resource_iteration_events.yml b/db/docs/resource_iteration_events.yml index 46a9e88fd9a..fe39c2cc04a 100644 --- a/db/docs/resource_iteration_events.yml +++ b/db/docs/resource_iteration_events.yml @@ -3,7 +3,7 @@ table_name: resource_iteration_events classes: - ResourceIterationEvent feature_categories: -- planning_analytics +- team_planning description: Records the addition and removal of issues to iterations introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37617 milestone: '13.3' diff --git a/db/docs/resource_label_events.yml b/db/docs/resource_label_events.yml index 9de636ea874..9997cb576aa 100644 --- a/db/docs/resource_label_events.yml +++ b/db/docs/resource_label_events.yml @@ -3,7 +3,7 @@ table_name: resource_label_events classes: - ResourceLabelEvent feature_categories: -- planning_analytics +- team_planning description: Records the addition and removal of labels from resources that can be labelled; such as issues, MRs and epics introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/6697 milestone: '11.2' diff --git a/db/docs/resource_link_events.yml b/db/docs/resource_link_events.yml index cfa04aa522c..7d367a76c2c 100644 --- a/db/docs/resource_link_events.yml +++ b/db/docs/resource_link_events.yml @@ -3,7 +3,7 @@ table_name: resource_link_events classes: - WorkItems::ResourceLinkEvent feature_categories: -- planning_analytics +- team_planning description: Records the change of parent link on work items along with timestamps introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114394 milestone: '15.11' diff --git a/db/docs/resource_milestone_events.yml b/db/docs/resource_milestone_events.yml index 02962bc0056..5ec3271492f 100644 --- a/db/docs/resource_milestone_events.yml +++ b/db/docs/resource_milestone_events.yml @@ -3,7 +3,7 @@ table_name: resource_milestone_events classes: - ResourceMilestoneEvent feature_categories: -- planning_analytics +- team_planning description: Records the addition and removal of issues to milestones introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23965 milestone: '12.8' diff --git a/db/docs/resource_state_events.yml b/db/docs/resource_state_events.yml index 2390cd26bac..b93c7e51dd0 100644 --- a/db/docs/resource_state_events.yml +++ b/db/docs/resource_state_events.yml @@ -3,7 +3,7 @@ table_name: resource_state_events classes: - ResourceStateEvent feature_categories: -- planning_analytics +- team_planning description: Records the change of state of issues between opened and closed introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28926 milestone: '13.0' diff --git a/db/docs/resource_weight_events.yml b/db/docs/resource_weight_events.yml index 12cf9b27d19..2abd3be48f8 100644 --- a/db/docs/resource_weight_events.yml +++ b/db/docs/resource_weight_events.yml @@ -3,7 +3,7 @@ table_name: resource_weight_events classes: - ResourceWeightEvent feature_categories: -- planning_analytics +- team_planning description: Records the change of weight on issues along with timestamps introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/21515 milestone: '12.7' diff --git a/db/migrate/20230412185920_validate_ci_job_artifacts_file_final_path.rb b/db/migrate/20230412185920_validate_ci_job_artifacts_file_final_path.rb new file mode 100644 index 00000000000..464ac1eec72 --- /dev/null +++ b/db/migrate/20230412185920_validate_ci_job_artifacts_file_final_path.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class ValidateCiJobArtifactsFileFinalPath < Gitlab::Database::Migration[2.1] + def up + constraint_name = text_limit_name(:ci_job_artifacts, :file_final_path) + validate_check_constraint :ci_job_artifacts, constraint_name + end + + # No-op + def down; end +end diff --git a/db/schema_migrations/20230412185920 b/db/schema_migrations/20230412185920 new file mode 100644 index 00000000000..4609cd6d2bf --- /dev/null +++ b/db/schema_migrations/20230412185920 @@ -0,0 +1 @@ +772e42722b07397934795164c2587e44ed4c0552d5c07eb8d867e66ee6795c65
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index a84efd98fb4..79bc3c1c793 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13343,7 +13343,8 @@ CREATE TABLE ci_job_artifacts ( partition_id bigint DEFAULT 100 NOT NULL, accessibility smallint DEFAULT 0 NOT NULL, file_final_path text, - CONSTRAINT check_27f0f6dbab CHECK ((file_store IS NOT NULL)) + CONSTRAINT check_27f0f6dbab CHECK ((file_store IS NOT NULL)), + CONSTRAINT check_9f04410cf4 CHECK ((char_length(file_final_path) <= 1024)) ); CREATE SEQUENCE ci_job_artifacts_id_seq @@ -26635,9 +26636,6 @@ ALTER TABLE ONLY chat_teams ALTER TABLE vulnerability_scanners ADD CONSTRAINT check_37608c9db5 CHECK ((char_length(vendor) <= 255)) NOT VALID; -ALTER TABLE ci_job_artifacts - ADD CONSTRAINT check_9f04410cf4 CHECK ((char_length(file_final_path) <= 1024)) NOT VALID; - ALTER TABLE sprints ADD CONSTRAINT check_ccd8a1eae0 CHECK ((start_date IS NOT NULL)) NOT VALID; diff --git a/doc/administration/auth/ldap/index.md b/doc/administration/auth/ldap/index.md index 9a237ea8a7a..7687f7c9340 100644 --- a/doc/administration/auth/ldap/index.md +++ b/doc/administration/auth/ldap/index.md @@ -1046,7 +1046,7 @@ For more information on synchronizing users and groups between LDAP and GitLab, 1. In the configuration file, change: - `omniauth_auto_link_user` to `saml` only. - - `auto_link_ldap_user` to false. + - `omniauth_auto_link_ldap_user` to false. - `ldap_enabled` to `false`. You can also comment out the LDAP provider settings. diff --git a/doc/development/database/efficient_in_operator_queries.md b/doc/development/database/efficient_in_operator_queries.md index 78b310ae708..a770dfe6531 100644 --- a/doc/development/database/efficient_in_operator_queries.md +++ b/doc/development/database/efficient_in_operator_queries.md @@ -433,7 +433,7 @@ construct the following table: For the `issue_types` query we can construct a value list without querying a table: ```ruby -value_list = Arel::Nodes::ValuesList.new([[Issue.issue_types[:incident]],[Issue.issue_types[:test_case]]]) +value_list = Arel::Nodes::ValuesList.new([[WorkItems::Type.base_types[:incident]],[WorkItems::Type.base_types[:test_case]]]) issue_type_values = Arel::Nodes::Grouping.new(value_list).as('issue_type_values (value)').to_sql array_scope = Group diff --git a/doc/development/testing_guide/testing_migrations_guide.md b/doc/development/testing_guide/testing_migrations_guide.md index 1b1fdcca003..f69f0d1febb 100644 --- a/doc/development/testing_guide/testing_migrations_guide.md +++ b/doc/development/testing_guide/testing_migrations_guide.md @@ -290,7 +290,7 @@ RSpec.describe MigrateIncidentIssuesToIncidentType do .from(issue_type) .to(incident_type) - expect(other_issue.reload.issue_type).to eql(issue_type) + expect(other_issue.reload.issue_type).to eq(issue_type) end end diff --git a/doc/integration/jira/dvcs/index.md b/doc/integration/jira/dvcs/index.md index 6575d92d108..6a197ce5c76 100644 --- a/doc/integration/jira/dvcs/index.md +++ b/doc/integration/jira/dvcs/index.md @@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w WARNING: The Jira DVCS connector for Jira Cloud was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/362168) in GitLab 15.1 -and is planned for removal in 16.0. Use the [GitLab for Jira Cloud app](../connect-app.md) instead. +and [removed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118126) in 16.0. Use the [GitLab for Jira Cloud app](../connect-app.md) instead. The Jira DVCS connector was also deprecated for Jira 8.13 and earlier. You can only use the Jira DVCS connector with Jira Data Center or Jira Server in Jira 8.14 and later. Upgrade your Jira instance to Jira 8.14 or later, and reconfigure the Jira integration in your GitLab instance. Use the Jira DVCS (distributed version control system) connector if you self-host diff --git a/doc/user/project/service_desk.md b/doc/user/project/service_desk.md index f23635ed50f..bd5c8214e68 100644 --- a/doc/user/project/service_desk.md +++ b/doc/user/project/service_desk.md @@ -101,6 +101,8 @@ visible in the email template. For more information, see #### Thank you email +> `%{ISSUE_DESCRIPTION}` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/223751) in GitLab 16.0. + When a user submits an issue through Service Desk, GitLab sends a **thank you email**. To create a custom email template, in the `.gitlab/service_desk_templates/` @@ -110,6 +112,7 @@ You can use these placeholders to be automatically replaced in each email: - `%{ISSUE_ID}`: issue IID - `%{ISSUE_PATH}`: project path appended with the issue IID +- `%{ISSUE_DESCRIPTION}`: issue description based on the original email - `%{UNSUBSCRIBE_URL}`: unsubscribe URL - `%{SYSTEM_HEADER}`: [system header message](../admin_area/appearance.md#system-header-and-footer-messages) - `%{SYSTEM_FOOTER}`: [system footer message](../admin_area/appearance.md#system-header-and-footer-messages) @@ -120,6 +123,8 @@ the response email does not contain the issue link. #### New note email +> `%{ISSUE_DESCRIPTION}` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/223751) in GitLab 16.0. + When a user-submitted issue receives a new comment, GitLab sends a **new note email**. To create a custom email template, in the `.gitlab/service_desk_templates/` @@ -129,6 +134,10 @@ You can use these placeholders to be automatically replaced in each email: - `%{ISSUE_ID}`: issue IID - `%{ISSUE_PATH}`: project path appended with the issue IID +- `%{ISSUE_DESCRIPTION}`: issue description at the time email is generated. + If a user has edited the description, it might contain sensitive information that is not intended + to be delivered to external participants. Use this placeholder only if you never modify + descriptions or your team is aware of the template design. - `%{NOTE_TEXT}`: note text - `%{UNSUBSCRIBE_URL}`: unsubscribe URL - `%{SYSTEM_HEADER}`: [system header message](../admin_area/appearance.md#system-header-and-footer-messages) diff --git a/lib/api/todos.rb b/lib/api/todos.rb index 57745ee8802..7e6b21d6121 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -88,6 +88,8 @@ module API use :pagination, :todo_filters end get do + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/408576') + todos = paginate(find_todos.with_entity_associations) todos = ::Todos::AllowedTargetFilterService.new(todos, current_user).execute options = { with: Entities::Todo, current_user: current_user } diff --git a/lib/banzai/filter/references/issue_reference_filter.rb b/lib/banzai/filter/references/issue_reference_filter.rb index e186c8b1f73..9ad9d286ce3 100644 --- a/lib/banzai/filter/references/issue_reference_filter.rb +++ b/lib/banzai/filter/references/issue_reference_filter.rb @@ -22,7 +22,7 @@ module Banzai end def parent_records(parent, ids) - parent.issues.where(iid: ids.to_a).includes(:project, :namespace) + parent.issues.where(iid: ids.to_a).includes(:project, :namespace, :work_item_type) end def object_link_text_extras(issue, matches) @@ -40,7 +40,7 @@ module Banzai private def additional_object_attributes(issue) - { issue_type: issue.issue_type } + { issue_type: issue.work_item_type.base_type } end def issue_path(issue, project) diff --git a/lib/gitlab/database/migration.rb b/lib/gitlab/database/migration.rb index 4d38920f571..fbb71c1ccfd 100644 --- a/lib/gitlab/database/migration.rb +++ b/lib/gitlab/database/migration.rb @@ -53,6 +53,7 @@ module Gitlab class V2_1 < V2_0 # rubocop:disable Naming/ClassAndModuleCamelCase include Gitlab::Database::MigrationHelpers::AutomaticLockWritesOnTables + include Gitlab::Database::Migrations::RunnerBackoff::MigrationHelpers end def self.[](version) diff --git a/lib/gitlab/database/migrations/runner_backoff/active_record_mixin.rb b/lib/gitlab/database/migrations/runner_backoff/active_record_mixin.rb new file mode 100644 index 00000000000..b5348f4b4e6 --- /dev/null +++ b/lib/gitlab/database/migrations/runner_backoff/active_record_mixin.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Migrations + module RunnerBackoff + module ActiveRecordMixin + module ActiveRecordMigrationProxyRunnerBackoff + # Regular AR migrations don't have this, + # only ones inheriting from Gitlab::Database::Migration have + def enable_runner_backoff? + !!migration.try(:enable_runner_backoff?) + end + end + + module ActiveRecordMigratorRunnerBackoff + def execute_migration_in_transaction(migration) + if migration.enable_runner_backoff? + RunnerBackoff::Communicator.execute_with_lock(migration) { super } + else + super + end + end + end + + def self.patch! + ActiveRecord::MigrationProxy.prepend(ActiveRecordMigrationProxyRunnerBackoff) + ActiveRecord::Migrator.prepend(ActiveRecordMigratorRunnerBackoff) + end + end + end + end + end +end diff --git a/lib/gitlab/database/migrations/runner_backoff/communicator.rb b/lib/gitlab/database/migrations/runner_backoff/communicator.rb new file mode 100644 index 00000000000..874dfc56832 --- /dev/null +++ b/lib/gitlab/database/migrations/runner_backoff/communicator.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Migrations + module RunnerBackoff + class Communicator + EXPIRY = 1.minute + KEY = 'gitlab/database/migration/runner/backoff' + + def self.execute_with_lock(migration, &block) + new(migration).execute_with_lock(&block) + end + + def self.backoff_runner? + return false if ::Feature.disabled?(:runner_migrations_backoff, type: :ops) + + Gitlab::ExclusiveLease.new(KEY, timeout: EXPIRY).exists? + end + + def initialize(migration, logger: Gitlab::AppLogger) + @migration = migration + @logger = logger + end + + def execute_with_lock + log(message: 'Executing migration with Runner backoff') + + set_lock + yield if block_given? + ensure + remove_lock + end + + private + + attr_reader :logger, :migration + + def set_lock + raise 'Could not set backoff key' unless exclusive_lease.try_obtain + + log(message: 'Runner backoff key is set') + end + + def remove_lock + exclusive_lease.cancel + + log(message: 'Runner backoff key was removed') + end + + def exclusive_lease + @exclusive_lease ||= Gitlab::ExclusiveLease.new(KEY, timeout: EXPIRY) + end + + def log(params) + logger.info(log_params.merge(params)) + end + + def log_params + { class: migration.name } + end + end + end + end + end +end diff --git a/lib/gitlab/database/migrations/runner_backoff/migration_helpers.rb b/lib/gitlab/database/migrations/runner_backoff/migration_helpers.rb new file mode 100644 index 00000000000..b9e564d85e6 --- /dev/null +++ b/lib/gitlab/database/migrations/runner_backoff/migration_helpers.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Migrations + module RunnerBackoff + module MigrationHelpers + extend ActiveSupport::Concern + + class_methods do + def enable_runner_backoff! + @enable_runner_backoff = true # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + + def enable_runner_backoff? + !!@enable_runner_backoff + end + end + + delegate :enable_runner_backoff?, to: :class + end + end + end + end +end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 3a65c7c334d..11eb0a584ab 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -464,7 +464,8 @@ module Gitlab end def fetch_body_from_gitaly - self.class.get_message(@repository, id) + # #to_s is required to ensure BatchLoader is not returned. + self.class.get_message(@repository, id).to_s end def self.valid?(commit_id) diff --git a/lib/object_storage/direct_upload.rb b/lib/object_storage/direct_upload.rb index fee006f4e0f..588e3c3e89f 100644 --- a/lib/object_storage/direct_upload.rb +++ b/lib/object_storage/direct_upload.rb @@ -23,9 +23,9 @@ module ObjectStorage MINIMUM_MULTIPART_SIZE = 5.megabytes attr_reader :config, :credentials, :bucket_name, :object_name - attr_reader :has_length, :maximum_size + attr_reader :has_length, :maximum_size, :skip_delete - def initialize(config, object_name, has_length:, maximum_size: nil) + def initialize(config, object_name, has_length:, maximum_size: nil, skip_delete: false) unless has_length raise ArgumentError, 'maximum_size has to be specified if length is unknown' unless maximum_size end @@ -36,6 +36,7 @@ module ObjectStorage @object_name = object_name @has_length = has_length @maximum_size = maximum_size + @skip_delete = skip_delete end def to_hash @@ -44,7 +45,7 @@ module ObjectStorage GetURL: get_url, StoreURL: store_url, DeleteURL: delete_url, - SkipDelete: false, + SkipDelete: skip_delete, MultipartUpload: multipart_upload_hash, CustomPutHeaders: true, PutHeaders: upload_options diff --git a/lib/object_storage/pending_direct_upload.rb b/lib/object_storage/pending_direct_upload.rb new file mode 100644 index 00000000000..3e84bc4ebc9 --- /dev/null +++ b/lib/object_storage/pending_direct_upload.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module ObjectStorage + class PendingDirectUpload + KEY = 'pending_direct_uploads' + + def self.prepare(location_identifier, path) + ::Gitlab::Redis::SharedState.with do |redis| + # We need to store the location_identifier together with the timestamp to properly delete + # this object if ever this upload gets stale. The location identifier will be used + # by the clean up worker to properly generate the storage options through ObjectStorage::Config.for_location + redis.hset(KEY, key(location_identifier, path), Time.current.utc.to_i) + end + end + + def self.exists?(location_identifier, path) + ::Gitlab::Redis::SharedState.with do |redis| + redis.hexists(KEY, key(location_identifier, path)) + end + end + + def self.complete(location_identifier, path) + ::Gitlab::Redis::SharedState.with do |redis| + redis.hdel(KEY, key(location_identifier, path)) + end + end + + def self.key(location_identifier, path) + [location_identifier, path].join(':') + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 85f28265094..2ea1a7e0073 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -24000,9 +24000,6 @@ msgstr "" msgid "Integrations|Restrict to branch (optional)" msgstr "" -msgid "Integrations|Return to GitLab for Jira" -msgstr "" - msgid "Integrations|SSL verification" msgstr "" @@ -25331,9 +25328,6 @@ msgstr "" msgid "JiraService|Work on Jira issues without leaving GitLab. Add a Jira menu to access a read-only list of your Jira issues. %{jira_issues_link_start}Learn more.%{link_end}" msgstr "" -msgid "JiraService|You can now close this window and%{br}return to the GitLab for Jira application." -msgstr "" - msgid "JiraService|You must configure Jira before enabling this integration. %{jira_doc_link_start}Learn more.%{link_end}" msgstr "" @@ -51149,9 +51143,6 @@ msgstr "" msgid "You are receiving this message because you are a GitLab administrator for %{url}." msgstr "" -msgid "You are signed in to GitLab as:" -msgstr "" - msgid "You are trying to upload something other than an image. Please upload a .png, .jpg, .jpeg, .gif, .bmp, .tiff or .ico." msgstr "" diff --git a/package.json b/package.json index 3843c327717..29221c7b4be 100644 --- a/package.json +++ b/package.json @@ -122,7 +122,7 @@ "clipboard": "^2.0.8", "compression-webpack-plugin": "^5.0.2", "copy-webpack-plugin": "^6.4.1", - "core-js": "^3.30.1", + "core-js": "^3.30.2", "cron-validator": "^1.1.1", "cronstrue": "^1.122.0", "cropper": "^2.3.0", diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/move_project_create_fork_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/move_project_create_fork_spec.rb index a391e6313a6..a13b1517740 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/move_project_create_fork_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/move_project_create_fork_spec.rb @@ -37,6 +37,7 @@ module QA Resource::Repository::ProjectPush.fabricate! do |push| push.project = second_fork_project + push.user = user push.file_name = 'new_file' push.file_content = '# This is a new file' push.commit_message = 'Add new file' diff --git a/spec/controllers/concerns/analytics/cycle_analytics/value_stream_actions_spec.rb b/spec/controllers/concerns/analytics/cycle_analytics/value_stream_actions_spec.rb index a6a0f2c11b4..28a0ce437de 100644 --- a/spec/controllers/concerns/analytics/cycle_analytics/value_stream_actions_spec.rb +++ b/spec/controllers/concerns/analytics/cycle_analytics/value_stream_actions_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Analytics::CycleAnalytics::ValueStreamActions, type: :controller, - feature_category: :planning_analytics do + feature_category: :team_planning do subject(:controller) do Class.new(ApplicationController) do include Analytics::CycleAnalytics::ValueStreamActions diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 35efcb664c0..fe4b80e12fe 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -532,7 +532,7 @@ RSpec.describe Groups::GroupMembersController do it 'is successful' do delete :destroy, params: { group_id: group, id: membership } - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:see_other) end end diff --git a/spec/controllers/jira_connect/subscriptions_controller_spec.rb b/spec/controllers/jira_connect/subscriptions_controller_spec.rb index e9c94f09c99..7aa45ae379c 100644 --- a/spec/controllers/jira_connect/subscriptions_controller_spec.rb +++ b/spec/controllers/jira_connect/subscriptions_controller_spec.rb @@ -56,26 +56,6 @@ RSpec.describe JiraConnect::SubscriptionsController do expect(json_response).to include('subscriptions_path' => jira_connect_subscriptions_path) end - context 'when not signed in to GitLab' do - it 'contains a login path' do - expect(json_response).to include('login_path' => jira_connect_users_path) - end - end - - context 'when signed in to GitLab' do - let(:user) { create(:user) } - - before do - sign_in(user) - - get :index, params: { jwt: jwt } - end - - it 'does not contain a login path' do - expect(json_response).to include('login_path' => nil) - end - end - context 'with context qsh' do # The JSON endpoint will be requested by frontend using a JWT that Atlassian provides via Javascript. # This JWT will likely use a context-qsh because Atlassian don't know for which endpoint it will be used. diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 6929d0e28e2..5f606b1f4f3 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -243,7 +243,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do get :new, params: { namespace_id: project.namespace, project_id: project } expect(assigns(:issue)).to be_a_new(Issue) - expect(assigns(:issue).issue_type).to eq('issue') + expect(assigns(:issue).work_item_type.base_type).to eq('issue') end where(:conf_value, :conf_result) do @@ -280,7 +280,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do get :new, params: { namespace_id: project.namespace, project_id: project, issue: { issue_type: issue_type } } end - subject { assigns(:issue).issue_type } + subject { assigns(:issue).work_item_type.base_type } it { is_expected.to eq('issue') } @@ -619,7 +619,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do subject expect(response).to have_gitlab_http_status(:ok) - expect(issue.reload.issue_type).to eql('incident') + expect(issue.reload.work_item_type.base_type).to eq('incident') end end @@ -1075,7 +1075,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it 'sets the correct issue_type' do issue = post_new_issue(issue_type: 'incident') - expect(issue.issue_type).to eq('incident') expect(issue.work_item_type.base_type).to eq('incident') end end @@ -1084,7 +1083,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it 'defaults to issue type' do issue = post_new_issue(issue_type: 'task') - expect(issue.issue_type).to eq('issue') expect(issue.work_item_type.base_type).to eq('issue') end end @@ -1093,7 +1091,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it 'defaults to issue type' do issue = post_new_issue(issue_type: 'objective') - expect(issue.issue_type).to eq('issue') expect(issue.work_item_type.base_type).to eq('issue') end end @@ -1102,7 +1099,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it 'defaults to issue type' do issue = post_new_issue(issue_type: 'key_result') - expect(issue.issue_type).to eq('issue') expect(issue.work_item_type.base_type).to eq('issue') end end @@ -1152,7 +1148,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do expect(issue).to be_a(Issue) expect(issue.persisted?).to eq(true) - expect(issue.issue_type).to eq('issue') expect(issue.work_item_type.base_type).to eq('issue') end @@ -1403,7 +1398,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do context 'setting issue type' do let(:issue_type) { 'issue' } - subject { post_new_issue(issue_type: issue_type)&.issue_type } + subject { post_new_issue(issue_type: issue_type)&.work_item_type&.base_type } it { is_expected.to eq('issue') } @@ -1468,7 +1463,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it "deletes the issue" do delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid, destroy_confirm: true } - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:see_other) expect(controller).to set_flash[:notice].to(/The issue was successfully deleted\./) end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index fd77d07705d..f78d50bba24 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -851,7 +851,7 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review it "deletes the merge request" do delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: merge_request.iid, destroy_confirm: true } - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:see_other) expect(controller).to set_flash[:notice].to(/The merge request was successfully deleted\./) end diff --git a/spec/frontend/ml/experiment_tracking/components/model_experiments_header_spec.js b/spec/frontend/ml/experiment_tracking/components/model_experiments_header_spec.js new file mode 100644 index 00000000000..0794d4747b3 --- /dev/null +++ b/spec/frontend/ml/experiment_tracking/components/model_experiments_header_spec.js @@ -0,0 +1,35 @@ +import { GlBadge } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import ModelExperimentsHeader from '~/ml/experiment_tracking/components/model_experiments_header.vue'; + +describe('ml/experiment_tracking/components/model_experiments_header.vue', () => { + let wrapper; + + const createWrapper = () => { + wrapper = shallowMount(ModelExperimentsHeader, { + propsData: { pageTitle: 'Some Title' }, + slots: { + default: 'Slot content', + }, + }); + }; + + beforeEach(createWrapper); + + const findBadge = () => wrapper.findComponent(GlBadge); + const findTitle = () => wrapper.find('h3'); + + it('renders title', () => { + expect(findTitle().text()).toBe('Some Title'); + }); + + it('link points to documentation', () => { + expect(findBadge().attributes().href).toBe( + '/help/user/project/ml/experiment_tracking/index.md', + ); + }); + + it('renders slots', () => { + expect(wrapper.html()).toContain('Slot content'); + }); +}); diff --git a/spec/frontend/ml/experiment_tracking/routes/candidates/show/__snapshots__/ml_candidates_show_spec.js.snap b/spec/frontend/ml/experiment_tracking/routes/candidates/show/__snapshots__/ml_candidates_show_spec.js.snap index e1ea3a351ec..0d2615e3b80 100644 --- a/spec/frontend/ml/experiment_tracking/routes/candidates/show/__snapshots__/ml_candidates_show_spec.js.snap +++ b/spec/frontend/ml/experiment_tracking/routes/candidates/show/__snapshots__/ml_candidates_show_spec.js.snap @@ -2,33 +2,16 @@ exports[`MlCandidatesShow renders correctly 1`] = ` <div> - <incubation-alert-stub - featurename="Machine learning experiment tracking" - linktofeedbackissue="https://gitlab.com/gitlab-org/gitlab/-/issues/381660" - /> - - <div - class="detail-page-header gl-flex-wrap" + <model-experiments-header-stub + pagetitle="Model candidate details" > - <div - class="detail-page-header-body" - > - <h1 - class="page-title gl-font-size-h-display flex-fill" - > - - Model candidate details - - </h1> - - <delete-button-stub - actionprimarytext="Delete candidate" - deleteconfirmationtext="Deleting this candidate will delete the associated parameters, metrics, and metadata." - deletepath="path_to_candidate" - modaltitle="Delete candidate?" - /> - </div> - </div> + <delete-button-stub + actionprimarytext="Delete candidate" + deleteconfirmationtext="Deleting this candidate will delete the associated parameters, metrics, and metadata." + deletepath="path_to_candidate" + modaltitle="Delete candidate?" + /> + </model-experiments-header-stub> <table class="candidate-details gl-w-full" diff --git a/spec/frontend/ml/experiment_tracking/routes/candidates/show/ml_candidates_show_spec.js b/spec/frontend/ml/experiment_tracking/routes/candidates/show/ml_candidates_show_spec.js index b2a16b7ae78..d7044cbcd9b 100644 --- a/spec/frontend/ml/experiment_tracking/routes/candidates/show/ml_candidates_show_spec.js +++ b/spec/frontend/ml/experiment_tracking/routes/candidates/show/ml_candidates_show_spec.js @@ -1,7 +1,8 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import MlCandidatesShow from '~/ml/experiment_tracking/routes/candidates/show'; +import { TITLE_LABEL } from '~/ml/experiment_tracking/routes/candidates/show/translations'; import DeleteButton from '~/ml/experiment_tracking/components/delete_button.vue'; -import IncubationAlert from '~/vue_shared/components/incubation/incubation_alert.vue'; +import ModelExperimentsHeader from '~/ml/experiment_tracking/components/model_experiments_header.vue'; describe('MlCandidatesShow', () => { let wrapper; @@ -36,12 +37,8 @@ describe('MlCandidatesShow', () => { beforeEach(createWrapper); - const findAlert = () => wrapper.findComponent(IncubationAlert); const findDeleteButton = () => wrapper.findComponent(DeleteButton); - - it('shows incubation warning', () => { - expect(findAlert().exists()).toBe(true); - }); + const findHeader = () => wrapper.findComponent(ModelExperimentsHeader); it('shows delete button', () => { expect(findDeleteButton().exists()).toBe(true); @@ -51,6 +48,10 @@ describe('MlCandidatesShow', () => { expect(findDeleteButton().props('deletePath')).toBe('path_to_candidate'); }); + it('passes the right title', () => { + expect(findHeader().props('pageTitle')).toBe(TITLE_LABEL); + }); + it('renders correctly', () => { expect(wrapper.element).toMatchSnapshot(); }); diff --git a/spec/frontend/shortcuts_spec.js b/spec/frontend/shortcuts_spec.js index e72de11d921..88ad9204d08 100644 --- a/spec/frontend/shortcuts_spec.js +++ b/spec/frontend/shortcuts_spec.js @@ -23,8 +23,6 @@ describe('Shortcuts', () => { new Shortcuts(); // eslint-disable-line no-new new MarkdownPreview(); // eslint-disable-line no-new - jest.spyOn(document.querySelector('.js-new-note-form .js-md-preview-button'), 'focus'); - jest.spyOn(document.querySelector('.edit-note .js-md-preview-button'), 'focus'); jest.spyOn(document.querySelector('#search'), 'focus'); jest.spyOn(Mousetrap.prototype, 'stopCallback'); @@ -36,31 +34,6 @@ describe('Shortcuts', () => { resetHTMLFixture(); }); - describe('toggleMarkdownPreview', () => { - it('focuses preview button in form', () => { - Shortcuts.toggleMarkdownPreview( - createEvent('KeyboardEvent', document.querySelector('.js-new-note-form .js-note-text')), - ); - - expect( - document.querySelector('.js-new-note-form .js-md-preview-button').focus, - ).toHaveBeenCalled(); - }); - - it('focuses preview button inside edit comment form', () => { - document.querySelector('.js-note-edit').click(); - - Shortcuts.toggleMarkdownPreview( - createEvent('KeyboardEvent', document.querySelector('.edit-note .js-note-text')), - ); - - expect( - document.querySelector('.js-new-note-form .js-md-preview-button').focus, - ).not.toHaveBeenCalled(); - expect(document.querySelector('.edit-note .js-md-preview-button').focus).toHaveBeenCalled(); - }); - }); - describe('markdown shortcuts', () => { let shortcutElements; diff --git a/spec/frontend/vue_merge_request_widget/components/states/merge_failed_pipeline_confirmation_dialog_spec.js b/spec/frontend/vue_merge_request_widget/components/states/merge_failed_pipeline_confirmation_dialog_spec.js index 7d471b91c37..d321ff6e668 100644 --- a/spec/frontend/vue_merge_request_widget/components/states/merge_failed_pipeline_confirmation_dialog_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/states/merge_failed_pipeline_confirmation_dialog_spec.js @@ -3,6 +3,8 @@ import MergeFailedPipelineConfirmationDialog from '~/vue_merge_request_widget/co import { trimText } from 'helpers/text_helper'; describe('MergeFailedPipelineConfirmationDialog', () => { + const mockModalHide = jest.fn(); + let wrapper; const GlModal = { @@ -13,7 +15,7 @@ describe('MergeFailedPipelineConfirmationDialog', () => { </div> `, methods: { - hide: jest.fn(), + hide: mockModalHide, }, }; @@ -37,6 +39,10 @@ describe('MergeFailedPipelineConfirmationDialog', () => { createComponent(); }); + afterEach(() => { + mockModalHide.mockReset(); + }); + it('should render informational text explaining why merging immediately can be dangerous', () => { expect(trimText(wrapper.text())).toContain( 'The latest pipeline for this merge request did not succeed. The latest changes are unverified. Are you sure you want to attempt to merge?', @@ -50,12 +56,10 @@ describe('MergeFailedPipelineConfirmationDialog', () => { }); it('when the cancel button is clicked should emit cancel and call hide', () => { - jest.spyOn(findModal().vm, 'hide'); - findCancelBtn().vm.$emit('click'); expect(wrapper.emitted('cancel')).toHaveLength(1); - expect(findModal().vm.hide).toHaveBeenCalled(); + expect(mockModalHide).toHaveBeenCalled(); }); it('should emit cancel when the hide event is emitted', () => { diff --git a/spec/graphql/types/issue_type_spec.rb b/spec/graphql/types/issue_type_spec.rb index 87f99878a4d..a9fe85ac62f 100644 --- a/spec/graphql/types/issue_type_spec.rb +++ b/spec/graphql/types/issue_type_spec.rb @@ -266,7 +266,7 @@ RSpec.describe GitlabSchema.types['Issue'] do context 'for an incident' do before do issue.update!( - issue_type: Issue.issue_types[:incident], + issue_type: WorkItems::Type.base_types[:incident], work_item_type: WorkItems::Type.default_by_type(:incident) ) end @@ -280,44 +280,4 @@ RSpec.describe GitlabSchema.types['Issue'] do end end end - - describe 'type' do - let_it_be(:issue) { create(:issue, project: project) } - - let(:query) do - %( - query { - issue(id: "#{issue.to_gid}") { - type - } - } - ) - end - - subject(:execute) { GitlabSchema.execute(query, context: { current_user: user }).as_json } - - context 'when the issue_type_uses_work_item_types_table feature flag is enabled' do - it 'gets the type field from the work_item_types table' do - expect_next_instance_of(::IssuePresenter) do |presented_issue| - expect(presented_issue).to receive_message_chain(:work_item_type, :base_type) - end - - execute - end - end - - context 'when the issue_type_uses_work_item_types_table feature flag is disabled' do - before do - stub_feature_flags(issue_type_uses_work_item_types_table: false) - end - - it 'does not get the type field from the work_item_types table' do - expect_next_instance_of(::IssuePresenter) do |presented_issue| - expect(presented_issue).not_to receive(:work_item_type) - end - - execute - end - end - end end diff --git a/spec/helpers/integrations_helper_spec.rb b/spec/helpers/integrations_helper_spec.rb index 8be847e1c6c..4f1e6c86fea 100644 --- a/spec/helpers/integrations_helper_spec.rb +++ b/spec/helpers/integrations_helper_spec.rb @@ -170,13 +170,13 @@ RSpec.describe IntegrationsHelper do end it "return the correct i18n issue type" do - expect(described_class.integration_issue_type(issue.issue_type)).to eq(expected_i18n_issue_type) + expect(described_class.integration_issue_type(issue.work_item_type.base_type)).to eq(expected_i18n_issue_type) end end it "only consider these enumeration values are valid" do expected_valid_types = %w[issue incident test_case requirement task objective key_result] - expect(Issue.issue_types.keys).to contain_exactly(*expected_valid_types) + expect(WorkItems::Type.base_types.keys).to contain_exactly(*expected_valid_types) end end diff --git a/spec/helpers/issue_type_helper_spec.rb b/spec/helpers/issue_type_helper_spec.rb deleted file mode 100644 index 7984eaeb5da..00000000000 --- a/spec/helpers/issue_type_helper_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe IssueTypeHelper, feature_category: :team_planning do - describe '.issue_type_for' do - let(:input_issue_type) { :incident } - let(:issue) { build(:issue, input_issue_type) } - - subject(:issue_type) { helper.issue_type_for(issue) } - - context 'when issue is nil' do - let(:issue) { nil } - - it { is_expected.to be_nil } - end - - context 'when issue_type_uses_work_item_types_table feature flag is enabled' do - it 'gets type from the work_item_types table' do - expect(issue).to receive(:work_item_type).and_call_original - expect(issue_type).to eq(input_issue_type.to_s) - end - end - - context 'when issue_type_uses_work_item_types_table feature flag is disabled' do - before do - stub_feature_flags(issue_type_uses_work_item_types_table: false) - end - - it 'gets type from the issue_type column' do - expect(issue).to receive(:issue_type).and_call_original - expect(issue_type).to eq(input_issue_type.to_s) - end - end - end -end diff --git a/spec/lib/banzai/filter/references/reference_cache_spec.rb b/spec/lib/banzai/filter/references/reference_cache_spec.rb index 7e5ca00f118..577e4471433 100644 --- a/spec/lib/banzai/filter/references/reference_cache_spec.rb +++ b/spec/lib/banzai/filter/references/reference_cache_spec.rb @@ -76,11 +76,11 @@ RSpec.describe Banzai::Filter::References::ReferenceCache, feature_category: :te cache_single.load_records_per_parent end.count - expect(control_count).to eq 2 + expect(control_count).to eq 3 # Since this is an issue filter that is not batching issue queries # across projects, we have to account for that. # 1 for original issue, 2 for second route/project, 1 for other issue - max_count = control_count + 3 + max_count = control_count + 4 expect do cache.load_references_per_parent(filter.nodes) diff --git a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb index d0b85a1d043..58d6b9b9a2c 100644 --- a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Banzai::Pipeline::GfmPipeline, feature_category: :team_planning do describe 'integration between parsing regular and external issue references' do - let(:project) { create(:project, :with_redmine_integration, :public) } + let_it_be(:project) { create(:project, :with_redmine_integration, :public) } context 'when internal issue tracker is enabled' do context 'when shorthand pattern #ISSUE_ID is used' do diff --git a/spec/lib/gitlab/database/migrations/runner_backoff/active_record_mixin_spec.rb b/spec/lib/gitlab/database/migrations/runner_backoff/active_record_mixin_spec.rb new file mode 100644 index 00000000000..ddf11598d21 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/runner_backoff/active_record_mixin_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::RunnerBackoff::ActiveRecordMixin, feature_category: :database do + let(:migration_class) { Gitlab::Database::Migration[2.1] } + + describe described_class::ActiveRecordMigrationProxyRunnerBackoff do + let(:migration) { instance_double(migration_class) } + + let(:class_def) do + Class.new do + attr_reader :migration + + def initialize(migration) + @migration = migration + end + end.prepend(described_class) + end + + describe '#enable_runner_backoff?' do + subject { class_def.new(migration).enable_runner_backoff? } + + it 'delegates to #migration' do + expect(migration).to receive(:enable_runner_backoff?).and_return(true) + + expect(subject).to eq(true) + end + + it 'returns false if migration does not implement it' do + expect(migration).to receive(:respond_to?).with(:enable_runner_backoff?).and_return(false) + + expect(subject).to eq(false) + end + end + end + + describe described_class::ActiveRecordMigratorRunnerBackoff do + let(:class_def) do + Class.new do + attr_reader :receiver + + def initialize(receiver) + @receiver = receiver + end + + def execute_migration_in_transaction(migration) + receiver.execute_migration_in_transaction(migration) + end + end.prepend(described_class) + end + + let(:receiver) { instance_double(ActiveRecord::Migrator, 'receiver') } + + subject { class_def.new(receiver) } + + before do + allow(migration).to receive(:name).and_return('TestClass') + allow(receiver).to receive(:execute_migration_in_transaction) + end + + context 'with runner backoff disabled' do + let(:migration) { instance_double(migration_class, enable_runner_backoff?: false) } + + it 'calls super method' do + expect(receiver).to receive(:execute_migration_in_transaction).with(migration) + + subject.execute_migration_in_transaction(migration) + end + end + + context 'with runner backoff enabled' do + let(:migration) { instance_double(migration_class, enable_runner_backoff?: true) } + + it 'calls super method' do + expect(Gitlab::Database::Migrations::RunnerBackoff::Communicator) + .to receive(:execute_with_lock).with(migration).and_call_original + + expect(receiver).to receive(:execute_migration_in_transaction) + .with(migration) + + subject.execute_migration_in_transaction(migration) + end + end + end + + describe '.patch!' do + subject { described_class.patch! } + + it 'patches MigrationProxy' do + expect(ActiveRecord::MigrationProxy) + .to receive(:prepend) + .with(described_class::ActiveRecordMigrationProxyRunnerBackoff) + + subject + end + + it 'patches Migrator' do + expect(ActiveRecord::Migrator) + .to receive(:prepend) + .with(described_class::ActiveRecordMigratorRunnerBackoff) + + subject + end + end +end diff --git a/spec/lib/gitlab/database/migrations/runner_backoff/communicator_spec.rb b/spec/lib/gitlab/database/migrations/runner_backoff/communicator_spec.rb new file mode 100644 index 00000000000..cfc3fb398e2 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/runner_backoff/communicator_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::RunnerBackoff::Communicator, :clean_gitlab_redis_shared_state, feature_category: :database do + let(:migration) { instance_double(Gitlab::Database::Migration[2.1], name: 'TestClass') } + + describe '.execute_with_lock' do + it 'delegates to a new instance object' do + expect_next_instance_of(described_class, migration) do |communicator| + expect(communicator).to receive(:execute_with_lock).and_call_original + end + + expect { |b| described_class.execute_with_lock(migration, &b) }.to yield_control + end + end + + describe '.backoff_runner?' do + subject { described_class.backoff_runner? } + + it { is_expected.to be_falsey } + + it 'is true when the lock is held' do + described_class.execute_with_lock(migration) do + is_expected.to be_truthy + end + end + + it 'reads from Redis' do + recorder = RedisCommands::Recorder.new { subject } + expect(recorder.log).to include([:exists, 'gitlab:exclusive_lease:gitlab/database/migration/runner/backoff']) + end + + context 'with runner_migrations_backoff disabled' do + before do + stub_feature_flags(runner_migrations_backoff: false) + end + + it 'is false when the lock is held' do + described_class.execute_with_lock(migration) do + is_expected.to be_falsey + end + end + end + end + + describe '#execute_with_lock' do + include ExclusiveLeaseHelpers + + let(:communicator) { described_class.new(migration) } + let!(:lease) { stub_exclusive_lease(described_class::KEY, :uuid, timeout: described_class::EXPIRY) } + + it { expect { |b| communicator.execute_with_lock(&b) }.to yield_control } + + it 'raises error if it can not set the key' do + expect(lease).to receive(:try_obtain).ordered.and_return(false) + + expect { communicator.execute_with_lock { 1 / 0 } }.to raise_error 'Could not set backoff key' + end + + it 'removes the lease after executing the migration' do + expect(lease).to receive(:try_obtain).ordered.and_return(true) + expect(lease).to receive(:cancel).ordered.and_return(true) + + expect { communicator.execute_with_lock }.not_to raise_error + end + + context 'with logger' do + let(:logger) { instance_double(Gitlab::AppLogger) } + let(:communicator) { described_class.new(migration, logger: logger) } + + it 'logs messages around execution' do + expect(logger).to receive(:info).ordered + .with({ class: 'TestClass', message: 'Executing migration with Runner backoff' }) + expect(logger).to receive(:info).ordered + .with({ class: 'TestClass', message: 'Runner backoff key is set' }) + expect(logger).to receive(:info).ordered + .with({ class: 'TestClass', message: 'Runner backoff key was removed' }) + + communicator.execute_with_lock + end + end + end +end diff --git a/spec/lib/gitlab/database/migrations/runner_backoff/migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/runner_backoff/migration_helpers_spec.rb new file mode 100644 index 00000000000..9eefc34a7cc --- /dev/null +++ b/spec/lib/gitlab/database/migrations/runner_backoff/migration_helpers_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::RunnerBackoff::MigrationHelpers, feature_category: :database do + let(:class_def) do + Class.new.prepend(described_class) + end + + describe '.enable_runner_backoff!' do + it 'sets the flag' do + expect { class_def.enable_runner_backoff! } + .to change { class_def.enable_runner_backoff? } + .from(false).to(true) + end + end + + describe '.enable_runner_backoff?' do + subject { class_def.enable_runner_backoff? } + + it { is_expected.to be_falsy } + + it 'returns true if the flag is set' do + class_def.enable_runner_backoff! + + is_expected.to be_truthy + end + end + + describe '#enable_runner_backoff?' do + subject { class_def.new.enable_runner_backoff? } + + it { is_expected.to be_falsy } + + it 'returns true if the flag is set' do + class_def.enable_runner_backoff! + + is_expected.to be_truthy + end + end +end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 26af9d5d5b8..e5f8918f7bb 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -RSpec.describe Gitlab::Git::Commit do +RSpec.describe Gitlab::Git::Commit, feature_category: :source_code_management do let(:repository) { create(:project, :repository).repository.raw } let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } @@ -61,10 +61,41 @@ RSpec.describe Gitlab::Git::Commit do context 'body_size greater than threshold' do let(:body_size) { described_class::MAX_COMMIT_MESSAGE_DISPLAY_SIZE + 1 } - it 'returns the suject plus a notice about message size' do + it 'returns the subject plus a notice about message size' do expect(commit.safe_message).to eq("My commit\n\n--commit message is too big") end end + + context "large commit message" do + let(:user) { create(:user) } + let(:sha) { create_commit_with_large_message } + let(:commit) { repository.commit(sha) } + + def create_commit_with_large_message + repository.commit_files( + user, + branch_name: 'HEAD', + message: "Repeat " * 10 * 1024, + actions: [] + ).newrev + end + + it 'returns a String' do + # When #message is called, its encoding is forced from + # ASCII-8BIT to UTF-8, and the method returns a + # string. Calling #message again may cause BatchLoader to + # return since the encoding has been modified to UTF-8, and + # the encoding helper will return the original object unmodified. + # + # To ensure #fetch_body_from_gitaly returns a String, invoke + # #to_s. In the test below, do a strict type check to ensure + # that a String is always returned. Note that the Rspec + # matcher be_instance_of(String) appears to evaluate the + # BatchLoader result, so we have to do a strict comparison + # here. + 2.times { expect(String === commit.message).to be true } + end + end end end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 0c6a832a730..62fcb4821fc 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -193,7 +193,7 @@ RSpec.describe Gitlab::ReferenceExtractor do end context 'with an external issue tracker' do - let(:project) { create(:project, :with_jira_integration) } + let_it_be(:project) { create(:project, :with_jira_integration) } let(:issue) { create(:issue, project: project) } context 'when GitLab issues are enabled' do diff --git a/spec/lib/object_storage/direct_upload_spec.rb b/spec/lib/object_storage/direct_upload_spec.rb index 82eede96deb..4fcc0e3f306 100644 --- a/spec/lib/object_storage/direct_upload_spec.rb +++ b/spec/lib/object_storage/direct_upload_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ObjectStorage::DirectUpload do +RSpec.describe ObjectStorage::DirectUpload, feature_category: :shared do let(:region) { 'us-east-1' } let(:path_style) { false } let(:use_iam_profile) { false } diff --git a/spec/lib/object_storage/pending_direct_upload_spec.rb b/spec/lib/object_storage/pending_direct_upload_spec.rb new file mode 100644 index 00000000000..af08b9c8188 --- /dev/null +++ b/spec/lib/object_storage/pending_direct_upload_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ObjectStorage::PendingDirectUpload, :clean_gitlab_redis_shared_state, feature_category: :shared do + let(:location_identifier) { :artifacts } + let(:path) { 'some/path/123' } + + describe '.prepare' do + it 'creates a redis entry for the given location identifier and path' do + freeze_time do + described_class.prepare(location_identifier, path) + + ::Gitlab::Redis::SharedState.with do |redis| + key = described_class.key(location_identifier, path) + expect(redis.hget('pending_direct_uploads', key)).to eq(Time.current.utc.to_i.to_s) + end + end + end + end + + describe '.exists?' do + let(:path) { 'some/path/123' } + + subject { described_class.exists?(given_identifier, given_path) } + + before do + described_class.prepare(location_identifier, path) + end + + context 'when there is a matching redis entry for the given path under the location identifier' do + let(:given_identifier) { location_identifier } + let(:given_path) { path } + + it { is_expected.to eq(true) } + end + + context 'when there is a matching redis entry for the given path under a different location identifier' do + let(:given_identifier) { :uploads } + let(:given_path) { path } + + it { is_expected.to eq(false) } + end + + context 'when there is no matching redis entry for the given path under the location identifier' do + let(:given_identifier) { location_identifier } + let(:given_path) { 'wrong/path/123' } + + it { is_expected.to eq(false) } + end + end + + describe '.complete' do + it 'deletes the redis entry for the given path' do + described_class.prepare(location_identifier, path) + + expect(described_class.exists?(location_identifier, path)).to eq(true) + + described_class.complete(location_identifier, path) + + expect(described_class.exists?(location_identifier, path)).to eq(false) + end + end + + describe '.key' do + subject { described_class.key(location_identifier, path) } + + it { is_expected.to eq("#{location_identifier}:#{path}") } + end +end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 7bbbd10ec8d..d9e53fb7e9a 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -225,7 +225,7 @@ RSpec.describe Commit, 'Mentionable' do end context 'with external issue tracker' do - let(:project) { create(:project, :with_jira_integration, :repository) } + let_it_be(:project) { create(:project, :with_jira_integration, :repository) } it 'is true if external issues referenced' do allow(commit.raw).to receive(:message).and_return 'JIRA-123' diff --git a/spec/models/concerns/prometheus_adapter_spec.rb b/spec/models/concerns/prometheus_adapter_spec.rb index d3a44ac8403..31ab8c23a84 100644 --- a/spec/models/concerns/prometheus_adapter_spec.rb +++ b/spec/models/concerns/prometheus_adapter_spec.rb @@ -6,7 +6,7 @@ RSpec.describe PrometheusAdapter, :use_clean_rails_memory_store_caching do include PrometheusHelpers include ReactiveCachingHelpers - let(:project) { create(:project, :with_prometheus_integration) } + let_it_be(:project) { create(:project, :with_prometheus_integration) } let(:integration) { project.prometheus_integration } let(:described_class) do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index dfb7de34993..f9a391e9054 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1387,7 +1387,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ context 'when the environment is available' do context 'with a deployment service' do - let(:project) { create(:project, :with_prometheus_integration, :repository) } + let_it_be(:project) { create(:project, :with_prometheus_integration, :repository) } context 'and a deployment' do let!(:deployment) { create(:deployment, environment: environment) } @@ -1460,7 +1460,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end context 'when the environment is unavailable' do - let(:project) { create(:project, :with_prometheus_integration) } + let_it_be(:project) { create(:project, :with_prometheus_integration) } before do environment.stop @@ -1487,7 +1487,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end describe '#metrics' do - let(:project) { create(:project, :with_prometheus_integration) } + let_it_be(:project) { create(:project, :with_prometheus_integration) } subject { environment.metrics } @@ -1523,7 +1523,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end describe '#additional_metrics' do - let(:project) { create(:project, :with_prometheus_integration) } + let_it_be(:project) { create(:project, :with_prometheus_integration) } let(:metric_params) { [] } subject { environment.additional_metrics(*metric_params) } diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index a533079f906..8aa9b12c4f0 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -284,7 +284,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, context 'cluster belongs to projects group' do let_it_be(:group) { create(:group) } - let(:project) { create(:project, :with_prometheus_integration, group: group) } + let_it_be(:project) { create(:project, :with_prometheus_integration, group: group) } let_it_be(:cluster) { create(:cluster_for_group, groups: [group]) } it 'returns true' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 52593f024fa..ead15b0b640 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -316,6 +316,19 @@ RSpec.describe Issue, feature_category: :team_planning do issue.save! end.to raise_error(Issue::IssueTypeOutOfSyncError) end + + it 'uses attributes to compare both issue_type values' do + issue_type = WorkItems::Type.default_by_type(:issue) + issue = build(:issue, issue_type: :issue, work_item_type: issue_type) + + attributes = double(:attributes) + allow(issue).to receive(:attributes).and_return(attributes) + + expect(attributes).to receive(:[]).with('issue_type').twice.and_return('issue') + expect(issue_type).to receive(:base_type).and_call_original + + issue.save! + end end describe '#record_create_action' do @@ -1778,44 +1791,54 @@ RSpec.describe Issue, feature_category: :team_planning do end end - describe '#issue_type_supports?' do + describe '#issue_type' do let_it_be(:issue) { create(:issue) } - it 'raises error when feature is invalid' do - expect { issue.issue_type_supports?(:unkown_feature) }.to raise_error(ArgumentError) - end + context 'when the issue_type_uses_work_item_types_table feature flag is enabled' do + it 'gets the type field from the work_item_types table' do + expect(issue).to receive_message_chain(:work_item_type, :base_type) - context 'when issue_type_uses_work_item_types_table feature flag is disabled' do - before do - stub_feature_flags(issue_type_uses_work_item_types_table: false) + issue.issue_type end - it 'uses the issue_type column' do - expect(issue).to receive(:issue_type).and_call_original - expect(issue).not_to receive(:work_item_type).and_call_original + context 'when the issue is not persisted' do + it 'uses the default work item type' do + non_persisted_issue = build(:issue, work_item_type: nil) - issue.issue_type_supports?(:assignee) + expect(non_persisted_issue.issue_type).to eq(described_class::DEFAULT_ISSUE_TYPE.to_s) + end end end - context 'when issue_type_uses_work_item_types_table feature flag is enabled' do - it 'uses the work_item_types table' do - expect(issue).not_to receive(:issue_type).and_call_original - expect(issue).to receive(:work_item_type).and_call_original + context 'when the issue_type_uses_work_item_types_table feature flag is disabled' do + before do + stub_feature_flags(issue_type_uses_work_item_types_table: false) + end - issue.issue_type_supports?(:assignee) + it 'does not get the value from the work_item_types table' do + expect(issue).not_to receive(:work_item_type) + + issue.issue_type end context 'when the issue is not persisted' do it 'uses the default work item type' do - non_persisted_issue = build(:issue) + non_persisted_issue = build(:issue, work_item_type: nil) - expect(non_persisted_issue.issue_type_supports?(:assignee)).to be_truthy + expect(non_persisted_issue.issue_type).to eq(described_class::DEFAULT_ISSUE_TYPE.to_s) end end end end + describe '#issue_type_supports?' do + let_it_be(:issue) { create(:issue) } + + it 'raises error when feature is invalid' do + expect { issue.issue_type_supports?(:unkown_feature) }.to raise_error(ArgumentError) + end + end + describe '#supports_assignee?' do Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter::WIDGETS_FOR_TYPE.each_pair do |base_type, widgets| specify do diff --git a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb index 3d3d699542b..7f8450778b5 100644 --- a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb +++ b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb @@ -174,8 +174,21 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego expect(json_response['RemoteObject']).to have_key('StoreURL') expect(json_response['RemoteObject']).to have_key('DeleteURL') expect(json_response['RemoteObject']).to have_key('MultipartUpload') + expect(json_response['RemoteObject']['SkipDelete']).to eq(true) expect(json_response['MaximumSize']).not_to be_nil end + + context 'when ci_artifacts_upload_to_final_location flag is disabled' do + before do + stub_feature_flags(ci_artifacts_upload_to_final_location: false) + end + + it 'does not skip delete' do + subject + + expect(json_response['RemoteObject']['SkipDelete']).to eq(false) + end + end end context 'when direct upload is disabled' do @@ -374,29 +387,53 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let(:object) do fog_connection.directories.new(key: 'artifacts').files.create( # rubocop:disable Rails/SaveBang - key: 'tmp/uploads/12312300', + key: remote_path, body: 'content' ) end let(:file_upload) { fog_to_uploaded_file(object) } - before do - upload_artifacts(file_upload, headers_with_token, 'file.remote_id' => remote_id) - end + context 'when uploaded file has matching pending remote upload to its final location' do + let(:remote_path) { '12345/foo-bar-123' } + let(:object_remote_id) { remote_path } + let(:remote_id) { remote_path } + + before do + allow(JobArtifactUploader).to receive(:generate_final_store_path).and_return(remote_path) - context 'when valid remote_id is used' do - let(:remote_id) { '12312300' } + ObjectStorage::PendingDirectUpload.prepare( + JobArtifactUploader.storage_location_identifier, + remote_path + ) + + upload_artifacts(file_upload, headers_with_token, 'file.remote_id' => remote_path) + end it_behaves_like 'successful artifacts upload' end - context 'when invalid remote_id is used' do - let(:remote_id) { 'invalid id' } + context 'when uploaded file is uploaded to temporary location' do + let(:object_remote_id) { JobArtifactUploader.generate_remote_id } + let(:remote_path) { File.join(ObjectStorage::TMP_UPLOAD_PATH, object_remote_id) } + + before do + upload_artifacts(file_upload, headers_with_token, 'file.remote_id' => remote_id) + end + + context 'and matching temporary remote_id is used' do + let(:remote_id) { object_remote_id } + + it_behaves_like 'successful artifacts upload' + end + + context 'and invalid remote_id is used' do + let(:remote_id) { JobArtifactUploader.generate_remote_id } - it 'responds with bad request' do - expect(response).to have_gitlab_http_status(:internal_server_error) - expect(json_response['message']).to eq("Missing file") + it 'responds with internal server error' do + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(json_response['message']).to eq("Missing file") + end end end end diff --git a/spec/requests/api/graphql/mutations/issues/create_spec.rb b/spec/requests/api/graphql/mutations/issues/create_spec.rb index d2d2f0014d6..b5a9c549045 100644 --- a/spec/requests/api/graphql/mutations/issues/create_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/create_spec.rb @@ -66,7 +66,6 @@ RSpec.describe 'Create an issue', feature_category: :team_planning do created_issue = Issue.last expect(created_issue.work_item_type.base_type).to eq('task') - expect(created_issue.issue_type).to eq('task') end end diff --git a/spec/requests/api/graphql/mutations/work_items/convert_spec.rb b/spec/requests/api/graphql/mutations/work_items/convert_spec.rb index 4274287f077..42c6705e357 100644 --- a/spec/requests/api/graphql/mutations/work_items/convert_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/convert_spec.rb @@ -50,7 +50,6 @@ RSpec.describe "Converts a work item to a new type", feature_category: :team_pla end.to change { work_item.reload.work_item_type }.to(new_type) expect(response).to have_gitlab_http_status(:success) - expect(work_item.reload.issue_type).to eq('incident') expect(work_item.reload.work_item_type.base_type).to eq('incident') expect(mutation_response['workItem']).to include('id' => work_item.to_global_id.to_s) expect(work_item.reload.milestone).to be_nil diff --git a/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb b/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb index d2df3e8bda2..6a6ad1b14fd 100644 --- a/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb @@ -45,7 +45,6 @@ RSpec.describe "Create a work item from a task in a work item's description", fe expect(response).to have_gitlab_http_status(:success) expect(work_item.description).to eq("- [ ] #{created_work_item.to_reference}+") - expect(created_work_item.issue_type).to eq('task') expect(created_work_item.work_item_type.base_type).to eq('task') expect(created_work_item.work_item_parent).to eq(work_item) expect(created_work_item).to be_confidential diff --git a/spec/requests/api/graphql/mutations/work_items/create_spec.rb b/spec/requests/api/graphql/mutations/work_items/create_spec.rb index c576d4d286a..fca3c84e534 100644 --- a/spec/requests/api/graphql/mutations/work_items/create_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/create_spec.rb @@ -30,7 +30,6 @@ RSpec.describe 'Create a work item', feature_category: :team_planning do created_work_item = WorkItem.last expect(response).to have_gitlab_http_status(:success) - expect(created_work_item.issue_type).to eq('task') expect(created_work_item).to be_confidential expect(created_work_item.work_item_type.base_type).to eq('task') expect(mutation_response['workItem']).to include( diff --git a/spec/requests/jira_connect/users_controller_spec.rb b/spec/requests/jira_connect/users_controller_spec.rb deleted file mode 100644 index c02bd324708..00000000000 --- a/spec/requests/jira_connect/users_controller_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe JiraConnect::UsersController, feature_category: :integrations do - describe 'GET /-/jira_connect/users' do - let_it_be(:user) { create(:user) } - - before do - sign_in(user) - end - - context 'with a valid host' do - let(:return_to) { 'https://testcompany.atlassian.net/plugins/servlet/ac/gitlab-jira-connect-staging.gitlab.com/gitlab-configuration' } - - it 'includes a return url' do - get '/-/jira_connect/users', params: { return_to: return_to } - - expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to include('Return to GitLab') - end - end - - context 'with an invalid host' do - let(:return_to) { 'https://evil.com' } - - it 'does not include a return url' do - get '/-/jira_connect/users', params: { return_to: return_to } - - expect(response).to have_gitlab_http_status(:ok) - expect(response.body).not_to include('Return to GitLab') - end - end - - context 'with a script injected' do - let(:return_to) { 'javascript://test.atlassian.net/%250dalert(document.domain)' } - - it 'does not include a return url' do - get '/-/jira_connect/users', params: { return_to: return_to } - - expect(response).to have_gitlab_http_status(:ok) - expect(response.body).not_to include('Return to GitLab') - end - end - end -end diff --git a/spec/requests/projects/cycle_analytics_events_spec.rb b/spec/requests/projects/cycle_analytics_events_spec.rb index 3f9dd74c145..0adf0b525a9 100644 --- a/spec/requests/projects/cycle_analytics_events_spec.rb +++ b/spec/requests/projects/cycle_analytics_events_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'value stream analytics events', feature_category: :planning_analytics do +RSpec.describe 'value stream analytics events', feature_category: :team_planning do include CycleAnalyticsHelpers let(:user) { create(:user) } diff --git a/spec/serializers/issue_sidebar_basic_entity_spec.rb b/spec/serializers/issue_sidebar_basic_entity_spec.rb index f24e379ec67..d81d87f4060 100644 --- a/spec/serializers/issue_sidebar_basic_entity_spec.rb +++ b/spec/serializers/issue_sidebar_basic_entity_spec.rb @@ -45,7 +45,7 @@ RSpec.describe IssueSidebarBasicEntity do context 'for an incident issue' do before do issue.update!( - issue_type: Issue.issue_types[:incident], + issue_type: WorkItems::Type.base_types[:incident], work_item_type: WorkItems::Type.default_by_type(:incident) ) end diff --git a/spec/serializers/jira_connect/app_data_serializer_spec.rb b/spec/serializers/jira_connect/app_data_serializer_spec.rb index 9c10a8a54a1..1ade3dea6e7 100644 --- a/spec/serializers/jira_connect/app_data_serializer_spec.rb +++ b/spec/serializers/jira_connect/app_data_serializer_spec.rb @@ -4,12 +4,10 @@ require 'spec_helper' RSpec.describe JiraConnect::AppDataSerializer do describe '#as_json' do - subject(:app_data_json) { described_class.new(subscriptions, signed_in).as_json } + subject(:app_data_json) { described_class.new(subscriptions).as_json } let_it_be(:subscriptions) { create_list(:jira_connect_subscription, 2) } - let(:signed_in) { false } - it 'uses the subscription entity' do expect(JiraConnect::SubscriptionEntity).to receive(:represent).with(subscriptions) @@ -23,12 +21,5 @@ RSpec.describe JiraConnect::AppDataSerializer do end it { is_expected.to include(subscriptions_path: '/-/jira_connect/subscriptions') } - it { is_expected.to include(login_path: '/-/jira_connect/users') } - - context 'when signed in' do - let(:signed_in) { true } - - it { is_expected.to include(login_path: nil) } - end end end diff --git a/spec/services/ci/job_artifacts/create_service_spec.rb b/spec/services/ci/job_artifacts/create_service_spec.rb index 5d9f30c11eb..f71d7feb04a 100644 --- a/spec/services/ci/job_artifacts/create_service_spec.rb +++ b/spec/services/ci/job_artifacts/create_service_spec.rb @@ -78,11 +78,39 @@ RSpec.describe Ci::JobArtifacts::CreateService, :clean_gitlab_redis_shared_state context 'when object storage is enabled' do context 'and direct upload is enabled' do + let(:final_store_path) { '12/34/abc-123' } + before do stub_artifacts_object_storage(JobArtifactUploader, direct_upload: true) + allow(JobArtifactUploader).to receive(:generate_final_store_path).and_return(final_store_path) end - it_behaves_like 'uploading to temp location', :object_storage + it 'includes the authorize headers' do + expect(authorize[:status]).to eq(:success) + + expect(authorize[:headers][:RemoteObject][:ID]).to eq(final_store_path) + + # We are not testing the entire headers here because this is fully tested + # in workhorse_authorize's spec. We just want to confirm that it indeed used the final path + # by checking some indicators in the headers returned. + expect(authorize[:headers][:RemoteObject][:StoreURL]) + .to include(final_store_path) + + # We have to ensure to tell Workhorse to skip deleting the file after upload + # because we are uploading the file to its final location + expect(authorize[:headers][:RemoteObject][:SkipDelete]).to eq(true) + end + + it_behaves_like 'handling lsif artifact' + it_behaves_like 'validating requirements' + + context 'with ci_artifacts_upload_to_final_location feature flag disabled' do + before do + stub_feature_flags(ci_artifacts_upload_to_final_location: false) + end + + it_behaves_like 'uploading to temp location', :object_storage + end end context 'and direct upload is disabled' do @@ -401,7 +429,7 @@ RSpec.describe Ci::JobArtifacts::CreateService, :clean_gitlab_redis_shared_state end end - shared_examples_for 'handling remote uploads to temporary location' do + shared_examples_for 'handling uploads' do context 'when artifacts file is uploaded' do it 'creates a new job artifact' do expect { execute }.to change { Ci::JobArtifact.count }.by(1) @@ -469,7 +497,7 @@ RSpec.describe Ci::JobArtifacts::CreateService, :clean_gitlab_redis_shared_state let(:remote_id) { 'generated-remote-id-12345' } let(:remote_store_path) { ObjectStorage::TMP_UPLOAD_PATH } - it_behaves_like 'handling remote uploads to temporary location' + it_behaves_like 'handling uploads' it_behaves_like 'handling dotenv', :object_storage it_behaves_like 'handling object storage errors' it_behaves_like 'validating requirements' @@ -480,7 +508,7 @@ RSpec.describe Ci::JobArtifacts::CreateService, :clean_gitlab_redis_shared_state file_to_upload('spec/fixtures/ci_build_artifacts.zip', sha256: artifacts_sha256) end - it_behaves_like 'handling remote uploads to temporary location' + it_behaves_like 'handling uploads' it_behaves_like 'handling dotenv', :local_storage it_behaves_like 'validating requirements' end diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index fecfc3f3d64..8368a34caf0 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -175,31 +175,37 @@ RSpec.describe Issues::BuildService, feature_category: :team_planning do describe 'setting issue type' do context 'with a corresponding WorkItems::Type' do + let_it_be(:type_task) { WorkItems::Type.default_by_type(:task) } + let_it_be(:type_task_id) { type_task.id } let_it_be(:type_issue_id) { WorkItems::Type.default_issue_type.id } let_it_be(:type_incident_id) { WorkItems::Type.default_by_type(:incident).id } - - where(:issue_type, :current_user, :work_item_type_id, :resulting_issue_type) do - nil | ref(:guest) | ref(:type_issue_id) | 'issue' - 'issue' | ref(:guest) | ref(:type_issue_id) | 'issue' - 'incident' | ref(:guest) | ref(:type_issue_id) | 'issue' - 'incident' | ref(:reporter) | ref(:type_incident_id) | 'incident' + let(:combined_params) { { work_item_type: type_task, issue_type: 'issue' } } + let(:work_item_params) { { work_item_type_id: type_task_id } } + + where(:issue_params, :current_user, :work_item_type_id, :resulting_issue_type) do + { issue_type: nil } | ref(:guest) | ref(:type_issue_id) | 'issue' + { issue_type: 'issue' } | ref(:guest) | ref(:type_issue_id) | 'issue' + { issue_type: 'incident' } | ref(:guest) | ref(:type_issue_id) | 'issue' + { issue_type: 'incident' } | ref(:reporter) | ref(:type_incident_id) | 'incident' + ref(:combined_params) | ref(:reporter) | ref(:type_task_id) | 'task' + ref(:work_item_params) | ref(:reporter) | ref(:type_task_id) | 'task' # update once support for test_case is enabled - 'test_case' | ref(:guest) | ref(:type_issue_id) | 'issue' + { issue_type: 'test_case' } | ref(:guest) | ref(:type_issue_id) | 'issue' # update once support for requirement is enabled - 'requirement' | ref(:guest) | ref(:type_issue_id) | 'issue' - 'invalid' | ref(:guest) | ref(:type_issue_id) | 'issue' + { issue_type: 'requirement' } | ref(:guest) | ref(:type_issue_id) | 'issue' + { issue_type: 'invalid' } | ref(:guest) | ref(:type_issue_id) | 'issue' # ensure that we don't set a value which has a permission check but is an invalid issue type - 'project' | ref(:guest) | ref(:type_issue_id) | 'issue' + { issue_type: 'project' } | ref(:guest) | ref(:type_issue_id) | 'issue' end with_them do let(:user) { current_user } it 'builds an issue' do - issue = build_issue(issue_type: issue_type) + issue = build_issue(**issue_params) - expect(issue.issue_type).to eq(resulting_issue_type) expect(issue.work_item_type_id).to eq(work_item_type_id) + expect(issue.attributes['issue_type']).to eq(resulting_issue_type) end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 57d4525673e..d58a52dd35b 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -145,7 +145,6 @@ RSpec.describe Issues::CreateService, feature_category: :team_planning do expect(issue).to be_persisted expect(issue).to be_a(::Issue) expect(issue.work_item_type.base_type).to eq('issue') - expect(issue.issue_type).to eq('issue') end end @@ -686,7 +685,6 @@ RSpec.describe Issues::CreateService, feature_category: :team_planning do created_issue = Issue.last - expect(created_issue.issue_type).to eq('incident') expect(created_issue.work_item_type).to eq(WorkItems::Type.default_by_type('incident')) end end diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index 88fef1aa0b3..e7587e59a91 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -165,4 +165,16 @@ module StubObjectStorage </InitiateMultipartUploadResult> EOS end + + def stub_object_storage_multipart_init_with_final_store_path(full_path, upload_id = "upload_id") + stub_request(:post, %r{\A#{full_path}\?uploads\z}) + .to_return status: 200, body: <<-EOS.strip_heredoc + <?xml version="1.0" encoding="UTF-8"?> + <InitiateMultipartUploadResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/"> + <Bucket>example-bucket</Bucket> + <Key>example-object</Key> + <UploadId>#{upload_id}</UploadId> + </InitiateMultipartUploadResult> + EOS + end end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 4efcb914173..9f259101ffc 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -8639,7 +8639,6 @@ - './spec/requests/jira_connect/oauth_application_ids_controller_spec.rb' - './spec/requests/jira_connect/oauth_callbacks_controller_spec.rb' - './spec/requests/jira_connect/subscriptions_controller_spec.rb' -- './spec/requests/jira_connect/users_controller_spec.rb' - './spec/requests/jira_routing_spec.rb' - './spec/requests/jwks_controller_spec.rb' - './spec/requests/jwt_controller_spec.rb' diff --git a/spec/support/shared_examples/requests/api/graphql/issue_list_shared_examples.rb b/spec/support/shared_examples/requests/api/graphql/issue_list_shared_examples.rb index 6c8b792bf92..930c47dac52 100644 --- a/spec/support/shared_examples/requests/api/graphql/issue_list_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/graphql/issue_list_shared_examples.rb @@ -480,6 +480,7 @@ RSpec.shared_examples 'graphql issue list request spec' do context 'when fetching escalation status' do let_it_be(:escalation_status) { create(:incident_management_issuable_escalation_status, issue: issue_a) } + let_it_be(:incident_type) { WorkItems::Type.default_by_type(:incident) } let(:fields) do <<~QUERY @@ -491,7 +492,7 @@ RSpec.shared_examples 'graphql issue list request spec' do end before do - issue_a.update_columns(issue_type: Issue.issue_types[:incident]) + issue_a.update_columns(issue_type: WorkItems::Type.base_types[:incident], work_item_type_id: incident_type.id) end it 'returns the escalation status values' do diff --git a/spec/support/shared_examples/services/incident_shared_examples.rb b/spec/support/shared_examples/services/incident_shared_examples.rb index a87e7c1f801..db2b448f567 100644 --- a/spec/support/shared_examples/services/incident_shared_examples.rb +++ b/spec/support/shared_examples/services/incident_shared_examples.rb @@ -12,7 +12,6 @@ # include_examples 'incident issue' RSpec.shared_examples 'incident issue' do it 'has incident as issue type' do - expect(issue.issue_type).to eq('incident') expect(issue.work_item_type.base_type).to eq('incident') end end @@ -29,7 +28,6 @@ end # include_examples 'not an incident issue' RSpec.shared_examples 'not an incident issue' do it 'has not incident as issue type' do - expect(issue.issue_type).not_to eq('incident') expect(issue.work_item_type.base_type).not_to eq('incident') end end diff --git a/spec/tooling/danger/specs/feature_category_suggestion_spec.rb b/spec/tooling/danger/specs/feature_category_suggestion_spec.rb index 3956553f488..87eb20e5e50 100644 --- a/spec/tooling/danger/specs/feature_category_suggestion_spec.rb +++ b/spec/tooling/danger/specs/feature_category_suggestion_spec.rb @@ -27,7 +27,7 @@ RSpec.describe Tooling::Danger::Specs::FeatureCategorySuggestion, feature_catego [ " require 'spec_helper'", " \n", - " RSpec.describe Projects::SummaryController, feature_category: :planning_analytics do", + " RSpec.describe Projects::SummaryController, feature_category: :team_planning do", " end", "RSpec.describe Projects::SummaryController do", " let_it_be(:user) { create(:user) }", @@ -41,7 +41,7 @@ RSpec.describe Tooling::Danger::Specs::FeatureCategorySuggestion, feature_catego " end", " \n", "RSpec.describe Projects :aggregate_failures,", - " feature_category: planning_analytics do", + " feature_category :team_planning do", " \n", "RSpec.describe Epics :aggregate_failures,", " ee: true do", @@ -57,14 +57,14 @@ RSpec.describe Tooling::Danger::Specs::FeatureCategorySuggestion, feature_catego let(:changed_lines) do [ - "+ RSpec.describe Projects::SummaryController, feature_category: :planning_analytics do", + "+ RSpec.describe Projects::SummaryController, feature_category: :team_planning do", "+RSpec.describe Projects::SummaryController do", "+ let_it_be(:user) { create(:user) }", "- end", "+ describe 'GET \"time_summary\"' do", "+ RSpec.describe Projects::SummaryController do", "+RSpec.describe Projects :aggregate_failures,", - "+ feature_category: planning_analytics do", + "+ feature_category: :team_planning do", "+RSpec.describe Epics :aggregate_failures,", "+ ee: true do", "+RSpec.describe Issues :aggregate_failures,", diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index ef46803fc56..1566021934a 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -18,10 +18,12 @@ class Implementation < GitlabUploader end end -RSpec.describe ObjectStorage do +# TODO: Update feature_category once object storage group ownership has been determined. +RSpec.describe ObjectStorage, :clean_gitlab_redis_shared_state, feature_category: :shared do let(:uploader_class) { Implementation } let(:object) { build_stubbed(:user) } - let(:uploader) { uploader_class.new(object, :file) } + let(:file_column) { :file } + let(:uploader) { uploader_class.new(object, file_column) } describe '#object_store=' do before do @@ -103,6 +105,34 @@ RSpec.describe ObjectStorage do expect(subject).to eq("my/prefix/user/#{object.id}/filename") end end + + context 'when model has final path defined for the file column' do + # Changing this to `foo` to make a point that not all uploaders are mounted + # as `file`. They can be mounted as different names, for example, `avatar`. + let(:file_column) { :foo } + + before do + allow(object).to receive(:foo_final_path).and_return('123-final-path') + end + + it 'uses the final path instead' do + expect(subject).to eq('123-final-path') + end + + context 'and a bucket prefix is configured' do + before do + allow(uploader_class).to receive(:object_store_options) do + double( + bucket_prefix: 'my/prefix' + ) + end + end + + it 'uses the prefix with the final path' do + expect(subject).to eq("my/prefix/123-final-path") + end + end + end end end end @@ -494,8 +524,15 @@ RSpec.describe ObjectStorage do describe '.workhorse_authorize' do let(:has_length) { true } let(:maximum_size) { nil } + let(:use_final_store_path) { false } - subject { uploader_class.workhorse_authorize(has_length: has_length, maximum_size: maximum_size) } + subject do + uploader_class.workhorse_authorize( + has_length: has_length, + maximum_size: maximum_size, + use_final_store_path: use_final_store_path + ) + end context 'when FIPS is enabled', :fips_mode do it 'response enables FIPS' do @@ -528,18 +565,23 @@ RSpec.describe ObjectStorage do shared_examples 'uses remote storage' do it_behaves_like 'returns the maximum size given' do - it "returns remote store" do + it "returns remote object properties for a temporary upload" do is_expected.to have_key(:RemoteObject) expect(subject[:RemoteObject]).to have_key(:ID) expect(subject[:RemoteObject]).to include(Timeout: a_kind_of(Integer)) expect(subject[:RemoteObject][:Timeout]).to be(ObjectStorage::DirectUpload::TIMEOUT) - expect(subject[:RemoteObject]).to have_key(:GetURL) - expect(subject[:RemoteObject]).to have_key(:DeleteURL) - expect(subject[:RemoteObject]).to have_key(:StoreURL) - expect(subject[:RemoteObject][:GetURL]).to include(described_class::TMP_UPLOAD_PATH) - expect(subject[:RemoteObject][:DeleteURL]).to include(described_class::TMP_UPLOAD_PATH) - expect(subject[:RemoteObject][:StoreURL]).to include(described_class::TMP_UPLOAD_PATH) + + upload_path = File.join(described_class::TMP_UPLOAD_PATH, subject[:RemoteObject][:ID]) + + expect(subject[:RemoteObject][:GetURL]).to include(upload_path) + expect(subject[:RemoteObject][:DeleteURL]).to include(upload_path) + expect(subject[:RemoteObject][:StoreURL]).to include(upload_path) + expect(subject[:RemoteObject][:SkipDelete]).to eq(false) + + ::Gitlab::Redis::SharedState.with do |redis| + expect(redis.hlen(ObjectStorage::PendingDirectUpload::KEY)).to be_zero + end end end end @@ -551,12 +593,12 @@ RSpec.describe ObjectStorage do expect(subject[:RemoteObject]).to have_key(:MultipartUpload) expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:PartSize) - expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:PartURLs) - expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:CompleteURL) - expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:AbortURL) - expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(include(described_class::TMP_UPLOAD_PATH)) - expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to include(described_class::TMP_UPLOAD_PATH) - expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to include(described_class::TMP_UPLOAD_PATH) + + upload_path = File.join(described_class::TMP_UPLOAD_PATH, subject[:RemoteObject][:ID]) + + expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(include(upload_path)) + expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to include(upload_path) + expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to include(upload_path) end end end @@ -570,6 +612,80 @@ RSpec.describe ObjectStorage do end end + shared_examples 'handling object storage final upload path' do |multipart| + context 'when use_final_store_path is true' do + let(:use_final_store_path) { true } + let(:final_store_path) { File.join('@final', 'abc', '123', 'somefilename') } + let(:escaped_path) { escape_path(final_store_path) } + + before do + stub_object_storage_multipart_init_with_final_store_path("#{storage_url}#{final_store_path}") if multipart + + allow(uploader_class).to receive(:generate_final_store_path).and_return(final_store_path) + end + + it 'uses the full path instead of the temporary one' do + expect(subject[:RemoteObject][:ID]).to eq(final_store_path) + + expect(subject[:RemoteObject][:GetURL]).to include(escaped_path) + expect(subject[:RemoteObject][:StoreURL]).to include(escaped_path) + + if multipart + expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(include(escaped_path)) + expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to include(escaped_path) + expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to include(escaped_path) + end + + expect(subject[:RemoteObject][:SkipDelete]).to eq(true) + + expect( + ObjectStorage::PendingDirectUpload.exists?(uploader_class.storage_location_identifier, final_store_path) + ).to eq(true) + end + + context 'and bucket prefix is configured' do + let(:prefixed_final_store_path) { "my/prefix/#{final_store_path}" } + let(:escaped_path) { escape_path(prefixed_final_store_path) } + + before do + allow(uploader_class.object_store_options).to receive(:bucket_prefix).and_return('my/prefix') + + if multipart + stub_object_storage_multipart_init_with_final_store_path("#{storage_url}#{prefixed_final_store_path}") + end + end + + it 'sets the remote object ID to the final path without prefix' do + expect(subject[:RemoteObject][:ID]).to eq(final_store_path) + end + + it 'returns the final path with prefix' do + expect(subject[:RemoteObject][:GetURL]).to include(escaped_path) + expect(subject[:RemoteObject][:StoreURL]).to include(escaped_path) + + if multipart + expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(include(escaped_path)) + expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to include(escaped_path) + expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to include(escaped_path) + end + end + + it 'creates the pending upload entry without the prefix' do + is_expected.to have_key(:RemoteObject) + + expect( + ObjectStorage::PendingDirectUpload.exists?(uploader_class.storage_location_identifier, final_store_path) + ).to eq(true) + end + end + end + + def escape_path(path) + # This is what the private method Fog::AWS::Storage#object_to_path will do to the object name + Fog::AWS.escape(path).gsub('%2F', '/') + end + end + context 'when object storage is disabled' do before do allow(Gitlab.config.uploads.object_store).to receive(:enabled) { false } @@ -613,6 +729,8 @@ RSpec.describe ObjectStorage do expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) end end + + it_behaves_like 'handling object storage final upload path' end context 'for unknown length' do @@ -633,6 +751,8 @@ RSpec.describe ObjectStorage do expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to start_with(storage_url) end end + + it_behaves_like 'handling object storage final upload path', :multipart end end @@ -660,6 +780,8 @@ RSpec.describe ObjectStorage do expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) end end + + it_behaves_like 'handling object storage final upload path' end context 'for unknown length' do @@ -673,6 +795,8 @@ RSpec.describe ObjectStorage do expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) end end + + it_behaves_like 'handling object storage final upload path' end end @@ -701,6 +825,8 @@ RSpec.describe ObjectStorage do expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) end end + + it_behaves_like 'handling object storage final upload path' end context 'for unknown length' do @@ -721,6 +847,8 @@ RSpec.describe ObjectStorage do expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to start_with(storage_url) end end + + it_behaves_like 'handling object storage final upload path', :multipart end end end @@ -880,9 +1008,10 @@ RSpec.describe ObjectStorage do expect(uploader).to be_exists expect(uploader).to be_cached + expect(uploader.cache_only).to be_falsey expect(uploader).not_to be_file_storage expect(uploader.path).not_to be_nil - expect(uploader.path).not_to include('tmp/cache') + expect(uploader.path).to include('tmp/uploads') expect(uploader.path).not_to include('tmp/cache') expect(uploader.object_store).to eq(described_class::Store::REMOTE) end @@ -905,41 +1034,167 @@ RSpec.describe ObjectStorage do expect(uploader.object_store).to eq(described_class::Store::REMOTE) end end + + context 'when uploaded file remote_id matches a pending direct upload entry' do + let(:object) { build_stubbed(:ci_job_artifact) } + let(:final_path) { '@final/test/123123' } + let(:fog_config) { Gitlab.config.uploads.object_store } + let(:bucket) { 'uploads' } + let(:uploaded_file) { UploadedFile.new(temp_file.path, filename: "my_file.txt", remote_id: final_path) } + let(:fog_file_path) { final_path } + + let(:fog_connection_2) do + stub_object_storage_uploader( + config: fog_config, + uploader: uploader_class, + direct_upload: true + ) + end + + let!(:fog_file_2) do + fog_connection_2.directories.new(key: bucket).files.create( # rubocop:disable Rails/SaveBang + key: fog_file_path, + body: 'content' + ) + end + + before do + ObjectStorage::PendingDirectUpload.prepare( + uploader_class.storage_location_identifier, + final_path + ) + end + + it 'file to be cached and remote stored with final path set' do + expect { subject }.not_to raise_error + + expect(uploader).to be_exists + expect(uploader).to be_cached + expect(uploader.cache_only).to be_falsey + expect(uploader).not_to be_file_storage + expect(uploader.path).to eq(uploaded_file.remote_id) + expect(uploader.object_store).to eq(described_class::Store::REMOTE) + + expect(object.file_final_path).to eq(uploaded_file.remote_id) + end + + context 'when bucket prefix is configured' do + let(:fog_config) do + Gitlab.config.uploads.object_store.tap do |config| + config[:remote_directory] = 'main-bucket' + config[:bucket_prefix] = 'uploads' + end + end + + let(:bucket) { 'main-bucket' } + let(:fog_file_path) { "uploads/#{final_path}" } + + it 'stores the file final path in the db without the prefix' do + expect { subject }.not_to raise_error + + expect(uploader.store_path).to eq("uploads/#{final_path}") + expect(object.file_final_path).to eq(final_path) + end + end + + context 'when file is stored' do + subject do + uploader.store!(uploaded_file) + end + + it 'file to be remotely stored in permament location' do + subject + + expect(uploader).to be_exists + expect(uploader).not_to be_cached + expect(uploader.path).to eq(uploaded_file.remote_id) + end + + it 'does not trigger Carrierwave copy and delete because it is already in the final location' do + expect_next_instance_of(CarrierWave::Storage::Fog::File) do |instance| + expect(instance).not_to receive(:copy_to) + expect(instance).not_to receive(:delete) + end + + subject + end + end + end end end end end describe '#retrieve_from_store!' do - [:group, :project, :user].each do |model| - context "for #{model}s" do - let(:models) { create_list(model, 3, :with_avatar).map(&:reload) } - let(:avatars) { models.map(&:avatar) } + context 'uploaders that includes the RecordsUploads extension' do + [:group, :project, :user].each do |model| + context "for #{model}s" do + let(:models) { create_list(model, 3, :with_avatar).map(&:reload) } + let(:avatars) { models.map(&:avatar) } + + it 'batches fetching uploads from the database' do + # Ensure that these are all created and fully loaded before we start + # running queries for avatars + models + + expect { avatars }.not_to exceed_query_limit(1) + end + + it 'does not attempt to replace methods' do + models.each do |model| + expect(model.avatar.upload).to receive(:method_missing).and_call_original - it 'batches fetching uploads from the database' do - # Ensure that these are all created and fully loaded before we start - # running queries for avatars - models + model.avatar.upload.path + end + end - expect { avatars }.not_to exceed_query_limit(1) + it 'fetches a unique upload for each model' do + expect(avatars.map(&:url).uniq).to eq(avatars.map(&:url)) + expect(avatars.map(&:upload).uniq).to eq(avatars.map(&:upload)) + end end + end + end - it 'does not attempt to replace methods' do - models.each do |model| - expect(model.avatar.upload).to receive(:method_missing).and_call_original + describe 'filename' do + let(:model) { create(:ci_job_artifact, :remote_store, :archive) } - model.avatar.upload.path - end + before do + stub_artifacts_object_storage + end + + shared_examples 'ensuring correct filename' do + it 'uses the original filename' do + expect(model.reload.file.filename).to eq('ci_build_artifacts.zip') end + end - it 'fetches a unique upload for each model' do - expect(avatars.map(&:url).uniq).to eq(avatars.map(&:url)) - expect(avatars.map(&:upload).uniq).to eq(avatars.map(&:upload)) + context 'when model has final path defined for the file column' do + before do + model.update_column(:file_final_path, 'some/final/path/abc-123') end + + it_behaves_like 'ensuring correct filename' + end + + context 'when model has no final path defined for the file column' do + it_behaves_like 'ensuring correct filename' end end end + describe '.generate_final_store_path' do + subject(:final_path) { uploader_class.generate_final_store_path } + + before do + allow(Digest::SHA2).to receive(:hexdigest).and_return('somehash1234') + end + + it 'returns the generated hashed path' do + expect(final_path).to eq('@final/so/me/hash1234') + end + end + describe 'OpenFile' do subject { ObjectStorage::Concern::OpenFile.new(file) } diff --git a/yarn.lock b/yarn.lock index 567fcef51aa..456225203a2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4218,10 +4218,10 @@ core-js-pure@^3.0.0: resolved "https://registry.yarnpkg.com/core-js-pure/-/core-js-pure-3.6.5.tgz#c79e75f5e38dbc85a662d91eea52b8256d53b813" integrity sha512-lacdXOimsiD0QyNf9BC/mxivNJ/ybBGJXQFKzRekp1WTHoVUWsUHEn+2T8GJAzzIhyOuXA+gOxCVN3l+5PLPUA== -core-js@^3.29.1, core-js@^3.30.1, core-js@^3.6.5: - version "3.30.1" - resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.30.1.tgz#fc9c5adcc541d8e9fa3e381179433cbf795628ba" - integrity sha512-ZNS5nbiSwDTq4hFosEDqm65izl2CWmLz0hARJMyNQBgkUZMIF51cQiMvIQKA6hvuaeWxQDP3hEedM1JZIgTldQ== +core-js@^3.29.1, core-js@^3.30.2, core-js@^3.6.5: + version "3.30.2" + resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.30.2.tgz#6528abfda65e5ad728143ea23f7a14f0dcf503fc" + integrity sha512-uBJiDmwqsbJCWHAwjrx3cvjbMXP7xD72Dmsn5LOJpiRmE3WbBbN5rCqQ2Qh6Ek6/eOrjlWngEynBWo4VxerQhg== core-util-is@~1.0.0: version "1.0.3" |