refs https://github.com/TryGhost/Team/issues/586
A discussion in the Members team resulted in us determining that we do
not need to enforce unique names for Products. Stripe does not enforce
uniqueness for their Products, and we feel it's not necessary for us to.
refs 37ebe723c6
- `package-json` was a standalone library using dependency injection so
we could pull it out into its own package in Utils
- this was done in the commit referenced above
- this commit removes the implementation and tests in Ghost and replaces
the require in the initialization wrapper with the new package
refs https://github.com/TryGhost/Team/issues/581
refs https://github.com/TryGhost/Team/issues/582
When publishing a post via the API it was possible to send it using `?email_recipient_filter=all/free/paid` which allowed you to send to members only based on their payment status which is quite limiting for some sites.
This PR updates the `?email_recipient_filter` query param to support Ghost's `?filter` param syntax which enables more specific recipient lists, eg:
`?email_recipient_filter=status:free` = free members only
`?email_recipient_filter=status:paid` = paid members only
`?email_recipient_filter=label:vip` = members that have the `vip` label attached
`?email_recipient_filter=status:paid,label:vip` = paid members and members that have the `vip` label attached
The older `free/paid` values are still supported by the API for backwards compatibility.
- updates `Post` and `Email` models to transform legacy `free` and `paid` values to their NQL equivalents on read/write
- lets us not worry about supporting legacy values elsewhere in the code
- cleanup migration to transform all rows slated for 5.0
- removes schema and API `isIn` validations for recipient filters so allow free-form filters
- updates posts API input serializers to transform `free` and `paid` values in the `?email_recipient_filter` param to their NQL equivalents for backwards compatibility
- updates Post API controllers `edit` methods to run a query using the supplied filter to verify that it's valid
- updates `mega` service to use the filter directly when selecting recipients
refs https://github.com/TryGhost/Team/issues/581
Editors are allowed to restrict post visibility and send emails to particular member segments, they need to be able to read labels so that they can select them in a member segment.
refs https://github.com/TryGhost/Team/issues/637
refs 75169b705b
With custom prices, Portal now needs to show all available custom prices in the UI as well as product's name and description in the Portal UI. This change adds product information to member site settings for Portal UI.
refs https://github.com/TryGhost/Team/issues/586
refs aa12770329
Using `id` as ghost id for subscription prices can be confusing as everything in the method refers ids to be stripe ids. This change updates the ghost id value to use `price_id` key in the serialization
refs https://github.com/TryGhost/Team/issues/496
We want to give more control over the default selection of email recipients when publishing a post, to do that we need somewhere to store those settings. These settings are site-wide and intended for use by admins to control the default editor behaviour for all staff users. They _do not_ control API behaviour, if you want to send email when publishing via the API it's still necessary to explicitly opt in to that using the `?email_recipients_filter=` query param.
- new `editor` settings group to indicate that these settings only affect the UI rather than the API
- `editor_default_email_recipients` controls overall behaviour, string/enum with these allowed values:
- `'disabled'`: no option to send email is shown in the editor's publishing dropdown
- `'visibility'`: (default) selected member segment is dynamic and matches the post visibility filter
- `'filter'`: specific member filter defined in `editor_default_email_recipients_filter` setting
- `editor_default_email_recipients_filter` is an NQL string for selecting members, used when `editor_default_email_recipients` is set to `'filter'`
- default value is `'all'`
- the segment string can be any valid NQL filter with the additional special-case values of `'all'` and `'none'`
- we need the basePath concept for the main i18n class so we can pull it out into a module
- we already had this in the themeI18n class, so I just had to move it up
- also I added a default of __dirname, so we don't have to declare this constantly in the tests
- Reworking the location of i18n in boot has fixed the main error
- However, many of our tests depend on i18n being loaded but don't explicitly call init
- There are many ways we could fix this in our tests, but I don't want to spend more time on this now
no issue
- `Error` is very generic for this case and `IncorrectUsageError`
will populate the resulting error with the correct error code
- the `message` was pulled out to its own statement so we can avoid long
lines
no issue
- we're preparing the `package-json` lib to be extracted out of Ghost into
its own package so moving the initialization wrapper outside of the
folder makes the process a lot easier
refs https://github.com/TryGhost/Team/issues/586
refs 33f26fbf32
As part of serializing subscriptions with prices, we previously attached only the stripe price id to the price object for subscription. This change updates the price object to include both Ghost id and stripe price id for the object, as Portal needs to check the Ghost price id for logged in members to verify their current plan.
- final preparation for moving i18n out of Ghost core
- logging is passed in via DI
- theme i18n needs a config value, but no need to pass all of config for one parameter, a better pattern is to pass the one value needed
- preparation for moving the base class out of Ghost
- refactored so that all the logic for file loading and fallbacks live in the base class
- theme i18n now only overrides init with the properties it needs, filepath generation and error handling
- this makes it much easier to move the i18n file out, and eventually have theme i18n live elsewhere too
- also prepares for using DI for logging
- calling i18n as a global const like this requires it to be loaded before anything else, when we have to manage this with the init() flow
- wrapping it inside the function where it's used ensures we don't call i18n til we need it
- also improved the i18n called without init error to include the key it was called with
- Note: added a forced error to show that this was previously happening at the wrong time
- i18n is required by ghost-server to log server start messages, and so gets initialised as part of the ghost-server load
- moving this into the right place means we can see how long it takes in the debug logs
- previously the debug log lines for i18n showed 0/1ms, which is not correct as this contains a sync file load operation!
- we should consider if we want to have i18n be a requirement for ghost server, or if we want static messages
- when activating a theme, we need to load the current locale
- this request used to be buried deep in the themeI18n init call
- now we surface it in the bridge and pass it down, which is closer to what we want to do with eventually initialising the frontend
with everything it needs up front (or not initialising it, if it isn't needed)
- in the related helpers we depend on the site.locale value instead of proxy -> themeI18n -> settingsCache drastically simplifying the code and removing deep requires
- site.locale is updated via middleware and can be relied upon
- the core i18n library and theme i18n library have slightly different methods of getting a candidate string
- both of them use forms of jsonpath, meaning they both require jsonpath as a dependency
- to try to get to a point of being able to rip more things out of ghost, we want to have less dependencies
- so instead of overloading the method, we pass in a stringMode as an argument
- eventually we might not need an overloaded class for themeI18n at all, which would simplify the codebase
- preparation for using DI instead of requires, so we can move this out of Ghost
- have done this for both the main i18n and theme i18n file
- refactored the constructor
- We only require a single value from urlUtils, the url for the site
- Move that logic back to the boot file makes it much more explict
- Will help if we want to refactor how urlUtils works, or when we want to move ghost-server out of core
- single authors were deprecated in v1.22 when we added multiple authors
- we always thought we'd clean this up a lot sooner, but it's stuck because it's an annoying thing to break people's shit over
- still saying "remove in vX" isn't useful, we need to know how long a feature has been deprecated so we can judge whether it's safe to remove
refs https://github.com/TryGhost/Team/issues/637
With custom prices, Portal now needs to show all available custom prices in the UI instead of just `monthly` and `yearly` prices. This change adds a list of all custom prices to Portal site settings for the default product which Portal will use to show the available prices in UI.
Note: As part of cleanup, the stripe price ids will be removed from the prices list.
Also:
- Fixes product name in serialised subscriptions
- Adds `type` value in serialised price object
refs https://github.com/TryGhost/Team/issues/586
The `products` and `stripe_prices` tables are missing a description
column which will be used by Portal to display information about the
products and prices
- Preparing to cleanup / change how we use events across Ghost
- Removing this unused bit of additional complexity makes it easier to reason about what we need
refs 829e8ed010
- i18n is used everywhere but only requires shared or external packages, therefore it's a good candidate for living in shared
- this reduces invalid requires across frontend and server, and lets us use it everywhere until we come up with a better option
refs https://github.com/TryGhost/Team/issues/579
The previous `members_allow_free_signup` -> `members_signup_access` migration made a direct correlation between the toggle `true/false` to `all/invite` under the assumption that behaviour between the two settings would be identical. The assumption was incorrect and the behaviour is changing so `invite` forces invite-only mode, stopping all front-end signup to free or paid plans with the free plan now being disabled via the portal plans setting.
- check existing `members_signup_access` setting and if it's `'invite'` migrate it to `'all'` where signup should still be possible. The "invite-only" mode should only be active if certain conditions are met:
- Stripe is not configured ("allow free member signup" off and no Stripe showed "invite-only" in portal)
- Stripe is configured but no plans are selected in portal (no plans showed "invite-only" in portal)
- when migrating `'invite'` to `'all'`, also remove `'free'` plan from the `portal_plans` setting to avoid previously paid-only sites unexpectedly showing a free plan on signup
refs https://github.com/TryGhost/Team/issues/634
- find earliest backup file created when a 4.3 migration was run, if found use the `members_allow_free_signup` value from there to change `members_signup_access` from `'all'` to `'invite'` if necessary
- Having these as destructured from the same package is hindering refactoring now
- Events should really only ever be used server-side
- i18n should be a shared module for now so it can be used everywhere until we figure out something better
- Having them seperate also allows us to lint them properly
refs https://github.com/TryGhost/Team/issues/588
- This check allows for a on/off switch to be set up on the instance and control limits around sending emails
- An example configuration for such check would look like following in config's hostSettings section, e.g.:
```
"emails": {
"disabled": true,
"error": "Email sending has been temporarily disabled whilst your account is under review."
}
```
refs https://github.com/TryGhost/Team/issues/637
refs https://github.com/TryGhost/Team/issues/591
We need to run migrations which will update the `portal_plans` setting
to use id's rather than names. This migration relies on the
`stripe_prices` table being complete populated. The migration to
populate the `stripe_prices` table was not added as a "normal"
migration because it needs to access the Stripe API over the network.
Any migrations that rely on this are unable to be run in a "normal"
migration as that cannot be sure that the database is in the correct
state.
The `portal_plans` setting migration is therefore run in code, and needs
access to the Settings model in order to modify the database.
closes https://github.com/TryGhost/Team/issues/645
- we did some refactoring in [1] to turn promise chained code into
async/await, but this removed an early `return` from the code
- therefore we'd continue on to further code, which breaks for obscure
reasons that weren't apparent from the error
- this commit adds back a return at the end of the block where we handle
staff API tokens to match the same functionality as before
- this is regression that landed in 4.3.0 and would break staff user
tokens
[1]: b677927322 (diff-bc0bedcac8ec9646d0644c86a91e46f4759bc1b0c2aebac54a2b26ec474c3d15L148-L155)
refs https://github.com/TryGhost/Team/issues/579
Updates minimum Portal version to handle the new `members_signup_access` setting and explicitly handle the `invite` option to make Portal work in invite only mode if selected
refs https://github.com/TryGhost/Team/issues/634
- the migration moving `members_allow_free_signup` to `members_signup_access` was expecting a raw boolean setting value but the actual value is a string so always evaluated as truthy making all sites look like they had "allow free members signup" toggled on when generating the new setting's value
- updated to check for an explicit string value in `up` and set an explicit string value in `down`
refs https://github.com/TryGhost/Team/issues/579
The new signup access setting allows site owner to set the type of access level allowed for a member which Portal needs to handle
refs https://github.com/TryGhost/Team/issues/579
`members_signup_access = 'invite'` now forces invite-only mode so both free and paid setups both use the `'all'` setting. To ensure we're properly allowing/disabling free (self signup) signups in the members API we need to update `allowSelfSignup()` to take additional settings into account.
- `true` when Stripe is not connected. There are no paid plans available in this configuration so free signup is always enabled. To disable free signup on a site with no Stripe setup the members signup access should be set to `invite` or `none`.
- `true` when Stripe is configured and free plan is enabled in portal, without it Members API would not send magic link emails to signup requests
- `false` in all other situations such as invite-only and members-disabled signup access modes, or when the free plan has been disabled in portal configuration
refs https://github.com/TryGhost/Team/issues/634
- the migration moving `members_allow_free_signup` to `members_signup_access` was expecting a raw boolean setting value but the actual value is a string so always evaluated as truthy making all sites look like they had "allow free members signup" toggled on when generating the new setting's value
- updated to check for an explicit string value in `up` and set an explicit string value in `down`
refs: bf0823c9a2
refs: ae86254972
- continuing the work of splitting up the theme service into logical components
Themes Service
- The serverside theme service now serves just the API and boot
- It loads the theme and passes it to the theme-engine via the bridge
This achieves the bare minimum goal of removing all the cross requires between server and frontend around themes
There is still a lot more to do to achieve an ideal architecture here as laid out in ae86254972
refs: bf0823c9a2
- continuing the work of splitting up the theme service into logical components
- am about to move the theme service to core/server so it should require i18n directly
refs: bf0823c9a2
- continuing the work of splitting up the theme service into logical components
- this file is not part of the theme engine so it should use the bridge not the engine
- am about to move the theme service to core/server so this will make even more sense then
refs: bf0823c9a2
- continuing the work of splitting up the theme service into logical components
Theme activations are a trickier piece of the theme split puzzle because they are called from the API and theme service on boot in different ways.
Activations require a theme to have been validated at different levels. Validations are also tricky - do they belong to the theme engine, or the theme service?
There are then several different flows for activations:
- On Boot
- API "activate" call
- API override on upload or install via setFromZip, which is a method in the storage layer
These calls all have quite different logical flows at the moment, and need to be unified
For now, I've moved the existing "activate" function onto the bridge. This allows the theme service to be split from the frontend, and refactoring can start from there.
I hope to move this so there is less code in the actual bridge very soon, but my goal is not to require any server packages in the frontend part of this
I think ideally:
- all activation code, including validation, should probably be part of the theme engine
- the theme engine should offer 3 methods: getActive() canActivate() and activate()
- the theme service is then only responsible for loading themes in and out of storage, JSON responses for the API, and handing themes to the frontend via the bridge at the appropriate moment
refs https://github.com/TryGhost/Team/issues/616
We need a way to assign Products to Members via a Subscription, and we've
followed the same pattern as the editSubscription method for the Members API
controller, which acts upon Subscriptions as a nested resource.
Subscriptions now are linked to products, and we've included those links by
default in the Member Admin API as we already include subscriptions by
default, and Products are now a core part of the Members feature-set.
- Modules in /shared are supposed to be standalone modules that can be required by the server or frontend
- As the server shouldn't require the frontend, and vice versa, shared modules should require neither
- Otherwise it just becomes a crutch for allowing cross-depenencies, and will create circular dependencies
The Bridge
- The bridge file is not meant to be a crutch sat allowing cross-dependencies, but rather a new component that manages the flow of data
- That data flows from the server/boot process TO the frontend, and should not flow in the other direction
- The management of that flow of data is necessarily hacky at the moment, but over time the architecture here should get clearer and better
- Still, for the time being it will need to handle requiring across components until that architecture matures
- Therefore, it should live in core root, not in core/shared
