Commit Graph

114 Commits

Author SHA1 Message Date
Simon Backx
b6519e0f1f
Removed usage of unquoted ids in filter strings (#19070)
fixes GRO-34
fixes GRO-33

This is a revision of a previous commit, that broke the browser tests
because changes in the data generator (requiring bookshelf had side
effects).

This adds a new way to run all tests with enforced numeric ObjectIDs.
These numeric ids cause issues if they are used withing NQL filters. So
they surface tiny bugs in our codebase.

You can run tests using this option via:
NUMERIC_IDS=1 yarn test:e2e

Removed some defensive logic that could be explained by this discovered
issue.
2023-11-21 09:45:36 +01:00
Simon Backx
d5492bd63c
Revert "Removed usage of unquoted ids in filter strings" (#19052)
Reverts TryGhost/Ghost#19031

Browser tests are failing with an unknown cause
2023-11-20 13:50:07 +00:00
Simon Backx
320eaac4c4
Removed usage of unquoted ids in filter strings (#19031)
fixes GRO-34
fixes GRO-33

This also adds a new way to run all tests with enforced numeric ObjectIDs.
These numeric ids cause issues if they are used withing NQL filters. So they
surface tiny bugs in our codebase.

You can run tests using this option via:
NUMERIC_IDS=1 yarn test:e2e

Also removed some defensive logic that could be explained by unquoted ids.
2023-11-20 14:00:31 +01:00
Jono M
5e057dee11
Added tests to AdminX framework package (#19022)
refs https://github.com/TryGhost/Product/issues/4159

---

<!-- Leave the line below if you'd like GitHub Copilot to generate a
summary from your commit -->
<!--
copilot:summary
-->
### <samp>🤖[[deprecated]](https://githubnext.com/copilot-for-prs-sunset)
Generated by Copilot at 9e68f4d</samp>

This pull request refactors several components in the `admin-x-settings`
app to use common hooks from the `@tryghost/admin-x-framework` package,
which reduces code duplication and improves consistency. It also updates
the `package.json` file and adds unit tests for the `admin-x-framework`
package, which improves the formatting, testing, and dependency
management. Additionally, it makes some minor changes to the `hooks.ts`,
`FrameworkProvider.tsx`, and `.eslintrc.cjs` files in the
`admin-x-framework` package, which enhance the public API and the
linting configuration.
2023-11-20 11:00:51 +00:00
Simon Backx
e5f644c27f
🐛 Fixed contain/starts/endsWith filters with /, _ or % in them (#19015)
fixes GRO-25

Updated @tryghost/nql to 0.12.0 and other packages that depend on it

1. SQLite: when a filter string contains /.

When we use a NQL contain/starts/endsWith filter that contains a slash,
underlyingly the whole filter will get converted to a MongoDB query, in
which we just use a regexp to represent the filter. In here we will
escape the slash: \/ as expected in a regexp. Later when we convert this
MongoDB query back to knex/SQL, we use a SQL LIKE query. Currently we
don't remove the escaping here for a normal slash. MySQL seems to ignore
this (kinda incorrect). SQLite doesn't like it, and this breaks queries
on SQLite that use slashes. The solution here is simple: remove the
backslash escaping when converting the regexp to LIKE, just like we do
with other special regexp characters.

2. We don't escape % and _, which have a special meaning in LIKE queries

Usage of % and _ is now as expected and doesn't have the special SQL
meaning anymore.
2023-11-16 09:35:20 +00:00
renovate[bot]
057d9599f5 Update TryGhost packages 2023-10-31 20:54:17 +01:00
Daniel Lockyer
85d41d0562 Aligned dependencies with resolution values
- this commit brings all dependencies up-to-date with the version set as
  a resolution
2023-10-13 08:37:36 +02:00
Daniel Lockyer
85098e07d4 Configured all unit tests to use dot reporter
refs https://ghost.slack.com/archives/C02G9E68C/p1696490748701419

- this configures mocha to use the dot reporter because the default is
  way too verbose in CI
2023-10-05 12:24:24 +02:00
Kevin Ansfield
562e8af26a Fixed saving collection card with 'latest' collection on sqlite
no issue

- added passthrough of `transaction` property when fetching post IDs otherwise SQLite will error with ` "Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call?"`
2023-09-25 15:11:05 +01:00
Fabien "egg" O'Carroll
9b2a94f931 Implemented "virtual" Collection for "latest"
refs https://github.com/TryGhost/Arch/issues/95

Rather than storing all of the relations between the latest collection and
posts, we know that it contains all posts. This means we don't have to keep the
collections posts in sync. Instead we can fetch them from the posts table. This
saves a lot of work during recalculation.
2023-09-22 19:05:37 +07:00
Fabien "egg" O'Carroll
45c1a82909 Forced "latest" Collection Entity to have no posts
refs https://github.com/TryGhost/Arch/issues/95

We're going to be treating the "latest" Collection as a "virtual" Collection
where we will hydrate the posts from the posts repository. This will allow for
more performant queries, and less work to keep the "latest" Collection in sync
2023-09-22 19:05:37 +07:00
Fabien "egg" O'Carroll
3a624ec3a1 Fixed return values of CollectionService
Our service layer should not expose the Entities, it should return the DTOs.
This is to allow us to make internal refactors without having to modify the
entire stack.
2023-09-22 19:05:37 +07:00
Fabien "egg" O'Carroll
673b06119b Wired up CollectionPost events to Collection
https://github.com/TryGhost/Arch/issues/95

Entities don't dispatch their own events, instead they add to an event array
and are dispatched once persisted.
2023-09-22 18:32:17 +07:00
Fabien "egg" O'Carroll
637724ba66 Added CollectionPost events
refs https://github.com/TryGhost/Arch/issues/95

These events can be used to know when an automatic collections posts have been
updated, as well as by the repository to optimise the storage of
CollectionPosts
2023-09-22 18:32:17 +07:00
Naz
99f29a169c Removed post delete related event handling
closes https://github.com/TryGhost/Arch/issues/91

- We have on cascade delete (a9f9f6121a/ghost/core/core/server/data/schema/schema.js (L1068)) on `post_id` column which handles post deletion logic automatically on DB level.
- The commented out handlers in the long term should be hooked up with public CollectionService methods on the client side.
2023-09-15 13:50:25 +08:00
Kevin Ansfield
1c68bd6779
Fixed transaction errors using PostsService inside of Post model's onSaving hook (#18148)
no issue

`PostsService` and `CollectionsService` were missing some passthroughs and had differing naming for a transaction instance on the `options` object which meant SQLite would hang if the Lexical renderer called out to `PostsService.browsePosts`

- added passthrough of `transacting` to the Lexical renderer ready for implementation of the collection-fetching function
- added rename of `options.transacting` to `options.transaction` and passthrough from `PostsService` to `CollectionsService` (passthrough from collections repository to bookshelf and required `transaction->transacting` was already in place)
2023-09-14 17:22:10 +00:00
Daniel Lockyer
6dc1d08590 Re-enabled general eslint rules in TS config
refs https://github.com/TryGhost/DevOps/issues/50

- when creating a TS config in our `eslint-plugin-ghost` dependency, I
  only extended the recommended config, which left out a lot of
  stylistic things we used to enforce in JS
- this fixes that by bumping the dependency to a version which extends
  those shared configs, and fixes all the code that currently goes
  against those rules
2023-09-08 13:47:42 +02:00
Fabien "egg" O'Carroll
c9fff97c75 fixup! Added logging to collections event handlers 2023-09-08 14:46:38 +07:00
Fabien "egg" O'Carroll
ff8a3fbf32 fixup! Added logging to collections event handlers 2023-09-08 14:46:38 +07:00
Fabien "egg" O'Carroll
c6f908d31d Added logging to collections event handlers
This will help us with debugging issues we're seeing with large sites
2023-09-08 14:46:38 +07:00
Naz
fc5f139d39 Removed stray console.log
refs 367584f1c2

- I fucked up. ESLint fucked up. Life happened to all of us
2023-09-08 15:28:47 +08:00
Naz
367584f1c2 Optimized PostEditedEvent processing in collections
refs https://github.com/TryGhost/Arch/issues/86

- When PostEditedEvent data contains no visible changes we can skip the matching collections update process alltogether. Each call to `updatePostInMatchingCollections` creates a transaction in addition to fetching all collections. There's no need to process anything when there are no relevant changes in the post edit!
2023-09-08 13:46:44 +08:00
Daniel Lockyer
c6cb35074a Updated linting and testing packages 2023-09-01 15:51:17 +02:00
Naz
608d4dfa10 Optimized the amount of logs produced by collections module
refs https://github.com/TryGhost/Arch/issues/82

- Collections logs are too verbose causing noise.
- Moved some of the logs to use "debug" for now and made summarized logs for the information that we still need while collections code is actively monitored. The event info logs can be removed once we are passed the active phase of rolling out the collections feature
2023-09-01 12:40:54 +08:00
Naz
acccc16614 Added collections update after bulk adding tags
refs https://github.com/TryGhost/Arch/issues/77

- During initial development we have missed to support collections update when tags are added to posts in bulk. It's especially valid usecase since we can define automatic collection with a filter containing not yet existing tags.
2023-08-23 17:12:08 +08:00
Fabien "egg" O'Carroll
62d5ca558d Validated filters for collections
refs https://github.com/TryGhost/Arch/issues/47

This ensures that we only have collections which have a valid filter in terms of
  - Valid NQL string
  - Uses only properties which are valid to filter on
  - Only has an empty filter in the case of the "latest" collection
2023-08-23 15:49:06 +07:00
Fabien "egg" O'Carroll
ad62cd4822 Removed edit method from Collection
We want to keep Entities as basic as possible, particularly when setting
property, so that each property can handle its own validation.
2023-08-17 09:45:41 +01:00
Naz
aaf0998129 Fixed transaction completion error on collection update
refs https://github.com/TryGhost/Arch/issues/16

- Without an extra await in the update function the passed in transaction would complete before all updates had a chance to run within this transaction.
2023-08-01 15:11:00 +08:00
Naz
fe4c0b18cf Removed unnecessary getBulk in PostsRepository
refs https://github.com/TryGhost/Arch/issues/16

- The method ended up being used only once, so it makes no sense to complicate the interface without any gain.
2023-08-01 15:11:00 +08:00
Naz
8635f4efeb Added collections handling for PostsBulkFeaturedEvent/PostsBulkUnfeaturedEvent
refs https://github.com/TryGhost/Arch/issues/16

- When posts produce PostsBulkFeaturedEvent/PostsBulkUnfeaturedEvent the collections having a featured filter should update the posts belonging to them.
2023-08-01 15:11:00 +08:00
Naz
8046c33194 Added collections handling for PostsBulkUnpublishedEvent
refs https://github.com/TryGhost/Arch/issues/16

- When posts produce PostsBulkUnpublishedEvent the collections having a published_at filter should update the posts belonging to them
2023-08-01 15:11:00 +08:00
Naz
a44d4a24d3 Fixed "any" types for Knex transaction objects
no issue

- We should use specific types wherever possible instead of "any" as it's an antipattern that spreads like cancer through the codebase.
2023-08-01 15:11:00 +08:00
Naz
f4143a8939 Fixed build scripts for post-events package 2023-08-01 15:11:00 +08:00
Naz
9b257f3966 Update of collection posts on bulk destroy
refs https://github.com/TryGhost/Arch/issues/16

- When the bulk destroy is done on posts Collections need to know about the update and remove the stored posts from collections.
2023-08-01 15:11:00 +08:00
Daniel Lockyer
6dd18d81d4 Enabled no-explicit-any for majority of packages
refs https://github.com/TryGhost/DevOps/issues/50

- we should default to keeping the rule on and so I've excluded lines
  that currently use `any` to avoid the need to go and fix them all up
2023-07-27 16:49:08 +02:00
Fabien "egg" O'Carroll
fa40485fb1 Handled updating Collections on TagDeletedEvent
closes https://github.com/TryGhost/Arch/issues/62

Because there are many ways in which filters can rely on tags, we will just
recalculate all automatic collections for now, rather than attempting to do
optimised updates.
2023-07-27 16:33:16 +02:00
Fabien "egg" O'Carroll
9288f56649 Improved type definitions
The PostRepository type was using `any` (an anti pattern) rather than
`PostCollection`, and we had optional properties, which are not really
optional. This cleans up the types and updates the tests alongside them.
2023-07-27 16:33:16 +02:00
Daniel Lockyer
c97cc08455 Updated ESLint config for React+Typescript packages
refs https://github.com/TryGhost/DevOps/issues/50

- `react-app` comes from `eslint-config-react-app`, which is a CRA package
- we're moving away from that so this commit switches the linting over
  to a more recently updated plugin
- once that was removed, we started using a newer version of
  `@typescript-eslint/eslint-plugin`, so there were plenty of
  updates/exemptions to make
2023-07-27 11:40:31 +02:00
Fabien "egg" O'Carroll
607ea8dcd7 Added TagDeletedEvent
refs https://github.com/TryGhost/Arch/issues/60

This will be used to update collections when a tag is deleted. Like the Post
events this should not be in the collections package, instead we should have
these as part of the tags and posts packages. These packages don't exist right
now, so I'm following the existing pattern.
2023-07-26 12:54:42 +02:00
Daniel Lockyer
b0cf1f949a Switched TypeScript eslint config to custom plugin
refs https://github.com/TryGhost/DevOps/issues/50

- this switches the .eslint configs from `node` to `ts`, which is a new
  config to support eslint for TypeScript
- also makes minor changes to adhere to these new rules
2023-07-25 16:12:35 +02:00
Naz
694ab1d32d Added support for expansions in collection filters
refs https://github.com/TryGhost/Arch/issues/46

- Similarly to post filters, collection filters now support both 'tag' and 'tags' nql filter keys when defining a filter for related tag slugs. For example, both `tag:avocado` and `tags:avocado` would both be valid collection filters that would filter by the same 'slug' property of the tags assigned to a post.
- Along with these changes had to rework the tags property of the collection posts to match the shape used in post resources. Moved from:
`tags: ['bacon', 'broc']`
to
`tags:[{slug: 'bacon'}, {slug: 'broc'}]`
2023-07-25 19:01:56 +08:00
Daniel Lockyer
d5cbb33e54 Pinned dependencies 2023-07-24 16:21:47 +02:00
Daniel Lockyer
bb4c3de992 Deduplicated ts-node and typescript dependencies
refs https://github.com/TryGhost/DevOps/issues/50

- this commit deduplicates where we define the versions of these
  packages to make them easier to maintain
2023-07-24 16:21:47 +02:00
Naz
9a38c65ee6 Cleaned up CollectionsService from unused code
refs https://github.com/TryGhost/Arch/issues/16

- Some of the methods became unused due to moving the posts handling out of the collections service and cleaning up the event handling system.
2023-07-21 19:46:22 +08:00
Naz
6361423ff7 Fixed ability to remove post from an automatic collection
refs https://github.com/TryGhost/Arch/issues/16

- Post can be removed from a manual collection, but never from an automatic collection.
2023-07-21 19:46:22 +08:00
Naz
d7bbb0b935 Reworked PostsRepository to use model layer
refs https://github.com/TryGhost/Arch/issues/16

- Using the API directly on the repository level prevented us from ensuring collection consistency through transactions.
- This change migrates the PostsRepository to use Bookshelf model layer directly, which also allows to put queries into transactions.
- Additional optimization here was removing the `getAllPosts` method from CollectionService. This is an attempt to reduce the API surface of the  of the service before calling it a GA.
2023-07-21 19:46:22 +08:00
Naz
3dc27f505c Wrapped collection's creation in transaction
refs https://github.com/TryGhost/Arch/issues/16

- This is needed to avoid stale data/race conditions when processing collection post updates
2023-07-21 19:46:22 +08:00
Naz
0880770d50 Wrapped collection's post adding logic in transaction
refs https://github.com/TryGhost/Arch/issues/16

- Having transactional collection post updates makes sure there are no race conditions when updating collection_posts relations. Without the transactions collection was prone to update relations based on a stale state causing problems like described in the linked issue
2023-07-21 19:46:22 +08:00
Naz
04c92d2ca5 Removed CollectionResourceChangeEvent handling
refs https://github.com/TryGhost/Arch/issues/16

- The generic "UpdateAllCollections" logic should not be used as the mapped Post's delete/add/edit events should be sufficient in maintaining collection's state
2023-07-21 19:46:22 +08:00
Naz
53f9f954c1 Added tag filter support to collections
refs https://github.com/TryGhost/Arch/issues/41

- When an new collection is created the relational "tags" filter is now picked up properly and appropriate posts matching the tag filter are assigned and stored in the collection. Example collection filter that is now supported: `tags:['bacon']`
- Additionally cleaned up returned collection post DTOs, so we return as little data as possible and add only the fields that are needed
2023-07-18 20:18:54 +08:00