`where` and `scope` on associations

[amir-s]

Is it possible to do something like

let user = await User.find(1);
let posts = await user.posts.where({visible: true});

where the user hasMany('posts').

or maybe creating scopes on Post model and applying the scope to the relation instead of doing .where?

[PhilWaldmann]

Sure!

I've updated the docs and also published a new version.

You could do the following things:

// user definition
this.hasMany('posts', {scope: 'isVisible'})

// post definition
this.scope('isVisible', function(){
  this.where({visible: true})
})

or simpler without a scope:

// user definition
this.hasMany('posts', {conditions: {visible: true}})

and than you could fetch your models like this:

const user = await User.find(1).include('posts')
// or via
const user = await User.find(1)
const posts = await user.posts

Both ways of fetching the relation should use the relation conditions and scope option (if available).

[amir-s]

Thanks a lot :)

I tried it yesterday and it wasn't working. Tried it with the new version and it works now, it must have been a side effect of a bug that you have fixed!

Thanks!

[amir-s]

Actually I think there are some issues, what user.posts returns is not exactly what something like Post.where({user_id: 42}) returns. for example, i can't do await user.posts.clone(). It throws:

TypeError: Cannot read property 'chain' of undefined
    at Object.options.getFromCache (/usr/app/node_modules/openrecord/lib/base/relations/bulk_fetch.js:119:82)
    at Object.options.loadWithCollection (/usr/app/node_modules/openrecord/lib/base/relations.js:123:26)
    at Array.exec (/usr/app/node_modules/openrecord/lib/base/exec.js:24:32)
    at Array.then (/usr/app/node_modules/openrecord/lib/base/exec.js:66:17)
    at process._tickCallback (internal/process/next_tick.js:160:7)

Also, i think anything that is applied to user.posts, should not mutate the state of user.posts query itself, so user.posts should return a clone if itself. Currently, if I do user.posts.count(), it affects the next query user.posts.limit(10).

i'm using sqlite btw.

[PhilWaldmann]

user.posts.count() should definitely not affect user.posts.limit(10). Could you provide an example or test?!

I've just pushed some more relation tests + a few bugfixes. However, I don't think that it will solve any of your described issues.

Thanks, Phil

[amir-s]

Hmmm! ok, i think my mistake was to assign the user.posts to another variable, like let query = user.posts, and then do query.count(), and query.limit(10).

[amir-s]

but still, shouldn't I be able to do something like this?

let query = user.posts;
let count = await query.clone().count();
let firstTwo = await query.clone().limit(2);

error TypeError: Cannot set property 'posts' of undefined
    at Object.options.setResult (/<user>/node_modules/openrecord/lib/base/relations.js:169:36)
    at Array.options.add (/<user>/node_modules/openrecord/lib/base/relations/has_many.js:107:15)
    at /<user>/node_modules/openrecord/lib/base/collection.js:263:26
    at Array.forEach (<anonymous>)
    at Array.add (/<user>/node_modules/openrecord/lib/base/collection.js:189:13)
    at Array.new (/<user>/node_modules/openrecord/lib/base/collection.js:40:12)
    at Array.<anonymous> (/<user>/node_modules/openrecord/lib/base/collection.js:130:32)
    at task (/<user>/node_modules/openrecord/lib/base/interceptors.js:329:28)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

source code:

```js const Store = require('openrecord/store/sqlite3'); function setup () { this.createTable('users', function(){ this.string('name'); }); this.createTable('posts', function () { this.string('title'); this.boolean('visible'); this.integer('user_id'); }); } const store = new Store({ type: 'sqlite3', file: './db.sqlite3', autoLoad: true, autoSave: true, migrations: [ setup, ] }); class User extends store.BaseModel{ static definition(){ this.hasMany('posts'); } } store.Model(User); class Post extends store.BaseModel{ static definition(){ this.belongsTo('user'); } } store.Model(Post); store.ready(async () => { try { let user = await User.new({ name: 'Amir', posts: [{ title: 'post by amir 1', visible: true }, { title: 'post by amir 2', visible: false }, { title: 'post by amir 3', visible: true }, { title: 'post by amir 4', visible: false }] }); await user.save(); user = await User.find(1); let query = user.posts; let count = await query.clone().count(); let firstTwo = await query.clone().limit(2); console.log(count); console.log(firstTwo); }catch (e) { console.log("error", e); } }); ```

[PhilWaldmann]

I've fixed the problem! clone will copy all internal state, even nested objects. Your query object has a relation to the user record, which is stored inside that internal state. But the cloned record is just an object (it does not clone the prototype!), and therefore does not have any relational information... That's where it crashed! However, cloning a relation should break the connection to it's parent anyways.

Thanks for your example. I've also added some more test and the ability to do a record.has_many_through_relation.count()

[amir-s]

awesome!

i just tried with the latest master and it seems to be working perfectly! thanks for the explanation as well 👍

another quick question, it seems that i have to run store.ready() if i want the store to populate the modal store.Modal('blah'). again, i don't know if that is expected, maybe it is, and we just need to update the docs.

[amir-s]

Another issue that I have found is that creating a relation when creating the model itself, wipes out all the references.

let user1 = await User.create({
  name: 'Amir 1',
  posts: [{
    title: 'post by amir 1',
    visible: true
  }]
});

look at the second query:

  openrecord:save insert into `users` (`name`) values ('Amir 1') +0ms
  openrecord:save update `posts` set `user_id` = NULL +7ms
  openrecord:save insert into `posts` (`title`, `user_id`, `visible`) values ('post by amir 1', 3, true) +1ms

```js const Store = require('openrecord/store/sqlite3'); function setup () { this.createTable('users', function(){ this.string('name'); }); this.createTable('posts', function () { this.string('title'); this.boolean('visible'); this.integer('user_id'); }); } const store = new Store({ type: 'sqlite3', file: './db.sqlite3', autoLoad: true, autoSave: true, migrations: [ setup, ] }); class User extends store.BaseModel{ static definition(){ this.hasMany('posts'); } } store.Model(User); class Post extends store.BaseModel{ static definition(){ this.belongsTo('user'); } } store.Model(Post); store.ready(async () => { try { let user1 = await User.create({ name: 'Amir 1', posts: [{ title: 'post by amir 1', visible: true }] }); let user2 = await User.create({ name: 'Amir 2', posts: [{ title: 'post by amir 2', visible: true }] }); user1 = await User.find(user1.id); user2 = await User.find(user2.id); console.log(await user1.posts.count()); // should be 1, but it's zero console.log(await user2.posts.count()); }catch (e) { console.log("error", e); } }); ```

[amir-s]

I've been trying to debug, but i'm getting kinda lost as i don't have enough context around the codebase! :D I might be wrong here! So, it seems that it happens here

https://github.com/PhilWaldmann/openrecord/blob/0d34727db7983b0afcdc0fd2841740ba42249d21/lib/base/relations/has_many.js#L183-L188

The conditions here is false, the conditions is set before actually running the lazy operation, and at that point, the parent object has not been saved yet, so its id is null. So this check fails https://github.com/PhilWaldmann/openrecord/blob/0d34727db7983b0afcdc0fd2841740ba42249d21/lib/base/relations/utils.js#L16-L19 and the condition never gets added.

[PhilWaldmann]

That's a bingo!

yes, first it converts all records, which should be removed (e.g. if you set user1.posts = null), to a conditions object (basically {user_id: [1]} in this example). If there are no records or matching data (user1.id) it will return false, to indicate that to little information is available L169. If too little information is available AND the relation was loaded L171 - stop! (nothing to remove!) But in case the relation was not loaded, we would miss some updates. That's where the second call to utils.recordsToConditions kicks in. And the check for sufficient information was missing. (b50087a) I've added a test as well

[PhilWaldmann]

That's a feature I've added recently. I hope there are now enough tests to call it stable ;)

Thanks for intense testing!

[amir-s]

Great! I like one-liner fixes 👍 :D Tested the latest master, and indeed it works as expected!

Thanks a lot for taking the time! I'm pushing myself to figure out how everything works so I can fix these small things myself :)

I think we can close this issue for good!

[PhilWaldmann]

Perfekt! My list for v2.0.0 is almost done... So I'll probably release it today