Commit a5a44490 authored by Nick Thomas's avatar Nick Thomas 🌴 Committed by Mayra Cabrera

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 eb3f465e
......@@ -9,12 +9,10 @@ module ProjectAPICompatibility
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 @@ describe ProjectAPICompatibility do
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)
end
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
# 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)
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)
expect(project.auto_devops_enabled?).to eq(expected)
end
end
end
# 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')
describe '#auto_devops_deploy_strategy' do
where(
initial: [:missing, *ProjectAutoDevops.deploy_strategies.keys],
final: ProjectAutoDevops.deploy_strategies.keys
)
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