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

$unwind stage is not built for hasMany relation fields #10

Open
DavidKrpt opened this issue Mar 4, 2020 · 1 comment
Open

$unwind stage is not built for hasMany relation fields #10

DavidKrpt opened this issue Mar 4, 2020 · 1 comment

Comments

@DavidKrpt
Copy link
Contributor

there's a test implying that

it('Should aggregate where hasMany relation properties with $unwind', callback)

but after logging the resulting pipeline, I saw that it had no $unwind stage.

The only place in the code where the $unwind stage is built is in the buildLookup function in an if block:

  function buildLookup (aggregate, where, parentRelation = null) {
    _.each(where, (value, key) => {
      const keys = key.split('.');
      const headKey = keys.shift();
      const relation = (parentRelation ? parentRelation.modelTo : Model).relations[headKey];
      if (!relation) return;
      const lookupOpts = buildLookupOptsFromRelation(relation, parentRelation);
      aggregate.lookup(lookupOpts);
      if (_.includes(['hasOne', 'belongsTo'], relation.type)) {
        let unwindPath = relation.name;
        const parentRelationName = _.get(parentRelation, 'name', '');
        if (parentRelationName.length) {
          unwindPath = `${parentRelationName}.${unwindPath}`;
        }
        aggregate.unwind({
          path: `$${ unwindPath}`,
          preserveNullAndEmptyArrays: true,
        });
      }
      /* istanbul ignore else */
      if (keys.length) {
        buildLookup(aggregate, {[keys.join('.')]: value}, relation);
      }
    });
  }

the $unwind stage is built only when the following condition is true.

if (_.includes(['hasOne', 'belongsTo'], relation.type))

this seemed a bit odd. I tried 2 things:

  1. inverting the condition so that $unwind is built only for hasMany relations.
  2. removing the if block altogether.

In both cases all the tests passed, and the results of my requests stayed the same: hasMany relational field filters work only if the relation is 1 level deep and the field by which I'm filtering is not a foreign key.
I'm still investigating and I would appreciate any suggestion !

By the way, what does this comment mean /* istanbul ignore else */ ? :)

Thanks in advance and sorry for jumping in with multiple issues.

@Akeri
Copy link
Member

Akeri commented Mar 5, 2020

Hi David, thank you for using this module and taking your time reporting issues.
I would like to be able to review it calmly and fix them. Meanwhile feel free to send PR if you find a solution. Thank you!

By the way, /* istanbul ignore else */ means that the else block for that "if" should be ignored for the coverage percetange. Actually there is no "else" but Istambull was detecting it as a coverage lack.

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

2 participants