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

feat/Add support for Model.insertMany #24

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

RAYDENFilipp
Copy link

@RAYDENFilipp RAYDENFilipp commented Nov 1, 2020

The plugin didn't support pre-hook for insertMany event, so it has been added.

Colses the issue #23 .

Changes include:

  • Add new pre-hook insertMany
    • add processDocumentsOnInsertMany function and its tests for better coverage
    • processDocumentsOnInsertMany does the vary same as a callback, passed to post-save hook with the only difference that it receives document-objects array and getting/setting fields is performed on each document and next is called only if the last document has been processed.
  • Change mocha/should/nyc pack with jest to allow for a set of similar test suites, like describe.each, it.each.
  • Since insertMany should be tested with an array of documents, but tests would be the same as with testing new Model, the following has been made:
    • Two suites of test cases were created: 'create document with "new Model"' and 'create documents with "Model.insertMany"'
    • For each suite its own function-creator has been made, createDocumentWithNewModel & createDocumentsWithInsertMany, each receiving an array of docs with a difference that createDocumentWithNewModel also receiving an array, but only one document from there will be processed. Array of documents is used in both suites to allow describe.each to properly work.
    • Efforts have been made to adhere to the code style (use of vars, function declarations instead of expressions/arrow functions);

P.S. another good idea to unite both hooks would have been to use validate since it triggered in both cases.

If this PR still seems too hard to follow along, it is possible to discuss over Skype, Whatsapp, or anything similar.

@RAYDENFilipp RAYDENFilipp changed the title feat/Add support for Model.insertMany W.I.P.: feat/Add support for Model.insertMany Nov 1, 2020
@RAYDENFilipp RAYDENFilipp changed the title W.I.P.: feat/Add support for Model.insertMany feat/Add support for Model.insertMany Nov 9, 2020
@proswdev proswdev closed this Aug 16, 2022
@proswdev proswdev reopened this Aug 16, 2022
@proswdev
Copy link
Owner

Hi RAYDENFilipp. Sorry for the late response would be a bit of an understatement ;-)
I would like to include your PR but not ready to move the testing framework over to jest for various internal reasons.
If you're still willing and able to include the PR, could you please switch testing back to mocha?
Thx, Eric

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.

None yet

2 participants