- without this, Node will try and resolve the domain name but local DNS
resolvers can take a while to timeout, which causes the tests to timeout
- `nodemailer-direct-transport` calls `dns.resolveMx`, so if we stub that
function and return an empty array, we can avoid any real DNS lookups
refs https://github.com/TryGhost/Toolbox/issues/523
- The reverted fix did not take into account the "original path" of the
files would be truncated. This path has to be full relative to the root
of the zip to later be used during importer url substitution logic.
- This reverts commit 831a76505c.
Because there is no guarantee about a daily job running exactly once a
day, we need to store the last time that the email was sent, so that we
can refrain from sending one if it's been less than a day since the
last.
A setting has been used for this as we don't currently have a pattern
for it, we might want to consider moving this to some kind of cache
based solution in future. This has been added as a core setting so that
we don't expose it via the API.
The setting is stored as a number to allow us to store value as unix timestamp.
---------
Co-authored-by: Rishabh <zrishabhgarg@gmail.com>
no issue
When using `getLazyRelation` on an optional relation that is not set, it
will return a newly created model instead of a model from the database.
- Adds a new require option to `getLazyRelation`, that throws an error
if the relation is not set (off by default to match existing use cases)
- This caused a bug (not visible because we always pass a newsletter id)
in email previews, where when the newsletter id was not explicitly set,
it would use `newsletter = (await post.getLazyRelation('newsletter')) ??
(await this.models.Newsletter.getDefaultNewsletter());`, which always
returned the first one, and could return a newly initiated newsletter
with all properties set to undefined.
- Some page snapshots are altered by this, because the usage of
`getLazyRelation` on a post no longer sets the email relation to some
new model.
- this cleans up all imports or variables that aren't currently being used
- this really helps keep the tests clean by only allowing what is needed
- I've left `should` as an exemption for now because we need to clean up
how it is used
closes https://github.com/TryGhost/Team/issues/2531
This commit fixes the issue where non-canonical URLs are included in the
XML sitemap, leading to poor SEO for our user's sites. The solution
implemented is to exclude any page or post that specifies a canonical
URL in its metadata from the sitemap.
To achieve this, a condition has been added to the 'addUrl' method,
which checks for the existence of a canonical URL in the metadata of the
resource being added to the sitemap. If a canonical URL is present, the
resource is excluded from the sitemap.
With this fix, our user's sites will have better SEO and improved search
engine visibility.
no issue
- Nock doesn't support multiple calls to enableNetConnect -> only the last one counts. This fixes that issue.
- Some tests interacted directly with nock instead of using the mockManager to restore everything.
https://github.com/TryGhost/Toolbox/issues/523
- During import process of content files the files from the root directory were also copied over. This is causing chaos in the root of content folder with files that only needed for data import. For example, the csv files needed for Revue import were also copied over by "file importer" even though those do not belong to any content.
- Any content import files - images, media, files, should be in according folders in the imported zip file. The root files in the base zip directory are for data-related imports
fixes https://github.com/TryGhost/Team/issues/2611
The old email flow is no longer used since we introduced the email stability flow. This commit removes the related code and tests. The general test coverage decreased a bit as a result, because the old email flow probably had a high test coverage. The new flow is in separate packages, so it couldn't contribute to a higher test coverage (but it does have 100% unit test coverage).
refs https://github.com/TryGhost/Team/issues/2667
Some tests still accessed the internet. Now network access is disabled
by default. This change also introduces two helper methods related to
networking (mocking Slack and Mailgun).
This fixes two unreliable tests:
- Staff service was accessing a Slack test API -> timeout possible
- MentionSendingService was trying to send webmentions for every post
publish/change -> possible timeouts and job issues
refs: https://github.com/TryGhost/Toolbox/issues/389
Instead of logging errors, this will warn when adding a duplicate URL in the test environment.
At the moment, this is happening a lot in the test suite. While we also need to fix the root cause of this so we're not erroring in the product, it's a massive amount of spam in the logs when running the test suite which could prevent us from finding other errors which are causing issues.
refs https://github.com/TryGhost/Toolbox/issues/523
- When migrating or importing ZIP files into Ghost there's often a need to include document files.
- When document files are present in the imported zip file they are now copied across and processed along with the rest of import files: json, images, csvs, etc.
- The importer also searches for use of the document files in the imported "posts" substituting the links with local ones
- The document files importer recognizes media files inside of "files" or "content/files" folders present in the zip.
- The supported media file extensions are same as for file upload widget:
".pdf",".json",".jsonld",".odp",".ods",".odt",".ppt",".pptx",".rtf",".txt",".xls",".xlsx",".xml"
with following content-types:
"application/pdf", "application/json", "application/ld+json", "application/vnd.oasis.opendocument.presentation", "application/vnd.oasis.opendocument.spreadsheet", "application/vnd.oasis.opendocument.text", "application/vnd.ms-powerpoint", "application/vnd.openxmlformats-officedocument.presentationml.presentation", "application/rtf", "text/plain", "application/vnd.ms-excel", "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", "application/xml", "application/atom+xml"
refs https://github.com/TryGhost/Toolbox/issues/523
refs c2534e3c86/packages/mg-assetscraper/lib/AssetScraper.js (L14-L16)
refs https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types
- Importer needs to process and recognize document files like pdfs, presentations etc to be able to import them into sites file storage.
- The handler allows a new root directory "files" to place imported documents
- The handler adds validation and processing for following file extensions:
".pdf",
".json",
".jsonld",
".odp",
".ods",
".odt",
".ppt",
".pptx",
".rtf",
".txt",
".xls",
".xlsx",
".xml"
- With following content types:
"application/pdf",
"application/json",
"application/ld+json",
"application/vnd.oasis.opendocument.presentation",
"application/vnd.oasis.opendocument.spreadsheet",
"application/vnd.oasis.opendocument.text",
"application/vnd.ms-powerpoint",
"application/vnd.openxmlformats-officedocument.presentationml.presentation",
"application/rtf",
"text/plain",
"application/vnd.ms-excel",
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
"application/xml",
"application/atom+xml"
refs https://github.com/TryGhost/Toolbox/issues/523
- When migrating or importing ZIP files into Ghost there's often a need to include more than just post content and images.
- When media files are present in the imported zip file they are now copied across and processed along with the rest of import files: json, images, csvs, etc.
- The importer also searches for use of the media files in the imported "posts" substituting the links with local ones
- The media importer recognizes media files inside of "media" or "content/media" folders present in the zip. The supported media file extensions are same as for media upload widget:
".mp4", ".webm", ".ogv", ".mp3", ".wav", ".ogg", ".m4a"
with following content-types:
"video/mp4", "video/webm", "video/ogg", "audio/mpeg", "audio/vnd.wav", "audio/wave", "audio/wav", "audio/x-wav", "audio/ogg", "audio/mp4", "audio/x-m4a"
refs https://github.com/TryGhost/Toolbox/issues/523
- This is ground work before introducing a "media" content type importer
- Previous "image" file name was not describing well what the importer was capable of doing
refs https://github.com/TryGhost/Toolbox/issues/523
- We need to be able to use different storage mechanisms when importing different types of content
- Having the storage passed in using constructor DI allows to have more flexible storage mechanism in the Images importer (soon to become a generic file importer)
refs https://github.com/TryGhost/Toolbox/issues/523
- The class syntax would allow swapping out the storage mechanism in the importer making it universal to use with other file types like media or generic files.
refs https://github.com/TryGhost/Toolbox/issues/523
- The "importer/index.test.js" test suite is testing more than it should. ImageHandler test suite section is one of the examples of test cases that should live in a separate file.
- Having these tests in different files makes it easier to reason about coverage and extract to it's own packages.
refs https://github.com/TryGhost/Toolbox/issues/523
- When a zip file is imported into Ghost we need to recognize and process media files with following extensions:
".mp4",".webm", ".ogv", ".mp3", ".wav", ".ogg", ".m4a"
- The media files can come from a "media" or "content/media" folder inside of zip file
closes https://github.com/TryGhost/Ghost/issues/16332
Passing `SafeString` input to `asset` helper was resulting in the
exception being thrown. This meant that we couldn’t combine `asset`
helper with other helpers which produce `SafeString` e.g. `concat`
helper for string concatenation.
refs: https://github.com/TryGhost/Toolbox/issues/389
Calling validate always uses the cache system, so this commit makes sure that the cache system is always initialised correctly by the tests.
no issue
- Reduced the amount of diffeerent properties by not populating a `currentARR` and `currentMembers` fields, but use a `currentValue` instead.
- The type of milestone can still be determined by its `type` property, so we actually don't need two different props here
refs https://github.com/TryGhost/Toolbox/issues/522
- Having simpler method signature makes it easier to use it in different context - needed for changes in public resource repository
- TLDR of the changes - reduced parameter 'frame.options' -> 'options'
Refs TryGhost/Team#2459
-upgraded got from v9.6.0 to v11.8.6 to support following redirects (and
other fixes)
-got v12+ requires ESM, so we do not want to upgrade further at this
time
-required changes to a few libraries that use externalRequests
-mention discovery service tests updated to test for follow redirects
refs
https://www.notion.so/ghost/Marketing-Milestone-email-campaigns-1d2c9dee3cfa4029863edb16092ad5c4?pvs=4
- Added a `slack-notifications` repository which handles sending Slack
messages to a URL as defined in our Ghost(Pro) config (also includes a
global switch to disable the feature if needed) and listens to
`MilestoneCreatedEvents`.
- Added a `slack-notification` service which listens to the events on
boot.
- In order to have access to further information such as the reason why
a Milestone email hasn't been sent, or the current ARR or Member value
as comparison to the achieved milestone, I added a `meta` object to the
`MilestoneCreatedEvent` which then gets accessible by the event
subscriber. This avoid doing further requests to the DB as we need to
have this information in relation to the event occurred.
---------
Co-authored-by: Fabien "egg" O'Carroll <fabien@allou.is>
no issue
- The way we're going to implement milestones diverged from the original idea of handling email sending within the milestone-emails package, as we'll be sending events instead and will utilise the StaffService to listen to them and send the emails
- This renames the package as well as the service in core itself and all relevant tests
closes https://github.com/TryGhost/Team/issues/2558
- bumped `kg-lexical` packages so we're working with latest suite of default nodes and renderer
- added a `render()` method directly to our `lexicalLib` object
- allows us to pass through all of Ghost's config for image transforms etc in one place rather than every time we want to render something
fixes https://github.com/TryGhost/Team/issues/2542
fixes https://github.com/TryGhost/Team/issues/2543
fixes https://github.com/TryGhost/Team/issues/2544
- Hides incomplete subscriptions
- Shows Past Due subscriptions
- Fixed UI issues with 3+ subscriptions
- Fixed missing complimentary subscription when one subscription was
incomplete/inactive
- Fixed sending a paid subscription started email for incomplete
subscriptions. This change also required us to actually send the email
when the incomplete subscription eventually becomes active. So the
introduction of a new `SubscriptionActivatedEvent` made sense/was
required (because sending a SubscriptionCreatedEvent again would cause
other issues).
refs https://github.com/TryGhost/Team/issues/2534
This is so that we can support soft deletes for Mentions.
We need to add the defaults to the model so that write continue to work.
Co-authored-by: Fabien "egg" O'Carroll <fabien@allou.is>
refs https://github.com/TryGhost/Team/issues/2526
- created a migration for a new boolean column in users that would
determine if the staff user gets an email when the publication receive a
new mention.
refs https://github.com/TryGhost/Toolbox/issues/497
refs fb7532bf5d
- We downgraded the 'GS090-NO-PRICE-DATA-CURRENCY-CONTEXT' rule in gscan to non-fatal, meaning Ghost should not be throwing an error but instead render an empty value for {{price}} helper when price data is empty.
- For example, a legacy syntax like this: '{{price currency=@price.currency}}' should not cause a page render error but return an empty price string.
- The pattern of returning an empty string instead of crashing is used in other helpers like {{img_url}} and and {{url}}
closes https://github.com/TryGhost/Team/issues/2420
- Added user roles and permissions for the mentions admin API.
- We only have a `browse` function for our current use case, accessible
by `administrator` and `admin integration`.
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
no issue
There are a couple of issues with resetting the Ghost instance between
E2E test files:
These issues came to the surface because of new tests written in
https://github.com/TryGhost/Ghost/pull/16117
**1. configUtils.restore does not work correctly**
`config.reset()` is a callback based method. On top of that, it doesn't
really work reliably (https://github.com/indexzero/nconf/issues/93)
What kinda happens, is that you first call `config.reset` but
immediately after you correcty reset the config using the `config.set`
calls afterwards. But since `config.reset` is async, that reset will
happen after all those sets, and the end result is that it isn't reset
correctly.
This mainly caused issues in the new updated images tests, which were
updating the config `imageOptimization.contentImageSizes`, which is a
deeply nested config value. Maybe some references to objects are reused
in nconf that cause this issue?
Wrapping `config.reset()` in a promise does fix the issue.
**2. Adapters cache not reset between tests**
At the start of each test, we set `paths:contentPath` to a nice new
temporary directory. But if a previous test already requests a
localStorage adapter, that adapter would have been created and in the
constructor `paths:contentPath` would have been passed. That same
instance will be reused in the next test run. So it won't read the new
config again. To fix this, we need to reset the adapter instances
between E2E tests.
How was this visible? Test uploads were stored in the actual git
repository, and not in a temporary directory. When writing the new image
upload tests, this also resulted in unreliable test runs because some
image names were already taken (from previous test runs).
**3. Old 2E2 test Ghost server not stopped**
Sometimes we still need access to the frontend test server using
`getAgentsWithFrontend`. But that does start a new Ghost server which is
actually listening for HTTP traffic. This could result in a fatal error
in tests because the port is already in use. The issue is that old E2E
tests also start a HTTP server, but they don't stop the server. When you
used the old `startGhost` util, it would check if a server was already
running and stop it first. The new `getAgentsWithFrontend` now also has
the same functionality to fix that issue.
refs https://github.com/TryGhost/Team/issues/2476
When upgrading from a Complimentary subscription with an expiry, to a paid Subscription of the same Tier, the Member was eventually losing access to the Tier when the complimentary subscription expires as the `expiry_at` on the mapping was not removed. This change fixes the code by setting expiry as null when a member upgrades their subscription to paid. This also adds 2 migrations to fix any side-effects on existing sites -
- Removed invalid expiry tier expiry date for paid members
- Restored missing tier mapping for paid members
We were incorrectly handling a "no resource found" return value from the
ResourceService, instead of an object with `null` values, we were expecting a
`null` value - so we were considering all URL's to be pointing toward a
resource.
refs https://github.com/TryGhost/Team/issues/2466
Now that we're checking for resources at the URL and rejecting if
there isn't one found, we want to make sure that we can handle pages
which are not a resource.
The idea here is to make a HEAD request to determine whether or not
the page exists. We don't need the full response so HEAD saves us some
bandwidth and we allow both 2xx and 3xx status codes because Ghost has
redirects to add missing trailing slashes, which may not be present in
the URL we're passed.
refs https://github.com/TryGhost/Team/issues/2466
The existing implementation was a very basic check to get us to the
first milestone. By checking if the page points to a resource we can
know for sure the URL exists on the site.
refs https://github.com/TryGhost/Toolbox/issues/503
- There was an error thrown due to empty "model._changed" field
- When attached or detached events (e.g. tag.attached) are sent through, their models do not contain any _changed properties. This was taken into account when checking for route related resource changes
refs https://github.com/TryGhost/Toolbox/issues/503
- The listener was not covered during quick and dirty implementation. While in the area did some cleanup to the sitemap manager test
- One of the problems I've stumbled upon when adding a test is having multiple instances of SiteManager in the test, which in turn created multiple "subscribe" events and repeat handle executions. Fixed it by having just one site manager instance (a singleton) as that's the pattern that used in main codebase
refs https://github.com/TryGhost/Toolbox/issues/503
- Full URL regeneration process was happening even when only unrelated to URL generation fields were updated (e.g. 'plaintext' change in post does not affect the URL of the post). Stopping the "resource updated" event processing early circumvents full url regeneration inside of DynamicRouting, which can be quite heavy depending on routing configuration
- The URLResourceUpdatedEvent is supposed to be emmited whenever there's an update to the resource already associated with the URL and no url-affecting fields were touched.
fixes https://github.com/TryGhost/Team/issues/2432
Adds outbound_link_tagging setting (enabled by default and behind
feature flag). If the feature flag is enabled, and the setting is
disabled, we won't add ?ref to links in emails.
This includes new E2E tests for email click tracking, which were also
extended to check outbound link tagging (for both MEGA and the new email
stability flow).
Also fixes a test fixture for the comments_enabled setting.
This allows us to share the implementation with other parts of the codebase, the
specific usecase here being fetching the metadata from webmention sources, for
display in the mentions UI, which will be borrowing a lot of stuff from the
bookmark card.
refs https://github.com/TryGhost/Team/issues/2400
- we've deemed it useful to start to return `Content-Version` for all
API requests, because it becomes useful to know which version of Ghost
a response has come from in logs
- this should also help us detect Admin<->Ghost API mismatches, which
was the cause of a bug recently (ref'd issue)
We've wrapped both changes in a try/catch to make sure this has no
adverse affects. The endpoint currently doesn't exist - we're only
adding this to get an idea of how much traffic we'll expect to see.
Long term we'll want to read the endpoint from the webmention service.
refs https://github.com/TryGhost/Team/issues/2393
- During boot and loading the active theme, we now cache the result of
the gscan validation. Cache configuration can happen in
`adapters.cache.gscan`
- We now also return non-fatal errors when activating or adding a theme.
- When the `themeErrorsNotification` feature flag is on, we fetch the
active theme (which includes the validation information) when loading
admin
- If the currently active theme has errors, we show an error
notification that can open the error modal
- Added a new endpoint: `/ghost/api/admin/themes/active/` that returns
the result of the last gscan validation of the active theme. If no cache
is available, it will run a new gscan validation.
- Added new permissions for the active action/endpoint (author, editor,
administrator)
refs https://github.com/TryGhost/Toolbox/issues/497
- During gscan fatal error downgrade to non-fatal some of the deprecated helpers were a bit vague to debug with no information on which exact "resource" was invalid
- Added resource name to the log for clarity. Should make life easier when debugging potential get helper misuses
no issue
With the increased usage of DomainEvents, it gets harder to build
reliable tests without having to resort to timeouts. This utility method
allows us to wait for all events to be processed before continuing with
the test.
This change should speed up tests and make them more reliable.
It only adds extra code when running tests and shouldn't impact
production.
There are currently two issues with the suppressions table:
- We have some incorrect rows
- We have missing UNIQUE constraints
We want to completely wipe the tables and start fresh, as well as make
sure that the UNIQUE constraints are added, so we drop the table
completely, and then re-add it, which should result in an empty
suppressions table with all expected constraints.
We've also renamed the `email_address` column to `email` to match our
`users` & `members` tables
fixes https://github.com/TryGhost/Team/issues/2366
refs https://ghost.slack.com/archives/C02G9E68C/p1670232405014209
Probem described in issue.
In the old MEGA flow:
- The `email_verification_required` check is now repeated inside the job
In the new email service flow:
- The `email_verification_required` is now checked (didn't happen
before)
- When generating the email batch recipients, we only include members
that were created before the email was created. That way it is
impossible to avoid limit checks by inserting new members between
creating an email and sending an email.
- We don't need to repeat the check inside the job because of the above
changes
Improved handling of large imports:
- When checking `email_verification_required`, we now also check if the
import threshold is reached (a new method is introduced in
vertificationTrigger specifically for this usage). If it is, we start
the verification progress. This is required for long running imports
that only check the verification threshold at the very end.
- This change increases the concurrency of fastq to 3 (refs
https://ghost.slack.com/archives/C02G9E68C/p1670232405014209). So when
running a long import, it is now possible to send emails without having
to wait for the import. Above change makes sure it is not possible to
get around the verification limits.
Refactoring:
- Removed the need to use `updateVerificationTrigger` by making
thresholds getters instead of fixed variables.
- Improved awaiting of members import job in regression test
fixes https://github.com/TryGhost/Team/issues/1996
**Issue**
Our Magic links are valid for 24 hours. After first usage, the token
lives for a further 10 minutes, so that in the case of email servers or
clients that "visit" links, the token can still be used.
The implementation of the 10 minute window uses setTimeout, meaning if
the process is interrupted, the 10 minute window is ignored completely,
and the token will continue to live for the remainder of it's 24 hour
validity period. To prevent that, the tokens are cleared on boot at the
moment.
**Solution**
To remove the boot clearing logic, we need to make sure the tokens are
only valid for 10 minutes after first use even during restarts.
This commit adds 3 new fields to the SingleUseToken model:
- updated_at: for storing the last time the token was changed/used). Not
really used atm.
- first_used_at: for storing the first time the token was used
- used_count: for storing the number of times the token has been used
Using these fields:
- A token can only be used 3 times
- A token is only valid for 10 minutes after first use, even if the
server restarts in between
- A token is only valid for 24 hours after creation (not changed)
We now also delete expired tokens in a separate job instead of on boot /
in a timeout.
refs: https://www.getrevue.co/app/offboard
- Revue is stopping all paid subscriptions on 20th Dec, and shutting down on Jan 18th.
- This update allows Ghost to accept and handle the zip file Revue are providing as an export in Labs > Importer
- It will import posts (as best as we can with the data provided) and subscribers as free members
- At present it doesn't import paid subscribers, as we don't have that info, but you can disconnect Revue from your Stripe account to prevent all your subscriptions being cancelled & there's the option this can be fixed later
- There will be further updates to polish up this tooling - this is just a first pass to try to get something in people's hands
Co-authored-by: Paul Davis <PaulAdamDavis@users.noreply.github.com>
refs https://ghost.slack.com/archives/C02G9E68C/p1670960248186789
This reverts a change that was made here:
f4fdb4fa6c (r93071549),
but it still moved the original code to a new location in the
LastSeenAtUpdater
It includes a new E2E test to make sure timezones are supported
correctly.
- By not using Bookshelf, we no longer fire webhook calls
- By not using the member repository, we don't fetch and update the
member model and the labels relation in a forUpdate transaction, which
caused deadlock issues on the labels/members_labels tables which were
hard to resolve. Until now I was unable to find the other conflicting
transaction that caused this deadlock. Moving to raw knex (instead of
Bookshelf) and only updating the last_updated_at column should remove
the deadlock issue.
This removed the test for the email service wrapper, since it started
failing for an unknown reason and the test didn't make much sense (was
added earlier only to bump test threshold).
- The get helper can sometimes take a long time, and in themes that have many get helpers, the request can take far too long to respond
- This adds a timeout to the get helper, so that the page render doesn't block forever
- This won't abort the request to the DB, but instead just means the page will render sooner, and without the get block
no issue
- The sleep method has been used in 8 modules reimplementing the same thing over and over again. It's usually a sign of async event processing outside of the request/response loop. It's good to have a single point of implementation for a "hack" like this, so we could track it easier and address the even processing delay in a more optimal way centrally if it ever becomes a bottleneck
We can fetch the same event multiple times from Mailgun so we need to
be able to protect against inserting duplicate events in the
database. This will allow us to catch duplicate errors on insert when
handling complaint events.
refs https://github.com/TryGhost/Team/issues/2225
- updated the `formatOnWrite` transform map for posts to include the new `nodes` and `transformMap` options used by `urlUtils` for transforming node payload data
- added `nodes` to the `lexicalLib` module that pulls in our default nodes to be passed in to the URL transform utilities
- added `urlTransformMap` to the `lexicalLib` module that maps transform type and data type to URL transform utility functions that accept a single URL argument
refs https://github.com/TryGhost/Team/issues/2317
This table is used for persisting the email suppression list.
We don't have a member_id column because emails, not members are suppressed.
fixes https://github.com/TryGhost/Team/issues/2308
- Still has some missing pieces, but mostly works.
- Uses new handlebars template for emails
- When sending emails with the new email stability flag enabled, one
test email is now sent via the default smtp ghost mailer.
refs https://github.com/TryGhost/Team/issues/2291
When sending out mails to individual recipients, its possible that recipient gets a temporary or permanent failure for receiving the mail. Temporary failures can generally get resolved after a bit when the recipient’s mail server accepts the email, unlike permanent failures. For both customer visibility and easier debugging on what went wrong while delivering to a particular recipient, we’ll store the permanent/temporary failure for a recipient.
- migration adds a new table that stores the failure information for the recipients
refs: https://github.com/TryGhost/Team/issues/1121
refs: 54574025e0
- The previous change to fall back to a generic error on the server side is resulting in lots of much less useful Sentry reports
- For unexpected errors, change what's sent to Sentry back to context
- This is done by adding a specific code, so we don't have to match on a string that might change
- Also add the error type, id, code & statusCode as tags to the events - these are searchable structured data
- Adding code as a tag also makes it possible to find all errors that showed the generic message
- As demonstrated by my comments in the boot file, I thought sentry was already depending on the version package
- IMO it's undesirable to require package.json directly esp when we have a tool setup and ready for tis
- Added a bunch of tests to show that Sentry does roughly what we think
fixes https://github.com/TryGhost/Team/issues/2284
New batch sending flow (still WIP). Logs the sent emails instead of actually sending them. Unit tests are coming in later commits.
refs https://github.com/TryGhost/Team/issues/2280
We are moving away from storing html and plaintext on email and instead will store the email data in source and source_type columns which allows us to store the email in other formats like mobiledoc and lexical. Storing in those formats allows greater flexibility for later html generation
- adds new `source` column that stores `mobiledoc`/`lexical`/`html` data for a newsletter
- adds new `source_type` column that stores one of `mobiledoc`/`lexical`/`html` to identify type of source
closes https://github.com/TryGhost/Team/issues/2290
Currently, if the whole batch of email fails to send we don’t capture
any errors directly tied to the batch. This makes it hard to debug which
and why a batch failed when debugging email errors. Going forward we'll
store the error information for a failing email batch directly that
allows easier debugging for batch.
- `error_status_code` : Captures statusCode returned by Mailgun,
available in error.status from the example batch error
- `error_message` : Captures short error message from Mailgun and
status, available in context object of batch error
- `error_data` : Captures while whole error json for a batch. As
mentioned in pitch, this will be huge data and we’ll figure out long
term how to best use this.
- updated the cover image to be simpler
- made the change in text fixtures as well, just to keep the fixtures in sync
Co-authored-by: Hannah Wolfe <github.erisds@gmail.com>
refs https://github.com/TryGhost/Team/issues/2216
The `membersActivity` flag was an alpha feature to test the first versions of member analytics, and is no longer active or in use. This change removes the remaining pieces of code that are setup behind that flag and are no longer in use or accessible.
closes https://github.com/TryGhost/Team/issues/2207
- adds conditional to the post email serializer to switch between
`mobiledocLib` and `lexicalLib` depending on which format the post
contains
closes https://github.com/TryGhost/Team/issues/2211
We were allowing paid Tiers to be imported with non-integer prices which was
causing the Admin to be bricked when attempting to load them. This adds some
validation to the price data of Tiers.
refs https://github.com/TryGhost/Toolbox/issues/461
- The 'vary' header with 'Origin' value should only be set when an OPTIONS header is processed. Otherwise we are prone to leaking the vary header modification to further down in the request pipeline
refs https://github.com/TryGhost/Toolbox/issues/461
- The unit test was never using the "OPTIONS" request method, which did not actually trigger the full logic of the "cors" module used under the hood.
- Using the correct request method triggers all the right pathways and tests the state that's closer to the real world - for example the response does get "ended" instead of calling the "next" middleware.
refs https://github.com/TryGhost/Toolbox/issues/461
- Having a 'Origin' in vary header value present on each `OPTIONS` allows to correctly bucket "allowed CORS" and "disallowed CORS" responses in shared caches
refs https://github.com/TryGhost/Toolbox/issues/464
Bceause the import does not use the API, any backwards compat code we put in the
API does not get run for imports, this means we need to update the importer to
map the stripe_prices data onto the products table so that we have valid data in
the database.
refs: a8b1676734
- Extended the newly created handlebars test utils with a shouldCompileToError method
- Updated the price helper tests tp use shouldCompileToExpected and shouldCompileToError
- This allows us to test our handlebars helpers in a much more conisstent way
no issue
- There are currently two patterns in our handlebars helper unit tests:
1. Treating the helper as a function, and doing a function call
- This is the original way the tests were done, and they're not great as they're approximating how the helpers are really used
2. Using a template string, and rendering the string using a method called shouldCompileToExpected
- These tests are more realistic and powerful and also easier to read
- The new method is only being used in a few places so far, and each place had re-created the `shouldCompileToExpected` method
- Therefore I've moved this method into a util that should make it easier to write unit tests for handlebars helpers
- I also renamed the method in the excerpt tests, because it doesn't do the same thing, it's just a wrapper around a function call rather than compiling a string
The aim is to refactor all of our handlebars helper tests to use `shouldCompileToExpected`
refs https://github.com/TryGhost/Team/issues/2168
- site owners can now disable tracking sources from analytics settings.
- this change removes the loading of attribution script if tracking is
turned off so we don't capture any post/page or external source
attributions
refs https://github.com/TryGhost/Team/issues/2168
- the new setting allows site owners to control if they want to track
the sources for new member signups and subscriptions
- its switched on by default, but can be toggled off from new analytics
settings page
closesTryGhost/Team#2159
- Added column to email table
- Hide the feedback tab on frontend depending on the column value
Co-authored-by: Daniel Lockyer <daniellockyer@fastmail.com>
refs https://github.com/TryGhost/Team/issues/2034
- this table will be used to link Stripe subscriptions to Ghost
subscriptions via a foreign key that we add at a later point
- this also includes `constraintName` as the auto-generated one would be
too long for MySQL 8
refs https://github.com/TryGhost/Team/issues/2104
- adds edit permissions for links endpoints to fixtures
- new `bulkEdit` endpoint will use the permissions and allow fixing newsletter links via Admin
refs 5fcf5098a8
- links browse endpoint had permissions switched off unintentionally and was also missing the necessary permissions in fixtures.
- enables permissions for browse endpoint and adds migration insert permissions in DB
fixes https://github.com/TryGhost/Team/issues/2091
fixes https://github.com/TryGhost/Team/issues/2089
- Added new fixtures to make testing easier for the activity feed
- Improved E2E test coverage of activity feed with separate test file
- Added data.post_id filter to enable filtering by events related to a
given post
- Fixed return types in JSDoc of test agents (TypeScript interprets
these as `typeof Agent` if we don't add `InstanceType<Agent>`)
- Added total pagination metadata to activity feed API (to allow a basic
type of pagination using filters)
fixes https://github.com/TryGhost/Team/issues/2096
When generating the recipient data for emails, the email clicks
implementation is resulting in a recipient variable being added called
replacement_xxx once for each link containing the same UUID.
This generates a lot of unnecessary data overhead for emails, and it
turns out that mailgun has a 25MB message limit. We wouldn't have come
close if we only included the uuid once.
fixes https://github.com/TryGhost/Team/issues/2102
- this column was added with `nullable: true` but it should never be
nullable, so we should drop the nullable status whilst it's easy to
refs https://github.com/TryGhost/Team/issues/1765
In order to better handle deleted objects in Stripe we want to decouple
Members from Stripe.
These changes allow us to have the Tier concept completely independent
of the Stripe tables, such that the Stripe data can be generated as/when
it's needed - which will help to protect against missing data.
refs https://github.com/TryGhost/Team/issues/2030
- adds `subscriptions` table to the DB schema
- this new table is aimed to support a native "subscription" primitive in Ghost
that most resembles previously used `members_stripe_customers_subscriptions` table
refs https://github.com/TryGhost/Toolbox/issues/441
- whilst reviewing another PR, I noticed we were incorrectly using
`maxLength` instead of `maxlength` in the schema column definition
- it turns out we've already been doing this wrong for a while with
other columns
- this key is not acted upon, so the maximum column length was not applied
- fixing up the DB to the correct maximum length is something to fix in the
future but right now, the schema does not reflect the size of the
column that actually got created
- the fallback when `maxlength` is not provided is currently 191 [0], so
this commit switches the schema and migrations to using the correct
key name and column length that they are using when applied
[0]: 24670aa555/ghost/core/core/server/data/schema/commands.js (L27)
refs https://github.com/TryGhost/Toolbox/issues/441
- we tend to have a mix of `bool` and `boolean` in the schema and
migrations, which has become a real nit for me at this point
- we don't do any special handling between `bool` and `boolean`, it's
just something we pass to Knex
- `bool` is an alias for `boolean` but `boolean` is actually documented - https://knexjs.org/guide/schema-builder.html#boolean
- this commit switches Ghost to only using `boolean` in the schema and
migrations, and removes `bool` from the allowlist in tests to prevent
us from adding it again in the future
- this should make absolutely no difference to the DB because both
resulted in the same column
refs https://github.com/TryGhost/Toolbox/issues/441
- I'm currently working on cleaning up our uses of `bool` and `boolean`
in favor of `boolean`, and I've noticed we only handle converting
numbers into booleans when the type is `bool`, so validation would
otherwise fail
- given these can be used interchangeably, we should also support
converting the numbers into booleans when the type is `boolean`
- this is going to get cleaned up again when I remove `bool` but this
fixes the validation bug for now
closes: https://github.com/TryGhost/Ghost/issues/14981
- Taxonomy-specific sitemaps were invalid xml when there was no data
- These invalid empty sitemaps were referenced in the index sitemap causing SEO tools to report errors
refs https://github.com/TryGhost/Toolbox/issues/441
- this is only v1 of the test I would like but it validates the keys on
a column definition are part of an allowlist
- this has already uncovered a bug with `maxLength` (vs `maxlength`)
fixes https://github.com/TryGhost/Team/issues/2054
This change adds the sentiment and positive_feedback counts to the posts models. This change isn't really ideal because there are some problems here:
- sentiment isn't really a count
- we don't need to include the sentiment and positive_feedback as a default for posts (but the same is true for attribution)
It would make sense to move this to separate endpoints that only fetch the analytics for a given post when the analytics page is opened. But for our initial skateboard version of audience feedback this should be a good start to already see the data.
fixes https://github.com/TryGhost/Team/issues/2008
- New column that stores email click tracking at the time it was created
- Improved frontend side checks for when to show analytics
refs https://github.com/TryGhost/Toolbox/issues/425
refs https://github.com/TryGhost/Toolbox/issues/280
- The versioned API responses vary based on requested version (passed in request's 'accept-version' header). shared caches that sit between Ghost's origin server and the browser would be putting responses with same Vary into the same caching bucket, which is incorrect.
- This change makes response's Vary more granular and tells caching mechanisms to take 'Accept-Version' request header into account when caching.
- Informative read on the topic - https://www.fastly.com/blog/getting-most-out-vary-fastly
- bumps member attribution script from alpha feature to now load for all sites. The script captures recent url history in localstorage to capture correct attribution for members.
- script is only loaded on the site if members is enabled
refs https://github.com/TryGhost/Ghost/pull/15471#discussion_r979902374
- the accent color value used by default content cta was copying the global site property which is redundant, and can be directly used
- originally, the accentColor property was extended to allow a fallback value for content ctas, but was later removed as we added default value to global site property directly
- the accentColor property is now deprecated and will be removed in next version, as existing themes might be relying on it for custom cta helpers
closes https://github.com/TryGhost/Team/issues/1898
- the default content cta always used the terminology as `post` when showing message that users don't have access to some content
- this caused confusion when users were looking at a page and message showed "This post is for subscribers only"
- updates the message to correctly reflect `page` vs `post` on the default cta
refs TryGhost/Team#1931
- referrer source, medium and url will be stored in the events table along with rest of attribution data
- stores referrer information on two tables
- `members_created_events` for signups
- `members_subscription_created_events` for paid conversions
no issue
- The explore endpoint needs to expose the total amount of published posts
- To be more consistent, this PR creates a PostStats class which is exposed as `stats` method within the PostService; just like it's done with the MemberService
- Moved existing method to return the date of the most recently published post into the stats service
- Updated the explore service test to reflect the new return property
no issue
- added `PostRevsion` model
- duplicated `mobiledoc_revision` creation routine in Post model's onSaving hook to create `post_revision` when model's `lexical` field has changed
- updated `mobiledoc_revision` creation to skip when `lexical` field is populated
no issue
- initially this will perform the same function as `mobiledoc_revisions` but storing `lexical` instead of `mobiledoc`
- naming is intentionally generic ready for later expansions
closes https://github.com/TryGhost/Team/issues/1908
### Problem:
- We need tracking on the paywall links in each email. (we cannot ignore them because those buttons are probably gonna have a higher paid conversion attribution than others).
- Currently we only add the paywall HTML to an email when processing each batch. So if we batch an email to 1.000 recipients per 100, we'll generate the paywall HTML 10 times.
- We cannot replace links in `renderEmailForSegment` because that methods will get called multiple times. We don't want to have multiple redirect instances created for the same link in the same email.
### Solution:
- Move the generation of the paywall to the `serialize` method of the post email serializer.
- Surround the generated paywall with HTML-comments so we can remove it if required in `renderEmailForSegment` depending on the member segment we are sending the email to.
---
### Before:
**Serialize output:**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!--members-only-->
<p>Content visible for paid members only</p>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
To be modified later by `renderEmailForSegment`:
**Paid members (nothing changed):**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!--members-only-->
<p>Content visible for paid members only</p>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
**Free members (paywall _added_):**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<h2>Generated paywall here</h2>
<a href="https://subscribe.com">Subscribe to read the full post</a>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
### After this change:
**Serialize output:**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!--members-only-->
<p>Content visible for paid members only</p>
<!-- PAYWALL -->
<h2>Generated paywall here</h2>
<a href="https://subscribe.com/?tracked">Subscribe to read the full post</a>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
To be modified later by `renderEmailForSegment`:
**Paid members (paywall removed):**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!--members-only-->
<p>Content visible for paid members only</p>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
**Free members (members-only content removed):**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!-- PAYWALL -->
<h2>Generated paywall here</h2>
<a href="https://subscribe.com/?tracked">Subscribe to read the full post</a>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
no issue
- added `@tryghost/kg-lexical-html-renderer` dependency
- added `lexical` lib following the same pattern as our `mobiledoc` lib
- updated the Post model's `onSaving` hook to generate the `html` value from `lexical` when present
fixes https://github.com/TryGhost/Team/issues/1909
- The feature image caption is already escaped on the frontend
- Doing it again in the backend breaks the possibility to add links to the caption
- I checked and the `feature_image_alt` is not escaped in the frontend.
fixes https://github.com/TryGhost/Team/issues/1900
refs https://github.com/TryGhost/Team/issues/1901
- Defaults to the same value as the current email_track_opens setting for existing installations, otherwise defaults to true
- Had to use a custom migration because the `addSetting` helper doesn't support using an existing setting as current value
- Added a minimal UI to change the setting, but this still needs some design magic 🪄✨
- Link replacement is disabled if `email_track_clicks` is disabled. In the future we might consider to still do parial additions, such as source attribution and maybe redirects (to discuss).
We're going to be adding more redirection logic into Ghost and it's
going to get confusing if we have names this generic. This makes it
clear which feature this service is related to.
Ideally in the future we can combine all of these into one redirects
service, but for now we will be running a specific service per feature
closes https://github.com/TryGhost/Team/issues/1884
- adds `post.lexical` ready for use by the lexical-powered editor re-write
- fulfils the same purpose as `posts.mobiledoc` so uses the same field properties
- added `lexical` to allowed formats in Post model so it won't be included by default in API responses meaning tests/snapshots don't need updating at present
refs https://github.com/TryGhost/Team/issues/1865
- refactors staff service to listen to member and subscription events
- triggers email alerts based on events instead of directly calling the service
- removes staff service dependency for members api
closes https://github.com/TryGhost/Team/issues/1772
- The user facing side of comments recently replaced `bio` with `expertise`.
- To remain consistent we replaced all the references of `bio` with `expertise` throughout the codebase.
- This includes a database column name changing migration, within the `members` table.
- Bumped up the comments-ui version to a new minor (0.10.x) as its a breaking change.
- this prevents the referrer/referer header being sent for requests that go to external domains
- this in turn prevents preview URLs from appearing in the analytics of sites that are linked to and clicked on from previews
- otherwise, preview URLs can be leaked to the owners of the linked and clicked sites
refs https://github.com/TryGhost/Team/issues/1795
- Snapshots help us detect unexpected changes in the `<head>` of all sites (e.g., newly introduced script tags)
- Added ghost_head tests for comment count helper
refs https://github.com/TryGhost/Team/issues/1871
This commit adds a test to the serialize method of `post-emaiserializer`. It checks whether the generated email HTML is valid and standard HTML5 and that all properties are escaped.
To do this validation, I depend on the new `html-validate` dev dependency. Just parsing the HTML with a HTML parser is not enough to guarantee that the HTML is okay.
Apart from that this fixes:
- Removed the sanitizeHTML method and replaced it with normal HTML escaping. We don't want to allow any HTML in the escaped fields. Whereas `sanitizeHTML` still allows valid HTML, but we don't want that and want the same behaviour as on the site. E.g., a post with a title `All your need to know about the <br /> tag` should actually render the same title and non-html content, being `All your need to know about the <br /> tag`
- The file, nft and audio card didn't (always) escape the injected HTML fields (new version @tryghost/kg-default-cards)
- `@tryghost/string` is bumped because it contains the new escapeHtml method
refs https://github.com/TryGhost/Ghost/pull/15375
- we currently pass all properties for the `tags` property of a
`page`/`post` body down further into Ghost, which is causing issues
because it's handling properties it doesn't expect
- this is showing up because it's triggering save history events for
tags when a post is edited
- this commit introduces a clean util which has an allowlist of
properties allows on tag relations
- this list was taken from the schema: 128f8fb006/packages/admin-api-schema/lib/schemas/posts.json (L214-L227)
refs https://github.com/TryGhost/Team/issues/1879
OpenSea updated their URL format for NFTs after adding support for Solana
which broke our regex, this updates to support the new format.
refs https://github.com/TryGhost/Toolbox/issues/384
- Existing adapter config was based on the notion there can only be one configuration per one adapter class. With adapter cache now allowing instantiating multiple adapter instances with the same base class it opened up a possibility to have shared configuration for a base class and then extend/override it in "feature" configurations (see tests in this commit for specific examples)
fixes https://github.com/TryGhost/Team/issues/1855
fixes https://github.com/TryGhost/Team/issues/1866
This commit moves all duplicate methods to get the support email address to a single location. Also methods to get the default email domain are moved.
For the location, I initially wanted to put it at the settings service. But that service doesn't feel like the right place. Instead I created a new settings helpers service. This service takes the settingsCache, urlUtils and config and calculates some special 'calculated' settings based on those:
- Support email methods
- Stripe (active) keys / stripe connected (also removed some duplicate code that calculated the keys in a couple of places)
- All the calculated settings are moved to the settings helpers
I'm not 100% confident in whether this is the right place to put the helpers. Suggestions are welcome.
fixes https://github.com/TryGhost/Team/issues/1870
Disables email sanitization that was enabled earlier because this bug is more important and urgent.
The recently introduced email sanitzation removes HTML comments from the post html.
- This breaks the email paid preview, because it depends on the `<!--members-only-->` comment.
- Breaks the Outlook comments `<!--[if !mso !vml]-->`
This commit reverts this change.
refs https://github.com/TryGhost/Team/issues/1771
We don't have access to `req.brute.reset` due to the way the flow
works, we have one endpoint which sends an email with a magic link,
and another route which handles the login. We don't want to apply
brute force protection to both because our rate limiting is designed
for API requests not web page visits (which is how login is handled).
Because of this we require access to the underlying ExpressBrute
instance exposed by the spam-protection module, so that we can
perform the reset.
refs https://github.com/TryGhost/Toolbox/issues/389
- The e2e test suite log was full of ERR_NOCK_NO_MATCH warnings when the logging level was set to "warn". The cause of this warning was legit duplicated webhook trigger processing on test environment. Gah!
- The source of duplicate webhook processing was duplication of event handlers. Event handlers were registered multiple times for same event because of the singleton nature of the "common/events" module - it remains the same instance and is not cleaned up between reboots. The deeper issue of events module initialization should be solved separately, this slightly hacky approach fixes the problem now and highlights it to be tackled in the future.
