Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move "activeTheme" from config to a setting #107

Closed
ErisDS opened this issue Jun 1, 2013 · 10 comments
Closed

Move "activeTheme" from config to a setting #107

ErisDS opened this issue Jun 1, 2013 · 10 comments
Assignees
Milestone

Comments

@ErisDS
Copy link
Member

ErisDS commented Jun 1, 2013

There isn't a UI yet - that comes in #488

This issue is just to change where this info is loaded from, from config to settings.

This will then require a bit of jiggery-pokery with the order of loading stuff in ghost.init. I think it will need to go dataprovider > settings (& permissions?) > paths & plugins, and also some jiggery pokery with the config in index so that initTheme (which requires the activeTheme setting, and therefore must now happen after ghost.init) happens in the right place.

@ErisDS
Copy link
Member Author

ErisDS commented Jun 17, 2013

We shouldn't do this until we know we're going to have an alternate way to activate a theme other than edit the config file.

@ErisDS ErisDS mentioned this issue Jul 14, 2013
6 tasks
@ErisDS ErisDS mentioned this issue Aug 21, 2013
@ErisDS
Copy link
Member Author

ErisDS commented Aug 22, 2013

This would be a good one to pick up so that we can get #488 done 🐨

@sebgie
Copy link
Contributor

sebgie commented Aug 23, 2013

I would like to take this next? Is this okay @ErisDS?

@ghost ghost assigned sebgie Aug 23, 2013
@JohnONolan
Copy link
Member

Assigned!

sebgie added a commit to sebgie/Ghost that referenced this issue Aug 26, 2013
closes TryGhost#107
- removed "activeTheme" from config.js
- read "activeTheme" from db setting
sebgie added a commit to sebgie/Ghost that referenced this issue Aug 26, 2013
closes TryGhost#107
- removed "activeTheme" from config.js
- read "activeTheme" from db setting
@sebgie
Copy link
Contributor

sebgie commented Aug 26, 2013

The correct activeTheme setting is not available as long as ghost.init() is not executed. Maybe this could be addressed in #360?

Currently the setting in the db contains the path to the themes directory ('content/themes/casper'). Could be changed to be just the theme name ('casper') but would break all existing installations.

First PR was before first coffee, sorry for that :-/.

@ErisDS
Copy link
Member Author

ErisDS commented Aug 26, 2013

Yep, the issue states that there are a few changes that need to be made to the loading order. This is not a straight forward issue and may require quite a lot of refactoring.

@ErisDS
Copy link
Member Author

ErisDS commented Aug 27, 2013

Is there any update on this?

@sebgie
Copy link
Contributor

sebgie commented Aug 27, 2013

No there is no update to that. The PR should work as described and simply take the activeTheme from settings.
Changing of the setting from theme path to theme name could be done with #488.

@ErisDS
Copy link
Member Author

ErisDS commented Aug 27, 2013

I misunderstood what you had written as saying that it didn't work. However, thinking about it I probably shouldn't have made this and #488 separate issues as I cannot merge #107 on its own. A lot of people, including John, are relying on the ability to switch theme for theme development - this is my fault sorry.

@sebgie
Copy link
Contributor

sebgie commented Aug 27, 2013

Sorry for not being clear. You can merge the two issues and close the PR if you want to?

Difference for theme developers would be that they have to change the path to the theme in the database and not in config.js. Restart of Ghost is still necessary.

sebgie added a commit to sebgie/Ghost that referenced this issue Aug 30, 2013
closes TryGhost#488 and TryGhost#107
- added dropdown for theme selection on general page
- added GET /api/v0.1/themes to retrieve available themes
- modified settings model to get available themes
- modified updateSettignsCache to remove path from settings.activeTheme
sebgie added a commit to sebgie/Ghost that referenced this issue Aug 30, 2013
closes TryGhost#488 and TryGhost#107
- added dropdown for theme selection on general page
- added GET /api/v0.1/themes to retrieve available themes
- modified settings model to get available themes
- modified updateSettignsCache to remove path from settings.activeTheme
sebgie added a commit to sebgie/Ghost that referenced this issue Aug 30, 2013
closes TryGhost#488 and TryGhost#107
- added dropdown for theme selection on general page
- added GET /api/v0.1/themes to retrieve available themes
- modified settings model to get available themes
- modified updateSettignsCache to remove path from settings.activeTheme
@ErisDS ErisDS closed this as completed Sep 4, 2013
daniellockyer pushed a commit that referenced this issue Jul 20, 2022
refs ##11434

- Added method to allow updating single subscription. Only `cancel_at_period_end` field can be updated. 
- Middleware is needed to allow Ghost Core to cancel/uncancel member's subscription. 
- Relies on the request containing identity information to be able to verify if subscription belongs to the user
- When member could not be identified by the identity information present in the request we should throw instead of continuing processing
- Handling and messaging inspired by https://github.com/TryGhost/Ghost/blob/3.1.1/core/server/services/mega/mega.js#L132
- When the user initiates subscription cancellation we can safely mark the subscription as canceled so that it's not shown in the interface on subsequent request. Otherwise, we end up in a situation where we still return the subscription in the period until Stripe triggers the webhook.
- Added boolean coercion for cancel_at_period_end parameter. If anything but boolean is passed to Stripe API it throws an error.  Coercing the value on our side is a gives a better dev experience
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants