fix https://linear.app/tryghost/issue/SLO-88/typeerror-cannot-read-properties-of-null-reading-relations
- in the event that we make it through the version mismatch code, but
without a key, which is possible if you send a request like POST
/ghost/api/v2/content/posts/`, then the version mismatch code will try
and look up the API key attached to a null key, which won't work
- we should handle this case and soft return, to avoid trying to read
`.relations` from `null`
- I'm not entirely convinced by how this code works in general, it seems
quite confusing to reason about, but this commit should solve the HTTP
500 we've been seeing from this
- perhaps in the future we can return earlier in the flow if we receive
a `null` key
ref https://linear.app/tryghost/issue/ENG-881/stripe-tax-checkout-instantiation-fails-for-free-members-when-choosing
- For existing customers to be able to upgrade their account with automatic tax enabled, we need to pass in `customer_update[address]:auto` as per Stripe documentation.
- Automatic tax calculation in Checkout requires a valid address on the Customer. Add a valid address to the Customer or set either 'customer_update[address]' to 'auto' or 'customer_update[shipping]' to 'auto' to save the address entered in Checkout to the Customer.
- We update the existing customer details by passing in address `auto` when they upgrade their accounts.
- Stripe captures the billing address information by default when new accounts are created and then that is used to calculate the tax rate.
fix https://linear.app/tryghost/issue/SLO-82/query-error-unexpected-character-in-filter-at-char-1
- previously, we weren't handling a parsing error, and just bubbling it
back up the chain
- this would result in an InternalServerError somewhere, which caused
500s
- we can handle this, because it's just a bad filter
- this adds handling so we return a 422 upon receiving an invalid filter
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
ref https://linear.app/tryghost/issue/ENG-881/stripe-tax-checkout-instantiation-fails-for-free-members-when-choosing
- For existing customers to be able to upgrade their account with automatic tax enabled, we need to pass in `customer_update[address]:auto` as per Stripe documentation.
- Automatic tax calculation in Checkout requires a valid address on the Customer. Add a valid address to the Customer or set either 'customer_update[address]' to 'auto' or 'customer_update[shipping]' to 'auto' to save the address entered in Checkout to the Customer.
- We update the existing customer details by passing in address `auto` when they upgrade their accounts.
- Stripe captures the billing address information by default when new accounts are created and then that is used to calculate the tax rate.
- 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
closes https://linear.app/tryghost/issue/MOM-60
- when the dropdown opens near the end of the document, clicking the links sometimes did nothing and showed an error in the console
- we have a mousedown event handler on an element that surrounds the main editing element that re-focuses the editor when clicked in order to make the "focus editor" click target larger and more natural-feeling but it was inadvertently re-focusing when the mousedown event fired for an element in the dropdown list when the list was positioned outside of the main editor element. This lead to timing issues with the bookmark node being removed on blur because it was empty followed by an error from the node's component's async event handlers which were trying to set values on the now-removed node
- by switching from `event.target.closest()` to looping over `event.composedPath()` when checking to see if we should skip re-focusing we're more resilient to DOM manipulations occurring between event triggers and function calls because we'll always be given the list of elements that existed at the time the event fired
reduce word size to fit properly within button without making style
changes (_economize_ and _poupe_ have the exact same meaning)
Co-authored-by: Ryan Feigenbaum <48868107+royalfig@users.noreply.github.com>
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.
ref https://linear.app/tryghost/issue/MOM-25
All these intersection types are getting a bit out of hand - but we can clean
up all of this once we're past prototyping phase.
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
no issue
- The `RedirectsImporter` used by the data generator was creating
redirects with the wrong length for the `from` field, which didn't match
the actual behavior of Ghost.
- This commit corrects the length from 32 to 8, which is the actual
length of the `from` field in production.
- This change has no impact on Ghost's behavior, but makes the data
generator more representative of real world data for more accurate
testing.
fixes
https://linear.app/tryghost/issue/DES-260/footer-link-text-smaller-than-regular-text
There was a bit of CSS in a media query aimed at other parts of the
newsletter template that was causing the footer styling to break. I
added some more specific styling for the footer as well, to make sure
span's within the `<p>` element are covered as well.
closes https://linear.app/tryghost/issue/MOM-49
- bumped koenig-lexical so the bookmark card has group support for testing
- updated `searchLinks` function passed to Koenig to match expected grouped results shape
ref https://linear.app/tryghost/issue/MOM-25
This matches the way that mastodon handles the key url and may be the reason
these documents are incompatible. This also removes the `username` key as that
isn't used anywhere, instead we have a username property which is rendered as
the ActivityPub compat preferredUsername key.
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
no issue
- updated search to add `status` to the search results
- added filtering to the editor's `searchLinks()` method
- prevented TaskCancellation errors being thrown from the search task being cast to a Promise
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
closes https://linear.app/tryghost/issue/MOM-1/
- added `feature.internalLinking` and `searchLinks` properties to the `cardConfig` object passed to the editor
- `searchLinks()` uses Admin's internal search to fetch and filter results
- called with no search term to obtain default links to show as soon as the bookmark card is inserted, in our case we show the last 5 published posts. Result is cached for the duration of the editing session to avoid API queries/loading state after the first fetch
- flattens search results for now because Koenig doesn't yet support grouped results
- bumps version of `@tryghost/koenig-lexical` to support the feature flag
ref https://linear.app/tryghost/issue/MOM-25
Whilst we're experimenting it's gonna be easier to not have to think about
caching affecting things. We'll disable it completely for now, and then decide
on a caching strategy that suits us down the line.
ref https://linear.app/tryghost/issue/MOM-25
This _might_ be the reason that Mastodon doesn't recognise our Actor, but
either way it's the correct thing to do so that JSON-LD parsers correctly
understand that publicKey field
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 https://linear.app/tryghost/issue/MOM-32
This adds the basic building blocks for an Outbox for an Actor, currently it's
hardcoded - which'll let us at lest test integration with other platforms.
JSONLDService is an awful name, but it's late and this is a prototype.
no-issue
This makes the code easier to understand and maintain, and reduces the overhead
of converting to/from a Map. It also changes the URLs and makes them path based
no-issue
This lets us have an unauthed endpoint for reading the outbox, long term we'll
probably wanna have this on the frontend URL but we don't have NestJS wired up
there yet.
no-issue
This is consistent with our main NestJS Module and allows the values to be
introspected by other code, rather than be stored internaly in decorator
metadata, which makes it easier to debug.
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.
ref DES-205
- label name is now used as the title on label pills instead of static
text
- label names will now be truncated when it takes more than 2 lines
instead of 1
ref https://linear.app/tryghost/issue/MOM-1
- renamed `searchable` to `groupName` so it better matches usage and avoids leaking internal naming to external clients
- added `url` to the fetched data for each data type as the editor will want to use front-end URLs in content
- added acceptance tests to help avoid regressions as we further generalise/optimise the search behaviour
- 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.
Got some code for us? Awesome 🎊!
Please include a description of your change & check your PR against this
list, thanks!
- [x] There's a clear use-case for this code change, explained below
- [x] Commit message has a short title & references relevant issues
- [x] The build will pass (run `yarn test:all` and `yarn lint`)
We appreciate your contribution!
Explanation: There are some missing accents in:
4c598a1e6d/ghost/i18n/locales/es/comments.json (L18-L19)
And
4c598a1e6d/ghost/i18n/locales/es/comments.json (L37)
Specifically in: Conviertete, Se. So instead of including accents I just
used simpler words so it sounds as a more natural translation, I have
already translated my whole newsletter https://crecimientoconsciente.co/
to Spanish I'm just finishing some wording details.
Also if you could please give a check to this
[comment](https://github.com/TryGhost/Ghost/issues/16628#issuecomment-1990569446)
in milestone 3 of translations for official support in email paywall
cta.
Co-authored-by: Ryan Feigenbaum <48868107+royalfig@users.noreply.github.com>
This PR adds Japanese translation to the comment resources
(ghost/i18n/locales/ja/comments.json). Currently, all of them are empty
and Japanese translations are not supplied.
The PR also adds Japanese translations to a few missing phrases in the
portal language resources (ghost/i18n/locales/ja/portal.json).
---------
Co-authored-by: Ryan Feigenbaum <48868107+royalfig@users.noreply.github.com>
This PR will add Persian language locale (fa/fa_IR) for Ghost
- [x] The build will pass (run `yarn test:all` and `yarn lint`)
---------
Co-authored-by: Ryan Feigenbaum <48868107+royalfig@users.noreply.github.com>
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
fix https://linear.app/tryghost/issue/ENG-805/
refs https://owasp.org/www-community/attacks/CSV_Injection
- it's possible for certain fields in a member CSV export to be executed
by software that opens the CSVs
- we can protect against this for the user by escaping any forumulae in
the CSV fields
- papaparse provides this option natively, so it's just a case of
providing the field to the unparse method
- credits to Harvey Spec (phulelouch) for reporting
closes https://linear.app/tryghost/issue/IPC-117/fix-ghost-orb-logo-not-being-animated-in-chrome-or-arc
- Chrome wasn't respecting the `muted` attribute when the dashboard is loaded without any interaction resulting in the video not auto playing
- fixed by adding a `{{autoplay}}` modifier that explicitly sets the `muted` property on the video before calling `.play()` which appears to bypass the interaction-required block
