ref 8ea1dfb
ref https://linear.app/tryghost/issue/ONC-111
* undid the reversion for the performance improvements
* built upon new tests for the posts list functionality in admin,
including right click actions
* added tests for pages view in Admin
This was reverted because it broke the Pages list view in Admin, which
is a thin extension of the Posts functionality in admin (route &
controller). That has been fixed and tests added.
This was originally reverted because the changes to improve loading
response times broke right click (bulk) actions in the posts list. This
was not caught because it turned out we had near-zero test coverage of
that part of the codebase. Test coverage has been expanded for the posts
list, and while not comprehensive, is a much better place for us to be
in.
fixes https://linear.app/tryghost/issue/ONC-206
ref https://app.incident.io/ghost/incidents/90
- when multiple member filters are used in combination, NQL sometimes
hit a limitation that results in the wrong members being returned
- while we work on the NQL limitation, we are temporarily disabling bulk
member deletion when more than one member filter has been applied
no ref
- while reviewing the newsletter flows, it was apparent that we were
missing test coverage
Some of the tests in Portal are a bit redundant with tests added for
child components, but it didn't seem worth removing them after getting
them to work. There was a bug in our Portal fixture data that requires a
few changes, as well as some small adjustments for making tests easier
(testing-lib-react has `getByTestId` and simply a `querySelector` to use
alternate test attributes).
ref
https://linear.app/tryghost/issue/PA-71/remove-cache-bust-from-projs-in-admin
ref
15ed2eb245
- This cache buster was added in March to mitigate a client side error in pro.js,
to effectively force browsers to redownload the fixed version of the file.
- It's not needed anymore, as the error has been fixed for a few months
now, so we can safely remove it.
ref https://linear.app/tryghost/issue/SLO-160
- in the Members list, we were using VerticalCollection with
`@bufferSize` set to 20, which means that 20 additional items before and
after the visible items in the viewport were pre-loaded
- however, scrolling down too quickly (e.g. dragging the scrollbar thumb
to the bottom) breaks the list
- with this fix, we adjust `@estimateHeight` parameter to the correct
item size, and reduce the `@bufferSize`
REF MOM-315
- Changed to column layout
- Fixed broken currency dropdown
- Included a link to Stripe terms & conditions
- Renamed from "Tips or donations" to "Tips & donations"
Right now identity tokens can only be fetched by the Owner, which means they
implicitly have the Owner role, but we want to expand that. The first step is
adding the role to the token, and then we need to update each place which uses
the token and add an assertion that the role is correct.
- right now, it loops through all packages serially, which isn't
effectively using multi-core machines
- by using `concurrently`, we can rely on it to use all the cores it
can, so this should dramatically speed up the bundling step
ref DES-571
- padding does not work well with paragraph inside blockquote as
horizontal spacing on Outlook
- using margin instead of padding makes sure the spacing is consistent
across Outlook versions
ref DES-571
- iOS Mail app ignores spacing on the \<blockquote\> element, but will
respect spacing on the \<p\> element inside it
- for that reason, we started to enforce always rendering \<p\> inside
\<blockquote\> for emails
- these changes move the spacing related styles from blockquote to p
inside
closes https://linear.app/tryghost/issue/ENG-1432
- bumps `kg-lexical-html-render` package with required rendering change
- bumps `koenig-lexical` with TypeError fix and improved handling of failed image uploads
- bumps other packages that were missed from previous bumps, fixes split versions of underlying lexical packages
fixes https://linear.app/tryghost/issue/ONC-189
- commit 4084a3d introduced a regression that caused member subscription
details to not be rendered for active/canceled subscriptions
- with this fix, the rendering logic in Admin for member subscription
details has been fully moved to a helper and is now covered by
additional unit tests
ref https://github.com/TryGhost/Ghost/pull/20503
- undid the reversion for the performance improvements
- built upon new tests for the posts list functionality in admin,
including right click actions
This was originally reverted because the changes to improve loading
response times broke right click (bulk) actions in the posts list. This
was not caught because it turned out we had near-zero test coverage of
that part of the codebase. Test coverage has been expanded for the posts
list, and while not comprehensive, is a much better place for us to be
in.
ref e626dd9
There has been some flakiness in Github CI with the new tests for the
probe library. We'll start with extending timeouts in case CI is running
particularly slowly.
ref https://linear.app/tryghost/issue/ENG-1360
Not *all* functionality has been covered by these tests. There's a few
missing pieces from our mirage build and use that likely doesn't need
full coverage within the admin package. Regardless, this view has
dramatically more coverage at this point.
ref https://linear.app/tryghost/issue/ENG-1254
- when a subscription is canceled automatically by Stripe (e.g. due to
multiple failed payments), we now send a staff notification
- logic before: if a member cancels a sub in Portal, then send a staff
notification
- logic now: if a subscription was active, but is now set to cancel
immediately or at the end of the billing period, then send a staff
notification.
- with that logic change, we now send a cancellation staff notification
when:
1. A member cancels their sub in Portal (existing)
2. A staff member cancels a member sub in Stripe (new)
3. A staff member cancels a member sub in Admin (new)
4. A sub is canceled automatically by Stripe because of multiple failed
payments (new)
- the copy of the staff notification email has also been updated to take
into account 1) manual vs automatic cancellations, and 2) immediate vs
end of billing period cancellations
ref https://linear.app/tryghost/issue/ONC-160
- POST is incorrect as the form itself doesn't post to any path; all we
want are the authentication flows to kick off on submit
We've had reports of users experiencing a 404 error on attempting to
sign in to Ghost Admin (at /ghost/), where the login form seems to
submit a POST request to the /ghost/ path (we don't have a route for
that method, hence the 404; only GET). While I haven't been able to
reproduce the issue, there's very few places in Ghost that actually
issue a POST request.
Removing this method here has no impact to Ghost auth and may prevent
some unexpected default behavior from the browser.
ref https://linear.app/tryghost/issue/ENG-1408/
- added additional safeguards to the image size dimensions probing
For some reason that requires further investigation, the
probe-image-size package was silently failing (neither resolving nor
rejecting) for a particular URL. This was causing Ghost to hang on to
serving the request, and after a few of these came in, ultimately caused
Ghost to stop being responsive.
Rather than trying to patch a dependency, we'll wrap the call to this
package and use the same timeout we pass into the package (which is
ignored in this particular case) as an additional safeguard.
ref DES-263
- we've recently started forcing white background color to the bookmark
card by default
- the reason was making it look good regardless of the site background
color
- it caused an issue to some sites, mostly in dark mode, because the
text color was inherited from the theme
- this sets explicit color to the bookmark content which is consistent
with the nft card
refs
[ENG-1355](https://linear.app/tryghost/issue/ENG-1355/site-boot-cycling-due-to-free-tier-having-a-currency)
Fixed data importer allowing invalid free product to be imported which
could cause Ghost to not start due to the error:
`ValidationError: Free Tiers cannot have a currency`
It should not be possible to import a free product with pricing data (as
that means its not free 😄)
ref
https://linear.app/tryghost/issue/ENG-1251/support-escalation-re-offers-not-tracking
- Offer Redemptions were not being persisted in the database for
existing free members who upgrade to a paid plan with an offer, which
resulted in inaccurate offer redemption counts. This made it difficult
to assess the performance of an offer.
- Previously, Ghost recorded an offer redemption in the DB in response
to the `SubscriptionCreatedEvent`, under the assumption that the offer
details would be included in this event. This assumption was valid for
brand new members starting a subscription with an offer, but not for
existing free members upgrading to a paid plan with an offer.
- For existing free members, the subscription is first stored in Ghost
in response to the `customer.subscription.created` Stripe webhook. At
this point, the offer/discount is not attached to the subscription, so
the `SubscriptionCreatedEvent` triggers without the offer information,
and the offer redemption is not recorded. After the
`checkout.session.completed` webhook is received (which _does_ include
the offer details), the subscription is updated in Ghost, but the Offer
Redemption is not stored.
- For brand new members, the `customer.subscription.created` webhook
no-ops, because the member and Stripe Customer don't exist yet.
Therefore, the subscription is first created in Ghost in response to the
`checkout.session.completed` webhook, which _does_ include the offer
information, so the offer information is included in the
`SubscriptionCreatedEvent` and the offer redemption is recorded as
expected.
- This change adds a new `OfferRedemptionEvent`, which triggers
either: (1) when a new subscription is created with an offer (as in the
case of a brand new member), or (2) when an existing subscription is
first updated to include an offer (as in the case of an existing free
member upgrading with an offer). The Offer Redemption is then persisted
in the DB in response to the `OfferRedemptionEvent` rather than the
`SubscriptionCreatedEvent`.
ref https://linear.app/tryghost/issue/ENG-1266
- Mexico changed tz to not participate in DST
- our package was a couple years behind, so we likely have fixes for
other countries/regions, too
ref https://linear.app/tryghost/issue/ONC-154
- the query params did not carry through on portal sign up links because
of the hash creating an ignored fragment
(/#/portal/signup?ref=something)
Now when we check link attribution, we'll attempt to run the same logic
for the referrer source after stripping out `#/portal` from the URL.
Otherwise we should continue to treat these fragments as fragments to be
ignored by the client.
NOTE: We do not have e2e tests that cover member signup on the front end
and the data entered in the back end. The tests we have mock only the
server side of things. The test added here only covers the data that is
generated from the front end request (at this time), *not* the front end
request itself, meaning it's fragile.
Fixes
https://linear.app/tryghost/issue/DES-324/complimentary-plan-issues
We were showing renewal copy for subscriptions that are forever
complimentary. We also had a trailing en-dash in the Member detail
screen when their subscription was complimentary and had no end date.
Those things are solved now. We don't show dates or renewal copy when we
don't need to.
ref https://ghost.slack.com/archives/CTH5NDJMS/p1720422460943619
- bumping 'lexical' from 0.13.1 to 0.14.2 created a few selection bugs
in the editor
- this commit reverts 'lexical' back to 0.13.1 and any related changes
in the editor codebase
ref ENG-945
- Fixed an issue where upload a broken redirects yaml will override the
last working yaml.
- Instead it will now do the validation before saving and overriding the
yaml.
no issue
- we're no longer making use of the websockets experiment so it's just bloat
- this is the whole feature in a single commit in case we need to revive it at some point
- this change contains the removal of the `promise.allsettled` package,
as this is not needed on Node 12+, which removes 75 further dependencies
in production mode
ref https://linear.app/tryghost/issue/ENG-1254
- we currently only store a cancellation reason when a member cancels
manually in Portal
- we now also store "Payment failed" when the cancellation is automatic
due to several payment failures
refs
[ENG-709](https://linear.app/tryghost/issue/ENG-709/%F0%9F%90%9B-bad-redirects-causing-container-tear-down)
Added validation to prevent RegEx's susceptible to ReDoS from being used
with custom redirects. Also moved error details out of `context` and
into `errorDetails` to be consistent with error logging elsewhere as
well as fix issue in admin-x where blank screen would be shown when an
error occurred during redirects upload (due to logic not accounting for
`context` being an object)
fixes https://linear.app/tryghost/issue/ENG-642
- When a subscription is in the `canceled` state the corresponding
Member has no access to the Ghost site. The only time a Member will
continue to have access if their subscription is due to cancel at the
period end is if it is still in an active state, which is one of
`active` `trialing` `unpaid` or `past_due`
- When a subscription is canceled immediately (i.e. before the end of
the current billing period), we now render "Ended" without a date,
because we don't store the cancellation date in the subscription object.
We previously used "Ended {current_period_end}" which would sometimes
lead to dates in the future
- Bonus: refactored code and added unit tests
---------
Co-authored-by: Sag <guptazy@gmail.com>
- the existing code creates a new moment instance, takes away some days
and then formats the result
- this is run for every entry of the member attribution stats, which
means dashboards for big sites with a lot of attribution data become
slow
- this value doesn't change across each iteration of the filter, so we
can just extract it out and calculate it once
- this commit removes this code block from the flamegraph completely
fix https://linear.app/tryghost/issue/ENG-779/%F0%9F%90%9B-cmds-does-not-save-member-profile-changes
- previously, pressing Cmd+S on a member profile would save the profile,
but the dirty attributes weren't being cleaned, so the application
would trigger the leave confirmation when exiting
- now, we've fixed the code to keep a dynamic scratch member,
- long term, we should get rid of the scratch model, but this still
allows us to fix the bug for now
closes https://linear.app/tryghost/issue/ENG-1255
- updated Koenig packages including:
- addition of `/preview` for public preview card
- fix for HTML import from Google Docs
- fix for embed thumbnails being cut off in email
- fix for wide image card width on medium screens
- multiple fixes for unhandled (but non user-visible) errors causing noise in console and error logging
ref https://linear.app/tryghost/issue/ONC-111
- changed posts fetching/display behavior to be client-side instead of server-side
- admin will issue (potentially multiple) requests based on the desired status(es)
- updated admin acceptance test for missing coverage
I've pulled the sort from the database query as this triple sort
performs very poorly at scale (taking ~4s+ past ~20k posts sometimes).
Instead, we now split up the fetch to grab only one status at a time and
use the front-end logic to handle displaying scheduled, then drafts,
then published. This should result in a much more responsive view.
We will separately change the default sort on the Admin API as that was the ultimate intent for this change.
- `faker` was the original dependency but the maintainer ended up
deleting the repo, so development continued in `@faker-js/faker`
- we're already using that dependency, so we can make a few simple
changes and remove the old dependency from our repo
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.
fix https://linear.app/tryghost/issue/SLO-168/rangeerror-maximum-call-stack-size-exceeded
- this code takes the API output and reduces it down to collect together
stats per date
- the current code is recursive, and we've seen errors with the
recursion hitting a `RangeError: Maximum call stack size exceeded`
error
- as well as that, we're doing a lot of array concat'ing and cloning,
which burns memory and CPU time
- instead, we can just use `.reduce`
- the new implementation is much faster than the existing one (1ms vs
85ms) and uses no recursion, so those errors should go away
- I've also verified that the output is the same between the two
functions
ref ONC-109
- Attempt to fix flaky Admin test, "Publish flow members enabled can
schedule publish+send"
- Adjusted the time calculation to the nearest minute to avoid
off-by-one minute errors
- Added `waitFor` to ensure elements are present and stable before
making assertions.
- Rounded the new scheduled date and time to the nearest minute to
maintain consistency
- Included extra `waitFor` and `settled` calls to allow time for UI
elements to fully load and reflect changes before assertions.
REF https://linear.app/tryghost/issue/MOM-238
- Updated feature image action button styles
- Aligned button and tooltip styles with the rest of the editor
- Updated `koenig-lexical` version to pull in new toolbar design
fixes https://linear.app/tryghost/issue/SLO-156
- we have reached our 10k replays per month quota in 20 days, by using a
100% error sampling rate
- we would need a sampling rate < 0.64% to stay under the quota
- from now on, we will be using a 50% error sampling rate to have a bit
of margin, and have a rounder number that is easier to reason about (1
out of 2 error sessions are recorded in Sentry)
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/SLO-127
- problem: contributors see an empty list of labels in the Signup card,
even if some exist
- cause: contributors do not have permission to browse labels
- solution: hide the label input entirely for contributors in the Signup
card, based on the new `renderLabels` config parameter
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.
https://github.com/TryGhost/Ghost/issues/20445
- after switching to a browser-like user-agent, YouTube started responding with a "supported browsers" message rather than the actual video meaning bookmark creation failed
- when trying other user-agents it was discovered that nytimes.com (why the user-agent was originally changed) had a problem with the github.com address in the user-agent. By switching to using https://ghost.org instead the request was allowed through fixing both YouTube and NYTimes embeds
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
closes https://linear.app/tryghost/issue/ENG-762
- nytimes.com and other sites return 403 responses when requests do not match typical browser user-agents
- our bookmark fetching requests were using `Ghost(https://github.com/TryGhost/Ghost)` meaning bookmark creation failed for these user-agent-blocking sites
- switched to using a standard browser user-agent string to avoid such blocks
closes https://github.com/TryGhost/Ghost/issues/20028
It's fairly common practice for oembed providers to skip some of the "required" fields from the oembed spec such as `height` when it doesn't make sense for the embeddable content, this was the case with Bluesky embeds which return `height: null`
- removed validation for `height` being present in the response for it to be recognised as an embed because we don't use it anywhere and the validation is blocking otherwise valid embeds
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
ref https://linear.app/tryghost/issue/SLO-127
- problem: when using a card with a button (Button, Email CTA, Header,
Product), the Button URL suggestions fail to load for Contributors,
Authors, and Editors
- cause: Contributors, Authors and Editors don’t have permission to
fetch offers, and this causes the entire list of button url suggestions
to break
- solution: if offers fail to fetch for any reason, the rest of the url
suggestions for cards with a button is now still populated (i.e. offers
URLs are ignored)
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.
ref https://linear.app/tryghost/issue/ONC-94
- adds Sentry logging for the unsaved changes modal in Admin (tags,
members, collections)
This will let us monitor for spikes in rates so we know if a change to
Ghost has caused faulty logic in detecting dirty models.
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/ENG-1211
- the post slug now re-generates based on the post title for draft posts
unless manually set
- updated unit tests to be a bit more comprehensive
It's frequently the case that draft posts might have WIP titles. We
would generate a post slug based on the title and never change it, so
unless writers remembered to come back in to update it for their final
post, it could look off to readers. This should make that a bit more
intelligent.
Going forward, we will change the slug unless we expect it to be a custom slug (user-set in the side panel). If the title is cleared out and saved, we will also reset it. We will only ever automatically generate & change the slug for draft posts.
closes https://linear.app/tryghost/issue/ONC-96
- better matches Ghost's current usage
- avoids mismatch in expected data when `post_revisions` is populated in future
ref https://linear.app/tryghost/issue/ENG-661
- added a dirty check to ignore the `direction` field from the lexical
object; this is set dynamically and shouldn't be serialized, see
facebook/lexical/issues/4998
- fixed a bug where the modal wouldn't display on leaving the editor if
the post had no revisions (e.g. import); this could result in content
being saved over published content with no user action
- added Sentry logging for the modal
We would sometimes see the "Are you sure?" modal pop up when opening a
post in the editor and attempting to navigate away immediately, without
any changes to the post. This appears to be an issue with the serialized
Lexical data, which would change after loading into the editor,
resulting in the scratch and model's lexical values to differ, making
Admin think the user changed the content.
Ideally we'll see a fix upstream (or fix it ourselves). We may need to
revisit this if we experience other such situations. It's awfully
difficult to be able to set a flag saying 'the editor is done loading',
so this seems to be the best path for the moment.
Testing is difficult because we don't actually load the new Lexical editor into e2e/acceptance tests. I've added a unit test that can at least simulate the editor state changing on editor load.
no issue
- we weren't adding an `order` param to our posts/pages requests used to populate the search index which meant the default Admin API ordering was applied which isn't optimal for this use-case
- switched to ordering by `updated_at` to use a simple order that has an optimised index in the database
