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

Island rhythms/gh 6963 Can overwrite foreignField #12657

Merged
merged 10 commits into from Nov 27, 2022
Merged

Conversation

IslandRhythms
Copy link
Collaborator

No description provided.

@IslandRhythms IslandRhythms requested review from vkarpov15 and hasezoey and removed request for vkarpov15 November 4, 2022 20:53
Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some things still need to be done:

  • fix the tests (they seem to be consistently failing), the added checks dont check if the previous field is actually defined (see options)
  • from what i can tell the options localField and foreignField are new to populate-options and have not been added to the jsdoc or types yet
  • a npm run lint --fix would be good

lib/model.js Outdated Show resolved Hide resolved
lib/model.js Outdated Show resolved Hide resolved
@IslandRhythms IslandRhythms self-assigned this Nov 5, 2022
Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, though from what i can tell, the "new" options are not in the jsdoc or types

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this works, but has a few issues that we need to patch up. I'll take a look.

lib/model.js Outdated Show resolved Hide resolved
@vkarpov15 vkarpov15 merged commit 6bf13f8 into 6.8 Nov 27, 2022
@vkarpov15 vkarpov15 deleted the IslandRhythms/gh-6963 branch November 27, 2022 16:59
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.

Allow overwriting localField and foreignField when populating virtuals
3 participants