Improve application settings and write tests

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

app/controllers/application_controller.rb

 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)

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
 

app/controllers/sessions_controller.rb

 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.

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

app/helpers/application_settings_helper.rb

 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

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,

app/services/base_service.rb

 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)

app/services/gravatar_service.rb

 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,

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'

app/views/admin/application_settings/edit.html.haml

-%h1 Editing application_setting
-
-= render 'form'
-
-= link_to 'Back', admin_application_settings_path

app/views/admin/application_settings/show.html.haml

-%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'

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

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

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

features/admin/settings.feature

+@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

features/steps/admin/settings.rb

+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

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
   # ----------------------------------------

lib/gitlab/current_settings.rb

+module Gitlab
+  module CurrentSettings
+    def current_application_settings
+      ApplicationSetting.current
+    end
+  end
+end

spec/models/application_setting_spec.rb

+require 'spec_helper'
+
+describe ApplicationSetting, models: true do
+  describe 'should exists on start' do
+    it { ApplicationSetting.count.should_not be_zero }
+  end
+end