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

fix(model): allow virtual ref function to return arrays #5834

Merged
merged 3 commits into from
Nov 21, 2017

Conversation

brunohcastro
Copy link

Allow the virtual ref function to return an array of model names.

When referencing fields inside an array of subdocuments, the ref function was expecting a single string as the model name. This was very limiting when using dynamic references on an array of subdocuments.

To solve this I just did a simple type check. If the return value of the ref function is already an array, there's no need to change it. If the result is anything else the code resume with the previous behaviour. This enabled me to use something as:

const appointmentServiceSchema = new Schema({
  serviceId:    Schema.Types.ObjectId,
  serviceType:  String,
  providerId:   Schema.Types.ObjectId,
  providerType: String,
  notes:        String,
  cost:         Number
}, { toJSON: { virtuals: true } });

const appointmentSchema = new Schema({
  services:         [appointmentServiceSchema]
}, { toJSON: { virtuals: true } });

appointmentServiceSchema.virtual('service', {
  ref() {
    return this.services.map((item) => item.serviceType);
  },
  localField:   'serviceId',
  foreignField: '_id',
  justOne:      true
});

appointmentServiceSchema.virtual('provider', {
  ref() {
    return this.services.map((item) => item.providerType);
  },
  localField:   'providerId',
  foreignField: '_id',
  justOne:      true
});

Sorry but I didn't have time to write the tests yet. Just did the quick fix and used right away.

@vkarpov15 vkarpov15 added this to the 4.13.5 milestone Nov 21, 2017
@vkarpov15
Copy link
Collaborator

Nice, thanks 👍 🍻

@vkarpov15 vkarpov15 merged commit ad69054 into Automattic:4.13 Nov 21, 2017
@vkarpov15
Copy link
Collaborator

Oh woops why did this go into the 4.13 branch?

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

Successfully merging this pull request may close these issues.

2 participants