Commit 8f0a2b6d authored by Shinya Maeda's avatar Shinya Maeda

Implement Ci::NestedUniquenessValidator

parent 6128f286
......@@ -33,7 +33,8 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
end
def update
if schedule.update(schedule_params)
if Ci::UpdatePipelineScheduleService
.new(@project, current_user, schedule_params).execute(schedule)
redirect_to namespace_project_pipeline_schedules_path(@project.namespace.becomes(Namespace), @project)
else
render :edit
......
......@@ -15,11 +15,6 @@ module Ci
validates :cron_timezone, cron_timezone: true, presence: { unless: :importing? }
validates :ref, presence: { unless: :importing? }
validates :description, presence: true
validates :variables, uniqueness_of_in_memory: {
:collection => :variables,
:attrs => [:pipeline_schedule_id, :key],
:message => ['variables.key', 'keys are duplicated']
}
before_save :set_next_run_at
......
module Ci
class CreatePipelineScheduleService < BaseService
def execute
project.pipeline_schedules.create(pipeline_schedule_params)
pipeline_schedule = project.pipeline_schedules.build(pipeline_schedule_params)
if Ci::NestedUniquenessValidator.duplicated?(pipeline_schedule_params['variables_attributes'], 'key')
pipeline_schedule.errors.add('variables.key', "keys are duplicated")
return pipeline_schedule
end
pipeline_schedule.save
pipeline_schedule
end
private
def pipeline_schedule_params
params.merge(owner: current_user)
@pipeline_schedule_params ||= params.merge(owner: current_user)
end
end
end
module Ci
class UpdatePipelineScheduleService < BaseService
def execute(pipeline_schedule)
if Ci::NestedUniquenessValidator.duplicated?(pipeline_schedule_params['variables_attributes'], 'key')
pipeline_schedule.errors.add('variables.key', "keys are duplicated")
return false
end
pipeline_schedule.update(pipeline_schedule_params)
end
private
def pipeline_schedule_params
@pipeline_schedule_params ||= params.merge(owner: current_user)
end
end
end
# UniquenessOfInMemoryValidator
#
# This validtor is designed for especially the following condition
# - Use `accepts_nested_attributes_for :xxx` in a parent model
# - Use `validates :xxx, uniqueness: { scope: :xxx_id }` in a child model
#
# Inspired by https://stackoverflow.com/a/2883129/2522666
module ActiveRecord
class Base
# Validate that the the objects in +collection+ are unique
# when compared against all their non-blank +attrs+. If not
# add +message+ to the base errors.
def validate_uniqueness_of_in_memory(collection, attrs, message)
hashes = collection.inject({}) do |hash, record|
key = attrs.map { |a| record.send(a).to_s }.join
if key.blank? || record.marked_for_destruction?
key = record.object_id
end
hash[key] = record unless hash[key]
hash
end
if collection.length > hashes.length
self.errors.add(*message)
end
end
end
end
class UniquenessOfInMemoryValidator < ActiveModel::Validator
def validate(record)
record.validate_uniqueness_of_in_memory(
record.public_send(options[:collection]),
options[:attrs],
options[:message])
end
end
module Ci
module NestedUniquenessValidator
class << self
def duplicated?(nested_attributes, unique_key)
return false unless nested_attributes.is_a?(Array)
nested_attributes.map { |v| v[unique_key] }.uniq.length != nested_attributes.length
end
end
end
end
......@@ -267,8 +267,7 @@ describe Projects::PipelineSchedulesController do
end
it 'returns an error that variables are duplciated' do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
go
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
end
......@@ -340,6 +339,21 @@ describe Projects::PipelineSchedulesController do
expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(-1)
end
end
context 'when deletes and creates the same keys' do
let(:schedule) do
basic_param.merge({
variables_attributes: [{ id: pipeline_schedule_variable.id, key: pipeline_schedule_variable.key, _destroy: true },
{ key: pipeline_schedule_variable.key, value: 'new_value' }]
})
end
it 'returns an error that variables are duplciated' do
go
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
end
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