cult-of-coders / grapher

Grapher: Meteor Collection Joins + Reactive GraphQL like queries
https://atmospherejs.com/cultofcoders/grapher
MIT License
276 stars 53 forks source link

Allow params to work for Global Exposure #206

Open jthomaschewski opened 6 years ago

jthomaschewski commented 6 years ago

In 1.3 resolver-links where removed. According to the migration guide "reducers" are recommended as a replacement. Unfortunately the reducer function has only access to the plain object and it doesn't seem possible to get access to the filters of the query or to define filters for the sub query.

E.g. this was possible with links

MyCollection.createQuery({
  some: 1,
  other: {
    $filter({filters, params}) {
      filters.searchTerm = params.searchTerm
    }
  }
})

The filters where passed through to the links resolver function (second parameter).

And chance to get the possibility of accessing filters or passing through parameters in any way down to reducer functions?

Thanks!

theodorDiaconu commented 6 years ago

@jbbr I thought about this before and it was implemented since 1.3 release.

Look at the docs: https://github.com/cult-of-coders/grapher/blob/master/docs/reducers.md#params-aware-reducers

Let me know if everything works as you expect it to.

jthomaschewski commented 6 years ago

@theodorDiaconu thanks for pointing to the docs. Unfortunately the second parameter of the reduce function is always undefined in my case. I had a look at the code and it seems as only the object (first parameter) is passed through, to the defined function:

return this.reduceFunction.call(null, object); https://github.com/cult-of-coders/grapher/blob/master/lib/query/nodes/reducerNode.js#L13

theodorDiaconu commented 6 years ago

@jbbr something is very odd.... I could have swore I tested this function and adapted it properly. You are right. I will fix this ASAP!

jthomaschewski commented 6 years ago

Thank you! It's really nice to see that you keep maintaining this package as promised :)

jthomaschewski commented 6 years ago

@theodorDiaconu Unfortunately this doesn't seem to work on the client.

It seems, as if the method prepareForProcess consumes the params on the client-side and the server call does only include the body object with the resulting filters and options but without the raw params.

I think that the exposure-meteor method has to be modified to pass-through the params.

Code to reproduce:

// reducer on server
    paramBasedReducer: {
        body: {
            _id: 1,
        },
        reduce(object, params) {
            return params.element;
        }
    }
// query on client
const query = createQuery({
            authors: {
                $options: {limit: 1},
                paramBasedReducer: 1,
            }
        });

        query.setParams({
            element: 'TEST_STRING'
        });

        const data = await query.fetchSync();
        data.forEach(author => {
            assert.equal(author.paramBasedReducer, 'TEST_STRING');
        })

=> assert author.paramBasedReducer => undefined params in reducer function is {}

jthomaschewski commented 6 years ago

Workaround (not pretty):

theodorDiaconu commented 6 years ago

Ugh, very ugly indeed. Will take care of this

theodorDiaconu commented 6 years ago

@jbbr I identified the issue, it's because you need to use a namedQuery for params to work. Params aren't passed to server if you're using Global Exposure, with a named query things will work. This is not a bug but a design flaw.

Twisterking commented 6 years ago

Is this somehow still present, even for named queries? I have a named query, living in its own file and using export default, which is also using a reducer (defined in another file).

The reducer itself works, but my reduce() function always only receives the object itself, no params, no options, no nothing. 😢 What am I doing wrong?

Items.addLinks({
  'openOrderItems': {
    collection: OpenOrdersBody,
    inversedBy: 'item'
  },
  'category': {
    type: 'one',
    collection: Categories,
    field: 'categoryId'
  },
  'specialPrices': {
    collection: Prices,
    inversedBy: 'item'
  }
});

Items.addReducers({
  'specialPrice': { 
    body: {
      specialPrices: {
        itemId: 1,
        userId: 1,
        groupId: 1,
        priceInfo: 1
      }
    },
    reduce(item) { // no more params :(
      console.log(arguments); // array of 2 items, but the second one (params) is always undefined
      return item.specialPrices[0] || null;
    }
  }
});
theodorDiaconu commented 6 years ago

@Twisterking how are you calling the query ?

Twisterking commented 6 years ago

Like so: (client)

withQuery((props) => createQuery({ myNamedQuery: { ...myParams } }), { reactive: true }) 
theodorDiaconu commented 6 years ago

@Twisterking I think something may not be good. Can you try using query.clone() from the query that you expose

Twisterking commented 6 years ago

Hmm where exactly? In my expose.js, where also my named itemsQuery query is imported and then .expose()d, I tried .clone().expose() but nothing changed. .expose().clone() throws an errors. Or maybe I am misunderstanding you?

theodorDiaconu commented 6 years ago

import query from 'xxx';

withQuery(() => query.clone());

Twisterking commented 6 years ago

Okay. I tied:

import itemsQuery from '/imports/db/items/queries/itemsQuery';

and

withQuery((props) => itemsQuery.clone(myParams))

nothing changed. params is still undefined in my reducer.

theodorDiaconu commented 6 years ago

I understand I will look into this.

Twisterking commented 6 years ago

thanks a bunch!

Twisterking commented 6 years ago

Sidenote: this bug is also present for the $postFilter() function. According to docs: $postFilter(results, params) { ... return results }, but for me params is always undefined, just like in the reducers as stated above.