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

Make addFilterArg asynchronous #31

Closed
jean343 opened this issue Nov 7, 2016 · 11 comments
Closed

Make addFilterArg asynchronous #31

jean343 opened this issue Nov 7, 2016 · 11 comments

Comments

@jean343
Copy link

jean343 commented Nov 7, 2016

I am using relay and graphql-compose, and I want to make a query for Item which are child of categories.
My schema looks like:

var CategorySchema = mongoose.Schema({
  site: Schema.Types.ObjectId,
  node: String,
});
var ItemSchema = mongoose.Schema({
  site: Schema.Types.ObjectId,
  ASIN: {type: String, unique: true},
  category: {type: Schema.Types.ObjectId, index: true},
});

And my filter looks like:

const findManyResolver = ItemTC.getResolver('connection')
  .addFilterArg({
    name: 'Category',
    type: 'String',
    query: (query, value, resolveParams) => {
      var site = await Site.get();
      query.category = await Category.findOne({site, node: value}).exec();
    },
  });
ItemTC.setResolver('connection', findManyResolver);

My problem is that I cannot make asynchronous database queries in addFilterArg.

From the client, I want to query and sort Items which are under a certain category where I know the node string, but not the _id.

@jean343
Copy link
Author

jean343 commented Nov 7, 2016

I hacked in an await in
https://github.com/nodkz/graphql-compose/blob/0b8deaec0fa0f788731ebca4ec43f97b49047373/src/resolver.js#L362

And everything seems to work properly.

@nodkz
Copy link
Member

nodkz commented Nov 7, 2016

I do not think about async in query. And can not understood how you solved it. Can you open PR with your fix?

And what about such query definition (add async to arrow function):

    query: async (query, value, resolveParams) => {
      var site = await Site.get();
      query.category = await Category.findOne({site, node: value}).exec();
    },

Does it work or not?

@jean343
Copy link
Author

jean343 commented Nov 7, 2016

Thanks for the reply, I was missing async in my code snippet, but I had it in my real code. No, it does not fix the issue.
In my case, the category is set in my async function, but the query sent to the database does not have category defined... yet.

In the file resolver.js line 362, we can see opts.query be called (this is my async function) and it does not wait for the promise to be resolved. So, resolveParams.rawQuery is changed in my function, but the query is executed without waiting for my database call.

I fixed it 'incorrectly' by changing addFilterArg in resolver.js

if (isFunction(opts.query)) {
  resolver.setResolve((resolveParams: ResolveParams) => {
    const value = objectPath.get(resolveParams, ['args', 'filter', opts.name]);
    if (value) {
      if (!resolveParams.rawQuery) {
        resolveParams.rawQuery = {}; // eslint-disable-line
      }
      opts.query(resolveParams.rawQuery, value, resolveParams).then(function(){
        return resolveNext(resolveParams);
      })
    } else {
        return resolveNext(resolveParams);
    }
  });

Or if es7 is supported, the following could probably work

if (isFunction(opts.query)) {
  resolver.setResolve(async(resolveParams: ResolveParams) => {
    const value = objectPath.get(resolveParams, ['args', 'filter', opts.name]);
    if (value) {
      if (!resolveParams.rawQuery) {
        resolveParams.rawQuery = {}; // eslint-disable-line
      }
      await opts.query(resolveParams.rawQuery, value, resolveParams);
    }
    return resolveNext(resolveParams);
  });

Thanks!

@zapkub
Copy link
Contributor

zapkub commented Mar 17, 2017

Has anyone create this issue PR yet ?

@jean343
Copy link
Author

jean343 commented Mar 17, 2017

I did not create a PR.

@zapkub
Copy link
Contributor

zapkub commented Mar 25, 2017

It will be cool if you would ! 😎😀 I 'm stuck in this issue too

Anyway this is how I solve it for now.

// Instead of modify query in filterArg
// Wrap resolver with async function and modify your filter 
// before going to resolve() 

 TC.setResolver('findOne', TC.getResolver('findOne')
    .wrapResolve(next => async (rp) => {
      const result = await rp.context.Plant.findOne({ $or: [{ key: rp.args.key }] });
      if (result) {
        rp.args.filter = Object.assign({ plantId: result._id.toString() }, rp.args.filter) // eslint-disable-line
      }
      return next(rp);
    }))

@toverux
Copy link
Contributor

toverux commented Jul 26, 2018

Same here!
I wanted to check permissions in a filter's query function (permissions cheking is async in my app).
I'm also needing it for another more complex use-case.

I'm workarounding it like indicated above.

@nodkz nodkz closed this as completed in fe60a99 Jul 26, 2018
@nodkz
Copy link
Member

nodkz commented Jul 26, 2018

Done! Will be available on npm in minutes.

someResolver.addFilterArg({
    name: 'isActive',
    type: 'Boolean!',
    description: 'Active status filter',
    query: async (query, value, resolveParams) => {
        const checkPermissions = await Promise.resolve('accessGranted');
        if (checkPermissions) {
          query.isActive = value;
        }
    },
});

@nodkz
Copy link
Member

nodkz commented Jul 26, 2018

🎉 This issue has been resolved in version 4.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nodkz
Copy link
Member

nodkz commented Jul 26, 2018

@toverux thank you for reminding me of this old problem. I completely forgot about it.
Fix is just changes in 7 lines of code with tests. 🤗

@toverux
Copy link
Contributor

toverux commented Jul 26, 2018

Oh @nodkz, did I already said that you're the best maintainer on earth?
Thanks a lot !

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

No branches or pull requests

4 participants