Revert extra validation for duplication between same keys on a submit
This commit is contained in:
parent
6c5c525a2a
commit
f433fee057
|
@ -33,8 +33,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
|
|||
end
|
||||
|
||||
def update
|
||||
if Ci::CreatePipelineScheduleService
|
||||
.new(@project, current_user, schedule_params).update(schedule)
|
||||
if schedule.update(schedule_params)
|
||||
redirect_to namespace_project_pipeline_schedules_path(@project.namespace.becomes(Namespace), @project)
|
||||
else
|
||||
render :edit
|
||||
|
|
|
@ -1,39 +1,13 @@
|
|||
module Ci
|
||||
class CreatePipelineScheduleService < BaseService
|
||||
def execute
|
||||
pipeline_schedule = project.pipeline_schedules.build(pipeline_schedule_params)
|
||||
|
||||
if variable_keys_duplicated?
|
||||
pipeline_schedule.errors.add('variables.key', "keys are duplicated")
|
||||
|
||||
return pipeline_schedule
|
||||
end
|
||||
|
||||
pipeline_schedule.save
|
||||
pipeline_schedule
|
||||
end
|
||||
|
||||
def update(pipeline_schedule)
|
||||
if variable_keys_duplicated?
|
||||
pipeline_schedule.errors.add('variables.key', "keys are duplicated")
|
||||
|
||||
return false
|
||||
end
|
||||
|
||||
pipeline_schedule.update(pipeline_schedule_params)
|
||||
project.pipeline_schedules.create(pipeline_schedule_params)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def pipeline_schedule_params
|
||||
@pipeline_schedule_params ||= params.merge(owner: current_user)
|
||||
end
|
||||
|
||||
def variable_keys_duplicated?
|
||||
attributes = pipeline_schedule_params['variables_attributes']
|
||||
return false unless attributes.is_a?(Array)
|
||||
|
||||
attributes.map { |v| v['key'] }.uniq.length != attributes.length
|
||||
params.merge(owner: current_user)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -142,21 +142,22 @@ describe Projects::PipelineSchedulesController do
|
|||
end
|
||||
end
|
||||
|
||||
context 'when variables_attributes has two variables and duplicted' do
|
||||
let(:schedule) do
|
||||
basic_param.merge({
|
||||
variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' } ]
|
||||
})
|
||||
end
|
||||
# This test no longer passes, since we removed a custom validation
|
||||
# context 'when variables_attributes has two variables and duplicted' do
|
||||
# let(:schedule) do
|
||||
# basic_param.merge({
|
||||
# variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' } ]
|
||||
# })
|
||||
# end
|
||||
|
||||
it 'returns an error that the keys of variable are duplicated' do
|
||||
expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
|
||||
.to change { Ci::PipelineSchedule.count }.by(0)
|
||||
.and change { Ci::PipelineScheduleVariable.count }.by(0)
|
||||
# it 'returns an error that the keys of variable are duplicated' do
|
||||
# expect { post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule }
|
||||
# .to change { Ci::PipelineSchedule.count }.by(0)
|
||||
# .and change { Ci::PipelineScheduleVariable.count }.by(0)
|
||||
|
||||
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
|
||||
end
|
||||
end
|
||||
# expect(assigns(:schedule).errors['variables.key']).not_to be_empty
|
||||
# end
|
||||
# end
|
||||
end
|
||||
|
||||
describe 'security' do
|
||||
|
@ -260,20 +261,21 @@ describe Projects::PipelineSchedulesController do
|
|||
end
|
||||
end
|
||||
|
||||
context 'when params include two duplicated variables' do
|
||||
let(:schedule) do
|
||||
basic_param.merge({
|
||||
variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' } ]
|
||||
})
|
||||
end
|
||||
# This test no longer passes, since we removed a custom validation
|
||||
# context 'when params include two duplicated variables' do
|
||||
# let(:schedule) do
|
||||
# basic_param.merge({
|
||||
# variables_attributes: [ { key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' } ]
|
||||
# })
|
||||
# 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
|
||||
# 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
|
||||
|
||||
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
|
||||
end
|
||||
end
|
||||
# expect(assigns(:schedule).errors['variables.key']).not_to be_empty
|
||||
# end
|
||||
# end
|
||||
end
|
||||
|
||||
context 'when a pipeline schedule has one variable' do
|
||||
|
|
Loading…
Reference in New Issue