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

Add groupingKey option, to be able to store multiple trees in one collection #8

Merged
merged 3 commits into from Nov 23, 2014

Conversation

rubenstolk
Copy link
Contributor

Currently, it's not possible to have multiple trees in one collection. This PR makes that easily possible by specifying a groupingKey as option.

In case of for example multi tenancy and you specify a property tenant on your model that is used for nested sets, simply set groupingKey to tenant and the update queries now make sure that only the matching elements get updated.

@luccastera
Copy link
Collaborator

@rubenstolk Thanks for the PR. It looks like a good feature to have.

Can you please add some tests and document how to use this feature in the README file?

@rubenstolk
Copy link
Contributor Author

@dambalah Done.

@rubenstolk
Copy link
Contributor Author

@dambalah Also bumped the version to 0.0.6, would be nice if you can publish to NPM

@luccastera
Copy link
Collaborator

@rubenstolk The tests you've added look great but it looks like we are no longer testing the plugin in case somebody does not use a groupingKey. Could you please make sure we are still covering that path?

Thank you!

@rubenstolk
Copy link
Contributor Author

Does this help now?

@luccastera
Copy link
Collaborator

@rubenstolk Great. Thanks!

luccastera added a commit that referenced this pull request Nov 23, 2014
Add groupingKey option, to be able to store multiple trees in one collection
@luccastera luccastera merged commit 7bebe92 into groupdock:master Nov 23, 2014
@luccastera
Copy link
Collaborator

@rubenstolk
Copy link
Contributor Author

Great, thanks!

Ruben Stolk

CHANGER, Raising the bar in Online Experience!

+91 95 6105 0034
+31 6 46 11 80 37

INDIA OFFICE
308 Sky Max, Datta Mandir Chowk, Viman Nagar, Pune – 411014, India (
http://goo.gl/maps/muZw7 http://g.co/maps/g9y7n)
+91 20 65 7373 33

NETHERLANDS OFFICE
Maerten Trompstraat 25, 2628 RC Delft, Netherlands (http://goo.gl/QQHfWE
http://g.co/maps/tcpwn)
+31 88 1001300

http://changer.nl

On Sun, Nov 23, 2014 at 11:18 PM, Luc Castera notifications@github.com
wrote:

@rubenstolk https://github.com/rubenstolk published on npm
https://www.npmjs.org/package/mongoose-nested-set


Reply to this email directly or view it on GitHub
#8 (comment)
.

@rubenstolk
Copy link
Contributor Author

One more remark, the node version is set to < 0.7.x in package.json, AFAIK
the package works absolutely fine in 0.10 and probably higher.

Ruben Stolk

CHANGER, Raising the bar in Online Experience!

+91 95 6105 0034
+31 6 46 11 80 37

INDIA OFFICE
308 Sky Max, Datta Mandir Chowk, Viman Nagar, Pune – 411014, India (
http://goo.gl/maps/muZw7 http://g.co/maps/g9y7n)
+91 20 65 7373 33

NETHERLANDS OFFICE
Maerten Trompstraat 25, 2628 RC Delft, Netherlands (http://goo.gl/QQHfWE
http://g.co/maps/tcpwn)
+31 88 1001300

http://changer.nl

On Sun, Nov 23, 2014 at 11:22 PM, Ruben Stolk | Changer <
ruben.stolk@changer.nl> wrote:

Great, thanks!

Ruben Stolk

CHANGER, Raising the bar in Online Experience!

+91 95 6105 0034
+31 6 46 11 80 37

INDIA OFFICE
308 Sky Max, Datta Mandir Chowk, Viman Nagar, Pune – 411014, India (
http://goo.gl/maps/muZw7 http://g.co/maps/g9y7n)
+91 20 65 7373 33

NETHERLANDS OFFICE
Maerten Trompstraat 25, 2628 RC Delft, Netherlands (http://goo.gl/QQHfWE
http://g.co/maps/tcpwn)
+31 88 1001300

http://changer.nl

On Sun, Nov 23, 2014 at 11:18 PM, Luc Castera notifications@github.com
wrote:

@rubenstolk https://github.com/rubenstolk published on npm
https://www.npmjs.org/package/mongoose-nested-set


Reply to this email directly or view it on GitHub
#8 (comment)
.

@rubenstolk
Copy link
Contributor Author

And, we got it to work also with mongoose 3.x, any reason why you've set it
to 2.5?

Ruben Stolk

CHANGER, Raising the bar in Online Experience!

+91 95 6105 0034
+31 6 46 11 80 37

INDIA OFFICE
308 Sky Max, Datta Mandir Chowk, Viman Nagar, Pune – 411014, India (
http://goo.gl/maps/muZw7 http://g.co/maps/g9y7n)
+91 20 65 7373 33

NETHERLANDS OFFICE
Maerten Trompstraat 25, 2628 RC Delft, Netherlands (http://goo.gl/QQHfWE
http://g.co/maps/tcpwn)
+31 88 1001300

http://changer.nl

On Sun, Nov 23, 2014 at 11:27 PM, Ruben Stolk | Changer <
ruben.stolk@changer.nl> wrote:

One more remark, the node version is set to < 0.7.x in package.json, AFAIK
the package works absolutely fine in 0.10 and probably higher.

Ruben Stolk

CHANGER, Raising the bar in Online Experience!

+91 95 6105 0034
+31 6 46 11 80 37

INDIA OFFICE
308 Sky Max, Datta Mandir Chowk, Viman Nagar, Pune – 411014, India (
http://goo.gl/maps/muZw7 http://g.co/maps/g9y7n)
+91 20 65 7373 33

NETHERLANDS OFFICE
Maerten Trompstraat 25, 2628 RC Delft, Netherlands (http://goo.gl/QQHfWE
http://g.co/maps/tcpwn)
+31 88 1001300

http://changer.nl

On Sun, Nov 23, 2014 at 11:22 PM, Ruben Stolk | Changer <
ruben.stolk@changer.nl> wrote:

Great, thanks!

Ruben Stolk

CHANGER, Raising the bar in Online Experience!

+91 95 6105 0034
+31 6 46 11 80 37

INDIA OFFICE
308 Sky Max, Datta Mandir Chowk, Viman Nagar, Pune – 411014, India (
http://goo.gl/maps/muZw7 http://g.co/maps/g9y7n)
+91 20 65 7373 33

NETHERLANDS OFFICE
Maerten Trompstraat 25, 2628 RC Delft, Netherlands (http://goo.gl/QQHfWE
http://g.co/maps/tcpwn)
+31 88 1001300

http://changer.nl

On Sun, Nov 23, 2014 at 11:18 PM, Luc Castera notifications@github.com
wrote:

@rubenstolk https://github.com/rubenstolk published on npm
https://www.npmjs.org/package/mongoose-nested-set


Reply to this email directly or view it on GitHub
#8 (comment)
.

@luccastera
Copy link
Collaborator

Thanks: updated package.json for the node version eeaa990

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