refs https://github.com/TryGhost/Team/issues/1104
- bumped `@tryghost/custom-theme-settings-service` so it throws a more appropriate `ValidationError` when setting keys don't exist or a select value is not known
- changed the custom theme settings service to have a `.init()` method which creates an instance of the service under `.api` so that we're able to create the instance at a particular point in the boot process when we know the models have been initialised
- there were problems in tests because the service was being initialised through the require chain before models were initialised through the boot process
- fixed incorrect `camelCase` of resource name in API responses
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Aleksander Chromik <aleksander.chromik@footballco.com>
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Aleksander Chromik <aleksander.chromik@footballco.com>
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Aleksander Chromik <aleksander.chromik@footballco.com>
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Aleksander Chromik <aleksander.chromik@footballco.com>
closes https://github.com/TryGhost/Team/issues/1125
refs 3c822e0457
- Email-only is not considered a general availability feature and can be used without special flags.
- It allows to publish a new post type "email only" that only goes out as an email newletter and is available through an undescoverable URL (does not appear anywhere publicly similarly to preview posts) on the site.
refs https://github.com/TryGhost/Team/issues/1090
This updates the Members & Offers modules to provide support for using
Offers in Stripe Checkout.
Members module now needs a handle to the Offers module repository, and
as such we have had to reorder the services boot order.
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Aleksander Chromik <aleksander.chromik@footballco.com>
refs https://github.com/TryGhost/Team/issues/1083
This adds the Offers service which will allow us to build out an Admin
API to communicate with it. As well as wiring up the redirects to the
site app.
refs: TryGhost#13380
- The i18n package is deprecated. It is being replaced with the tpl package.
- Important Note: invalidFile error message does not have fields for variables, but it is being passed variables.
refs: #13380
The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Aleksander Chromik <aleksander.chromik@footballco.com>
refs: #13380
* The i18n package is deprecated. It is being replaced with the tpl package.
* Replaced i18n.t w/ tpl helper in invites.js
* Replaced i18n.t w/ tpl helper in labels.js
refs #13380
The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Aleksander Chromik <aleksander.chromik@footballco.com>
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
- The changed files are under `core/server/api/v2` and `core/server/api/v3`.
refs: #13380
The i18n package is deprecated. It is being replaced with the tpl package.
The files changed are under `core/server/api/v2` and `core/server/api/v3`.
refs: #13380
The i18n dependency is deprecated, it's being replaced by the tpl one.
The `tags.js` files changed is under the following folders:
- core/server/api/canary/
- core/server/api/v2/
- core/server/api/v3/
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
The files changed is under `core/server/api/v2` and `core/server/api/v3`.
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
Replace the deprecated dependency `i18n` with `tpl` in the following files under
`core/server/api/v2/`:
- authors-public.js
- pages-public.js
- session.js
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Aleksander Chromik <aleksander.chromik@footballco.com>
refs: #13380
- The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Aleksander Chromik <aleksander.chromik@footballco.com>
- The helper registration code is "framework" code and very specific
- At the moment the "theme engine" is full of lots of disparate theme related stuff
- I'm trying to make the frontend framework code clearer and also expand it to make it more useful
- The helper system now also exposes 3 methods allowing you to register a directory, a helper or an alias
- I've updated the codebase to use these both for our core helpers and for "apps"
refs https://github.com/TryGhost/Team/issues/1083
The Offers service is going to need access to the StripeAPIService too,
so we need to move it out of the @tryghost/members-api module and make
it accessible to both.
refs https://linear.app/tryghost/issue/CORE-86/fix-failing-site-instance-when-redirects-file-is-invalid
refs 260a47da83
- Added validation logic to catch redirects files having invalid RegEx expressions when they are introduced into the system (on upload)
- This way the error happening in the refed commit would have not happened as the validator would not have passed it through
- Moved up the "Router" declaration in custom-redirects as it needs to happen before any other bit of logic has a chance to throw
refs: #13380
- This is part of the ongoing push to get rid of the deprecated i18n.t calls
- In this case, it highlights just how little work we've done on API errors - we should have a full list of action messages, but there's just 1 :(
- This is initial ground work to enable us to do a full error audit
- We want to prevent Ghost admin from ever showing any unhandled errors
- Additionally we want to ensure all handled errors are well worded & have context+help
no issue
If Ghost was booted or a theme activated with the `customThemeSettings` flag disabled but with a theme that has custom settings, enabling the flag later on wouldn't show the settings in Admin or make the settings available in the front-end. Similarly, disabling `customThemeSettings` when Ghost had been booted/or theme activated with it enabled meant that settings were still available on the front-end.
- added an event listener for `settings.labs.edited` that fully re-activates a theme so that it's passed through gscan again and the custom theme settings passed back are included/excluded based on the flag value and any required settings sync with the database is performed
refs: #13380
The i18n package is deprecated. It is being replaced with the tpl
package.
* Replaced i18n.t w/ tpl helper in email
* Replaced i18n.t w/ tpl helper in integrations
refs: #13380
- this is to replace i18n.t with tpl because i18n.t is deprecated
- Replaced i18n.t with tpl helper in email-post.js
- Replaced i18n.t with tpl helper in email-preview.js
* Replaced i18n.t w/ tpl helper in users
refs: #13380
The i18n package is deprecated. It is being replaced with the tpl package.
* Replaced i18n.t w/ tpl helper in authentication
refs: #13380
The i18n package is deprecated. It is being replaced with the tpl package.
Co-authored-by: Kenneth Fitzgerald <fitzgeraldkd@gmail.com>
refs https://github.com/TryGhost/Team/issues/1088
- adds schema for new offers table
- adds permission fixtures for new offers table
- adds migrations for new table and permissions
Co-authored-by: Fabien O'Carroll <fabien@allou.is>
refs: https://github.com/TryGhost/Ghost/issues/13380
- this is a refactor we are looking to do everywhere
- this commit is a reference for how to do the refactor: in small minimal pieces
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- When the yaml parser is injected through a DI it's easier to test and later on the redirects service initialization would use same pattern with exactly the same yamlParse funciton
- Next step is getting yaml parser into an outside module
- Also simplified getSettingFilePath method while swapping to an updated yaml parser implementation. Now this method function is exactly like the one used in redirects
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
refs 5715aa2155 (diff-48644be82a9b957e5e627bf7b0f2f73cdb1d63851ffad68c7c178c5886495bb8R52-R57)
- Simplified the yaml parser implementation to take in a single parameter, this move will allove to simplify the logic in the route settings + opens a door to unify handling with redirects yaml parsing!
- We loose the "filename" from the error information but that was a generic "routes.yaml" anyway and would be thrown only when somebody uploaded a routes.yaml file (no real added value).
- The debug statement should be moved to contain related filepath+other info to the calling module instead
- An additional error handler was borrowed from the redirects yaml parsing logic that was introduced in a referenced commit - it still makes sense to keep it for routes.yaml configuration
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- It's a step to making the module follow class+DI pattern before fully extracting it into an external libarary
- Reminder, doing in Ghost repo instead of substituting big chunks all at once to have clear history of how the service evolved prior to the extraction into external lib!
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- Ensure settings had only one method but would benefit from class+DI pattern before extracting it into an outside module.
- The logic is now also less coupled with "routes" and single source/destination paths. It's all configureable instead and might be reused if similar pattern is needed for example with redirect settings defaults.
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
refs 7528ec8c3b
- The way the custom redirects middleware was organized made it extremely hard to unit test it (had to stub the redirects service methods etc). With a new organization it's possible to provide needed redirects configs to the method which makes the actual redirects Router logic testable and the code less coupled with redirects services
- This was meant to be an attempt to extract more of the slow redirects regression tests, which failed. Instead found this weak spot that could be improved and gained:
- shaved 4s of time as two slow regression test cases are now gone
- there's now a base to build upon when getting more coverage for the custom redirects middleware
refs https://github.com/TryGhost/Team/issues/1070
- bumped `@tryghost/custom-theme-settings-service` for access to `.updateSettings()`
- added `PUT /custom_theme_settings` route that delegates to `customThemeSettingsService.updateSettings()` to perform the db and cache updates
- invalidates the cache in Ghost because a theme setting change will mean the front-end output will change
refs https://github.com/TryGhost/Team/issues/1090
This updates the members-api to allow passing an Offer ID when creating
a Stripe Checkout Session. This will be used for the 1-day version of
Offers.
- i18n is an old pattern we are getting rid of in favour of tpl
- after removing i18n from helpers, there wasn't many usages of i18n left in the frontend, this removes whats left!
- this was done on a branch at the same time as Naz's commits removing i18n from the settings-related files
- hence some of these changes are minor amends to add additional messages/change names, rather than just straightup i18n->tpl
- it's a merge of both our refactors :)
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- There's no reason for the boot to block the event by loading route settings sychronously
- The only leftover use of a sync loader might also be refactored in some way to avoid blocking the event loo - for example by caching the value on the service layer.
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- The only allowed route settings name is 'routes.yaml', which removes a need to parameterize the function as the location is permanent anyway
- Simplifying the function in any possible way before extracting the common bits into an external lib
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- Frontend is not meant to know about the underlying source of the "routes" configuration, so any reads/edits/validations are being moved into a backend service. This should also simplify the coupling of the backend with the frontend where the latter will get a JSON blob with all needed configuration during the boot
- Nother problem the "get" method had was hiding an underlying function it was doing - reading the file from the filesystem SYNCRONOUSLY. It might be a thing we need to do during the "web" app initialization, but there's no clear need to do this in a sync fassion during the bootup for example. Also having a more explicit name should help :)
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
refs c1c9bf0866
- Actions logic related to file system operations (like ensuring files exist) should be done on the backend. Now the route settings initialization logic lives on the backend it makes sense to keep the file closer to the source.
- The move is the opposite to the one refed in the commit with a
difference that the file now lives in "route-settings"
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- 'knowSettings' was based on a "configurable" array of settings that might be configured in Ghost. The multitude never happened! The only setting the frontend takes care of is routes.yaml file (redirects is also kind of a setting but is a separate concept for now).
- Having just one type of file to deal with allows to simplify implementation significantly, which helps before a big refactor
refs https://github.com/TryGhost/Team/issues/1070
- bumped `@tryghost/custom-theme-settings-service` to get access to `.listSettings()` method
- added GET `/api/canary/admin/theme_settings/` route behind `'customThemeSettings'` feature flag that uses the custom theme settings service to return settings resources that are a combination of the theme-provided definition and the saved value
refs https://github.com/TryGhost/Team/issues/1070
- added `@tryghost/custom-theme-settings-service` as a dependency
- `core/server/services/custom-theme-settings` creates an instance of the new service passing in the model used for storing the setting keys/values and a cache instance
- requiring `core/shared/services/custom-theme-settings-cache` creates a cache instance, it has no dependencies so can be required anywhere and the first require will initialize the shared instance
- updated the theme activation bridge to trigger the theme settings service to sync the newly activated theme settings and populate the cache
- updated theme validation to pass `labs` through as an option so that we get custom theme settings back as part of the checked theme as that's what is passed to the custom theme settings service
refs https://github.com/TryGhost/Team/issues/1070
- stores values of custom theme settings
- will be merged with full settings data parsed from themes for API output
- will be cached and made available for lookup in themes to avoid db roundtrips
- stores type of custom theme settings so we can coerce values and know if the type has changed when syncing
- records will be synced with themes upon activation
refs https://github.com/TryGhost/Ghost/security/advisories/GHSA-65p7-pjj8-ggmr
This updates the signup/signin flow for members to no longer support the
email address change flow - which had missing authentication. It has
been replaced with a dedicated email change flow, and Portal has been
updated to use it.
refs https://github.com/TryGhost/Team/issues/694
refs https://linear.app/tryghost/issue/CORE-13
- The index file in services/settings was containning logic and started throwing an additional lint warning due to module length.
- The extracted secret settings utils were used in multiple places and were a good candidate to live in it's own small module
refs https://github.com/TryGhost/Team/issues/694
refs https://linear.app/tryghost/issue/CORE-13
- The controller code is not meant to contain complex business logic. Removed complexity in settings.edit method
- Have brought up to sync v3 controller code to the changes that were done in v4. Didn't touch v2 controller as it had slight API differences, so avoided going on another trip into the unknown
- Migrating v3 controller was pretty straigh forward as it's an exact copy of the v4 one (at least for the methods that were extracted)
refs https://github.com/TryGhost/Team/issues/694
refs https://linear.app/tryghost/issue/CORE-13
- The controller code is not meant to contain complex business logic. Removed complexity in settings.edit method
- Have separated the regular editing from "Stripe Data" editing to keep the dependency on the members service still in the controller reducing coupling of the settings BREAD service to the minimum.
- The stripeConnectData passed into `edit` method still feels out of place (maybe it should be passed as an array already that's ready to be merged with the rest of settings, but that was left for another refactor in the future)
refs https://github.com/TryGhost/Team/issues/694
refs https://linear.app/tryghost/issue/CORE-13
- The controller code is not meant to contain complex business logic.
Reduced complexity in the settings.read method
- Broke the usual "xxxService" naming pattern here in favor of "xxxBREADService" pattern that members package has been experimenting with recently (0469707f2e/packages/members-api/lib/services/member-bread.js (L25)). This naming choice was made because we already had a "SettingsService" and it would've become quite convoluted distinguishing the naming or doing renames for the sake of having a new temporary location for read/edit/add methods
- Also duplicated `hideValueIfSecret` method code with an intention to move it fully into the BREAD service once the refactoring is completed
refs https://github.com/TryGhost/Team/issues/1055
We use the models defined in Ghost as our database connection to store
the analytic events. So we must pass this down to the Members module so
that we can store the events
refs https://github.com/TryGhost/Ghost/security/advisories/GHSA-wfrj-qqc2-83cm
refs https://github.com/advisories/GHSA-48ww-j4fc-435p
- a vulnerability in `nodemailer` means that the `sendmail` transport is
vulnerable to command injection for flags passed to the `sendmail`
binary
- updating to the latest version of Nodemailer required creating
`@tryghost/nodemailer`, which is a wrapper around Nodemailer and
several plugins that used to be in the core
- this commit switches to using that package, and fixes up some small
code + test changes
no-issue
We're seeing problems with large sites taking a long time to run this
migration. The aim here is to refactor it so that it is faster to run.
We've achieved that by reducing the number of database queries needed,
firstly by selecting members with a join to their events (rather than
fetching the events on a member-by-member basis) we also batch the
necessary updates to further reduce db query time.
refs https://github.com/TryGhost/Team/issues/1053
This table is going to be completely deleted at some point in the
future. It serves as a persistent datastore for a spike into collection
analytic events for members. We've opted for a generic table, rather
than a table for each event, so that we can push the DB to the limit in
terms of the length of the table, and find out performance issues A$AP
refs https://github.com/TryGhost/Team/issues/1047
Rendering segmented emails uses `cheerio` to parse and re-render the html but this had a side-effect of converting the `$#39;` char code to the more modern `$apos;` code resulting in Outlook not understanding quotes inside inlined CSS and showing the raw char code if it appeared in the email contents.
- extracted our handling of the unsupported char codes from the main email html generation into a function so that it can be re-used when generating segmented html
refs 70627d84a7
refs 44035fd591
refs https://github.com/TryGhost/Team/issues/477
- When v4 Webhook API was changed removing redundant code v3 API code should've been updated as well. Making this change before extracting logic out into a WebhooksService to have clear chain of why the code that doesn't look the same has been substituted
refs https://github.com/TryGhost/Team/issues/694
refs https://linear.app/tryghost/issue/CORE-10/tackle-integrationsjs
- The controller code is not meant to contain complex business logic.
- Added a test case checking 'PUT' endpoint for integrations to ensure
proper 'NotFound' handling. Found that previous implemenation was
buggy - threw a 500 as 'models.Integration.NotFoundError' that was removed
in previous commit didn't catch a needed error.
no issue
It was possible for authenticated/trusted admin users to make GET requests to localhost via the oembed service by crafting a redirect that used 0.0.0.0.
- added the 0.* default route/routing block to the private IP regex used to block requests when we're contacting external sites
- added an additional IP or localhost check in the oembed service when fetching bookmark card data
- as per 5a5a5d162e, the Bookshelf registry plugin is now in core
- we no longer need to explicitly load the plugin, and it displays a
warning if you do
- this change also turns `._models` into `.registry.models`, so our code has
been updated to reflect that
- as per https://github.com/bookshelf/bookshelf/wiki/Migrating-from-0.15.1-to-1.0.0#default-to-require-true-on-modelfetch-and-collectionfetchone, models will now default to `{require:true}` during a fetch, which changes how Bookshelf will respond when a models yields no results
- instead of passing a `null` result, it will reject with an error, so we'd need to switch to `.catch`ing everything
- our code is set up to handle all these null results and switching style is not currently on the cards so we want to use the existing behaviour for now
- to enable this, the `requireFetch` option needs to be added to the model definitions
refs https://github.com/TryGhost/Team/issues/1030
The usage of `setComplimentarySubscription` is for pre-Tiers enabled
sites only. We didn't see this issue before because the `comped` flag
was incorrectly being set to `false` by default. Since it was fixed in
https://github.com/TryGhost/Ghost/commit/ae844db60 the `comped` flag was
then getting sent up, and creating the subscription.
We've moved the usage of `setComplimentarySubscription` to behind the
feature flag so that we do not use old behaviour when Tiers are enabled
refs https://github.com/TryGhost/Ghost/pull/13276
- when removing the labs flag a conditional in the email processor checking for the labs flag being enabled was replaced with a check for a member segment being present. This meant that email batches with `member_segment: null` representing all members that didn't have content specifically aimed at them were not having the segmented content stripped before sending
refs https://github.com/TryGhost/Team/issues/1006
Moving the logic of disconnecting Stripe into the members-api module
decouples the Ghost API from the Members API internals. This method can
now be updated independently of Ghost, to implement the deletion of
webhooks from Stripe.
refs https://github.com/TryGhost/Team/issues/1000
Some free members were created with a status of 'comped', this resulted
in MemberStatusEvents being created with a `to_status` of 'comped'.
In 4.12 we fixed the status for all free members, but we did not update
the associated member_status_event.
refs https://github.com/TryGhost/Team/issues/995
Since we reintroduced the comped status, we did not update the
subscription handling to correctly set members to a status of comped
when they were on a 'Complimentary' plan. This meant that 'comped' members
had a status of 'paid'. The changes to @tryghost/members-api ensure that
handling subscriptions going forward will not result in this error.
Since we handle the Complimentary plan correctly now, we do not need to
manually check for the existence of one, we can instead rely on the
status to set the `comped` flag.
* Migrated members comped status to reflect subscriptions
refs https://github.com/TryGhost/Team/issues/995
Due to a bug in subscription handling, members with Complimentary stripe
subscriptions were incorrectly given a status of 'paid'.
The goal of this migration is to fix existing broken members, and it
will be accompanied by a fix which prevents the bug for future members.
Since we are updating the status properties for members, we must ensure
that we also update the relevant member_status_events entries too, so
that we do not have incompatible sums between events and statuses.
For example, if we were to use events to graph comped members over time,
we would want the current count to match the query on statuses:
`SELECT COUNT(*) FROM members WHERE status='comped';`
refs https://github.com/TryGhost/Ghost/issues/12942
The function signature of this method has changed, and was only updated
in the canary API, this meant that API requests attempting to link a
stripe customer to a member would error for the v3 API.
closes https://github.com/TryGhost/Team/issues/1024
Our importer would set the default value of all posts_meta keys to
`null`. This is an invalid value for the `email_only` key which only
accepts booleans.
Since we are already looping over the schema to create the default
values, we can use the `defaultTo` property in the schema to use the
intended default, and fall back to `null` if it doesn't exist.
We've used the `Reflect.has` function to determine if the `defaultTo`
key exists, as opposed to a truthy check, because it's possible that a
falsy value (e.g. false, in the case of email_only) can be used as the
default.
refs https://github.com/TryGhost/Team/issues/694
- The canary schedules controller was refactored to use newly introduced post-scheduling service in a previous commit. This is a follow up to match v2/v3 controllers as they had identical code to the canary one.
refs https://github.com/TryGhost/Team/issues/694
- The controller code is not meant to contain complex business logic.
- Kept the pattern used in all modules under services/themes. The install module shold be refactored into a class with DI pattern when touched next.
refs https://github.com/TryGhost/Team/issues/694
- Additional try/catch block needed in async/await implementation increased method complexity and broke the complexity linting rule. This is a dirty way to fix the warning. Ideally the implementation should stay with async/await syntax and instead move the custom error handling logic into some different layer. For example we could introduce a separate "stage" in the API framework's "pipeline" where we'd catch and handle in a generic way all of the "unique" types of errors. It would make sense to have a generic handler because this same code happens in labels, member and few more places.
refs baccbb4942
refs https://github.com/TryGhost/Team/issues/694
- The change is here to remove yet another ESLint method complexity error
- The custom error handling complexity was introduced here in a referenced commit without an obvious reason. The specifics of how the "sendTestEmail" method handles errors should not leak out from the method, if there are errors in the response they should be handled internally and the method would uniformly reject with a single error.
refs https://github.com/TryGhost/Team/issues/694
- The code complexity in the email preview's read controller method was breaking the complexity rule in ESLint. To reduce the complexity extracted common parts into mega service
refs https://github.com/TryGhost/Team/issues/694
- async/await has been a standard way to handle async code throughout the codebase. Refactoring it before moving code makes it way easier to reason about similarities between multiple controllers
refs https://github.com/TryGhost/Team/issues/947
- During the work of the UI and moving `email_only` flag to publish menu it created the situation where the publishing of the post was at the same time as adding `email_only` flag, resulted in not picking up teh `sent` status as the `posts_meta` model and record were's available during save.
- Adding the incoming attribute check for email_only flag covers this situation
no-issue
Writing code outside of Ghost which deals with the models is currently
done by passing the models which are needed to the external module,
rather than the instance of ghostBookshelf. This does not give us a way
to create transaction to run queries in. This method is designed as a
simple way to give all models the power to create a transaction for
themselves.
This will be used in @tryghost/members-api for example to ensure that
failures in communication with the Stripe API will rollback the related
inserts in the database.
refs https://github.com/TryGhost/Team/issues/873
This includes the update to @tryghost/members-api which includes the new
MemberBREADService which is used to handle the logic for controller
methods outside of the controller.
With it, we've introduced the concept of a dummy subscription for comped
members. This gives API consumers a way to get the created_at date for a
comped members access to a product.
no-issue
The @tryghost/members-api module is being updated to export a BREAD
service which will be used to move the logic from the controller into.
This service is currently designed to returns objects rather than
models, as it has to do manipulation of the returned data at the object
level. This update to the serializer will allow a seamless transition to
the use of the BREAD service and allow us to pull out the logic from the
controller sooner!
refs 3f0bab4389
- the internal `request` lib we had was replaced with `@tryghost/request` in
the referenced commit
- this lib was not deleted, so it's still lingering around
- this commit deletes that file to clean it up
no-issue
The bluebird library is unecessary in this module, as all uses of it
were wrapped in `async` functions which will return a native Promise
rather than a bluebird one.
refs https://ghost.slack.com/archives/C02G9E68C/p1629822160273500
refs https://github.com/TryGhost/Team/issues/1007
- `:root` selector wasn't working as expected and ended up matching HRs within content
- switched to wrapping the post html inside a `<body>` element before parsing so that we have a proper top-level element for direct child selectors to match against
refs https://github.com/TryGhost/Team/issues/1007
- the new `email-cta` card allows surrounding dividers to be added when rendering, however if the card is at the beginning or end of the post then these would double-up with the already existing dividers at the beginning and end of the post content in the email template
- not wanting leading/trailing HR's is specific to the email template so it made sense to adjust the renderer output in Ghost's email generating rather than forcing all mobiledoc->html rendering to remove leading/trailing HR's
refs https://github.com/TryGhost/Team/issues/873
The @tryghost/members-api module needs access to this model in order to
create events when members access to products are updated.
refs https://github.com/TryGhost/Team/issues/873
We need a relation between members and their product events so that we
can pull out the events for a particular member to generate the start
date of their comped access to a product.
refs https://github.com/TryGhost/Team/issues/946
This adds the initial bulk actions endpoint used for the members
filtering feature. The idea is to eventually move bulk destroy into this
endpoint to and provide a consistent interface for applying bulk actions
to members.
The @tryghost/members-api package has been bumped to include the new
bulkEdit method.
The sinon.restore in tests was moved to an afterEach so that stubs did
not effect other tests.
refs https://github.com/TryGhost/Team/issues/946
In order to bulk remove relations between members and labels we need a
way to get hold of all of the existing relations between a label and a
set of members.
refs https://github.com/TryGhost/Team/issues/873
This table is to track events related to members be given or having
removed access to products. It will allow us to provide start dates for
access for complimentary members, as well as being able to track access
to products over time, either for individual members or for aggregates.
no issue
- The method was super hard to read with unintuitive catches in multiple places and lots of conditional logic. There's still more to reshuffle here, but that would be for the next time. At least now the data flow is clear within the method
no issue
- Logic with slightly more complex structure belongs to the service. Extracting it there also show's how little of an API the oembed service should actually expose
refs https://github.com/TryGhost/Team/issues/714
In order to order products by their monthly price we need to apply a
join with the stripe_prices table when querying so we have access to the
amount column of stripe_prices.
As this ordering is core to how the tiers feature is intended to work,
we have added it as the default order. But this can be overriden by
manually passing the order option.
Also ensured that we do not create duplicate products in test fixtures
refs https://github.com/TryGhost/Team/issues/990
- Relying on uuid instead of slug makes the posts less discoverable and partially soves discoverability through overriden robots.txt files
refs https://github.com/TryGhost/Team/issues/958
- The module contains a service class and not an api index as index.js file should. This rename also fixes an ESLint warning around index.js file being too complicated.
- The serivice should ideally be extracted into the member repository in the future iteration
refs 9e2b21578a
Since the ref'd commit the labs middleware was moved to the shared labs module
and this require path no longer exists. This does not break anything as any module
still using this would error when reading the labs property
- Unquestionably, at some point we need to rework the API code so that we have less stuff everywhere
- However, the max-lines index.js rule exists as a proxy to find index.js files which are not exposing Public API, but rather contain logic
- These 6 cases are all valid index.js files as the expose the Public API of the module
- Therefore, I've added an override and an override notice explaining.
refs https://github.com/TryGhost/Team/issues/946
This refactor pulls out the core logic so that we can easily add other
bulk operations without having to duplicate even more logic.
It also gives a consistent return value between bulk operations, renaming
`unsuccessfulIds` and `unsuccessfulRecords` to `unsuccessfulData`
We also add a bulkEdit method which will be used to bulk unsubscribe members
from the newsletter.
refs https://github.com/TryGhost/Team/issues/959
Since we had a bug where members with a canceled subscription would have
a status of 'comped' we must fix any existing members in this state.
We update all members which have no products to a status of 'free',
which is the definition of a 'free' member.
refs https://github.com/TryGhost/Team/issues/899
- The internal API is needed to be able to fetch email-only posts through email router. The concept is similar to Preview API with a difference that only posts with `sent` status are accessible and there is content-gating present.
refs https://github.com/TryGhost/Team/issues/953
- We need to track email-only posts that have been sent out. New status was chosen as a way to differenciate such posts.
- Introducing a new "email post" type, conceptually like "page", was considered. Because there is no clear roadmap for "email post" becoming a bigger part of the product yet and a lot of uncertainty around this concept, overhead needed to introduce a new type was just too much to do at this moment. It's still a possibility in the future
no-issue
This moves the logic out of the controller and into the members-api
member repository. Removing complexity from the controllers and
out into services is desirable to reduce code in the Ghost codebase
and move logic into modules which can be tested easier.
closes https://github.com/TryGhost/Team/issues/952
- The `/email/` route will be a home for email only posts. We are adding the route preemptively to have the crowlers update their caches before the feature sees the light of The Internet
refs https://github.com/TryGhost/Team/issues/948
- The frontend route `/email/:uuid` is aliased to the preview as a temporary solution. It fulfills the premise of the email-only post anyway - not being accessible publicly and only shared through email.
- The tests for the new route are missing as adding them was way more problematic than I envisoned. They are in the works and will be added as a follow up commit next.
refs https://github.com/TryGhost/Team/issues/949
- When post is marked as "email-only" we can send it out to the selected audience when publishing without making the post publicly available
- The feature is available for experimentation behind "email only" alpha flag available in labs
refs https://github.com/TryGhost/Team/issues/949
- Initializing PostsService with almost identical parameters is burdensome, having a single factory method in create instances is far more maintainable
refs https://github.com/TryGhost/Team/issues/949
- It's relly hard to grasp what's going on in ifs with multiple conditions that are written down in a signle, gazzilion-line format. Having a nice column as way more readable
refs https://github.com/TryGhost/Team/issues/949
- The post model handling related to newsletter sending and email recipient filter logic were duplicating across v3/v4(canary) APIs and it made sense to extract it into a posts service.
- This will allow for a central place to handle about to land logic for email_only newsletter handling.
refs https://github.com/TryGhost/Team/issues/949
- The code is exactly the same in six (!) places. It's beyond unmaintainable to add another line to any of these place, which will be needed for `email_only` handling.
- The newly created posts service is a temporary, slightly better solution that complies with codebase's best practice of extracting new services using class with DI pattern
refs https://github.com/TryGhost/Team/issues/949
refs e64274bb45
- This refactor is needed to bring the code in line with the rest of pages API controllers
- Next step will extract shared code patterns into a separate module
refs https://github.com/TryGhost/Team/issues/949
refs e64274bb45
- This refactor is needed to bring the code in line with the rest of pages API controllers
- Next step will extract shared code patterns into a separate module
refs https://github.com/TryGhost/Team/issues/949
- This refactor is needed to bring the code in line with the rest of post API controllers
- Next step will extract shared code patterns into a separate module
https://github.com/TryGhost/Team/issues/893
- The assignment is not that obvious and might be confusing without wider context, which is why it warrants to have a clarifying comment. This became apparent during code review
refs https://github.com/TryGhost/framework/pull/19
The @tryghost/bookshelf-filter plugin no longer bundles hardcoded
relations and expansion definitions, instead leaving it up to the
library consumer to implement.
This PR adds the preexisting relations and expansions to the relevant
models, in order to preserve our existing filtering functionality.
refs https://github.com/TryGhost/Team/issues/560
refs 69b773d112
The endpoint `/members/api/session/` is used by Portal for fetching member session while setting up and redirecting to Stripe Checkout flow. The status code returned by API for logged out member is changed from 4xx Unauthorized to 204 No Content, which is consistent with the status code returned while fetching member data when logged out. This API is made just before initiating the checkout session, and is not noticable in most cases due to redirect to Stripe Checkout and got missed.
refs https://github.com/TryGhost/Team/issues/928
- applied same darkening of accent color in emails as we use in editor when there's insufficient contrast of accent color against a white background
refs https://github.com/TryGhost/Team/issues/928
- duplicated email template so email-cta changes can go into the labs version
- added `accentContrastColor` to template settings for using white/black depending on the accent color
- added `.gh-btn-accent` styles to the email template (email-cta card already uses those for the button)
refs https://github.com/TryGhost/Team/issues/912
- When the improt acceedes the threshold for the first time we need a way to notify configured escalationAddress to verify the instance owner's email address.
closes https://github.com/TryGhost/Team/issues/913
- Having a limit service rule triggered was a temporary hack to get a basic email blocking version working
- As the freeze value is now persisted in the DB it's possible to read and rely on it to throw an error straight from MEGA.
refs https://github.com/TryGhost/Team/issues/912
- Previous logic was a bit misleading because it prevented from reading the real threshold configured with an instance once the verified flag was present in the config.
- The reshuffle made here allows to check the freeze logic based on the threshold and then process the returned result accordingly instead of having hidden logic behind "importThreshold" config value
refs https://github.com/TryGhost/Team/issues/912
- The email freeze state has to be stored somewhere to make it through the instance restart and settings table is the best place for it.
refs https://github.com/TryGhost/Team/issues/912
- We need a place to persist the email freeze state between instance restarts - settings table record is the best place for it
refs https://github.com/TryGhost/Team/issues/927
- the `email-cta` card can be segmented so only free or paid members can see the content, it should be possible for authors to preview what that will look like in either case
refs https://github.com/TryGhost/Team/issues/912
- The membersApi variable can be in uninitialized state. It should be accessed through membersService getter to make sure it's always correctly referenced
refs https://github.com/TryGhost/Team/issues/935
The problem was incorrect operator precedence when multiple statements existed in the filter original filter when we transform it to enforce `subscribed:true` before sending.
- free only - subscribed:true+status:free - no issue
- paid only - subscribed:true+status:-free - no issue
- all - subscribed:true+status:-free,status:free - the ,status:free part is treated as a separate OR statement meaning the subscribed:true is not applied to it and free members that are unsubscribed will receive the email
- extracted the filter transform into a separate function so it can be unit tested
- updated the transform to use `()` for operator precedence, eg: `subscribed:true+(status:-free,status:free)`
- used transform function in `addEmail()` and `getEmailMemberRows()`
- fixed `sent/send` typo in error message
refs d60d348c88
- When the import triggers a background job the meta response should contain no data otherwise the client can mistake it for completed import
refs a7dd7bb64b
- The error was introduced in the refed commit. Object.assign method only works when the first parameter is an object otherwise it fails.
refs https://github.com/TryGhost/Team/issues/912
- Exposing a single method out of the service makes the API surface smaller - more readable.
- Additionlally having a wrapping method in service will be helpful for other triggers that are going to be executed in later iterations
refs https://github.com/TryGhost/Team/issues/907
The theme middleware makes several calls to the content api in order to
populate global theme data for use in templates. By adding this
middleware after the static theme files, we remove redundant calls.
refs https://github.com/TryGhost/Team/issues/916
- The constructor API should have as small of a surface as possible, there's no need to pass around whole ghostMailer instance
refs https://github.com/TryGhost/Team/issues/916
- The constructor API should have as small of a surface as possible, there's no need to pass around whole settingsCache instance
refs https://github.com/TryGhost/Team/issues/916
- The refactor was done follow the DI Constructor pattern with single options Object parameter
- It didn't make sense to have a "config" object inside of options object containing just one property
issue https://github.com/TryGhost/Team/issues/614
- The feature flag was called `oauthLogin` instead of simply `oauth` to avoid clashes in the frontend `feature` service as it is merging the config and labs properties.
refs https://github.com/TryGhost/Team/issues/916
- While investigating members importer related codebase this legacy module was spotted. It's not used anywhere and doesn't serve any particular purpose.
refs https://github.com/TryGhost/Team/issues/664
The new WellKnownController and middleware handles exposing a JSON Web
Key Set for us.
In order to serve the keys on /members/.well-known/jwks.json without a
trailing slash, we must mount the wellKnown middleware before the
frontend.
refs 2f1123d6ca
Usage of the raw Error class has been deprecated in favour of our own
errors, which are more descriptive and have built in HTTP status codes.
This also updates the same errors to use @tryghost/tpl for the error
messages, which is the new pattern we are following in order for us to
deprecate the i18n module.
no issue
- i18n is deprecated in favour of `tpl`
- normalized method syntax so `add` matches the rest of the controller's methods (fixed a complexity warning but was not the primary intention)
refs 2f1123d6ca
refs 6f1a3e1774
- As per refed commits, we are removing deprecated use of `new Error()` in the codebase
- Exposed few internal from commands module methods for easier testing, otherwise it was turning into neverending mocking show
refs https://github.com/TryGhost/Team/issues/696
The userAuth spam prevention logic is reused, but a new piece of
middleware has to be created so that we can use a custom lookup key to
conatin the member email.
We must also add json parsing middleware to the route so that the brute
middleware can read the email.
The express body-parser middleware handles multiple instances on the
same route, so this doesn't cause problems upstream.
https://github.com/expressjs/body-parser/blob/1.19.0/lib/types/json.js#L99-L103
closes https://github.com/TryGhost/Team/issues/743
Unlike tags, a label has a unique constraint on its `name`. So saving a new label on member with the same name as existing label fails with error due to unique constraint error.
- adds id for new label to match existing label if they are the same name, which avoids creating a new label
refs https://github.com/TryGhost/Team/issues/880
The aggregate for `paid_delta` was incorrect as it did not handle the
case where an event went from paid->comped or from comped->paid. This
resulted in an overcount for paid members.
refs https://github.com/TryGhost/Team/issues/860
- Slow unit tests cause longer waiting time to deliver code to main. Before this fix the test was taking a whooping 6s on average
- The main cause of the delay was a downstream's package (got) default retry logic that was taking up a lot of time bypassing the retry logic present in the default scheduler itself
refs https://github.com/TryGhost/Team/issues/756
When running the tests it was possible for this middleware to be
instantiated before the settings cache, resulting in an undefined
'session_secret' setting being passed. This would cause tests to fail.
Tracking this down proved difficult, so the fix was made here, by
instantiating the express-session middleware only once a request needs
to use it, we can be confident that Ghost has completely started.
refs 2f1123d6ca
refs 6f1a3e1774
- As per refed commits, we are removing deprecated use of `new Error()` in the codebase
- This bit cleans up the rest of `new Error()` usage in MEGA service
no issue
- Exposing internal methods out of the module is a non-standard practice. Adding `_` prefix allows to signal that this method is not for general use.
- When mega is refactored into a proper class this method will become exposed anyways
refs 2f1123d6ca
refs 6f1a3e1774
- As per refed commits, we are removing deprecated use of `new Error()` in the codebase
- This bit cleans up `new Error()` usage in MEGA service
refs 2f1123d6ca
refs 6f1a3e1774
- The use of new Error() has been deprecated. Refactoring the migration to use `createIrreversibleMigration` made most sense to have central error handling for migration which are not meant to be reverted.
no issue
- This mehod has an important `tableSpec` parameter which MUST be present when creating a new table migration. Having a description in form of the JSDoc somewhat helps this cause
- Next best improvement would be throwing an error if the parameter wasn't present, but that would require a bigger refactor backporting all usages of `addTable` method
