Read about our upcoming Code of Conduct on this issue

Commit 4257b9a3 authored by Nick Thomas's avatar Nick Thomas Committed by Mayra Cabrera
Browse files

Fix the project auto devops API

If `project_auto_devops.enabled` is nil for a project, when setting any
auto devops values via the API, we try to create a new row in the DB,
instead of re-using the existing one. This leads to the project_id
being set to nil, and the database `NOT NULL` constraint leading to a
500 response.

This commit resolves the issue by correctly detecting the presence of a
ProjectAutoDevops row and re-using it. Persistence is also moved away
from explicit `update!` calls and into relying on `autosave: true` on
the model.
parent dab600ceadec
......@@ -9,12 +9,10 @@ def build_git_strategy=(value)
end
def auto_devops_enabled=(value)
self.build_auto_devops if self.auto_devops&.enabled.nil?
self.auto_devops.update! enabled: value
(auto_devops || build_auto_devops).enabled = value
end
def auto_devops_deploy_strategy=(value)
self.build_auto_devops if self.auto_devops&.enabled.nil?
self.auto_devops.update! deploy_strategy: value
(auto_devops || build_auto_devops).deploy_strategy = value
end
end
......@@ -277,7 +277,7 @@ class Project < ApplicationRecord
has_many :project_deploy_tokens
has_many :deploy_tokens, through: :project_deploy_tokens
has_one :auto_devops, class_name: 'ProjectAutoDevops'
has_one :auto_devops, class_name: 'ProjectAutoDevops', inverse_of: :project, autosave: true
has_many :custom_attributes, class_name: 'ProjectCustomAttribute'
has_many :project_badges, class_name: 'ProjectBadge'
......
......@@ -5,7 +5,7 @@ class ProjectAutoDevops < ApplicationRecord
ignore_column :domain
belongs_to :project
belongs_to :project, inverse_of: :auto_devops
enum deploy_strategy: {
continuous: 0,
......
---
title: Fix the project auto devops API
merge_request: 30946
author:
type: fixed
......@@ -16,23 +16,44 @@
expect(project.build_allow_git_fetch).to eq(false)
end
# auto_devops_enabled
it "converts auto_devops_enabled=false to auto_devops_enabled?=false" do
expect(project.auto_devops_enabled?).to eq(true)
project.update!(auto_devops_enabled: false)
expect(project.auto_devops_enabled?).to eq(false)
describe '#auto_devops_enabled' do
where(
initial: [:missing, nil, false, true],
final: [nil, false, true]
)
with_them do
before do
project.build_auto_devops(enabled: initial) unless initial == :missing
end
it "converts auto_devops_enabled=true to auto_devops_enabled?=true" do
expect(project.auto_devops_enabled?).to eq(true)
project.update!(auto_devops_enabled: true)
expect(project.auto_devops_enabled?).to eq(true)
# Implicit auto devops when enabled is nil
let(:expected) { final.nil? ? true : final }
it 'sets the correct value' do
project.update!(auto_devops_enabled: final)
expect(project.auto_devops_enabled?).to eq(expected)
end
end
end
describe '#auto_devops_deploy_strategy' do
where(
initial: [:missing, *ProjectAutoDevops.deploy_strategies.keys],
final: ProjectAutoDevops.deploy_strategies.keys
)
# auto_devops_deploy_strategy
it "converts auto_devops_deploy_strategy=timed_incremental to auto_devops.deploy_strategy=timed_incremental" do
expect(project.auto_devops).to be_nil
project.update!(auto_devops_deploy_strategy: 'timed_incremental')
expect(project.auto_devops.deploy_strategy).to eq('timed_incremental')
with_them do
before do
project.build_auto_devops(deploy_strategy: initial) unless initial == :missing
end
it 'sets the correct value' do
project.update!(auto_devops_deploy_strategy: final)
expect(project.auto_devops.deploy_strategy).to eq(final)
end
end
end
end
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