Vincit / objection.js

An SQL-friendly ORM for Node.js
https://vincit.github.io/objection.js
MIT License
7.23k stars 636 forks source link

Patching a model instance does not return the instance #158

Closed ersims closed 8 years ago

ersims commented 8 years ago

Hi,

I have encountered an issue with patching model instances. Documentation indicates that patching a model instance should return the updated model. In my case it returns a number (number of affected rows?). Complete code to reproduce below.

I am using the latest release of Objection.js (0.5.2) and Knex.js (0.11.7) and the issue exists for postgres as well.

Code to reproduce

'use strict';

const Knex = require('knex');
const Model = require('objection').Model;

const knex = Knex({
    client: 'sqlite3',
    useNullAsDefault: true,
    connection: {
        filename: ':memory:'
    }
});
Model.knex(knex);
class Person extends Model {
    static get tableName() { return 'Person'; }
}

knex.schema.createTable('Person', (table) => {
    table.increments('id').primary();
    table.string('firstName').notNullable();
    table.string('lastName').notNullable();
}).then(() => {
    return Person
        .query()
        .insertAndFetch({
            firstName: 'TestFirstName',
            lastName: 'TestLastName'
        });
}).then((person) => {
    console.log(person);
    return person.$query().patch({lastName: 'Cooper'}).then((patchedPerson) => {
        console.log(patchedPerson.lastName, patchedPerson); // Should give --> 'Cooper'.
        console.log(person); // Should be updated?
    });
});

Expected result

Person { firstName: 'TestFirstName', lastName: 'TestLastName', id: 1 }
Cooper Person { firstName: 'TestFirstName', lastName: 'Cooper', id: 1 }
Person { firstName: 'TestFirstName', lastName: 'Cooper', id: 1 }

Actual result

Person { firstName: 'TestFirstName', lastName: 'TestLastName', id: 1 }
undefined 1
Person { firstName: 'TestFirstName', lastName: 'TestLastName', id: 1 }

Workaround A simple workaround is to re-query the model instance after patching it

Person
    .query()
    .insertAndFetch({
        firstName: 'TestFirstName',
        lastName: 'TestLastName'
    }).then((person) => {
        return person.$query().patch({lastName: 'Cooper'}).then(() => {
            return person.$query().then((patchedPerson) => {
                console.log(patchedPerson);
                return patchedPerson;
            });
        });
    });
ersims commented 8 years ago

Just realized that this is intentional (#139 and #34), and I totally agree with a patchAndFetch method to solve this usecase.

The documentation should probably be changed to not indicate that the patch method returns the updated model.

koskimas commented 8 years ago

I'll update the documentation. Also the original person should be updated so we may have a bug there. I'll look into that one.