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
refs https://github.com/TryGhost/Team/issues/948
- The frontend route `/email/:uuid` is aliased to the preview as a temporary solution. It fulfills the premise of the email-only post anyway - not being accessible publicly and only shared through email.
- The tests for the new route are missing as adding them was way more problematic than I envisoned. They are in the works and will be added as a follow up commit next.
refs https://github.com/TryGhost/Team/issues/948
- When working on related problem noticed these declarations were scattered all over the place in the test without special purpose. Moved them all up into a central "once per suite" stub. Not sure if this subing is working at all, but at least it will be easier to change it once per suite in the future once someone spots a problem with this approach
refs https://github.com/TryGhost/Team/issues/948
- While working on referenced issue came around adding tests to new top level route - '/email/' and it was pretty hard to navigate through single blob of tests. Broke them down by the area/route into separate suites for better mainteinability
refs https://github.com/TryGhost/Team/issues/949
- When post is marked as "email-only" we can send it out to the selected audience when publishing without making the post publicly available
- The feature is available for experimentation behind "email only" alpha flag available in labs
refs https://github.com/TryGhost/Team/issues/912
- We need a place to persist the email freeze state between instance restarts - settings table record is the best place for it
refs d60d348c88
- When the import triggers a background job the meta response should contain no data otherwise the client can mistake it for completed import
refs bf587d4055
- The change broke regression tests and due to this change being out of
scope of the cycle dicided to rever it and increase the time limit on
unit tests instead
- We should focus on refactoring moving slow unit tests in the next
cleanup period
no issue
- Two affected unit tests were triggering the "slow unit test" error on CI which gave a clue they were either:
- written badly
OR
- were not unit tests at all
- The latter proved to be true and the tests were moved where they belong - into the regression test suite with much more forgiving running itme constraints.
- Unit tests should run quick ALWAYS as that's the first thing that gets triggered before pushing improvements to main. Faster - better.
- This isn't really a "service" - it's a set of utilities for working with labs flags
- It's also required all over the place, and doesn't require anything that isn't shared
- Therefore, it should live in shared
- This isn't really a "service" - it's a set of utilities for working with labs flags
- It's also required all over the place, and doesn't require anything that isn't shared
- Therefore, it should live in shared
issue https://github.com/TryGhost/Team/issues/859
- Added invalidation to PUT /authentication/setup
- Added invalidation to POST /db
- Added invalidation to DELETE /db
- Added invalidation to GET /slugs/:type/:name
- Removed invalidation from PUT /users/:id/token
refs 8a1fd1f57f
refs 5584430ddc
- The change to async/await in the original commit 558443 was causing problems in downstream dependencies (create-error package) where it was loosing a context of "this". It's not a direct dependency so I didn't go yak shaving into where exacly the context is lost.
- The fix to keep a correct context of "this" was sticking to an existing pattern using regular function returning promises. Once we need to redo them into async/await we can investigate if there's a way around create-error's context prolbem
This commit achieves a few things:
- ☑️ No longer having to remember whether a command is yarn something or grunt something
- ☑️ Simplification of tools hopefully making them easier to remember and use
- ☑️ Complete removal of the need for grunt from our test tooling
Several of the tools still use grunt under the hood, but the **entrypoint** should aways be `yarn xxx`.
- `grunt main` -> `yarn main`
- `grunt dev` -> `yarn dev`
- `grunt build` -> `yarn build`
- `grunt test:file-or-folder` -> `yarn test file-or-folder`
- `grunt test-unit` -> `yarn test:unit`
- `grunt test-acceptance` -> `yarn test:acceptance`
- `grunt test-regression` -> `yarn test:regression`
- `grunt validate` -> removed due to lack of use
There is now also `yarn test:all` to run all 3 classes of tests
This PR also reorders & restructures the Gruntfile extensively so that:
- The remaining useful commands are all at the top of the file
- Config and other blah happens after all the useful commands
- All release-only config happens in the release task at the very end of the file
---
DONE:
* Removed all references to npm/bower
* Removed all references to lint / deprecated command
* Moved debug to yarn dev:debug
* Removed all references to travis
* Removed broken help task + useless comment
* Removed unused knex-migrator and clean:test setup tasks
* Added new test commands, removed grunt validate
* Moved stubClientFiles to test utility and use in the few tests that need it
* Used mocha in yarn directly except grunt test:x
* Swapped grunt test for yarn test
* extensive cleanup and reshuffling
