✨ Move activation to themes endpoint #8093

ErisDS · 2017-03-02T20:12:14Z

@kevinansfield this is a WIP PR to demo a new endpoint for themes

PUT /themes/:name/activate

Works the same as setting the active theme via settings.

Returns the entire GET /themes/ list

NOTE: this also readds the code needed to ensure that the active property is set on the correct theme for both the browse endpoint and this new endpoint.

If you think this PUT endpoint should return something different, let me know!

browse will now include the correct activated theme again
PUT /theme/:name/activate will activate a theme
~~TODO: tests!~~

requires TryGhost/Ghost#8093 - adds `theme.activate()` method and associated adapter method for activating themes rather than relying on `settings.activeTheme` - minor refactors to the `modals/upload-theme` component to use a full theme model

core/server/api/themes.js

        debug('got result');
        return Promise.resolve({themes: result});
    },

+    activate: function activate(options) {


requires TryGhost/Ghost#8093 - adds `theme.activate()` method and associated adapter method for activating themes rather than relying on `settings.activeTheme` - minor refactors to the `modals/upload-theme` component to use a full theme model

- browse will now include the correct activated theme again - PUT /theme/:name/activate will activate a theme TODO: tests!

- tests now read from a temp directory not content/themes - all tests check errors and responses

ErisDS · 2017-03-07T09:04:26Z

@kevinansfield this PR is ready. I updated the endpoints as we talked about (probably worth double checking against Ghost-Admin#561), then the Ghost-Admin PR is unblocked without requiring any DB fudgery for permissions.

I will finish #8104 to add permissions and more API improvements a bit later, but before the alpha.

The bonus to this change, for me, is that we now have 2 clear ways in which themes are activated:

Active theme is activated on boot
When the /activate endpoint is called

Previously 2. was part of the settings edit endpoint, meaning there was no easy way too hook in.

core/server/api/themes.js

@@ -26,11 +27,24 @@ var debug = require('debug')('ghost:api:themes'),
 themes = {
    browse: function browse() {
        debug('browsing');
-        var result = packageUtils.filterPackages(themeList.getAll());
+        var result = themeList.toAPI(themeList.getAll(), settingsCache.get('activeTheme'));


kevinansfield · 2017-03-07T12:47:18Z

@ErisDS when testing this the /upload endpoint is always returning with active: true even when the upload is not overwriting an existing theme - re-fetching the /themes endpoint confirms that the newly uploaded theme isn't active

ErisDS · 2017-03-07T13:10:44Z

@kevinansfield ah yes, derp, will fix

ErisDS · 2017-03-07T13:45:34Z

Fixed it + tests to catch this if I break it again

requires TryGhost/Ghost#8093 - adds `theme.activate()` method and associated adapter method for activating themes rather than relying on `settings.activeTheme` - minor refactors to the `modals/upload-theme` component to use a full theme model

refs TryGhost#8093 - add permission to activate themes - update tests - also: update tests for invites TODO: change how the active theme setting is updated to reduce extra permissions

refs #8093 ✨ Add activate theme permission - add permission to activate themes - update tests - also: update tests for invites TODO: change how the active theme setting is updated to reduce extra permissions ✨ Move theme validation to gscan - add a new gscan validation method and use it for upload - update activate endpoint to do validation also using gscan - change to using SettingsModel instead of API so that we don't call validation or permissions on the settings API - remove validation from the settings model - remove the old validation function - add new invalid theme message to translations & remove a bunch of theme validation related unused keys 📖 Planned changes 🚨 Tests for theme activation API endpoint 🐛 Don't allow deleting the active theme 🚫 Prevent activeTheme being set via settings API - We want to control how this happens in future. - We still want to store the information in settings, via the model. - We just don't want to be able to change this info via the settings edit endpoint 🐛 ✨ Fix warnings for uploads & add for activations - warnings for uploads were broken in f8b498d - fix the response + adds tests to cover that warnings are correctly returned - add the same response to activations + more tests - activations now return a single theme object - the theme that was activated + any warnings 🎨 Improve how we generate theme API responses - remove the requirement to pass in the active theme! - move this to a specialist function, away from the list 🎨 Do not load gscan on boot

ErisDS assigned kevinansfield Mar 2, 2017

kevinansfield mentioned this pull request Mar 3, 2017

🎨 move theme activation to /themes endpoint TryGhost/Admin#561

Merged

kirrg001 reviewed Mar 3, 2017

View reviewed changes

core/server/api/themes.js

debug('got result');

return Promise.resolve({themes: result});

},

activate: function activate(options) {

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

ErisDS force-pushed the themes-api-activate branch from be75b50 to a7bf0c4 Compare March 6, 2017 18:52

ErisDS added 3 commits March 7, 2017 07:41

✨ Move activation to themes endpoint

8bf55f7

- browse will now include the correct activated theme again - PUT /theme/:name/activate will activate a theme TODO: tests!

🎨 Cleanup themes API a bit

d3f2f1c

🚨 Imprv tests: use tmpdir and check errors

8170d13

- tests now read from a temp directory not content/themes - all tests check errors and responses

ErisDS force-pushed the themes-api-activate branch from a7bf0c4 to 8170d13 Compare March 7, 2017 07:44

ErisDS mentioned this pull request Mar 7, 2017

✨ Themes API activation permissions & validation #8104

Merged

4 tasks

ErisDS changed the title ~~[WIP] ✨ Move activation to themes endpoint~~ ✨ Move activation to themes endpoint Mar 7, 2017

kirrg001 reviewed Mar 7, 2017

View reviewed changes

🐛 Ensure upload reflects active theme

77f57dc

ErisDS added the themes label Mar 7, 2017

kevinansfield merged commit 94d53cf into TryGhost:master Mar 8, 2017

kevinansfield deleted the themes-api-activate branch March 8, 2017 10:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Move activation to themes endpoint #8093

✨ Move activation to themes endpoint #8093

ErisDS commented Mar 2, 2017 •

edited

Loading

This comment was marked as abuse.

This comment was marked as abuse.

ErisDS commented Mar 7, 2017 •

edited

Loading

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

kevinansfield commented Mar 7, 2017 •

edited

Loading

ErisDS commented Mar 7, 2017

ErisDS commented Mar 7, 2017

✨ Move activation to themes endpoint #8093

✨ Move activation to themes endpoint #8093

Conversation

ErisDS commented Mar 2, 2017 • edited Loading

This comment was marked as abuse.

This comment was marked as abuse.

ErisDS commented Mar 7, 2017 • edited Loading

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

kevinansfield commented Mar 7, 2017 • edited Loading

ErisDS commented Mar 7, 2017

ErisDS commented Mar 7, 2017

ErisDS commented Mar 2, 2017 •

edited

Loading

ErisDS commented Mar 7, 2017 •

edited

Loading

kevinansfield commented Mar 7, 2017 •

edited

Loading