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
https://github.com/TryGhost/Toolbox/issues/140
- Allows to fully start an instance with a custom routes.yaml file without a need to do workarounds - e.g. we used to call an internal Admin API from a regression test suite, which is not a good practice
- Providing "routesFilePath" to startGhost method copies that file to the default settings location and start the instance with that configuration. No need to do API calls or check if the routing service "isFinished"
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
- our themeErrorRenderer is only used in the frontend.. move it there
- this required exposing prepareError as shared middleware
- TODO: move these shared compontents to @tryghost/error
refs: 0799f02e80
refs: 5e931e2e37
- with the referenced two commits I replaced our old HTML renderer with some code borrowed heavily from finalHandler
- I had intended to modify this further to out put our message, context and help error messages
- However, I ended up doing this in prepareError so it's done for all error renderers
- There's now very little point keeping duplicated code from finalHandler just to output the status code
- If we remove this code, express will fall back to finalHandler anyway, so the output is near identical
- got rid of old _private & variable pattern in favour of const and module.exports
- changed weird capitalisation naming conventions to be camelCase
- removed some very old TODOs that we're never gonna get TODONE
- these are mostly old ideas that never made it, and it's been so long they're clearly not important
refs: https://github.com/TryGhost/Toolbox/issues/105
Lint rules prevent:
* Invalid naming conventions for new migrations
* Loop constructs in migrations - these should be used with caution
and are therefore a warning rule, use `// eslint-disable-next-line
no-restricted-syntax` to prevent this rule from firing where a loop is
required
* Returing within a loop - this is usually meant to be a
continue/break
* Multiple joins - these can be badly performing migrations, so should
be treated with caution, disable the rule for the line if the risk is
understood / the migration cannot be written without it
refs: https://github.com/TryGhost/Toolbox/issues/105
The idea here is to ensure we're at least warning on bad migrations patterns. If a pattern is already in use in an existing migration, we can use `eslint-disable-next-line no-restricted-syntax` to override it. For new migrations which still need these features this step will force the user to think about the performance of the construct they are using
refs: 4474ca1a1d
refs: 0799f02e80
The BasicErrorRenderer was created as a fallback for when we needed to not render templates, which is
chiefly when we're trying to render a 404 for an image. Using a template puts us at risk of an infinite 404 loop
if the missing image is referenced in the 404 template.
As of 0799f02e, the HTMLErrorRenderer no longer uses templates - instead we serve a very simple HTML page.
This can be used instead of the BasicErrorRenderer, as it results in a properly formatted error.
Even when sending responses in plain text, the content type is returned as HTML and therefore having an
unformatted error makes no sense - if we really need a non-html format I guess there should be no body at all.
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/Team/issues/1239
- bumps `@tryghost/html-to-mobiledoc` that uses a new parser plugin for transforming `<blockquote class="kg-blockquote-alt">` to an `aside` section in mobiledoc as that's what we use as a workaround for storing alternative blockquote style
refs https://github.com/TryGhost/Toolbox/issues/120
- Having an "options" parameter in the controller definition was missleading as if the `url` or `ref` parameters were expected as a part of the qurey parameter. These variables should be provided as a part of the request body, thus having them in "data" attribute is more accurate
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 https://github.com/TryGhost/Toolbox/issues/139
- Having tight coupling with backup file path calculation for redirects makes it extremely hard to test. In addition, having it injected will make it easier to swap this dependency to the mechanism similar to one used for routes files
refs https://github.com/TryGhost/Toolbox/issues/139
- The custom redirects services belong in the initServicesForFrontend because frontend depends on these to function properly. When placed in general init section the middleware would not get initialized properly before it's used by the "frontend express app"
refs https://github.com/TryGhost/Toolbox/issues/139
- The pattern we use accross the codebase is a single "options" object passed into a constructor instead of passing multiple parametes. Fixed the broken pattern in CustomRedirectsAPI constructor
refs https://github.com/TryGhost/Team/issues/1206
- This add a warning when the `card_asset` config is set so that Ghost doesn't include the callout card css AND the callout card css isn't in the theme
- The update also contains a fix to correctly detect partials named `fill`
- The update also improves the error content when gscan finds an unkown partial
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
- moving this middleware because we're about to add a second piece of middleware
- it's easier to see what we have when each middleware is in its own file rather than in one big middleware.js file
refs https://github.com/TryGhost/Team/issues/1236
We want to ensure that Offers share a name with the correspondent coupon
in Stripe, which have a max length of 40 characters, so we are applying
the same restriction to Offers.
refs https://github.com/TryGhost/Team/issues/1236
We use Offer names for the Stripe Coupon name - which has a limit of 40
characters. We are now introducing a limit of 40 characters to Offer
names too. This migration ensures that all our data in the DB is valid.
refs https://github.com/TryGhost/Team/issues/1235
- we are seeing `oembed-parser` 1.5.2 have intermittent issues when
fetching oembed data
- we're not sure of the reason but reverting the dependency to 1.4.9 seems to fix
the issue
- this commit reverted the bump in Ghost and adds it to Renovate's ignore
list so it isn't automatically bumped in the future
no issue
When switching the oembed service to async/await the error handling was not correctly refactored. `this.errorHandler(url)` was returning a curried function so it could be used as `.catch(this.errorHandler(url))` but that's not how it's being used after the async/await change meaning we were returning a function rather than the result of that function.
- `this.errorHandler(url)` is now only used in one place where `url` is available so removed the method and moved the body of the curried function inline into the `catch` handler
- added a message to the logged error so it's more clear what the log refers to
refs https://github.com/TryGhost/Team/issues/1236
We use Offer names for the Stripe Coupon name - which has a limit of 40
characters. We are now introducing a limit of 40 characters to Offer
names too. This migration ensures that all our data in the DB is valid.
no issue
- this commit adds a counter for the number of boots we do in tests
- which therefore allows us to calculate the average boot time we
experience
- only useful for debugging test performance
refs https://github.com/TryGhost/Ghost/pull/13716
refs https://github.com/actions/setup-node/issues/317#issuecomment-929694556
- the `setup-node` GitHub Action seems to use a shell command to get the
cache path, but these are colorised when `FORCE_COLOR` is enabled
- this causes the Action to fail to read the path correctly
- the comment referenced above suggests to remove `FORCE_COLOR` but it's
nice to have colored output for our tests
- instead, I'm disabling the environment variable on the `setup-node`
action so it still works
- I've tested with the referenced PR and this unblocks dependency caching 🎉
