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 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
refs https://github.com/TryGhost/Team/issues/841
When using our development tooling Ghost should always start, instead of
exiting with an error. This check for the WEBHOOK_SECRET env var was the
primary cause of Ghost erroring in development, so it's been switched
with a warning.
refs: f9a3f7d955
- The test for overriding a theme (uploading a theme with the same name as the currently active theme) doesn't test the right codepath
- It incorrectly assumes uploading the same theme twice results in an override, but this is only true for the active theme
- This change splits the override test out into it's own test, and only tests overriding by changing the active theme first
- Also fixed a minor comment type whilst here
refs https://github.com/TryGhost/action-deploy-theme/issues/45
- added missing `throw error` in the `setFromZip()` catch which was hiding the underlying error when a theme uploaded and saved successfully but other code had failed
- fixed incorrect method name `activator.activateFromOverride` -> `activator.activateFromAPIOverride`
refs https://github.com/TryGhost/action-deploy-theme/issues/45
- added missing `throw error` in the `setFromZip()` catch which was hiding the underlying error when a theme uploaded and saved successfully but other code had failed
- fixed incorrect method name `activator.activateFromOverride` -> `activator.activateFromAPIOverride`
no issue
- In the current iteration of the gated email project, we are returning a null segment instead of returning the correct list of segmented users as a temporary measure. The expectation was to clear all segmented cards and it's now the case.
- This isn't really a "service" - it's a set of utilities for working with labs flags
- It's also required all over the place, and doesn't require anything that isn't shared
- Therefore, it should live in shared
- Replaced requiring SafeString all the way from the theme engine, with using express-hbs directly
- This is quite a big require, just for the safe string function, but without this we have to tie labs to our theme layer
- Also removed i18n and updated the jsdoc for enabledHelper
- The labs service can be moved to shared now!
- This isn't really a "service" - it's a set of utilities for working with labs flags
- It's also required all over the place, and doesn't require anything that isn't shared
- Therefore, it should live in shared
- Replaced requiring SafeString all the way from the theme engine, with using express-hbs directly
- This is quite a big require, just for the safe string function, but without this we have to tie labs to our theme layer
- Also removed i18n and updated the jsdoc for enabledHelper
- The labs service can be moved to shared now!
- This stops the mounting of the admin and frontend from being buried deep in express initialisation
- Instead it's explicit, which makes two things almost possible:
1. we can potentially boot the frontend or backend independently
2. we can pass services and settings loaded during boot into the frontend
- This needs more work, but we can start to group all the frontend code together
- Meanwhile we also need to rip apart the routing and url services to decouple the frontend from the backend fully
- BABY STEPS!
closes https://github.com/TryGhost/Team/issues/737
- without an explicit `width: auto` on images Gmail on Android will make not make the image responsive, instead it was keeping the 1200px intrinsic width of the image and shrinking other content around it to match
closes https://github.com/TryGhost/Team/issues/819
- adds guard for an empty buffer when reading file from storage for resizing, if a blank image is loaded then redirect to the original file
issue https://github.com/TryGhost/Team/issues/859
- Added invalidation to PUT /authentication/setup
- Added invalidation to POST /db
- Added invalidation to DELETE /db
- Added invalidation to GET /slugs/:type/:name
- Removed invalidation from PUT /users/:id/token
- This is a precursor to trying to split apart into:
- model events + webhooks system which makes sense
- frontend events which should be independent or removed
- maybe some concept of a settings manager that we can use in various places to bind logic 🤔
- other usages of events that should be refactored to not use events
refs https://github.com/TryGhost/Team/issues/856
- The default internal version of the API is expected to be the latest one available which is v4/canary at the moment.
- There will be more information posted in the referenced issue later around how to approach the "default version", for now it's just a change to make a small step into a right direction.
refs https://github.com/TryGhost/Team/issues/856
- There were two problems with routes.js files defining API routes:
- First, the module requires wen too deep into the "api" module and used specific api modules directly. We have an "index.js" file which defines an API for whole API, it should be used as an entry point to anything to do with the API.
- Second, The naming was inconsistent between the routes.js files for "api", "apiV2", "apiCanary" - it is an extra maintenance burden to go on and change each "api" name when the new version is introduced. The only thing that should be changed within these files is a single line on very top that "requires" a specific API version like so: "const api = require('../../../../api').canary;" - way less maintenance to change that canary to v5 instead of doing an extra rename for all "apiCanary" to "apiV5"
refs: https://github.com/TryGhost/Team/issues/831
- This ultimately fixes the index.js file
- It also makes it super clear what methods in the themeService are used by the API, and which are part of the service loading logic
- It also moves the activate and init function into a single file in a way that highlights they are very similar
- They are also very similar to what happens in storage.setFromZip but that code is mixed up with storage code at the moment
- This is a slightly weird thing, but the intention is to highlight that there are 3 different code paths that can activate a theme
- Ideally we want to unify all the codepaths more, but for now this at least helps us see what is happening where
