diff options
31 files changed, 232 insertions, 500 deletions
@@ -49,7 +49,7 @@ gem 'omniauth-shibboleth', '~> 1.3.0' gem 'omniauth-twitter', '~> 1.4' gem 'omniauth_crowd', '~> 2.4.0' gem 'omniauth-authentiq', '~> 0.3.3' -gem 'gitlab-omniauth-openid-connect', '~> 0.4.0', require: 'omniauth_openid_connect' +gem 'gitlab-omniauth-openid-connect', '~> 0.8.0', require: 'omniauth_openid_connect' gem 'omniauth-salesforce', '~> 1.0.5' gem 'omniauth-atlassian-oauth2', '~> 0.2.0' gem 'rack-oauth2', '~> 1.16.0' diff --git a/Gemfile.lock b/Gemfile.lock index da52c302210..b27a1ceef56 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -494,7 +494,7 @@ GEM gitlab-mail_room (0.0.9) gitlab-markup (1.7.1) gitlab-net-dns (0.9.1) - gitlab-omniauth-openid-connect (0.4.0) + gitlab-omniauth-openid-connect (0.8.0) addressable (~> 2.7) omniauth (~> 1.9) openid_connect (~> 1.2) @@ -1493,7 +1493,7 @@ DEPENDENCIES gitlab-mail_room (~> 0.0.9) gitlab-markup (~> 1.7.1) gitlab-net-dns (~> 0.9.1) - gitlab-omniauth-openid-connect (~> 0.4.0) + gitlab-omniauth-openid-connect (~> 0.8.0) gitlab-sidekiq-fetcher (= 0.5.6) gitlab-styles (~> 6.2.0) gitlab_chronic_duration (~> 0.10.6.2) diff --git a/app/assets/javascripts/pages/projects/blob/show/index.js b/app/assets/javascripts/pages/projects/blob/show/index.js index e5b49dbcede..b365e039191 100644 --- a/app/assets/javascripts/pages/projects/blob/show/index.js +++ b/app/assets/javascripts/pages/projects/blob/show/index.js @@ -2,7 +2,7 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import TableOfContents from '~/blob/components/table_contents.vue'; import PipelineTourSuccessModal from '~/blob/pipeline_tour_success_modal.vue'; -import { BlobViewer } from '~/blob/viewer/index'; +import { BlobViewer, initAuxiliaryViewer } from '~/blob/viewer/index'; import GpgBadges from '~/gpg_badges'; import createDefaultClient from '~/lib/graphql'; import initBlob from '~/pages/projects/init_blob'; @@ -39,6 +39,8 @@ if (viewBlobEl) { }); }, }); + + initAuxiliaryViewer(); } else { new BlobViewer(); // eslint-disable-line no-new initBlob(); diff --git a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_table.vue b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_table.vue index 949a58f80fd..47fc7023222 100644 --- a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_table.vue +++ b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_table.vue @@ -55,7 +55,7 @@ export default { label: s__('Pipeline|Stages'), thClass: DEFAULT_TH_CLASSES, tdClass: DEFAULT_TD_CLASS, - columnClass: 'gl-w-quarter', + columnClass: 'gl-w-15p', thAttr: { 'data-testid': 'stages-th' }, }, { @@ -70,14 +70,12 @@ export default { key: 'actions', thClass: DEFAULT_TH_CLASSES, tdClass: DEFAULT_TD_CLASS, - columnClass: 'gl-w-15p', + columnClass: 'gl-w-20p', thAttr: { 'data-testid': 'actions-th' }, }, ], components: { GlTable, - LinkedPipelinesMiniList: () => - import('ee_component/vue_shared/components/linked_pipelines_mini_list.vue'), PipelinesCommit, PipelineMiniGraph, PipelineOperations, @@ -184,23 +182,12 @@ export default { <div class="stage-cell"> <!-- This empty div should be removed, see https://gitlab.com/gitlab-org/gitlab/-/issues/323488 --> <div></div> - <linked-pipelines-mini-list - v-if="item.triggered_by" - :triggered-by="[item.triggered_by]" - data-testid="mini-graph-upstream" - /> <pipeline-mini-graph v-if="item.details && item.details.stages && item.details.stages.length > 0" - class="gl-display-inline" :stages="item.details.stages" :update-dropdown="updateGraphDropdown" @pipelineActionRequestComplete="onPipelineActionRequestComplete" /> - <linked-pipelines-mini-list - v-if="item.triggered.length" - :triggered="item.triggered" - data-testid="mini-graph-downstream" - /> </div> </template> diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 0dbf7d40f87..545c7ac7c2f 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -74,11 +74,6 @@ class ProjectsController < Projects::ApplicationController @project = ::Projects::CreateService.new(current_user, project_params(attributes: project_params_create_attributes)).execute if @project.saved? - experiment(:new_project_readme, actor: current_user).track( - :created, - property: active_new_project_tab, - value: project_params[:initialize_with_readme].to_i - ) redirect_to( project_path(@project, custom_import_params), notice: _("Project '%{project_name}' was successfully created.") % { project_name: @project.name } diff --git a/app/experiments/new_project_readme_experiment.rb b/app/experiments/new_project_readme_experiment.rb deleted file mode 100644 index c5c41330949..00000000000 --- a/app/experiments/new_project_readme_experiment.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -class NewProjectReadmeExperiment < ApplicationExperiment # rubocop:disable Gitlab/NamespacedClass - include ProjectCommitCount - - INITIAL_WRITE_LIMIT = 3 - EXPERIMENT_START_DATE = DateTime.parse('2021/1/20') - MAX_ACCOUNT_AGE = 7.days - - exclude { context.value[:actor].nil? } - exclude { context.actor.created_at < MAX_ACCOUNT_AGE.ago } - - def control_behavior - false # we don't want the checkbox to be checked - end - - def candidate_behavior - true # check the checkbox by default - end - - def track_initial_writes(project) - return unless should_track? # early return if we don't need to ask for commit counts - return unless project.created_at > EXPERIMENT_START_DATE # early return for older projects - return unless (count = commit_count(project)) < INITIAL_WRITE_LIMIT - - track(:write, property: project.created_at.to_s, value: count) - end - - private - - def commit_count(project) - commit_count_for(project, - default_count: INITIAL_WRITE_LIMIT, - max_count: INITIAL_WRITE_LIMIT, - experiment: name - ) - end -end diff --git a/app/models/discussion.rb b/app/models/discussion.rb index 076d8cc280c..203e14f1227 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -163,16 +163,15 @@ class Discussion end def cache_key - # Need this so cache will be invalidated when note within a discussion - # has been deleted. - notes_sha = Digest::SHA1.hexdigest(notes.map(&:id).join(':')) + # Need to use the notes' cache key so cache will be invalidated when note + # within a discussion has been deleted or has different data after post + # processing of content. + notes_sha = Digest::SHA1.hexdigest(notes.map(&:post_processed_cache_key).join(':')) [ CACHE_VERSION, - notes.last.latest_cached_markdown_version, id, notes_sha, - notes.max_by(&:updated_at).updated_at, resolved_at ].join(':') end diff --git a/app/models/environment.rb b/app/models/environment.rb index 558963c98c4..125218ea21d 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -5,6 +5,7 @@ class Environment < ApplicationRecord include ReactiveCaching include FastDestroyAll::Helpers include Presentable + include NullifyIfBlank self.reactive_cache_refresh_interval = 1.minute self.reactive_cache_lifetime = 55.seconds @@ -14,6 +15,7 @@ class Environment < ApplicationRecord belongs_to :project, required: true use_fast_destroy :all_deployments + nullify_if_blank :external_url has_many :all_deployments, class_name: 'Deployment' has_many :deployments, -> { visible } @@ -33,7 +35,6 @@ class Environment < ApplicationRecord has_one :upcoming_deployment, -> { running.distinct_on_environment }, class_name: 'Deployment', inverse_of: :environment has_one :latest_opened_most_severe_alert, -> { order_severity_with_open_prometheus_alert }, class_name: 'AlertManagement::Alert', inverse_of: :environment - before_validation :nullify_external_url before_validation :generate_slug, if: ->(env) { env.slug.blank? } before_save :set_environment_type @@ -230,10 +231,6 @@ class Environment < ApplicationRecord ref.to_s == last_deployment.try(:ref) end - def nullify_external_url - self.external_url = nil if self.external_url.blank? - end - def set_environment_type names = name.split('/') diff --git a/app/models/note.rb b/app/models/note.rb index ed341e58436..b31b1a20cd8 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -586,6 +586,13 @@ class Note < ApplicationRecord review.present? || !author.can_trigger_notifications? end + def post_processed_cache_key + cache_key_items = [cache_key] + cache_key_items << Digest::SHA1.hexdigest(redacted_note_html) if redacted_note_html.present? + + cache_key_items.join(':') + end + private # Using this method followed by a call to *save* may result in *ActiveRecord::RecordNotUnique* exception diff --git a/app/views/projects/_new_project_fields.html.haml b/app/views/projects/_new_project_fields.html.haml index 55696337bc1..026c7a0d79d 100644 --- a/app/views/projects/_new_project_fields.html.haml +++ b/app/views/projects/_new_project_fields.html.haml @@ -54,8 +54,7 @@ .form-group.row.initialize-with-readme-setting %div{ :class => "col-sm-12" } .form-check - - experiment(:new_project_readme, actor: current_user) do |e| - = check_box_tag 'project[initialize_with_readme]', '1', e.run, class: 'form-check-input', data: { qa_selector: "initialize_with_readme_checkbox", track_label: "#{track_label}", track_event: "activate_form_input", track_property: "init_with_readme", track_value: "" } + = check_box_tag 'project[initialize_with_readme]', '1', true, class: 'form-check-input', data: { qa_selector: "initialize_with_readme_checkbox", track_label: "#{track_label}", track_event: "activate_form_input", track_property: "init_with_readme", track_value: "" } = label_tag 'project[initialize_with_readme]', class: 'form-check-label' do .option-title %strong= s_('ProjectsNew|Initialize repository with a README') diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index 94c1788a3ba..4a49e18eb9b 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -126,7 +126,6 @@ class PostReceive # rubocop:disable Scalability/IdempotentWorker end def after_project_changes_hooks(project, user, refs, changes) - experiment(:new_project_readme, actor: user).track_initial_writes(project) experiment(:empty_repo_upload, project: project).track_initial_write repository_update_hook_data = Gitlab::DataBuilder::Repository.update(project, user, changes, refs) SystemHooksService.new.execute_hooks(repository_update_hook_data, :repository_update_hooks) diff --git a/doc/administration/gitaly/praefect.md b/doc/administration/gitaly/praefect.md index 044b8793b38..9c254ea9793 100644 --- a/doc/administration/gitaly/praefect.md +++ b/doc/administration/gitaly/praefect.md @@ -1600,6 +1600,10 @@ When creating the storage, see some ### Move Repositories +WARNING: +To move repositories into a Gitaly Cluster in GitLab versions 13.12 to 14.1, you must +[enable the `gitaly_replicate_repository_direct_fetch` feature flag](../feature_flags.md). + To migrate to Gitaly Cluster, existing repositories stored outside Gitaly Cluster must be moved. There is no automatic migration but the moves can be scheduled with the GitLab API. diff --git a/doc/administration/gitaly/troubleshooting.md b/doc/administration/gitaly/troubleshooting.md index ab6f493cf0f..6a24682de55 100644 --- a/doc/administration/gitaly/troubleshooting.md +++ b/doc/administration/gitaly/troubleshooting.md @@ -370,3 +370,11 @@ the `praefect` command: $ sudo /opt/gitlab/embedded/bin/praefect -config /var/opt/gitlab/praefect/config.toml sql-migrate praefect sql-migrate: OK (applied 21 migrations) ``` + +### Requests fail with 'repo scoped: invalid Repository' errors + +This indicates that the virtual storage name used in the +[Praefect configuration](praefect.md#praefect) does not match the storage name used in +[`git_data_dirs` setting](praefect.md#gitaly) for GitLab. + +Resolve this by matching the virtual storage names used in Praefect and GitLab configuration. diff --git a/qa/qa/page/project/new.rb b/qa/qa/page/project/new.rb index 170cc14b27f..06e476f009a 100644 --- a/qa/qa/page/project/new.rb +++ b/qa/qa/page/project/new.rb @@ -83,8 +83,8 @@ module QA click_button 'Repo by URL' end - def enable_initialize_with_readme - check_element(:initialize_with_readme_checkbox) + def disable_initialize_with_readme + uncheck_element(:initialize_with_readme_checkbox) end end end diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index d111b070863..70cec0cf747 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -88,7 +88,7 @@ module QA new_page.choose_name(@name) new_page.add_description(@description) new_page.set_visibility(@visibility) - new_page.enable_initialize_with_readme if @initialize_with_readme + new_page.disable_initialize_with_readme unless @initialize_with_readme new_page.create_new_project end end diff --git a/scripts/api/cancel_pipeline.rb b/scripts/api/cancel_pipeline.rb index 0965877a69a..f3aea90f54f 100755 --- a/scripts/api/cancel_pipeline.rb +++ b/scripts/api/cancel_pipeline.rb @@ -10,7 +10,8 @@ class CancelPipeline DEFAULT_OPTIONS = { project: ENV['CI_PROJECT_ID'], pipeline_id: ENV['CI_PIPELINE_ID'], - api_token: ENV['GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN'] + # Default to "CI scripts API usage" at https://gitlab.com/gitlab-org/gitlab/-/settings/access_tokens + api_token: ENV['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] }.freeze def initialize(options) diff --git a/scripts/api/download_job_artifact.rb b/scripts/api/download_job_artifact.rb index 8e2207c6fa7..d725de3f1b6 100755 --- a/scripts/api/download_job_artifact.rb +++ b/scripts/api/download_job_artifact.rb @@ -11,7 +11,8 @@ require 'net/http' class ArtifactFinder DEFAULT_OPTIONS = { project: ENV['CI_PROJECT_ID'], - api_token: ENV['GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN'] + # Default to "CI scripts API usage" at https://gitlab.com/gitlab-org/gitlab/-/settings/access_tokens + api_token: ENV['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] }.freeze def initialize(options) diff --git a/scripts/api/get_job_id.rb b/scripts/api/get_job_id.rb index dd0b7fbada0..f6f1f326225 100755 --- a/scripts/api/get_job_id.rb +++ b/scripts/api/get_job_id.rb @@ -11,7 +11,8 @@ class JobFinder pipeline_id: ENV['CI_PIPELINE_ID'], pipeline_query: {}, job_query: {}, - api_token: ENV['GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN'] + # Default to "CI scripts API usage" at https://gitlab.com/gitlab-org/gitlab/-/settings/access_tokens + api_token: ENV['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] }.freeze def initialize(options) diff --git a/scripts/api/play_job.rb b/scripts/api/play_job.rb index 408dfdf1ef0..2c5cc75619d 100755 --- a/scripts/api/play_job.rb +++ b/scripts/api/play_job.rb @@ -10,7 +10,8 @@ class PlayJob DEFAULT_OPTIONS = { project: ENV['CI_PROJECT_ID'], pipeline_id: ENV['CI_PIPELINE_ID'], - api_token: ENV['GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN'] + # Default to "CI scripts API usage" at https://gitlab.com/gitlab-org/gitlab/-/settings/access_tokens + api_token: ENV['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] }.freeze def initialize(options) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 46c17d6a6fe..d8a1580ad67 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -435,32 +435,6 @@ RSpec.describe ProjectsController do end end - describe 'POST create' do - let!(:project_params) do - { - path: 'foo', - description: 'bar', - namespace_id: user.namespace.id, - visibility_level: Gitlab::VisibilityLevel::PUBLIC, - initialize_with_readme: 1 - } - end - - before do - sign_in(user) - end - - it 'tracks a created event for the new_project_readme experiment', :experiment do - expect(experiment(:new_project_readme)).to track( - :created, - property: 'blank', - value: 1 - ).with_context(actor: user).on_next_instance - - post :create, params: { project: project_params } - end - end - describe 'POST #archive' do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } diff --git a/spec/experiments/new_project_readme_experiment_spec.rb b/spec/experiments/new_project_readme_experiment_spec.rb deleted file mode 100644 index e5ecc4662f6..00000000000 --- a/spec/experiments/new_project_readme_experiment_spec.rb +++ /dev/null @@ -1,75 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe NewProjectReadmeExperiment, :experiment do - subject { described_class.new(actor: actor) } - - let(:actor) { User.new(id: 42, created_at: Time.current) } - - describe "exclusions" do - let(:threshold) { described_class::MAX_ACCOUNT_AGE } - - it { is_expected.to exclude(actor: User.new(created_at: (threshold + 1.minute).ago)) } - it { is_expected.not_to exclude(actor: User.new(created_at: (threshold - 1.minute).ago)) } - end - - describe "the control behavior" do - subject { described_class.new(actor: actor).run(:control) } - - it { is_expected.to be false } - end - - describe "the candidate behavior" do - subject { described_class.new(actor: actor).run(:candidate) } - - it { is_expected.to be true } - end - - context "when tracking initial writes" do - let!(:project) { create(:project, :repository) } - - before do - stub_experiments(new_project_readme: :control) - end - - it "tracks an event for the first commit on a project with a repository" do - expect(subject).to receive(:commit_count_for).with(project, default_count: described_class::INITIAL_WRITE_LIMIT, max_count: described_class::INITIAL_WRITE_LIMIT, experiment: 'new_project_readme').and_return(1) - expect(subject).to receive(:track).with(:write, property: project.created_at.to_s, value: 1).and_call_original - - subject.track_initial_writes(project) - end - - it "tracks an event for the second commit on a project with a repository" do - allow(subject).to receive(:commit_count_for).and_return(2) - - expect(subject).to receive(:track).with(:write, property: project.created_at.to_s, value: 2).and_call_original - - subject.track_initial_writes(project) - end - - it "doesn't track if the repository has more then 2 commits" do - allow(subject).to receive(:commit_count_for).and_return(3) - - expect(subject).not_to receive(:track) - - subject.track_initial_writes(project) - end - - it "doesn't track when we generally shouldn't" do - allow(subject).to receive(:should_track?).and_return(false) - - expect(subject).not_to receive(:track) - - subject.track_initial_writes(project) - end - - it "doesn't track if the project is older" do - expect(project).to receive(:created_at).and_return(described_class::EXPERIMENT_START_DATE - 1.minute) - - expect(subject).not_to receive(:track) - - subject.track_initial_writes(project) - end - end -end diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 595304789a6..677052eebea 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -584,199 +584,205 @@ RSpec.describe 'File blob', :js do end end - describe '.gitlab-ci.yml' do + context 'files with auxiliary viewers' do before do - project.add_maintainer(project.creator) - - Files::CreateService.new( - project, - project.creator, - start_branch: 'master', - branch_name: 'master', - commit_message: "Add .gitlab-ci.yml", - file_path: '.gitlab-ci.yml', - file_content: File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) - ).execute - - visit_blob('.gitlab-ci.yml') + stub_feature_flags(refactor_blob_viewer: true) end - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows that configuration is valid - expect(page).to have_content('This GitLab CI configuration is valid.') + describe '.gitlab-ci.yml' do + before do + project.add_maintainer(project.creator) - # shows a learn more link - expect(page).to have_link('Learn more') - end - end - end + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add .gitlab-ci.yml", + file_path: '.gitlab-ci.yml', + file_content: File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) + ).execute - describe '.gitlab/route-map.yml' do - before do - project.add_maintainer(project.creator) + visit_blob('.gitlab-ci.yml') + end - Files::CreateService.new( - project, - project.creator, - start_branch: 'master', - branch_name: 'master', - commit_message: "Add .gitlab/route-map.yml", - file_path: '.gitlab/route-map.yml', - file_content: <<-MAP.strip_heredoc - # Team data - - source: 'data/team.yml' - public: 'team/' - MAP - ).execute + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that configuration is valid + expect(page).to have_content('This GitLab CI configuration is valid.') - visit_blob('.gitlab/route-map.yml') + # shows a learn more link + expect(page).to have_link('Learn more') + end + end end - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows that map is valid - expect(page).to have_content('This Route Map is valid.') + describe '.gitlab/route-map.yml' do + before do + project.add_maintainer(project.creator) + + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add .gitlab/route-map.yml", + file_path: '.gitlab/route-map.yml', + file_content: <<-MAP.strip_heredoc + # Team data + - source: 'data/team.yml' + public: 'team/' + MAP + ).execute - # shows a learn more link - expect(page).to have_link('Learn more') + visit_blob('.gitlab/route-map.yml') end - end - end - describe '.gitlab/dashboards/custom-dashboard.yml' do - before do - project.add_maintainer(project.creator) + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that map is valid + expect(page).to have_content('This Route Map is valid.') - Files::CreateService.new( - project, - project.creator, - start_branch: 'master', - branch_name: 'master', - commit_message: "Add .gitlab/dashboards/custom-dashboard.yml", - file_path: '.gitlab/dashboards/custom-dashboard.yml', - file_content: file_content - ).execute + # shows a learn more link + expect(page).to have_link('Learn more') + end + end end - context 'with metrics_dashboard_exhaustive_validations feature flag off' do + describe '.gitlab/dashboards/custom-dashboard.yml' do before do - stub_feature_flags(metrics_dashboard_exhaustive_validations: false) - visit_blob('.gitlab/dashboards/custom-dashboard.yml') + project.add_maintainer(project.creator) + + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add .gitlab/dashboards/custom-dashboard.yml", + file_path: '.gitlab/dashboards/custom-dashboard.yml', + file_content: file_content + ).execute end - context 'valid dashboard file' do - let(:file_content) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) } + context 'with metrics_dashboard_exhaustive_validations feature flag off' do + before do + stub_feature_flags(metrics_dashboard_exhaustive_validations: false) + visit_blob('.gitlab/dashboards/custom-dashboard.yml') + end - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows that dashboard yaml is valid - expect(page).to have_content('Metrics Dashboard YAML definition is valid.') + context 'valid dashboard file' do + let(:file_content) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) } - # shows a learn more link - expect(page).to have_link('Learn more') + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that dashboard yaml is valid + expect(page).to have_content('Metrics Dashboard YAML definition is valid.') + + # shows a learn more link + expect(page).to have_link('Learn more') + end end end - end - context 'invalid dashboard file' do - let(:file_content) { "dashboard: 'invalid'" } + context 'invalid dashboard file' do + let(:file_content) { "dashboard: 'invalid'" } - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows that dashboard yaml is invalid - expect(page).to have_content('Metrics Dashboard YAML definition is invalid:') - expect(page).to have_content("panel_groups: should be an array of panel_groups objects") + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that dashboard yaml is invalid + expect(page).to have_content('Metrics Dashboard YAML definition is invalid:') + expect(page).to have_content("panel_groups: should be an array of panel_groups objects") - # shows a learn more link - expect(page).to have_link('Learn more') + # shows a learn more link + expect(page).to have_link('Learn more') + end end end end - end - context 'with metrics_dashboard_exhaustive_validations feature flag on' do - before do - stub_feature_flags(metrics_dashboard_exhaustive_validations: true) - visit_blob('.gitlab/dashboards/custom-dashboard.yml') - end + context 'with metrics_dashboard_exhaustive_validations feature flag on' do + before do + stub_feature_flags(metrics_dashboard_exhaustive_validations: true) + visit_blob('.gitlab/dashboards/custom-dashboard.yml') + end - context 'valid dashboard file' do - let(:file_content) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) } + context 'valid dashboard file' do + let(:file_content) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) } - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows that dashboard yaml is valid - expect(page).to have_content('Metrics Dashboard YAML definition is valid.') + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that dashboard yaml is valid + expect(page).to have_content('Metrics Dashboard YAML definition is valid.') - # shows a learn more link - expect(page).to have_link('Learn more') + # shows a learn more link + expect(page).to have_link('Learn more') + end end end - end - context 'invalid dashboard file' do - let(:file_content) { "dashboard: 'invalid'" } + context 'invalid dashboard file' do + let(:file_content) { "dashboard: 'invalid'" } - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows that dashboard yaml is invalid - expect(page).to have_content('Metrics Dashboard YAML definition is invalid:') - expect(page).to have_content("root is missing required keys: panel_groups") + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that dashboard yaml is invalid + expect(page).to have_content('Metrics Dashboard YAML definition is invalid:') + expect(page).to have_content("root is missing required keys: panel_groups") - # shows a learn more link - expect(page).to have_link('Learn more') + # shows a learn more link + expect(page).to have_link('Learn more') + end end end end end - end - context 'LICENSE' do - before do - visit_blob('LICENSE') - end + context 'LICENSE' do + before do + visit_blob('LICENSE') + end - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows license - expect(page).to have_content('This project is licensed under the MIT License.') + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows license + expect(page).to have_content('This project is licensed under the MIT License.') - # shows a learn more link - expect(page).to have_link('Learn more', href: 'http://choosealicense.com/licenses/mit/') + # shows a learn more link + expect(page).to have_link('Learn more', href: 'http://choosealicense.com/licenses/mit/') + end end end - end - context '*.gemspec' do - before do - project.add_maintainer(project.creator) + context '*.gemspec' do + before do + project.add_maintainer(project.creator) - Files::CreateService.new( - project, - project.creator, - start_branch: 'master', - branch_name: 'master', - commit_message: "Add activerecord.gemspec", - file_path: 'activerecord.gemspec', - file_content: <<-SPEC.strip_heredoc - Gem::Specification.new do |s| - s.platform = Gem::Platform::RUBY - s.name = "activerecord" - end - SPEC - ).execute + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add activerecord.gemspec", + file_path: 'activerecord.gemspec', + file_content: <<-SPEC.strip_heredoc + Gem::Specification.new do |s| + s.platform = Gem::Platform::RUBY + s.name = "activerecord" + end + SPEC + ).execute - visit_blob('activerecord.gemspec') - end + visit_blob('activerecord.gemspec') + end - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows names of dependency manager and package - expect(page).to have_content('This project manages its dependencies using RubyGems.') + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows names of dependency manager and package + expect(page).to have_content('This project manages its dependencies using RubyGems.') - # shows a learn more link - expect(page).to have_link('Learn more', href: 'https://rubygems.org/') + # shows a learn more link + expect(page).to have_link('Learn more', href: 'https://rubygems.org/') + end end end end diff --git a/spec/features/projects/user_creates_project_spec.rb b/spec/features/projects/user_creates_project_spec.rb index a5b51bac747..96051e8d9f9 100644 --- a/spec/features/projects/user_creates_project_spec.rb +++ b/spec/features/projects/user_creates_project_spec.rb @@ -8,8 +8,6 @@ RSpec.describe 'User creates a project', :js do before do sign_in(user) create(:personal_key, user: user) - - stub_experiments(new_project_readme: :candidate) end it 'creates a new project' do @@ -18,7 +16,6 @@ RSpec.describe 'User creates a project', :js do find('[data-qa-panel-name="blank_project"]').click fill_in(:project_name, with: 'Empty') - # part of the new_project_readme experiment expect(page).to have_checked_field 'Initialize repository with a README' uncheck 'Initialize repository with a README' diff --git a/spec/frontend/pipelines/mock_data.js b/spec/frontend/pipelines/mock_data.js index 02ec529eb12..7e3c3727c9d 100644 --- a/spec/frontend/pipelines/mock_data.js +++ b/spec/frontend/pipelines/mock_data.js @@ -471,84 +471,3 @@ export const mockSearch = [ export const mockBranchesAfterMap = ['branch-1', 'branch-10', 'branch-11']; export const mockTagsAfterMap = ['tag-3', 'tag-2', 'tag-1', 'main-tag']; - -export const triggered = [ - { - id: 602, - user: { - id: 1, - name: 'Administrator', - username: 'root', - state: 'active', - avatar_url: - 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', - web_url: 'http://gdk.test:3000/root', - show_status: false, - path: '/root', - }, - active: false, - coverage: null, - source: 'pipeline', - source_job: { name: 'trigger_job_on_mr' }, - path: '/root/job-log-sections/-/pipelines/602', - details: { - status: { - icon: 'status_success', - text: 'passed', - label: 'passed', - group: 'success', - tooltip: 'passed', - has_details: true, - details_path: '/root/job-log-sections/-/pipelines/602', - illustration: null, - favicon: - '/assets/ci_favicons/favicon_status_success-8451333011eee8ce9f2ab25dc487fe24a8758c694827a582f17f42b0a90446a2.png', - }, - }, - project: { - id: 36, - name: 'job-log-sections', - full_path: '/root/job-log-sections', - full_name: 'Administrator / job-log-sections', - }, - }, -]; - -export const triggeredBy = { - id: 614, - user: { - id: 1, - name: 'Administrator', - username: 'root', - state: 'active', - avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', - web_url: 'http://gdk.test:3000/root', - show_status: false, - path: '/root', - }, - active: false, - coverage: null, - source: 'web', - source_job: { name: null }, - path: '/root/trigger-downstream/-/pipelines/614', - details: { - status: { - icon: 'status_success', - text: 'passed', - label: 'passed', - group: 'success', - tooltip: 'passed', - has_details: true, - details_path: '/root/trigger-downstream/-/pipelines/614', - illustration: null, - favicon: - '/assets/ci_favicons/favicon_status_success-8451333011eee8ce9f2ab25dc487fe24a8758c694827a582f17f42b0a90446a2.png', - }, - }, - project: { - id: 42, - name: 'trigger-downstream', - full_path: '/root/trigger-downstream', - full_name: 'Administrator / trigger-downstream', - }, -}; diff --git a/spec/frontend/pipelines/pipelines_table_spec.js b/spec/frontend/pipelines/pipelines_table_spec.js index 07d9b208776..68b0dfc018e 100644 --- a/spec/frontend/pipelines/pipelines_table_spec.js +++ b/spec/frontend/pipelines/pipelines_table_spec.js @@ -12,7 +12,6 @@ import PipelinesTimeago from '~/pipelines/components/pipelines_list/time_ago.vue import eventHub from '~/pipelines/event_hub'; import CiBadge from '~/vue_shared/components/ci_badge_link.vue'; import CommitComponent from '~/vue_shared/components/commit.vue'; -import { triggeredBy, triggered } from './mock_data'; jest.mock('~/pipelines/event_hub'); @@ -60,8 +59,6 @@ describe('Pipelines Table', () => { const findStagesTh = () => wrapper.findByTestId('stages-th'); const findTimeAgoTh = () => wrapper.findByTestId('timeago-th'); const findActionsTh = () => wrapper.findByTestId('actions-th'); - const findUpstream = () => wrapper.findByTestId('mini-graph-upstream'); - const findDownstream = () => wrapper.findByTestId('mini-graph-downstream'); beforeEach(() => { pipeline = createMockPipeline(); @@ -139,8 +136,6 @@ describe('Pipelines Table', () => { describe('stages cell', () => { it('should render a pipeline mini graph', () => { expect(findPipelineMiniGraph().exists()).toBe(true); - expect(findUpstream().exists()).toBe(false); - expect(findDownstream().exists()).toBe(false); }); it('should render the right number of stages', () => { @@ -178,57 +173,6 @@ describe('Pipelines Table', () => { expect(eventHub.$emit).toHaveBeenCalledWith('refreshPipelinesTable'); }); - - describe('upstream linked pipelines', () => { - beforeEach(() => { - pipeline = createMockPipeline(); - pipeline.triggered_by = triggeredBy; - - createComponent({ pipelines: [pipeline] }); - }); - - it('should render only a upstream pipeline', () => { - expect(findUpstream().exists()).toBe(true); - expect(findDownstream().exists()).toBe(false); - }); - - it('should pass an array of the correct data to the linked pipeline component', () => { - const triggeredByProps = findUpstream().props('triggeredBy'); - - expect(triggeredByProps).toEqual(expect.any(Array)); - expect(triggeredByProps).toHaveLength(1); - expect(triggeredByProps[0]).toBe(triggeredBy); - }); - }); - - describe('downstream linked pipelines', () => { - beforeEach(() => { - pipeline = createMockPipeline(); - pipeline.triggered = triggered; - - createComponent({ pipelines: [pipeline] }); - }); - - it('should render only a downstream pipeline', () => { - expect(findDownstream().exists()).toBe(true); - expect(findUpstream().exists()).toBe(false); - }); - }); - - describe('upstream and downstream linked pipelines', () => { - beforeEach(() => { - pipeline = createMockPipeline(); - pipeline.triggered = triggered; - pipeline.triggered_by = triggeredBy; - - createComponent({ pipelines: [pipeline] }); - }); - - it('should render both downstream and upstream pipelines', () => { - expect(findDownstream().exists()).toBe(true); - expect(findUpstream().exists()).toBe(true); - }); - }); }); describe('duration cell', () => { diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb index 998204626d3..4e2463ab3f3 100644 --- a/spec/models/diff_discussion_spec.rb +++ b/spec/models/diff_discussion_spec.rb @@ -129,10 +129,10 @@ RSpec.describe DiffDiscussion do describe '#cache_key' do it 'returns the cache key with the position sha' do - notes_sha = Digest::SHA1.hexdigest("#{diff_note.id}") + notes_sha = Digest::SHA1.hexdigest("#{diff_note.post_processed_cache_key}") position_sha = Digest::SHA1.hexdigest(diff_note.position.to_json) - expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{diff_note.latest_cached_markdown_version}:#{subject.id}:#{notes_sha}:#{diff_note.updated_at}::#{position_sha}") + expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}::#{position_sha}") end end end diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb index 2b33de96e04..212619a1c3d 100644 --- a/spec/models/discussion_spec.rb +++ b/spec/models/discussion_spec.rb @@ -53,10 +53,10 @@ RSpec.describe Discussion do end describe '#cache_key' do - let(:notes_sha) { Digest::SHA1.hexdigest("#{first_note.id}:#{second_note.id}:#{third_note.id}") } + let(:notes_sha) { Digest::SHA1.hexdigest("#{first_note.post_processed_cache_key}:#{second_note.post_processed_cache_key}:#{third_note.post_processed_cache_key}") } - it 'returns the cache key with ID and latest updated note updated at' do - expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{third_note.latest_cached_markdown_version}:#{subject.id}:#{notes_sha}:#{third_note.updated_at}:") + it 'returns the cache key' do + expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}:") end context 'when discussion is resolved' do @@ -65,7 +65,7 @@ RSpec.describe Discussion do end it 'returns the cache key with resolved at' do - expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{third_note.latest_cached_markdown_version}:#{subject.id}:#{notes_sha}:#{third_note.updated_at}:#{subject.resolved_at}") + expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}:#{subject.resolved_at}") end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 18a172b72d7..fae58326f07 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -14,6 +14,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do subject(:environment) { create(:environment, project: project) } it { is_expected.to be_kind_of(ReactiveCaching) } + it { is_expected.to nullify_if_blank(:external_url) } it { is_expected.to belong_to(:project).required } it { is_expected.to have_many(:deployments) } @@ -412,15 +413,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end - describe '#nullify_external_url' do - it 'replaces a blank url with nil' do - env = build(:environment, external_url: "") - - expect(env.save).to be true - expect(env.external_url).to be_nil - end - end - describe '#includes_commit?' do let(:project) { create(:project, :repository) } diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 2afe9a0f29b..d7eddaa4294 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1538,4 +1538,24 @@ RSpec.describe Note do expect(attachment).not_to be_exist end end + + describe '#post_processed_cache_key' do + let(:note) { build(:note) } + + it 'returns cache key by default' do + expect(note.post_processed_cache_key).to eq(note.cache_key) + end + + context 'when note has redacted_note_html' do + let(:redacted_note_html) { 'redacted note html' } + + before do + note.redacted_note_html = redacted_note_html + end + + it 'returns cache key with redacted_note_html sha' do + expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{Digest::SHA1.hexdigest(redacted_note_html)}") + end + end + end end diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb index 595222a9eb2..f2e8377cf23 100644 --- a/spec/requests/projects/merge_requests_discussions_spec.rb +++ b/spec/requests/projects/merge_requests_discussions_spec.rb @@ -54,7 +54,8 @@ RSpec.describe 'merge requests discussions' do end context 'caching', :use_clean_rails_memory_store_caching do - let!(:first_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } + let(:reference) { create(:issue, project: project) } + let!(:first_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, note: "reference: #{reference.to_reference}") } let!(:second_note) { create(:diff_note_on_merge_request, in_reply_to: first_note, noteable: merge_request, project: project) } let!(:award_emoji) { create(:award_emoji, awardable: first_note) } @@ -93,6 +94,16 @@ RSpec.describe 'merge requests discussions' do end end + context 'when a note in a discussion got its reference state updated' do + before do + reference.close! + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end + end + context 'when a note in a discussion got resolved' do before do travel_to(1.minute.from_now) do @@ -147,17 +158,6 @@ RSpec.describe 'merge requests discussions' do end end - context 'when cached markdown version gets bump' do - before do - settings = Gitlab::CurrentSettings.current_application_settings - settings.update!(local_markdown_version: settings.local_markdown_version + 1) - end - - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } - end - end - context 'when the diff note position changes' do before do # This replicates a position change wherein timestamps aren't updated diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 04a38874905..f26b3668653 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -85,14 +85,6 @@ RSpec.describe PostReceive do perform end - it 'tracks an event for the new_project_readme experiment', :experiment do - expect_next_instance_of(NewProjectReadmeExperiment, :new_project_readme, nil, actor: empty_project.owner) do |e| - expect(e).to receive(:track_initial_writes).with(empty_project) - end - - perform - end - it 'tracks an event for the empty_repo_upload experiment', :experiment do expect_next_instance_of(EmptyRepoUploadExperiment) do |e| expect(e).to receive(:track_initial_write) |