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

Query middleware support #10

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Query middleware support #10

wants to merge 9 commits into from

Conversation

ramiel
Copy link
Owner

@ramiel ramiel commented Aug 26, 2016

Update counters on findOneAndUpdate and update
Based on the work by @linusbrolin.

  • Add tests
  • Rebase on latest develop
  • Add tests for multi option (on update)
  • Add same functionality for referenced fileds
  • Try to avoid a query to know if the upsert inserted a document

@linusbrolin
Copy link

linusbrolin commented Aug 26, 2016

I found another bug, when using scoped counter (I never tested that before, my bad), where the reference value(s) would be null.
I've fixed it in my fork (same branch as before), so you should probably update this PR to the latest code from my branch.

@ramiel
Copy link
Owner Author

ramiel commented Aug 26, 2016

I'll do, and I will add the related test

@ramiel ramiel force-pushed the query-middleware-support branch from 0753518 to 12939b8 Compare August 26, 2016 15:13
@coveralls
Copy link

coveralls commented Aug 26, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 12939b8 on query-middleware-support into 727be42 on develop.

@coveralls
Copy link

coveralls commented Aug 26, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling b50319d on query-middleware-support into 16e3d01 on develop.

linusbrolin and others added 7 commits August 26, 2016 19:11
This will make the sequence plugin work with upsert operations.
Unfortunately it seems there is no way to know in the query if an upsert
will result in an insert or update, according to this mongoose issue:
Automattic/mongoose#2118

So the only way to know is to make another query to find out.
…al update query

This should prevent any possible error due to undefined _id in the
_update object
@ramiel ramiel force-pushed the query-middleware-support branch from b50319d to 96d6639 Compare August 26, 2016 17:11
@coveralls
Copy link

coveralls commented Aug 26, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 96d6639 on query-middleware-support into 65ec0a1 on develop.

ramiel added 2 commits August 26, 2016 19:42
Adds tests which demonstrate that reference fileds are not upated
correctly
Looks like those hooks are complex to manage
@coveralls
Copy link

coveralls commented Aug 26, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 98b11b8 on query-middleware-support into 65ec0a1 on develop.

@blocks-master
Copy link

any update ? It's still not working for findOneAndUpdate

@linusbrolin
Copy link

linusbrolin commented Jun 15, 2021

@ramiel I have now rebased my branch on the latest code from your master branch, and updated the code accordingly.
https://github.com/linusbrolin/mongoose-sequence/tree/query-middleware-support
It should work with the parallel_hooks option, but I have not added any tests for it due to limited amount of time.
I am however using it in a work project and it is working there.

If you could add the needed tests then I would very much appreciate it, otherwise I'll see if I can find time to do that myself.

For now I have added a check for the multi option on update, and exit the hook if true.
And I have not added support for updateMany either, yet, due to limited time.

But I honestly think we should at the very least get support for upserting single documents released now, as it's been 5 years since my original PR!
Or if someone wants to help adding support for updateMany and update with multi: true?

Let me know what you think.

@linusbrolin
Copy link

@ramiel I have now yet again updated my branch to the latest code from your master branch, and updated my code to work in mongoose 7+ (replaced callbacks with promise).
https://github.com/linusbrolin/mongoose-sequence/tree/query-middleware-support

I also tried to sync my branch with your query-middleware-branch, but since that branch is so old and out of date, it messed up all my commit history, so I had no choice but to revert that attempt. :(

This means that the tests that are currently only in your query-middleware-support branch, are missing in my branch.
And since your query-middleware-support branch is so different from your master branch, including the entire tests file, it is no simple task for me to try and manually sync those tests into my branch.

I really cannot spend more time on this, it's literally been over 8 years(!!) and still not merged.

I really hope you can move over the tests to my current branch, or sync in my current branch into yours, and finally get this thing merged and released.

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.

4 participants