refs https://github.com/TryGhost/Team/issues/2255
These methods will be used by the Mailgun implementation of EmailSuppressionList
so that emails are removed from both our internal list and Mailguns.
refs https://github.com/TryGhost/Team/issues/2339
- Includes a new pattern in the job manager that allows us to properly
await jobs.
- Added new convenience mocking methods to stub settings
- Tests the main flows for bulk sending:
- Sending in multiple batches
- Sending to multiple segments
- Handling a failed batch and retrying that batch
- Fixes bug in batch generation (ordering not working)
In a different PR I'll add more detailed tests.
refs https://github.com/TryGhost/Team/issues/2327
As part of improving visibility for email failures, this change adds a new debug screen that allows visualising the email failures for a post. The screen is hidden on the UI for now and only accessible via URL directly.
Co-authored-by: Djordje Vlaisavljevic <dzvlais@gmail.com>
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.
no issue
- This module is marked for deletion with a todo comment. It also popped up as least covered place in the codebase, because it was never used.
fixes https://github.com/TryGhost/Team/issues/2332
Saves events in the database and collects error information.
Do note that we can emit the same events multiple times, and as a result
out of order. That means we should correctly handle that a delivered
event might be fired after a permanent failure. So a delivered event is
ignored if the email is already marked as failed. Also delivered_at is
reset to null when we receive a permanent failure.
closes https://github.com/TryGhost/Team/issues/2286
- use the `registerAPI` prop to get access to a basic API for focusing and inserting paragraphs
- replaced commented mobiledoc based title key handling with lexical handling
closes https://github.com/TryGhost/Team/issues/2309
- adds new mailgun provider to send out batch emails
- updates sending service to send email id for mailgun provider, allows tagging mail with email id
refs a2d487e074
- Same reasoning as in referenced commit: "Database schema definition file is a special type of "configuration" file containing mostly static declarations. This sort of code should not be tested by unit tests, rather by e2e tests"
refs https://github.com/TryGhost/Toolbox/issues/486
- Frontend helpers are extremely hard to cover fully in e2e tests and are better suited to be covered by unit tests (which they have very hight coverage with)
closes https://github.com/TryGhost/Team/issues/2324
- It seemed like the "limit" query parameter did not work properly returning multiple entries from the endpoint. In reality the whole query string was ignored because of an error in the "filter" part of the query ^_^
refs https://github.com/TryGhost/Team/issues/2326
- The job takes considerably longer to run with MySQL, so needed a longer sleep time. It's a temporary fix to unblock a broken build. We should investigate why the job takes so long to run on MySQL
refs fc9f8aebc1
- With integration & regression test suites included in the e2e coverage reports the coverage has jumped up nicely. We should keep the threshold as hight as possible from accidentally reducing the coverage quality
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.
fixes https://github.com/TryGhost/Team/issues/2310
This moves the processing of the events from the event-processor to a
new email-event-processor in the email-service package.
- The `EmailEventProcessor` only translates events from
providerId/emailId to their known emailId, memberId and recipientId, and
dispatches the corresponding events.
- Since `EmailEventProcessor` runs in a separate worker thread, we can't
listen for the dispatched events on the main thread. To accomplish this
communication, the events dispatched from the `EmailEventProcessor`
class are 'posted' via the postMessage method and redispatched on the
main thread.
- A new `EmailEventStorage` class reacts to the email events and stores
it in the database. This code mostly corresponds to the (now deleted)
subclass of the old `EmailEventProcessor`
- Updating a members last_seen_at timestamp has moved to the
lastSeenAtUpdater.
- Email events no longer store `ObjectID` because these are not
encodable across threads via postMessage
- Includes new E2E tests that test the storage of all supported Mailgun
events. Note that in these tests we run the processing on the main
thread instead of on a separate thread (couldn't do this because
stubbing is not possible across threads)
There are some missing pieces that will get added in later PRs (this PR
focuses on porting the existing functionality):
- Handling temporary failures/bounces
- Capturing the error messages of bounce events
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/Toolbox/issues/475
refs https://github.com/TryGhost/Toolbox/issues/117
- The frontend/src folded would never get picked up by a code coverage tooling as scripts there are dynamically minimized and served from the server
- There's nothing to cover under frontend/public
- Logic behind these changes is the same as in second referenced issue
fixes https://github.com/TryGhost/Team/issues/2326
When importing more than 500 members, we didn't testImportThreshold at
the right time. It was called too early because the importing job was
not awaited. This also adds an E2E test for this case.
closesTryGhost/Team#2313
- Added Sent event to Post analytics and Members feed. Now post can be
Sent or Received or Bounced.
- Excluded Delivered event from Sent filter on backend.
refs https://github.com/TryGhost/Ghost/security/advisories/GHSA-9gh8-wp53-ccc6
refs https://github.com/TryGhost/Toolbox/issues/465
- Bookshelf relations allows us to edit relational records by default, which was used liberally in the codebase.
- Not having a clear track record of editable relations left the model layer prone to triggering unwanted nested saves and created a vulnerability where members were able to edit newsletter settings.
- With explicit editable relations it's easier to keep track of relations having editable access to related records. Makes the relational data modification pattern safer to use too.
- Anyone running 5.x should update to 5.24.1
Credits: Dave McDaniel and other members of [Cisco Talos](https://talosintelligence.com/vulnerability_reports)
refs https://github.com/TryGhost/Ghost/security/advisories/GHSA-9gh8-wp53-ccc6
refs https://github.com/TryGhost/Toolbox/issues/465
- Bookshelf relations allows us to edit relational records by default, which was used liberally in the codebase.
- Not having a clear track record of editable relations left the model layer prone to triggering unwanted nested saves and created a vulnerability where members were able to edit newsletter settings.
- With explicit editable relations it's easier to keep track of relations having editable access to related records. Makes the relational data modification pattern safer to use too.
- Anyone running 5.x should update to 5.24.1
Credits: Dave McDaniel and other members of [Cisco Talos](https://talosintelligence.com/vulnerability_reports)
closes Sentry ADMIN-CC8
closes Sentry ADMIN-DDM
closes Sentry ADMIN-C9F
- there are situations where the editor range when key commands are triggered does not have a head section which then throws errors due to the commands assuming there will always be a section present
- added a guard to key commands that use the head section to abort early and fall back to default Mobiledoc behaviour when the section is missing
closes sentry ADMIN-C7S
- we can't guarantee that the iframe being swapped to has rendered yet at the time we swap iframes so we need a guard around the `contentDocument.body` property existing before setting it's `scrollTop` value
refs https://github.com/TryGhost/Team/issues/2280
refs 9b0c21e0a2
As part of the email stability work, we added new `source` and `source_type` columns to `email` table, which allows us to store the email source information. The source for all existing emails before the stability work was always `html`, while newer emails will store `mobiledoc` or `lexical` directly.
While the `source` for all existing emails was populated with the `html` as part of above migration, we also need to store the right `source` for all new emails created till the feature is under a flag.
This change updates the current email flow to also store `source` with html data, so it can be used in future with new email service and allows removing old `html` column.
closesTryGhost/Team#2264
- Instead of relative paths, we can use absolute. It helps to keep code cleaner and don't worry about import when doing refactoring.
Relative paths require rewriting them in case a file is moved to another directory.
refs https://github.com/TryGhost/Team/issues/2253
refs https://github.com/TryGhost/Team/issues/2254
This package is analogous to the @tryghost/member-events package. The
events here will be consumed by the EmailSuppressionList
implementation and used to add emails to said list. They'll be
dispatched by the code which handles events received from Mailgun.
refs: https://github.com/TryGhost/Team/issues/1121
- We've decided on one preferred message for unexpected errors
- We want to use this everywhere where we don't know what to display
- We now have a GENERIC_ERROR_MESSAGE constant that we should use
refs: https://github.com/TryGhost/Team/issues/1121
- showAPIError is a method intended for formatting errors from the Ghost API
- Ghost API Errors do not have a detail field, therefore this code was redundant
- there are also no related tests
- removing now because I'm trying to cleanup and streamline all our error handling code
refs: https://github.com/TryGhost/Team/issues/1121
- In certain cases our API sends the same data for message and context.
- We will also fix this server-side, but we should also be smart in the UI and not show duplicate info
no issue
- "unhandled" `TransitionAborted` errors almost always occur as part of expected application behaviour and were causing a lot of noise in Sentry making it harder to track down real errors
- when a `TransitionAborted` error occurs outside of expected behaviour it will usually be accompanied by other errors that do get logged
- there's a long-standing Ember issue about how aborted transition errors should be handled at https://github.com/emberjs/ember.js/issues/12505
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.
refs: https://github.com/TryGhost/Team/issues/2289
refs: https://github.com/TryGhost/express-hbs/issues/161
- Themes that resuse layouts as templates trigger horrible errors, which are thrown as 500s
- But there's nothing the server is doing wrong, it's a theme user, so we downgrade these to 400s
- There is more to do here to improve the errors shown, but this is just a first step to ensure that theme issues don't look like server failures
refs: https://github.com/TryGhost/Toolbox/issues/479
Framework includes:
* command to run tests
* command to record tests
* mechanism for starting and stopping Ghost before and after each suite of tests
* mechanism for loading fixtures into Ghost before starting tests
* sample test for controlling Ghost Admin
closes https://github.com/TryGhost/Team/issues/2295
Problem:
- `<GhUploader>` is not yet converted to an Octane component so it's arguments are not read-only
- when a file is selected it sets it's `files` property which in turn updates the tracked `files` property that was passed in, and then again updates it to an empty file list when the input field is cleared
- that tracked property was never cleared once the product image was uploaded resulting in a "re-upload" attempt with an empty file list every time the product card was put back into edit mode
Fix:
- added a guard in `<GhUploader>` so it doesn't try to upload an empty file list if one is passed in as an attribute
- added a reset of the tracked `files` property in the product card once the image upload is complete
no issue
- The name of the "StaffService" is ambiguous and too generic. Lack of good naming makes one to dig into the implementation details figuring out what the service does.
- Should be named a more descriptive way
refs: https://github.com/TryGhost/Team/issues/1121
refs: dfffa309a8
- This makes a fundamental change to Ghost's server side error handling, so that no unhandled errors are used as API responses
- Anything that has been handled and rethrown as a Ghost error cna be trusted
- We also already trust a couple of known errors from bookshelf and handlebars
- Everything else is assumed to be a code error, and should not be shown as the main message
- Instead we use our generic fallback message and use the OG error as context
fixes https://github.com/TryGhost/Team/issues/2282
Added a new email service package that is used when the email stability
flag is enabled. Currently not yet implemented so will throw an error
for all entry points (if flag enabled).
Removed usage of `labs.isSet.bind` across the code, because that breaks
the stubbing of labs by `mockManager.mockLabsEnabled` and
`mockManager.mockLabsDisabled`. `flag => labs.isSet(flag)` should be
used instead.
All email depending tests now disable the `emailStability` feature flag
to keep the tests passing + make sure we still run all the tests for the
old flow while the email stability package is being built.
refs https://github.com/TryGhost/Team/issues/2268
The approach of using the service to lead email suppression data as
opposed to bookshelf relations allows us to wire things up without
having implemented the database. The getBulkSuppressionData allows us to
do this without much of a DB performance hit.
closes https://github.com/TryGhost/Team/issues/2126
- Cleaned up the following GA flags: `newsletterPaywall`, `freeTrial`, `compExpiring`, `searchHelper`, `emailAlerts`, `fixNewsletterLinks`.
refs https://github.com/TryGhost/Team/issues/2276
Portal died with an unintelligible error about portal plans/includes being undefined when there was another react error on the page. The other error somehow caused the `site` data to be empty object for Portal, and there was one place where we didn't check for the empty object while transforming the site data which caused the failure.
This change adds the guard for failing check so we don't throw the error in such cases and process as normal.
refs https://github.com/TryGhost/Team/issues/2267
This will eventually be replaced by a Mailgun specific implementation,
but for now we're using mock one which responds with fake data for
local development.
refs https://github.com/TryGhost/Team/issues/2267
This package contains definitions for the public interface of the
email suppression list as well as an abstract implementation.
refs: https://github.com/TryGhost/Team/issues/1121
- This makes several key changes to the way errors are handled in the member importer, to ensure that we only show error messages to users that we wrote.
- Fundamentally, we no longer trust all API errors, and instead only trust a set of very specific API errors. Anything outside of that is replaced with a generic error message.
- Also switches the server-side error generated for email verification (which can throw during member import) to be a HostLimitError, as that is a more appropriate class.
- Note: there are many other parts of Ghost admin that need a similar overhaul, and a similar change we need to introduce server side to fully resolve the underlying issue of bubbling up code errors to the UI.
no issue
Zip files involved a call to `await`, which caused the microtask queue to be processed. The zip file is deleted during this time because the request has already completed.
no refs.
- Navigating to `/settings/labs/import` opened the modal but also transitioned to a blank page in the background. Restructured content import modal so it's opening on top of the Labs page.
refs https://github.com/TryGhost/Team/issues/2262
Makes sure we only loop active Stripe prices. If we find an inactive
price, we also update it in our database now after this change.
refs https://github.com/TryGhost/Team/issues/2246
- This change helps avoid race conditions due to a lack of a transaction
in the email job. It also moves the status check before creating the
email batches (can take a while) to prevent other timing issues in case
the job got scheduled multiple times.
- Sets the patch option to true when changing the status of an email
batch. If we don't do this, the bookshelf-relations plugin might try to
save relations too. This could have caused a 'no rows updated' error.
- Added a test that tests if the email job can only run once
- Added logging to batching logic
closes https://github.com/TryGhost/Team/issues/2274
When clicking on a tier, just before being redirected to the Stripe checkout, a Portal modal incorrectly showed the "Now check your email" message. Instead, this now shows a loading popup as expected while the checkout session is being created and redirected to on latest portal version
closes https://github.com/TryGhost/Team/issues/2274
When clicking on a tier, just before being redirected to the Stripe checkout, a Portal modal incorrectly showed the "Now check your email" message. Instead, this now shows a loading popup as expected while the checkout session is being created and redirected to.
refs https://github.com/TryGhost/Toolbox/issues/475
- Having a single coverage config file for unit and e2e type of tests doesn't play well for tracking coverage progress. The unit test ones are run often when developing, so one can observe the changes in coverage. The e2e tests are ran mostly on CI environment and have different purposes/reach comparing to unit tests.
refs https://github.com/TryGhost/Toolbox/issues/475
- Increased statements and lines minimum required numbers as we have increased out baseline and should keep the bar as hight as possible
closes https://github.com/TryGhost/Toolbox/issues/475
- We did not have visibility and history into test coverage statistics in non-unit test suites. This data is useful identifying problematic areas and can be used to keep the code quality under control
- Enabled test coverage for e2e tests, including integration and regression tests
- Decreased the "branches" coverage requirement for the tests to pass (did not want to introduce an additional c8 configuration file just yet)
- 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: ced1fdbdd4
- This is a follow-up to #15826 that I forgot to include in the first commit. Fixes an issue where the placeholder image would offset the youtube iframe embed on initial load.
fixesTryGhost/Team#2266
The getCreatedEvents method was removed but was still used in the verification trigger. This commit adds the method again as a quick fix. A better fix will be commited later: https://github.com/TryGhost/Ghost/pull/15831, which includes tests that also have been ran on this change (but keeping it out of this commit to keep changes minimal).
closes https://github.com/TryGhost/Ghost/issues/14020
- Added CSS for `amp-youtube` to set the dimensions of the iframe instead of passing explicit height and width
attributes.
- Instead we set the video width to the viewport width and then set the height to a standard 16:9 ratio of YouTube videos.
fixes https://github.com/TryGhost/Team/issues/2085
Don't load relations we don't need anymore for the posts table. And
reload the individual post when we open the analytics page with more
relations that we actually need.
refs https://github.com/TryGhost/Team/issues/2216
This change removes old analytics code which was added under `membersActivity` flag as an experimental alpha feature to test the first versions of member analytics, and is no longer active or in use.
This change removes the remaining services and its usage that were created to manage this version of analytics but is no longer active or maintained.
- removes `members-analytics-ingress` service that was used to ingest events from Portal in this experimental feature
- removes `member-analytics-service` service that managed the events from this experimental feature
- removes usages of the 2 services and their dependency in `members-api`
- removes `member-analytic-event` model as the corresponding table for it does not exist anymore and was dropped in 5.0
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.
refs https://github.com/TryGhost/Team/issues/2216
- the analytics tag was an old alpha concept and has been moved away from since so its safe to remove
- the flag that enabled this feature no longer exists so its completely inaccessible
- There was some formatting problems on the Dashboard with certain events
- There was inconsitencies with how we formatted some events over others
- Removed 'on' and just using '-' for all
refs https://github.com/TryGhost/Team/issues/2243
no issue
- `when` makes the modifier a little easier to read for it's typical use-case, eg:
- `{{scroll-to when=(eq entry.slug this.entry)}}`
- `{{scroll-to when=(eq this.focusArea "analytics")}}`
