geddy / model

Datastore-agnostic ORM in JavaScript
265 stars 55 forks source link

Data from database different than model returns #246

Open OscarGodson opened 9 years ago

OscarGodson commented 9 years ago

Database shows this in my families table:

+--------------+---------------------+---------------------+--------------------------------------+----------+
| name         | created_at          | updated_at          | id                                   | disabled |
+--------------+---------------------+---------------------+--------------------------------------+----------+
| Your Family  | 2014-12-30 00:19:19 | 2014-12-30 00:34:05 | 89C79DCB-9001-4F4F-8588-4942796B6A70 |        1 |
+--------------+---------------------+---------------------+--------------------------------------+----------+

When I make this call:

UserModel.first({id: query, disabled: false}, {includes: ['families', 'familyMemberships']}, function (err, data) {
  if (err) return callback.call(this, createResponse.error());
  if (!data) return callback.call(this, createResponse.error(404, 'No user found'));
  console.log(data.families)
});

I get this and the disabled prop says 1 (note I removed internal props and methods from this response)


[  
  { type: 'Family',
    id: '89C79DCB-9001-4F4F-8588-4942796B6A70',
    createdAt: Mon Dec 29 2014 16:19:19 GMT-0800 (PST),
    updatedAt: Mon Dec 29 2014 16:34:05 GMT-0800 (PST),
    name: 'Your Family',
    disabled: 0 }]

My user model has that property set to a boolean, but I also tried number. I'm not sure what else to try? I pass model true/false and it seems to save the data fine in the database as I can see 1 for true and 0 for false toggles correctly, but getting it always returns 0.

OscarGodson commented 9 years ago

It does seem tho that looking up this family like this:

    FamilyModel.first({id: query, disabled: false}, {includes: ['users', 'familyMemberships']}, function (err, familyModel) {
      if (err) return callback.call(this, createResponse.error());
      if (!familyModel) return callback.call(this, createResponse.error(404, 'No family found'));
      console.log(familyModel)
    });

returns my expected

{
"message": "No family found"
}
OscarGodson commented 9 years ago

oh...I think I may know what it is... is afterCreate supposed to fire even on reads? It appears as tho it's firing "create" on associated models. This is why it works if you hit it directly, but if you ask for the association its like its creating it new. This also means all my default props im setting in afterCreate are overriding whats actually in the DB

ben-ng commented 9 years ago

Yes, create fires even on reads. I'm not sure if this is correct behavior anymore, since we now have the reify scenario.

Since the create method doesn't actually save models to the database (you still have to call save), create is really more of an initialize method, so the behavior is correct if you look at it that way.

See here, where models are initialized during a query using the create method.

I agree that this behavior is confusing though. What do you think @mde?

OscarGodson commented 9 years ago

@ben-ng afterCreate tho is the suggested way of setting defaults as far as i know. Is there some other method or way in the model to set defaults on create?

The hardest part is that this fires only on associated models. If I read Foo model it doesn't fire. If I read Foo model with Bar and Baz associations then Bar and Baz get the create but Foo doesn't. At least if Foo did it then I could account for that a little easier, but currently I have to come up with a way to know if its being called as an association or as a "top level" model.

OscarGodson commented 9 years ago

So, at least for now and in my case, I'm getting around it with a tiny module:

module.exports = function (prop, def) {
  return prop === undefined ? def : prop;
};

I use it like

var setAsModelDefault = require('../utils/set-as-model-default');
this.afterCreate = function () {
  this.disabled = setAsModelDefault(this.disabled, false);
};

Not ideal, but it works for me

ben-ng commented 9 years ago

Ahh i see. I'll take a look at making the behavior consistent then. I think the right thing to do is probably to not fire create on reads anymore. Its going to be a breaking change, but its the most logical to me. We should have an init event take its place. Then, init will happen on both queries and user-performed initializations, create only on user-performed initializations, and reify on queries.