Vincit / objection.js

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

Define named filters on model #375

Closed drew-r closed 7 years ago

drew-r commented 7 years ago

Currently you can supply named filters when doing eager stuff, e.g

teams
      .query()
      .eager('users(active)', {
        active: (builder) => builder.where('active', true)
      })      

It would be cool if you could define these named filters at the model level on the related model itself. That is to say,

class UserModel {
    static tableName = 'users';
    static namedFilters = {
        active: (builder) => builder.where('active', true)
    };
}

Thinking here is that the filter implementation is a concern of the model being filtered, not some related consumer of the model. Probably the named filters passed immediately to the query should override any defined on the model.

koskimas commented 7 years ago

You can do this:

teams
      .query()
      .eager('players(active)', UserModel.namedFilters)   

or this:

class BaseQueryBuilder extends Model.QueryBuilder {
  eager(expr, filters) {
    return super.eager(expr, Object.assign({}, this.modelClass().namedFilters, filters);
  }
}

class BaseModel extends Model {
  static get QueryBuilder() {
    return BaseQueryBuilder;
  }
}

// UserModel.js

class UserModel extends BaseModel {
    static tableName = 'users';
    static namedFilters = {
        active: (builder) => builder.where('active', true)
    };
}

I think this is pretty easy to implement on the application side and maybe shouldn't be added to objection.

drew-r commented 7 years ago

Thanks @koskimas I came up with the same kind of implementation as your first example and have also used functions to create parameterised filters. When working with multiple relations I was looking at merging named filters from multiple models but you run into trouble where filters from different models could overwrite one another - 'active' could mean different filters to different models.

I resorted to dedupe them by mapping:

.eager('[users(activeUser) widgets(activeWidget)]', { activeUser: UserModel.namedFilters.active, activeWidget: WidgetModel.namedFilters.active })

or just giving them unique names in the first place.

Being able to use dot notation to refer to named queries may ease this... maybe this is already possible?

An objection implementation would avoid this problem as it would resolve named filter via the model the filter applies to i.e [users(active) widgets(active)] would use UserModel.namedFilters.active and WidgetModel.namedFilters.active respectively. But you then lose the ability to parameterise anything unless that is somehow built into the DSL which seems way over the top. The more I think about this the more it says 'too complicated' ... but you surprised me before with the join algo so maybe you have some good idea ;)

Second example is interesting also but I think this.modelClass() would be a TeamModel when querying teams and so wouldn't find your UserModel.namedFilters?

koskimas commented 7 years ago

My second example would probably work (to some extent) how you want if you are using the default WHERE IN based eager algorithm since it calls eager recursively.

drew-r commented 7 years ago

Forgive me I don't follow... can you expand?

koskimas commented 7 years ago

It was poorly written and incorrect. Never mind 😄

koskimas commented 7 years ago

After giving this some thought, I think I'll implement this. There are some fun things you can do with this. For example graphql-like selects:

class Person extends Model {
  static get namedFilters() {
    return _.mapValues(this.jsonSchema.properties, (value, key) => {
      return (builder) => builder.select(key);
    });
  }
}

Movie
  .query()
  .eager('actors(firstName, lastName, age)')