Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-05-06 09:10:11 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2021-05-06 09:10:11 +0300
commit0a0e8803b0e3e2fb83d74c9bafc32f4e9d825bcc (patch)
tree12ca59ec66c2ec7d405101869a7384d2034d308d
parent806b829e76120085d80759dabc110f0328cfb7ac (diff)
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--app/assets/javascripts/pipeline_editor/components/ui/pipeline_editor_empty_state.vue4
-rw-r--r--app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue2
-rw-r--r--app/assets/javascripts/vue_shared/components/vuex_module_provider.vue5
-rw-r--r--app/finders/deployments_finder.rb95
-rw-r--r--app/models/deployment.rb4
-rw-r--r--changelogs/unreleased/optimize-deployment-finder.yml5
-rw-r--r--config/feature_flags/development/deployments_finder_implicitly_enforce_ordering_for_updated_at_filter.yml8
-rw-r--r--db/migrate/20210420210642_recreate_index_for_project_deployments_with_environment_id_and_date_at.rb23
-rw-r--r--db/schema_migrations/202104202106421
-rw-r--r--db/structure.sql2
-rw-r--r--doc/development/code_review.md4
-rw-r--r--doc/user/application_security/index.md13
-rw-r--r--lib/api/deployments.rb2
-rw-r--r--lib/gitlab/cycle_analytics/summary/deploy.rb2
-rw-r--r--spec/finders/deployments_finder_spec.rb156
-rw-r--r--spec/frontend/vue_shared/components/vuex_module_provider_spec.js20
-rw-r--r--spec/models/deployment_spec.rb29
-rw-r--r--spec/requests/api/deployments_spec.rb25
18 files changed, 338 insertions, 62 deletions
diff --git a/app/assets/javascripts/pipeline_editor/components/ui/pipeline_editor_empty_state.vue b/app/assets/javascripts/pipeline_editor/components/ui/pipeline_editor_empty_state.vue
index b5f89088705..0ac4a40ff4a 100644
--- a/app/assets/javascripts/pipeline_editor/components/ui/pipeline_editor_empty_state.vue
+++ b/app/assets/javascripts/pipeline_editor/components/ui/pipeline_editor_empty_state.vue
@@ -40,9 +40,7 @@ export default {
</script>
<template>
<div>
- <div class="gl-mb-5">
- <pipeline-editor-file-nav v-if="showFileNav" v-on="$listeners" />
- </div>
+ <pipeline-editor-file-nav v-if="showFileNav" v-on="$listeners" />
<div class="gl-display-flex gl-flex-direction-column gl-align-items-center gl-mt-11">
<img :src="emptyStateIllustrationPath" />
<h1 class="gl-font-size-h1">{{ $options.i18n.title }}</h1>
diff --git a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue
index 11f484b2cdf..18c9ed7a6f0 100644
--- a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue
+++ b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue
@@ -20,7 +20,7 @@ export default {
},
props: {
target: {
- type: HTMLAnchorElement,
+ type: HTMLElement,
required: true,
},
user: {
diff --git a/app/assets/javascripts/vue_shared/components/vuex_module_provider.vue b/app/assets/javascripts/vue_shared/components/vuex_module_provider.vue
index 8c004851eca..eff39e2fb89 100644
--- a/app/assets/javascripts/vue_shared/components/vuex_module_provider.vue
+++ b/app/assets/javascripts/vue_shared/components/vuex_module_provider.vue
@@ -2,7 +2,10 @@
export default {
provide() {
return {
- vuexModule: this.vuexModule,
+ // We can't use this.vuexModule due to bug in vue-apollo when
+ // provide is called in beforeCreate
+ // See https://github.com/vuejs/vue-apollo/pull/1153 for details
+ vuexModule: this.$options.propsData.vuexModule,
};
},
props: {
diff --git a/app/finders/deployments_finder.rb b/app/finders/deployments_finder.rb
index 8fec7932926..a54408e7216 100644
--- a/app/finders/deployments_finder.rb
+++ b/app/finders/deployments_finder.rb
@@ -16,14 +16,25 @@
class DeploymentsFinder
attr_reader :params
+ # Warning:
+ # These const are directly used in Deployment Rest API, thus
+ # modifying these values could implicity change the API interface or introduce a breaking change.
+ # Also, if you add a sort value, make sure that the new query will stay
+ # performant with the other filtering/sorting parameters.
+ # The composed query could be significantly slower when the filtering and sorting columns are different.
+ # See https://gitlab.com/gitlab-org/gitlab/-/issues/325627 for example.
ALLOWED_SORT_VALUES = %w[id iid created_at updated_at ref finished_at].freeze
DEFAULT_SORT_VALUE = 'id'
ALLOWED_SORT_DIRECTIONS = %w[asc desc].freeze
DEFAULT_SORT_DIRECTION = 'asc'
+ InefficientQueryError = Class.new(StandardError)
+
def initialize(params = {})
@params = params
+
+ validate!
end
def execute
@@ -38,6 +49,32 @@ class DeploymentsFinder
private
+ def validate!
+ if filter_by_updated_at? && filter_by_finished_at?
+ raise InefficientQueryError, 'Both `updated_at` filter and `finished_at` filter can not be specified'
+ end
+
+ # Currently, the inefficient parameters are allowed in order to avoid breaking changes in Deployment API.
+ # We'll switch to a hard error in https://gitlab.com/gitlab-org/gitlab/-/issues/328500.
+ if (filter_by_updated_at? && !order_by_updated_at?) || (!filter_by_updated_at? && order_by_updated_at?)
+ Gitlab::ErrorTracking.track_and_raise_for_dev_exception(
+ InefficientQueryError.new('`updated_at` filter and `updated_at` sorting must be paired')
+ )
+ end
+
+ if (filter_by_finished_at? && !order_by_finished_at?) || (!filter_by_finished_at? && order_by_finished_at?)
+ raise InefficientQueryError, '`finished_at` filter and `finished_at` sorting must be paired'
+ end
+
+ if filter_by_finished_at? && !filter_by_successful_deployment?
+ raise InefficientQueryError, '`finished_at` filter must be combined with `success` status filter.'
+ end
+
+ if params[:environment].present? && !params[:project].present?
+ raise InefficientQueryError, '`environment` filter must be combined with `project` scope.'
+ end
+ end
+
def init_collection
if params[:project].present?
params[:project].deployments
@@ -49,6 +86,8 @@ class DeploymentsFinder
end
def sort(items)
+ sort_params = build_sort_params
+ optimize_sort_params!(sort_params)
items.order(sort_params) # rubocop: disable CodeReuse/ActiveRecord
end
@@ -67,8 +106,8 @@ class DeploymentsFinder
end
def by_environment(items)
- if params[:environment].present?
- items.for_environment_name(params[:environment])
+ if params[:project].present? && params[:environment].present?
+ items.for_environment_name(params[:project], params[:environment])
else
items
end
@@ -84,14 +123,58 @@ class DeploymentsFinder
items.for_status(params[:status])
end
- def sort_params
+ def build_sort_params
order_by = ALLOWED_SORT_VALUES.include?(params[:order_by]) ? params[:order_by] : DEFAULT_SORT_VALUE
order_direction = ALLOWED_SORT_DIRECTIONS.include?(params[:sort]) ? params[:sort] : DEFAULT_SORT_DIRECTION
- { order_by => order_direction }.tap do |sort_values|
- sort_values['id'] = 'desc' if sort_values['updated_at']
- sort_values['id'] = sort_values.delete('created_at') if sort_values['created_at'] # Sorting by `id` produces the same result as sorting by `created_at`
+ { order_by => order_direction }
+ end
+
+ def optimize_sort_params!(sort_params)
+ # Implicitly enforce the ordering when filtered by `updated_at` column for performance optimization.
+ # See https://gitlab.com/gitlab-org/gitlab/-/issues/325627#note_552417509.
+ # We remove this in https://gitlab.com/gitlab-org/gitlab/-/issues/328500.
+ if filter_by_updated_at? && implicitly_enforce_ordering_for_updated_at_filter?
+ sort_params.replace('updated_at' => sort_params.each_value.first)
end
+
+ if sort_params['created_at'] || sort_params['iid']
+ # Sorting by `id` produces the same result as sorting by `created_at` or `iid`
+ sort_params.replace(id: sort_params.each_value.first)
+ elsif sort_params['updated_at']
+ # This adds the order as a tie-breaker when multiple rows have the same updated_at value.
+ # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/20848.
+ sort_params.merge!(id: :desc)
+ end
+ end
+
+ def filter_by_updated_at?
+ params[:updated_before].present? || params[:updated_after].present?
+ end
+
+ def filter_by_finished_at?
+ params[:finished_before].present? || params[:finished_after].present?
+ end
+
+ def filter_by_successful_deployment?
+ params[:status].to_s == 'success'
+ end
+
+ def order_by_updated_at?
+ params[:order_by].to_s == 'updated_at'
+ end
+
+ def order_by_finished_at?
+ params[:order_by].to_s == 'finished_at'
+ end
+
+ def implicitly_enforce_ordering_for_updated_at_filter?
+ return false unless params[:project].present?
+
+ ::Feature.enabled?(
+ :deployments_finder_implicitly_enforce_ordering_for_updated_at_filter,
+ params[:project],
+ default_enabled: :yaml)
end
# rubocop: disable CodeReuse/ActiveRecord
diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 1ccf06cd539..50732f200ba 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -32,8 +32,8 @@ class Deployment < ApplicationRecord
delegate :kubernetes_namespace, to: :deployment_cluster, allow_nil: true
scope :for_environment, -> (environment) { where(environment_id: environment) }
- scope :for_environment_name, -> (name) do
- joins(:environment).where(environments: { name: name })
+ scope :for_environment_name, -> (project, name) do
+ where(environment_id: Environment.select(:id).where(project: project, name: name))
end
scope :for_status, -> (status) { where(status: status) }
diff --git a/changelogs/unreleased/optimize-deployment-finder.yml b/changelogs/unreleased/optimize-deployment-finder.yml
new file mode 100644
index 00000000000..964258298d9
--- /dev/null
+++ b/changelogs/unreleased/optimize-deployment-finder.yml
@@ -0,0 +1,5 @@
+---
+title: Recreate index for deployments updated_at and finished_at
+merge_request: 59771
+author:
+type: performance
diff --git a/config/feature_flags/development/deployments_finder_implicitly_enforce_ordering_for_updated_at_filter.yml b/config/feature_flags/development/deployments_finder_implicitly_enforce_ordering_for_updated_at_filter.yml
new file mode 100644
index 00000000000..bc8e3ca3997
--- /dev/null
+++ b/config/feature_flags/development/deployments_finder_implicitly_enforce_ordering_for_updated_at_filter.yml
@@ -0,0 +1,8 @@
+---
+name: deployments_finder_implicitly_enforce_ordering_for_updated_at_filter
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59771
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/329286
+milestone: '13.12'
+type: development
+group: group::release
+default_enabled: false
diff --git a/db/migrate/20210420210642_recreate_index_for_project_deployments_with_environment_id_and_date_at.rb b/db/migrate/20210420210642_recreate_index_for_project_deployments_with_environment_id_and_date_at.rb
new file mode 100644
index 00000000000..2674d9e50c8
--- /dev/null
+++ b/db/migrate/20210420210642_recreate_index_for_project_deployments_with_environment_id_and_date_at.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+# This migration recreates the index that introduced in 20210326035553_add_index_for_project_deployments_with_environment_id_and_updated_at.rb.
+class RecreateIndexForProjectDeploymentsWithEnvironmentIdAndDateAt < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ OLD_INDEX_NAME = 'index_deployments_on_project_and_environment_and_updated_at'
+ NEW_INDEX_NAME = 'index_deployments_on_project_and_environment_and_updated_at_id'
+
+ def up
+ add_concurrent_index :deployments, [:project_id, :environment_id, :updated_at, :id], name: NEW_INDEX_NAME
+ remove_concurrent_index_by_name :deployments, OLD_INDEX_NAME
+ end
+
+ def down
+ add_concurrent_index :deployments, [:project_id, :environment_id, :updated_at], name: OLD_INDEX_NAME
+ remove_concurrent_index_by_name :deployments, NEW_INDEX_NAME
+ end
+end
diff --git a/db/schema_migrations/20210420210642 b/db/schema_migrations/20210420210642
new file mode 100644
index 00000000000..dd5d165df5e
--- /dev/null
+++ b/db/schema_migrations/20210420210642
@@ -0,0 +1 @@
+b7f75e3b443bfcb1aea812ad1682a31a99021f41ef4d47bdf600437db6f4f2f3 \ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 94fb77644ca..561e1083326 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -22754,7 +22754,7 @@ CREATE INDEX index_deployments_on_id_and_status_and_created_at ON deployments US
CREATE INDEX index_deployments_on_id_where_cluster_id_present ON deployments USING btree (id) WHERE (cluster_id IS NOT NULL);
-CREATE INDEX index_deployments_on_project_and_environment_and_updated_at ON deployments USING btree (project_id, environment_id, updated_at);
+CREATE INDEX index_deployments_on_project_and_environment_and_updated_at_id ON deployments USING btree (project_id, environment_id, updated_at, id);
CREATE INDEX index_deployments_on_project_and_finished ON deployments USING btree (project_id, finished_at) WHERE (status = 2);
diff --git a/doc/development/code_review.md b/doc/development/code_review.md
index 75e2dc69821..3bd0a784ffb 100644
--- a/doc/development/code_review.md
+++ b/doc/development/code_review.md
@@ -432,6 +432,10 @@ When in doubt, ask someone from `@gitlab-com/gl-security/appsec` to review the m
enough to `master`.
- When you set the MR to "Merge When Pipeline Succeeds", you should take over
subsequent revisions for anything that would be spotted after that.
+- For merge requests that have had [Squash and
+ merge](../user/project/merge_requests/squash_and_merge.md#squash-and-merge) set,
+ the squashed commit’s default commit message is taken from the merge request title.
+ You're encouraged to [select a commit with a more informative commit message](../user/project/merge_requests/squash_and_merge.md#overview) before merging.
Thanks to **Pipeline for Merged Results**, authors no longer have to rebase their
branch as frequently anymore (only when there are conflicts) because the Merge
diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md
index b552a3f9a1f..6caab1ee383 100644
--- a/doc/user/application_security/index.md
+++ b/doc/user/application_security/index.md
@@ -104,19 +104,6 @@ rules:
- if: $CI_PIPELINE_SOURCE == "merge_request_event"
```
-## Security Scanning with Auto DevOps
-
-When [Auto DevOps](../../topics/autodevops/) is enabled, all GitLab Security scanning tools are configured using default settings.
-
-- [Auto SAST](../../topics/autodevops/stages.md#auto-sast)
-- [Auto Secret Detection](../../topics/autodevops/stages.md#auto-secret-detection)
-- [Auto DAST](../../topics/autodevops/stages.md#auto-dast)
-- [Auto Dependency Scanning](../../topics/autodevops/stages.md#auto-dependency-scanning)
-- [Auto License Compliance](../../topics/autodevops/stages.md#auto-license-compliance)
-- [Auto Container Scanning](../../topics/autodevops/stages.md#auto-container-scanning)
-
-While you cannot directly customize Auto DevOps, you can [include the Auto DevOps template in your project's `.gitlab-ci.yml` file](../../topics/autodevops/customize.md#customizing-gitlab-ciyml).
-
## View security scan information in merge requests **(FREE)**
> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4393) in GitLab Free 13.5.
diff --git a/lib/api/deployments.rb b/lib/api/deployments.rb
index 0a6ecf2919c..80a50ded522 100644
--- a/lib/api/deployments.rb
+++ b/lib/api/deployments.rb
@@ -41,6 +41,8 @@ module API
.execute.with_api_entity_associations
present paginate(deployments), with: Entities::Deployment
+ rescue DeploymentsFinder::InefficientQueryError => e
+ bad_request!(e.message)
end
desc 'Gets a specific deployment' do
diff --git a/lib/gitlab/cycle_analytics/summary/deploy.rb b/lib/gitlab/cycle_analytics/summary/deploy.rb
index c247ef0d2a7..e5bf6ef616f 100644
--- a/lib/gitlab/cycle_analytics/summary/deploy.rb
+++ b/lib/gitlab/cycle_analytics/summary/deploy.rb
@@ -16,7 +16,7 @@ module Gitlab
def deployments_count
DeploymentsFinder
- .new(project: @project, finished_after: @from, finished_before: @to, status: :success)
+ .new(project: @project, finished_after: @from, finished_before: @to, status: :success, order_by: :finished_at)
.execute
.count
end
diff --git a/spec/finders/deployments_finder_spec.rb b/spec/finders/deployments_finder_spec.rb
index 3db3be0b64a..0c8a06d7e28 100644
--- a/spec/finders/deployments_finder_spec.rb
+++ b/spec/finders/deployments_finder_spec.rb
@@ -5,6 +5,58 @@ require 'spec_helper'
RSpec.describe DeploymentsFinder do
subject { described_class.new(params).execute }
+ describe "validation" do
+ context 'when both updated_at and finished_at filters are specified' do
+ let(:params) { { updated_before: 1.day.ago, finished_before: 1.day.ago } }
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(
+ described_class::InefficientQueryError,
+ 'Both `updated_at` filter and `finished_at` filter can not be specified')
+ end
+ end
+
+ context 'when updated_at filter and id sorting' do
+ let(:params) { { updated_before: 1.day.ago, order_by: :id } }
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(
+ described_class::InefficientQueryError,
+ '`updated_at` filter and `updated_at` sorting must be paired')
+ end
+ end
+
+ context 'when finished_at filter and id sorting' do
+ let(:params) { { finished_before: 1.day.ago, order_by: :id } }
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(
+ described_class::InefficientQueryError,
+ '`finished_at` filter and `finished_at` sorting must be paired')
+ end
+ end
+
+ context 'when finished_at filter with failed status filter' do
+ let(:params) { { finished_before: 1.day.ago, order_by: :finished_at, status: :failed } }
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(
+ described_class::InefficientQueryError,
+ '`finished_at` filter must be combined with `success` status filter.')
+ end
+ end
+
+ context 'when environment filter with non-project scope' do
+ let(:params) { { environment: 'production' } }
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(
+ described_class::InefficientQueryError,
+ '`environment` filter must be combined with `project` scope.')
+ end
+ end
+ end
+
describe "#execute" do
context 'when project or group is missing' do
let(:params) { {} }
@@ -20,13 +72,24 @@ RSpec.describe DeploymentsFinder do
describe 'filtering' do
context 'when updated_at filters are specified' do
- let(:params) { { **base_params, updated_before: 1.day.ago, updated_after: 3.days.ago } }
- let!(:deployment_1) { create(:deployment, :success, project: project, updated_at: 2.days.ago) }
- let!(:deployment_2) { create(:deployment, :success, project: project, updated_at: 4.days.ago) }
- let!(:deployment_3) { create(:deployment, :success, project: project, updated_at: 1.hour.ago) }
+ let_it_be(:deployment_1) { create(:deployment, :success, project: project, updated_at: 48.hours.ago) }
+ let_it_be(:deployment_2) { create(:deployment, :success, project: project, updated_at: 47.hours.ago) }
+ let_it_be(:deployment_3) { create(:deployment, :success, project: project, updated_at: 4.days.ago) }
+ let_it_be(:deployment_4) { create(:deployment, :success, project: project, updated_at: 1.hour.ago) }
+ let(:params) { { **base_params, updated_before: 1.day.ago, updated_after: 3.days.ago, order_by: :updated_at } }
it 'returns deployments with matched updated_at' do
- is_expected.to match_array([deployment_1])
+ is_expected.to match_array([deployment_2, deployment_1])
+ end
+
+ context 'when deployments_finder_implicitly_enforce_ordering_for_updated_at_filter feature flag is disabled' do
+ before do
+ stub_feature_flags(deployments_finder_implicitly_enforce_ordering_for_updated_at_filter: false)
+ end
+
+ it 'returns deployments with matched updated_at' do
+ is_expected.to match_array([deployment_1, deployment_2])
+ end
end
end
@@ -72,30 +135,34 @@ RSpec.describe DeploymentsFinder do
let(:params) { { **base_params, order_by: order_by, sort: sort } }
- let!(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: 2.days.ago, updated_at: Time.now, finished_at: Time.now) }
- let!(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'feature', created_at: 1.day.ago, updated_at: 2.hours.ago, finished_at: 2.hours.ago) }
- let!(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'video', created_at: Time.now, updated_at: 1.hour.ago, finished_at: 1.hour.ago) }
+ let!(:deployment_1) { create(:deployment, :success, project: project, ref: 'master', created_at: 2.days.ago, updated_at: Time.now, finished_at: Time.now) }
+ let!(:deployment_2) { create(:deployment, :success, project: project, ref: 'feature', created_at: 1.day.ago, updated_at: 2.hours.ago, finished_at: 2.hours.ago) }
+ let!(:deployment_3) { create(:deployment, :success, project: project, ref: 'video', created_at: Time.now, updated_at: 1.hour.ago, finished_at: 1.hour.ago) }
where(:order_by, :sort, :ordered_deployments) do
'created_at' | 'asc' | [:deployment_1, :deployment_2, :deployment_3]
'created_at' | 'desc' | [:deployment_3, :deployment_2, :deployment_1]
'id' | 'asc' | [:deployment_1, :deployment_2, :deployment_3]
'id' | 'desc' | [:deployment_3, :deployment_2, :deployment_1]
- 'iid' | 'asc' | [:deployment_3, :deployment_1, :deployment_2]
- 'iid' | 'desc' | [:deployment_2, :deployment_1, :deployment_3]
+ 'iid' | 'asc' | [:deployment_1, :deployment_2, :deployment_3]
+ 'iid' | 'desc' | [:deployment_3, :deployment_2, :deployment_1]
'ref' | 'asc' | [:deployment_2, :deployment_1, :deployment_3]
'ref' | 'desc' | [:deployment_3, :deployment_1, :deployment_2]
- 'updated_at' | 'asc' | [:deployment_2, :deployment_3, :deployment_1]
- 'updated_at' | 'desc' | [:deployment_1, :deployment_3, :deployment_2]
- 'finished_at' | 'asc' | [:deployment_2, :deployment_3, :deployment_1]
- 'finished_at' | 'desc' | [:deployment_1, :deployment_3, :deployment_2]
+ 'updated_at' | 'asc' | described_class::InefficientQueryError
+ 'updated_at' | 'desc' | described_class::InefficientQueryError
+ 'finished_at' | 'asc' | described_class::InefficientQueryError
+ 'finished_at' | 'desc' | described_class::InefficientQueryError
'invalid' | 'asc' | [:deployment_1, :deployment_2, :deployment_3]
- 'iid' | 'err' | [:deployment_3, :deployment_1, :deployment_2]
+ 'iid' | 'err' | [:deployment_1, :deployment_2, :deployment_3]
end
with_them do
it 'returns the deployments ordered' do
- expect(subject).to eq(ordered_deployments.map { |name| public_send(name) })
+ if ordered_deployments == described_class::InefficientQueryError
+ expect { subject }.to raise_error(described_class::InefficientQueryError)
+ else
+ expect(subject).to eq(ordered_deployments.map { |name| public_send(name) })
+ end
end
end
end
@@ -112,8 +179,20 @@ RSpec.describe DeploymentsFinder do
end
end
- describe 'tie-breaker for `finished_at` sorting' do
- let(:params) { { **base_params, order_by: 'updated_at', sort: 'asc' } }
+ describe 'transform `iid` sorting to `id` sorting' do
+ let(:params) { { **base_params, order_by: 'iid', sort: 'asc' } }
+
+ it 'sorts by only one column' do
+ expect(subject.order_values.size).to eq(1)
+ end
+
+ it 'sorts by `id`' do
+ expect(subject.order_values.first.to_sql).to eq(Deployment.arel_table[:id].asc.to_sql)
+ end
+ end
+
+ describe 'tie-breaker for `updated_at` sorting' do
+ let(:params) { { **base_params, updated_after: 1.day.ago, order_by: 'updated_at', sort: 'asc' } }
it 'sorts by two columns' do
expect(subject.order_values.size).to eq(2)
@@ -136,25 +215,56 @@ RSpec.describe DeploymentsFinder do
end
end
+ describe 'enforce sorting to `updated_at` sorting' do
+ let(:params) { { **base_params, updated_before: 1.day.ago, order_by: 'id', sort: 'asc' } }
+
+ before do
+ allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
+ end
+
+ it 'sorts by only one column' do
+ expect(subject.order_values.size).to eq(2)
+ end
+
+ it 'sorts by `updated_at`' do
+ expect(subject.order_values.first.to_sql).to eq(Deployment.arel_table[:updated_at].asc.to_sql)
+ expect(subject.order_values.second.to_sql).to eq(Deployment.arel_table[:id].desc.to_sql)
+ end
+
+ context 'when deployments_finder_implicitly_enforce_ordering_for_updated_at_filter feature flag is disabled' do
+ before do
+ stub_feature_flags(deployments_finder_implicitly_enforce_ordering_for_updated_at_filter: false)
+ end
+
+ it 'sorts by only one column' do
+ expect(subject.order_values.size).to eq(1)
+ end
+
+ it 'sorts by `id`' do
+ expect(subject.order_values.first.to_sql).to eq(Deployment.arel_table[:id].asc.to_sql)
+ end
+ end
+ end
+
context 'when filtering by finished time' do
let!(:deployment_1) { create(:deployment, :success, project: project, finished_at: 2.days.ago) }
let!(:deployment_2) { create(:deployment, :success, project: project, finished_at: 4.days.ago) }
let!(:deployment_3) { create(:deployment, :success, project: project, finished_at: 5.hours.ago) }
context 'when filtering by finished_after and finished_before' do
- let(:params) { { **base_params, finished_after: 3.days.ago, finished_before: 1.day.ago } }
+ let(:params) { { **base_params, finished_after: 3.days.ago, finished_before: 1.day.ago, status: :success, order_by: :finished_at } }
it { is_expected.to match_array([deployment_1]) }
end
context 'when the finished_before parameter is missing' do
- let(:params) { { **base_params, finished_after: 3.days.ago } }
+ let(:params) { { **base_params, finished_after: 3.days.ago, status: :success, order_by: :finished_at } }
it { is_expected.to match_array([deployment_1, deployment_3]) }
end
context 'when finished_after is missing' do
- let(:params) { { **base_params, finished_before: 3.days.ago } }
+ let(:params) { { **base_params, finished_before: 3.days.ago, status: :success, order_by: :finished_at } }
it { is_expected.to match_array([deployment_2]) }
end
@@ -188,10 +298,6 @@ RSpec.describe DeploymentsFinder do
'iid' | 'desc'
'ref' | 'asc'
'ref' | 'desc'
- 'updated_at' | 'asc'
- 'updated_at' | 'desc'
- 'finished_at' | 'asc'
- 'finished_at' | 'desc'
'invalid' | 'asc'
'iid' | 'err'
end
diff --git a/spec/frontend/vue_shared/components/vuex_module_provider_spec.js b/spec/frontend/vue_shared/components/vuex_module_provider_spec.js
index 01fe5af49fd..ebd396bd87c 100644
--- a/spec/frontend/vue_shared/components/vuex_module_provider_spec.js
+++ b/spec/frontend/vue_shared/components/vuex_module_provider_spec.js
@@ -1,5 +1,6 @@
-import { mount } from '@vue/test-utils';
+import { mount, createLocalVue } from '@vue/test-utils';
import Vue from 'vue';
+import VueApollo from 'vue-apollo';
import VuexModuleProvider from '~/vue_shared/components/vuex_module_provider.vue';
const TestComponent = Vue.extend({
@@ -14,7 +15,7 @@ describe('~/vue_shared/components/vuex_module_provider', () => {
const findProvidedVuexModule = () => wrapper.find('[data-testid="vuexModule"]').text();
- beforeEach(() => {
+ const createComponent = (extraParams = {}) => {
wrapper = mount(VuexModuleProvider, {
propsData: {
vuexModule: TEST_VUEX_MODULE,
@@ -22,10 +23,25 @@ describe('~/vue_shared/components/vuex_module_provider', () => {
slots: {
default: TestComponent,
},
+ ...extraParams,
});
+ };
+
+ afterEach(() => {
+ wrapper.destroy();
});
it('provides "vuexModule" set from prop', () => {
+ createComponent();
+ expect(findProvidedVuexModule()).toBe(TEST_VUEX_MODULE);
+ });
+
+ it('does not blow up when used with vue-apollo', () => {
+ // See https://github.com/vuejs/vue-apollo/pull/1153 for details
+ const localVue = createLocalVue();
+ localVue.use(VueApollo);
+
+ createComponent({ localVue });
expect(findProvidedVuexModule()).toBe(TEST_VUEX_MODULE);
});
});
diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb
index 64c7311a84f..bcd237cbd38 100644
--- a/spec/models/deployment_spec.rb
+++ b/spec/models/deployment_spec.rb
@@ -72,6 +72,35 @@ RSpec.describe Deployment do
end
end
+ describe '.for_environment_name' do
+ subject { described_class.for_environment_name(project, environment_name) }
+
+ let_it_be(:project) { create(:project, :repository) }
+ let_it_be(:production) { create(:environment, :production, project: project) }
+ let_it_be(:staging) { create(:environment, :staging, project: project) }
+ let_it_be(:other_project) { create(:project, :repository) }
+ let_it_be(:other_production) { create(:environment, :production, project: other_project) }
+ let(:environment_name) { production.name }
+
+ context 'when deployment belongs to the environment' do
+ let!(:deployment) { create(:deployment, project: project, environment: production) }
+
+ it { is_expected.to eq([deployment]) }
+ end
+
+ context 'when deployment belongs to the same project but different environment name' do
+ let!(:deployment) { create(:deployment, project: project, environment: staging) }
+
+ it { is_expected.to be_empty }
+ end
+
+ context 'when deployment belongs to the same environment name but different project' do
+ let!(:deployment) { create(:deployment, project: other_project, environment: other_production) }
+
+ it { is_expected.to be_empty }
+ end
+ end
+
describe '.success' do
subject { described_class.success }
diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb
index c89c59a2151..bbfe37cb70b 100644
--- a/spec/requests/api/deployments_spec.rb
+++ b/spec/requests/api/deployments_spec.rb
@@ -12,9 +12,11 @@ RSpec.describe API::Deployments do
describe 'GET /projects/:id/deployments' do
let_it_be(:project) { create(:project, :repository) }
- let_it_be(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: Time.now, updated_at: Time.now) }
- let_it_be(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'master', created_at: 1.day.ago, updated_at: 2.hours.ago) }
- let_it_be(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'master', created_at: 2.days.ago, updated_at: 1.hour.ago) }
+ let_it_be(:production) { create(:environment, :production, project: project) }
+ let_it_be(:staging) { create(:environment, :staging, project: project) }
+ let_it_be(:deployment_1) { create(:deployment, :success, project: project, environment: production, ref: 'master', created_at: Time.now, updated_at: Time.now) }
+ let_it_be(:deployment_2) { create(:deployment, :success, project: project, environment: staging, ref: 'master', created_at: 1.day.ago, updated_at: 2.hours.ago) }
+ let_it_be(:deployment_3) { create(:deployment, :success, project: project, environment: staging, ref: 'master', created_at: 2.days.ago, updated_at: 1.hour.ago) }
def perform_request(params = {})
get api("/projects/#{project.id}/deployments", user), params: params
@@ -36,17 +38,26 @@ RSpec.describe API::Deployments do
context 'with updated_at filters specified' do
it 'returns projects deployments with last update in specified datetime range' do
- perform_request({ updated_before: 30.minutes.ago, updated_after: 90.minutes.ago })
+ perform_request({ updated_before: 30.minutes.ago, updated_after: 90.minutes.ago, order_by: :updated_at })
expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_pagination_headers
expect(json_response.first['id']).to eq(deployment_3.id)
end
+
+ context 'when forbidden order_by is specified' do
+ it 'returns projects deployments with last update in specified datetime range' do
+ perform_request({ updated_before: 30.minutes.ago, updated_after: 90.minutes.ago, order_by: :id })
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ expect(json_response['message']).to include('`updated_at` filter and `updated_at` sorting must be paired')
+ end
+ end
end
context 'with the environment filter specifed' do
it 'returns deployments for the environment' do
- perform_request({ environment: deployment_1.environment.name })
+ perform_request({ environment: production.name })
expect(json_response.size).to eq(1)
expect(json_response.first['iid']).to eq(deployment_1.iid)
@@ -68,7 +79,7 @@ RSpec.describe API::Deployments do
end
it 'returns ordered deployments' do
- expect(json_response.map { |i| i['id'] }).to eq([deployment_2.id, deployment_1.id, deployment_3.id])
+ expect(json_response.map { |i| i['id'] }).to eq([deployment_3.id, deployment_2.id, deployment_1.id])
end
context 'with invalid order_by' do
@@ -475,7 +486,7 @@ RSpec.describe API::Deployments do
let(:project) { create(:project, :repository) }
let!(:deployment) { create(:deployment, :success, project: project) }
- subject { get api("/projects/#{project.id}/deployments?order_by=updated_at&sort=asc", user) }
+ subject { get api("/projects/#{project.id}/deployments?order_by=id&sort=asc", user) }
it 'succeeds', :aggregate_failures do
subject