Commit 5ca65cb8 authored by Bala Kumar's avatar Bala Kumar

Modify Union sql construction

Changelog: fixed
parent eb835d99
...@@ -8,7 +8,6 @@ class Deployment < ApplicationRecord ...@@ -8,7 +8,6 @@ class Deployment < ApplicationRecord
include Importable include Importable
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
include FastDestroyAll include FastDestroyAll
include FromUnion
StatusUpdateError = Class.new(StandardError) StatusUpdateError = Class.new(StandardError)
StatusSyncError = Class.new(StandardError) StatusSyncError = Class.new(StandardError)
......
...@@ -21,11 +21,13 @@ module Preloaders ...@@ -21,11 +21,13 @@ module Preloaders
def load_deployment_association(association_name, association_attributes) def load_deployment_association(association_name, association_attributes)
return unless environments.present? return unless environments.present?
union_arg = environments.inject([]) do |result, environment| # Not using Gitlab::SQL::Union as `order_by` in the SQL constructed is ignored.
result << environment.association(association_name).scope # See:
end # 1) https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/sql/union.rb#L7
# 2) https://gitlab.com/gitlab-org/gitlab/-/issues/353966#note_860928647
union_sql = Deployment.from_union(union_arg).to_sql union_sql = environments.map do |environment|
"(#{environment.association(association_name).scope.to_sql})"
end.join(' UNION ')
deployments = Deployment deployments = Deployment
.from("(#{union_sql}) #{::Deployment.table_name}") .from("(#{union_sql}) #{::Deployment.table_name}")
......
...@@ -6,7 +6,7 @@ RSpec.describe Projects::EnvironmentsController do ...@@ -6,7 +6,7 @@ RSpec.describe Projects::EnvironmentsController do
include MetricsDashboardHelpers include MetricsDashboardHelpers
include KubernetesHelpers include KubernetesHelpers
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project, :repository) }
let_it_be(:maintainer) { create(:user, name: 'main-dos').tap { |u| project.add_maintainer(u) } } let_it_be(:maintainer) { create(:user, name: 'main-dos').tap { |u| project.add_maintainer(u) } }
let_it_be(:reporter) { create(:user, name: 'repo-dos').tap { |u| project.add_reporter(u) } } let_it_be(:reporter) { create(:user, name: 'repo-dos').tap { |u| project.add_reporter(u) } }
...@@ -55,11 +55,11 @@ RSpec.describe Projects::EnvironmentsController do ...@@ -55,11 +55,11 @@ RSpec.describe Projects::EnvironmentsController do
let(:environments) { json_response['environments'] } let(:environments) { json_response['environments'] }
context 'with default parameters' do context 'with default parameters' do
before do subject { get :index, params: environment_params(format: :json) }
get :index, params: environment_params(format: :json)
end
it 'responds with a flat payload describing available environments' do it 'responds with a flat payload describing available environments' do
subject
expect(environments.count).to eq 3 expect(environments.count).to eq 3
expect(environments.first).to include('name' => 'production', 'name_without_type' => 'production') expect(environments.first).to include('name' => 'production', 'name_without_type' => 'production')
expect(environments.second).to include('name' => 'staging/review-1', 'name_without_type' => 'review-1') expect(environments.second).to include('name' => 'staging/review-1', 'name_without_type' => 'review-1')
...@@ -69,9 +69,28 @@ RSpec.describe Projects::EnvironmentsController do ...@@ -69,9 +69,28 @@ RSpec.describe Projects::EnvironmentsController do
end end
it 'sets the polling interval header' do it 'sets the polling interval header' do
subject
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response.headers['Poll-Interval']).to eq("3000") expect(response.headers['Poll-Interval']).to eq("3000")
end end
context 'validates latest deployment' do
let_it_be(:test_environment) do
create(:environment, project: project, name: 'staging/review-4', state: :available)
end
before do
create_list(:deployment, 2, :success, environment: test_environment, project: project)
end
it 'responds with the latest deployment for the environment' do
subject
environment = environments.find { |env| env['id'] == test_environment.id }
expect(environment['last_deployment']['id']).to eq(test_environment.deployments.last.id)
end
end
end end
context 'when a folder-based nested structure is requested' do context 'when a folder-based nested structure is requested' do
......
...@@ -204,6 +204,25 @@ RSpec.describe EnvironmentSerializer do ...@@ -204,6 +204,25 @@ RSpec.describe EnvironmentSerializer do
json json
end end
# Validates possible bug that can arise when order_by is not honoured in the preloader.
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/353966#note_861381504
it 'fetches the last and upcoming deployment correctly' do
last_deployment = nil
upcoming_deployment = nil
create(:environment, project: project).tap do |environment|
create(:deployment, :success, environment: environment, project: project)
last_deployment = create(:deployment, :success, environment: environment, project: project)
create(:deployment, :running, environment: environment, project: project)
upcoming_deployment = create(:deployment, :running, environment: environment, project: project)
end
response_json = json
expect(response_json.last[:last_deployment][:id]).to eq(last_deployment.id)
expect(response_json.last[:upcoming_deployment][:id]).to eq(upcoming_deployment.id)
end
end end
def create_environment_with_associations(project) def create_environment_with_associations(project)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment