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

issue #45 - implement setNext changes to allow its usage within pre-s… #46

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

rernens
Copy link

@rernens rernens commented Feb 15, 2019

Allow use a setNext within the pre-save hook in making save optional through new args.

Preserve current behaviour in making save the default behaviour if no arg set

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.6%) to 98.425% when pulling c8474d5 on rernens:setNext-within-pre-save-hook into 94b4daf on ramiel:develop.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.6%) to 98.425% when pulling c8474d5 on rernens:setNext-within-pre-save-hook into 94b4daf on ramiel:develop.

@ramiel
Copy link
Owner

ramiel commented Mar 1, 2019

Hello @rernens , sorry for coming back so late to you but I had no time at all to look at this. I ned to fin the time to look at it carefully. Maybe you could add some test case? Also, if you need it in your projects you can include it in your dependencies like this in your package.json

"mongoose-sequence": "rernens/mongoose-sequence#setNext-within-pre-save-hook"

I'm sure you already know.
Sorry, this will take some other time for me. Busy days! Thank you for it in any case!!

@ramiel
Copy link
Owner

ramiel commented Mar 1, 2019

So, your intent is to make this kind of calls?

this.setNext('employees_counter', false, (err) => {
  ...
});

and so the save won't be called? I understand. I also remember that a previous version of this plugin did not save and you had to call save explicitely. I think this PR is a good idea. It needs some test and an update to the documentation. I hope you can do it or I must take over whenever I'll have time.

@rernens
Copy link
Author

rernens commented Mar 4, 2019

@ramiel

Hi Fabrizio

You are right, this PR allows to keep the current behavior as well as integrating the required behavior to use setNext within a pre-save hook.

I am using it in a web app that went live yesterday.

While as busy as you are, I will try to find some time over the next 72 hours, to change the doc and write some test.

@cristizuum
Copy link

Hi guys, any update on this PR? We need this please...

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