- `.substr()` is deprecated so we replace it with .slice() which works similarly but isn't deprecated
Signed-off-by: Tobias Speicher <rootcommander@gmail.com>
refs https://github.com/TryGhost/Team/issues/1248
This is the underlying cause of the problems we've seen whilst handling
Stripe webhooks. A transaction ensures that the operations are atomic,
but not that they can run concurrently.
If you have some code which does this, concurrently:
1. Starts a transaction
2. Reads a value
3. Changes the values
4. Ends the transaction
Without applying the `FOR UPDATE` lock - you will have both sequenes
read the same value at step 2.
With the `FOR UPDATE` lock - one of the sequences will hang at step 2,
waiting for the other transaction to end, at which point it will resume
and read the _changed_ value.
Because the `edit` method explicitly does a read followed by a write, we
have also add the `FOR UPDATE` lock to this by default, to avoid any race
conditions. This does however require that `edit` is called within a
transaction. An issue here https://github.com/TryGhost/Team/issues/1503
considers running in a transaction by default.
refs https://github.com/TryGhost/Team/issues/1284
When you create a new post with a tag slug that contains spaces, those spaces will get replaced by dashes. But instead of reusing an existing tag, a new tag is always created.
- New tag slugs are cleaned up before matching with existing tags in the Post model onSaving method
- Cleaned up multiple loops in onSaving of Post model
- Cleaned up syntax when cleaning up tag slug
- Added tests for slugs with spaces
- Added test for too long tag slug causing duplication
refs https://github.com/TryGhost/Team/issues/1408
- switched from `@nexes/nql` to `@tryghost/nql` and bumped `@tryghost/bookshelf-plugins` to get access to the latest NQL version across the app
- adds "contains" operator support
- `:~'string'` - contains
- `:-~'string'` - does not contain
- `:~^'string'` - starts with
- `:-~^'string'` - does not start with
- `:~$'string'` - ends with
- `:-~$'string'` - does not end with
- enables `'` escaping in strings, eg `'O\'Nolan'`
no-issue
Knex.js aliases "bool" -> "boolean" - this means that you can use either
one in our schema.json file and it'll correctly create the column. Our
model plugin however would only handle columns which used the "bool"
alias. This fixes the plugin to handle both strings.
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
- as per 5a5a5d162e, the Bookshelf registry plugin is now in core
- we no longer need to explicitly load the plugin, and it displays a
warning if you do
- this change also turns `._models` into `.registry.models`, so our code has
been updated to reflect that
refs https://github.com/TryGhost/Team/issues/946
This refactor pulls out the core logic so that we can easily add other
bulk operations without having to duplicate even more logic.
It also gives a consistent return value between bulk operations, renaming
`unsuccessfulIds` and `unsuccessfulRecords` to `unsuccessfulData`
We also add a bulkEdit method which will be used to bulk unsubscribe members
from the newsletter.
- This is a precursor to trying to split apart into:
- model events + webhooks system which makes sense
- frontend events which should be independent or removed
- maybe some concept of a settings manager that we can use in various places to bind logic 🤔
- other usages of events that should be refactored to not use events
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
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 188de00489
- this fix was incorrect - the function should have been on the
prototype but I'd moved it over incorrectly into the static class functions
- this commit moves `defaultColumnsToFetch` to the prototype functions
and reverts the referenced commit back to `this.prototype` as expected
- this wasn't including the custom columns from the `post` model, which
was causing tests to fail
- pro tip: run tests!
refs a457631a20
- `defaultColumnsToFetch` was moved to the CRUD plugin in the referenced
commit, which makes it a function on `this` instead of `this.prototype`
- this means the function doesn't exist and Admin throws an error when
you start typing in the search bar because the API 500s
- this commit switches it to `this.defaultColumnsToFetch()`
refs https://github.com/TryGhost/Team/issues/808
- see referenced issue for context, but turning this function into
async-await seems to have broken error handling when deleting things
that don't exist
- i'm really not sure why - but my running theory is that it's something
to do with Bluebird Promises vs native Promises
- this should keep the same functionality until I can investigate what
is going on
no issue
- `setId` is only used within the `events` plugin and it makes sense to
keep code together
- we don't lose anything by putting it here, but it should make it
easier to test in the future
no issue
- `formatOnWrite` doesn't override anything in Bookshelf but we use it
within the `override` plugin and sub-models may override it, so it's
easier to keep these things together
no issue
- we were only importing the `db` to access the `knex` instance, but
we can get this through the Bookshelf instance
- switches to pulling out `knex` from Bookshelf so we can remove the
remaining local require
