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

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

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

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

app/views/variables/index.html.haml → 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:"

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

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