From f8391bd782ef9a19b6c8595331ada49721bd89be Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Fri, 7 Jul 2017 01:24:49 +0900
Subject: Remove validates :key, uniqueness due to new validator covers the
 case

---
 app/models/ci/pipeline_schedule_variable.rb        |  2 -
 .../projects/pipeline_schedules_controller_spec.rb | 98 ++++++++++++++--------
 2 files changed, 64 insertions(+), 36 deletions(-)

diff --git a/app/models/ci/pipeline_schedule_variable.rb b/app/models/ci/pipeline_schedule_variable.rb
index ee5b8733fac..1ff177616e8 100644
--- a/app/models/ci/pipeline_schedule_variable.rb
+++ b/app/models/ci/pipeline_schedule_variable.rb
@@ -4,7 +4,5 @@ module Ci
     include HasVariable
 
     belongs_to :pipeline_schedule
-
-    validates :key, uniqueness: { scope: :pipeline_schedule_id }
   end
 end
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index d28dd1e3e48..41bf5580993 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -189,47 +189,77 @@ describe Projects::PipelineSchedulesController do
             key: 'CCC', pipeline_schedule: pipeline_schedule)
         end
 
-        context 'when params include one variable' do
-          context 'when adds a new variable' do
-            let(:schedule) do
-              basic_param.merge({
-                variables_attributes: [{ key: 'AAA', value: 'AAA123' }]
-              })
-            end
-
-            it 'adds the new variable' do
-              expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(1)
-
-              pipeline_schedule.reload
-              expect(pipeline_schedule.variables.last.key).to eq('AAA')
-            end
+        context 'when adds a new variable' do
+          let(:schedule) do
+            basic_param.merge({
+              variables_attributes: [{ key: 'AAA', value: 'AAA123' }]
+            })
+          end
+
+          it 'adds the new variable' do
+            expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(1)
+
+            pipeline_schedule.reload
+            expect(pipeline_schedule.variables.last.key).to eq('AAA')
+          end
+        end
+
+        context 'when adds a new duplicated variable' do
+          let(:schedule) do
+            basic_param.merge({
+              variables_attributes: [{ key: 'CCC', value: 'AAA123' }]
+            })
+          end
+
+          it 'returns an error' do
+            expect { go }.not_to change { Ci::PipelineScheduleVariable.count }
+
+            pipeline_schedule.reload
+            expect(assigns(:schedule).errors['variables']).not_to be_empty
           end
+        end
 
-          context 'when updates a variable' do
-            let(:schedule) do
-              basic_param.merge({
-                variables_attributes: [{ id: pipeline_schedule_variable.id, value: 'new_value' }]
-              })
-            end
+        context 'when updates a variable' do
+          let(:schedule) do
+            basic_param.merge({
+              variables_attributes: [{ id: pipeline_schedule_variable.id, value: 'new_value' }]
+            })
+          end
 
-            it 'updates the variable' do
-              expect { go }.not_to change { Ci::PipelineScheduleVariable.count }
+          it 'updates the variable' do
+            expect { go }.not_to change { Ci::PipelineScheduleVariable.count }
 
-              pipeline_schedule_variable.reload
-              expect(pipeline_schedule_variable.value).to eq('new_value')
-            end
+            pipeline_schedule_variable.reload
+            expect(pipeline_schedule_variable.value).to eq('new_value')
           end
+        end
 
-          context 'when deletes a variable' do
-            let(:schedule) do
-              basic_param.merge({
-                variables_attributes: [{ id: pipeline_schedule_variable.id, _destroy: true }]
-              })
-            end
+        context 'when deletes a variable' do
+          let(:schedule) do
+            basic_param.merge({
+              variables_attributes: [{ id: pipeline_schedule_variable.id, _destroy: true }]
+            })
+          end
 
-            it 'delete the existsed variable' do
-              expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(-1)
-            end
+          it 'delete the existsed variable' do
+            expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(-1)
+          end
+        end
+
+        context 'when deletes and creates a same key simultaneously' do
+          let(:schedule) do
+            basic_param.merge({
+              variables_attributes: [{ id: pipeline_schedule_variable.id, _destroy: true },
+                                     { key: 'CCC', value: 'CCC123' }]
+            })
+          end
+
+          it 'updates the variable' do
+            expect { go }.not_to change { Ci::PipelineScheduleVariable.count }
+
+            pipeline_schedule.reload
+            expect(pipeline_schedule.variables.last.key).to eq('CCC')
+            expect(pipeline_schedule.variables.last.value).to eq('CCC123')
           end
         end
       end
-- 
cgit v1.2.1