ref https://linear.app/tryghost/issue/SLO-173/removed-distinct-from-member-count-query
Performance of GET /members API can be improved by dropping the distinct from the total members count query.
select count(distinct members.id) as aggregate from `members`; // 275ms
select count(*) as aggregate from `members`; // 30ms
In this case we know that the result set will always be unique.
ref https://linear.app/tryghost/issue/ONC-111
- added composite index to posts_tags for post_id,tag_id for faster
lookup
- added composite index to posts for updated_at; this is commonly used
by get helpers on the front end to display data like the latest posts
In testing, this provided a very dramatic improvement for simple get
helper requests like 'filter="id:-{{post.id}}+tag:sampleTag" limit="3"'
which are by default sorted by updated_at desc. I'm not entirely clear
why when sorting by published_at we do not need a composite index - so
far it doesn't seem to be necessary. This should cover the primary cases
for get helpers - the latest posts with a given tag or set of tags.
ref https://linear.app/tryghost/issue/CFR-43/
ref 9d9a421
We recently stopped `select *` from posts when making Content API
requests. This is now being applied to the pages endpoint to help
improve performance. These fields were already being stripped out in the
output serializer, and they will now no longer be returned from the db
at all, reducing the amount of data transferred.
ref https://linear.app/tryghost/issue/CFR-35
- performance improvement intended for the content api/get helpers
The posts table is shared by posts and pages and seldom is queried for
both. It makes sense to add an index on type, and from the perspective
of the content API, also on status as you're almost only ever querying
for published posts or published pages.
closes https://linear.app/tryghost/issue/ONC-115
- OpenTelemetry was throwing errors when viewing posts
- disabled the instrumentation in development mode so it requires explicit config to enable
no issue
Full details coming soon to https://ghost.org/changelog
- Link toolbar and bookmark cards now let you search your existing posts/pages/tags/authors in addition to manually entering the URL
- Typing "@" inside your content lets you quickly search and add a text link
- Typing "@" on a blank paragraph provides a quick way to search and add a bookmark
no issue
- removed labs flag
- removed labs flag conditionals
- removed code related to old setup/done screen
- fixed tests that weren't correctly running against the GA flag code
ref
https://linear.app/tryghost/issue/ENG-1240/race-condition-when-updating-members-last-seen-at-timestamp
When members click a link in an email, Ghost updates the member's
`last_seen_at` timestamp, but it should only update the timestamp if the
member hasn't yet been seen in the current day (based on the
publication's timezone).
Currently there is a race condition present where multiple simultaneous
requests from the same member (if e.g. an email link checker is
following all links in an email) can cause the `last_seen_at` timestamp
to be updated multiple times in the same day for the same member. These
additional queries add a significant load on Ghost and its database,
which can contribute to the exhaustion of the connection pool and
eventually requests may time out.
The primary motivation for this change is to avoid that race condition
by adding a lock to the member row, checking if `last_seen_at` has
already been updated in the current day, and only updating it if it
hasn't.
Another beneficial side-effect of this change is that it avoids locking
the `labels` and `newsletters` tables, which are locked when we update
the `last_seen_at` timestamp in the `members` table currently. This
should improve Ghost's ability to handle a large influx of requests to
redirect endpoints (confirmed with load tests), which tend to happen
immediately after a publisher sends an email.
This commit adds OpenTelemetry instrumentation to Ghost's backend, which
allows us to view traces similar to what we see in Sentry Performance
locally.
OpenTelemetry is enabled if `NODE_ENV === 'development'` or if it is
explicitly enabled via config with `opentelemetry:enabled`.
It also adds a [Jaeger](https://www.jaegertracing.io/) container to
Ghost's docker-compose file for viewing the traces. There's no setup
required (beyond running `yarn docker:reset` to pickup the changes in
the docker-compose file the first time — but this will also reset your
DB so be careful). This will launch the Jaeger container, and you can
view the UI to see the traces at `http://localhost:16686/search`.
no issue
Typing "@" in the editor will immediately trigger an internal link search to make it faster to link to one of your articles. After typing "@" continue typing to search, results can be selected using Up/Down arrow keys or the mouse, then pressing Enter or clicking will insert the selected result's title pre-linked. Pressing Escape or moving the cursor out of the search box will cancel the search.
- removed labs flag
- updated Koenig feature flag for at-linking to use the same flag as our internal linking beta
DES-459
The font size of subtitles/excerpts in newsletters was similar to the
body font size which doesn't reflect the content hierarchy
appropriately. Also, the spacing should be adjusted to represent that
the title and the subtitle belong together.
ref
https://linear.app/tryghost/issue/ENG-1240/race-condition-when-updating-members-last-seen-at-timestamp
When members click a link in an email, Ghost updates the member's
`last_seen_at` timestamp, but it should only update the timestamp if the
member hasn't yet been seen in the current day (based on the
publication's timezone).
Currently there is a race condition present where multiple simultaneous
requests from the same member (if e.g. an email link checker is
following all links in an email) can cause the `last_seen_at` timestamp
to be updated multiple times in the same day for the same member. These
additional queries add a significant load on Ghost and its database,
which can contribute to the exhaustion of the connection pool and
eventually requests may time out.
The primary motivation for this change is to avoid that race condition
by adding a lock to the member row, checking if `last_seen_at` has
already been updated in the current day, and only updating it if it
hasn't.
Another beneficial side-effect of this change is that it avoids locking
the `labels` and `newsletters` tables, which are locked when we update
the `last_seen_at` timestamp in the `members` table currently. This
should improve Ghost's ability to handle a large influx of requests to
redirect endpoints (confirmed with load tests), which tend to happen
immediately after a publisher sends an email.
Fixes
https://linear.app/tryghost/issue/DES-4/image-caption-size-in-email-newsletter.
There were no styles defined for captions for cards beyond the featured
image (bookmark, gallery, video), and we had no way of targeting those
captions with CSS. They are now wrapped in a div with a specific class,
which allows for more selective styling, and are styled similarly to the
caption of the featured image.
ref https://linear.app/tryghost/issue/MOM-117
ref https://linear.app/tryghost/issue/MOM-70
- moved current search into new `search-provider` service and updated `search` service to use the provider service internally
- added `search-provider-beta` service
- uses `flexsearch` as the underlying index for each document so we have better indexing and matching compared to the naive exact-match search we had previously
- adds `excerpt` matching for posts and pages
- keeps results output the same as the original search provider
- added `internalLinkingSearchImprovements` labs flag so we can test this internally before reaching our internal linking beta testers
- updated `search` service to switch between providers based on labs flag
ref DES-347
- adjusted title and excerpt length of latest posts in emails
- as the layout is same (horizontal) on both desktop and mobile, truncateHtml() needed some update
- now maxLength is expected to be larger than maxLengthMobile, because the mobile layout isn't stacked anymore
- some spacing adjustment has been made as well
ref
https://linear.app/tryghost/issue/KTLO-58/dont-send-ghost-acess-cookies-if-no-member-is-logged-in
- Currently when member's caching is enabled, but no member is logged
in, we always send `ghost-access=null;` and `ghost-access-hmac=null;`
cookies in the requests to `/members/api/member/`. This is done to clear
the cookies, but an unintended consequence is that these requests can
never be cached since there is a cookie in the response.
- This PR removes the cookies from the requests when no member is logged
in, the cookies will not be sent, allowing the requests to be cached
- It also unsets the cookies when deleting a member's session, so that
the cookies are not sent in the requests after the member logs out
- This should improve the cache hit ratio with members caching enabled
no issue
- the query can take a very long time to run on large sites causing problems during the upgrade process
- impact from not populating the column:
- only has an effect when the inline excerpt beta is enabled
- when beta enabled, if a revision created before the upgrade is restored then the excerpt will be removed (will be visibly empty in preview before restoring, if any edit has occurred on the post after upgrading then it can still be recovered by restoring the later version or copy/pasting from the preview)
no issue
Full details coming soon to https://ghost.org/changelog/
- when enabled in newsletter design settings a post's custom excerpt will be displayed as a subtitle in the email
ref https://linear.app/tryghost/issue/CFR-31/
- when relations were included, erroneous logic resulted in the model's
default sort being applied
- the model default sort is not intended for the content API and
needlessly slowed down responses
- there's a change for users here that should be incredibly unlikely to
be hit; default sort is `published_at desc` which will be secondarily
sorted by `id desc` instead of `published_at desc, updated_at desc, id
desc`
This is a very significant performance improvement for content API
requests with includes for sites with a significant amount of data,
which will primarily impact those using Ghost as a CMS or theme {{#get}}
helpers.
no issue
We've settled on using "excerpt" naming in place of "subtitle" to better reflect the underlying property name and tie in with themes and historical usage.
- added migration to rename the `show_subtitle` newsletter setting to `show_excerpt`
- renamed all places in the codebase that referenced subtitle
closes https://linear.app/tryghost/issue/MOM-170
When the subtitle field is included in the editor it creates a disconnect with post revisions if the underlying custom excerpt data is not included so we'd like to both preview and restore the subtitle when the in-editor subtitle field is enabled.
- added `post_revisions.custom_excerpt` column to schema
- added migration to add `post_revisions.custom_excerpt` to existing databases
- added migration to populate `post_revisions.custom_excerpt` with the current `post.custom_excerpt` value from the associated record
- ensures no data is inadvertently lost when restoring an old version
- using current data matches what would have happened previously where custom_excerpt was never overwritten when restoring an old version
- updated post revisions handling to accept the `custom_excerpt` field
- updated Admin's revision preview and restoration to display and set the `custom_excerpt` field
closes https://linear.app/tryghost/issue/MOM-194
- whilst working on the feature our naming changed from "Subhead" to "Subtitle"
- this rename of the newsletter design setting column brings naming back into a consistent state before public release
closes https://linear.app/tryghost/issue/MOM-83
- added additional labs flag to allow internal testing prior to private beta release
- bumped Koenig packages containing support for @-link feature
REF MOM-119
- Split subhead feature flag into two: editorSubtitle and
newsletterSubtitle
- Updated UI copy, feature flag names and class names from subhead to
subtitle
refs MOM-152 MOM-148 MOM-151
- Added Subheads behind a flag + toggle in settings.
- Removes Excerpt fields from post settings if flag is enabled.
- Added subhead toggle in newsletter settings.
- Loads of styling
---------
Co-authored-by: Sanne de Vries <sannedv@protonmail.com>
We want to use a randomly generated 64 byte secret for the hmac, and
utf8 encoding isn't nice to work with for this, so we're going to use a
base64 string and decode it into a buffer for the secret.
- we don't need this in `ghost/core` as it's not used in there
- we need to declare this dependency for the apps, as they use it for
running tests
- this doesn't change the lockfile but it means we're declaring the
dependency in the right places now
ref
https://linear.app/tryghost/issue/KTLO-45/deploy-members-caching-solution-to-a-single-site-to-validate-and-test
Currently we only cache publicly available content. Any content that is
accessed by a logged in member is only cached for that specific member
based on their cookie. As a result, almost all requests from logged in
members bypass our caching layer and reach Ghost, which adds unnecessary
load to Ghost and its database.
This change adds experimental headers that allow our CDN to understand
which tier to cache the content against, and securely tell the CDN which
tier a logged in member has access to. With these changes, we can cache
the member content against the tier, rather than the individual member,
which should result in a higher cache HIT ratio and reduce the load on
Ghost.
For requests to the frontend of the site, Ghost will set a custom
`X-Member-Cache-Tier` header to the ID of the tier of the member who is
accessing the content. This tells the CDN which tier to cache the
content against.
For requests to either `/members/?token=...` endpoint (the magic link
endpoint) or `/members/api/member`, Ghost will set a `ghost-access` and
`ghost-access-hmac` cookie with the ID of the tier of the logged in
member. With these two pieces of information, our CDN can serve cached
content to logged in members.
These headers are experimental, and can only be enabled via Ghost's
config. To enable these headers, set `cacheMembersContent:enabled` to
`true` and provide an HMAC key in `cacheMembersContent:hmacSecret`.
ref https://linear.app/tryghost/issue/CFR-29
- Removed the mobiledoc and lexical columns from the posts input
serializer, meaning they will no longer be queried for.
Get helpers are essentially a gateway to the Content API. We already
strip out the mobiledoc and lexical fields in the output
serializer/returned response, but this means we're passing the mobiledoc
and lexical fields back from the db. This is pointless and these fields
are substantial in size - by far the largest fields in the whole ghost
db - leading to slowed performance.
I've updated the posts input serializer to strip out the lexical and mobiledoc
columns so we stop doing a `select *` with every query.
ref ENG-824
- the bug is causing resize prefixes being added to images served from
outside of Ghost.
- this now would only append the prefex to images served by Ghost and
other images urls' would get served as is.
- we can determine that by checking whether imageName doesn't exist,
meaning the source is a third party.
- this mostly affect edge case users, eg where a feature image url was
passed in via the API and doesn't get served by Ghost.
refs CFR-21
Reorganised middleware execution so that member data is not redundantly loaded for static assets or the sitemap.
---------
Co-authored-by: Michael Barrett <mike@ghost.org>
refs #20197
- adds a jackspeak resolution to Ghost core so we can try and ensure the compatible version of jackspeak/string-width is used when the lockfile is regenerated
ref https://linear.app/tryghost/issue/MOM-73
We need to add body parsing middleware here, so that NestJS has access to it.
We also attach the rawBody which is used to validate the HTTP Signatures
closes https://linear.app/tryghost/issue/MOM-80
- updated internal linking search results items
- removed visibility text from meta data
- added additional icon for paid/specific tier visibility
- added titles to icons
- bumped `@tryghost/koenig-lexical` to include support for meta icon titles
- bumped other Koenig packages due to sub-dependency updates
ref https://linear.app/tryghost/issue/CFR-27
- updated packages to include performance improvement for NQL filter
strings including multiple neq filters for the same resource
- bumped `bookshelf-plugins`
- bumped NQL versions
We identified a performance fix that allows us to combine not equal
(neq) filters for the same resource in a logically-equivalent way that
also has far more performant resulting SQL.
We're effectively automatically combining strings like
'tag:-tag1+tag:-tag2` into 'tag:-[tag1,tag2]'.
fix https://linear.app/tryghost/issue/SLO-104/cannot-read-properties-of-undefined-reading-0-an-unexpected-error
- if the request body didn't contain the correct keys, it'd just HTTP
500 out of there
- this adds some optional chaining so we end up with undefined if
anything isn't as expected, and the following if-statement does the
rest of the check for us
- this also adds a breaking test (the first E2E test for authentication, yay!)
refs https://docs.sentry.io/platforms/javascript/configuration/filtering/#using--1%20
- this simplifies our logic to determine whether we should send events
by moving the code to `beforeSend`
- `errorHandler` is going away in Sentry v8 so this results in a shorter
diff in the future
- the logic should be the same, always send non-Ghost errors, and only
send HTTP 500 Ghost errors
fix https://linear.app/tryghost/issue/SLO-101/http-500-with-invalid-multipart-data
- previously, busboy would error out if we supplied a body that was
invalid (such as an empty FormData)
- we would then return a HTTP 500 to the user, which causes all manner
of problems
- now we catch errors from busboy and return a nice BadRequestError
fix https://linear.app/tryghost/issue/SLO-85/fix-http-500-on-contentposts
- in the event we give the incorrect format in a filter, MySQL will
throw an error and we'll throw a HTTP 500 error
- we can capture this error and return a more useful error to the user
- ideally we'd do this in a validation step before attempting the query,
but parsing this out of NQL and detecting which columns are DATETIME
could be quite tricky
- this updates a bunch of places where we're just using Object to cheat
the system
- doing this means editor autocomplete and basic type checking is better
because we now have proper types in place
- functionality should not change, these are just comments
fix https://linear.app/tryghost/issue/SLO-87/cannot-read-properties-of-undefined-reading-createimpl-an-unexpected
refs https://github.com/jsdom/jsdom/issues/3709
- in the event we are given some HTML to parse, and that fails, we
currently return a HTTP 500 because it's unhandled
- the instance we saw was due to `<constructor>` crashing jsdom, we've
opened an issue for that
- in terms of handling the error gracefully, we can surround the code
in a try-catch and return a more suitable error. I've gone for a
ValidationError for now - you could debate whether a different one is
more appropriate
- also added Sentry error capturing so we're not blind to these,
ultimately we should make sure the parser can handle all
user-submitted data
- this adds a simple set of types to the @tryghost/api-framework
package that should describe all of the keys available on a
controller, and then rolls it out to all API controllers
- unfortunately, due to https://github.com/microsoft/TypeScript/issues/47107, we have
to split apart `module.exports` into a variable assignment in order for type-checking
to be done
- the main benefit of this is that `frame` is now typed, and editors understand what keys
are available, so intellisense works properly
- `statusCode` should be a number, but we were passing a string
- this doesn't really affect anything, but tsserver was flagging it up
as the wrong type
- we should pass it as `err` and not `error`
- this probably slipped in because the catch parameter is called
`error`, so I've updated that and fixed the references
- this helps tsserver figure out what the type of things is around our
codebase
- nothing crazy, mostly Express types for the middleware, application and router levels
fix https://linear.app/tryghost/issue/SLO-95/unexpected-end-of-multipart-data-for-broken-image-upload-request
- in the event the client sends an invalid body to the image or media
upload endpoints, Dicer will throw an error if the boundary data is
malformed
- previously, we've just been bubbling that up as an InternalServerError
and that results in an HTTP 500
- we can capture errors produced by dicer and return a handled
BadRequestError, as it's the client's fault
- also includes breaking tests
fix https://linear.app/tryghost/issue/SLO-94/unexpected-field-when-given-broken-image-upload-request
- in the event the body of an image or media upload request is malformed
(broken metadata / blob or something), we get a MulterError and this
bubbles up as an InternalServerError and spits out a HTTP 500
- we can capture this and return a BadRequestError, as it's the client's
fault for not providing the correct body
- this implements that and adds breaking tests
ref
https://linear.app/tryghost/issue/ENG-902/add-an-optional-timeout-in-the-redis-cache-adapter-in-case-redis
- Added an optional timeout parameter to AdapterCacheRedis, so that the
`get(key)` method will return `null` after the timeout if it hasn't
received a response from Redis
- When load testing the `LinkRedirectRepository` with the Redis cache
enabled on staging, we noticed that for some reason Redis stopped
responding to commands for ~30 seconds.
- The `LinkRedirectRepository` was waiting for the Redis cache to
respond and resulted in a drastic increase in response times for link
redirects
- This change will allow us to set a timeout on the `get(key)` method,
so that if Redis doesn't respond within the timeout, the method will
return `null` as if it were a cache miss.
- Then the `LinkRedirectRepository` will fall back to the database and
return the link redirect from the database instead of waiting
indefinitely for Redis to respond
fix https://linear.app/tryghost/issue/SLO-93/undefined-path-error-with-bad-image-upload
- in the event we receive a request to upload an image, that doesn't
contain an image, we still try and unlink the files
- this is a dangling promise, so it doesn't cause an explicit HTTP
error, but it does show up as a console error
- fixed it by checking for the path, and early returning if it doesn't
exist
- also added a test that would fail without this
ref https://linear.app/tryghost/issue/SLO-78
- the `POST /members/api/member` endpoint is solely used by the alpha
feature `membersSpamPrevention` and should not be available otherwise
fix https://linear.app/tryghost/issue/SLO-79/incorrectusageerror-the-url-httpsblogkongregatecompercentc0-couldnt-be
- we added this Sentry captureException whilst fixing a bug where
decodeUrl could fail, and throw a 500 exception
- we added handling for that case and returned an empty string, but we
also added Sentry error capturing
- at this point, I don't think we need to be capturing errors in Sentry,
because the issue is already handled, and it only usually happens with
malicious/incorrect URLs
- this is our #2 cause of Sentry alerts, so it's good to clean it up
- we don't need to deep require into the library as it exports what we
need on the surface
- this should unblock https://github.com/TryGhost/Ghost/pull/19002, as
it's randomly failing with this require
refs
[ENG-827](https://linear.app/tryghost/issue/ENG-827/🐛-crash-on-resizing-animated-gif)
Added a timeout to the image resizing middleware to prevent crashes when
an image is taking too long to resize. When the timeout is reached and
the image has not been resized, the middleware will return the original
image
ref
https://linear.app/tryghost/issue/ENG-851/implement-a-minimal-but-complete-version-of-redirect-caching-to
ref https://app.incident.io/ghost/incidents/55
Often immediately after sending an email, sites receive a large volume
of requests to LinkRedirect endpoints from members clicking on the links in
the email.
We currently don't cache any of these requests in our CDN, because we
also record click events, update the member's `last_seen_at` timestamp,
and send webhooks in response to these clicks, so Ghost needs to handle
each of these requests itself. This means that each of these LinkRedirect requests
hits Ghost, and currently all these requests hit the database to lookup
where to redirect the member to.
Each one of these requests can make up to 11 database queries, which can
quickly exhaust Ghost's database connection pool. Even though the
LinkRedirect lookup query is fairly cheap and quick, these queries aren't
prioritized over the "record" queries Ghost needs to handle, so they can
get stuck behind other queries in the queue and eventually timeout.
The result is that members are unable to actually reach the destination
of the link they clicked on, instead receiving a 500 error in Ghost, or
it can take a long time (60s+) for the redirect to happen.
This PR uses our existing `adapterManager` to cache the redirect lookups
either in-memory or in Redis (if configured — by default there is no caching). This only removes 1 out of
11 queries per redirect request, so it won't reduce the load on the DB
drastically, but it at least decouples the serving of the LinkRedirect from
the DB so the member can be redirected even if the DB is under heavy
load.
Local load testing results have shown a decrease in response times from
60 seconds to ~50ms for the redirect requests when handling 500 requests
per second, and reduced the 500 error rate to 0.
ref https://linear.app/tryghost/issue/ENG-826
- Changed staff deletion logic to do a bulk insert when adding a tag to
the users' associated posts
Staff deletion logic has really poor performance at scale because we do
individual updates for every post. If a user has dozens+ posts
(especially in a large db with thousands of posts), this can take >60s
and look like a timeout. Ultimately this should probably be a jobbed off
process, but for the time being we can improve this by doing a bulk
insert.
Note that this update uses the pattern for the bulk tagging of posts
from the right click (bulk) actions in the posts lists in Admin. With
bulk actions, **we do not trigger web hooks or the post.edited events**.
We will document this and follow up on this separately.
ref MOM61
- Adds admin-x react app we’ll use as ActivityPub playground to the
sidebar nav behind the feature flag.
- Wired up routing to Ember
- Setup the project as `admin-x-activitypub`
---------
Co-authored-by: Ronald Langeveld <hi@ronaldlangeveld.com>
ref https://linear.app/tryghost/issue/MOM-29
This is very rough, and all still behind a flag. The idea is that any public
post which is published gets added to the Outbox of the site Actor. We also
dispatch an event, which will be used to deliver the Activity to any relevant
inboxes, but that is outside the scope of this commit.
refs https://ghost-foundation.sentry.io/issues/5135326925/
- the service tends to 503 all the time, and we don't really care enough
for it to ping us in Sentry, as it's not something we control
- we can still keep logging the errors in case we need to go and look at
what went wrong
refs #20050
- Renovate seems to be unable to bump the package past the security
release, but unfortunately this release contains a breaking bug
- this commit manually bumps the package so we can get things flowing
again
- the security release doesn't really affect us, but we should still try
and keep on the latest
ref
https://linear.app/tryghost/issue/ENG-845/error-attempted-to-set-lexical-on-the-deleted-record
ref
[https://linear.app/tryghost/issue/ENG-854/🐛-deleting-imported-posts-makes-ghost-unresponsive](https://linear.app/tryghost/issue/ENG-854/%F0%9F%90%9B-deleting-imported-posts-makes-ghost-unresponsive)
- When deleting a post in the editor's Post Settings Menu, if the post
has unsaved changes (indicated by the hasDirtyAttributes property in the
editor), Admin will crash because it tries to save a post revision
before leaving the editor, but the post has already been deleted so
saving fails.
- This can occur when editing a post and quickly deleting it from the
Post Settings Menu before saving is completed.
- It can also occur when attempting to delete an imported post, as the
editor will parse the lexical from the server and may make some minor,
invisible-to-the-user changes to the lexical string locally (e.g. JSON
formatting, or updating the JSON to use extended version of base lexical
nodes), which triggers the same error.
- This fix bypasses the attempt to save a post revision when leaving the
editor if the post is already deleted, which allows the transition back
to the Posts route to succeed.
refs f39d1d3aa3
- similar to the commit above, the JSON parser changed between Node 18
and Node 20, so the error message changed too
- we actually just want to check the error is forwarded to the user, so
we can do that by getting the error message from JSON.parse and check
against that
ref https://linear.app/tryghost/issue/MOM-48
This required some structural changes to our NestJS setup so that we can mount
it on multiple parts of the Ghost express app.
We've used the RouterModule to allow adding submodules that are mounted on
different paths, and we've had to be explicit about the base path for each
module. We've also had to switch back to using the Module decorator, because
RouterModule doesn't work with DynamicModule definitions.
Now that the NestJS app has knowledge of the full path, we need to "reset" the
url & baseUrl when passing the request into NestJS so that it can correctly
match the path. This is probably needed for the frontend too, for subdirs, but
that causes further issues - as this in prototype stage, we'll look later
Another issue is that NestJS replaces the express app instance with its own,
which isn't an issue for the Admin API (though we've fixed it anyway for
consistency), but did cause problems for the frontend, because the express app
is where view engine and directory information is stored.
The fix for this is to save a reference to the original ghost express
application, and reattach it to the request if it is not handled by Nest
Now that we have the Nest app mounted on the frontend, we're able to have it
handle the /.well-known/webfinger route with a proper controller, which is nice!
ref MOM-31
ref https://linear.app/tryghost/issue/MOM-31
We'll be building a lot of the new code for ActivityPub in Nest, so we'll need
to have it enabled in Ghost to work.
- The RSS cache has lived for a really long time, but I'm not sure it's useful
- Want to be able to determine if it gets used much, and if not, then we can remove it
ref 78311591d0
- updated tests to not click a button on the setup/done screen that is no longer shown
- fixed setup flow showing an alert bar due to not handling the `TransitionAborted` error that is thrown by the setup/done->dashboard redirect
ref https://linear.app/tryghost/issue/KTLO-1/members-spam-signups
- Some customers are seeing many spammy signups ("hundreds a day") — our
hypothesis is that bots and/or email link checkers are able to signup by
simply following the link in the email without even loading the page in
a browser.
- Currently new members signup by clicking a magic link in an email,
which is a simple GET request. When the user (or a bot) clicks that link, Ghost
creates the member and signs them in for the first time.
- This change, behind an alpha flag, requires a new member to click the
link in the email, which takes them to a new frontend route `/confirm_signup/`, then submit a form on the page which sends a POST request to the
server. If JavaScript is enabled, the form will be submitted
automatically so the only change to the user is an extra flash/redirect
before being signed in and redirected to the homepage.
- This change is behind the alpha flag `membersSpamPrevention` so we can
test it out on a few customer's sites and see if it helps reduce the
spam signups. With the flag off, the signup flow remains the same as
before.
ref https://linear.app/tryghost/issue/ENG-790/remove-use-of-sub-queries-in-email-analytics
- the `delivered_at` column is typically entirely/nearly entirely filled with values meaning the `IS NOT NULL` query matches a huge number of rows that MySQL has to fetch from the index to count
- using `IS NULL` switches that behaviour around as it will now match very few rows which has been shown in testing to be considerably quicker
- after switching to `IS NULL` the query returns an "undelivered" count rather than a "delivered" count, in order to keep the rest of the system behaviour the same we can calculate the delivered count by subtracting the query result from the total number of emails sent which we can fetch using a very fast primary key lookup query on the `emails` table
closes https://linear.app/tryghost/issue/ENG-790/remove-use-of-sub-queries-in-email-analytics
Avoiding sub queries means we don't have a process tied up for longer than necessary and we can more easily see if one of the queries is non-performant.
- extracted the count queries into separate queries and used the retrieved values in the final update query
- removed a query by moving the email open rate calculation into JS as we've already fetched the necessary data before that point
ref https://linear.app/tryghost/issue/CFR-13
- enabled saving traces on browser test failure; this makes troubleshooting a lot easier
- updated handling in offers tests to ensure the tier has fully loaded in the UI (not just `networkidle`)
- updated publishing test to examine the publish button reaction to the save action response instead of a 300ms pause
In general, our tests use a lot of watching for 'networkidle' - and sometimes just raw timeouts - which do not scale well into running tests on CI. In particular, 'networkidle' does not work if we're expecting to see React components' state updates propagate and re-render. We should always instead look to the content which encapsulates the response and the UI updates. This is something we should tackle on a larger scale.
ref ENG-774
ref https://linear.app/tryghost/issue/ENG-774
Staff Tokens will have both a `user` and an `apiKey` present on the
`loadedPermissions`.
The check here for `apiKey` was written when we could assume that an
`apiKey` was an Admin Integration - so it completely overwrote the
previous `allowed` list. When we added the concept of Staff Tokens -
this resulted in a privilege escalation.
This is a good lesson in not using proxies or indicators for data, as
changes elsewhere can invalidate them - if we had been specific and
checked the role of the current actor we wouldn't've had this bug!
ref https://linear.app/tryghost/issue/CFR-4/
- added request queueing middleware (express-queue) to handle high
request volume
- added new config option `optimization.requestQueue`
- added new config option `optimization.requestConcurrency`
- added logging of request queue depth - `req.queueDepth`
We've done a fair amount of investigation around improving Ghost's
resiliency to high request volume. While we believe this to be partly
due to database connection contention, it also seems Ghost gets
overwhelmed by the requests themselves. Implementing a simple queueing
system allows us a simple lever to change the volume of requests Ghost
is actually ingesting at any given time and gives us options besides
simply increasing database connection pool size.
---------
Co-authored-by: Michael Barrett <mike@ghost.org>
ref ENG-728
ref https://linear.app/tryghost/issue/ENG-728
This is not used anywhere, and makes the code more complicated, it's a good
step toward simplifying permissions and pulling them out of the database.
ref ENG-728
ref https://linear.app/tryghost/issue/ENG-728
This is NOT a functionality change. The Post#permissible method unit
tests have been updated to pass `true` as `hasUserPermission` and we can
see that the permission functionality remains the same.
The permissible method of the post model is responsible for removing
permission based on the data that is being modified, but the permissions
module is setup to allow the permissible method to grant permission -
this means that we call permissible, even if the current actor doesn't
have permission, this results in code that is hard to understand and
manage.
We are going to be instead returning early if an actor does not have
permission, this will allow permissible method signatures to be greatly
simplified (removing the need for hasUserPermission, hasApiKeyPermission
& hasMemberPermission arguments).
fixes https://linear.app/tryghost/issue/ENG-746/http-500-responses-when-handle-image-sizes-middleware-hits-missing
- in the event a request comes in for a resized image, but the source
image does not exist, we return a rendered 404 page
- we do this because we pass the NotFoundError to `next`, which skips
over the static asset code where we return a plaintext 404
- also included a breaking test that ensure we go to the next middleware
without an error
ref ENG-761
ref https://linear.app/tryghost/issue/ENG-761
Creating these pipelines is expensive, and we don't want to do it
repeatedly for the same controller. Adding caching should reduce the
amount of time spent setting up pipelines for each usage of the `get`
helper.
ref https://linear.app/tryghost/issue/IPC-66/onboarding-checklist-v1
- Adds a basic version of a new onboarding checklist behind the feature
flag, without incomplete/complete state logic
- Links to Design settings, Members screen and new post
- Opens amodal that we’ll use as Share modal
---------
Co-authored-by: Daniël van der Winden <danielvanderwinden@ghost.org>
ref [ENG-747](https://linear.app/tryghost/issue/ENG-747/)
ref https://linear.app/tryghost/issue/ENG-747
H'okay - so what we're trying to do here is make get helper queries more
cacheable. The way we're doing that is by modifying the filter used when
we're trying to remove a single post from the query.
The idea is that we can remove that restriction on the filter, increase
the number of posts fetched by 1 and then filter the fetched posts back
down, this means that the same query, but filtering different posts,
will be updated to make _exactly_ the same query, and so share a cache!
We've been purposefully restrictive in the types of filters we
manipulate, so that we only deal with the simplest cases and the code is
easier to understand.
ref ENG-742
ref https://linear.app/tryghost/issue/ENG-742
We don't do any parsing of layouts in gscan, which means themes can be
uploaded which use non-existent files for their layout.
We can catch the error in the res.render call, and wrap it, just like we
do for missing templates (e.g. the StaticRoutesRouter)
closes ENG-632
- This listens to a new property in the `milestones` config to set a minimum value of Milestones we wanna use the Slack notification service for
refs https://linear.app/tryghost/issue/ENG-750
- when adding a recommendation, we fetch the recommended site's metadata
- before this change, if the metadata fetch failed for some reason, we'd show an error and block the recommendation from being added
- after this change, we use fallback values if the metadata fails to fetch, instead of blocking the recommendation from being added. We use the site domain as the title and leave the rest empty (no favicon, no description)
- this change also means we are not checking whether a site exists or not for the publisher anymore. It’s then up to the publisher to make sure they don’t enter broken URLs
ref ENG-747
ref https://linear.app/tryghost/issue/ENG-747
H'okay - so what we're trying to do here is make get helper queries more
cacheable. The way we're doing that is by modifying the filter used when
we're trying to remove a single post from the query.
The idea is that we can remove that restriction on the filter, increase
the number of posts fetched by 1 and then filter the fetched posts back
down, this means that the same query, but filtering different posts,
will be updated to make _exactly_ the same query, and so share a cache!
We've been purposefully restrictive in the types of filters we
manipulate, so that we only deal with the simplest cases and the code is
easier to understand.
