From bda2c44a0b2f032d8c3eb9b9b8d79933fb40e651 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 27 Apr 2016 09:01:43 +0100 Subject: [PATCH 1/3] Project variables UI Closes #14091 --- .../projects/variables_controller.rb | 30 ++++++++++++-- .../projects/variables/_content.html.haml | 8 ++++ app/views/projects/variables/_form.html.haml | 10 +++++ app/views/projects/variables/_table.html.haml | 24 +++++++++++ app/views/projects/variables/index.html.haml | 17 ++++++++ app/views/projects/variables/show.html.haml | 41 ++++--------------- config/routes.rb | 2 +- 7 files changed, 94 insertions(+), 38 deletions(-) create mode 100644 app/views/projects/variables/_content.html.haml create mode 100644 app/views/projects/variables/_form.html.haml create mode 100644 app/views/projects/variables/_table.html.haml create mode 100644 app/views/projects/variables/index.html.haml diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index 00234654578..6f068729390 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/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 diff --git a/app/views/projects/variables/_content.html.haml b/app/views/projects/variables/_content.html.haml new file mode 100644 index 00000000000..0249e0c1bf1 --- /dev/null +++ b/app/views/projects/variables/_content.html.haml @@ -0,0 +1,8 @@ +%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. diff --git a/app/views/projects/variables/_form.html.haml b/app/views/projects/variables/_form.html.haml new file mode 100644 index 00000000000..a5bae83e0ce --- /dev/null +++ b/app/views/projects/variables/_form.html.haml @@ -0,0 +1,10 @@ += 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" diff --git a/app/views/projects/variables/_table.html.haml b/app/views/projects/variables/_table.html.haml new file mode 100644 index 00000000000..1a733200098 --- /dev/null +++ b/app/views/projects/variables/_table.html.haml @@ -0,0 +1,24 @@ +.table-responsive + %table.table + %colgroup + %col + %col + %col{ width: 100 } + %thead + %th Key + %th Value + %th + %tbody + - @project.variables.each do |variable| + %tr + %td= variable.key + %td= variable.value + %td + = link_to namespace_project_variable_path(@project.namespace, @project, variable), class: "btn btn-transparent" do + %span.sr-only + Update + = icon("pencil") + = link_to namespace_project_variable_path(@project.namespace, @project, variable), class: "btn btn-transparent", method: :delete, data: { confirm: "Are you sure?" } do + %span.sr-only + Remove + = icon("trash") diff --git a/app/views/projects/variables/index.html.haml b/app/views/projects/variables/index.html.haml new file mode 100644 index 00000000000..89ef79fb2e9 --- /dev/null +++ b/app/views/projects/variables/index.html.haml @@ -0,0 +1,17 @@ +- 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.profile-settings-message.text-center.append-bottom-0 + No variables found, add one with the form above. + - else + = render "table" diff --git a/app/views/projects/variables/show.html.haml b/app/views/projects/variables/show.html.haml index ca284b84d39..297a53ca98c 100644 --- a/app/views/projects/variables/show.html.haml +++ b/app/views/projects/variables/show.html.haml @@ -1,36 +1,9 @@ - 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" diff --git a/config/routes.rb b/config/routes.rb index 9e776a1f541..511ab5cfcd7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -660,7 +660,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 From 99428f193b2fe51b16e43f7cd98029326b5c30f9 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 27 Apr 2016 12:17:17 +0100 Subject: [PATCH 2/3] Updated project variable tests --- app/views/projects/variables/_table.html.haml | 6 +- spec/features/variables_spec.rb | 59 ++++++++++++++----- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/app/views/projects/variables/_table.html.haml b/app/views/projects/variables/_table.html.haml index 1a733200098..f51a52d7cb3 100644 --- a/app/views/projects/variables/_table.html.haml +++ b/app/views/projects/variables/_table.html.haml @@ -1,4 +1,4 @@ -.table-responsive +.table-responsive.variables-table %table.table %colgroup %col @@ -14,11 +14,11 @@ %td= variable.key %td= variable.value %td - = link_to namespace_project_variable_path(@project.namespace, @project, variable), class: "btn btn-transparent" do + = 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", method: :delete, data: { confirm: "Are you sure?" } do + = 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") diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb index afea1840cd7..48e2dae4884 100644 --- a/spec/features/variables_spec.rb +++ b/spec/features/variables_spec.rb @@ -1,24 +1,53 @@ require 'spec_helper' -describe "Variables" do - let(:user) { create(:user) } - before { login_as(user) } +describe 'Project variables', js: true do + let(:user) { create(: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] + before do + 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 - 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" + expect(page).to_not have_selector('variables-table') + end - expect(page).to have_content("Variables were successfully updated.") - expect(@project.variables.count).to eq(1) + it 'should edit variable' do + page.within('.variables-table') do + find('.btn-variable-edit').click + end + + fill_in('variable_key', with: 'key') + fill_in('variable_value', with: 'key value') + click_button('Save variable') + + page.within('.variables-table') do + expect(page).to have_content('key') end end end From c9b3c08c21daf421ec36828c990ccceb21e1577f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 16 May 2016 10:26:11 +0100 Subject: [PATCH 3/3] Changed table border color Fixed issue with form errors not displaying --- .../stylesheets/framework/variables.scss | 2 +- app/views/projects/variables/_table.html.haml | 25 ++++++++++--------- app/views/projects/variables/index.html.haml | 2 +- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 5fa4c266607..7360bdbcc82 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/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; /* diff --git a/app/views/projects/variables/_table.html.haml b/app/views/projects/variables/_table.html.haml index f51a52d7cb3..6c43f822db4 100644 --- a/app/views/projects/variables/_table.html.haml +++ b/app/views/projects/variables/_table.html.haml @@ -10,15 +10,16 @@ %th %tbody - @project.variables.each do |variable| - %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") + - 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") diff --git a/app/views/projects/variables/index.html.haml b/app/views/projects/variables/index.html.haml index 89ef79fb2e9..09bb54600af 100644 --- a/app/views/projects/variables/index.html.haml +++ b/app/views/projects/variables/index.html.haml @@ -11,7 +11,7 @@ %h5.prepend-top-0 Your variables (#{@project.variables.size}) - if @project.variables.empty? - %p.profile-settings-message.text-center.append-bottom-0 + %p.settings-message.text-center.append-bottom-0 No variables found, add one with the form above. - else = render "table"