Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UpsertGraph deletes rows for relation which is not mentioned in graph #1455

Closed
lynxtaa opened this issue Aug 30, 2019 · 8 comments
Closed

UpsertGraph deletes rows for relation which is not mentioned in graph #1455

lynxtaa opened this issue Aug 30, 2019 · 8 comments
Labels

Comments

@lynxtaa
Copy link

lynxtaa commented Aug 30, 2019

I'm using [email protected] and have these models:

class Role extends Model {
  static get tableName() {
    return 'roles'
  }

  static get relationMappings() {
    return {
      sets: {
        relation: Model.HasManyRelation,
        modelClass: 'Set',
        join: { from: 'roles.id', to: 'sets.roleId' },
      },
    }
  }
}

class Set extends Model {
  static get tableName() {
    return 'sets'
  }

  static get relationMappings() {
    return {
      setAttributes: {
        relation: Model.HasManyRelation,
        modelClass: 'SetAttribute',
        join: { from: 'sets.id', to: 'sets_attributes.setId' },
      },
    }
  }
}

class SetAttribute extends Model {
  static get tableName() {
    return 'sets_attributes'
  }
}

Suppose role with ID 1 already has set with ID 1.

If I'm using upsertGraph like this, It works correctly -- new row is inserted in "sets" table:

transaction(Role.knex(), trx =>
  Role.query(trx).upsertGraph({
    id: 1,
    sets: [{ id: 1 }, { name: 'New Set' }],
  }),
)

But if I pass "setAttributes" for newly created set, Objection deletes all setAttributes for the first set (with id=1):

transaction(Role.knex(), trx =>
  Role.query(trx).upsertGraph({
    id: 1,
    sets: [
      { id: 1 },
      { name: 'New Set', setAttributes: [{ name: 'New Set Attribute' }] },
    ],
  }),
)

Is this behaviour expected?

@koskimas
Copy link
Collaborator

Could you provide a reproduction for this? I don't believe this is really happening. There are tests for this case. I'll fix it immediately if you can provide a reproduction.

@lynxtaa
Copy link
Author

lynxtaa commented Aug 30, 2019

Thanks for a quick response! I've created reproducible example: https://github.com/lynxtaa/objection-issue

@koskimas
Copy link
Collaborator

Thanks for the repro! I'll see what's going on asap.

@lynxtaa
Copy link
Author

lynxtaa commented Sep 7, 2019

@koskimas any updates on this?

@koskimas
Copy link
Collaborator

I'll post here once I find time to work on this.

@koskimas koskimas added the bug label Sep 21, 2019
@koskimas
Copy link
Collaborator

Fix coming up soon. Sorry it took so long.

@koskimas
Copy link
Collaborator

Oh man, this was a pretty nasty bug! I can't believe why nobody reported this earlier. objection 1.6.10 will be released in couple of minutes.

@lynxtaa
Copy link
Author

lynxtaa commented Sep 21, 2019

@koskimas Thank you! We are using Objection.js in production and very happy with it

koskimas added a commit that referenced this issue Sep 21, 2019
capaj added a commit to capaj/objection.js that referenced this issue Mar 10, 2020
# Conflicts:
#	.travis.yml
#	doc/changelog/README.md
#	doc/guide/query-examples.md
#	lib/queryBuilder/QueryBuilder.js
#	lib/queryBuilder/QueryBuilderOperationSupport.js
#	lib/queryBuilder/graph/GraphUpsert.js
#	lib/queryBuilder/operations/UpdateAndFetchOperation.js
#	lib/relations/Relation.js
#	lib/transaction.js
#	package.json
#	tests/integration/misc/Vincit#1455.js
#	tests/integration/patch.js
#	typings/objection/index.d.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants