diff --git a/doc/release-notes/changelog.md b/doc/release-notes/changelog.md index 3f27ba756..303a4e84a 100644 --- a/doc/release-notes/changelog.md +++ b/doc/release-notes/changelog.md @@ -77,6 +77,7 @@ - `insertGraph()` and `upsertGraph()` reject only after the queries they already started have finished, so a transaction is rolled back after them. Before, rows of these queries could be committed after the rollback on SQLite. [#335](https://github.com/ditojs/objection/pull/335) - `upsertGraph()` sets the foreign key of a model that a `#ref` relates into a `HasManyRelation` or `HasOneRelation`. Before, the row was left without it, and the id of the owner could be cleared [#311](https://github.com/ditojs/objection/issues/311) - `patch()` and `merge()` with field expression keys like `'meta:color'` keep writing them when the query is built more than once, e.g. with `toKnexQuery()` before running it, or through a `clone()`, instead of resolving to 0 without a write. Building the query no longer changes the data of `patch()` and `merge()`: the patched model keeps these keys until the write has run, so `$afterUpdate()` sees them, and then loses them, also with `snakeCaseMappers()`, so the models that `$query().patch()`, `patchAndFetch*()` and `upsertGraph()` return don't have them [#53](https://github.com/ditojs/objection/issues/53) +- `upsertGraph()` with `relate` and `noUpdate` writes the foreign keys of models that it relates through a `HasManyRelation` or `HasOneRelation`, like it does for `BelongsToOneRelation` and `ManyToManyRelation`, instead of leaving them unrelated without an error, as it did since 1.5.0 [#53](https://github.com/ditojs/objection/issues/53) ### Types diff --git a/lib/queryBuilder/graph/GraphOptions.js b/lib/queryBuilder/graph/GraphOptions.js index df412e8e3..ccc2a5e59 100644 --- a/lib/queryBuilder/graph/GraphOptions.js +++ b/lib/queryBuilder/graph/GraphOptions.js @@ -147,7 +147,7 @@ export class GraphOptions { // Whether a node that the graph relates or patches is written with `patch()` // or `update()`. With `noUpdate`, it's neither, and only the foreign keys of - // its BelongsToOneRelations are patched. + // its relates and BelongsToOneRelations are patched. shouldPatch(node) { return !this._hasOption(node, NO_UPDATE) && !this._hasOption(node, UPDATE); } diff --git a/lib/queryBuilder/graph/patch/GraphPatchAction.js b/lib/queryBuilder/graph/patch/GraphPatchAction.js index 3cab53d46..7569d71aa 100644 --- a/lib/queryBuilder/graph/patch/GraphPatchAction.js +++ b/lib/queryBuilder/graph/patch/GraphPatchAction.js @@ -36,6 +36,14 @@ export class GraphPatchAction extends GraphAction { const changedPropsBecauseOfBelongsToMany = this.graphData.actions.get(node) === 'patch' ? this._setBelongsToManyKeys(node) : []; + // HasManyRelation and HasOneRelation relates write the related object's + // foreign keys, also with `noUpdate`. + const changedPropsBecauseOfRelate = + this.graphData.actions.get(node) === 'relate' && + node.parentEdge.relation.isObjectionHasManyRelation + ? node.parentEdge.relation.relatedProp.props + : []; + const handleUpdate = () => { const { changedProps, unchangedProps } = this._findChanges(node); const allProps = [...changedProps, ...unchangedProps]; @@ -47,6 +55,7 @@ export class GraphPatchAction extends GraphAction { ...changedPropsBecauseOfBelongsToOneInsert, ...changePropsBecauseOfBelongsToOneDelete, ...changedPropsBecauseOfBelongsToMany, + ...changedPropsBecauseOfRelate, ], // Remove id properties from the props to update. With upsertGraph diff --git a/tests/integration/upsertGraph.js b/tests/integration/upsertGraph.js index 9f0fa2396..8c9c3787b 100644 --- a/tests/integration/upsertGraph.js +++ b/tests/integration/upsertGraph.js @@ -2862,6 +2862,22 @@ export default (session) => { }); }); + it('should only write the foreign key when hasManyRelation and hasOneRelation models are related with noUpdate: true', async () => { + await Model1.query(session.knex).upsertGraph( + { + id: 1, + model1Relation1Inverse: { id: 4, model1Prop1: 'this should not be written to db' }, + model1Relation2: [{ idCol: 1, model2Prop1: 'this should not be written to db' }], + }, + { fetchStrategy, relate: true, noUpdate: true, noDelete: true }, + ); + + const model1 = await Model1.query(session.knex).findById(4); + expect(model1).toMatchObject({ id: 4, model1Id: 1, model1Prop1: 'manyToMany 1' }); + const model2 = await Model2.query(session.knex).findById(1); + expect(model2).toMatchObject({ idCol: 1, model1Id: 1, model2Prop1: 'hasMany 1' }); + }); + it('should not update other than the relation properties when belongsToOneRelation is unrelated but the parent has noUpdate: true', () => { const upsert1 = { id: 2,