🔥✨ No more availableThemes #8085

ErisDS · 2017-03-02T11:29:58Z

I've been waiting to push up this PR for quite a while 😹

This PR completely removes the concept of "availableThemes" from both config & settings.
It replaces it instead with a concept of themes.list. Which has a nice API similar to our shiny new settings cache.
This opens the door for significant further simplification of code in the settings API/models etc.

TODO:
The last commit has a list of areas I'm still not happy with - the API for themes lib is confusing, we want to be able to hide away much more internal logic and make it easier for the themes JSON API to interact with this library.

I also need to improve test coverage around the themes lib, I have deliberately left this as I want the structure of the objects to change.

NOTE: master still has zero support for custom templates and custom error templates 😁 Will fix that after this goes in.

- this gets rid of the only places where settings.availableThemes are used

- this is no longer used anywhere - also get rid of every related call to updateSettingsCache

- Creates a tailor-made in-memory cache for themes inside the theme module - Add methods for getting & setting items on the cache - Move all references to config.availableThemes to use the new cache - This can be abstracted later to support other kinds of caches?

Still TODO: simplifying/clarifying: - what is the structure of the internal list - what is the difference between a package list, and a theme list? - what is the difference between reading a theme and loading it? - how do we update the theme list (add/remove) - how do we refresh the theme list? (hot reload?!) - how do we get from an internal list, to one that is sent as part of the API? - how are we going to handle theme storage: read/write, such that the path is configurable

core/server/api/settings.js

-        res;
-
-    // @TODO: remove availableThemes from settings cache and create an endpoint to fetch themes
-    if (settings.activeTheme && themes) {


core/server/themes/loader.js

 };

 module.exports = {
    init: initThemes,
-    load: loadThemes
+    loadAllThemes: loadAllThemes,


core/server/api/themes.js


                if (!theme) {
                    throw new errors.NotFoundError({message: i18n.t('errors.api.themes.themeDoesNotExist')});
                }

+                themeUtils.list.del(name);


core/server/themes/list.js

+        delete themeListCache[key];
+    },
+
+    init: function init(themes) {


core/server/themes/loader.js

    debug('loading themes', Object.keys(themes));
-    config.set('paths:availableThemes', themes);
-    settingsApi.updateSettingsCache();
+    list.init(themes);


core/server/themes/list.js

+        delete themeListCache[key];
+    },
+
+    init: function init(themes) {


ErisDS · 2017-03-02T16:01:09Z

Done + travis has finally finished 😁

kirrg001

LGTM 👍

ErisDS added 4 commits March 1, 2017 13:10

🎨 Switch themes API to use config.availableThemes

9735b44

- this gets rid of the only places where settings.availableThemes are used

🔥 Get rid of settings.availableThemes

b641e7d

- this is no longer used anywhere - also get rid of every related call to updateSettingsCache

ErisDS assigned kirrg001 Mar 2, 2017

ErisDS requested a review from kirrg001 March 2, 2017 11:32

ErisDS added the themes label Mar 2, 2017