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.
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/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'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)
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
- 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
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 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
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 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
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
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
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
- 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/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
refs https://github.com/TryGhost/Team/issues/2371
- playwright tests were broken due to state changes based on prev tests that were not accounted for
- in case of multiple newsletters, portal tests expected another step between stripe checkout for newsletter selection
- site settings test was disabling members, but not re-enabling it back
refs TryGhost/Team#2371
- check that members can unsubscribe from newsletters by toggling
preferences in their account settings
- check that member can log out
refs
f5aae1e2c5
refs
0f9ed54a6f
- changing playwright portal tests to work for single tier setup caused failure for comped upgrade tests as they were relying on button text that changed
refs. https://github.com/TryGhost/Team/issues/2371
- two extra assertion was needed for discount and free-trials to check
if the offers are listed in ‘Active’ offers and the URLs load portal
refs https://github.com/TryGhost/Team/issues/2371
- Test enabling private site and checking access with a password
- The test flow is lacking a check for site access through password
due to a Playwirght bug. This should be cleaned up in the future
refs. https://github.com/TryGhost/Team/issues/2371
- deleteAllMembers was an unnecessary step
- since there's a generated code appended to the name of the archived offer, it had to be shorter to avoid potential naming conflicts
refs https://github.com/TryGhost/Team/issues/2371
- Adds a test for publishing and sending
- Adds a test for email only sending
- Updated some util methods in the publishing spec to remove the dependency on the post bookmark (which is not present for email only posts)
refs https://github.com/TryGhost/Team/issues/2371
Note that the "Choose" button is "Continue" when running this test
standalone so currently it needs to run with the full suite.
refs https://github.com/TryGhost/Team/issues/2371
- extracting the re-used actions to utils allows tests to be self-descriptive rather than relying on comments and keeps the selectors and related actions in one place to help refactoring if/when they change
refs. https://github.com/TryGhost/Team/issues/2371
- Test for archived offers should be moved to ‘Archived’ view of the offer list in Admin, and the offer URL should redirect to the site's homepage for logged out visitors
refs https://github.com/TryGhost/Team/issues/2371
- bumped timeout between saving and refreshing to account for slower
speeds in CI
- increased specificity for the frontend text comparisons so the output
when failing is smaller and easier to parse
refs https://github.com/TryGhost/Team/issues/2371
- the 100ms timeout was enough for local tests to pass but was still failing on CI
- bumped to 200ms and skipped the creation of a new paragraph to reduce what the editor is doing
refs https://github.com/TryGhost/Team/issues/2371
- added timeout between clicking the editor and starting to type otherwise some of the typing events could be missed causing a mismatch in actual vs expected output
ref https://github.com/TryGhost/Team/issues/2371
- updated Member exports with csv validation
- added member fixtures to be loaded into Ghost to ensure filtering
works correctly when downloading / exporting members csv.
refs https://github.com/TryGhost/Team/issues/2371
- The "data-test-*" selectors in playwright did not work with publishing channel selectors. This is a quick hack to enable working around it
refs https://github.com/TryGhost/Team/issues/2367
This ensures that a Member is not considered subscribed to any emails, so that
counts for newsletter recipients are correct. Eventually we will filter members
on their email suppression status but this is not implemented yet.
Refs https://github.com/TryGhost/Team/issues/2371
- Tests whether the post access selection of public, members, or paid-members matches the expected post visibility on the frontend.
refs https://github.com/TryGhost/Team/issues/2371
- Adds a test that schedules a post 5 seconds in the future and waits
for it to be published
- Reduced the time restrictions for scheduling:
- The minimum time in the frontend is now 5 seconds in the future (came
from 5 minutes in the future)
- The time picker now suggests 10 minutes in the future instead of the
minimum scheduling time (came from 5 minutes)
- In the backend, a post will be allowed to be scheduled if it is at
least 2 minutes in the past (came from 2 minutes in the future)
- The scheduler will publish a post if it is at least 5 minutes in the
past, and maximum 5 minutes in the future (came from 2 minutes)
refs https://github.com/TryGhost/Team/issues/2371
- tests that a free member can upgrade to a paid tier via stripe checkout and the payment details are reflected in portal and member detail page on admin
refs https://github.com/TryGhost/Team/issues/2371
- tests modifying the content of a published post
- extracted publish flow into a `publishPost` function that returns a new browser page object with the newly created post loaded
refs https://github.com/TryGhost/Team/issues/2369
- this checks whether the Offer redemption count is set to 1, which
would be indicative that the Offer was successfully counted as
redeemed
refs https://github.com/TryGhost/Toolbox/issues/479
- this includes a handful of improvements to get Playwright working on a
local environment including:
- adding `testing-browser` environment so we don't nuke `development`
environments, and makes all the necessary changes to get Ghost to
behave when this is running
- stopped running one global instance of Ghost as this doesn't provide
a clean environment
- copies a few default fixtures that are needed for the new
environment
- we should start to keep tests grouped by their area, so first we split
by Admin tests and then Portal tests, and within that we split into
setup/Tiers/Offers etc
refs https://github.com/TryGhost/Toolbox/issues/476
- The email verification trigger and host settings related bugs have been a cause of bugs in past releases. The admin client verification source did not have any test coverage in the past.
- The members test suite size is getting out of hand. This test is quite verbose, because of the state it's trying to check.
- In the future we should consider splitting up Member API (and probably other) test suites into smaller pieces.
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
no issue
This will need some work, since we are introducing a 500ms delay to wait for a network request to return. Ideally the tier expander should eventually populate itself.
no issue
Local tests can now setup Stripe during the global setup process, and the webhook server is run out-of-process.
Running tests in CI against localhost will use environment variables to setup Stripe.
Providing a test URL will avoid setting up Stripe and will assume that it is already done.
no issue
This commit allows tests to run remotely by replacing selectors with production-suitable ones (no [data-test...]).
It also allows running locally with Stripe webhooks by adding a new global setup function.
fixes https://github.com/TryGhost/Team/issues/2346
- Adds email batch browse endpoint
- Adds email recipient failures browse endpoint
- Adds new fixtures and E2E tests for the new API
- Added support for snapshot tests to have 'nullable' types.
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.
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.
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/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 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
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)
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/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
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/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
- 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>
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
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/Ghost/issues/14882
This commit totally removes Bluebird from the importer. Updated `@tryghost/promise` to use native async/await and refactored importer logic to avoid the need of `reflect()`.
fixes https://github.com/TryGhost/Team/issues/1903
MembersAgent.loginAs sends email, asynchronously via events. Which
conflicts with tests that also test emails. We cannot properly await
these events, so this is currently fixed with a timeout of 200ms. But
this was too random and unreliable.
closes https://github.com/TryGhost/Team/issues/2222
Whilst we were checking for Stripe objects being active, we were not
checking for them existing in Stripe. This adds handling to all read
request to Stripe in the payment link flow, so that we can gracefully
handle deleted objects.
We've also included an automated test which fails without this fix.
We've also improved the query to find Stripe Prices which will result
in less request to the Stripe API to check if it is valid.
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/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/461
- The codebase has ambiguous behavior with OPTIONS request. Adding tests covering edge cases for all possible variations of OPTIONS responses is the first step to solving cahceability of these requests.
- The obvious question if you look into the changeset itself would also be: "WTF did you do with test suite naming? What are these changes in admin and click tracking suites? You having a bad day Naz?". The answer is "yes" (╯°□°)╯︵ ┻━┻
- On a serious note. I've introduced multiple hacks here that should be fixed:
1. Forced test suite execution order for options request - extreme blasphemy. This was last resort decision. I went deep into trying to fixup the server shutdown in the "admin" test suite, which cascaded into failing "click tracking" suite, which has shortcomings on it's own (see notes left in that suite)
2. Exposed "ghostServer" from the e2e-framework's "getAgentsWithFrontend" method. Exposing ghostServer to be able to shut it down (or do other manipulations) was one of the pitfalls we had in the previous test utils, which ended up plaguing the test codebase. Ideally the framework should only be exposing the agents and the rest would happen behind the scenes.
- To fix the hacks above I've raised a cleanup issue (https://github.com/TryGhost/Toolbox/issues/471). I'm very sorry for this mess. The issue at hand has very little to do with fixing the e2e framework, so leaving things "as is".
refs https://github.com/TryGhost/Toolbox/issues/461
- When testing OPTIONS requests there is a need to get all possible agents available in the system. The "getAgentsWithFrontend" serves exactly this purpose - create all possible agents while starting Ghost instance only once
- This is groundwork for OPTIONS request caching tests and improvements
refs https://github.com/TryGhost/Ghost/commit/1f300fb781f0
The full customer object was not being passed to the StripeAPI service
when it already exists, this was resulting in inconsistent behaviour when
sending the customerEmail param to the API, causing `invalid_email`
errors to be thrown from Stripe and breaking the checkout.
closes https://github.com/TryGhost/Team/issues/2196
We were incorrectly assuming that all requests would have the
`customerEmail` passed in the body. Instead we were incorrectly
passing `undefined` or `''` as the `customerEmail` property to stripe,
which resulted in a validation error.
We've updated the code to pass `null` in the case of a falsy value,
which the Stripe API handles without error.
closes https://github.com/TryGhost/Team/issues/2195
The issue here is two-fold, and specific to using Offers so was not
caught by any automated tests. First, we were incorrectly comparing
the tier.id to the offer.tier.id - this is because the Tier objects id
property is an instance of ObjectID rather than a string.
Secondly we were passing through the cadence parameter from the
request body, but when using Offers this is not including in the
request, so we must pull the data off of the Offer object instead and
pass that to the payments service.
closes https://github.com/TryGhost/Ghost/issues/15740
The validation function for a Tier description was not returning the
validated value, which meant we were unable to set the Tier
description.
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`
- These tests are very slow, and make the build fail about 2/3 times
- Temporarily skipping until we can fix, as I want to get all our outstanding hacktoberfest PRs merged
fixes https://github.com/TryGhost/Team/issues/2175
- New event type `aggregated_click_event` that is disabled by default in all the existing activity feeds
- This returns click events, but only the first click events for each member/post combination.
- It includes the total count of unique link clicks for that member on that post combination
- Had to resort to some custom knex queries to make this work easily
- Requires `@tryghost/bookshelf-pagination@0.1.31`, included in `@tryghost/bookshelf-plugins@0.6.1` (this fixes an issue with custom selects breaking the total count query of pages)
- Went a bit overboard with the pagination tests to cover as much unknown edge cases as possible
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
fixes https://github.com/TryGhost/Team/issues/2129
- This changes how the activity feed API parses the filter.
- We now parse the filter early to a MongoDB filter, and split it in two. One of the filters is applied to the pageActions, and the other one is used individually for every event type. We now allow to use grouping and OR's inside the filters because of this change. As long as we don't combine filters on 'type' with other filters inside grouped filters or OR, then it is allowed.
- We make use of mongoTransformer to manually inject a mongo filter without needing to parse it from a string value again (that would make it a lot harder because we would have to convert the splitted filter back to a string and we currently don't have methods for that).
- Added sorting by id for events with the same timestamp (required for reliable pagination)
- Added id to each event (required for pagination)
- Added more tests for filters
- Added test for pagination
- Removed unsued getSubscriptions and getVolume methods
Used new mongo utility methods introduced here: https://github.com/TryGhost/NQL/pull/49
refs https://github.com/TryGhost/Team/issues/2077
- Members and Posts test suites were using a broad tiers property matcher, which is an anti-pattern for snapshot tests. Without more specific snapshots it would be very hard to track down tier-related breaking changes!
- This change is groundwork for a refactor coming in tier usage at API's output serializers
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/Ghost/issues/15537
- snapshot test created to add confidence to webhook stability and increase overall test coverage.
Co-authored-by: Kritika Sharma <kritikasharma@Kritikas-MacBook-Pro-2.local>
fixes https://github.com/TryGhost/Team/issues/2137
For the analytics page, we need the sent events to show up immediately
after sending an email. Otherwise we need to wait for emails to be
marked as received (which takes too long) before being able to show them
on the analytics page.
This adds the email_sent_event, which is hidden by default everywhere
and used on the analytics page.
refs: https://github.com/TryGhost/Ghost/issues/14882
- Opted to use the in-house `sequence` function when refactoring Bluebird's `Promise.each` to avoid deadlock issues (see 734ef66e6c).
-It's hard to know without tonnes of context if any `Promise.each` are safe to refactor to `Promise.all`.
refs https://github.com/TryGhost/Team/issues/2116
- allows site owners to edit a link in a post that has already been sent out, fixing any typos or other mistakes
- resets click counter for the edited link back to 0 so site owners can see the clicks on new link, doesn't change the overall click count
refs https://github.com/TryGhost/Team/issues/2135
The email link redirects on Pro are cached as 302 redirects in Varnish, so we're missing further clicks after the first one for each member, until the cache is invalidated. This change invalidates cache on link edits to ensure that we correctly redirect members to updated link everytime
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/2090
- This changes how sentiment is exposed in the API. Now it is exposed as a `sentiment` relation, directly on the model (no longer in counts). Internally we still use `count.sentiment`.
- Content API users (and themes) can include the 'sentiment' relation and order by sentiment.
- Updated Admin to use sentiment instead of count.sentiment
