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.
- We've been seeing weird errors with tables not existing when running tests locally
- This appears to happen after an error causes the tests to abort
- This change includes two fixes:
1. we triggers a full DB reset just before the entire test suite runs
- this is done by wrapping the override file for tests that use a db, and supplying a mochaGlobalSetup hook
2. catch errors when attempting to do a fast truncation-based reset & init, and do a full reset & init instead
- These two changes should ensure the DB is always in the state we expect when running a new test suite
closes https://github.com/TryGhost/Team/issues/1426
When fetching tiers using the content API, we incorrectly returned all tiers including archived ones unless the active:true filter is passed. Correct behaviour is to always hide archived tiers, so this filter should not be required.
- forces `active:true` filter for tiers content api browse
- updates test to check for archived test removal in tiers content api
- the setup() function returns a function that's expected to be called as a mocha hook
- wrapping this in a function means it doesn't get called properly
- therefore the db setup was never being called for this test
fixes: https://github.com/TryGhost/Team/issues/1424
- This moves the require of the members service after Ghost has been booted normally.
- This gives the tests time to properly setup temporary folders before the paths get used.
- In turn this prevents issues in the legacy-members tests, which were affected by the Member Service being called too early in this test.
refs: https://github.com/TryGhost/Toolbox/issues/245
- Ghost's API framework has a mixed up concept of what a serializer is. Mappers are true serializers! What we call serializers are little more than a small formatting step.
- This PR splits mappers into individual files and uses the endpoint's docname as the mapper name. This will help us to automate the calling of a mapper for an endpoint later.
- This is one tiny step in reworking the framework to need less code to make it work, and to have clearer concepts for how to do things.
As multiple tiers is now GA, we want to allow devs to be able to work with Tiers and offers via content/Admin API. This change -
- updates fixtures to add permissions to admin integration role for new sites
- adds migration to update existing sites to have correct permissions for role
- whitelists add/edit/read/browse on tiers and offers API for integrations
refs https://github.com/TryGhost/Toolbox/issues/169
- There was not much clarity around how the tokens created for the versioned API audience would behave when non-versioned API is introduced. The tests added here illustrate the tokens being forward compabible (created for versioned -> verified at non-versioned API) and not backwards combatible (created for non-versioned -> verified at versioned)
refs https://github.com/TryGhost/Toolbox/issues/169
- Before releasing Ghost v5 we would like to move all canary-related URLs to a non-versioned format, which will become a default in v5.
- 'canary' is by definition unstable, so breaking any unprepared client explicitly using the canary is expected
- Removed the aliased /content/ and /admin/ apps from app.js because with updated configuration they become duplicates of 'canary' endpoints
refs https://github.com/TryGhost/Team/issues/1267
Multiple tiers is coming out of beta -
- allows site owners to create multiple tiers
- allows setting individual welcome page for each tier
- allows setting visibility for individual tiers for portal and themes
refs https://github.com/TryGhost/Team/issues/1363
- When uploading a zip of images in Settings > Labs > [Import], it will skip images that have an uppercase extension, citing an 'unsupported file type' error.
- Cause: Glob ignored those files when matching extensions in ImportManager
- Fix: Added nocase option where needed
- Extended tests to also test the processZip method of ImportManager with getFilesFromZip
- Added isValidZip for zip with uppercase image
- Cleaned up JSDoc in ImportManager, and replaced some older JS syntax
Fixed zipContainsMultipleDataFormats error never thrown:
When a zip combines two data formats, no error was thrown.
- The promise error was only returned in an _.each loop, but never thrown
- Previously when combining multiple data types in a zip file, no error got thrown
- Added a test for this error
- Also added a test for noContentToImport error
Other errors and fixes:
- Added missing length in getBaseDirectory check
- getContentTypes fixed (returned duplicate values). Type error came up after adding all JSDocs
- updated tests to match real types from JSDoc and pass type validations
- Rewrote some methods in the async await syntax
- Added tests for ImportManager clean up
- Our old fixtures were designed as a guide to getting started to Ghost, but they got in the way
- The old fixtures now live as part of ghost.org/resources - a living guide to starting with Ghost
- These new fixtures mean the site is ready to go as soon as it's setup
Co-authored-by: Hannah Wolfe <github.erisds@gmail.com>
- we send the roles data array in when we're changing the role of the
user
- if we send an empty array, we don't want to edit the user's role
- the code _thought_ that's what it was doing, but we only check the
falsiness of the array, which is truthy for `[]`
- it also needs to check the length of the array
- this commit includes a test which would fail with a 500 error without
the fix
refs https://github.com/TryGhost/Team/issues/1284
When you create a new post with a tag slug that contains spaces, those spaces will get replaced by dashes. But instead of reusing an existing tag, a new tag is always created.
- New tag slugs are cleaned up before matching with existing tags in the Post model onSaving method
- Cleaned up multiple loops in onSaving of Post model
- Cleaned up syntax when cleaning up tag slug
- Added tests for slugs with spaces
- Added test for too long tag slug causing duplication
Some tests need to run without Stripe connected/enabled, which is the
default at the moment. This method will disconnect stripe for the
lifecycle of the API agent.
refs https://github.com/TryGhost/Team/issues/1188
The products API did return a 500 error when you tried to store product benefits with an empty name. This should throw a 422 Validation Error instead. This change includes some tests for this error. The error itself has been solved by updating the bookshelf-relations dependency earlier.
- Added test when creating a new product with an empty benefit name
- Added test when updating an existing product with new benefits, with an empty name
- Added a test that creates a tier with benefits
no issue
- The support for the misformated roon imports was temporary anyway, based on the comments in code. It's also unecessary to keep around any code related to Roon as it's been _ages_ since anybody needed this kind of migration
refs https://github.com/nodeca/probe-image-size/blob/master/CHANGELOG.md#600---2020-11-04
- `probe-image-size` v6 now supports `.ico` files so we can
allow probing of dimensions via this library rather than falling back
to downloading the entire image via `image-size`
- also updates a test because .ico files no longer use the internal
request lib, which simplifies things a little bit
- Have ensured we have 100% coverage of core/server/api/shared/serializers/handle.js
- This meant I had to swap around two validation clauses as one was unreachable
- I have done this as I want to make some changes in this area of the codebase, and want to ensure we have tests
and a clear understanding of what this code does before I change it
refs https://github.com/TryGhost/Team/issues/1408
- switched from `@nexes/nql` to `@tryghost/nql` and bumped `@tryghost/bookshelf-plugins` to get access to the latest NQL version across the app
- adds "contains" operator support
- `:~'string'` - contains
- `:-~'string'` - does not contain
- `:~^'string'` - starts with
- `:-~^'string'` - does not start with
- `:~$'string'` - ends with
- `:-~$'string'` - does not end with
- enables `'` escaping in strings, eg `'O\'Nolan'`
Welcome pages are now moved as a property of individual tiers instead of global property on settings. Since we removed the alpha flag for new welcome page behavior, tests had to be updated to test the flow against new behavior.
refs https://github.com/TryGhost/Team/issues/1387
This will allow us to move from the portal_products and portal_plans
settings to using the visibility property on tiers to determine whether
or not a tier should be visible in Portal.
This also fixes a bug with the Tiers Admin API read method permissions.
- we catch error arising from creating webhooks and check for specific codes
- if our error does not match one of those codes, we don't propagate the
error up
- this becomes a problem if saving a webhook fails for some other reason
because upstream code assumes we return an error or model
- this commit re-throws the error and adds a test that would have caught
this
refs https://github.com/TryGhost/Team/issues/1004
Replaces {{products}} helper usage with updated {{tiers}} helper. Default output for {{tiers}} helper is the same as {{products}} helper.
refs https://github.com/TryGhost/Team/issues/1004
- adds new `{{tiers}}` helper behind `multipleProducts` flag
- `{{tiers}}` outputs a string with list of tiers that have access to specific post when used in a post context in theme
- outputs empty string when used out of a post context and without access to `visibility` property
- uses tiers attached to post column for data
refs https://github.com/TryGhost/Team/issues/1004
The `tiers` column for a post/page only contained data if its visibility is set to `tiers`, otherwise its empty. This is because originally the purpose of `tiers` column on `post` was to capture specific tiers with access to post.
The best way to ensure a consistent behavior for `tiers` column data on post is to update it to always contain list of all `tiers` that have access to post, and not just when the visibility is `tiers`. This means the value is set to all tiers when visibility is one of public|members, and only paid tiers when visibility is `paid`. This change also allows on frontend to get all relevant `tiers` information for a post locally within post context instead of relying on additional information from outside.
This change -
- updates the output serializer for post/page to add all desired tiers manually in case of visibility is not `tiers`
- updates tests
- We have an existing pattern for using `visibility: public` instead of `visible: true|false`
- We no-op the existing migration and roll forward so that we don't have to manually revert db changes
refs https://github.com/TryGhost/Team/issues/1367
- The {{access}} property of a post always returned false for non-public content
- Added the member context to the get, next-post and prev-post helpers
- The get, next-post and prev-post helpers didn't add the member context to the internal API calls
- Added the members context to these calls
- Added tests that check if the member context is passed to the API calls
- Transformed next_post helper tests to async await syntax
- Transformed prev_post helper tests to async await syntax
refs https://github.com/TryGhost/Team/issues/1386
- The current match handler supports normal (in)equality operators, but no numeric comparisons (<, >, <=, >=)
- A use case for these new operators is to show the latest post in a separate way from other posts
Includes unit tests to check the new behaviour.
Run via `yarn test test/unit/frontend/helpers/match.test.js`
refs https://github.com/TryGhost/Team/issues/1313
When adding the tiers endpoint the Content API was missed, this is
needed so that themes can access Tiers via the `{{#get}}` helper.
refs https://github.com/TryGhost/Toolbox/issues/169
- Before next major version release we need to prepare for removal of API versioning.
- This change allows unversioned API requests to work under following endpoints:
- /ghost/api/admin
- /ghost/api/content
- This change should allow further preparation of the API clients (SDKs, Integrations, etc.) to non-versioned APIs in Ghost instances in ^5.0.0
- Changed default e2e test targets to non-versioned API. It's a trial, to have working examples. In the future all tests should switch to use only non-versioned endpoints.
refs https://github.com/TryGhost/Toolbox/issues/169
- As Ghost prepares to drop API versioning in future major release it the authentication mechanism should take into account non-versioned token audience support. The audience for non-versioned api requests would be limited to "admin" rather than "canary/admin"
refs https://github.com/TryGhost/Team/issues/1362
- Casper is already installed, so the installation from github always fail.
- There is no need to display an error message in that case.
Added regression tests for blog setup with the default theme:
- Check whether there are no notifications after completing the setup
- Also test the setup with the default theme
refs https://github.com/TryGhost/Team/issues/1306
- This adds a `MemberPageViewEvent` event when a page is viewed by a member (post/page/tag/author/...)
- Integrates the `LastSeenAtUpdater` service that listens to the `MemberPageViewEvent` events to update `member.last_seen_at`
- Follows the latest testing recommendation (end to end test + testing for side-effects)
refs https://github.com/TryGhost/Team/issues/1387
We are moving away from the portal_products setting to instead store
each tiers visiblity on the tier itself. This column will be used for
that data.
Both of the default Tiers should be visible, but newly created tiers
should not be.
refs f48892028d
refs 33da584161
- There was another mather added recently: anyDateWithTimezoneOffset. The naming of date-related matchers was getting long and confusing. Renamed existing date matcher to better reflect what they do and they types of matches they are responsible for, following renames have been done:
- anyShortDate -> anyISODate
- anyDate -> anyISODateTime
- anyDateWithTimezoneOffset -> anyISODateTimeWithTZ
refs https://github.com/TryGhost/Team/issues/1382
- Added two possible new setup values: `accentColor` and `description` to define the brand colour and site description on initial setup
- Updated tests to reflect those changes
- Only the params when passed and fall back to default site description
refs https://github.com/TryGhost/Team/issues/1313
Rather than removing the /products API we're adding a /tiers API as
a first step towards renaming "products" to "tiers". The initial idea was
to alias the URL's but out API framework doesn't easily allow for this so
we've duplicated it instead.
refs https://github.com/TryGhost/Toolbox/issues/215
- The conversion should serve as a reference test. Eventually the aim is to have all tests converted to use "utils/e2e-framework" instead of previously used plethora of utils and whatnot
refs https://github.com/TryGhost/Toolbox/issues/215
- The conversion should serve as a reference test. Eventually the aim is to have all tests converted to use "utils/e2e-framework" instead of previously used plethora of utils and whatnot
refs https://github.com/TryGhost/Toolbox/issues/215
- The conversion should serve as a reference test. It was also a massive LoC drop when converting, felt almost criminal not to do it!
refs https://github.com/TryGhost/Toolbox/issues/215
- The ContentAPI needs it's own test agent, so we can write e2e tests.
- The main method mostly to be used by the test suites is "authenticate" - it add necessary authentication keys to the request. The agent is not authenticated by default because there are suites that need to test the "non authenticated" requests. Also, there's a need to have the default API key inserted from fixtures level before authenticating (it's not strictly necessary because the key is not dynamic, but I think coupling this point would be a bad move)
refs https://github.com/TryGhost/Toolbox/issues/215
- Ghost tests had difficulty running sometimes when the versions for jest-snapshot package did not match in Ghost and @tryghost/express-test
- This is the error that was showing up: `IncorrectUsageError: Unable to run snapshot tests, current test was not configured`
- The reason why snapshot tests were misconfigured was multiple instances of SnapshotManager, which broke the singleton pattern
- Having jest-snapshot embeded within express-test makes sure the versions stay the same across the clients
- The version bump also introduces passing "queryParams" parameter into the Agent constructor - enables configuring query parameters that would appear in each agent's request. Example usecase - Content API authentication parameter "key" would be nice to "remember" and add to every request URL
- I recently added a bunch of strict rules to our eslint plugin around returns: ca9af37866
- These mostly are issues that occur whilst writing code, that you spot and fix as you're developing, but they're annoying to notice/find and eslint can be used to flag them quickly
- There are of course, edge cases where you don't need to return from array fns, but this rule also suggests better patterns might be available
- For our excert helper and new assertEvent helper, I've updated the code to use simpler patterns that are easier to read, so as to avoid the warnings
- For our old API I've simply disabled the rule as we're about to delete this code
refs https://github.com/TryGhost/Toolbox/issues/214
- Disconnected Stripe state is now a special case after the defaul test configuration was changed to have fake Stripe keys included mimicing configured Stripe
- The disconnected state is now achieved through an API call instead of mocking internal services from the test level. This seems like a cleaner approach, although still a bit cumbersome
refs https://github.com/TryGhost/Toolbox/issues/214
- These variables need to be present in the configuration during the Ghost's boot time initialization, which caused a need to remember to mock settings cache before agent initialization.
- By moving the values into default settings config it removes the need to do any work during test environment setup. Yey!
- We should put default values in to test-specific settings-defaults.json In similar situations. Specifically, when we find a need to mock settings cache to be able to start Ghost instance in a certain state
refs https://github.com/TryGhost/Toolbox/issues/214
- Having this config for test environment allows to pre-populate default settings values in the settings table
- Right now the default-settings.json is an exact copy of the original "/data/schema/default-settings/default-settings.json". Having a starter file as an exact copy, allows to track the differences between environments as they are introduced easier
refs https://github.com/TryGhost/Toolbox/issues/214
- The values configuration for the settings table need to become configurable to be able to run our test environment with a pre-defined set of configurations (e.g Stripe-related values).
- This change makes it possible to define the default settings file location (currently a JSON)
- A new key is now exposed through the "paths.defaultSettings" key in settings, which can be overloaded for the needs of the environment
refs https://github.com/TryGhost/Toolbox/issues/214
- The `defaultSettings` path name in the config (one pointing to routes yaml file) creates confusion with the `defaultSettings` which populate defaults for in the database settings table.
- Furthermore, the name collision creates a problem when trying to make database default settings dynamic - being able to load them from configurable file path.
- Rename makes "routing" explicit to avoid ambiguity and free up the name for the database defaults
- The value seems to be safe to be renamed as all keys used in `overrides.json` are taking priority - the name "defaultRouteSettings" hasn't surfaced at any point in the git history
refs https://github.com/TryGhost/Toolbox/issues/214
- TestAgent was used to initialize both Admin & Member API agents, which is somewhat confusing because Member API does not have the same "loginAs" functionality like Admin API does
- Having distinct agents for each API makes the class API cleaner with possibility to extract common functionality even further
- Fixed test fixtures so that members with subscriptions also have products/tiers
- Fixed test fixtures so that default&free tiers can be updated for tests
- Added tests for the signin functionality and welcome page redirects
- Extended `setupStripe` to setup other Members settings - this needs some more
thought around how we proceed
refs https://github.com/TryGhost/Team/issues/1355
- Uses the models to query the database for testing the event presence.
- Prototyped an util to help with event assertion and correct error messages and lower lines of codes.
- Given there are side-effects between tests (adding or removing members), the event count depends on the previous tests, this isn't optimal.
- the previous logic only allowed one flag to be mocked at a time because it kept recalling sinon.stub
- now it's possible to mock multiple flags with different settings as we always just add to the same stub
no issue
- The test was failing because the match for a the data has changed, which was returned in a short format YYYY-MM-DD.
- Added a new matcher for short date format -anyShortDate. Can be used for match dynamic dates in short format
refs https://github.com/TryGhost/Toolbox/issues/214
- Having to fill out `process.env` variables in tests is a frustrating developer experience. Test environment should be configured with smart defaults, so the developer writing test cases would modify variables like this only is special test cases.
https://github.com/TryGhost/Team/issues/1374
When we receive a web hook to update payment details for a customer
which is not associated with a Member, instead of proceeding and erroring
we now return early and respond to Stripe with a 200. A test has been
added to ensure this
refs https://github.com/TryGhost/Toolbox/issues/207
- we want to start writing some E2E tests that involve automated
pointing and clicking around the frontend of Ghost to test that
members of Ghost sites can still do what we expect
- we've decided to look in to Playwright for this - it looks __really__
nice
- this is a VERY basic first test - it'll check for a 200 on the
homepage of whatever we provide as the TEST_URL env variable, or
default to a (manually-run) Ghost instance on port 2368
- also adds a `yarn test:browser` command to run the tests using the
Playwright CLI, and a sample GitHub Action workflow which we can
manually run with a site URL
- there's a lot more to add here in terms of test framework but this
gets us started
refs https://github.com/TryGhost/Toolbox/issues/214
- Calling `getMembersAPIAgent` and `getAdminAPIAgent` separately was booting Ghost twice, which caused a significant performance degradation.
- Additionally, having two calls was slightly ugly and having once utility function that delivers multiple agents at once feels like more readable syntax
- for some reason, this test seems to be failing now we've pulled it out
of the general CI
- it makes sense when the repo is clean, because the html files don't
exist, but I don't understand how they were working before... 🤔
- anyway, we should be overriding the path to the test fixtures admin
view files here
- this fixes the unit tests
- Often in our API we want to check that the location string looks roughly right for a resource
- At the moment we're matching any String, this upgrades the check to look for resource URLs
We've split the tests into two describes, one for when Stripe is enabled
and one without, because we setup Stripe in the before method.
We use nock to mock the Stripe server and assert that there is no call
to delete the subscription.
- I noticed our test fixture theme was still throwing errors during tests because it had partials that are no longer in use
- Updated the docs on how to update Casper to include ensuring Casper was on a valid version AND removing Casper before
copying it across
- Performed the exact steps in the README
- This ensures that the Casper version we use in our tests is not throwing tonnes of errors and is up to date
refs https://github.com/TryGhost/Team/issues/1338
This adds some initial scaffolding to make it easier to test Members with Stripe
- `mockStripe` method to disable the network, so we can use nock to intercept
Stripe API calls
- `setupStripe` method to be called _before_ getting an agent so that Stripe is
configured
- `getMembersAPIAgent` to get an agent for the Members frontend API
refs https://github.com/TryGhost/Toolbox/issues/209
refs https://github.com/TryGhost/Toolbox/issues/210
- Fixed request header processing by the e2e-framework where it failed to lowercase incoming header keys. This bug made it harder to test code paths which involve header checking, e.g. following code: `req.headers['stripe-signature']` would not get a correct value if the header was specified as Stripe-Signature
- Additional output to the status code assertions was added - this allows to have more context when an invalid response code comes back in a test.
no issue
- The members importer api tests don't use the e2e framework, so it's better to move them in their own file as we did for the e2e regression tests.
- Fixed a content-disposition header issue
refs 531ec579a2
- This test broke the build in commit 531ec579a2
- For some reason when running just the test file, it passes. When running the full e2e-api test suite, it fails.
- I'm commenting it to unblock the broken build and move on to other tests
refs: https://github.com/Ghost/Ghost/commit/b5ee17b25
- When moving the site tests into e2e-api they broke because they were getting the changed settings value from the settings test
- The solution is to ensure the settingsCache is reset between tests, which is what this commit does
- This commit also renames the shutdown method to reset, because this is not a permanent operation
- It also renames the resetDb method to resetData, because the concept is we want the internal data to be reset, not just the DB
- We're in the process of rolling out a new e2e-framework, and getting rid of the regression tests
- We'll eventually merge all the tests together and into using the same framework
refs https://github.com/TryGhost/Team/issues/1353
- Doing a renaming-only commit to avoid merge conflicts while I work on the new members.test.js test file
- The new `members.test.js` file will set the `multipleProducts` flag to true
no-issue
Because we were returning the call to `boot`, rather than awaiting it,
it meant that once a test was provided with an API Agent the Ghost
application hadn't necessarily started. This was noticed whilst working
on the Members API which requires the frontend to be loaded.
When the frontend is loaded we must also wait for the url service to
have finished initialising, so that we do not run into 503 maintenance
errors when hitting the frontend.
no-issue
The Members API is served on a different endpoint to the Admin API, and
also requires that the frontend is booted. This agent is to be used when
testing the Members API, e.g. Stripe webhooks or Members config.
- The admin API test agent doesn't boot the Ghost frontend, which is where members routes are currently located
- This means we can't test members properly as we don't have access to webhooks etc
- This change adds a members option to getAdminAPIAgent(), which in turn enables the frontend
- We do it this way so that we can easily change the implementation later, e.g. if we have the option to boot members directly
- When starting ghost for e2e tests we create a content folder in the os tmp dir
- This means that the folder can change between suites as ghost is started and restarted
- For the most part this is fine, but theme storage caches the path to config (which makes sense, it's not meant to change whilst Ghost is in-memory)
- This is a quick-n-dirty fix that just makes it possible to update that path in the tests, so we know it's in sync
- Ideally we'd not cache the path, use a function to fetch it etc, or fully reset the theme storage layer, but this is the fix I have working today
and so it's going in to unblock things for now
- This part of the framework can be used in isolation
- Using mockManager everywhere makes it more visible how to use it
- Aside: fixed .getAdminAPIAgent not needing a URL in site tests whilst cleaning up
- Can now easily mock labs flags as enabled or disabled using mockManager
- Updated some bits of code that directly mocked labs
- Aside: improved error thrown when things go wrong booting Ghost
refs daeb06e835
- This is an additive change, and the parameter is optional so it's better to ship it right away (testing was hard as it's a chicken and an egg problem to have a feature flag in the setup route).
refs: https://github.com/TryGhost/Toolbox/issues/158
- We only use chai in a handful of places now, and it seems totally unnecessary
- Use assert instead
- Made other minor changes with a view to this being a reference aka "perfect" test suite
refs: https://github.com/TryGhost/Toolbox/issues/158
- if we had to combine a large list of params then this would make sense
- we could/should also add a .query() method to our agent
- however, I don't think this is worthwhile/necessary for just 2 params right now :)
