refs https://github.com/TryGhost/Team/issues/907
The logic to populate the `@price` data and the `@products` data both
rely on the same product data, but were each making their own request to
the API. This refactor removes the request from the legacy `@price`
data, which should cut the database queries in half.
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 16728a3ef1
- Same reason as in refed commit, ltdc:
- Traditionally all of Ghost's public-facing text was written in British English
- We're changing that to US English because that's more common
- US English should also be used in code e.g. properties are called color not colour
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
refs https://github.com/TryGhost/Team/issues/527
refs https://github.com/TryGhost/Ghost/issues/10790
- Frontent has to have as few as possible coupling points with the Ghost Server API. By design that point has been a "proxy.api" property that will become more and more constraint in the future based to limit the surface of frontend interaction with servers's API
- Removing `.../server/api` requires in favor of using a proxy decreases direct coupling
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
- All the code for creating these errors is now replaced with a single function
- This is useful DRY as it helps make code more readable
- This gets rid of the override of the error type to ThemeWorksButHasErrors - which is both weird and afaict not used anywhere
refs: 076ad99593
- as of 076ad99593 we no longer use the error property of the active theme anywhere
- cleaning up and removing this usage reduces the code pathways and makes the init fn a bit clearer
refs https://github.com/TryGhost/Team/issues/856
- This syntax gives easier understanding of modules dependencies and improves searchability. For exampke, I was looking for all "api" uses exposed by the server proxy and didn't have a clear picture into which modules used it.
- The change was made during a short-lived try to limit the use of "api" in the server proxy :) I thought it would be helpful when bumping the defult server API exposed internally. Next time!
- We use bluebird inconsistently throughout the codebase now
- The original reason why we needed to use it so heavily was so that all promises returned had the bluebird behaviour, including catch predicates
- Most other usage is explicit, but this is really hard to detect and hasn't made it to standard promises, so we should get rid of this pattern
- The router bootstrap is no longer allowed to fetch it's own settings, but rather is passed them
- This moves the call to the site routes.js file, which isn't much better but it's a start
- The goal is to always pass these in from the boot process, or from the bridge reloader
- Reduced the number of levels in our debug naming in the frontend
- Unified components like "themes" and "routing" under one name
- Should help to make debug slightly more useful again
refs https://github.com/TryGhost/Team/issues/726
- The refed feature got broken during the refactors. Even though this area is covered by unit tests the "this context" testing should probably done on an integration test level, which we don't have a clear pattern for just yet
- this was skipped on boot, but then called in the test utilities, but only on restart 🙈
- this means that yarn test:acceptance (i.e. running all tests) works, but if you try to run just test/api-acceptance/themes_spec.js it would fail because that uses a fresh boot not a restart/reload
- I've changed this as keeping the test using the real boot, rather than the made-up acceptance-utils tasks as much as possible is way better
refs https://github.com/TryGhost/Team/issues/790
The schema validations are used at the model layer to validate inputs
and need to be updated in order for us to reintroduce the 'comped'
status.
refs https://github.com/TryGhost/Team/issues/790
Since version 4.6 the 'comped' status has not been used. Any members
which were given complimentary plans since then will have had a `status`
of 'paid', and therefore the corresponding members_status_events row
would have a `to_status` of 'paid'.
This migration is designed to fix these members_status_events rows by
ensuring that the last (chronologically) members_status_event row for a
comped member has a to status of 'comped'.
Unfortuantely this migration loses information which makes writing a
perfect inverse migraion impossible. Alternative down migrations were
considered, but these would lose further information.
refs https://github.com/TryGhost/Team/issues/790
In order to track when a member was comped, as well as to differentiate
paid members from comped, we are reintroducing the 'comped' status. This
migration will updated members with a Complimentary Stripe Subscription
to a status of 'comped'. It is essentially a reversal of the 4.6
migration.
no issue
- incorrect syntax was used in the error handlers inside of the `for` loop, by using `return` when logging the whole for-loop was aborted whereas we want to log and continue processing the rest of the items
refs https://github.com/TryGhost/Team/issues/853
A refactor of `urlUtils` usage in 4.6.1 left a buggy 4.0 migration that did not transform URLs inside of mobiledoc cards. Anyone upgrading from 3.x to 4.6.1-4.8.4 would end up with inconsistent URL formats and potentially broken images.
- fixed 4.0 migration by passing our mobiledoc cards list in when transforming mobiledoc urls
- added a new migration that re-applies the missed URL transforms and content re-generation for any site that did a 3.x upgrade to a buggy 4.x version
refs 8a1fd1f57f
refs 5584430ddc
- The change to async/await in the original commit 558443 was causing problems in downstream dependencies (create-error package) where it was loosing a context of "this". It's not a direct dependency so I didn't go yak shaving into where exacly the context is lost.
- The fix to keep a correct context of "this" was sticking to an existing pattern using regular function returning promises. Once we need to redo them into async/await we can investigate if there's a way around create-error's context prolbem
closes https://github.com/TryGhost/Team/issues/817
refs 6d083ee00e/packages/bookshelf-pagination/lib/bookshelf-pagination.js (L256)
- The 500 error is not the best we can do in this situation and throwing a 400 just like we doo in a referenced commit would keep the convention
- The underlying problem of the bug is bigger - we allow the fields named the same way as relations to leak into the db query and that causes an incorrect SQL syntax. It's a bigger problem which would need a separate, holistic approach
refs https://github.com/TryGhost/Team/issues/849
With multiple products, we have re-enabled segmentation by product for posts behind alpha feature flag. This change handles the default content cta to show custom message if the post's access is restricted to specific products when behind the flag.
refs https://github.com/TryGhost/Team/issues/849
As part of work for segmented post access with multiple products, the custom filter for post access is stored in `visibility` field on posts but passed with `visibility_filter` property on API. This change -
- updates input serializer of posts to transform `visibility` and `visibility_filter` properties correctly
- updates output serializer for canary to transform and send `visibility_filter` attribute with filter value
- updates output serializer for v3 to ignore any custom filter on visibility and return `paid` instead as v3 didn't have a concept of custom filter
refs https://github.com/TryGhost/Team/issues/849
Custom post visibility (behind alpha flag) is added to the API using new `visibility_filter` attribute that stores the custom filter. This change -
- updates validator for visibility to check new `visibility_filter` property
- cleans usage of i18n in favor of tpl
refs https://github.com/TryGhost/Team/issues/727
- The version was forgotten to get a bump durin g 4.0 release. The API version used by update check should be the same as internal default.
- Because the current internal default is mistakenly set to v3 API it's still not optimal but will need a holistic solution in the future
refs https://github.com/TryGhost/Team/issues/781
refs 813d288eb2#
- The 500 error was introduced through a refed commit long time ago when (probably?) there were no other safeguards preventing from serving content through a theme with errors. Since than we have multiple safeguards when aploading/activating the theme with errors and the default handling when such error occurs is more graceful - a 400 with specific error details is shown
- We need this change to land before bumping gscan that introduces more suphisticated error detaction in theme templates. Otherwise, people upgrading to new version and having an error on an unused template or somewhere undetected previously woul end up with a bricked site showing a generic 500 - not a great experience!
refs: fbf0636936
- I renamed this pattern in a bunch of places, but missed a few, leaving the code messy and confusing
- This makes the naming consistent
no refs
- adds `price` data on subscription from related `stripe_price` on updating a member via frontend
- removes inconsistency between `GET` and `PUT` data for logged in member on a site
refs https://github.com/TryGhost/Team/issues/828
- Previous method had a bug where it didn't take into account cases when onlya single card with a segment filter has been used leaving the members not covered by that filter without an email
refs https://github.com/TryGhost/Team/issues/828
- Before sending out batches with members we need to partition all members based on the segment they belong to. Special segment "unsegmented" is used in case none of the segments used in the emal cards cover part of the members set (for example only free members card used when emailing all members)
refs https://github.com/TryGhost/Team/issues/845
refs 517d2abc5c
- updated router response formatting functions and `{{#get}}` helper response handling to make any `feature_image_caption` properties in the response a `SafeString` instance so triple-curlies are not needed when using the property in themes
- The main aim here is to end up with a simple and clear public API for the meta module
- Secondarily, we want to make it a bit clearer which bits don't really belong here so we can see what to do with them
- To achieve this, the main logic has been moved into get-meta (although there's still some logic here which needs moving further)
- The index.js now has a small clear public API, and the proxy, which is the only way this is consumed, is able to use the public API directly
- This function is quite different to the others, as it generates an excerpt from HTML (truncating)
- Most functions in the meta data folder just contain content negotiation logic, like if post then feature_image else cover_image type things
- This function is more like a library and shouldn't live in Ghost, it should probably be in @tryghost/helpers
- It's definitely something we'd love to rewrite to work better tooooo
refs https://github.com/TryGhost/Team/issues/839
It's now possible to set alt and caption for post feature images using `feature_image_alt` and `feature_image_caption` fields on a post resource.
- `feature_image_alt` - plain text, limited to 191 chars (alt text is not recommended to be longer than 125 chars, screen readers may cut the description off at that point)
- `feature_image_caption` - basic HTML, limited to 65535 chars
Alt and caption will be automatically used inside of newsletter content, for your website content make sure your theme is updated to use the v4 API and make use of the new properties.
---
- removed `featureImageMeta` labs flag
- getting rid of instances of new Error as we should always use @tryghost/errors
- Whilst here, got rid of i18n but discovered the messages were missing!
- This is my fault, they disappeared when I removed external apps and clearly removed too much: 8c1a0b8d0c (diff-0f5cc40aa8906a1be1bad2002a35361bbf9e766e46b3b29be10f4f479265426a)
- Therefore, I have restored these messages in the places where they were used, except amp_content, where I have written a new message, as the message that was there was not relevant
- This is part of the quest to separate the frontend and server & get rid of all the places where there are cross-requires
- At the moment the settings cache is one big shared cache used by the frontend and server liberally
- This change doesn't really solve the fundamental problems, as we still depend on events, and requires from inside frontend
- However it allows us to control the misuse slightly better by getting rid of restricted requires and turning on that eslint ruleset
- requiring lib/common/events makes the settings cache tightly coupled to the server
- moving this up to settings index means the cache itself can be moved to a shared component/moved out of Ghost
- the index then becomes the settings manager
- questionable whether the event listeners & updater part of this shouldn't be part of a manager, independent of the actual cache 🤔
refs https://github.com/TryGhost/Team/issues/828
- We need a way to recreate a filter that was used to create an email content for specific email_recipient. By saving member_segment value for each email_batch we can traverse back to the filter that was applied during email creation.
- shutdown removed listeners, which should really be done before adding them anyway!
- reset sets the cache back to an empty object, which was already done by init
- merge these into one reset function that fully resets the cache
- all instances of shutdown were called before an init call, and now called during init, therefore these can be removed
- acceptance utils had an instance of calling shutdown and reset together as part of stopping Ghost, reworked that to be clearer
refs https://github.com/TryGhost/Team/issues/828
- When sending email batches out they need to be created without mixing different member segments. This allows for easier reasoning about what data has been sent out to each specific email recipient
- Modified email batches to chunk based on segments defined in the HTML content of the post
refs https://github.com/TryGhost/Team/issues/828
- When detecting email segments and later creating a member filter out of this data we only care about unique segments otherwise we'd be creating multiple batches with the same segment filter
refs https://github.com/TryGhost/Team/issues/828
- This is experimental segment extraction logic, more to follow. Alllows to extract arrays of email segments used in the email's HTML content
- At the moment the bootstrap.start method asks the settings service for its settings
- This couples the routing and settings services together - when maybe we want to use a different method to generate settings
- By passing the settings to the routing service at the right time, we open up possibilities for refactoring
- The main goal here is getting this settings related code out of the routing service as it really doesn't belong there
- This settings file is used purely by the API to get and set files - its not really anything to do with actual routing
- This file calls out to the bridge to do a reload, which helps decouple slightly
- More refactoring is needed to get rid of the urlService dependency
- Note this file is really similar to the redirects one, it would be good to merge them
- At the moment the bootstrap.start method asks the settings service for its settings
- This couples the routing and settings services together - when maybe we want to use a different method to generate settings
- By passing the settings to the routing service at the right time, we open up possibilities for refactoring
- broken down large function into smaller functions to reduce repeated code
- try to make this and the redirects equivalent look similar
- this code is the getter and setter for the API
- TODO: I think this can be further refactored into a settings file class
- Allows for slight decoupling of API and frontend with route settings being updated
- Activate theme now calls the same codepath to reload the frontend
- Yet another step on the path to make it possible to init/reload/run the frontend independently from the server
