Merge branch 'master' into 8-8-stable

CHANGELOG

@@ -45,7 +45,7 @@ v 8.8.0 (unreleased)
   - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
   - Add eager load paths to help prevent dependency load issues in Sidekiq workers. !3724
   - Added multiple colors for labels in dropdowns when dups happen.
-  - Always group commits by server timezone, not commit timestamp
+  - Show commits in the same order as `git log`
   - Improve description for the Two-factor Authentication sign-in screen. (Connor Shea)
   - API support for the 'since' and 'until' operators on commit requests (Paco Guzman)
   - Fix Gravatar hint in user profile when Gravatar is disabled. !3988 (Artem Sidorenko)

app/assets/stylesheets/framework/variables.scss

@@ -12,7 +12,7 @@ $gutter_inner_width: 258px;
  */
 $border-color:       #e5e5e5;
 $focus-border-color: #3aabf0;
-$table-border-color: #ececec;
+$table-border-color: #f0f0f0;
 $background-color:   #fafafa;
 
 /*

app/controllers/projects/variables_controller.rb

@@ -3,20 +3,44 @@ class Projects::VariablesController < Projects::ApplicationController
 
   layout 'project_settings'
 
+  def index
+    @variable = Ci::Variable.new
+  end
+
   def show
+    @variable = @project.variables.find(params[:id])
   end
 
   def update
-    if project.update_attributes(project_params)
+    @variable = @project.variables.find(params[:id])
+
+    if @variable.update_attributes(project_params)
+      redirect_to namespace_project_variables_path(project.namespace, project), notice: 'Variable was successfully updated.'
+    else
+      render action: "show"
+    end
+  end
+
+  def create
+    @variable = Ci::Variable.new(project_params)
+
+    if @variable.valid? && @project.variables << @variable
       redirect_to namespace_project_variables_path(project.namespace, project), notice: 'Variables were successfully updated.'
     else
-      render action: 'show'
+      render action: "index"
+    end
   end
+
+  def destroy
+    @key = @project.variables.find(params[:id])
+    @key.destroy
+
+    redirect_to namespace_project_variables_path(project.namespace, project), notice: 'Variable was successfully removed.'
   end
 
   private
 
   def project_params
-    params.require(:project).permit({ variables_attributes: [:id, :key, :value, :_destroy] })
+    params.require(:variable).permit([:id, :key, :value, :_destroy])
   end
 end

app/models/merge_request_diff.rb

@@ -98,9 +98,7 @@ class MergeRequestDiff < ActiveRecord::Base
     commits = compare.commits
 
     if commits.present?
-      commits = Commit.decorate(commits, merge_request.source_project).
-        sort_by(&:created_at).
-        reverse
+      commits = Commit.decorate(commits, merge_request.source_project).reverse
     end
 
     commits

app/views/projects/commits/_commits.html.haml

@@ -3,7 +3,7 @@
 
 - commits, hidden = limited_commits(@commits)
 
-- commits.group_by { |c| c.committed_date.in_time_zone.to_date }.sort.reverse.each do |day, commits|
+- commits.chunk { |c| c.committed_date.in_time_zone.to_date }.each do |day, commits|
   .row.commits-row
     .col-md-2.hidden-xs.hidden-sm
       %h5.commits-row-date

app/views/projects/deploy_keys/index.html.haml

@@ -19,7 +19,7 @@
       %ul.well-list
         = render @enabled_keys
     - else
-      .profile-settings-message.text-center
+      .settings-message.text-center
         No deploy keys found. Create one with the form above or add existing one below.
     %h5.prepend-top-default
       Deploy keys from projects you have access to (#{@available_project_keys.size})
@@ -27,7 +27,7 @@
       %ul.well-list
         = render @available_project_keys
     - else
-      .profile-settings-message.text-center
+      .settings-message.text-center
         No deploy keys from your projects could be found. Create one with the form above or add existing one below.
     - if @available_public_keys.any?
       %h5.prepend-top-default

app/views/projects/hooks/index.html.haml

@@ -80,5 +80,5 @@
         - @hooks.each do |hook|
           = render "project_hook", hook: hook
     - else
-      %p.profile-settings-message.text-center.append-bottom-0
+      %p.settings-message.text-center.append-bottom-0
         No webhooks found, add one in the form above.

app/views/projects/protected_branches/_branches_list.html.haml

 %h5.prepend-top-0
   Already Protected (#{@branches.size})
 - if @branches.empty?
-  %p.profile-settings-message.text-center
+  %p.settings-message.text-center
     No branches are protected, protect a branch with the form above.
 - else
   - can_admin_project = can?(current_user, :admin_project, @project)

app/views/projects/triggers/index.html.haml

@@ -18,7 +18,7 @@
             %th
           = render partial: 'trigger', collection: @triggers, as: :trigger
     - else
-      %p.profile-settings-message.text-center.append-bottom-default
+      %p.settings-message.text-center.append-bottom-default
         There are no triggers to use, add one by the button below.
 
     = form_for @trigger, url: url_for(controller: 'projects/triggers', action: 'create') do |f|

app/views/projects/variables/_content.html.haml

+%h4.prepend-top-0
+  Secret Variables
+%p
+  These variables will be set to environment by the runner.
+%p
+  So you can use them for passwords, secret keys or whatever you want.
+%p
+  The value of the variable can be visible in build log if explicitly asked to do so.

app/views/projects/variables/_form.html.haml

+= form_for [@project.namespace.becomes(Namespace), @project, @variable] do |f|
+  = form_errors(@variable)
+
+  .form-group
+    = f.label :key, "Key", class: "label-light"
+    = f.text_field :key, class: "form-control", placeholder: "PROJECT_VARIABLE", required: true
+  .form-group
+    = f.label :value, "Value", class: "label-light"
+    = f.text_area :value, class: "form-control", placeholder: "PROJECT_VARIABLE", required: true
+  = f.submit btn_text, class: "btn btn-save"

app/views/projects/variables/_table.html.haml

+.table-responsive.variables-table
+  %table.table
+    %colgroup
+      %col
+      %col
+      %col{ width: 100 }
+    %thead
+      %th Key
+      %th Value
+      %th
+    %tbody
+      - @project.variables.each do |variable|
+        - if variable.id?
+          %tr
+            %td= variable.key
+            %td= variable.value
+            %td
+              = link_to namespace_project_variable_path(@project.namespace, @project, variable), class: "btn btn-transparent btn-variable-edit" do
+                %span.sr-only
+                  Update
+                = icon("pencil")
+              = link_to namespace_project_variable_path(@project.namespace, @project, variable), class: "btn btn-transparent btn-variable-delete", method: :delete, data: { confirm: "Are you sure?" } do
+                %span.sr-only
+                  Remove
+                = icon("trash")

app/views/projects/variables/index.html.haml

+- page_title "Variables"
+
+.row.prepend-top-default.append-bottom-default
+  .col-lg-3
+    = render "content"
+  .col-lg-9
+    %h5.prepend-top-0
+      Add a variable
+    = render "form", btn_text: "Add new variable"
+    %hr
+    %h5.prepend-top-0
+      Your variables (#{@project.variables.size})
+    - if @project.variables.empty?
+      %p.settings-message.text-center.append-bottom-0
+        No variables found, add one with the form above.
+    - else
+      = render "table"

app/views/projects/variables/show.html.haml

 - page_title "Variables"
-%h3.page-title
-  Secret Variables
 
-%p.light
-  These variables will be set to environment by the runner.
-  %br
-  So you can use them for passwords, secret keys or whatever you want.
-  %br
-  The value of the variable can be visible in build log if explicitly asked to do so.
-
-%hr
-
-
-= nested_form_for @project, url: url_for(controller: 'projects/variables', action: 'update'), html: { class: 'form-horizontal' }  do |f|
-  = form_errors(@project)
-
-  = f.fields_for :variables do |variable_form|
-    .form-group
-      = variable_form.label :key, 'Key', class: 'control-label'
-      .col-sm-10
-        = variable_form.text_field :key, class: 'form-control', placeholder: "PROJECT_VARIABLE"
-
-    .form-group
-      = variable_form.label :value, 'Value', class: 'control-label'
-      .col-sm-10
-        = variable_form.text_area :value, class: 'form-control', rows: 2, placeholder: ""
-
-        = variable_form.link_to_remove "Remove this variable", class: 'btn btn-danger pull-right prepend-top-10'
-    %hr
-  %p
-    .clearfix
-      = f.link_to_add "Add a variable", :variables, class: 'btn btn-success pull-right'
-
-  .form-actions
-    = f.submit 'Save changes', class: 'btn btn-save', return_to: request.original_url
+.row.prepend-top-default.append-bottom-default
+  .col-lg-3
+    = render "content"
+  .col-lg-9
+    %h5.prepend-top-0
+      Update variable
+    = render "form", btn_text: "Save variable"

config/routes.rb

@@ -663,7 +663,7 @@ Rails.application.routes.draw do
         end
 
         resources :protected_branches, only: [:index, :create, :update, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
-        resource :variables, only: [:show, :update]
+        resources :variables, only: [:index, :show, :update, :create, :destroy]
         resources :triggers, only: [:index, :create, :destroy]
 
         resources :builds, only: [:index, :show], constraints: { id: /\d+/ } do

spec/features/variables_spec.rb

 require 'spec_helper'
 
-describe "Variables" do
+describe 'Project variables', js: true do
   let(:user)     { create(:user) }
-  before { login_as(user) }
+  let(:project)  { create(:project) }
+  let(:variable) { create(:ci_variable, key: 'test') }
 
-  describe "specific runners" do
   before do
-      @project = FactoryGirl.create :empty_project
-      @project.team << [user, :master]
+    login_as(user)
+    project.team << [user, :master]
+    project.variables << variable
+
+    visit namespace_project_variables_path(project.namespace, project)
+  end
+
+  it 'should show list of variables' do
+    page.within('.variables-table') do
+      expect(page).to have_content(variable.key)
+    end
+  end
+
+  it 'should add new variable' do
+    fill_in('variable_key', with: 'key')
+    fill_in('variable_value', with: 'key value')
+    click_button('Add new variable')
+
+    page.within('.variables-table') do
+      expect(page).to have_content('key')
+    end
+  end
+
+  it 'should delete variable' do
+    page.within('.variables-table') do
+      find('.btn-variable-delete').click
+    end
+
+    expect(page).to_not have_selector('variables-table')
+  end
+
+  it 'should edit variable' do
+    page.within('.variables-table') do
+      find('.btn-variable-edit').click
     end
 
-    it "creates variable", js: true do
-      visit namespace_project_variables_path(@project.namespace, @project)
-      click_on "Add a variable"
-      fill_in "Key", with: "SECRET_KEY"
-      fill_in "Value", with: "SECRET_VALUE"
-      click_on "Save changes"
+    fill_in('variable_key', with: 'key')
+    fill_in('variable_value', with: 'key value')
+    click_button('Save variable')
 
-      expect(page).to have_content("Variables were successfully updated.")
-      expect(@project.variables.count).to eq(1)
+    page.within('.variables-table') do
+      expect(page).to have_content('key')
     end
   end
 end