Merge branch 'fix-variable-saving-error' into 'master'

Fix variable saving error

This is regression not allowing to save the variables if the variable name exists in other projects. This also fixes error rendering issue for variables, which previously was rendered on project's settings page.

See merge request !237

6 files changed, 21 insertions, 5 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f2c33fa..eddfded 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,8 @@ v7.14.0 (unreleased)
   - Rename type(s) to stage(s)
   - Add missing stage when doing retry
   - Require variable keys to be not-empty and unique
+  - Fix variable saving issue
+  - Display variable saving errors in variables page not the project's
   
 v7.13.1
   - Fix: user could steal specific runner
diff --git a/app/controllers/variables_controller.rb b/app/controllers/variables_controller.rb
index 6eb908e..00f60f3 100644
--- a/app/controllers/variables_controller.rb
+++ b/app/controllers/variables_controller.rb
@@ -6,7 +6,17 @@ class VariablesController < ApplicationController
 
   layout 'project'
 
-  def index
+  def show
+  end
+
+  def update
+    if project.update_attributes(project_params)
+      EventService.new.change_project_settings(current_user, project)
+
+      redirect_to project_variables_path(project), notice: 'Variables were successfully updated.'
+    else
+      render action: 'show'
+    end
   end
 
   private
@@ -14,4 +24,8 @@ class VariablesController < ApplicationController
   def project
     @project ||= Project.find(params[:project_id])
   end
+
+  def project_params
+    params.require(:project).permit({ variables_attributes: [:id, :key, :value, :_destroy] })
+  end
 end
diff --git a/app/models/variable.rb b/app/models/variable.rb
index 559b9f2..c084b89 100644
--- a/app/models/variable.rb
+++ b/app/models/variable.rb
@@ -15,7 +15,7 @@ class Variable < ActiveRecord::Base
   belongs_to :project
 
   validates_presence_of :key
-  validates_uniqueness_of :key
+  validates_uniqueness_of :key, scope: :project_id
 
   attr_encrypted :value, mode: :per_attribute_iv_and_salt, key: GitlabCi::Application.secrets.db_key_base
 end
diff --git a/app/views/variables/index.html.haml b/app/views/variables/show.html.haml
index 64a4451..e296e17 100644
--- a/app/views/variables/index.html.haml
+++ b/app/views/variables/show.html.haml
@@ -7,7 +7,7 @@
 %hr
 
 
-= nested_form_for @project, html: { class: 'form-horizontal' } do |f|
+= nested_form_for @project, url: url_for(controller: 'variables', action: 'update'), html: { class: 'form-horizontal' }  do |f|
   - if @project.errors.any?
     #error_explanation
       %p.lead= "#{pluralize(@project.errors.count, "error")} prohibited this project from being saved:"
diff --git a/config/routes.rb b/config/routes.rb
index e92e726..cd0054f 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -66,7 +66,7 @@ Rails.application.routes.draw do
     resources :runner_projects, only: [:create, :destroy]
 
     resources :events, only: [:index]
-    resources :variables, only: [:index]
+    resource :variables, only: [:show, :update]
   end
 
   resource :user_sessions do
diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb
index 21a7a11..2bb0d9d 100644
--- a/spec/features/variables_spec.rb
+++ b/spec/features/variables_spec.rb
@@ -18,7 +18,7 @@ describe "Variables" do
       fill_in "Value", with: "SECRET_VALUE"
       click_on "Save changes"
       
-      page.should have_content("Project was successfully updated.")
+      page.should have_content("Variables were successfully updated.")
       @project.variables.count.should == 1
     end