Unable to reset model instance attribute in sailsjs

170 views Asked by At

Before we start: I have made a very small git repo with seed data and two REST endpoints to test this issue here: https://github.com/juanpasolano/sails-nested-test

So I have 3 models: Appointment which has many procedure which has one to one procedureItem. Since there is not nested population in sails I am getting procedures with procedureItem by hand using something like:

  Appointment.find(1).exec(function(err, appointments){
        if(err) return res.negotiate(err);
        async.eachSeries(appointments, function(appointment, cb){
            Procedure.find({appointment: appointment.id}).populate('procedureItem').exec(function(errP, procedures){
                if(errP) return cb(errP);
                appointment.procedures = procedures;
                appointment.proceduress = procedures;
                cb()
            })
        }, function(errE){
            if(errE) return cb(errE);
            res.ok(appointments)

So the issue is when I want to replace the proceduresattribute with the new array procedures (which has the nested depth i need) it just doesn't get set, if you hit the endpoints of the demo, there is no procedures attribute available.
As a test I have attached the same new procedures array to proceduress (double s) attribute and this one gets properly set.
I have tried using the .toObject() method with no luck.
Have tried sails 0.10.5 and 0.11.0 with no luck.

3

There are 3 answers

8
Bulkin On

Yes, there is no procedures attribute, as Appointment has MANY Procedure, so there is no field procedures in DB in Appointment table. So if you do not populate like:

Appointment.find(1).populate('procedures').exec(function(err, appointments){
.....
}

There will not be attribute procedures in appointment Object

1
Evilsanta On

I have the exactly same problem a while ago, I believe it is because the "procedure" attribute is in use by the model and not be able to set. I end up cloning each row and then the value was able to be set.

To change your code:

if(err) return res.negotiate(err);
        var toreturn=_.map(appointments, function(a){
            var b=_.clone(a); // so you clone it to a new obj, then you can set the procedure... 
            b.procedures = 'aaaa'
            return b;
        })
        //res.json(toreturn); // Here you will get appointments with procedures='aaa'
        async.eachSeries(toreturn, function(appointment, cb){
            Procedure.find({appointment: appointment.id}).populate('procedureItem').exec(function(errP, procedures){
                if(errP) return cb(errP);
                appointment.proceduress = procedures;
                appointment.procedures = procedures;
                cb()
            })
        }, function(errE){
            if(errE) return cb(errE);
            res.ok(toreturn)
        })

I do not know why this happened exactly, but this is just one solution.

On a side note, may I suggest you instead of doing async for each appoint, will takes a query per appoint, do two big find, and then loop to merge them?

Something like this:

var globalApps=[];
Appointment.find().then(function(apps){
    globalApps=apps;
    var appIDs=apps.map(function(a){return a.id}); //Get all appointment IDs
    return Procedure.find({appointment:appIDs}).populate('procedureItem');
}).then(function(procs){ //This procs is all procedures match the ids
    var toReturn =_.map(globalApps,function(a){
        var b=_.clone(a);
        b.procedure=_.find(procs,{appointment:b.id}); //  This is O(n^2) complexity matching, but you can do something smart.
       return b;
    }); 
    // Now you have toReturn as the appointment array.
   return res.json(toReturn);
})

I have spent a lot of time on this problem before, glad someone had the same problem

0
sgress454 On

@sgress454 from Balderdash here. The issue you're encountering is that the procedures attribute isn't a plain Javascript variable; it has a "getter" and a "setter", which are necessary in order to allow these to work:

appointment.procedures.add(<some Procedure obj or id>);
appointment.procedures.remove(<some Procedure id>);

So attempting to set the procedures attribute to a new value won't work, because of the setter. Cloning the appointment is an acceptable workaround (you can just do appointment.toObject()) as it transforms those special attributes into plain old Javascript variables. But in your case, it seems like you don't actually need to replace the procedures array--you just want each procedure's procedureItem to be filled out. And since only attributes representing many-to-many associations have getters and setters, you can accomplish this without any cloning at all:

Appointment
    // Find the appointments
    .find({...some criteria...})
    // Populate each appointment's procedure
    .populate('procedure')
    .exec(function(err, appointments) {
        if (err) {return res.negotiate(err);}
        // Loop through the returned appointments
        async.each(appointments, function(appointment, appointmentCb) {
            // For each one, loop through its procedures
            async.each(appointment.procedures, function(procedure, procedureCb) {
                // Look up the related procedureItem
                ProcedureItem.findOne(procedure.procedureItem).exec(function(err, procedureItem) {
                    if (err) {return procedureCb(err);}
                    // Attach the procedureItem object to the procedure
                    procedure.procedureItem = procedureItem;
                    return procedureCb();
                });
            }, appointmentCb);
        }, function done(err) {
            if (err) {return res.negotiate(err);}  
            res.ok(appointments);
        });
    });

Note that there's a lot of optimization that could be done here--you could pluck out all of the ProcedureItem IDs after retrieving appointments, look up all the ProcedureItem instances at once, and then map them onto the procedures afterwards, saving a ton of queries.