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

save() callback does not work if autoFetch enabled #256

Closed
wp-tbikbulatov opened this issue Jul 30, 2013 · 8 comments
Closed

save() callback does not work if autoFetch enabled #256

wp-tbikbulatov opened this issue Jul 30, 2013 · 8 comments

Comments

@wp-tbikbulatov
Copy link

In the code below save() method (in announcement_controller.js) does not saving changes in user_id field. If i uncomment a validation rules in the announcement model definition then it's saving successfully, but save() method`s callback does not work. It saves successfully if i remove "autoFetch: true" (v2.0.15).

models.js:

    db.define('user', {
        email    : {type: 'text', required: true},
        name     : {type: 'text', required: true},
        contacts : String,
        created  : Date
    }/*, {
            validations: {
                user_id: [
                    orm.validators.required("required"),
                    orm.validators.notEmptyString("not_empty")
                ]
            }
        }*/
    );

    db.define('announcement', {
        address   : String,
        photo     : String,
        created   : Date,
        success   : Boolean
    });
    db.models.announcement
        .hasOne('user', db.models.user, {required: true, reverse: 'announcements', autoFetch : true});

announcement_controller.js:

    var Announcement = db.models.announcement;
    Announcement.get(params.id, function (err, item) {
        item.save(req.body.announcement[0], function (err) {
            if (err) return next(err);

            send({item: item});
        });
    });
@dresende
Copy link
Owner

Can you please test the latest git version?

# this gets package from git
npm install dresende/node-orm2

Also, if the version doesn't work, can you give some more information about params.id and req.body.announcement[0] ?

@wp-tbikbulatov
Copy link
Author

I've tested on the latest git version and it's still does not work. There is request data (all foreign keys are valid):

body: {
    "announcement": {
        "0": {
            "address": "Test st., b.10",
            "photo": "jpg",
            "user_id": "3"
        }
    }
}

params: {
    "id": "17"
}

@dresende
Copy link
Owner

dresende commented Aug 1, 2013

I'm not having this problem, with or without the validations. Thinking about it, it does not make sense that the validations in another model will interfere with this model. Are you sure it's that controller code? Isn't other that gets a user or something?

@gaetsd
Copy link

gaetsd commented Aug 2, 2013

Hi, I think it's the same issue. I write this test:


var orm = require('orm');
orm.connect("mysql://[email protected]/test?debug=true", function (err, db){
    console.log("CONNECTED TO DATABASE");
    ///////////////////////
    // CREATE THE PARENT
    ///////////////////////
    var Person = db.define('person', {
        name : String
    });
    ///////////////////////
    // CREATE THE CHILD
    ///////////////////////
    var Animal = db.define('animal', {
        name : String
    });
    ///////////////////////
    // LINK THEM UP
    ///////////////////////
    Person.hasMany("pets", Animal,{}, {
        autoFetch: true
    });
    ///////////////////////
    // Sync
    ///////////////////////
    db.sync(function(){
        console.log("SYNC'D DB `test`");
        ///////////////////////
        // Test insert
        ///////////////////////
        Person.create({
            name: 'John'
        }, function(err, john){
            
            Animal.create({
                name: 'rex'
            }, function(err, rex){
                john.pets = [rex];
                john.save(function(err){
                    if(err) console.log('err ' + err);
                    
                    console.log(john);
                    
                    console.log('END');
                    
                });
            
            }); 
        });
    });
});

When I show John, he has rex as pet but in database he hasn't it.

It's funny because if I add the line john.name = 'Max'; just before save, it's work.

@notheotherben
Copy link
Collaborator

If you try using john.setPets(rex, function(err) { ... }) it should work, sounds like the problem is because you're assigning a value to john.pets instead. That being said, it's a valid use case and should function correctly.

It does introduce the question of how to handle such things however, take this situation for example:

  • John already has 2 pets: [rex, lizzy]
  • We create a new pet tiger and set john.pets = [rex, tiger]
  • john.save(...)

Do we remove lizzy from the database now? (Being that it's a 1-n relationship without a parent) or keep lizzy, optimistically hoping that someone will do a jane.pets = [lizzy] at some point? Maybe it's just safer to disable the setter portion of 1-n association properties on such models to avoid this problem altogether?

@dresende Sounds to me like it isn't incrementing the changes counter for some reason, and assuming (incorrectly) that the changes have already been saved to the DB. Hence the setting of the name property (which would obviously increment the changes counter) fixing it.

@dresende
Copy link
Owner

dresende commented Aug 2, 2013

Yes, this can probably be fixed by monitoring association keys like we monitor model properties. I'll see if I can make a simple fix.

@dresende
Copy link
Owner

dresende commented Aug 2, 2013

Baah.. took me 1h to figure the right line to change :) It feels really good when you make a small change and it just works.

@dresende
Copy link
Owner

This has regressed but should now be fixed in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants