Add Gitlab::Themes module; remove Gitlab::Theme

app/assets/stylesheets/pages/profiles/preferences.scss

@@ -3,11 +3,12 @@
     margin-right: 20px;
     text-align: center;
 
-    .application-theme-preview {
+    .preview {
+      @include border-radius(4px);
+
       height: 80px;
-      width: 160px;
       margin-bottom: 10px;
-      @include border-radius(4px);
+      width: 160px;
 
       &.ui_blue {
         background: $theme-blue;

app/helpers/application_helper.rb

@@ -139,11 +139,8 @@ module ApplicationHelper
   end
 
   def app_theme
-    Gitlab::Theme.css_class_by_id(current_user.try(:theme_id))
-  end
-
-  def theme_type
-    Gitlab::Theme.type_css_class_by_id(current_user.try(:theme_id))
+    theme = Gitlab::Themes.by_id(current_user.try(:theme_id))
+    theme.css_class
   end
 
   def user_color_scheme_class

app/views/profiles/preferences/show.html.haml

@@ -10,38 +10,11 @@
     .panel-heading
       Application theme
     .panel-body
-      .themes_opts
+      - Gitlab::Themes.each do |theme|
         = label_tag do
-          .prev.default
-          = f.radio_button :theme_id, 1
-          Graphite
-
-        = label_tag do
-          .prev.classic
-          = f.radio_button :theme_id, 2
-          Charcoal
-
-        = label_tag do
-          .prev.modern
-          = f.radio_button :theme_id, 3
-          Green
-
-        = label_tag do
-          .prev.gray
-          = f.radio_button :theme_id, 4
-          Gray
-
-        = label_tag do
-          .prev.violet
-          = f.radio_button :theme_id, 5
-          Violet
-
-        = label_tag do
-          .prev.blue
-          = f.radio_button :theme_id, 6
-          Blue
-      %br
-      .clearfix
+          .preview{class: theme.css_class}
+          = f.radio_button :theme_id, theme.id
+          = theme.name
 
   .panel.panel-default.code-preview-theme
     .panel-heading

app/views/profiles/preferences/update.js.erb

 // Remove body class for any previous theme, re-add current one
-$('body').removeClass('<%= Gitlab::Theme.body_classes %>')
-$('body').addClass('<%= app_theme %> <%= theme_type %>')
+$('body').removeClass('<%= Gitlab::Themes.body_classes %>')
+$('body').addClass('<%= app_theme %>')

config/initializers/1_settings.rb

@@ -103,7 +103,7 @@ Settings['gitlab'] ||= Settingslogic.new({})
 Settings.gitlab['default_projects_limit'] ||= 10
 Settings.gitlab['default_branch_protection'] ||= 2
 Settings.gitlab['default_can_create_group'] = true if Settings.gitlab['default_can_create_group'].nil?
-Settings.gitlab['default_theme'] = Gitlab::Theme::MARS if Settings.gitlab['default_theme'].nil?
+Settings.gitlab['default_theme'] = Gitlab::Themes::APPLICATION_DEFAULT if Settings.gitlab['default_theme'].nil?
 Settings.gitlab['host']       ||= 'localhost'
 Settings.gitlab['ssh_host']   ||= Settings.gitlab.host
 Settings.gitlab['https']        = false if Settings.gitlab['https'].nil?

lib/gitlab/theme.rb

-module Gitlab
-  class Theme
-    BASIC  = 1 unless const_defined?(:BASIC)
-    MARS   = 2 unless const_defined?(:MARS)
-    MODERN = 3 unless const_defined?(:MODERN)
-    GRAY   = 4 unless const_defined?(:GRAY)
-    COLOR  = 5 unless const_defined?(:COLOR)
-    BLUE   = 6 unless const_defined?(:BLUE)
-
-    def self.classes
-      @classes ||= {
-        BASIC  => 'ui_basic',
-        MARS   => 'ui_mars',
-        MODERN => 'ui_modern',
-        GRAY   => 'ui_gray',
-        COLOR  => 'ui_color',
-        BLUE   => 'ui_blue'
-      }
-    end
-
-    def self.css_class_by_id(id)
-      id ||= Gitlab.config.gitlab.default_theme
-      classes[id]
-    end
-
-    def self.types
-      @types ||= {
-        BASIC  => 'light_theme',
-        MARS   => 'dark_theme',
-        MODERN => 'dark_theme',
-        GRAY   => 'dark_theme',
-        COLOR  => 'dark_theme',
-        BLUE   => 'light_theme'
-      }
-    end
-
-    def self.type_css_class_by_id(id)
-      id ||= Gitlab.config.gitlab.default_theme
-      types[id]
-    end
-
-    # Convenience method to get a space-separated String of all the theme
-    # classes that might be applied to the `body` element
-    #
-    # Returns a String
-    def self.body_classes
-      (classes.values + types.values).uniq.join(' ')
-    end
-  end
-end

lib/gitlab/themes.rb

+module Gitlab
+  # Module containing GitLab's application theme definitions and helper methods
+  # for accessing them.
+  module Themes
+    # Theme ID used when no `default_theme` configuration setting is provided.
+    APPLICATION_DEFAULT = 2
+
+    # Struct class representing a single Theme
+    Theme = Struct.new(:id, :name, :css_class)
+
+    # All available Themes
+    THEMES = [
+      Theme.new(1, 'Graphite', 'ui_graphite'),
+      Theme.new(2, 'Charcoal', 'ui_charcoal'),
+      Theme.new(3, 'Green',    'ui_green'),
+      Theme.new(4, 'Gray',     'ui_gray'),
+      Theme.new(5, 'Violet',   'ui_violet'),
+      Theme.new(6, 'Blue',     'ui_blue')
+    ].freeze
+
+    # Convenience method to get a space-separated String of all the theme
+    # classes that might be applied to the `body` element
+    #
+    # Returns a String
+    def self.body_classes
+      THEMES.collect(&:css_class).uniq.join(' ')
+    end
+
+    # Get a Theme by its ID
+    #
+    # If the ID is invalid, returns the default Theme.
+    #
+    # id - Integer ID
+    #
+    # Returns a Theme
+    def self.by_id(id)
+      THEMES.detect { |t| t.id == id } || default
+    end
+
+    # Get the default Theme
+    #
+    # Returns a Theme
+    def self.default
+      by_id(default_id)
+    end
+
+    # Iterate through each Theme
+    #
+    # Yields the Theme object
+    def self.each(&block)
+      THEMES.each(&block)
+    end
+
+    private
+
+    def self.default_id
+      id = Gitlab.config.gitlab.default_theme.to_i
+
+      # Prevent an invalid configuration setting from causing an infinite loop
+      if id < THEMES.first.id || id > THEMES.last.id
+        APPLICATION_DEFAULT
+      else
+        id
+      end
+    end
+  end
+end

spec/features/profiles/preferences_spec.rb

@@ -8,14 +8,14 @@ describe 'Profile > Preferences' do
   end
 
   describe 'User changes their application theme', js: true do
-    let(:default_class) { Gitlab::Theme.css_class_by_id(nil) }
-    let(:theme_5_class) { Gitlab::Theme.css_class_by_id(5) }
+    let(:default) { Gitlab::Themes.default }
+    let(:theme)   { Gitlab::Themes.by_id(5) }
 
     before do
       visit profile_preferences_path
     end
 
-    it 'changes immediately' do
+    it 'reflects the changes immediately' do
       expect(page).to have_selector("body.#{default.css_class}")
 
       choose "user_theme_id_#{theme.id}"

spec/lib/gitlab/themes_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Themes do
+  describe '.body_classes' do
+    it 'returns a space-separated list of class names' do
+      css = described_class.body_classes
+
+      expect(css).to include('ui_graphite')
+      expect(css).to include(' ui_charcoal ')
+      expect(css).to include(' ui_blue')
+    end
+  end
+
+  describe '.by_id' do
+    it 'returns a Theme by its ID' do
+      expect(described_class.by_id(1).name).to eq 'Graphite'
+      expect(described_class.by_id(6).name).to eq 'Blue'
+    end
+  end
+
+  describe '.default' do
+    it 'returns the default application theme' do
+      allow(described_class).to receive(:default_id).and_return(2)
+      expect(described_class.default.id).to eq 2
+    end
+
+    it 'prevents an infinite loop when configuration default is invalid' do
+      default = described_class::APPLICATION_DEFAULT
+      themes  = described_class::THEMES
+
+      config = double(default_theme: 0).as_null_object
+      allow(Gitlab).to receive(:config).and_return(config)
+      expect(described_class.default.id).to eq default
+
+      config = double(default_theme: themes.size + 5).as_null_object
+      allow(Gitlab).to receive(:config).and_return(config)
+      expect(described_class.default.id).to eq default
+    end
+  end
+
+  describe '.each' do
+    it 'passes the block to the THEMES Array' do
+      ids = []
+      described_class.each { |theme| ids << theme.id }
+      expect(ids).not_to be_empty
+
+      # TODO (rspeicher): RSpec 3.x
+      # expect(described_class.each).to yield_with_arg(described_class::Theme)
+    end
+  end
+end

spec/models/user_spec.rb

@@ -329,12 +329,12 @@ describe User do
     end
 
     describe 'with default overrides' do
-      let(:user) { User.new(projects_limit: 123, can_create_group: false, can_create_team: true, theme_id: Gitlab::Theme::BASIC) }
+      let(:user) { User.new(projects_limit: 123, can_create_group: false, can_create_team: true, theme_id: 1) }
 
       it "should apply defaults to user" do
         expect(user.projects_limit).to eq(123)
         expect(user.can_create_group).to be_falsey
-        expect(user.theme_id).to eq(Gitlab::Theme::BASIC)
+        expect(user.theme_id).to eq(1)
       end
     end
   end