This is a pretty simple way for us to track which webmentions are sent
by Ghost. Although it's easily spoofed, so are other approaches like
using a header (e.g. User-Agent). If we find that this data is being
spoofed we can look at different approach.
Becuase our receiving implementation stores the payload of the
Webmention, we'll be able to know inside Ghost which Mentions
originated from another Ghost installation, which is useful for stats
and gives us the possibility to display that information in the feed.
Longer term we might want to consider storing this data in a separate
column for Mentions, rather than the `payload` column - but that is
outside the scope of this change.
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}}
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`.
closes https://github.com/TryGhost/Ghost/issues/16125
We weren't taking into account any existing email segment set on the
post. This is usually not an issue because during the publishing flow
the post.emailSegment and the selectedRecipientFilter are kept in sync,
but it becomes and issue when the email fails to send and is later
retried - we now have an inconsistency between the two values.
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/2419
We use a job queue to ensure that webmentions can be processed outside of
the request/response cycle, but still finish executing if the processed is closed.
With this we're able to update the e2e tests to await the processing of the mention
rather than sleepign for arbitrary lengths of time, and we've reintroduced the tests
removed previously
- aa14207b69
- 48e9393159
refs https://github.com/TryGhost/Toolbox/issues/500
refs https://ghost.notion.site/Data-Types-e5dc54dd0078443f9afd6b2abda443c4
- There current notification logic for incompatible integrations did not take into account the source of the trigger, which might have been causing emails to instance owners that did not ever set up custom integration - so they had nothing to fix.
- The "internal" and "core" integrations are maintained/controlled by the Ghost team, so there should never be a notification going out to the instance owner about possible incompatibility in the code they do not control.
- Along with changed updated the unit test threshold in the packages that were touched to 100%. As that's the standard for all new packages.
closes https://github.com/TryGhost/Ghost/issues/16125
We weren't taking into account any existing email segment set on the
post. This is usually not an issue because during the publishing flow
the post.emailSegment and the selectedRecipientFilter are kept in sync,
but it becomes and issue when the email fails to send and is later
retried - we now have an inconsistency between the two values.
no issue
When a site doesn't have any emails on boot, it doesn't schedule the email analytics job. With this change, the new email flow will also restart that job after an email has been created.
refs https://github.com/TryGhost/Team/issues/2486
Stop the event fetching loop as soon as we receive events that were
created later then when we started the loop. This ensures that we don't
miss events if we receive a giant batch of events that take a long time
to process.
no issue
- Ghost users that make >= $100 MRR will see a dismissible notification that invites them to the Ghost Referral program
- Only applies to Admin and Owner users and when Stripe is setup and connected in live mode
- By saving a `referralInviteDismissed` property to the users' `accessibility` JSON object we can determine if the notification has been dismissed and won't show it again
- Added new `gh-referral-invite` component
no refs.
This commit fixes a couple of UX issues on the email debug screen:
- shows [...] button only for errors actually longer than the available
space to avoid confusion about where there's more error text
- use actual avatars instead of fake red/blue dots to make it consistent
with the rest of the app
- adds click through to member details screen to easily access member
data if needed
- updates text select for provider ID for easier copying
- removes unused "Download full error message" icon
fixes https://github.com/TryGhost/Team/issues/2484
The flow only send the email to segments that were targeted in the email
content. But if a part of the email is only visible for `status:free`,
that doesn't mean we don't want to send the email to `status:-free`.
This has been corrected in the new email flow.
fixes https://github.com/TryGhost/Team/issues/2484
The flow only send the email to segments that were targeted in the email
content. But if a part of the email is only visible for `status:free`,
that doesn't mean we don't want to send the email to `status:-free`.
This has been corrected in the new email flow.
refs https://github.com/TryGhost/Team/issues/2482
This change adds a small sleep in between dispatching events in the
worker thread that reads the events from Mailgun. That should reduce the
amount of queries we fire parallel to each other and could cause the
connection pool to run out of connections.
It also reduces the amount of concurrent sending to 2 from 10. Also to
make sure the connection pool doesn't run out of connections while
sending emails, and to reduce the chance of new connections falling back
on a (delayed) replicated database.
closes https://github.com/TryGhost/Team/issues/2429
- sends email notifications to staff users when their site receives a Webmention.
- currently behind a flag, that can be toggled in the labs settings.
refs https://github.com/TryGhost/Team/issues/2482
This change adds a small sleep in between dispatching events in the
worker thread that reads the events from Mailgun. That should reduce the
amount of queries we fire parallel to each other and could cause the
connection pool to run out of connections.
It also reduces the amount of concurrent sending to 2 from 10. Also to
make sure the connection pool doesn't run out of connections while
sending emails, and to reduce the chance of new connections falling back
on a (delayed) replicated database.
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
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
Portal currently has a Globals.js file that spells out all the colors in use in Portal, which should make it easy to customize the portal colors to match the chosen theme. There are a bunch of hardcoded values and this PR deals with those. The final outcome of these changes is absolutely invisible.
Co-authored-by: Peter Zimon <zimo@ghost.org>
This test is failing because the `sleep` isn't long enough. Removing this test
until we've refactored to use the jobs service, at which point we can remove the
sleep and wait for the job to be complete.
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.
closes https://github.com/TryGhost/Team/issues/2388
We have seen examples of sites with member emails that have invalid characters that can cause an entire email send to fail, or just cause a failure to those addresses. The issue that allowed members with invalid email address to be saved was patched earlier, but its possible there are still sites that contain some of those invalid email addresses.
This change updates new sending service to filter out the recipients with invalid email address before passing them to mail provider, so these rogue addresses don't affect the whole batch in anyway. We also trim the recipient emails to clear out any spaces first, which is the most likely culprit.
- uses new email validator that detects invalid email addresses with special chars
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
- The "last_seen" property is not used in routing calculations. Without it the routing service was triggering an expensive process on each user login.
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
no issue
- clicks on the iframe never bubble out of the iframe so weren't captured by the dropdown-closing event listener
- added an event listener directly on the iframe's body element when we render the iframe's content that manually calls out to our generic dropdown closing method
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
- The Dynamic URL service no longer generates "url.added" event when only a partial resource update happened - only non-url forming properties were modified. The sitemaps service still needs to know when to update the lastmod ("Last Modified") field associated with specific URL.
refs https://github.com/TryGhost/Toolbox/issues/503
- Tier's are sometimes dynamically generated and are present in the "_changed" properties, causing full URL regeneration. They have no effect on post's URL, so should not trigger URL regeneration.
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.
refs https://github.com/TryGhost/Toolbox/issues/503
- Reusing existing events inside of dynamic routing would only contribute to general confusion that is already there. Having separate "DomainEvents" is the best practice used throughout the code which is substituting generic events.
- The URLResourceUpdatedEvent is supposed to be emmited whenever there's an updated to the resource already associated with the URL circumventing full url regeneration process inside of DynamicRouting
no issue
Tests stopped working because the Mailgun mocker stopped working since we moved to the new email flow.
This also fixes a unit test that needed to get updated.
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.
fixes https://github.com/TryGhost/Team/issues/2461
- Ignores 'edited' links when there is only one second differences.
- Make sure we don't set updatedAt when linking a post to a redirect
refs https://github.com/TryGhost/Toolbox/issues/501
- this reverts commit 48dda23554
- also includes a resolution for `@elastic/elasticsearch` so we don't
run a version that is potentially problematic - see referenced issue
for context
refs https://github.com/TryGhost/Team/issues/2466
This initial implementation just checks that we're on the right origin and
subdomain, but should be extended to check if the URL actually resolves to a
page hosted on the site!
refs https://github.com/TryGhost/Team/issues/2465
We've restricted this to Post resources for now until we update the Mention
entity to be able to handle multiple resource types.
refs https://github.com/TryGhost/Toolbox/issues/503
refs https://github.com/TryGhost/Toolbox/issues/406
- In Ghost 5.x we dropped multi-versioned API, which means there's no need to track resource configs dynamically as there can only be one version
- Along with removed "initResourceConfig" refactored the "config" file itself to be injected into Resource's constructor - allows for easier testing.
no issue
Using the slash menu it was possible to insert cards that shouldn't have been accessible based on their availability checks. This was happening because we were only hiding the visibility of the cards in the template rather than completely removing them from the slash command matching logic.
- added `{{card-menu-items}}` helper that combines the availability matching and snippet section addition to return a complete array of sections+items that match the current system state and post type
- added `@menuItems` argument set to the output of `{{card-menu-items}}` to the two card menu components so they are working against a pre-filtered list of menu items
- lets us remove duplication of code that handled pushing snippets section into the menus
- removed availability check conditionals from `<KoenigMenuContent>` as the menu items passed in are now pre-filtered
https://github.com/TryGhost/Team/issues/2458
This is an initial pass at pulling metadata from webmention sources, we've also
updated the fake data to pull from some real-world sites which implement
webmentions. We've reused the oembed service here, long term it would be nice to
pull the metadata parsing/pulling part out, so that we can have more generic
error messages.
Based on a discussion in slack we want to make all metadata properties optional,
with the exception of the title, which will default to the host of the source
URL if it's missing.
This is so that we can accept as many webmentions as possible and convert them
into Mentions. If we were to have strictly validation, we'd end up having to
drop webmentions that didn't match our criteria, and lose important data.
Giving the title a default allows us to provide a consistent UI experience too.
refs https://github.com/TryGhost/Team/issues/2419
This is the initial stab at having everything wired up, we're not
using a queue but we are handling the processing of the Webmention
asyncrounsly so that the HTTP response can be end immediately.
We've also laid the groundwork for extending and implementing the
correct processing of Webmentions, for example checking if the target
URL exists in the system, pulling out the metadata from the Webmention
source and fetching any internal resources.
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/2435
We've made these fields optional, and we may need to extend this to other fields
too as we discover more about the data we're able to get access to.
- we don't end up using the inserted model from Bookshelf, so we
shouldn't be performing a SELECT on the entry
- this disables refreshing the model using Bookshelf's `autoRefresh:
false` and allows the key through the sanitization for `add
refs https://github.com/TryGhost/Ghost/issues/15502
- the amazing `i18next-parser` dependency will extract our translated
strings from Portal and dump them into locale files, so we never have
to add them manually
refs https://github.com/TryGhost/Ghost/issues/15502
- plain JSON files are cleaner and less overwhelming than boilerplate JS
files, and given they're going to be automatically generated, we
probably won't be able to support comments anyway
refs https://github.com/TryGhost/Ghost/issues/15502
- this is an early implementation of an i18n provider by
exporting an instance of `i18next`
- there's a lot more to be done here but baby steps :)
fixes https://github.com/TryGhost/Team/issues/2200
When zipping a folder that contains files with UTF-8 characters in the filename, using the MacOS Archive Utility, the resulting zip will be missing some UTF-8 configuration bit. This breaks the unzipper, causing it to decode the filenames using the wrong encodign.
When the file names are long, and become longer than the length allowed by the OS, an ENAMETOOLONG error is thrown. This error is not handled by the importer, and causes the import to fail.
This adds a specific check for this error so we can show a clear error message to the user, that helps them to resolve the issue. We are currently unable to fix the issue on our side, because of a lack of well supported zip libraries for node.
refs acf0baa8c7
Due to the bump in express-test, we now handle string bodies 'properly'. So they now pass all the Express middlewares. In the past this failing test did not really pass by the bodyParser.raw middleware,
so the content-type check on the `bodyParser.raw({type: 'application/json'})` middleware was not executed. Now it is, and the test fails because the content-type header was not set to application/json.
no issue
When fetching the suppression list data for emails with a plus sign, the
parsing of the NQL filter fails:
```at Child.applyDefaultAndCustomFilters (/Ghost/node_modules/@tryghost/bookshelf-filter/lib/bookshelf-filter.js:66:23)
[ghost] email:[simon+test@ghos
[ghost] ------------^
[ghost] Expecting 'OR', 'RBRACKET', got 'AND'
```
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)
refs https://github.com/TryGhost/Toolbox/issues/499
- The mockManager's sentEmailCount is left here to avoid breaking many tests that already depend on this method. With future improvements to email snapshot tests this method should not be used. Instead, emailMockReceiver's own sentEmailCount method should be used directly.
ref https://github.com/TryGhost/Team/issues/2421
- added the Mentions API endpoint to Admin
- setup initial mention model in the Ember Store to be able to dev with the endpoint
- added basic routing to access the `/mentions` page that is currently behind feature flags
- Setup basic testing with a mirage mock endpoint.
refs https://github.com/TryGhost/Team/issues/2416
This extends the mock API to use a more formal pattern of moving our
entity code into a separate package, and use the service/repository
patterns we've been work toward.
The repository is currently in memory, this allows us to start using
the API without having to make commitments to the database structure.
We've also injected a single fake webmention for testing. I'd expect
the Mention object to change a lot from this initial definition as we
gain more information about the type of data we expect to see.
refs https://github.com/TryGhost/Team/issues/2416
This doesn't even return a Mention in the correct format at the moment,
but it's just to get an endpoint there, behind a flag and returning data
so that we can start playing with the API and having it hooked up the
the Admin.
The next step will be fleshing this out further and defining the
services and repository to back it, as well as updating the Admin so that
we can fetch mentions to display in the UI
refs https://github.com/TryGhost/Toolbox/issues/499
refs 6bcc47a0ad
- Using module directly caused issues with snapshots manager instance initialization (mocha hooks did not apply to a correct instance)
- See refed commit for more
refs https://github.com/TryGhost/Toolbox/issues/499
- Outgoing emails have been a weak point of Ghost's stability recently. The concept of "emailMockReceiver" similarly to "webhookMockReceiver", allows to test side-effects like outgoing emails.
- This is a first iteration which should lay groundwork for testing all outgoing emails in the future
- The change adds a new concept of "email mock receiver" which is very similar to how the "webhook mock receiver" works. The email mock receiver exposes two methods to record and verify snapshots:
- matchHTMLSnapshot - records and verifies only the HTML content of the outgoint email
- matchMetadataSnapshot - records and verifies all the non-HTML properties sent along an email content, e.g.: to address, plaintext, subject, etc.
- What's missing is matching content based on dynamic content like dates, links with JWT tokens, etc.
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.
closes https://github.com/TryGhost/Team/issues/2276
Portal had died with an unintelligible error about portal plans/includes being undefined when there was missing site data in some extreme edge cases. This change catches any errors in site transformation and logs it to console instead of crashing portal unexpectedly
- in the event the Mailgun config doesn't exist, we return `null` from
this function
- this updates the jsdoc to correct the return type of `getInstance`
no refs.
Post feedback popups were not optimised for mobile usage at all. All the content was on top of the window which is really hard to reach using a single hand/thumb and it just looked like a scaled down version of a desktop modal.
closes https://github.com/TryGhost/Team/issues/2206
- removes `www.` from the url shown on links table in post analytics
- we had previously removed http(s) protocol from it as well, and they are only shown while editing the url
This introduces the new suppressions feature which will automatically
unsubscribe members from newsletters when their email is added to the
suppression list in Mailgun, this is usually due to emails either
permanently bouncing to the address, or the member making a spam
complaint.
Both Members and Admins are able to see that the email has been added to
the list, and Members are be able to request their email be removed from
the list via Portal.
Overall this feature should improve delivery rates of newsletters and
improve the rating of the domain you're sending from.
closes https://github.com/TryGhost/Team/issues/2338
If a site has the Free tier hidden from the Portal, and subsequently the Stripe connection is disconnected, this produces a dead-end state where no new members can sign up and the Free tier cannot be reactivated again in Portal settings as its hidden. This change -
- enables free tier toggle to be always shown on site irrespective of Stripe connection
fixes https://github.com/TryGhost/Team/issues/2383
A user could use `{uuid}` inside an email only content and it would work. This currently isn't supposed to be used outside internal features (link click tracking, feedback buttons). For now this is only fixed in the new email flow under the email stability flag.
fixes https://github.com/TryGhost/Team/issues/2339
The email service is now fully covered by tests, and this commit also forces the test coverage to remain 100% after future changes.
closes https://github.com/TryGhost/Team/issues/2011
- Gives publishers the ability to filter members based on which offer they used (redeemed) when they subscribed for a paid membership.
- On the offers page, the redemption count number links to a the members page with the filter already applied making it easy to have insight on which members used the offer / coupon.
We have been adding emails to the suppressions table which are not on
the suppression list in Mailgun due to a misunderstanding of how
Mailgun handles 5xx error codes.
We're seeing behaviour from Mailgun where permanent failures with a
5xx error code are not being added to their internal suppression list,
which is resulting in the Ghost list becoming out of sync with
Mailgun.
Rather than adding emails to the suppression list when Mailgun does,
we're instead going to add emails _after_ Mailgun does, by waiting for
an error code which tells us the email is already on the suppression
list.
Those codes are 605 for previous bounces and 607 for previous spam complaints.
We have been adding emails to the suppressions table which are not on
the suppression list in Mailgun due to a misunderstanding of how
Mailgun handles 5xx error codes.
We're seeing behaviour from Mailgun where permanent failures with a
5xx error code are not being added to their internal suppression list,
which is resulting in the Ghost list becoming out of sync with
Mailgun.
Rather than adding emails to the suppression list when Mailgun does,
we're instead going to add emails _after_ Mailgun does, by waiting for
an error code which tells us the email is already on the suppression
list.
Those codes are 605 for previous bounces and 607 for previous spam complaints.
fixes https://github.com/TryGhost/Team/issues/2398
There was an error when fetching the existing email recipient failure. It ended up matching all recipient failures. The result was that only one failure was stored in the database.
refs https://github.com/TryGhost/Team/issues/2371
- cleans up and adds comments for portal playwright tests
- updates data test attributes for portal trigger and popup selectors for consistency
- updates data attribute usage for offers
refs https://github.com/TryGhost/Team/issues/2371
- in case all tiers are archived before new tier is created, the add tier section can be collapsed and will need to be opened first before going through add tier flow
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)
closes https://github.com/TryGhost/Toolbox/issues/497
- The classification of fatal/non-fatal errors has been updated to only be fatal when causing page renders with 5xx or 4xx responses.
- Some of the rules checking Ghost 5.x compatibility have been relaxed to only be "error" with the gscan version bump
- You can find more details on which exact rules were relaxed in the gscan's commit log - https://github.com/TryGhost/gscan/compare/v4.35.1...v4.36.0
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
refs https://github.com/TryGhost/Toolbox/issues/406
refs b2a3e03ef3
- The "code" property in the global "error" variable (accessible via {{error.*}}) has been long deprecated - time to go, bye!
- When {{code}} or {{error.code}} helpers are used in the templates they will output an empty string from now on. Use {{statusCode}} instead!
- the test was using incorrect test state that was copied over from adding label test
- also adds guard for empty newsletters in member filters as in some cases it might not exist as found by test
fixes https://github.com/TryGhost/Ghost/issues/16057
Briefly, Ghost created two Customer objects via the Stripe API when an
existing subscriber would upgrade to a paid subscription, one in an API
call to create the Customer and then a second as a side effect of an API
call to create a Checkout session for the user. The fix is passing the
reference to the Customer object to the API call to create the Checkout
session; Stripe will no longer redundantly create a Customer object in
this case.
This largely impacts the owner's experience of the Stripe Dashboard; it
will correct their new Customer count (going forward) and make searches
for users by name or email address return one responsive object which
has the actual subscription in it versus returning two and forcing them
to look in each to e.g. refund a transaction or similar.
no issue
When using admin as a user with author or editor permissions, admin tried to load the member counts in order to display them in the menu. But authors and editors are not allowed to see the members. So the request returned a 403.
It is not necessary to load the member counts for authors and editors, so we can just skip the request.
fixes https://github.com/TryGhost/Team/issues/2246
This solution adds some retries when fetching the recipients for a
batch. For an unknown reason the recipients can be empty (while they
aren't in the database). This should fix the issue for now until we find
more information about the root cause.
refs. https://github.com/TryGhost/Team/issues/2393
- a labs flag had to be created so we avoid working in branches
- permanent notification toast was added to make theme errors more discoverable
- static modal was needed to hold theme error details
When Mailgun fails to deliver an email to an address because the
address has already bounced before, it gives us a permanent fail event
with a 605 error code rather than a 5xx one. Because we want to
"backfill" our suppressions data with previously bounced email
addresses, we want to handle this specific error code.
We may update this logic in the future based on new information from
Mailgun with respect to their 6xx error codes and the
meanings/underlying cause of theme.
This also moves the tests which check for whether or not emails are
suppressed into their own fail so that we do not pollute the event
storage tests, and adds more tests cases.
We also fix a leaky sinon stub which we were not resetting in the email
event storage tests
The email_recipient fixtures were using duplicate and mismatched email addresses
rather than having them correctly map to the Members, which is required for testing
email suppressions.
no refs.
- Stripe Connect footer in the Portal settings sidebar was covering settings on small screens
- the border for Stripe Connect box was not visible in dark mode
- the "Save and Close" button background was not visible in dark mode in Portal settings
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.
closes https://github.com/TryGhost/Team/issues/2361
If a free trial tier existed on site and its set to 'Invite only' in membership settings, the free trial copy still showed on portal.
- removes free trial copy from portal if site is invite only
- adds playwright test to make sure free trial copy is not shown for invite only sites
- for some reason, Node 18 detects a lower coverage than the configured
threshold so this fails
- I've temporarily lowered the threshold until we can investigate why
refs https://github.com/TryGhost/Toolbox/issues/488
- Node 18 is now LTS so we're adding support for it
- this adds Node 18.12.1 (the latest security release) to our supported
ranges and CI
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
The MailgunEmailSuppression list was incorrectly adding emails
to the suppression list for permanent failure events which have
an error code outside of the 5xx range.
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: 5f90baf6fe
- Ghost has a character limit on post slugs of 191 characters,
- Sometimes, the slug that is generated from the title in Revue content is longer than this, causing the import to fail.
- This PR trims that generated post slug to 190 characters.
closes https://github.com/TryGhost/Team/issues/2380
- improved offer validation for `amount` field to cover all type/amount cases
- added validate-on-blur to the amount field to match our standard validation behaviour
- added re-validation of the amount field when the type is changed and the amount gets reset
- removed the internal parsing of a decimal trial days entry to an integer so the field value matches what is set internally and we let the user know that partial trial days are not supported
Non-user-facing refactors:
- renamed `_saveOfferProperty` to `_updateOfferProperty` to better reflect what it does
- fixed missing indentation for conditional blocks in the offer template
- this was all getting terribly behind so I've done several things:
- majority of `@tryghost/*` except Lexical packages
- gscan + knex-migrator to remove old `@tryghost/errors` usage
- bumped lockfile
refs:
5f90baf6fe
- The check for hasIssuesCSV didn't normalize the filename first,
meaning the importer is super sensitive to zip structure
- This allows for zips that contain a directory, so that it will still
be processed as a revue import, not a Ghost import
refs https://github.com/TryGhost/Team/issues/2235
We found some cases which can cause a site to have member emails that have invalid characters like `member@example.com�`. This happened due to the `validator` version used by Ghost not able to catch some specific cases as invalid email, allowing members to be created with them either via Admin or Importer or direct signup. Portal UI already blocked these email as invalid. This change:
- updates `@tryghost/validator` to include a latest version of email validator that catches these invalid cases
- doesn't allow member creation with invalid email like above
- doesn't allow existing member emails to be edited to invalid
fixes https://github.com/TryGhost/Team/issues/2374
When clicking 'Show active offers' in the archived offers view, it would
always return to an empty offers list, even when there are active
offers.
refs:
5f90baf6fe
- The OG implementation of importing revue subscribers was very naive
- This sures it up to use our proper member importer, which makes sure
everything works perfectly:
- adds an import label
- ensures members are subscribed to newsletters
Co-authored-by: Kevin Ansfield <kevin@lookingsideways.co.uk>
refs: 5f90baf6fe
- Blockquotes without any wrapping tag get converted to Mobiledoc weirdly. Wrapping them in `<p>` tags helps solve that
- Also removes empty paragraph tags which cause unwanted large blank spaces in content
- Remove internal #revue from Revue content
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>
fixes https://github.com/TryGhost/Team/issues/2386
**Issue:**
- When trying to import a member that already exists, and has
'subscribed' set to 'true' in the CSV, the newsletters the member is
subscribed to are reset to the default newsletters.
- When ediging a member with the API and setting `subscribed` to true,
the same happens.
**Cause:**
A faulty check for the `status` property of a newsletter.
Fixed and added a new E2E test.
- Now that the importer runs in a job, it seems sensble that we should
do this
- If posts are imported with HTML set, but not mobiledoc, we now convert html -> mobiledoc
- Note: This also converts the mobiledoc -> html so _may_ be lossy
- Without this, imports that only have HTML, not mobiledoc, would have
resulted in empty posts, so lossy > empty
no refs
- this adds a theme documentation link to the design settings footer if there's an URL set in the package.json of the theme. An example of the package.json property: "docs": "https://example.com"
closes https://github.com/TryGhost/Team/issues/2012
- Members can now be filtered based on the newsletters they are
subscribed to.
- Defaults to the existing newsletter filtering if user does not have
more than 1 newsletter.
refs: 8ed5f9784d
- When importing content from a JSON file in Settings > Labs, a public tag
like `Import 2022-12-03 19:57` gets added to each newly imported post.
- This tag should not be public. It definitely serves a useful
purpose but has no useful function for readers of the site and should
not be shown to readers.
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).
refs TryGhost/Team#2294
- If user enter the slug name and then leave the post page, we should
wait until the slug would be saved on backend. The problem can be
reproduced with slow internet connection.
- 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
refs https://github.com/TryGhost/Team/issues/2371
- test publishes a post with access for a single tier then checks the front-end with no member, member on wrong tier, and member on right tier
refs: https://github.com/TryGhost/Ghost/issues/15725
- our users are having difficulties getting onboarded with mailgun
- we're adding an explicit and unique tag to all requests, to help mailgun detect when mail is being sent from Ghost
closes sentry Admin-423
- there may be times when the mousemove event handler fires when the document is not in a ready state resulting in an attempt to get a document position that doesn't exist
- should fix `Could not find parent section from element node` errors
refs https://github.com/TryGhost/Ghost/issues/14101
- migrated component to Glimmer
- swapped usage of `ValidationState` mixin for `{{validation-status}}` modifier
- updated modifier to accept custom error/success classes
- removed unnecessary/unused code in the `gh-benefit-item.js` backing class
- adds test that cover creating and signing up to multiple-month/forever offers
- checks that the offer information is shown to members during signup and in account detail
closes https://github.com/TryGhost/Team/issues/2382
The preview text is getting set to subject line in the new email flow so it repeats multiple times in the inbox(subject+preview+title). This was because the new flow doesn't use the post serialisation that the old system did, causing excerpt to be empty in the email rendering.
Old system was using post serialisation here -
a721e4f2d7/ghost/core/core/server/services/mega/post-email-serializer.js (L136-L139).
This change adds explicit method to calculate the preview text for email in email renderer service using same logic as used in old system.
Co-authored-by: Simon Backx <git@simonbackx.com>
closes https://github.com/TryGhost/Team/issues/2376#event-8026429598
- if an offer is expired/in past, we no longer show it in member account info against the price
- one-time offers are never showed in portal in member account detail, as the payment information shown to member in Portal points to charge at next payment
- if trial days are over for a subscription, portal doesn't show any offer data on member account detail
