refs https://github.com/TryGhost/Toolbox/issues/151
refs cbec6aa49e
- Without the await the try/catch block does not catch a pottential validation error straight away, which leads to a 500 error instead of a validation error being returned. The regression was introduced during the refactor (part of referenced commit).
This reverts commit 303ea87897.
- Although gscan catches these now, we have a number of sites that have slipped through the net
- Reverting until we get them all cleaned up
- one big file full of stuff is never good for clarity
- separating it out helps us see what requires what
- it also highlights the awful naming and opaque behaviour we have in themes - much to do, but this helps us start
refs https://github.com/TryGhost/Toolbox/issues/151
refs cbec6aa49e
- The error was happening due to incorrect "this" context. Because the filename and extension are only used once in this class and only for the purposes of the error message have moved the whole thing into the error message itself. No need to keep additional variables around when there's no clear usecase.
refs https://github.com/TryGhost/Team/issues/1067
As part of the work of automatically logging members in after payment,
we want to revisit the emails. Currently after payment we send an email
asking a member to _confirm_ their subscription, and that they can
ignore the email to cancel the subscription. This is not the case
however, as the member has already been subscribed.
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.
no refs
- In the custom theme settings, the `color` default error was saying `null` and empty string values were allowed. They weren't. The description is now fixed
refs https://github.com/TryGhost/Team/issues/1243
When invalid subscriptions without any price data are included in the
API, we are faced with errors due to the data being in an undefined
state. This updates the API to not respond with these invalid
subscriptions.
refs https://github.com/TryGhost/Team/issues/1234
Sharp can occasionally fail resizing, this is usually due the the
underlying libvips library failing. We do not want this to cause an
error however, instead we should just show the original image - as
resizing is an optimisation, rather than a requirement.
refs https://github.com/TryGhost/Team/issues/1237#issuecomment-981770688
- API key names for external services now follow a standard pattern
- top-level key of the service name
- public/private and read/write perms inside the name, eg. `publicReadOnlyApiKey`
- updated test to match expected API key name
refs https://github.com/TryGhost/Team/issues/1001
We fall back to existing behaviour if no API key is present, or if there
is an error communicating with the Twitter API. We're also currently
requesting all the data, which will be thinned down once we understand
what we need.
This also includes a custom renderer for embeds of type "twitter" which
will be used to output the custom HTML for emails
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"
