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

*Feature Request* Use fields in Populate Virtuals #5704

Closed
florianbepunkt opened this issue Oct 11, 2017 · 5 comments
Closed

*Feature Request* Use fields in Populate Virtuals #5704

florianbepunkt opened this issue Oct 11, 2017 · 5 comments
Labels
needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue
Milestone

Comments

@florianbepunkt
Copy link

It would be useful for some use cases to set the references in populated virtuals based on existing fields in the doc, if this is possible. See the following example. If the field internalOrderId in the Refund document is no null, it uses sourceOrderId as localField, if it is null, it uses _id.

You can do something similar already when you populate fields using a dynamic ref, but since I can't use the _id field as reference this is not an option. As far as I know, you can't do the same when you use populate virtuals.

RefundSchema.virtual('order', {
  ref: 'Order',
  localField: (this.internalOrderId !== null) ? 'sourceOrderId' : 'internalOrderId',
  foreignField: (this.internalOrderId !== null) ? 'sourceId' : '_id',
  justOne: false
});
@vkarpov15 vkarpov15 added this to the 4.13 milestone Oct 14, 2017
@vkarpov15
Copy link
Collaborator

I like this idea, should be doable, will investigate for next minor release.

@florianbepunkt
Copy link
Author

florianbepunkt commented Dec 14, 2017

@vkarpov15 We just updated our mongoose version and I tried to get this to work. Could you please add the syntax to the documentation or provide a basic example?

I have this but it throws an error TypeError: Invalid path. Must be either string or array

Baiscally what I try to accomplish here is to set the virtual dynamically based on a field of the source document (RefundSchema)

RefundSchema.virtual('order', {
  ref: 'Order',
  localField: function() {
    if (this.internalOrderId) {
      return this.internalOrderId;
    } else {
      return this.sourceOrderId;
    }
  },
  foreignField: function() {
    if (this.internalOrderId) {
      return this._id;
    } else {
      return this.sourceId;
    }
  },
  justOne: function() {
    if (this.internalOrderId) {
      return true;
    } else {
      return false;
    }
  },
});

EDIT: Is it by any chance that the foreignField does not support documents _id field?

@florianbepunkt
Copy link
Author

florianbepunkt commented Dec 14, 2017

Okay, I figured out the syntax but there seems to be a bug.... I have the following virtual with dynamic localField and foreignField

RefundSchema.virtual('order', {
  ref: 'Order',
  localField: function() {
    if (this.internalOrderId) {
      // CASE A
      return 'internalOrderId';
    } else {
      // CASE B
      return 'sourceOrderId';
    }
  },
  foreignField: function() {
    if (this.internalOrderId) {
      // CASE A
      return '_id';
    } else {
      // CASE B
      return 'sourceId';
    }
  },
  justOne: false,
});

Now I have two queries that produce inconsistent results:

// QUERY A - returns doc with _id 5a317e080732e735527911fe whic is CASE A for sure
Refund.find({_id: '5a317e080732e735527911fe'}).populate('order').exec().then((resp) => {
  console.log(JSON.stringify(resp, null, 3));
})

// QUERY B - should return CASE A docs and CASE b docs
Refund.find({}).populate('order').exec().then((resp) => {
  console.log(JSON.stringify(resp, null, 3));
})

Actual Behaviour
Query A does populate the order field. However Query B returns (amongst others) the document from Query A (5a317e080732e735527911fe), but does not populate the order field

Expected Behaviour
Both queries should populate the order field for document 5a317e080732e735527911fe

@vkarpov15 vkarpov15 reopened this Dec 19, 2017
@vkarpov15 vkarpov15 modified the milestones: 4.13, 4.13.8 Dec 19, 2017
@vkarpov15 vkarpov15 added needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue and removed fixed? labels Dec 19, 2017
@vkarpov15
Copy link
Collaborator

Thanks for reporting @florianbepunkt , will investigate

@vkarpov15 vkarpov15 modified the milestones: 4.13.8, 4.13.9 Dec 27, 2017
@vkarpov15 vkarpov15 modified the milestones: 4.13.9, 4.13.10 Jan 7, 2018
@vkarpov15 vkarpov15 modified the milestones: 4.13.10, 4.13.11 Jan 28, 2018
@vkarpov15 vkarpov15 modified the milestones: 4.13.11, 4.13.12 Feb 8, 2018
@vkarpov15 vkarpov15 modified the milestones: 4.13.12, 4.13.13 Mar 14, 2018
@vkarpov15 vkarpov15 modified the milestones: 4.13.13, 5.1.2 May 17, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.1.2, 5.1.3 May 19, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.1.3, 5.1.4, 5.1.5 May 28, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.1.5, 5.1.6 Jun 11, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.1.6, 5.1.7 Jun 19, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.1.7, 5.1.8, 5.2.1 Jun 26, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.2.2, 5.2.4 Jul 8, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.2.4, 5.2.6 Jul 16, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.2.6, 5.2.7, 5.2.9 Jul 30, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.2.9, 5.2.11 Aug 17, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.2.12, 5.2.13 Aug 29, 2018
@vkarpov15 vkarpov15 modified the milestones: 5.2.16, 5.2.18 Sep 19, 2018
vkarpov15 added a commit that referenced this issue Sep 27, 2018
vkarpov15 added a commit that referenced this issue Sep 27, 2018
@vkarpov15
Copy link
Collaborator

Thanks for your patience @florianbepunkt , we really kicked this issue very far down the road. The issue should be fixed in 5.2.18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue
Projects
None yet
Development

No branches or pull requests

3 participants