refs https://github.com/TryGhost/Team/issues/1071
Default content visiblity for specific tiers is now stored split between `default_content_visiblity` and `default_content_visibility_tiers` setting, with former storing the value as `tiers` and the latter stores the list of tiers that the visibility is restricted to. This migration transforms all existing sites that have default visibility stored as an NQL string from previous versions to follow the new model and store correctly on the new setting.
refs https://github.com/TryGhost/Team/issues/1071
Default content visibility for a post can be one of `public|members|paid|tiers`, where `tiers` denotes visibility restricted to specific tiers. This change adds a new setting to store the tier ids when default content visibility is set to `tiers`. This closely matches how the visibility is stored on `posts` table as well, with `visibility` stored as `tiers` and tiers data is stored on tiers pivot table.
refs TryGhost/Team#1071
- new `tiers` key is now attached to posts/pages API response to include tiers visibility
- updates expected response for post/page in tests to include `tiers`
closes: https://github.com/TryGhost/Toolbox/issues/193
- Our Casper fixture was several years out of date.
- I'd already updated the ghost-api to point to the latest API version, which was the main difference
- This makes sure the full fixture is up to date and correct, and therefore that we're truly testing if Ghost right now works
- It also adds instructions for how to update it in future
GOTCHA: the mock-express-style tests are failing if the API difference between test-theme and casper are different
- I've tried to look into why this is - it's something to do with the overridden route settings not working properly if the API version changes
- Given that we may not keep this style of testing AND we are definitely not keeping API versions, I'm ignoreing this for now
- To get around it, I'm setting both themes to be v4 API, so that Casper is exactly as in main
refs: https://github.com/TryGhost/Toolbox/issues/168
- Upgraded the remaining themes to be pinned to the canary API
- This required one minor fix because the edit URL has changed for authors in v4/canary
- I also ripped out everywhere that the theme ghost-api version was being pinned to canary, as this was unreliable and only happening in a few places
- It's also going to be unnecessary code as soon as we finish changing to only having one API version
- This test failed for me intermittently because the posts would be out of order
- I assume this is due to my super-powered M1 mac 😂
- This rewrite only aims to remove the dependency between the insertion order and the output order
- Everything else should be the same, and it still tests that the posts that are meant to be members only are exactly that
refs: https://github.com/TryGhost/Toolbox/issues/168
- The fixture represented Casper at Ghost version 1.0 which pre-dated the introduction of the posts_per_page config in package.json
- When Casper was upgraded to 2.0 in the fixtures, the lack of pagination broke the e2e tests for pagination
- This change introduces proper code to stub and override posts_per_page rather than keeping and using the old casper-1.4 fixture
- It also required me to remove a handful of CSS-based checks which are no longer true in the new theme version, but also didn't really add anything to the tests
no-issue
This test should have always been a unit test, but it now no longer
serves a purpose, as we do not rely on the event being emitted - it
would also not break Ghost if the event _was_ emitted, so we should not
be testing for that.
refs: https://github.com/TryGhost/Toolbox/issues/168
- All of our unversioned tests should be running against canary already
- These tests are erroneously running on the wrong version
We're going to be dropping the idea of having multiple versions of the API in each Ghost version.
Because this has not achieved the goal of making it easier to make breaking changes, but it has
created an ordinate amount of technical debt and maintenance overhead.
As we know this is going away in the next major, there is no benefit to us constantly running tests
that check if those versions still work, especially given how long they take.
Instead we're starting work to ensure that all of our test work on canary, and that canary has
excellent test coverage so that we can be sure that our one API version works really well and that
any changes, no matter how subtle are deliberate, tracked and understood.
refs: https://github.com/TryGhost/Toolbox/issues/168
- These are all places where we reference an API version like v2 or v3 but it's not actually
used or relevant.
- The aim is to get rid of all mentions of these old versions to make it clearer that we're only running tests on canary
refs: https://github.com/TryGhost/Toolbox/issues/168
We're going to be dropping the idea of having multiple versions of the API in each Ghost version.
Because this has not achieved the goal of making it easier to make breaking changes, but it has
created an ordinate amount of technical debt and maintenance overhead.
As we know this is going away in the next major, there is no benefit to us constantly running tests
that check if those versions still work, especially given how long they take.
Instead we're starting work to ensure that all of our test work on canary, and that canary has
excellent test coverage so that we can be sure that our one API version works really well and that
any changes, no matter how subtle are deliberate, tracked and understood.
refs https://github.com/TryGhost/Team/issues/1257
Offer Redemptions were being overcounted due to the way we were updating
Stripe configuration for the Members service. We would create a new
instance of the members-api, which would have event handlers for
creating Offer Redemptions - by creating a new instance each time Stripe
config changed, we would overcount them.
Here we've pulled out Stripe related logic into the Stripe service, and
updated it internally - rather than creating a new instance. This means
that we've been able to remove all of the logic for re-instantiating the
members-api.
- Bumped members-api & stripe-service
- Removed reinstantiation of members-api
- Used stripe service to execute migrations
- Updated Stripe Service to handle webhooks & migrations
- Used webhook controller from stripe service
- Used disconnect method from stripe service
- Removed unused stripe dependency
- Removed Stripe webhook config from members-api
Since we now have 2 products by default for all ghost sites, free and default paid, the usage of default product which so far was using first product needs to be updated to use the first paid product.
- updates default product usage to use first paid tier
- updates tests
refs https://github.com/TryGhost/Team/issues/1189
Support for AMP is slowly in decline, and makes developing new cards trickier,
since AMP no longer has an effect of SEO we're going to disable it by default
as a first step toward moving away from it.
Co-authored-by: Thibaut Patel <thibaut@ghost.org>
closes https://github.com/TryGhost/Zapier/issues/56
- fixes tag creation when creating posts with `tags: [{slug: 'new'}]` which should be supported
- assigning tags with only `{slug: 'new'}` was triggering our validation for the required `name` property then bubbling up to the `bookshelf-relations` library resulting in a 500 error
- the fix applied here is to set the `name` field to the same as the `slug` field if a name is not provided
refs https://github.com/TryGhost/Toolbox/issues/129
- The "Origin" header is require in Admin API requests so it makes sense to bake it into a default request made by most tests. Reduces unnecesary fluf around test request setup and removes a "config" dependency in each tests suite using the "e2e framework"
refs https://github.com/TryGhost/Toolbox/issues/158
- Allows for much smaller amount of code to configure a test to work with chai-jest-snapshots. They now work automatically for all regression tests and could be enabled for other suites by adding the "--require=./test/utils/snapshots.js" parameter in respective test:* package script
- Regenerated snapshot for authentication test as the naming structure
changed a little with the snapshot metadata being taken on a higher
level in the test (uses the suite name instead of a specific describe it
used to be called from)
refs https://github.com/TryGhost/Toolbox/issues/158
- Moving common mocking/stubbing/spying logic into an outside utils module allows test suites to keep agnostic towards which framework powers mocking etc. Should also substitute email service stubbing used in multiple places
- Bonus, got rid of should dependency, which is deprecated
refs https://github.com/TryGhost/Toolbox/issues/129
- This is an example of how the converted test suite syntax could look like when using jest snapshots.
- The tradeoff is not that visible just yet as these tests were mostly checking few fields, but when the whole range of admin API tests is convered we'll be able to get rid of the "checkResponse" utiliti methods along with all the supporting luggage!
refs https://github.com/TryGhost/Toolbox/issues/129
- Having a fresh module should allow building new testing utility concepts from the ground up without being tied by the mystery baggage of the legacy localUtils/testUtils
- The outline of the concepts the framework will be handling is in the commont on the top of the e2e-framework file
refs https://github.com/TryGhost/Toolbox/issues/152
- This refactor fixes the rest of authentication test suite and outlines the state setup steps that are absolutely needed for tests that rely on any db state. It's still counterintuitive why the fixture initialization has to be called and in that specific order, so next will be refactoring in this area to simplify the initi code
refs https://github.com/TryGhost/Toolbox/issues/152
- Passing around plain options object tends to become quite unreadable long term. While these new utils are being shaped up it's still easy to change interface and introduce new parameters with time as needed.
refs https://github.com/TryGhost/Toolbox/issues/152
refs 5bea089dfe
refs 16ad5f73c4
- The refactor is heavily inspired by the referenced commit from @ErisDS
- This refactor is meant to serve as a template for further refactors and unification of the abstraction over "request". Should allow us to be more agnostic towards the library that's powering the request thing. For example, mock-express style of request handling could substitute or get substututed easily if all tests are behind similar "get(Authenticated)Agent" interface
refs https://github.com/TryGhost/Toolbox/issues/152
- This is a continuation of an experiment to switch over to serverless boot in regression tests. Just a proofe of concept that authentication scenarios would also work and the expectations don't collapse unexpectedly.
- Nothing too crazy so far, easy and straight forward substitution of the config "url" with an express app passed to the supertest agent
refs https://github.com/TryGhost/Toolbox/issues/152
- Being able to set up test suites without blocking a port opens a door to new ways to run tests - for example this has been one of the blockers for running mocha tests in parallel
- Additional benefit is lighter statrup, which reducec the test execution time slightly. Doesn't seem like much but these things stack up!
refs https://github.com/TryGhost/Toolbox/issues/152
- Have skimmed through the test suites in hopes to find some quick performance wins to bring the runtime speed closer to the one in "main". Haven't been successful to identify major wins, cleaned up a couple of small bits.
- We'll have to live with a tradeoff between maintainability/unified boot VS cost of mainitaining a fake boot process. Imo extra couple seconds of runtime is worth it.
refs 3c7a8dead4
- The tests needed adjustments with the native boot mechanism.
- The number of returned posts changed in the test resutls because duing native boot we also insert fixtures which add to the number of initial posts
- The vhost regression suite is still failing and I had no strength to figure out why. The redirect it fails with makes no sense, the clue here is that the test doesn't fail when running in isolation, so probably has to do with some leftover overrides from the previous test cases.
refs 3c7a8dead4
- The boot process has been using an asyc method to load the routes file, which is the case now for these tests since the switch to raw boot method instead of mimicking it manually
refs 3c7a8dead4
- Simplifies the state initialization code significantly and reuses native boot mechanism instead of mimicking it (it was a headache to maintain with all the internal services moving around)
refs: TryGhost/Toolbox#147
* Replaces all references to isIgnitionError with isGhostError
* Switches use of GhostError to InternalServerError - as GhostError is no longer public
There are places where InternalServerError is not the valid error, and new errors should be added to the @tryghost/errors package to ensure that we can use semantically correct errors in those cases.
closes https://github.com/TryGhost/Toolbox/issues/148
- These regression tests introduce very little additional value in exchange for an expensive time to run them. Because we mostly care about stable support for the latest stable API version, the older API version test for "internal" API can go away
- In case there are bugs found we do care about in the v2/v3 APIs we can always revert some of these tests.
https://github.com/TryGhost/Toolbox/issues/140
- This test was bloating the regression/site suite and was using hacks (calling the Admin API) to create a custom redirects state
- It suits way better in e2e frontend test suite with less hacky approach to start the Ghsot instance - using custom routes file path to initialize the instance
refs https://github.com/TryGhost/Toolbox/issues/140
- Same/similar RSS tests were present in regression and e2e tests suites. It made sense to move missing cases from regression to e2e. This saves us time bootstraping db state in multiple places and keeps all test cases regarding single feature in same place
refs https://github.com/TryGhost/Toolbox/issues/139
- With ec2aed5ce8 the DynamiRedirectsManager has reached 100% test coverage and most of the tests present in the removed regression suite have been ported to unit tests
- No need to keep slow tests around! :)
refs https://github.com/TryGhost/Toolbox/issues/139
- The regression test suite for redirects functionality for way too big. And each restart was causing massive overhead. It's enough to have a single exhaustive test using multiple input files
- The tests testing API endpoints should've been e2e tests to start with
- The rest is covered in the unit tests for redirects api service
refs 91efa4605c
- Referenced commit introduced a double json-stringification to uploaded redirects.json files.
- The endpoint has no stability index of any sort and is meant to be dropped in Ghost v5. It's best to rework the redirects to the yaml format as descirbe here - https://ghost.org/docs/tutorials/implementing-redirects/#file-structure
refs https://github.com/TryGhost/Toolbox/issues/139
- The v2 and v3 redirects APIs are unofficial and should not be used by anyone in production. There's no good reason to maintain expensive to run test suites for old unofficial APIs.
- The test cases in canary suite covers the functionality of redirects enough to be sure they work as expeted
refs https://github.com/TryGhost/Toolbox/issues/138
- These tests still rely on the frontend to be present. Needs further investigation to remove "frontend: true" flag - it slows down test runs!
refs https://github.com/TryGhost/Toolbox/issues/138
- Final batch of the refactor to async/await syntax. Doing these refactors before modifying "testUtils.startGhost" everywhere to boot only with the backend
refs https://github.com/TryGhost/Toolbox/issues/138
- This is a continuation of a bigger refactor to use async/await syntax before migrating "startGhost" methods to only use backend boot
- Removed a little bit of dead code (like admin user creation) which should speed up test execution too!
- Refactored user variables to be declared closer to their usecases instead of being high up in a global scope - variables shoul not live that far apart from the code that uses them
refs https://github.com/TryGhost/Toolbox/issues/138
- First batch of the refactor to async/await syntax. Next one will cover the rest. Doing these refactors before modifying "testUtils.startGhost" everywhere to boot only with the backend
refs https://github.com/TryGhost/Toolbox/issues/138
- There is no good reason to keep this extra variable around just call "stop" in couple very specific cases. Even for those cases, there's `testUtils.stopGhost` method which achieves the same without additional variable to track.
refs https://github.com/TryGhost/Toolbox/issues/138
- Having the "ghost" alias only added cognitive load when reading through the test code and didn't provide any additional value. Removed the pattern to keep things simpler and more explicit
refs https://github.com/TryGhost/Toolbox/issues/138
- Using asycn/await syntax is way more readable and allows to identify further reusable patterns in test initialization. This refactor also served as an exploreation around how the code looks like at this point
refs https://github.com/TryGhost/Toolbox/issues/135
- Without sensible defaults the web app was not initializing either the backend nor the frontned parts of the application. Fixed the defaults so the problem doesn't happen again and optimized mock-express-style initialization to only initialize the frontend routing
refs: https://github.com/TryGhost/Toolbox/issues/130
The API version stays at v2 unless we stub the getFrontendApiVersion method. But stubbing the method doesn't get picked up unless we actually restart Ghost.
TODO: Maybe change the default here so we don't need to restart Ghost just to test the current version's API
https://github.com/TryGhost/Toolbox/issues/130
The transaction no longer commits in the promise chain, which wasn't
valid logic for a transaction, since it is commited automatically when
the promise chain resolves, and rollsback automatically when the
promise chain rejects.
This makes code which fails during the transaction error in the right
place, instead of getting stuck here. (Especially good for writing
tests).
The tests for this code can now live in the integration folder.
refs https://github.com/TryGhost/Team/issues/1212
This now emits the event when the service is reconfigured, rather than
when we issue the reconfigure command, which causes the event and the
action to be run in the wrong order. This would then cause knock on effects
of having the database in an undefined state - with stripe data in not linked
to the current Stripe account.
refs https://github.com/TryGhost/Team/issues/807
The launch wizard completed flag was previously stored at per user level in accessibility column of user table, so an administrator still got the option to complete the launch wizard even if the owner had completed it previously, which is not expected pattern. This change moves the launch complete flag for Admin to common settings from per user level so a site only needs to complete the launch wizard once irrespective of which user completes it
- adds new `editor_is_launch_complete` setting to track if a site launch steps are completed in Admin
- adds new migration util to easily allow adding new setting
- adds migration to introduce new `editor_is_launch_complete` setting
- adds migration to update launch complete flag for a site if any of the users have already completed the launch steps
refs refs https://linear.app/tryghost/issue/CORE-84/have-a-look-at-the-eggs-redirects-refactor-branch
- The tests needed to have a clean state with empty redirects file, which was previously ensured through "configUtils". Because configUtils don't play ball with the class initialization pattern this approach was chosen
- It's an end-to-end test with lots of logic and pobably would be enough to run against single API endpoint. Leaving it as is and to be improved in the future
refs https://linear.app/tryghost/issue/CORE-84/have-a-look-at-the-eggs-redirects-refactor-branch
- The problem this change is addressing is inability to override config values once the code is extracted into a class+DI pattern
- The work around is restarting the instance with the configuration testing expected behavior - in this case missing or existing types of redirects files
no issue
The way GA flags were introduced means that they stop existing in the `'labs'` setting in the db and are instead forced to always return `true` when checking the flag in the labs service. However, Admin which uses the flags fetches them via the `/settings/` API endpoint which was only returning the raw labs setting db value meaning GA flags appeared to be disabled unless the flag had previously been enabled and no settings save had occured.
- updated the settings bread service to replace the labs setting value with the JSON stringified output of `labs.getAll()` which is the ultimate source-of-truth for a feature being enabled/disabled
- extracted `browse()` behaviour to an internal `_formatBrowse()` method so we can apply the same filtering/modification for output of `browse()` and `edit()`
Co-authored-by: Fabien O'Carroll <fabien@allou.is>
closes https://github.com/TryGhost/Team/issues/1150
Our override of the base Bookshelf `insert` operation so that our own `formatOnWrite()` method is called on attributes was working on a false assumption that an `attrs` attribute is passed in as it is for the `update` operation. Instead Bookshelf's base update uses the `model.attributes` values to create an `attrs` object that is then passed through the usual `.format()` method meaning that our `insert` override was not actually doing anything.
- added a failing regression test for the `formatOnWrite()` override behaviour
- adjusted our insert/update overrides to set an internal `_isWriting` property on the model, then if that property is true our `.format()` override (which is called by Bookshelf on a generated `attrs` object during inserts) we manually call our `.formatOnWrite()` method
- updated both overrides even though `update` was working for consistency and less cognitive overhead for reasoning between two different approaches
- These don't make sense and we're working on improving testing across the board
- We'll make sure our testing best practices are documented when they've settled
- the integrationTesting utils are specific to the express mock style of testing
- all other tests can use the url-service-utils to check the url service is finished
- done a fastest-possible overhaul on this style of tests to try to get them to work independently again
This is a pattern that was introduced a while ago to try to speed up our e2e tests and I'm not sure if it's staying or going
It uses a minimal frontend-only version of the boot process and a custom-built express testing tool
However it's really old and out of date because of the boot refactor and several changes since
This highlights the key problem with it - it doesn't rely on any of our "core" boot process, it makes it up, and therefore how reliable are these tests?
Ideally we need to get these tests working with the real boot process in some capacity
We would then need to make sure we have all the tests in e2e-frontend written in this style
- this test file uses a different pattern to the other test files
- not yet sure if the pattern is terrible or genius, need to assess before moving it into a folder full of what are meant to be exemplary tests
closes https://github.com/TryGhost/Team/issues/1125
refs 3c822e0457
- Email-only is not considered a general availability feature and can be used without special flags.
- It allows to publish a new post type "email only" that only goes out as an email newletter and is available through an undescoverable URL (does not appear anywhere publicly similarly to preview posts) on the site.
- e2e tests are tests that cover critical functionality by booting ghost
- integration tests are more like unit tests, but need to initialise and use a db
- so settings shouldn't start Ghost, url service is critical and should be in integration, and preview is critical and should be in e2e
- some tests are necessarily driven from the db
- these are like unit tests, except they only make sense if using the db - else you have to stub too much to make them worthwhile
- for these rare but important cases, we have the clear concept of integration tests
- We have a bunch of important server-related e2e tests
- Make these clear in their own folder
- "server" is everything that isn't the api or the frontend - kind of a catch-all concept
refs https://github.com/TryGhost/Team/issues/1088
- adds schema for new offers table
- adds permission fixtures for new offers table
- adds migrations for new table and permissions
Co-authored-by: Fabien O'Carroll <fabien@allou.is>
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
refs 7528ec8c3b
- The way the custom redirects middleware was organized made it extremely hard to unit test it (had to stub the redirects service methods etc). With a new organization it's possible to provide needed redirects configs to the method which makes the actual redirects Router logic testable and the code less coupled with redirects services
- This was meant to be an attempt to extract more of the slow redirects regression tests, which failed. Instead found this weak spot that could be improved and gained:
- shaved 4s of time as two slow regression test cases are now gone
- there's now a base to build upon when getting more coverage for the custom redirects middleware
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- Frontend is not meant to know about the underlying source of the "routes" configuration, so any reads/edits/validations are being moved into a backend service. This should also simplify the coupling of the backend with the frontend where the latter will get a JSON blob with all needed configuration during the boot
- Nother problem the "get" method had was hiding an underlying function it was doing - reading the file from the filesystem SYNCRONOUSLY. It might be a thing we need to do during the "web" app initialization, but there's no clear need to do this in a sync fassion during the bootup for example. Also having a more explicit name should help :)
refs https://github.com/TryGhost/Team/issues/1070
- stores values of custom theme settings
- will be merged with full settings data parsed from themes for API output
- will be cached and made available for lookup in themes to avoid db roundtrips
- stores type of custom theme settings so we can coerce values and know if the type has changed when syncing
- records will be synced with themes upon activation
refs https://github.com/TryGhost/Team/issues/1053
This table is going to be completely deleted at some point in the
future. It serves as a persistent datastore for a spike into collection
analytic events for members. We've opted for a generic table, rather
than a table for each event, so that we can push the DB to the limit in
terms of the length of the table, and find out performance issues A$AP
closes https://github.com/TryGhost/Team/issues/860
refs 5405b6ca7c
- The slow test was running slow because it's not a "unit test" it is testing much more. Moved it to a correct suite - regression which simplified the logic a lot (no need to mock db calls).
- Brought back the 2000ms limit as the bottleneck has been solved
refs https://github.com/TryGhost/Team/issues/947
- During the work of the UI and moving `email_only` flag to publish menu it created the situation where the publishing of the post was at the same time as adding `email_only` flag, resulted in not picking up teh `sent` status as the `posts_meta` model and record were's available during save.
- Adding the incoming attribute check for email_only flag covers this situation
refs https://github.com/TryGhost/Team/issues/873
This table is to track events related to members be given or having
removed access to products. It will allow us to provide start dates for
access for complimentary members, as well as being able to track access
to products over time, either for individual members or for aggregates.
refs https://github.com/TryGhost/Team/issues/990
- Relying on uuid instead of slug makes the posts less discoverable and partially soves discoverability through overriden robots.txt files
refs https://github.com/TryGhost/Team/issues/953
- Emails posts should be not explorable by the rest of the frontend similarly to the draft or scheduled posts. Email posts should also keep the content gating, so that specific parts of content can still be gated based on the post's visibility setup
- A separate frontend router was chosen to implement this part of the system instead of a moutable express app due to increased complexity to introduce the latter approach.
- All "sent" email-only posts will be accessible through the `/email/:slug/` route
refs https://github.com/TryGhost/Team/issues/953
- We need to track email-only posts that have been sent out. New status was chosen as a way to differenciate such posts.
- Introducing a new "email post" type, conceptually like "page", was considered. Because there is no clear roadmap for "email post" becoming a bigger part of the product yet and a lot of uncertainty around this concept, overhead needed to introduce a new type was just too much to do at this moment. It's still a possibility in the future
