From ea4777ff501e370a39ae30e76a955136afe3c1fa Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 15:19:13 +0100
Subject: Add features for list and show details of variables in API

---
 app/models/ci/variable.rb           |  1 +
 lib/api/api.rb                      |  2 +
 lib/api/entities.rb                 |  4 ++
 lib/api/variables.rb                | 43 +++++++++++++++++++++
 spec/factories/ci/variables.rb      | 25 +++++++++++++
 spec/requests/api/variables_spec.rb | 75 +++++++++++++++++++++++++++++++++++++
 6 files changed, 150 insertions(+)
 create mode 100644 lib/api/variables.rb
 create mode 100644 spec/factories/ci/variables.rb
 create mode 100644 spec/requests/api/variables_spec.rb

diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb
index 56759d3e50f..0e2712086ca 100644
--- a/app/models/ci/variable.rb
+++ b/app/models/ci/variable.rb
@@ -9,6 +9,7 @@
 #  encrypted_value      :text
 #  encrypted_value_salt :string(255)
 #  encrypted_value_iv   :string(255)
+#  gl_project_id        :integer
 #
 
 module Ci
diff --git a/lib/api/api.rb b/lib/api/api.rb
index 7834262d612..a9e1913f0f2 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -54,5 +54,7 @@ module API
     mount Keys
     mount Tags
     mount Triggers
+
+    mount Variables
   end
 end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 26e7c956e8f..f71d072f269 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -365,5 +365,9 @@ module API
     class TriggerRequest < Grape::Entity
       expose :id, :variables
     end
+
+    class Variable < Grape::Entity
+      expose :id, :key, :value
+    end
   end
 end
diff --git a/lib/api/variables.rb b/lib/api/variables.rb
new file mode 100644
index 00000000000..6517150f6f4
--- /dev/null
+++ b/lib/api/variables.rb
@@ -0,0 +1,43 @@
+module API
+  # Projects variables API
+  class Variables < Grape::API
+    before { authenticate! }
+    before { authorize_admin_project }
+
+    resource :projects do
+      # Get project variables
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   page (optional) - The page number for pagination
+      #   per_page (optional) - The value of items per page to show
+      # Example Request:
+      #   GET /projects/:id/variables
+      get ':id/variables' do
+        variables = user_project.variables
+        present paginate(variables), with: Entities::Variable
+      end
+
+      # Get specifica bariable of a project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   variable_id (required) - The ID OR `key` of variable to show; if variable_id contains only digits it's treated
+      #                            as ID other ways it's treated as `key`
+      # Example Reuest:
+      #   GET /projects/:id/variables/:variable_id
+      get ':id/variables/:variable_id' do
+        variable_id = params[:variable_id]
+        variables = user_project.variables
+        variables =
+          if variable_id.match(/^\d+$/)
+            variables.where(id: variable_id.to_i)
+          else
+            variables.where(key: variable_id)
+          end
+
+        present variables.first, with: Entities::Variable
+      end
+    end
+  end
+end
diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb
new file mode 100644
index 00000000000..c3dcb678da7
--- /dev/null
+++ b/spec/factories/ci/variables.rb
@@ -0,0 +1,25 @@
+# == Schema Information
+#
+# Table name: ci_variables
+#
+#  id                   :integer          not null, primary key
+#  project_id           :integer          not null
+#  key                  :string(255)
+#  value                :text
+#  encrypted_value      :text
+#  encrypted_value_salt :string(255)
+#  encrypted_value_iv   :string(255)
+#  gl_project_id        :integer
+#
+
+# Read about factories at https://github.com/thoughtbot/factory_girl
+
+FactoryGirl.define do
+  factory :ci_variable, class: Ci::Variable do
+    id 1
+    key 'TEST_VARIABLE_1'
+    value 'VALUE_1'
+
+    project factory: :empty_project
+  end
+end
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
new file mode 100644
index 00000000000..8f66f5432b6
--- /dev/null
+++ b/spec/requests/api/variables_spec.rb
@@ -0,0 +1,75 @@
+require 'spec_helper'
+
+describe API::API, api: true do
+  include ApiHelpers
+
+  let(:user) { create(:user) }
+  let(:user2) { create(:user) }
+  let!(:project) { create(:project, creator_id: user.id) }
+  let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) }
+  let!(:developer) { create(:project_member, user: user2, project: project, access_level: ProjectMember::DEVELOPER) }
+  let!(:variable) { create(:ci_variable, project: project) }
+
+  describe 'GET /projects/:id/variables' do
+    context 'authorized user with proper permissions' do
+      it 'should return project variables' do
+        get api("/projects/#{project.id}/variables", user)
+
+        expect(response.status).to eq(200)
+        expect(json_response).to be_a(Array)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not return project variables' do
+        get api("/projects/#{project.id}/variables", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not return project variables' do
+        get api("/projects/#{project.id}/variables")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe 'GET /projects/:id/variables/:variable_id' do
+    context 'authorized user with proper permissions' do
+      it 'should return project variable details when ID is used as :variable_id' do
+        get api("/projects/#{project.id}/variables/1", user)
+
+        expect(response.status).to eq(200)
+        expect(json_response['key']).to eq('TEST_VARIABLE_1')
+        expect(json_response['value']).to eq('VALUE_1')
+      end
+
+      it 'should return project variable details when `key` is used as :variable_id' do
+        get api("/projects/#{project.id}/variables/TEST_VARIABLE_1", user)
+
+        expect(response.status).to eq(200)
+        expect(json_response['id']).to eq(1)
+        expect(json_response['value']).to eq('VALUE_1')
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not return project variable details' do
+        get api("/projects/#{project.id}/variables/1", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not return project variable details' do
+        get api("/projects/#{project.id}/variables/1")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+end
-- 
cgit v1.2.1


From a692ce1c079703c4f3947e1d0a29547189e94d0f Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 16:25:49 +0100
Subject: Add update feature for variables API

---
 lib/api/variables.rb                | 21 ++++++++++++++-
 spec/requests/api/variables_spec.rb | 52 +++++++++++++++++++++++++++++++------
 2 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index 6517150f6f4..6522ecba70c 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -24,7 +24,7 @@ module API
       #   id (required) - The ID of a project
       #   variable_id (required) - The ID OR `key` of variable to show; if variable_id contains only digits it's treated
       #                            as ID other ways it's treated as `key`
-      # Example Reuest:
+      # Example Request:
       #   GET /projects/:id/variables/:variable_id
       get ':id/variables/:variable_id' do
         variable_id = params[:variable_id]
@@ -38,6 +38,25 @@ module API
 
         present variables.first, with: Entities::Variable
       end
+
+      # Update existing variable of a project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   variable_id (required) - The ID of a variable
+      #   key (optional) - new value for `key` field of variable
+      #   value (optional) - new value for `value` field of variable
+      # Example Request:
+      #   PUT /projects/:id/variables/:variable_id
+      put ':id/variables/:variable_id' do
+        variable = user_project.variables.where(id: params[:variable_id].to_i).first
+
+        variable.key = params[:key]
+        variable.value = params[:value]
+        variable.save!
+
+        present variable, with: Entities::Variable
+      end
     end
   end
 end
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 8f66f5432b6..3f58277c4ae 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -40,25 +40,25 @@ describe API::API, api: true do
   describe 'GET /projects/:id/variables/:variable_id' do
     context 'authorized user with proper permissions' do
       it 'should return project variable details when ID is used as :variable_id' do
-        get api("/projects/#{project.id}/variables/1", user)
+        get api("/projects/#{project.id}/variables/#{variable.id}", user)
 
         expect(response.status).to eq(200)
-        expect(json_response['key']).to eq('TEST_VARIABLE_1')
-        expect(json_response['value']).to eq('VALUE_1')
+        expect(json_response['key']).to eq(variable.key)
+        expect(json_response['value']).to eq(variable.value)
       end
 
       it 'should return project variable details when `key` is used as :variable_id' do
-        get api("/projects/#{project.id}/variables/TEST_VARIABLE_1", user)
+        get api("/projects/#{project.id}/variables/#{variable.key}", user)
 
         expect(response.status).to eq(200)
-        expect(json_response['id']).to eq(1)
-        expect(json_response['value']).to eq('VALUE_1')
+        expect(json_response['id']).to eq(variable.id)
+        expect(json_response['value']).to eq(variable.value)
       end
     end
 
     context 'authorized user with invalid permissions' do
       it 'should not return project variable details' do
-        get api("/projects/#{project.id}/variables/1", user2)
+        get api("/projects/#{project.id}/variables/#{variable.id}", user2)
 
         expect(response.status).to eq(403)
       end
@@ -66,7 +66,43 @@ describe API::API, api: true do
 
     context 'unauthorized user' do
       it 'should not return project variable details' do
-        get api("/projects/#{project.id}/variables/1")
+        get api("/projects/#{project.id}/variables/#{variable.id}")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe 'PUT /projects/:id/variables/:variable_id' do
+    context 'authorized user with proper permissions' do
+      it 'should update variable data' do
+        initial_variable = project.variables.first
+        key_before = initial_variable.key
+        value_before = initial_variable.value
+
+        put api("/projects/#{project.id}/variables/#{variable.id}", user), key: 'TEST_VARIABLE_1_UP', value: 'VALUE_1_UP'
+
+        updated_variable = project.variables.first
+
+        expect(response.status).to eq(200)
+        expect(key_before).to eq(variable.key)
+        expect(value_before).to eq(variable.value)
+        expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP')
+        expect(updated_variable.value).to eq('VALUE_1_UP')
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not update variable' do
+        put api("/projects/#{project.id}/variables/#{variable.id}", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not return project variable details' do
+        put api("/projects/#{project.id}/variables/#{variable.id}")
 
         expect(response.status).to eq(401)
       end
-- 
cgit v1.2.1


From 0d014feb1d216e692882976f0d70c3227eaec4ca Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 16:56:03 +0100
Subject: Add delete feature to variables API

---
 lib/api/variables.rb                | 12 ++++++++++++
 spec/requests/api/variables_spec.rb | 29 ++++++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index 6522ecba70c..c70c7cd9d7b 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -57,6 +57,18 @@ module API
 
         present variable, with: Entities::Variable
       end
+
+      # Delete existing variable of a project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   variable_id (required) - The ID of a variable
+      # Exanoke Reqyest:
+      #   DELETE /projects/:id/variables/:variable_id
+      delete ':id/variables/:variable_id' do
+        variable = user_project.variables.where(id: params[:variable_id].to_i).first
+        variable.destroy
+      end
     end
   end
 end
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 3f58277c4ae..385db2409bd 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -101,11 +101,38 @@ describe API::API, api: true do
     end
 
     context 'unauthorized user' do
-      it 'should not return project variable details' do
+      it 'should not update variable' do
         put api("/projects/#{project.id}/variables/#{variable.id}")
 
         expect(response.status).to eq(401)
       end
     end
   end
+
+  describe 'DELETE /projects/:id/variables/:variable_id' do
+    context 'authorized user with proper permissions' do
+      it 'should delete variable' do
+        expect do
+          delete api("/projects/#{project.id}/variables/#{variable.id}", user)
+        end.to change{project.variables.count}.by(-1)
+        expect(response.status).to eq(200)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not delete variable' do
+        delete api("/projects/#{project.id}/variables/#{variable.id}", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not delete variable' do
+        delete api("/projects/#{project.id}/variables/#{variable.id}")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
 end
-- 
cgit v1.2.1


From c5177dd5e2171b047a695802c979cf779522ba8a Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 17:03:11 +0100
Subject: Add missing 'not_found' checks in variables API

---
 lib/api/variables.rb                |  7 +++++++
 spec/requests/api/variables_spec.rb | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index c70c7cd9d7b..dac2ba679c7 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -36,6 +36,8 @@ module API
             variables.where(key: variable_id)
           end
 
+        return not_found!('Variable') if variables.empty?
+
         present variables.first, with: Entities::Variable
       end
 
@@ -51,6 +53,8 @@ module API
       put ':id/variables/:variable_id' do
         variable = user_project.variables.where(id: params[:variable_id].to_i).first
 
+        return not_found!('Variable') unless variable
+
         variable.key = params[:key]
         variable.value = params[:value]
         variable.save!
@@ -67,6 +71,9 @@ module API
       #   DELETE /projects/:id/variables/:variable_id
       delete ':id/variables/:variable_id' do
         variable = user_project.variables.where(id: params[:variable_id].to_i).first
+
+        return not_found!('Variable') unless variable
+
         variable.destroy
       end
     end
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 385db2409bd..b35ee2d32d1 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -54,6 +54,12 @@ describe API::API, api: true do
         expect(json_response['id']).to eq(variable.id)
         expect(json_response['value']).to eq(variable.value)
       end
+
+      it 'should responde with 404 Not Found if requesting non-existing variable' do
+        get api("/projects/#{project.id}/variables/9999", user)
+
+        expect(response.status).to eq(404)
+      end
     end
 
     context 'authorized user with invalid permissions' do
@@ -90,6 +96,12 @@ describe API::API, api: true do
         expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP')
         expect(updated_variable.value).to eq('VALUE_1_UP')
       end
+
+      it 'should responde with 404 Not Found if requesting non-existing variable' do
+        put api("/projects/#{project.id}/variables/9999", user)
+
+        expect(response.status).to eq(404)
+      end
     end
 
     context 'authorized user with invalid permissions' do
@@ -117,6 +129,12 @@ describe API::API, api: true do
         end.to change{project.variables.count}.by(-1)
         expect(response.status).to eq(200)
       end
+
+      it 'should responde with 404 Not Found if requesting non-existing variable' do
+        delete api("/projects/#{project.id}/variables/9999", user)
+
+        expect(response.status).to eq(404)
+      end
     end
 
     context 'authorized user with invalid permissions' do
-- 
cgit v1.2.1


From 937567b767e6d7b34dcaa1d9c83fc75464638683 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 22:30:07 +0100
Subject: Add create feature to variables API

---
 lib/api/variables.rb                | 20 +++++++++++++++++++
 spec/factories/ci/variables.rb      |  2 +-
 spec/requests/api/variables_spec.rb | 38 +++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index dac2ba679c7..fc63ac2f56a 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -41,6 +41,24 @@ module API
         present variables.first, with: Entities::Variable
       end
 
+      # Create a new variable in project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   key (required) - The key of variable being created
+      #   value (required) - The value of variable being created
+      # Example Request:
+      #   POST /projects/:id/variables
+      post ':id/variables' do
+        required_attributes! [:key, :value]
+
+        variable = user_project.variables.create(key: params[:key], value: params[:value])
+        return render_validation_error!(variable) unless variable.valid?
+        variable.save!
+
+        present variable, with: Entities::Variable
+      end
+
       # Update existing variable of a project
       #
       # Parameters:
@@ -75,6 +93,8 @@ module API
         return not_found!('Variable') unless variable
 
         variable.destroy
+
+        present variable, with: Entities::Variable
       end
     end
   end
diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb
index c3dcb678da7..a19b9fc72f2 100644
--- a/spec/factories/ci/variables.rb
+++ b/spec/factories/ci/variables.rb
@@ -16,7 +16,7 @@
 
 FactoryGirl.define do
   factory :ci_variable, class: Ci::Variable do
-    id 1
+    id 10
     key 'TEST_VARIABLE_1'
     value 'VALUE_1'
 
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index b35ee2d32d1..bf0dd77473a 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -79,6 +79,44 @@ describe API::API, api: true do
     end
   end
 
+  describe 'POST /projects/:id/variables' do
+    context 'authorized user with proper permissions' do
+      it 'should create variable' do
+        expect do
+          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2'
+        end.to change{project.variables.count}.by(1)
+
+        expect(response.status).to eq(201)
+        expect(json_response['key']).to eq('TEST_VARIABLE_2')
+        expect(json_response['value']).to eq('VALUE_2')
+      end
+
+      it 'should not allow to duplicate variable key' do
+        expect do
+          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_1', value: 'VALUE_2'
+        end.to change{project.variables.count}.by(0)
+
+        expect(response.status).to eq(400)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not create variable' do
+        post api("/projects/#{project.id}/variables", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not create variable' do
+        post api("/projects/#{project.id}/variables")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
   describe 'PUT /projects/:id/variables/:variable_id' do
     context 'authorized user with proper permissions' do
       it 'should update variable data' do
-- 
cgit v1.2.1


From 16bd4df083135e2e4a263b2e1bdd71b78a875ef7 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 22:59:06 +0100
Subject: Fix a typo in method description

---
 lib/api/variables.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index fc63ac2f56a..b8bbcb6ce3b 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -18,7 +18,7 @@ module API
         present paginate(variables), with: Entities::Variable
       end
 
-      # Get specifica bariable of a project
+      # Get specifica variable of a project
       #
       # Parameters:
       #   id (required) - The ID of a project
-- 
cgit v1.2.1


From b60c146267dfa8dc1c170426e1817c6b2a168d1a Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 7 Jan 2016 13:49:38 +0100
Subject: Change :variable_id to :key as resource ID in API

---
 app/models/ci/variable.rb           |  6 +++++-
 lib/api/variables.rb                | 41 ++++++++++++++----------------------
 spec/requests/api/variables_spec.rb | 42 +++++++++++++------------------------
 3 files changed, 36 insertions(+), 53 deletions(-)

diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb
index 0e2712086ca..1d9ee91a31c 100644
--- a/app/models/ci/variable.rb
+++ b/app/models/ci/variable.rb
@@ -18,8 +18,12 @@ module Ci
     
     belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id
 
-    validates_presence_of :key
     validates_uniqueness_of :key, scope: :gl_project_id
+    validates :key,
+      presence: true,
+      length: { within: 0..255 },
+      format: { with: /\A[a-zA-Z0-9_]+\z/,
+                message: "can contain only letters, digits and '_'." }
 
     attr_encrypted :value, mode: :per_attribute_iv_and_salt, key: Gitlab::Application.secrets.db_key_base
   end
diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index b8bbcb6ce3b..cc038e5731d 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -22,19 +22,12 @@ module API
       #
       # Parameters:
       #   id (required) - The ID of a project
-      #   variable_id (required) - The ID OR `key` of variable to show; if variable_id contains only digits it's treated
-      #                            as ID other ways it's treated as `key`
+      #   key (required) - The `key` of variable
       # Example Request:
-      #   GET /projects/:id/variables/:variable_id
-      get ':id/variables/:variable_id' do
-        variable_id = params[:variable_id]
-        variables = user_project.variables
-        variables =
-          if variable_id.match(/^\d+$/)
-            variables.where(id: variable_id.to_i)
-          else
-            variables.where(key: variable_id)
-          end
+      #   GET /projects/:id/variables/:key
+      get ':id/variables/:key' do
+        key = params[:key]
+        variables = user_project.variables.where(key: key)
 
         return not_found!('Variable') if variables.empty?
 
@@ -45,8 +38,8 @@ module API
       #
       # Parameters:
       #   id (required) - The ID of a project
-      #   key (required) - The key of variable being created
-      #   value (required) - The value of variable being created
+      #   key (required) - The key of variable
+      #   value (required) - The value of variable
       # Example Request:
       #   POST /projects/:id/variables
       post ':id/variables' do
@@ -63,17 +56,15 @@ module API
       #
       # Parameters:
       #   id (required) - The ID of a project
-      #   variable_id (required) - The ID of a variable
-      #   key (optional) - new value for `key` field of variable
-      #   value (optional) - new value for `value` field of variable
+      #   key (optional) - The `key` of variable
+      #   value (optional) - New value for `value` field of variable
       # Example Request:
-      #   PUT /projects/:id/variables/:variable_id
-      put ':id/variables/:variable_id' do
-        variable = user_project.variables.where(id: params[:variable_id].to_i).first
+      #   PUT /projects/:id/variables/:key
+      put ':id/variables/:key' do
+        variable = user_project.variables.where(key: params[:key]).first
 
         return not_found!('Variable') unless variable
 
-        variable.key = params[:key]
         variable.value = params[:value]
         variable.save!
 
@@ -84,11 +75,11 @@ module API
       #
       # Parameters:
       #   id (required) - The ID of a project
-      #   variable_id (required) - The ID of a variable
+      #   key (required) - The ID of a variable
       # Exanoke Reqyest:
-      #   DELETE /projects/:id/variables/:variable_id
-      delete ':id/variables/:variable_id' do
-        variable = user_project.variables.where(id: params[:variable_id].to_i).first
+      #   DELETE /projects/:id/variables/:key
+      delete ':id/variables/:key' do
+        variable = user_project.variables.where(key: params[:key]).first
 
         return not_found!('Variable') unless variable
 
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index bf0dd77473a..214d7d5a0cc 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -37,26 +37,17 @@ describe API::API, api: true do
     end
   end
 
-  describe 'GET /projects/:id/variables/:variable_id' do
+  describe 'GET /projects/:id/variables/:key' do
     context 'authorized user with proper permissions' do
-      it 'should return project variable details when ID is used as :variable_id' do
-        get api("/projects/#{project.id}/variables/#{variable.id}", user)
-
-        expect(response.status).to eq(200)
-        expect(json_response['key']).to eq(variable.key)
-        expect(json_response['value']).to eq(variable.value)
-      end
-
-      it 'should return project variable details when `key` is used as :variable_id' do
+      it 'should return project variable details' do
         get api("/projects/#{project.id}/variables/#{variable.key}", user)
 
         expect(response.status).to eq(200)
-        expect(json_response['id']).to eq(variable.id)
         expect(json_response['value']).to eq(variable.value)
       end
 
       it 'should responde with 404 Not Found if requesting non-existing variable' do
-        get api("/projects/#{project.id}/variables/9999", user)
+        get api("/projects/#{project.id}/variables/non_existing_variable", user)
 
         expect(response.status).to eq(404)
       end
@@ -64,7 +55,7 @@ describe API::API, api: true do
 
     context 'authorized user with invalid permissions' do
       it 'should not return project variable details' do
-        get api("/projects/#{project.id}/variables/#{variable.id}", user2)
+        get api("/projects/#{project.id}/variables/#{variable.key}", user2)
 
         expect(response.status).to eq(403)
       end
@@ -72,7 +63,7 @@ describe API::API, api: true do
 
     context 'unauthorized user' do
       it 'should not return project variable details' do
-        get api("/projects/#{project.id}/variables/#{variable.id}")
+        get api("/projects/#{project.id}/variables/#{variable.key}")
 
         expect(response.status).to eq(401)
       end
@@ -117,26 +108,23 @@ describe API::API, api: true do
     end
   end
 
-  describe 'PUT /projects/:id/variables/:variable_id' do
+  describe 'PUT /projects/:id/variables/:key' do
     context 'authorized user with proper permissions' do
       it 'should update variable data' do
         initial_variable = project.variables.first
-        key_before = initial_variable.key
         value_before = initial_variable.value
 
-        put api("/projects/#{project.id}/variables/#{variable.id}", user), key: 'TEST_VARIABLE_1_UP', value: 'VALUE_1_UP'
+        put api("/projects/#{project.id}/variables/#{variable.key}", user), value: 'VALUE_1_UP'
 
         updated_variable = project.variables.first
 
         expect(response.status).to eq(200)
-        expect(key_before).to eq(variable.key)
         expect(value_before).to eq(variable.value)
-        expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP')
         expect(updated_variable.value).to eq('VALUE_1_UP')
       end
 
       it 'should responde with 404 Not Found if requesting non-existing variable' do
-        put api("/projects/#{project.id}/variables/9999", user)
+        put api("/projects/#{project.id}/variables/non_existing_variable", user)
 
         expect(response.status).to eq(404)
       end
@@ -144,7 +132,7 @@ describe API::API, api: true do
 
     context 'authorized user with invalid permissions' do
       it 'should not update variable' do
-        put api("/projects/#{project.id}/variables/#{variable.id}", user2)
+        put api("/projects/#{project.id}/variables/#{variable.key}", user2)
 
         expect(response.status).to eq(403)
       end
@@ -152,24 +140,24 @@ describe API::API, api: true do
 
     context 'unauthorized user' do
       it 'should not update variable' do
-        put api("/projects/#{project.id}/variables/#{variable.id}")
+        put api("/projects/#{project.id}/variables/#{variable.key}")
 
         expect(response.status).to eq(401)
       end
     end
   end
 
-  describe 'DELETE /projects/:id/variables/:variable_id' do
+  describe 'DELETE /projects/:id/variables/:key' do
     context 'authorized user with proper permissions' do
       it 'should delete variable' do
         expect do
-          delete api("/projects/#{project.id}/variables/#{variable.id}", user)
+          delete api("/projects/#{project.id}/variables/#{variable.key}", user)
         end.to change{project.variables.count}.by(-1)
         expect(response.status).to eq(200)
       end
 
       it 'should responde with 404 Not Found if requesting non-existing variable' do
-        delete api("/projects/#{project.id}/variables/9999", user)
+        delete api("/projects/#{project.id}/variables/non_existing_variable", user)
 
         expect(response.status).to eq(404)
       end
@@ -177,7 +165,7 @@ describe API::API, api: true do
 
     context 'authorized user with invalid permissions' do
       it 'should not delete variable' do
-        delete api("/projects/#{project.id}/variables/#{variable.id}", user2)
+        delete api("/projects/#{project.id}/variables/#{variable.key}", user2)
 
         expect(response.status).to eq(403)
       end
@@ -185,7 +173,7 @@ describe API::API, api: true do
 
     context 'unauthorized user' do
       it 'should not delete variable' do
-        delete api("/projects/#{project.id}/variables/#{variable.id}")
+        delete api("/projects/#{project.id}/variables/#{variable.key}")
 
         expect(response.status).to eq(401)
       end
-- 
cgit v1.2.1


From b60445906849e84ff52ac6a5d7d501bb5a21eb60 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 7 Jan 2016 14:10:49 +0100
Subject: Update ./doc/api

---
 doc/api/README.md    |   1 +
 doc/api/variables.md | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/api/entities.rb  |   2 +-
 3 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 doc/api/variables.md

diff --git a/doc/api/README.md b/doc/api/README.md
index 25a31b235cc..198b4ddfee1 100644
--- a/doc/api/README.md
+++ b/doc/api/README.md
@@ -23,6 +23,7 @@
 - [Namespaces](namespaces.md)
 - [Settings](settings.md)
 - [Keys](keys.md)
+- [Variables](variables.md)
 
 ## Clients
 
diff --git a/doc/api/variables.md b/doc/api/variables.md
new file mode 100644
index 00000000000..cdc9ba42254
--- /dev/null
+++ b/doc/api/variables.md
@@ -0,0 +1,106 @@
+# Variables
+
+## Variables keys
+
+All variable keys must contains only letters, digits and '\_'. They must also be no longer than 255 characters.
+
+## List project variables
+
+Get list of variables of a project.
+
+```
+GET /projects/:id/variables
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+
+```json
+[
+    {
+        "key": "TEST_VARIABLE_1",
+        "value": "TEST_1"
+    },
+    {
+        "key": "TEST_VARIABLE_2",
+        "value": "TEST_2"
+    }
+]
+```
+
+## Show variable details
+
+Get details of specifica variable of a project.
+
+```
+GET /projects/:id/variables/:key
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `key` (required) - The `key` of variable
+
+```json
+{
+    "key": "TEST_VARIABLE_1",
+    "value": "TEST_1"
+}
+```
+
+## Create variable
+
+Create new variable in project.
+
+```
+POST /projects/:id/variables
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `key` (required) - The `key` for variable
+- `value` (required) - The `value` for variable
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "new value"
+}
+```
+
+## Update variable
+
+Update variable.
+
+```
+PUT /projects/:id/variables/:key
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `key` (required) - The `key` for variable
+- `value` (required) - The `value` for variable
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "updated value"
+}
+```
+
+## Remove variable
+
+Remove variable.
+
+```
+DELETE /projects/:id/variables/:key
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `key` (required) - The `key` for variable
+
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index f71d072f269..db3164d9d9c 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -367,7 +367,7 @@ module API
     end
 
     class Variable < Grape::Entity
-      expose :id, :key, :value
+      expose :key, :value
     end
   end
 end
-- 
cgit v1.2.1


From bc7ef8e5b7a002ca6bc2d7a5e6be11b4a59b6710 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Tue, 29 Dec 2015 17:53:55 -0200
Subject: Add ldap_blocked as new state to users state machine

---
 app/models/user.rb       | 12 +++++++++++-
 spec/models/user_spec.rb | 44 ++++++++++++++++++++++++++++----------------
 2 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 46b36c605b0..67b47b0f329 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -198,16 +198,26 @@ class User < ActiveRecord::Base
       transition active: :blocked
     end
 
+    event :ldap_block do
+      transition active: :ldap_blocked
+    end
+
     event :activate do
       transition blocked: :active
     end
+
+    state :blocked, :ldap_blocked do
+      def blocked?
+        true
+      end
+    end
   end
 
   mount_uploader :avatar, AvatarUploader
 
   # Scopes
   scope :admins, -> { where(admin: true) }
-  scope :blocked, -> { with_state(:blocked) }
+  scope :blocked, -> { with_states(:blocked, :ldap_blocked) }
   scope :active, -> { with_state(:active) }
   scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all }
   scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members)') }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 3cd63b2b0e8..0bef68e2885 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -569,27 +569,39 @@ describe User, models: true do
     end
   end
 
-  describe :ldap_user? do
-    it "is true if provider name starts with ldap" do
-      user = create(:omniauth_user, provider: 'ldapmain')
-      expect( user.ldap_user? ).to be_truthy
-    end
+  context 'ldap synchronized user' do
+    describe :ldap_user? do
+      it 'is true if provider name starts with ldap' do
+        user = create(:omniauth_user, provider: 'ldapmain')
+        expect(user.ldap_user?).to be_truthy
+      end
 
-    it "is false for other providers" do
-      user = create(:omniauth_user, provider: 'other-provider')
-      expect( user.ldap_user? ).to be_falsey
+      it 'is false for other providers' do
+        user = create(:omniauth_user, provider: 'other-provider')
+        expect(user.ldap_user?).to be_falsey
+      end
+
+      it 'is false if no extern_uid is provided' do
+        user = create(:omniauth_user, extern_uid: nil)
+        expect(user.ldap_user?).to be_falsey
+      end
     end
 
-    it "is false if no extern_uid is provided" do
-      user = create(:omniauth_user, extern_uid: nil)
-      expect( user.ldap_user? ).to be_falsey
+    describe :ldap_identity do
+      it 'returns ldap identity' do
+        user = create :omniauth_user
+        expect(user.ldap_identity.provider).not_to be_empty
+      end
     end
-  end
 
-  describe :ldap_identity do
-    it "returns ldap identity" do
-      user = create :omniauth_user
-      expect(user.ldap_identity.provider).not_to be_empty
+    describe '#ldap_block' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain', name: 'John Smith') }
+
+      it 'blocks user flaging the action caming from ldap' do
+        user.ldap_block
+        expect(user.blocked?).to be_truthy
+        expect(user.ldap_blocked?).to be_truthy
+      end
     end
   end
 
-- 
cgit v1.2.1


From ba9855d4877998e3574907cc542fcab15a9d1353 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Tue, 29 Dec 2015 18:58:38 -0200
Subject: Prevent ldap_blocked users from being unblocked by the Admin UI

---
 app/assets/stylesheets/framework/buttons.scss   |  3 +++
 app/controllers/admin/users_controller.rb       |  4 ++-
 app/views/admin/users/index.html.haml           |  7 ++++-
 spec/controllers/admin/users_controller_spec.rb | 35 ++++++++++++++++++-------
 4 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss
index 97a94638847..e2376363485 100644
--- a/app/assets/stylesheets/framework/buttons.scss
+++ b/app/assets/stylesheets/framework/buttons.scss
@@ -132,6 +132,9 @@
       margin-right: 0px;
     }
   }
+  &.disabled {
+    pointer-events: auto !important;
+  }
 }
 
 .btn-block {
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index d7c927d444c..87f4fb455b8 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -40,7 +40,9 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def unblock
-    if user.activate
+    if user.ldap_blocked?
+      redirect_back_or_admin_user(alert: "This user cannot be unlocked manually from GitLab")
+    elsif user.activate
       redirect_back_or_admin_user(notice: "Successfully unblocked")
     else
       redirect_back_or_admin_user(alert: "Error occurred. User was not unblocked")
diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml
index a92c9c152b9..911c4d0cf12 100644
--- a/app/views/admin/users/index.html.haml
+++ b/app/views/admin/users/index.html.haml
@@ -90,7 +90,12 @@
               &nbsp;
               = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: "btn btn-xs"
               - unless user == current_user
-                - if user.blocked?
+                - if user.ldap_blocked?
+                  = link_to '#', title: 'Cannot unblock LDAP blocked users', data: {toggle: 'tooltip'}, class: 'btn btn-xs btn-success disabled' do
+                    %i.fa.fa-lock
+                    Unblock
+                  = ''
+                - elsif user.blocked?
                   = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success"
                 - else
                   = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs btn-warning"
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 8b7af4d3a0a..5b1f65d7aff 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -34,17 +34,34 @@ describe Admin::UsersController do
   end
 
   describe 'PUT unblock/:id' do
-    let(:user) { create(:user) }
-
-    before do
-      user.block
+    context 'ldap blocked users' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain') }
+
+      before do
+        user.ldap_block
+      end
+
+      it 'will not unblock user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_truthy
+        expect(flash[:alert]).to eq 'This user cannot be unlocked manually from GitLab'
+      end
     end
 
-    it 'unblocks user' do
-      put :unblock, id: user.username
-      user.reload
-      expect(user.blocked?).to be_falsey
-      expect(flash[:notice]).to eq 'Successfully unblocked'
+    context 'manually blocked users' do
+      let(:user) { create(:user) }
+
+      before do
+        user.block
+      end
+
+      it 'unblocks user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_falsey
+        expect(flash[:notice]).to eq 'Successfully unblocked'
+      end
     end
   end
 
-- 
cgit v1.2.1


From 6e7db8e23e169bcbf0847ece27b9e44e00ae572b Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Wed, 30 Dec 2015 16:52:02 -0200
Subject: Prevent ldap_blocked users from being blocked/unblocked by the API

---
 doc/api/users.md                |  6 ++++--
 lib/api/users.rb                | 12 ++++++++----
 spec/requests/api/users_spec.rb | 23 ++++++++++++++++++-----
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/doc/api/users.md b/doc/api/users.md
index 773fe36d277..b7fc903825e 100644
--- a/doc/api/users.md
+++ b/doc/api/users.md
@@ -558,7 +558,8 @@ Parameters:
 
 - `uid` (required) - id of specified user
 
-Will return `200 OK` on success, or `404 User Not Found` is user cannot be found.
+Will return `200 OK` on success, `404 User Not Found` is user cannot be found or 
+`403 Forbidden` when trying to block an already blocked user by LDAP synchronization.
 
 ## Unblock user
 
@@ -572,4 +573,5 @@ Parameters:
 
 - `uid` (required) - id of specified user
 
-Will return `200 OK` on success, or `404 User Not Found` is user cannot be found.
+Will return `200 OK` on success, `404 User Not Found` is user cannot be found or
+`403 Forbidden` when trying to unblock a user blocked by LDAP synchronization.
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 0d7813428e2..01fd90139b0 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -284,10 +284,12 @@ module API
         authenticated_as_admin!
         user = User.find_by(id: params[:id])
 
-        if user
+        if !user
+          not_found!('User')
+        elsif !user.ldap_blocked?
           user.block
         else
-          not_found!('User')
+          forbidden!('LDAP blocked users cannot be modified by the API')
         end
       end
 
@@ -299,10 +301,12 @@ module API
         authenticated_as_admin!
         user = User.find_by(id: params[:id])
 
-        if user
+        if !user
+          not_found!('User')
+        elsif !user.ldap_blocked?
           user.activate
         else
-          not_found!('User')
+          forbidden!('LDAP blocked users cannot be unblocked by the API')
         end
       end
     end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 4f278551d07..b82c5c7685f 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -8,6 +8,8 @@ describe API::API, api: true  do
   let(:key)   { create(:key, user: user) }
   let(:email)   { create(:email, user: user) }
   let(:omniauth_user) { create(:omniauth_user) }
+  let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') }
+  let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
 
   describe "GET /users" do
     context "when unauthenticated" do
@@ -783,6 +785,12 @@ describe API::API, api: true  do
       expect(user.reload.state).to eq('blocked')
     end
 
+    it 'should not re-block ldap blocked users' do
+      put api("/users/#{ldap_blocked_user.id}/block", admin)
+      expect(response.status).to eq(403)
+      expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
+    end
+
     it 'should not be available for non admin users' do
       put api("/users/#{user.id}/block", user)
       expect(response.status).to eq(403)
@@ -797,7 +805,9 @@ describe API::API, api: true  do
   end
 
   describe 'PUT /user/:id/unblock' do
+    let(:blocked_user)  { create(:user, state: 'blocked') }
     before { admin }
+
     it 'should unblock existing user' do
       put api("/users/#{user.id}/unblock", admin)
       expect(response.status).to eq(200)
@@ -805,12 +815,15 @@ describe API::API, api: true  do
     end
 
     it 'should unblock a blocked user' do
-      put api("/users/#{user.id}/block", admin)
-      expect(response.status).to eq(200)
-      expect(user.reload.state).to eq('blocked')
-      put api("/users/#{user.id}/unblock", admin)
+      put api("/users/#{blocked_user.id}/unblock", admin)
       expect(response.status).to eq(200)
-      expect(user.reload.state).to eq('active')
+      expect(blocked_user.reload.state).to eq('active')
+    end
+
+    it 'should not unblock ldap blocked users' do
+      put api("/users/#{ldap_blocked_user.id}/unblock", admin)
+      expect(response.status).to eq(403)
+      expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
     end
 
     it 'should not be available for non admin users' do
-- 
cgit v1.2.1


From d6dc088affeee4568e771e1d7894e0bcdb955af8 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Wed, 30 Dec 2015 20:56:26 -0200
Subject: LDAP synchronization block/unblock new states

---
 lib/gitlab/ldap/access.rb           |  6 +++---
 spec/lib/gitlab/ldap/access_spec.rb | 34 ++++++++++++++--------------------
 2 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index c438a3d167b..76cb48d7aa6 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -37,15 +37,15 @@ module Gitlab
 
           # Block user in GitLab if he/she was blocked in AD
           if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter)
-            user.block
+            user.ldap_block
             false
           else
-            user.activate if user.blocked? && !ldap_config.block_auto_created_users
+            user.activate if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked?
             true
           end
         else
           # Block the user if they no longer exist in LDAP/AD
-          user.block 
+          user.ldap_block
           false
         end
       rescue
diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb
index a628d0c0157..f58d70e809c 100644
--- a/spec/lib/gitlab/ldap/access_spec.rb
+++ b/spec/lib/gitlab/ldap/access_spec.rb
@@ -13,64 +13,59 @@ describe Gitlab::LDAP::Access, lib: true do
       end
 
       it { is_expected.to be_falsey }
-      
+
       it 'should block user in GitLab' do
         access.allowed?
         expect(user).to be_blocked
+        expect(user).to be_ldap_blocked
       end
     end
 
     context 'when the user is found' do
       before do
-        allow(Gitlab::LDAP::Person).
-          to receive(:find_by_dn).and_return(:ldap_user)
+        allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user)
       end
 
       context 'and the user is disabled via active directory' do
         before do
-          allow(Gitlab::LDAP::Person).
-            to receive(:disabled_via_active_directory?).and_return(true)
+          allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(true)
         end
 
         it { is_expected.to be_falsey }
 
-        it "should block user in GitLab" do
+        it 'should block user in GitLab' do
           access.allowed?
           expect(user).to be_blocked
+          expect(user).to be_ldap_blocked
         end
       end
 
       context 'and has no disabled flag in active diretory' do
         before do
           user.block
-
-          allow(Gitlab::LDAP::Person).
-            to receive(:disabled_via_active_directory?).and_return(false)
+          allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false)
         end
 
         it { is_expected.to be_truthy }
 
         context 'when auto-created users are blocked' do
-
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).
-              to receive(:block_auto_created_users).and_return(true)
+            allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(true)
           end
 
-          it "does not unblock user in GitLab" do
+          it 'does not unblock user in GitLab' do
             access.allowed?
             expect(user).to be_blocked
+            expect(user).not_to be_ldap_blocked # this block is handled by omniauth not by our internal logic
           end
         end
 
-        context "when auto-created users are not blocked" do
-
+        context 'when auto-created users are not blocked' do
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).
-              to receive(:block_auto_created_users).and_return(false)
+            allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(false)
           end
 
-          it "should unblock user in GitLab" do
+          it 'should unblock user in GitLab' do
             access.allowed?
             expect(user).not_to be_blocked
           end
@@ -80,8 +75,7 @@ describe Gitlab::LDAP::Access, lib: true do
       context 'without ActiveDirectory enabled' do
         before do
           allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
-          allow_any_instance_of(Gitlab::LDAP::Config).
-            to receive(:active_directory).and_return(false)
+          allow_any_instance_of(Gitlab::LDAP::Config).to receive(:active_directory).and_return(false)
         end
 
         it { is_expected.to be_truthy }
-- 
cgit v1.2.1


From ec67e9be1d7486199b47e19c766202a8bfdefe93 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Wed, 6 Jan 2016 05:38:52 -0200
Subject: Repair ldap_blocked state when no ldap identity exist anymore

---
 app/controllers/admin/identities_controller.rb     |  2 ++
 app/models/identity.rb                             |  4 +++
 app/models/user.rb                                 |  1 +
 app/services/repair_ldap_blocked_user_service.rb   | 15 +++++++++
 .../admin/identities_controller_spec.rb            | 26 +++++++++++++++
 spec/models/identity_spec.rb                       | 38 ++++++++++++++++++++++
 .../repair_ldap_blocked_user_service_spec.rb       | 23 +++++++++++++
 7 files changed, 109 insertions(+)
 create mode 100644 app/services/repair_ldap_blocked_user_service.rb
 create mode 100644 spec/controllers/admin/identities_controller_spec.rb
 create mode 100644 spec/models/identity_spec.rb
 create mode 100644 spec/services/repair_ldap_blocked_user_service_spec.rb

diff --git a/app/controllers/admin/identities_controller.rb b/app/controllers/admin/identities_controller.rb
index e383fe38ea6..9ba10487512 100644
--- a/app/controllers/admin/identities_controller.rb
+++ b/app/controllers/admin/identities_controller.rb
@@ -26,6 +26,7 @@ class Admin::IdentitiesController < Admin::ApplicationController
 
   def update
     if @identity.update_attributes(identity_params)
+      RepairLdapBlockedUserService.new(@user, @identity).execute
       redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully updated.'
     else
       render :edit
@@ -34,6 +35,7 @@ class Admin::IdentitiesController < Admin::ApplicationController
 
   def destroy
     if @identity.destroy
+      RepairLdapBlockedUserService.new(@user, @identity).execute
       redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully removed.'
     else
       redirect_to admin_user_identities_path(@user), alert: 'Failed to remove user identity.'
diff --git a/app/models/identity.rb b/app/models/identity.rb
index 8bcdc194953..830b99fa3f2 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -18,4 +18,8 @@ class Identity < ActiveRecord::Base
   validates :provider, presence: true
   validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
   validates :user_id, uniqueness: { scope: :provider }
+
+  def is_ldap?
+    provider.starts_with?('ldap')
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 67b47b0f329..5eed9cf91c7 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -196,6 +196,7 @@ class User < ActiveRecord::Base
   state_machine :state, initial: :active do
     event :block do
       transition active: :blocked
+      transition ldap_blocked: :blocked
     end
 
     event :ldap_block do
diff --git a/app/services/repair_ldap_blocked_user_service.rb b/app/services/repair_ldap_blocked_user_service.rb
new file mode 100644
index 00000000000..ceca15414e0
--- /dev/null
+++ b/app/services/repair_ldap_blocked_user_service.rb
@@ -0,0 +1,15 @@
+class RepairLdapBlockedUserService
+  attr_accessor :user, :identity
+
+  def initialize(user, identity)
+    @user, @identity = user, identity
+  end
+
+  def execute
+    if identity.destroyed?
+      user.block if identity.is_ldap? && user.ldap_blocked? && !user.ldap_user?
+    else
+      user.block if !identity.is_ldap? && user.ldap_blocked? && !user.ldap_user?
+    end
+  end
+end
diff --git a/spec/controllers/admin/identities_controller_spec.rb b/spec/controllers/admin/identities_controller_spec.rb
new file mode 100644
index 00000000000..c131d22a30a
--- /dev/null
+++ b/spec/controllers/admin/identities_controller_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe Admin::IdentitiesController do
+  let(:admin) { create(:admin) }
+  before { sign_in(admin) }
+
+  describe 'UPDATE identity' do
+    let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') }
+
+    it 'repairs ldap blocks' do
+      expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute)
+
+      put :update, user_id: user.username, id: user.ldap_identity.id, identity: { provider: 'twitter' }
+    end
+  end
+
+  describe 'DELETE identity' do
+    let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') }
+
+    it 'repairs ldap blocks' do
+      expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute)
+
+      delete :destroy, user_id: user.username, id: user.ldap_identity.id
+    end
+  end
+end
diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb
new file mode 100644
index 00000000000..107bfc17782
--- /dev/null
+++ b/spec/models/identity_spec.rb
@@ -0,0 +1,38 @@
+# == Schema Information
+#
+# Table name: identities
+#
+#  id         :integer          not null, primary key
+#  extern_uid :string(255)
+#  provider   :string(255)
+#  user_id    :integer
+#  created_at :datetime
+#  updated_at :datetime
+#
+
+require 'spec_helper'
+
+RSpec.describe Identity, models: true do
+
+  describe 'relations' do
+    it { is_expected.to belong_to(:user) }
+  end
+
+  describe 'fields' do
+    it { is_expected.to respond_to(:provider) }
+    it { is_expected.to respond_to(:extern_uid) }
+  end
+
+  describe '#is_ldap?' do
+    let(:ldap_identity) { create(:identity, provider: 'ldapmain') }
+    let(:other_identity) { create(:identity, provider: 'twitter') }
+
+    it 'returns true if it is a ldap identity' do
+      expect(ldap_identity.is_ldap?).to be_truthy
+    end
+
+    it 'returns false if it is not a ldap identity' do
+      expect(other_identity.is_ldap?).to be_falsey
+    end
+  end
+end
diff --git a/spec/services/repair_ldap_blocked_user_service_spec.rb b/spec/services/repair_ldap_blocked_user_service_spec.rb
new file mode 100644
index 00000000000..2a2114d038c
--- /dev/null
+++ b/spec/services/repair_ldap_blocked_user_service_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe RepairLdapBlockedUserService, services: true do
+  let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
+  let(:identity) { user.ldap_identity }
+  subject(:service) { RepairLdapBlockedUserService.new(user, identity) }
+
+  describe '#execute' do
+    it 'change to normal block after destroying last ldap identity' do
+      identity.destroy
+      service.execute
+
+      expect(user.reload).not_to be_ldap_blocked
+    end
+
+    it 'change to normal block after changing last ldap identity to another provider' do
+      identity.update_attribute(:provider, 'twitter')
+      service.execute
+
+      expect(user.reload).not_to be_ldap_blocked
+    end
+  end
+end
-- 
cgit v1.2.1


From 47e4613f4adc2d6ef4b066a87ec772ef8044bdd5 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Thu, 7 Jan 2016 14:01:01 -0200
Subject: Code style fixes and some code simplified

---
 app/controllers/admin/identities_controller.rb         |  4 ++--
 app/services/repair_ldap_blocked_user_service.rb       | 18 ++++++++++--------
 lib/gitlab/ldap/access.rb                              |  4 +++-
 spec/services/repair_ldap_blocked_user_service_spec.rb |  2 +-
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/app/controllers/admin/identities_controller.rb b/app/controllers/admin/identities_controller.rb
index 9ba10487512..79a53556f0a 100644
--- a/app/controllers/admin/identities_controller.rb
+++ b/app/controllers/admin/identities_controller.rb
@@ -26,7 +26,7 @@ class Admin::IdentitiesController < Admin::ApplicationController
 
   def update
     if @identity.update_attributes(identity_params)
-      RepairLdapBlockedUserService.new(@user, @identity).execute
+      RepairLdapBlockedUserService.new(@user).execute
       redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully updated.'
     else
       render :edit
@@ -35,7 +35,7 @@ class Admin::IdentitiesController < Admin::ApplicationController
 
   def destroy
     if @identity.destroy
-      RepairLdapBlockedUserService.new(@user, @identity).execute
+      RepairLdapBlockedUserService.new(@user).execute
       redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully removed.'
     else
       redirect_to admin_user_identities_path(@user), alert: 'Failed to remove user identity.'
diff --git a/app/services/repair_ldap_blocked_user_service.rb b/app/services/repair_ldap_blocked_user_service.rb
index ceca15414e0..863cef7ff61 100644
--- a/app/services/repair_ldap_blocked_user_service.rb
+++ b/app/services/repair_ldap_blocked_user_service.rb
@@ -1,15 +1,17 @@
 class RepairLdapBlockedUserService
-  attr_accessor :user, :identity
+  attr_accessor :user
 
-  def initialize(user, identity)
-    @user, @identity = user, identity
+  def initialize(user)
+    @user = user
   end
 
   def execute
-    if identity.destroyed?
-      user.block if identity.is_ldap? && user.ldap_blocked? && !user.ldap_user?
-    else
-      user.block if !identity.is_ldap? && user.ldap_blocked? && !user.ldap_user?
-    end
+    user.block if ldap_hard_blocked?
+  end
+
+  private
+
+  def ldap_hard_blocked?
+    user.ldap_blocked? && !user.ldap_user?
   end
 end
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index 76cb48d7aa6..ebd9260ad5d 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -40,7 +40,9 @@ module Gitlab
             user.ldap_block
             false
           else
-            user.activate if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked?
+            if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked?
+              user.activate
+            end
             true
           end
         else
diff --git a/spec/services/repair_ldap_blocked_user_service_spec.rb b/spec/services/repair_ldap_blocked_user_service_spec.rb
index 2a2114d038c..ce7d1455975 100644
--- a/spec/services/repair_ldap_blocked_user_service_spec.rb
+++ b/spec/services/repair_ldap_blocked_user_service_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe RepairLdapBlockedUserService, services: true do
   let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
   let(:identity) { user.ldap_identity }
-  subject(:service) { RepairLdapBlockedUserService.new(user, identity) }
+  subject(:service) { RepairLdapBlockedUserService.new(user) }
 
   describe '#execute' do
     it 'change to normal block after destroying last ldap identity' do
-- 
cgit v1.2.1


From ac6a10f3e88c5d2081b8638df63016089517a844 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Tue, 12 Jan 2016 12:29:10 -0200
Subject: Codestyle changes

---
 app/models/identity.rb       | 2 +-
 lib/api/users.rb             | 6 +++---
 spec/models/identity_spec.rb | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/models/identity.rb b/app/models/identity.rb
index 830b99fa3f2..e1915b079d4 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -19,7 +19,7 @@ class Identity < ActiveRecord::Base
   validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
   validates :user_id, uniqueness: { scope: :provider }
 
-  def is_ldap?
+  def ldap?
     provider.starts_with?('ldap')
   end
 end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 01fd90139b0..fd2128bd179 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -303,10 +303,10 @@ module API
 
         if !user
           not_found!('User')
-        elsif !user.ldap_blocked?
-          user.activate
-        else
+        elsif user.ldap_blocked?
           forbidden!('LDAP blocked users cannot be unblocked by the API')
+        else
+          user.activate
         end
       end
     end
diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb
index 107bfc17782..5afe042e154 100644
--- a/spec/models/identity_spec.rb
+++ b/spec/models/identity_spec.rb
@@ -28,11 +28,11 @@ RSpec.describe Identity, models: true do
     let(:other_identity) { create(:identity, provider: 'twitter') }
 
     it 'returns true if it is a ldap identity' do
-      expect(ldap_identity.is_ldap?).to be_truthy
+      expect(ldap_identity.ldap?).to be_truthy
     end
 
     it 'returns false if it is not a ldap identity' do
-      expect(other_identity.is_ldap?).to be_falsey
+      expect(other_identity.ldap?).to be_falsey
     end
   end
 end
-- 
cgit v1.2.1


From efb3395b4fc0425ebbc2437ad03f0cd5fc851863 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Tue, 12 Jan 2016 19:32:44 +0100
Subject: Remove blank line

---
 lib/api/api.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/api/api.rb b/lib/api/api.rb
index a9e1913f0f2..098dd975840 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -54,7 +54,6 @@ module API
     mount Keys
     mount Tags
     mount Triggers
-
     mount Variables
   end
 end
-- 
cgit v1.2.1


From 9d7f88c12258e27a189e8229090920db0627e88b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Tue, 12 Jan 2016 18:10:06 +0100
Subject: Show referenced MRs & Issues only when the current viewer can access
 them

---
 CHANGELOG                                     |   1 +
 app/controllers/projects/issues_controller.rb |   2 +-
 app/models/issue.rb                           |   4 +-
 app/views/projects/notes/_notes.html.haml     |   1 +
 features/project/issues/references.feature    |  31 ++++++++
 features/steps/project/issues/references.rb   | 106 ++++++++++++++++++++++++++
 features/steps/shared/note.rb                 |   6 ++
 features/steps/shared/project.rb              |   7 ++
 8 files changed, 155 insertions(+), 3 deletions(-)
 create mode 100644 features/project/issues/references.feature
 create mode 100644 features/steps/project/issues/references.rb

diff --git a/CHANGELOG b/CHANGELOG
index ab34661ce05..f765899b42d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -38,6 +38,7 @@ v 8.4.0 (unreleased)
   - Ajax filter by message for commits page
   - API: Add support for deleting a tag via the API (Robert Schilling)
   - Allow subsequent validations in CI Linter
+  - Show referenced MRs & Issues only when the current viewer can access them
 
 v 8.3.3
   - Preserve CE behavior with JIRA integration by only calling API if URL is set
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index b59b52291fb..f476afb2d92 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -61,7 +61,7 @@ class Projects::IssuesController < Projects::ApplicationController
     @note = @project.notes.new(noteable: @issue)
     @notes = @issue.notes.nonawards.with_associations.fresh
     @noteable = @issue
-    @merge_requests = @issue.referenced_merge_requests
+    @merge_requests = @issue.referenced_merge_requests(current_user)
 
     respond_with(@issue)
   end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index f52e47f3e62..7beba984608 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -85,10 +85,10 @@ class Issue < ActiveRecord::Base
     reference
   end
 
-  def referenced_merge_requests
+  def referenced_merge_requests(current_user = nil)
     Gitlab::ReferenceExtractor.lazily do
       [self, *notes].flat_map do |note|
-        note.all_references.merge_requests
+        note.all_references(current_user).merge_requests
       end
     end.sort_by(&:iid)
   end
diff --git a/app/views/projects/notes/_notes.html.haml b/app/views/projects/notes/_notes.html.haml
index ca60dd239b2..a4ff947c656 100644
--- a/app/views/projects/notes/_notes.html.haml
+++ b/app/views/projects/notes/_notes.html.haml
@@ -8,4 +8,5 @@
 - else
   - @notes.each do |note|
     - next unless note.author
+    - next if note.cross_reference? && note.referenced_mentionables(current_user).empty?
     = render note
diff --git a/features/project/issues/references.feature b/features/project/issues/references.feature
new file mode 100644
index 00000000000..bf7a4c6cb91
--- /dev/null
+++ b/features/project/issues/references.feature
@@ -0,0 +1,31 @@
+@project_issues
+Feature: Project Issues References
+  Background:
+    Given I sign in as "John Doe"
+    And "John Doe" owns public project "Community"
+    And project "Community" has "Public Issue 01" open issue
+    And I logout
+    And I sign in as "Mary Jane"
+    And "Mary Jane" owns private project "Private Library"
+    And project "Private Library" has "Fix NS-01" open merge request
+    And project "Private Library" has "Private Issue 01" open issue
+    And I visit merge request page "Fix NS-01"
+    And I leave a comment referencing issue "Public Issue 01" from "Fix NS-01" merge request
+    And I visit issue page "Private Issue 01"
+    And I leave a comment referencing issue "Public Issue 01" from "Private Issue 01" issue
+    And I logout
+
+  @javascript
+  Scenario: Viewing the public issue as a "John Doe"
+    Given I sign in as "John Doe"
+    When I visit issue page "Public Issue 01"
+    Then I should not see any related merge requests
+    And I should see no notes at all
+
+  @javascript
+  Scenario: Viewing the public issue as "Mary Jane"
+    Given I sign in as "Mary Jane"
+    When I visit issue page "Public Issue 01"
+    Then I should see the "Fix NS-01" related merge request
+    And I should see a note linking to "Fix NS-01" merge request
+    And I should see a note linking to "Private Issue 01" issue
diff --git a/features/steps/project/issues/references.rb b/features/steps/project/issues/references.rb
new file mode 100644
index 00000000000..9c7725e8c14
--- /dev/null
+++ b/features/steps/project/issues/references.rb
@@ -0,0 +1,106 @@
+class Spinach::Features::ProjectIssuesReferences < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedNote
+  include SharedProject
+  include SharedUser
+
+  step 'project "Community" has "Public Issue 01" open issue' do
+    project = Project.find_by(name: 'Community')
+    create(:issue,
+           title: 'Public Issue 01',
+           project: project,
+           author: project.users.first,
+           description: '# Description header'
+          )
+  end
+
+  step 'project "Private Library" has "Fix NS-01" open merge request' do
+    project = Project.find_by(name: 'Private Library')
+    create(:merge_request,
+           title: 'Fix NS-01',
+           source_project: project,
+           target_project: project,
+           source_branch: 'fix',
+           target_branch: 'master',
+           author: project.users.first,
+           description: '# Description header'
+          )
+  end
+
+  step 'project "Private Library" has "Private Issue 01" open issue' do
+    project = Project.find_by(name: 'Private Library')
+    create(:issue,
+           title: 'Private Issue 01',
+           project: project,
+           author: project.users.first,
+           description: '# Description header'
+          )
+  end
+
+  step 'I leave a comment referencing issue "Public Issue 01" from "Fix NS-01" merge request' do
+    project = Project.find_by(name: 'Private Library')
+    issue = Issue.find_by!(title: 'Public Issue 01')
+
+    page.within(".js-main-target-form") do
+      fill_in "note[note]", with: "##{issue.to_reference(project)}"
+      click_button "Add Comment"
+    end
+  end
+
+  step 'I leave a comment referencing issue "Public Issue 01" from "Private Issue 01" issue' do
+    project = Project.find_by(name: 'Private Library')
+    issue = Issue.find_by!(title: 'Public Issue 01')
+
+    page.within(".js-main-target-form") do
+      fill_in "note[note]", with: "##{issue.to_reference(project)}"
+      click_button "Add Comment"
+    end
+  end
+
+  step 'I visit merge request page "Fix NS-01"' do
+    mr = MergeRequest.find_by(title: "Fix NS-01")
+    visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr)
+  end
+
+  step 'I visit issue page "Private Issue 01"' do
+    issue = Issue.find_by(title: "Private Issue 01")
+    visit namespace_project_issue_path(issue.project.namespace, issue.project, issue)
+  end
+
+  step 'I visit issue page "Public Issue 01"' do
+    issue = Issue.find_by(title: "Public Issue 01")
+    visit namespace_project_issue_path(issue.project.namespace, issue.project, issue)
+  end
+
+  step 'I should not see any related merge requests' do
+    page.within '.issue-details' do
+      expect(page).not_to have_content('.merge-requests')
+    end
+  end
+
+  step 'I should see the "Fix NS-01" related merge request' do
+    page.within '.merge-requests' do
+      expect(page).to have_content("1 Related Merge Request")
+      expect(page).to have_content('Fix NS-01')
+    end
+  end
+
+  step 'I should see a note linking to "Fix NS-01" merge request' do
+    project = Project.find_by(name: 'Community')
+    mr = MergeRequest.find_by(title: 'Fix NS-01')
+    page.within('.notes') do
+      expect(page).to have_content('Mary Jane')
+      expect(page).to have_content("mentioned in merge request #{mr.to_reference(project)}")
+    end
+  end
+
+  step 'I should see a note linking to "Private Issue 01" issue' do
+    project = Project.find_by(name: 'Community')
+    issue = Issue.find_by(title: 'Private Issue 01')
+    page.within('.notes') do
+      expect(page).to have_content('Mary Jane')
+      expect(page).to have_content("mentioned in issue #{issue.to_reference(project)}")
+    end
+  end
+
+end
diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb
index f6aabfefeff..6de58c6e2b1 100644
--- a/features/steps/shared/note.rb
+++ b/features/steps/shared/note.rb
@@ -106,6 +106,12 @@ module SharedNote
     end
   end
 
+  step 'I should see no notes at all' do
+    page.within('.notes') do
+      expect(page).to_not have_css('.note')
+    end
+  end
+
   # Markdown
 
   step 'I leave a comment with a header containing "Comment with a header"' do
diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb
index da643bf3ba9..43a15f43470 100644
--- a/features/steps/shared/project.rb
+++ b/features/steps/shared/project.rb
@@ -181,6 +181,13 @@ module SharedProject
     project.team << [user, :master]
   end
 
+  step '"Mary Jane" owns private project "Private Library"' do
+    user = user_exists('Mary Jane', username: 'mary_jane')
+    project = Project.find_by(name: 'Private Library')
+    project ||= create(:project, name: 'Private Library', namespace: user.namespace)
+    project.team << [user, :master]
+  end
+
   step 'public empty project "Empty Public Project"' do
     create :project_empty_repo, :public, name: "Empty Public Project"
   end
-- 
cgit v1.2.1


From df548285804fdc40ac7c4f36601e87a534792a4a Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Wed, 13 Jan 2016 12:47:11 +0100
Subject: Add some fixes after review

---
 lib/api/variables.rb | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index cc038e5731d..0c3fb5c8a77 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -27,11 +27,11 @@ module API
       #   GET /projects/:id/variables/:key
       get ':id/variables/:key' do
         key = params[:key]
-        variables = user_project.variables.where(key: key)
+        variable = user_project.variables.find_by(key: key.to_s)
 
-        return not_found!('Variable') if variables.empty?
+        return not_found!('Variable') unless variable
 
-        present variables.first, with: Entities::Variable
+        present variable, with: Entities::Variable
       end
 
       # Create a new variable in project
@@ -46,10 +46,12 @@ module API
         required_attributes! [:key, :value]
 
         variable = user_project.variables.create(key: params[:key], value: params[:value])
-        return render_validation_error!(variable) unless variable.valid?
-        variable.save!
 
-        present variable, with: Entities::Variable
+        if variable.valid?
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
+        end
       end
 
       # Update existing variable of a project
@@ -61,14 +63,16 @@ module API
       # Example Request:
       #   PUT /projects/:id/variables/:key
       put ':id/variables/:key' do
-        variable = user_project.variables.where(key: params[:key]).first
+        variable = user_project.variables.find_by(key: params[:key].to_s)
 
         return not_found!('Variable') unless variable
 
-        variable.value = params[:value]
-        variable.save!
-
-        present variable, with: Entities::Variable
+        attrs = attributes_for_keys [:value]
+        if variable.update(attrs)
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
+        end
       end
 
       # Delete existing variable of a project
@@ -79,10 +83,9 @@ module API
       # Exanoke Reqyest:
       #   DELETE /projects/:id/variables/:key
       delete ':id/variables/:key' do
-        variable = user_project.variables.where(key: params[:key]).first
+        variable = user_project.variables.find_by(key: params[:key].to_s)
 
         return not_found!('Variable') unless variable
-
         variable.destroy
 
         present variable, with: Entities::Variable
-- 
cgit v1.2.1


From 99393cde942841e1bee656cebf8e16a25d1300cb Mon Sep 17 00:00:00 2001
From: Marin Jankovski <maxlazio@gmail.com>
Date: Wed, 13 Jan 2016 13:49:10 +0100
Subject: Version 8.4.0.rc1

---
 VERSION | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/VERSION b/VERSION
index ce669730119..408340137f0 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-8.4.0.pre
+8.4.0.rc1
\ No newline at end of file
-- 
cgit v1.2.1


From 5efbfa14d4666655edd6d79a3a352a134382b664 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 13 Jan 2016 16:37:17 +0100
Subject: Move complex view condition to a model method

This is moved to a model method rather than an helper method because the
API will need it too.
---
 app/models/note.rb                        | 4 ++++
 app/views/projects/notes/_notes.html.haml | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/models/note.rb b/app/models/note.rb
index 3d5b663c99f..3e1375e5ad6 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -358,6 +358,10 @@ class Note < ActiveRecord::Base
     !system? && !is_award
   end
 
+  def cross_reference_not_visible_for?(user)
+    cross_reference? && referenced_mentionables(user).empty?
+  end
+
   # Checks if note is an award added as a comment
   #
   # If note is an award, this method sets is_award to true
diff --git a/app/views/projects/notes/_notes.html.haml b/app/views/projects/notes/_notes.html.haml
index a4ff947c656..62db86fb181 100644
--- a/app/views/projects/notes/_notes.html.haml
+++ b/app/views/projects/notes/_notes.html.haml
@@ -2,11 +2,14 @@
   - @discussions.each do |discussion_notes|
     - note = discussion_notes.first
     - if note_for_main_target?(note)
+      - next if note.cross_reference_not_visible_for?(current_user)
+
       = render discussion_notes
     - else
       = render 'projects/notes/discussion', discussion_notes: discussion_notes
 - else
   - @notes.each do |note|
     - next unless note.author
-    - next if note.cross_reference? && note.referenced_mentionables(current_user).empty?
+    - next if note.cross_reference_not_visible_for?(current_user)
+
     = render note
-- 
cgit v1.2.1


From 5e452d3794ffa4996611ecf53c6098f4a3913d4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 13 Jan 2016 16:38:42 +0100
Subject: Improve & adds specs for Issue/MR references

- Improve specs for private Issue/MR referenced in public Issue
- Add specs for private Issue/MR referenced in public MR
---
 features/project/issues/references.feature         |  28 +++--
 features/project/merge_requests/references.feature |  31 +++++
 features/steps/project/issues/references.rb        | 101 +---------------
 .../steps/project/merge_requests/references.rb     |   7 ++
 features/steps/shared/issuable.rb                  | 134 +++++++++++++++++++++
 features/steps/shared/note.rb                      |   4 +-
 features/steps/shared/project.rb                   |  48 +++++---
 7 files changed, 218 insertions(+), 135 deletions(-)
 create mode 100644 features/project/merge_requests/references.feature
 create mode 100644 features/steps/project/merge_requests/references.rb

diff --git a/features/project/issues/references.feature b/features/project/issues/references.feature
index bf7a4c6cb91..4ae2d653337 100644
--- a/features/project/issues/references.feature
+++ b/features/project/issues/references.feature
@@ -2,30 +2,32 @@
 Feature: Project Issues References
   Background:
     Given I sign in as "John Doe"
+    And public project "Community"
     And "John Doe" owns public project "Community"
-    And project "Community" has "Public Issue 01" open issue
+    And project "Community" has "Community issue" open issue
     And I logout
     And I sign in as "Mary Jane"
-    And "Mary Jane" owns private project "Private Library"
-    And project "Private Library" has "Fix NS-01" open merge request
-    And project "Private Library" has "Private Issue 01" open issue
-    And I visit merge request page "Fix NS-01"
-    And I leave a comment referencing issue "Public Issue 01" from "Fix NS-01" merge request
-    And I visit issue page "Private Issue 01"
-    And I leave a comment referencing issue "Public Issue 01" from "Private Issue 01" issue
+    And private project "Enterprise"
+    And "Mary Jane" owns private project "Enterprise"
+    And project "Enterprise" has "Enterprise issue" open issue
+    And project "Enterprise" has "Enterprise fix" open merge request
+    And I visit issue page "Enterprise issue"
+    And I leave a comment referencing issue "Community issue"
+    And I visit merge request page "Enterprise fix"
+    And I leave a comment referencing issue "Community issue"
     And I logout
 
   @javascript
   Scenario: Viewing the public issue as a "John Doe"
     Given I sign in as "John Doe"
-    When I visit issue page "Public Issue 01"
+    When I visit issue page "Community issue"
     Then I should not see any related merge requests
     And I should see no notes at all
 
   @javascript
   Scenario: Viewing the public issue as "Mary Jane"
     Given I sign in as "Mary Jane"
-    When I visit issue page "Public Issue 01"
-    Then I should see the "Fix NS-01" related merge request
-    And I should see a note linking to "Fix NS-01" merge request
-    And I should see a note linking to "Private Issue 01" issue
+    When I visit issue page "Community issue"
+    Then I should see the "Enterprise fix" related merge request
+    And I should see a note linking to "Enterprise fix" merge request
+    And I should see a note linking to "Enterprise issue" issue
diff --git a/features/project/merge_requests/references.feature b/features/project/merge_requests/references.feature
new file mode 100644
index 00000000000..571612261a9
--- /dev/null
+++ b/features/project/merge_requests/references.feature
@@ -0,0 +1,31 @@
+@project_merge_requests
+Feature: Project Merge Requests References
+  Background:
+    Given I sign in as "John Doe"
+    And public project "Community"
+    And "John Doe" owns public project "Community"
+    And project "Community" has "Community fix" open merge request
+    And I logout
+    And I sign in as "Mary Jane"
+    And private project "Enterprise"
+    And "Mary Jane" owns private project "Enterprise"
+    And project "Enterprise" has "Enterprise issue" open issue
+    And project "Enterprise" has "Enterprise fix" open merge request
+    And I visit issue page "Enterprise issue"
+    And I leave a comment referencing issue "Community fix"
+    And I visit merge request page "Enterprise fix"
+    And I leave a comment referencing issue "Community fix"
+    And I logout
+
+  @javascript
+  Scenario: Viewing the public issue as a "John Doe"
+    Given I sign in as "John Doe"
+    When I visit issue page "Community fix"
+    Then I should see no notes at all
+
+  @javascript
+  Scenario: Viewing the public issue as "Mary Jane"
+    Given I sign in as "Mary Jane"
+    When I visit issue page "Community fix"
+    And I should see a note linking to "Enterprise fix" merge request
+    And I should see a note linking to "Enterprise issue" issue
diff --git a/features/steps/project/issues/references.rb b/features/steps/project/issues/references.rb
index 9c7725e8c14..69e8b5cbde5 100644
--- a/features/steps/project/issues/references.rb
+++ b/features/steps/project/issues/references.rb
@@ -1,106 +1,7 @@
 class Spinach::Features::ProjectIssuesReferences < Spinach::FeatureSteps
   include SharedAuthentication
+  include SharedIssuable
   include SharedNote
   include SharedProject
   include SharedUser
-
-  step 'project "Community" has "Public Issue 01" open issue' do
-    project = Project.find_by(name: 'Community')
-    create(:issue,
-           title: 'Public Issue 01',
-           project: project,
-           author: project.users.first,
-           description: '# Description header'
-          )
-  end
-
-  step 'project "Private Library" has "Fix NS-01" open merge request' do
-    project = Project.find_by(name: 'Private Library')
-    create(:merge_request,
-           title: 'Fix NS-01',
-           source_project: project,
-           target_project: project,
-           source_branch: 'fix',
-           target_branch: 'master',
-           author: project.users.first,
-           description: '# Description header'
-          )
-  end
-
-  step 'project "Private Library" has "Private Issue 01" open issue' do
-    project = Project.find_by(name: 'Private Library')
-    create(:issue,
-           title: 'Private Issue 01',
-           project: project,
-           author: project.users.first,
-           description: '# Description header'
-          )
-  end
-
-  step 'I leave a comment referencing issue "Public Issue 01" from "Fix NS-01" merge request' do
-    project = Project.find_by(name: 'Private Library')
-    issue = Issue.find_by!(title: 'Public Issue 01')
-
-    page.within(".js-main-target-form") do
-      fill_in "note[note]", with: "##{issue.to_reference(project)}"
-      click_button "Add Comment"
-    end
-  end
-
-  step 'I leave a comment referencing issue "Public Issue 01" from "Private Issue 01" issue' do
-    project = Project.find_by(name: 'Private Library')
-    issue = Issue.find_by!(title: 'Public Issue 01')
-
-    page.within(".js-main-target-form") do
-      fill_in "note[note]", with: "##{issue.to_reference(project)}"
-      click_button "Add Comment"
-    end
-  end
-
-  step 'I visit merge request page "Fix NS-01"' do
-    mr = MergeRequest.find_by(title: "Fix NS-01")
-    visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr)
-  end
-
-  step 'I visit issue page "Private Issue 01"' do
-    issue = Issue.find_by(title: "Private Issue 01")
-    visit namespace_project_issue_path(issue.project.namespace, issue.project, issue)
-  end
-
-  step 'I visit issue page "Public Issue 01"' do
-    issue = Issue.find_by(title: "Public Issue 01")
-    visit namespace_project_issue_path(issue.project.namespace, issue.project, issue)
-  end
-
-  step 'I should not see any related merge requests' do
-    page.within '.issue-details' do
-      expect(page).not_to have_content('.merge-requests')
-    end
-  end
-
-  step 'I should see the "Fix NS-01" related merge request' do
-    page.within '.merge-requests' do
-      expect(page).to have_content("1 Related Merge Request")
-      expect(page).to have_content('Fix NS-01')
-    end
-  end
-
-  step 'I should see a note linking to "Fix NS-01" merge request' do
-    project = Project.find_by(name: 'Community')
-    mr = MergeRequest.find_by(title: 'Fix NS-01')
-    page.within('.notes') do
-      expect(page).to have_content('Mary Jane')
-      expect(page).to have_content("mentioned in merge request #{mr.to_reference(project)}")
-    end
-  end
-
-  step 'I should see a note linking to "Private Issue 01" issue' do
-    project = Project.find_by(name: 'Community')
-    issue = Issue.find_by(title: 'Private Issue 01')
-    page.within('.notes') do
-      expect(page).to have_content('Mary Jane')
-      expect(page).to have_content("mentioned in issue #{issue.to_reference(project)}")
-    end
-  end
-
 end
diff --git a/features/steps/project/merge_requests/references.rb b/features/steps/project/merge_requests/references.rb
new file mode 100644
index 00000000000..ab2ae6847a2
--- /dev/null
+++ b/features/steps/project/merge_requests/references.rb
@@ -0,0 +1,7 @@
+class Spinach::Features::ProjectMergeRequestsReferences < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedIssuable
+  include SharedNote
+  include SharedProject
+  include SharedUser
+end
diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb
index e6d1b8b8efc..4c5f7488efb 100644
--- a/features/steps/shared/issuable.rb
+++ b/features/steps/shared/issuable.rb
@@ -5,6 +5,99 @@ module SharedIssuable
     find(:css, '.issuable-edit').click
   end
 
+  step 'project "Community" has "Community issue" open issue' do
+    create_issuable_for_project(
+      project_name: 'Community',
+      title: 'Community issue'
+    )
+  end
+
+  step 'project "Community" has "Community fix" open merge request' do
+    create_issuable_for_project(
+      project_name: 'Community',
+      type: :merge_request,
+      title: 'Community fix'
+    )
+  end
+
+  step 'project "Enterprise" has "Enterprise issue" open issue' do
+    create_issuable_for_project(
+      project_name: 'Enterprise',
+      title: 'Enterprise issue'
+    )
+  end
+
+  step 'project "Enterprise" has "Enterprise fix" open merge request' do
+    create_issuable_for_project(
+      project_name: 'Enterprise',
+      type: :merge_request,
+      title: 'Enterprise fix'
+    )
+  end
+
+  step 'I leave a comment referencing issue "Community issue"' do
+    leave_reference_comment(
+      issuable: Issue.find_by(title: 'Community issue'),
+      from_project_name: 'Enterprise'
+    )
+  end
+
+  step 'I leave a comment referencing issue "Community fix"' do
+    leave_reference_comment(
+      issuable: MergeRequest.find_by(title: 'Community fix'),
+      from_project_name: 'Enterprise'
+    )
+  end
+
+  step 'I visit issue page "Enterprise issue"' do
+    issue = Issue.find_by(title: 'Enterprise issue')
+    visit namespace_project_issue_path(issue.project.namespace, issue.project, issue)
+  end
+
+  step 'I visit merge request page "Enterprise fix"' do
+    mr = MergeRequest.find_by(title: 'Enterprise fix')
+    visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr)
+  end
+
+  step 'I visit issue page "Community issue"' do
+    issue = Issue.find_by(title: 'Community issue')
+    visit namespace_project_issue_path(issue.project.namespace, issue.project, issue)
+  end
+
+  step 'I visit issue page "Community fix"' do
+    mr = MergeRequest.find_by(title: 'Community fix')
+    visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr)
+  end
+
+  step 'I should not see any related merge requests' do
+    page.within '.issue-details' do
+      expect(page).not_to have_content('.merge-requests')
+    end
+  end
+
+  step 'I should see the "Enterprise fix" related merge request' do
+    page.within '.merge-requests' do
+      expect(page).to have_content('1 Related Merge Request')
+      expect(page).to have_content('Enterprise fix')
+    end
+  end
+
+  step 'I should see a note linking to "Enterprise fix" merge request' do
+    visible_note(
+      issuable: MergeRequest.find_by(title: 'Enterprise fix'),
+      from_project_name: 'Community',
+      user_name: 'Mary Jane'
+    )
+  end
+
+  step 'I should see a note linking to "Enterprise issue" issue' do
+    visible_note(
+      issuable: Issue.find_by(title: 'Enterprise issue'),
+      from_project_name: 'Community',
+      user_name: 'Mary Jane'
+    )
+  end
+
   step 'I click link "Edit" for the merge request' do
     edit_issuable
   end
@@ -12,4 +105,45 @@ module SharedIssuable
   step 'I click link "Edit" for the issue' do
     edit_issuable
   end
+
+  def create_issuable_for_project(project_name:, title:, type: :issue)
+    project = Project.find_by(name: project_name)
+
+    attrs = {
+      title: title,
+      author: project.users.first,
+      description: '# Description header'
+    }
+
+    case type
+    when :issue
+      attrs.merge!(project: project)
+    when :merge_request
+      attrs.merge!(
+        source_project: project,
+        target_project: project,
+        source_branch: 'fix',
+        target_branch: 'master'
+      )
+    end
+
+    create(type, attrs)
+  end
+
+  def leave_reference_comment(issuable:, from_project_name:)
+    project = Project.find_by(name: from_project_name)
+
+    page.within('.js-main-target-form') do
+      fill_in 'note[note]', with: "##{issuable.to_reference(project)}"
+      click_button 'Add Comment'
+    end
+  end
+
+  def visible_note(issuable:, from_project_name:, user_name:)
+    project = Project.find_by(name: from_project_name)
+
+    expect(page).to have_content(user_name)
+    expect(page).to have_content("mentioned in #{issuable.class.to_s.titleize.downcase} #{issuable.to_reference(project)}")
+  end
+
 end
diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb
index 6de58c6e2b1..444d6726f99 100644
--- a/features/steps/shared/note.rb
+++ b/features/steps/shared/note.rb
@@ -107,9 +107,7 @@ module SharedNote
   end
 
   step 'I should see no notes at all' do
-    page.within('.notes') do
-      expect(page).to_not have_css('.note')
-    end
+    expect(page).to_not have_css('.note')
   end
 
   # Markdown
diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb
index 43a15f43470..5420c451519 100644
--- a/features/steps/shared/project.rb
+++ b/features/steps/shared/project.rb
@@ -161,31 +161,33 @@ module SharedProject
   end
 
   step '"John Doe" owns private project "Enterprise"' do
-    user = user_exists("John Doe", username: "john_doe")
-    project = Project.find_by(name: "Enterprise")
-    project ||= create(:empty_project, name: "Enterprise", namespace: user.namespace)
-    project.team << [user, :master]
+    user_owns_project(
+      user_name: 'John Doe',
+      project_name: 'Enterprise'
+    )
   end
 
-  step '"John Doe" owns internal project "Internal"' do
-    user = user_exists("John Doe", username: "john_doe")
-    project = Project.find_by(name: "Internal")
-    project ||= create :empty_project, :internal, name: 'Internal', namespace: user.namespace
-    project.team << [user, :master]
+  step '"Mary Jane" owns private project "Enterprise"' do
+    user_owns_project(
+      user_name: 'Mary Jane',
+      project_name: 'Enterprise'
+    )
   end
 
-  step '"John Doe" owns public project "Community"' do
-    user = user_exists("John Doe", username: "john_doe")
-    project = Project.find_by(name: "Community")
-    project ||= create :empty_project, :public, name: 'Community', namespace: user.namespace
-    project.team << [user, :master]
+  step '"John Doe" owns internal project "Internal"' do
+    user_owns_project(
+      user_name: 'John Doe',
+      project_name: 'Internal',
+      visibility: :internal
+    )
   end
 
-  step '"Mary Jane" owns private project "Private Library"' do
-    user = user_exists('Mary Jane', username: 'mary_jane')
-    project = Project.find_by(name: 'Private Library')
-    project ||= create(:project, name: 'Private Library', namespace: user.namespace)
-    project.team << [user, :master]
+  step '"John Doe" owns public project "Community"' do
+    user_owns_project(
+      user_name: 'John Doe',
+      project_name: 'Community',
+      visibility: :public
+    )
   end
 
   step 'public empty project "Empty Public Project"' do
@@ -220,4 +222,12 @@ module SharedProject
       expect(page).to have_content("skipped")
     end
   end
+
+  def user_owns_project(user_name:, project_name:, visibility: :private)
+    user = user_exists(user_name, username: user_name.underscore)
+    project = Project.find_by(name: project_name)
+    project ||= create(:empty_project, visibility, name: project_name, namespace: user.namespace)
+    project.team << [user, :master]
+  end
+
 end
-- 
cgit v1.2.1


From 79361b207ed70e1b954ca85c97a86e459e3e3976 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 14:49:02 -0500
Subject: Add js-requires-input to form

---
 app/views/admin/broadcast_messages/index.html.haml | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 17dffebd360..5d8250f787f 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -7,7 +7,7 @@
   %i.fa.fa-bullhorn
   %span Your message here
 
-= form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal'} do |f|
+= form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal js-requires-input'} do |f|
   -if @broadcast_message.errors.any?
     .alert.alert-danger
       - @broadcast_message.errors.full_messages.each do |msg|
@@ -17,8 +17,7 @@
     .col-sm-10
       = f.text_area :message, class: "form-control", rows: 2, required: true
       %div
-        = link_to '#', class: 'js-toggle-colors-link' do
-          Customize colors
+        = link_to 'Customize colors', '#', class: 'js-toggle-colors-link'
   .form-group.js-toggle-colors-container.hide
     = f.label :color, "Background Color", class: 'control-label'
     .col-sm-10
-- 
cgit v1.2.1


From 1c7febc3e38053e8d01cf7a30d3da23b95f30808 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 15:08:42 -0500
Subject: Style the broadcast message form

---
 app/assets/stylesheets/framework/forms.scss        |  4 ++++
 app/views/admin/broadcast_messages/index.html.haml | 13 ++++++++-----
 app/views/layouts/_broadcast.html.haml             |  2 +-
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/app/assets/stylesheets/framework/forms.scss b/app/assets/stylesheets/framework/forms.scss
index 032d343df44..d0cb91a5b64 100644
--- a/app/assets/stylesheets/framework/forms.scss
+++ b/app/assets/stylesheets/framework/forms.scss
@@ -78,6 +78,10 @@ label {
   padding: 8px $gl-padding;
 }
 
+.form-control-inline {
+  display: inline;
+}
+
 .wiki-content {
   margin-top: 35px;
 }
diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 5d8250f787f..6e8122a5137 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -1,10 +1,13 @@
 - page_title "Broadcast Messages"
+
 %h3.page-title
   Broadcast Messages
 %p.light
-  Broadcast messages are displayed for every user and can be used to notify users about scheduled maintenance, recent upgrades and more.
+  Broadcast messages are displayed for every user and can be used to notify
+  users about scheduled maintenance, recent upgrades and more.
+
 .broadcast-message-preview
-  %i.fa.fa-bullhorn
+  = icon('bullhorn')
   %span Your message here
 
 = form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal js-requires-input'} do |f|
@@ -21,7 +24,7 @@
   .form-group.js-toggle-colors-container.hide
     = f.label :color, "Background Color", class: 'control-label'
     .col-sm-10
-      = f.color_field :color, value: "#eb9532", class: "form-control"
+      = f.color_field :color, value: "#E75E40", class: "form-control"
   .form-group.js-toggle-colors-container.hide
     = f.label :font, "Font Color", class: 'control-label'
     .col-sm-10
@@ -29,11 +32,11 @@
   .form-group
     = f.label :starts_at, class: 'control-label'
     .col-sm-10.datetime-controls
-      = f.datetime_select :starts_at
+      = f.datetime_select :starts_at, {}, class: 'form-control form-control-inline'
   .form-group
     = f.label :ends_at, class: 'control-label'
     .col-sm-10.datetime-controls
-      = f.datetime_select :ends_at
+      = f.datetime_select :ends_at, {}, class: 'form-control form-control-inline'
   .form-actions
     = f.submit "Add broadcast message", class: "btn btn-create"
 
diff --git a/app/views/layouts/_broadcast.html.haml b/app/views/layouts/_broadcast.html.haml
index e7d477c225e..ad7851c0aa3 100644
--- a/app/views/layouts/_broadcast.html.haml
+++ b/app/views/layouts/_broadcast.html.haml
@@ -1,4 +1,4 @@
 - if broadcast_message.present?
   .broadcast-message{ style: broadcast_styling(broadcast_message) }
-    %i.fa.fa-bullhorn
+    = icon('bullhorn')
     = broadcast_message.message
-- 
cgit v1.2.1


From 7e24c5c45afe1ada044a782f27d3f3413043b1e6 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 15:19:13 -0500
Subject: Move broadcast message form to a partial

---
 app/assets/javascripts/admin.js.coffee             |  3 +-
 app/views/admin/broadcast_messages/_form.html.haml | 37 ++++++++++++++++++++++
 app/views/admin/broadcast_messages/index.html.haml | 33 +------------------
 3 files changed, 39 insertions(+), 34 deletions(-)
 create mode 100644 app/views/admin/broadcast_messages/_form.html.haml

diff --git a/app/assets/javascripts/admin.js.coffee b/app/assets/javascripts/admin.js.coffee
index bcb2e6df7c0..e9a26dd7200 100644
--- a/app/assets/javascripts/admin.js.coffee
+++ b/app/assets/javascripts/admin.js.coffee
@@ -10,8 +10,7 @@ class @Admin
 
     $('body').on 'click', '.js-toggle-colors-link', (e) ->
       e.preventDefault()
-      $('.js-toggle-colors-link').hide()
-      $('.js-toggle-colors-container').show()
+      $('.js-toggle-colors-container').toggle()
 
     $('input#broadcast_message_color').on 'input', ->
       previewColor = $('input#broadcast_message_color').val()
diff --git a/app/views/admin/broadcast_messages/_form.html.haml b/app/views/admin/broadcast_messages/_form.html.haml
new file mode 100644
index 00000000000..0ca3be0bd54
--- /dev/null
+++ b/app/views/admin/broadcast_messages/_form.html.haml
@@ -0,0 +1,37 @@
+.broadcast-message-preview
+  = icon('bullhorn')
+  %span Your message here
+
+= form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal js-requires-input'} do |f|
+  -if @broadcast_message.errors.any?
+    .alert.alert-danger
+      - @broadcast_message.errors.full_messages.each do |msg|
+        %p= msg
+  .form-group
+    = f.label :message, class: 'control-label'
+    .col-sm-10
+      = f.text_area :message, class: "form-control", rows: 2, required: true
+  .form-group.js-toggle-colors-container
+    .col-sm-10.col-sm-offset-2
+      = link_to 'Customize colors', '#', class: 'js-toggle-colors-link'
+  .form-group.js-toggle-colors-container.hide
+    = f.label :color, "Background Color", class: 'control-label'
+    .col-sm-10
+      = f.color_field :color, value: "#E75E40", class: "form-control"
+  .form-group.js-toggle-colors-container.hide
+    = f.label :font, "Font Color", class: 'control-label'
+    .col-sm-10
+      = f.color_field :font, value: "#FFFFFF", class: "form-control"
+  .form-group
+    = f.label :starts_at, class: 'control-label'
+    .col-sm-10.datetime-controls
+      = f.datetime_select :starts_at, {}, class: 'form-control form-control-inline'
+  .form-group
+    = f.label :ends_at, class: 'control-label'
+    .col-sm-10.datetime-controls
+      = f.datetime_select :ends_at, {}, class: 'form-control form-control-inline'
+  .form-actions
+    - if @broadcast_message.persisted?
+      = f.submit "Update broadcast message", class: "btn btn-create"
+    - else
+      = f.submit "Add broadcast message", class: "btn btn-create"
diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 6e8122a5137..04864d4afef 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -6,39 +6,8 @@
   Broadcast messages are displayed for every user and can be used to notify
   users about scheduled maintenance, recent upgrades and more.
 
-.broadcast-message-preview
-  = icon('bullhorn')
-  %span Your message here
+= render 'form'
 
-= form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal js-requires-input'} do |f|
-  -if @broadcast_message.errors.any?
-    .alert.alert-danger
-      - @broadcast_message.errors.full_messages.each do |msg|
-        %p= msg
-  .form-group
-    = f.label :message, class: 'control-label'
-    .col-sm-10
-      = f.text_area :message, class: "form-control", rows: 2, required: true
-      %div
-        = link_to 'Customize colors', '#', class: 'js-toggle-colors-link'
-  .form-group.js-toggle-colors-container.hide
-    = f.label :color, "Background Color", class: 'control-label'
-    .col-sm-10
-      = f.color_field :color, value: "#E75E40", class: "form-control"
-  .form-group.js-toggle-colors-container.hide
-    = f.label :font, "Font Color", class: 'control-label'
-    .col-sm-10
-      = f.color_field :font, value: "#FFFFFF", class: "form-control"
-  .form-group
-    = f.label :starts_at, class: 'control-label'
-    .col-sm-10.datetime-controls
-      = f.datetime_select :starts_at, {}, class: 'form-control form-control-inline'
-  .form-group
-    = f.label :ends_at, class: 'control-label'
-    .col-sm-10.datetime-controls
-      = f.datetime_select :ends_at, {}, class: 'form-control form-control-inline'
-  .form-actions
-    = f.submit "Add broadcast message", class: "btn btn-create"
 
 -if @broadcast_messages.any?
   %ul.bordered-list.broadcast-messages
-- 
cgit v1.2.1


From 6ae39c2cd1edcc845136739d42baf032120e3ddc Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 15:56:15 -0500
Subject: Remove alert_type attribute from BroadcastMessage

---
 app/controllers/admin/broadcast_messages_controller.rb        | 11 +++++++----
 app/models/broadcast_message.rb                               |  1 -
 ...0151231202530_remove_alert_type_from_broadcast_messages.rb |  5 +++++
 db/schema.rb                                                  |  1 -
 spec/models/broadcast_message_spec.rb                         |  1 -
 5 files changed, 12 insertions(+), 7 deletions(-)
 create mode 100644 db/migrate/20151231202530_remove_alert_type_from_broadcast_messages.rb

diff --git a/app/controllers/admin/broadcast_messages_controller.rb b/app/controllers/admin/broadcast_messages_controller.rb
index 497c34f8f49..bc702c022b6 100644
--- a/app/controllers/admin/broadcast_messages_controller.rb
+++ b/app/controllers/admin/broadcast_messages_controller.rb
@@ -31,9 +31,12 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
   end
 
   def broadcast_message_params
-    params.require(:broadcast_message).permit(
-      :alert_type, :color, :ends_at, :font,
-      :message, :starts_at
-    )
+    params.require(:broadcast_message).permit(%i(
+      color
+      ends_at
+      font
+      message
+      starts_at
+    ))
   end
 end
diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index ad514706160..da0d8a2d65f 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -6,7 +6,6 @@
 #  message    :text             not null
 #  starts_at  :datetime
 #  ends_at    :datetime
-#  alert_type :integer
 #  created_at :datetime
 #  updated_at :datetime
 #  color      :string(255)
diff --git a/db/migrate/20151231202530_remove_alert_type_from_broadcast_messages.rb b/db/migrate/20151231202530_remove_alert_type_from_broadcast_messages.rb
new file mode 100644
index 00000000000..78fdfeaf5cf
--- /dev/null
+++ b/db/migrate/20151231202530_remove_alert_type_from_broadcast_messages.rb
@@ -0,0 +1,5 @@
+class RemoveAlertTypeFromBroadcastMessages < ActiveRecord::Migration
+  def change
+    remove_column :broadcast_messages, :alert_type, :integer
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index ecbe575bf83..42c3e79f9d7 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -82,7 +82,6 @@ ActiveRecord::Schema.define(version: 20160113111034) do
     t.text     "message",    null: false
     t.datetime "starts_at"
     t.datetime "ends_at"
-    t.integer  "alert_type"
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "color"
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index e4cac105110..001b2facaab 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/spec/models/broadcast_message_spec.rb
@@ -6,7 +6,6 @@
 #  message    :text             not null
 #  starts_at  :datetime
 #  ends_at    :datetime
-#  alert_type :integer
 #  created_at :datetime
 #  updated_at :datetime
 #  color      :string(255)
-- 
cgit v1.2.1


From 12903762d15cef256f579b1ddb692a0254cace52 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 15:56:59 -0500
Subject: Simplify broadcast message JS

Also resets the default example message when the user input is blank.
---
 app/assets/javascripts/admin.js.coffee | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/assets/javascripts/admin.js.coffee b/app/assets/javascripts/admin.js.coffee
index e9a26dd7200..eb951f71711 100644
--- a/app/assets/javascripts/admin.js.coffee
+++ b/app/assets/javascripts/admin.js.coffee
@@ -13,15 +13,16 @@ class @Admin
       $('.js-toggle-colors-container').toggle()
 
     $('input#broadcast_message_color').on 'input', ->
-      previewColor = $('input#broadcast_message_color').val()
+      previewColor = $(@).val()
       $('div.broadcast-message-preview').css('background-color', previewColor)
 
     $('input#broadcast_message_font').on 'input', ->
-      previewColor = $('input#broadcast_message_font').val()
+      previewColor = $(@).val()
       $('div.broadcast-message-preview').css('color', previewColor)
 
     $('textarea#broadcast_message_message').on 'input', ->
-      previewMessage = $('textarea#broadcast_message_message').val()
+      previewMessage = $(@).val()
+      previewMessage = "Your message here" if previewMessage.trim() == ''
       $('div.broadcast-message-preview span').text(previewMessage)
 
     $('.log-tabs a').click (e) ->
-- 
cgit v1.2.1


From 8086b2bd2ecb0ff647da766c436eda47b7434599 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 16:04:07 -0500
Subject: Simplify BroadcastMessage factory

Also make the feature tests less brittle.
---
 features/admin/broadcast_messages.feature  |  2 +-
 features/steps/admin/broadcast_messages.rb |  7 +++----
 spec/factories/broadcast_messages.rb       | 13 +++++++------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/features/admin/broadcast_messages.feature b/features/admin/broadcast_messages.feature
index b2c3112320a..98d894a4fb6 100644
--- a/features/admin/broadcast_messages.feature
+++ b/features/admin/broadcast_messages.feature
@@ -6,7 +6,7 @@ Feature: Admin Broadcast Messages
     And I visit admin messages page
 
   Scenario: See broadcast messages list
-    Then I should be all broadcast messages
+    Then I should see all broadcast messages
 
   Scenario: Create a broadcast message
     When submit form with new broadcast message
diff --git a/features/steps/admin/broadcast_messages.rb b/features/steps/admin/broadcast_messages.rb
index f6daf852977..b59799c2f47 100644
--- a/features/steps/admin/broadcast_messages.rb
+++ b/features/steps/admin/broadcast_messages.rb
@@ -4,10 +4,10 @@ class Spinach::Features::AdminBroadcastMessages < Spinach::FeatureSteps
   include SharedAdmin
 
   step 'application already has admin messages' do
-    FactoryGirl.create(:broadcast_message, message: "Migration to new server")
+    FactoryGirl.create(:broadcast_message, :expired, message: "Migration to new server")
   end
 
-  step 'I should be all broadcast messages' do
+  step 'I should see all broadcast messages' do
     expect(page).to have_content "Migration to new server"
   end
 
@@ -27,10 +27,9 @@ class Spinach::Features::AdminBroadcastMessages < Spinach::FeatureSteps
 
   step 'submit form with new customized broadcast message' do
     fill_in 'broadcast_message_message', with: 'Application update from 4:00 CST to 5:00 CST'
-    click_link "Customize colors"
     fill_in 'broadcast_message_color', with: '#f2dede'
     fill_in 'broadcast_message_font', with: '#b94a48'
-    select '2018', from: "broadcast_message_ends_at_1i"
+    select Date.today.next_year.year, from: "broadcast_message_ends_at_1i"
     click_button "Add broadcast message"
   end
 
diff --git a/spec/factories/broadcast_messages.rb b/spec/factories/broadcast_messages.rb
index ea0039d39e6..0c7bc573612 100644
--- a/spec/factories/broadcast_messages.rb
+++ b/spec/factories/broadcast_messages.rb
@@ -6,7 +6,6 @@
 #  message    :text             not null
 #  starts_at  :datetime
 #  ends_at    :datetime
-#  alert_type :integer
 #  created_at :datetime
 #  updated_at :datetime
 #  color      :string(255)
@@ -18,10 +17,12 @@
 FactoryGirl.define do
   factory :broadcast_message do
     message "MyText"
-    starts_at "2013-11-12 13:43:25"
-    ends_at "2013-11-12 13:43:25"
-    alert_type 1
-    color "#555555"
-    font "#BBBBBB"
+    starts_at Date.today
+    ends_at Date.tomorrow
+
+    trait :expired do
+      starts_at 5.days.ago
+      ends_at 3.days.ago
+    end
   end
 end
-- 
cgit v1.2.1


From 5a1706791e46fd69a67fcdebfc384a702b00a7ff Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 16:42:56 -0500
Subject: Update broadcast_message helper

Now it returns the fully-formatted message so we can be consistent about
how it's shown.
---
 app/helpers/application_helper.rb              |  4 ---
 app/helpers/broadcast_messages_helper.rb       | 20 ++++++++----
 app/views/layouts/_broadcast.html.haml         |  5 +--
 spec/helpers/broadcast_messages_helper_spec.rb | 42 ++++++++++++++++++--------
 4 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 436fbcd4138..f3a2723ee0d 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -181,10 +181,6 @@ module ApplicationHelper
     end
   end
 
-  def broadcast_message
-    BroadcastMessage.current
-  end
-
   # Render a `time` element with Javascript-based relative date and tooltip
   #
   # time       - Time object
diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index 6484dca6b55..44bb09b74f4 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -1,16 +1,24 @@
 module BroadcastMessagesHelper
-  def broadcast_styling(broadcast_message)
-    styling = ''
+  def broadcast_message(message = BroadcastMessage.current)
+    return unless message.present?
+
+    content_tag :div, class: 'broadcast-message', style: broadcast_message_style(message) do
+      icon('bullhorn') << ' ' << message.message
+    end
+  end
+
+  def broadcast_message_style(broadcast_message)
+    style = ''
 
     if broadcast_message.color.present?
-      styling << "background-color: #{broadcast_message.color}"
-      styling << '; ' if broadcast_message.font.present?
+      style << "background-color: #{broadcast_message.color}"
+      style << '; ' if broadcast_message.font.present?
     end
 
     if broadcast_message.font.present?
-      styling << "color: #{broadcast_message.font}"
+      style << "color: #{broadcast_message.font}"
     end
 
-    styling
+    style
   end
 end
diff --git a/app/views/layouts/_broadcast.html.haml b/app/views/layouts/_broadcast.html.haml
index ad7851c0aa3..3a7e0929c16 100644
--- a/app/views/layouts/_broadcast.html.haml
+++ b/app/views/layouts/_broadcast.html.haml
@@ -1,4 +1 @@
-- if broadcast_message.present?
-  .broadcast-message{ style: broadcast_styling(broadcast_message) }
-    = icon('bullhorn')
-    = broadcast_message.message
+= broadcast_message
diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb
index c7c6f45d144..0fb8a7284f3 100644
--- a/spec/helpers/broadcast_messages_helper_spec.rb
+++ b/spec/helpers/broadcast_messages_helper_spec.rb
@@ -1,22 +1,40 @@
 require 'spec_helper'
 
 describe BroadcastMessagesHelper do
-  describe 'broadcast_styling' do
-    let(:broadcast_message) { double(color: '', font: '') }
+  describe 'broadcast_message' do
+    it 'returns nil when no current message' do
+      expect(helper.broadcast_message(nil)).to be_nil
+    end
+
+    it 'includes the current message' do
+      current = double(message: 'Current Message')
+
+      allow(helper).to receive(:broadcast_message_style).and_return(nil)
+
+      expect(helper.broadcast_message(current)).to include 'Current Message'
+    end
+
+    it 'includes custom style' do
+      current = double(message: 'Current Message')
+
+      allow(helper).to receive(:broadcast_message_style).and_return('foo')
+
+      expect(helper.broadcast_message(current)).to include 'style="foo"'
+    end
+  end
+
+  describe 'broadcast_message_style' do
+    it 'defaults to no style' do
+      broadcast_message = spy
 
-    context "default style" do
-      it "should have no style" do
-        expect(broadcast_styling(broadcast_message)).to eq ''
-      end
+      expect(helper.broadcast_message_style(broadcast_message)).to eq ''
     end
 
-    context "customized style" do
-      let(:broadcast_message) { double(color: "#f2dede", font: '#b94a48') }
+    it 'allows custom style' do
+      broadcast_message = double(color: '#f2dede', font: '#b94a48')
 
-      it "should have a customized style" do
-        expect(broadcast_styling(broadcast_message)).
-          to match('background-color: #f2dede; color: #b94a48')
-      end
+      expect(helper.broadcast_message_style(broadcast_message)).
+        to match('background-color: #f2dede; color: #b94a48')
     end
   end
 end
-- 
cgit v1.2.1


From df496fcaf08b085816a45d31d02f1ea230454b63 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 17:07:11 -0500
Subject: Update BroadcastMessage model

- Adds default values for `color` and `font` attributes
- Adds `active?`, `started?`, `ended?`, and 'status' methods
---
 app/models/broadcast_message.rb       | 27 ++++++++++-
 spec/factories/broadcast_messages.rb  |  5 ++
 spec/models/broadcast_message_spec.rb | 91 +++++++++++++++++++++++++++++++++--
 3 files changed, 117 insertions(+), 6 deletions(-)

diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index da0d8a2d65f..188545f9ae5 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -22,7 +22,32 @@ class BroadcastMessage < ActiveRecord::Base
   validates :color, allow_blank: true, color: true
   validates :font,  allow_blank: true, color: true
 
+  default_value_for :color, '#E75E40'
+  default_value_for :font,  '#FFFFFF'
+
   def self.current
-    where("ends_at > :now AND starts_at < :now", now: Time.zone.now).last
+    where("ends_at > :now AND starts_at <= :now", now: Time.zone.now).last
+  end
+
+  def active?
+    started? && !ended?
+  end
+
+  def started?
+    Time.zone.now >= starts_at
+  end
+
+  def ended?
+    ends_at < Time.zone.now
+  end
+
+  def status
+    if active?
+      'Active'
+    elsif ended?
+      'Expired'
+    else
+      'Pending'
+    end
   end
 end
diff --git a/spec/factories/broadcast_messages.rb b/spec/factories/broadcast_messages.rb
index 0c7bc573612..978a7d4cecb 100644
--- a/spec/factories/broadcast_messages.rb
+++ b/spec/factories/broadcast_messages.rb
@@ -24,5 +24,10 @@ FactoryGirl.define do
       starts_at 5.days.ago
       ends_at 3.days.ago
     end
+
+    trait :future do
+      starts_at 5.days.from_now
+      ends_at 6.days.from_now
+    end
   end
 end
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index 001b2facaab..57550725ae3 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/spec/models/broadcast_message_spec.rb
@@ -15,6 +15,8 @@
 require 'spec_helper'
 
 describe BroadcastMessage, models: true do
+  include ActiveSupport::Testing::TimeHelpers
+
   subject { create(:broadcast_message) }
 
   it { is_expected.to be_valid }
@@ -34,20 +36,99 @@ describe BroadcastMessage, models: true do
     it { is_expected.not_to allow_value('000').for(:font) }
   end
 
-  describe :current do
+  describe '.current' do
     it "should return last message if time match" do
-      broadcast_message = create(:broadcast_message, starts_at: Time.now.yesterday, ends_at: Time.now.tomorrow)
-      expect(BroadcastMessage.current).to eq(broadcast_message)
+      message = create(:broadcast_message)
+
+      expect(BroadcastMessage.current).to eq message
     end
 
     it "should return nil if time not come" do
-      create(:broadcast_message, starts_at: Time.now.tomorrow, ends_at: Time.now + 2.days)
+      create(:broadcast_message, :future)
+
       expect(BroadcastMessage.current).to be_nil
     end
 
     it "should return nil if time has passed" do
-      create(:broadcast_message, starts_at: Time.now - 2.days, ends_at: Time.now.yesterday)
+      create(:broadcast_message, :expired)
+
       expect(BroadcastMessage.current).to be_nil
     end
   end
+
+  describe '#active?' do
+    it 'is truthy when started and not ended' do
+      message = build(:broadcast_message)
+
+      expect(message).to be_active
+    end
+
+    it 'is falsey when ended' do
+      message = build(:broadcast_message, :expired)
+
+      expect(message).not_to be_active
+    end
+
+    it 'is falsey when not started' do
+      message = build(:broadcast_message, :future)
+
+      expect(message).not_to be_active
+    end
+  end
+
+  describe '#started?' do
+    it 'is truthy when starts_at has passed' do
+      message = build(:broadcast_message)
+
+      travel_to(3.days.from_now) do
+        expect(message).to be_started
+      end
+    end
+
+    it 'is falsey when starts_at is in the future' do
+      message = build(:broadcast_message)
+
+      travel_to(3.days.ago) do
+        expect(message).not_to be_started
+      end
+    end
+  end
+
+  describe '#ended?' do
+    it 'is truthy when ends_at has passed' do
+      message = build(:broadcast_message)
+
+      travel_to(3.days.from_now) do
+        expect(message).to be_ended
+      end
+    end
+
+    it 'is falsey when ends_at is in the future' do
+      message = build(:broadcast_message)
+
+      travel_to(3.days.ago) do
+        expect(message).not_to be_ended
+      end
+    end
+  end
+
+  describe '#status' do
+    it 'returns Active' do
+      message = build(:broadcast_message)
+
+      expect(message.status).to eq 'Active'
+    end
+
+    it 'returns Expired' do
+      message = build(:broadcast_message, :expired)
+
+      expect(message.status).to eq 'Expired'
+    end
+
+    it 'returns Pending' do
+      message = build(:broadcast_message, :future)
+
+      expect(message.status).to eq 'Pending'
+    end
+  end
 end
-- 
cgit v1.2.1


From 540ae3c3658d051f852b2c10fa61c557521196e1 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 17:22:40 -0500
Subject: Update Broadcast Message features

- Removes redundant "Create a broadcast message" scenario that was
  entirely covered by the "Create a customized broadcast message"
  scenario.
- Adds "Edit an existing broadcast message" scenario
- Adds "Remove an existing broadcast message" scenario
---
 features/admin/broadcast_messages.feature  | 18 ++++++++++++------
 features/steps/admin/broadcast_messages.rb | 30 ++++++++++++++++++++++--------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/features/admin/broadcast_messages.feature b/features/admin/broadcast_messages.feature
index 98d894a4fb6..fd3bac77f86 100644
--- a/features/admin/broadcast_messages.feature
+++ b/features/admin/broadcast_messages.feature
@@ -2,20 +2,26 @@
 Feature: Admin Broadcast Messages
   Background:
     Given I sign in as an admin
-    And application already has admin messages
+    And application already has a broadcast message
     And I visit admin messages page
 
   Scenario: See broadcast messages list
     Then I should see all broadcast messages
 
-  Scenario: Create a broadcast message
-    When submit form with new broadcast message
-    Then I should be redirected to admin messages page
-    And I should see newly created broadcast message
-
   Scenario: Create a customized broadcast message
     When submit form with new customized broadcast message
     Then I should be redirected to admin messages page
     And I should see newly created broadcast message
     Then I visit dashboard page
     And I should see a customized broadcast message
+
+  Scenario: Edit an existing broadcast message
+    When I edit an existing broadcast message
+    And I change the broadcast message text
+    Then I should be redirected to admin messages page
+    And I should see the updated broadcast message
+
+  Scenario: Remove an existing broadcast message
+    When I remove an existing broadcast message
+    Then I should be redirected to admin messages page
+    And I should not see the removed broadcast message
diff --git a/features/steps/admin/broadcast_messages.rb b/features/steps/admin/broadcast_messages.rb
index b59799c2f47..6cacdf4764c 100644
--- a/features/steps/admin/broadcast_messages.rb
+++ b/features/steps/admin/broadcast_messages.rb
@@ -1,9 +1,8 @@
 class Spinach::Features::AdminBroadcastMessages < Spinach::FeatureSteps
   include SharedAuthentication
   include SharedPaths
-  include SharedAdmin
 
-  step 'application already has admin messages' do
+  step 'application already has a broadcast message' do
     FactoryGirl.create(:broadcast_message, :expired, message: "Migration to new server")
   end
 
@@ -11,12 +10,6 @@ class Spinach::Features::AdminBroadcastMessages < Spinach::FeatureSteps
     expect(page).to have_content "Migration to new server"
   end
 
-  step 'submit form with new broadcast message' do
-    fill_in 'broadcast_message_message', with: 'Application update from 4:00 CST to 5:00 CST'
-    select '2018', from: "broadcast_message_ends_at_1i"
-    click_button "Add broadcast message"
-  end
-
   step 'I should be redirected to admin messages page' do
     expect(current_path).to eq admin_broadcast_messages_path
   end
@@ -37,4 +30,25 @@ class Spinach::Features::AdminBroadcastMessages < Spinach::FeatureSteps
     expect(page).to have_content 'Application update from 4:00 CST to 5:00 CST'
     expect(page).to have_selector %(div[style="background-color: #f2dede; color: #b94a48"])
   end
+
+  step 'I edit an existing broadcast message' do
+    click_link 'Edit'
+  end
+
+  step 'I change the broadcast message text' do
+    fill_in 'broadcast_message_message', with: 'Application update RIGHT NOW'
+    click_button 'Update broadcast message'
+  end
+
+  step 'I should see the updated broadcast message' do
+    expect(page).to have_content "Application update RIGHT NOW"
+  end
+
+  step 'I remove an existing broadcast message' do
+    click_link 'Remove'
+  end
+
+  step 'I should not see the removed broadcast message' do
+    expect(page).not_to have_content 'Migration to new server'
+  end
 end
-- 
cgit v1.2.1


From 00e8700433b3b1ad11252448af5be58913539d85 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 17:55:14 -0500
Subject: Broadcast Messages can now be edited

Closes #3046
---
 .../admin/broadcast_messages_controller.rb         | 22 +++++++++---
 app/views/admin/broadcast_messages/_form.html.haml | 10 +++---
 app/views/admin/broadcast_messages/edit.html.haml  |  3 ++
 app/views/admin/broadcast_messages/index.html.haml | 40 +++++++++++++---------
 config/routes.rb                                   |  2 +-
 5 files changed, 49 insertions(+), 28 deletions(-)
 create mode 100644 app/views/admin/broadcast_messages/edit.html.haml

diff --git a/app/controllers/admin/broadcast_messages_controller.rb b/app/controllers/admin/broadcast_messages_controller.rb
index bc702c022b6..4735b27c65d 100644
--- a/app/controllers/admin/broadcast_messages_controller.rb
+++ b/app/controllers/admin/broadcast_messages_controller.rb
@@ -1,8 +1,12 @@
 class Admin::BroadcastMessagesController < Admin::ApplicationController
-  before_action :broadcast_messages
+  before_action :finder, only: [:edit, :update, :destroy]
 
   def index
-    @broadcast_message = BroadcastMessage.new
+    @broadcast_messages = BroadcastMessage.reorder("starts_at ASC").page(params[:page])
+    @broadcast_message  = BroadcastMessage.new
+  end
+
+  def edit
   end
 
   def create
@@ -15,8 +19,16 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
     end
   end
 
+  def update
+    if @broadcast_message.update(broadcast_message_params)
+      redirect_to admin_broadcast_messages_path, notice: 'Broadcast Message was successfully updated.'
+    else
+      render :edit
+    end
+  end
+
   def destroy
-    BroadcastMessage.find(params[:id]).destroy
+    @broadcast_message.destroy
 
     respond_to do |format|
       format.html { redirect_back_or_default(default: { action: 'index' }) }
@@ -26,8 +38,8 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
 
   protected
 
-  def broadcast_messages
-    @broadcast_messages ||= BroadcastMessage.order("starts_at DESC").page(params[:page])
+  def finder
+    @broadcast_message = BroadcastMessage.find(params[:id])
   end
 
   def broadcast_message_params
diff --git a/app/views/admin/broadcast_messages/_form.html.haml b/app/views/admin/broadcast_messages/_form.html.haml
index 0ca3be0bd54..953b8b69368 100644
--- a/app/views/admin/broadcast_messages/_form.html.haml
+++ b/app/views/admin/broadcast_messages/_form.html.haml
@@ -1,6 +1,6 @@
-.broadcast-message-preview
+.broadcast-message-preview{ style: broadcast_message_style(@broadcast_message) }
   = icon('bullhorn')
-  %span Your message here
+  %span= @broadcast_message.message || "Your message here"
 
 = form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal js-requires-input'} do |f|
   -if @broadcast_message.errors.any?
@@ -10,18 +10,18 @@
   .form-group
     = f.label :message, class: 'control-label'
     .col-sm-10
-      = f.text_area :message, class: "form-control", rows: 2, required: true
+      = f.text_area :message, class: "form-control js-quick-submit", rows: 2, required: true
   .form-group.js-toggle-colors-container
     .col-sm-10.col-sm-offset-2
       = link_to 'Customize colors', '#', class: 'js-toggle-colors-link'
   .form-group.js-toggle-colors-container.hide
     = f.label :color, "Background Color", class: 'control-label'
     .col-sm-10
-      = f.color_field :color, value: "#E75E40", class: "form-control"
+      = f.color_field :color, class: "form-control"
   .form-group.js-toggle-colors-container.hide
     = f.label :font, "Font Color", class: 'control-label'
     .col-sm-10
-      = f.color_field :font, value: "#FFFFFF", class: "form-control"
+      = f.color_field :font, class: "form-control"
   .form-group
     = f.label :starts_at, class: 'control-label'
     .col-sm-10.datetime-controls
diff --git a/app/views/admin/broadcast_messages/edit.html.haml b/app/views/admin/broadcast_messages/edit.html.haml
new file mode 100644
index 00000000000..45e053eb31d
--- /dev/null
+++ b/app/views/admin/broadcast_messages/edit.html.haml
@@ -0,0 +1,3 @@
+- page_title "Broadcast Messages"
+
+= render 'form'
diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 04864d4afef..6c72b28ee0c 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -8,24 +8,30 @@
 
 = render 'form'
 
+%br.clearfix
 
 -if @broadcast_messages.any?
-  %ul.bordered-list.broadcast-messages
-    - @broadcast_messages.each do |broadcast_message|
-      %li
-        .pull-right
-          - if broadcast_message.starts_at
-            %strong
-              #{broadcast_message.starts_at.to_s(:short)}
-            \...
-          - if broadcast_message.ends_at
-            %strong
-              #{broadcast_message.ends_at.to_s(:short)}
-          &nbsp;
-          = link_to [:admin, broadcast_message], method: :delete, remote: true, class: 'remove-row btn btn-xs' do
-            %i.fa.fa-times.cred
-
-        .message= broadcast_message.message
-
+  %table.table
+    %thead
+      %tr
+        %th Status
+        %th Preview
+        %th Starts
+        %th Ends
+        %th &nbsp;
+    %tbody
+      - @broadcast_messages.each do |message|
+        %tr
+          %td
+            = message.status
+          %td
+            = broadcast_message(message)
+          %td
+            = message.starts_at.to_s(:iso861)
+          %td
+            = message.ends_at.to_s(:iso861)
+          %td
+            = link_to icon('pencil-square-o'), edit_admin_broadcast_message_path(message), title: 'Edit', class: 'btn btn-xs'
+            = link_to icon('times'), admin_broadcast_message_path(message), method: :delete, remote: true, title: 'Remove', class: 'js-remove-tr btn btn-xs btn-danger'
 
   = paginate @broadcast_messages
diff --git a/config/routes.rb b/config/routes.rb
index 3d5c70987c8..05d6ff1e884 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -219,7 +219,7 @@ Rails.application.routes.draw do
       get :test
     end
 
-    resources :broadcast_messages, only: [:index, :create, :destroy]
+    resources :broadcast_messages, only: [:index, :edit, :create, :update, :destroy]
     resource :logs, only: [:show]
     resource :background_jobs, controller: 'background_jobs', only: [:show]
 
-- 
cgit v1.2.1


From c13b5acb16c7813d6913e26cc7ae67f691f914d6 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 31 Dec 2015 20:04:55 -0500
Subject: Update CHANGELOG

[ci skip]
---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index 4221fb9df55..94c764eb6b4 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -43,6 +43,7 @@ v 8.4.0 (unreleased)
   - API: Add support for deleting a tag via the API (Robert Schilling)
   - Allow subsequent validations in CI Linter
   - Fix Encoding::CompatibilityError bug when markdown content has some complex URL (Jason Lee)
+  - Allow broadcast messages to be edited
 
 v 8.3.4
   - Use gitlab-workhorse 0.5.4 (fixes API routing bug)
-- 
cgit v1.2.1


From 843662821ddbf2d06aa2da72ce32717cebecb7c6 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Wed, 13 Jan 2016 11:46:32 -0500
Subject: Move `BroadcastMessage#status` to a helper since it's presentational

---
 app/helpers/broadcast_messages_helper.rb           | 10 ++++++++++
 app/models/broadcast_message.rb                    | 10 ----------
 app/views/admin/broadcast_messages/index.html.haml |  2 +-
 spec/helpers/broadcast_messages_helper_spec.rb     | 20 ++++++++++++++++++++
 spec/models/broadcast_message_spec.rb              | 20 --------------------
 5 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index 44bb09b74f4..1ed8c710f77 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -21,4 +21,14 @@ module BroadcastMessagesHelper
 
     style
   end
+
+  def broadcast_message_status(broadcast_message)
+    if broadcast_message.active?
+      'Active'
+    elsif broadcast_message.ended?
+      'Expired'
+    else
+      'Pending'
+    end
+  end
 end
diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 188545f9ae5..61119633717 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -40,14 +40,4 @@ class BroadcastMessage < ActiveRecord::Base
   def ended?
     ends_at < Time.zone.now
   end
-
-  def status
-    if active?
-      'Active'
-    elsif ended?
-      'Expired'
-    else
-      'Pending'
-    end
-  end
 end
diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 6c72b28ee0c..3c4ef4fe428 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -23,7 +23,7 @@
       - @broadcast_messages.each do |message|
         %tr
           %td
-            = message.status
+            = broadcast_message_status(message)
           %td
             = broadcast_message(message)
           %td
diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb
index 0fb8a7284f3..157cc4665a2 100644
--- a/spec/helpers/broadcast_messages_helper_spec.rb
+++ b/spec/helpers/broadcast_messages_helper_spec.rb
@@ -37,4 +37,24 @@ describe BroadcastMessagesHelper do
         to match('background-color: #f2dede; color: #b94a48')
     end
   end
+
+  describe 'broadcast_message_status' do
+    it 'returns Active' do
+      message = build(:broadcast_message)
+
+      expect(helper.broadcast_message_status(message)).to eq 'Active'
+    end
+
+    it 'returns Expired' do
+      message = build(:broadcast_message, :expired)
+
+      expect(helper.broadcast_message_status(message)).to eq 'Expired'
+    end
+
+    it 'returns Pending' do
+      message = build(:broadcast_message, :future)
+
+      expect(helper.broadcast_message_status(message)).to eq 'Pending'
+    end
+  end
 end
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index 57550725ae3..f6f84db57e6 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/spec/models/broadcast_message_spec.rb
@@ -111,24 +111,4 @@ describe BroadcastMessage, models: true do
       end
     end
   end
-
-  describe '#status' do
-    it 'returns Active' do
-      message = build(:broadcast_message)
-
-      expect(message.status).to eq 'Active'
-    end
-
-    it 'returns Expired' do
-      message = build(:broadcast_message, :expired)
-
-      expect(message.status).to eq 'Expired'
-    end
-
-    it 'returns Pending' do
-      message = build(:broadcast_message, :future)
-
-      expect(message.status).to eq 'Pending'
-    end
-  end
 end
-- 
cgit v1.2.1


From 55d0323961e4db8d29450f35b169029d51e30bda Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Wed, 13 Jan 2016 12:01:25 -0500
Subject: Remove (invalid) timestamp formatting

---
 app/views/admin/broadcast_messages/index.html.haml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 3c4ef4fe428..49e33698b63 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -27,9 +27,9 @@
           %td
             = broadcast_message(message)
           %td
-            = message.starts_at.to_s(:iso861)
+            = message.starts_at
           %td
-            = message.ends_at.to_s(:iso861)
+            = message.ends_at
           %td
             = link_to icon('pencil-square-o'), edit_admin_broadcast_message_path(message), title: 'Edit', class: 'btn btn-xs'
             = link_to icon('times'), admin_broadcast_message_path(message), method: :delete, remote: true, title: 'Remove', class: 'js-remove-tr btn btn-xs btn-danger'
-- 
cgit v1.2.1


From 1f0b8c32e75b446848cead98c550e750801be534 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 13 Jan 2016 18:18:59 +0100
Subject: Add spec for Note#cross_reference_not_visible_for?

---
 spec/models/note_spec.rb | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 151a29e974b..65e6a7df3b4 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -178,6 +178,30 @@ describe Note, models: true do
     end
   end
 
+  describe "cross_reference_not_visible_for?" do
+    let(:private_user)    { create(:user) }
+    let(:private_project) { create(:project, namespace: private_user.namespace).tap { |p| p.team << [private_user, :master] } }
+    let(:private_issue)   { create(:issue, project: private_project) }
+
+    let(:ext_proj)  { create(:project, :public) }
+    let(:ext_issue) { create(:issue, project: ext_proj) }
+
+    let(:note) {
+      create :note,
+        noteable: ext_issue, project: ext_proj,
+        note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
+        system: true
+    }
+
+    it "returns true" do
+      expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
+    end
+
+    it "returns false" do
+      expect(note.cross_reference_not_visible_for?(private_user)).to be_falsy
+    end
+  end
+
   describe "set_award!" do
     let(:issue) { create :issue }
 
-- 
cgit v1.2.1


From 9e701ccd48ed442124509aeb68fe6788579efdde Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Wed, 13 Jan 2016 18:47:39 +0100
Subject: Fix some typos

---
 lib/api/variables.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index 0c3fb5c8a77..d9a055f6c92 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -18,7 +18,7 @@ module API
         present paginate(variables), with: Entities::Variable
       end
 
-      # Get specifica variable of a project
+      # Get specific variable of a project
       #
       # Parameters:
       #   id (required) - The ID of a project
@@ -80,7 +80,7 @@ module API
       # Parameters:
       #   id (required) - The ID of a project
       #   key (required) - The ID of a variable
-      # Exanoke Reqyest:
+      # Example Request:
       #   DELETE /projects/:id/variables/:key
       delete ':id/variables/:key' do
         variable = user_project.variables.find_by(key: params[:key].to_s)
-- 
cgit v1.2.1


From d338087605791e408e696b11974995be9ebff80e Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Wed, 13 Jan 2016 18:51:06 +0100
Subject: Add 'Build' prefix to Variables entry name in API docs index

---
 doc/api/README.md          |   2 +-
 doc/api/build_variables.md | 106 +++++++++++++++++++++++++++++++++++++++++++++
 doc/api/variables.md       | 106 ---------------------------------------------
 3 files changed, 107 insertions(+), 107 deletions(-)
 create mode 100644 doc/api/build_variables.md
 delete mode 100644 doc/api/variables.md

diff --git a/doc/api/README.md b/doc/api/README.md
index 198b4ddfee1..c3401bcbc9d 100644
--- a/doc/api/README.md
+++ b/doc/api/README.md
@@ -23,7 +23,7 @@
 - [Namespaces](namespaces.md)
 - [Settings](settings.md)
 - [Keys](keys.md)
-- [Variables](variables.md)
+- [Build Variables](build_variables.md)
 
 ## Clients
 
diff --git a/doc/api/build_variables.md b/doc/api/build_variables.md
new file mode 100644
index 00000000000..2b804fd9051
--- /dev/null
+++ b/doc/api/build_variables.md
@@ -0,0 +1,106 @@
+# Build Variables
+
+## Variables keys
+
+All variable keys must contains only letters, digits and '\_'. They must also be no longer than 255 characters.
+
+## List project variables
+
+Get list of variables of a project.
+
+```
+GET /projects/:id/variables
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+
+```json
+[
+    {
+        "key": "TEST_VARIABLE_1",
+        "value": "TEST_1"
+    },
+    {
+        "key": "TEST_VARIABLE_2",
+        "value": "TEST_2"
+    }
+]
+```
+
+## Show variable details
+
+Get details of specifica variable of a project.
+
+```
+GET /projects/:id/variables/:key
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `key` (required) - The `key` of variable
+
+```json
+{
+    "key": "TEST_VARIABLE_1",
+    "value": "TEST_1"
+}
+```
+
+## Create variable
+
+Create new variable in project.
+
+```
+POST /projects/:id/variables
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `key` (required) - The `key` for variable
+- `value` (required) - The `value` for variable
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "new value"
+}
+```
+
+## Update variable
+
+Update variable.
+
+```
+PUT /projects/:id/variables/:key
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `key` (required) - The `key` for variable
+- `value` (required) - The `value` for variable
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "updated value"
+}
+```
+
+## Remove variable
+
+Remove variable.
+
+```
+DELETE /projects/:id/variables/:key
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `key` (required) - The `key` for variable
+
diff --git a/doc/api/variables.md b/doc/api/variables.md
deleted file mode 100644
index cdc9ba42254..00000000000
--- a/doc/api/variables.md
+++ /dev/null
@@ -1,106 +0,0 @@
-# Variables
-
-## Variables keys
-
-All variable keys must contains only letters, digits and '\_'. They must also be no longer than 255 characters.
-
-## List project variables
-
-Get list of variables of a project.
-
-```
-GET /projects/:id/variables
-```
-
-Parameters:
-
-- `id` (required) - The ID of a project
-
-```json
-[
-    {
-        "key": "TEST_VARIABLE_1",
-        "value": "TEST_1"
-    },
-    {
-        "key": "TEST_VARIABLE_2",
-        "value": "TEST_2"
-    }
-]
-```
-
-## Show variable details
-
-Get details of specifica variable of a project.
-
-```
-GET /projects/:id/variables/:key
-```
-
-Parameters:
-
-- `id` (required) - The ID of a project
-- `key` (required) - The `key` of variable
-
-```json
-{
-    "key": "TEST_VARIABLE_1",
-    "value": "TEST_1"
-}
-```
-
-## Create variable
-
-Create new variable in project.
-
-```
-POST /projects/:id/variables
-```
-
-Parameters:
-
-- `id` (required) - The ID of a project
-- `key` (required) - The `key` for variable
-- `value` (required) - The `value` for variable
-
-```json
-{
-    "key": "NEW_VARIABLE",
-    "value": "new value"
-}
-```
-
-## Update variable
-
-Update variable.
-
-```
-PUT /projects/:id/variables/:key
-```
-
-Parameters:
-
-- `id` (required) - The ID of a project
-- `key` (required) - The `key` for variable
-- `value` (required) - The `value` for variable
-
-```json
-{
-    "key": "NEW_VARIABLE",
-    "value": "updated value"
-}
-```
-
-## Remove variable
-
-Remove variable.
-
-```
-DELETE /projects/:id/variables/:key
-```
-
-Parameters:
-
-- `id` (required) - The ID of a project
-- `key` (required) - The `key` for variable
-
-- 
cgit v1.2.1


From 0c10aee59677e2dadfef6538a74fe1e28fcdd37e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 13 Jan 2016 19:42:36 +0100
Subject: Ensure the API doesn't return notes that the current user shouldn't
 see

---
 lib/api/notes.rb                | 21 +++++++++++++++--
 spec/requests/api/notes_spec.rb | 51 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 3efdfe2d46e..174473f5371 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -20,7 +20,19 @@ module API
         #   GET /projects/:id/snippets/:noteable_id/notes
         get ":id/#{noteables_str}/:#{noteable_id_str}/notes" do
           @noteable = user_project.send(:"#{noteables_str}").find(params[:"#{noteable_id_str}"])
-          present paginate(@noteable.notes), with: Entities::Note
+
+          # We exclude notes that are cross-references and that cannot be viewed
+          # by the current user. By doing this exclusion at this level and not
+          # at the DB query level (which we cannot in that case), the current
+          # page can have less elements than :per_page even if
+          # there's more than one page.
+          notes =
+            # paginate() only works with a relation. This could lead to a
+            # mismatch between the pagination headers info and the actual notes
+            # array returned, but this is really a edge-case.
+            paginate(@noteable.notes).
+            reject { |n| n.cross_reference_not_visible_for?(current_user) }
+          present notes, with: Entities::Note
         end
 
         # Get a single +noteable+ note
@@ -35,7 +47,12 @@ module API
         get ":id/#{noteables_str}/:#{noteable_id_str}/notes/:note_id" do
           @noteable = user_project.send(:"#{noteables_str}").find(params[:"#{noteable_id_str}"])
           @note = @noteable.notes.find(params[:note_id])
-          present @note, with: Entities::Note
+
+          if @note.cross_reference_not_visible_for?(current_user)
+            not_found!("Note")
+          else
+            present @note, with: Entities::Note
+          end
         end
 
         # Create a new +noteable+ note
diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb
index 8b177af4689..565805d870c 100644
--- a/spec/requests/api/notes_spec.rb
+++ b/spec/requests/api/notes_spec.rb
@@ -10,6 +10,24 @@ describe API::API, api: true  do
   let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) }
   let!(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) }
   let!(:snippet_note) { create(:note, noteable: snippet, project: project, author: user) }
+
+  # For testing the cross-reference of a private issue in a public issue
+  let(:private_user)    { create(:user) }
+  let(:private_project) {
+    create(:project, namespace: private_user.namespace).
+    tap { |p| p.team << [private_user, :master] }
+  }
+  let(:private_issue)   { create(:issue, project: private_project) }
+  let(:ext_proj)  { create(:project, :public) }
+  let(:ext_issue) { create(:issue, project: ext_proj) }
+
+  let!(:cross_reference_note) {
+    create :note,
+    noteable: ext_issue, project: ext_proj,
+    note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
+    system: true
+  }
+
   before { project.team << [user, :reporter] }
 
   describe "GET /projects/:id/noteable/:noteable_id/notes" do
@@ -25,6 +43,24 @@ describe API::API, api: true  do
         get api("/projects/#{project.id}/issues/123/notes", user)
         expect(response.status).to eq(404)
       end
+
+      context "that references a private issue" do
+        it "should return an empty array" do
+          get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user)
+          expect(response.status).to eq(200)
+          expect(json_response).to be_an Array
+          expect(json_response).to be_empty
+        end
+
+        context "and current user can view the note" do
+          it "should return an empty array" do
+            get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", private_user)
+            expect(response.status).to eq(200)
+            expect(json_response).to be_an Array
+            expect(json_response.first['body']).to eq(cross_reference_note.note)
+          end
+        end
+      end
     end
 
     context "when noteable is a Snippet" do
@@ -68,6 +104,21 @@ describe API::API, api: true  do
         get api("/projects/#{project.id}/issues/#{issue.id}/notes/123", user)
         expect(response.status).to eq(404)
       end
+
+      context "that references a private issue" do
+        it "should return a 404 error" do
+          get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", user)
+          expect(response.status).to eq(404)
+        end
+
+        context "and current user can view the note" do
+          it "should return an issue note by id" do
+            get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", private_user)
+            expect(response.status).to eq(200)
+            expect(json_response['body']).to eq(cross_reference_note.note)
+          end
+        end
+      end
     end
 
     context "when noteable is a Snippet" do
-- 
cgit v1.2.1


From 3b7f34281e4d1c4ca626578ddc9a1b9eda7e7538 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Wed, 13 Jan 2016 19:57:23 +0100
Subject: Modify :ci_variable factory

---
 spec/factories/ci/variables.rb      | 7 ++-----
 spec/requests/api/variables_spec.rb | 4 ++--
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb
index a19b9fc72f2..8f62d64411b 100644
--- a/spec/factories/ci/variables.rb
+++ b/spec/factories/ci/variables.rb
@@ -16,10 +16,7 @@
 
 FactoryGirl.define do
   factory :ci_variable, class: Ci::Variable do
-    id 10
-    key 'TEST_VARIABLE_1'
-    value 'VALUE_1'
-
-    project factory: :empty_project
+    sequence(:key) { |n| "VARIABLE_#{n}" }
+    value 'VARIABLE_VALUE'
   end
 end
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 214d7d5a0cc..9744729ba0c 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -46,7 +46,7 @@ describe API::API, api: true do
         expect(json_response['value']).to eq(variable.value)
       end
 
-      it 'should responde with 404 Not Found if requesting non-existing variable' do
+      it 'should respond with 404 Not Found if requesting non-existing variable' do
         get api("/projects/#{project.id}/variables/non_existing_variable", user)
 
         expect(response.status).to eq(404)
@@ -84,7 +84,7 @@ describe API::API, api: true do
 
       it 'should not allow to duplicate variable key' do
         expect do
-          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_1', value: 'VALUE_2'
+          post api("/projects/#{project.id}/variables", user), key: variable.key, value: 'VALUE_2'
         end.to change{project.variables.count}.by(0)
 
         expect(response.status).to eq(400)
-- 
cgit v1.2.1


From 2b81248ad5329a717d2f10f8c19453b989d202ac Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Wed, 13 Jan 2016 20:23:48 +0100
Subject: Modify builds API documentation style [ci skip]

---
 doc/api/build_variables.md | 84 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 64 insertions(+), 20 deletions(-)

diff --git a/doc/api/build_variables.md b/doc/api/build_variables.md
index 2b804fd9051..5da5fb8f4a6 100644
--- a/doc/api/build_variables.md
+++ b/doc/api/build_variables.md
@@ -1,9 +1,5 @@
 # Build Variables
 
-## Variables keys
-
-All variable keys must contains only letters, digits and '\_'. They must also be no longer than 255 characters.
-
 ## List project variables
 
 Get list of variables of a project.
@@ -12,9 +8,19 @@ Get list of variables of a project.
 GET /projects/:id/variables
 ```
 
-Parameters:
+### Parameters
+
+| Attribute | Type    | required | Description         |
+|-----------|---------|----------|---------------------|
+| id        | integer | yes      | The ID of a project |
+
+### Example of request
+
+```
+curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables"
+```
 
-- `id` (required) - The ID of a project
+### Example of response
 
 ```json
 [
@@ -37,10 +43,20 @@ Get details of specifica variable of a project.
 GET /projects/:id/variables/:key
 ```
 
-Parameters:
+### Parameters
 
-- `id` (required) - The ID of a project
-- `key` (required) - The `key` of variable
+| Attribute | Type    | required | Description           |
+|-----------|---------|----------|-----------------------|
+| id        | integer | yes      | The ID of a project   |
+| key       | string  | yes      | The `key` of variable |
+
+### Example of request
+
+```
+curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/TEST_VARIABLE_1"
+```
+
+### Example of response
 
 ```json
 {
@@ -57,11 +73,21 @@ Create new variable in project.
 POST /projects/:id/variables
 ```
 
-Parameters:
+### Parameters
+
+| Attribute | Type    | required | Description           |
+|-----------|---------|----------|-----------------------|
+| id        | integer | yes      | The ID of a project   |
+| key       | string  | yes      | The `key` of variable; must have no more than 255 characters; only `A-Z`, `a-z`, `0-9`, and `_` are allowed |
+| value     | string  | yes      | The `value` of variable |
 
-- `id` (required) - The ID of a project
-- `key` (required) - The `key` for variable
-- `value` (required) - The `value` for variable
+### Example of request
+
+```
+curl -X POST -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables" -F "key=NEW_VARIABLE" -F "value=new value"
+```
+
+### Example of response
 
 ```json
 {
@@ -78,11 +104,21 @@ Update variable.
 PUT /projects/:id/variables/:key
 ```
 
-Parameters:
+### Parameters
 
-- `id` (required) - The ID of a project
-- `key` (required) - The `key` for variable
-- `value` (required) - The `value` for variable
+| Attribute | Type    | required | Description             |
+|-----------|---------|----------|-------------------------|
+| id        | integer | yes      | The ID of a project     |
+| key       | string  | yes      | The `key` of variable   |
+| value     | string  | yes      | The `value` of variable |
+
+### Example of request
+
+```
+curl -X PUT -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/NEW_VARIABLE" -F "value=updated value"
+```
+
+### Example of response
 
 ```json
 {
@@ -99,8 +135,16 @@ Remove variable.
 DELETE /projects/:id/variables/:key
 ```
 
-Parameters:
+### Parameters
+
+| Attribute | Type    | required | Description             |
+|-----------|---------|----------|-------------------------|
+| id        | integer | yes      | The ID of a project     |
+| key       | string  | yes      | The `key` of variable   |
 
-- `id` (required) - The ID of a project
-- `key` (required) - The `key` for variable
+### Example of request
+
+```
+curl -X DELETE -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/VARIABLE_1"
+```
 
-- 
cgit v1.2.1


From 650a3f2a410660c4ed96119048903b8b91e35421 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Wed, 13 Jan 2016 16:07:20 -0500
Subject: Update button styles for Milestones#show

---
 app/views/projects/milestones/show.html.haml | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml
index 1670ea8741a..d1db0f64f88 100644
--- a/app/views/projects/milestones/show.html.haml
+++ b/app/views/projects/milestones/show.html.haml
@@ -20,16 +20,16 @@
   .pull-right
     - if can?(current_user, :admin_milestone, @project)
       - if @milestone.active?
-        = link_to 'Close Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-close btn-grouped"
+        = link_to 'Close Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-close btn-nr btn-grouped"
       - else
-        = link_to 'Reopen Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-grouped"
+        = link_to 'Reopen Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-nr btn-grouped"
 
-      = link_to namespace_project_milestone_path(@project.namespace, @project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-remove" do
-        %i.fa.fa-trash-o
+      = link_to namespace_project_milestone_path(@project.namespace, @project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-nr btn-remove" do
+        = icon('trash-o')
         Delete
 
-      = link_to edit_namespace_project_milestone_path(@project.namespace, @project, @milestone), class: "btn btn-grouped" do
-        %i.fa.fa-pencil-square-o
+      = link_to edit_namespace_project_milestone_path(@project.namespace, @project, @milestone), class: "btn btn-grouped btn-nr" do
+        = icon('pencil-square-o')
         Edit
 
 .detail-page-description.gray-content-block.second-block
-- 
cgit v1.2.1


From da3c57080f54948caab5d12626257c91637ded92 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Wed, 13 Jan 2016 16:07:36 -0500
Subject: Fix misaligned edit button in milestone collection partial

---
 app/views/projects/milestones/_milestone.html.haml | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/views/projects/milestones/_milestone.html.haml b/app/views/projects/milestones/_milestone.html.haml
index d6a44c9f0a1..67d95ab0364 100644
--- a/app/views/projects/milestones/_milestone.html.haml
+++ b/app/views/projects/milestones/_milestone.html.haml
@@ -21,10 +21,11 @@
       = render 'shared/milestone_expired', milestone: milestone
     .col-sm-6
       - if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
-        = link_to edit_namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone), class: "btn btn-xs edit-milestone-link btn-grouped" do
-          %i.fa.fa-pencil-square-o
+        = link_to edit_namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone), class: "btn btn-xs" do
+          = icon('pencil-square-o')
           Edit
+        \
         = link_to 'Close Milestone', namespace_project_milestone_path(@project.namespace, @project, milestone, milestone: {state_event: :close }), method: :put, remote: true, class: "btn btn-xs btn-close"
         = link_to namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-xs btn-remove" do
-          %i.fa.fa-trash-o
+          = icon('trash-o')
           Delete
-- 
cgit v1.2.1


From f11ba743a58ce769cdf6ea7427392836355c067d Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Wed, 13 Jan 2016 22:44:11 +0100
Subject: Add some cosmetic changes to variables API documentation [ci skip]

---
 doc/api/build_variables.md | 66 ++++++++++++++++------------------------------
 1 file changed, 22 insertions(+), 44 deletions(-)

diff --git a/doc/api/build_variables.md b/doc/api/build_variables.md
index 5da5fb8f4a6..b96f1bdac8a 100644
--- a/doc/api/build_variables.md
+++ b/doc/api/build_variables.md
@@ -2,26 +2,20 @@
 
 ## List project variables
 
-Get list of variables of a project.
+Get list of a project's build variables.
 
 ```
 GET /projects/:id/variables
 ```
 
-### Parameters
-
 | Attribute | Type    | required | Description         |
 |-----------|---------|----------|---------------------|
-| id        | integer | yes      | The ID of a project |
-
-### Example of request
+| `id`      | integer | yes      | The ID of a project |
 
 ```
 curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables"
 ```
 
-### Example of response
-
 ```json
 [
     {
@@ -37,27 +31,21 @@ curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3
 
 ## Show variable details
 
-Get details of specifica variable of a project.
+Get the details of a project's specific build variable.
 
 ```
 GET /projects/:id/variables/:key
 ```
 
-### Parameters
-
 | Attribute | Type    | required | Description           |
 |-----------|---------|----------|-----------------------|
-| id        | integer | yes      | The ID of a project   |
-| key       | string  | yes      | The `key` of variable |
-
-### Example of request
+| `id`      | integer | yes      | The ID of a project   |
+| `key`     | string  | yes      | The `key` of a variable |
 
 ```
 curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/TEST_VARIABLE_1"
 ```
 
-### Example of response
-
 ```json
 {
     "key": "TEST_VARIABLE_1",
@@ -67,28 +55,22 @@ curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3
 
 ## Create variable
 
-Create new variable in project.
+Create a new build variable.
 
 ```
 POST /projects/:id/variables
 ```
 
-### Parameters
-
 | Attribute | Type    | required | Description           |
 |-----------|---------|----------|-----------------------|
-| id        | integer | yes      | The ID of a project   |
-| key       | string  | yes      | The `key` of variable; must have no more than 255 characters; only `A-Z`, `a-z`, `0-9`, and `_` are allowed |
-| value     | string  | yes      | The `value` of variable |
-
-### Example of request
+| `id`      | integer | yes      | The ID of a project   |
+| `key`     | string  | yes      | The `key` of a variable; must have no more than 255 characters; only `A-Z`, `a-z`, `0-9`, and `_` are allowed |
+| `value`   | string  | yes      | The `value` of a variable |
 
 ```
 curl -X POST -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables" -F "key=NEW_VARIABLE" -F "value=new value"
 ```
 
-### Example of response
-
 ```json
 {
     "key": "NEW_VARIABLE",
@@ -98,28 +80,22 @@ curl -X POST -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.co
 
 ## Update variable
 
-Update variable.
+Update a project's build variable.
 
 ```
 PUT /projects/:id/variables/:key
 ```
 
-### Parameters
-
 | Attribute | Type    | required | Description             |
 |-----------|---------|----------|-------------------------|
-| id        | integer | yes      | The ID of a project     |
-| key       | string  | yes      | The `key` of variable   |
-| value     | string  | yes      | The `value` of variable |
-
-### Example of request
+| `id`      | integer | yes      | The ID of a project     |
+| `key`     | string  | yes      | The `key` of a variable   |
+| `value`   | string  | yes      | The `value` of a variable |
 
 ```
 curl -X PUT -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/NEW_VARIABLE" -F "value=updated value"
 ```
 
-### Example of response
-
 ```json
 {
     "key": "NEW_VARIABLE",
@@ -129,22 +105,24 @@ curl -X PUT -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com
 
 ## Remove variable
 
-Remove variable.
+Remove a project's build variable.
 
 ```
 DELETE /projects/:id/variables/:key
 ```
 
-### Parameters
-
 | Attribute | Type    | required | Description             |
 |-----------|---------|----------|-------------------------|
-| id        | integer | yes      | The ID of a project     |
-| key       | string  | yes      | The `key` of variable   |
-
-### Example of request
+| `id`      | integer | yes      | The ID of a project     |
+| `key`     | string  | yes      | The `key` of a variable |
 
 ```
 curl -X DELETE -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/VARIABLE_1"
 ```
 
+```json
+{
+    "key": "VARIABLE_1",
+    "value": "VALUE_1"
+}
+```
-- 
cgit v1.2.1


From eee16ca9ccd34ff950b685f4db57518207055a36 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Wed, 13 Jan 2016 23:59:44 +0100
Subject: Update CHANGELOG [ci skip]

---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index b430d4981a9..40e3ea31969 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -43,6 +43,7 @@ v 8.4.0 (unreleased)
   - API: Add support for deleting a tag via the API (Robert Schilling)
   - Allow subsequent validations in CI Linter
   - Fix Encoding::CompatibilityError bug when markdown content has some complex URL (Jason Lee)
+  - Add API support for managing build variables of project
 
 v 8.3.4
   - Use gitlab-workhorse 0.5.4 (fixes API routing bug)
-- 
cgit v1.2.1


From d11ca78cdcd761f7f9b6388474765ede51011085 Mon Sep 17 00:00:00 2001
From: DJ Mountney <dmountney@perforce.com>
Date: Wed, 13 Jan 2016 17:01:05 -0800
Subject: Fix the undefinded variable error in Project's safe_import_url method

---
 app/models/project.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index 31990485f7d..7e131151513 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -397,7 +397,7 @@ class Project < ActiveRecord::Base
     result.password = '*****' unless result.password.nil?
     result.to_s
   rescue
-    original_url
+    self.import_url
   end
 
   def check_limit
-- 
cgit v1.2.1


From c7f7a1ae923532ec3b2e2d25ce97ebbac83cf2e0 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Wed, 13 Jan 2016 17:20:13 -0800
Subject: Update to Go 1.5.3

---
 doc/install/installation.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/install/installation.md b/doc/install/installation.md
index e645445df2a..00030729a4b 100644
--- a/doc/install/installation.md
+++ b/doc/install/installation.md
@@ -135,11 +135,11 @@ gitlab-workhorse we need a Go compiler. The instructions below assume you
 use 64-bit Linux. You can find downloads for other platforms at the [Go download
 page](https://golang.org/dl).
 
-    curl -O --progress https://storage.googleapis.com/golang/go1.5.1.linux-amd64.tar.gz
-    echo '46eecd290d8803887dec718c691cc243f2175fe0  go1.5.1.linux-amd64.tar.gz' | shasum -c - && \
-      sudo tar -C /usr/local -xzf go1.5.1.linux-amd64.tar.gz
+    curl -O --progress https://storage.googleapis.com/golang/go1.5.3.linux-amd64.tar.gz
+    echo '43afe0c5017e502630b1aea4d44b8a7f059bf60d7f29dfd58db454d4e4e0ae53  go1.5.3.linux-amd64.tar.gz' | shasum -c - && \
+      sudo tar -C /usr/local -xzf go1.5.3.linux-amd64.tar.gz
     sudo ln -sf /usr/local/go/bin/{go,godoc,gofmt} /usr/local/bin/
-    rm go1.5.1.linux-amd64.tar.gz
+    rm go1.5.3.linux-amd64.tar.gz
 
 ## 4. System Users
 
-- 
cgit v1.2.1


From fc7e6e8ffcb76e92b625b6490a966c830adc750a Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Thu, 14 Jan 2016 03:08:40 -0200
Subject: Fix Admin/Users view to position buttons without spacing magic

---
 app/assets/stylesheets/framework/buttons.scss |  3 +++
 app/views/admin/users/index.html.haml         | 30 +++++++++++++--------------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss
index e2376363485..bb29829b7a1 100644
--- a/app/assets/stylesheets/framework/buttons.scss
+++ b/app/assets/stylesheets/framework/buttons.scss
@@ -131,6 +131,9 @@
     &:last-child {
       margin-right: 0px;
     }
+    &.btn-xs {
+      margin-right: 3px;
+    }
   }
   &.disabled {
     pointer-events: auto !important;
diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml
index 911c4d0cf12..8312642b6c3 100644
--- a/app/views/admin/users/index.html.haml
+++ b/app/views/admin/users/index.html.haml
@@ -88,19 +88,19 @@
                 %i.fa.fa-envelope
                 = mail_to user.email, user.email, class: 'light'
               &nbsp;
-              = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: "btn btn-xs"
-              - unless user == current_user
-                - if user.ldap_blocked?
-                  = link_to '#', title: 'Cannot unblock LDAP blocked users', data: {toggle: 'tooltip'}, class: 'btn btn-xs btn-success disabled' do
-                    %i.fa.fa-lock
-                    Unblock
-                  = ''
-                - elsif user.blocked?
-                  = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success"
-                - else
-                  = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs btn-warning"
-                - if user.access_locked?
-                  = link_to 'Unlock', unlock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success", data: { confirm: 'Are you sure?' }
-                - if user.can_be_removed?
-                  = link_to 'Destroy', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All issues, merge requests and groups linked to this user will also be removed! Maybe block the user instead? Are you sure?" }, method: :delete, class: "btn btn-xs btn-remove"
+              .pull-right
+                = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: 'btn-grouped btn btn-xs'
+                - unless user == current_user
+                  - if user.ldap_blocked?
+                    = link_to '#', title: 'Cannot unblock LDAP blocked users', data: {toggle: 'tooltip'}, class: 'btn-grouped btn btn-xs btn-success disabled' do
+                      %i.fa.fa-lock
+                      Unblock
+                  - elsif user.blocked?
+                    = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: 'btn-grouped btn btn-xs btn-success'
+                  - else
+                    = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: 'btn-grouped btn btn-xs btn-warning'
+                  - if user.access_locked?
+                    = link_to 'Unlock', unlock_admin_user_path(user), method: :put, class: 'btn-grouped btn btn-xs btn-success', data: { confirm: 'Are you sure?' }
+                  - if user.can_be_removed?
+                    = link_to 'Destroy', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All issues, merge requests and groups linked to this user will also be removed! Maybe block the user instead? Are you sure?" }, method: :delete, class: 'btn-grouped btn btn-xs btn-remove'
     = paginate @users, theme: "gitlab"
-- 
cgit v1.2.1


From dd6fc01ff8a073880b67a323a547edeb5d63f167 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Thu, 14 Jan 2016 03:31:27 -0200
Subject: fixed LDAP activation on login to use new ldap_blocked state

---
 app/models/user.rb                  | 1 +
 lib/gitlab/ldap/access.rb           | 4 +---
 spec/lib/gitlab/ldap/access_spec.rb | 5 ++---
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 5eed9cf91c7..592468933ed 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -205,6 +205,7 @@ class User < ActiveRecord::Base
 
     event :activate do
       transition blocked: :active
+      transition ldap_blocked: :active
     end
 
     state :blocked, :ldap_blocked do
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index ebd9260ad5d..a659d179b5f 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -40,9 +40,7 @@ module Gitlab
             user.ldap_block
             false
           else
-            if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked?
-              user.activate
-            end
+            user.activate if user.ldap_blocked?
             true
           end
         else
diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb
index f58d70e809c..32a19bf344b 100644
--- a/spec/lib/gitlab/ldap/access_spec.rb
+++ b/spec/lib/gitlab/ldap/access_spec.rb
@@ -42,7 +42,6 @@ describe Gitlab::LDAP::Access, lib: true do
 
       context 'and has no disabled flag in active diretory' do
         before do
-          user.block
           allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false)
         end
 
@@ -50,7 +49,7 @@ describe Gitlab::LDAP::Access, lib: true do
 
         context 'when auto-created users are blocked' do
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(true)
+            user.block
           end
 
           it 'does not unblock user in GitLab' do
@@ -62,7 +61,7 @@ describe Gitlab::LDAP::Access, lib: true do
 
         context 'when auto-created users are not blocked' do
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(false)
+            user.ldap_block
           end
 
           it 'should unblock user in GitLab' do
-- 
cgit v1.2.1


From 818607f0115a2c67792fa83e9b39d74c443a8856 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Thu, 14 Jan 2016 09:41:11 +0100
Subject: Improve the consistency of commit titles, branch names, tag names,
 issue/MR titles, on their respective project pages

---
 CHANGELOG                                        | 1 +
 app/assets/stylesheets/pages/branches.scss       | 3 +++
 app/assets/stylesheets/pages/commit.scss         | 4 ++++
 app/assets/stylesheets/pages/groups.scss         | 5 +++++
 app/assets/stylesheets/pages/issues.scss         | 2 +-
 app/assets/stylesheets/pages/merge_requests.scss | 2 +-
 app/assets/stylesheets/pages/tags.scss           | 3 +++
 app/views/projects/branches/_branch.html.haml    | 2 +-
 app/views/projects/commits/_commit.html.haml     | 2 +-
 app/views/projects/tags/_tag.html.haml           | 2 +-
 app/views/projects/tags/show.html.haml           | 4 ++--
 app/views/shared/groups/_group.html.haml         | 3 +--
 12 files changed, 24 insertions(+), 9 deletions(-)
 create mode 100644 app/assets/stylesheets/pages/branches.scss
 create mode 100644 app/assets/stylesheets/pages/tags.scss

diff --git a/CHANGELOG b/CHANGELOG
index b430d4981a9..c89f322da48 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.4.0 (unreleased)
+  - Improve the consistency of commit titles, branch names, tag names, issue/MR titles, on their respective project pages
   - Autocomplete data is now always loaded, instead of when focusing a comment text area (Yorick Peterse)
   - Improved performance of finding issues for an entire group (Yorick Peterse)
   - Added custom application performance measuring system powered by InfluxDB (Yorick Peterse)
diff --git a/app/assets/stylesheets/pages/branches.scss b/app/assets/stylesheets/pages/branches.scss
new file mode 100644
index 00000000000..abae5c3d0a5
--- /dev/null
+++ b/app/assets/stylesheets/pages/branches.scss
@@ -0,0 +1,3 @@
+.branch-name{
+  font-weight: 600;
+}
diff --git a/app/assets/stylesheets/pages/commit.scss b/app/assets/stylesheets/pages/commit.scss
index 17245d3be7b..05d2f306d0f 100644
--- a/app/assets/stylesheets/pages/commit.scss
+++ b/app/assets/stylesheets/pages/commit.scss
@@ -2,6 +2,10 @@
   display: block;
 }
 
+.commit-row-title .commit-title {
+  font-weight: 600;
+}
+
 .commit-author, .commit-committer{
   display: block;
   color: #999;
diff --git a/app/assets/stylesheets/pages/groups.scss b/app/assets/stylesheets/pages/groups.scss
index 263993f59a5..3404c2631e1 100644
--- a/app/assets/stylesheets/pages/groups.scss
+++ b/app/assets/stylesheets/pages/groups.scss
@@ -11,3 +11,8 @@
     height: 42px;
   }
 }
+
+.content-list .group-name {
+  font-weight: 600;
+  color: #4c4e54;
+}
diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss
index 1e1af662850..ad92cc22815 100644
--- a/app/assets/stylesheets/pages/issues.scss
+++ b/app/assets/stylesheets/pages/issues.scss
@@ -6,7 +6,7 @@
     .issue-title {
       margin-bottom: 5px;
       font-size: $list-font-size;
-      font-weight: bold;
+      font-weight: 600;
     }
 
     .issue-info {
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 82effde0bf3..efd33df2e99 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -150,7 +150,7 @@
     .merge-request-title {
       margin-bottom: 5px;
       font-size: $list-font-size;
-      font-weight: bold;
+      font-weight: 600;
     }
 
     .merge-request-info {
diff --git a/app/assets/stylesheets/pages/tags.scss b/app/assets/stylesheets/pages/tags.scss
new file mode 100644
index 00000000000..e9cd6dc6c5e
--- /dev/null
+++ b/app/assets/stylesheets/pages/tags.scss
@@ -0,0 +1,3 @@
+.tag-name{
+  font-weight: 600;
+}
diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml
index a234536723e..d276e5932d1 100644
--- a/app/views/projects/branches/_branch.html.haml
+++ b/app/views/projects/branches/_branch.html.haml
@@ -6,7 +6,7 @@
 %li(class="js-branch-#{branch.name}")
   %div
     = link_to namespace_project_tree_path(@project.namespace, @project, branch.name) do
-      %strong.str-truncated= branch.name
+      .branch-name.str-truncated= branch.name
     &nbsp;
     - if branch.name == @repository.root_ref
       %span.label.label-primary default
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index 012825f0fdb..4d4b410ee29 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -11,7 +11,7 @@
 = cache(cache_key) do
   %li.commit.js-toggle-container{ id: "commit-#{commit.short_id}" }
     .commit-row-title
-      %strong.str-truncated
+      .commit-title.str-truncated
         = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-row-message"
         - if commit.description?
           %a.text-expander.js-toggle-button ...
diff --git a/app/views/projects/tags/_tag.html.haml b/app/views/projects/tags/_tag.html.haml
index 28b706c5c7e..56a7ced1236 100644
--- a/app/views/projects/tags/_tag.html.haml
+++ b/app/views/projects/tags/_tag.html.haml
@@ -3,7 +3,7 @@
 %li
   %div
     = link_to namespace_project_tag_path(@project.namespace, @project, tag.name) do
-      %strong
+      .tag-name
         = icon('tag')
         = tag.name
     - if tag.message.present?
diff --git a/app/views/projects/tags/show.html.haml b/app/views/projects/tags/show.html.haml
index b594d4f1f27..dbb20347860 100644
--- a/app/views/projects/tags/show.html.haml
+++ b/app/views/projects/tags/show.html.haml
@@ -17,8 +17,8 @@
       .pull-right
         = link_to namespace_project_tag_path(@project.namespace, @project, @tag.name), class: 'btn btn-remove remove-row grouped has_tooltip', title: "Delete tag", method: :delete, data: { confirm: "Deleting the '#{@tag.name}' tag cannot be undone. Are you sure?" } do
           %i.fa.fa-trash-o
-  .title
-    %strong= @tag.name
+  .tag-name.title
+    = @tag.name
     - if @tag.message.present?
       %span.light
         &nbsp;
diff --git a/app/views/shared/groups/_group.html.haml b/app/views/shared/groups/_group.html.haml
index a54c5fa8c33..f4cfa29ae56 100644
--- a/app/views/shared/groups/_group.html.haml
+++ b/app/views/shared/groups/_group.html.haml
@@ -10,8 +10,7 @@
         %i.fa.fa-sign-out
 
   = image_tag group_icon(group), class: "avatar s46 hidden-xs"
-  = link_to group, class: 'group-name' do
-    %strong= group.name
+  = link_to group.name, group, class: 'group-name'
 
   - if group_member
     as
-- 
cgit v1.2.1


From e918493f55eb27cdb779f0bc2d8cbbace8b69aa9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Thu, 14 Jan 2016 10:04:48 +0100
Subject: Fix specs and rubocop warnings

---
 features/steps/shared/project.rb |  2 +-
 spec/models/note_spec.rb         |  4 ++--
 spec/requests/api/notes_spec.rb  | 11 ++++++-----
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb
index 5420c451519..d3501b5f5cb 100644
--- a/features/steps/shared/project.rb
+++ b/features/steps/shared/project.rb
@@ -224,7 +224,7 @@ module SharedProject
   end
 
   def user_owns_project(user_name:, project_name:, visibility: :private)
-    user = user_exists(user_name, username: user_name.underscore)
+    user = user_exists(user_name, username: user_name.gsub(/\s/, '').underscore)
     project = Project.find_by(name: project_name)
     project ||= create(:empty_project, visibility, name: project_name, namespace: user.namespace)
     project.team << [user, :master]
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 65e6a7df3b4..9182b42661d 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -186,12 +186,12 @@ describe Note, models: true do
     let(:ext_proj)  { create(:project, :public) }
     let(:ext_issue) { create(:issue, project: ext_proj) }
 
-    let(:note) {
+    let(:note) do
       create :note,
         noteable: ext_issue, project: ext_proj,
         note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
         system: true
-    }
+    end
 
     it "returns true" do
       expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb
index 565805d870c..d8bbd107269 100644
--- a/spec/requests/api/notes_spec.rb
+++ b/spec/requests/api/notes_spec.rb
@@ -13,20 +13,21 @@ describe API::API, api: true  do
 
   # For testing the cross-reference of a private issue in a public issue
   let(:private_user)    { create(:user) }
-  let(:private_project) {
+  let(:private_project) do
     create(:project, namespace: private_user.namespace).
     tap { |p| p.team << [private_user, :master] }
-  }
-  let(:private_issue)   { create(:issue, project: private_project) }
+  end
+  let(:private_issue)    { create(:issue, project: private_project) }
+
   let(:ext_proj)  { create(:project, :public) }
   let(:ext_issue) { create(:issue, project: ext_proj) }
 
-  let!(:cross_reference_note) {
+  let!(:cross_reference_note) do
     create :note,
     noteable: ext_issue, project: ext_proj,
     note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
     system: true
-  }
+  end
 
   before { project.team << [user, :reporter] }
 
-- 
cgit v1.2.1


From 4435a3a8c380002356bfb3d76acd8609e4c3fb25 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Thu, 14 Jan 2016 12:05:47 +0100
Subject: Fix version

---
 VERSION | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/VERSION b/VERSION
index 408340137f0..ce669730119 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-8.4.0.rc1
\ No newline at end of file
+8.4.0.pre
-- 
cgit v1.2.1