From d7fe5cc8f2344efdcb2f5d000e7c3b80eb65bb35 Mon Sep 17 00:00:00 2001
From: Martin Linkhorst <m.linkhorst@gmail.com>
Date: Thu, 20 Jun 2013 22:12:30 +0200
Subject: [PATCH 1/5] move themes code into OpenProject's library

---
 app/helpers/application_helper.rb                      |  2 +-
 app/views/settings/_display.html.erb                   |  2 +-
 lib/{redmine => open_project}/themes.rb                |  8 ++++----
 lib/{redmine => open_project}/themes/default_theme.rb  |  6 +++---
 lib/{redmine => open_project}/themes/theme.rb          |  6 +++---
 lib/{redmine => open_project}/themes/view_helpers.rb   | 10 +++++-----
 lib/redmine.rb                                         |  2 +-
 .../themes/default_theme_spec.rb                       |  2 +-
 .../lib/{redmine => open_project}/themes/theme_spec.rb |  2 +-
 .../themes/view_helpers_spec.rb                        |  2 +-
 spec/lib/{redmine => open_project}/themes_spec.rb      |  2 +-
 test/integration/lib/redmine/themes_test.rb            |  2 +-
 12 files changed, 23 insertions(+), 23 deletions(-)
 rename lib/{redmine => open_project}/themes.rb (84%)
 rename lib/{redmine => open_project}/themes/default_theme.rb (88%)
 rename lib/{redmine => open_project}/themes/theme.rb (95%)
 rename lib/{redmine => open_project}/themes/view_helpers.rb (87%)
 rename spec/lib/{redmine => open_project}/themes/default_theme_spec.rb (99%)
 rename spec/lib/{redmine => open_project}/themes/theme_spec.rb (99%)
 rename spec/lib/{redmine => open_project}/themes/view_helpers_spec.rb (99%)
 rename spec/lib/{redmine => open_project}/themes_spec.rb (99%)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 0680179482..fb223d8fcc 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -482,7 +482,7 @@ module ApplicationHelper
   # Returns the theme, controller name, and action as css classes for the
   # HTML body.
   def body_css_classes
-    theme = Redmine::Themes.theme(Setting.ui_theme)
+    theme = OpenProject::Themes.theme(Setting.ui_theme)
 
     css = ['theme-' + theme.name.to_s]
 
diff --git a/app/views/settings/_display.html.erb b/app/views/settings/_display.html.erb
index dd83748549..33b3265f46 100644
--- a/app/views/settings/_display.html.erb
+++ b/app/views/settings/_display.html.erb
@@ -13,7 +13,7 @@ See doc/COPYRIGHT.rdoc for more details.
 <%= form_tag({:action => 'edit', :tab => 'display'}) do %>
 
 <div class="box tabular settings">
-<p><%= setting_select :ui_theme, Redmine::Themes.collect { |theme| [theme.name, theme.identifier] }, :label => :label_theme %></p>
+<p><%= setting_select :ui_theme, OpenProject::Themes.collect { |theme| [theme.name, theme.identifier] }, :label => :label_theme %></p>
 
 <p id="setting_available_languages"><%= setting_multiselect :available_languages, all_lang_options_for_select(false) %></p>
 
diff --git a/lib/redmine/themes.rb b/lib/open_project/themes.rb
similarity index 84%
rename from lib/redmine/themes.rb
rename to lib/open_project/themes.rb
index fd7f2f848a..1c176e86f1 100644
--- a/lib/redmine/themes.rb
+++ b/lib/open_project/themes.rb
@@ -9,10 +9,10 @@
 #
 # See doc/COPYRIGHT.rdoc for more details.
 #++
-require 'redmine/themes/theme'
-require 'redmine/themes/default_theme' # always load the default theme
+require 'open_project/themes/theme'
+require 'open_project/themes/default_theme' # always load the default theme
 
-module Redmine
+module OpenProject
   module Themes
     class << self
       delegate :new_theme, :themes, :all, to: Theme
@@ -44,6 +44,6 @@ module Redmine
 end
 
 # add view helpers to application
-require 'redmine/themes/view_helpers'
+require 'open_project/themes/view_helpers'
 
 ActiveSupport.run_load_hooks(:themes)
diff --git a/lib/redmine/themes/default_theme.rb b/lib/open_project/themes/default_theme.rb
similarity index 88%
rename from lib/redmine/themes/default_theme.rb
rename to lib/open_project/themes/default_theme.rb
index f790afe126..00b426888e 100644
--- a/lib/redmine/themes/default_theme.rb
+++ b/lib/open_project/themes/default_theme.rb
@@ -10,11 +10,11 @@
 # See doc/COPYRIGHT.rdoc for more details.
 #++
 
-require 'redmine/themes/theme'
+require 'open_project/themes/theme'
 
-module Redmine
+module OpenProject
   module Themes
-    class DefaultTheme < Redmine::Themes::Theme
+    class DefaultTheme < OpenProject::Themes::Theme
       def identifier
         :default
       end
diff --git a/lib/redmine/themes/theme.rb b/lib/open_project/themes/theme.rb
similarity index 95%
rename from lib/redmine/themes/theme.rb
rename to lib/open_project/themes/theme.rb
index 5059d69a9a..2f77c8bc8c 100644
--- a/lib/redmine/themes/theme.rb
+++ b/lib/open_project/themes/theme.rb
@@ -12,7 +12,7 @@
 require 'singleton'
 require 'active_support/descendants_tracker'
 
-module Redmine
+module OpenProject
   module Themes
     class Theme
       class SubclassResponsibility < StandardError
@@ -81,13 +81,13 @@ module Redmine
         delegate :each, to: :themes
       end
 
-      # "Redmine::Themes::AwesomeTheme".demodulize.underscore.dasherize.to_sym => :"awesome-theme"
+      # 'OpenProject::Themes::GoofyTheme' => :'goofy-theme'
       def identifier
         @identifier ||= self.class.to_s.demodulize.underscore.dasherize.to_sym
       end
       attr_writer :identifier
 
-      # "Redmine::Themes::AwesomeTheme".demodulize.titleize => "Awesome Theme"
+      # 'OpenProject::Themes::GoofyTheme' => 'Goofy Theme'
       def name
         @name ||= self.class.to_s.demodulize.titleize
       end
diff --git a/lib/redmine/themes/view_helpers.rb b/lib/open_project/themes/view_helpers.rb
similarity index 87%
rename from lib/redmine/themes/view_helpers.rb
rename to lib/open_project/themes/view_helpers.rb
index bf9184ad48..f4ec2d7dc3 100644
--- a/lib/redmine/themes/view_helpers.rb
+++ b/lib/open_project/themes/view_helpers.rb
@@ -10,16 +10,16 @@
 # See doc/COPYRIGHT.rdoc for more details.
 #++
 
-require 'redmine/themes'
+require 'open_project/themes'
 
-module Redmine
+module OpenProject
   module Themes
     module ViewHelpers
       # returns the theme currently configured by the settings
       # if none is configured or one cannot be found it returns the default theme
-      # which means this helper always returns a Redmine::Themes::Theme subclass
+      # which means this helper always returns a OpenProject::Themes::Theme subclass
       def current_theme
-        Redmine::Themes.current_theme
+        OpenProject::Themes.current_theme
       end
 
       # overrides image_tag defined in ActionView::Helpers::AssetTagHelpers (Rails 4)
@@ -43,5 +43,5 @@ end
 module ApplicationHelper
   # including a module is way better than defining methods directly in the application helper's module
   # it plays nicely with inheritence and it will show up in ApplicationHelper.ancestors list
-  include Redmine::Themes::ViewHelpers
+  include OpenProject::Themes::ViewHelpers
 end
diff --git a/lib/redmine.rb b/lib/redmine.rb
index e72dc35414..effeb0abcc 100644
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -17,7 +17,7 @@ require 'redmine/search'
 require 'redmine/custom_field_format'
 require 'redmine/mime_type'
 require 'redmine/core_ext'
-require 'redmine/themes'
+require 'open_project/themes'
 require 'redmine/hook'
 require 'redmine/plugin'
 require 'redmine/notifiable'
diff --git a/spec/lib/redmine/themes/default_theme_spec.rb b/spec/lib/open_project/themes/default_theme_spec.rb
similarity index 99%
rename from spec/lib/redmine/themes/default_theme_spec.rb
rename to spec/lib/open_project/themes/default_theme_spec.rb
index 15b8bb0867..8ce3d49f32 100644
--- a/spec/lib/redmine/themes/default_theme_spec.rb
+++ b/spec/lib/open_project/themes/default_theme_spec.rb
@@ -11,7 +11,7 @@
 
 require 'spec_helper'
 
-module Redmine
+module OpenProject
   module Themes
     describe DefaultTheme do
       let(:theme) { DefaultTheme.instance }
diff --git a/spec/lib/redmine/themes/theme_spec.rb b/spec/lib/open_project/themes/theme_spec.rb
similarity index 99%
rename from spec/lib/redmine/themes/theme_spec.rb
rename to spec/lib/open_project/themes/theme_spec.rb
index 83c7bc635b..6292077020 100644
--- a/spec/lib/redmine/themes/theme_spec.rb
+++ b/spec/lib/open_project/themes/theme_spec.rb
@@ -11,7 +11,7 @@
 
 require 'spec_helper'
 
-module Redmine
+module OpenProject
   module Themes
     describe Theme do
       before { Theme.clear }
diff --git a/spec/lib/redmine/themes/view_helpers_spec.rb b/spec/lib/open_project/themes/view_helpers_spec.rb
similarity index 99%
rename from spec/lib/redmine/themes/view_helpers_spec.rb
rename to spec/lib/open_project/themes/view_helpers_spec.rb
index 8cead57f3b..c2464dbd60 100644
--- a/spec/lib/redmine/themes/view_helpers_spec.rb
+++ b/spec/lib/open_project/themes/view_helpers_spec.rb
@@ -11,7 +11,7 @@
 
 require 'spec_helper'
 
-module Redmine
+module OpenProject
   module Themes
     describe ViewHelpers do
       let(:helpers) { ApplicationController.helpers }
diff --git a/spec/lib/redmine/themes_spec.rb b/spec/lib/open_project/themes_spec.rb
similarity index 99%
rename from spec/lib/redmine/themes_spec.rb
rename to spec/lib/open_project/themes_spec.rb
index 0a76d9a57d..ae9df4c7ef 100644
--- a/spec/lib/redmine/themes_spec.rb
+++ b/spec/lib/open_project/themes_spec.rb
@@ -11,7 +11,7 @@
 
 require 'spec_helper'
 
-module Redmine
+module OpenProject
   describe Themes do
     before { Themes.clear_themes }
 
diff --git a/test/integration/lib/redmine/themes_test.rb b/test/integration/lib/redmine/themes_test.rb
index 683ef26cc5..09d151eb4e 100644
--- a/test/integration/lib/redmine/themes_test.rb
+++ b/test/integration/lib/redmine/themes_test.rb
@@ -17,7 +17,7 @@ class ThemesTest < ActionDispatch::IntegrationTest
   fixtures :all
 
   def setup
-    @theme = Redmine::Themes.default_theme
+    @theme = OpenProject::Themes.default_theme
     Setting.ui_theme = @theme.identifier
   end
 

From 963bbc92c5ef27e50d4f84ebf324b2dc4493940c Mon Sep 17 00:00:00 2001
From: Martin Linkhorst <m.linkhorst@gmail.com>
Date: Tue, 25 Jun 2013 00:31:42 +0200
Subject: [PATCH 2/5] fix a wrong css class regarding themes

---
 app/helpers/application_helper.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index fb223d8fcc..57b5c2416a 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -484,7 +484,7 @@ module ApplicationHelper
   def body_css_classes
     theme = OpenProject::Themes.theme(Setting.ui_theme)
 
-    css = ['theme-' + theme.name.to_s]
+    css = ['theme-' + theme.identifier.to_s]
 
     if params[:controller] && params[:action]
       css << 'controller-' + params[:controller]

From d65add5df0d1ce30552d34fb851430cce99a86b7 Mon Sep 17 00:00:00 2001
From: Martin Linkhorst <m.linkhorst@gmail.com>
Date: Tue, 25 Jun 2013 00:38:36 +0200
Subject: [PATCH 3/5] refactor themes backend * extract theme finding code from
 the Theme class into a ThemeFinder module * omit a trailing 'Theme' in the
 class name for default identifier and name attributes of themes * remove the
 default? method * do not rely on AS::DescendantsTracker anymore, although it
 was nice

---
 lib/open_project/themes.rb                    |  11 +-
 lib/open_project/themes/default_theme.rb      |  16 --
 lib/open_project/themes/theme.rb              |  74 +++-------
 lib/open_project/themes/theme_finder.rb       |  64 ++++++++
 .../open_project/themes/default_theme_spec.rb |   6 -
 .../open_project/themes/theme_finder_spec.rb  | 137 ++++++++++++++++++
 spec/lib/open_project/themes/theme_spec.rb    | 115 ++++-----------
 7 files changed, 256 insertions(+), 167 deletions(-)
 create mode 100644 lib/open_project/themes/theme_finder.rb
 create mode 100644 spec/lib/open_project/themes/theme_finder_spec.rb

diff --git a/lib/open_project/themes.rb b/lib/open_project/themes.rb
index 1c176e86f1..0cd01e1621 100644
--- a/lib/open_project/themes.rb
+++ b/lib/open_project/themes.rb
@@ -9,16 +9,19 @@
 #
 # See doc/COPYRIGHT.rdoc for more details.
 #++
+
 require 'open_project/themes/theme'
+require 'open_project/themes/theme_finder'
 require 'open_project/themes/default_theme' # always load the default theme
 
 module OpenProject
   module Themes
     class << self
-      delegate :new_theme, :themes, :all, to: Theme
+      delegate :new_theme, to: Theme
+      delegate :all, :themes, :clear_themes, to: ThemeFinder
 
       def theme(identifier)
-        Theme.fetch(identifier) { default_theme }
+        ThemeFinder.fetch(identifier) { default_theme }
       end
 
       def default_theme
@@ -33,10 +36,6 @@ module OpenProject
         Setting.ui_theme.to_s.to_sym.presence
       end
 
-      def clear_themes
-        Theme.clear
-      end
-
       include Enumerable
       delegate :each, to: :themes
     end
diff --git a/lib/open_project/themes/default_theme.rb b/lib/open_project/themes/default_theme.rb
index 00b426888e..583c1d2add 100644
--- a/lib/open_project/themes/default_theme.rb
+++ b/lib/open_project/themes/default_theme.rb
@@ -15,22 +15,10 @@ require 'open_project/themes/theme'
 module OpenProject
   module Themes
     class DefaultTheme < OpenProject::Themes::Theme
-      def identifier
-        :default
-      end
-
-      def name
-        'Default'
-      end
-
       def assets_path
         @assets_path ||= Rails.root.join('app/assets').to_s
       end
 
-      def stylesheet_manifest
-        'default.css'
-      end
-
       def assets_prefix
         ''
       end
@@ -39,10 +27,6 @@ module OpenProject
         []
       end
 
-      def default?
-        true
-      end
-
       def image_overridden?(source)
         false
       end
diff --git a/lib/open_project/themes/theme.rb b/lib/open_project/themes/theme.rb
index 2f77c8bc8c..0bed358258 100644
--- a/lib/open_project/themes/theme.rb
+++ b/lib/open_project/themes/theme.rb
@@ -9,8 +9,9 @@
 #
 # See doc/COPYRIGHT.rdoc for more details.
 #++
+
 require 'singleton'
-require 'active_support/descendants_tracker'
+require 'open_project/themes/theme_finder'
 
 module OpenProject
   module Themes
@@ -19,77 +20,44 @@ module OpenProject
       end
 
       class << self
-        include ActiveSupport::DescendantsTracker
-
-        def inherited(base)
-          super                          # call to ActiveSupport::DescendantsTracker
-          base.send :include, Singleton  # make all theme classes singletons
-          clear_cache                    # clear the themes cache
-
-          # register the theme's stylesheet manifest with rails' asset pipeline
-          # we need to wrap the call to #stylesheet_manifest in a Proc,
-          # because when this code is executed the theme class (base) hasn't had
-          # a chance to override the method yet
-          Rails.application.config.assets.precompile << Proc.new {
-            base.instance.stylesheet_manifest unless base.abstract?
-          }
+        def inherited(subclass)
+          # make all theme classes singletons
+          subclass.send :include, Singleton
+
+          # register the theme with the ThemeFinder
+          ThemeFinder.register_theme(subclass.instance)
         end
 
         def new_theme(identifier = nil)
           theme = Class.new(self).instance
-          theme.identifier = identifier
+          theme.identifier = identifier if identifier
           theme
         end
 
-        def themes
-          @_themes ||= (descendants - abstract_themes).map(&:instance)
-        end
-        alias_method :all, :themes
-
-        def registered_themes
-          @_registered_themes ||= \
-            themes.each_with_object(Hash.new) do |theme, themes|
-              themes[theme.identifier] = theme
-            end
-        end
-        delegate :fetch, to: :registered_themes
-
-        def clear
-          direct_descendants.clear && clear_cache
-        end
-
-        def clear_cache
-          @_themes = @_registered_themes = nil
-        end
-
         def abstract!
-          Theme.abstract_themes << self
+          @abstract = true
+
+          # tell ThemeFinder to forget the theme
+          ThemeFinder.forget_theme(instance)
 
           # undefine methods responsible for creating instances
           singleton_class.send :remove_method, *[:new, :allocate, :instance]
         end
 
         def abstract?
-          self.in?(Theme.abstract_themes)
-        end
-
-        def abstract_themes
-          @_abstract_themes ||= Array.new
+          @abstract
         end
-
-        include Enumerable
-        delegate :each, to: :themes
       end
 
-      # 'OpenProject::Themes::GoofyTheme' => :'goofy-theme'
+      # 'OpenProject::Themes::GoofyTheme' => :'goofy'
       def identifier
-        @identifier ||= self.class.to_s.demodulize.underscore.dasherize.to_sym
+        @identifier ||= self.class.to_s.gsub(/Theme$/, '').demodulize.underscore.dasherize.to_sym
       end
       attr_writer :identifier
 
-      # 'OpenProject::Themes::GoofyTheme' => 'Goofy Theme'
+      # 'OpenProject::Themes::GoofyTheme' => 'Goofy'
       def name
-        @name ||= self.class.to_s.demodulize.titleize
+        @name ||= self.class.to_s.gsub(/Theme$/, '').demodulize.titleize
       end
 
       def stylesheet_manifest
@@ -134,12 +102,8 @@ module OpenProject
         end
       end
 
-      def default?
-        false
-      end
-
       include Comparable
-      delegate :'<=>', to: :'self.class'
+      delegate :'<=>', :abstract?, to: :'self.class'
 
       include Singleton
       abstract!
diff --git a/lib/open_project/themes/theme_finder.rb b/lib/open_project/themes/theme_finder.rb
new file mode 100644
index 0000000000..c81de5b77b
--- /dev/null
+++ b/lib/open_project/themes/theme_finder.rb
@@ -0,0 +1,64 @@
+#-- encoding: UTF-8
+#-- copyright
+# OpenProject is a project management system.
+#
+# Copyright (C) 2012-2013 the OpenProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+module OpenProject
+  module Themes
+    module ThemeFinder
+      class << self
+        def themes
+          @_themes ||= []
+        end
+        alias_method :all, :themes
+
+        def registered_themes
+          @_registered_themes ||= \
+            themes.each_with_object({}) do |theme, themes|
+              themes[theme.identifier] = theme
+            end
+        end
+        delegate :fetch, to: :registered_themes
+
+        def register_theme(theme)
+          self.themes << theme
+          clear_cache
+
+          # register the theme's stylesheet manifest with rails' asset pipeline
+          # we need to wrap the call to #stylesheet_manifest in a Proc,
+          # because when this code is executed the theme instance (theme) hasn't had
+          # a chance to override the method yet
+          Rails.application.config.assets.precompile << Proc.new {
+            theme.stylesheet_manifest unless theme.abstract?
+          }
+        end
+
+        # TODO: remove stylesheet_manifest from Rails' precompile list
+        def forget_theme(theme)
+          themes.delete(theme)
+          clear_cache
+        end
+
+        # TODO: remove stylesheet_manifest from Rails' precompile list
+        def clear_themes
+          themes.clear
+          clear_cache
+        end
+
+        def clear_cache
+          @_registered_themes = nil
+        end
+
+        include Enumerable
+        delegate :each, to: :themes
+      end
+    end
+  end
+end
diff --git a/spec/lib/open_project/themes/default_theme_spec.rb b/spec/lib/open_project/themes/default_theme_spec.rb
index 8ce3d49f32..e7c05a30e6 100644
--- a/spec/lib/open_project/themes/default_theme_spec.rb
+++ b/spec/lib/open_project/themes/default_theme_spec.rb
@@ -81,12 +81,6 @@ module OpenProject
           expect(theme.image_overridden?('theme_spec.rb')).to be_false
         end
       end
-
-      describe '#default?' do
-        it "returns true" do
-          expect(DefaultTheme.instance).to be_default
-        end
-      end
     end
 
     describe ViewHelpers do
diff --git a/spec/lib/open_project/themes/theme_finder_spec.rb b/spec/lib/open_project/themes/theme_finder_spec.rb
new file mode 100644
index 0000000000..15ca6802ca
--- /dev/null
+++ b/spec/lib/open_project/themes/theme_finder_spec.rb
@@ -0,0 +1,137 @@
+#-- copyright
+# OpenProject is a project management system.
+#
+# Copyright (C) 2012-2013 the OpenProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+module OpenProject
+  module Themes
+    describe ThemeFinder do
+      before { ThemeFinder.clear_themes }
+
+      describe '.themes' do
+        it "returns all instances of descendants of themes" do
+          theme = Theme.new_theme
+          expect(ThemeFinder.themes).to include theme
+        end
+
+        # the before filter above removes the default theme as well. to test
+        # the correct behaviour we just spec that the default theme class
+        # was loaded (by looking through all subclasses of BasicObject)
+        it "always includes the default theme" do
+          loaded_classes = Object.descendants
+          expect(loaded_classes).to include Themes::DefaultTheme
+        end
+
+        # test through the theme instances classes because
+        # an abstract theme can't have an instance
+        it "filters out themes marked as abstract" do
+          theme_class = Class.new(Theme) { abstract! }
+          theme_classes = ThemeFinder.themes.map(&:class)
+          expect(theme_classes).to_not include theme_class
+        end
+
+        it "subclasses of abstract themes aren't abstract by default" do
+          abstract_theme_class = Class.new(Theme) { abstract! }
+          theme = Class.new(abstract_theme_class).instance
+          expect(ThemeFinder.themes).to include theme
+        end
+      end
+
+      describe '.registered_themes' do
+        it "returns a hash of themes with their identifiers as keys" do
+          theme = Theme.new_theme(:new_theme)
+          expect(ThemeFinder.registered_themes).to include :new_theme => theme
+        end
+      end
+
+      describe '.register_theme' do
+        it "remembers whatever is passed in (this is called by #inherited hook)" do
+          theme = stub # do not invoke inherited callback
+          ThemeFinder.register_theme(theme)
+          expect(ThemeFinder.themes).to include theme
+        end
+
+        # TODO: clean me up
+        it "registers the theme's stylesheet manifest for precompilation" do
+          Class.new(Theme) { def stylesheet_manifest; 'stylesheet_path.css'; end }
+
+          # TODO: gives an error on the whole list
+          # TODO: remove themes from the list, when clear_themes is called
+          precompile_list = Rails.application.config.assets.precompile
+          precompile_list = Array(precompile_list.last)
+          precompile_list.map! { |element| element.respond_to?(:call) ? element.call : element }
+
+          expect(precompile_list).to include 'stylesheet_path.css'
+        end
+
+        it "clears the cache successfully" do
+          ThemeFinder.registered_themes # fill the cache
+          theme = Theme.new_theme(:new_theme)
+          expect(ThemeFinder.registered_themes).to include :new_theme => theme
+        end
+      end
+
+      describe '.forget_theme' do
+        it "removes the theme from the themes list" do
+          theme = Theme.new_theme(:new_theme)
+          ThemeFinder.forget_theme(theme)
+          expect(ThemeFinder.themes).to_not include theme
+        end
+      end
+
+      describe '.clear_cache' do
+        it "removes the theme from the registered themes list and clears the cache" do
+          theme = Theme.new_theme(:new_theme)
+          ThemeFinder.registered_themes # fill the cache
+          ThemeFinder.forget_theme(theme)
+          expect(ThemeFinder.registered_themes).to_not include :new_theme => theme
+        end
+      end
+
+      describe '.abstract!' do
+        it "abstract themes won't show up in the themes llist" do
+          abstract_theme_class = Class.new(Theme) { abstract! }
+          theme_classes = ThemeFinder.themes.map(&:class)
+          expect(theme_classes).to_not include abstract_theme_class
+        end
+
+        it "the basic theme class is abstract" do
+          theme_classes = ThemeFinder.themes.map(&:class)
+          expect(theme_classes).to_not include Theme
+        end
+      end
+
+      describe '.clear_themes' do
+        it "it wipes out all registered themes" do
+          theme_class = Class.new(Theme)
+          ThemeFinder.clear_themes
+          expect(ThemeFinder.themes).to be_empty
+        end
+
+        it "clears the registered themes cache" do
+          theme = Theme.new_theme(:new_theme)
+          ThemeFinder.registered_themes # fill the cache
+          ThemeFinder.clear_themes
+          expect(ThemeFinder.registered_themes).to_not include :new_theme => theme
+        end
+      end
+
+      describe '.each' do
+        it "iterates over all themes" do
+          Theme.new_theme(:new_theme)
+          themes = []
+          ThemeFinder.each { |theme| themes << theme.identifier }
+          expect(themes).to eq [:new_theme]
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/open_project/themes/theme_spec.rb b/spec/lib/open_project/themes/theme_spec.rb
index 6292077020..08f03228ce 100644
--- a/spec/lib/open_project/themes/theme_spec.rb
+++ b/spec/lib/open_project/themes/theme_spec.rb
@@ -13,8 +13,12 @@ require 'spec_helper'
 
 module OpenProject
   module Themes
+    GoofyTheme = Class.new(Theme)
+
     describe Theme do
-      before { Theme.clear }
+      before { ThemeFinder.clear_themes }
+
+      # class methods
 
       describe '.new_theme' do
         it "returns a new theme" do
@@ -28,52 +32,7 @@ module OpenProject
         end
       end
 
-      describe '.themes' do
-        it "returns all instances of descendants of themes" do
-          theme = Theme.new_theme
-          expect(Theme.themes).to include theme
-        end
-
-        # the before filter above removes the default theme as well. to test
-        # the correct behaviour we just spec that the default theme class
-        # was loaded (by looking through all subclasses of BasicObject)
-        it "always includes the default theme" do
-          loaded_classes = Object.descendants
-          expect(loaded_classes).to include Themes::DefaultTheme
-        end
-
-        # test through the theme instances classes because
-        # an abstract theme can't have an instance
-        it "filters out themes marked as abstract" do
-          theme_class = Class.new(Theme) { abstract! }
-          theme_classes = Theme.themes.map(&:class)
-          expect(theme_classes).to_not include theme_class
-        end
-
-        it "subclasses of abstract themes aren't abstract by default" do
-          abstract_theme_class = Class.new(Theme) { abstract! }
-          child_theme_class = Class.new(abstract_theme_class)
-          expect(Theme.themes).to include child_theme_class.instance
-        end
-      end
-      
-      describe '.registered_themes' do
-        it "returns a hash of themes if their identifiers as keys" do
-          theme = Theme.new_theme(:new_theme)
-          expect(Theme.registered_themes).to include :new_theme => theme
-        end
-      end
-
       describe '.abstract!' do
-        it "marks the theme class as abstract" do
-          theme_class = Class.new(Theme) { abstract! }
-          expect(Theme.abstract_themes).to include theme_class
-        end
-
-        it "the basic theme class is abstract" do
-          expect(Theme.abstract_themes).to include Theme
-        end
-
         it "abstract themes have no instance" do
           theme_class = Class.new(Theme) { abstract! }
           expect { theme_class.instance }.to raise_error NoMethodError
@@ -89,26 +48,7 @@ module OpenProject
         end
       end
 
-      describe '.descendants' do
-        it "it rememberes all classes that descend from Theme" do
-          theme_class = Class.new(Theme)
-          expect(Theme.descendants).to include theme_class
-        end
-
-        it "it works on multiple levels" do
-          theme_class = Class.new(Class.new(Theme))
-          expect(Theme.descendants).to include theme_class
-        end
-      end
-
-      describe '.clear' do
-        it "it wipes out all remembered descendants" do
-          theme_class = Class.new(Theme)
-          Theme.clear
-          expect(Theme.descendants).to be_empty
-        end
-      end
-
+      # duplicates singleton code, just to make sure
       describe '.instance' do
         it "is an instance of the class" do
           theme_class = Class.new(Theme)
@@ -122,22 +62,14 @@ module OpenProject
       end
 
       describe '.inherited' do
-        it "it is aware of the new theme (clears the cache when subclassing)" do
-          Theme.themes
+        it "is aware of the new theme after inheriting" do
           theme = Theme.new_theme
-          expect(Theme.themes).to include theme
+          expect(ThemeFinder.themes).to include theme
         end
       end
 
-      describe '.each' do
-        it "iterates over all themes" do
-          Theme.new_theme(:new_theme)
-          themes = []
-          Theme.each { |theme| themes << theme.identifier }
-          expect(themes).to eq [:new_theme]
-        end
-      end
-      
+      # instance methods
+
       describe '#assets_path' do
         it "should raise exception telling it is sublass responsibility" do
           theme = Theme.new_theme(:new_theme)
@@ -145,6 +77,27 @@ module OpenProject
         end
       end
 
+      describe '#identifier' do
+        it 'symbolizes the identifier from the class name by default' do
+          theme = GoofyTheme.instance
+          expect(theme.identifier).to eq :goofy
+        end
+      end
+
+      describe '#name' do
+        it 'titlelizes the name from the class name by default' do
+          theme = GoofyTheme.instance
+          expect(theme.name).to eq 'Goofy'
+        end
+      end
+
+      describe '#stylesheet_manifest' do
+        it 'stringifies the identier and appends the css extension' do
+          theme = Theme.new_theme(:goofy)
+          expect(theme.stylesheet_manifest).to eq 'goofy.css'
+        end
+      end
+
       describe '#overridden_images' do
         let(:theme) { Theme.new_theme }
 
@@ -258,12 +211,6 @@ module OpenProject
           expect(Class.new(Theme).instance).to_not eq Class.new(Theme).instance
         end
       end
-
-      describe '#default?' do
-        it "returns false" do
-          expect(Theme.new_theme).to_not be_default
-        end
-      end
     end
 
     describe ViewHelpers do

From 0f6e32cfd799851588d4b8d7ee964309468342e9 Mon Sep 17 00:00:00 2001
From: Martin Linkhorst <m.linkhorst@gmail.com>
Date: Tue, 9 Jul 2013 17:32:09 +0200
Subject: [PATCH 4/5] let's call the default theme 'OpenProject'. alternatively
 one could have changed the class DefaultTheme to OpenProjectTheme and let
 that be the default theme.

---
 lib/open_project/themes/default_theme.rb           | 4 ++++
 spec/lib/open_project/themes/default_theme_spec.rb | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/open_project/themes/default_theme.rb b/lib/open_project/themes/default_theme.rb
index 583c1d2add..bd0ca1184e 100644
--- a/lib/open_project/themes/default_theme.rb
+++ b/lib/open_project/themes/default_theme.rb
@@ -15,6 +15,10 @@ require 'open_project/themes/theme'
 module OpenProject
   module Themes
     class DefaultTheme < OpenProject::Themes::Theme
+      def name
+        'OpenProject'
+      end
+
       def assets_path
         @assets_path ||= Rails.root.join('app/assets').to_s
       end
diff --git a/spec/lib/open_project/themes/default_theme_spec.rb b/spec/lib/open_project/themes/default_theme_spec.rb
index e7c05a30e6..67a23f6a4e 100644
--- a/spec/lib/open_project/themes/default_theme_spec.rb
+++ b/spec/lib/open_project/themes/default_theme_spec.rb
@@ -16,8 +16,14 @@ module OpenProject
     describe DefaultTheme do
       let(:theme) { DefaultTheme.instance }
 
+      describe '#name' do
+        it 'is called OpenProject' do
+          expect(theme.name).to eq 'OpenProject'
+        end
+      end
+
       describe '#stylesheet_manifest' do
-        it 'it is default with a css extension' do
+        it 'is default with a css extension' do
           expect(theme.stylesheet_manifest).to eq 'default.css'
         end
       end

From f99184c5596cf80c6510aba47199bdf031f87e20 Mon Sep 17 00:00:00 2001
From: Michael Frister <m.frister@finn.de>
Date: Thu, 11 Jul 2013 14:38:44 +0200
Subject: [PATCH 5/5] Remove (hopefully) superfluous TODOs

[ci skip]
---
 lib/open_project/themes/theme_finder.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/open_project/themes/theme_finder.rb b/lib/open_project/themes/theme_finder.rb
index c81de5b77b..0332a03702 100644
--- a/lib/open_project/themes/theme_finder.rb
+++ b/lib/open_project/themes/theme_finder.rb
@@ -40,13 +40,11 @@ module OpenProject
           }
         end
 
-        # TODO: remove stylesheet_manifest from Rails' precompile list
         def forget_theme(theme)
           themes.delete(theme)
           clear_cache
         end
 
-        # TODO: remove stylesheet_manifest from Rails' precompile list
         def clear_themes
           themes.clear
           clear_cache