refs https://github.com/TryGhost/Team/issues/1595
- Since adding multiple newsletters, posts may be linked to a related newsletter
- We don't export newsletters, so the related newsletter_id doesn't exist and fails the FK check on import
refs: https://github.com/TryGhost/Toolbox/issues/229
- We're getting rid of the concept of api versions inside of Ghost
- Instead of storing the supposed api version a webhook was created with, store the current ghost version
- This way we can determine if anything signicant changes in future and we need to update webhooks or something
- webhooks are one of the remaining places where we need some sort of api version handling
- in order to fixup the tests for this, I wanted to first change them to use the e2e framework
- attempting to make our framework a one-stop-shop for all requires (except assert)
- using utils for tests instead of lots of requires makes it easier to reason about how our tests interface with our code
- helps with refactoring later, and making sure that tests really do what we expect
refs https://github.com/TryGhost/Team/issues/1529
- the sender email addresses for newsletters require verification to set.
- this ensures there isn't a way around that by modifying an export file then importing it by setting it to null on import.
This pattern is similar to the current `members_from_address` setting which is excluded when importing.
refs: https://github.com/TryGhost/Toolbox/issues/229
- our api-key audience handling code is still relying on internal api version config
- the regex used is also buggy (it expects 3 parts, which isn't true without versions) and doesn't always match, in which case it can cause the tests to hang
- we already had some very similar code in the version-rewrite middleware which is also validates exact values for version and api type
- moved this code into a util inside api-version-compatibility-service
- using this code, all the tests still pass as is, but when I start to adjust them to cover more cases, none hang (test changes coming in a separate commit)
- updated usage of url-utils.urlFor to work with v3
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Hannah Wolfe <github.erisds@gmail.com>
refs https://github.com/TryGhost/Team/issues/1579
- When writing to the database, the header_image is tranformed to the transformReady path
- When reading from the database, the transformReady path is transformed to an absolute path
- Includes a test when adding a newsletter
Migration:
- Updates all newsletter who have a header_image to make sure it is saved in transform ready format
- Down operation is required to work with the old model logic and transforms it back to an absolute format
closes: https://github.com/TryGhost/Toolbox/issues/315
- For all the current versioned URLs, rewrite the URL as unversioned
- Add the accept-version header
- Add the deprecation header
- Add the link header
- This then does the content-version middleware afterwards, ensuring that rewritten requests get this in the response
refs https://github.com/TryGhost/Team/issues/1495
For single newsletters, the unsubscribe link on emails auto unsubscribed member from the newsletter. In case of multiple newsletters, we were missing the newsletter information on unsubscribe URL that will allow us to auto unsubscribe member from that specific newsletter as they intended, while allowing them option to manage other newsletter preferences via Portal UI. This change -
- adds relevant newsletter UUID on the unsubscribe url in emails
- allows portal to auto unsubscribe members from desired newsletter
refs https://github.com/TryGhost/Toolbox/issues/292
- Following the concept of having as little code in Ghost core as possible :) The email content generation is also needed to be reused in the version mismatch handling package.
refs https://github.com/TryGhost/Toolbox/issues/292
- There's a need to reuse these utils in the version mismatch notification service. Having loads of tightly coupled dependencies makes it super hard to rip out this module for reuse
- It's a groundwork for extraction of the email-utils package
- Rewrote the unit tests that were written for these utils previously - they weren't testing anything useful. The goal of this util is to generate specific content based on provided data and available templates - now the tests do test those specific things, not the mailer itself!
- This is preparation work for getting rid of API versions
- The existing code used api versions for members, but the members API is not versioned
- This caused a bug as issuer was begin set to {{admin_url}}/ghost/api/undefined
- The updated code returns the correct value and is unit tested
- Whilst cleaning up I also swapped the usage of urlUtils to consistently use urlFor, as that is our main helper
refs https://github.com/TryGhost/Team/issues/1564
- While creating a newsletter
- While editing a newsletter
- Includes tests and updated snapshots
- Igored sort_order to snapshot test because sort_order is different in CI than locally so had to ignore it in the snapshot.
- Asserting for the exact error message thrown was a bad idea
as it is different between different versions of Node... derp
- Also, don't really care, I'm just asserting that the serialize function errors under certain conditions
This reverts commit 9a0d143fb1
- main is now a precursor of 5.0 which should have email notification turned on
- had to add missing `err` in the errorHandler middleware as it was not triggering the versionMissmatchHandler otherwise
refs 275107d423
- Because there might be multiple authors being added at the same time with different values to the posts_authors table these operations should not be done in parallel! Making post insertion sequential fixed the deadlock
closes https://github.com/TryGhost/Toolbox/issues/268
- Adds more coverage to the author reassignment method as it hasn't been covered much. It should put a good base to expand upon in case a bug pops up
closes: https://github.com/TryGhost/Toolbox/issues/319
- at the moment, content-version is only set if one of our endpoints touches the request
- this was demonstrated in the e2e tests, where many of the tests that set accept-version did not receive accept-version
- by moving the middleware out of the http module and onto the api app we ensure it's always done
- I put the code in the api-version-compatibility service to keep it all co-located
- ideally we will refactor that service slightly so it only exposes middleware
closes: https://github.com/TryGhost/Toolbox/issues/317
- Added two tests for unknown versions with accept-versions set ahead and behind
- Ahead passes, but we get an error for behind
- Refactored the api-version-compatibility-service to expose its own middleware so the init sequence is correct
refs https://github.com/TryGhost/Toolbox/issues/226
- We are on the finish line to release 5.0 - the instance should be validating it's themes agains v5 set of rules.
- The prominent deprecations are `{{author}}` and `@blog` helpers removals - will throw 'fatal' errors when encountered.
refs https://github.com/TryGhost/Toolbox/issues/308
- we have a pattern of using plurals around Ghost but this was singular
- this shouldn't change any API functionality, it's just code
refactoring
closes: https://github.com/TryGhost/Toolbox/issues/296
- This is a small change to permit any known API version to redirect to an unversioned URL
- We include v2 because although it should have been deleted in 5.0 anyway, in the spirit of the change away from versioned URLs there's
absolutely no sense in forcing people to update clients that still work for no reason.
- We use a 307, because this preserves the original HTTP method, allowing POSTS, PUTs and DELETEs through as well as GETs
- We set the accept-version header on the redirect, meaning that for example with a request to the old /v4/ api, Ghost will respond as though
the client sent `accept-version: v4.0` and if there are known breaking changes, it may choose to inform the admin and owner users of these
refs https://github.com/TryGhost/Toolbox/issues/308
- this endpoint is currently used to send a test email with the post
- it currently returns a 200 with whatever the response of the mail
service is
- this body isn't used in Admin nor is useful generally because it just
contains the ID of the mailgun response
- it's better than we change it to 204 and no response
- this commit does that and updates the tests
refs https://github.com/TryGhost/Toolbox/issues/268
- After the logic change the number of posts no longer decreases - reassignment does not delete posts
- In a follow up commit will add a more sophisticated check for actual reassignment
refs https://github.com/TryGhost/Toolbox/issues/268
- When the user is removed our current pattern was deleting their posts. This didn't work well and created all sorts of problems
- As a solution we now reassign any posts that are only authored by the deleted user to the owner user
- This change also reduced the dependency on "author" field
refs https://github.com/TryGhost/Toolbox/issues/308
- I recently pluralised the API endpoint but never made the changes to
the controller file and everywhere else it's needed
- this commit cleans up that inconsistency so it should be clearer
refs https://github.com/TryGhost/Toolbox/issues/308
- we had a few mentions of `subscribers` in the test suite data generator
but this shouldn't be used any more because it's an ancienttttt concept
- removing this for v5 as it helps to clean the codebase
refs https://github.com/TryGhost/Toolbox/issues/308
- these endpoints return no body but they were implemented to return a
200 error code
- 204 would be more suitable so this commit changes that for v5
- also removes the passthrough serializer and updates the snapshots to
reflect the changes
refs https://github.com/TryGhost/Toolbox/issues/308
- this endpoint has no body to return but it was initially implemented
as returning a 200 which we couldn't change until 5.0
- this changes it to a 204, removes the serializer and updates tests to
take this into account
refs https://github.com/TryGhost/Toolbox/issues/308
- I've just renamed the `/authentication/passwordreset` API endpoint to
`/authentication/password_reset` and noticed the body object is also
badly named
- this clears that up in code and tests
refs https://github.com/TryGhost/Toolbox/issues/308
- we have a pattern of using plurals for API endpoints but it was missed
when we implemented email previews
- this fixes that for v5 and updates the tests accordingly
- there's some cleanup here to fix the API controller name too which
I'll add to the list
refs https://github.com/TryGhost/Toolbox/issues/308
- this endpoint isn't used by Admin, nor Ghost, and isn't documented
publicly
- we are nuking it in v5 so the easiest step to achieve that is by
removing the API route mounts
- there's plenty of cleanup here, including refactoring other API
controllers to avoid using the `mail` API controller, but this is the
easiest way to achieve what we want
refs: cf514cdf7
- in commit cf514cdf7 we moved the loadSettings call up to the bridge
- here we can call the async method, so we can remove loadSettingsSync altogether
- all the tests have now been changed to use the async method
refs: https://github.com/TryGhost/Toolbox/issues/229
- we are getting rid of the concept of having multiple api versions in a single ghost install
- removed all the code for multiple api versions & left canary wired up, but without the version in the URL
- TODO: reorganise the folders so there's no canary folder when we're closer to shipping
we need to minimise the pain of merging changes across from main for now
refs: https://github.com/TryGhost/Toolbox/issues/228
- we are getting rid of the concept of having multiple api versions in a single ghost install
- we no longer need versioned routing configs
- we are getting rid of the concept of having multiple api versions in a single ghost install
- we no longer need to pass the apiVersion around using res.locals
- To simplify code that uses our frontend proxy the proxy now _only_ exposes canary
refs: https://github.com/TryGhost/Toolbox/issues/228
- we are getting rid of the concept of api versions from Ghost
- this means getting rid of them from the frontend as well, and from themes
refs TryGhost/Team#1495
With multiple newsletters, members are allowed to manage their newsletter pref via email unsubscribe link with member uuid. Since Portal needs to manage member's newsletter pref via their UUID, we need new endpoints on members that allow fetch/update of newsletter subscriptions via only uuid. The endpoints return only limited data for a member that are needed for the UI.
- adds endpoint to fetch newsletter subscriptions for member via uuid
- adds endpoint to update newsletter subscriptions for member via uuid
refs https://github.com/TryGhost/Team/issues/1495
With multiple newsletters, members are allowed to manage their newsletter pref via email unsubscribe link with member uuid. Since Portal is now taking over handling unsubscribe for members, we don't need to keep the current `/unsubscribe` page as Portal can load the member's newsletter pref on site home page directly.
The redirect change is only enabled behind the `multipleNewslettersUI` flag as its in beta.
- Newsletter defaults were a static object with the default UUID resolved once
- This caused conflicts on subsequent create requests
- Defaults are now resolved as a function so each call gets a new UUID
- Promoted the flag to beta to allow manual enabling via the database
- The UI for this flag is intentionally still gated behind developer experiments in Admin
refs TryGhost/Team#1566
- Mocking a labs flag (regardless of enabled/disabled) currently has a side effect of setting any other flag to undefined.
- This meant in a test where we set a flag e.g. members-importer where we set multipleProducts, multipleNewsletters is always disabled
- This fix preserves the default state of all labs flags that are not mocked so that labs behaves how we expect
- Removed usage of GA flags in tests
- Removed tests that had GA flags disabled
Co-authored-by: Simon Backx <simon@ghost.org>
refs https://github.com/TryGhost/Team/issues/1550
- Updated email template and seder options to use the settings specified for the related newsletter
- Falls back to the default newsletter, and uses the default newsletter settings for the publishing preview because we only assign a newsletter at the point a post is published
Co-authored-by: Thibaut Patel <thibaut.patel@gmail.com>
Co-authored-by: Matt Hanley <git@matthanley.co.uk>
refs https://github.com/TryGhost/Team/issues/1545
**Changes (`members-api`)**
- Compare via https://github.com/TryGhost/Members/compare/%40tryghost/members-api%406.0.0...%40tryghost/members-api%406.1.0
- Added mapping from member subscribed to newsletters on edit/create
- When editing or creating a member with the subscribed property, it is mapped to the corresponding newletters value
- Defaults to all active newsletters with visibility = members and subscribe_on_signup = true
**Tests**
- Adds test that adds a member with subscribed = true
- Adds test that adds a member with subscribed = false
- Adds test that edits a member with subscribed = true
- Adds test that edits a member with subscribed = false
refs https://github.com/TryGhost/Team/issues/1561
With multiple newsletters, unsubscribe links will also need to have a unique reference to the newsletter that the email is for, so that we can unsubscribe members from the particular newsletter automatically when they click on the link.
As our existing pattern for members is to use UUID as the external unique reference, this change adds UUID to newsletter schema and populates the existing newsletters with a UUID value.
- adds new `uuid` column to newsletter schema
- updates newsletter model to add default uuid
- updates default newsletter migration to add `uuid`
- drops nullable on `uuid` column later in migrations once we have populated existing newsletters
closes: https://github.com/TryGhost/Team/issues/1553
- we want to be explicit in what values are supported
- we want the values that are supported to be supported to also be explicit and clear
- without this API users can set the value to anything and the active/not active logic will work, until such time as we introduce further statuses
- this means introducing a new status could be a potential breaking change and lands us in horrible hot water
refs: https://github.com/TryGhost/Team/issues/1557
- There is currently no default order for the members API
- This is done at the API level purely for the endpoint, not in the model using orderDefaultOptions
- This is because orderDefaultOptions affects findPage and findPage is wrapped by membersAPI.members.list
- That in turn is used in multiple places, e.g. getting member counts for emails and getting members for exports
- There is currently no created_at DESC index on the table, so we don't to impact performance too much
- This ensures it's only affected when paginating
refs https://github.com/TryGhost/Team/issues/1478
- Moved all admin API members tests to enable the multiple newsletters flag
- Checks if the susbcribe events are added correctly when adding or removing newsletters
- Checks if susbcribe events are added for default newsletters
refs https://github.com/TryGhost/Team/issues/1550
- Switched to using the newsletter design settings over the global settings
- Made the `newsletter_id` property available in the Admin API Post resource
- Added the `showHeaderName` variable that can be used in the post html template
refs https://github.com/TryGhost/Team/issues/1545
- Remapped `member.subscribed` value based on newsletter subscriptions in API output
- Enabled filtering by subscribed status for multiple newsletters
Co-authored-by: Matt Hanley <git@matthanley.co.uk>
refs https://github.com/TryGhost/Team/issues/1552
- The API doesn't enforce a unique sort order but we infer the "default newsletter" based on ordering so we need to ensure a consistent and deterministic ordering behaviour
- Many of these tests were using API calls to get IDs or check side effects
- This makes snap files much harder to read, where keeping tests more minimal gives us the same test coverage
- Also updated the tests to include members, so we have some real live counts
refs https://github.com/TryGhost/Team/issues/1524
- We need to fetch the post newsletter to grab the slug as it's needed for the member NQL filter.
- We can then use the newsletter slug and append it in the existing member NQL filter.
- Removed `subscribed:true` when an email is sent to a newsletter and replaced it with the newsletter id
- Added `status:-free` when an email is sent to a newsletter with `visibility` set to `paid`
- Added tests what happens when you publish without newsletter_id
- Added tests what happens when you publish with newsletter_id
Co-authored-by: Simon Backx <simon@ghost.org>
- Updated the member fixtures to have 6 subscribed and 2 unsubscribed members
- Added an inactive newsletter
- Updated newsletter-member relations to reflect the 6 subscribed and 2 unsubscribed, but with different cases:
- 3 subscribed to default
- 1 subscribed to secondary only
- 1 subscribed to default + secondary
- 1 subscribed to secondary + inactive
- 1 subscribed to inactive only (i.e. not subscribed)
- 1 not subscribed at all
- With these changes, I needed to update the members snap as 2 members appear as subscribed:false in many tests
- I also needed to update some posts regression tests, as 2 less members get emailed in 2 tests
refs https://github.com/TryGhost/Team/issues/1524
- This enables admins in the ghost admin to have an overview of the total posts/members associated with a newsletter.
- Follows the `?include=count.x` convention used by other resources
refs https://github.com/TryGhost/Team/issues/1495
- removes subscription from all newsletters for a member on click of unsubscribe link in email
- allows the new multiple newsletter system to work with existing unsubscribe flow
refs https://github.com/TryGhost/Team/issues/1478
The `newsletter_id` is nullable for now to remain compatible until we have a proper data migration + updated code to set it on inserts
refs https://github.com/TryGhost/Team/issues/1532
- Added before the migration in https://github.com/TryGhost/Ghost/pull/14468 to populate the default newsletter
- The fixture for the default newsletter has a different value than the model and schema default
- This is because by default the newsletter name is the same as the site title, and the site title is already shown
refs https://github.com/TryGhost/Toolbox/issues/280
- ctd of putting pieces together to allow Ghost notifying owner and admin users about version mismatch errors
- The `@tryghost/mw-api-version-mismatch` in a combination with api version compatibility service make the whole notification process play nicely :)
- The flow of the logic from the request to a sent notification email is following:
1. Request comes is with an Accept-Version header that's behind current Ghost version and is not supported
2. mw-error-handler middleware's 'resourceNotFound' detects such request and returns a 406 with a special 'code' identifying if the version of the client is ahead or behind
3. mw-api-version-mismatch intercepts the 406 request with "code === 'UPDATE_CLIENT'` and calls up APIVersionCompatibilityService
4. emails are sent out to active owner and admin users
- The above flow is also illustratd in the e2e tests that come with the changeset
refs https://github.com/TryGhost/Toolbox/issues/292
- When version missmatch handling is done in Ghost we need to store the 'Accept-Version' header values that have been already processed in the past (to avoid sending notifications about the same mismatch multiple times)
- The `version_notifications` will be storing an array with handled versions like so: `['v3.44', 'v4.23', 'v4.39']`.
- The emailing logic and processing is slightly similar to how "notification" key is handled, that's why I've placed the definition of this new key close by.
refs https://github.com/TryGhost/Team/issues/1520
**Changes in members repo**
Bumped to `5.12.0`, with the following changes:
- Compare differences via https://github.com/TryGhost/Members/compare/%40tryghost/members-api%405.11.1...%40tryghost/members-api%405.12.0
- Instead of doing the matching of the offers and subscriptions by looking at the offer redemptions, we can now look at the offer_id from subscriptions.
- This also fixes an issue where we don't attach the offer object to subscriptions in the members' browse method
- Updated browse behaviour to match the read behaviour of members (product relation needs to get loaded because it is missing in member.products if the subscription is expired).
**Tests**
- Includes test to see if the API correctly returns the offer object when fetching one or multiple members
- Check if the return format is the same for the read, edit and browse members admin API endpoints (offer was missing in subscriptions)
- Snapshot files have been updated because now the offer is returned in subscriptions (content length increased)
refs https://github.com/TryGhost/Team/issues/1533
- Retrieves one newsletter
- Makes the newsletter resource consistent with the other resources
- Solves an issue with the admin expecting the route to exist
refs https://github.com/TryGhost/Team/issues/1533
- Needed to create the read newsletter endpoint to make the newsletter resource more consistent with the other resources
- Read is available to admins like other newsletter actions
refs https://github.com/TryGhost/Team/issues/1519
**Tests:**
- Tests whether the metadata from an offer is read correctly and stored in the database
- Test that invalid offer ids are ignored
**Changes in members repository:**
- Compare changes here: https://github.com/TryGhost/Members/compare/%40tryghost/members-api%405.9.2...%40tryghost/members-api%405.11.1
- The `offer_id` column of subscriptions is set based on the coupon id from Stripe
- `getByStripeCouponId` method added in the offers repository (required to look up an offer from a stripe_coupon_id)
- the `members-payments` package was bumped twice (once for changes, once for undoing those changes, my bad). Nothing else has changed in that package.
refs https://github.com/TryGhost/Team/issues/1469
Previously, members were subscribed to all available newsletters by default when added. This change updates the default newsletters subscription for member to take into account newsletter preferences for auto opt-in(`subscribe_on_signup`) as well as `visibility`.
refs 3381dae1e6
refs 4a4b0cc8a6
refs f432ee9aa6
- The initial tests didn't take into account the veersion of the Ghost instance would be changing contantly. The "context" part of the returned version missmatch errors should be a regex with only partially hardcoded message.
refs 3381dae1e6
- these tests contain snapshots that hardcode the Ghost version into them
- this is incorrect but we need to unblock `main` so I'm commenting them
out until someone can look at them
refs https://github.com/TryGhost/Team/issues/1519
Migration that adds the (nullable) offer_id column to members_stripe_customers_subscriptions.
- New reliable way to know which offer is active for a given subscription (currently we compare the tier and cadence in offer redemptions)
- We'll create a separate migration to backfill all the offer_ids, but only after we updated the code to also store them correctly for new or updated subscriptions (https://github.com/TryGhost/Team/issues/1520)
- Allows us to backfill the MRR of all subscriptions to account for forever offers
refs https://github.com/TryGhost/Team/issues/1451
refs https://github.com/TryGhost/Team/issues/1456
Tests for updating `members-api` package to `5.9.0` (happened in earlier commit), which includes the following changes since `5.8.0`:
* Simplifies the calculation of MRR deltas, which will make it easier to update MRR to include offers and cancellations in the future.
* Adjusted MRR and MRR delta calculation to consider "forever" duration offers (only if dashboardv5 flag is enabled)
* Uses the discount information from Stripe to calculate the MRR (this was the easiest way to include it + also supports manually created discounts from users)
* Full difference in https://github.com/TryGhost/Members/pull/387
New tests:
- Checks calculation of MRR when using forever vs repeating discounts
- Checks calculation of MRR with dashboard v5 flag enabled/disabled
- Checks calculation of MRR for yearly and monthly subscriptions with forever offers
- Checks updates of MRR and MRR_delta when adding a forever discount to an existing subscription
- Checks updates of MRR and MRR_delta when canceling a subscription with a discount
refs TryGhost/Team#1513
- nullable `sender_name` allows us to use auto fallback of site title for sender name without setting any explicit value for it.
no issue
- There's no need to init whole models module in this test. Simplified mocks are easier to understand without any "magic" that's going on behind model's module "init()" call
refs https://github.com/TryGhost/Team/issues/1490
With multiple newsletters, members can choose their newsletter subscription preference in Portal while signing up. This change handles newsletter preference data for both free and paid members via magic link/stripe checkout and saves it for the newly created member.
refs https://github.com/TryGhost/Team/issues/1502
- Support the `newsletter_id` only when sending a newsletter
- Default to the default newsletter when `newsletter_id` isn't specified
- Ignore the `newsletter_id` parameter when passed in the post body
refs https://github.com/TryGhost/Team/issues/1504
- The permissions were missing in the fixture file
- This caused some Ghost installs to not have the right permissions
- This is fixed by adding the missing permissions to the fixture file and creating a migration to resolve the missing permissions
refs https://github.com/TryGhost/Team/issues/1484
While sending a post to a specific newsletter, we'll need to get list/count of members eligible to receive the post. This change enables members admin API to filter list of members on specific newsletter by their slug.
closes https://github.com/TryGhost/Team/issues/1491
With multiple newsletters feature, a site should always have at-least one newsletter by default. Also, as with the default product, the default newsletter also needs to be renamed to the site title during the setup flow.
- adds default newsletter to main and test fixtures
- updates setup flow to rename newsletter name and sender name to site title
- updates model to extend default value for fields
- updates test
refs eee8f364de
- Part of the snapshot file was not commited with a referenced commit
- Also while at it, had another look at naming used and have cleared up couple places
refs https://github.com/TryGhost/Team/issues/1457
We want to save the MRR with a subscription to simplify the calculation of the total MRR once, in 5.0, we also take 'forever' offers into account into the MRR (so we can just SUM the MRR of all subscriptions).
- Sets the MRR to 0 for now.
- Separate commit will fill in all the values in a data migration, but this needs to get merged first because we need this new column in order to update the members-api package (so we already save the MRR before doing the data migration).
- Updated `test/e2e-api/admin/legacy-members.test.js` with improved body assertions.
refs https://github.com/TryGhost/Team/issues/1500
- newsletter schema columns were using incorrect syntax for `isIn` validations
- adds schema test to make sure schema `isIn` validations are in correct format
- the route settings code was factored out into its own service, but the tests were not moved
- this moves them so it's clearer what tests are for what service
refs https://github.com/TryGhost/Team/issues/1497
- This enables us to keep track of which newsletter an email was sent to even if the related post is deleted.
- Adds the `newsletter_id` property to the email API
refs https://github.com/TryGhost/Team/issues/1500
The newsletter table schema has bunch of changes to go through for new and existing columns, this consolidates the schema changes into a single re-create table migration that drops and adds the newsletter table with correct schema. The table re-create migration needs to run before any of the tables using newsletter as foreign key. The changes include -
- new columns for design related fields
- new slug column for filtering
- unique constraint to `name` column
- remove `default` column (noops the existing default column migration)
- `sender_reply_to` has a default of newsletter and a validation of ['newsletter', 'support']
- updated default values for `subscribe_on_signup` and `recipient_filter`
* Added subscription_id to MRR events
refs https://github.com/TryGhost/Team/issues/1453
As part of 5.0 we want to have cancelled evetns affect MRR. We are going
to backfill and begin populating cancelled events _before_ 5.0 however,
adding a migration to set the MRR deltas as part of 5.0
This migration will need to associate the cancelled events with a
subscription, so we are adding the subscription_id now. This will allow
us to guarantee that all cancelled events will have a subscription_id.
The column is missing a NOT NULL and REFERENCES constraint because
we have not populated the missing values. These will be added in the future
once we have populated the column for all rows
refs https://github.com/TryGhost/Team/issues/1248
This is the underlying cause of the problems we've seen whilst handling
Stripe webhooks. A transaction ensures that the operations are atomic,
but not that they can run concurrently.
If you have some code which does this, concurrently:
1. Starts a transaction
2. Reads a value
3. Changes the values
4. Ends the transaction
Without applying the `FOR UPDATE` lock - you will have both sequenes
read the same value at step 2.
With the `FOR UPDATE` lock - one of the sequences will hang at step 2,
waiting for the other transaction to end, at which point it will resume
and read the _changed_ value.
Because the `edit` method explicitly does a read followed by a write, we
have also add the `FOR UPDATE` lock to this by default, to avoid any race
conditions. This does however require that `edit` is called within a
transaction. An issue here https://github.com/TryGhost/Team/issues/1503
considers running in a transaction by default.
refs https://github.com/TryGhost/Toolbox/issues/280
- The Accept-Version/Content-Version header handling is shared between all APIs and should not structurally belong to any of the admin/content/members folders
refs https://github.com/TryGhost/Toolbox/issues/280
- This change covers two use cases:
- The accept-version > current version + the request cannot be served: ERROR CASE 1
- The accept-version < current version + the request cannot be served: ERROR CASE 2
- Along with 406 status there's additional information about the probable cause and action to be taken by the Ghost site owner or an integration talking to the Ghost API.
- These errors is designed to allow introducing breaking API changes gradually and have meaningful information when the requests cannot be server any longer
refs https://github.com/TryGhost/Toolbox/issues/280
- In response to 'Accept-Version' header in the request headers, Ghost will always respond with a content-version header indicating the version of the Ghost install that is responding. This should signal to the client the content version that is bein g served
- This is a bare bones implementation and more logic with edge cases where `content-version` is served with a version value of "best format API could respond with" will be added later.
refs https://github.com/TryGhost/Team/issues/1470
Instead of counting the MRR by resolving all the deltas from the past until now, we should start with the current calculated MRR and resolve it until the first event. That would give a more accurate recent MRR (in exchange for a less accurate MRR for older data) and allows us to limit the amount of returned days in the future.
- Includes MRR stats service that can fetch the current MRR per currency
- The service can return a history of the MRR for every day and currency
- New admin API endpoint /stats/mrr that returns the MRR history
- Includes tests for these new service and endpoint
refs https://github.com/TryGhost/Team/issues/1474
- The `default` concept will be replaced by the first newsletter based on the `sort_order`
- This removes the `default` value from the newsletter API
- This simplifies the design to make the api and datastructure more maintainable
refs https://github.com/TryGhost/Team/issues/1471
- This is a many-to-one relation so that many posts can be linked to a specific newsletter
- The `newsletters` table had to come first in the schema file so that it's initialized before the `posts` table (because of the foreign key)
- Updated the model to make sure the new field doesn't leak in the API for now
- This migration isn't using the `createAddColumnMigration` util because of a performance issue. In MySQL, adding/dropping a column without `algorithm=copy` uses the INPLACE algorithm which was too slow on big posts tables (~3 minutes for 10k posts). Switching to the COPY algorithm fixed the issue (~3 seconds for 10k posts).
- SQLite isn't using the codepath where we run a raw SQL query because `knex` is doing multiple queries to add/remove a column
refs https://github.com/TryGhost/Team/issues/1469
Currently, all new members get auto subscribed to the default newsletter. This change adds same behavior with multiple newsletters by auto subscribing all available newsletters on site for new members(If flag is enabled).
Note: In future, this will also take into consideration the `subscribe_on_signup` flag for a newsletter to filter which newsletters should a member be auto-subscribed.
- adds newsletters service for working with newsletter data
- bumps `@tryghost/members-api` package which handles default subscription
- adds new test fixture/data for newsletters
- The url service was moved from frontend to server some time ago but the tests were forgotten
- This is only being done now because in 5.0 major changes are happening and it'll be annoying if the
files move on that branch
refs https://github.com/TryGhost/Team/issues/1372
- Added tests for Stripe webhooks that cancel a subscription (paid and complimentary)
- Tests for manually adding a complimentary member, and removing it again (the 'new' way)
- Added tests for creating new members (paid, complimentary)
- Includes tests for the `stripe_customer_id` property when creating new members (this is broken, fixed by https://github.com/TryGhost/Members/pull/378#issuecomment-1070835800)
- Deduplicated some nock code for Stripe
- Improved event assertions and interference between multiple tests in the giant members test file (by asserting only for the affected member id instead of all events).
- This fits more closely, as this service is to so with rendering helpers and small parts
- Whereas we want to use "rendering" for things concerned with rendering pages
refs https://github.com/TryGhost/Team/issues/1469
- updates member model to add relation to newsletter via pivot table
- updates member api serializer to include newsletter data
- updates tests
refs: https://github.com/TryGhost/Ghost/commit/11867ab43
- These checks live in the wrong place. They are mostly a frontend thing
- The only server place they were used was slack and that was fixed in 11867ab43
- Moving these to the frontend they fit neatly into the frontend data service
- This is the only piece of server code that relies on the schema.checks, the rest are all frontend
- IMO this code should actually check for the post properties that the slack message needs
- OR it should switch based on the event type
- either way there's no need to have a shared util for this simple use case
- especially becaue it's confusing the use case for it and creating cross-coupling between server and frontend
- Some of the helpers inside the routing service would be better suited to their own service
- These two helpers fetchData and entryLookup talk to the API to get data & so make a decent start for a data service
- The data service would be the single point of contact with the API for the frontend
- Doing this now cos I'm moving some files around ahead of deleting things for 5.0
refs https://github.com/TryGhost/Team/issues/1469
With multiple newsletters, members will now be able to subscribe to one or more newsletters on the site. Previously, the subscription to default newsletter for a member was controlled via a single boolean `subscribed` column on the member table.
This change allows mapping multiple newsletters to a member via new pivot table that stores relation between a member and newsletter.
- adds new `members_newsletters` pivot table
- update tests
- Updated express-test to latest version with new expectEmptyBody assertion
- Updated all the tests that used matchBodySnapshot for an empty body to use expectEmptyBody instead
- Updated all the snapshots that were affected manually, and verified running the tests works as expected
refs https://github.com/TryGhost/Team/issues/1463
- Allow admins to perform all newsletter operations
- We can adjust and be more permissive in the future if needed
- Added the tests back as permissions are configured correctly now
refs TryGhost/Team#1458
refs TryGhost/Team#1459
refs TryGhost/Team#1372
- Added a new stats service, which is divided into several categories. Currently only the 'members' category for member related stats.
- When there are missing or corrupt members status events in the DB, the totals returned by the old member stats endpoint (`/members/stats/count`) were wrong. This is fixed in the new service by counting in reverse order and starting with the actual totals.
- New Stats API, with the new `/stats/members/count-history` endpoint.
- This new endpoint also returns the paid deltas -> dashboard 5.0 will show subscribed and canceled paid members for each day
- Includes tests for the new stats service and endpoint
refs https://github.com/TryGhost/Team/issues/1463
- This enables listing, creating and editing newsletters
- The tests are commented out as the permissions will be added in a follow-up commit
- Snippets are are one of the most recently implemented full e2e features
- https://github.com/TryGhost/Ghost/commit/13f653a12 updated the serialier pattern
- This updates the tests so _everything_ is shiny and new
refs: https://github.com/TryGhost/Toolbox/issues/245
- The default behaviour of a serializer is to call a mapper for each object
- Instead of all the boilerplate code we had in the snippets serializer, all we need is a single mapper function
- Added tests for the mapper function as well
refs https://github.com/TryGhost/Team/issues/1433
- The `default` property stores whether a newsletter is set as default by the admin
- The `status` property stores whether a newsletter is archived or not
- The `recipient_filter` property is only storing whether a newsletter is "paid-only" or not for now, although it can be expanded to more specific filters in the future
- The `subscribe_on_signup` property stores whether a new member should be automatically signed up to the newsletter
- The `sort_order` property enables displaying the newsletter list in an order chosen by the admins
refs: https://github.com/TryGhost/Toolbox/issues/245
- .all methods are fallback serializers not to be run as well as a custom serializer
- The default serializer is also a fallback
- The "All" file with before and after are global hooks that _always_ get run as well as other serializers
- There's a lot of room for further improvement here especially with naming but this logic makes more sense
for the usecases AND doesn't affect v2 & v3 etc. We can do another pass after 5.0
- Allows mocking lib/common/events - stubs event.emit
- Means we can check that events fired, but their sideeffects won't happen, useful for things like bulk email
- I also reorganised the mock manager file to group related functions together
refs: https://github.com/TryGhost/Team/issues/1446
- changed mail API tests to use the new e2e test framework
- added the missing retry test - which has the wrong response format :(
mail
refs https://github.com/TryGhost/Toolbox/issues/213
- `better-sqlite3` doesn't like multiple queries in the same statement
so we can make the change here to split them up ahead of the switch
refs: https://github.com/TryGhost/Ghost/commit/e68cb8b31
- I found that some of the places we use rewire are totally unnecessary
- Rewire seems to mess with coverage sometimes
- It's also a code smell in general so I've ripped it out where possible
refs: 0ef5a5c97a
- As per the previous commit, our mixed filename casing inadvertently resulted in a bug
- The casing in the codebase is meant to be kebab-case always, so fixing this everywhere that's relevant to the API whilst there's a good reason
refs: https://github.com/TryGhost/Toolbox/issues/245
refs: https://github.com/TryGhost/Team/issues/1360
- As a result of my changes in https://github.com/TryGhost/Ghost/commit/3bd4d098 the members connect endpoint had started returning JSON
- This is because the members connect endpoint relied on the old default behaviour of the serializer being to return no response, whereas now it does our default JSON response format
- I had written a tool to iterate over all endpoints and ensure that they all had explicit serializers before changing the default behaviour, but it missed this endpoint due to the snake case naming
- I have double checked and this was the only missed endpoint, the only other one was member_signin_urls.permissions but that was not a true endpoint and was removed in https://github.com/TryGhost/Ghost/commit/202696382
- Note: the snapshot file for this test was generated from running the test against https://github.com/TryGhost/Ghost/commit/e6b92aed9 - one commit before I added the new default behaviour.
- Without the new serializer this test fails on main
- With the new serialzier, this test passes again, showing the response format has gone back to what we expect
refs https://github.com/TryGhost/Toolbox/issues/241
- The `engines.ghost-api` property has been deprecated and the support for it will be dropped in Ghost v5 due to versionless nature of the Content API.
- When uploading a new theme or activating existing one that uses ghost-api in it's config a warning will be shown to the user
refs: https://github.com/TryGhost/Toolbox/issues/245
- we don't need this serializer because the default serializer will call the tags mapper
- for now I've changed, rather than removed the tag serializer test as this shows default works the same!
refs: https://github.com/TryGhost/Toolbox/issues/245
- There's no need for a mapper or serializer as labels uses the default behaviour
- Added a full suite of tests, consolidating from regression and using the new framework to prove nothing is broken
- settings cache was appearing as untested because we use rewire!
- rewire was totally unnecessary in this case, so I removed it
- updated to 100% test coverage whilst there, including removing one unreachable branch
- commented some undesirable behaviour I found whilst trying to reach all branches
- I don't want to change the behaviour to return false correctly without having a reason beyond improving coverage
refs: https://github.com/TryGhost/Toolbox/issues/245
- Added a serializer called default to the canary API
- Ideally, this would be part of the shared framework, but this would change v2/v3 and we're about to get rid of them
- Therefore, we change just canary for now, and we can refactor again later.
- Added wiring to handler that uses the default serializer, if there is a default, and isn't an explicit serializer for the endpoint
- Removed the invites serializer, so that one endpoint now uses the default
Note: previous commits have added explicit serializers to every endpoint, this is the first step towards paring
that back so that we have less serializers overall, not more!
refs: https://github.com/TryGhost/Toolbox/issues/245
- Upload, updateMembersEmail, validateMembersEmailUpdate & disconnectStripeConnectIntegration were all missing serializers
- Upload is an as-is response, same as download
- updateMembersEmail, validateMembersEmailUpdate & disconnectStripeConnectIntegration are all passthroughs with no response
- With the upcoming refactor we want to have all the serializers defined explicitly
- This will allow us to change the default behaviour
- Updated the file based tests to prove the body doesn't change
- Tests were added to cover updateMembersEmail, validateMembersEmailUpdate & disconnectStripeConnectIntegration in 68c1bc0285
refs: https://github.com/TryGhost/Toolbox/issues/245
- sendTestEmail was missing a serializer because it's deliberately a passthrough
- With the upcoming refactor we want to have all the serializers defined explicitly
- This will allow us to change the default behaviour
- Updated the tests to prove the body doesn't change
refs: https://github.com/TryGhost/Toolbox/issues/245
- The destroy endpoint was missing a serializer
- Instead of adding one, I've refactored to use an all method that's a passthrough
- Updated the tests to use the same pattern as others to make it clearer this is tested
refs: https://github.com/TryGhost/Toolbox/issues/245
- The destroy endpoint was missing a serializer
- As this serializer uses the same createSerializer pattern as members, I've copied the passthrough from members into here
- This ensures the behaviour will stay the same when the default behaviour changes
- Updated the tests to prove the body doesn't change
refs: https://github.com/TryGhost/Toolbox/issues/245
- The destroy endpoint was missing a serializer
- Instead of adding one, we've refactored to use the standard structure for this serializer
- Updated the tests to prove the body doesn't change
refs: https://github.com/TryGhost/Toolbox/issues/245
- There was only a serializer in place for redirects.download.
- Upload was falling through, which means nothing happens by default atm
- We want to change this default, so I'm making sure all our routes have serializers declared and tests
- Updated the tests and checked the behaviour was the same before and after:
- We can't use our new framework here yet because it doesn't support uploads or downloads
- Instead, just add simple matching for the body of the responses
refs https://github.com/TryGhost/Toolbox/issues/241
- The `engines.ghost-api` property has been deprecated and the support for it will be dropped in Ghost v5 due to versionless nature of the Content API.
- When uploading a new theme or activating existing one that uses ghost-api in it's config a warning will be shown to the user
- split out the two tests that use files, as the new framework doesn't support this yet
- convert the 3 existing tests for the settings endpoint to use the new framework
- introduced a new pattern of using a function to generate a matching array, so that we can do extra stuff
- in this case, just one of the items needed a matcher for the value, which results in slightly weird code
- wrapping in a function gives us somewhere to do this and leave a comment
- some tests were inconsistently using copy-pasted matchers
- instead, we should define them once and use them everywere
- this keeps the tests easy to read and maintain
refs https://github.com/TryGhost/Team/issues/1141
- switched to the same member fetch method as used in `GET /member/:id/` so there's consistent data available when rendering the API responses
- Attempted to make it clearer what the db utils do and what each one is for
- Clearly marked the old clearData & initData as deprecated, we don't want to mix db and url service resets
- The fast truncateAll method can fail
- If it does, use knex-migrate to do a safe reset of the DB instead
- This is being done to try to make test runs more reliable in the face of database errors
- if the database does not exist, ignore the error
- tables that don't exist don't need truncating
- instead wait until the next thing calls init, that will cause a fresh db to be created in the correct state
- inside of reset, if the truncate fails then do a full reset instead
- some code in tests were still using various unreliable methods to determine the database in use
- DatabaseInfo is the correct method to check if the db is mysql or sqlite
- Exposed DatabaseInfo via our test db-utils to make this easier
refs c4470ff732
- labs flag was removed under the false assumption it was a client-side only flag but the `last_seen_at` property in API responses was also gated meaning the member details screen showed "Not seen yet" and the members list did not show the last seen date of all members when filtering
refs https://github.com/TryGhost/Team/issues/1367
Because we are passing through a different member object as the context
in the get helper, the content gating was not working correctly, as the
member was missing a status property, this adds the property which fixes
content gating.
- Added extra tests for get helper {{access}} property
- Added extra test for {{access}} property in next_post helper
- In the future we might want to update the tests so they test the whole request -> HBS context flow. Currently the has context is still stubbed manually.
