Improve application settings and write tests

20 files changed, 123 insertions, 77 deletions

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index d6e950b0007..39ca0b4feba 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -4,13 +4,13 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   def show
   end
 
-  def edit
-  end
-
   def update
-    @application_setting.update_attributes(application_setting_params)
-
-    redirect_to admin_application_settings_path
+    if @application_setting.update_attributes(application_setting_params)
+      redirect_to admin_application_settings_path,
+        notice: 'Application settings saved successfully'
+    else
+      render :show
+    end
   end
 
   private
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 4b8cae469e3..b83de68c5d2 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -1,6 +1,8 @@
 require 'gon'
 
 class ApplicationController < ActionController::Base
+  include Gitlab::CurrentSettings
+
   before_filter :authenticate_user_from_token!
   before_filter :authenticate_user!
   before_filter :reject_blocked!
@@ -13,7 +15,7 @@ class ApplicationController < ActionController::Base
 
   protect_from_forgery with: :exception
 
-  helper_method :abilities, :can?
+  helper_method :abilities, :can?, :current_application_settings
 
   rescue_from Encoding::CompatibilityError do |exception|
     log_exception(exception)
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 7c15eab4345..981dc2d8023 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -26,8 +26,8 @@ class RegistrationsController < Devise::RegistrationsController
   private
 
   def signup_enabled?
-    unless ApplicationSetting.current.signup_enabled
-      redirect_to new_user_session_path
+    if current_application_settings.signup_enabled?
+      redirect_to(new_user_session_path)
     end
   end
 
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 5ced98152a5..7b6982c5074 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -1,16 +1,16 @@
 class SessionsController < Devise::SessionsController
-
   def new
-    redirect_path = if request.referer.present? && (params['redirect_to_referer'] == 'yes')
-                     referer_uri = URI(request.referer)
-                     if referer_uri.host == Gitlab.config.gitlab.host
-                       referer_uri.path
-                     else
-                       request.fullpath
-                     end
-                   else
-                     request.fullpath
-                   end
+    redirect_path =
+      if request.referer.present? && (params['redirect_to_referer'] == 'yes')
+        referer_uri = URI(request.referer)
+        if referer_uri.host == Gitlab.config.gitlab.host
+          referer_uri.path
+        else
+          request.fullpath
+        end
+      else
+        request.fullpath
+      end
 
     # Prevent a 'you are already signed in' message directly after signing:
     # we should never redirect to '/users/sign_in' after signing in successfully.
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index c339b3597ec..f21b0bd1f50 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -310,12 +310,4 @@ module ApplicationHelper
     request.env['rack.session']['user_return_to'] ==
       '/'
   end
-
-  def signup_enabled?
-    ApplicationSetting.current.signup_enabled
-  end
-
-  def signin_enabled?
-    ApplicationSetting.current.signin_enabled
-  end
 end
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index bb39a3cf4f0..16db33efd33 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -1,2 +1,13 @@
 module ApplicationSettingsHelper
+  def signup_enabled?
+    current_application_settings.signup_enabled
+  end
+
+  def signin_enabled?
+    current_application_settings.signin_enabled
+  end
+
+  def extra_sign_in_text
+    current_application_settings.sign_in_text
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 7dae318e780..6e5ac9b39c8 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -51,14 +51,15 @@ require 'file_size_validator'
 
 class User < ActiveRecord::Base
   include Gitlab::ConfigHelper
-  extend Gitlab::ConfigHelper
   include TokenAuthenticatable
+  extend Gitlab::ConfigHelper
+  extend Gitlab::CurrentSettings
 
   default_value_for :admin, false
   default_value_for :can_create_group, gitlab_config.default_can_create_group
   default_value_for :can_create_team, false
   default_value_for :hide_no_ssh_key, false
-  default_value_for :projects_limit, gitlab_config.default_projects_limit
+  default_value_for :projects_limit, current_application_settings.default_projects_limit
   default_value_for :theme_id, gitlab_config.default_theme
 
   devise :database_authenticatable, :lockable, :async,
diff --git a/app/services/base_service.rb b/app/services/base_service.rb
index 0d46eeaa18f..bb51795df7c 100644
--- a/app/services/base_service.rb
+++ b/app/services/base_service.rb
@@ -1,4 +1,6 @@
 class BaseService
+  include Gitlab::CurrentSettings
+
   attr_accessor :project, :current_user, :params
 
   def initialize(project, user, params = {})
@@ -29,6 +31,10 @@ class BaseService
     SystemHooksService.new
   end
 
+  def current_application_settings
+    ApplicationSetting.current
+  end
+
   private
 
   def error(message)
diff --git a/app/services/gravatar_service.rb b/app/services/gravatar_service.rb
index d8c9436aaa5..4bee0c26a68 100644
--- a/app/services/gravatar_service.rb
+++ b/app/services/gravatar_service.rb
@@ -1,6 +1,8 @@
 class GravatarService
+  include Gitlab::CurrentSettings
+
   def execute(email, size = nil)
-    if ApplicationSetting.current.gravatar_enabled && email.present?
+    if current_application_settings.gravatar_enabled? && email.present?
       size = 40 if size.nil? || size <= 0
 
       sprintf gravatar_url,
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 846d74d433d..5ca9585e9a9 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -5,25 +5,29 @@
         - @application_setting.errors.full_messages.each do |msg|
           %p= msg
 
-  .form-group
-    = f.label :default_projects_limit, class: 'control-label'
-    .col-sm-10
-      = f.number_field :default_projects_limit, class: 'form-control'
-  .form-group
-    = f.label :signup_enabled, class: 'control-label'
-    .col-sm-10
-      = f.check_box :signup_enabled, class: 'checkbox'
-  .form-group
-    = f.label :signin_enabled, class: 'control-label'
-    .col-sm-10
-      = f.check_box :signin_enabled, class: 'checkbox'
-  .form-group
-    = f.label :gravatar_enabled, class: 'control-label'
-    .col-sm-10
-      = f.check_box :gravatar_enabled, class: 'checkbox'
-  .form-group
-    = f.label :sign_in_text, class: 'control-label'
-    .col-sm-10
-      = f.text_area :sign_in_text, class: 'form-control'
+  %fieldset
+    %legend Features
+    .form-group
+      = f.label :signup_enabled, class: 'control-label'
+      .col-sm-10
+        = f.check_box :signup_enabled, class: 'checkbox'
+    .form-group
+      = f.label :signin_enabled, class: 'control-label'
+      .col-sm-10
+        = f.check_box :signin_enabled, class: 'checkbox'
+    .form-group
+      = f.label :gravatar_enabled, class: 'control-label'
+      .col-sm-10
+        = f.check_box :gravatar_enabled, class: 'checkbox'
+  %fieldset
+    %legend Misc
+    .form-group
+      = f.label :default_projects_limit, class: 'control-label'
+      .col-sm-10
+        = f.number_field :default_projects_limit, class: 'form-control'
+    .form-group
+      = f.label :sign_in_text, class: 'control-label'
+      .col-sm-10
+        = f.text_area :sign_in_text, class: 'form-control'
   .form-actions
     = f.submit 'Save', class: 'btn btn-primary'
diff --git a/app/views/admin/application_settings/edit.html.haml b/app/views/admin/application_settings/edit.html.haml
deleted file mode 100644
index 62c0617ca4f..00000000000
--- a/app/views/admin/application_settings/edit.html.haml
+++ /dev/null
@@ -1,5 +0,0 @@
-%h1 Editing application_setting
-
-= render 'form'
-
-= link_to 'Back', admin_application_settings_path
diff --git a/app/views/admin/application_settings/show.html.haml b/app/views/admin/application_settings/show.html.haml
index 1c77886546d..39b66647a5a 100644
--- a/app/views/admin/application_settings/show.html.haml
+++ b/app/views/admin/application_settings/show.html.haml
@@ -1,18 +1,3 @@
-%table.table
-  %tr
-    %td Default projects limit:
-    %td= @application_setting.default_projects_limit
-  %tr
-    %td Signup enabled:
-    %td= @application_setting.signup_enabled
-  %tr
-    %td Signin enabled:
-    %td= @application_setting.signin_enabled
-  %tr
-    %td Gravatar enabled:
-    %td= @application_setting.gravatar_enabled
-  %tr
-    %td Sign in text:
-    %td= @application_setting.sign_in_text
-
-= link_to 'Edit', edit_admin_application_settings_path
+%h3.page-title Application settings
+%hr
+= render 'form'
diff --git a/app/views/layouts/devise.html.haml b/app/views/layouts/devise.html.haml
index 8b3872e535d..857ebd9b8d9 100644
--- a/app/views/layouts/devise.html.haml
+++ b/app/views/layouts/devise.html.haml
@@ -25,8 +25,8 @@
                 Perform code reviews and enhance collaboration with merge requests.
                 Each project can also have an issue tracker and a wiki.
 
-            - if extra_config.has_key?('sign_in_text')
-              = markdown(extra_config.sign_in_text)
+            - if extra_sign_in_text.present?
+              = markdown(extra_sign_in_text)
 
     %hr
     .container
diff --git a/app/views/layouts/nav/_admin.html.haml b/app/views/layouts/nav/_admin.html.haml
index ea503a9cc2e..fdc517617e3 100644
--- a/app/views/layouts/nav/_admin.html.haml
+++ b/app/views/layouts/nav/_admin.html.haml
@@ -40,3 +40,8 @@
       %span
         Background Jobs
 
+  = nav_link(controller: :application_settings) do
+    = link_to admin_application_settings_path do
+      %i.fa.fa-cogs
+      %span
+        Settings
diff --git a/config/routes.rb b/config/routes.rb
index 7760f32dc36..c4df4283cba 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -109,7 +109,7 @@ Gitlab::Application.routes.draw do
       end
     end
 
-    resource :application_settings
+    resource :application_settings, only: [:show, :update]
 
     root to: "dashboard#index"
   end
diff --git a/features/admin/settings.feature b/features/admin/settings.feature
new file mode 100644
index 00000000000..8799c053ea2
--- /dev/null
+++ b/features/admin/settings.feature
@@ -0,0 +1,9 @@
+@admin
+Feature: Admin Settings
+  Background:
+    Given I sign in as an admin
+    And I visit admin settings page
+
+  Scenario: Change application settings
+    When I disable gravatars and save form
+    Then I should be see gravatar disabled
diff --git a/features/steps/admin/settings.rb b/features/steps/admin/settings.rb
new file mode 100644
index 00000000000..e8168e85def
--- /dev/null
+++ b/features/steps/admin/settings.rb
@@ -0,0 +1,16 @@
+class Spinach::Features::AdminSettings < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedPaths
+  include SharedAdmin
+  include Gitlab::CurrentSettings
+
+  step 'I disable gravatars and save form' do
+    uncheck 'Gravatar enabled'
+    click_button 'Save'
+  end
+
+  step 'I should be see gravatar disabled' do
+    current_application_settings.gravatar_enabled.should be_false
+    page.should have_content 'Application settings saved successfully'
+  end
+end
diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb
index e657fceb704..689b297dffc 100644
--- a/features/steps/shared/paths.rb
+++ b/features/steps/shared/paths.rb
@@ -167,6 +167,10 @@ module SharedPaths
     visit admin_teams_path
   end
 
+  step 'I visit admin settings page' do
+    visit admin_application_settings_path
+  end
+
   # ----------------------------------------
   # Generic Project
   # ----------------------------------------
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
new file mode 100644
index 00000000000..3467bb892fc
--- /dev/null
+++ b/lib/gitlab/current_settings.rb
@@ -0,0 +1,7 @@
+module Gitlab
+  module CurrentSettings
+    def current_application_settings
+      ApplicationSetting.current
+    end
+  end
+end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
new file mode 100644
index 00000000000..3a8d52c11c4
--- /dev/null
+++ b/spec/models/application_setting_spec.rb
@@ -0,0 +1,7 @@
+require 'spec_helper'
+
+describe ApplicationSetting, models: true do
+  describe 'should exists on start' do
+    it { ApplicationSetting.count.should_not be_zero }
+  end
+end