refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- This is a micro-step towards getting rid of multiple "knownSettings" concept. Since the introduction of an array of knowSettings there was never-ever a need to handle anything but a single `routes.yaml` file. Getting rid of this concept first to have a simpler module. Next step would be getting rid of filesystem reads/writes in the "frontend"
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/Ghost/security/advisories/GHSA-wfrj-qqc2-83cm
refs https://github.com/advisories/GHSA-48ww-j4fc-435p
- a vulnerability in `nodemailer` means that the `sendmail` transport is
vulnerable to command injection for flags passed to the `sendmail`
binary
- updating to the latest version of Nodemailer required creating
`@tryghost/nodemailer`, which is a wrapper around Nodemailer and
several plugins that used to be in the core
- this commit switches to using that package, and fixes up some small
code + test changes
closes: CORE-34
refs: https://github.com/TryGhost/Team/issues/1044
- this is a super basic fix, it adds a max nodes concept and limits the node in each sub-sitemap to 50k by default
- this will prevent the error in google console
- a better fix is in progress, but we want to at least solve the errors ASAP
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: CORE-33
Two bugs:
- lodash isEmpty and handlebars util isEmpty are not the same
- I literally had the truthy and falsy cases the wrong way around 🙈
Notes:
- I have, for now, copied the isEmpty util from handlebars. It's so small it doesn't seem worth trying to require the util right now, although in future it'd be nice if that was easier to do
- Adding the management for the conditional being a SafeString allows the match helper to be a subexpression of itself, I can see this pattern being useful later in combo with the any and all helpers
closes: CORE-32
- The match helper is a very logical helper, designed to support many many type of comparison
- The existing test pattern had gotten long and unweildy already and I'd barely scratched the surface
- Introduce a new pattern, that allows for very quickly and easily describing template strings and what their evaluation should be
- This quickly highlights how many broken cases there are right now!
Notes:
- I could have condensed the pattern further with test groups and iterating over to create describe blocks
- I feel the current implementation is the right balance between condensed and legible to make this maintainable
- I could also have used the same pattern for the block helper, but at that point I felt it more important that the logic I was testing was clear, rather than condensed
refs https://github.com/TryGhost/Team/issues/694
refs https://linear.app/tryghost/issue/CORE-10/tackle-integrationsjs
- The controller code is not meant to contain complex business logic.
- Added a test case checking 'PUT' endpoint for integrations to ensure
proper 'NotFound' handling. Found that previous implemenation was
buggy - threw a 500 as 'models.Integration.NotFoundError' that was removed
in previous commit didn't catch a needed error.
no issue
It was possible for authenticated/trusted admin users to make GET requests to localhost via the oembed service by crafting a redirect that used 0.0.0.0.
- added the 0.* default route/routing block to the private IP regex used to block requests when we're contacting external sites
- added an additional IP or localhost check in the oembed service when fetching bookmark card data
refs d735e3a4f3
refs c7972c20a5
- the unit tests are currently tied to the labs service data rather than testing independent behaviour meaning that the test needs to be updated when the particular alpha flag they are tied to gets removed
- Bookshelf now seems to error out when the response is undefined and
expects something
- this commit updates the test to the same empty response we use
elsewhere
refs https://github.com/TryGhost/Team/issues/1004
- adds new `{{products}}` helper behind `multipleProducts` flag
- `{{products}}` outputs a string with list of products 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 all available posts for a site via the global products data
- updates {{content}} helper cta to use this new helper to show list of tiers with access to post
refs 025eb8bd79
`mutipleProducts` has now been moved out of alpha to beta, though the alpha feature tests were still using it to verify alpha behavior. The tests are now updated to use `dashboardTwo` feature as base alpha feature instead.
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/986
This updates the @tryghost/members-api module to return the full member
object from getMemberIdentityData, which is used to populate req.member
used by themes to construct the `@member` template data.
The full object is read from the service which handles all additional
properties and logic for retrieving members, including the dummy
subscriptions for comped members.
refs https://github.com/TryGhost/Team/issues/946
This adds the initial bulk actions endpoint used for the members
filtering feature. The idea is to eventually move bulk destroy into this
endpoint to and provide a consistent interface for applying bulk actions
to members.
The @tryghost/members-api package has been bumped to include the new
bulkEdit method.
The sinon.restore in tests was moved to an afterEach so that stubs did
not effect other tests.
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/714
In order to order products by their monthly price we need to apply a
join with the stripe_prices table when querying so we have access to the
amount column of stripe_prices.
As this ordering is core to how the tiers feature is intended to work,
we have added it as the default order. But this can be overriden by
manually passing the order option.
Also ensured that we do not create duplicate products in test fixtures
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
closes https://github.com/TryGhost/Team/issues/952
- The `/email/` route will be a home for email only posts. We are adding the route preemptively to have the crowlers update their caches before the feature sees the light of The Internet
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 https://github.com/TryGhost/Team/issues/935
The problem was incorrect operator precedence when multiple statements existed in the filter original filter when we transform it to enforce `subscribed:true` before sending.
- free only - subscribed:true+status:free - no issue
- paid only - subscribed:true+status:-free - no issue
- all - subscribed:true+status:-free,status:free - the ,status:free part is treated as a separate OR statement meaning the subscribed:true is not applied to it and free members that are unsubscribed will receive the email
- extracted the filter transform into a separate function so it can be unit tested
- updated the transform to use `()` for operator precedence, eg: `subscribed:true+(status:-free,status:free)`
- used transform function in `addEmail()` and `getEmailMemberRows()`
- fixed `sent/send` typo in error message
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 2f1123d6ca
refs 6f1a3e1774
- As per refed commits, we are removing deprecated use of `new Error()` in the codebase
- Exposed few internal from commands module methods for easier testing, otherwise it was turning into neverending mocking show
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.
refs https://github.com/TryGhost/Team/issues/542
The members-api has been updated to use the passed `created_at`
property when creating members for the status change events, as well as
using the subscription start_date for members paid subscription events
This is a duplicate of 7eb821b3f5 but
with updated dependencies.
refs https://github.com/TryGhost/Team/issues/860
- Slow unit tests cause longer waiting time to deliver code to main. Before this fix the test was taking a whooping 6s on average
- The main cause of the delay was a downstream's package (got) default retry logic that was taking up a lot of time. The test is meant to check a "500" response handling not a retry logic, so disabling it for this specific test makes sense
refs https://github.com/TryGhost/Team/issues/860
- `nock` has been used in other similar unit tests. It was proven to have easy to understand and use API. By not making up custom code through express/httpServer stuff we reduce the amount of boilerplate code significantly - makes tests a lot more readable
- Bonus, removes couplint dependency to "shared" express module
refs 2f1123d6ca
refs 6f1a3e1774
- As per refed commits, we are removing deprecated use of `new Error()` in the codebase
- This bit cleans up the rest of `new Error()` usage in MEGA service
no issue
- Exposing internal methods out of the module is a non-standard practice. Adding `_` prefix allows to signal that this method is not for general use.
- When mega is refactored into a proper class this method will become exposed anyways
refs 2f1123d6ca
refs 6f1a3e1774
- As per refed commits, we are removing deprecated use of `new Error()` in the codebase
- This bit cleans up `new Error()` usage in MEGA service
refs: f9a3f7d955
- The test for overriding a theme (uploading a theme with the same name as the currently active theme) doesn't test the right codepath
- It incorrectly assumes uploading the same theme twice results in an override, but this is only true for the active theme
- This change splits the override test out into it's own test, and only tests overriding by changing the active theme first
- Also fixed a minor comment type whilst here
closes https://github.com/TryGhost/Team/issues/607
- patch was added in `bookshelf-include-count@0.1.3` to fix member counts for labels
- bumps `bookshelf-plugins` to include the patch
- 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
closes https://github.com/TryGhost/Team/issues/819
- adds guard for an empty buffer when reading file from storage for resizing, if a blank image is loaded then redirect to the original file
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 https://github.com/TryGhost/Team/issues/856
- The default internal version of the API is expected to be the latest one available which is v4/canary at the moment.
- There will be more information posted in the referenced issue later around how to approach the "default version", for now it's just a change to make a small step into a right direction.
refs https://github.com/TryGhost/Team/issues/856
- These tests were created during work on v4 without a particular goal behind them. Because they don't do anything extra comparing to the existing tests there is no sense to keep them around
- In an ideal world, our acceptance tests would be much, much faster
- ATM we output how much time is spent on reloading Ghost for each suite, but this is output with console
- Changed this to use DEBUG, so we don't clutter the UI normally
- Added further debug statements, and a cumulative time, so we can see where time is spent/wasted
- Added a DEBUG command for running acceptance tests with this output
- This shows us that reloading Ghost accounts for 50% of the test time
- this was skipped on boot, but then called in the test utilities, but only on restart 🙈
- this means that yarn test:acceptance (i.e. running all tests) works, but if you try to run just test/api-acceptance/themes_spec.js it would fail because that uses a fresh boot not a restart/reload
- I've changed this as keeping the test using the real boot, rather than the made-up acceptance-utils tasks as much as possible is way better
refs https://github.com/TryGhost/Team/issues/861
- The test case illustrates that it's already possible to achieve what's described in the referenced issue (bug where the content is not filtered correctly when a single email segment card is present)
- The bug fix for a general issue should be using this parser's ability to properly filter html/plaintext content
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
closes https://github.com/TryGhost/Team/issues/817
refs 6d083ee00e/packages/bookshelf-pagination/lib/bookshelf-pagination.js (L256)
- The 500 error is not the best we can do in this situation and throwing a 400 just like we doo in a referenced commit would keep the convention
- The underlying problem of the bug is bigger - we allow the fields named the same way as relations to leak into the db query and that causes an incorrect SQL syntax. It's a bigger problem which would need a separate, holistic approach
refs https://github.com/TryGhost/Team/issues/849
As part of work for segmented post access with multiple products, the custom filter for post access is stored in `visibility` field on posts but passed with `visibility_filter` property on API. This change -
- updates input serializer of posts to transform `visibility` and `visibility_filter` properties correctly
- updates output serializer for canary to transform and send `visibility_filter` attribute with filter value
- updates output serializer for v3 to ignore any custom filter on visibility and return `paid` instead as v3 didn't have a concept of custom filter
refs: fbf0636936
- I renamed this pattern in a bunch of places, but missed a few, leaving the code messy and confusing
- This makes the naming consistent
refs https://github.com/TryGhost/Team/issues/828
- Before sending out batches with members we need to partition all members based on the segment they belong to. Special segment "unsegmented" is used in case none of the segments used in the emal cards cover part of the members set (for example only free members card used when emailing all members)
refs https://github.com/TryGhost/Team/issues/845
refs 517d2abc5c
- updated router response formatting functions and `{{#get}}` helper response handling to make any `feature_image_caption` properties in the response a `SafeString` instance so triple-curlies are not needed when using the property in themes
- This function is quite different to the others, as it generates an excerpt from HTML (truncating)
- Most functions in the meta data folder just contain content negotiation logic, like if post then feature_image else cover_image type things
- This function is more like a library and shouldn't live in Ghost, it should probably be in @tryghost/helpers
- It's definitely something we'd love to rewrite to work better tooooo
- we moved the meta folder out of data a really long time ago
- we also moved the sitemap out of data/xml into a service
- this moves the tests to roughly match
- This is part of the quest to separate the frontend and server & get rid of all the places where there are cross-requires
- At the moment the settings cache is one big shared cache used by the frontend and server liberally
- This change doesn't really solve the fundamental problems, as we still depend on events, and requires from inside frontend
- However it allows us to control the misuse slightly better by getting rid of restricted requires and turning on that eslint ruleset
- requiring lib/common/events makes the settings cache tightly coupled to the server
- moving this up to settings index means the cache itself can be moved to a shared component/moved out of Ghost
- the index then becomes the settings manager
- questionable whether the event listeners & updater part of this shouldn't be part of a manager, independent of the actual cache 🤔
- shutdown removed listeners, which should really be done before adding them anyway!
- reset sets the cache back to an empty object, which was already done by init
- merge these into one reset function that fully resets the cache
- all instances of shutdown were called before an init call, and now called during init, therefore these can be removed
- acceptance utils had an instance of calling shutdown and reset together as part of stopping Ghost, reworked that to be clearer
refs https://github.com/TryGhost/Team/issues/828
- When detecting email segments and later creating a member filter out of this data we only care about unique segments otherwise we'd be creating multiple batches with the same segment filter
refs https://github.com/TryGhost/Team/issues/828
- This is experimental segment extraction logic, more to follow. Alllows to extract arrays of email segments used in the email's HTML content
- Makes the logic for determining the admin and frontend vhost args independent and easier to test
- Moved the tests to specifically test the vhost utils & removed proxyquire as a dependency
- We want to breakdown the current parent app into the existing core/app.js and boot code, allowing us to decouple the backend and frontend further
- This is all part of the refactoring to separate server and frontend completely
refs d9ddc2db6a
refs https://github.com/TryGhost/Team/issues/754
- The tests were written with falsy assumptions and validation added in refed commit have uncovered it!
- A secondary issue touched here is additional JSON object serialization that is used in the "input serializer" -d9ddc2db6a/core/server/api/v2/utils/serializers/input/settings.js (L107-L110)
- The additional stringification should not be there at all. It covers for a mistaken internal use of Settings API where raw objects are passed around instead of serialized JSON Objects (see commets left with this changeset for details)
refs https://github.com/TryGhost/Team/issues/754
refs a7dec233ba
- Additional validation protects from problems like the ones in refed commit from even getting through to the database.
- At the moment only used notificatons and couple more settings to ensure they are arrays when passed into the API. This is to avoid making big change in settings straight away - this is a problematic area which needs cautious approach.
- Ideally in the future the list of settings to check the "array" type (and other types) should be automatically generated based on the default-settings.json (or whatever way we define settings in the db a that moment)
- There's an ugly code-tripplication going on in this change. This is a separate topic that will be addressed once we work on API cleanup.
refs https://github.com/TryGhost/Team/issues/804
The associative table is used to implement the many-to-many relationship
between Products and Benefits. The `sort_order` column is needed because
a product's benefits should be orderable by an admin.
refs https://github.com/TryGhost/Team/issues/804
Benefits are tag-like resources which will be associated with Products.
The first iteration just requires a name for the benefit, which will be
stored as plaintext.
refs https://github.com/TryGhost/Team/issues/754
closes https://github.com/TryGhost/Ghost/issues/13088
refs a7dec233ba
- The corrupted data recovery mechanism for notifications is needed to be able to fix the data stored in `settings` table under `notifications` key. There was no validation in place, which has caused some instances to store data in unreadable/writable state
- The recovery mechanism is in place to avoid adding migrations every time we spot a broken notifications data (will be fixed by validation soon).
- The notification data is also NOT critical but valuable for system functioning properly, that's the reason why the data "healing" happens in less secure "fire-and-forget" way
- The referenced commit is where the "bigger" problem that was causing the data corruption was at. This change is a "cleanup" after what has happened there - storing Ghost error object in `value` for `notifications` key
issue https://github.com/TryGhost/Team/issues/750
- Only accessible by admins
- Resets all staff users' passwords and prevents them to log-in
- Sends them a reset email password to give them back access to their account
- Closes all existing staff user sessions
refs 474e6c4c45
- The method was not easy to understand after skimming through it.
- As we are working on developing a similar pattern for upcoming similar featured created a basic test suited to see input/output relation clearly
refs https://github.com/TryGhost/Team/issues/793
New settings added for newsletter customisation options:
- `newsletter_header_image` - `null/"$url"`
- `newsletter_show_header_icon` - `"true/false"`
- `newsletter_show_header_title` - `"true/false"`
- `newsletter_title_alignment` - `"center/left"`
- `newsletter_title_font_category` - `"serif/sans_serif"`
- `newsletter_show_feature_image` - `"true/false"`
`newsletter_show_header` has been dropped because the same functionality can be achieved by setting both `newsletter_show_header_icon` and `newsletter_show_header_title` to `false`
---
- migration to convert and delete `newsletter_show_header` setting
- removed `newsletter_show_header` from default settings to ensure it doesn't get re-created
- replaced main labs template and template settings generation with the labs template
- deleted labs template
- stubUrlUtils is now doing the same thing as our existing configUtils but is slightly harder to use
- swapped this out, and made stubUrlUtils an internal-only utility
refs https://github.com/TryGhost/Team/issues/768
- `portal_products` stores list of products available in Portal
- adds new `portal_products` setting to default settings
- adds migration to populate `portal_products` with current product so its available by default
- update tests
- We were using the same bind pattern for both internal-only and public helpers
- Binding helpers to config makes them available throughout the codebase
- Removing the binding doesn't make the code much more complicated, but it does make the Public API of the config module a lot clearer
- Renamed the file in line with our rules around index.js files
- Cleaned up some outdated code patterns
- Want to make the config module a little clearer in what it does
- Part of the effort to split Ghost down into smaller, decoupled pieces
- Moved out our internal validator tooling to a separate library
- Replaced all usage of our own tooling and validatorjs directly with @tryghost/validator
- Removed the validatorjs dependency and removed the renovate pin
- This gives us a consistant, smaller, clearer public API for validations
- It will eventually be used on Ghost Admin too
- This way we can start getting up to date with validator whilst not increasing build size
no issue
- we're going to pull this out into the framework monorepo but
refactoring it here first makes it a lot easier to extract without
losing the history
no issue
The only pieces of Ghost-Ignition used in Ghost were debug and
logging. Both of these modules have been superceded by the Framework
monorepo, and all usages of Ignition have now been removed, replaced
with @tryghost/debug and @tryghost/logging.
no issue
- we're going to pull this out into the `framework` monorepo but
refactoring it here first makes it a lot easier to extract without
losing the history
- note: this is very temporary and will be extracted soon
- renamed our internal validation library to "validator" - which is the same as the tool it wraps
- updated the public api so that validator methods are directly exposed
- this will make it a drop-in replacement for validator-js
- in turn, this allows us to pull this out into @tryghost/validator, and use our own wrapper instead of the 3rd party library
- General code cleanup
- Removed unused notContains rule
- Swapped custom empty rule for builtin isEmpty rule
- Dropped usage of .extend on validator, as this was removed 2 years ago!
- This will allow us to upgrade the validator dependency to a much newer version
- Changed our internal validator module to only expose the functions we use.
- This gives us a clearer Public API
- It makes it easier to see if we are affected by changes in validator
- It's still easy to add another validator, we just have to update what we require
- We can potentially use this to make smaller builds esp for client-side usage
- Once ripped out into a module we can use ES imports :D
- Rejigged and _slightly_ improved the tests
refs:
- cf15f60085
- dd20cc649b
- ccf27f7009
- abf146d61f
- 2b54c92a14
- bb029a53f6
- 95bd7ee675
- 9018b4df22
- df01a6e5f4
- d313726b34
- these plugins were in a state where they were independent enough to be
pulled out into their own packages, which is what we did in the
referenced commits above
- each package is named like `@tryghost/bookshelf-<plugin>`
- to avoid requiring multiple packages into Ghost, we've also created a
wrapper package called `@tryghost/bookshelf-plugins` which re-exports
all these plugins, so the changes in Ghost are very simple - dbebdd43b5
- this commit deletes the plugins + tests, and replaces with our new
package with some minor code changes
- This is super specific code relating only to validating passwords.
- It's needed as a shared validator as we use other funnels to help people setup Ghost on Pro, but currently it's hard-baked into Ghost
- It's also not the greatest code. It'd be nice to be able to rework it and know that would automatically update everywhere passwords are set
- This is a really specific piece of code related to validating models against our internal schema.js format
- This doesn't make sense without a schema.js file
- It does depend on the internal validator and validate tools - but those are used elsewhere too, and can reasonably be moved out of the codebase
- I don't see schema.js moving out of the codebase any time soon. We can move the validator but it would be a class that requires schema via DI
- For now my focus is on getting the data/validation tooling separated and making clear sense
- Improving data/schema can come later :)
- The data/validation module is made up of several loosely related things with lots of dependencies
- Separating out the various components makes it possible to see what's what, and importantly what has complex dependencies
- validator + validate probably go togetheri in an external module, the other two files should probably have their own homes in related areas of ghost e.g. schema -> data/schema/validate.js
refs https://github.com/TryGhost/Team/issues/770
We want post feature image functionality to better match what's available inside the editor, to do that we'll need somewhere to store alt and caption meta data. `posts_meta` chosen because even though we want to make this generic for other tables in the future those tables also have a `feature_image` (or closely related) field.
- updated schema with new columns
- added migration to create columns
- cleaned new columns from API output
- not output on v2/v3
- conditionally output on v4/canary output based on labs flag
- bumped `@tryghost/admin-api-schema` to allow new columns through in canary API requests
- silently clean properties from input when labs flag is disabled
- updated acceptance tests so they fail if `admin-api-schema` is not letting the new fields through
no issue
Shows impact of new code behind labs flags through the existing acceptance/regression tests. Allows for existing tests to be updated to match new behaviour rather than requiring separate tests where individual flags are enabled. Should result in minimal test updating once code reaches GA.
- adds a forced `'labs:enabled'` fixture op that edits the `labs` setting to enable all flags then restarts the settings service to pick up the new setting
- modifies labs service to not remove ALPHA_FEATURE labs settings when running in a testing environment
- Traditionally all of Ghost's public-facing text was written in British English
- We're changing that to US English because that's more common
- US English should also be used in code e.g. properties are called color not colour
- most of these changes are in comments, but I've changed them so that we have US English in front of us always
- fixed a few other typos I noticed whilst there
refs https://github.com/TryGhost/Team/issues/772
- When the feature is introduced into Ghost at it's first lifecycle stage - "alpha" the rule is to have a "enableDeveloperExperiments" flag along with labs toggle turned on before it's usagble in the codebase
- The changeset introduced a "ALPHA_KEYS" concept which should allow distinguishing alpha flags from beta flags.
closes https://github.com/TryGhost/Team/issues/761
With multiple products, each product can have an active monthly/yearly price, so we no longer store the monthly/yearly price ids in global settings but instead store them in product table directly. This means we need to update our global `@price` helper to also use the updated schema and use the monthly/yearly prices from product table instead of settings data.
refs: https://github.com/TryGhost/Team/issues/759
- wired up a matchHelper feature flag & used the labsEnabledHelper tool to gate the helper
- added a first version of the match helper, which is intended to replace the has helper
- this is an experimental helper and may or may not make it to GA
- match is a simple comparison helper, right now it does a very basic equals or not equals comparison
- much more functionality is needed to reach parity with has
refs https://github.com/TryGhost/Team/issues/757
- The "type" value in settings is meant to be representing the data type stored in the "value" field. It was an overlooked bug in v4 API adding a mapper to group->type
refs https://github.com/TryGhost/Team/issues/757
- Groups are meant to represent the group to which the settings key belongs and have the same value as stored in the DB. This was an evolution of v2/v3 API's "type"
- Having a fixed value for groups in a test will make it easier to track the "type" change that is about to land
refs https://github.com/TryGhost/Team/issues/757
- There is no usecase for editing "labs" settings outside of canary/v4 API versions. Removing support for older versions makes the supported API surface smaller (easy maintenance).
refs https://github.com/TryGhost/Team/issues/757
- To safeguard from mise of a very permissing "object" value of the "labs" setting this change introduces an "allowlist" approach to filtering unrecognized labs flags
- Should allow maintainers to have a clear view of which labs flags are currently in use and manage them accordingly
refs https://github.com/TryGhost/Team/issues/757
refs https://github.com/TryGhost/Team/issues/332
refs ea6d656457
- We have a need a quick way to add features behind flags. The old way of "labs" is the quickest way to achieve this. It has ready tooling around it and well understood pitfalls. This change reintroduces "labs" group & key in settings table in the same shape it used to be (see reffed commit)
- Next step will be introducing very basic guard rails to protect from pitfalls previous implementation of "labs" had. This will include an allowlist based input validation for lab's object's data
- The labs being an "object" type is an EXCEPTION. Even though it's an antipattern we aim to move away from, for now it's the lowest impact solution that will unblock the use of flags in the system. A proper solution will come at some point.
- we don't need to use _.escape from lodash as we already have escapeExpression from handlebars
- it's more correct to use the escape utility from our theme engine when escaping strings _for_ our theme engine!
- Note there is a minor difference between the two:
- Lodash: &, <, >, " and '
- refs: https://lodash.com/docs/4.17.15#escape
- Handlebars: &, <, >, ", ', ` and =
- refs: https://handlebarsjs.com/api-reference/utilities.html#helper-utilities
- This could cause slightly weird behaviour in themes around ` and = characters, but as it's just convering to html entities it should be fine
* 🐛 Fixed saving Members with Complimentary plans
refs https://github.com/TryGhost/Team/issues/758
Since 4.6 The Admin is using the comped flag again, rather than creating
subscriptions for zero-amount prices directly. With the `comped` flag
removed, the default state was for it to be falsy in the Admin, and when
saved would trigger the legacy comped flow, cancelling the subscription.
This reverts commit 57a176ff3d.
refs https://github.com/TryGhost/Team/issues/728
- The code of update check has been extracted into it's own package as a part of TryGhost/Core monorepo. This commit is a cleanup of the leftover files
refs https://github.com/TryGhost/Team/issues/728
- This is continuation of the previous commit. TLDR: Passing only the necessary parameter data makes it easier to reason about what dependencies the UpdateCheckService has to deal with
- Instead of passing in a whole GhostMailer instance passing only an email sending function, which again - makes things way more manageable to reason about
- The end of refactor, next will be a move of the UpdateCheckService into a separate module in tryghost/core
refs https://github.com/TryGhost/Team/issues/728
- This is continuation of the previous commit. TLDR: Passing only the necessary parameter data makes it easier to reason about what dependencies the UpdateCheckService has to deal with
- Burned ghostVersion module passing in vafor of just one additional config parameter. Now the module along with unit tests can be easily extracted out of the codebase!
refs https://github.com/TryGhost/Team/issues/728
- This is continuation of the previous commit. TLDR: Passing only the necessary API endpoint function makes it easier to reason about what dependencies the UpdateCheckService has to deal with
- Substituted a parameter with already existing 'siteUrl' config value. No need to duplicate work!
refs https://github.com/TryGhost/Team/issues/728
- This is continuation of the previous commit. TLDR: Passing only the necessary API endpoint function makes it easier to reason about what dependencies the UpdateCheckService has to deal with
- Limited urlUtils to only one function as that's all the UpdateCheck uses. Next step will be removing the function completely as and passing a 'blogURL' as a config value (way better readability this way)
refs https://github.com/TryGhost/Team/issues/728
- This is continuation of the previous commit. TLDR: Passing only the necessary API endpoint function makes it easier to reason about what dependencies the UpdateCheckService has to deal with
- There are 8 different configs that NotificationService depends upon it will need some further investigation around which ones are even needed anymore and the naming is not the best. To keep the time cap at bay leaving it at what it is.
refs https://github.com/TryGhost/Team/issues/728
- Previous name was after the do-all-the-things mega module that have now become an "initializer" for the UpdateCheckService class. The unit tests are testing the latter, so the rename is a cleanup from the previous ways
refs https://github.com/TryGhost/Team/issues/728
- This is the end of the refactor for update check tests. There are no dependencies for Ghost server process to run this test suite!
- Next steps will be extracting the service to an outside module
refs https://github.com/TryGhost/Team/issues/728
- Rewire is usually a sign of massive module that is impossible to unit test without mocking big parts of that module. With a refactored UpdateCheckService and a brand new test coverage there is no need to use rewire and leave the "smell"
refs https://github.com/TryGhost/Team/issues/728
- This is a continuation of the test coverage for the UpdateCheckService.
- Covers scpecial cases of notification processing within Update Check
- The refactor inside the update check service was a convenience to get rid or the Bluebird dependency completely. Also, some minor preventative code added to avoid errors from referencing undefined objects
refs https://github.com/TryGhost/Team/issues/728
- This is a continuation of the test coverage for the UpdateCheckService.
- Covers a basic check for what kind of data is sent from the service to external analytics
refs https://github.com/TryGhost/Team/issues/728
- This approach is moving away from stubbing and "rewiring" massive internal modules and instead only stubs needed dependencies for the UpdateCheckService class
- Also eliminates the need to depend on the database fixtures which should speed up test execution and move it to proper "unit test" class when the test rewrite is complete
- starting and stopping Ghost is part of our rather unloved acceptance test framework
- moving them into their own file to make the different pieces clearer and also to start to make improvements
- first improvement had to happen as an aside - exposing the existingData property via a function and making the API clearer
- this was a weird thing set on module.exports, very hidden and hard to follow
- Note: stopGhost is only used once in the regression/modles/model_posts_spec.js file to make the test run fast enough...
- These tests are effectively "DB integration" tests e.g. non-unit tests because they do use the DB, they need their own framework
refs https://github.com/TryGhost/Team/issues/710
refs https://github.com/TryGhost/Team/issues/725
Products will now have a single monthly and yearly price which will be
used throughout Themes, Portal & Admin. These columns will be used to
track the current prices for each of them, and will update anytime we
change the pricing of a product.
Due to a circular table dependency we have not added a foreign key
constraint to the new columns, this will be handled at a later date. It
is tracked in issue 725 references above
- use testUtils.startGhost() directly
- use async+await for tests
- simplify tests down using expect(fn) to make them more readable
- don't require config directly
- removed TODO which is no longer valid
- All this is essentially ground work before introducing a better framework for running acceptance tests
- These tests protect some advanced, yet important behaviour around URLs
- Moved them out of regression tests into acceptance, as we're slowly trying to clean up regression tests :)
- Checking for differences in behaviour when urls are configured with or without slashes should fall to unit testing
- Generally testing that a correctly configured subdirectory site does what's expected is an acceptance test
- Ghost's 301 to trailing slash behaviour is also well tested, and doesn't need extensive acceptance tests when using a subdir
- Instead, test that the non-subdirectory equivalent route is 404ing as expected, to check that routes are ONLY mounted on the subdir, as well as mounted at all
refs a1556797b6
- The test was using an outdated syntax which broke after migration to async/await in the tested funciton
- Updated test is much more readable and should handle promise rejections (async function throws) universally
refs https://github.com/TryGhost/Team/issues/726
- When UpdateCheck service sends a notification with "type: 'alert'" an email goes out to admin users with the "message" content of the notification.
- This functionality is aimed to handling critical messages like urgent instance updates
- Next step will be getting as much of the update check code extracted into a "service" and then moved out of Ghost's codebase
closes https://github.com/TryGhost/Ghost/issues/12986
refs 1345268089
As part of changes in 4.6, the default price ids for monthly/yearly prices are stored in new settings - `members_monthly_price_id`, `members_yearly_price_id` - which are used to determine current active prices for the site from list of all existing prices. While the last commit updated the prices to use the settings, the data for currency was still used from non-zero prices instead of the new settings value.
- Updated tests to check price currency
closes https://github.com/TryGhost/Ghost/issues/12980
closes https://github.com/TryGhost/Team/issues/730
As part of changes in 4.6, the default price ids for monthly/yearly prices are stored in new settings - `members_monthly_price_id`, `members_yearly_price_id` - which are used to determine current active prices for the site from list of all existing prices. The `@price` helper was incorrectly still relying on the old logic for active monthly/yearly price using the first active price with matching nickname, and resulted in showing incorrect price data on the theme.
- Updated tests to check price data using settings value
refs: 895bffec1f
- Allowing tests to be skipped masks issues where people temporarily skip tests to fix later and accidentally commit
- Converting this to an error allows us to see when this happens easily
- In future, when we're done with the dance of using ESLint rules to signpost codebase improvements, we may want to downgrade this back to a warning
- This would enable us to have deliberate, temporarily skipped tests in the codebase, if there's a usecase for that with pushing to main regularly
refs: f1be3418d9
- Since I refactored the boot process & subsequently the test tools, I have been hitting an issue where subdirectory tests don't work if the app has already been started without a subdirectory
- Turns out it's due to the rootApp getting cached, and not replaced no matter what you do, even though if you debug it _looks_ like it's been replaced
- This makes sense as the rootApp is in a separate file and therefore subject to the node module cache and we're using const everywhere
- Therefore, I have added a single line to the test utils to destroy this cache for this file, and the tests now work perfectly
- Next: refactor the test utilities some more to make all this much, much more straightforward and easy to understand
refs https://github.com/TryGhost/Team/issues/694
- This refactor is not ideal but moves us closer to the desired form of class with injectable (and testable) parameters. Allowed to refactor the test slightly so at least we can check if schedulerd subscribed events work and if they trigger the adapter with correct data
- Ideally the api/model calls shoudl be abstracted away as well, but that's for another time
- Also got rid of completely pointless "adapters/scheduling" unit test. All it was checking was if the "init" method was called int the passe in object
no-issue
These tests were previously skipped due to the NONE algorithm tests
failing. These were failing for good reason, we do NOT want the none
algorithm to be accepted. The tests have been updated to reflect that,
and unskipped.
no-issue
These tests were skipped out due to adding with the subscriptions
relation not working. However this functionality is redundant, as it is
not used anywhere. The tests have been updated to test that we can read
the subscriptions relation correctly.
no-issue
We no longer want to include skipped tests in the codebase. These tests
were added as placeholders until a point at which we were able to mock
Stripe.
We haven't got time during cleanup to mock Stripe so removing these
tests for now.
refs https://github.com/TryGhost/Team/issues/694
- Unskipping the test does not seem to be causing a failure any longer. Removed the skip as it would be causing linting errors as we tighten up the `.skip` rules
refs https://github.com/TryGhost/Team/issues/694
- The previous `.then` chaining was outdated, while in this part of code did a tiny cleanup which should improve future maintenance slightly
refs https://github.com/TryGhost/Team/issues/694
- The previous `.then` chaining was outdated and was causing 2x ghost instance initialization per test suite
- With a refactor there's only one intance initialization per suite (saves running time!) and we use more readable async/await syntax, which should make things more maintainable
refs https://github.com/TryGhost/Team/issues/694
- The tests were skipped and were throwing an eslint warning, which soon will become an error.
- They were skipped initially as they used an outdated authorization mechanism. With migration to token based auth the tests are green again
no issue
- The test was skipped, which should not have been commited into the trunk in the first place. The checks were reworked to reflect the reality - the endpoint ignores the "group" qurey parameter
refs https://github.com/TryGhost/Team/issues/610
- When either schema or default settings changes it's often forgotten to check if exporter handles updated tables/keys properly
- These tests are meant to serve as a reminder to check exporter and modify what's needed when changes are introduced into schema/default settings
no-issue
This protects our tests against changes to the database schema, which
helps us decouple the API from the database, and make tests less
brittle. It also forces us to manually update the tests if we do make a
change to the API!
refs https://github.com/TryGhost/Team/issues/693
Since we've got rid of the concept of Complimentary with the Custom
Prices work, we're removing the 'comped' status from members. This
involves a migration for existing members, a schema update for the
validation, and a bump to members-api to no longer use the 'comped'
status for new members.
We also update the aggregation of the MemberStatusEvent to consider the
'comped' status as 'paid', and that there are 0 'comped' status events
in the database.
We can consider a migration for this data in the future, either adding
new status events moving from 'comped' to 'paid', or by modifying
existing status events. However both of these are very difficulty to
write a down migration for, and might be best saved for a major version.
- @tryghost/members-api@1.7.0 is the version that includes the required
changes, however we have already bumped to 1.8.0 in Ghost
closes https://github.com/TryGhost/Team/issues/699
With custom products, saving a member with subscriptions on member detail page in Admin throws errors on console, though the save is successful. This breaks the Admin as user needs to refresh the screen again to get rid of error. This change -
- updates the response on member save to return `price` object in subscription
- updates tests
no refs
Since backend now allows multiple prices but we want the prices to be currently limited to monthly/yearly on UI, we need new settings to store the current monthly/yearly price by the site owner. These settings determine the active prices shown in Admin / Portal for the site till we allow all custom products/prices again.
refs 06dd9bac59
- This is a continuation of the work started in the refed commit. In short: allowlisting response checks wherever possible
- The file is now "schema" depencency free. Should serve as an example for other test "utils" modules.
refs https://github.com/TryGhost/Team/issues/560
refs 196cdafe6b
Last commit updated the response status for member endpoint - `/members/api/member` - from 401 to 204. This change updates the test to handle new response code.
refs https://github.com/TryGhost/Team/issues/687
- The frontmatter field has leaked into the API layer unintentionally when it was introduced into the DB schema during 4.0 release.
- The fix add the field to "trim" list in all API. A proper validation and handling will be add per API as usecase for the field becomes clear
refs https://github.com/TryGhost/Team/issues/687
- The approach of generating validation properties using `/server/data/schema` package's tables object is prone to leaking unwanted database fields into API responses
- This refactor takes a tiny step into direction of relying on "allowlist" approach for properties in the API response resources.
- Apart from solving the described property leak problem it also moves toward decoupling tests from `/core/server` dependencies!
refs https://github.com/TryGhost/Team/issues/581
closes https://github.com/TryGhost/Team/issues/582
Emails can now be sent to members with specific associated labels or products by specifying an NQL string. We want to bring the same members segment feature to content by allowing `visibility` to be an NQL filter string on top of the `public/members/paid` special-case strings.
As an example it's possible to set `posts.visibility` to `label:vip` to make a post available only to those members with the `vip` label.
- removed enum validations for `visibility` so it now accepts any string or `null`
- bumped `@tryghost/admin-api-schema` for API-level validation changes
- added nql validation to API input validators by running the visibility query against the members model
- added transform of NQL to special-case visibility values when saving post model
- ensures there's a single way of representing "members" and "paid" where NQL gives multiple ways of representing the same segment
- useful for keeping theme-level checks such as `{{#has visibility="paid"}}` working as expected
- updated content-gating to parse nql from post's visibility and use it to query the currently logged in member to see if there's a match
- bumped @tryghost/members-api to include label and product data when loading member
refs https://github.com/TryGhost/Team/issues/671
When turning on custom products, existing sites should have default price descriptions that match existing values for prices. This change sets the default description for Free price to match existing hardcoded value.
- Not sure why, but this is set to off in our base plugin
- It masks issues where people have temporarily skipped tests to fix later and then forgot
- Enabling this to allow us to review all those places, but we should also try to leave it on and set to error
no-issue
Our base model will only automatically convert numbers to booleans if
the type is 'bool' - however this column was incorrectly added with a
type of 'boolean'. Lucklily - knex with both MySQL & SQLite3 will add
a column with the same type for both of these, so no migration is needed
to fix it.
refs https://github.com/TryGhost/Team/issues/668
Since we no longer store price data in the settings we must use the api
to read the stripe prices for the default price, so that we can maintain
backwards compatibility for the `@price` data in themes.
refs https://github.com/TryGhost/Team/issues/637
The "free" price - when Members signup without using Stripe, should have
a name and description, so that it can be displayed in Portal in a
similar way to paid price's. As there is only ever one, and it is not a
fully fledged price, a setting makes more sense than a dedicated db
table.
refs https://github.com/TryGhost/Team/issues/586
We are no longer using the `stripe_plans` setting, instead we are using
the `stripe_prices` database table. However, we must keep the setting as
the migration from the setting to the database is not done as a standard
migration, but in code. This means our code has to still read and pass
the setting because we will never know if the migration in code has run
yet.
The `portal_plans` setting has been updated to only include 'free' by
default, because the setting must include id's now rather than names.
refs https://github.com/TryGhost/Team/issues/586
A discussion in the Members team resulted in us determining that we do
not need to enforce unique names for Products. Stripe does not enforce
uniqueness for their Products, and we feel it's not necessary for us to.
